[MBI] Move the various Create* methods from Renderer to AgentSchedulingGroup.

The AgentSchedulingGroup interface is (currently) associated with the
Renderer interface, so message ordering is preserved. For now, the ASG
simply calls the appropriate method implementations on RenderThreadImpl.

Bug: 1111231
Change-Id: I1dc1ffced95f6210894550d1864a93ef92b4b943
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409721
Commit-Queue: Tal Pressman <talp@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811689}

content/browser/renderer_host/agent_scheduling_group_host.cc

@@ -127,6 +127,11 @@ bool AgentSchedulingGroupHost::MaybeAssociatedRemote::is_bound() {
   return absl::visit([](auto& remote) { return remote.is_bound(); }, remote_);
 }
 
+mojom::AgentSchedulingGroup*
+AgentSchedulingGroupHost::MaybeAssociatedRemote::get() {
+  return absl::visit([](auto& r) { return r.get(); }, remote_);
+}
+
 // AgentSchedulingGroupHost:
 
 // static
@@ -236,16 +241,30 @@ mojom::RouteProvider* AgentSchedulingGroupHost::GetRemoteRouteProvider() {
 
 void AgentSchedulingGroupHost::CreateFrame(mojom::CreateFrameParamsPtr params) {
   SetUpMojoIfNeeded();
-  process_.GetRendererInterface()->CreateFrame(std::move(params));
+  mojo_remote_.get()->CreateFrame(std::move(params));
 }
 
 void AgentSchedulingGroupHost::CreateView(mojom::CreateViewParamsPtr params) {
   SetUpMojoIfNeeded();
-  process_.GetRendererInterface()->CreateView(std::move(params));
+  mojo_remote_.get()->CreateView(std::move(params));
 }
 
 void AgentSchedulingGroupHost::DestroyView(int32_t routing_id) {
-  process_.GetRendererInterface()->DestroyView(routing_id);
+  if (mojo_remote_.is_bound())
+    mojo_remote_.get()->DestroyView(routing_id);
+}
+
+void AgentSchedulingGroupHost::CreateFrameProxy(
+    int32_t routing_id,
+    int32_t render_view_routing_id,
+    const base::Optional<base::UnguessableToken>& opener_frame_token,
+    int32_t parent_routing_id,
+    const FrameReplicationState& replicated_state,
+    const base::UnguessableToken& frame_token,
+    const base::UnguessableToken& devtools_frame_token) {
+  mojo_remote_.get()->CreateFrameProxy(
+      routing_id, render_view_routing_id, opener_frame_token, parent_routing_id,
+      replicated_state, frame_token, devtools_frame_token);
 }
 
 void AgentSchedulingGroupHost::GetRoute(

content/browser/renderer_host/agent_scheduling_group_host.h

@@ -81,6 +81,14 @@ class CONTENT_EXPORT AgentSchedulingGroupHost
   void CreateFrame(mojom::CreateFrameParamsPtr params);
   void CreateView(mojom::CreateViewParamsPtr params);
   void DestroyView(int32_t routing_id);
+  void CreateFrameProxy(
+      int32_t routing_id,
+      int32_t render_view_routing_id,
+      const base::Optional<base::UnguessableToken>& opener_frame_token,
+      int32_t parent_routing_id,
+      const FrameReplicationState& replicated_state,
+      const base::UnguessableToken& frame_token,
+      const base::UnguessableToken& devtools_frame_token);
 
  private:
   // `MaybeAssociatedReceiver` and `MaybeAssociatedRemote` are temporary helper
@@ -136,6 +144,7 @@ class CONTENT_EXPORT AgentSchedulingGroupHost
 
     void reset();
     bool is_bound();
+    mojom::AgentSchedulingGroup* get();
 
    private:
     absl::variant<mojo::Remote<mojom::AgentSchedulingGroup>,

content/browser/renderer_host/render_frame_proxy_host.cc

@@ -253,10 +253,12 @@ bool RenderFrameProxyHost::InitRenderFrameProxy() {
   }
 
   int view_routing_id = GetRenderViewHost()->GetRoutingID();
-  GetProcess()->GetRendererInterface()->CreateFrameProxy(
-      routing_id_, view_routing_id, opener_frame_token, parent_routing_id,
-      frame_tree_node_->current_replication_state(), frame_token_,
-      frame_tree_node_->devtools_frame_token());
+  static_cast<SiteInstanceImpl*>(site_instance_.get())
+      ->GetAgentSchedulingGroup()
+      .CreateFrameProxy(routing_id_, view_routing_id, opener_frame_token,
+                        parent_routing_id,
+                        frame_tree_node_->current_replication_state(),
+                        frame_token_, frame_tree_node_->devtools_frame_token());
 
   SetRenderFrameProxyCreated(true);
 

content/browser/site_per_process_browsertest.cc

@@ -52,6 +52,7 @@
 #include "components/viz/common/features.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/gpu/compositor_util.h"
+#include "content/browser/renderer_host/agent_scheduling_group_host.h"
 #include "content/browser/renderer_host/cross_process_frame_connector.h"
 #include "content/browser/renderer_host/frame_navigation_entry.h"
 #include "content/browser/renderer_host/frame_tree.h"
@@ -6193,6 +6194,8 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
   SiteInstance* site_instance_a = root->current_frame_host()->GetSiteInstance();
   RenderProcessHost* process_a =
       root->render_manager()->current_frame_host()->GetProcess();
+  AgentSchedulingGroupHost* agent_scheduling_group_a =
+      AgentSchedulingGroupHost::Get(*site_instance_a, *process_a);
   int new_routing_id = process_a->GetNextRoutingID();
   int view_routing_id =
       root->frame_tree()->GetRenderViewHost(site_instance_a)->GetRoutingID();
@@ -6207,7 +6210,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
   // Send the message to create a proxy for B's new child frame in A.  This
   // used to crash, as parent_routing_id refers to a proxy that doesn't exist
   // anymore.
-  process_a->GetRendererInterface()->CreateFrameProxy(
+  agent_scheduling_group_a->CreateFrameProxy(
       new_routing_id, view_routing_id, base::nullopt, parent_routing_id,
       FrameReplicationState(), base::UnguessableToken::Create(),
       base::UnguessableToken::Create());
@@ -6252,6 +6255,10 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
   // attempted.
   RenderProcessHost* process =
       node->render_manager()->speculative_frame_host()->GetProcess();
+  AgentSchedulingGroupHost* agent_scheduling_group =
+      AgentSchedulingGroupHost::Get(
+          *node->render_manager()->speculative_frame_host()->GetSiteInstance(),
+          *process);
   RenderProcessHostWatcher watcher(
       process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
   int frame_routing_id =
@@ -6282,7 +6289,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
     params->frame_owner_properties = blink::mojom::FrameOwnerProperties::New();
     params->frame_token = frame_token;
     params->devtools_frame_token = base::UnguessableToken::Create();
-    process->GetRendererInterface()->CreateFrame(std::move(params));
+    agent_scheduling_group->CreateFrame(std::move(params));
   }
 
   // Disable the BackForwardCache to ensure the old process is going to be
@@ -6322,6 +6329,9 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, ParentDetachRemoteChild) {
   // observer to ensure there is no crash when a new RenderFrame creation is
   // attempted.
   RenderProcessHost* process = node->current_frame_host()->GetProcess();
+  AgentSchedulingGroupHost* agent_scheduling_group =
+      AgentSchedulingGroupHost::Get(
+          *node->current_frame_host()->GetSiteInstance(), *process);
   RenderProcessHostWatcher watcher(
       process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
   int frame_routing_id = node->current_frame_host()->GetRoutingID();
@@ -6372,7 +6382,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, ParentDetachRemoteChild) {
     params->replication_state.unique_name = "name";
     params->frame_token = frame_token;
     params->devtools_frame_token = base::UnguessableToken::Create();
-    process->GetRendererInterface()->CreateFrame(std::move(params));
+    agent_scheduling_group->CreateFrame(std::move(params));
   }
 
   // The test must wait for the process to exit, but if there is no leak, the

content/renderer/agent_scheduling_group.cc

@@ -5,8 +5,8 @@
 #include "content/renderer/agent_scheduling_group.h"
 
 #include "base/feature_list.h"
+#include "base/util/type_safety/pass_key.h"
 #include "content/public/common/content_features.h"
-#include "content/public/renderer/render_thread.h"
 #include "content/renderer/render_thread_impl.h"
 
 namespace content {
@@ -21,6 +21,8 @@ using ::mojo::PendingRemote;
 using ::mojo::Receiver;
 using ::mojo::Remote;
 
+using PassKey = ::util::PassKey<AgentSchedulingGroup>;
+
 namespace {
 RenderThreadImpl& ToImpl(RenderThread& render_thread) {
   DCHECK(RenderThreadImpl::current());
@@ -137,6 +139,32 @@ void AgentSchedulingGroup::RemoveRoute(int32_t routing_id) {
   RenderThread::Get()->RemoveRoute(routing_id);
 }
 
+void AgentSchedulingGroup::CreateView(mojom::CreateViewParamsPtr params) {
+  ToImpl(render_thread_).CreateView(std::move(params), PassKey());
+}
+
+void AgentSchedulingGroup::DestroyView(int32_t view_id) {
+  ToImpl(render_thread_).DestroyView(view_id, PassKey());
+}
+
+void AgentSchedulingGroup::CreateFrame(mojom::CreateFrameParamsPtr params) {
+  ToImpl(render_thread_).CreateFrame(std::move(params), PassKey());
+}
+
+void AgentSchedulingGroup::CreateFrameProxy(
+    int32_t routing_id,
+    int32_t render_view_routing_id,
+    const base::Optional<base::UnguessableToken>& opener_frame_token,
+    int32_t parent_routing_id,
+    const FrameReplicationState& replicated_state,
+    const base::UnguessableToken& frame_token,
+    const base::UnguessableToken& devtools_frame_token) {
+  ToImpl(render_thread_)
+      .CreateFrameProxy(routing_id, render_view_routing_id, opener_frame_token,
+                        parent_routing_id, replicated_state, frame_token,
+                        devtools_frame_token, PassKey());
+}
+
 void AgentSchedulingGroup::GetRoute(
     int32_t routing_id,
     mojo::PendingAssociatedReceiver<blink::mojom::AssociatedInterfaceProvider>

content/renderer/agent_scheduling_group.h

@@ -104,6 +104,19 @@ class CONTENT_EXPORT AgentSchedulingGroup
         remote_;
   };
 
+  // mojom::AgentSchedulingGroup:
+  void CreateView(mojom::CreateViewParamsPtr params) override;
+  void DestroyView(int32_t view_id) override;
+  void CreateFrame(mojom::CreateFrameParamsPtr params) override;
+  void CreateFrameProxy(
+      int32_t routing_id,
+      int32_t render_view_routing_id,
+      const base::Optional<base::UnguessableToken>& opener_frame_token,
+      int32_t parent_routing_id,
+      const FrameReplicationState& replicated_state,
+      const base::UnguessableToken& frame_token,
+      const base::UnguessableToken& devtools_frame_token) override;
+
   // mojom::RouteProvider
   void GetRoute(
       int32_t routing_id,

content/renderer/render_thread_impl.cc

@@ -158,8 +158,8 @@
 #endif
 
 #if defined(OS_WIN)
-#include <windows.h>
 #include <objbase.h>
+#include <windows.h>
 #endif
 
 #ifdef ENABLE_VTUNE_JIT_INTERFACE
@@ -181,20 +181,21 @@
 #include "base/test/clang_profiling.h"
 #endif
 
-using base::ThreadRestrictions;
-using blink::WebDocument;
-using blink::WebFrame;
-using blink::WebNetworkStateNotifier;
-using blink::WebRuntimeFeatures;
-using blink::WebScriptController;
-using blink::WebSecurityPolicy;
-using blink::WebString;
-using blink::WebView;
-
 namespace content {
 
 namespace {
 
+using ::base::ThreadRestrictions;
+using ::blink::WebDocument;
+using ::blink::WebFrame;
+using ::blink::WebNetworkStateNotifier;
+using ::blink::WebRuntimeFeatures;
+using ::blink::WebScriptController;
+using ::blink::WebSecurityPolicy;
+using ::blink::WebString;
+using ::blink::WebView;
+using ::util::PassKey;
+
 #if defined(OS_ANDROID)
 // Unique identifier for each output surface created.
 uint32_t g_next_layer_tree_frame_sink_id = 1;
@@ -218,14 +219,19 @@ base::LazyInstance<scoped_refptr<base::SingleThreadTaskRunner>>::
 
 // v8::MemoryPressureLevel should correspond to base::MemoryPressureListener.
 static_assert(static_cast<v8::MemoryPressureLevel>(
-    base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE) ==
-        v8::MemoryPressureLevel::kNone, "none level not align");
-static_assert(static_cast<v8::MemoryPressureLevel>(
-    base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE) ==
-        v8::MemoryPressureLevel::kModerate, "moderate level not align");
-static_assert(static_cast<v8::MemoryPressureLevel>(
-    base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) ==
-        v8::MemoryPressureLevel::kCritical, "critical level not align");
+                  base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE) ==
+                  v8::MemoryPressureLevel::kNone,
+              "none level not align");
+static_assert(
+    static_cast<v8::MemoryPressureLevel>(
+        base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE) ==
+        v8::MemoryPressureLevel::kModerate,
+    "moderate level not align");
+static_assert(
+    static_cast<v8::MemoryPressureLevel>(
+        base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) ==
+        v8::MemoryPressureLevel::kCritical,
+    "critical level not align");
 
 // WebMemoryPressureLevel should correspond to base::MemoryPressureListener.
 static_assert(static_cast<blink::WebMemoryPressureLevel>(
@@ -243,21 +249,20 @@ static_assert(
         blink::kWebMemoryPressureLevelCritical,
     "blink::WebMemoryPressureLevelCritical not align");
 
-void* CreateHistogram(
-    const char *name, int min, int max, size_t buckets) {
+void* CreateHistogram(const char* name, int min, int max, size_t buckets) {
   if (min <= 0)
     min = 1;
   std::string histogram_name;
   RenderThreadImpl* render_thread_impl = RenderThreadImpl::current();
   if (render_thread_impl) {  // Can be null in tests.
-    histogram_name = render_thread_impl->
-        histogram_customizer()->ConvertToCustomHistogramName(name);
+    histogram_name = render_thread_impl->histogram_customizer()
+                         ->ConvertToCustomHistogramName(name);
   } else {
     histogram_name = std::string(name);
   }
-  base::HistogramBase* histogram = base::Histogram::FactoryGet(
-      histogram_name, min, max, buckets,
-      base::Histogram::kUmaTargetedHistogramFlag);
+  base::HistogramBase* histogram =
+      base::Histogram::FactoryGet(histogram_name, min, max, buckets,
+                                  base::Histogram::kUmaTargetedHistogramFlag);
   return histogram;
 }
 
@@ -390,7 +395,8 @@ RenderThreadImpl::HistogramCustomizer::HistogramCustomizer() {
 RenderThreadImpl::HistogramCustomizer::~HistogramCustomizer() {}
 
 void RenderThreadImpl::HistogramCustomizer::RenderViewNavigatedToHost(
-    const std::string& host, size_t view_count) {
+    const std::string& host,
+    size_t view_count) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableHistogramCustomizer)) {
     return;
@@ -1380,7 +1386,7 @@ void RenderThreadImpl::OnChannelError() {
   // more informative stack, since we will otherwise just crash later when we
   // try to restart it.
   CHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSingleProcess));
+      switches::kSingleProcess));
   ChildThreadImpl::OnChannelError();
 }
 
@@ -1821,7 +1827,8 @@ gpu::GpuChannelHost* RenderThreadImpl::GetGpuChannel() {
   return gpu_->GetGpuChannel().get();
 }
 
-void RenderThreadImpl::CreateView(mojom::CreateViewParamsPtr params) {
+void RenderThreadImpl::CreateView(mojom::CreateViewParamsPtr params,
+                                  PassKey<AgentSchedulingGroup>) {
   CompositorDependencies* compositor_deps = this;
   is_scroll_animator_enabled_ = params->web_preferences.enable_scroll_animator;
   // TODO(crbug.com/1111231): For as long as views are created via the
@@ -1836,7 +1843,8 @@ void RenderThreadImpl::CreateView(mojom::CreateViewParamsPtr params) {
                          GetWebMainThreadScheduler()->DefaultTaskRunner());
 }
 
-void RenderThreadImpl::DestroyView(int32_t view_id) {
+void RenderThreadImpl::DestroyView(int32_t view_id,
+                                   PassKey<AgentSchedulingGroup>) {
   RenderViewImpl* view = RenderViewImpl::FromRoutingID(view_id);
   DCHECK(view);
 
@@ -1850,7 +1858,8 @@ void RenderThreadImpl::DestroyView(int32_t view_id) {
       base::BindOnce(&RenderViewImpl::Destroy, base::Unretained(view)));
 }
 
-void RenderThreadImpl::CreateFrame(mojom::CreateFrameParamsPtr params) {
+void RenderThreadImpl::CreateFrame(mojom::CreateFrameParamsPtr params,
+                                   PassKey<AgentSchedulingGroup>) {
   CompositorDependencies* compositor_deps = this;
   mojo::PendingRemote<service_manager::mojom::InterfaceProvider>
       interface_provider(
@@ -1912,7 +1921,8 @@ void RenderThreadImpl::CreateFrameProxy(
     int32_t parent_routing_id,
     const FrameReplicationState& replicated_state,
     const base::UnguessableToken& frame_token,
-    const base::UnguessableToken& devtools_frame_token) {
+    const base::UnguessableToken& devtools_frame_token,
+    PassKey<AgentSchedulingGroup>) {
   // TODO(crbug.com/1111231): For as long as frame proxies are created via the
   // `Renderer` interface (as opposed to `AgentSchedulingGroup`), we will always
   // have *exactly one* `AgentSchedulingGroup` in the process.
@@ -2330,8 +2340,7 @@ void RenderThreadImpl::UnfreezableMessageFilter::
 
 // Called on the listener thread.
 void RenderThreadImpl::UnfreezableMessageFilter::
-    RemoveListenerUnfreezableTaskRunner(
-        int32_t routing_id) {
+    RemoveListenerUnfreezableTaskRunner(int32_t routing_id) {
   base::AutoLock lock(unfreezable_task_runners_lock_);
   unfreezable_task_runners_.erase(routing_id);
 }

content/renderer/render_thread_impl.h

@@ -27,6 +27,7 @@
 #include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
+#include "base/util/type_safety/pass_key.h"
 #include "build/build_config.h"
 #include "cc/mojom/render_frame_metadata.mojom.h"
 #include "content/child/child_thread_impl.h"
@@ -214,6 +215,22 @@ class CONTENT_EXPORT RenderThreadImpl
   // viz::mojom::CompositingModeWatcher implementation.
   void CompositingModeFallbackToSoftware() override;
 
+  // Formerly in mojom::Renderer (moved to mojom::AgentSchedulingGroup):
+  void CreateView(mojom::CreateViewParamsPtr params,
+                  util::PassKey<AgentSchedulingGroup>);
+  void DestroyView(int32_t view_id, util::PassKey<AgentSchedulingGroup>);
+  void CreateFrame(mojom::CreateFrameParamsPtr params,
+                   util::PassKey<AgentSchedulingGroup>);
+  void CreateFrameProxy(
+      int32_t routing_id,
+      int32_t render_view_routing_id,
+      const base::Optional<base::UnguessableToken>& opener_frame_token,
+      int32_t parent_routing_id,
+      const FrameReplicationState& replicated_state,
+      const base::UnguessableToken& frame_token,
+      const base::UnguessableToken& devtools_frame_token,
+      util::PassKey<AgentSchedulingGroup>);
+
   // Whether gpu compositing is being used or is disabled for software
   // compositing. Clients of the compositor should give resources that match
   // the appropriate mode.
@@ -439,9 +456,6 @@ class CONTENT_EXPORT RenderThreadImpl
   void OnGetAccessibilityTree();
 
   // mojom::Renderer:
-  void CreateView(mojom::CreateViewParamsPtr params) override;
-  void DestroyView(int32_t view_id) override;
-  void CreateFrame(mojom::CreateFrameParamsPtr params) override;
   void CreateAgentSchedulingGroup(
       mojo::PendingRemote<mojom::AgentSchedulingGroupHost>
           agent_scheduling_group_host,
@@ -452,14 +466,6 @@ class CONTENT_EXPORT RenderThreadImpl
           agent_scheduling_group_host,
       mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup>
           agent_scheduling_group) override;
-  void CreateFrameProxy(
-      int32_t routing_id,
-      int32_t render_view_routing_id,
-      const base::Optional<base::UnguessableToken>& opener_frame_token,
-      int32_t parent_routing_id,
-      const FrameReplicationState& replicated_state,
-      const base::UnguessableToken& frame_token,
-      const base::UnguessableToken& devtools_frame_token) override;
   void OnNetworkConnectionChanged(
       net::NetworkChangeNotifier::ConnectionType type,
       double max_bandwidth_mbps) override;