Title: [249353] trunk
Revision
249353
Author
cdu...@apple.com
Date
2019-08-30 17:34:07 -0700 (Fri, 30 Aug 2019)

Log Message

Add support for postMessage buffering between the service worker and window
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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (249352 => 249353)


--- trunk/LayoutTests/ChangeLog	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/ChangeLog	2019-08-31 00:34:07 UTC (rev 249353)
@@ -1,3 +1,20 @@
+2019-08-30  Chris Dumez  <cdu...@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  Saam Barati  <sbar...@apple.com>
 
         [WHLSL] Remove getters/setters/anders

Modified: trunk/LayoutTests/TestExpectations (249352 => 249353)


--- trunk/LayoutTests/TestExpectations	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/TestExpectations	2019-08-31 00:34:07 UTC (rev 249353)
@@ -208,7 +208,6 @@
 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 ]

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (249352 => 249353)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2019-08-31 00:34:07 UTC (rev 249353)
@@ -1,3 +1,14 @@
+2019-08-30  Chris Dumez  <cdu...@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  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r249338.

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https-expected.txt (249352 => 249353)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https-expected.txt	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-to-client-message-queue.https-expected.txt	2019-08-31 00:34:07 UTC (rev 249353)
@@ -1,9 +1,7 @@
 
-Harness Error (TIMEOUT), message = null
+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. 
 
-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. 
-

Modified: trunk/LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt (249352 => 249353)


--- trunk/LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt	2019-08-31 00:34:07 UTC (rev 249353)
@@ -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

Modified: trunk/LayoutTests/platform/win/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt (249352 => 249353)


--- trunk/LayoutTests/platform/win/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/platform/win/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt	2019-08-31 00:34:07 UTC (rev 249353)
@@ -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

Modified: trunk/LayoutTests/resources/testharnessreport.js (249352 => 249353)


--- trunk/LayoutTests/resources/testharnessreport.js	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/LayoutTests/resources/testharnessreport.js	2019-08-31 00:34:07 UTC (rev 249353)
@@ -98,7 +98,7 @@
         // 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);
     });
 }

Modified: trunk/Source/WebCore/ChangeLog (249352 => 249353)


--- trunk/Source/WebCore/ChangeLog	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/ChangeLog	2019-08-31 00:34:07 UTC (rev 249353)
@@ -1,3 +1,57 @@
+2019-08-30  Chris Dumez  <cdu...@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.fra...@apple.com>
 
         Minor optimization in determineNonLayerDescendantsPaintedContent()

Modified: trunk/Source/WebCore/dom/Document.cpp (249352 => 249353)


--- trunk/Source/WebCore/dom/Document.cpp	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/dom/Document.cpp	2019-08-31 00:34:07 UTC (rev 249353)
@@ -193,6 +193,7 @@
 #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 @@
 
     // 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()

Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (249352 => 249353)


--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp	2019-08-31 00:34:07 UTC (rev 249353)
@@ -612,6 +612,18 @@
     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());

Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.h (249352 => 249353)


--- trunk/Source/WebCore/dom/ScriptExecutionContext.h	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.h	2019-08-31 00:34:07 UTC (rev 249353)
@@ -255,6 +255,7 @@
     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

Modified: trunk/Source/WebCore/workers/service/SWClientConnection.cpp (249352 => 249353)


--- trunk/Source/WebCore/workers/service/SWClientConnection.cpp	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/workers/service/SWClientConnection.cpp	2019-08-31 00:34:07 UTC (rev 249353)
@@ -125,10 +125,8 @@
     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)

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp (249352 => 249353)


--- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp	2019-08-31 00:34:07 UTC (rev 249353)
@@ -64,8 +64,13 @@
 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::startMessages()
 {
+    m_messageQueue.resume();
 }
 
 void ServiceWorkerContainer::jobFailedWithException(ServiceWorkerJob& job, const Exception& exception)
@@ -475,7 +481,8 @@
     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 @@
     return !hasPendingActivity();
 }
 
+void ServiceWorkerContainer::suspend(ReasonForSuspension)
+{
+    m_messageQueue.suspend();
+}
+
+void ServiceWorkerContainer::resume()
+{
+    m_messageQueue.resume();
+}
+
 SWClientConnection& ServiceWorkerContainer::ensureSWClientConnection()
 {
     ASSERT(scriptExecutionContext());
@@ -636,6 +653,7 @@
     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 @@
     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)

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h (249352 => 249353)


--- trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerContainer.h	2019-08-31 00:34:07 UTC (rev 249353)
@@ -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 @@
     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 @@
 
     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 @@
 
     uint64_t m_lastOngoingSettledRegistrationIdentifier { 0 };
     HashMap<uint64_t, ServiceWorkerRegistrationKey> m_ongoingSettledRegistrations;
+    GenericEventQueue m_messageQueue;
 
 };
 

Modified: trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm (249352 => 249353)


--- trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm	2019-08-31 00:29:02 UTC (rev 249352)
+++ trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm	2019-08-31 00:34:07 UTC (rev 249353)
@@ -1726,6 +1726,9 @@
     WebThreadLock();
 #endif
 
+    if (done)
+        return;
+
     updateDisplay();
 
     invalidateAnyPreviousWaitToDumpWatchdog();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to