Manage ServiceWorkerDispatcherHost in ServiceWorkerContextCore

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 3ed67ba33f4abb82da49716386d9da2e94b86508..fb5d4d7123a266e96aef82bbfce4a59cdf62a583 100644
--- a/content/browser/service_worker/service_worker_provider_host.h
+++ b/content/browser/service_worker/service_worker_provider_host.h
@@ -20,6 +20,7 @@
 #include "base/memory/weak_ptr.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/common/content_export.h"
+#include "content/common/service_worker/service_worker_provider.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/common/request_context_frame_type.h"
 #include "content/public/common/request_context_type.h"
@@ -71,22 +72,25 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
       bool are_ancestors_secure,
       const WebContentsGetter& web_contents_getter);
 
+  // Used to create a ServiceWorkerProviderHost when the renderer-side provider
+  // is created. This ProviderHost will be created for the process specified by
+  // |process_id|.
+  static std::unique_ptr<ServiceWorkerProviderHost> Create(
+      int process_id,
+      ServiceWorkerProviderHostInfo info,
+      base::WeakPtr<ServiceWorkerContextCore> context,
+      ServiceWorkerDispatcherHost* dispatcher_host);
+
+  static std::unique_ptr<ServiceWorkerProviderHost> CreateForTesting(

[horo, 2017/01/26 02:40:23]

How about adding "bool is_parent_frame_secure" argument?
You can remove FrameSecurityLevel::UNINITIALIZED and make
|parent_frame_security_level_| const, and remove set_parent_frame_secure().

[falken, 2017/01/26 05:15:43]

Can this be moved to a file like
service_worker/test/service_worker_test_utils.h? I'd like to avoid test-only
code in the release binary.

[shimazu, 2017/01/26 09:27:23]

Acknowledged.

[shimazu, 2017/02/07 08:47:02]

Done.

+      int process_id,
+      int provider_id,
+      ServiceWorkerProviderType type,
+      base::WeakPtr<ServiceWorkerContextCore> context,
+      int route_id = MSG_ROUTING_NONE,
+      ServiceWorkerDispatcherHost* dispatcher_host = nullptr);

[falken, 2017/01/26 05:15:43]

The style guide discourages default arguments. Could we possibly avoid them here
with better named functions (CreateForServiceWorkerContext,
CreateWithDispatcherHost, etc?). Or should everyone just pass
ServiceWorkerProviderHostInfo struct, and we have helpers for creating that Info
struct?

[shimazu, 2017/01/26 09:27:23]

Acknowledged.

[shimazu, 2017/02/07 08:47:02]

I've tried to add CreateForXXX to test_utils and used
ServcieWorkerProviderHostInfo everywhere.

+
   enum class FrameSecurityLevel { UNINITIALIZED, INSECURE, SECURE };
 
-  // When this provider host is for a Service Worker context, |route_id| is
-  // MSG_ROUTING_NONE. When this provider host is for a Document,
-  // |route_id| is the frame ID of the Document. When this provider host is for
-  // a Shared Worker, |route_id| is the Shared Worker route ID.
-  // |provider_type| gives additional information whether the provider is
-  // created for controller (ServiceWorker) or controllee (Document or
-  // SharedWorker).
-  ServiceWorkerProviderHost(int render_process_id,
-                            int route_id,
-                            int provider_id,
-                            ServiceWorkerProviderType provider_type,
-                            FrameSecurityLevel parent_frame_security_level,
-                            base::WeakPtr<ServiceWorkerContextCore> context,
-                            ServiceWorkerDispatcherHost* dispatcher_host);
   virtual ~ServiceWorkerProviderHost();
 
   const std::string& client_uuid() const { return client_uuid_; }
@@ -247,7 +251,7 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   bool GetRegistrationForReady(const GetRegistrationForReadyCallback& callback);
 
   // Methods to support cross site navigations.
-  void PrepareForCrossSiteTransfer();
+  std::unique_ptr<ServiceWorkerProviderHost> PrepareForCrossSiteTransfer();
   void CompleteCrossSiteTransfer(
       int new_process_id,
       int new_frame_id,
@@ -324,6 +328,14 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
     ~OneShotGetReadyCallback();
   };
 
+  ServiceWorkerProviderHost(int render_process_id,
+                            int route_id,
+                            int provider_id,
+                            ServiceWorkerProviderType provider_type,
+                            FrameSecurityLevel parent_frame_security_level,
+                            base::WeakPtr<ServiceWorkerContextCore> context,
+                            ServiceWorkerDispatcherHost* dispatcher_host);
+
   // ServiceWorkerRegistration::Listener overrides.
   void OnVersionAttributesChanged(
       ServiceWorkerRegistration* registration,