Queue a microtask when a waitUntil() promise is settled
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Feb 2018 11:50:47 +0000 (11:50 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Feb 2018 11:50:47 +0000 (11:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182372
<rdar://problem/37101019>

Reviewed by Mark Lam.

LayoutTests/imported/w3c:

Reaseline WPT test now that all checks are passing.

* web-platform-tests/service-workers/service-worker/extendable-event-async-waituntil.https-expected.txt:

Source/JavaScriptCore:

Export a symbol so it can be used in WebCore.

* runtime/JSGlobalObject.h:

Source/WebCore:

Queue a microtask when a waitUntil() promise is settled, as per:
- https://w3c.github.io/ServiceWorker/#dom-extendableevent-waituntil (step 5)

Otherwise, we decrement m_pendingPromiseCount too quickly and it may cause
following calls to waitUntil() to throw when they shouldn't.

No new tests, rebaselined existing test.

* workers/service/ExtendableEvent.cpp:
(WebCore::ExtendableEvent::addExtendLifetimePromise):

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

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/extendable-event-async-waituntil.https-expected.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/WebCore/ChangeLog
Source/WebCore/workers/service/ExtendableEvent.cpp

index f358f56..2afaad8 100644 (file)
@@ -1,3 +1,15 @@
+2018-02-01  Chris Dumez  <cdumez@apple.com>
+
+        Queue a microtask when a waitUntil() promise is settled
+        https://bugs.webkit.org/show_bug.cgi?id=182372
+        <rdar://problem/37101019>
+
+        Reviewed by Mark Lam.
+
+        Reaseline WPT test now that all checks are passing.
+
+        * web-platform-tests/service-workers/service-worker/extendable-event-async-waituntil.https-expected.txt:
+
 2018-02-01  Ms2ger  <Ms2ger@igalia.com>
 
         Update imagebitmap tests.
index a68d177..b65fc0e 100644 (file)
@@ -3,11 +3,11 @@
 PASS Test calling waitUntil in a different task without an existing extension throws 
 PASS Test calling waitUntil in a different microtask without an existing extension throws 
 PASS Test calling waitUntil in a different task with an existing extension succeeds 
-FAIL Test calling waitUntil with an existing extension promise handler succeeds assert_unreached: unexpected rejection: assert_equals: expected "OK" but got "InvalidStateError" Reached unreachable code
+PASS Test calling waitUntil with an existing extension promise handler succeeds 
 PASS Test calling waitUntil at the end of the microtask turn throws 
 PASS Test calling waitUntil after the current extension expired in a different task fails 
 PASS Test calling waitUntil on a script constructed ExtendableEvent throws exception 
 PASS Test calling waitUntil asynchronously with pending respondWith promise. 
-FAIL Test calling waitUntil synchronously inside microtask of respondWith promise. assert_unreached: unexpected rejection: assert_equals: expected "OK" but got "InvalidStateError" Reached unreachable code
+PASS Test calling waitUntil synchronously inside microtask of respondWith promise. 
 PASS Test calling waitUntil asynchronously inside microtask of respondWith promise. 
 
index 36063b3..8d93a9e 100644 (file)
@@ -1,3 +1,15 @@
+2018-02-01  Chris Dumez  <cdumez@apple.com>
+
+        Queue a microtask when a waitUntil() promise is settled
+        https://bugs.webkit.org/show_bug.cgi?id=182372
+        <rdar://problem/37101019>
+
+        Reviewed by Mark Lam.
+
+        Export a symbol so it can be used in WebCore.
+
+        * runtime/JSGlobalObject.h:
+
 2018-01-31  Don Olmstead  <don.olmstead@sony.com>
 
         [CMake] Make JavaScriptCore headers copies
index 6d1aac8..1a7eb81 100644 (file)
@@ -832,7 +832,7 @@ public:
     static bool shouldInterruptScriptBeforeTimeout(const JSGlobalObject*) { return false; }
     static RuntimeFlags javaScriptRuntimeFlags(const JSGlobalObject*) { return RuntimeFlags(); }
 
-    void queueMicrotask(Ref<Microtask>&&);
+    JS_EXPORT_PRIVATE void queueMicrotask(Ref<Microtask>&&);
 
     bool evalEnabled() const { return m_evalEnabled; }
     bool webAssemblyEnabled() const { return m_webAssemblyEnabled; }
index 40c74b8..a43ab30 100644 (file)
@@ -1,3 +1,22 @@
+2018-02-01  Chris Dumez  <cdumez@apple.com>
+
+        Queue a microtask when a waitUntil() promise is settled
+        https://bugs.webkit.org/show_bug.cgi?id=182372
+        <rdar://problem/37101019>
+
+        Reviewed by Mark Lam.
+
+        Queue a microtask when a waitUntil() promise is settled, as per:
+        - https://w3c.github.io/ServiceWorker/#dom-extendableevent-waituntil (step 5)
+
+        Otherwise, we decrement m_pendingPromiseCount too quickly and it may cause
+        following calls to waitUntil() to throw when they shouldn't.
+
+        No new tests, rebaselined existing test.
+
+        * workers/service/ExtendableEvent.cpp:
+        (WebCore::ExtendableEvent::addExtendLifetimePromise):
+
 2018-02-01  Antti Koivisto  <antti@apple.com>
 
         Invalidate style for sibling combinators accurately on class change
index 6353499..902a7a8 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(SERVICE_WORKER)
 
 #include "JSDOMPromise.h"
+#include <runtime/Microtask.h>
 
 namespace WebCore {
 
@@ -60,21 +61,44 @@ ExceptionOr<void> ExtendableEvent::waitUntil(Ref<DOMPromise>&& promise)
     return { };
 }
 
+class FunctionMicrotask final : public JSC::Microtask {
+public:
+    static Ref<FunctionMicrotask> create(Function<void()>&& function)
+    {
+        return adoptRef(*new FunctionMicrotask(WTFMove(function)));
+    }
+
+private:
+    explicit FunctionMicrotask(Function<void()>&& function)
+        : m_function(WTFMove(function))
+    {
+    }
+
+    void run(JSC::ExecState*) final
+    {
+        m_function();
+    }
+
+    Function<void()> m_function;
+};
+
 void ExtendableEvent::addExtendLifetimePromise(Ref<DOMPromise>&& promise)
 {
-    promise->whenSettled([this, protectedThis = makeRefPtr(this), settledPromise = promise.ptr()] () {
-        --m_pendingPromiseCount;
+    promise->whenSettled([this, protectedThis = makeRefPtr(this), settledPromise = promise.ptr()] () mutable {
+        settledPromise->globalObject()->queueMicrotask(FunctionMicrotask::create([this, protectedThis = WTFMove(protectedThis)] {
+            --m_pendingPromiseCount;
 
-        // FIXME: Let registration be the context object's relevant global object's associated service worker's containing service worker registration.
-        // FIXME: If registration's uninstalling flag is set, invoke Try Clear Registration with registration.
-        // FIXME: If registration is not null, invoke Try Activate with registration.
+            // FIXME: Let registration be the context object's relevant global object's associated service worker's containing service worker registration.
+            // FIXME: If registration's uninstalling flag is set, invoke Try Clear Registration with registration.
+            // FIXME: If registration is not null, invoke Try Activate with registration.
 
-        if (m_pendingPromiseCount)
-            return;
+            if (m_pendingPromiseCount)
+                return;
 
-        auto settledPromises = WTFMove(m_extendLifetimePromises);
-        if (auto handler = WTFMove(m_whenAllExtendLifetimePromisesAreSettledHandler))
-            handler(WTFMove(settledPromises));
+            auto settledPromises = WTFMove(m_extendLifetimePromises);
+            if (auto handler = WTFMove(m_whenAllExtendLifetimePromisesAreSettledHandler))
+                handler(WTFMove(settledPromises));
+        }));
     });
 
     m_extendLifetimePromises.add(WTFMove(promise));