Title: [253752] trunk
Revision
253752
Author
cdu...@apple.com
Date
2019-12-19 07:59:40 -0800 (Thu, 19 Dec 2019)

Log Message

imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html is flaky
https://bugs.webkit.org/show_bug.cgi?id=205408

Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Rebaseline test now that it is consistently passing.

* web-platform-tests/service-workers/service-worker/skip-waiting-installed.https-expected.txt:

Source/WebCore:

imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html has been
flaky since it was imported. We now queue a task on the HTML event loop to resolve the skipWaiting promise
so that its ordering is correct, between the active event being fired and the service worker state becoming
"activated".

No new tests, upskipped existing test.

* workers/service/ServiceWorkerGlobalScope.cpp:
(WebCore::ServiceWorkerGlobalScope::skipWaiting):
* workers/service/context/SWContextManager.h:
* workers/service/server/SWServerToContextConnection.cpp:
(WebCore::SWServerToContextConnection::skipWaiting):
* workers/service/server/SWServerToContextConnection.h:

Source/WebKit:

* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
(WebKit::WebSWServerToContextConnection::didFinishSkipWaiting): Deleted.
* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
* NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in:
* WebProcess/Storage/WebSWContextManagerConnection.cpp:
(WebKit::WebSWContextManagerConnection::skipWaiting):
(WebKit::WebSWContextManagerConnection::didFinishSkipWaiting): Deleted.
* WebProcess/Storage/WebSWContextManagerConnection.h:
* WebProcess/Storage/WebSWContextManagerConnection.messages.in:

LayoutTests:

Unskip test.

* TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (253751 => 253752)


--- trunk/LayoutTests/ChangeLog	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/LayoutTests/ChangeLog	2019-12-19 15:59:40 UTC (rev 253752)
@@ -1,3 +1,14 @@
+2019-12-19  Chris Dumez  <cdu...@apple.com>
+
+        imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=205408
+
+        Reviewed by Youenn Fablet.
+
+        Unskip test.
+
+        * TestExpectations:
+
 2019-12-19  Wenson Hsieh  <wenson_hs...@apple.com>
 
         REGRESSION (r251015): Hitting return before a space deletes text after the insertion position

Modified: trunk/LayoutTests/TestExpectations (253751 => 253752)


--- trunk/LayoutTests/TestExpectations	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/LayoutTests/TestExpectations	2019-12-19 15:59:40 UTC (rev 253752)
@@ -275,10 +275,6 @@
 imported/w3c/web-platform-tests/server-timing/navigation_timing_idl.html [ Skip ]
 imported/w3c/web-platform-tests/server-timing/navigation_timing_idl.https.html [ Skip ]
 
-# This test seems wrong as the order of setting ServiceWorker#state to 'activated' and resolving skipWaiting() promise is not deterministic.
-# Run this test to ensure this test does not crash.
-webkit.org/b/188246 imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html [ Pass Failure ]
-
 # Console log lines may appear in a different order so we silence them.
 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html [ DumpJSConsoleLogInStdErr Failure Pass ]
 imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ DumpJSConsoleLogInStdErr ]

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (253751 => 253752)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2019-12-19 15:59:40 UTC (rev 253752)
@@ -1,3 +1,14 @@
+2019-12-19  Chris Dumez  <cdu...@apple.com>
+
+        imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=205408
+
+        Reviewed by Youenn Fablet.
+
+        Rebaseline test now that it is consistently passing.
+
+        * web-platform-tests/service-workers/service-worker/skip-waiting-installed.https-expected.txt:
+
 2019-12-18  Chris Dumez  <cdu...@apple.com>
 
         Resync web-platform-tests/dom tests from upstream

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https-expected.txt (253751 => 253752)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https-expected.txt	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https-expected.txt	2019-12-19 15:59:40 UTC (rev 253752)
@@ -1,4 +1,3 @@
 
+PASS Test skipWaiting when a installed worker is waiting 
 
-FAIL Test skipWaiting when a installed worker is waiting assert_equals: skipWaiting promise should be resolved with undefined expected "PASS" but got "FAIL: Promise should be resolved before ServiceWorker#state is set to activated"
-

Modified: trunk/Source/WebCore/ChangeLog (253751 => 253752)


--- trunk/Source/WebCore/ChangeLog	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebCore/ChangeLog	2019-12-19 15:59:40 UTC (rev 253752)
@@ -1,5 +1,26 @@
 2019-12-19  Chris Dumez  <cdu...@apple.com>
 
+        imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=205408
+
+        Reviewed by Youenn Fablet.
+
+        imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html has been
+        flaky since it was imported. We now queue a task on the HTML event loop to resolve the skipWaiting promise
+        so that its ordering is correct, between the active event being fired and the service worker state becoming
+        "activated".
+
+        No new tests, upskipped existing test.
+
+        * workers/service/ServiceWorkerGlobalScope.cpp:
+        (WebCore::ServiceWorkerGlobalScope::skipWaiting):
+        * workers/service/context/SWContextManager.h:
+        * workers/service/server/SWServerToContextConnection.cpp:
+        (WebCore::SWServerToContextConnection::skipWaiting):
+        * workers/service/server/SWServerToContextConnection.h:
+
+2019-12-19  Chris Dumez  <cdu...@apple.com>
+
         Stop blocking the worker thread in WorkerMessagePortChannelProvider::postMessageToRemote()
         https://bugs.webkit.org/show_bug.cgi?id=205414
 

Modified: trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp (253751 => 253752)


--- trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp	2019-12-19 15:59:40 UTC (rev 253752)
@@ -71,8 +71,10 @@
             connection->skipWaiting(identifier, [workerThread = WTFMove(workerThread), requestIdentifier] {
                 workerThread->runLoop().postTask([requestIdentifier](auto& context) {
                     auto& scope = downcast<ServiceWorkerGlobalScope>(context);
-                    if (auto promise = scope.m_pendingSkipWaitingPromises.take(requestIdentifier))
-                        promise->resolve();
+                    scope.eventLoop().queueTask(TaskSource::DOMManipulation, [scope = makeRef(scope), requestIdentifier]() mutable {
+                        if (auto promise = scope->m_pendingSkipWaitingPromises.take(requestIdentifier))
+                            promise->resolve();
+                    });
                 });
             });
         }

Modified: trunk/Source/WebCore/workers/service/context/SWContextManager.h (253751 => 253752)


--- trunk/Source/WebCore/workers/service/context/SWContextManager.h	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebCore/workers/service/context/SWContextManager.h	2019-12-19 15:59:40 UTC (rev 253752)
@@ -56,7 +56,7 @@
         virtual void didFinishActivation(ServiceWorkerIdentifier) = 0;
         virtual void setServiceWorkerHasPendingEvents(ServiceWorkerIdentifier, bool) = 0;
         virtual void workerTerminated(ServiceWorkerIdentifier) = 0;
-        virtual void skipWaiting(ServiceWorkerIdentifier, Function<void()>&&) = 0;
+        virtual void skipWaiting(ServiceWorkerIdentifier, CompletionHandler<void()>&&) = 0;
         virtual void setScriptResource(ServiceWorkerIdentifier, const URL&, const ServiceWorkerContextData::ImportedScript&) = 0;
 
         using FindClientByIdentifierCallback = CompletionHandler<void(ExceptionOr<Optional<ServiceWorkerClientData>>&&)>;

Modified: trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp (253751 => 253752)


--- trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.cpp	2019-12-19 15:59:40 UTC (rev 253752)
@@ -109,12 +109,12 @@
     }
 }
 
-void SWServerToContextConnection::skipWaiting(ServiceWorkerIdentifier serviceWorkerIdentifier, uint64_t callbackID)
+void SWServerToContextConnection::skipWaiting(ServiceWorkerIdentifier serviceWorkerIdentifier, CompletionHandler<void()>&& completionHandler)
 {
     if (auto* worker = SWServerWorker::existingWorkerForIdentifier(serviceWorkerIdentifier))
         worker->skipWaiting();
 
-    didFinishSkipWaiting(callbackID);
+    completionHandler();
 }
 
 void SWServerToContextConnection::setScriptResource(ServiceWorkerIdentifier serviceWorkerIdentifier, URL&& scriptURL, String&& script, URL&& responseURL, String&& mimeType)

Modified: trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h (253751 => 253752)


--- trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebCore/workers/service/server/SWServerToContextConnection.h	2019-12-19 15:59:40 UTC (rev 253752)
@@ -57,7 +57,6 @@
     virtual void findClientByIdentifierCompleted(uint64_t requestIdentifier, const Optional<ServiceWorkerClientData>&, bool hasSecurityError) = 0;
     virtual void matchAllCompleted(uint64_t requestIdentifier, const Vector<ServiceWorkerClientData>&) = 0;
     virtual void claimCompleted(uint64_t requestIdentifier) = 0;
-    virtual void didFinishSkipWaiting(uint64_t callbackID) = 0;
 
     // Messages back from the SW host process
     WEBCORE_EXPORT void scriptContextFailedToStart(const Optional<ServiceWorkerJobDataIdentifier>&, ServiceWorkerIdentifier, const String& message);
@@ -65,7 +64,7 @@
     WEBCORE_EXPORT void didFinishInstall(const Optional<ServiceWorkerJobDataIdentifier>&, ServiceWorkerIdentifier, bool wasSuccessful);
     WEBCORE_EXPORT void didFinishActivation(ServiceWorkerIdentifier);
     WEBCORE_EXPORT void setServiceWorkerHasPendingEvents(ServiceWorkerIdentifier, bool hasPendingEvents);
-    WEBCORE_EXPORT void skipWaiting(ServiceWorkerIdentifier, uint64_t callbackID);
+    WEBCORE_EXPORT void skipWaiting(ServiceWorkerIdentifier, CompletionHandler<void()>&&);
     WEBCORE_EXPORT void workerTerminated(ServiceWorkerIdentifier);
     WEBCORE_EXPORT void findClientByIdentifier(uint64_t clientIdRequestIdentifier, ServiceWorkerIdentifier, ServiceWorkerClientIdentifier);
     WEBCORE_EXPORT void matchAll(uint64_t requestIdentifier, ServiceWorkerIdentifier, const ServiceWorkerClientQueryOptions&);

Modified: trunk/Source/WebKit/ChangeLog (253751 => 253752)


--- trunk/Source/WebKit/ChangeLog	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/ChangeLog	2019-12-19 15:59:40 UTC (rev 253752)
@@ -1,5 +1,22 @@
 2019-12-19  Chris Dumez  <cdu...@apple.com>
 
+        imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=205408
+
+        Reviewed by Youenn Fablet.
+
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp:
+        (WebKit::WebSWServerToContextConnection::didFinishSkipWaiting): Deleted.
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h:
+        * NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in:
+        * WebProcess/Storage/WebSWContextManagerConnection.cpp:
+        (WebKit::WebSWContextManagerConnection::skipWaiting):
+        (WebKit::WebSWContextManagerConnection::didFinishSkipWaiting): Deleted.
+        * WebProcess/Storage/WebSWContextManagerConnection.h:
+        * WebProcess/Storage/WebSWContextManagerConnection.messages.in:
+
+2019-12-19  Chris Dumez  <cdu...@apple.com>
+
         Stop blocking the worker thread in WorkerMessagePortChannelProvider::postMessageToRemote()
         https://bugs.webkit.org/show_bug.cgi?id=205414
 

Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp (253751 => 253752)


--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp	2019-12-19 15:59:40 UTC (rev 253752)
@@ -125,11 +125,6 @@
     send(Messages::WebSWContextManagerConnection::ClaimCompleted { requestIdentifier });
 }
 
-void WebSWServerToContextConnection::didFinishSkipWaiting(uint64_t callbackID)
-{
-    send(Messages::WebSWContextManagerConnection::DidFinishSkipWaiting { callbackID });
-}
-
 void WebSWServerToContextConnection::connectionIsNoLongerNeeded()
 {
     m_connection.serverToContextConnectionNoLongerNeeded();

Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h (253751 => 253752)


--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h	2019-12-19 15:59:40 UTC (rev 253752)
@@ -93,7 +93,6 @@
     void findClientByIdentifierCompleted(uint64_t requestIdentifier, const Optional<WebCore::ServiceWorkerClientData>&, bool hasSecurityError) final;
     void matchAllCompleted(uint64_t requestIdentifier, const Vector<WebCore::ServiceWorkerClientData>&) final;
     void claimCompleted(uint64_t requestIdentifier) final;
-    void didFinishSkipWaiting(uint64_t callbackID) final;
 
     void connectionIsNoLongerNeeded() final;
 

Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in (253751 => 253752)


--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in	2019-12-19 15:59:40 UTC (rev 253752)
@@ -30,7 +30,7 @@
     DidFinishInstall(Optional<WebCore::ServiceWorkerJobDataIdentifier> jobDataIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, bool wasSuccessful);
     DidFinishActivation(WebCore::ServiceWorkerIdentifier identifier);
     SetServiceWorkerHasPendingEvents(WebCore::ServiceWorkerIdentifier identifier, bool hasPendingEvents);
-    SkipWaiting(WebCore::ServiceWorkerIdentifier identifier, uint64_t callbackID)
+    SkipWaiting(WebCore::ServiceWorkerIdentifier identifier) -> () Async
     WorkerTerminated(WebCore::ServiceWorkerIdentifier identifier);
     FindClientByIdentifier(uint64_t requestIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, struct WebCore::ServiceWorkerClientIdentifier clientIdentifier);
     MatchAll(uint64_t matchAllRequestIdentifier, WebCore::ServiceWorkerIdentifier serviceWorkerIdentifier, struct WebCore::ServiceWorkerClientQueryOptions options);

Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp (253751 => 253752)


--- trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp	2019-12-19 15:59:40 UTC (rev 253752)
@@ -286,11 +286,9 @@
     m_connectionToNetworkProcess->send(Messages::WebSWServerToContextConnection::SetServiceWorkerHasPendingEvents(serviceWorkerIdentifier, hasPendingEvents), 0);
 }
 
-void WebSWContextManagerConnection::skipWaiting(ServiceWorkerIdentifier serviceWorkerIdentifier, WTF::Function<void()>&& callback)
+void WebSWContextManagerConnection::skipWaiting(ServiceWorkerIdentifier serviceWorkerIdentifier, CompletionHandler<void()>&& callback)
 {
-    auto callbackID = ++m_previousRequestIdentifier;
-    m_skipWaitingRequests.add(callbackID, WTFMove(callback));
-    m_connectionToNetworkProcess->send(Messages::WebSWServerToContextConnection::SkipWaiting(serviceWorkerIdentifier, callbackID), 0);
+    m_connectionToNetworkProcess->sendWithAsyncReply(Messages::WebSWServerToContextConnection::SkipWaiting(serviceWorkerIdentifier), WTFMove(callback));
 }
 
 void WebSWContextManagerConnection::setScriptResource(ServiceWorkerIdentifier serviceWorkerIdentifier, const URL& url, const ServiceWorkerContextData::ImportedScript& script)
@@ -347,12 +345,6 @@
         callback();
 }
 
-void WebSWContextManagerConnection::didFinishSkipWaiting(uint64_t callbackID)
-{
-    if (auto callback = m_skipWaitingRequests.take(callbackID))
-        callback();
-}
-
 void WebSWContextManagerConnection::close()
 {
     RELEASE_LOG(ServiceWorker, "Service worker process is requested to stop all service workers");

Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h (253751 => 253752)


--- trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h	2019-12-19 15:59:40 UTC (rev 253752)
@@ -76,7 +76,7 @@
     void findClientByIdentifier(WebCore::ServiceWorkerIdentifier, WebCore::ServiceWorkerClientIdentifier, FindClientByIdentifierCallback&&) final;
     void matchAll(WebCore::ServiceWorkerIdentifier, const WebCore::ServiceWorkerClientQueryOptions&, WebCore::ServiceWorkerClientsMatchAllCallback&&) final;
     void claim(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&) final;
-    void skipWaiting(WebCore::ServiceWorkerIdentifier, Function<void()>&&) final;
+    void skipWaiting(WebCore::ServiceWorkerIdentifier, CompletionHandler<void()>&&) final;
     void setScriptResource(WebCore::ServiceWorkerIdentifier, const URL&, const WebCore::ServiceWorkerContextData::ImportedScript&) final;
     bool isThrottleable() const final;
 
@@ -95,7 +95,6 @@
     void findClientByIdentifierCompleted(uint64_t requestIdentifier, Optional<WebCore::ServiceWorkerClientData>&&, bool hasSecurityError);
     void matchAllCompleted(uint64_t matchAllRequestIdentifier, Vector<WebCore::ServiceWorkerClientData>&&);
     void claimCompleted(uint64_t claimRequestIdentifier);
-    void didFinishSkipWaiting(uint64_t callbackID);
     void setUserAgent(String&& userAgent);
     void close();
     void setThrottleState(bool isThrottleable);
@@ -113,7 +112,6 @@
     HashMap<uint64_t, FindClientByIdentifierCallback> m_findClientByIdentifierRequests;
     HashMap<uint64_t, WebCore::ServiceWorkerClientsMatchAllCallback> m_matchAllRequests;
     HashMap<uint64_t, WTF::CompletionHandler<void()>> m_claimRequests;
-    HashMap<uint64_t, WTF::Function<void()>> m_skipWaitingRequests;
     uint64_t m_previousRequestIdentifier { 0 };
     String m_userAgent;
     bool m_isThrottleable { true };

Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in (253751 => 253752)


--- trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in	2019-12-19 15:48:26 UTC (rev 253751)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in	2019-12-19 15:59:40 UTC (rev 253752)
@@ -35,7 +35,6 @@
     FindClientByIdentifierCompleted(uint64_t clientIdRequestIdentifier, Optional<WebCore::ServiceWorkerClientData> data, bool hasSecurityError)
     MatchAllCompleted(uint64_t matchAllRequestIdentifier, Vector<WebCore::ServiceWorkerClientData> clientsData)
     ClaimCompleted(uint64_t claimRequestIdentifier)
-    DidFinishSkipWaiting(uint64_t callbackID)
     SetUserAgent(String userAgent)
     UpdatePreferencesStore(struct WebKit::WebPreferencesStore store)
     Close()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to