Changeset 292468 in webkit

Timestamp:

Apr 6, 2022 8:20:50 AM (4 months ago)

Author:

youenn@apple.com

Message:

In case of COOP-based process swap, we need to make sure the document gets controlled by its matching service worker registration
​https://bugs.webkit.org/show_bug.cgi?id=238800
<rdar://problem/91288849>

Reviewed by Chris Dumez.

Source/WebKit:

Make sure in case loader is transferred to call controlClient in the new WebProcess.
To do so, we also update page and frame IDs.
Covered by API test.

• NetworkProcess/NetworkConnectionToWebProcess.cpp:
• NetworkProcess/NetworkLoadParameters.h:
• NetworkProcess/NetworkResourceLoadParameters.h:
• NetworkProcess/NetworkResourceLoader.cpp:
• NetworkProcess/NetworkResourceLoader.h:
• NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
• NetworkProcess/ServiceWorker/WebSWServerConnection.h:

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkLoadParameters.h (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.h (modified) (1 diff)
• Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (modified) (2 diffs)
• Source/WebKit/NetworkProcess/NetworkResourceLoader.h (modified) (4 diffs)
• Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp (modified) (2 diffs)
• Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-04-06  Youenn Fablet  <youenn@apple.com>
+
+        In case of COOP-based process swap, we need to make sure the document gets controlled by its matching service worker registration
+        https://bugs.webkit.org/show_bug.cgi?id=238800
+        <rdar://problem/91288849>
+
+        Reviewed by Chris Dumez.
+
+        Make sure in case loader is transferred to call controlClient in the new WebProcess.
+        To do so, we also update page and frame IDs.
+        Covered by API test.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        * NetworkProcess/NetworkLoadParameters.h:
+        * NetworkProcess/NetworkResourceLoadParameters.h:
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        * NetworkProcess/NetworkResourceLoader.h:
+        * NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
+        * NetworkProcess/ServiceWorker/WebSWServerConnection.h:
+
 2022-04-06  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

@@ -590 +590 @@
                 CONNECTION_RELEASE_LOG(Loading, "scheduleResourceLoad: Resuming existing NetworkResourceLoader");
                 m_networkResourceLoaders.add(identifier, *existingLoader);
-                existingLoader->transferToNewWebProcess(*this, identifier);
+                existingLoader->transferToNewWebProcess(*this, loadParameters);
                 return;
             }

trunk/Source/WebKit/NetworkProcess/NetworkLoadParameters.h

@@ -45 +45 @@
 class NetworkLoadParameters {
 public:
-    WebPageProxyIdentifier webPageProxyID;
-    WebCore::PageIdentifier webPageID;
-    WebCore::FrameIdentifier webFrameID;
+    mutable WebPageProxyIdentifier webPageProxyID;
+    mutable WebCore::PageIdentifier webPageID;
+    mutable WebCore::FrameIdentifier webFrameID;
     RefPtr<WebCore::SecurityOrigin> topOrigin;
     RefPtr<WebCore::SecurityOrigin> sourceOrigin;

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.h

@@ -57 +57 @@
     RefPtr<SandboxExtension> resourceSandboxExtension; // Created automatically for the sender.
     mutable Seconds maximumBufferingTime;
-    WebCore::FetchOptions options;
+    mutable WebCore::FetchOptions options;
     std::optional<WebCore::ContentSecurityPolicyResponseHeaders> cspResponseHeaders;
     WebCore::CrossOriginEmbedderPolicy parentCrossOriginEmbedderPolicy;

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

@@ -577 +577 @@
 }
 
-void NetworkResourceLoader::transferToNewWebProcess(NetworkConnectionToWebProcess& newConnection, WebCore::ResourceLoaderIdentifier newCoreIdentifier)
+void NetworkResourceLoader::transferToNewWebProcess(NetworkConnectionToWebProcess& newConnection, const NetworkResourceLoadParameters& parameters)
 {
     m_connection = newConnection;
-    m_parameters.identifier = newCoreIdentifier;
+    m_parameters.identifier = parameters.identifier;
+    m_parameters.webPageProxyID = parameters.webPageProxyID;
+    m_parameters.webPageID = parameters.webPageID;
+    m_parameters.webFrameID = parameters.webFrameID;
+    m_parameters.options.clientIdentifier = parameters.options.clientIdentifier;
 
 #if ENABLE(SERVICE_WORKER)
     ASSERT(m_responseCompletionHandler || m_cacheEntryWaitingForContinueDidReceiveResponse || m_serviceWorkerFetchTask);
+    if (m_serviceWorkerRegistration) {
+        if (auto* swConnection = newConnection.swConnection())
+            swConnection->transferServiceWorkerLoadToNewWebProcess(*this, *m_serviceWorkerRegistration);
+    }
 #else
     ASSERT(m_responseCompletionHandler || m_cacheEntryWaitingForContinueDidReceiveResponse);
@@ -1209 +1217 @@
 #if ENABLE(SERVICE_WORKER)
     if (parameters().options.mode == FetchOptions::Mode::Navigate) {
+        m_serviceWorkerRegistration = { };
         if (auto serviceWorkerFetchTask = m_connection->createFetchTask(*this, newRequest)) {
             LOADER_RELEASE_LOG("continueWillSendRequest: Created a ServiceWorkerFetchTask to handle the redirect (fetchIdentifier=%" PRIu64 ")", serviceWorkerFetchTask->fetchIdentifier().toUInt64());

trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h

@@ -41 +41 @@
 #include <WebCore/PrivateClickMeasurement.h>
 #include <WebCore/ResourceResponse.h>
+#include <WebCore/SWServerRegistration.h>
 #include <WebCore/SecurityPolicyViolationEvent.h>
 #include <WebCore/SharedBuffer.h>
@@ -94 +95 @@
     void abort();
 
-    void transferToNewWebProcess(NetworkConnectionToWebProcess&, WebCore::ResourceLoaderIdentifier);
+    void transferToNewWebProcess(NetworkConnectionToWebProcess&, const NetworkResourceLoadParameters&);
 
     // Message handlers.
@@ -155 +156 @@
     void setResultingClientIdentifier(String&& identifier) { m_resultingClientIdentifier = WTFMove(identifier); }
     const String& resultingClientIdentifier() const { return m_resultingClientIdentifier; }
+    void setServiceWorkerRegistration(WebCore::SWServerRegistration& serviceWorkerRegistration) { m_serviceWorkerRegistration = serviceWorkerRegistration; }
 #endif
 
@@ -289 +291 @@
     std::unique_ptr<ServiceWorkerFetchTask> m_serviceWorkerFetchTask;
     String m_resultingClientIdentifier;
+    WeakPtr<WebCore::SWServerRegistration> m_serviceWorkerRegistration;
 #endif
     NetworkResourceLoadIdentifier m_resourceLoadID;

trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp

@@ -205 +205 @@
         controlClient(loader.parameters(), *registration, request);
         loader.setResultingClientIdentifier(loader.parameters().options.clientIdentifier->toString());
+        loader.setServiceWorkerRegistration(*registration);
     } else {
         if (!loader.parameters().serviceWorkerRegistrationIdentifier)
@@ -667 +668 @@
 }
 
+void WebSWServerConnection::transferServiceWorkerLoadToNewWebProcess(NetworkResourceLoader& loader, WebCore::SWServerRegistration& registration)
+{
+    controlClient(loader.parameters(), registration, loader.originalRequest());
+}
+
 } // namespace WebKit
 

trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.h

@@ -85 +85 @@
     void fetchTaskTimedOut(WebCore::ServiceWorkerIdentifier);
 
+    void transferServiceWorkerLoadToNewWebProcess(NetworkResourceLoader&, WebCore::SWServerRegistration&);
+
 private:
     // Implement SWServer::Connection (Messages to the client WebProcess)

trunk/Tools/ChangeLog

@@ +1 @@
+2022-04-06  Youenn Fablet  <youenn@apple.com>
+
+        In case of COOP-based process swap, we need to make sure the document gets controlled by its matching service worker registration
+        https://bugs.webkit.org/show_bug.cgi?id=238800
+        <rdar://problem/91288849>
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
+
 2022-04-06  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm

@@ -3078 +3078 @@
 "    };"
 "}"
+""
+"function countServiceWorkerClients() {"
+"    worker.postMessage('countServiceWorkerClients');"
+"    navigator.serviceWorker.onmessage = (event) => {"
+"        alert(event.data);"
+"    };"
+"}"
 "</script>"_s;
 
@@ -3100 +3107 @@
 "       return;"
 "   }"
+"   if (event.data === 'countServiceWorkerClients') {"
+"       let currentClients = await self.clients.matchAll();"
+"       event.source.postMessage(currentClients.length + ' client(s)');"
+"       return;"
+"   }"
 "});"_s;
 
@@ -3180 +3192 @@
     [webView1 evaluateJavaScript:[NSString stringWithFormat:@"navigateOtherClientToURL('%@?swap')", baseURL] completionHandler: nil];
     EXPECT_WK_STREQ([webView1 _test_waitForAlert], "client");
+
+    [webView1 evaluateJavaScript:@"countServiceWorkerClients()" completionHandler: nil];
+    EXPECT_WK_STREQ([webView1 _test_waitForAlert], "1 client(s)");
 }