Reuse existing web processes for running service workers
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Oct 2019 07:54:00 +0000 (07:54 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Oct 2019 07:54:00 +0000 (07:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202195

Reviewed by Chris Dumez.

Source/WebCore:

Update implementation to be able to run service workers jointly with page.
Add internals API to get the process ID.
This allows writing tests to check whether a service worker is in the same process as its client or not.

Test: http/wpt/service-workers/service-worker-different-process.https.html

* testing/Internals.cpp:
(WebCore::Internals::processIdentifier const):
* testing/Internals.h:
* testing/Internals.idl:
* testing/ServiceWorkerInternals.cpp:
(WebCore::ServiceWorkerInternals::processIdentifier const):
* testing/ServiceWorkerInternals.h:
* testing/ServiceWorkerInternals.idl:
* workers/service/ServiceWorkerProvider.cpp:
(WebCore::ServiceWorkerProvider::registerServiceWorkerClients):
Do not register dummy documents whose sole purpose is to do loading for service workers.
* workers/service/context/SWContextManager.cpp:
(WebCore::SWContextManager::setConnection):
Now that connections might be created more than once on a given process,
Make sure that the replaced connection is stopped or there is no replaced connection.
(WebCore::SWContextManager::stopAllServiceWorkers):
Add routine to stop all service workers running in a given web process.
* workers/service/context/SWContextManager.h:
(WebCore::SWContextManager::Connection::isClosed const):
(WebCore::SWContextManager::Connection::setAsClosed):

Source/WebKit:

When network process asks for a service worker context connection,
we now iterate through existing web processes and reuse one if both session
and registrable domain match.
We then ask the web process to create a context connection to the network process.

When network process no longer needs the connection, it instructs the UIProcess
that will update its state so that the web process is no longer considered as running
service workers.
UIProcess then instructs the web process to stop its service workers and its connection.

Later on, the same web process may be reused for running service workers in which case
a new connection will replace the stopped connection.

Similarly, on network process crash, all web process running service workers are updated
so that they are no longer considered as running service workers.

Add a boolean state to WebProcessPool to control whether creating a separate service worker process.

We no longer terminate the web process when stopping service workers or when network process crash.
We use the enableTermination/disableTermination at context connection start/stop time.
We consider that the context connection is similar to running a page in the process and creating/removing a page
calls disableTermination/enableTermination.

NetworkProcess is handling the management of service worker processes by checking for clients.
In case there is no client, the process is terminated.
This removes the need for the WebProcessPool service worker process timer.
This patch removes this timer.

* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
(WebKit::NetworkConnectionToWebProcess::establishSWContextConnection):
(WebKit::NetworkConnectionToWebProcess::closeSWContextConnection):
(WebKit::NetworkConnectionToWebProcess::serverToContextConnectionNoLongerNeeded):
* NetworkProcess/NetworkConnectionToWebProcess.h:
* NetworkProcess/NetworkConnectionToWebProcess.messages.in:
* NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:
* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
(WebKit::WebSWServerToContextConnection::WebSWServerToContextConnection):
(WebKit::WebSWServerToContextConnection::~WebSWServerToContextConnection):
(WebKit::WebSWServerToContextConnection::messageSenderConnection const):
(WebKit::WebSWServerToContextConnection::connectionIsNoLongerNeeded):
(WebKit::WebSWServerToContextConnection::startFetch):
* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
* UIProcess/API/C/WKContext.cpp:
(WKContextSetUseSeparateServiceWorkerProcess):
* UIProcess/API/C/WKContextPrivate.h:
* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _setUseSeparateServiceWorkerProcess:]):
(-[WKProcessPool _webPageContentProcessCount]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::workerContextConnectionNoLongerNeeded):
* UIProcess/Network/NetworkProcessProxy.h:
* UIProcess/Network/NetworkProcessProxy.messages.in:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::networkProcessCrashed):
(WebKit::WebProcessPool::establishWorkerContextConnectionToNetworkProcess):
(WebKit::WebProcessPool::removeFromServiceWorkerProcesses):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::createWebPage):
(WebKit::WebProcessPool::terminateNetworkProcess):
(WebKit::WebProcessPool::setUseSeparateServiceWorkerProcess):
* UIProcess/WebProcessPool.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::createForServiceWorkers):
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
(WebKit::WebProcessProxy::didStartProvisionalLoadForMainFrame):
(WebKit::WebProcessProxy::disableServiceWorkers):
(WebKit::WebProcessProxy::enableServiceWorkers):
* UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::isMatchingRegistrableDomain const):
* WebProcess/Storage/WebSWContextManagerConnection.cpp:
(WebKit::m_userAgent):
(WebKit::WebSWContextManagerConnection::terminateWorker):
(WebKit::WebSWContextManagerConnection::syncTerminateWorker):
(WebKit::WebSWContextManagerConnection::close):
* WebProcess/Storage/WebSWContextManagerConnection.h:
* WebProcess/Storage/WebSWContextManagerConnection.messages.in:
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::networkProcessConnectionClosed):
(WebKit::WebProcess::registerServiceWorkerClients):

Tools:

Add support for enforcing a separate process for service workers.
This is useful for tests trying to crash the service worker process.

* TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
Update test to use serviceWorkerProcessCount.
Add test to check for in process and out of process service workers.
* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::setUseSeparateServiceWorkerProcess):
* WebKitTestRunner/InjectedBundle/TestRunner.h:
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetStateToConsistentValues):
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

LayoutTests:

* http/tests/workers/service/resources/postmessage-after-sw-process-crash.js:
(async.doTest):
* http/tests/workers/service/resources/postmessage-after-terminate.js:
(async.doTest):
* http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js:
(async.doTest):
* http/wpt/service-workers/online.https.html:
* http/wpt/service-workers/service-worker-different-process.https-expected.txt: Added.
* http/wpt/service-workers/service-worker-different-process.https.html: Added.
* http/wpt/service-workers/service-worker-process-worker.js: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@251067 268f45cc-cd09-0410-ab3c-d52691b4dbfc

47 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash.js
LayoutTests/http/tests/workers/service/resources/postmessage-after-terminate.js
LayoutTests/http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js
LayoutTests/http/wpt/service-workers/online.https.html
LayoutTests/http/wpt/service-workers/service-worker-different-process.https-expected.txt [new file with mode: 0644]
LayoutTests/http/wpt/service-workers/service-worker-different-process.https.html [new file with mode: 0644]
LayoutTests/http/wpt/service-workers/service-worker-process-worker.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebCore/testing/ServiceWorkerInternals.cpp
Source/WebCore/testing/ServiceWorkerInternals.h
Source/WebCore/testing/ServiceWorkerInternals.idl
Source/WebCore/workers/service/ServiceWorkerProvider.cpp
Source/WebCore/workers/service/context/SWContextManager.cpp
Source/WebCore/workers/service/context/SWContextManager.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp
Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h
Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in
Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h
Source/WebKit/UIProcess/API/C/WKContext.cpp
Source/WebKit/UIProcess/API/C/WKContextPrivate.h
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/WebProcessProxy.cpp
Source/WebKit/UIProcess/WebProcessProxy.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in
Source/WebKit/WebProcess/WebProcess.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestInvocation.cpp

index c2b5796..c3ff76e 100644 (file)
@@ -1,3 +1,21 @@
+2019-10-14  youenn fablet  <youenn@apple.com>
+
+        Reuse existing web processes for running service workers
+        https://bugs.webkit.org/show_bug.cgi?id=202195
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/workers/service/resources/postmessage-after-sw-process-crash.js:
+        (async.doTest):
+        * http/tests/workers/service/resources/postmessage-after-terminate.js:
+        (async.doTest):
+        * http/tests/workers/service/resources/postmessage-after-terminating-hung-worker.js:
+        (async.doTest):
+        * http/wpt/service-workers/online.https.html:
+        * http/wpt/service-workers/service-worker-different-process.https-expected.txt: Added.
+        * http/wpt/service-workers/service-worker-different-process.https.html: Added.
+        * http/wpt/service-workers/service-worker-process-worker.js: Added.
+
 2019-10-12  Ryosuke Niwa  <rniwa@webkit.org>
 
         [iOS] Crash in WebCore::DOMWindow::incrementScrollEventListenersCount
index 4f72164..1204cc6 100644 (file)
@@ -37,10 +37,20 @@ navigator.serviceWorker.addEventListener("message", function(event) {
     finishSWTest();
 });
 
-navigator.serviceWorker.register("resources/postmessage-after-sw-process-crash-worker.js", { }).then(function(registration) {
-    worker = registration.installing;
-    log("* Sending State to Service Worker");
-    worker.postMessage("SetState");
-    log("* Asking Service Worker if it received the state");
-    worker.postMessage("HasState");
-});
+async function doTest()
+{
+    if (window.testRunner) {
+        testRunner.setUseSeparateServiceWorkerProcess(true);
+        await fetch("").then(() => { }, () => { });
+    }
+
+    navigator.serviceWorker.register("resources/postmessage-after-sw-process-crash-worker.js", { }).then(function(registration) {
+        worker = registration.installing;
+        log("* Sending State to Service Worker");
+        worker.postMessage("SetState");
+        log("* Asking Service Worker if it received the state");
+        worker.postMessage("HasState");
+    });
+}
+
+doTest();
index 9d5f975..ac7cad7 100644 (file)
@@ -10,6 +10,15 @@ navigator.serviceWorker.addEventListener("message", function(event) {
         finishSWTest();
 });
 
-navigator.serviceWorker.register("resources/postmessage-echo-worker.js", { }).then(function(registration) {
+async function doTest()
+{
+    if (window.testRunner) {
+        testRunner.setUseSeparateServiceWorkerProcess(true);
+        await fetch("").then(() => { }, () => { });
+    }
+    const registration = await navigator.serviceWorker.register("resources/postmessage-echo-worker.js", { });
     registration.installing.postMessage("Message 1");
-});
+}
+
+doTest();
index e0f2e8f..86a431b 100644 (file)
@@ -23,7 +23,17 @@ navigator.serviceWorker.addEventListener("message", function(event) {
     }
 });
 
-navigator.serviceWorker.register("resources/postmessage-echo-worker-mayhang.js", { }).then(function(registration) {
+async function doTest()
+{
+    if (window.testRunner) {
+        testRunner.setUseSeparateServiceWorkerProcess(true);
+        await fetch("").then(() => { }, () => { });
+    }
+    const registration = await navigator.serviceWorker.register("resources/postmessage-echo-worker-mayhang.js", { });
     worker = registration.installing;
     worker.postMessage("HANG");
-});
+}
+
+doTest();
+
index fa58d7d..4b92b4b 100644 (file)
@@ -9,6 +9,11 @@ var scope = "";
 var activeWorker;
 
 promise_test(async (test) => {
+    if (window.testRunner) {
+        testRunner.setUseSeparateServiceWorkerProcess(true);
+        await fetch("").then(() => { }, () => { });
+    }
+
     var registration = await navigator.serviceWorker.register("online-worker.js", { scope : scope });
     activeWorker = registration.active;
     if (activeWorker)
diff --git a/LayoutTests/http/wpt/service-workers/service-worker-different-process.https-expected.txt b/LayoutTests/http/wpt/service-workers/service-worker-different-process.https-expected.txt
new file mode 100644 (file)
index 0000000..f19a788
--- /dev/null
@@ -0,0 +1,4 @@
+
+PASS Setup worker 
+PASS Test that service worker is in a different process 
+
diff --git a/LayoutTests/http/wpt/service-workers/service-worker-different-process.https.html b/LayoutTests/http/wpt/service-workers/service-worker-different-process.https.html
new file mode 100644 (file)
index 0000000..7832639
--- /dev/null
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+var activeWorker;
+
+promise_test(async (test) => {
+    if (window.testRunner) {
+        testRunner.setUseSeparateServiceWorkerProcess(true);
+        await fetch("").then(() => { }, () => { });
+    }
+
+    var registration = await navigator.serviceWorker.register("service-worker-process-worker.js", { scope : "" });
+    activeWorker = registration.active;
+    if (activeWorker)
+        return;
+    activeWorker = registration.installing;
+    return new Promise(resolve => {
+        activeWorker.addEventListener('statechange', () => {
+            if (activeWorker.state === "activated")
+                resolve();
+        });
+    });
+}, "Setup worker");
+
+promise_test(async (test) => {
+    let resolve;
+    const promise = new Promise(r => resolve = r);
+    navigator.serviceWorker.addEventListener("message", (e) => resolve(e.data), {once: true}); 
+    activeWorker.postMessage("PID");
+    assert_not_equals(await promise, window.internals.processIdentifier);
+}, "Test that service worker is in a different process");
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/wpt/service-workers/service-worker-process-worker.js b/LayoutTests/http/wpt/service-workers/service-worker-process-worker.js
new file mode 100644 (file)
index 0000000..6ae5770
--- /dev/null
@@ -0,0 +1,3 @@
+self.addEventListener("message", (event) => {
+    event.source.postMessage(self.internals ? internals.processIdentifier : "needs internal API");
+});
index b76d319..cb5ee37 100644 (file)
@@ -1,3 +1,37 @@
+2019-10-14  youenn fablet  <youenn@apple.com>
+
+        Reuse existing web processes for running service workers
+        https://bugs.webkit.org/show_bug.cgi?id=202195
+
+        Reviewed by Chris Dumez.
+
+        Update implementation to be able to run service workers jointly with page.
+        Add internals API to get the process ID.
+        This allows writing tests to check whether a service worker is in the same process as its client or not.
+
+        Test: http/wpt/service-workers/service-worker-different-process.https.html
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::processIdentifier const):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+        * testing/ServiceWorkerInternals.cpp:
+        (WebCore::ServiceWorkerInternals::processIdentifier const):
+        * testing/ServiceWorkerInternals.h:
+        * testing/ServiceWorkerInternals.idl:
+        * workers/service/ServiceWorkerProvider.cpp:
+        (WebCore::ServiceWorkerProvider::registerServiceWorkerClients):
+        Do not register dummy documents whose sole purpose is to do loading for service workers.
+        * workers/service/context/SWContextManager.cpp:
+        (WebCore::SWContextManager::setConnection):
+        Now that connections might be created more than once on a given process,
+        Make sure that the replaced connection is stopped or there is no replaced connection.
+        (WebCore::SWContextManager::stopAllServiceWorkers):
+        Add routine to stop all service workers running in a given web process.
+        * workers/service/context/SWContextManager.h:
+        (WebCore::SWContextManager::Connection::isClosed const):
+        (WebCore::SWContextManager::Connection::setAsClosed):
+
 2019-10-13  Tim Horton  <timothy_horton@apple.com>
 
         Stop 'using namespace *Names' in files generated by make_names.pl
index 28b2063..3a9e6c2 100644 (file)
 #include <wtf/Language.h>
 #include <wtf/MemoryPressureHandler.h>
 #include <wtf/MonotonicTime.h>
+#include <wtf/ProcessID.h>
 #include <wtf/URLHelpers.h>
 #include <wtf/text/StringBuilder.h>
 #include <wtf/text/StringConcatenateNumbers.h>
@@ -5251,4 +5252,9 @@ void Internals::setMaxCanvasPixelMemory(unsigned size)
     HTMLCanvasElement::setMaxPixelMemoryForTesting(size);
 }
 
+int Internals::processIdentifier() const
+{
+    return getCurrentProcessID();
+}
+
 } // namespace WebCore
index 7fca656..ba2bb97 100644 (file)
@@ -892,6 +892,8 @@ public:
     void setMockWebAuthenticationConfiguration(const MockWebAuthenticationConfiguration&);
 #endif
 
+    int processIdentifier() const;
+
 private:
     explicit Internals(Document&);
     Document* contextDocument() const;
index eafff68..1eee3c9 100644 (file)
@@ -723,6 +723,7 @@ enum CompositingPolicy {
 
     boolean isAnyWorkletGlobalScopeAlive();
 
+    readonly attribute long processIdentifier;
     DOMString serviceWorkerClientIdentifier(Document document);
     Promise<void> storeRegistrationsOnDisk();
 
index 5544561..e8d30b4 100644 (file)
@@ -31,6 +31,7 @@
 #include "FetchEvent.h"
 #include "JSFetchResponse.h"
 #include "SWContextManager.h"
+#include <wtf/ProcessID.h>
 
 namespace WebCore {
 
@@ -102,6 +103,11 @@ bool ServiceWorkerInternals::isThrottleable() const
     return connection ? connection->isThrottleable() : true;
 }
 
+int ServiceWorkerInternals::processIdentifier() const
+{
+    return getCurrentProcessID();
+}
+
 } // namespace WebCore
 
 #endif
index 9348a0c..6babeb2 100644 (file)
@@ -55,6 +55,8 @@ public:
 
     bool isThrottleable() const;
 
+    int processIdentifier() const;
+
 private:
     explicit ServiceWorkerInternals(ServiceWorkerIdentifier);
 
index 535dd80..9271ce4 100644 (file)
@@ -38,4 +38,6 @@
 
     readonly attribute DOMString processName;
     readonly attribute boolean isThrottleable;
+
+    readonly attribute long processIdentifier;
 };
index a713454..443a92c 100644 (file)
@@ -29,6 +29,9 @@
 #if ENABLE(SERVICE_WORKER)
 
 #include "Document.h"
+#include "Frame.h"
+#include "FrameLoader.h"
+#include "FrameLoaderClient.h"
 #include "LegacySchemeRegistry.h"
 #include "SWClientConnection.h"
 
@@ -64,6 +67,8 @@ void ServiceWorkerProvider::registerServiceWorkerClients()
 {
     setMayHaveRegisteredServiceWorkers();
     for (auto* document : Document::allDocuments()) {
+        if (!document->page() || document->page()->mainFrame().loader().client().isServiceWorkerFrameLoaderClient())
+            continue;
         if (LegacySchemeRegistry::canServiceWorkersHandleURLScheme(document->url().protocol().toStringWithoutCopying()))
             document->setServiceWorkerConnection(&serviceWorkerConnection());
     }
index 774f385..a6de7d2 100644 (file)
@@ -42,7 +42,7 @@ SWContextManager& SWContextManager::singleton()
 
 void SWContextManager::setConnection(std::unique_ptr<Connection>&& connection)
 {
-    ASSERT(!m_connection);
+    ASSERT(!m_connection || m_connection->isClosed());
     m_connection = WTFMove(connection);
 }
 
@@ -177,6 +177,16 @@ SWContextManager::ServiceWorkerTerminationRequest::ServiceWorkerTerminationReque
     m_timeoutTimer.startOneShot(timeout);
 }
 
+void SWContextManager::stopAllServiceWorkers()
+{
+    auto serviceWorkers = WTFMove(m_workerMap);
+    for (auto& serviceWorker : serviceWorkers.values()) {
+        serviceWorker->setAsTerminatingOrTerminated();
+        m_pendingServiceWorkerTerminationRequests.add(serviceWorker->identifier(), makeUnique<ServiceWorkerTerminationRequest>(*this, serviceWorker->identifier(), workerTerminationTimeout));
+        serviceWorker->thread().stop([] { });
+    }
+}
+
 } // namespace WebCore
 
 #endif
index 628c3f3..493463e 100644 (file)
@@ -65,6 +65,14 @@ public:
         virtual void claim(ServiceWorkerIdentifier, CompletionHandler<void()>&&) = 0;
 
         virtual bool isThrottleable() const = 0;
+
+        bool isClosed() const { return m_isClosed; }
+
+    protected:
+        void setAsClosed() { m_isClosed = true; }
+
+    private:
+        bool m_isClosed { false };
     };
 
     WEBCORE_EXPORT void setConnection(std::unique_ptr<Connection>&&);
@@ -87,6 +95,11 @@ public:
 
     ServiceWorkerThreadProxy* workerByID(ServiceWorkerIdentifier identifier) { return m_workerMap.get(identifier); }
 
+    WEBCORE_EXPORT void stopAllServiceWorkers();
+
+    static constexpr Seconds workerTerminationTimeout { 10_s };
+    static constexpr Seconds syncWorkerTerminationTimeout { 100_ms }; // Only used by layout tests.
+
 private:
     SWContextManager() = default;
 
index 3c5546b..8d6b0ae 100644 (file)
@@ -1,3 +1,92 @@
+2019-10-14  youenn fablet  <youenn@apple.com>
+
+        Reuse existing web processes for running service workers
+        https://bugs.webkit.org/show_bug.cgi?id=202195
+
+        Reviewed by Chris Dumez.
+
+        When network process asks for a service worker context connection,
+        we now iterate through existing web processes and reuse one if both session
+        and registrable domain match.
+        We then ask the web process to create a context connection to the network process.
+
+        When network process no longer needs the connection, it instructs the UIProcess
+        that will update its state so that the web process is no longer considered as running
+        service workers.
+        UIProcess then instructs the web process to stop its service workers and its connection.
+
+        Later on, the same web process may be reused for running service workers in which case
+        a new connection will replace the stopped connection.
+
+        Similarly, on network process crash, all web process running service workers are updated
+        so that they are no longer considered as running service workers.
+
+        Add a boolean state to WebProcessPool to control whether creating a separate service worker process.
+
+        We no longer terminate the web process when stopping service workers or when network process crash.
+        We use the enableTermination/disableTermination at context connection start/stop time.
+        We consider that the context connection is similar to running a page in the process and creating/removing a page
+        calls disableTermination/enableTermination.
+
+        NetworkProcess is handling the management of service worker processes by checking for clients.
+        In case there is no client, the process is terminated.
+        This removes the need for the WebProcessPool service worker process timer.
+        This patch removes this timer.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
+        (WebKit::NetworkConnectionToWebProcess::establishSWContextConnection):
+        (WebKit::NetworkConnectionToWebProcess::closeSWContextConnection):
+        (WebKit::NetworkConnectionToWebProcess::serverToContextConnectionNoLongerNeeded):
+        * NetworkProcess/NetworkConnectionToWebProcess.h:
+        * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
+        * NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
+        (WebKit::WebSWServerToContextConnection::WebSWServerToContextConnection):
+        (WebKit::WebSWServerToContextConnection::~WebSWServerToContextConnection):
+        (WebKit::WebSWServerToContextConnection::messageSenderConnection const):
+        (WebKit::WebSWServerToContextConnection::connectionIsNoLongerNeeded):
+        (WebKit::WebSWServerToContextConnection::startFetch):
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
+        * UIProcess/API/C/WKContext.cpp:
+        (WKContextSetUseSeparateServiceWorkerProcess):
+        * UIProcess/API/C/WKContextPrivate.h:
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _setUseSeparateServiceWorkerProcess:]):
+        (-[WKProcessPool _webPageContentProcessCount]):
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::workerContextConnectionNoLongerNeeded):
+        * UIProcess/Network/NetworkProcessProxy.h:
+        * UIProcess/Network/NetworkProcessProxy.messages.in:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::networkProcessCrashed):
+        (WebKit::WebProcessPool::establishWorkerContextConnectionToNetworkProcess):
+        (WebKit::WebProcessPool::removeFromServiceWorkerProcesses):
+        (WebKit::WebProcessPool::disconnectProcess):
+        (WebKit::WebProcessPool::createWebPage):
+        (WebKit::WebProcessPool::terminateNetworkProcess):
+        (WebKit::WebProcessPool::setUseSeparateServiceWorkerProcess):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::createForServiceWorkers):
+        (WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
+        (WebKit::WebProcessProxy::didStartProvisionalLoadForMainFrame):
+        (WebKit::WebProcessProxy::disableServiceWorkers):
+        (WebKit::WebProcessProxy::enableServiceWorkers):
+        * UIProcess/WebProcessProxy.h:
+        (WebKit::WebProcessProxy::isMatchingRegistrableDomain const):
+        * WebProcess/Storage/WebSWContextManagerConnection.cpp:
+        (WebKit::m_userAgent):
+        (WebKit::WebSWContextManagerConnection::terminateWorker):
+        (WebKit::WebSWContextManagerConnection::syncTerminateWorker):
+        (WebKit::WebSWContextManagerConnection::close):
+        * WebProcess/Storage/WebSWContextManagerConnection.h:
+        * WebProcess/Storage/WebSWContextManagerConnection.messages.in:
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::networkProcessConnectionClosed):
+        (WebKit::WebProcess::registerServiceWorkerClients):
+
 2019-10-12  Chris Dumez  <cdumez@apple.com>
 
         Back/Forward cache does not work after doing a favorite navigation
index 6218a6e..247c71a 100644 (file)
@@ -74,6 +74,9 @@
 #include "WebPaymentCoordinatorProxyMessages.h"
 #endif
 
+#undef RELEASE_LOG_IF_ALLOWED
+#define RELEASE_LOG_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_IF(m_sessionID.isAlwaysOnLoggingAllowed(), channel, "%p - NetworkConnectionToWebProcess::" fmt, this, ##__VA_ARGS__)
+
 namespace WebKit {
 using namespace WebCore;
 
@@ -219,19 +222,15 @@ void NetworkConnectionToWebProcess::didReceiveMessage(IPC::Connection& connectio
         return;
     }
     if (decoder.messageReceiverName() == Messages::WebSWServerToContextConnection::messageReceiverName()) {
-        ASSERT(m_swContextConnection);
-        if (m_swContextConnection) {
+        if (m_swContextConnection)
             m_swContextConnection->didReceiveMessage(connection, decoder);
-            return;
-        }
+        return;
     }
 
     if (decoder.messageReceiverName() == Messages::ServiceWorkerFetchTask::messageReceiverName()) {
-        ASSERT(m_swContextConnection);
-        if (m_swContextConnection) {
+        if (m_swContextConnection)
             m_swContextConnection->didReceiveFetchTaskMessage(connection, decoder);
-            return;
-        }
+        return;
     }
 #endif
 
@@ -887,7 +886,20 @@ void NetworkConnectionToWebProcess::establishSWServerConnection()
 void NetworkConnectionToWebProcess::establishSWContextConnection(RegistrableDomain&& registrableDomain)
 {
     if (auto* server = m_networkProcess->swServerForSessionIfExists(m_sessionID))
-        m_swContextConnection = makeUnique<WebSWServerToContextConnection>(m_networkProcess, WTFMove(registrableDomain), *server, m_connection.get());
+        m_swContextConnection = makeUnique<WebSWServerToContextConnection>(*this, WTFMove(registrableDomain), *server);
+}
+
+void NetworkConnectionToWebProcess::closeSWContextConnection()
+{
+    m_swContextConnection = nullptr;
+}
+
+void NetworkConnectionToWebProcess::serverToContextConnectionNoLongerNeeded()
+{
+    RELEASE_LOG_IF_ALLOWED(ServiceWorker, "serverToContextConnectionNoLongerNeeded - WebProcess %llu no longer useful for running service workers", webProcessIdentifier().toUInt64());
+    m_networkProcess->parentProcessConnection()->send(Messages::NetworkProcessProxy::WorkerContextConnectionNoLongerNeeded { webProcessIdentifier() }, 0);
+
+    m_swContextConnection = nullptr;
 }
 #endif
 
index 7d8ba32..64e6d71 100644 (file)
@@ -155,6 +155,10 @@ public:
 
     void checkProcessLocalPortForActivity(const WebCore::MessagePortIdentifier&, CompletionHandler<void(WebCore::MessagePortChannelProvider::HasActivity)>&&);
 
+#if ENABLE(SERVICE_WORKER)
+    void serverToContextConnectionNoLongerNeeded();
+#endif
+
 private:
     NetworkConnectionToWebProcess(NetworkProcess&, WebCore::ProcessIdentifier, PAL::SessionID, IPC::Connection::Identifier);
 
@@ -214,6 +218,7 @@ private:
 #if ENABLE(SERVICE_WORKER)
     void establishSWServerConnection();
     void establishSWContextConnection(WebCore::RegistrableDomain&&);
+    void closeSWContextConnection();
     void unregisterSWConnection();
 #endif
 
index 2ad313a..3bcd0eb 100644 (file)
@@ -81,6 +81,7 @@ messages -> NetworkConnectionToWebProcess LegacyReceiver {
 #if ENABLE(SERVICE_WORKER)
     EstablishSWServerConnection()
     EstablishSWContextConnection(WebCore::RegistrableDomain domain)
+    CloseSWContextConnection()
 #endif
 
     CreateNewMessagePortChannel(struct WebCore::MessagePortIdentifier port1, struct WebCore::MessagePortIdentifier port2)
index 19326f2..d629660 100644 (file)
@@ -34,6 +34,8 @@
 #include <wtf/NeverDestroyed.h>
 #include <wtf/RunLoop.h>
 
+#undef RELEASE_LOG_IF_ALLOWED
+#undef RELEASE_LOG_ERROR_IF_ALLOWED
 #define RELEASE_LOG_IF_ALLOWED(sessionID, fmt, ...) RELEASE_LOG_IF(sessionID.isAlwaysOnLoggingAllowed(), Network, "%p - NetworkHTTPSUpgradeChecker::" fmt, this, ##__VA_ARGS__)
 #define RELEASE_LOG_ERROR_IF_ALLOWED(sessionID, fmt, ...) RELEASE_LOG_ERROR_IF(sessionID.isAlwaysOnLoggingAllowed(), Network, "%p - NetworkHTTPSUpgradeChecker::" fmt, this, ##__VA_ARGS__)
 
index 85d9e95..6c14e6d 100644 (file)
@@ -30,6 +30,7 @@
 
 #include "FormDataReference.h"
 #include "Logging.h"
+#include "NetworkConnectionToWebProcess.h"
 #include "NetworkProcess.h"
 #include "ServiceWorkerFetchTask.h"
 #include "ServiceWorkerFetchTaskMessages.h"
 namespace WebKit {
 using namespace WebCore;
 
-WebSWServerToContextConnection::WebSWServerToContextConnection(NetworkProcess& networkProcess, RegistrableDomain&& registrableDomain, SWServer& server, Ref<IPC::Connection>&& connection)
+WebSWServerToContextConnection::WebSWServerToContextConnection(NetworkConnectionToWebProcess& connection, RegistrableDomain&& registrableDomain, SWServer& server)
     : SWServerToContextConnection(WTFMove(registrableDomain))
-    , m_ipcConnection(WTFMove(connection))
-    , m_networkProcess(networkProcess)
+    , m_connection(connection)
     , m_server(makeWeakPtr(server))
 {
     server.addContextConnection(*this);
@@ -53,14 +53,17 @@ WebSWServerToContextConnection::WebSWServerToContextConnection(NetworkProcess& n
 
 WebSWServerToContextConnection::~WebSWServerToContextConnection()
 {
-    connectionClosed();
+    auto fetches = WTFMove(m_ongoingFetches);
+    for (auto& fetch : fetches.values())
+        fetch->fail(ResourceError { errorDomainWebKitInternal, 0, { }, "Service Worker context closed"_s });
+
     if (m_server && m_server->contextConnectionForRegistrableDomain(registrableDomain()) == this)
         m_server->removeContextConnection(*this);
 }
 
 IPC::Connection* WebSWServerToContextConnection::messageSenderConnection() const
 {
-    return m_ipcConnection.ptr();
+    return &m_connection.connection();
 }
 
 uint64_t WebSWServerToContextConnection::messageSenderDestinationID() const
@@ -68,13 +71,6 @@ uint64_t WebSWServerToContextConnection::messageSenderDestinationID() const
     return 0;
 }
 
-void WebSWServerToContextConnection::connectionClosed()
-{
-    auto fetches = WTFMove(m_ongoingFetches);
-    for (auto& fetch : fetches.values())
-        fetch->fail(ResourceError { errorDomainWebKitInternal, 0, { }, "Service Worker context closed"_s });
-}
-
 void WebSWServerToContextConnection::postMessageToServiceWorkerClient(const ServiceWorkerClientIdentifier& destinationIdentifier, const MessageWithMessagePorts& message, ServiceWorkerIdentifier sourceIdentifier, const String& sourceOrigin)
 {
     if (!m_server)
@@ -136,9 +132,7 @@ void WebSWServerToContextConnection::didFinishSkipWaiting(uint64_t callbackID)
 
 void WebSWServerToContextConnection::connectionIsNoLongerNeeded()
 {
-    RELEASE_LOG(ServiceWorker, "Service worker process is no longer needed, terminating it");
-    terminate();
-    connectionClosed();
+    m_connection.serverToContextConnectionNoLongerNeeded();
 }
 
 void WebSWServerToContextConnection::setThrottleState(bool isThrottleable)
@@ -147,17 +141,12 @@ void WebSWServerToContextConnection::setThrottleState(bool isThrottleable)
     send(Messages::WebSWContextManagerConnection::SetThrottleState { isThrottleable });
 }
 
-void WebSWServerToContextConnection::terminate()
-{
-    send(Messages::WebSWContextManagerConnection::TerminateProcess());
-}
-
 void WebSWServerToContextConnection::startFetch(PAL::SessionID sessionID, WebSWServerConnection& contentConnection, FetchIdentifier contentFetchIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, const ResourceRequest& request, const FetchOptions& options, const IPC::FormDataReference& data, const String& referrer)
 {
     auto serverConnectionIdentifier = contentConnection.identifier();
     auto fetchIdentifier = FetchIdentifier::generate();
 
-    auto result = m_ongoingFetches.add(fetchIdentifier, makeUnique<ServiceWorkerFetchTask>(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_networkProcess->serviceWorkerFetchTimeout()));
+    auto result = m_ongoingFetches.add(fetchIdentifier, makeUnique<ServiceWorkerFetchTask>(sessionID, contentConnection, *this, contentFetchIdentifier, serviceWorkerIdentifier, m_connection.networkProcess().serviceWorkerFetchTimeout()));
 
     ASSERT(!m_ongoingFetchIdentifiers.contains({ serverConnectionIdentifier, contentFetchIdentifier }));
     m_ongoingFetchIdentifiers.add({ serverConnectionIdentifier, contentFetchIdentifier }, fetchIdentifier);
index e30f341..dd9531d 100644 (file)
@@ -49,21 +49,17 @@ class SessionID;
 
 namespace WebKit {
 
-class NetworkProcess;
+class NetworkConnectionToWebProcess;
 class WebSWServerConnection;
 
 class WebSWServerToContextConnection : public WebCore::SWServerToContextConnection, public IPC::MessageSender, public IPC::MessageReceiver {
 public:
-    WebSWServerToContextConnection(NetworkProcess&, WebCore::RegistrableDomain&&, WebCore::SWServer&, Ref<IPC::Connection>&&);
+    WebSWServerToContextConnection(NetworkConnectionToWebProcess&, WebCore::RegistrableDomain&&, WebCore::SWServer&);
     ~WebSWServerToContextConnection();
 
-    IPC::Connection* ipcConnection() const { return m_ipcConnection.ptr(); }
-
     // IPC::MessageReceiver
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
 
-    void terminate();
-
     void startFetch(PAL::SessionID, WebSWServerConnection&, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier, const WebCore::ResourceRequest&, const WebCore::FetchOptions&, const IPC::FormDataReference&, const String&);
     void cancelFetch(WebCore::SWServerConnectionIdentifier, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier);
     void continueDidReceiveFetchResponse(WebCore::SWServerConnectionIdentifier, WebCore::FetchIdentifier, WebCore::ServiceWorkerIdentifier);
@@ -98,8 +94,7 @@ private:
 
     void connectionClosed();
 
-    Ref<IPC::Connection> m_ipcConnection;
-    Ref<NetworkProcess> m_networkProcess;
+    NetworkConnectionToWebProcess& m_connection;
     WeakPtr<WebCore::SWServer> m_server;
 
     HashMap<ServiceWorkerFetchTask::Identifier, WebCore::FetchIdentifier> m_ongoingFetchIdentifiers;
index 5e3cede..faf8a52 100644 (file)
@@ -667,3 +667,8 @@ void WKContextClearLegacyPrivateBrowsingLocalStorage(WKContextRef contextRef, vo
             callback(context);
     });
 }
+
+void WKContextSetUseSeparateServiceWorkerProcess(WKContextRef contextRef, bool useSeparateServiceWorkerProcess)
+{
+    WebKit::toImpl(contextRef)->setUseSeparateServiceWorkerProcess(useSeparateServiceWorkerProcess);
+}
index 92495ca..723b5e4 100644 (file)
@@ -124,6 +124,8 @@ WK_EXPORT void WKContextSyncLocalStorage(WKContextRef contextRef, void* context,
 typedef void (*WKContextClearLegacyPrivateBrowsingLocalStorageCallback)(void* functionContext);
 WK_EXPORT void WKContextClearLegacyPrivateBrowsingLocalStorage(WKContextRef contextRef, void* context, WKContextClearLegacyPrivateBrowsingLocalStorageCallback callback);
 
+WK_EXPORT void WKContextSetUseSeparateServiceWorkerProcess(WKContextRef context, bool forceServiceWorkerProcess);
+
 #ifdef __cplusplus
 }
 #endif
index 607f20f..0b810b5 100644 (file)
@@ -448,6 +448,11 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     _processPool->terminateServiceWorkerProcesses();
 }
 
+- (void)_setUseSeparateServiceWorkerProcess:(BOOL)useSeparateServiceWorkerProcess
+{
+    _processPool->setUseSeparateServiceWorkerProcess(useSeparateServiceWorkerProcess);
+}
+
 - (pid_t)_networkProcessIdentifier
 {
     return _processPool->networkProcessIdentifier();
@@ -504,12 +509,12 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
 
 - (size_t)_webPageContentProcessCount
 {
-    auto allWebProcesses = _processPool->processes();
+    auto result = _processPool->processes().size();
 #if ENABLE(SERVICE_WORKER)
-    return allWebProcesses.size() - _processPool->serviceWorkerProxiesCount();
-#else
-    return allWebProcesses.size();
+    if (_processPool->useSeparateServiceWorkerProcess())
+        result -= _processPool->serviceWorkerProxiesCount();
 #endif
+    return result;
 }
 
 - (void)_preconnectToServer:(NSURL *)serverURL
index ef8bb56..3a81d59 100644 (file)
 - (NSUInteger)_maximumSuspendedPageCount WK_API_AVAILABLE(macos(10.14.4), ios(12.2));
 - (NSUInteger)_processCacheCapacity WK_API_AVAILABLE(macos(10.14.4), ios(12.2));
 - (NSUInteger)_processCacheSize WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (void)_setUseSeparateServiceWorkerProcess:(BOOL)forceServiceWorkerProcess WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 // Test only. Returns web processes running web pages (does not include web processes running service workers)
 - (size_t)_webPageContentProcessCount WK_API_AVAILABLE(macos(10.13.4), ios(11.3));
index 6e30a07..ca1f5ab 100644 (file)
@@ -1241,6 +1241,12 @@ void NetworkProcessProxy::establishWorkerContextConnectionToNetworkProcess(Regis
 {
     m_processPool.establishWorkerContextConnectionToNetworkProcess(*this, WTFMove(registrableDomain), sessionID);
 }
+
+void NetworkProcessProxy::workerContextConnectionNoLongerNeeded(WebCore::ProcessIdentifier identifier)
+{
+    if (auto* process = WebProcessProxy::processForIdentifier(identifier))
+        process->disableServiceWorkers();
+}
 #endif
 
 void NetworkProcessProxy::requestStorageSpace(PAL::SessionID sessionID, const WebCore::ClientOrigin& origin, uint64_t currentQuota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t> quota)>&& completionHandler)
index e65aaf4..fbf2ce5 100644 (file)
@@ -246,6 +246,7 @@ private:
 
 #if ENABLE(SERVICE_WORKER)
     void establishWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain&&, PAL::SessionID);
+    void workerContextConnectionNoLongerNeeded(WebCore::ProcessIdentifier);
 #endif
 
     void requestStorageSpace(PAL::SessionID, const WebCore::ClientOrigin&, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t> quota)>&&);
index 3ab0db0..f5fb565 100644 (file)
@@ -62,6 +62,7 @@ messages -> NetworkProcessProxy LegacyReceiver {
 
 #if ENABLE(SERVICE_WORKER)
     EstablishWorkerContextConnectionToNetworkProcess(WebCore::RegistrableDomain registrableDomain, PAL::SessionID sessionID)
+    WorkerContextConnectionNoLongerNeeded(WebCore::ProcessIdentifier identifier)
 #endif
 
     RequestStorageSpace(PAL::SessionID sessionID, struct WebCore::ClientOrigin origin, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired) -> (Optional<uint64_t> newQuota) Async
index b72d3d4..e360081 100644 (file)
@@ -671,6 +671,11 @@ void WebProcessPool::networkProcessCrashed(NetworkProcessProxy& networkProcessPr
     auto& newNetworkProcess = ensureNetworkProcess();
     for (auto& reply : pendingReplies)
         newNetworkProcess.getNetworkProcessConnection(*reply.first, WTFMove(reply.second));
+
+#if ENABLE(SERVICE_WORKER)
+    while (!m_serviceWorkerProcesses.isEmpty())
+        m_serviceWorkerProcesses.begin()->value->disableServiceWorkers();
+#endif
 }
 
 void WebProcessPool::getNetworkProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply&& reply)
@@ -697,24 +702,57 @@ void WebProcessPool::establishWorkerContextConnectionToNetworkProcess(NetworkPro
     }
 
     RegistrableDomainWithSessionID registrableDomainWithSessionID { RegistrableDomain { registrableDomain }, sessionID };
-    if (m_serviceWorkerProcesses.contains(registrableDomainWithSessionID))
+    if (m_serviceWorkerProcesses.contains(registrableDomainWithSessionID)) {
+        RELEASE_LOG_IF(sessionID.isAlwaysOnLoggingAllowed(), ServiceWorker, "WebProcessPool::establishWorkerContextConnectionToNetworkProcess process already created");
         return;
+    }
 
     if (m_serviceWorkerProcesses.isEmpty())
         sendToAllProcesses(Messages::WebProcess::RegisterServiceWorkerClients { });
 
-    auto serviceWorkerProcessProxy = WebProcessProxy::createForServiceWorkers(*this, WTFMove(registrableDomain), *websiteDataStore);
-    m_serviceWorkerProcesses.add(WTFMove(registrableDomainWithSessionID), serviceWorkerProcessProxy.ptr());
+    WebProcessProxy* serviceWorkerProcessProxy { nullptr };
+    if (!m_useSeparateServiceWorkerProcess) {
+        for (auto& process : m_processes) {
+            if (process == m_prewarmedProcess || process == m_dummyProcessProxy)
+                continue;
+            if (&process->websiteDataStore() != websiteDataStore)
+                continue;
+            if (!process->isMatchingRegistrableDomain(registrableDomain))
+                continue;
+
+            serviceWorkerProcessProxy = process.get();
+            serviceWorkerProcessProxy->enableServiceWorkers();
+
+            RELEASE_LOG_IF(sessionID.isAlwaysOnLoggingAllowed(), ServiceWorker, "WebProcessPool::establishWorkerContextConnectionToNetworkProcess reusing an existing web process %p, process identifier %d", serviceWorkerProcessProxy, serviceWorkerProcessProxy->processIdentifier());
+            break;
+        }
+    }
+
+    if (!serviceWorkerProcessProxy) {
+        auto newProcessProxy = WebProcessProxy::createForServiceWorkers(*this, WTFMove(registrableDomain), *websiteDataStore);
+        serviceWorkerProcessProxy = newProcessProxy.ptr();
 
-    updateProcessAssertions();
-    initializeNewWebProcess(serviceWorkerProcessProxy, websiteDataStore);
+        RELEASE_LOG_IF(sessionID.isAlwaysOnLoggingAllowed(), ServiceWorker, "WebProcessPool::establishWorkerContextConnectionToNetworkProcess creating a new service worker process %p, process identifier %d", serviceWorkerProcessProxy, serviceWorkerProcessProxy->processIdentifier());
 
-    auto* serviceWorkerProcessProxyPtr = serviceWorkerProcessProxy.ptr();
-    m_processes.append(WTFMove(serviceWorkerProcessProxy));
+        updateProcessAssertions();
+        initializeNewWebProcess(newProcessProxy, websiteDataStore);
+        m_processes.append(WTFMove(newProcessProxy));
+    }
 
-    serviceWorkerProcessProxyPtr->establishServiceWorkerContext(m_serviceWorkerPreferences ? m_serviceWorkerPreferences.value() : m_defaultPageGroup->preferences().store());
+    ASSERT(!m_serviceWorkerProcesses.contains(registrableDomainWithSessionID));
+    m_serviceWorkerProcesses.add(WTFMove(registrableDomainWithSessionID), serviceWorkerProcessProxy);
+
+    serviceWorkerProcessProxy->establishServiceWorkerContext(m_serviceWorkerPreferences ? m_serviceWorkerPreferences.value() : m_defaultPageGroup->preferences().store());
     if (!m_serviceWorkerUserAgent.isNull())
-        serviceWorkerProcessProxyPtr->setServiceWorkerUserAgent(m_serviceWorkerUserAgent);
+        serviceWorkerProcessProxy->setServiceWorkerUserAgent(m_serviceWorkerUserAgent);
+}
+
+void WebProcessPool::removeFromServiceWorkerProcesses(WebProcessProxy& process)
+{
+    RegistrableDomainWithSessionID key { process.registrableDomain(), process.websiteDataStore().sessionID() };
+    ASSERT(m_serviceWorkerProcesses.contains(key));
+    ASSERT(m_serviceWorkerProcesses.get(key) == &process);
+    m_serviceWorkerProcesses.remove(key);
 }
 #endif
 
@@ -1096,8 +1134,9 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
 
 #if ENABLE(SERVICE_WORKER)
     if (process->isRunningServiceWorkers()) {
-        auto* removedProcess = m_serviceWorkerProcesses.take(RegistrableDomainWithSessionID { process->registrableDomain(), process->websiteDataStore().sessionID() });
-        ASSERT_UNUSED(removedProcess, removedProcess == process);
+        auto iterator = m_serviceWorkerProcesses.find(RegistrableDomainWithSessionID { process->registrableDomain(), process->websiteDataStore().sessionID() });
+        if (iterator != m_serviceWorkerProcesses.end() && iterator->value == process)
+            m_serviceWorkerProcesses.remove(iterator);
         updateProcessAssertions();
     }
 #endif
@@ -1112,15 +1151,6 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
 #endif
 
     removeProcessFromOriginCacheSet(*process);
-
-#if ENABLE(SERVICE_WORKER)
-    // FIXME: We should do better than this. For now, we just destroy the ServiceWorker process
-    // whenever there is no regular WebContent process remaining.
-    if (m_processes.size() == m_serviceWorkerProcesses.size()) {
-        if (!m_serviceWorkerProcessesTerminationTimer.isActive())
-            m_serviceWorkerProcessesTerminationTimer.startOneShot(serviceWorkerTerminationDelay);
-    }
-#endif
 }
 
 WebProcessProxy& WebProcessPool::processForRegistrableDomain(WebsiteDataStore& websiteDataStore, WebPageProxy* page, const RegistrableDomain& registrableDomain)
@@ -1210,8 +1240,6 @@ Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API:
     auto page = process->createWebPage(pageClient, WTFMove(pageConfiguration));
 
 #if ENABLE(SERVICE_WORKER)
-    ASSERT(!process->isRunningServiceWorkers());
-
     if (!m_serviceWorkerPreferences) {
         m_serviceWorkerPreferences = page->preferencesStore();
         for (auto* serviceWorkerProcess : m_serviceWorkerProcesses.values())
@@ -1691,6 +1719,11 @@ void WebProcessPool::clearCachedCredentials()
 
 void WebProcessPool::terminateNetworkProcess()
 {
+#if ENABLE(SERVICE_WORKER)
+    while (!m_serviceWorkerProcesses.isEmpty())
+        m_serviceWorkerProcesses.begin()->value->disableServiceWorkers();
+#endif
+
     if (!m_networkProcess)
         return;
     
@@ -2421,4 +2454,19 @@ void WebProcessPool::clearWebProcessIsPlayingAudibleMedia(WebCore::ProcessIdenti
     }
 }
 
+void WebProcessPool::setUseSeparateServiceWorkerProcess(bool useSeparateServiceWorkerProcess)
+{
+    if (m_useSeparateServiceWorkerProcess == useSeparateServiceWorkerProcess)
+        return;
+
+    RELEASE_LOG(ServiceWorker, "WebProcessPool::setUseSeparateServiceWorkerProcess %d", useSeparateServiceWorkerProcess);
+
+    m_useSeparateServiceWorkerProcess = useSeparateServiceWorkerProcess;
+#if ENABLE(SERVICE_WORKER)
+    while (!m_serviceWorkerProcesses.isEmpty())
+        m_serviceWorkerProcesses.begin()->value->disableServiceWorkers();
+#endif
+    terminateNetworkProcess();
+}
+
 } // namespace WebKit
index 60ef493..9e1af72 100644 (file)
@@ -387,6 +387,7 @@ public:
     bool isServiceWorkerPageID(WebPageProxyIdentifier) const;
 #if ENABLE(SERVICE_WORKER)
     void establishWorkerContextConnectionToNetworkProcess(NetworkProcessProxy&, WebCore::RegistrableDomain&&, PAL::SessionID);
+    void removeFromServiceWorkerProcesses(WebProcessProxy&);
     size_t serviceWorkerProxiesCount() const { return m_serviceWorkerProcesses.size(); }
     void setAllowsAnySSLCertificateForServiceWorker(bool allows) { m_allowsAnySSLCertificateForServiceWorker = allows; }
     bool allowsAnySSLCertificateForServiceWorker() const { return m_allowsAnySSLCertificateForServiceWorker; }
@@ -519,6 +520,9 @@ public:
     
     PlugInAutoStartProvider& plugInAutoStartProvider() { return m_plugInAutoStartProvider; }
 
+    void setUseSeparateServiceWorkerProcess(bool);
+    bool useSeparateServiceWorkerProcess() const { return m_useSeparateServiceWorkerProcess; }
+
 private:
     void platformInitialize();
 
@@ -793,6 +797,7 @@ private:
 #else
     bool m_isDelayedWebProcessLaunchDisabled { false };
 #endif
+    bool m_useSeparateServiceWorkerProcess { false };
 };
 
 template<typename T>
index 9960acf..6988654 100644 (file)
@@ -141,7 +141,7 @@ Ref<WebProcessProxy> WebProcessProxy::createForServiceWorkers(WebProcessPool& pr
 {
     auto proxy = adoptRef(*new WebProcessProxy(processPool, &websiteDataStore, IsPrewarmed::No));
     proxy->m_registrableDomain = WTFMove(registrableDomain);
-    proxy->m_serviceWorkerInformation = ServiceWorkerInformation { WebPageProxyIdentifier::generate(), PageIdentifier::generate() };
+    proxy->enableServiceWorkers();
     proxy->connect();
     return proxy;
 }
@@ -962,6 +962,9 @@ bool WebProcessProxy::canTerminateAuxiliaryProcess()
     if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache || m_shutdownPreventingScopeCount)
         return false;
 
+    if (isRunningServiceWorkers())
+        return false;
+
     if (!m_processPool->shouldTerminate(this))
         return false;
 
@@ -1466,6 +1469,9 @@ void WebProcessProxy::didStartProvisionalLoadForMainFrame(const URL& url)
 
     auto registrableDomain = WebCore::RegistrableDomain { url };
     if (m_registrableDomain && *m_registrableDomain != registrableDomain) {
+#if ENABLE(SERVICE_WORKER)
+        disableServiceWorkers();
+#endif
         // Null out registrable domain since this process has now been used for several domains.
         m_registrableDomain = WebCore::RegistrableDomain { };
         return;
@@ -1562,6 +1568,26 @@ void WebProcessProxy::updateServiceWorkerPreferencesStore(const WebPreferencesSt
     ASSERT(m_serviceWorkerInformation);
     send(Messages::WebSWContextManagerConnection::UpdatePreferencesStore { store }, 0);
 }
+
+void WebProcessProxy::disableServiceWorkers()
+{
+    if (!m_serviceWorkerInformation)
+        return;
+
+    m_serviceWorkerInformation = { };
+    processPool().removeFromServiceWorkerProcesses(*this);
+
+    send(Messages::WebSWContextManagerConnection::Close { }, 0);
+
+    maybeShutDown();
+}
+
+void WebProcessProxy::enableServiceWorkers()
+{
+    ASSERT(m_registrableDomain && !m_registrableDomain->isEmpty());
+    ASSERT(!m_serviceWorkerInformation);
+    m_serviceWorkerInformation = ServiceWorkerInformation { WebPageProxyIdentifier::generate(), PageIdentifier::generate() };
+}
 #endif
 
 } // namespace WebKit
index 832349a..c07b1ee 100644 (file)
@@ -123,10 +123,14 @@ public:
     WebProcessPool* processPoolIfExists() const;
     WebProcessPool& processPool() const;
 
+    bool isMatchingRegistrableDomain(const WebCore::RegistrableDomain& domain) const { return m_registrableDomain ? *m_registrableDomain == domain : false; }
     WebCore::RegistrableDomain registrableDomain() const { return m_registrableDomain.valueOr(WebCore::RegistrableDomain { }); }
     void setIsInProcessCache(bool);
     bool isInProcessCache() const { return m_isInProcessCache; }
 
+    void enableServiceWorkers();
+    void disableServiceWorkers();
+
     WebsiteDataStore& websiteDataStore() const { ASSERT(m_websiteDataStore); return *m_websiteDataStore; }
     void setWebsiteDataStore(WebsiteDataStore&);
     
index 751e510..6222808 100644 (file)
@@ -68,9 +68,6 @@ namespace WebKit {
 using namespace PAL;
 using namespace WebCore;
 
-static const Seconds asyncWorkerTerminationTimeout { 10_s };
-static const Seconds syncWorkerTerminationTimeout { 100_ms }; // Only used by layout tests.
-
 ServiceWorkerFrameLoaderClient::ServiceWorkerFrameLoaderClient(WebSWContextManagerConnection& connection, WebPageProxyIdentifier webPageProxyID, PageIdentifier pageID, FrameIdentifier frameID, const String& userAgent)
     : m_connection(connection)
     , m_webPageProxyID(webPageProxyID)
@@ -99,6 +96,7 @@ WebSWContextManagerConnection::WebSWContextManagerConnection(Ref<IPC::Connection
 {
     updatePreferencesStore(store);
     m_connectionToNetworkProcess->send(Messages::NetworkConnectionToWebProcess::EstablishSWContextConnection { m_registrableDomain }, 0);
+    WebProcess::singleton().disableTermination();
 }
 
 WebSWContextManagerConnection::~WebSWContextManagerConnection() = default;
@@ -253,12 +251,12 @@ void WebSWContextManagerConnection::softUpdate(WebCore::ServiceWorkerIdentifier
 
 void WebSWContextManagerConnection::terminateWorker(ServiceWorkerIdentifier identifier)
 {
-    SWContextManager::singleton().terminateWorker(identifier, asyncWorkerTerminationTimeout, nullptr);
+    SWContextManager::singleton().terminateWorker(identifier, SWContextManager::workerTerminationTimeout, nullptr);
 }
 
 void WebSWContextManagerConnection::syncTerminateWorker(ServiceWorkerIdentifier identifier, Messages::WebSWContextManagerConnection::SyncTerminateWorker::DelayedReply&& reply)
 {
-    SWContextManager::singleton().terminateWorker(identifier, syncWorkerTerminationTimeout, WTFMove(reply));
+    SWContextManager::singleton().terminateWorker(identifier, SWContextManager::syncWorkerTerminationTimeout, WTFMove(reply));
 }
 
 void WebSWContextManagerConnection::postMessageToServiceWorkerClient(const ServiceWorkerClientIdentifier& destinationIdentifier, const MessageWithMessagePorts& message, ServiceWorkerIdentifier sourceIdentifier, const String& sourceOrigin)
@@ -348,10 +346,14 @@ void WebSWContextManagerConnection::didFinishSkipWaiting(uint64_t callbackID)
         callback();
 }
 
-void WebSWContextManagerConnection::terminateProcess()
+void WebSWContextManagerConnection::close()
 {
-    RELEASE_LOG(ServiceWorker, "Service worker process is exiting because it is no longer needed");
-    _exit(EXIT_SUCCESS);
+    RELEASE_LOG(ServiceWorker, "Service worker process is requested to stop all service workers");
+    setAsClosed();
+
+    m_connectionToNetworkProcess->send(Messages::NetworkConnectionToWebProcess::CloseSWContextConnection { }, 0);
+    SWContextManager::singleton().stopAllServiceWorkers();
+    WebProcess::singleton().enableTermination();
 }
 
 void WebSWContextManagerConnection::setThrottleState(bool isThrottleable)
index 7d62601..247625e 100644 (file)
@@ -95,7 +95,7 @@ private:
     void claimCompleted(uint64_t claimRequestIdentifier);
     void didFinishSkipWaiting(uint64_t callbackID);
     void setUserAgent(String&& userAgent);
-    NO_RETURN void terminateProcess();
+    void close();
     void setThrottleState(bool isThrottleable);
 
     Ref<IPC::Connection> m_connectionToNetworkProcess;
index ead8cbd..709b685 100644 (file)
@@ -39,7 +39,7 @@ messages -> WebSWContextManagerConnection {
     DidFinishSkipWaiting(uint64_t callbackID)
     SetUserAgent(String userAgent)
     UpdatePreferencesStore(struct WebKit::WebPreferencesStore store)
-    TerminateProcess()
+    Close()
     SetThrottleState(bool isThrottleable)
 }
 
index 340c9c4..e3430c2 100644 (file)
@@ -1225,10 +1225,8 @@ void WebProcess::networkProcessConnectionClosed(NetworkProcessConnection* connec
 #endif
 
 #if ENABLE(SERVICE_WORKER)
-    if (SWContextManager::singleton().connection()) {
-        RELEASE_LOG(ServiceWorker, "Service worker process is exiting because network process is gone");
-        _exit(EXIT_SUCCESS);
-    }
+    if (SWContextManager::singleton().connection())
+        SWContextManager::singleton().stopAllServiceWorkers();
 #endif
 
     m_networkProcessConnection = nullptr;
@@ -1798,8 +1796,6 @@ void WebProcess::establishWorkerContextConnectionToNetworkProcess(uint64_t pageG
 
 void WebProcess::registerServiceWorkerClients()
 {
-    // We do not want to register service worker dummy documents.
-    ASSERT(!SWContextManager::singleton().connection());
     ServiceWorkerProvider::singleton().registerServiceWorkerClients();
 }
 
index 89924d5..d1dd0d6 100644 (file)
@@ -1,3 +1,25 @@
+2019-10-14  youenn fablet  <youenn@apple.com>
+
+        Reuse existing web processes for running service workers
+        https://bugs.webkit.org/show_bug.cgi?id=202195
+
+        Reviewed by Chris Dumez.
+
+        Add support for enforcing a separate process for service workers.
+        This is useful for tests trying to crash the service worker process.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
+        Update test to use serviceWorkerProcessCount.
+        Add test to check for in process and out of process service workers.
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::setUseSeparateServiceWorkerProcess):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
+
 2019-10-12  Chris Dumez  <cdumez@apple.com>
 
         Back/Forward cache does not work after doing a favorite navigation
index 57004bf..0c5dd96 100644 (file)
@@ -1243,7 +1243,7 @@ TEST(ServiceWorkers, ServiceWorkerProcessCreation)
     done = false;
 
     // Make sure that loading the simple page did not start the service worker process.
-    EXPECT_EQ(1u, webView.get().configuration.processPool._webProcessCountIgnoringPrewarmed);
+    EXPECT_EQ(0u, webView.get().configuration.processPool._serviceWorkerProcessCount);
 
     webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:newConfiguration.get()]);
     EXPECT_EQ(2u, webView.get().configuration.processPool._webProcessCountIgnoringPrewarmed);
@@ -1253,7 +1253,7 @@ TEST(ServiceWorkers, ServiceWorkerProcessCreation)
     done = false;
 
     // Make sure that loading this page did start the service worker process.
-    EXPECT_EQ(3u, webView.get().configuration.processPool._webProcessCountIgnoringPrewarmed);
+    EXPECT_EQ(1u, webView.get().configuration.processPool._serviceWorkerProcessCount);
 
     [[configuration websiteDataStore] removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {
         done = true;
@@ -1910,6 +1910,53 @@ TEST(ServiceWorkers, ParallelProcessLaunch)
     waitUntilServiceWorkerProcessCount(processPool, 2);
 }
 
+static size_t launchServiceWorkerProcess(bool useSeparateServiceWorkerProcess)
+{
+    [WKWebsiteDataStore _allowWebsiteDataRecordsForAllOrigins];
+
+    auto* dataStore = dataStoreWithRegisteredServiceWorkerScheme(@"sw");
+
+    // Start with a clean slate data store
+    [dataStore removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:^() {
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [configuration setWebsiteDataStore:dataStore];
+
+    auto messageHandler = adoptNS([[SWMessageHandler alloc] init]);
+    [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"sw"];
+
+    auto handler1 = adoptNS([[SWSchemes alloc] init]);
+    handler1->resources.set("sw://host/main.html", ResourceInfo { @"text/html", mainBytes });
+    handler1->resources.set("sw://host/sw.js", ResourceInfo { @"application/javascript", scriptBytes });
+    handler1->resources.set("sw://host2/main.html", ResourceInfo { @"text/html", mainBytes });
+    handler1->resources.set("sw://host2/sw.js", ResourceInfo { @"application/javascript", scriptBytes });
+    [configuration setURLSchemeHandler:handler1.get() forURLScheme:@"sw"];
+
+    auto *processPool = configuration.get().processPool;
+    [processPool _setUseSeparateServiceWorkerProcess: useSeparateServiceWorkerProcess];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw://host/main.html"]];
+
+    [webView loadRequest:request];
+
+    waitUntilServiceWorkerProcessCount(processPool, 1);
+    return webView.get().configuration.processPool._webProcessCountIgnoringPrewarmed;
+}
+
+TEST(ServiceWorkers, OutOfAndInProcessServiceWorker)
+{
+    bool useSeparateServiceWorkerProcess = false;
+    EXPECT_EQ(1u, launchServiceWorkerProcess(useSeparateServiceWorkerProcess));
+
+    useSeparateServiceWorkerProcess = true;
+    EXPECT_EQ(2u, launchServiceWorkerProcess(useSeparateServiceWorkerProcess));
+}
+
 TEST(ServiceWorkers, ThrottleCrash)
 {
     ASSERT(mainBytes);
index 9570144..e26b4fa 100644 (file)
@@ -366,6 +366,7 @@ interface TestRunner {
 
     void terminateNetworkProcess();
     void terminateServiceWorkerProcess();
+    void setUseSeparateServiceWorkerProcess(boolean value);
 
     readonly attribute unsigned long serverTrustEvaluationCallbackCallsCount;
 
index 4617ee3..917de41 100644 (file)
@@ -1264,6 +1264,13 @@ void TestRunner::terminateServiceWorkerProcess()
     WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr, nullptr);
 }
 
+void TestRunner::setUseSeparateServiceWorkerProcess(bool value)
+{
+    WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("SetUseSeparateServiceWorkerProcess"));
+    WKRetainPtr<WKBooleanRef> messageBody = adoptWK(WKBooleanCreate(value));
+    WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), messageName.get(), messageBody.get(), nullptr);
+}
+
 static unsigned nextUIScriptCallbackID()
 {
     static unsigned callbackID = FirstUIScriptCallbackID;
index 6271d52..d91db78 100644 (file)
@@ -465,6 +465,7 @@ public:
 
     void terminateNetworkProcess();
     void terminateServiceWorkerProcess();
+    void setUseSeparateServiceWorkerProcess(bool);
 
     void removeAllSessionCredentials(JSValueRef);
     void callDidRemoveAllSessionCredentialsCallback();
index c1d20d9..e9d428e 100644 (file)
@@ -978,6 +978,8 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options, Re
 
     WKContextClearCurrentModifierStateForTesting(TestController::singleton().context());
 
+    WKContextSetUseSeparateServiceWorkerProcess(TestController::singleton().context(), false);
+
     // FIXME: This function should also ensure that there is only one page open.
 
     // Reset the EventSender for each test.
index a8f9a79..8fe9279 100644 (file)
@@ -1618,6 +1618,13 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
         return nullptr;
     }
 
+    if (WKStringIsEqualToUTF8CString(messageName, "SetUseSeparateServiceWorkerProcess")) {
+        ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID());
+        auto useSeparateServiceWorkerProcess = WKBooleanGetValue(static_cast<WKBooleanRef>(messageBody));
+        WKContextSetUseSeparateServiceWorkerProcess(TestController::singleton().context(), useSeparateServiceWorkerProcess);
+        return nullptr;
+    }
+
     if (WKStringIsEqualToUTF8CString(messageName, "TerminateNetworkProcess")) {
         ASSERT(!messageBody);
         TestController::singleton().terminateNetworkProcess();