Title: [292539] trunk
Revision
292539
Author
you...@apple.com
Date
2022-04-07 09:10:36 -0700 (Thu, 07 Apr 2022)

Log Message

Use the same callback mechanism for ServiceWorker openWindow and navigate in UIProcess
https://bugs.webkit.org/show_bug.cgi?id=238924

Reviewed by Chris Dumez.

Source/WebKit:

Reuse WebFrameProxy navigation delegate for openWindow once the main frame is created.
This ensures we get the same behavior for both code paths and makes sure openWindow does not hang if a delegate cancels the load.

Covered by API test.

* UIProcess/WebFrameProxy.cpp:
(WebKit::WebFrameProxy::transferNavigationCallbackToFrame):
(WebKit::WebFrameProxy::setNavigationCallback):
* UIProcess/WebFrameProxy.h:
(WebKit::WebFrameProxy::transferNavigationCallbackToFrame): Deleted.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didCreateMainFrame):
(WebKit::WebPageProxy::callLoadCompletionHandlersIfNecessary):
* UIProcess/WebPageProxy.h:
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::openWindowFromServiceWorker):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
(-[ServiceWorkerPSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[ServiceWorkerPSONNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
(-[ServiceWorkerOpenWindowWebsiteDataStoreDelegate initWithConfiguration:]):
(-[ServiceWorkerOpenWindowWebsiteDataStoreDelegate websiteDataStore:openWindow:fromServiceWorkerOrigin:completionHandler:]):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (292538 => 292539)


--- trunk/Source/WebKit/ChangeLog	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Source/WebKit/ChangeLog	2022-04-07 16:10:36 UTC (rev 292539)
@@ -1,3 +1,27 @@
+2022-04-07  Youenn Fablet  <you...@apple.com>
+
+        Use the same callback mechanism for ServiceWorker openWindow and navigate in UIProcess
+        https://bugs.webkit.org/show_bug.cgi?id=238924
+
+        Reviewed by Chris Dumez.
+
+        Reuse WebFrameProxy navigation delegate for openWindow once the main frame is created.
+        This ensures we get the same behavior for both code paths and makes sure openWindow does not hang if a delegate cancels the load.
+
+        Covered by API test.
+
+        * UIProcess/WebFrameProxy.cpp:
+        (WebKit::WebFrameProxy::transferNavigationCallbackToFrame):
+        (WebKit::WebFrameProxy::setNavigationCallback):
+        * UIProcess/WebFrameProxy.h:
+        (WebKit::WebFrameProxy::transferNavigationCallbackToFrame): Deleted.
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didCreateMainFrame):
+        (WebKit::WebPageProxy::callLoadCompletionHandlersIfNecessary):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::openWindowFromServiceWorker):
+
 2022-04-07  Chris Dumez  <cdu...@apple.com>
 
         Drop unused EditorClient::getAutoCorrectSuggestionForMisspelledWord()

Modified: trunk/Source/WebKit/UIProcess/WebFrameProxy.cpp (292538 => 292539)


--- trunk/Source/WebKit/UIProcess/WebFrameProxy.cpp	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Source/WebKit/UIProcess/WebFrameProxy.cpp	2022-04-07 16:10:36 UTC (rev 292539)
@@ -282,6 +282,17 @@
     m_frameLoadState.setUnreachableURL(unreachableURL);
 }
 
+void WebFrameProxy::transferNavigationCallbackToFrame(WebFrameProxy& frame)
+{
+    frame.setNavigationCallback(WTFMove(m_navigateCallback));
+}
+
+void WebFrameProxy::setNavigationCallback(CompletionHandler<void(std::optional<WebCore::PageIdentifier>)>&& navigateCallback)
+{
+    ASSERT(!m_navigateCallback);
+    m_navigateCallback = WTFMove(navigateCallback);
+}
+
 #if ENABLE(CONTENT_FILTERING)
 bool WebFrameProxy::didHandleContentFilterUnblockNavigation(const ResourceRequest& request)
 {

Modified: trunk/Source/WebKit/UIProcess/WebFrameProxy.h (292538 => 292539)


--- trunk/Source/WebKit/UIProcess/WebFrameProxy.h	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Source/WebKit/UIProcess/WebFrameProxy.h	2022-04-07 16:10:36 UTC (rev 292539)
@@ -130,7 +130,8 @@
     void collapseSelection();
 #endif
 
-    void transferNavigationCallbackToFrame(WebFrameProxy& frame) { frame.m_navigateCallback = WTFMove(m_navigateCallback); }
+    void transferNavigationCallbackToFrame(WebFrameProxy&);
+    void setNavigationCallback(CompletionHandler<void(std::optional<WebCore::PageIdentifier>)>&&);
 
 private:
     WebFrameProxy(WebPageProxy&, WebCore::FrameIdentifier);

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (292538 => 292539)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2022-04-07 16:10:36 UTC (rev 292539)
@@ -4610,6 +4610,9 @@
 
     m_mainFrame = WebFrameProxy::create(*this, frameID);
 
+    if (m_serviceWorkerOpenWindowCompletionCallback)
+        m_mainFrame->setNavigationCallback(WTFMove(m_serviceWorkerOpenWindowCompletionCallback));
+
     // Add the frame to the process wide map.
     m_process->frameCreated(frameID, *m_mainFrame);
 }
@@ -4956,7 +4959,7 @@
         m_serviceWorkerLaunchCompletionHandler(false);
 
     if (m_serviceWorkerOpenWindowCompletionCallback)
-        m_serviceWorkerOpenWindowCompletionCallback(success);
+        m_serviceWorkerOpenWindowCompletionCallback(success ? webPageID() : std::optional<WebCore::PageIdentifier> { });
 #endif
 }
 

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (292538 => 292539)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2022-04-07 16:10:36 UTC (rev 292539)
@@ -2087,7 +2087,7 @@
     void interactionRegions(WebCore::FloatRect rectInContentCoordinates, CompletionHandler<void(Vector<WebCore::InteractionRegion>)>&&);
 
 #if ENABLE(SERVICE_WORKER)
-    void setServiceWorkerOpenWindowCompletionCallback(CompletionHandler<void(bool)>&& completionCallback)
+    void setServiceWorkerOpenWindowCompletionCallback(CompletionHandler<void(std::optional<WebCore::PageIdentifier>)>&& completionCallback)
     {
         ASSERT(!m_serviceWorkerOpenWindowCompletionCallback);
         m_serviceWorkerOpenWindowCompletionCallback = WTFMove(completionCallback);
@@ -3137,7 +3137,7 @@
 #if ENABLE(SERVICE_WORKER)
     bool m_isServiceWorkerPage { false };
     CompletionHandler<void(bool)> m_serviceWorkerLaunchCompletionHandler;
-    CompletionHandler<void(bool)> m_serviceWorkerOpenWindowCompletionCallback;
+    CompletionHandler<void(std::optional<WebCore::PageIdentifier>)> m_serviceWorkerOpenWindowCompletionCallback;
 #endif
 
     RunLoop::Timer<WebPageProxy> m_tryCloseTimeoutTimer;

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (292538 => 292539)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2022-04-07 16:10:36 UTC (rev 292539)
@@ -2135,18 +2135,8 @@
             return;
         }
 
-        auto innerCallback = [pageID = newPage->identifier(), callback = WTFMove(callback)] (bool success) mutable {
-            auto* newPage = WebProcessProxy::webPage(pageID);
-            if (!newPage || !success) {
-                callback(std::nullopt);
-                return;
-            }
-
-            callback(newPage->webPageID());
-        };
-
 #if ENABLE(SERVICE_WORKER)
-        newPage->setServiceWorkerOpenWindowCompletionCallback(WTFMove(innerCallback));
+        newPage->setServiceWorkerOpenWindowCompletionCallback(WTFMove(callback));
 #endif
     };
 

Modified: trunk/Tools/ChangeLog (292538 => 292539)


--- trunk/Tools/ChangeLog	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Tools/ChangeLog	2022-04-07 16:10:36 UTC (rev 292539)
@@ -1,3 +1,16 @@
+2022-04-07  Youenn Fablet  <you...@apple.com>
+
+        Use the same callback mechanism for ServiceWorker openWindow and navigate in UIProcess
+        https://bugs.webkit.org/show_bug.cgi?id=238924
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
+        (-[ServiceWorkerPSONNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
+        (-[ServiceWorkerPSONNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
+        (-[ServiceWorkerOpenWindowWebsiteDataStoreDelegate initWithConfiguration:]):
+        (-[ServiceWorkerOpenWindowWebsiteDataStoreDelegate websiteDataStore:openWindow:fromServiceWorkerOrigin:completionHandler:]):
+
 2022-04-07  Jonathan Bedard  <jbed...@apple.com>
 
         [git-webkit] Clear merging-blocked label on PR update

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (292538 => 292539)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	2022-04-07 16:09:50 UTC (rev 292538)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	2022-04-07 16:10:36 UTC (rev 292539)
@@ -51,6 +51,7 @@
 #import <WebKit/_WKRemoteObjectInterface.h>
 #import <WebKit/_WKRemoteObjectRegistry.h>
 #import <WebKit/_WKWebsiteDataStoreConfiguration.h>
+#import <WebKit/_WKWebsiteDataStoreDelegate.h>
 #import <wtf/Deque.h>
 #import <wtf/HashMap.h>
 #import <wtf/RetainPtr.h>
@@ -3084,6 +3085,13 @@
 "        alert(event.data);"
 "    };"
 "}"
+""
+"function openWindowToURL(url) {"
+"    worker.postMessage({openWindowToURL: url});"
+"    navigator.serviceWorker._onmessage_ = (event) => {"
+"        alert(event.data);"
+"    };"
+"}"
 "</script>"_s;
 
 static constexpr auto ServiceWorkerWindowClientNavigateJS =
@@ -3111,9 +3119,18 @@
 "       event.source.postMessage(currentClients.length + ' client(s)');"
 "       return;"
 "   }"
+"   if (event.data && event.data.openWindowToURL) {"
+"       await self.clients.openWindow(event.data.openWindowToURL).then((client) => {"
+"           event.source.postMessage(client ? 'client' : 'none');"
+"       }, (e) => {"
+"           event.source.postMessage('failed');"
+"       });"
+"       return;"
+"   }"
 "});"_s;
 
-
+static bool shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+static bool shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
 @interface ServiceWorkerPSONNavigationDelegate : NSObject <WKNavigationDelegatePrivate> {
     @public void (^decidePolicyForNavigationAction)(WKNavigationAction *, void (^)(WKNavigationActionPolicy));
     @public void (^didStartProvisionalNavigationHandler)();
@@ -3131,12 +3148,12 @@
 
 - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
 {
-    decisionHandler(WKNavigationActionPolicyAllow);
+    decisionHandler(shouldServiceWorkerPSONNavigationDelegateAllowNavigation ? WKNavigationActionPolicyAllow : WKNavigationActionPolicyCancel);
 }
 
 - (void)webView:(WKWebView *)webView decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
 {
-    decisionHandler(WKNavigationResponsePolicyAllow);
+    decisionHandler(shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse ? WKNavigationResponsePolicyAllow : WKNavigationResponsePolicyCancel);
 }
 
 @end
@@ -3166,6 +3183,8 @@
     TestWebKitAPI::HTTPServer server({
         { "/"_s, { ServiceWorkerWindowClientNavigateMain } },
         { "/?test"_s, { ServiceWorkerWindowClientNavigateMain } },
+        { "/?fail1"_s, { ServiceWorkerWindowClientNavigateMain } },
+        { "/?fail2"_s, { ServiceWorkerWindowClientNavigateMain } },
         { "/?swap"_s, { {{ "Content-Type"_s, "application/html"_s }, { "Cross-Origin-Opener-Policy"_s, "same-origin"_s } }, ServiceWorkerWindowClientNavigateMain } },
         { "/sw.js"_s, { {{ "Content-Type"_s, "application/_javascript_"_s }}, ServiceWorkerWindowClientNavigateJS } }
     }, TestWebKitAPI::HTTPServer::Protocol::Http, nullptr, nullptr, 8091);
@@ -3194,6 +3213,19 @@
 
     [webView1 evaluateJavaScript:@"countServiceWorkerClients()" completionHandler: nil];
     EXPECT_WK_STREQ([webView1 _test_waitForAlert], "1 client(s)");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = false;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
+    [webView1 evaluateJavaScript:[NSString stringWithFormat:@"navigateOtherClientToURL('%@?fail1')", baseURL] completionHandler: nil];
+    EXPECT_WK_STREQ([webView1 _test_waitForAlert], "none");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = false;
+    [webView1 evaluateJavaScript:[NSString stringWithFormat:@"navigateOtherClientToURL('%@?fail2')", baseURL] completionHandler: nil];
+    EXPECT_WK_STREQ([webView1 _test_waitForAlert], "none");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
 }
 
 TEST(ServiceWorker, WindowClientNavigateCrossOrigin)
@@ -3231,3 +3263,121 @@
     [webView1 evaluateJavaScript:[NSString stringWithFormat:@"navigateOtherClientToURL('%@')", [[server2.request() URL] absoluteString]] completionHandler: nil];
     EXPECT_WK_STREQ([webView1 _test_waitForAlert], "none");
 }
+
+@interface ServiceWorkerOpenWindowWebsiteDataStoreDelegate: NSObject <_WKWebsiteDataStoreDelegate> {
+@private
+    WKWebViewConfiguration* _configuration;
+    RetainPtr<ServiceWorkerPSONNavigationDelegate> _navigationDelegate;
+    RetainPtr<TestWKWebView> _webView;
+}
+- (instancetype)initWithConfiguration:(WKWebViewConfiguration*)configuration;
+@end
+
+@implementation ServiceWorkerOpenWindowWebsiteDataStoreDelegate { }
+- (instancetype)initWithConfiguration:(WKWebViewConfiguration*)configuration
+{
+    _configuration = configuration;
+    return self;
+}
+
+- (void)websiteDataStore:(WKWebsiteDataStore *)dataStore openWindow:(NSURL *)url fromServiceWorkerOrigin:(WKSecurityOrigin *)serviceWorkerOrigin completionHandler:(void (^)(WKWebView *newWebView))completionHandler
+{
+    _navigationDelegate = adoptNS([[ServiceWorkerPSONNavigationDelegate alloc] init]);
+    _webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 300, 300) configuration:_configuration addToWindow:YES]);
+    [_webView setNavigationDelegate:_navigationDelegate.get()];
+
+    [_webView loadRequest:[NSURLRequest requestWithURL:url]];
+    completionHandler(_webView.get());
+}
+
+@end
+
+TEST(ServiceWorker, OpenWindowWebsiteDataStoreDelegate)
+{
+    [WKWebsiteDataStore _allowWebsiteDataRecordsForAllOrigins];
+
+    // Start with a clean slate data store
+    [[WKWebsiteDataStore defaultDataStore] removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+    auto preferences = [configuration preferences];
+    for (_WKInternalDebugFeature *feature in [WKPreferences _internalDebugFeatures]) {
+        if ([feature.key isEqualToString:@"ServiceWorkersUserGestureEnabled"])
+            [preferences _setEnabled:NO forInternalDebugFeature:feature];
+    }
+
+    auto dataStoreDelegate = adoptNS([[ServiceWorkerOpenWindowWebsiteDataStoreDelegate alloc] initWithConfiguration:configuration.get()]);
+    [[configuration websiteDataStore] set_delegate:dataStoreDelegate.get()];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 300, 300) configuration:configuration.get() addToWindow:YES]);
+
+    TestWebKitAPI::HTTPServer server({
+        { "/"_s, { ServiceWorkerWindowClientNavigateMain } },
+        { "/sw.js"_s, { {{ "Content-Type"_s, "application/_javascript_"_s }}, ServiceWorkerWindowClientNavigateJS } }
+    }, TestWebKitAPI::HTTPServer::Protocol::Http, nullptr, nullptr, 8091);
+
+    [webView loadRequest:server.request()];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "successfully registered");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
+    [webView evaluateJavaScript:[NSString stringWithFormat:@"openWindowToURL('%@')", [[server.request() URL] absoluteString]] completionHandler: nil];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "client");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = false;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
+    [webView evaluateJavaScript:[NSString stringWithFormat:@"openWindowToURL('%@')", [[server.request() URL] absoluteString]] completionHandler: nil];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "none");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = false;
+    [webView evaluateJavaScript:[NSString stringWithFormat:@"openWindowToURL('%@')", [[server.request() URL] absoluteString]] completionHandler: nil];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "none");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
+}
+
+TEST(ServiceWorker, OpenWindowCOOP)
+{
+    [WKWebsiteDataStore _allowWebsiteDataRecordsForAllOrigins];
+
+    // Start with a clean slate data store
+    [[WKWebsiteDataStore defaultDataStore] removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+    auto preferences = [configuration preferences];
+    for (_WKInternalDebugFeature *feature in [WKPreferences _internalDebugFeatures]) {
+        if ([feature.key isEqualToString:@"ServiceWorkersUserGestureEnabled"])
+            [preferences _setEnabled:NO forInternalDebugFeature:feature];
+    }
+
+    auto dataStoreDelegate = adoptNS([[ServiceWorkerOpenWindowWebsiteDataStoreDelegate alloc] initWithConfiguration:configuration.get()]);
+    [[configuration websiteDataStore] set_delegate:dataStoreDelegate.get()];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 300, 300) configuration:configuration.get() addToWindow:YES]);
+
+    TestWebKitAPI::HTTPServer server({
+        { "/"_s, { ServiceWorkerWindowClientNavigateMain } },
+        { "/sw.js"_s, { {{ "Content-Type"_s, "application/_javascript_"_s }}, ServiceWorkerWindowClientNavigateJS } },
+        { "/?swap"_s, { {{ "Content-Type"_s, "application/html"_s }, { "Cross-Origin-Opener-Policy"_s, "same-origin"_s } }, ServiceWorkerWindowClientNavigateMain } }
+    }, TestWebKitAPI::HTTPServer::Protocol::Http, nullptr, nullptr, 8091);
+
+    [webView loadRequest:server.request()];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "successfully registered");
+
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigation = true;
+    shouldServiceWorkerPSONNavigationDelegateAllowNavigationResponse = true;
+    [webView evaluateJavaScript:[NSString stringWithFormat:@"openWindowToURL('%@?swap')", [[server.request() URL] absoluteString]] completionHandler: nil];
+    EXPECT_WK_STREQ([webView _test_waitForAlert], "client");
+}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to