Add support for postMessage buffering between the service worker and window
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Sep 2019 06:10:28 +0000 (06:10 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Sep 2019 06:10:28 +0000 (06:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201169

Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Rebaseline WPT test that is now passing.

* web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https-expected.txt:

Source/WebCore:

As per the Service Worker specification, a service worker client's message
queue is initially disabled and only gets enabled after:
- The DOMContentLoaded event has been fired
or
- The client sets the navigator.serviceWorker.onmessage event handler
or
- navigator.serviceWorker.startMessages() is called

While the message queue is disabled, messages posted by the service worker
to the client simply get queued and only get processed once the queue gets
enabled.

No new tests, rebaselined existing test.

* dom/Document.cpp:
(WebCore::Document::finishedParsing):
Call startMessages() on the ServiceWorkerContainer once the DOMContentLoaded event has
been fired.

* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::ensureServiceWorkerContainer):
* dom/ScriptExecutionContext.h:
* workers/service/SWClientConnection.cpp:
(WebCore::SWClientConnection::postMessageToServiceWorkerClient):
Fix a bug where a service worker would not be able to post a message to a client until
that client has accessed navigator.serviceWorker (since the ServiceWorkerContainer is
lazy initialized). To address the issue, we now initialize the ServiceWorkerContainer
when a message is received from the service worker. Previously, messages were just
getting dropped.

* workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::ServiceWorkerContainer):
When the ServiceWorkerContainer is constructed, suspend its message queue if its context
document is still parsing.

(WebCore::ServiceWorkerContainer::startMessages):
Resume the message queue when startMessages() is called.

(WebCore::ServiceWorkerContainer::postMessage):
Enqueue the event instead of firing it right away.

(WebCore::ServiceWorkerContainer::addEventListener):
if navigator.serviceWorker.onmessage event handler gets set by the JavaScript, call
startMessages().

* workers/service/ServiceWorkerContainer.h:

LayoutTests:

* TestExpectations:
Unskip test that is no longer timing out.

* resources/testharnessreport.js:
(self.testRunner.add_completion_callback):
Use testRunner.forceImmediateCompletion() instead of notifyDone() for WPT tests.
testRunner.notifyDone() does not work in case of load error or when the load
does not finish. The WPT test was timing out because the load does not finish for
testing purposes.

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https-expected.txt
LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt
LayoutTests/platform/win/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt
LayoutTests/resources/testharnessreport.js
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/ScriptExecutionContext.cpp
Source/WebCore/dom/ScriptExecutionContext.h
Source/WebCore/workers/service/SWClientConnection.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.cpp
Source/WebCore/workers/service/ServiceWorkerContainer.h
Tools/DumpRenderTree/mac/DumpRenderTree.mm

index b402d73d9b6a590ca3ee045678a62a95ea99d996..ce714a9af2e4473f767393667fba1041c3616cd8 100644 (file)
@@ -1,3 +1,20 @@
+2019-09-07  Chris Dumez  <cdumez@apple.com>
+
+        Add support for postMessage buffering between the service worker and window
+        https://bugs.webkit.org/show_bug.cgi?id=201169
+
+        Reviewed by Youenn Fablet.
+
+        * TestExpectations:
+        Unskip test that is no longer timing out.
+
+        * resources/testharnessreport.js:
+        (self.testRunner.add_completion_callback):
+        Use testRunner.forceImmediateCompletion() instead of notifyDone() for WPT tests.
+        testRunner.notifyDone() does not work in case of load error or when the load
+        does not finish. The WPT test was timing out because the load does not finish for
+        testing purposes.
+
 2019-09-07  Chris Dumez  <cdumez@apple.com>
 
         Rewrite http/tests/workers/service/serviceworker-private-browsing.https.html as an API test
index 10f3d41fcc8de4dc8cba978866af148ec866b910..9f1434c0b013fbf3cd6637d38795356958624064 100644 (file)
@@ -209,7 +209,6 @@ imported/w3c/web-platform-tests/service-workers/service-worker/fetch-canvas-tain
 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-mixed-content-to-inscope.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-mixed-content-to-outscope.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/multipart-image.https.html [ Skip ]
-imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-using-registration.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https.html [ Skip ]
 imported/w3c/web-platform-tests/service-workers/service-worker/update-not-allowed.https.html [ Skip ]
index cb112536bbc2a4b59c295bb62ccd6e69b6c25916..cb46355bf4d14f7cda23174ee1b305e90f87893e 100644 (file)
@@ -1,3 +1,14 @@
+2019-09-07  Chris Dumez  <cdumez@apple.com>
+
+        Add support for postMessage buffering between the service worker and window
+        https://bugs.webkit.org/show_bug.cgi?id=201169
+
+        Reviewed by Youenn Fablet.
+
+        Rebaseline WPT test that is now passing.
+
+        * web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https-expected.txt:
+
 2019-09-07  Chris Dumez  <cdumez@apple.com>
 
         [Service Workers] Drop support for registration resurrection
index 0e890b039f37fe9ae08d88e43c52a008d0276761..25aadba004bb79936bac71f61261d240ecfb35f8 100644 (file)
@@ -1,9 +1,7 @@
 
-Harness Error (TIMEOUT), message = null
-
-TIMEOUT Messages from ServiceWorker to Client only received after DOMContentLoaded event. Test timed out
-NOTRUN Messages from ServiceWorker to Client only received after calling startMessages(). 
-NOTRUN Messages from ServiceWorker to Client only received after setting onmessage. 
-NOTRUN Microtasks run before dispatching messages after calling startMessages(). 
-NOTRUN Microtasks run before dispatching messages after setting onmessage. 
+PASS Messages from ServiceWorker to Client only received after DOMContentLoaded event. 
+PASS Messages from ServiceWorker to Client only received after calling startMessages(). 
+PASS Messages from ServiceWorker to Client only received after setting onmessage. 
+PASS Microtasks run before dispatching messages after calling startMessages(). 
+PASS Microtasks run before dispatching messages after setting onmessage. 
 
index 2e5ea8374185870d58593fa3bb76d7b8d9b7e7f0..951dfbac9caeef872090ac357f1c4c0c1e679eaa 100644 (file)
@@ -1,4 +1,3 @@
-CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click')
 
 
 FAIL Do not navigate to 404 for anchor with download assert_unreached: Navigated instead of downloading Reached unreachable code
index 2e5ea8374185870d58593fa3bb76d7b8d9b7e7f0..951dfbac9caeef872090ac357f1c4c0c1e679eaa 100644 (file)
@@ -1,4 +1,3 @@
-CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click')
 
 
 FAIL Do not navigate to 404 for anchor with download assert_unreached: Navigated instead of downloading Reached unreachable code
index 9f5ae2c415e19e9ea3c9280594320e1181dda16f..d9842f88f3a80143b673e1520f59174afde8458a 100644 (file)
@@ -98,7 +98,7 @@ if (self.testRunner) {
         // Wait for any other completion callbacks, which may eliminate test elements
         // from the page and therefore reduce the output.
         setTimeout(function () {
-            testRunner.notifyDone();
+            testRunner.forceImmediateCompletion();
         }, 0);
     });
 }
index ff78c5e886703f8ac75112208a099ef8101883ef..263f3f01dbbd5ec68c6cfef86ffd7179e3dd7f41 100644 (file)
@@ -1,3 +1,57 @@
+2019-09-07  Chris Dumez  <cdumez@apple.com>
+
+        Add support for postMessage buffering between the service worker and window
+        https://bugs.webkit.org/show_bug.cgi?id=201169
+
+        Reviewed by Youenn Fablet.
+
+        As per the Service Worker specification, a service worker client's message
+        queue is initially disabled and only gets enabled after:
+        - The DOMContentLoaded event has been fired
+        or
+        - The client sets the navigator.serviceWorker.onmessage event handler
+        or
+        - navigator.serviceWorker.startMessages() is called
+
+        While the message queue is disabled, messages posted by the service worker
+        to the client simply get queued and only get processed once the queue gets
+        enabled.
+
+        No new tests, rebaselined existing test.
+
+        * dom/Document.cpp:
+        (WebCore::Document::finishedParsing):
+        Call startMessages() on the ServiceWorkerContainer once the DOMContentLoaded event has
+        been fired.
+
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::ensureServiceWorkerContainer):
+        * dom/ScriptExecutionContext.h:
+        * workers/service/SWClientConnection.cpp:
+        (WebCore::SWClientConnection::postMessageToServiceWorkerClient):
+        Fix a bug where a service worker would not be able to post a message to a client until
+        that client has accessed navigator.serviceWorker (since the ServiceWorkerContainer is
+        lazy initialized). To address the issue, we now initialize the ServiceWorkerContainer
+        when a message is received from the service worker. Previously, messages were just
+        getting dropped.
+
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::ServiceWorkerContainer):
+        When the ServiceWorkerContainer is constructed, suspend its message queue if its context
+        document is still parsing.
+
+        (WebCore::ServiceWorkerContainer::startMessages):
+        Resume the message queue when startMessages() is called.
+
+        (WebCore::ServiceWorkerContainer::postMessage):
+        Enqueue the event instead of firing it right away.
+
+        (WebCore::ServiceWorkerContainer::addEventListener):
+        if navigator.serviceWorker.onmessage event handler gets set by the JavaScript, call
+        startMessages().
+
+        * workers/service/ServiceWorkerContainer.h:
+
 2019-09-07  Chris Dumez  <cdumez@apple.com>
 
         [Service Workers] Drop support for registration resurrection
index 87383d3ce9a23b6829048af0f83f5e2ddcda2d73..c86aa99fe665fdd44a0817cd803ba07623e94632 100644 (file)
 #include "SegmentedString.h"
 #include "SelectorQuery.h"
 #include "ServiceWorkerClientData.h"
+#include "ServiceWorkerContainer.h"
 #include "ServiceWorkerProvider.h"
 #include "Settings.h"
 #include "ShadowRoot.h"
@@ -5708,6 +5709,14 @@ void Document::finishedParsing()
 
     // Parser should have picked up all speculative preloads by now
     m_cachedResourceLoader->clearPreloads(CachedResourceLoader::ClearPreloadsMode::ClearSpeculativePreloads);
+
+#if ENABLE(SERVICE_WORKER)
+    if (RuntimeEnabledFeatures::sharedFeatures().serviceWorkerEnabled()) {
+        // Stop queuing service worker client messages now that the DOMContentLoaded event has been fired.
+        if (auto* serviceWorkerContainer = this->serviceWorkerContainer())
+            serviceWorkerContainer->startMessages();
+    }
+#endif
 }
 
 void Document::clearSharedObjectPool()
index 04afe9d78ad0bd1028a6703c0fc3d4f653b13de9..a9d820f60421b97c225d5bb5d14508b2dfb951c6 100644 (file)
@@ -612,6 +612,18 @@ ServiceWorkerContainer* ScriptExecutionContext::serviceWorkerContainer()
     return navigator ? &navigator->serviceWorker() : nullptr;
 }
 
+ServiceWorkerContainer* ScriptExecutionContext::ensureServiceWorkerContainer()
+{
+    NavigatorBase* navigator = nullptr;
+    if (is<Document>(*this)) {
+        if (auto* window = downcast<Document>(*this).domWindow())
+            navigator = &window->navigator();
+    } else
+        navigator = &downcast<WorkerGlobalScope>(*this).navigator();
+        
+    return navigator ? &navigator->serviceWorker() : nullptr;
+}
+
 bool ScriptExecutionContext::postTaskTo(const DocumentOrWorkerIdentifier& contextIdentifier, WTF::Function<void(ScriptExecutionContext&)>&& task)
 {
     ASSERT(isMainThread());
index 22587ef58dc65313dbe72c48e7b4ae90e68e167d..842262009e6dee201f95a1a0afdf501ac4a81452 100644 (file)
@@ -255,6 +255,7 @@ public:
     ServiceWorker* serviceWorker(ServiceWorkerIdentifier identifier) { return m_serviceWorkers.get(identifier); }
 
     ServiceWorkerContainer* serviceWorkerContainer();
+    ServiceWorkerContainer* ensureServiceWorkerContainer();
 
     WEBCORE_EXPORT static bool postTaskTo(const DocumentOrWorkerIdentifier&, WTF::Function<void(ScriptExecutionContext&)>&&);
 #endif
index 2fb830bdb5b0d48a9416424db579ad05d23dc183..860a82aba266efefa1203ffebf8c447e3858d037 100644 (file)
@@ -123,10 +123,8 @@ void SWClientConnection::postMessageToServiceWorkerClient(DocumentIdentifier des
     if (!destinationDocument)
         return;
 
-    destinationDocument->postTask([message = WTFMove(message), sourceData = WTFMove(sourceData), sourceOrigin = WTFMove(sourceOrigin)](auto& context) mutable {
-        if (auto* container = context.serviceWorkerContainer())
-            container->postMessage(WTFMove(message), WTFMove(sourceData), WTFMove(sourceOrigin));
-    });
+    if (auto* container = destinationDocument->ensureServiceWorkerContainer())
+        container->postMessage(WTFMove(message), WTFMove(sourceData), WTFMove(sourceOrigin));
 }
 
 void SWClientConnection::updateRegistrationState(ServiceWorkerRegistrationIdentifier identifier, ServiceWorkerRegistrationState state, const Optional<ServiceWorkerData>& serviceWorkerData)
index 4c68b90d60d03fa4db7b168071637d6cc2719eb9..949bbe5e3eaf7f9c315e92a1b8c62004390caf03 100644 (file)
@@ -70,8 +70,13 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(ServiceWorkerContainer);
 ServiceWorkerContainer::ServiceWorkerContainer(ScriptExecutionContext* context, NavigatorBase& navigator)
     : ActiveDOMObject(context)
     , m_navigator(navigator)
+    , m_messageQueue(*this)
 {
     suspendIfNeeded();
+    
+    // We should queue messages until the DOMContentLoaded event has fired or startMessages() has been called.
+    if (is<Document>(context) && downcast<Document>(*context).parsing())
+        m_messageQueue.suspend();
 }
 
 ServiceWorkerContainer::~ServiceWorkerContainer()
@@ -310,6 +315,7 @@ void ServiceWorkerContainer::getRegistrations(Ref<DeferredPromise>&& promise)
 
 void ServiceWorkerContainer::startMessages()
 {
+    m_messageQueue.resume();
 }
 
 void ServiceWorkerContainer::jobFailedWithException(ServiceWorkerJob& job, const Exception& exception)
@@ -407,7 +413,8 @@ void ServiceWorkerContainer::postMessage(MessageWithMessagePorts&& message, Serv
     MessageEventSource source = RefPtr<ServiceWorker> { ServiceWorker::getOrCreate(context, WTFMove(sourceData)) };
 
     auto messageEvent = MessageEvent::create(MessagePort::entanglePorts(context, WTFMove(message.transferredPorts)), message.message.releaseNonNull(), sourceOrigin, { }, WTFMove(source));
-    dispatchEvent(messageEvent);
+    
+    m_messageQueue.enqueueEvent(WTFMove(messageEvent));
 }
 
 void ServiceWorkerContainer::notifyRegistrationIsSettled(const ServiceWorkerRegistrationKey& registrationKey)
@@ -517,6 +524,16 @@ bool ServiceWorkerContainer::canSuspendForDocumentSuspension() const
     return !hasPendingActivity();
 }
 
+void ServiceWorkerContainer::suspend(ReasonForSuspension)
+{
+    m_messageQueue.suspend();
+}
+
+void ServiceWorkerContainer::resume()
+{
+    m_messageQueue.resume();
+}
+
 SWClientConnection& ServiceWorkerContainer::ensureSWClientConnection()
 {
     ASSERT(scriptExecutionContext());
@@ -568,6 +585,7 @@ void ServiceWorkerContainer::stop()
     m_isStopped = true;
     removeAllEventListeners();
     m_readyPromise = nullptr;
+    m_messageQueue.close();
     auto jobMap = WTFMove(m_jobMap);
     for (auto& ongoingJob : jobMap.values()) {
         if (ongoingJob.job->cancelPendingLoad())
@@ -612,6 +630,16 @@ bool ServiceWorkerContainer::isAlwaysOnLoggingAllowed() const
     return false;
 }
 
+bool ServiceWorkerContainer::addEventListener(const AtomString& eventType, Ref<EventListener>&& eventListener, const AddEventListenerOptions& options)
+{
+    // Setting the onmessage EventHandler attribute on the ServiceWorkerContainer should start the messages
+    // automatically.
+    if (eventListener->isAttribute() && eventType == eventNames().messageEvent)
+        startMessages();
+
+    return EventTargetWithInlineData::addEventListener(eventType, WTFMove(eventListener), options);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(SERVICE_WORKER)
index 507451fa0e1a0dda998930a395f6884ecd41c5fa..ec563a0ab8fae77fdf36a7da96ff2412fdfb0ab2 100644 (file)
@@ -30,6 +30,7 @@
 #include "ActiveDOMObject.h"
 #include "DOMPromiseProxy.h"
 #include "EventTarget.h"
+#include "GenericEventQueue.h"
 #include "SWClientConnection.h"
 #include "SWServer.h"
 #include "ServiceWorkerJobClient.h"
@@ -90,6 +91,8 @@ public:
     NavigatorBase* navigator() { return &m_navigator; }
 
 private:
+    bool addEventListener(const AtomString& eventType, Ref<EventListener>&&, const AddEventListenerOptions& = { }) final;
+
     void scheduleJob(std::unique_ptr<ServiceWorkerJob>&&);
 
     void jobFailedWithException(ServiceWorkerJob&, const Exception&) final;
@@ -106,8 +109,12 @@ private:
 
     SWClientConnection& ensureSWClientConnection();
 
+    // ActiveDOMObject.
     const char* activeDOMObjectName() const final;
     bool canSuspendForDocumentSuspension() const final;
+    void suspend(ReasonForSuspension) final;
+    void resume() final;
+    
     ScriptExecutionContext* scriptExecutionContext() const final { return ActiveDOMObject::scriptExecutionContext(); }
     EventTargetInterface eventTargetInterface() const final { return ServiceWorkerContainerEventTargetInterfaceType; }
     void refEventTarget() final;
@@ -137,6 +144,7 @@ private:
 
     uint64_t m_lastOngoingSettledRegistrationIdentifier { 0 };
     HashMap<uint64_t, ServiceWorkerRegistrationKey> m_ongoingSettledRegistrations;
+    GenericEventQueue m_messageQueue;
 
 };
 
index a2a87e7ddceb51370785bca18b7fc461e41b90c9..47c0f8858185615d4c512afedc50821896a2894a 100644 (file)
@@ -1725,6 +1725,9 @@ void dump()
     WebThreadLock();
 #endif
 
+    if (done)
+        return;
+
     updateDisplay();
 
     invalidateAnyPreviousWaitToDumpWatchdog();