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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@252300 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index cad813c..9053f4f 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+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>
 
         XMLHttpRequestUpload should be exposed in dedicated workers
diff --git a/Source/WebCore/workers/service/server/SWServer.cpp b/Source/WebCore/workers/service/server/SWServer.cpp
index dbe9a30..c764baa 100644
--- a/Source/WebCore/workers/service/server/SWServer.cpp
+++ b/Source/WebCore/workers/service/server/SWServer.cpp
@@ -445,7 +445,7 @@
         RELEASE_LOG_ERROR(ServiceWorker, "%p - SWServer::didFinishInstall: Failed SW install for job %s", this, jobDataIdentifier->loggingString().utf8().data());
 
     if (auto* jobQueue = m_jobQueues.get(worker.registrationKey()))
-        jobQueue->didFinishInstall(*jobDataIdentifier, worker.identifier(), wasSuccessful);
+        jobQueue->didFinishInstall(*jobDataIdentifier, worker, wasSuccessful);
 }
 
 void SWServer::didFinishActivation(SWServerWorker& worker)
diff --git a/Source/WebCore/workers/service/server/SWServerJobQueue.cpp b/Source/WebCore/workers/service/server/SWServerJobQueue.cpp
index 6d7cda0..487cc75 100644
--- a/Source/WebCore/workers/service/server/SWServerJobQueue.cpp
+++ b/Source/WebCore/workers/service/server/SWServerJobQueue.cpp
@@ -177,25 +177,21 @@
 }
 
 // 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.
         if (!registration->getNewestWorker())
@@ -211,12 +207,9 @@
         registration->updateWorkerState(*waitingWorker, ServiceWorkerState::Redundant);
     }
 
-    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();
 
diff --git a/Source/WebCore/workers/service/server/SWServerJobQueue.h b/Source/WebCore/workers/service/server/SWServerJobQueue.h
index 75a0cbd..0a9777e 100644
--- a/Source/WebCore/workers/service/server/SWServerJobQueue.h
+++ b/Source/WebCore/workers/service/server/SWServerJobQueue.h
@@ -34,6 +34,8 @@
 
 namespace WebCore {
 
+class SWServerWorker;
+
 class SWServerJobQueue {
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -51,7 +53,7 @@
     void scriptFetchFinished(SWServer::Connection&, const ServiceWorkerFetchResult&);
     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);
     void cancelJobsFromServiceWorker(ServiceWorkerIdentifier);
diff --git a/Source/WebCore/workers/service/server/SWServerWorker.cpp b/Source/WebCore/workers/service/server/SWServerWorker.cpp
index 37e5700..655d70d 100644
--- a/Source/WebCore/workers/service/server/SWServerWorker.cpp
+++ b/Source/WebCore/workers/service/server/SWServerWorker.cpp
@@ -118,14 +118,24 @@
 
 void SWServerWorker::didFinishInstall(const Optional<ServiceWorkerJobDataIdentifier>& jobDataIdentifier, bool wasSuccessful)
 {
+    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);
 }
 
 void SWServerWorker::didFinishActivation()
 {
+    auto state = this->state();
+    if (state == ServiceWorkerState::Redundant)
+        return;
+
     ASSERT(m_server);
+    RELEASE_ASSERT(state == ServiceWorkerState::Activating);
     if (m_server)
         m_server->didFinishActivation(*this);
 }