ServiceWorker: Keep the worker alive until FetchEvent.waitUntil settles

content/browser/service_worker/service_worker_version.h

Index: content/browser/service_worker/service_worker_version.h
diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h
index 9b88f791b286971ad5e1ad1853fa8260df2dfe62..bf9941f478e7253896c00d1f2a96e5a87bceb243 100644
--- a/content/browser/service_worker/service_worker_version.h
+++ b/content/browser/service_worker/service_worker_version.h
@@ -65,7 +65,9 @@ class CONTENT_EXPORT ServiceWorkerVersion
     : NON_EXPORTED_BASE(public base::RefCounted<ServiceWorkerVersion>),
       public EmbeddedWorkerInstance::Listener {
  public:
-  typedef base::Callback<void(ServiceWorkerStatusCode)> StatusCallback;
+  using StatusCallback = base::Callback<void(ServiceWorkerStatusCode)>;
+  using FetchResponseCallback = base::Callback<
+      void(int, ServiceWorkerFetchEventResult, const ServiceWorkerResponse&)>;
 
   enum RunningStatus {
     STOPPED = EmbeddedWorkerInstance::STOPPED,
@@ -252,6 +254,14 @@ class CONTENT_EXPORT ServiceWorkerVersion
   template <typename ResponseMessage>
   void DispatchSimpleEvent(int request_id, const IPC::Message& message);
 
+  // For fetch event, response is returned earlier than the event is done.
+  // In order to handle the both (response and the end of event),
+  // DispatchFetchEvent takes two request ids and handles each callbacks.
+  void DispatchFetchEvent(int response_request_id,

[Marijn Kruisselbrink, 2016/06/02 16:58:55]

It seems very unfortunate to go back to having code specific for certain events
in ServiceWorkerVersion....

First of all I don't understand why you need two request ids? It is perfectly
fine from SWVersions point of view to have multiple replies for a single
request. The main limitation you'd run into is that currently DispatchEvent (and
SWVersion::EventResponseHandler) assume all replies for a particular request id
are the same type of IPC message. One hacky way (but maybe not worse than adding
fetch specific logic to SWVersion) would thus be to use one IPC message for both
the waitUntil and the respondWith result (you'd need to add some kind of field
to the message then to be able to distinguish the two).

Another way would be to add a lower-level overload of DispatchEvent that would
allow specifying a custom EmbeddedWorkerInstance::Listener instance rather than
have SWVersion creates its own EventResponseHandler. That custom listener would
have to duplicate some of the code in EventResponseHandler, which might be
unfortunate though. Or SWVersion could expose EventResponseHandler as an
abstract (non-templated) base-class that just deals with the add/remove listener
part and checking of request_id, with a separate internal
BasicEventResponseHandler that adds the dispatch-to-callback bit it currently
has (and for fetch you'd then have a different subclass that has slightly more
complicated dispatch logic to support multiple IPC types).

Or some other way DispatchEvent could be made more flexible to allow multiple
IPC types (perhaps as simple as an overload to support exactly two IPC types;
which should be fairly straight forward).

+                          int event_request_id,
+                          const ServiceWorkerFetchRequest& fetch_request,
+                          const FetchResponseCallback& callback);
+
   // Adds and removes |provider_host| as a controllee of this ServiceWorker.
   // A potential controllee is a host having the version as its .installing
   // or .waiting version.
@@ -355,6 +365,7 @@ class CONTENT_EXPORT ServiceWorkerVersion
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest,
                            RequestCustomizedTimeoutKill);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, MixedRequestTimeouts);
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerURLRequestJobTest, EarlyResponse);
 
   class Metrics;
   class PingController;
@@ -432,6 +443,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
       std::priority_queue<RequestInfo,
                           std::vector<RequestInfo>,
                           std::greater<RequestInfo>>;
+  using WebStatusCallback =
+      base::Callback<void(int, blink::WebServiceWorkerEventResult)>;
 
   // EmbeddedWorkerInstance Listener implementation which calls a callback
   // on receiving a particular IPC message. ResponseMessage is the type of