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)>;
 
   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 146 matching lines @@
   // Connects to a specific mojo service exposed by the (running) service
   // worker. If a connection to a service for the same Interface already exists
   // this will return that existing connection. The |request_id| must be a value
   // previously returned by StartRequest. If the connection to the service
   // fails or closes before the request finished, the error callback associated
   // with |request_id| is called.
   // Only call GetMojoServiceForRequest once for a specific |request_id|.
   template <typename Interface>
   base::WeakPtr<Interface> GetMojoServiceForRequest(int request_id);
 
-  // Dispatches an event. If dispatching the event fails, the error callback
-  // associated with the |request_id| is called. Any messages sent back in
-  // response to this event are passed on to the response |callback|.
+  // Dispatches an event. This method assumes you already registered callbacks
+  // to receive the IPCs and does not create EventResponseHandler. If
+  // dispatching the event fails, all of the error callbacks associated with the
+  // |request_id|s are called. Any messages sent back in  response to this event
+  // are passed on to the response |callback|.
+  // This must be called when the worker is running.
+  template <typename... RequestIDs>
+  ServiceWorkerStatusCode DispatchEvent(const IPC::Message& message,
+                                        int request_id,
+                                        const RequestIDs&... rest);

[falken, 2016/06/06 10:01:40]

Does RequestIDs need to be a template type? I'd rather keep this dump and simple
if possible.
Can you just have one-int and two-int overloaded functions? Or take in a vector
of ints?

[shimazu, 2016/06/07 02:27:40]

Yes, vector<int> and two-int arguments will also work, but I thought
variable-length arguments is better because it's more general. This can be used
for any number of responses. Moreover, practically we will use this for only two
or three replies, so I thought template is more handy than vector<int> for the
users of this function.

[falken, 2016/06/07 07:56:17]

I'd prefer either vector<int> or two-int over the template. It'll keep the code
and stacktraces simple. There's not many uses of typename... in our codebase.

[shimazu, 2016/06/09 05:44:21]

I see. I don't have so much preference for the template. As you said, vector<>
is simpler stacktrace and codes.
Done.

+  ServiceWorkerStatusCode DispatchEvent(const IPC::Message& message);
+
+  // This method registers a callback corresponding to |request_id|, but
+  // this doesn't dispatch a message.
   // ResponseMessage is the type of the IPC message that is used for the
   // response, and its first argument MUST be the request_id.
-  // This must be called when the worker is running.
+  // Callback registration should be done once for each request.
   template <typename ResponseMessage, typename ResponseCallbackType>
-  void DispatchEvent(int request_id,
-                     const IPC::Message& message,
-                     const ResponseCallbackType& callback);
+  void RegisterRequestCallback(int request_id,
+                               const ResponseCallbackType& callback);
 
-  // For simple events where the full functionality of DispatchEvent is not
-  // needed, this method can be used instead. The ResponseMessage must consist
+  // This method registers the simple callback corresponding to |request_id|,
+  // but this doesn't dispatch a message.  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.
+  // This is internally used for DispatchSimpleEvent.
+  // Callback registration should be done once for each request.
+  template <typename ResponseMessage>
+  void RegisterSimpleRequest(int request_id);
+
+  // This is a wrapper method integrating one RegisterSimpleRequest and one
+  // DispatchEvent. For simple events where the full functionality of
+  // RegisterRequestCallback/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);
 
   // 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);
 
@@ skipping 87 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 13 matching lines @@
 
     CallbackType callback;
     base::TimeTicks start_time;
     ServiceWorkerMetrics::EventType event_type;
     // Name of the mojo service this request is associated with. Used to call
     // the callback when a connection closes with outstanding requests.
     // Compared as pointer, so should only contain static strings. Typically
     // this would be Interface::Name_ for some mojo interface.
     const char* mojo_service = nullptr;
     std::unique_ptr<EmbeddedWorkerInstance::Listener> listener;
+    bool is_dispatched;

[falken, 2016/06/06 10:01:40]

Need to initialize this.

[shimazu, 2016/06/09 05:44:21]

Done.

   };
 
   // Base class to enable storing a list of mojo interface pointers for
   // arbitrary interfaces. The destructor is also responsible for calling the
   // error callbacks for any outstanding requests using this service.
   class CONTENT_EXPORT BaseMojoServiceWrapper {
    public:
     BaseMojoServiceWrapper(ServiceWorkerVersion* worker,
                            const char* service_name);
     virtual ~BaseMojoServiceWrapper();
@@ skipping 24 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 258 matching lines @@
     interface.set_connection_error_handler(
         base::Bind(&ServiceWorkerVersion::OnMojoConnectionError,
                    weak_factory_.GetWeakPtr(), Interface::Name_));
     service = new MojoServiceWrapper<Interface>(this, std::move(interface));
     mojo_services_.add(Interface::Name_, base::WrapUnique(service));
   }
   request->mojo_service = Interface::Name_;
   return service->GetWeakPtr();
 }
 
-template <typename ResponseMessage, typename ResponseCallbackType>
-void ServiceWorkerVersion::DispatchEvent(int request_id,
-                                         const IPC::Message& message,
-                                         const ResponseCallbackType& callback) {
+template <typename... RequestIDs>
+ServiceWorkerStatusCode ServiceWorkerVersion::DispatchEvent(
+    const IPC::Message& message,
+    int request_id,
+    const RequestIDs&... rest) {
   DCHECK_EQ(RUNNING, running_status());
   PendingRequest<StatusCallback>* request = custom_requests_.Lookup(request_id);
   DCHECK(request) << "Invalid request id";
-  DCHECK(!request->listener) << "Request already dispatched an IPC event";
-
-  ServiceWorkerStatusCode status = embedded_worker_->SendMessage(message);
+  DCHECK(!request->is_dispatched) << "Request already dispatched an IPC event";
+  const ServiceWorkerStatusCode status = DispatchEvent(message, rest...);
   if (status != SERVICE_WORKER_OK) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(request->callback, status));
     custom_requests_.Remove(request_id);
   } else {
-    request->listener.reset(
-        new EventResponseHandler<ResponseMessage, ResponseCallbackType>(
-            embedded_worker()->AsWeakPtr(), request_id, callback));
+    request->is_dispatched = true;
   }
+  return status;
 }
 
 template <typename ResponseMessage>
 void ServiceWorkerVersion::DispatchSimpleEvent(int request_id,
                                                const IPC::Message& message) {
-  DispatchEvent<ResponseMessage>(
-      request_id, message,
+  RegisterSimpleRequest<ResponseMessage>(request_id);
+  DispatchEvent(message, request_id);
+}
+
+template <typename ResponseMessage, typename ResponseCallbackType>
+void ServiceWorkerVersion::RegisterRequestCallback(
+    int request_id,
+    const ResponseCallbackType& callback) {
+  PendingRequest<StatusCallback>* request = custom_requests_.Lookup(request_id);
+  DCHECK(request) << "Invalid request id";
+  DCHECK(!request->listener) << "Callback was already registered";
+  DCHECK(!request->is_dispatched) << "Request already dispatched an IPC event";
+  request->listener.reset(
+      new EventResponseHandler<ResponseMessage, ResponseCallbackType>(
+          embedded_worker()->AsWeakPtr(), request_id, callback));
+}
+
+template <typename ResponseMessage>
+void ServiceWorkerVersion::RegisterSimpleRequest(int request_id) {
+  RegisterRequestCallback<ResponseMessage>(
+      request_id,
       base::Bind(&ServiceWorkerVersion::OnSimpleEventResponse, this));
 }
 
 template <typename ResponseMessage, typename CallbackType>
 bool ServiceWorkerVersion::EventResponseHandler<ResponseMessage, CallbackType>::
     OnMessageReceived(const IPC::Message& message) {
   if (message.type() != ResponseMessage::ID)
     return false;
   int received_request_id;
   bool result = base::PickleIterator(message).ReadInt(&received_request_id);
   if (!result || received_request_id != request_id_)
     return false;
 
   CallbackType protect(callback_);
   // Essentially same code as what IPC_MESSAGE_FORWARD expands to.
   void* param = nullptr;
   if (!ResponseMessage::Dispatch(&message, &callback_, this, param,
                                  &CallbackType::Run))
     message.set_dispatch_error();
 
   // 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_