imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Dec 2019 15:59:40 +0000 (15:59 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Dec 2019 15:59:40 +0000 (15:59 +0000)
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:

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-installed.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp
Source/WebCore/workers/service/context/SWContextManager.h
Source/WebCore/workers/service/server/SWServerToContextConnection.cpp
Source/WebCore/workers/service/server/SWServerToContextConnection.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.cpp
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.h
Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerToContextConnection.messages.in
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in

index 567d1c6..17a2ef4 100644 (file)
@@ -1,3 +1,14 @@
+2019-12-19  Chris Dumez  <cdumez@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_hsieh@apple.com>
 
         REGRESSION (r251015): Hitting return before a space deletes text after the insertion position
index bce2767..c8bd6e6 100644 (file)
@@ -275,10 +275,6 @@ imported/w3c/web-platform-tests/server-timing/test_server_timing.https.html [ Sk
 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 ]
index 940eadd..356737b 100644 (file)
@@ -1,3 +1,14 @@
+2019-12-19  Chris Dumez  <cdumez@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  <cdumez@apple.com>
 
         Resync web-platform-tests/dom tests from upstream
index 095f16d..2fab8fb 100644 (file)
@@ -1,4 +1,3 @@
 
-
-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"
+PASS Test skipWaiting when a installed worker is waiting 
 
index 7744b1f..aa0ee46 100644 (file)
@@ -1,5 +1,26 @@
 2019-12-19  Chris Dumez  <cdumez@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  <cdumez@apple.com>
+
         Stop blocking the worker thread in WorkerMessagePortChannelProvider::postMessageToRemote()
         https://bugs.webkit.org/show_bug.cgi?id=205414
 
index 1217a2c..d18d136 100644 (file)
@@ -71,8 +71,10 @@ void ServiceWorkerGlobalScope::skipWaiting(Ref<DeferredPromise>&& promise)
             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();
+                    });
                 });
             });
         }
index 09bbcb5..f778e23 100644 (file)
@@ -56,7 +56,7 @@ public:
         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>>&&)>;
index fa1c4cd..a367371 100644 (file)
@@ -109,12 +109,12 @@ void SWServerToContextConnection::claim(uint64_t requestIdentifier, ServiceWorke
     }
 }
 
-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)
index 8222999..33f12c6 100644 (file)
@@ -57,7 +57,6 @@ public:
     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 @@ public:
     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&);
index 19710a3..fdf868c 100644 (file)
@@ -1,5 +1,22 @@
 2019-12-19  Chris Dumez  <cdumez@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  <cdumez@apple.com>
+
         Stop blocking the worker thread in WorkerMessagePortChannelProvider::postMessageToRemote()
         https://bugs.webkit.org/show_bug.cgi?id=205414
 
index 907713f..0596480 100644 (file)
@@ -125,11 +125,6 @@ void WebSWServerToContextConnection::claimCompleted(uint64_t requestIdentifier)
     send(Messages::WebSWContextManagerConnection::ClaimCompleted { requestIdentifier });
 }
 
-void WebSWServerToContextConnection::didFinishSkipWaiting(uint64_t callbackID)
-{
-    send(Messages::WebSWContextManagerConnection::DidFinishSkipWaiting { callbackID });
-}
-
 void WebSWServerToContextConnection::connectionIsNoLongerNeeded()
 {
     m_connection.serverToContextConnectionNoLongerNeeded();
index 04c8d63..973d8c9 100644 (file)
@@ -93,7 +93,6 @@ private:
     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;
 
index f4fde5f..25e96f3 100644 (file)
@@ -30,7 +30,7 @@ messages -> WebSWServerToContextConnection NotRefCounted {
     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);
index 2dd75cc..b50ebf9 100644 (file)
@@ -286,11 +286,9 @@ void WebSWContextManagerConnection::setServiceWorkerHasPendingEvents(ServiceWork
     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 @@ void WebSWContextManagerConnection::claimCompleted(uint64_t claimRequestIdentifi
         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");
index 6990332..6f7dc75 100644 (file)
@@ -76,7 +76,7 @@ private:
     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 @@ private:
     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 @@ private:
     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 };
index 1d339cc..afe3fc4 100644 (file)
@@ -35,7 +35,6 @@ messages -> WebSWContextManagerConnection NotRefCounted {
     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()