[PM] Adding a RenderProcessHostProxy

Similar to WebContentsProxy this CL adds support for a ProcessNode
to retrieve (on the UI thread) the corresponding RenderProcessHost.

Bug: 993569
Change-Id: I3bf9bec1ff75783ae1fb19cfc5b277652623bf35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752884
Commit-Queue: Brian Geffon <bgeffon@chromium.org>
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687350}

chrome/browser/BUILD.gn

@@ -1152,7 +1152,9 @@ jumbo_split_static_library("browser") {
     "performance_manager/public/graph/process_node.h",
     "performance_manager/public/graph/system_node.h",
     "performance_manager/public/graph/worker_node.h",
+    "performance_manager/public/render_process_host_proxy.h",
     "performance_manager/public/web_contents_proxy.h",
+    "performance_manager/render_process_host_proxy.cc",
     "performance_manager/render_process_user_data.cc",
     "performance_manager/render_process_user_data.h",
     "performance_manager/web_contents_proxy.cc",

chrome/browser/performance_manager/browser_child_process_watcher.cc

@@ -13,6 +13,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/performance_manager/graph/process_node_impl.h"
 #include "chrome/browser/performance_manager/performance_manager.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/child_process_termination_info.h"
 #include "content/public/common/process_type.h"
@@ -21,7 +22,8 @@ namespace performance_manager {
 
 BrowserChildProcessWatcher::BrowserChildProcessWatcher()
     : browser_process_node_(
-          PerformanceManager::GetInstance()->CreateProcessNode()) {
+          PerformanceManager::GetInstance()->CreateProcessNode(
+              RenderProcessHostProxy())) {
   OnProcessLaunched(base::Process::Current(), browser_process_node_.get());
   BrowserChildProcessObserver::Add(this);
 }
@@ -39,7 +41,8 @@ void BrowserChildProcessWatcher::BrowserChildProcessLaunchedAndConnected(
     const content::ChildProcessData& data) {
   if (data.process_type == content::PROCESS_TYPE_GPU) {
     std::unique_ptr<ProcessNodeImpl> gpu_node =
-        PerformanceManager::GetInstance()->CreateProcessNode();
+        PerformanceManager::GetInstance()->CreateProcessNode(
+            RenderProcessHostProxy());
     OnProcessLaunched(data.GetProcess(), gpu_node.get());
     gpu_process_nodes_[data.id] = std::move(gpu_node);
   }

chrome/browser/performance_manager/graph/graph_test_harness.h

@@ -18,6 +18,7 @@
 #include "chrome/browser/performance_manager/graph/process_node_impl.h"
 #include "chrome/browser/performance_manager/graph/system_node_impl.h"
 #include "chrome/browser/performance_manager/graph/worker_node_impl.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace performance_manager {
@@ -86,6 +87,18 @@ struct TestNodeWrapper<FrameNodeImpl>::Factory {
   }
 };
 
+// A specialized factory function for ProcessNodes which will provide an empty
+// RenderProcessHostProxy when it's not needed.
+template <>
+struct TestNodeWrapper<ProcessNodeImpl>::Factory {
+  static std::unique_ptr<ProcessNodeImpl> Create(
+      GraphImpl* graph,
+      RenderProcessHostProxy proxy = RenderProcessHostProxy()) {
+    // Provide an empty RenderProcessHostProxy by default.
+    return std::make_unique<ProcessNodeImpl>(graph, std::move(proxy));
+  }
+};
+
 // A specialized factory function for page nodes that helps fill out some
 // common values.
 template <>

chrome/browser/performance_manager/graph/mock_graphs.cc

@@ -17,7 +17,7 @@
 namespace performance_manager {
 
 TestProcessNodeImpl::TestProcessNodeImpl(GraphImpl* graph)
-    : ProcessNodeImpl(graph) {}
+    : ProcessNodeImpl(graph, RenderProcessHostProxy()) {}
 
 void TestProcessNodeImpl::SetProcessWithPid(base::ProcessId pid,
                                             base::Process process,

chrome/browser/performance_manager/graph/process_node_impl.cc

@@ -13,8 +13,11 @@
 
 namespace performance_manager {
 
-ProcessNodeImpl::ProcessNodeImpl(GraphImpl* graph)
-    : TypedNodeBase(graph), binding_(this) {
+ProcessNodeImpl::ProcessNodeImpl(GraphImpl* graph,
+                                 RenderProcessHostProxy render_process_proxy)
+    : TypedNodeBase(graph),
+      binding_(this),
+      render_process_host_proxy_(std::move(render_process_proxy)) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
@@ -195,6 +198,12 @@ uint64_t ProcessNodeImpl::GetPrivateFootprintKb() const {
   return private_footprint_kb();
 }
 
+const RenderProcessHostProxy& ProcessNodeImpl::GetRenderProcessHostProxy()
+    const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return render_process_host_proxy();
+}
+
 void ProcessNodeImpl::OnAllFramesInProcessFrozen() {
   for (auto& observer : observers())
     observer.OnAllFramesInProcessFrozen(this);

chrome/browser/performance_manager/graph/process_node_impl.h

@@ -16,6 +16,7 @@
 #include "chrome/browser/performance_manager/graph/properties.h"
 #include "chrome/browser/performance_manager/observers/graph_observer.h"
 #include "chrome/browser/performance_manager/public/graph/process_node.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
 
 namespace performance_manager {
 
@@ -43,7 +44,8 @@ class ProcessNodeImpl
  public:
   static constexpr NodeTypeEnum Type() { return NodeTypeEnum::kProcess; }
 
-  explicit ProcessNodeImpl(GraphImpl* graph);
+  ProcessNodeImpl(GraphImpl*, RenderProcessHostProxy render_process_proxy);
+
   ~ProcessNodeImpl() override;
 
   void Bind(
@@ -92,6 +94,10 @@ class ProcessNodeImpl
     return main_thread_task_load_is_low_.value();
   }
 
+  const RenderProcessHostProxy& render_process_host_proxy() const {
+    return render_process_host_proxy_;
+  }
+
   double cpu_usage() const { return cpu_usage_; }
 
   // Add |frame_node| to this process.
@@ -129,6 +135,7 @@ class ProcessNodeImpl
   double GetCpuUsage() const override;
   base::TimeDelta GetCumulativeCpuUsage() const override;
   uint64_t GetPrivateFootprintKb() const override;
+  const RenderProcessHostProxy& GetRenderProcessHostProxy() const override;
 
   void OnAllFramesInProcessFrozen();
 
@@ -144,6 +151,8 @@ class ProcessNodeImpl
   base::Time launch_time_;
   base::Optional<int32_t> exit_status_;
 
+  const RenderProcessHostProxy render_process_host_proxy_;
+
   ObservedProperty::NotifiesAlways<
       base::TimeDelta,
       &GraphImplObserver::OnExpectedTaskQueueingDurationSample,

chrome/browser/performance_manager/performance_manager.cc

@@ -169,9 +169,10 @@ std::unique_ptr<PageNodeImpl> PerformanceManager::CreatePageNode(
                                       contents_proxy, is_visible, is_audible);
 }
 
-std::unique_ptr<ProcessNodeImpl> PerformanceManager::CreateProcessNode() {
+std::unique_ptr<ProcessNodeImpl> PerformanceManager::CreateProcessNode(
+    RenderProcessHostProxy proxy) {
   return CreateNodeImpl<ProcessNodeImpl>(
-      base::OnceCallback<void(ProcessNodeImpl*)>());
+      base::OnceCallback<void(ProcessNodeImpl*)>(), proxy);
 }
 
 std::unique_ptr<WorkerNodeImpl> PerformanceManager::CreateWorkerNode(

chrome/browser/performance_manager/performance_manager.h

@@ -16,6 +16,7 @@
 #include "base/sequenced_task_runner.h"
 #include "chrome/browser/performance_manager/graph/graph_impl.h"
 #include "chrome/browser/performance_manager/public/graph/worker_node.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
 #include "chrome/browser/performance_manager/public/web_contents_proxy.h"
 #include "chrome/browser/performance_manager/webui_graph_dump_impl.h"
 #include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
@@ -104,7 +105,8 @@ class PerformanceManager {
       const WebContentsProxy& contents_proxy,
       bool is_visible,
       bool is_audible);
-  std::unique_ptr<ProcessNodeImpl> CreateProcessNode();
+  std::unique_ptr<ProcessNodeImpl> CreateProcessNode(
+      RenderProcessHostProxy proxy);
   std::unique_ptr<WorkerNodeImpl> CreateWorkerNode(
       WorkerNode::WorkerType worker_type,
       ProcessNodeImpl* process_node,

chrome/browser/performance_manager/performance_manager_unittest.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/performance_manager/graph/frame_node_impl.h"
 #include "chrome/browser/performance_manager/graph/page_node_impl.h"
 #include "chrome/browser/performance_manager/graph/process_node_impl.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace performance_manager {
@@ -54,7 +55,7 @@ class PerformanceManagerTest : public testing::Test {
 
 TEST_F(PerformanceManagerTest, InstantiateNodes) {
   std::unique_ptr<ProcessNodeImpl> process_node =
-      performance_manager()->CreateProcessNode();
+      performance_manager()->CreateProcessNode(RenderProcessHostProxy());
   EXPECT_NE(nullptr, process_node.get());
   std::unique_ptr<PageNodeImpl> page_node =
       performance_manager()->CreatePageNode(WebContentsProxy(), false, false);
@@ -75,7 +76,7 @@ TEST_F(PerformanceManagerTest, InstantiateNodes) {
 TEST_F(PerformanceManagerTest, BatchDeleteNodes) {
   // Create a page node and a small hierarchy of frames.
   std::unique_ptr<ProcessNodeImpl> process_node =
-      performance_manager()->CreateProcessNode();
+      performance_manager()->CreateProcessNode(RenderProcessHostProxy());
   std::unique_ptr<PageNodeImpl> page_node =
       performance_manager()->CreatePageNode(WebContentsProxy(), false, false);
 

chrome/browser/performance_manager/public/graph/process_node.h

@@ -19,6 +19,7 @@ namespace performance_manager {
 
 class FrameNode;
 class ProcessNodeObserver;
+class RenderProcessHostProxy;
 
 // A process node follows the lifetime of a RenderProcessHost.
 // It may reference zero or one processes at a time, but during its lifetime, it
@@ -92,6 +93,10 @@ class ProcessNode : public Node {
   // process, in kilobytes.
   virtual uint64_t GetPrivateFootprintKb() const = 0;
 
+  // Returns a proxy to the RenderProcessHost associated with this node. The
+  // proxy may only be dereferenced on the UI thread.
+  virtual const RenderProcessHostProxy& GetRenderProcessHostProxy() const = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ProcessNode);
 };

chrome/browser/performance_manager/public/render_process_host_proxy.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PERFORMANCE_MANAGER_PUBLIC_RENDER_PROCESS_HOST_PROXY_H_
+#define CHROME_BROWSER_PERFORMANCE_MANAGER_PUBLIC_RENDER_PROCESS_HOST_PROXY_H_
+
+namespace content {
+class RenderProcessHost;
+}  // namespace content
+
+namespace performance_manager {
+
+// A RenderProcessHostProxy is used to post messages out of the performance
+// manager sequence that are bound for a RenderProcessHost running on the UI
+// thread. The object is bound to the UI thread. A RenderProcessHostProxy is
+// conceputally equivalent to a WeakPtr<RenderProcessHost>. Copy and assignment
+// are explicitly allowed for this object.
+class RenderProcessHostProxy {
+ public:
+  RenderProcessHostProxy();
+  RenderProcessHostProxy(const RenderProcessHostProxy& other);
+  ~RenderProcessHostProxy();
+  RenderProcessHostProxy& operator=(const RenderProcessHostProxy& other);
+
+  // Allows resolving this proxy to the underlying RenderProcessHost. This must
+  // only be called on the UI thread. Returns nullptr if the RenderProcessHost
+  // no longer exists.
+  content::RenderProcessHost* Get() const;
+
+ protected:
+  friend class RenderProcessUserData;
+  friend class RenderProcessHostProxyTest;
+
+  explicit RenderProcessHostProxy(int render_process_host_id);
+
+ private:
+  int render_process_host_id_ = -1;
+};
+
+}  // namespace performance_manager
+
+#endif  // CHROME_BROWSER_PERFORMANCE_MANAGER_PUBLIC_RENDER_PROCESS_HOST_PROXY_H_

chrome/browser/performance_manager/render_process_host_proxy.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
+
+#include "content/public/browser/render_process_host.h"
+
+namespace performance_manager {
+
+RenderProcessHostProxy::RenderProcessHostProxy() = default;
+RenderProcessHostProxy::~RenderProcessHostProxy() = default;
+RenderProcessHostProxy::RenderProcessHostProxy(
+    const RenderProcessHostProxy& other) = default;
+RenderProcessHostProxy& RenderProcessHostProxy::operator=(
+    const RenderProcessHostProxy& other) = default;
+
+content::RenderProcessHost* RenderProcessHostProxy::Get() const {
+  return content::RenderProcessHost::FromID(render_process_host_id_);
+}
+
+RenderProcessHostProxy::RenderProcessHostProxy(int render_process_host_id)
+    : render_process_host_id_(render_process_host_id) {
+  DCHECK(render_process_host_id_ >= 0);
+}
+
+}  // namespace performance_manager

chrome/browser/performance_manager/render_process_host_proxy_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
+
+#include "base/memory/weak_ptr.h"
+#include "base/process/process.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/performance_manager/graph/process_node_impl.h"
+#include "chrome/browser/performance_manager/performance_manager_test_harness.h"
+#include "chrome/browser/performance_manager/public/graph/process_node.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
+#include "chrome/browser/performance_manager/render_process_user_data.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/mock_render_process_host.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace performance_manager {
+
+class RenderProcessHostProxyTest
+    : public performance_manager::PerformanceManagerTestHarness {
+ protected:
+  RenderProcessHostProxyTest() {}
+  ~RenderProcessHostProxyTest() override {}
+
+  void SetUp() override { PerformanceManagerTestHarness::SetUp(); }
+  void TearDown() override { PerformanceManagerTestHarness::TearDown(); }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RenderProcessHostProxyTest);
+};
+
+TEST_F(RenderProcessHostProxyTest, RPHDeletionInvalidatesProxy) {
+  //  content::RenderProcessHost* host(
+  //      rph_factory_->CreateRenderProcessHost(profile_, nullptr));
+  std::unique_ptr<TestingProfileManager> profile_manager(
+      new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
+  ASSERT_TRUE(profile_manager->SetUp());
+
+  // Owned by profile_manager.
+  TestingProfile* profile(
+      profile_manager->CreateTestingProfile("RPHTestProfile"));
+
+  std::unique_ptr<content::MockRenderProcessHostFactory> rph_factory(
+      new content::MockRenderProcessHostFactory());
+  scoped_refptr<content::SiteInstance> site_instance(
+      content::SiteInstance::Create(profile));
+
+  // Owned by rph_factory.
+  content::RenderProcessHost* host(
+      rph_factory->CreateRenderProcessHost(profile, site_instance.get()));
+
+  // Now create a RenderProcessUserData which creates a ProcessNode.
+  auto* render_process_user_data =
+      RenderProcessUserData::GetOrCreateForRenderProcessHost(host);
+  ASSERT_NE(render_process_user_data, nullptr);
+  ProcessNode* process_node = render_process_user_data->process_node();
+  ASSERT_NE(process_node, nullptr);
+
+  content::RenderProcessHost* proxy_contents = nullptr;
+
+  auto deref_proxy = base::BindLambdaForTesting(
+      [&proxy_contents](const RenderProcessHostProxy& proxy,
+                        base::OnceClosure quit_loop) {
+        proxy_contents = proxy.Get();
+        std::move(quit_loop).Run();
+      });
+
+  // Bounce over to the PM sequence, retrieve the proxy, bounce back to the UI
+  // thread, dereference it if possible, and save the returned contents. To be
+  // fair, it's entirely valid to grab the weak pointer directly on the UI
+  // thread, as the lifetime of the process node is managed there and the
+  // property being accessed is thread safe. However, this test aims to simulate
+  // what would happen with a policy message being posted from the graph.
+  {
+    base::RunLoop run_loop;
+    PerformanceManager::GetInstance()->CallOnGraph(
+        FROM_HERE,
+        base::BindLambdaForTesting(
+            [&deref_proxy, process_node,
+             quit_loop = run_loop.QuitClosure()](GraphImpl* graph) {
+              base::PostTask(
+                  FROM_HERE, {content::BrowserThread::UI},
+                  base::BindOnce(deref_proxy,
+                                 process_node->GetRenderProcessHostProxy(),
+                                 std::move(quit_loop)));
+            }));
+    run_loop.Run();
+
+    // We should see the RPH via the proxy.
+    EXPECT_EQ(host, proxy_contents);
+  }
+
+  // Run the same test but make sure the RPH is gone first.
+  {
+    base::RunLoop run_loop;
+    PerformanceManager::GetInstance()->CallOnGraph(
+        FROM_HERE,
+        base::BindLambdaForTesting([&rph_factory, &deref_proxy, process_node,
+                                    host, quit_loop = run_loop.QuitClosure()](
+                                       GraphImpl* graph) {
+          base::PostTask(
+              FROM_HERE, {content::BrowserThread::UI},
+              base::BindLambdaForTesting([&rph_factory, host]() {
+                rph_factory->Remove(
+                    reinterpret_cast<content::MockRenderProcessHost*>(host));
+                delete host;
+              }));
+          base::PostTask(
+              FROM_HERE, {content::BrowserThread::UI},
+              base::BindOnce(deref_proxy,
+                             process_node->GetRenderProcessHostProxy(),
+                             std::move(quit_loop)));
+        }));
+    run_loop.Run();
+
+    // The contents was destroyed on the UI thread prior to dereferencing the
+    // proxy, so it should return nullptr.
+    EXPECT_EQ(proxy_contents, nullptr);
+  }
+}
+
+}  // namespace performance_manager

chrome/browser/performance_manager/render_process_user_data.cc

@@ -14,6 +14,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/performance_manager/graph/process_node_impl.h"
 #include "chrome/browser/performance_manager/performance_manager.h"
+#include "chrome/browser/performance_manager/public/render_process_host_proxy.h"
 #include "content/public/browser/child_process_termination_info.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_switches.h"
@@ -30,9 +31,10 @@ RenderProcessUserData* RenderProcessUserData::first_ = nullptr;
 
 RenderProcessUserData::RenderProcessUserData(
     content::RenderProcessHost* render_process_host)
-    : host_(render_process_host),
-      process_node_(PerformanceManager::GetInstance()->CreateProcessNode()) {
+    : host_(render_process_host) {
   host_->AddObserver(this);
+  process_node_ = PerformanceManager::GetInstance()->CreateProcessNode(
+      RenderProcessHostProxy(host_->GetID()));
 
   // Push this instance to the list.
   next_ = first_;

chrome/test/BUILD.gn

@@ -3032,6 +3032,7 @@ test("unit_tests") {
     "../browser/performance_manager/persistence/site_data/site_data_writer_unittest.cc",
     "../browser/performance_manager/persistence/site_data/unittest_utils.cc",
     "../browser/performance_manager/persistence/site_data/unittest_utils.h",
+    "../browser/performance_manager/render_process_host_proxy_unittest.cc",
     "../browser/performance_manager/web_contents_proxy_unittest.cc",
     "../browser/performance_manager/webui_graph_dump_impl_unittest.cc",
     "../browser/performance_monitor/metric_evaluator_helper_win_unittest.cc",