Changeset 252300 in webkit

Timestamp:

Nov 8, 2019 7:02:59 PM (5 years ago)

Author:

youenn@apple.com

Message:

SWServerWorker in redundant state do not need to process messages from the context process
​https://bugs.webkit.org/show_bug.cgi?id=204019

Reviewed by Chris Dumez.

When a service worker is marked as redundant in network process, it might still receive some messages like didFinishActivation or didFinishInstall.
In that state, we do not need to process these messages.
Did some refactoring to directly pass the service worker to the job queue.
This makes it clear the registration and service worker are the correct ones.

• workers/service/server/SWServer.cpp:

(WebCore::SWServer::didFinishInstall):
Pass the worker instead of its identifier.

• workers/service/server/SWServerJobQueue.cpp:

(WebCore::SWServerJobQueue::didFinishInstall):
Now that we are given the service worker, get its registration directly.

• workers/service/server/SWServerJobQueue.h:
• workers/service/server/SWServerWorker.cpp:

(WebCore::SWServerWorker::didFinishInstall):
Exit early in redundant state.
(WebCore::SWServerWorker::didFinishActivation):
Exit early in redundant state.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• workers/service/server/SWServer.cpp (modified) (1 diff)
• workers/service/server/SWServerJobQueue.cpp (modified) (2 diffs)
• workers/service/server/SWServerJobQueue.h (modified) (2 diffs)
• workers/service/server/SWServerWorker.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-11-08  Youenn Fablet  <youenn@apple.com>
+
+        SWServerWorker in redundant state do not need to process messages from the context process
+        https://bugs.webkit.org/show_bug.cgi?id=204019
+
+        Reviewed by Chris Dumez.
+
+        When a service worker is marked as redundant in network process, it might still receive some messages like didFinishActivation or didFinishInstall.
+        In that state, we do not need to process these messages.
+        Did some refactoring to directly pass the service worker to the job queue.
+        This makes it clear the registration and service worker are the correct ones.
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::didFinishInstall):
+        Pass the worker instead of its identifier.
+        * workers/service/server/SWServerJobQueue.cpp:
+        (WebCore::SWServerJobQueue::didFinishInstall):
+        Now that we are given the service worker, get its registration directly.
+        * workers/service/server/SWServerJobQueue.h:
+        * workers/service/server/SWServerWorker.cpp:
+        (WebCore::SWServerWorker::didFinishInstall):
+        Exit early in redundant state.
+        (WebCore::SWServerWorker::didFinishActivation):
+        Exit early in redundant state.
+
 2019-11-08  youenn fablet  <youenn@apple.com>
 

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

@@ -446 +446 @@
 
     if (auto* jobQueue = m_jobQueues.get(worker.registrationKey()))
-        jobQueue->didFinishInstall(*jobDataIdentifier, worker.identifier(), wasSuccessful);
+        jobQueue->didFinishInstall(*jobDataIdentifier, worker, wasSuccessful);
 }
 

trunk/Source/WebCore/workers/service/server/SWServerJobQueue.cpp

@@ -178 +178 @@
 
 // https://w3c.github.io/ServiceWorker/#install
-void SWServerJobQueue::didFinishInstall(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, ServiceWorkerIdentifier identifier, bool wasSuccessful)
+void SWServerJobQueue::didFinishInstall(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, SWServerWorker& worker, bool wasSuccessful)
 {
     if (!isCurrentlyProcessingJob(jobDataIdentifier))
         return;
 
-    auto* registration = m_server.getRegistration(m_registrationKey);
+    auto* registration = worker.registration();
     ASSERT(registration);
-    ASSERT(registration->installingWorker());
-    ASSERT(registration->installingWorker()->identifier() == identifier);
+    ASSERT(registration->installingWorker() == &worker);
 
     if (!wasSuccessful) {
-        RefPtr<SWServerWorker> worker = m_server.workerByID(identifier);
-        RELEASE_ASSERT(worker);
-
-        worker->terminate();
+        worker.terminate();
         // Run the Update Registration State algorithm passing registration, "installing" and null as the arguments.
         registration->updateRegistrationState(ServiceWorkerRegistrationState::Installing, nullptr);
         // Run the Update Worker State algorithm passing registration's installing worker and redundant as the arguments.
-        registration->updateWorkerState(*worker, ServiceWorkerState::Redundant);
+        registration->updateWorkerState(worker, ServiceWorkerState::Redundant);
 
         // If newestWorker is null, invoke Clear Registration algorithm passing registration as its argument.
@@ -212 +208 @@
     }
 
-    auto* installing = registration->installingWorker();
-    ASSERT(installing);
-
-    registration->updateRegistrationState(ServiceWorkerRegistrationState::Waiting, installing);
+    registration->updateRegistrationState(ServiceWorkerRegistrationState::Waiting, &worker);
     registration->updateRegistrationState(ServiceWorkerRegistrationState::Installing, nullptr);
-    registration->updateWorkerState(*installing, ServiceWorkerState::Installed);
+    registration->updateWorkerState(worker, ServiceWorkerState::Installed);
 
     finishCurrentJob();

trunk/Source/WebCore/workers/service/server/SWServerJobQueue.h

@@ -35 +35 @@
 namespace WebCore {
 
+class SWServerWorker;
+
 class SWServerJobQueue {
     WTF_MAKE_FAST_ALLOCATED;
@@ -52 +54 @@
     void scriptContextFailedToStart(const ServiceWorkerJobDataIdentifier&, ServiceWorkerIdentifier, const String& message);
     void scriptContextStarted(const ServiceWorkerJobDataIdentifier&, ServiceWorkerIdentifier);
-    void didFinishInstall(const ServiceWorkerJobDataIdentifier&, ServiceWorkerIdentifier, bool wasSuccessful);
+    void didFinishInstall(const ServiceWorkerJobDataIdentifier&, SWServerWorker&, bool wasSuccessful);
     void didResolveRegistrationPromise();
     void cancelJobsFromConnection(SWServerConnectionIdentifier);

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

@@ -119 +119 @@
 void SWServerWorker::didFinishInstall(const Optional<ServiceWorkerJobDataIdentifier>& jobDataIdentifier, bool wasSuccessful)
 {
-    ASSERT(m_server);
+    auto state = this->state();
+    if (state == ServiceWorkerState::Redundant)
+        return;
+
+    ASSERT(m_server);
+    RELEASE_ASSERT(state == ServiceWorkerState::Installing);
     if (m_server)
         m_server->didFinishInstall(jobDataIdentifier, *this, wasSuccessful);
@@ -126 +131 @@
 void SWServerWorker::didFinishActivation()
 {
-    ASSERT(m_server);
+    auto state = this->state();
+    if (state == ServiceWorkerState::Redundant)
+        return;
+
+    ASSERT(m_server);
+    RELEASE_ASSERT(state == ServiceWorkerState::Activating);
     if (m_server)
         m_server->didFinishActivation(*this);