[webkit-changes] [295101] trunk

Wed, 01 Jun 2022 13:16:08 -0700

Title: [295101] trunk
Revision
295101
Author
achristen...@apple.com
Date
2022-06-01 13:15:16 -0700 (Wed, 01 Jun 2022)

Log Message

Allow decidePolicyForNavigation* decisionHandlers to be called on non-main runloops
https://bugs.webkit.org/show_bug.cgi?id=241157
<rdar://94130705>

Reviewed by Brady Eidson.

Wouldn't it be nice if all apps used your APIs exactly how you want them to?
This is not the case.  People call decision handlers on non-main threads.
When this happens, just hop to the main thread to avoid threading issues.

* Source/WebKit/UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationResponse):

Canonical link: https://commits.webkit.org/251196@main

Modified Paths

Diff

Modified: trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm (295100 => 295101)


--- trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm	2022-06-01 18:52:36 UTC (rev 295100)
+++ trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm	2022-06-01 20:15:16 UTC (rev 295101)
@@ -537,41 +537,39 @@
                 [NSException raise:NSInvalidArgumentException format:@"WKWebpagePreferences._customNavigatorPlatform must be nil for subframe navigations."];
         }
 
-        switch (actionPolicy) {
-        case WKNavigationActionPolicyAllow:
-        case _WKNavigationActionPolicyAllowInNewProcess:
-            tryInterceptNavigation(WTFMove(navigationAction), webPageProxy, [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool interceptedNavigation) mutable {
-                if (interceptedNavigation) {
-                    localListener->ignore();
-                    return;
-                }
+        ensureOnMainRunLoop([navigationAction = WTFMove(navigationAction), webPageProxy = WTFMove(webPageProxy), actionPolicy, localListener = WTFMove(localListener), apiWebsitePolicies = WTFMove(apiWebsitePolicies)] () mutable {
+            switch (actionPolicy) {
+            case WKNavigationActionPolicyAllow:
+            case _WKNavigationActionPolicyAllowInNewProcess:
+                tryInterceptNavigation(WTFMove(navigationAction), webPageProxy, [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool interceptedNavigation) mutable {
+                    if (interceptedNavigation) {
+                        localListener->ignore();
+                        return;
+                    }
 
-                localListener->use(websitePolicies.get(), actionPolicy == _WKNavigationActionPolicyAllowInNewProcess ? ProcessSwapRequestedByClient::Yes : ProcessSwapRequestedByClient::No);
-            });
-        
-            break;
+                    localListener->use(websitePolicies.get(), actionPolicy == _WKNavigationActionPolicyAllowInNewProcess ? ProcessSwapRequestedByClient::Yes : ProcessSwapRequestedByClient::No);
+                });
+                break;
 
-        case WKNavigationActionPolicyCancel:
-            localListener->ignore();
-            break;
+            case WKNavigationActionPolicyCancel:
+                localListener->ignore();
+                break;
 
-            ALLOW_DEPRECATED_DECLARATIONS_BEGIN
-        case _WKNavigationActionPolicyDownload:
-            ALLOW_DEPRECATED_DECLARATIONS_END
-            localListener->download();
-            break;
+            case WKNavigationActionPolicyDownload:
+                localListener->download();
+                break;
 
-        case _WKNavigationActionPolicyAllowWithoutTryingAppLink:
-            trySOAuthorization(WTFMove(navigationAction), webPageProxy, [localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)] (bool optimizedLoad) {
-                if (optimizedLoad) {
-                    localListener->ignore();
-                    return;
-                }
+            case _WKNavigationActionPolicyAllowWithoutTryingAppLink:
+                trySOAuthorization(WTFMove(navigationAction), webPageProxy, [localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)] (bool optimizedLoad) {
+                    if (optimizedLoad) {
+                        localListener->ignore();
+                        return;
+                    }
 
-                localListener->use(websitePolicies.get());
-            });
-            break;
-        }
+                    localListener->use(websitePolicies.get());
+                });
+                break;
+            }        });
     };
 
     if (delegateHasWebpagePreferences) {
@@ -578,7 +576,7 @@
         if (m_navigationState->m_navigationDelegateMethods.webViewDecidePolicyForNavigationActionWithPreferencesDecisionHandler)
             [navigationDelegate webView:m_navigationState->m_webView decidePolicyForNavigationAction:wrapper(navigationAction) preferences:wrapper(defaultWebsitePolicies) decisionHandler:makeBlockPtr(WTFMove(decisionHandlerWithPreferencesOrPolicies)).get()];
         else
-            [(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState->m_webView decidePolicyForNavigationAction:wrapper(navigationAction) preferences:wrapper(defaultWebsitePolicies) userInfo:userInfo ? static_cast<id<NSSecureCoding>>(userInfo->wrapper()) : nil decisionHandler:makeBlockPtr(WTFMove(decisionHandlerWithPreferencesOrPolicies)).get()];
+            [(id<WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState->m_webView decidePolicyForNavigationAction:wrapper(navigationAction) preferences:wrapper(defaultWebsitePolicies) userInfo:userInfo ? static_cast<id<NSSecureCoding>>(userInfo->wrapper()) : nil decisionHandler:makeBlockPtr(WTFMove(decisionHandlerWithPreferencesOrPolicies)).get()];
     } else {
         auto decisionHandler = [decisionHandlerWithPreferencesOrPolicies = WTFMove(decisionHandlerWithPreferencesOrPolicies)] (WKNavigationActionPolicy actionPolicy) mutable {
             decisionHandlerWithPreferencesOrPolicies(actionPolicy, nil);
@@ -654,24 +652,25 @@
         return;
 
     auto checker = CompletionHandlerCallChecker::create(navigationDelegate.get(), @selector(webView:decidePolicyForNavigationResponse:decisionHandler:));
-    [navigationDelegate webView:m_navigationState->m_webView decidePolicyForNavigationResponse:wrapper(navigationResponse) decisionHandler:makeBlockPtr([localListener = WTFMove(listener), checker = WTFMove(checker)](WKNavigationResponsePolicy responsePolicy) {
+    [navigationDelegate webView:m_navigationState->m_webView decidePolicyForNavigationResponse:wrapper(navigationResponse) decisionHandler:makeBlockPtr([localListener = WTFMove(listener), checker = WTFMove(checker)](WKNavigationResponsePolicy responsePolicy) mutable {
         if (checker->completionHandlerHasBeenCalled())
             return;
         checker->didCallCompletionHandler();
+        ensureOnMainRunLoop([responsePolicy, localListener = WTFMove(localListener)] {
+            switch (responsePolicy) {
+            case WKNavigationResponsePolicyAllow:
+                localListener->use();
+                break;
 
-        switch (responsePolicy) {
-        case WKNavigationResponsePolicyAllow:
-            localListener->use();
-            break;
+            case WKNavigationResponsePolicyCancel:
+                localListener->ignore();
+                break;
 
-        case WKNavigationResponsePolicyCancel:
-            localListener->ignore();
-            break;
-
-        case WKNavigationResponsePolicyDownload:
-            localListener->download();
-            break;
-        }
+            case WKNavigationResponsePolicyDownload:
+                localListener->download();
+                break;
+            }
+        });
     }).get()];
 }

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm (295100 => 295101)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm	2022-06-01 18:52:36 UTC (rev 295100)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm	2022-06-01 20:15:16 UTC (rev 295101)
@@ -215,3 +215,31 @@
     [webView loadHTMLString:html baseURL:nil];
     TestWebKitAPI::Util::run(&done);
 }
+
+TEST(WKNavigationAction, NonMainThread)
+{
+    TestWebKitAPI::HTTPServer server({
+        { "/"_s, { "hi"_s } },
+    });
+
+    auto delegate = adoptNS([TestNavigationDelegate new]);
+    auto webView = adoptNS([WKWebView new]);
+    [webView setNavigationDelegate:delegate.get()];
+    __block bool done = false;
+    delegate.get().decidePolicyForNavigationAction = ^(WKNavigationAction *action, void (^completionHandler)(WKNavigationActionPolicy)) {
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+            completionHandler(WKNavigationActionPolicyAllow);
+        });
+    };
+    delegate.get().decidePolicyForNavigationResponse = ^(WKNavigationResponse *action, void (^completionHandler)(WKNavigationResponsePolicy)) {
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+            completionHandler(WKNavigationResponsePolicyAllow);
+        });
+    };
+    delegate.get().didFinishNavigation = ^(WKWebView *, WKNavigation *) {
+        done = true;
+    };
+
+    [webView loadRequest:server.request()];
+    TestWebKitAPI::Util::run(&done);
+}

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to