ServiceWorker: Keep the worker alive until FetchEvent.waitUntil settles

content/browser/service_worker/service_worker_version.h

 // Copyright 2013 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 CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 
 #include <stdint.h>
 
 #include <functional>
@@ skipping 47 matching lines @@
 
 // This class corresponds to a specific version of a ServiceWorker
 // script for a given pattern. When a script is upgraded, there may be
 // more than one ServiceWorkerVersion "running" at a time, but only
 // one of them is activated. This class connects the actual script with a
 // running worker.
 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,
     STARTING = EmbeddedWorkerInstance::STARTING,
     RUNNING = EmbeddedWorkerInstance::RUNNING,
     STOPPING = EmbeddedWorkerInstance::STOPPING,
   };
 
   // Current version status; some of the status (e.g. INSTALLED and ACTIVATED)
   // should be persisted unlike running status.
@@ skipping 166 matching lines @@
 
   // For simple events where the full functionality of DispatchEvent is not
   // needed, this method can be used instead. The ResponseMessage must consist
   // of just a request_id and a blink::WebServiceWorkerEventResult field. The
   // result is converted to a ServiceWorkerStatusCode and passed to the error
   // handler associated with the request. Additionally this methods calls
   // FinishRequest before passing the reply to the callback.
   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.
   void AddControllee(ServiceWorkerProviderHost* provider_host);
   void RemoveControllee(ServiceWorkerProviderHost* provider_host);
 
   // Returns if it has controllee.
   bool HasControllee() const { return !controllee_map_.empty(); }
   std::map<std::string, ServiceWorkerProviderHost*> controllee_map() {
     return controllee_map_;
@@ skipping 83 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionBrowserTest,
                            TimeoutWorkerInEvent);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerStallInStoppingTest, DetachThenStart);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerStallInStoppingTest, DetachThenRestart);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest,
                            RegisterForeignFetchScopes);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, RequestCustomizedTimeout);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest,
                            RequestCustomizedTimeoutKill);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, MixedRequestTimeouts);
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerURLRequestJobTest, EarlyResponse);
 
   class Metrics;
   class PingController;
 
   struct RequestInfo {
     RequestInfo(int id,
                 ServiceWorkerMetrics::EventType event_type,
                 const base::TimeTicks& expiration,
                 TimeoutBehavior timeout_behavior);
     ~RequestInfo();
@@ skipping 57 matching lines @@
     mojo::InterfacePtr<Interface> interface_;
     base::WeakPtrFactory<Interface> weak_ptr_factory_;
   };
 
   typedef ServiceWorkerVersion self;
   using ServiceWorkerClients = std::vector<ServiceWorkerClientInfo>;
   using RequestInfoPriorityQueue =
       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
   // the IPC message to listen for, while CallbackType should be a callback
   // with same arguments as the IPC message.
   // Additionally only calls the callback for messages with a specific request
   // id, which must be the first argument of the IPC message.
   template <typename ResponseMessage, typename CallbackType>
   class EventResponseHandler : public EmbeddedWorkerInstance::Listener {
    public:
@@ skipping 313 matching lines @@
 
   // At this point |this| can have been deleted, so don't do anything other
   // than returning.
 
   return true;
 }
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_