[ServiceWorker] Allow waitUntil to be called multiple times asynchronously

third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp

Index: third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp
diff --git a/third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp b/third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp
index d8056ae69e40194bdf9fdb55b9627aaf8768753c..40944092c85fe8fae6203da0c99a962c64069b4e 100644
--- a/third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp
+++ b/third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp
@@ -12,7 +12,9 @@
 #include "core/dom/ExecutionContext.h"
 #include "modules/serviceworkers/ServiceWorkerGlobalScope.h"
 #include "platform/LayoutTestSupport.h"
+#include "platform/bindings/Microtask.h"
 #include "platform/wtf/Assertions.h"
+#include "platform/wtf/Functional.h"
 #include "public/platform/Platform.h"
 #include "public/platform/modules/serviceworker/WebServiceWorkerEventResult.h"
 #include "v8/include/v8.h"
@@ -64,12 +66,25 @@ class WaitUntilObserver::ThenFunction final : public ScriptFunction {
   ScriptValue Call(ScriptValue value) override {
     DCHECK(observer_);
     DCHECK(resolve_type_ == kFulfilled || resolve_type_ == kRejected);
+    // According from step 4 of ExtendableEvent::waitUntil() in spec:
+    // https://w3c.github.io/ServiceWorker/#dom-extendableevent-waituntil
+    // At this time point the microtask A running resolve/reject function of
+    // this promise has already been queued, in order to allow microtask A to
+    // call waitUntil, we enqueue another microtask B to delay the promise
+    // settled notification to |observer_|, thus A will run before B so A can
+    // call waitUntil well, but any other microtask C possibly enqueued by A
+    // will run after B so C maybe can't call waitUntil if there has no any
+    // extend lifetime promise at that time.
     if (resolve_type_ == kRejected) {
-      observer_->OnPromiseRejected();
+      Microtask::EnqueueMicrotask(
+          WTF::Bind(&WaitUntilObserver::OnPromiseRejected,
+                    WrapPersistent(observer_.Get())));
       value =
           ScriptPromise::Reject(value.GetScriptState(), value).GetScriptValue();
     } else {
-      observer_->OnPromiseFulfilled();
+      Microtask::EnqueueMicrotask(
+          WTF::Bind(&WaitUntilObserver::OnPromiseFulfilled,
+                    WrapPersistent(observer_.Get())));
     }
     observer_ = nullptr;
     return value;
@@ -90,28 +105,50 @@ void WaitUntilObserver::WillDispatchEvent() {
   // When handling a notificationclick or paymentrequest event, we want to
   // allow one window to be focused or opened. These calls are allowed between
   // the call to willDispatchEvent() and the last call to
-  // decrementPendingActivity(). If waitUntil() isn't called, that means
+  // DecrementPendingPromise(). If waitUntil() isn't called, that means
   // between willDispatchEvent() and didDispatchEvent().
   if (type_ == kNotificationClick || type_ == kPaymentRequest)
     execution_context_->AllowWindowInteraction();
 
-  IncrementPendingActivity();
+  DCHECK_EQ(EventDispatchState::kInitial, event_dispatch_state_);
+  event_dispatch_state_ = EventDispatchState::kDispatching;
 }
 
 void WaitUntilObserver::DidDispatchEvent(bool event_dispatch_failed) {
   event_dispatch_state_ = event_dispatch_failed
                               ? EventDispatchState::kFailed
-                              : EventDispatchState::kCompleted;
-  DecrementPendingActivity();
+                              : EventDispatchState::kDispatched;
+  MaybeCompleteEvent();
 }
 
 void WaitUntilObserver::WaitUntil(ScriptState* script_state,
                                   ScriptPromise script_promise,
                                   ExceptionState& exception_state) {
-  if (event_dispatch_state_ != EventDispatchState::kInitial) {
-    exception_state.ThrowDOMException(kInvalidStateError,
-                                      "The event handler is already finished.");
-    return;
+  if (pending_promise_ == 0) {
+    switch (event_dispatch_state_) {
+      case EventDispatchState::kInitial:
+        NOTREACHED();
+        return;
+      case EventDispatchState::kDispatching:
+        if (v8::MicrotasksScope::IsRunningMicrotasks(
+                script_state->GetIsolate())) {
+          exception_state.ThrowDOMException(
+              kInvalidStateError,
+              "Although the event handler is not finished yet, it's running "

[shimazu, 2017/05/23 09:08:43]

I think it's okay to have the same message with the case of kDispatched/kFailed
because the event handler itself has finished here even though the subsequent
microtasks are running.

[leonhsl(Using Gerrit), 2017/05/23 09:40:18]

Done.

+              "microtask, and no any extend lifetime promises are "
+              "outstanding.");
+          return;
+        }
+        break;
+      case EventDispatchState::kDispatched:
+      case EventDispatchState::kFailed:
+        exception_state.ThrowDOMException(
+            kInvalidStateError,
+            "The event handler is already finished "
+            "and no any extend lifetime promises are "
+            "outstanding.");
+        return;
+    }
   }
 
   if (!execution_context_)
@@ -126,7 +163,7 @@ void WaitUntilObserver::WaitUntil(ScriptState* script_state,
     consume_window_interaction_timer_.StartOneShot(WindowInteractionTimeout(),
                                                    BLINK_FROM_HERE);
 
-  IncrementPendingActivity();
+  IncrementPendingPromise();
   script_promise.Then(ThenFunction::CreateFunction(script_state, this,
                                                    ThenFunction::kFulfilled),
                       ThenFunction::CreateFunction(script_state, this,
@@ -145,25 +182,44 @@ WaitUntilObserver::WaitUntilObserver(ExecutionContext* context,
           &WaitUntilObserver::ConsumeWindowInteraction) {}
 
 void WaitUntilObserver::OnPromiseFulfilled() {
-  DecrementPendingActivity();
+  DecrementPendingPromise();
 }
 
 void WaitUntilObserver::OnPromiseRejected() {
   has_rejected_promise_ = true;
-  DecrementPendingActivity();
+  DecrementPendingPromise();
 }
 
-void WaitUntilObserver::IncrementPendingActivity() {
-  ++pending_activity_;
+void WaitUntilObserver::IncrementPendingPromise() {
+  ++pending_promise_;
 }
 
-void WaitUntilObserver::DecrementPendingActivity() {
-  DCHECK_GT(pending_activity_, 0);
-  if (!execution_context_ ||
-      (event_dispatch_state_ != EventDispatchState::kFailed &&
-       --pending_activity_))
+void WaitUntilObserver::DecrementPendingPromise() {
+  DCHECK_GT(pending_promise_, 0);
+  --pending_promise_;
+  MaybeCompleteEvent();
+}
+
+void WaitUntilObserver::MaybeCompleteEvent() {
+  if (!execution_context_)
     return;
 
+  switch (event_dispatch_state_) {
+    case EventDispatchState::kInitial:
+      NOTREACHED();
+      return;
+    case EventDispatchState::kDispatching:
+      // Still dispatching, do not complete the event.
+      return;
+    case EventDispatchState::kDispatched:
+      if (pending_promise_ != 0)
+        return;
+      break;
+    case EventDispatchState::kFailed:
+      // Dispatch had some error, complete the event immediatelly.
+      break;
+  }
+
   ServiceWorkerGlobalScopeClient* client =
       ServiceWorkerGlobalScopeClient::From(execution_context_);
   WebServiceWorkerEventResult result =