Get rid of microtask in ServiceWorkerContainer::jobResolvedWithRegistration()

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

Reviewed by Youenn Fablet.

Get rid of microtask in ServiceWorkerContainer::jobResolvedWithRegistration(). It
is no longer needed and MicrotaskQueue::mainThreadQueue() is only safe to use from
the main thread, as its name suggest. ServiceWorkerContainer are also instantiated
in Service worker threads nowadays.

* workers/service/SWClientConnection.cpp:
(WebCore::SWClientConnection::registrationJobResolvedInServer):
* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
* workers/service/ServiceWorkerContainer.h:
* workers/service/ServiceWorkerJob.cpp:
(WebCore::ServiceWorkerJob::resolvedWithRegistration):
* workers/service/ServiceWorkerJob.h:
* workers/service/ServiceWorkerJobClient.h:

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index f1638b6..b01d6fc 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2017-12-01  Chris Dumez  <cdumez@apple.com>
+
+        Get rid of microtask in ServiceWorkerContainer::jobResolvedWithRegistration()
+        https://bugs.webkit.org/show_bug.cgi?id=180263
+
+        Reviewed by Youenn Fablet.
+
+        Get rid of microtask in ServiceWorkerContainer::jobResolvedWithRegistration(). It
+        is no longer needed and MicrotaskQueue::mainThreadQueue() is only safe to use from
+        the main thread, as its name suggest. ServiceWorkerContainer are also instantiated
+        in Service worker threads nowadays.
+
+        * workers/service/SWClientConnection.cpp:
+        (WebCore::SWClientConnection::registrationJobResolvedInServer):
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
+        * workers/service/ServiceWorkerContainer.h:
+        * workers/service/ServiceWorkerJob.cpp:
+        (WebCore::ServiceWorkerJob::resolvedWithRegistration):
+        * workers/service/ServiceWorkerJob.h:
+        * workers/service/ServiceWorkerJobClient.h:
+
 2017-12-01  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Free FontFaceSets may include fonts that were never actually added to them

diff --git a/Source/WebCore/workers/service/SWClientConnection.cpp b/Source/WebCore/workers/service/SWClientConnection.cpp
index 48f7e4b..ec50f65 100644 (file)
--- a/Source/WebCore/workers/service/SWClientConnection.cpp
+++ b/Source/WebCore/workers/service/SWClientConnection.cpp
@@ -87,10 +87,7 @@ void SWClientConnection::registrationJobResolvedInServer(const ServiceWorkerJobD
     }
 
     auto key = registrationData.key;
-    job->resolvedWithRegistration(WTFMove(registrationData), [this, protectedThis = makeRef(*this), key, shouldNotifyWhenResolved] {
-        if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes)
-            didResolveRegistrationPromise(key);
-    });
+    job->resolvedWithRegistration(WTFMove(registrationData), shouldNotifyWhenResolved);
 }
 
 void SWClientConnection::unregistrationJobResolvedInServer(const ServiceWorkerJobDataIdentifier& jobDataIdentifier, bool unregistrationResult)

diff --git a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
index 8beb086..27df9a0 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
+++ b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
@@ -35,7 +35,6 @@
 #include "JSDOMPromiseDeferred.h"
 #include "JSServiceWorkerRegistration.h"
 #include "Logging.h"
-#include "Microtasks.h"
 #include "NavigatorBase.h"
 #include "ResourceError.h"
 #include "ScriptExecutionContext.h"
@@ -308,20 +307,29 @@ void ServiceWorkerContainer::scheduleTaskToFireUpdateFoundEvent(ServiceWorkerReg
         registration->scheduleTaskToFireUpdateFoundEvent();
 }
 
-void ServiceWorkerContainer::jobResolvedWithRegistration(ServiceWorkerJob& job, ServiceWorkerRegistrationData&& data, WTF::Function<void()>&& promiseResolvedHandler)
+void ServiceWorkerContainer::jobResolvedWithRegistration(ServiceWorkerJob& job, ServiceWorkerRegistrationData&& data, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
 {
     auto guard = WTF::makeScopeExit([this, &job] {
         jobDidFinish(job);
     });
 
+    WTF::Function<void()> notifyWhenResolvedIfNeeded = [] { };
+    if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes) {
+        notifyWhenResolvedIfNeeded = [connection = m_swConnection, registrationKey = data.key.isolatedCopy()]() mutable {
+            callOnMainThread([connection = WTFMove(connection), registrationKey = WTFMove(registrationKey)] {
+                connection->didResolveRegistrationPromise(registrationKey);
+            });
+        };
+    }
+
     if (isStopped()) {
-        promiseResolvedHandler();
+        notifyWhenResolvedIfNeeded();
         return;
     }
 
-    scriptExecutionContext()->postTask([this, protectedThis = makeRef(*this), job = makeRef(job), data = WTFMove(data), promiseResolvedHandler = WTFMove(promiseResolvedHandler)](ScriptExecutionContext& context) mutable {
+    scriptExecutionContext()->postTask([this, protectedThis = makeRef(*this), job = makeRef(job), data = WTFMove(data), notifyWhenResolvedIfNeeded = WTFMove(notifyWhenResolvedIfNeeded)](ScriptExecutionContext& context) mutable {
         if (isStopped()) {
-            promiseResolvedHandler();
+            notifyWhenResolvedIfNeeded();
             return;
         }
 
@@ -331,9 +339,7 @@ void ServiceWorkerContainer::jobResolvedWithRegistration(ServiceWorkerJob& job,
 
         job->promise().resolve<IDLInterface<ServiceWorkerRegistration>>(WTFMove(registration));
 
-        MicrotaskQueue::mainThreadQueue().append(std::make_unique<VoidMicrotask>([promiseResolvedHandler = WTFMove(promiseResolvedHandler)] {
-            promiseResolvedHandler();
-        }));
+        notifyWhenResolvedIfNeeded();
     });
 }
 

diff --git a/Source/WebCore/workers/service/ServiceWorkerContainer.h b/Source/WebCore/workers/service/ServiceWorkerContainer.h
index 285c681..812af34 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerContainer.h
+++ b/Source/WebCore/workers/service/ServiceWorkerContainer.h
@@ -87,7 +87,7 @@ private:
     void scheduleJob(Ref<ServiceWorkerJob>&&);
 
     void jobFailedWithException(ServiceWorkerJob&, const Exception&) final;
-    void jobResolvedWithRegistration(ServiceWorkerJob&, ServiceWorkerRegistrationData&&, WTF::Function<void()>&& promiseResolvedHandler) final;
+    void jobResolvedWithRegistration(ServiceWorkerJob&, ServiceWorkerRegistrationData&&, ShouldNotifyWhenResolved) final;
     void jobResolvedWithUnregistrationResult(ServiceWorkerJob&, bool unregistrationResult) final;
     void startScriptFetchForJob(ServiceWorkerJob&) final;
     void jobFinishedLoadingScript(ServiceWorkerJob&, const String&) final;

diff --git a/Source/WebCore/workers/service/ServiceWorkerJob.cpp b/Source/WebCore/workers/service/ServiceWorkerJob.cpp
index 7c52695..8336a78 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerJob.cpp
+++ b/Source/WebCore/workers/service/ServiceWorkerJob.cpp
@@ -58,13 +58,13 @@ void ServiceWorkerJob::failedWithException(const Exception& exception)
     m_client->jobFailedWithException(*this, exception);
 }
 
-void ServiceWorkerJob::resolvedWithRegistration(ServiceWorkerRegistrationData&& data, WTF::Function<void()>&& promiseResolvedHandler)
+void ServiceWorkerJob::resolvedWithRegistration(ServiceWorkerRegistrationData&& data, ShouldNotifyWhenResolved shouldNotifyWhenResolved)
 {
     ASSERT(currentThread() == m_creationThread);
     ASSERT(!m_completed);
 
     m_completed = true;
-    m_client->jobResolvedWithRegistration(*this, WTFMove(data), WTFMove(promiseResolvedHandler));
+    m_client->jobResolvedWithRegistration(*this, WTFMove(data), shouldNotifyWhenResolved);
 }
 
 void ServiceWorkerJob::resolvedWithUnregistrationResult(bool unregistrationResult)

diff --git a/Source/WebCore/workers/service/ServiceWorkerJob.h b/Source/WebCore/workers/service/ServiceWorkerJob.h
index 4c578af..c8a970a 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerJob.h
+++ b/Source/WebCore/workers/service/ServiceWorkerJob.h
@@ -56,7 +56,7 @@ public:
     WEBCORE_EXPORT ~ServiceWorkerJob();
 
     void failedWithException(const Exception&);
-    void resolvedWithRegistration(ServiceWorkerRegistrationData&&, WTF::Function<void()>&& promiseResolvedHandler);
+    void resolvedWithRegistration(ServiceWorkerRegistrationData&&, ShouldNotifyWhenResolved);
     void resolvedWithUnregistrationResult(bool);
     void startScriptFetch();
 

diff --git a/Source/WebCore/workers/service/ServiceWorkerJobClient.h b/Source/WebCore/workers/service/ServiceWorkerJobClient.h
index 02a8c44..415a69c 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerJobClient.h
+++ b/Source/WebCore/workers/service/ServiceWorkerJobClient.h
@@ -42,7 +42,7 @@ public:
     virtual ~ServiceWorkerJobClient() = default;
 
     virtual void jobFailedWithException(ServiceWorkerJob&, const Exception&) = 0;
-    virtual void jobResolvedWithRegistration(ServiceWorkerJob&, ServiceWorkerRegistrationData&&, WTF::Function<void()>&& promiseResolvedHandler) = 0;
+    virtual void jobResolvedWithRegistration(ServiceWorkerJob&, ServiceWorkerRegistrationData&&, ShouldNotifyWhenResolved) = 0;
     virtual void jobResolvedWithUnregistrationResult(ServiceWorkerJob&, bool unregistrationResult) = 0;
     virtual void startScriptFetchForJob(ServiceWorkerJob&) = 0;
     virtual void jobFinishedLoadingScript(ServiceWorkerJob&, const String&) = 0;