Add support for postMessage buffering between the service worker and window
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Aug 2019 19:03:35 +0000 (19:03 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Aug 2019 19:03:35 +0000 (19:03 +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@249338 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 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/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

index b994397..f3f0fb5 100644 (file)
@@ -1,3 +1,20 @@
+2019-08-30  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-08-30  Devin Rousso  <drousso@apple.com>
 
         Unreviewed, fix test failure after r249305
index 6ea5e55..585d479 100644 (file)
@@ -208,7 +208,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 ff8ceeb..037895e 100644 (file)
@@ -1,3 +1,14 @@
+2019-08-30  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-08-30  Youenn Fablet  <youenn@apple.com>
 
         Bind WPT server hostname
index 0e890b0..25aadba 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 2e5ea83..951dfba 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 9f5ae2c..d9842f8 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 a7bc10c..edc5263 100644 (file)
@@ -1,3 +1,57 @@
+2019-08-30  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-08-30  Simon Fraser  <simon.fraser@apple.com>
 
         Add system tracing points for compositing updates, and touch-event dispatching
index 149ae27..596f075 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 04afe9d..a9d820f 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 22587ef..8422620 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 9a3140e..fd171a9 100644 (file)
@@ -125,10 +125,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 59c71c4..03b27fb 100644 (file)
@@ -64,8 +64,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()
@@ -378,6 +383,7 @@ void ServiceWorkerContainer::didFinishGetRegistrationsRequest(uint64_t pendingPr
 
 void ServiceWorkerContainer::startMessages()
 {
+    m_messageQueue.resume();
 }
 
 void ServiceWorkerContainer::jobFailedWithException(ServiceWorkerJob& job, const Exception& exception)
@@ -475,7 +481,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)
@@ -585,6 +592,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());
@@ -636,6 +653,7 @@ void ServiceWorkerContainer::stop()
     removeAllEventListeners();
     m_pendingPromises.clear();
     m_readyPromise = nullptr;
+    m_messageQueue.close();
     auto jobMap = WTFMove(m_jobMap);
     for (auto& ongoingJob : jobMap.values()) {
         if (ongoingJob.job->cancelPendingLoad())
@@ -680,6 +698,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 3bd6b9b..db27f43 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;
@@ -109,8 +112,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;
@@ -154,6 +161,7 @@ private:
 
     uint64_t m_lastOngoingSettledRegistrationIdentifier { 0 };
     HashMap<uint64_t, ServiceWorkerRegistrationKey> m_ongoingSettledRegistrations;
+    GenericEventQueue m_messageQueue;
 
 };