We should be able to recover after a Service Worker process crash
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Dec 2017 07:37:50 +0000 (07:37 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Dec 2017 07:37:50 +0000 (07:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180477

Reviewed by Brady Eidson and Youenn Fablet.

Source/WebCore:

Test: http/tests/workers/service/postmessage-after-sw-process-crash.https.html

* workers/service/server/SWServer.cpp:
(WebCore::SWServer::serverToContextConnectionCreated):
Once the connection with the context process is established, process "run service worker"
requests that ocurred while establishing the connection.

(WebCore::SWServer::runServiceWorkerIfNecessary):
Take in a lambda function that gets called after the "run service worker" request
is processed. We used to assert that we had a connection to the context process.
We now wait for the connection to be established to process the request, thus
making the operation asynchronous.

(WebCore::SWServer::runServiceWorker):
Split some logic out of runServiceWorkerIfNecessary() to reuse in serverToContextConnectionCreated().

(WebCore::SWServer::markAllWorkersAsTerminated):
Add method to mark all service workers as terminated. This is called when the Service
Worker process crashes.

* workers/service/server/SWServer.h:

Source/WebKit:

* StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::startFetch):
(WebKit::WebSWServerConnection::postMessageToServiceWorkerFromClient):
(WebKit::WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker):
Update calls to SWServer::runServiceWorkerIfNecessary() that that it is asynchronous
and takes in a lambda.

* StorageProcess/ServiceWorker/WebSWServerConnection.h:
(WebKit::WebSWServerConnection::ipcConnection const):
Add getter for the underlying IPC connection.

* StorageProcess/StorageProcess.cpp:
(WebKit::StorageProcess::didClose):
(WebKit::StorageProcess::connectionToContextProcessWasClosed):
Move some code to connectionToContextProcessWasClosed() to avoid duplication.
Also, relaunch the Service Worker process if it has exited but we still
have SWServer connections to regular Web Processes.

(WebKit::StorageProcess::needsServerToContextConnection const):
Utility function to determine if we still need the service worker process.
The current rule is that we need the service worker (aka "context") process
if we still have SWServer connections to regular Web Processes.

* StorageProcess/StorageProcess.h:

* StorageProcess/StorageToWebProcessConnection.cpp:
(WebKit::StorageToWebProcessConnection::didClose):
If didClose() is called for the connection to the service worker context,
let the StorageProcess know so that it can clear its state and relaunch
the process if necessary.

* UIProcess/API/C/WKContext.cpp:
(WKContextTerminateServiceWorkerProcess):
* UIProcess/API/C/WKContextPrivate.h:
* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _terminateServiceWorkerProcess]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
Add SPI to terminate the service worker process.

* UIProcess/WebProcessPool.cpp:
(WebKit::m_serviceWorkerProcessTerminationTimer):
(WebKit::WebProcessPool::createNewWebProcess):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::terminateServiceWorkerProcess):
* UIProcess/WebProcessPool.h:
We used to shutdown the ServiceWorker process right away as soon as the last regular
WebProcess was gone. We now give it a grace period of 5 seconds in case a new
WebProcess gets launched shortly after.

Tools:

Add testRunner API to terminate the Service Worker process.

* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::terminateServiceWorkerProcess):
* WebKitTestRunner/InjectedBundle/TestRunner.h:
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::terminateServiceWorkerProcess):
* WebKitTestRunner/TestController.h:
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):

LayoutTests:

Add layout test coverage.

* http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt: Added.
* http/tests/workers/service/postmessage-after-sw-process-crash.https.html: Added.
* http/tests/workers/service/resources/postmessage-after-sw-process-crash.js: Added.

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

26 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https.html [new file with mode: 0644]
LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/workers/service/server/SWServer.cpp
Source/WebCore/workers/service/server/SWServer.h
Source/WebKit/ChangeLog
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h
Source/WebKit/StorageProcess/StorageProcess.cpp
Source/WebKit/StorageProcess/StorageProcess.h
Source/WebKit/StorageProcess/StorageToWebProcessConnection.cpp
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/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestController.h
Tools/WebKitTestRunner/TestInvocation.cpp

index 184e160..970ca1c 100644 (file)
@@ -1,3 +1,16 @@
+2017-12-06  Chris Dumez  <cdumez@apple.com>
+
+        We should be able to recover after a Service Worker process crash
+        https://bugs.webkit.org/show_bug.cgi?id=180477
+
+        Reviewed by Brady Eidson and Youenn Fablet.
+
+        Add layout test coverage.
+
+        * http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt: Added.
+        * http/tests/workers/service/postmessage-after-sw-process-crash.https.html: Added.
+        * http/tests/workers/service/resources/postmessage-after-sw-process-crash.js: Added.
+
 2017-12-06  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Support the decoding="sync/async" syntax for image async attribute
diff --git a/LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt b/LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https-expected.txt
new file mode 100644 (file)
index 0000000..c10c281
--- /dev/null
@@ -0,0 +1,8 @@
+* Sending 'Message 1' to Service Worker
+PASS: Client received message from service worker, origin: https://127.0.0.1:8443
+PASS: Service worker received message 'Message 1' from origin 'https://127.0.0.1:8443'
+* Simulating Service Worker process crash
+* Sending 'Message 2' to Service Worker
+PASS: Client received message from service worker, origin: https://127.0.0.1:8443
+PASS: Service worker received message 'Message 2' from origin 'https://127.0.0.1:8443'
+
diff --git a/LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https.html b/LayoutTests/http/tests/workers/service/postmessage-after-sw-process-crash.https.html
new file mode 100644 (file)
index 0000000..be6073d
--- /dev/null
@@ -0,0 +1,9 @@
+<html>
+<head>
+<script src="resources/sw-test-pre.js"></script>
+</head>
+<body>
+
+<script src="resources/postmessage-after-sw-process-crash.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash.js b/LayoutTests/http/tests/workers/service/resources/postmessage-after-sw-process-crash.js
new file mode 100644 (file)
index 0000000..4e5ed92
--- /dev/null
@@ -0,0 +1,28 @@
+var messageNumber = 1;
+navigator.serviceWorker.addEventListener("message", function(event) {
+    log("PASS: Client received message from service worker, origin: " + event.origin);
+    log(event.data);
+    if (messageNumber == 1) {
+        log("* Simulating Service Worker process crash");
+        testRunner.terminateServiceWorkerProcess();
+        setTimeout(function() {
+            log("* Sending 'Message 2' to Service Worker");
+            event.source.postMessage("Message 2");
+            messageNumber++;
+            handle = setTimeout(function() {
+                log("FAIL: Did not receive message from service worker process after the crash");
+                finishSWTest();
+            }, 1000);
+        }, 0);
+        return;
+    }
+    if (messageNumber == 2) {
+        clearTimeout(handle);
+        finishSWTest();
+    }
+});
+
+navigator.serviceWorker.register("resources/postmessage-echo-worker.js", { }).then(function(registration) {
+    log("* Sending 'Message 1' to Service Worker");
+    registration.installing.postMessage("Message 1");
+});
index 84319f1..5127916 100644 (file)
@@ -1,3 +1,32 @@
+2017-12-06  Chris Dumez  <cdumez@apple.com>
+
+        We should be able to recover after a Service Worker process crash
+        https://bugs.webkit.org/show_bug.cgi?id=180477
+
+        Reviewed by Brady Eidson and Youenn Fablet.
+
+        Test: http/tests/workers/service/postmessage-after-sw-process-crash.https.html
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::serverToContextConnectionCreated):
+        Once the connection with the context process is established, process "run service worker"
+        requests that ocurred while establishing the connection.
+
+        (WebCore::SWServer::runServiceWorkerIfNecessary):
+        Take in a lambda function that gets called after the "run service worker" request
+        is processed. We used to assert that we had a connection to the context process.
+        We now wait for the connection to be established to process the request, thus
+        making the operation asynchronous.
+
+        (WebCore::SWServer::runServiceWorker):
+        Split some logic out of runServiceWorkerIfNecessary() to reuse in serverToContextConnectionCreated().
+
+        (WebCore::SWServer::markAllWorkersAsTerminated):
+        Add method to mark all service workers as terminated. This is called when the Service
+        Worker process crashes.
+
+        * workers/service/server/SWServer.h:
+
 2017-12-06  Saam Barati  <sbarati@apple.com>
 
         Unreviewed. Fix iOS (and maybe other platform) build
index bafc64c..a1ec027 100644 (file)
@@ -421,11 +421,19 @@ void SWServer::updateWorker(Connection&, const ServiceWorkerJobDataIdentifier& j
 
 void SWServer::serverToContextConnectionCreated()
 {
-    ASSERT(SWServerToContextConnection::globalServerToContextConnection());
-    for (auto& data : m_pendingContextDatas)
+    auto* connection = SWServerToContextConnection::globalServerToContextConnection();
+    ASSERT(connection);
+
+    auto pendingContextDatas = WTFMove(m_pendingContextDatas);
+    for (auto& data : pendingContextDatas)
         installContextData(data);
-    
-    m_pendingContextDatas.clear();
+
+    auto serviceWorkerRunRequests = WTFMove(m_serviceWorkerRunRequests);
+    for (auto& item : serviceWorkerRunRequests) {
+        bool success = runServiceWorker(item.key);
+        for (auto& callback : item.value)
+            callback(success, *connection);
+    }
 }
 
 void SWServer::installContextData(const ServiceWorkerContextData& data)
@@ -446,29 +454,47 @@ void SWServer::installContextData(const ServiceWorkerContextData& data)
     connection->installServiceWorkerContext(data);
 }
 
-bool SWServer::invokeRunServiceWorker(ServiceWorkerIdentifier identifier)
+void SWServer::runServiceWorkerIfNecessary(ServiceWorkerIdentifier identifier, RunServiceWorkerCallback&& callback)
 {
+    auto* connection = SWServerToContextConnection::globalServerToContextConnection();
     if (auto* worker = m_runningOrTerminatingWorkers.get(identifier)) {
-        if (worker->isRunning())
-            return true;
+        if (worker->isRunning()) {
+            ASSERT(connection);
+            callback(true, *connection);
+            return;
+        }
+    }
+
+    if (!connection) {
+        m_serviceWorkerRunRequests.ensure(identifier, [&] {
+            return Vector<RunServiceWorkerCallback> { };
+        }).iterator->value.append(WTFMove(callback));
+        return;
     }
 
-    // Nobody should have a ServiceWorkerIdentifier for a SWServerWorker that doesn't exist.
+    callback(runServiceWorker(identifier), *connection);
+}
+
+bool SWServer::runServiceWorker(ServiceWorkerIdentifier identifier)
+{
     auto* worker = workerByID(identifier);
-    ASSERT(worker);
+    if (!worker)
+        return false;
 
     // If the registration for a working has been removed then the request to run
     // the worker is moot.
     if (!getRegistration(worker->registrationKey()))
         return false;
 
-    m_runningOrTerminatingWorkers.add(identifier, *worker);
+    auto addResult = m_runningOrTerminatingWorkers.add(identifier, *worker);
+    ASSERT_UNUSED(addResult, addResult.isNewEntry);
+
     worker->setState(SWServerWorker::State::Running);
 
     auto* connection = SWServerToContextConnection::globalServerToContextConnection();
     ASSERT(connection);
     connection->installServiceWorkerContext(worker->contextData());
-    
+
     return true;
 }
 
@@ -505,10 +531,14 @@ void SWServer::terminateWorkerInternal(SWServerWorker& worker, TerminationMode m
     };
 }
 
-void SWServer::workerContextTerminated(SWServerWorker& worker)
+void SWServer::markAllWorkersAsTerminated()
 {
-    ASSERT(worker.isTerminating());
+    while (!m_runningOrTerminatingWorkers.isEmpty())
+        workerContextTerminated(m_runningOrTerminatingWorkers.begin()->value);
+}
 
+void SWServer::workerContextTerminated(SWServerWorker& worker)
+{
     worker.setState(SWServerWorker::State::NotRunning);
 
     // At this point if no registrations are referencing the worker then it will be destroyed,
index d7be310..9854ebf 100644 (file)
@@ -140,6 +140,8 @@ public:
     void fireInstallEvent(SWServerWorker&);
     void fireActivateEvent(SWServerWorker&);
     WEBCORE_EXPORT SWServerWorker* workerByID(ServiceWorkerIdentifier) const;
+
+    WEBCORE_EXPORT void markAllWorkersAsTerminated();
     
     Connection* getConnection(SWServerConnectionIdentifier identifier) { return m_connections.get(identifier); }
     SWOriginStore& originStore() { return m_originStore; }
@@ -160,7 +162,8 @@ public:
     WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientIdentifier, ServiceWorkerClientData&&, const std::optional<ServiceWorkerIdentifier>& controllingServiceWorkerIdentifier);
     WEBCORE_EXPORT void unregisterServiceWorkerClient(const ClientOrigin&, ServiceWorkerClientIdentifier);
 
-    WEBCORE_EXPORT bool invokeRunServiceWorker(ServiceWorkerIdentifier);
+    using RunServiceWorkerCallback = WTF::Function<void(bool, SWServerToContextConnection&)>;
+    WEBCORE_EXPORT void runServiceWorkerIfNecessary(ServiceWorkerIdentifier, RunServiceWorkerCallback&&);
 
     void setClientActiveWorker(ServiceWorkerClientIdentifier, ServiceWorkerIdentifier);
     void resolveRegistrationReadyRequests(SWServerRegistration&);
@@ -180,6 +183,7 @@ private:
     void removeClientServiceWorkerRegistration(Connection&, ServiceWorkerRegistrationIdentifier);
 
     WEBCORE_EXPORT SWServerRegistration* doRegistrationMatching(const SecurityOriginData& topOrigin, const URL& clientURL);
+    bool runServiceWorker(ServiceWorkerIdentifier);
 
     void installContextData(const ServiceWorkerContextData&);
 
@@ -216,6 +220,7 @@ private:
     UniqueRef<SWOriginStore> m_originStore;
     RegistrationStore m_registrationStore;
     Deque<ServiceWorkerContextData> m_pendingContextDatas;
+    HashMap<ServiceWorkerIdentifier, Vector<RunServiceWorkerCallback>> m_serviceWorkerRunRequests;
 };
 
 } // namespace WebCore
index 201de22..55d1616 100644 (file)
@@ -1,3 +1,59 @@
+2017-12-06  Chris Dumez  <cdumez@apple.com>
+
+        We should be able to recover after a Service Worker process crash
+        https://bugs.webkit.org/show_bug.cgi?id=180477
+
+        Reviewed by Brady Eidson and Youenn Fablet.
+
+        * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
+        (WebKit::WebSWServerConnection::startFetch):
+        (WebKit::WebSWServerConnection::postMessageToServiceWorkerFromClient):
+        (WebKit::WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker):
+        Update calls to SWServer::runServiceWorkerIfNecessary() that that it is asynchronous
+        and takes in a lambda.
+
+        * StorageProcess/ServiceWorker/WebSWServerConnection.h:
+        (WebKit::WebSWServerConnection::ipcConnection const):
+        Add getter for the underlying IPC connection.
+
+        * StorageProcess/StorageProcess.cpp:
+        (WebKit::StorageProcess::didClose):
+        (WebKit::StorageProcess::connectionToContextProcessWasClosed):
+        Move some code to connectionToContextProcessWasClosed() to avoid duplication.
+        Also, relaunch the Service Worker process if it has exited but we still
+        have SWServer connections to regular Web Processes.
+
+        (WebKit::StorageProcess::needsServerToContextConnection const):
+        Utility function to determine if we still need the service worker process.
+        The current rule is that we need the service worker (aka "context") process
+        if we still have SWServer connections to regular Web Processes.
+
+        * StorageProcess/StorageProcess.h:
+
+        * StorageProcess/StorageToWebProcessConnection.cpp:
+        (WebKit::StorageToWebProcessConnection::didClose):
+        If didClose() is called for the connection to the service worker context,
+        let the StorageProcess know so that it can clear its state and relaunch
+        the process if necessary.
+
+        * UIProcess/API/C/WKContext.cpp:
+        (WKContextTerminateServiceWorkerProcess):
+        * UIProcess/API/C/WKContextPrivate.h:
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _terminateServiceWorkerProcess]):
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        Add SPI to terminate the service worker process.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::m_serviceWorkerProcessTerminationTimer):
+        (WebKit::WebProcessPool::createNewWebProcess):
+        (WebKit::WebProcessPool::disconnectProcess):
+        (WebKit::WebProcessPool::terminateServiceWorkerProcess):
+        * UIProcess/WebProcessPool.h:
+        We used to shutdown the ServiceWorker process right away as soon as the last regular
+        WebProcess was gone. We now give it a grace period of 5 seconds in case a new
+        WebProcess gets launched shortly after.
+
 2017-12-02  Darin Adler  <darin@apple.com>
 
         Modernize some aspects of text codecs, eliminate WebKit use of strcasecmp
index 4340b7b..5a1e913 100644 (file)
@@ -116,11 +116,18 @@ void WebSWServerConnection::updateWorkerStateInClient(ServiceWorkerIdentifier wo
     send(Messages::WebSWClientConnection::UpdateWorkerState(worker, state));
 }
 
-void WebSWServerConnection::startFetch(uint64_t fetchIdentifier, std::optional<ServiceWorkerIdentifier> serviceWorkerIdentifier, const ResourceRequest& request, const FetchOptions& options, const IPC::FormDataReference& formData)
+void WebSWServerConnection::startFetch(uint64_t fetchIdentifier, std::optional<ServiceWorkerIdentifier> serviceWorkerIdentifier, ResourceRequest&& request, FetchOptions&& options, IPC::FormDataReference&& formData)
 {
     // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
-    if (serviceWorkerIdentifier && !server().invokeRunServiceWorker(*serviceWorkerIdentifier))
+    if (serviceWorkerIdentifier) {
+        server().runServiceWorkerIfNecessary(*serviceWorkerIdentifier, [contentConnection = m_contentConnection.copyRef(), connectionIdentifier = identifier(), fetchIdentifier, serviceWorkerIdentifier = *serviceWorkerIdentifier, request = WTFMove(request), options = WTFMove(options), formData = WTFMove(formData)](bool success, auto& contextConnection) {
+            if (success)
+                sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::StartFetch { connectionIdentifier, fetchIdentifier, serviceWorkerIdentifier, request, options, formData });
+            else
+                contentConnection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, fetchIdentifier);
+        });
         return;
+    }
 
     // FIXME: If we don't have a ServiceWorkerIdentifier here then we need to create and run the appropriate Service Worker,
     // but it's unclear that we have enough information here to do that.
@@ -128,26 +135,28 @@ void WebSWServerConnection::startFetch(uint64_t fetchIdentifier, std::optional<S
     sendToContextProcess(Messages::WebSWContextManagerConnection::StartFetch { identifier(), fetchIdentifier, serviceWorkerIdentifier, request, options, formData });
 }
 
-void WebSWServerConnection::postMessageToServiceWorkerFromClient(ServiceWorkerIdentifier destinationIdentifier, const IPC::DataReference& message, ServiceWorkerClientIdentifier sourceIdentifier, ServiceWorkerClientData&& sourceData)
+void WebSWServerConnection::postMessageToServiceWorkerFromClient(ServiceWorkerIdentifier destinationIdentifier, IPC::DataReference&& message, ServiceWorkerClientIdentifier sourceIdentifier, ServiceWorkerClientData&& sourceData)
 {
     // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
-    if (!server().invokeRunServiceWorker(destinationIdentifier))
-        return;
-
-    sendToContextProcess(Messages::WebSWContextManagerConnection::PostMessageToServiceWorkerFromClient { destinationIdentifier, message, sourceIdentifier, WTFMove(sourceData) });
+    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceIdentifier, sourceData = WTFMove(sourceData)](bool success, auto& contextConnection) mutable {
+        if (success)
+            sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorkerFromClient { destinationIdentifier, message, sourceIdentifier, WTFMove(sourceData) });
+    });
 }
 
-void WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker(ServiceWorkerIdentifier destinationIdentifier, const IPC::DataReference& message, ServiceWorkerIdentifier sourceIdentifier)
+void WebSWServerConnection::postMessageToServiceWorkerFromServiceWorker(ServiceWorkerIdentifier destinationIdentifier, IPC::DataReference&& message, ServiceWorkerIdentifier sourceIdentifier)
 {
-    // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
-    if (!server().invokeRunServiceWorker(destinationIdentifier))
-        return;
-
     auto* sourceWorker = server().workerByID(sourceIdentifier);
     if (!sourceWorker)
         return;
 
-    sendToContextProcess(Messages::WebSWContextManagerConnection::PostMessageToServiceWorkerFromServiceWorker { destinationIdentifier, message, sourceWorker->data() });
+    // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
+    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceIdentifier, sourceData = sourceWorker->data()](bool success, auto& contextConnection) mutable {
+        if (!success)
+            return;
+
+        sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorkerFromServiceWorker { destinationIdentifier, message, WTFMove(sourceData) });
+    });
 }
 
 void WebSWServerConnection::didReceiveFetchResponse(uint64_t fetchIdentifier, const ResourceResponse& response)
@@ -232,6 +241,11 @@ template<typename U> void WebSWServerConnection::sendToContextProcess(U&& messag
         connection->send(WTFMove(message));
 }
 
+template<typename U> void WebSWServerConnection::sendToContextProcess(WebCore::SWServerToContextConnection& connection, U&& message)
+{
+    static_cast<WebSWServerToContextConnection&>(connection).send(WTFMove(message));
+}
+
 } // namespace WebKit
 
 #endif // ENABLE(SERVICE_WORKER)
index b650853..5034978 100644 (file)
@@ -52,6 +52,8 @@ public:
     WebSWServerConnection(const WebSWServerConnection&) = delete;
     ~WebSWServerConnection() final;
 
+    IPC::Connection& ipcConnection() const { return m_contentConnection.get(); }
+
     void disconnectedFromWebProcess();
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
     void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&);
@@ -79,10 +81,10 @@ private:
     void notifyClientsOfControllerChange(const HashSet<WebCore::DocumentIdentifier>& contextIdentifiers, const WebCore::ServiceWorkerData& newController);
     void registrationReady(uint64_t registrationReadyRequestIdentifier, WebCore::ServiceWorkerRegistrationData&&) final;
 
-    void startFetch(uint64_t fetchIdentifier, std::optional<WebCore::ServiceWorkerIdentifier>, const WebCore::ResourceRequest&, const WebCore::FetchOptions&, const IPC::FormDataReference&);
+    void startFetch(uint64_t fetchIdentifier, std::optional<WebCore::ServiceWorkerIdentifier>, WebCore::ResourceRequest&&, WebCore::FetchOptions&&, IPC::FormDataReference&&);
 
-    void postMessageToServiceWorkerFromClient(WebCore::ServiceWorkerIdentifier destination, const IPC::DataReference& message, WebCore::ServiceWorkerClientIdentifier sourceIdentifier, WebCore::ServiceWorkerClientData&& source);
-    void postMessageToServiceWorkerFromServiceWorker(WebCore::ServiceWorkerIdentifier destination, const IPC::DataReference& message, WebCore::ServiceWorkerIdentifier source);
+    void postMessageToServiceWorkerFromClient(WebCore::ServiceWorkerIdentifier destination, IPC::DataReference&& message, WebCore::ServiceWorkerClientIdentifier sourceIdentifier, WebCore::ServiceWorkerClientData&& source);
+    void postMessageToServiceWorkerFromServiceWorker(WebCore::ServiceWorkerIdentifier destination, IPC::DataReference&& message, WebCore::ServiceWorkerIdentifier source);
 
     void matchRegistration(uint64_t registrationMatchRequestIdentifier, const WebCore::SecurityOriginData& topOrigin, const WebCore::URL& clientURL);
     void getRegistrations(uint64_t registrationMatchRequestIdentifier, const WebCore::SecurityOriginData& topOrigin, const WebCore::URL& clientURL);
@@ -94,10 +96,10 @@ private:
     uint64_t messageSenderDestinationID() final { return identifier().toUInt64(); }
     
     template<typename U> void sendToContextProcess(U&& message);
+    template<typename U> static void sendToContextProcess(WebCore::SWServerToContextConnection&, U&& message);
 
     PAL::SessionID m_sessionID;
     Ref<IPC::Connection> m_contentConnection;
-    RefPtr<IPC::Connection> m_contextConnection;
     HashMap<WebCore::DocumentIdentifier, WebCore::ClientOrigin> m_clientOrigins;
 };
 
index 2f7a501..fcfe115 100644 (file)
@@ -86,8 +86,7 @@ void StorageProcess::didClose(IPC::Connection& connection)
 {
 #if ENABLE(SERVICE_WORKER)
     if (m_serverToContextConnection && m_serverToContextConnection->ipcConnection() == &connection) {
-        m_serverToContextConnection->connectionClosed();
-        m_serverToContextConnection = nullptr;
+        connectionToContextProcessWasClosed();
         return;
     }
 #else
@@ -96,6 +95,38 @@ void StorageProcess::didClose(IPC::Connection& connection)
     stopRunLoop();
 }
 
+#if ENABLE(SERVICE_WORKER)
+void StorageProcess::connectionToContextProcessWasClosed()
+{
+    if (!m_serverToContextConnection)
+        return;
+
+    bool shouldRelaunch = needsServerToContextConnection();
+
+    m_serverToContextConnection->connectionClosed();
+    m_serverToContextConnection = nullptr;
+
+    for (auto& swServer : m_swServers.values())
+        swServer->markAllWorkersAsTerminated();
+
+    if (shouldRelaunch)
+        createServerToContextConnection();
+}
+
+// The rule is that we need a context process (and a connection to it) as long as we have SWServerConnections to regular WebProcesses.
+bool StorageProcess::needsServerToContextConnection() const
+{
+    if (m_swServerConnections.isEmpty())
+        return false;
+
+    // If the last SWServerConnection is to the context process, then we no longer need the context connection.
+    if (m_swServerConnections.size() == 1 && m_serverToContextConnection && &m_swServerConnections.begin()->value->ipcConnection() == m_serverToContextConnection->ipcConnection())
+        return false;
+
+    return true;
+}
+#endif
+
 void StorageProcess::didReceiveMessage(IPC::Connection& connection, IPC::Decoder& decoder)
 {
     if (messageReceiverMap().dispatchMessage(connection, decoder))
index 171c7a6..3482e74 100644 (file)
@@ -98,6 +98,10 @@ public:
 
     void didReceiveStorageProcessMessage(IPC::Connection&, IPC::Decoder&);
 
+#if ENABLE(SERVICE_WORKER)
+    void connectionToContextProcessWasClosed();
+#endif
+
 private:
     StorageProcess();
 
@@ -133,6 +137,7 @@ private:
 
     void postMessageToServiceWorkerClient(const WebCore::ServiceWorkerClientIdentifier& destinationIdentifier, const IPC::DataReference& message, WebCore::ServiceWorkerIdentifier sourceIdentifier, const String& sourceOrigin);
     WebSWOriginStore& swOriginStoreForSession(PAL::SessionID);
+    bool needsServerToContextConnection() const;
 #endif
 #if ENABLE(INDEXED_DATABASE)
     Vector<WebCore::SecurityOriginData> indexedDatabaseOrigins(const String& path);
index 7548192..3282a34 100644 (file)
@@ -120,8 +120,17 @@ void StorageToWebProcessConnection::didReceiveSyncMessage(IPC::Connection& conne
     ASSERT_NOT_REACHED();
 }
 
-void StorageToWebProcessConnection::didClose(IPC::Connection&)
+void StorageToWebProcessConnection::didClose(IPC::Connection& connection)
 {
+    UNUSED_PARAM(connection);
+
+#if ENABLE(SERVICE_WORKER)
+    if (StorageProcess::singleton().globalServerToContextConnection() && StorageProcess::singleton().globalServerToContextConnection()->ipcConnection() == &connection) {
+        // Service Worker process exited.
+        StorageProcess::singleton().connectionToContextProcessWasClosed();
+    }
+#endif
+
 #if ENABLE(INDEXED_DATABASE)
     auto idbConnections = m_webIDBConnections;
     for (auto& connection : idbConnections.values())
index 5f20d12..af84bf2 100644 (file)
@@ -614,6 +614,11 @@ void WKContextTerminateNetworkProcess(WKContextRef context)
     toImpl(context)->terminateNetworkProcess();
 }
 
+void WKContextTerminateServiceWorkerProcess(WKContextRef context)
+{
+    toImpl(context)->terminateServiceWorkerProcess();
+}
+
 ProcessID WKContextGetNetworkProcessIdentifier(WKContextRef contextRef)
 {
     return toImpl(contextRef)->networkProcessIdentifier();
index 2de11de..a0fc1c2 100644 (file)
@@ -89,6 +89,7 @@ WK_EXPORT void WKContextWarmInitialProcess(WKContextRef context);
 WK_EXPORT void WKContextSetUsesNetworkProcess(WKContextRef, bool);
 
 WK_EXPORT void WKContextTerminateNetworkProcess(WKContextRef);
+WK_EXPORT void WKContextTerminateServiceWorkerProcess(WKContextRef);
 
 WK_EXPORT void WKContextSetAllowsAnySSLCertificateForWebSocketTesting(WKContextRef, bool);
 WK_EXPORT void WKContextSetAllowsAnySSLCertificateForServiceWorkerTesting(WKContextRef, bool);
index 0cc9880..7c947b0 100644 (file)
@@ -412,6 +412,11 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     _processPool->terminateNetworkProcess();
 }
 
+- (void)_terminateServiceWorkerProcess
+{
+    _processPool->terminateServiceWorkerProcess();
+}
+
 - (pid_t)_networkProcessIdentifier
 {
     return _processPool->networkProcessIdentifier();
index 0dc1286..adde7c1 100644 (file)
@@ -74,6 +74,7 @@
 // Test only. Should be called only while no web content processes are running.
 - (void)_terminateStorageProcess;
 - (void)_terminateNetworkProcess;
+- (void)_terminateServiceWorkerProcess;
 
 // Test only.
 - (pid_t)_networkProcessIdentifier WK_API_AVAILABLE(macosx(10.13), ios(11.0));
index ac7cbbc..20999cb 100644 (file)
@@ -115,6 +115,8 @@ namespace WebKit {
 
 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, processPoolCounter, ("WebProcessPool"));
 
+static const Seconds serviceWorkerTerminationDelay { 5_s };
+
 static uint64_t generateListenerIdentifier()
 {
     static uint64_t nextIdentifier = 1;
@@ -244,6 +246,7 @@ WebProcessPool::WebProcessPool(API::ProcessPoolConfiguration& configuration)
     , m_processSuppressionDisabledForPageCounter([this](RefCounterEvent) { updateProcessSuppressionState(); })
     , m_hiddenPageThrottlingAutoIncreasesCounter([this](RefCounterEvent) { m_hiddenPageThrottlingTimer.startOneShot(0_s); })
     , m_hiddenPageThrottlingTimer(RunLoop::main(), this, &WebProcessPool::updateHiddenPageThrottlingAutoIncreaseLimit)
+    , m_serviceWorkerProcessTerminationTimer(RunLoop::main(), this, &WebProcessPool::terminateServiceWorkerProcess)
 {
     if (m_configuration->shouldHaveLegacyDataStore())
         m_websiteDataStore = API::WebsiteDataStore::createLegacy(legacyWebsiteDataStoreConfiguration(m_configuration));
@@ -697,6 +700,10 @@ WebProcessProxy& WebProcessPool::createNewWebProcess(WebsiteDataStore& websiteDa
     auto& process = processProxy.get();
     initializeNewWebProcess(process, websiteDataStore);
     m_processes.append(WTFMove(processProxy));
+
+    if (m_serviceWorkerProcessTerminationTimer.isActive())
+        m_serviceWorkerProcessTerminationTimer.stop();
+
     return process;
 }
 
@@ -943,8 +950,8 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
     // 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() == 1 && m_processes[0] == m_serviceWorkerProcess) {
-        m_serviceWorkerProcess = nullptr;
-        m_processes.clear();
+        if (!m_serviceWorkerProcessTerminationTimer.isActive())
+            m_serviceWorkerProcessTerminationTimer.startOneShot(serviceWorkerTerminationDelay);
     }
 #endif
 }
@@ -1408,6 +1415,18 @@ void WebProcessPool::terminateNetworkProcess()
     m_didNetworkProcessCrash = true;
 }
 
+void WebProcessPool::terminateServiceWorkerProcess()
+{
+#if ENABLE(SERVICE_WORKER)
+    if (!m_serviceWorkerProcess)
+        return;
+
+    m_serviceWorkerProcess->requestTermination(ProcessTerminationReason::RequestedByClient);
+    ASSERT(!m_processes.contains(m_serviceWorkerProcess));
+    ASSERT(!m_serviceWorkerProcess);
+#endif
+}
+
 void WebProcessPool::syncNetworkProcessCookies()
 {
     sendSyncToNetworkingProcess(Messages::NetworkProcess::SyncAllCookies(), Messages::NetworkProcess::SyncAllCookies::Reply());
index 5f14501..bcd3165 100644 (file)
@@ -262,6 +262,7 @@ public:
     void clearCachedCredentials();
     void terminateStorageProcess();
     void terminateNetworkProcess();
+    void terminateServiceWorkerProcess();
 
     void syncNetworkProcessCookies();
 
@@ -633,6 +634,7 @@ private:
     Paths m_resolvedPaths;
 
     HashMap<PAL::SessionID, HashSet<WebPageProxy*>> m_sessionToPagesMap;
+    RunLoop::Timer<WebProcessPool> m_serviceWorkerProcessTerminationTimer;
 };
 
 template<typename T>
index eed0fa8..f211c12 100644 (file)
@@ -1,3 +1,22 @@
+2017-12-06  Chris Dumez  <cdumez@apple.com>
+
+        We should be able to recover after a Service Worker process crash
+        https://bugs.webkit.org/show_bug.cgi?id=180477
+
+        Reviewed by Brady Eidson and Youenn Fablet.
+
+        Add testRunner API to terminate the Service Worker process.
+
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::terminateServiceWorkerProcess):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::terminateServiceWorkerProcess):
+        * WebKitTestRunner/TestController.h:
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
+
 2017-12-02  Darin Adler  <darin@apple.com>
 
         Modernize some aspects of text codecs, eliminate WebKit use of strcasecmp
index a8bc7cb..4bbdec1 100644 (file)
@@ -304,6 +304,7 @@ interface TestRunner {
     void setWebRTCLegacyAPIEnabled(boolean value);
 
     void terminateNetworkProcess();
+    void terminateServiceWorkerProcess();
 
     void removeAllSessionCredentials(object callback);
 
index aa98d0c..6a35501 100644 (file)
@@ -1138,6 +1138,12 @@ void TestRunner::terminateNetworkProcess()
     WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr);
 }
 
+void TestRunner::terminateServiceWorkerProcess()
+{
+    WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("TerminateServiceWorkerProcess"));
+    WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr);
+}
+
 static unsigned nextUIScriptCallbackID()
 {
     static unsigned callbackID = FirstUIScriptCallbackID;
index d644943..6040a1b 100644 (file)
@@ -402,6 +402,7 @@ public:
     void setOpenPanelFiles(JSValueRef);
 
     void terminateNetworkProcess();
+    void terminateServiceWorkerProcess();
 
     void removeAllSessionCredentials(JSValueRef);
     void callDidRemoveAllSessionCredentialsCallback();
index 2700045..34bae26 100644 (file)
@@ -2274,6 +2274,11 @@ void TestController::terminateNetworkProcess()
     WKContextTerminateNetworkProcess(platformContext());
 }
 
+void TestController::terminateServiceWorkerProcess()
+{
+    WKContextTerminateServiceWorkerProcess(platformContext());
+}
+
 #if !PLATFORM(COCOA)
 void TestController::platformWillRunTest(const TestInvocation&)
 {
index b1ac4f7..ad5db6b 100644 (file)
@@ -188,6 +188,7 @@ public:
     void setOpenPanelFileURLs(WKArrayRef fileURLs) { m_openPanelFileURLs = fileURLs; }
 
     void terminateNetworkProcess();
+    void terminateServiceWorkerProcess();
 
     void removeAllSessionCredentials();
 
index 75372e7..f5466d8 100644 (file)
@@ -744,6 +744,12 @@ void TestInvocation::didReceiveMessageFromInjectedBundle(WKStringRef messageName
         return;
     }
 
+    if (WKStringIsEqualToUTF8CString(messageName, "TerminateServiceWorkerProcess")) {
+        ASSERT(!messageBody);
+        TestController::singleton().terminateServiceWorkerProcess();
+        return;
+    }
+
     if (WKStringIsEqualToUTF8CString(messageName, "RunUIProcessScript")) {
         WKDictionaryRef messageBodyDictionary = static_cast<WKDictionaryRef>(messageBody);
         WKRetainPtr<WKStringRef> scriptKey(AdoptWK, WKStringCreateWithUTF8CString("Script"));