Create ServiceWorkerProviderHost before starting worker

content/browser/service_worker/service_worker_provider_host.h

Index: content/browser/service_worker/service_worker_provider_host.h
diff --git a/content/browser/service_worker/service_worker_provider_host.h b/content/browser/service_worker/service_worker_provider_host.h
index f2ed389537b99cbe513b74dd77380e354a832b68..42cb565727d643d33fe35d478b0a0257c100c81a 100644
--- a/content/browser/service_worker/service_worker_provider_host.h
+++ b/content/browser/service_worker/service_worker_provider_host.h
@@ -58,17 +58,23 @@ class WebContents;
 // For providers hosting a running service worker, this class will observe
 // resource loads made directly by the service worker.
 //
-// A ServiceWorkerProviderHost instance is created when a
-// ServiceWorkerNetworkProvider is created on the renderer process, which
-// happens 1) when a document or worker (i.e., a service worker client) is
-// created, or 2) during service worker startup. Mojo's connection from
-// ServiceWorkerNetworkProvider is established on the creation time, and the
-// instance is destroyed on disconnection from the renderer side.
-// If PlzNavigate is turned on, an instance is pre-created on the browser
+// A ServiceWorkerProviderHost instance is possible to be created on the browser
+// and the renderer. 1) When it's for a document or worker (i.e., a service

[falken, 2017/06/14 08:10:35]

"created on the browser and the renderer" sounds like it can be created in the
renderer process.

How about:

A ServiceWorkerProviderHost is created in the following situations:
1) When it's for a document or worker...
2) When it's for a service worker context...
3) When PlzNavigate is turned on...

[shimazu, 2017/06/16 04:18:44]

Yeah, that looks great! Updated the comment.

+// worker client), the provider host is created when
+// ServiceWorkerNetworkProvider is created on the renderer process. Mojo's
+// connection from ServiceWorkerNetworkProvider is established on the creation
+// time.
+// 2) When it's for a service worker context, the provider host is created on
+// the browser process before launching the service worker's thread. Mojo's
+// connection to the renderer is established with the StartWorker message.
+// 3) When PlzNavigate is turned on, an instance is pre-created on the browser
 // before ServiceWorkerNetworkProvider is created on the renderer because
 // navigation is initiated on the browser side. In that case, establishment of
 // Mojo's connection will be deferred until ServiceWorkerNetworkProvider is
 // created on the renderer.
+// Destruction of the ServiceWorkerProviderHost instance happens on
+// disconnection of the Mojo's pipe from the renderer side regardless of what
+// the provider is for.
 class CONTENT_EXPORT ServiceWorkerProviderHost
     : public NON_EXPORTED_BASE(ServiceWorkerRegistration::Listener),
       public base::SupportsWeakPtr<ServiceWorkerProviderHost>,
@@ -91,6 +97,12 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
       bool are_ancestors_secure,
       const WebContentsGetter& web_contents_getter);
 
+  // Creates ServiceWorkerProviderHost for the worker's context. Information
+  // about the ServiceWorkerProviderHost is passed with the StartWorker message.

[falken, 2017/06/14 08:10:35]

The use of "context" here is confusing since the |context| param is not the
worker's context. Overall "context" is a really overloaded term for service
workers.

Recommend changing this to "PreCreateForHostedVersion".

// Creates a ServiceWorkerProviderHost for hosting a running service worker.

[kinuko, 2017/06/14 08:37:40]

Or PreCreateForController? (The version is actually not yet hosted yet...)

[shimazu, 2017/06/16 04:18:44]

Done.

+  static std::unique_ptr<ServiceWorkerProviderHost> PreCreateForWorkerContext(
+      ServiceWorkerVersion* hosted_version,
+      base::WeakPtr<ServiceWorkerContextCore> context);
+
   // Used to create a ServiceWorkerProviderHost when the renderer-side provider
   // is created. This ProviderHost will be created for the process specified by
   // |process_id|.
@@ -200,8 +212,6 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // Clears the associated registration and stop listening to it.
   void DisassociateRegistration();
 
-  void SetHostedVersion(ServiceWorkerVersion* version);
-
   // Returns a handler for a request, the handler may return NULL if
   // the request doesn't require special handling.
   std::unique_ptr<ServiceWorkerRequestHandler> CreateRequestHandler(
@@ -270,6 +280,11 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
       ServiceWorkerProviderHostInfo info,
       ServiceWorkerDispatcherHost* dispatcher_host);
 
+  // Completes initialization of provider hosts for controllers.

[falken, 2017/06/14 08:10:35]

When is this called?

[falken, 2017/06/14 08:30:18]

(I mean to add something to the documentation like this is called after a
process is allocated for the worker.)

[shimazu, 2017/06/16 04:18:43]

Done.

+  virtual void CompleteStartWorkerPreparation(
+      int process_id,
+      mojom::ServiceWorkerProviderClientInfoPtr* provider_client_info);

[falken, 2017/06/14 08:10:35]

Does this mutate |provider_client_info|? If so, |out_provider_client_info| or
maybe just |out_info|

[shimazu, 2017/06/16 04:18:44]

Done.

+
   // Sends event messages to the renderer. Events for the worker are queued up
   // until the worker thread id is known via SetReadyToSendMessagesToWorker().
   void SendUpdateFoundMessage(
@@ -305,6 +320,12 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   void BindWorkerFetchContext(
       mojom::ServiceWorkerWorkerClientAssociatedPtrInfo client_ptr_info);
 
+ protected:
+  ServiceWorkerProviderHost(int process_id,
+                            ServiceWorkerProviderHostInfo info,
+                            base::WeakPtr<ServiceWorkerContextCore> context,
+                            ServiceWorkerDispatcherHost* dispatcher_host);
+
  private:
   friend class ForeignFetchRequestHandlerTest;
   friend class LinkHeaderServiceWorkerTest;
@@ -335,11 +356,6 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
     ~OneShotGetReadyCallback();
   };
 
-  ServiceWorkerProviderHost(int process_id,
-                            ServiceWorkerProviderHostInfo info,
-                            base::WeakPtr<ServiceWorkerContextCore> context,
-                            ServiceWorkerDispatcherHost* dispatcher_host);
-
   // ServiceWorkerRegistration::Listener overrides.
   void OnVersionAttributesChanged(
       ServiceWorkerRegistration* registration,
@@ -373,6 +389,10 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   bool IsReadyToSendMessages() const;
   void Send(IPC::Message* message) const;
 
+  // Sets the version hosted by this provider. This method should be used only
+  // for provider hosts for controllers.
+  void SetHostedVersion(ServiceWorkerVersion* version);
+
   // Notifies the information about the controller and associated registration
   // to the provider on the renderer. This is for cross site transfer and
   // browser side navigation which need to decide which process will handle the