[ServiceWorker] waitUntil() should wait until all promises got resolved/rejected.

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef WaitUntilObserver_h
 #define WaitUntilObserver_h
 
 #include "modules/ModulesExport.h"
 #include "modules/serviceworkers/ServiceWorkerGlobalScopeClient.h"
 #include "platform/Timer.h"
@@ skipping 24 matching lines @@
     kBackgroundFetchAbort,
     kBackgroundFetchClick,
     kBackgroundFetchFail,
     kBackgroundFetched
   };
 
   static WaitUntilObserver* Create(ExecutionContext*, EventType, int event_id);
 
   // Must be called before and after dispatching the event.
   void WillDispatchEvent();
   void DidDispatchEvent(bool error_occurred);

[falken, 2017/05/09 08:07:31]

I see, it's not from your patch but |error_occurred| and |has_error| are a bit
inaccurately named (they are from an earlier time). Can you rename them to
|event_dispatch_failed|? You also will need to update the comments on callsites
that are currently "// error_occurred".

Now there's a big difference between |has_rejected_promise| and this flag, so I
want better names.

Also comments are good:
// Must be called before dispatching the event.
void WillDispatchEvent();

// Must be called after dispatching the event. If |event_dispatch_failed|, this
WaitUntilObserver will soon report to ServiceWorkerGlobalScopeClient that the
event finished, without waiting for all waitUntil promises to settle.

[leonhsl(Using Gerrit), 2017/05/10 05:03:34]

Done.

 
   // Observes the promise and delays calling the continuation until
   // the given promise is resolved or rejected.
   void WaitUntil(ScriptState*, ScriptPromise, ExceptionState&);
 
   // These methods can be called when the lifecycle of ExtendableEvent
   // observed by this WaitUntilObserver should be extended by other reason
   // than ExtendableEvent.waitUntil.
   // Note: There is no need to call decrementPendingActivity() after the context
   // is being destroyed.
   void IncrementPendingActivity();
   void DecrementPendingActivity();
 
   DECLARE_VIRTUAL_TRACE();
 
  private:
   friend class InternalsServiceWorker;
   class ThenFunction;
 
   WaitUntilObserver(ExecutionContext*, EventType, int event_id);
 
-  void ReportError(const ScriptValue&);
+  void ReportPromiseRejected(const ScriptValue&);

[falken, 2017/05/09 08:07:31]

Maybe just "OnPromiseRejected", as it's not really reporting to anything.

The param isn't used, so it can be removed.

Also a comment like would be good like:
// Called when a promise passed to a waitUntil() call that is associated with
this observer was rejected.

[leonhsl(Using Gerrit), 2017/05/10 05:03:34]

Removed this method, DecrementPendingActivity() covers its functionality.

 
   void ConsumeWindowInteraction(TimerBase*);
 
   Member<ExecutionContext> execution_context_;
   EventType type_;
   int event_id_;
   int pending_activity_ = 0;
   bool has_error_ = false;
+  bool has_rejected_promise_ = false;
   bool event_dispatched_ = false;

[falken, 2017/05/09 08:07:31]

Actually, instead of |has_error_| and |event_dispatched_|, can you change it to
an enum like: kInitial, kEventDispatchCompleted, kEventDispatchFailed

[leonhsl(Using Gerrit), 2017/05/10 05:03:34]

Done.

   double event_dispatch_time_ = 0;
   TaskRunnerTimer<WaitUntilObserver> consume_window_interaction_timer_;
 };
 
 }  // namespace blink
 
 #endif  // WaitUntilObserver_h