Promptly terminate service worker processes when they are no longer needed
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Mar 2018 22:16:25 +0000 (22:16 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Mar 2018 22:16:25 +0000 (22:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183873
<rdar://problem/38676995>

Reviewed by Youenn Fablet.

Source/WebCore:

The StorageProcess now keeps track of service worker clients for each security
origin. When there is no longer any clients for a given security origin, the
StorageProcess asks the service worker process for the given origin to terminate
and severs its connection to it.

Change is covered by API test.

* workers/service/server/SWServer.cpp:
(WebCore::SWServer::markAllWorkersForOriginAsTerminated):
Pass the security origin since this is called when a service worker process
crashes. When a service worker process for origin A crashes, we only want
to mark service workers in origin A as terminated, not ALL of them.

(WebCore::SWServer::registerServiceWorkerClient):
(WebCore::SWServer::unregisterServiceWorkerClient):
(WebCore::SWServer::needsServerToContextConnectionForOrigin const):
Tweak logic so that we only relaunch a service worker process if we still
have clients for its security origin.

* workers/service/server/SWServer.h:
(WebCore::SWServer::disableServiceWorkerProcessTerminationDelay):
Add a way to disable the service worker termination delay to facilitate
testing.

* workers/service/server/SWServerToContextConnection.h:

Source/WebKit:

The StorageProcess now keeps track of service worker clients for each security
origin. When there is no longer any clients for a given security origin, the
StorageProcess asks the service worker process for the given origin to terminate
and severs its connection to it.

* Shared/Storage/StorageProcessCreationParameters.h:
* StorageProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
(WebKit::WebSWServerToContextConnection::connectionMayNoLongerBeNeeded):
(WebKit::WebSWServerToContextConnection::terminate):
* StorageProcess/ServiceWorker/WebSWServerToContextConnection.h:
* StorageProcess/StorageProcess.cpp:
(WebKit::StorageProcess::connectionToContextProcessWasClosed):
(WebKit::StorageProcess::needsServerToContextConnectionForOrigin const):
(WebKit::StorageProcess::initializeWebsiteDataStore):
(WebKit::StorageProcess::swServerForSession):
(WebKit::StorageProcess::swContextConnectionMayNoLongerBeNeeded):
(WebKit::StorageProcess::disableServiceWorkerProcessTerminationDelay):
* StorageProcess/StorageProcess.h:
* StorageProcess/StorageProcess.messages.in:
* UIProcess/API/Cocoa/WKProcessPool.mm:
(-[WKProcessPool _disableServiceWorkerProcessTerminationDelay]):
* UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::ensureStorageProcessAndWebsiteDataStore):
(WebKit::WebProcessPool::disableServiceWorkerProcessTerminationDelay):
* UIProcess/WebProcessPool.h:
* WebProcess/Storage/WebSWContextManagerConnection.cpp:
(WebKit::WebSWContextManagerConnection::terminateProcess):
* WebProcess/Storage/WebSWContextManagerConnection.h:
* WebProcess/Storage/WebSWContextManagerConnection.messages.in:

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:

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

21 files changed:
Source/WebCore/ChangeLog
Source/WebCore/workers/service/server/SWServer.cpp
Source/WebCore/workers/service/server/SWServer.h
Source/WebCore/workers/service/server/SWServerToContextConnection.h
Source/WebKit/ChangeLog
Source/WebKit/Shared/Storage/StorageProcessCreationParameters.cpp
Source/WebKit/Shared/Storage/StorageProcessCreationParameters.h
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.cpp
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.h
Source/WebKit/StorageProcess/StorageProcess.cpp
Source/WebKit/StorageProcess/StorageProcess.h
Source/WebKit/StorageProcess/StorageProcess.messages.in
Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm

index 691fb12..1cf05c2 100644 (file)
@@ -1,3 +1,37 @@
+2018-03-23  Chris Dumez  <cdumez@apple.com>
+
+        Promptly terminate service worker processes when they are no longer needed
+        https://bugs.webkit.org/show_bug.cgi?id=183873
+        <rdar://problem/38676995>
+
+        Reviewed by Youenn Fablet.
+
+        The StorageProcess now keeps track of service worker clients for each security
+        origin. When there is no longer any clients for a given security origin, the
+        StorageProcess asks the service worker process for the given origin to terminate
+        and severs its connection to it.
+
+        Change is covered by API test.
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::markAllWorkersForOriginAsTerminated):
+        Pass the security origin since this is called when a service worker process
+        crashes. When a service worker process for origin A crashes, we only want
+        to mark service workers in origin A as terminated, not ALL of them.
+
+        (WebCore::SWServer::registerServiceWorkerClient):
+        (WebCore::SWServer::unregisterServiceWorkerClient):
+        (WebCore::SWServer::needsServerToContextConnectionForOrigin const):
+        Tweak logic so that we only relaunch a service worker process if we still
+        have clients for its security origin.
+
+        * workers/service/server/SWServer.h:
+        (WebCore::SWServer::disableServiceWorkerProcessTerminationDelay):
+        Add a way to disable the service worker termination delay to facilitate
+        testing.
+
+        * workers/service/server/SWServerToContextConnection.h:
+
 2018-03-23  Brady Eidson  <beidson@apple.com>
 
         Go to back/forward list items after a process-swapped navigation.
index d131efa..beef0e3 100644 (file)
@@ -641,10 +641,15 @@ void SWServer::terminateWorkerInternal(SWServerWorker& worker, TerminationMode m
     };
 }
 
-void SWServer::markAllWorkersAsTerminated()
+void SWServer::markAllWorkersForOriginAsTerminated(const SecurityOrigin& origin)
 {
-    while (!m_runningOrTerminatingWorkers.isEmpty())
-        workerContextTerminated(m_runningOrTerminatingWorkers.begin()->value);
+    Vector<SWServerWorker*> terminatedWorkers;
+    for (auto& worker : m_runningOrTerminatingWorkers.values()) {
+        if (origin.isSameSchemeHostPort(worker->securityOrigin()))
+            terminatedWorkers.append(worker.ptr());
+    }
+    for (auto& terminatedWorker : terminatedWorkers)
+        workerContextTerminated(*terminatedWorker);
 }
 
 void SWServer::workerContextTerminated(SWServerWorker& worker)
@@ -730,7 +735,7 @@ void SWServer::registerServiceWorkerClient(ClientOrigin&& clientOrigin, ServiceW
     ASSERT(!m_clientsById.contains(clientIdentifier));
     m_clientsById.add(clientIdentifier, WTFMove(data));
 
-    auto& clientIdentifiersForOrigin = m_clientIdentifiersPerOrigin.ensure(WTFMove(clientOrigin), [] {
+    auto& clientIdentifiersForOrigin = m_clientIdentifiersPerOrigin.ensure(clientOrigin, [] {
         return Clients { };
     }).iterator->value;
 
@@ -738,6 +743,10 @@ void SWServer::registerServiceWorkerClient(ClientOrigin&& clientOrigin, ServiceW
     clientIdentifiersForOrigin.identifiers.append(clientIdentifier);
     clientIdentifiersForOrigin.terminateServiceWorkersTimer = nullptr;
 
+    m_clientsBySecurityOrigin.ensure(clientOrigin.clientOrigin, [] {
+        return HashSet<ServiceWorkerClientIdentifier> { };
+    }).iterator->value.add(clientIdentifier);
+
     if (!controllingServiceWorkerRegistrationIdentifier)
         return;
 
@@ -769,11 +778,23 @@ void SWServer::unregisterServiceWorkerClient(const ClientOrigin& clientOrigin, S
                 if (worker->isRunning() && worker->origin() == clientOrigin)
                     terminateWorker(worker);
             }
+            if (!m_clientsBySecurityOrigin.contains(clientOrigin.clientOrigin)) {
+                if (auto* connection = SWServerToContextConnection::connectionForOrigin(clientOrigin.clientOrigin.securityOrigin()))
+                    connection->connectionMayNoLongerBeNeeded();
+            }
+
             m_clientIdentifiersPerOrigin.remove(clientOrigin);
         });
-        iterator->value.terminateServiceWorkersTimer->startOneShot(terminationDelay);
+        iterator->value.terminateServiceWorkersTimer->startOneShot(m_shouldDisableServiceWorkerProcessTerminationDelay ? 0_s : terminationDelay);
     }
 
+    auto clientsBySecurityOriginIterator = m_clientsBySecurityOrigin.find(clientOrigin.clientOrigin);
+    ASSERT(clientsBySecurityOriginIterator != m_clientsBySecurityOrigin.end());
+    auto& clientsForSecurityOrigin = clientsBySecurityOriginIterator->value;
+    clientsForSecurityOrigin.remove(clientIdentifier);
+    if (clientsForSecurityOrigin.isEmpty())
+        m_clientsBySecurityOrigin.remove(clientsBySecurityOriginIterator);
+
     auto registrationIterator = m_clientToControllingRegistration.find(clientIdentifier);
     if (registrationIterator == m_clientToControllingRegistration.end())
         return;
@@ -784,6 +805,11 @@ void SWServer::unregisterServiceWorkerClient(const ClientOrigin& clientOrigin, S
     m_clientToControllingRegistration.remove(registrationIterator);
 }
 
+bool SWServer::needsServerToContextConnectionForOrigin(const SecurityOrigin& origin) const
+{
+    return m_clientsBySecurityOrigin.contains(SecurityOriginData::fromSecurityOrigin(origin));
+}
+
 void SWServer::resolveRegistrationReadyRequests(SWServerRegistration& registration)
 {
     for (auto* connection : m_connections.values())
index 4c06e5e..606db46 100644 (file)
@@ -145,7 +145,7 @@ public:
     std::optional<ServiceWorkerClientData> serviceWorkerClientWithOriginByID(const ClientOrigin&, const ServiceWorkerClientIdentifier&) const;
     WEBCORE_EXPORT SWServerWorker* activeWorkerFromRegistrationID(ServiceWorkerRegistrationIdentifier);
 
-    WEBCORE_EXPORT void markAllWorkersAsTerminated();
+    WEBCORE_EXPORT void markAllWorkersForOriginAsTerminated(const SecurityOrigin&);
     
     Connection* getConnection(SWServerConnectionIdentifier identifier) { return m_connections.get(identifier); }
     SWOriginStore& originStore() { return m_originStore; }
@@ -177,6 +177,9 @@ public:
     WEBCORE_EXPORT void getOriginsWithRegistrations(Function<void(const HashSet<SecurityOriginData>&)>&&);
 
     PAL::SessionID sessionID() const { return m_sessionID; }
+    WEBCORE_EXPORT bool needsServerToContextConnectionForOrigin(const SecurityOrigin&) const;
+
+    void disableServiceWorkerProcessTerminationDelay() { m_shouldDisableServiceWorkerProcessTerminationDelay = true; }
 
 private:
     void registerConnection(Connection&);
@@ -215,6 +218,7 @@ private:
 
     HashMap<ServiceWorkerIdentifier, Ref<SWServerWorker>> m_runningOrTerminatingWorkers;
 
+    HashMap<SecurityOriginData, HashSet<ServiceWorkerClientIdentifier>> m_clientsBySecurityOrigin;
     struct Clients {
         Vector<ServiceWorkerClientIdentifier> identifiers;
         std::unique_ptr<Timer> terminateServiceWorkersTimer;
@@ -229,6 +233,7 @@ private:
     HashMap<RefPtr<SecurityOrigin>, HashMap<ServiceWorkerIdentifier, Vector<RunServiceWorkerCallback>>> m_serviceWorkerRunRequests;
     PAL::SessionID m_sessionID;
     bool m_importCompleted { false };
+    bool m_shouldDisableServiceWorkerProcessTerminationDelay { false };
     Vector<CompletionHandler<void()>> m_clearCompletionCallbacks;
     Vector<Function<void(const HashSet<SecurityOriginData>&)>> m_getOriginsWithRegistrationsCallbacks;
 };
index 4ac3d5f..6f102bc 100644 (file)
@@ -78,6 +78,8 @@ public:
 
     SecurityOrigin& origin() { return m_origin.get(); }
 
+    virtual void connectionMayNoLongerBeNeeded() = 0;
+
 protected:
     WEBCORE_EXPORT explicit SWServerToContextConnection(Ref<SecurityOrigin>&&);
 
index bc28809..9f96408 100644 (file)
@@ -1,3 +1,42 @@
+2018-03-23  Chris Dumez  <cdumez@apple.com>
+
+        Promptly terminate service worker processes when they are no longer needed
+        https://bugs.webkit.org/show_bug.cgi?id=183873
+        <rdar://problem/38676995>
+
+        Reviewed by Youenn Fablet.
+
+        The StorageProcess now keeps track of service worker clients for each security
+        origin. When there is no longer any clients for a given security origin, the
+        StorageProcess asks the service worker process for the given origin to terminate
+        and severs its connection to it.
+
+        * Shared/Storage/StorageProcessCreationParameters.h:
+        * StorageProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
+        (WebKit::WebSWServerToContextConnection::connectionMayNoLongerBeNeeded):
+        (WebKit::WebSWServerToContextConnection::terminate):
+        * StorageProcess/ServiceWorker/WebSWServerToContextConnection.h:
+        * StorageProcess/StorageProcess.cpp:
+        (WebKit::StorageProcess::connectionToContextProcessWasClosed):
+        (WebKit::StorageProcess::needsServerToContextConnectionForOrigin const):
+        (WebKit::StorageProcess::initializeWebsiteDataStore):
+        (WebKit::StorageProcess::swServerForSession):
+        (WebKit::StorageProcess::swContextConnectionMayNoLongerBeNeeded):
+        (WebKit::StorageProcess::disableServiceWorkerProcessTerminationDelay):
+        * StorageProcess/StorageProcess.h:
+        * StorageProcess/StorageProcess.messages.in:
+        * UIProcess/API/Cocoa/WKProcessPool.mm:
+        (-[WKProcessPool _disableServiceWorkerProcessTerminationDelay]):
+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::ensureStorageProcessAndWebsiteDataStore):
+        (WebKit::WebProcessPool::disableServiceWorkerProcessTerminationDelay):
+        * UIProcess/WebProcessPool.h:
+        * WebProcess/Storage/WebSWContextManagerConnection.cpp:
+        (WebKit::WebSWContextManagerConnection::terminateProcess):
+        * WebProcess/Storage/WebSWContextManagerConnection.h:
+        * WebProcess/Storage/WebSWContextManagerConnection.messages.in:
+
 2018-03-23  Brady Eidson  <beidson@apple.com>
 
         Go to back/forward list items after a process-swapped navigation.
index 6e5eabe..6f55c12 100644 (file)
@@ -41,7 +41,7 @@ void StorageProcessCreationParameters::encode(IPC::Encoder& encoder) const
     encoder << indexedDatabaseDirectory << indexedDatabaseDirectoryExtensionHandle;
 #endif
 #if ENABLE(SERVICE_WORKER)
-    encoder << serviceWorkerRegistrationDirectory << serviceWorkerRegistrationDirectoryExtensionHandle << urlSchemesServiceWorkersCanHandle;
+    encoder << serviceWorkerRegistrationDirectory << serviceWorkerRegistrationDirectoryExtensionHandle << urlSchemesServiceWorkersCanHandle << shouldDisableServiceWorkerProcessTerminationDelay;
 #endif
 }
 
@@ -71,6 +71,9 @@ bool StorageProcessCreationParameters::decode(IPC::Decoder& decoder, StorageProc
 
     if (!decoder.decode(result.urlSchemesServiceWorkersCanHandle))
         return false;
+
+    if (!decoder.decode(result.shouldDisableServiceWorkerProcessTerminationDelay))
+        return false;
 #endif
 
     return true;
index 67680df..fd97074 100644 (file)
@@ -54,6 +54,7 @@ struct StorageProcessCreationParameters {
     String serviceWorkerRegistrationDirectory;
     SandboxExtension::Handle serviceWorkerRegistrationDirectoryExtensionHandle;
     Vector<String> urlSchemesServiceWorkersCanHandle;
+    bool shouldDisableServiceWorkerProcessTerminationDelay { false };
 #endif
 };
 
index fccc837..14850ac 100644 (file)
@@ -28,6 +28,7 @@
 
 #if ENABLE(SERVICE_WORKER)
 
+#include "StorageProcess.h"
 #include "WebCoreArgumentCoders.h"
 #include "WebSWContextManagerConnectionMessages.h"
 #include <WebCore/ServiceWorkerContextData.h>
@@ -102,6 +103,16 @@ void WebSWServerToContextConnection::didFinishSkipWaiting(uint64_t callbackID)
     send(Messages::WebSWContextManagerConnection::DidFinishSkipWaiting { callbackID });
 }
 
+void WebSWServerToContextConnection::connectionMayNoLongerBeNeeded()
+{
+    StorageProcess::singleton().swContextConnectionMayNoLongerBeNeeded(*this);
+}
+
+void WebSWServerToContextConnection::terminate()
+{
+    send(Messages::WebSWContextManagerConnection::TerminateProcess());
+}
+
 } // namespace WebKit
 
 #endif // ENABLE(SERVICE_WORKER)
index d46af5b..bb10bde 100644 (file)
@@ -47,6 +47,8 @@ public:
     // IPC::MessageReceiver
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
 
+    void terminate();
+
 private:
     explicit WebSWServerToContextConnection(Ref<WebCore::SecurityOrigin>&&, Ref<IPC::Connection>&&);
 
@@ -65,6 +67,8 @@ private:
     void claimCompleted(uint64_t requestIdentifier) final;
     void didFinishSkipWaiting(uint64_t callbackID) final;
 
+    void connectionMayNoLongerBeNeeded() final;
+
     Ref<IPC::Connection> m_ipcConnection;
     
 }; // class WebSWServerToContextConnection
index bd1ad50..dd488c6 100644 (file)
@@ -27,6 +27,7 @@
 #include "StorageProcess.h"
 
 #include "ChildProcessMessages.h"
+#include "Logging.h"
 #include "StorageProcessCreationParameters.h"
 #include "StorageProcessMessages.h"
 #include "StorageProcessProxyMessages.h"
@@ -44,6 +45,7 @@
 #include <WebCore/ServiceWorkerClientIdentifier.h>
 #include <WebCore/TextEncoding.h>
 #include <pal/SessionID.h>
+#include <wtf/Algorithms.h>
 #include <wtf/CallbackAggregator.h>
 #include <wtf/CrossThreadTask.h>
 #include <wtf/MainThread.h>
@@ -111,32 +113,24 @@ void StorageProcess::didClose(IPC::Connection& connection)
 #if ENABLE(SERVICE_WORKER)
 void StorageProcess::connectionToContextProcessWasClosed(Ref<WebSWServerToContextConnection>&& serverToContextConnection)
 {
-    Ref<SecurityOrigin> origin = serverToContextConnection->origin();
-    bool shouldRelaunch = needsServerToContextConnectionForOrigin(origin);
-
     serverToContextConnection->connectionClosed();
-    m_serverToContextConnections.remove(origin.ptr());
+    m_serverToContextConnections.remove(&serverToContextConnection->origin());
 
     for (auto& swServer : m_swServers.values())
-        swServer->markAllWorkersAsTerminated();
+        swServer->markAllWorkersForOriginAsTerminated(serverToContextConnection->origin());
 
-    if (shouldRelaunch)
+    Ref<SecurityOrigin> origin = serverToContextConnection->origin();
+    if (needsServerToContextConnectionForOrigin(origin)) {
+        RELEASE_LOG(ServiceWorker, "Connection to service worker process was closed but is still needed, relaunching it");
         createServerToContextConnection(origin, std::nullopt);
+    }
 }
 
-// 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::needsServerToContextConnectionForOrigin(SecurityOrigin& origin) const
 {
-    if (m_swServerConnections.isEmpty())
-        return false;
-
-    auto* contextConnection = m_serverToContextConnections.get(&origin);
-
-    // If the last SWServerConnection is to the context process, then we no longer need the context connection.
-    if (m_swServerConnections.size() == 1 && contextConnection && &m_swServerConnections.begin()->value->ipcConnection() == contextConnection->ipcConnection())
-        return false;
-
-    return true;
+    return WTF::anyOf(m_swServers.values(), [&](auto& swServer) {
+        return swServer->needsServerToContextConnectionForOrigin(origin);
+    });
 }
 #endif
 
@@ -216,6 +210,8 @@ void StorageProcess::initializeWebsiteDataStore(const StorageProcessCreationPara
 
     for (auto& scheme : parameters.urlSchemesServiceWorkersCanHandle)
         registerURLSchemeServiceWorkersCanHandle(scheme);
+
+    m_shouldDisableServiceWorkerProcessTerminationDelay = parameters.shouldDisableServiceWorkerProcessTerminationDelay;
 #endif
 }
 
@@ -449,6 +445,8 @@ SWServer& StorageProcess::swServerForSession(PAL::SessionID sessionID)
     ASSERT(sessionID.isEphemeral() || !path.isEmpty());
 
     result.iterator->value = std::make_unique<SWServer>(makeUniqueRef<WebSWOriginStore>(), WTFMove(path), sessionID);
+    if (m_shouldDisableServiceWorkerProcessTerminationDelay)
+        result.iterator->value->disableServiceWorkerProcessTerminationDelay();
     return *result.iterator->value;
 }
 
@@ -536,6 +534,29 @@ void StorageProcess::unregisterSWServerConnection(WebSWServerConnection& connect
     m_swServerConnections.remove(connection.identifier());
     swOriginStoreForSession(connection.sessionID()).unregisterSWServerConnection(connection);
 }
+
+void StorageProcess::swContextConnectionMayNoLongerBeNeeded(WebSWServerToContextConnection& serverToContextConnection)
+{
+    auto& origin = serverToContextConnection.origin();
+    if (needsServerToContextConnectionForOrigin(origin))
+        return;
+
+    RELEASE_LOG(ServiceWorker, "Service worker process is no longer needed, terminating it");
+    serverToContextConnection.terminate();
+
+    serverToContextConnection.connectionClosed();
+    m_serverToContextConnections.remove(&origin);
+}
+
+void StorageProcess::disableServiceWorkerProcessTerminationDelay()
+{
+    if (m_shouldDisableServiceWorkerProcessTerminationDelay)
+        return;
+
+    m_shouldDisableServiceWorkerProcessTerminationDelay = true;
+    for (auto& swServer : m_swServers.values())
+        swServer->disableServiceWorkerProcessTerminationDelay();
+}
 #endif
 
 #if !PLATFORM(COCOA)
index d56ed40..d12fe31 100644 (file)
@@ -100,6 +100,8 @@ public:
     WebCore::SWServer& swServerForSession(PAL::SessionID);
     void registerSWServerConnection(WebSWServerConnection&);
     void unregisterSWServerConnection(WebSWServerConnection&);
+
+    void swContextConnectionMayNoLongerBeNeeded(WebSWServerToContextConnection&);
 #endif
 
     void didReceiveStorageProcessMessage(IPC::Connection&, IPC::Decoder&);
@@ -145,6 +147,8 @@ private:
     void postMessageToServiceWorkerClient(const WebCore::ServiceWorkerClientIdentifier& destinationIdentifier, WebCore::MessageWithMessagePorts&&, WebCore::ServiceWorkerIdentifier sourceIdentifier, const String& sourceOrigin);
     void postMessageToServiceWorker(WebCore::ServiceWorkerIdentifier destination, WebCore::MessageWithMessagePorts&&, const WebCore::ServiceWorkerOrClientIdentifier& source, WebCore::SWServerConnectionIdentifier);
 
+    void disableServiceWorkerProcessTerminationDelay();
+
     WebSWOriginStore& swOriginStoreForSession(PAL::SessionID);
     bool needsServerToContextConnectionForOrigin(WebCore::SecurityOrigin&) const;
 #endif
@@ -175,6 +179,7 @@ private:
 
     HashMap<RefPtr<WebCore::SecurityOrigin>, Ref<WebSWServerToContextConnection>> m_serverToContextConnections;
     bool m_waitingForServerToContextProcessConnection { false };
+    bool m_shouldDisableServiceWorkerProcessTerminationDelay { false };
     HashMap<PAL::SessionID, String> m_swDatabasePaths;
     HashMap<PAL::SessionID, std::unique_ptr<WebCore::SWServer>> m_swServers;
     HashMap<WebCore::SWServerConnectionIdentifier, WebSWServerConnection*> m_swServerConnections;
index ab2f487..81d1930 100644 (file)
@@ -45,5 +45,7 @@ messages -> StorageProcess LegacyReceiver {
     PostMessageToServiceWorkerClient(struct WebCore::ServiceWorkerClientIdentifier destinationIdentifier, struct WebCore::MessageWithMessagePorts message, WebCore::ServiceWorkerIdentifier sourceIdentifier, String sourceOrigin)
 
     PostMessageToServiceWorker(WebCore::ServiceWorkerIdentifier destination, struct WebCore::MessageWithMessagePorts message, WebCore::ServiceWorkerOrClientIdentifier source, WebCore::SWServerConnectionIdentifier connectionIdentifier)
+
+    DisableServiceWorkerProcessTerminationDelay()
 #endif
 }
index cbf999e..83366a0 100644 (file)
@@ -434,6 +434,11 @@ static NSDictionary *policiesHashMapToDictionary(const HashMap<String, HashMap<S
     _processPool->terminateServiceWorkerProcesses();
 }
 
+- (void)_disableServiceWorkerProcessTerminationDelay
+{
+    _processPool->disableServiceWorkerProcessTerminationDelay();
+}
+
 - (pid_t)_networkProcessIdentifier
 {
     return _processPool->networkProcessIdentifier();
index 9f0fa4d..44b762e 100644 (file)
@@ -81,6 +81,7 @@
 - (void)_terminateStorageProcess;
 - (void)_terminateNetworkProcess;
 - (void)_terminateServiceWorkerProcesses WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (void)_disableServiceWorkerProcessTerminationDelay WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 // Test only.
 - (pid_t)_networkProcessIdentifier WK_API_AVAILABLE(macosx(10.13), ios(11.0));
index c49c8ba..351bf6a 100644 (file)
@@ -557,6 +557,8 @@ void WebProcessPool::ensureStorageProcessAndWebsiteDataStore(WebsiteDataStore* r
 
         if (!m_schemesServiceWorkersCanHandle.isEmpty())
             parameters.urlSchemesServiceWorkersCanHandle = copyToVector(m_schemesServiceWorkersCanHandle);
+
+        parameters.shouldDisableServiceWorkerProcessTerminationDelay = m_shouldDisableServiceWorkerProcessTerminationDelay;
 #endif
 
         m_storageProcess = StorageProcessProxy::create(*this);
@@ -627,6 +629,18 @@ void WebProcessPool::establishWorkerContextConnectionToStorageProcess(StoragePro
 }
 #endif
 
+void WebProcessPool::disableServiceWorkerProcessTerminationDelay()
+{
+#if ENABLE(SERVICE_WORKER)
+    if (m_shouldDisableServiceWorkerProcessTerminationDelay)
+        return;
+
+    m_shouldDisableServiceWorkerProcessTerminationDelay = true;
+    if (m_storageProcess)
+        m_storageProcess->send(Messages::StorageProcess::DisableServiceWorkerProcessTerminationDelay(), 0);
+#endif
+}
+
 void WebProcessPool::willStartUsingPrivateBrowsing()
 {
     for (auto* processPool : allProcessPools())
index e4a3fa0..457f4c4 100644 (file)
@@ -272,6 +272,7 @@ public:
     void terminateStorageProcess();
     void terminateNetworkProcess();
     void terminateServiceWorkerProcesses();
+    void disableServiceWorkerProcessTerminationDelay();
 
     void syncNetworkProcessCookies();
 
@@ -519,6 +520,7 @@ private:
     HashMap<RefPtr<WebCore::SecurityOrigin>, ServiceWorkerProcessProxy*> m_serviceWorkerProcesses;
     bool m_waitingForWorkerContextProcessConnection { false };
     bool m_allowsAnySSLCertificateForServiceWorker { false };
+    bool m_shouldDisableServiceWorkerProcessTerminationDelay { false };
     String m_serviceWorkerUserAgent;
     std::optional<WebPreferencesStore> m_serviceWorkerPreferences;
     HashMap<String, bool> m_mayHaveRegisteredServiceWorkers;
index 30bc950..9473eef 100644 (file)
@@ -327,6 +327,12 @@ void WebSWContextManagerConnection::didFinishSkipWaiting(uint64_t callbackID)
         callback();
 }
 
+NO_RETURN void WebSWContextManagerConnection::terminateProcess()
+{
+    RELEASE_LOG(ServiceWorker, "Service worker process is exiting because it is no longer needed");
+    _exit(EXIT_SUCCESS);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(SERVICE_WORKER)
index 3cb7c68..8e8207d 100644 (file)
@@ -87,6 +87,7 @@ private:
     void claimCompleted(uint64_t claimRequestIdentifier);
     void didFinishSkipWaiting(uint64_t callbackID);
     void setUserAgent(String&& userAgent);
+    void terminateProcess();
 
     Ref<IPC::Connection> m_connectionToStorageProcess;
     uint64_t m_pageGroupID;
index 737c41f..27bee32 100644 (file)
@@ -36,6 +36,7 @@ messages -> WebSWContextManagerConnection {
     DidFinishSkipWaiting(uint64_t callbackID)
     SetUserAgent(String userAgent)
     UpdatePreferencesStore(struct WebKit::WebPreferencesStore store)
+    TerminateProcess()
 }
 
 #endif
index 08f93a1..f50dd2f 100644 (file)
@@ -1,3 +1,15 @@
+2018-03-23  Chris Dumez  <cdumez@apple.com>
+
+        Promptly terminate service worker processes when they are no longer needed
+        https://bugs.webkit.org/show_bug.cgi?id=183873
+        <rdar://problem/38676995>
+
+        Reviewed by Youenn Fablet.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
+
 2018-03-23  Brady Eidson  <beidson@apple.com>
 
         Go to back/forward list items after a process-swapped navigation.
index a0cfb6a..91668b9 100644 (file)
@@ -1282,6 +1282,15 @@ TEST(ServiceWorkers, NonDefaultSessionID)
     done = false;
 }
 
+void waitUntilServiceWorkerProcessCount(WKProcessPool *processPool, unsigned processCount)
+{
+    do {
+        if (processPool._serviceWorkerProcessCount == processCount)
+            return;
+        TestWebKitAPI::Util::spinRunLoop(1);
+    } while (true);
+}
+
 TEST(ServiceWorkers, ProcessPerOrigin)
 {
     ASSERT(mainBytes);
@@ -1311,8 +1320,13 @@ TEST(ServiceWorkers, ProcessPerOrigin)
     handler2->resources.set("sw2://host/sw.js", ResourceInfo { @"application/javascript", scriptBytes });
     [configuration setURLSchemeHandler:handler2.get() forURLScheme:@"sw2"];
 
-    [configuration.get().processPool _registerURLSchemeServiceWorkersCanHandle:@"sw1"];
-    [configuration.get().processPool _registerURLSchemeServiceWorkersCanHandle:@"sw2"];
+    WKProcessPool *processPool = configuration.get().processPool;
+    [processPool _registerURLSchemeServiceWorkersCanHandle:@"sw1"];
+    [processPool _registerURLSchemeServiceWorkersCanHandle:@"sw2"];
+
+    // Normally, service workers get terminated several seconds after their clients are gone.
+    // Disable this delay for the purpose of testing.
+    [processPool _disableServiceWorkerProcessTerminationDelay];
 
     RetainPtr<WKWebView> webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
 
@@ -1322,7 +1336,7 @@ TEST(ServiceWorkers, ProcessPerOrigin)
     TestWebKitAPI::Util::run(&done);
     done = false;
 
-    EXPECT_EQ(1U, webView1.get().configuration.processPool._serviceWorkerProcessCount);
+    EXPECT_EQ(1U, processPool._serviceWorkerProcessCount);
 
     RetainPtr<WKWebView> webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
     [webView2 loadRequest:request1];
@@ -1330,7 +1344,7 @@ TEST(ServiceWorkers, ProcessPerOrigin)
     TestWebKitAPI::Util::run(&done);
     done = false;
 
-    EXPECT_EQ(1U, webView2.get().configuration.processPool._serviceWorkerProcessCount);
+    EXPECT_EQ(1U, processPool._serviceWorkerProcessCount);
 
     RetainPtr<WKWebView> webView3 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
     NSURLRequest *request2 = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw2://host/main.html"]];
@@ -1339,7 +1353,21 @@ TEST(ServiceWorkers, ProcessPerOrigin)
     TestWebKitAPI::Util::run(&done);
     done = false;
 
-    EXPECT_EQ(2U, webView3.get().configuration.processPool._serviceWorkerProcessCount);
+    EXPECT_EQ(2U, processPool._serviceWorkerProcessCount);
+
+    NSURLRequest *aboutBlankRequest = [NSURLRequest requestWithURL:[NSURL URLWithString:@"about:blank"]];
+    [webView3 loadRequest:aboutBlankRequest];
+
+    waitUntilServiceWorkerProcessCount(processPool, 1);
+    EXPECT_EQ(1U, processPool._serviceWorkerProcessCount);
+
+    [webView2 loadRequest:aboutBlankRequest];
+    TestWebKitAPI::Util::spinRunLoop(10);
+    EXPECT_EQ(1U, processPool._serviceWorkerProcessCount);
+
+    [webView1 loadRequest:aboutBlankRequest];
+    waitUntilServiceWorkerProcessCount(processPool, 0);
+    EXPECT_EQ(0U, processPool._serviceWorkerProcessCount);
 }
 
 #endif // WK_API_ENABLED