Create ServiceWorkerProviderHost before starting worker

content/browser/service_worker/service_worker_test_utils.cc

Index: content/browser/service_worker/service_worker_test_utils.cc
diff --git a/content/browser/service_worker/service_worker_test_utils.cc b/content/browser/service_worker/service_worker_test_utils.cc
index 4c8f7390b91924cba6bc779d126ad9be89b5cfe6..5f29a3067d53934de6dc3d9719fdc20c98a24d72 100644
--- a/content/browser/service_worker/service_worker_test_utils.cc
+++ b/content/browser/service_worker/service_worker_test_utils.cc
@@ -7,6 +7,8 @@
 #include <utility>
 
 #include "content/browser/service_worker/service_worker_provider_host.h"
+#include "content/common/service_worker/service_worker_provider.mojom.h"
+#include "content/public/common/child_process_host.h"
 
 namespace content {
 
@@ -26,6 +28,12 @@ void ServiceWorkerRemoteProviderEndpoint::BindWithProviderHostInfo(
   info->host_request = mojo::MakeIsolatedRequest(&host_ptr_);
 }
 
+void ServiceWorkerRemoteProviderEndpoint::BindWithProviderClientInfo(
+    mojom::ServiceWorkerProviderClientInfo* info) {

[dcheng, 2017/06/20 21:09:47]

FWIW, I think it's a little surprising that this is passed by pointer rather
than by value, but this is test code, so I don't feel strongly.

Passing move-only types by value makes it clear that the thing you're calling
consumes members from it, whereas passing by pointer makes it unclear what
happens to it.

[shimazu, 2017/06/21 03:11:26]

Thanks. I agree that passing move-only object by value is straightforward.
But in this case, I think it's the same pattern with "request =
mojo::MakeRequest(&ptr)". Is this weird?

[dcheng, 2017/06/21 05:45:06]

I guess it's a matter of semantics. With MakeRequest, I expect it to modify
|ptr| but I don't expect it to consume (and possibly null out) the value in
|ptr|. This contrasts with this helper, which is essentially consuming the
information passed in |info|.

(I don't feel strongly about this, it's just a general suggestion)

[shimazu, 2017/06/21 06:27:14]

I see, unless the ProviderClientInfo is used on the caller side, it seems
simpler to pass the info. 
Updated.

+  client_request_ = std::move(info->client_request);
+  host_ptr_.Bind(std::move(info->host_ptr_info));
+}
+
 std::unique_ptr<ServiceWorkerProviderHost> CreateProviderHostForWindow(
     int process_id,
     int provider_id,
@@ -43,16 +51,19 @@ std::unique_ptr<ServiceWorkerProviderHost> CreateProviderHostForWindow(
 std::unique_ptr<ServiceWorkerProviderHost>
 CreateProviderHostForServiceWorkerContext(
     int process_id,
-    int provider_id,
     bool is_parent_frame_secure,
+    ServiceWorkerVersion* hosted_version,
     base::WeakPtr<ServiceWorkerContextCore> context,
     ServiceWorkerRemoteProviderEndpoint* output_endpoint) {
-  ServiceWorkerProviderHostInfo info(provider_id, MSG_ROUTING_NONE,
-                                     SERVICE_WORKER_PROVIDER_FOR_CONTROLLER,
-                                     is_parent_frame_secure);
-  output_endpoint->BindWithProviderHostInfo(&info);
-  return ServiceWorkerProviderHost::Create(process_id, std::move(info),
-                                           std::move(context), nullptr);
+  ServiceWorkerProviderHostInfo info(
+      kInvalidServiceWorkerProviderId, MSG_ROUTING_NONE,
+      SERVICE_WORKER_PROVIDER_FOR_CONTROLLER, is_parent_frame_secure);
+  std::unique_ptr<ServiceWorkerProviderHost> host =
+      ServiceWorkerProviderHost::PreCreateForController(std::move(context));
+  mojom::ServiceWorkerProviderClientInfoPtr provider_client_info =
+      host->CompleteStartWorkerPreparation(process_id, hosted_version);
+  output_endpoint->BindWithProviderClientInfo(provider_client_info.get());
+  return host;
 }
 
 std::unique_ptr<ServiceWorkerProviderHost> CreateProviderHostWithDispatcherHost(