[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..fbd8c047620ea84ca34fb3a2b2b5f8f1b20b03b2 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

[falken, 2017/05/24 08:24:18]

Add quote:

"Upon fulfillment or rejection of f, queue a microtask to run these substeps:

Decrement the pending promises count by one."

[leonhsl(Using Gerrit), 2017/05/25 00:16:40]

Done.

+    // 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,48 @@ 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())) {
+          break;
+        }
+      // Fall through:
+      // Although the event handler is not finished yet, it's running microtask,

[falken, 2017/05/24 08:24:18]

I'd like this explanation to better explain what you said in the comment thread
here about how didDispatchEvent() is called after both the event handler
execution finished and microtasks queued by the event handler execution
finished.

[leonhsl(Using Gerrit), 2017/05/25 00:16:40]

Done.

+      // and no any extend lifetime promises are outstanding, in such case we
+      // should also throw.
+      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 +161,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 +180,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:

[falken, 2017/05/24 08:24:18]

// Still waiting for a promise, do not complete the event.

[leonhsl(Using Gerrit), 2017/05/25 00:16:40]

Done.

+      if (pending_promise_ != 0)
+        return;

[falken, 2017/05/24 08:24:18]

// Dispatch finished and there are no pending promises, complete the event.

[leonhsl(Using Gerrit), 2017/05/25 00:16:40]

Done.

+      break;
+    case EventDispatchState::kFailed:
+      // Dispatch had some error, complete the event immediatelly.
+      break;
+  }
+
   ServiceWorkerGlobalScopeClient* client =
       ServiceWorkerGlobalScopeClient::From(execution_context_);
   WebServiceWorkerEventResult result =