Create ServiceWorkerProviderHost before starting worker

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 50adf19c784ffdcc79de8a15e9702d286bece151..351da240a4cf22f75c340c291877ac2f34c32149 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -847,6 +847,9 @@ ServiceWorkerVersion::PendingRequest::~PendingRequest() {}
 
 void ServiceWorkerVersion::OnThreadStarted() {
   DCHECK_EQ(EmbeddedWorkerStatus::STARTING, running_status());
+  DCHECK(provider_host_);
+  provider_host_->SetReadyToSendMessagesToWorker(
+      embedded_worker()->thread_id());
   // Activate ping/pong now that JavaScript execution will start.
   ping_controller_->Activate();
 }
@@ -856,6 +859,11 @@ void ServiceWorkerVersion::OnStarting() {
     observer.OnRunningStateChanged(this);
 }
 
+void ServiceWorkerVersion::OnStartWorkerMessageSent() {
+  DCHECK(pending_provider_host_);
+  context()->AddProviderHost(std::move(pending_provider_host_));

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

I have a feeling that SetHostedVersion should be called here rather than in
PreCreate method, as we're temporarily creating a cyclic reference until this
point... wdyt?

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

That's good point...
I tried to fix it by 

- moving SetHostedVersionId into CompleteStartWorkerPreparation
- having the provider host in ProviderClientInfoGetter which is newly introduced
on ps#18
- passing the getter to EWInstance::Start instead of provider_host
- using AddProviderHost and CompleteStartWorkerPreparation at the same function

I hope this makes things clearer. 
EWInstance doesn't have the provider host explicitly, and SWVersion doesn't have
the strong reference to the provider host. The references are safe if
ProviderClientInfoGetter ensures the cyclic reference doesn't happen.

+}
+
 void ServiceWorkerVersion::OnStarted() {
   DCHECK_EQ(EmbeddedWorkerStatus::RUNNING, running_status());
   RestartTick(&idle_time_);
@@ -1466,6 +1474,11 @@ void ServiceWorkerVersion::StartWorkerInternal() {
 
   StartTimeoutTimer();
 
+  DCHECK(!pending_provider_host_);
+  pending_provider_host_ =
+      ServiceWorkerProviderHost::PreCreateForWorkerContext(this, context());
+  provider_host_ = pending_provider_host_->AsWeakPtr();
+
   auto params = base::MakeUnique<EmbeddedWorkerStartParams>();
   params->service_worker_version_id = version_id_;
   params->scope = scope_;
@@ -1474,7 +1487,8 @@ void ServiceWorkerVersion::StartWorkerInternal() {
   params->pause_after_download = pause_after_download_;
 
   embedded_worker_->Start(
-      std::move(params), mojo::MakeRequest(&event_dispatcher_),
+      std::move(params), pending_provider_host_->AsWeakPtr(),
+      mojo::MakeRequest(&event_dispatcher_),
       base::Bind(&ServiceWorkerVersion::OnStartSentAndScriptEvaluated,
                  weak_factory_.GetWeakPtr()));
   event_dispatcher_.set_connection_error_handler(base::Bind(