Title: [262409] trunk
Revision
262409
Author
you...@apple.com
Date
2020-06-02 04:23:47 -0700 (Tue, 02 Jun 2020)

Log Message

[ Mac wk2 ] http/wpt/service-workers/service-worker-spinning-fetch.https.html is flaky failing.
https://bugs.webkit.org/show_bug.cgi?id=207515
<rdar://problem/59329307>

Reviewed by Chris Dumez.

Source/WebCore:

When a service worker is terminated, we remove it from the map in SWContextManager.
Shortly after a new service worker may be added to the map.
In that case, previously, we were potentially trying to decrement the message count of the old service worker thread, which is confusing the new service worker thread.
Instead, use WeakPtr to decrement if the service worker thread is still valid.
Covered by existing tests.

* workers/service/context/ServiceWorkerThread.cpp:
(WebCore::ServiceWorkerThread::queueTaskToPostMessage):
(WebCore::ServiceWorkerThread::queueTaskToFireInstallEvent):
(WebCore::ServiceWorkerThread::queueTaskToFireActivateEvent):
(WebCore::ServiceWorkerThread::start):
* workers/service/context/ServiceWorkerThread.h:

LayoutTests:

* http/wpt/service-workers/service-worker-spinning-fetch.https.html:
In case service worker gets closed, fetch failure might be logged as console log message.
* platform/mac-wk2/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (262408 => 262409)


--- trunk/LayoutTests/ChangeLog	2020-06-02 07:44:47 UTC (rev 262408)
+++ trunk/LayoutTests/ChangeLog	2020-06-02 11:23:47 UTC (rev 262409)
@@ -1,3 +1,15 @@
+2020-06-02  Youenn Fablet  <you...@apple.com>
+
+        [ Mac wk2 ] http/wpt/service-workers/service-worker-spinning-fetch.https.html is flaky failing.
+        https://bugs.webkit.org/show_bug.cgi?id=207515
+        <rdar://problem/59329307>
+
+        Reviewed by Chris Dumez.
+
+        * http/wpt/service-workers/service-worker-spinning-fetch.https.html:
+        In case service worker gets closed, fetch failure might be logged as console log message.
+        * platform/mac-wk2/TestExpectations:
+
 2020-06-01  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         [WebGPU] Update texture creation validation according to the discussion at https://github.com/gpuweb/gpuweb/pull/799/files

Modified: trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-fetch.https.html (262408 => 262409)


--- trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-fetch.https.html	2020-06-02 07:44:47 UTC (rev 262408)
+++ trunk/LayoutTests/http/wpt/service-workers/service-worker-spinning-fetch.https.html	2020-06-02 11:23:47 UTC (rev 262409)
@@ -1,4 +1,4 @@
-<!doctype html><!-- webkit-test-runner [ useServiceWorkerShortTimeout=true ] -->
+<!doctype html><!-- webkit-test-runner [ useServiceWorkerShortTimeout=true dumpJSConsoleLogInStdErr=true ] -->
 <html>
 <head>
 <script src=""

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (262408 => 262409)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2020-06-02 07:44:47 UTC (rev 262408)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2020-06-02 11:23:47 UTC (rev 262409)
@@ -1018,8 +1018,6 @@
 
 webkit.org/b/209624 [ Release ] tiled-drawing/scrolling/fixed/four-bars-zoomed.html [ Pass Failure ]
 
-webkit.org/b/209672 http/wpt/service-workers/service-worker-spinning-fetch.https.html [ Pass Failure Crash ]
-
 webkit.org/b/209769 [ Catalina ] tiled-drawing/scrolling/frames/frameset-nested-frame-scrollability.html [ Pass Failure ]
 
 webkit.org/b/209878  [ Debug ] webrtc/datachannel/multiple-connections.html [ Slow ]

Modified: trunk/Source/WebCore/ChangeLog (262408 => 262409)


--- trunk/Source/WebCore/ChangeLog	2020-06-02 07:44:47 UTC (rev 262408)
+++ trunk/Source/WebCore/ChangeLog	2020-06-02 11:23:47 UTC (rev 262409)
@@ -1,3 +1,24 @@
+2020-06-02  Youenn Fablet  <you...@apple.com>
+
+        [ Mac wk2 ] http/wpt/service-workers/service-worker-spinning-fetch.https.html is flaky failing.
+        https://bugs.webkit.org/show_bug.cgi?id=207515
+        <rdar://problem/59329307>
+
+        Reviewed by Chris Dumez.
+
+        When a service worker is terminated, we remove it from the map in SWContextManager.
+        Shortly after a new service worker may be added to the map.
+        In that case, previously, we were potentially trying to decrement the message count of the old service worker thread, which is confusing the new service worker thread.
+        Instead, use WeakPtr to decrement if the service worker thread is still valid.
+        Covered by existing tests.
+
+        * workers/service/context/ServiceWorkerThread.cpp:
+        (WebCore::ServiceWorkerThread::queueTaskToPostMessage):
+        (WebCore::ServiceWorkerThread::queueTaskToFireInstallEvent):
+        (WebCore::ServiceWorkerThread::queueTaskToFireActivateEvent):
+        (WebCore::ServiceWorkerThread::start):
+        * workers/service/context/ServiceWorkerThread.h:
+
 2020-06-01  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         [WebGPU] Update texture creation validation according to the discussion at https://github.com/gpuweb/gpuweb/pull/799/files

Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp (262408 => 262409)


--- trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp	2020-06-02 07:44:47 UTC (rev 262408)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.cpp	2020-06-02 11:23:47 UTC (rev 262409)
@@ -79,6 +79,7 @@
     , m_heartBeatTimeout(SWContextManager::singleton().connection()->shouldUseShortTimeout() ? heartBeatTimeoutForTest : heartBeatTimeout)
     , m_heartBeatTimer { *this, &ServiceWorkerThread::heartBeatTimerFired }
 {
+    ASSERT(isMainThread());
     AtomString::init();
 }
 
@@ -115,7 +116,7 @@
 void ServiceWorkerThread::queueTaskToPostMessage(MessageWithMessagePorts&& message, ServiceWorkerOrClientData&& sourceData)
 {
     auto serviceWorkerGlobalScope = makeRef(downcast<ServiceWorkerGlobalScope>(*workerGlobalScope()));
-    serviceWorkerGlobalScope->eventLoop().queueTask(TaskSource::DOMManipulation, [serviceWorkerGlobalScope = serviceWorkerGlobalScope.copyRef(), message = WTFMove(message), sourceData = WTFMove(sourceData), serviceWorkerIdentifier = this->identifier()]() mutable {
+    serviceWorkerGlobalScope->eventLoop().queueTask(TaskSource::DOMManipulation, [weakThis = makeWeakPtr(this), serviceWorkerGlobalScope = serviceWorkerGlobalScope.copyRef(), message = WTFMove(message), sourceData = WTFMove(sourceData)]() mutable {
         URL sourceURL;
         ExtendableMessageEventSource source;
         if (WTF::holds_alternative<ServiceWorkerClientData>(sourceData)) {
@@ -134,9 +135,9 @@
             source = WTFMove(sourceWorker);
         }
         fireMessageEvent(serviceWorkerGlobalScope, WTFMove(message), ExtendableMessageEventSource { source }, sourceURL);
-        callOnMainThread([serviceWorkerIdentifier] {
-            if (auto* serviceWorkerThreadProxy = SWContextManager::singleton().serviceWorkerThreadProxy(serviceWorkerIdentifier))
-                serviceWorkerThreadProxy->thread().finishedFiringMessageEvent();
+        callOnMainThread([weakThis = WTFMove(weakThis)] {
+            if (weakThis)
+                weakThis->finishedFiringMessageEvent();
         });
     });
 }
@@ -144,11 +145,11 @@
 void ServiceWorkerThread::queueTaskToFireInstallEvent()
 {
     auto serviceWorkerGlobalScope = makeRef(downcast<ServiceWorkerGlobalScope>(*workerGlobalScope()));
-    serviceWorkerGlobalScope->eventLoop().queueTask(TaskSource::DOMManipulation, [serviceWorkerGlobalScope = serviceWorkerGlobalScope.copyRef(), serviceWorkerIdentifier = this->identifier()] {
+    serviceWorkerGlobalScope->eventLoop().queueTask(TaskSource::DOMManipulation, [weakThis = makeWeakPtr(this), serviceWorkerGlobalScope = serviceWorkerGlobalScope.copyRef()]() mutable {
         auto installEvent = ExtendableEvent::create(eventNames().installEvent, { }, ExtendableEvent::IsTrusted::Yes);
         serviceWorkerGlobalScope->dispatchEvent(installEvent);
 
-        installEvent->whenAllExtendLifetimePromisesAreSettled([serviceWorkerIdentifier](HashSet<Ref<DOMPromise>>&& extendLifetimePromises) {
+        installEvent->whenAllExtendLifetimePromisesAreSettled([weakThis = WTFMove(weakThis)](HashSet<Ref<DOMPromise>>&& extendLifetimePromises) mutable {
             bool hasRejectedAnyPromise = false;
             for (auto& promise : extendLifetimePromises) {
                 if (promise->status() == DOMPromise::Status::Rejected) {
@@ -156,9 +157,9 @@
                     break;
                 }
             }
-            callOnMainThread([serviceWorkerIdentifier, hasRejectedAnyPromise] {
-                if (auto* serviceWorkerThreadProxy = SWContextManager::singleton().serviceWorkerThreadProxy(serviceWorkerIdentifier))
-                    serviceWorkerThreadProxy->thread().finishedFiringInstallEvent(hasRejectedAnyPromise);
+            callOnMainThread([weakThis = WTFMove(weakThis), hasRejectedAnyPromise] {
+                if (weakThis)
+                    weakThis->finishedFiringInstallEvent(hasRejectedAnyPromise);
             });
         });
     });
@@ -167,14 +168,14 @@
 void ServiceWorkerThread::queueTaskToFireActivateEvent()
 {
     auto serviceWorkerGlobalScope = makeRef(downcast<ServiceWorkerGlobalScope>(*workerGlobalScope()));
-    serviceWorkerGlobalScope->eventLoop().queueTask(TaskSource::DOMManipulation, [serviceWorkerGlobalScope = serviceWorkerGlobalScope.copyRef(), serviceWorkerIdentifier = this->identifier()]() mutable {
+    serviceWorkerGlobalScope->eventLoop().queueTask(TaskSource::DOMManipulation, [weakThis = makeWeakPtr(this), serviceWorkerGlobalScope = serviceWorkerGlobalScope.copyRef()]() mutable {
         auto activateEvent = ExtendableEvent::create(eventNames().activateEvent, { }, ExtendableEvent::IsTrusted::Yes);
         serviceWorkerGlobalScope->dispatchEvent(activateEvent);
 
-        activateEvent->whenAllExtendLifetimePromisesAreSettled([serviceWorkerIdentifier](HashSet<Ref<DOMPromise>>&&) {
-            callOnMainThread([serviceWorkerIdentifier] {
-                if (auto* serviceWorkerThreadProxy = SWContextManager::singleton().serviceWorkerThreadProxy(serviceWorkerIdentifier))
-                    serviceWorkerThreadProxy->thread().finishedFiringActivateEvent();
+        activateEvent->whenAllExtendLifetimePromisesAreSettled([weakThis = WTFMove(weakThis)](auto&&) mutable {
+            callOnMainThread([weakThis = WTFMove(weakThis)] {
+                if (weakThis)
+                    weakThis->finishedFiringActivateEvent();
             });
         });
     });
@@ -191,11 +192,11 @@
     m_state = State::Starting;
     startHeartBeatTimer();
 
-    WorkerThread::start([callback = WTFMove(callback), serviceWorkerIdentifier = this->identifier()](auto& errorMessage) mutable {
+    WorkerThread::start([callback = WTFMove(callback), weakThis = makeWeakPtr(this)](auto& errorMessage) mutable {
         bool doesHandleFetch = true;
-        if (auto* threadProxy = SWContextManager::singleton().workerByID(serviceWorkerIdentifier)) {
-            threadProxy->thread().finishedStarting();
-            doesHandleFetch = threadProxy->thread().doesHandleFetch();
+        if (weakThis) {
+            weakThis->finishedStarting();
+            doesHandleFetch = weakThis->doesHandleFetch();
         }
         callback(errorMessage, doesHandleFetch);
     });

Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.h (262408 => 262409)


--- trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.h	2020-06-02 07:44:47 UTC (rev 262408)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerThread.h	2020-06-02 11:23:47 UTC (rev 262409)
@@ -32,6 +32,7 @@
 #include "ServiceWorkerIdentifier.h"
 #include "Timer.h"
 #include "WorkerThread.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -44,7 +45,7 @@
 struct MessageWithMessagePorts;
 struct ServiceWorkerClientIdentifier;
 
-class ServiceWorkerThread : public WorkerThread {
+class ServiceWorkerThread : public WorkerThread, public CanMakeWeakPtr<ServiceWorkerThread, WeakPtrFactoryInitialization::Eager> {
 public:
     template<typename... Args> static Ref<ServiceWorkerThread> create(Args&&... args)
     {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to