Change how Web Inspector extension tabs are loaded.

https://bugs.webkit.org/show_bug.cgi?id=243073 rdar://86542412 Reviewed by Devin Rousso. Patch by Kiara Rose <[email protected]>. Add a dedicated loadURL method to load a URL in a tab instead of relying on evaluateScript. This avoids so extra indirection and makes is a little easier to debug loading issues. This change also sets the iframe.src up-front and avoids the empty load bounce. * Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js: (WI.WebInspectorExtensionController.prototype.navigateTabForExtension): Added. * Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js: (InspectorFrontendAPI.navigateTabForExtension): Added. * Source/WebKit/UIProcess/API/APIInspectorExtension.cpp: (API::InspectorExtension::navigateTab): Added. * Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm: (-[_WKInspector navigateExtensionTabWithIdentifier:toURL:completionHandler:]): Added. * Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.h: * Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.mm: (-[_WKInspectorExtension navigateToURL:inTabWithIdentifier:completionHandler:]): Added. * Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtensionHost.h: * Source/WebKit/UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm: (-[_WKRemoteWebInspectorViewController navigateExtensionTabWithIdentifier:toURL:completionHandler:]): Added. * Source/WebKit/UIProcess/Inspector/WebInspectorUIExtensionControllerProxy.cpp: (WebKit::WebInspectorUIExtensionControllerProxy::navigateTabForExtension): Added. * Source/WebKit/UIProcess/Inspector/WebInspectorUIExtensionControllerProxy.h: * Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.cpp: (WebKit::WebInspectorUIExtensionController::navigateTabForExtension): Added. * Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.h: * Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.messages.in: Canonical link: https://commits.webkit.org/252793@main
WebPlatformForEmbedded · Jul 25, 2022 · c55c39c · c55c39c
1 parent 5b2c235
commit c55c39c
Show file tree

Hide file tree

Showing 15 changed files with 152 additions and 9 deletions.
diff --git a/Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js b/Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js
@@ -197,6 +197,17 @@ WI.WebInspectorExtensionController = class WebInspectorExtensionController exten
         return target.PageAgent.reload.invoke({ignoreCache});
     }
 
+    navigateTabForExtension(extensionTabID, sourceURL)
+    {
+        let tabContentView = this._extensionTabContentViewForExtensionTabIDMap.get(extensionTabID);
+        if (!tabContentView) {
+            WI.reportInternalError("Unable to navigate extension tab with unknown extensionTabID: " + extensionTabID);
+            return WI.WebInspectorExtension.ErrorCode.InvalidRequest;
+        }
+
+        tabContentView.iframeURL = sourceURL;
+    }
+
     showExtensionTab(extensionTabID, options = {})
     {
         let tabContentView = this._extensionTabContentViewForExtensionTabIDMap.get(extensionTabID);

diff --git a/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js b/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js
@@ -245,4 +245,10 @@ InspectorFrontendAPI = {
     {
         return WI.sharedApp.extensionController.evaluateScriptInExtensionTab(extensionTabID, scriptSource);
     },
+
+    // Returns a string (WI.WebInspectorExtension.ErrorCode) if an error occurred.
+    navigateTabForExtension(extensionTabID, sourceURL)
+    {
+        return WI.sharedApp.extensionController.navigateTabForExtension(extensionTabID, sourceURL);
+    },
 };
diff --git a/Source/WebInspectorUI/UserInterface/Views/WebInspectorExtensionTabContentView.js b/Source/WebInspectorUI/UserInterface/Views/WebInspectorExtensionTabContentView.js
@@ -40,10 +40,10 @@ WI.WebInspectorExtensionTabContentView = class WebInspectorExtensionTabContentVi
         this._tabInfo = tabInfo;
         this._sourceURL = sourceURL;
 
-        this._iframeFinishedInitialLoad = false;
         this._whenPageAvailablePromise = new WI.WrappedPromise;
 
         this._iframeElement = this.element.appendChild(document.createElement("iframe"));
+        this._iframeElement.src = sourceURL;
         this._iframeElement.addEventListener("load", this._extensionFrameDidLoad.bind(this));
     }
 
@@ -78,6 +78,11 @@ WI.WebInspectorExtensionTabContentView = class WebInspectorExtensionTabContentVi
         return `ExtensionTab-${this._extension.extensionBundleIdentifier}-${this._tabInfo.displayName}`;
     }
 
+    set iframeURL(sourceURL)
+    {
+        this._iframeElement.src = sourceURL;
+    }
+
     whenPageAvailable()
     {
         return this._whenPageAvailablePromise.promise;
@@ -119,13 +124,6 @@ WI.WebInspectorExtensionTabContentView = class WebInspectorExtensionTabContentVi
 
     _extensionFrameDidLoad()
     {
-        // Bounce from the initial empty page to the requested sourceURL.
-        if (!this._iframeFinishedInitialLoad) {
-            this._iframeFinishedInitialLoad = true;
-            WI.sharedApp.extensionController.evaluateScriptInExtensionTab(this._extensionTabID, `document.location.replace("${this._sourceURL}");`);
-            return;
-        }
-
         // Signal that the page is available since we already bounced to the requested page.
         if (!this._whenPageAvailablePromise.settled)
             this._whenPageAvailablePromise.resolve(this._sourceURL);

diff --git a/Source/WebKit/UIProcess/API/APIInspectorExtension.cpp b/Source/WebKit/UIProcess/API/APIInspectorExtension.cpp
@@ -71,6 +71,16 @@ void InspectorExtension::evaluateScript(const WTF::String& scriptSource, const s
     m_extensionControllerProxy->evaluateScriptForExtension(m_identifier, scriptSource, frameURL, contextSecurityOrigin, useContentScriptContext, WTFMove(completionHandler));
 }
 
+void InspectorExtension::navigateTab(const Inspector::ExtensionTabID& extensionTabID, const WTF::URL& sourceURL, WTF::CompletionHandler<void(const std::optional<Inspector::ExtensionError>)>&& completionHandler)
+{
+    if (!m_extensionControllerProxy) {
+        completionHandler(Inspector::ExtensionError::ContextDestroyed);
+        return;
+    }
+
+    m_extensionControllerProxy->navigateTabForExtension(extensionTabID, sourceURL, WTFMove(completionHandler));
+}
+
 void InspectorExtension::reloadIgnoringCache(const std::optional<bool>& ignoreCache, const std::optional<WTF::String>& userAgent, const std::optional<WTF::String>& injectedScript,  WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&& completionHandler)
 {
     if (!m_extensionControllerProxy) {

diff --git a/Source/WebKit/UIProcess/API/APIInspectorExtension.h b/Source/WebKit/UIProcess/API/APIInspectorExtension.h
@@ -49,6 +49,7 @@ class InspectorExtension final : public API::ObjectImpl<Object::Type::InspectorE
 
     void createTab(const WTF::String& tabName, const WTF::URL& tabIconURL, const WTF::URL& sourceURL, WTF::CompletionHandler<void(Expected<Inspector::ExtensionTabID, Inspector::ExtensionError>)>&&);
     void evaluateScript(const WTF::String& scriptSource, const std::optional<WTF::URL>& frameURL, const std::optional<WTF::URL>& contextSecurityOrigin, const std::optional<bool>& useContentScriptContext, WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&&);
+    void navigateTab(const Inspector::ExtensionTabID&, const WTF::URL& sourceURL, WTF::CompletionHandler<void(const std::optional<Inspector::ExtensionError>)>&&);
     void reloadIgnoringCache(const std::optional<bool>& ignoreCache, const std::optional<WTF::String>& userAgent, const std::optional<WTF::String>& injectedScript, WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&&);
 
     // For testing.

diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm b/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm
@@ -290,4 +290,24 @@ - (void)showExtensionTabWithIdentifier:(NSString *)extensionTabIdentifier comple
 #endif
 }
 
+- (void)navigateExtensionTabWithIdentifier:(NSString *)extensionTabIdentifier toURL:(NSURL *)url completionHandler:(void(^)(NSError *))completionHandler
+{
+#if ENABLE(INSPECTOR_EXTENSIONS)
+    // It is an error to call this method prior to creating a frontend (i.e., with -connect or -show).
+    if (!_inspector->extensionController()) {
+        completionHandler([NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:@{ NSLocalizedFailureReasonErrorKey: Inspector::extensionErrorToString(Inspector::ExtensionError::InvalidRequest) }]);
+        return;
+    }
+
+    _inspector->extensionController()->navigateTabForExtension(extensionTabIdentifier, url, [protectedSelf = retainPtr(self), capturedBlock = makeBlockPtr(completionHandler)] (const std::optional<Inspector::ExtensionError>&& result) mutable {
+        if (result) {
+            capturedBlock([NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:@{ NSLocalizedFailureReasonErrorKey: Inspector::extensionErrorToString(result.value()) }]);
+            return;
+        }
+
+        capturedBlock(nil);
+    });
+#endif
+}
+
 @end
diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.h b/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.h
@@ -75,6 +75,14 @@ WK_CLASS_AVAILABLE(macos(12.0))
  */
 - (void)evaluateScript:(NSString *)scriptSource inTabWithIdentifier:(NSString *)tabIdentifier completionHandler:(void(^)(NSError * _Nullable, id result))completionHandler;
 
+/**
+ * @abstract Navigates a tab created by this _WKInspectorExtension to a new URL.
+ * @param url The url to be loaded.
+ * @param tabIdentifier Identifier for the Web Inspector tab in which to navigate.
+ * @param completionHandler A block to invoke when the operation completes or fails.
+ */
+- (void)navigateToURL:(NSURL *)url inTabWithIdentifier:(NSString *)tabIdentifier completionHandler:(void(^)(NSError * _Nullable))completionHandler;
+
 /**
  * @abstract Reloads the inspected page on behalf of the _WKInspectorExtension.
  * @param ignoreCache If YES, reloads the page while ignoring the cache.

diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.mm b/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.mm
@@ -122,6 +122,18 @@ - (void)evaluateScript:(NSString *)scriptSource inTabWithIdentifier:(NSString *)
     });
 }
 
+- (void)navigateToURL:(NSURL *)url inTabWithIdentifier:(NSString *)extensionTabIdentifier completionHandler:(void(^)(NSError *))completionHandler
+{
+    _extension->navigateTab(extensionTabIdentifier, url, [protectedSelf = retainPtr(self), capturedBlock = makeBlockPtr(WTFMove(completionHandler))] (const std::optional<Inspector::ExtensionError> result) mutable {
+        if (result) {
+            capturedBlock([NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:@{ NSLocalizedFailureReasonErrorKey: Inspector::extensionErrorToString(result.value()) }]);
+            return;
+        }
+
+        capturedBlock(nil);
+    });
+}
+
 - (void)reloadIgnoringCache:(BOOL)ignoreCache userAgent:(NSString *)userAgent injectedScript:(NSString *)injectedScript completionHandler:(void(^)(NSError *))completionHandler
 {
     std::optional<String> optionalUserAgent = userAgent ? std::make_optional(String(userAgent)) : std::nullopt;

diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtensionHost.h b/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtensionHost.h
@@ -63,6 +63,16 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)showExtensionTabWithIdentifier:(NSString *)extensionTabIdentifier completionHandler:(void(^)(NSError * _Nullable))completionHandler;
 
+/**
+ * @abstract Loads the extension tab with a specified URL.
+ * @param extensionTabIdentifier An identifier for an extension tab created using WKInspectorExtension methods.
+ * @param url The URL that the should be loaded in the extension tab.
+ * @param completionHandler The completion handler to be called when the load succeeds or fails.
+ * @discussion This method has no effect if the extensionTabIdentifier is invalid.
+ * It is an error to call this method prior to calling -[_WKInspectorIBActions show].
+ */
+- (void)navigateExtensionTabWithIdentifier:(NSString *)extensionTabIdentifier toURL:(NSURL *)url completionHandler:(void(^)(NSError * _Nullable))completionHandler;
+
 /**
  * @abstract The web view that is used to host extension tabs created via _WKInspectorExtension.
  * @discussion Browsing contexts for extension tabs are loaded in subframes of this web view.

diff --git a/Source/WebKit/UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm b/Source/WebKit/UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm
@@ -243,6 +243,26 @@ - (void)showExtensionTabWithIdentifier:(NSString *)extensionTabIdentifier comple
 #endif
 }
 
+- (void)navigateExtensionTabWithIdentifier:(NSString *)extensionTabIdentifier toURL:(NSURL *)url completionHandler:(void(^)(NSError * _Nullable))completionHandler
+{
+#if ENABLE(INSPECTOR_EXTENSIONS)
+    // It is an error to call this method prior to creating a frontend (i.e., with -connect or -show).
+    if (!m_remoteInspectorProxy->extensionController()) {
+        completionHandler([NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:@{ NSLocalizedFailureReasonErrorKey: Inspector::extensionErrorToString(Inspector::ExtensionError::InvalidRequest) }]);
+        return;
+    }
+
+    m_remoteInspectorProxy->extensionController()->navigateTabForExtension(extensionTabIdentifier, url, [protectedSelf = retainPtr(self), capturedBlock = makeBlockPtr(completionHandler)] (const std::optional<Inspector::ExtensionError> result) mutable {
+        if (result) {
+            capturedBlock([NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:@{ NSLocalizedFailureReasonErrorKey: Inspector::extensionErrorToString(result.value()) }]);
+            return;
+        }
+
+        capturedBlock(nil);
+    });
+#endif
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

diff --git a/Source/WebKit/UIProcess/Inspector/WebInspectorUIExtensionControllerProxy.cpp b/Source/WebKit/UIProcess/Inspector/WebInspectorUIExtensionControllerProxy.cpp
@@ -212,6 +212,18 @@ void WebInspectorUIExtensionControllerProxy::showExtensionTab(const Inspector::E
     });
 }
 
+void WebInspectorUIExtensionControllerProxy::navigateTabForExtension(const Inspector::ExtensionTabID& extensionTabIdentifier, const URL& sourceURL, WTF::CompletionHandler<void(const std::optional<Inspector::ExtensionError>)>&& completionHandler)
+{
+    whenFrontendHasLoaded([weakThis = WeakPtr { *this }, extensionTabIdentifier, sourceURL, completionHandler = WTFMove(completionHandler)] () mutable {
+        if (!weakThis || !weakThis->m_inspectorPage) {
+            completionHandler(Inspector::ExtensionError::InvalidRequest);
+            return;
+        }
+
+        weakThis->m_inspectorPage->sendWithAsyncReply(Messages::WebInspectorUIExtensionController::NavigateTabForExtension { extensionTabIdentifier, sourceURL }, WTFMove(completionHandler));
+    });
+}
+
 // API for testing.
 
 void WebInspectorUIExtensionControllerProxy::evaluateScriptInExtensionTab(const Inspector::ExtensionTabID& extensionTabID, const String& scriptSource, WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&& completionHandler)

diff --git a/Source/WebKit/UIProcess/Inspector/WebInspectorUIExtensionControllerProxy.h b/Source/WebKit/UIProcess/Inspector/WebInspectorUIExtensionControllerProxy.h
@@ -62,7 +62,7 @@ class WebInspectorUIExtensionControllerProxy final
     void evaluateScriptForExtension(const Inspector::ExtensionID&, const String& scriptSource, const std::optional<URL>& frameURL, const std::optional<URL>& contextSecurityOrigin, const std::optional<bool>& useContentScriptContext, WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&&);
     void reloadForExtension(const Inspector::ExtensionID&, const std::optional<bool>& ignoreCache, const std::optional<String>& userAgent, const std::optional<String>& injectedScript, WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&&);
     void showExtensionTab(const Inspector::ExtensionTabID&, CompletionHandler<void(Expected<void, Inspector::ExtensionError>)>&&);
-
+    void navigateTabForExtension(const Inspector::ExtensionTabID&, const URL& sourceURL, CompletionHandler<void(const std::optional<Inspector::ExtensionError>)>&&);
     // API for testing.
     void evaluateScriptInExtensionTab(const Inspector::ExtensionTabID&, const String& scriptSource, WTF::CompletionHandler<void(Inspector::ExtensionEvaluationResult)>&&);
 

diff --git a/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.cpp b/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.cpp
@@ -374,6 +374,39 @@ void WebInspectorUIExtensionController::showExtensionTab(const Inspector::Extens
     });
 }
 
+void WebInspectorUIExtensionController::navigateTabForExtension(const Inspector::ExtensionTabID& extensionTabIdentifier, const URL& sourceURL, WTF::CompletionHandler<void(const std::optional<Inspector::ExtensionError>&)>&& completionHandler)
+{
+    if (!m_frontendClient) {
+        completionHandler(Inspector::ExtensionError::InvalidRequest);
+        return;
+    }
+
+    Vector<Ref<JSON::Value>> arguments {
+        JSON::Value::create(extensionTabIdentifier),
+        JSON::Value::create(sourceURL.string()),
+    };
+    m_frontendClient->frontendAPIDispatcher().dispatchCommandWithResultAsync("navigateTabForExtension"_s, WTFMove(arguments), [weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)](InspectorFrontendAPIDispatcher::EvaluationResult&& result) mutable {
+        if (!weakThis) {
+            completionHandler(Inspector::ExtensionError::ContextDestroyed);
+            return;
+        }
+
+        auto* frontendGlobalObject = weakThis->m_frontendClient->frontendAPIDispatcher().frontendGlobalObject();
+        if (!frontendGlobalObject) {
+            completionHandler(Inspector::ExtensionError::ContextDestroyed);
+            return;
+        }
+
+        if (auto parsedError = weakThis->parseExtensionErrorFromEvaluationResult(result)) {
+            LOG(Inspector, "Internal error encountered while evaluating upon the frontend: %s", Inspector::extensionErrorToString(*parsedError).utf8().data());
+            completionHandler(parsedError);
+            return;
+        }
+
+        completionHandler(std::nullopt);
+    });
+}
+
 // WebInspectorUIExtensionController IPC messages for testing.
 
 void WebInspectorUIExtensionController::evaluateScriptInExtensionTab(const Inspector::ExtensionTabID& extensionTabID, const String& scriptSource, CompletionHandler<void(const IPC::DataReference&, const std::optional<WebCore::ExceptionDetails>&, const std::optional<Inspector::ExtensionError>&)>&& completionHandler)

diff --git a/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.h b/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.h
@@ -70,6 +70,7 @@ class WebInspectorUIExtensionController
     void evaluateScriptForExtension(const Inspector::ExtensionID&, const String& scriptSource, const std::optional<URL>& frameURL, const std::optional<URL>& contextSecurityOrigin, const std::optional<bool>& useContentScriptContext, CompletionHandler<void(const IPC::DataReference&, const std::optional<WebCore::ExceptionDetails>&, const std::optional<Inspector::ExtensionError>&)>&&);
     void reloadForExtension(const Inspector::ExtensionID&, const std::optional<bool>& ignoreCache, const std::optional<String>& userAgent, const std::optional<String>& injectedScript, CompletionHandler<void(const std::optional<Inspector::ExtensionError>&)>&&);
     void showExtensionTab(const Inspector::ExtensionTabID&, CompletionHandler<void(Expected<void, Inspector::ExtensionError>)>&&);
+    void navigateTabForExtension(const Inspector::ExtensionTabID&, const URL& sourceURL, CompletionHandler<void(const std::optional<Inspector::ExtensionError>&)>&&);
 
     // WebInspectorUIExtensionController IPC messages for testing.
     void evaluateScriptInExtensionTab(const Inspector::ExtensionTabID&, const String& scriptSource, CompletionHandler<void(const IPC::DataReference&, const std::optional<WebCore::ExceptionDetails>&, const std::optional<Inspector::ExtensionError>&)>&&);

diff --git a/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.messages.in b/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.messages.in
@@ -30,6 +30,7 @@ messages -> WebInspectorUIExtensionController NotRefCounted {
     EvaluateScriptForExtension(String extensionID, String scriptSource, std::optional<URL> frameURL, std::optional<URL> contextSecurityOrigin, std::optional<bool> useContentScriptContext) -> (IPC::DataReference resultData, std::optional<WebCore::ExceptionDetails> details, std::optional<Inspector::ExtensionError> error)
     ReloadForExtension(String extensionID, std::optional<bool> ignoreCache, std::optional<String> userAgent, std::optional<String> injectedScript) -> (std::optional<Inspector::ExtensionError> error)
     ShowExtensionTab(String extensionTabIdentifier) -> (Expected<void, Inspector::ExtensionError> result)
+    NavigateTabForExtension(String extensionTabIdentifier, URL sourceURL) -> (std::optional<Inspector::ExtensionError> error)
 
     // For testing.
     EvaluateScriptInExtensionTab(String extensionTabID, String scriptSource) -> (IPC::DataReference resultData, std::optional<WebCore::ExceptionDetails> details, std::optional<Inspector::ExtensionError> error)