Reduce use of SWServerToContextConnection::globalServerToContextConne…

…ction()

https://bugs.webkit.org/show_bug.cgi?id=183626

Reviewed by Youenn Fablet.

Source/WebCore:

Reduce use of SWServerToContextConnection::globalServerToContextConnection() as we are moving towards
having multiple context connections.

No new tests, no expected behavior change.

* workers/service/server/SWServer.cpp:
(WebCore::SWServer::matchAll):
(WebCore::SWServer::serverToContextConnectionCreated):
(WebCore::SWServer::runServiceWorkerIfNecessary):
(WebCore::SWServer::runServiceWorker):
(WebCore::SWServer::terminateWorkerInternal):
(WebCore::SWServer::markAllWorkersAsTerminated):
(WebCore::SWServer::workerContextTerminated):
(WebCore::SWServer::fireInstallEvent):
(WebCore::SWServer::fireActivateEvent):
* workers/service/server/SWServer.h:
* workers/service/server/SWServerToContextConnection.cpp:
(WebCore::SWServerToContextConnection::findClientByIdentifier):
(WebCore::SWServerToContextConnection::matchAll):
(WebCore::SWServerToContextConnection::claim):
* workers/service/server/SWServerWorker.cpp:
(WebCore::SWServerWorker::contextConnection):
(WebCore::SWServerWorker::matchAll):
* workers/service/server/SWServerWorker.h:

Source/WebKit:

Reduce use of SWServerToContextConnection::globalServerToContextConnection() as we are moving
towards having multiple context connections.

* StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::startFetch):
(WebKit::WebSWServerConnection::postMessageToServiceWorker):


Canonical link: https://commits.webkit.org/199295@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229615 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,35 @@
+2018-03-14  Chris Dumez  <cdumez@apple.com>
+
+        Reduce use of SWServerToContextConnection::globalServerToContextConnection()
+        https://bugs.webkit.org/show_bug.cgi?id=183626
+
+        Reviewed by Youenn Fablet.
+
+        Reduce use of SWServerToContextConnection::globalServerToContextConnection() as we are moving towards
+        having multiple context connections.
+
+        No new tests, no expected behavior change.
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::matchAll):
+        (WebCore::SWServer::serverToContextConnectionCreated):
+        (WebCore::SWServer::runServiceWorkerIfNecessary):
+        (WebCore::SWServer::runServiceWorker):
+        (WebCore::SWServer::terminateWorkerInternal):
+        (WebCore::SWServer::markAllWorkersAsTerminated):
+        (WebCore::SWServer::workerContextTerminated):
+        (WebCore::SWServer::fireInstallEvent):
+        (WebCore::SWServer::fireActivateEvent):
+        * workers/service/server/SWServer.h:
+        * workers/service/server/SWServerToContextConnection.cpp:
+        (WebCore::SWServerToContextConnection::findClientByIdentifier):
+        (WebCore::SWServerToContextConnection::matchAll):
+        (WebCore::SWServerToContextConnection::claim):
+        * workers/service/server/SWServerWorker.cpp:
+        (WebCore::SWServerWorker::contextConnection):
+        (WebCore::SWServerWorker::matchAll):
+        * workers/service/server/SWServerWorker.h:
+
 2018-03-14  Youenn Fablet  <youenn@apple.com>
 
         MessagePort should remove its listeners when being closed

Source/WebCore/workers/service/server/SWServer.cpp

@@ -410,7 +410,7 @@ void SWServer::didFinishActivation(SWServerWorker& worker)
 }
 
 // https://w3c.github.io/ServiceWorker/#clients-getall
-void SWServer::matchAll(SWServerWorker& worker, const ServiceWorkerClientQueryOptions& options, ServiceWorkerClientsMatchAllCallback&& callback)
+void SWServer::matchAll(SWServerWorker& worker, const ServiceWorkerClientQueryOptions& options, const ServiceWorkerClientsMatchAllCallback& callback)
 {
     // FIXME: Support reserved client filtering.
     // FIXME: Support WindowClient additional properties.
@@ -510,6 +510,10 @@ void SWServer::tryInstallContextData(ServiceWorkerContextData&& data)
 
 void SWServer::serverToContextConnectionCreated(SWServerToContextConnection& contextConnection)
 {
+    // FIXME: This will need to update only workers using this connection once we use several context connections.
+    for (auto* worker : SWServerWorker::allWorkers().values())
+        worker->setContextConnectionIdentifier(contextConnection.identifier());
+
     auto pendingContextDatas = WTFMove(m_pendingContextDatas);
     for (auto& data : pendingContextDatas)
         installContextData(data);
@@ -518,7 +522,7 @@ void SWServer::serverToContextConnectionCreated(SWServerToContextConnection& con
     for (auto& item : serviceWorkerRunRequests) {
         bool success = runServiceWorker(item.key);
         for (auto& callback : item.value)
-            callback(success, contextConnection);
+            callback(success ? &contextConnection : nullptr);
     }
 }
 
@@ -551,23 +555,28 @@ void SWServer::installContextData(const ServiceWorkerContextData& data)
 
 void SWServer::runServiceWorkerIfNecessary(ServiceWorkerIdentifier identifier, RunServiceWorkerCallback&& callback)
 {
-    auto* connection = SWServerToContextConnection::globalServerToContextConnection();
-    if (auto* worker = m_runningOrTerminatingWorkers.get(identifier)) {
-        if (worker->isRunning()) {
-            ASSERT(connection);
-            callback(true, *connection);
-            return;
-        }
+    auto* worker = workerByID(identifier);
+    if (!worker) {
+        callback(nullptr);
+        return;
     }
 
-    if (!connection) {
+    auto* contextConnection = worker->contextConnection();
+    if (worker->isRunning()) {
+        ASSERT(contextConnection);
+        callback(contextConnection);
+        return;
+    }
+
+    if (!contextConnection) {
         m_serviceWorkerRunRequests.ensure(identifier, [&] {
             return Vector<RunServiceWorkerCallback> { };
         }).iterator->value.append(WTFMove(callback));
         return;
     }
 
-    callback(runServiceWorker(identifier), *connection);
+    bool success = runServiceWorker(identifier);
+    callback(success ? contextConnection : nullptr);
 }
 
 bool SWServer::runServiceWorker(ServiceWorkerIdentifier identifier)
@@ -586,14 +595,10 @@ bool SWServer::runServiceWorker(ServiceWorkerIdentifier identifier)
 
     worker->setState(SWServerWorker::State::Running);
 
-    auto* connection = SWServerToContextConnection::globalServerToContextConnection();
-    ASSERT(connection);
-
-    // When re-running a service worker after a context process crash, the connection identifier may have changed
-    // so we update it here.
-    worker->setContextConnectionIdentifier(connection->identifier());
+    auto* contextConnection = worker->contextConnection();
+    ASSERT(contextConnection);
 
-    connection->installServiceWorkerContext(worker->contextData(), m_sessionID);
+    contextConnection->installServiceWorkerContext(worker->contextData(), m_sessionID);
 
     return true;
 }
@@ -617,42 +622,36 @@ void SWServer::terminateWorkerInternal(SWServerWorker& worker, TerminationMode m
 
     worker.setState(SWServerWorker::State::Terminating);
 
-    auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
-    ASSERT(contextConnectionIdentifier);
-    if (!contextConnectionIdentifier) {
-        LOG_ERROR("Request to terminate a worker whose contextConnectionIdentifier is invalid");
-        workerContextTerminated(worker);
-        return;
-    }
-
-    auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
-    ASSERT(connection);
-    if (!connection) {
+    auto* contextConnection = worker.contextConnection();
+    ASSERT(contextConnection);
+    if (!contextConnection) {
         LOG_ERROR("Request to terminate a worker whose context connection does not exist");
         workerContextTerminated(worker);
         return;
     }
 
     switch (mode) {
     case Asynchronous:
-        connection->terminateWorker(worker.identifier());
+        contextConnection->terminateWorker(worker.identifier());
         break;
     case Synchronous:
-        connection->syncTerminateWorker(worker.identifier());
+        contextConnection->syncTerminateWorker(worker.identifier());
         break;
     };
 }
 
 void SWServer::markAllWorkersAsTerminated()
 {
-    while (!m_runningOrTerminatingWorkers.isEmpty())
-        workerContextTerminated(m_runningOrTerminatingWorkers.begin()->value);
+    while (!m_runningOrTerminatingWorkers.isEmpty()) {
+        auto& worker = m_runningOrTerminatingWorkers.begin()->value;
+        worker->setContextConnectionIdentifier(std::nullopt);
+        workerContextTerminated(worker);
+    }
 }
 
 void SWServer::workerContextTerminated(SWServerWorker& worker)
 {
     worker.setState(SWServerWorker::State::NotRunning);
-    worker.setContextConnectionIdentifier(std::nullopt);
 
     if (auto* jobQueue = m_jobQueues.get(worker.registrationKey()))
         jobQueue->cancelJobsFromServiceWorker(worker.identifier());
@@ -665,35 +664,24 @@ void SWServer::workerContextTerminated(SWServerWorker& worker)
 
 void SWServer::fireInstallEvent(SWServerWorker& worker)
 {
-    auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
-    if (!contextConnectionIdentifier) {
-        LOG_ERROR("Request to fire install event on a worker whose contextConnectionIdentifier is invalid");
-        return;
-    }
-    auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
-    if (!connection) {
+    auto* contextConnection = worker.contextConnection();
+    if (!contextConnection) {
         LOG_ERROR("Request to fire install event on a worker whose context connection does not exist");
         return;
     }
 
-    connection->fireInstallEvent(worker.identifier());
+    contextConnection->fireInstallEvent(worker.identifier());
 }
 
 void SWServer::fireActivateEvent(SWServerWorker& worker)
 {
-    auto contextConnectionIdentifier = worker.contextConnectionIdentifier();
-    if (!contextConnectionIdentifier) {
-        LOG_ERROR("Request to fire install event on a worker whose contextConnectionIdentifier is invalid");
-        return;
-    }
-
-    auto* connection = SWServerToContextConnection::connectionForIdentifier(*contextConnectionIdentifier);
-    if (!connection) {
+    auto* contextConnection = worker.contextConnection();
+    if (!contextConnection) {
         LOG_ERROR("Request to fire install event on a worker whose context connection does not exist");
         return;
     }
 
-    connection->fireActivateEvent(worker.identifier());
+    contextConnection->fireActivateEvent(worker.identifier());
 }
 
 void SWServer::registerConnection(Connection& connection)

Source/WebCore/workers/service/server/SWServer.h

@@ -154,7 +154,7 @@ class SWServer {
     void didFinishInstall(const std::optional<ServiceWorkerJobDataIdentifier>&, SWServerWorker&, bool wasSuccessful);
     void didFinishActivation(SWServerWorker&);
     void workerContextTerminated(SWServerWorker&);
-    void matchAll(SWServerWorker&, const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&);
+    void matchAll(SWServerWorker&, const ServiceWorkerClientQueryOptions&, const ServiceWorkerClientsMatchAllCallback&);
     void claim(SWServerWorker&);
 
     WEBCORE_EXPORT void serverToContextConnectionCreated(SWServerToContextConnection&);
@@ -164,7 +164,7 @@ class SWServer {
     WEBCORE_EXPORT void registerServiceWorkerClient(ClientOrigin&&, ServiceWorkerClientData&&, const std::optional<ServiceWorkerRegistrationIdentifier>&);
     WEBCORE_EXPORT void unregisterServiceWorkerClient(const ClientOrigin&, ServiceWorkerClientIdentifier);
 
-    using RunServiceWorkerCallback = WTF::Function<void(bool, SWServerToContextConnection&)>;
+    using RunServiceWorkerCallback = WTF::Function<void(SWServerToContextConnection*)>;
     WEBCORE_EXPORT void runServiceWorkerIfNecessary(ServiceWorkerIdentifier, RunServiceWorkerCallback&&);
 
     void resolveRegistrationReadyRequests(SWServerRegistration&);

Source/WebCore/workers/service/server/SWServerToContextConnection.cpp

@@ -111,14 +111,14 @@ void SWServerToContextConnection::workerTerminated(ServiceWorkerIdentifier servi
 void SWServerToContextConnection::findClientByIdentifier(uint64_t requestIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, ServiceWorkerClientIdentifier clientId)
 {
     if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier))
-        globalServerToContextConnection()->findClientByIdentifierCompleted(requestIdentifier, worker->findClientByIdentifier(clientId), false);
+        worker->contextConnection()->findClientByIdentifierCompleted(requestIdentifier, worker->findClientByIdentifier(clientId), false);
 }
 
 void SWServerToContextConnection::matchAll(uint64_t requestIdentifier, ServiceWorkerIdentifier serviceWorkerIdentifier, const ServiceWorkerClientQueryOptions& options)
 {
     if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier)) {
-        worker->matchAll(options, [requestIdentifier] (auto&& data) {
-            globalServerToContextConnection()->matchAllCompleted(requestIdentifier, data);
+        worker->matchAll(options, [&] (auto&& data) {
+            worker->contextConnection()->matchAllCompleted(requestIdentifier, data);
         });
     }
 }
@@ -127,7 +127,7 @@ void SWServerToContextConnection::claim(uint64_t requestIdentifier, ServiceWorke
 {
     if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier)) {
         worker->claim();
-        globalServerToContextConnection()->claimCompleted(requestIdentifier);
+        worker->contextConnection()->claimCompleted(requestIdentifier);
     }
 }
 

Source/WebCore/workers/service/server/SWServerWorker.cpp

@@ -32,7 +32,7 @@
 
 namespace WebCore {
 
-static HashMap<ServiceWorkerIdentifier, SWServerWorker*>& allWorkers()
+HashMap<ServiceWorkerIdentifier, SWServerWorker*>& SWServerWorker::allWorkers()
 {
     static NeverDestroyed<HashMap<ServiceWorkerIdentifier, SWServerWorker*>> workers;
     return workers;
@@ -90,6 +90,14 @@ const ClientOrigin& SWServerWorker::origin() const
     return *m_origin;
 }
 
+SWServerToContextConnection* SWServerWorker::contextConnection()
+{
+    if (!m_contextConnectionIdentifier)
+        return nullptr;
+
+    return SWServerToContextConnection::connectionForIdentifier(*m_contextConnectionIdentifier);
+}
+
 void SWServerWorker::scriptContextFailedToStart(const std::optional<ServiceWorkerJobDataIdentifier>& jobDataIdentifier, const String& message)
 {
     m_server.scriptContextFailedToStart(jobDataIdentifier, *this, message);
@@ -120,9 +128,9 @@ std::optional<ServiceWorkerClientData> SWServerWorker::findClientByIdentifier(co
     return m_server.serviceWorkerClientWithOriginByID(origin(), clientId);
 }
 
-void SWServerWorker::matchAll(const ServiceWorkerClientQueryOptions& options, ServiceWorkerClientsMatchAllCallback&& callback)
+void SWServerWorker::matchAll(const ServiceWorkerClientQueryOptions& options, const ServiceWorkerClientsMatchAllCallback& callback)
 {
-    return m_server.matchAll(*this, options, WTFMove(callback));
+    return m_server.matchAll(*this, options, callback);
 }
 
 void SWServerWorker::claim()

Source/WebCore/workers/service/server/SWServerWorker.h

@@ -40,6 +40,7 @@ namespace WebCore {
 struct ClientOrigin;
 class SWServer;
 class SWServerRegistration;
+class SWServerToContextConnection;
 struct ServiceWorkerClientData;
 struct ServiceWorkerClientIdentifier;
 struct ServiceWorkerClientQueryOptions;
@@ -78,7 +79,6 @@ class SWServerWorker : public RefCounted<SWServerWorker> {
 
     ServiceWorkerIdentifier identifier() const { return m_data.identifier; }
 
-    std::optional<SWServerToContextConnectionIdentifier> contextConnectionIdentifier() const { return m_contextConnectionIdentifier; }
     void setContextConnectionIdentifier(std::optional<SWServerToContextConnectionIdentifier> identifier) { m_contextConnectionIdentifier = identifier; }
 
     ServiceWorkerState state() const { return m_data.state; }
@@ -93,18 +93,21 @@ class SWServerWorker : public RefCounted<SWServerWorker> {
     void didFinishActivation();
     void contextTerminated();
     WEBCORE_EXPORT std::optional<ServiceWorkerClientData> findClientByIdentifier(const ServiceWorkerClientIdentifier&) const;
-    void matchAll(const ServiceWorkerClientQueryOptions&, ServiceWorkerClientsMatchAllCallback&&);
+    void matchAll(const ServiceWorkerClientQueryOptions&, const ServiceWorkerClientsMatchAllCallback&);
     void claim();
 
     void skipWaiting();
     bool isSkipWaitingFlagSet() const { return m_isSkipWaitingFlagSet; }
 
     WEBCORE_EXPORT static SWServerWorker* existingWorkerForIdentifier(ServiceWorkerIdentifier);
+    static HashMap<ServiceWorkerIdentifier, SWServerWorker*>& allWorkers();
 
     const ServiceWorkerData& data() const { return m_data; }
     ServiceWorkerContextData contextData() const;
     const ClientOrigin& origin() const;
 
+    WEBCORE_EXPORT SWServerToContextConnection* contextConnection();
+
 private:
     SWServerWorker(SWServer&, SWServerRegistration&, std::optional<SWServerToContextConnectionIdentifier>, const URL&, const String& script, const ContentSecurityPolicyResponseHeaders&,  WorkerType, ServiceWorkerIdentifier);
 

Source/WebKit/ChangeLog

@@ -1,3 +1,17 @@
+2018-03-14  Chris Dumez  <cdumez@apple.com>
+
+        Reduce use of SWServerToContextConnection::globalServerToContextConnection()
+        https://bugs.webkit.org/show_bug.cgi?id=183626
+
+        Reviewed by Youenn Fablet.
+
+        Reduce use of SWServerToContextConnection::globalServerToContextConnection() as we are moving
+        towards having multiple context connections.
+
+        * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
+        (WebKit::WebSWServerConnection::startFetch):
+        (WebKit::WebSWServerConnection::postMessageToServiceWorker):
+
 2018-03-14  Chris Dumez  <cdumez@apple.com>
 
         Make policy decisions asynchronous

Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp

@@ -154,13 +154,13 @@ void WebSWServerConnection::startFetch(uint64_t fetchIdentifier, ServiceWorkerRe
         if (!StorageProcess::singleton().globalServerToContextConnection())
             StorageProcess::singleton().createServerToContextConnection(server().sessionID());
 
-        server().runServiceWorkerIfNecessary(serviceWorkerIdentifier, [weakThis = WTFMove(weakThis), this, fetchIdentifier, serviceWorkerIdentifier, request = WTFMove(request), options = WTFMove(options), formData = WTFMove(formData), referrer = WTFMove(referrer)](bool success, auto& contextConnection) {
+        server().runServiceWorkerIfNecessary(serviceWorkerIdentifier, [weakThis = WTFMove(weakThis), this, fetchIdentifier, serviceWorkerIdentifier, request = WTFMove(request), options = WTFMove(options), formData = WTFMove(formData), referrer = WTFMove(referrer)](auto* contextConnection) {
             if (!weakThis)
                 return;
 
-            if (success) {
+            if (contextConnection) {
                 SWSERVERCONNECTION_RELEASE_LOG_IF_ALLOWED("startFetch: Starting fetch %llu via service worker %llu", fetchIdentifier, serviceWorkerIdentifier.toUInt64());
-                sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::StartFetch { this->identifier(), fetchIdentifier, serviceWorkerIdentifier, request, options, formData, referrer });
+                sendToContextProcess(*contextConnection, Messages::WebSWContextManagerConnection::StartFetch { this->identifier(), fetchIdentifier, serviceWorkerIdentifier, request, options, formData, referrer });
             } else {
                 SWSERVERCONNECTION_RELEASE_LOG_ERROR_IF_ALLOWED("startFetch: fetchIdentifier: %llu -> DidNotHandle because failed to run service worker", fetchIdentifier);
                 m_contentConnection->send(Messages::ServiceWorkerClientFetch::DidNotHandle { }, fetchIdentifier);
@@ -198,9 +198,9 @@ void WebSWServerConnection::postMessageToServiceWorker(ServiceWorkerIdentifier d
         StorageProcess::singleton().createServerToContextConnection(server().sessionID());
 
     // It's possible this specific worker cannot be re-run (e.g. its registration has been removed)
-    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceData = WTFMove(*sourceData)](bool success, auto& contextConnection) mutable {
-        if (success)
-            sendToContextProcess(contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorker { destinationIdentifier, WTFMove(message), WTFMove(sourceData) });
+    server().runServiceWorkerIfNecessary(destinationIdentifier, [destinationIdentifier, message = WTFMove(message), sourceData = WTFMove(*sourceData)](auto* contextConnection) mutable {
+        if (contextConnection)
+            sendToContextProcess(*contextConnection, Messages::WebSWContextManagerConnection::PostMessageToServiceWorker { destinationIdentifier, WTFMove(message), WTFMove(sourceData) });
     });
 }
 
@@ -295,12 +295,6 @@ void WebSWServerConnection::unregisterServiceWorkerClient(const ServiceWorkerCli
     m_clientOrigins.remove(iterator);
 }
 
-template<typename U> void WebSWServerConnection::sendToContextProcess(U&& message)
-{
-    if (auto* connection = StorageProcess::singleton().globalServerToContextConnection())
-        connection->send(WTFMove(message));
-}
-
 template<typename U> void WebSWServerConnection::sendToContextProcess(WebCore::SWServerToContextConnection& connection, U&& message)
 {
     static_cast<WebSWServerToContextConnection&>(connection).send(WTFMove(message));