ServiceWorker: Implement StartWorker by using mojo

content/browser/service_worker/embedded_worker_instance.cc

Index: content/browser/service_worker/embedded_worker_instance.cc
diff --git a/content/browser/service_worker/embedded_worker_instance.cc b/content/browser/service_worker/embedded_worker_instance.cc
index 1e78b20545b3cc5244ac2099cd7d5179a9716d06..0f7e5303406f20b2d3999c061a538e385a3d7bcc 100644
--- a/content/browser/service_worker/embedded_worker_instance.cc
+++ b/content/browser/service_worker/embedded_worker_instance.cc
@@ -21,10 +21,12 @@
 #include "content/common/service_worker/embedded_worker_settings.h"
 #include "content/common/service_worker/embedded_worker_setup.mojom.h"
 #include "content/common/service_worker/service_worker_types.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/child_process_host.h"
+#include "content/public/common/content_switches.h"
 #include "ipc/ipc_message.h"
 #include "services/shell/public/cpp/interface_provider.h"
 #include "services/shell/public/cpp/interface_registry.h"
@@ -69,7 +71,7 @@ void NotifyWorkerVersionDoomedOnUI(int worker_process_id, int worker_route_id) {
       worker_process_id, worker_route_id);
 }
 
-void RegisterToWorkerDevToolsManagerOnUI(
+void RegisterToWorkerDevToolsManagerAndBindInterfaceOnUI(

[falken, 2016/09/07 05:01:41]

These are now two pretty unrelated tasks. Maybe just SetupOnUI or RegisterOnUI
naming is better?

[shimazu, 2016/09/12 06:28:19]

Done.

     int process_id,
     const ServiceWorkerContextCore* service_worker_context,
     const base::WeakPtr<ServiceWorkerContextCore>& service_worker_context_weak,
@@ -77,6 +79,7 @@ void RegisterToWorkerDevToolsManagerOnUI(
     const GURL& url,
     const GURL& scope,
     bool is_installed,
+    mojom::EmbeddedWorkerInstanceClientRequest request,
     const base::Callback<void(int worker_devtools_agent_route_id,
                               bool wait_for_debugger)>& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -92,6 +95,7 @@ void RegisterToWorkerDevToolsManagerOnUI(
                 service_worker_context, service_worker_context_weak,
                 service_worker_version_id, url, scope),
             is_installed);
+    rph->GetRemoteInterfaces()->GetInterface(std::move(request));
   }
   BrowserThread::PostTask(
       BrowserThread::IO,
@@ -211,8 +215,11 @@ class EmbeddedWorkerInstance::StartTask {
  public:
   enum class ProcessAllocationState { NOT_ALLOCATED, ALLOCATING, ALLOCATED };
 
-  StartTask(EmbeddedWorkerInstance* instance, const GURL& script_url)
+  StartTask(EmbeddedWorkerInstance* instance,
+            const GURL& script_url,
+            mojom::EmbeddedWorkerInstanceClientRequest request)
       : instance_(instance),
+        request_(std::move(request)),
         state_(ProcessAllocationState::NOT_ALLOCATED),
         is_installed_(false),
         started_during_browser_startup_(false),
@@ -342,15 +349,18 @@ class EmbeddedWorkerInstance::StartTask {
     GURL script_url(params->script_url);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(RegisterToWorkerDevToolsManagerOnUI, process_id,
-                   instance_->context_.get(), instance_->context_,
-                   service_worker_version_id, script_url, scope, is_installed_,
-                   base::Bind(&StartTask::OnRegisteredToDevToolsManager,
-                              weak_factory_.GetWeakPtr(), base::Passed(&params),
-                              is_new_process)));
-  }
-
-  void OnRegisteredToDevToolsManager(
+        base::Bind(
+            RegisterToWorkerDevToolsManagerAndBindInterfaceOnUI, process_id,
+            instance_->context_.get(), instance_->context_,
+            service_worker_version_id, script_url, scope, is_installed_,
+            base::Passed(&request_),
+            base::Bind(
+                &StartTask::OnRegisteredToDevToolsManagerAndBoundInterface,
+                weak_factory_.GetWeakPtr(), base::Passed(&params),
+                is_new_process)));
+  }
+
+  void OnRegisteredToDevToolsManagerAndBoundInterface(
       std::unique_ptr<EmbeddedWorkerMsg_StartWorker_Params> params,
       bool is_new_process,
       int worker_devtools_agent_route_id,
@@ -366,7 +376,11 @@ class EmbeddedWorkerInstance::StartTask {
 
     params->worker_devtools_agent_route_id = worker_devtools_agent_route_id;
     params->wait_for_debugger = wait_for_debugger;
-    SendStartWorker(std::move(params));
+
+    if (ServiceWorkerUtils::IsMojoForServiceWorkerEnabled())
+      instance_->SendMojoStartWorker(std::move(params));
+    else
+      SendStartWorker(std::move(params));
   }
 
   void SendStartWorker(
@@ -392,6 +406,7 @@ class EmbeddedWorkerInstance::StartTask {
 
   // |instance_| must outlive |this|.
   EmbeddedWorkerInstance* instance_;
+  mojom::EmbeddedWorkerInstanceClientRequest request_;
 
   StatusCallback start_callback_;
   ProcessAllocationState state_;
@@ -445,7 +460,11 @@ void EmbeddedWorkerInstance::Start(
   params->wait_for_debugger = false;
   params->settings.v8_cache_options = GetV8CacheOptions();
 
-  inflight_start_task_.reset(new StartTask(this, params->script_url));
+  mojom::EmbeddedWorkerInstanceClientRequest request =
+      GetProxy(&instance_client_);

[falken, 2016/09/07 05:01:41]

Where is GetProxy from? should this be mojo::GetProxy?

[shimazu, 2016/09/12 06:28:19]

Done.

+
+  inflight_start_task_.reset(
+      new StartTask(this, params->script_url, std::move(request)));
   inflight_start_task_->Start(std::move(params), callback);
 }
 
@@ -565,6 +584,33 @@ void EmbeddedWorkerInstance::OnRegisteredToDevToolsManager(
   }
 }
 
+void EmbeddedWorkerInstance::SendMojoStartWorker(
+    std::unique_ptr<EmbeddedWorkerMsg_StartWorker_Params> params) {
+  auto mojo_params = mojom::EmbeddedWorkerStartWorkerParams::New();

[dcheng, 2016/09/07 07:51:09]

It might be slightly simpler to just declare
EmbeddedWorkerMsg_StartWorker_Params as a native mojo struct for now, and then
just migrate over when we remove the non-mojo path.

Alternatively, we can typemap (go/chrome-mojo-docs/typemapping), which is
preferable to writing manual serialization code like this.

[shimazu, 2016/09/12 06:28:19]

Done.

+  mojo_params->embedded_worker_id = params->embedded_worker_id;
+  mojo_params->service_worker_version_id = params->service_worker_version_id;
+  mojo_params->scope = params->scope;
+  mojo_params->script_url = params->script_url;
+  mojo_params->worker_devtools_agent_route_id =
+      params->worker_devtools_agent_route_id;
+  mojo_params->pause_after_download = params->pause_after_download;
+  mojo_params->wait_for_debugger = params->wait_for_debugger;
+  mojo_params->is_installed = params->is_installed;
+
+  mojo_params->settings = mojom::EmbeddedWorkerSettings::New();
+  mojo_params->settings->data_saver_enabled =
+      params->settings.data_saver_enabled;
+  mojo_params->settings->v8_cache_options =
+      static_cast<mojom::V8CacheOptions>(params->settings.v8_cache_options);
+
+  instance_client_->StartWorker(std::move(mojo_params));
+  registry_->AddWorker(process_id(), embedded_worker_id());

[falken, 2016/09/07 05:01:41]

The registry already holds all workers, even ones that aren't started yet. So
AddWorker() seems like a misleading name. Are you trying to notify the registry
that this worker is starting up?

[shimazu, 2016/09/12 06:28:19]

Done.

+  TRACE_EVENT_ASYNC_STEP_PAST1("ServiceWorker", "EmbeddedWorkerInstance::Start",
+                               this, "SendStartWorker", "Status", "mojo");
+  OnStartWorkerMessageSent();
+  return;
+}
+
 void EmbeddedWorkerInstance::OnStartWorkerMessageSent() {
   if (!step_time_.is_null()) {
     base::TimeDelta duration = UpdateStepTime();