Create EmbeddedWorkerInstanceClient from the Render interface

This change is to create EmbeddedWorkerInstanceClient from the Renderer
interface. It used to be created from the InterfaceRegistry, but we didn't have
guarantee of IPC message orderings between StartWorker and messages onto the
Renderer interface. StartWorker needs to be run after initialization of the
renderer (crrev.com/c/1031693 will introduce SetUserAgent message), so this CL
is to put the very first IPC for the service worker, which is establishing the
Mojo connection, on the Renderer interface.

Bug: 818450
Change-Id: I5a5d64fa9aef73ca602633b2b3664110d0a761c9
Reviewed-on: https://chromium-review.googlesource.com/1053042
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557804}

content/browser/service_worker/embedded_worker_instance.cc

@@ -17,6 +17,7 @@
 #include "content/browser/service_worker/service_worker_content_settings_proxy_impl.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/common/content_switches_internal.h"
+#include "content/common/renderer.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"
@@ -128,7 +129,8 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
   // the process. If the process dies, |client_|'s connection error callback
   // will be called on the IO thread.
   if (request.is_pending()) {
-    BindInterface(rph, std::move(request));
+    rph->GetRendererInterface()->SetUpEmbeddedWorkerChannelForServiceWorker(
+        std::move(request));
   }
 
   // S13nServiceWorker:

content/browser/service_worker/embedded_worker_instance_unittest.cc

@@ -849,9 +849,11 @@ TEST_F(EmbeddedWorkerInstanceTest, FailToSendStartIPC) {
   base::RunLoop().RunUntilIdle();
 
   // Worker should handle the failure of binding on the remote side as detach.
-  ASSERT_EQ(1u, events_.size());
-  EXPECT_EQ(DETACHED, events_[0].type);
-  EXPECT_EQ(EmbeddedWorkerStatus::STARTING, events_[0].status);
+  ASSERT_EQ(3u, events_.size());
+  EXPECT_EQ(PROCESS_ALLOCATED, events_[0].type);
+  EXPECT_EQ(START_WORKER_MESSAGE_SENT, events_[1].type);
+  EXPECT_EQ(DETACHED, events_[2].type);
+  EXPECT_EQ(EmbeddedWorkerStatus::STARTING, events_[2].status);
   EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, worker->status());
 }
 

content/browser/service_worker/embedded_worker_test_helper.cc

@@ -23,12 +23,14 @@
 #include "content/browser/service_worker/service_worker_dispatcher_host.h"
 #include "content/browser/service_worker/service_worker_test_utils.h"
 #include "content/common/background_fetch/background_fetch_types.h"
+#include "content/common/renderer.mojom.h"
 #include "content/common/service_worker/service_worker_event_dispatcher.mojom.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/push_event_payload.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_context.h"
+#include "mojo/public/cpp/bindings/associated_binding_set.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
@@ -134,8 +136,7 @@ void EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::
 // static
 void EmbeddedWorkerTestHelper::MockEmbeddedWorkerInstanceClient::Bind(
     const base::WeakPtr<EmbeddedWorkerTestHelper>& helper,
-    mojo::ScopedMessagePipeHandle request_handle) {
-  mojom::EmbeddedWorkerInstanceClientRequest request(std::move(request_handle));
+    mojom::EmbeddedWorkerInstanceClientRequest request) {
   std::vector<std::unique_ptr<MockEmbeddedWorkerInstanceClient>>* clients =
       helper->mock_instance_clients();
   size_t next_client_index = helper->mock_instance_clients_next_index_;
@@ -323,6 +324,65 @@ class EmbeddedWorkerTestHelper::MockServiceWorkerEventDispatcher
   const int thread_id_;
 };
 
+class EmbeddedWorkerTestHelper::MockRendererInterface : public mojom::Renderer {
+ public:
+  explicit MockRendererInterface(base::WeakPtr<EmbeddedWorkerTestHelper> helper)
+      : helper_(helper) {}
+
+  void AddBinding(mojom::RendererAssociatedRequest request) {
+    bindings_.AddBinding(this, std::move(request));
+  }
+
+ private:
+  void CreateEmbedderRendererService(
+      service_manager::mojom::ServiceRequest service_request) override {
+    NOTREACHED();
+  }
+  void CreateView(mojom::CreateViewParamsPtr) override { NOTREACHED(); }
+  void CreateFrame(mojom::CreateFrameParamsPtr) override { NOTREACHED(); }
+  void SetUpEmbeddedWorkerChannelForServiceWorker(
+      mojom::EmbeddedWorkerInstanceClientRequest client_request) override {
+    MockEmbeddedWorkerInstanceClient::Bind(helper_, std::move(client_request));
+  }
+  void CreateFrameProxy(
+      int32_t routing_id,
+      int32_t render_view_routing_id,
+      int32_t opener_routing_id,
+      int32_t parent_routing_id,
+      const FrameReplicationState& replicated_state,
+      const base::UnguessableToken& devtools_frame_token) override {
+    NOTREACHED();
+  }
+  void OnNetworkConnectionChanged(
+      net::NetworkChangeNotifier::ConnectionType type,
+      double max_bandwidth_mbps) override {
+    NOTREACHED();
+  }
+  void OnNetworkQualityChanged(net::EffectiveConnectionType type,
+                               base::TimeDelta http_rtt,
+                               base::TimeDelta transport_rtt,
+                               double bandwidth_kbps) override {
+    NOTREACHED();
+  }
+  void SetWebKitSharedTimersSuspended(bool suspend) override { NOTREACHED(); }
+  void UpdateScrollbarTheme(
+      mojom::UpdateScrollbarThemeParamsPtr params) override {
+    NOTREACHED();
+  }
+  void OnSystemColorsChanged(int32_t aqua_color_variant,
+                             const std::string& highlight_text_color,
+                             const std::string& highlight_color) override {
+    NOTREACHED();
+  }
+  void PurgePluginListCache(bool reload_pages) override { NOTREACHED(); }
+  void SetProcessBackgrounded(bool backgrounded) override { NOTREACHED(); }
+  void SetSchedulerKeepActive(bool keep_active) override { NOTREACHED(); }
+  void ProcessPurgeAndSuspend() override { NOTREACHED(); }
+
+  base::WeakPtr<EmbeddedWorkerTestHelper> helper_;
+  mojo::AssociatedBindingSet<mojom::Renderer> bindings_;
+};
+
 EmbeddedWorkerTestHelper::EmbeddedWorkerTestHelper(
     const base::FilePath& user_data_directory)
     : EmbeddedWorkerTestHelper(user_data_directory, nullptr) {}
@@ -353,14 +413,26 @@ EmbeddedWorkerTestHelper::EmbeddedWorkerTestHelper(
   EnsureDispatcherHostForProcess(mock_render_process_id());
   EnsureDispatcherHostForProcess(new_render_process_id());
 
-  render_process_host_->OverrideBinderForTesting(
-      mojom::EmbeddedWorkerInstanceClient::Name_,
-      base::BindRepeating(&MockEmbeddedWorkerInstanceClient::Bind,
-                          AsWeakPtr()));
-  new_render_process_host_->OverrideBinderForTesting(
-      mojom::EmbeddedWorkerInstanceClient::Name_,
-      base::BindRepeating(&MockEmbeddedWorkerInstanceClient::Bind,
-                          AsWeakPtr()));
+  // Install a mocked mojom::Renderer interface to catch requests to
+  // establish Mojo connection for EWInstanceClient.
+  mock_renderer_interface_ =
+      std::make_unique<MockRendererInterface>(AsWeakPtr());
+
+  auto renderer_interface_ptr =
+      std::make_unique<mojom::RendererAssociatedPtr>();
+  mock_renderer_interface_->AddBinding(
+      mojo::MakeRequestAssociatedWithDedicatedPipe(
+          renderer_interface_ptr.get()));
+  render_process_host_->OverrideRendererInterfaceForTesting(
+      std::move(renderer_interface_ptr));
+
+  auto new_renderer_interface_ptr =
+      std::make_unique<mojom::RendererAssociatedPtr>();
+  mock_renderer_interface_->AddBinding(
+      mojo::MakeRequestAssociatedWithDedicatedPipe(
+          new_renderer_interface_ptr.get()));
+  new_render_process_host_->OverrideRendererInterfaceForTesting(
+      std::move(new_renderer_interface_ptr));
 }
 
 EmbeddedWorkerTestHelper::~EmbeddedWorkerTestHelper() {

content/browser/service_worker/embedded_worker_test_helper.h

@@ -72,7 +72,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
     ~MockEmbeddedWorkerInstanceClient() override;
 
     static void Bind(const base::WeakPtr<EmbeddedWorkerTestHelper>& helper,
-                     mojo::ScopedMessagePipeHandle request_handle);
+                     mojom::EmbeddedWorkerInstanceClientRequest request);
 
    protected:
     // mojom::EmbeddedWorkerInstanceClient implementation.
@@ -278,6 +278,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
 
  private:
   class MockServiceWorkerEventDispatcher;
+  class MockRendererInterface;
 
   void DidSimulateWorkerScriptCached(int embedded_worker_id,
                                      bool pause_after_download);
@@ -360,6 +361,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
 
   IPC::TestSink sink_;
 
+  std::unique_ptr<MockRendererInterface> mock_renderer_interface_;
   std::vector<std::unique_ptr<MockEmbeddedWorkerInstanceClient>>
       mock_instance_clients_;
   size_t mock_instance_clients_next_index_;

content/common/renderer.mojom

@@ -5,6 +5,7 @@
 module content.mojom;
 
 import "content/common/native_types.mojom";
+import "content/common/service_worker/embedded_worker.mojom";
 import "ipc/constants.mojom";
 import "mojo/public/mojom/base/time.mojom";
 import "mojo/public/mojom/base/unguessable_token.mojom";
@@ -195,6 +196,13 @@ interface Renderer {
                    FrameReplicationState replication_state,
                    mojo_base.mojom.UnguessableToken devtools_frame_token);
 
+  // Tells the renderer to create an EmbeddedWorkerInstanceClient, which is what
+  // manages service worker startup and shutdown.
+  // TODO(shimazu): Send all params for starting service worker to reduce the
+  // number of IPCs.
+  SetUpEmbeddedWorkerChannelForServiceWorker(
+      EmbeddedWorkerInstanceClient& client_request);
+
   // Tells the renderer that the network type has changed so that
   // navigator.onLine and navigator.connection can be updated.
   OnNetworkConnectionChanged(NetworkConnectionType connection_type,

content/public/app/mojo/content_renderer_manifest.json

@@ -13,8 +13,6 @@
           "content::mojom::ChildControl",
           "content::mojom::ChildHistogramFetcher",
           "content::mojom::ChildHistogramFetcherFactory",
-          "content::mojom::EmbeddedWorkerInstanceClient",
-          "content::mojom::EmbeddedWorkerSetup",
           "content::mojom::FrameFactory",
           "content::mojom::RenderWidgetWindowTreeClientFactory",
           "content::mojom::ResourceUsageReporter",

content/renderer/render_thread_impl.cc

@@ -809,6 +809,7 @@ void RenderThreadImpl::Init(
 
   auto registry = std::make_unique<service_manager::BinderRegistry>();
   InitializeWebKit(resource_task_queue, registry.get());
+  blink_initialized_time_ = base::TimeTicks::Now();
 
   // In single process the single process is all there is.
   webkit_shared_timer_suspended_ = false;
@@ -865,10 +866,6 @@ void RenderThreadImpl::Init(
   registry->AddInterface(base::BindRepeating(CreateResourceUsageReporter,
                                              weak_factory_.GetWeakPtr()),
                          base::ThreadTaskRunnerHandle::Get());
-  registry->AddInterface(
-      base::BindRepeating(&EmbeddedWorkerInstanceClientImpl::Create,
-                          base::TimeTicks::Now(), GetIOTaskRunner()),
-      GetWebMainThreadScheduler()->IPCTaskRunner());
 
   GetServiceManagerConnection()->AddConnectionFilter(
       std::make_unique<SimpleConnectionFilter>(std::move(registry)));
@@ -2256,6 +2253,12 @@ void RenderThreadImpl::CreateFrameProxy(
       replicated_state, devtools_frame_token);
 }
 
+void RenderThreadImpl::SetUpEmbeddedWorkerChannelForServiceWorker(
+    mojom::EmbeddedWorkerInstanceClientRequest client_request) {
+  EmbeddedWorkerInstanceClientImpl::Create(
+      blink_initialized_time_, GetIOTaskRunner(), std::move(client_request));
+}
+
 void RenderThreadImpl::OnNetworkConnectionChanged(
     net::NetworkChangeNotifier::ConnectionType type,
     double max_bandwidth_mbps) {

content/renderer/render_thread_impl.h

@@ -562,6 +562,8 @@ class CONTENT_EXPORT RenderThreadImpl
       int32_t parent_routing_id,
       const FrameReplicationState& replicated_state,
       const base::UnguessableToken& devtools_frame_token) override;
+  void SetUpEmbeddedWorkerChannelForServiceWorker(
+      mojom::EmbeddedWorkerInstanceClientRequest client_request) override;
   void OnNetworkConnectionChanged(
       net::NetworkChangeNotifier::ConnectionType type,
       double max_bandwidth_mbps) override;
@@ -650,6 +652,9 @@ class CONTENT_EXPORT RenderThreadImpl
   // Used on the render thread.
   std::unique_ptr<VideoCaptureImplManager> vc_manager_;
 
+  // The time Blink was initialized. Used for UMA.
+  base::TimeTicks blink_initialized_time_;
+
   // The count of RenderWidgets running through this thread.
   int widget_count_;