[MBI]: Move RouteProvider impl from ChildThreadImpl => RenderThreadImpl

This CL moves the RouteProvider implementation from ChildThreadImpl,
to RenderThreadImpl. This is because RenderThreadImpl is the only type
of ChildThread(Impl) that uses this functionality, therefore this CL
creates a healthier boundary between render threads and child threads.

At the same time, it simplifies a nasty code path present in both
RenderFrameImpl/RenderFrameProxy::GetRemoteAssociatedInterfaces() that
separates a "production" path vs a "test" path. We do this by
introducing the GetRemoteRouteProvider() method on the RenderThread
public API, and providing two implementations:
 - RenderThreadImpl
    - Responsible for returning a legitimate mojom::RouteProvider* that
      will be hooked up to a real cross-process implementation
 - ChildThreadImpl
    - Responsible for providing a dummy mojom::RouteProvider* bound to
      nothing, so that the messages sent over it are simply drained

This is a reasonable tradeoff over the alternative solution of
introducing a MockAgentSchedulingGroup to handle cases where its
RenderThread is a MockRenderThread. If things regarding RenderThread
should be mocked, then it makes sense for RenderThread implementations
to mock them, as opposed to have mocks appear in both MockRenderThread
*and* MockAgentSchedulingGroup, which is slightly more awkward and
cumbersome.


Bug: 1111231, 1132901
Change-Id: I903553a67f14d22d654556c4398d169196763a76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433793
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811451}

content/child/child_thread_impl.cc

@@ -769,14 +769,6 @@ IPC::MessageRouter* ChildThreadImpl::GetRouter() {
   return &router_;
 }
 
-mojom::RouteProvider* ChildThreadImpl::GetRemoteRouteProvider() {
-  if (!remote_route_provider_) {
-    DCHECK(channel_);
-    channel_->GetRemoteAssociatedInterface(&remote_route_provider_);
-  }
-  return remote_route_provider_.get();
-}
-
 bool ChildThreadImpl::OnMessageReceived(const IPC::Message& msg) {
   if (msg.routing_id() == MSG_ROUTING_CONTROL)
     return OnControlMessageReceived(msg);
@@ -787,17 +779,10 @@ bool ChildThreadImpl::OnMessageReceived(const IPC::Message& msg) {
 void ChildThreadImpl::OnAssociatedInterfaceRequest(
     const std::string& interface_name,
     mojo::ScopedInterfaceEndpointHandle handle) {
-  if (interface_name == mojom::RouteProvider::Name_) {
-    DCHECK(!route_provider_receiver_.is_bound());
-    route_provider_receiver_.Bind(
-        mojo::PendingAssociatedReceiver<mojom::RouteProvider>(
-            std::move(handle)),
-        ipc_task_runner_ ? ipc_task_runner_
-                         : base::ThreadTaskRunnerHandle::Get());
-  } else {
-    LOG(ERROR) << "Receiver for unknown Channel-associated interface: "
-               << interface_name;
-  }
+  // All associated interfaces are requested through RenderThreadImpl.
+  LOG(ERROR) << "Receiver for unknown Channel-associated interface: "
+             << interface_name;
+  NOTREACHED();
 }
 
 void ChildThreadImpl::ExposeInterfacesToBrowser(mojo::BinderMap binders) {
@@ -855,27 +840,18 @@ void ChildThreadImpl::EnsureConnected() {
   base::Process::TerminateCurrentProcessImmediately(0);
 }
 
-void ChildThreadImpl::GetRoute(
-    int32_t routing_id,
-    mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterfaceProvider>
-        receiver) {
-  associated_interface_provider_receivers_.Add(this, std::move(receiver),
-                                               routing_id);
+bool ChildThreadImpl::IsInBrowserProcess() const {
+  return static_cast<bool>(browser_process_io_runner_);
 }
 
 void ChildThreadImpl::GetAssociatedInterface(
+    int32_t routing_id,
     const std::string& name,
     mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterface>
         receiver) {
-  int32_t routing_id =
-      associated_interface_provider_receivers_.current_context();
   Listener* route = router_.GetRoute(routing_id);
   if (route)
     route->OnAssociatedInterfaceRequest(name, receiver.PassHandle());
 }
 
-bool ChildThreadImpl::IsInBrowserProcess() const {
-  return static_cast<bool>(browser_process_io_runner_);
-}
-
 }  // namespace content

content/child/child_thread_impl.h

@@ -64,12 +64,9 @@ namespace content {
 class InProcessChildThreadParams;
 
 // The main thread of a child process derives from this class.
-class CONTENT_EXPORT ChildThreadImpl
-    : public IPC::Listener,
-      virtual public ChildThread,
-      private base::FieldTrialList::Observer,
-      public mojom::RouteProvider,
-      public blink::mojom::AssociatedInterfaceProvider {
+class CONTENT_EXPORT ChildThreadImpl : public IPC::Listener,
+                                       virtual public ChildThread,
+                                       private base::FieldTrialList::Observer {
  public:
   struct CONTENT_EXPORT Options;
 
@@ -111,8 +108,6 @@ class CONTENT_EXPORT ChildThreadImpl
 
   IPC::MessageRouter* GetRouter();
 
-  mojom::RouteProvider* GetRemoteRouteProvider();
-
   IPC::SyncMessageFilter* sync_message_filter() const {
     return sync_message_filter_.get();
   }
@@ -166,6 +161,12 @@ class CONTENT_EXPORT ChildThreadImpl
 
   bool IsInBrowserProcess() const;
 
+  void GetAssociatedInterface(
+      int32_t routing_id,
+      const std::string& name,
+      mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterface>
+          receiver);
+
  private:
   // TODO(crbug.com/1111231): This class is a friend so that it can call our
   // private mojo implementation methods, acting as a pass-through. This is only
@@ -195,30 +196,12 @@ class CONTENT_EXPORT ChildThreadImpl
 
   void EnsureConnected();
 
-  // mojom::RouteProvider:
-  void GetRoute(
-      int32_t routing_id,
-      mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterfaceProvider>
-          receiver) override;
-
-  // blink::mojom::AssociatedInterfaceProvider:
-  void GetAssociatedInterface(
-      const std::string& name,
-      mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterface>
-          receiver) override;
-
 #if defined(OS_WIN)
   const mojo::Remote<mojom::FontCacheWin>& GetFontCacheWin();
 #endif
 
   base::Thread mojo_ipc_thread_{"Mojo IPC"};
   std::unique_ptr<mojo::core::ScopedIPCSupport> mojo_ipc_support_;
-
-  mojo::AssociatedReceiver<mojom::RouteProvider> route_provider_receiver_{this};
-  mojo::AssociatedReceiverSet<blink::mojom::AssociatedInterfaceProvider,
-                              int32_t>
-      associated_interface_provider_receivers_;
-  mojo::AssociatedRemote<mojom::RouteProvider> remote_route_provider_;
 #if defined(OS_WIN)
   mutable mojo::Remote<mojom::FontCacheWin> font_cache_win_;
 #endif

content/public/renderer/render_thread.h

@@ -35,13 +35,16 @@ namespace IPC {
 class MessageFilter;
 class SyncChannel;
 class SyncMessageFilter;
-}
+}  // namespace IPC
 
 namespace v8 {
 class Extension;
-}
+}  // namespace v8
 
 namespace content {
+namespace mojom {
+class RouteProvider;
+}  // namespace mojom
 
 class RenderThreadObserver;
 class ResourceDispatcherDelegate;
@@ -76,6 +79,8 @@ class CONTENT_EXPORT RenderThread : virtual public ChildThread {
   virtual void AddObserver(RenderThreadObserver* observer) = 0;
   virtual void RemoveObserver(RenderThreadObserver* observer) = 0;
 
+  virtual mojom::RouteProvider* GetRemoteRouteProvider() = 0;
+
   // Set the ResourceDispatcher delegate object for this process.
   virtual void SetResourceDispatcherDelegate(
       ResourceDispatcherDelegate* delegate) = 0;

content/public/test/mock_render_thread.cc

@@ -11,6 +11,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/unguessable_token.h"
 #include "build/build_config.h"
+#include "content/common/associated_interfaces.mojom.h"
 #include "content/common/frame_messages.h"
 #include "content/common/render_message_filter.mojom.h"
 #include "content/common/view_messages.h"
@@ -57,6 +58,18 @@ class MockRenderMessageFilterImpl : public mojom::RenderMessageFilter {
 #endif
 };
 
+// Some tests require that a valid mojo::RouteProvider* be accessed to send
+// messages over. The RouteProvider does not need to be bound to any real
+// implementation, so we simply bind it to a pipe that we'll forget about, as to
+// drain all messages sent over the remote.
+mojom::RouteProvider* GetStaticRemoteRouteProvider() {
+  static mojo::Remote<mojom::RouteProvider> remote;
+  if (!remote) {
+    ignore_result(remote.BindNewPipeAndPassReceiver());
+  }
+  return remote.get();
+}
+
 }  // namespace
 
 MockRenderThread::MockRenderThread()
@@ -158,6 +171,10 @@ void MockRenderThread::RemoveObserver(RenderThreadObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+mojom::RouteProvider* MockRenderThread::GetRemoteRouteProvider() {
+  return GetStaticRemoteRouteProvider();
+}
+
 void MockRenderThread::SetResourceDispatcherDelegate(
     ResourceDispatcherDelegate* delegate) {
 }

content/public/test/mock_render_thread.h

@@ -40,6 +40,7 @@ namespace mojom {
 class CreateNewWindowParams;
 class CreateNewWindowReply;
 class RenderMessageFilter;
+class RouteProvider;
 }
 
 // This class is a very simple mock of RenderThread. It simulates an IPC channel
@@ -68,6 +69,7 @@ class MockRenderThread : public RenderThread {
   void RemoveFilter(IPC::MessageFilter* filter) override;
   void AddObserver(RenderThreadObserver* observer) override;
   void RemoveObserver(RenderThreadObserver* observer) override;
+  mojom::RouteProvider* GetRemoteRouteProvider() override;
   void SetResourceDispatcherDelegate(
       ResourceDispatcherDelegate* delegate) override;
   void RecordAction(const base::UserMetricsAction& action) override;

content/renderer/render_frame_impl.cc

@@ -2891,23 +2891,14 @@ RenderFrameImpl::GetAssociatedInterfaceRegistry() {
 blink::AssociatedInterfaceProvider*
 RenderFrameImpl::GetRemoteAssociatedInterfaces() {
   if (!remote_associated_interfaces_) {
-    ChildThreadImpl* thread = ChildThreadImpl::current();
-    if (thread) {
-      mojo::PendingAssociatedRemote<blink::mojom::AssociatedInterfaceProvider>
-          remote_interfaces;
-      thread->GetRemoteRouteProvider()->GetRoute(
-          routing_id_, remote_interfaces.InitWithNewEndpointAndPassReceiver());
-      remote_associated_interfaces_ =
-          std::make_unique<blink::AssociatedInterfaceProvider>(
-              std::move(remote_interfaces),
-              GetTaskRunner(blink::TaskType::kInternalNavigationAssociated));
-    } else {
-      // In some tests the thread may be null,
-      // so set up a self-contained interface provider instead.
-      remote_associated_interfaces_ =
-          std::make_unique<blink::AssociatedInterfaceProvider>(
-              GetTaskRunner(blink::TaskType::kInternalNavigationAssociated));
-    }
+    mojo::PendingAssociatedRemote<blink::mojom::AssociatedInterfaceProvider>
+        remote_interfaces;
+    RenderThread::Get()->GetRemoteRouteProvider()->GetRoute(
+        routing_id_, remote_interfaces.InitWithNewEndpointAndPassReceiver());
+    remote_associated_interfaces_ =
+        std::make_unique<blink::AssociatedInterfaceProvider>(
+            std::move(remote_interfaces),
+            GetTaskRunner(blink::TaskType::kInternalNavigationAssociated));
   }
   return remote_associated_interfaces_.get();
 }

content/renderer/render_frame_proxy.cc

@@ -750,21 +750,13 @@ mojom::RenderFrameProxyHost* RenderFrameProxy::GetFrameProxyHost() {
 blink::AssociatedInterfaceProvider*
 RenderFrameProxy::GetRemoteAssociatedInterfaces() {
   if (!remote_associated_interfaces_) {
-    ChildThreadImpl* thread = ChildThreadImpl::current();
-    if (thread) {
-      mojo::PendingAssociatedRemote<blink::mojom::AssociatedInterfaceProvider>
-          remote_interfaces;
-      thread->GetRemoteRouteProvider()->GetRoute(
-          routing_id_, remote_interfaces.InitWithNewEndpointAndPassReceiver());
-      remote_associated_interfaces_ =
-          std::make_unique<blink::AssociatedInterfaceProvider>(
-              std::move(remote_interfaces));
-    } else {
-      // In some tests the thread may be null,
-      // so set up a self-contained interface provider instead.
-      remote_associated_interfaces_ =
-          std::make_unique<blink::AssociatedInterfaceProvider>(nullptr);
-    }
+    mojo::PendingAssociatedRemote<blink::mojom::AssociatedInterfaceProvider>
+        remote_interfaces;
+    RenderThread::Get()->GetRemoteRouteProvider()->GetRoute(
+        routing_id_, remote_interfaces.InitWithNewEndpointAndPassReceiver());
+    remote_associated_interfaces_ =
+        std::make_unique<blink::AssociatedInterfaceProvider>(
+            std::move(remote_interfaces));
   }
   return remote_associated_interfaces_.get();
 }

content/renderer/render_thread_impl.cc

@@ -630,6 +630,8 @@ void RenderThreadImpl::Init() {
       GetContentClient()->renderer()->CreateURLLoaderThrottleProvider(
           URLLoaderThrottleProviderType::kFrame);
 
+  GetAssociatedInterfaceRegistry()->AddInterface(base::BindRepeating(
+      &RenderThreadImpl::OnRouteProviderReceiver, base::Unretained(this)));
   GetAssociatedInterfaceRegistry()->AddInterface(base::BindRepeating(
       &RenderThreadImpl::OnRendererInterfaceReceiver, base::Unretained(this)));
 
@@ -890,6 +892,7 @@ void RenderThreadImpl::RegisterPendingFrameCreate(
 
 mojom::RendererHost* RenderThreadImpl::GetRendererHost() {
   if (!renderer_host_) {
+    DCHECK(GetChannel());
     GetChannel()->GetRemoteAssociatedInterface(&renderer_host_);
   }
   return renderer_host_.get();
@@ -950,6 +953,15 @@ RenderThreadImpl::CreateVideoFrameCompositorTaskRunner() {
   return video_frame_compositor_task_runner_;
 }
 
+mojom::RouteProvider* RenderThreadImpl::GetRemoteRouteProvider() {
+  if (!remote_route_provider_) {
+    DCHECK(GetChannel());
+    GetChannel()->GetRemoteAssociatedInterface(&remote_route_provider_);
+  }
+
+  return remote_route_provider_.get();
+}
+
 void RenderThreadImpl::InitializeWebKit(mojo::BinderMap* binders) {
   DCHECK(!blink_platform_impl_);
 
@@ -2029,6 +2041,26 @@ void RenderThreadImpl::OnMemoryPressure(
   }
 }
 
+void RenderThreadImpl::GetRoute(
+    int32_t routing_id,
+    mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterfaceProvider>
+        receiver) {
+  associated_interface_provider_receivers_.Add(this, std::move(receiver),
+                                               routing_id);
+}
+
+void RenderThreadImpl::GetAssociatedInterface(
+    const std::string& name,
+    mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterface>
+        receiver) {
+  int32_t routing_id =
+      associated_interface_provider_receivers_.current_context();
+  // We delegate to ChildThreadImpl when we actually need to communicate with
+  // IPC::Listeners, since it owns the router.
+  ChildThreadImpl::GetAssociatedInterface(routing_id, name,
+                                          std::move(receiver));
+}
+
 scoped_refptr<base::SingleThreadTaskRunner>
 RenderThreadImpl::GetMediaThreadTaskRunner() {
   DCHECK(main_thread_runner()->BelongsToCurrentThread());
@@ -2218,6 +2250,14 @@ void RenderThreadImpl::OnSyncMemoryPressure(
       v8_memory_pressure_level);
 }
 
+void RenderThreadImpl::OnRouteProviderReceiver(
+    mojo::PendingAssociatedReceiver<mojom::RouteProvider> receiver) {
+  DCHECK(!route_provider_receiver_.is_bound());
+  route_provider_receiver_.Bind(
+      std::move(receiver),
+      GetWebMainThreadScheduler()->DeprecatedDefaultTaskRunner());
+}
+
 void RenderThreadImpl::OnRendererInterfaceReceiver(
     mojo::PendingAssociatedReceiver<mojom::Renderer> receiver) {
   DCHECK(!renderer_receiver_.is_bound());

content/renderer/render_thread_impl.h

@@ -124,6 +124,8 @@ class CONTENT_EXPORT RenderThreadImpl
     : public RenderThread,
       public ChildThreadImpl,
       public mojom::Renderer,
+      public mojom::RouteProvider,
+      public blink::mojom::AssociatedInterfaceProvider,
       public viz::mojom::CompositingModeWatcher,
       public CompositorDependencies {
  public:
@@ -412,6 +414,8 @@ class CONTENT_EXPORT RenderThreadImpl
     video_frame_compositor_task_runner_ = task_runner;
   }
 
+  mojom::RouteProvider* GetRemoteRouteProvider() override;
+
  private:
   friend class RenderThreadImplBrowserTest;
   friend class AgentSchedulingGroup;
@@ -487,6 +491,18 @@ class CONTENT_EXPORT RenderThreadImpl
   void OnMemoryPressure(
       base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level);
 
+  // mojom::RouteProvider implementation:
+  void GetRoute(
+      int32_t routing_id,
+      mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterfaceProvider>
+          receiver) override;
+
+  // blink::mojom::AssociatedInterfaceProvider implementation:
+  void GetAssociatedInterface(
+      const std::string& name,
+      mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterface>
+          receiver) override;
+
   bool RendererIsHidden() const;
   void OnRendererHidden();
   void OnRendererVisible();
@@ -509,6 +525,8 @@ class CONTENT_EXPORT RenderThreadImpl
   std::unique_ptr<viz::SyntheticBeginFrameSource>
   CreateSyntheticBeginFrameSource();
 
+  void OnRouteProviderReceiver(
+      mojo::PendingAssociatedReceiver<mojom::RouteProvider> receiver);
   void OnRendererInterfaceReceiver(
       mojo::PendingAssociatedReceiver<mojom::Renderer> receiver);
 
@@ -610,6 +628,12 @@ class CONTENT_EXPORT RenderThreadImpl
 
   mojo::AssociatedRemote<mojom::RendererHost> renderer_host_;
 
+  mojo::AssociatedReceiver<mojom::RouteProvider> route_provider_receiver_{this};
+  mojo::AssociatedReceiverSet<blink::mojom::AssociatedInterfaceProvider,
+                              int32_t>
+      associated_interface_provider_receivers_;
+  mojo::AssociatedRemote<mojom::RouteProvider> remote_route_provider_;
+
   blink::AssociatedInterfaceRegistry associated_interfaces_;
 
   mojo::AssociatedReceiver<mojom::Renderer> renderer_receiver_{this};