Title: [229927] trunk
Revision
229927
Author
cdu...@apple.com
Date
2018-03-23 15:16:25 -0700 (Fri, 23 Mar 2018)

Log Message

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.

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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (229926 => 229927)


--- trunk/Source/WebCore/ChangeLog	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebCore/ChangeLog	2018-03-23 22:16:25 UTC (rev 229927)
@@ -1,3 +1,37 @@
+2018-03-23  Chris Dumez  <cdu...@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  <beid...@apple.com>
 
         Go to back/forward list items after a process-swapped navigation.

Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (229926 => 229927)


--- trunk/Source/WebCore/workers/service/server/SWServer.cpp	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp	2018-03-23 22:16:25 UTC (rev 229927)
@@ -641,10 +641,15 @@
     };
 }
 
-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 @@
     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 @@
     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 @@
                 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 @@
     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())

Modified: trunk/Source/WebCore/workers/service/server/SWServer.h (229926 => 229927)


--- trunk/Source/WebCore/workers/service/server/SWServer.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebCore/workers/service/server/SWServer.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -145,7 +145,7 @@
     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,7 +177,10 @@
     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&);
     void unregisterConnection(Connection&);
@@ -215,6 +218,7 @@
 
     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 @@
     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;
 };

Modified: trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h (229926 => 229927)


--- trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -78,6 +78,8 @@
 
     SecurityOrigin& origin() { return m_origin.get(); }
 
+    virtual void connectionMayNoLongerBeNeeded() = 0;
+
 protected:
     WEBCORE_EXPORT explicit SWServerToContextConnection(Ref<SecurityOrigin>&&);
 

Modified: trunk/Source/WebKit/ChangeLog (229926 => 229927)


--- trunk/Source/WebKit/ChangeLog	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/ChangeLog	2018-03-23 22:16:25 UTC (rev 229927)
@@ -1,3 +1,42 @@
+2018-03-23  Chris Dumez  <cdu...@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  <beid...@apple.com>
 
         Go to back/forward list items after a process-swapped navigation.

Modified: trunk/Source/WebKit/Shared/Storage/StorageProcessCreationParameters.cpp (229926 => 229927)


--- trunk/Source/WebKit/Shared/Storage/StorageProcessCreationParameters.cpp	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/Shared/Storage/StorageProcessCreationParameters.cpp	2018-03-23 22:16:25 UTC (rev 229927)
@@ -41,7 +41,7 @@
     encoder << indexedDatabaseDirectory << indexedDatabaseDirectoryExtensionHandle;
 #endif
 #if ENABLE(SERVICE_WORKER)
-    encoder << serviceWorkerRegistrationDirectory << serviceWorkerRegistrationDirectoryExtensionHandle << urlSchemesServiceWorkersCanHandle;
+    encoder << serviceWorkerRegistrationDirectory << serviceWorkerRegistrationDirectoryExtensionHandle << urlSchemesServiceWorkersCanHandle << shouldDisableServiceWorkerProcessTerminationDelay;
 #endif
 }
 
@@ -71,6 +71,9 @@
 
     if (!decoder.decode(result.urlSchemesServiceWorkersCanHandle))
         return false;
+
+    if (!decoder.decode(result.shouldDisableServiceWorkerProcessTerminationDelay))
+        return false;
 #endif
 
     return true;

Modified: trunk/Source/WebKit/Shared/Storage/StorageProcessCreationParameters.h (229926 => 229927)


--- trunk/Source/WebKit/Shared/Storage/StorageProcessCreationParameters.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/Shared/Storage/StorageProcessCreationParameters.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -54,6 +54,7 @@
     String serviceWorkerRegistrationDirectory;
     SandboxExtension::Handle serviceWorkerRegistrationDirectoryExtensionHandle;
     Vector<String> urlSchemesServiceWorkersCanHandle;
+    bool shouldDisableServiceWorkerProcessTerminationDelay { false };
 #endif
 };
 

Modified: trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.cpp (229926 => 229927)


--- trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.cpp	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.cpp	2018-03-23 22:16:25 UTC (rev 229927)
@@ -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 @@
     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)

Modified: trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.h (229926 => 229927)


--- trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/StorageProcess/ServiceWorker/WebSWServerToContextConnection.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -47,6 +47,8 @@
     // IPC::MessageReceiver
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
 
+    void terminate();
+
 private:
     explicit WebSWServerToContextConnection(Ref<WebCore::SecurityOrigin>&&, Ref<IPC::Connection>&&);
 
@@ -65,6 +67,8 @@
     void claimCompleted(uint64_t requestIdentifier) final;
     void didFinishSkipWaiting(uint64_t callbackID) final;
 
+    void connectionMayNoLongerBeNeeded() final;
+
     Ref<IPC::Connection> m_ipcConnection;
     
 }; // class WebSWServerToContextConnection

Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.cpp (229926 => 229927)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.cpp	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.cpp	2018-03-23 22:16:25 UTC (rev 229927)
@@ -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 @@
 #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 @@
 
     for (auto& scheme : parameters.urlSchemesServiceWorkersCanHandle)
         registerURLSchemeServiceWorkersCanHandle(scheme);
+
+    m_shouldDisableServiceWorkerProcessTerminationDelay = parameters.shouldDisableServiceWorkerProcessTerminationDelay;
 #endif
 }
 
@@ -449,6 +445,8 @@
     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 @@
     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)

Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.h (229926 => 229927)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -100,6 +100,8 @@
     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 @@
     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 @@
 
     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;

Modified: trunk/Source/WebKit/StorageProcess/StorageProcess.messages.in (229926 => 229927)


--- trunk/Source/WebKit/StorageProcess/StorageProcess.messages.in	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/StorageProcess/StorageProcess.messages.in	2018-03-23 22:16:25 UTC (rev 229927)
@@ -45,5 +45,7 @@
     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
 }

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (229926 => 229927)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm	2018-03-23 22:16:25 UTC (rev 229927)
@@ -434,6 +434,11 @@
     _processPool->terminateServiceWorkerProcesses();
 }
 
+- (void)_disableServiceWorkerProcessTerminationDelay
+{
+    _processPool->disableServiceWorkerProcessTerminationDelay();
+}
+
 - (pid_t)_networkProcessIdentifier
 {
     return _processPool->networkProcessIdentifier();

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (229926 => 229927)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -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));

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (229926 => 229927)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2018-03-23 22:16:25 UTC (rev 229927)
@@ -557,6 +557,8 @@
 
         if (!m_schemesServiceWorkersCanHandle.isEmpty())
             parameters.urlSchemesServiceWorkersCanHandle = copyToVector(m_schemesServiceWorkersCanHandle);
+
+        parameters.shouldDisableServiceWorkerProcessTerminationDelay = m_shouldDisableServiceWorkerProcessTerminationDelay;
 #endif
 
         m_storageProcess = StorageProcessProxy::create(*this);
@@ -627,6 +629,18 @@
 }
 #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())

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (229926 => 229927)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -272,6 +272,7 @@
     void terminateStorageProcess();
     void terminateNetworkProcess();
     void terminateServiceWorkerProcesses();
+    void disableServiceWorkerProcessTerminationDelay();
 
     void syncNetworkProcessCookies();
 
@@ -519,6 +520,7 @@
     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;

Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp (229926 => 229927)


--- trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp	2018-03-23 22:16:25 UTC (rev 229927)
@@ -327,6 +327,12 @@
         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)

Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h (229926 => 229927)


--- trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h	2018-03-23 22:16:25 UTC (rev 229927)
@@ -87,6 +87,7 @@
     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;

Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in (229926 => 229927)


--- trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in	2018-03-23 22:16:25 UTC (rev 229927)
@@ -36,6 +36,7 @@
     DidFinishSkipWaiting(uint64_t callbackID)
     SetUserAgent(String userAgent)
     UpdatePreferencesStore(struct WebKit::WebPreferencesStore store)
+    TerminateProcess()
 }
 
 #endif

Modified: trunk/Tools/ChangeLog (229926 => 229927)


--- trunk/Tools/ChangeLog	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Tools/ChangeLog	2018-03-23 22:16:25 UTC (rev 229927)
@@ -1,3 +1,15 @@
+2018-03-23  Chris Dumez  <cdu...@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  <beid...@apple.com>
 
         Go to back/forward list items after a process-swapped navigation.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm (229926 => 229927)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	2018-03-23 22:15:23 UTC (rev 229926)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm	2018-03-23 22:16:25 UTC (rev 229927)
@@ -1282,6 +1282,15 @@
     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,9 +1320,14 @@
     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()]);
 
     NSURLRequest *request1 = [NSURLRequest requestWithURL:[NSURL URLWithString:@"sw1://host/main.html"]];
@@ -1322,7 +1336,7 @@
     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 @@
     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 @@
     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
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to