PM: Change graph node lifetime and stop using mojo for local IPC.
This change retires the {Frame|Page|Process|System}ResourceCoordinator
classes. Instead owners of graph nodes maintain unique_ptrs to them,
and communicate via PostTask to the graph. There is now strict lifetime
ordering between the PerformanceManager and the graph nodes, all of
which have to be destroyed before the PerformanceManager itself is
destroyed.
A followup CL will retire the mojo interfaces and methods that have
been replaced.
Bug: 910288
Change-Id: I45fa86f648b79dae688574b8022a213b16ec0d30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1512301
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#640938}diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 65ea7792..724abf08 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -1072,16 +1072,12 @@
"performance_manager/common/page_almost_idle_data.h",
"performance_manager/decorators/page_almost_idle_decorator.cc",
"performance_manager/decorators/page_almost_idle_decorator.h",
- "performance_manager/frame_resource_coordinator.cc",
- "performance_manager/frame_resource_coordinator.h",
"performance_manager/graph/frame_node_impl.cc",
"performance_manager/graph/frame_node_impl.h",
"performance_manager/graph/graph.cc",
"performance_manager/graph/graph.h",
"performance_manager/graph/graph_introspector_impl.cc",
"performance_manager/graph/graph_introspector_impl.h",
- "performance_manager/graph/graph_node_provider_impl.cc",
- "performance_manager/graph/graph_node_provider_impl.h",
"performance_manager/graph/node_attached_data.cc",
"performance_manager/graph/node_attached_data.h",
"performance_manager/graph/node_attached_data_impl.h",
@@ -1102,21 +1098,14 @@
"performance_manager/observers/page_signal_generator_impl.h",
"performance_manager/observers/working_set_trimmer_win.cc",
"performance_manager/observers/working_set_trimmer_win.h",
- "performance_manager/page_resource_coordinator.cc",
- "performance_manager/page_resource_coordinator.h",
"performance_manager/performance_manager.cc",
"performance_manager/performance_manager.h",
"performance_manager/performance_manager_tab_helper.cc",
"performance_manager/performance_manager_tab_helper.h",
- "performance_manager/process_resource_coordinator.cc",
- "performance_manager/process_resource_coordinator.h",
"performance_manager/render_process_user_data.cc",
"performance_manager/render_process_user_data.h",
"performance_manager/resource_coordinator_clock.cc",
"performance_manager/resource_coordinator_clock.h",
- "performance_manager/resource_coordinator_interface.h",
- "performance_manager/system_resource_coordinator.cc",
- "performance_manager/system_resource_coordinator.h",
"performance_manager/webui_graph_dump_impl.cc",
"performance_manager/webui_graph_dump_impl.h",
"performance_monitor/metric_evaluator_helper_win.cc",
diff --git a/chrome/browser/performance_manager/browser_child_process_watcher.cc b/chrome/browser/performance_manager/browser_child_process_watcher.cc
index c0a35bf..cb95e85 100644
--- a/chrome/browser/performance_manager/browser_child_process_watcher.cc
+++ b/chrome/browser/performance_manager/browser_child_process_watcher.cc
@@ -7,10 +7,12 @@
#include <memory>
#include <utility>
+#include "base/bind.h"
#include "base/process/process.h"
#include "base/stl_util.h"
+#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/process_resource_coordinator.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"
@@ -18,22 +20,27 @@
namespace performance_manager {
BrowserChildProcessWatcher::BrowserChildProcessWatcher()
- : browser_node_(PerformanceManager::GetInstance()) {
- browser_node_.OnProcessLaunched(base::Process::Current());
+ : browser_process_node_(
+ PerformanceManager::GetInstance()->CreateProcessNode()) {
+ OnProcessLaunched(base::Process::Current(), browser_process_node_.get());
BrowserChildProcessObserver::Add(this);
}
BrowserChildProcessWatcher::~BrowserChildProcessWatcher() {
BrowserChildProcessObserver::Remove(this);
+
+ PerformanceManager* performance_manager = PerformanceManager::GetInstance();
+ performance_manager->DeleteNode(std::move(browser_process_node_));
+ for (auto& node : gpu_process_nodes_)
+ performance_manager->DeleteNode(std::move(node.second));
}
void BrowserChildProcessWatcher::BrowserChildProcessLaunchedAndConnected(
const content::ChildProcessData& data) {
if (data.process_type == content::PROCESS_TYPE_GPU) {
- std::unique_ptr<performance_manager::ProcessResourceCoordinator> gpu_node =
- std::make_unique<ProcessResourceCoordinator>(
- PerformanceManager::GetInstance());
- gpu_node->OnProcessLaunched(data.GetProcess());
+ std::unique_ptr<ProcessNodeImpl> gpu_node =
+ PerformanceManager::GetInstance()->CreateProcessNode();
+ OnProcessLaunched(data.GetProcess(), gpu_node.get());
gpu_process_nodes_[data.id] = std::move(gpu_node);
}
}
@@ -41,8 +48,10 @@
void BrowserChildProcessWatcher::BrowserChildProcessHostDisconnected(
const content::ChildProcessData& data) {
if (data.process_type == content::PROCESS_TYPE_GPU) {
- size_t removed = gpu_process_nodes_.erase(data.id);
- DCHECK_EQ(1u, removed);
+ auto it = gpu_process_nodes_.find(data.id);
+ DCHECK(it != gpu_process_nodes_.end());
+ PerformanceManager::GetInstance()->DeleteNode(std::move(it->second));
+ gpu_process_nodes_.erase(it);
}
}
@@ -66,8 +75,37 @@
// specifically on crash.
if (base::ContainsKey(gpu_process_nodes_, id)) {
auto* process_node = gpu_process_nodes_[id].get();
- process_node->SetProcessExitStatus(exit_code);
+ PerformanceManager* performance_manager = PerformanceManager::GetInstance();
+
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&ProcessNodeImpl::SetProcessExitStatus,
+ base::Unretained(process_node), exit_code));
}
}
+// static
+void BrowserChildProcessWatcher::OnProcessLaunched(
+ const base::Process& process,
+ ProcessNodeImpl* process_node) {
+ // TODO(siggi): Change this to pass the process into the graph node.
+ const base::ProcessId pid = process.Pid();
+ const base::Time launch_time =
+#if defined(OS_ANDROID)
+ // Process::CreationTime() is not available on Android. Since this method
+ // is called immediately after the process is launched, the process launch
+ // time can be approximated with the current time.
+ base::Time::Now();
+#else
+ process.CreationTime();
+#endif
+
+ PerformanceManager* performance_manager = PerformanceManager::GetInstance();
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&ProcessNodeImpl::SetPID,
+ base::Unretained(process_node), pid));
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&ProcessNodeImpl::SetLaunchTime,
+ base::Unretained(process_node), launch_time));
+}
+
} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/browser_child_process_watcher.h b/chrome/browser/performance_manager/browser_child_process_watcher.h
index cdb6a6d..d264315 100644
--- a/chrome/browser/performance_manager/browser_child_process_watcher.h
+++ b/chrome/browser/performance_manager/browser_child_process_watcher.h
@@ -10,12 +10,14 @@
#include "base/compiler_specific.h"
#include "base/containers/flat_map.h"
#include "base/macros.h"
-#include "chrome/browser/performance_manager/process_resource_coordinator.h"
+#include "base/process/process.h"
#include "content/public/browser/browser_child_process_observer.h"
#include "content/public/common/service_manager_connection.h"
namespace performance_manager {
+class ProcessNodeImpl;
+
// Responsible for maintaining the process nodes for the browser and the GPU
// process.
class BrowserChildProcessWatcher : public content::BrowserChildProcessObserver {
@@ -38,15 +40,15 @@
void GPUProcessExited(int id, int exit_code);
- performance_manager::ProcessResourceCoordinator browser_node_;
+ static void OnProcessLaunched(const base::Process& process,
+ ProcessNodeImpl* process_node);
+
+ std::unique_ptr<ProcessNodeImpl> browser_process_node_;
// Apparently more than one GPU process can be existent at a time, though
// secondaries are very transient. This map keeps track of all GPU processes
// by their unique ID from |content::ChildProcessData|.
- base::flat_map<
- int,
- std::unique_ptr<performance_manager::ProcessResourceCoordinator>>
- gpu_process_nodes_;
+ base::flat_map<int, std::unique_ptr<ProcessNodeImpl>> gpu_process_nodes_;
DISALLOW_COPY_AND_ASSIGN(BrowserChildProcessWatcher);
};
diff --git a/chrome/browser/performance_manager/chrome_content_browser_client_performance_manager_part.cc b/chrome/browser/performance_manager/chrome_content_browser_client_performance_manager_part.cc
index 67d78edc..a452293 100644
--- a/chrome/browser/performance_manager/chrome_content_browser_client_performance_manager_part.cc
+++ b/chrome/browser/performance_manager/chrome_content_browser_client_performance_manager_part.cc
@@ -7,19 +7,31 @@
#include <utility>
#include "base/bind.h"
+#include "base/logging.h"
#include "base/threading/sequenced_task_runner_handle.h"
+#include "chrome/browser/performance_manager/graph/process_node_impl.h"
+#include "chrome/browser/performance_manager/performance_manager.h"
#include "chrome/browser/performance_manager/render_process_user_data.h"
+#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
namespace {
-void BindProcessPerformanceManager(
+void BindProcessNode(
content::RenderProcessHost* render_process_host,
resource_coordinator::mojom::ProcessCoordinationUnitRequest request) {
performance_manager::RenderProcessUserData* user_data =
performance_manager::RenderProcessUserData::GetForRenderProcessHost(
render_process_host);
- user_data->process_resource_coordinator()->AddBinding(std::move(request));
+ performance_manager::PerformanceManager* performance_manager =
+ performance_manager::PerformanceManager::GetInstance();
+ DCHECK(performance_manager);
+
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&performance_manager::ProcessNodeImpl::AddBinding,
+ base::Unretained(user_data->process_node()),
+ std::move(request)));
}
} // namespace
@@ -35,7 +47,7 @@
blink::AssociatedInterfaceRegistry* associated_registry,
content::RenderProcessHost* render_process_host) {
registry->AddInterface(
- base::BindRepeating(&BindProcessPerformanceManager,
+ base::BindRepeating(&BindProcessNode,
base::Unretained(render_process_host)),
base::SequencedTaskRunnerHandle::Get());
diff --git a/chrome/browser/performance_manager/frame_resource_coordinator.cc b/chrome/browser/performance_manager/frame_resource_coordinator.cc
deleted file mode 100644
index a9d6caea..0000000
--- a/chrome/browser/performance_manager/frame_resource_coordinator.cc
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright 2017 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/frame_resource_coordinator.h"
-
-#include "base/bind.h"
-#include "chrome/browser/performance_manager/process_resource_coordinator.h"
-
-namespace performance_manager {
-
-FrameResourceCoordinator::FrameResourceCoordinator(
- PerformanceManager* performance_manager)
- : ResourceCoordinatorInterface(), weak_ptr_factory_(this) {
- resource_coordinator::CoordinationUnitID new_cu_id(
- resource_coordinator::CoordinationUnitType::kFrame,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- ResourceCoordinatorInterface::ConnectToService(performance_manager,
- new_cu_id);
-}
-
-FrameResourceCoordinator::~FrameResourceCoordinator() = default;
-
-void FrameResourceCoordinator::SetProcess(
- const ProcessResourceCoordinator& process) {
- if (!service_ || !process.service())
- return;
- process.service()->GetID(
- base::BindOnce(&FrameResourceCoordinator::SetProcessByID,
- weak_ptr_factory_.GetWeakPtr()));
-}
-
-void FrameResourceCoordinator::AddChildFrame(
- const FrameResourceCoordinator& child) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (!service_ || !child.service())
- return;
- // We could keep the ID around ourselves, but this hop ensures that the child
- // has been created on the service-side.
- child.service()->GetID(
- base::BindOnce(&FrameResourceCoordinator::AddChildFrameByID,
- weak_ptr_factory_.GetWeakPtr()));
-}
-
-void FrameResourceCoordinator::RemoveChildFrame(
- const FrameResourceCoordinator& child) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (!service_ || !child.service())
- return;
- child.service()->GetID(
- base::BindOnce(&FrameResourceCoordinator::RemoveChildFrameByID,
- weak_ptr_factory_.GetWeakPtr()));
-}
-
-void FrameResourceCoordinator::ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) {
- provider->CreateFrameCoordinationUnit(mojo::MakeRequest(&service_), cu_id);
-}
-
-void FrameResourceCoordinator::SetProcessByID(
- const resource_coordinator::CoordinationUnitID& process_id) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (service_)
- service_->SetProcess(process_id);
-}
-
-void FrameResourceCoordinator::AddChildFrameByID(
- const resource_coordinator::CoordinationUnitID& child_id) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (service_)
- service_->AddChildFrame(child_id);
-}
-
-void FrameResourceCoordinator::RemoveChildFrameByID(
- const resource_coordinator::CoordinationUnitID& child_id) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (service_)
- service_->RemoveChildFrame(child_id);
-}
-
-} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/frame_resource_coordinator.h b/chrome/browser/performance_manager/frame_resource_coordinator.h
deleted file mode 100644
index 3176f7d..0000000
--- a/chrome/browser/performance_manager/frame_resource_coordinator.h
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright 2017 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_FRAME_RESOURCE_COORDINATOR_H_
-#define CHROME_BROWSER_PERFORMANCE_MANAGER_FRAME_RESOURCE_COORDINATOR_H_
-
-#include "base/memory/weak_ptr.h"
-#include "base/threading/thread_checker.h"
-#include "chrome/browser/performance_manager/resource_coordinator_interface.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
-
-namespace performance_manager {
-
-class ProcessResourceCoordinator;
-
-class FrameResourceCoordinator
- : public ResourceCoordinatorInterface<
- resource_coordinator::mojom::FrameCoordinationUnitPtr,
- resource_coordinator::mojom::FrameCoordinationUnitRequest> {
- public:
- explicit FrameResourceCoordinator(PerformanceManager* performance_manager);
- ~FrameResourceCoordinator() override;
-
- void SetProcess(const ProcessResourceCoordinator& process);
- void AddChildFrame(const FrameResourceCoordinator& child);
- void RemoveChildFrame(const FrameResourceCoordinator& child);
-
- // Closes the connection to the service.
- void reset() { service_.reset(); }
-
- private:
- void ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) override;
-
- void SetProcessByID(
- const resource_coordinator::CoordinationUnitID& process_id);
- void AddChildFrameByID(
- const resource_coordinator::CoordinationUnitID& child_id);
- void RemoveChildFrameByID(
- const resource_coordinator::CoordinationUnitID& child_id);
-
- THREAD_CHECKER(thread_checker_);
-
- // The WeakPtrFactory should come last so the weak ptrs are invalidated
- // before the rest of the member variables.
- base::WeakPtrFactory<FrameResourceCoordinator> weak_ptr_factory_;
-
- DISALLOW_COPY_AND_ASSIGN(FrameResourceCoordinator);
-};
-
-} // namespace performance_manager
-
-#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_FRAME_RESOURCE_COORDINATOR_H_
diff --git a/chrome/browser/performance_manager/graph/README.md b/chrome/browser/performance_manager/graph/README.md
new file mode 100644
index 0000000..774c9fc
--- /dev/null
+++ b/chrome/browser/performance_manager/graph/README.md
@@ -0,0 +1,17 @@
+# Threading Model
+
+The [graph](graph.h) can only be accessed from a single sequence.
+
+Nodes can be created on any sequence, but as soon as they're added to a graph,
+they can only be used on the graph's sequence, with the exception of the id
+accessor.
+
+# Node Lifetime
+
+With the exception of the system node, which is a singleton, the Graph does not
+own nodes. The user of the graph is responsible for maintaining node ownership.
+Any node that has been added to the graph, must be removed from the graph
+with Graph::DestroyNode (TODO(siggi): rename to RemoveNode) before it's
+deleted.
+All nodes must be removed from the graph before the graph is destroyed.
+
diff --git a/chrome/browser/performance_manager/graph/frame_node_impl.cc b/chrome/browser/performance_manager/graph/frame_node_impl.cc
index 86343ebd..c71308d 100644
--- a/chrome/browser/performance_manager/graph/frame_node_impl.cc
+++ b/chrome/browser/performance_manager/graph/frame_node_impl.cc
@@ -20,13 +20,16 @@
for (size_t i = 0; i < base::size(intervention_policy_); ++i)
intervention_policy_[i] =
resource_coordinator::mojom::InterventionPolicy::kUnknown;
+
+ DETACH_FROM_SEQUENCE(sequence_checker_);
}
FrameNodeImpl::~FrameNodeImpl() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (parent_frame_coordination_unit_)
parent_frame_coordination_unit_->RemoveChildFrame(this);
if (page_coordination_unit_)
- page_coordination_unit_->RemoveFrame(this);
+ page_coordination_unit_->RemoveFrameImpl(this);
if (process_coordination_unit_)
process_coordination_unit_->RemoveFrame(this);
for (auto* child_frame : child_frame_coordination_units_)
@@ -56,7 +59,7 @@
DCHECK(false) << "Cyclic reference in frame coordination units detected!";
return;
}
- if (AddChildFrame(frame_cu)) {
+ if (AddChildFrameImpl(frame_cu)) {
frame_cu->AddParentFrame(this);
}
}
@@ -229,7 +232,7 @@
parent_frame_coordination_unit_ = parent_frame_cu;
}
-bool FrameNodeImpl::AddChildFrame(FrameNodeImpl* child_frame_cu) {
+bool FrameNodeImpl::AddChildFrameImpl(FrameNodeImpl* child_frame_cu) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return child_frame_coordination_units_.count(child_frame_cu)
? false
diff --git a/chrome/browser/performance_manager/graph/frame_node_impl.h b/chrome/browser/performance_manager/graph/frame_node_impl.h
index 8fd04bd..2fb4a432 100644
--- a/chrome/browser/performance_manager/graph/frame_node_impl.h
+++ b/chrome/browser/performance_manager/graph/frame_node_impl.h
@@ -90,7 +90,7 @@
// PageNodeImpl and ProcessNodeImpl respectively to
// manipulate their relationship.
void AddParentFrame(FrameNodeImpl* parent_frame_cu);
- bool AddChildFrame(FrameNodeImpl* child_frame_cu);
+ bool AddChildFrameImpl(FrameNodeImpl* child_frame_cu);
void RemoveParentFrame(FrameNodeImpl* parent_frame_cu);
bool RemoveChildFrame(FrameNodeImpl* child_frame_cu);
void AddPageNode(PageNodeImpl* page_cu);
diff --git a/chrome/browser/performance_manager/graph/graph.cc b/chrome/browser/performance_manager/graph/graph.cc
index 3af6b38b..cbc90ad 100644
--- a/chrome/browser/performance_manager/graph/graph.cc
+++ b/chrome/browser/performance_manager/graph/graph.cc
@@ -10,15 +10,12 @@
#include "base/bind_helpers.h"
#include "base/macros.h"
#include "chrome/browser/performance_manager/graph/frame_node_impl.h"
-#include "chrome/browser/performance_manager/graph/graph_node_provider_impl.h"
#include "chrome/browser/performance_manager/graph/node_base.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/graph/system_node_impl.h"
#include "chrome/browser/performance_manager/observers/coordination_unit_graph_observer.h"
#include "services/resource_coordinator/public/cpp/coordination_unit_types.h"
-#include "services/service_manager/public/cpp/bind_source_info.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
namespace ukm {
class UkmEntryBuilder;
@@ -26,10 +23,7 @@
namespace performance_manager {
-Graph::Graph()
- : system_coordination_unit_id_(
- resource_coordinator::CoordinationUnitType::kSystem,
- resource_coordinator::CoordinationUnitID::RANDOM_ID) {}
+Graph::Graph() = default;
Graph::~Graph() {
// Because the graph has ownership of the CUs, and because the process CUs
@@ -47,14 +41,6 @@
DCHECK_EQ(0u, processes_by_pid_.size());
}
-void Graph::OnStart(service_manager::BinderRegistryWithArgs<
- const service_manager::BindSourceInfo&>* registry) {
- // Create the singleton CoordinationUnitProvider.
- provider_ = std::make_unique<GraphNodeProviderImpl>(this);
- registry->AddInterface(base::BindRepeating(
- &GraphNodeProviderImpl::Bind, base::Unretained(provider_.get())));
-}
-
void Graph::RegisterObserver(std::unique_ptr<GraphObserver> observer) {
observer->set_coordination_unit_graph(this);
observers_.push_back(std::move(observer));
@@ -73,28 +59,17 @@
coordination_unit->BeforeDestroyed();
}
-FrameNodeImpl* Graph::CreateFrameNode(
- const resource_coordinator::CoordinationUnitID& id) {
- return FrameNodeImpl::Create(id, this);
-}
-
-PageNodeImpl* Graph::CreatePageNode(
- const resource_coordinator::CoordinationUnitID& id) {
- return PageNodeImpl::Create(id, this);
-}
-
-ProcessNodeImpl* Graph::CreateProcessNode(
- const resource_coordinator::CoordinationUnitID& id) {
- return ProcessNodeImpl::Create(id, this);
-}
-
SystemNodeImpl* Graph::FindOrCreateSystemNode() {
- NodeBase* system_cu = GetNodeByID(system_coordination_unit_id_);
- if (system_cu)
- return SystemNodeImpl::FromNodeBase(system_cu);
+ if (!system_node_) {
+ // Create the singleton SystemCU instance. Ownership is taken by the graph.
+ resource_coordinator::CoordinationUnitID id(
+ resource_coordinator::CoordinationUnitType::kSystem,
+ resource_coordinator::CoordinationUnitID::RANDOM_ID);
+ system_node_ = std::make_unique<SystemNodeImpl>(id, this);
+ AddNewNode(system_node_.get());
+ }
- // Create the singleton SystemCU instance. Ownership is taken by the graph.
- return SystemNodeImpl::Create(system_coordination_unit_id_, this);
+ return system_node_.get();
}
NodeBase* Graph::GetNodeByID(
@@ -102,7 +77,7 @@
const auto& it = coordination_units_.find(cu_id);
if (it == coordination_units_.end())
return nullptr;
- return it->second.get();
+ return it->second;
}
ProcessNodeImpl* Graph::GetProcessNodeByPid(base::ProcessId pid) {
@@ -142,14 +117,12 @@
return count;
}
-NodeBase* Graph::AddNewNode(std::unique_ptr<NodeBase> new_cu) {
- auto it = coordination_units_.emplace(new_cu->id(), std::move(new_cu));
+void Graph::AddNewNode(NodeBase* new_cu) {
+ auto it = coordination_units_.emplace(new_cu->id(), new_cu);
DCHECK(it.second); // Inserted successfully
- NodeBase* added_cu = it.first->second.get();
+ NodeBase* added_cu = it.first->second;
OnNodeCreated(added_cu);
-
- return added_cu;
}
void Graph::DestroyNode(NodeBase* cu) {
@@ -188,7 +161,7 @@
std::vector<CUType*> ret;
for (const auto& el : coordination_units_) {
if (el.first.type == type)
- ret.push_back(CUType::FromNodeBase(el.second.get()));
+ ret.push_back(CUType::FromNodeBase(el.second));
}
return ret;
}
diff --git a/chrome/browser/performance_manager/graph/graph.h b/chrome/browser/performance_manager/graph/graph.h
index 4cacb7d..18f315ba 100644
--- a/chrome/browser/performance_manager/graph/graph.h
+++ b/chrome/browser/performance_manager/graph/graph.h
@@ -21,17 +21,10 @@
#include "services/resource_coordinator/public/cpp/coordination_unit_id.h"
#include "services/resource_coordinator/public/cpp/coordination_unit_types.h"
-namespace service_manager {
-template <typename... BinderArgs>
-class BinderRegistryWithArgs;
-struct BindSourceInfo;
-} // namespace service_manager
-
namespace performance_manager {
class NodeBase;
class GraphObserver;
-class GraphNodeProviderImpl;
class FrameNodeImpl;
class PageNodeImpl;
class ProcessNodeImpl;
@@ -51,20 +44,11 @@
}
ukm::UkmRecorder* ukm_recorder() const { return ukm_recorder_; }
- void OnStart(service_manager::BinderRegistryWithArgs<
- const service_manager::BindSourceInfo&>* registry);
void RegisterObserver(std::unique_ptr<GraphObserver> observer);
void OnNodeCreated(NodeBase* coordination_unit);
void OnBeforeNodeDestroyed(NodeBase* coordination_unit);
- FrameNodeImpl* CreateFrameNode(
- const resource_coordinator::CoordinationUnitID& id);
- PageNodeImpl* CreatePageNode(
- const resource_coordinator::CoordinationUnitID& id);
- ProcessNodeImpl* CreateProcessNode(
- const resource_coordinator::CoordinationUnitID& id);
SystemNodeImpl* FindOrCreateSystemNode();
-
std::vector<ProcessNodeImpl*> GetAllProcessNodes();
std::vector<FrameNodeImpl*> GetAllFrameNodes();
std::vector<PageNodeImpl*> GetAllPageNodes();
@@ -77,6 +61,10 @@
return observers_;
}
+ // Lifetime management functions for node owners.
+ void AddNewNode(NodeBase* new_cu);
+ void DestroyNode(NodeBase* cu);
+
// A |key| of nullptr counts all instances associated with the |node|. A
// |node| of null counts all instances associated with the |key|. If both are
// null then the entire map size is provided.
@@ -84,15 +72,10 @@
const void* key) const;
private:
- using CUIDMap = std::unordered_map<resource_coordinator::CoordinationUnitID,
- std::unique_ptr<NodeBase>>;
+ using CUIDMap =
+ std::unordered_map<resource_coordinator::CoordinationUnitID, NodeBase*>;
using ProcessByPidMap = std::unordered_map<base::ProcessId, ProcessNodeImpl*>;
- // Lifetime management functions for NodeBase.
- friend class NodeBase;
- NodeBase* AddNewNode(std::unique_ptr<NodeBase> new_cu);
- void DestroyNode(NodeBase* cu);
-
// Process PID map for use by ProcessNodeImpl.
friend class ProcessNodeImpl;
void BeforeProcessPidChange(ProcessNodeImpl* process,
@@ -101,12 +84,11 @@
template <typename CUType>
std::vector<CUType*> GetAllNodesOfType();
- resource_coordinator::CoordinationUnitID system_coordination_unit_id_;
+ std::unique_ptr<SystemNodeImpl> system_node_;
CUIDMap coordination_units_;
ProcessByPidMap processes_by_pid_;
std::vector<std::unique_ptr<GraphObserver>> observers_;
ukm::UkmRecorder* ukm_recorder_ = nullptr;
- std::unique_ptr<GraphNodeProviderImpl> provider_;
// User data storage for the graph.
friend class NodeAttachedData;
diff --git a/chrome/browser/performance_manager/graph/graph_node_provider_impl.cc b/chrome/browser/performance_manager/graph/graph_node_provider_impl.cc
deleted file mode 100644
index b408024..0000000
--- a/chrome/browser/performance_manager/graph/graph_node_provider_impl.cc
+++ /dev/null
@@ -1,81 +0,0 @@
-// Copyright 2017 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/graph/graph_node_provider_impl.h"
-
-#include <memory>
-#include <utility>
-
-#include "base/bind.h"
-#include "base/macros.h"
-#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/graph/system_node_impl.h"
-#include "services/service_manager/public/cpp/bind_source_info.h"
-
-namespace performance_manager {
-
-GraphNodeProviderImpl::GraphNodeProviderImpl(Graph* coordination_unit_graph)
- : coordination_unit_graph_(coordination_unit_graph) {}
-
-GraphNodeProviderImpl::~GraphNodeProviderImpl() = default;
-
-void GraphNodeProviderImpl::OnConnectionError(NodeBase* coordination_unit) {
- coordination_unit->Destruct();
-}
-
-void GraphNodeProviderImpl::CreateFrameCoordinationUnit(
- resource_coordinator::mojom::FrameCoordinationUnitRequest request,
- const resource_coordinator::CoordinationUnitID& id) {
- FrameNodeImpl* frame_cu = coordination_unit_graph_->CreateFrameNode(id);
-
- frame_cu->Bind(std::move(request));
- auto& frame_cu_binding = frame_cu->binding();
-
- frame_cu_binding.set_connection_error_handler(
- base::BindOnce(&GraphNodeProviderImpl::OnConnectionError,
- base::Unretained(this), frame_cu));
-}
-
-void GraphNodeProviderImpl::CreatePageCoordinationUnit(
- resource_coordinator::mojom::PageCoordinationUnitRequest request,
- const resource_coordinator::CoordinationUnitID& id) {
- PageNodeImpl* page_cu = coordination_unit_graph_->CreatePageNode(id);
-
- page_cu->Bind(std::move(request));
- auto& page_cu_binding = page_cu->binding();
-
- page_cu_binding.set_connection_error_handler(
- base::BindOnce(&GraphNodeProviderImpl::OnConnectionError,
- base::Unretained(this), page_cu));
-}
-
-void GraphNodeProviderImpl::CreateProcessCoordinationUnit(
- resource_coordinator::mojom::ProcessCoordinationUnitRequest request,
- const resource_coordinator::CoordinationUnitID& id) {
- ProcessNodeImpl* process_cu = coordination_unit_graph_->CreateProcessNode(id);
-
- process_cu->Bind(std::move(request));
- auto& process_cu_binding = process_cu->binding();
-
- process_cu_binding.set_connection_error_handler(
- base::BindOnce(&GraphNodeProviderImpl::OnConnectionError,
- base::Unretained(this), process_cu));
-}
-
-void GraphNodeProviderImpl::GetSystemCoordinationUnit(
- resource_coordinator::mojom::SystemCoordinationUnitRequest request) {
- // Simply fetch the existing SystemCU and add an additional binding to it.
- coordination_unit_graph_->FindOrCreateSystemNode()->AddBinding(
- std::move(request));
-}
-
-void GraphNodeProviderImpl::Bind(
- resource_coordinator::mojom::CoordinationUnitProviderRequest request,
- const service_manager::BindSourceInfo& source_info) {
- bindings_.AddBinding(this, std::move(request));
-}
-
-} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/graph/graph_node_provider_impl.h b/chrome/browser/performance_manager/graph/graph_node_provider_impl.h
deleted file mode 100644
index 19739ea..0000000
--- a/chrome/browser/performance_manager/graph/graph_node_provider_impl.h
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright 2017 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_GRAPH_GRAPH_NODE_PROVIDER_IMPL_H_
-#define CHROME_BROWSER_PERFORMANCE_MANAGER_GRAPH_GRAPH_NODE_PROVIDER_IMPL_H_
-
-#include <memory>
-#include <vector>
-
-#include "chrome/browser/performance_manager/graph/graph.h"
-#include "mojo/public/cpp/bindings/binding_set.h"
-#include "mojo/public/cpp/bindings/interface_request.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit_provider.mojom.h"
-
-namespace service_manager {
-struct BindSourceInfo;
-} // namespace service_manager
-
-namespace performance_manager {
-
-class GraphNodeProviderImpl
- : public resource_coordinator::mojom::CoordinationUnitProvider {
- public:
- explicit GraphNodeProviderImpl(Graph* coordination_unit_graph);
- ~GraphNodeProviderImpl() override;
-
- void Bind(
- resource_coordinator::mojom::CoordinationUnitProviderRequest request,
- const service_manager::BindSourceInfo& source_info);
-
- void OnConnectionError(NodeBase* coordination_unit);
-
- // Overridden from resource_coordinator::mojom::CoordinationUnitProvider:
- void CreateFrameCoordinationUnit(
- resource_coordinator::mojom::FrameCoordinationUnitRequest request,
- const resource_coordinator::CoordinationUnitID& id) override;
- void CreatePageCoordinationUnit(
- resource_coordinator::mojom::PageCoordinationUnitRequest request,
- const resource_coordinator::CoordinationUnitID& id) override;
- void CreateProcessCoordinationUnit(
- resource_coordinator::mojom::ProcessCoordinationUnitRequest request,
- const resource_coordinator::CoordinationUnitID& id) override;
- void GetSystemCoordinationUnit(
- resource_coordinator::mojom::SystemCoordinationUnitRequest request)
- override;
-
- private:
- Graph* coordination_unit_graph_;
- mojo::BindingSet<resource_coordinator::mojom::CoordinationUnitProvider>
- bindings_;
-
- DISALLOW_COPY_AND_ASSIGN(GraphNodeProviderImpl);
-};
-
-} // namespace performance_manager
-
-#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_GRAPH_GRAPH_NODE_PROVIDER_IMPL_H_
diff --git a/chrome/browser/performance_manager/graph/graph_test_harness.cc b/chrome/browser/performance_manager/graph/graph_test_harness.cc
index 830260c3..3161af5 100644
--- a/chrome/browser/performance_manager/graph/graph_test_harness.cc
+++ b/chrome/browser/performance_manager/graph/graph_test_harness.cc
@@ -11,8 +11,7 @@
GraphTestHarness::GraphTestHarness()
: task_env_(base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
- base::test::ScopedTaskEnvironment::ExecutionMode::QUEUED),
- provider_(&coordination_unit_graph_) {}
+ base::test::ScopedTaskEnvironment::ExecutionMode::QUEUED) {}
GraphTestHarness::~GraphTestHarness() = default;
diff --git a/chrome/browser/performance_manager/graph/graph_test_harness.h b/chrome/browser/performance_manager/graph/graph_test_harness.h
index 24cd621..a1b11e8 100644
--- a/chrome/browser/performance_manager/graph/graph_test_harness.h
+++ b/chrome/browser/performance_manager/graph/graph_test_harness.h
@@ -6,12 +6,16 @@
#define CHROME_BROWSER_PERFORMANCE_MANAGER_GRAPH_GRAPH_TEST_HARNESS_H_
#include <stdint.h>
+#include <memory>
#include <string>
+#include <utility>
#include "base/test/scoped_task_environment.h"
+#include "chrome/browser/performance_manager/graph/frame_node_impl.h"
#include "chrome/browser/performance_manager/graph/graph.h"
-#include "chrome/browser/performance_manager/graph/graph_node_provider_impl.h"
#include "chrome/browser/performance_manager/graph/node_base.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/graph/system_node_impl.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -26,30 +30,65 @@
template <class NodeClass>
class TestNodeWrapper {
public:
+ static TestNodeWrapper<NodeClass> Create(
+ const resource_coordinator::CoordinationUnitID& cu_id,
+ Graph* graph) {
+ std::unique_ptr<NodeClass> node = std::make_unique<NodeClass>(cu_id, graph);
+ graph->AddNewNode(node.get());
+ return TestNodeWrapper<NodeClass>(std::move(node));
+ }
static TestNodeWrapper<NodeClass> Create(Graph* graph) {
resource_coordinator::CoordinationUnitID cu_id(
NodeClass::Type(), resource_coordinator::CoordinationUnitID::RANDOM_ID);
- return TestNodeWrapper<NodeClass>(NodeClass::Create(cu_id, graph));
+ return Create(cu_id, graph);
}
- explicit TestNodeWrapper(NodeClass* impl) : impl_(impl) { DCHECK(impl); }
+ explicit TestNodeWrapper(std::unique_ptr<NodeClass> impl)
+ : impl_(std::move(impl)) {
+ DCHECK(impl_.get());
+ }
~TestNodeWrapper() { reset(); }
- NodeClass* operator->() const { return impl_; }
+ NodeClass* operator->() const { return impl_.get(); }
- TestNodeWrapper(TestNodeWrapper&& other) : impl_(other.impl_) {}
+ TestNodeWrapper(TestNodeWrapper&& other) : impl_(std::move(other.impl_)) {}
- NodeClass* get() const { return impl_; }
+ NodeClass* get() const { return impl_.get(); }
void reset() {
if (impl_) {
- impl_->Destruct();
- impl_ = nullptr;
+ impl_->graph()->DestroyNode(impl_.get());
+ impl_.reset();
}
}
private:
- NodeClass* impl_;
+ std::unique_ptr<NodeClass> impl_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestNodeWrapper);
+};
+
+// This specialization is necessary because the graph has ownership of the
+// system node as it's a singleton. For the other node types the test wrapper
+// manages the node lifetime.
+template <>
+class TestNodeWrapper<SystemNodeImpl> {
+ public:
+ static TestNodeWrapper<SystemNodeImpl> Create(Graph* graph) {
+ return TestNodeWrapper<SystemNodeImpl>(graph->FindOrCreateSystemNode());
+ }
+
+ explicit TestNodeWrapper(SystemNodeImpl* impl) : impl_(impl) {}
+ TestNodeWrapper(TestNodeWrapper&& other) : impl_(other.impl_) {}
+ ~TestNodeWrapper() { reset(); }
+
+ SystemNodeImpl* operator->() const { return impl_; }
+ SystemNodeImpl* get() const { return impl_; }
+
+ void reset() { impl_ = nullptr; }
+
+ private:
+ SystemNodeImpl* impl_;
DISALLOW_COPY_AND_ASSIGN(TestNodeWrapper);
};
@@ -62,8 +101,7 @@
template <class NodeClass>
TestNodeWrapper<NodeClass> CreateCoordinationUnit(
resource_coordinator::CoordinationUnitID cu_id) {
- return TestNodeWrapper<NodeClass>(
- NodeClass::Create(cu_id, coordination_unit_graph()));
+ return TestNodeWrapper<NodeClass>::Create(cu_id, coordination_unit_graph());
}
template <class NodeClass>
@@ -84,12 +122,10 @@
protected:
base::test::ScopedTaskEnvironment& task_env() { return task_env_; }
Graph* coordination_unit_graph() { return &coordination_unit_graph_; }
- GraphNodeProviderImpl* provider() { return &provider_; }
private:
base::test::ScopedTaskEnvironment task_env_;
Graph coordination_unit_graph_;
- GraphNodeProviderImpl provider_;
};
} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/graph/graph_unittest.cc b/chrome/browser/performance_manager/graph/graph_unittest.cc
index 4d40133..0eee4bf 100644
--- a/chrome/browser/performance_manager/graph/graph_unittest.cc
+++ b/chrome/browser/performance_manager/graph/graph_unittest.cc
@@ -5,37 +5,13 @@
#include "chrome/browser/performance_manager/graph/graph.h"
#include "chrome/browser/performance_manager/graph/frame_node_impl.h"
+#include "chrome/browser/performance_manager/graph/graph_test_harness.h"
#include "chrome/browser/performance_manager/graph/mock_graphs.h"
#include "chrome/browser/performance_manager/graph/process_node_impl.h"
#include "chrome/browser/performance_manager/graph/system_node_impl.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace performance_manager {
-namespace {
-
-ProcessNodeImpl* CreateProcessNode(Graph* graph) {
- return graph->CreateProcessNode(resource_coordinator::CoordinationUnitID(
- resource_coordinator::CoordinationUnitType::kProcess,
- resource_coordinator::CoordinationUnitID::RANDOM_ID));
-}
-
-} // namespace
-
-TEST(GraphTest, DestructionWhileCUSOutstanding) {
- std::unique_ptr<Graph> graph(new Graph());
-
- for (size_t i = 0; i < 10; ++i) {
- ProcessNodeImpl* process = CreateProcessNode(graph.get());
- EXPECT_NE(nullptr, process);
-
- process->SetPID(i + 100);
- }
-
- EXPECT_NE(nullptr, graph->FindOrCreateSystemNode());
-
- // This should destroy all the CUs without incident.
- graph.reset();
-}
TEST(GraphTest, FindOrCreateSystemNode) {
Graph graph;
@@ -44,27 +20,22 @@
// A second request should return the same instance.
EXPECT_EQ(system_cu, graph.FindOrCreateSystemNode());
-
- // Destructing the system CU should be allowed.
- system_cu->Destruct();
-
- system_cu = graph.FindOrCreateSystemNode();
- EXPECT_NE(nullptr, system_cu);
}
TEST(GraphTest, GetProcessNodeByPid) {
Graph graph;
- ProcessNodeImpl* process = CreateProcessNode(&graph);
+ TestNodeWrapper<ProcessNodeImpl> process =
+ TestNodeWrapper<ProcessNodeImpl>::Create(&graph);
EXPECT_EQ(base::kNullProcessId, process->process_id());
static constexpr base::ProcessId kPid = 10;
EXPECT_EQ(nullptr, graph.GetProcessNodeByPid(kPid));
process->SetPID(kPid);
- EXPECT_EQ(process, graph.GetProcessNodeByPid(kPid));
+ EXPECT_EQ(process.get(), graph.GetProcessNodeByPid(kPid));
- process->Destruct();
+ process.reset();
EXPECT_EQ(nullptr, graph.GetProcessNodeByPid(12));
}
@@ -77,20 +48,22 @@
static constexpr base::ProcessId kPid = 10;
- ProcessNodeImpl* process1 = CreateProcessNode(&graph);
- ProcessNodeImpl* process2 = CreateProcessNode(&graph);
+ TestNodeWrapper<ProcessNodeImpl> process1 =
+ TestNodeWrapper<ProcessNodeImpl>::Create(&graph);
+ TestNodeWrapper<ProcessNodeImpl> process2 =
+ TestNodeWrapper<ProcessNodeImpl>::Create(&graph);
process1->SetPID(kPid);
- EXPECT_EQ(process1, graph.GetProcessNodeByPid(kPid));
+ EXPECT_EQ(process1.get(), graph.GetProcessNodeByPid(kPid));
// The second registration for the same PID should override the first one.
process2->SetPID(kPid);
- EXPECT_EQ(process2, graph.GetProcessNodeByPid(kPid));
+ EXPECT_EQ(process2.get(), graph.GetProcessNodeByPid(kPid));
// The destruction of the first process CU shouldn't clear the PID
// registration.
- process1->Destruct();
- EXPECT_EQ(process2, graph.GetProcessNodeByPid(kPid));
+ process1.reset();
+ EXPECT_EQ(process2.get(), graph.GetProcessNodeByPid(kPid));
}
TEST(GraphTest, GetAllCUsByType) {
diff --git a/chrome/browser/performance_manager/graph/node_base.cc b/chrome/browser/performance_manager/graph/node_base.cc
index a6ad917..f8a69662f 100644
--- a/chrome/browser/performance_manager/graph/node_base.cc
+++ b/chrome/browser/performance_manager/graph/node_base.cc
@@ -15,20 +15,12 @@
NodeBase::NodeBase(const resource_coordinator::CoordinationUnitID& id,
Graph* graph)
: graph_(graph), id_(id.type, id.id) {
- // TODO(siggi): The constructor needs to detach from the sequence once the
- // lifetime changes are done.
- // DETACH_FROM_SEQUENCE(sequence_checker_);
}
NodeBase::~NodeBase() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}
-void NodeBase::Destruct() {
- DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- graph_->DestroyNode(this);
-}
-
void NodeBase::BeforeDestroyed() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
for (auto& observer : observers_)
@@ -98,10 +90,4 @@
OnPropertyChanged(property_type, value);
}
-// static
-NodeBase* NodeBase::PassOwnershipToGraph(std::unique_ptr<NodeBase> new_cu) {
- auto *graph = new_cu->graph();
- return graph->AddNewNode(std::move(new_cu));
-}
-
} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/graph/node_base.h b/chrome/browser/performance_manager/graph/node_base.h
index 1e87d6f..819c97d 100644
--- a/chrome/browser/performance_manager/graph/node_base.h
+++ b/chrome/browser/performance_manager/graph/node_base.h
@@ -33,7 +33,6 @@
NodeBase(const resource_coordinator::CoordinationUnitID& id, Graph* graph);
virtual ~NodeBase();
- void Destruct();
void BeforeDestroyed();
void AddObserver(GraphObserver* observer);
void RemoveObserver(GraphObserver* observer);
@@ -44,6 +43,7 @@
const resource_coordinator::mojom::PropertyType property_type,
int64_t default_value) const;
+ // May be called on any thread.
const resource_coordinator::CoordinationUnitID& id() const { return id_; }
Graph* graph() const { return graph_; }
@@ -86,9 +86,6 @@
void SetProperty(resource_coordinator::mojom::PropertyType property_type,
int64_t value);
- // Passes the ownership of the newly created |new_cu| to its graph.
- static NodeBase* PassOwnershipToGraph(std::unique_ptr<NodeBase> new_cu);
-
Graph* const graph_;
const resource_coordinator::CoordinationUnitID id_;
@@ -106,15 +103,6 @@
class MojoRequestClass>
class CoordinationUnitInterface : public NodeBase, public MojoInterfaceClass {
public:
- static CoordinationUnitClass* Create(
- const resource_coordinator::CoordinationUnitID& id,
- Graph* graph) {
- std::unique_ptr<CoordinationUnitClass> new_cu =
- std::make_unique<CoordinationUnitClass>(id, graph);
- return static_cast<CoordinationUnitClass*>(
- PassOwnershipToGraph(std::move(new_cu)));
- }
-
static const CoordinationUnitClass* FromNodeBase(const NodeBase* cu) {
DCHECK(cu->id().type == CoordinationUnitClass::Type());
return static_cast<const CoordinationUnitClass*>(cu);
diff --git a/chrome/browser/performance_manager/graph/node_base_unittest.cc b/chrome/browser/performance_manager/graph/node_base_unittest.cc
index 851b797..555bcca 100644
--- a/chrome/browser/performance_manager/graph/node_base_unittest.cc
+++ b/chrome/browser/performance_manager/graph/node_base_unittest.cc
@@ -3,7 +3,6 @@
// found in the LICENSE file.
#include "chrome/browser/performance_manager/graph/node_base.h"
-#include "chrome/browser/performance_manager/graph/graph_node_provider_impl.h"
#include "chrome/browser/performance_manager/graph/graph_test_harness.h"
#include "chrome/browser/performance_manager/graph/mock_graphs.h"
#include "chrome/browser/performance_manager/graph/page_node_impl.h"
diff --git a/chrome/browser/performance_manager/graph/page_node_impl.cc b/chrome/browser/performance_manager/graph/page_node_impl.cc
index e0ca023..4110968 100644
--- a/chrome/browser/performance_manager/graph/page_node_impl.cc
+++ b/chrome/browser/performance_manager/graph/page_node_impl.cc
@@ -35,9 +35,12 @@
: CoordinationUnitInterface(id, graph),
visibility_change_time_(ResourceCoordinatorClock::NowTicks()) {
InvalidateAllInterventionPolicies();
+
+ DETACH_FROM_SEQUENCE(sequence_checker_);
}
PageNodeImpl::~PageNodeImpl() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
for (auto* child_frame : frame_coordination_units_)
child_frame->RemovePageNode(this);
}
@@ -49,7 +52,7 @@
FrameNodeImpl* frame_cu = FrameNodeImpl::GetNodeByID(graph_, cu_id);
if (!frame_cu)
return;
- if (AddFrame(frame_cu))
+ if (AddFrameImpl(frame_cu))
frame_cu->AddPageNode(this);
}
@@ -60,7 +63,7 @@
FrameNodeImpl* frame_cu = FrameNodeImpl::GetNodeByID(graph_, cu_id);
if (!frame_cu)
return;
- if (RemoveFrame(frame_cu))
+ if (RemoveFrameImpl(frame_cu))
frame_cu->RemovePageNode(this);
}
@@ -261,7 +264,7 @@
observer.OnPagePropertyChanged(this, property_type, value);
}
-bool PageNodeImpl::AddFrame(FrameNodeImpl* frame_cu) {
+bool PageNodeImpl::AddFrameImpl(FrameNodeImpl* frame_cu) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
const bool inserted = frame_coordination_units_.insert(frame_cu).second;
if (inserted) {
@@ -275,7 +278,7 @@
return inserted;
}
-bool PageNodeImpl::RemoveFrame(FrameNodeImpl* frame_cu) {
+bool PageNodeImpl::RemoveFrameImpl(FrameNodeImpl* frame_cu) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
bool removed = frame_coordination_units_.erase(frame_cu) > 0;
if (removed) {
diff --git a/chrome/browser/performance_manager/graph/page_node_impl.h b/chrome/browser/performance_manager/graph/page_node_impl.h
index 7a3075a7..0963a1d3 100644
--- a/chrome/browser/performance_manager/graph/page_node_impl.h
+++ b/chrome/browser/performance_manager/graph/page_node_impl.h
@@ -143,8 +143,8 @@
resource_coordinator::mojom::PropertyType property_type,
int64_t value) override;
- bool AddFrame(FrameNodeImpl* frame_cu);
- bool RemoveFrame(FrameNodeImpl* frame_cu);
+ bool AddFrameImpl(FrameNodeImpl* frame_cu);
+ bool RemoveFrameImpl(FrameNodeImpl* frame_cu);
// This is called whenever |num_frozen_frames_| changes, or whenever
// |frame_coordination_units_.size()| changes. It is used to synthesize the
diff --git a/chrome/browser/performance_manager/graph/process_node_impl.cc b/chrome/browser/performance_manager/graph/process_node_impl.cc
index 30626ca66..b6b75d6 100644
--- a/chrome/browser/performance_manager/graph/process_node_impl.cc
+++ b/chrome/browser/performance_manager/graph/process_node_impl.cc
@@ -13,9 +13,12 @@
ProcessNodeImpl::ProcessNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph)
- : CoordinationUnitInterface(id, graph) {}
+ : CoordinationUnitInterface(id, graph) {
+ DETACH_FROM_SEQUENCE(sequence_checker_);
+}
ProcessNodeImpl::~ProcessNodeImpl() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Make as if we're transitioning to the null PID before we die to clear this
// instance from the PID map.
if (process_id_ != base::kNullProcessId)
diff --git a/chrome/browser/performance_manager/graph/system_node_impl.cc b/chrome/browser/performance_manager/graph/system_node_impl.cc
index cf23570..3496179 100644
--- a/chrome/browser/performance_manager/graph/system_node_impl.cc
+++ b/chrome/browser/performance_manager/graph/system_node_impl.cc
@@ -21,7 +21,9 @@
Graph* graph)
: CoordinationUnitInterface(id, graph) {}
-SystemNodeImpl::~SystemNodeImpl() = default;
+SystemNodeImpl::~SystemNodeImpl() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
void SystemNodeImpl::OnProcessCPUUsageReady() {
SendEvent(resource_coordinator::mojom::Event::kProcessCPUUsageReady);
diff --git a/chrome/browser/performance_manager/page_resource_coordinator.cc b/chrome/browser/performance_manager/page_resource_coordinator.cc
deleted file mode 100644
index ed014e39..0000000
--- a/chrome/browser/performance_manager/page_resource_coordinator.cc
+++ /dev/null
@@ -1,100 +0,0 @@
-// Copyright 2017 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/page_resource_coordinator.h"
-#include "base/bind.h"
-
-namespace performance_manager {
-
-PageResourceCoordinator::PageResourceCoordinator(
- PerformanceManager* performance_manager)
- : ResourceCoordinatorInterface(), weak_ptr_factory_(this) {
- resource_coordinator::CoordinationUnitID new_cu_id(
- resource_coordinator::CoordinationUnitType::kPage,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- ResourceCoordinatorInterface::ConnectToService(performance_manager,
- new_cu_id);
-}
-
-PageResourceCoordinator::~PageResourceCoordinator() = default;
-
-void PageResourceCoordinator::SetIsLoading(bool is_loading) {
- if (!service_)
- return;
- service_->SetIsLoading(is_loading);
-}
-
-void PageResourceCoordinator::SetVisibility(bool visible) {
- if (!service_)
- return;
- service_->SetVisibility(visible);
-}
-
-void PageResourceCoordinator::SetUKMSourceId(int64_t ukm_source_id) {
- if (!service_)
- return;
- service_->SetUKMSourceId(ukm_source_id);
-}
-
-void PageResourceCoordinator::OnFaviconUpdated() {
- if (!service_)
- return;
- service_->OnFaviconUpdated();
-}
-
-void PageResourceCoordinator::OnTitleUpdated() {
- if (!service_)
- return;
- service_->OnTitleUpdated();
-}
-
-void PageResourceCoordinator::OnMainFrameNavigationCommitted(
- base::TimeTicks navigation_committed_time,
- uint64_t navigation_id,
- const std::string& url) {
- if (!service_)
- return;
- service_->OnMainFrameNavigationCommitted(navigation_committed_time,
- navigation_id, url);
-}
-
-void PageResourceCoordinator::AddFrame(const FrameResourceCoordinator& frame) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (!service_ || !frame.service())
- return;
- // We could keep the ID around ourselves, but this hop ensures that the child
- // has been created on the service-side.
- frame.service()->GetID(base::BindOnce(&PageResourceCoordinator::AddFrameByID,
- weak_ptr_factory_.GetWeakPtr()));
-}
-
-void PageResourceCoordinator::RemoveFrame(
- const FrameResourceCoordinator& frame) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- if (!service_ || !frame.service())
- return;
- frame.service()->GetID(
- base::BindOnce(&PageResourceCoordinator::RemoveFrameByID,
- weak_ptr_factory_.GetWeakPtr()));
-}
-
-void PageResourceCoordinator::ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) {
- provider->CreatePageCoordinationUnit(mojo::MakeRequest(&service_), cu_id);
-}
-
-void PageResourceCoordinator::AddFrameByID(
- const resource_coordinator::CoordinationUnitID& cu_id) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- service_->AddFrame(cu_id);
-}
-
-void PageResourceCoordinator::RemoveFrameByID(
- const resource_coordinator::CoordinationUnitID& cu_id) {
- DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
- service_->RemoveFrame(cu_id);
-}
-
-} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/page_resource_coordinator.h b/chrome/browser/performance_manager/page_resource_coordinator.h
deleted file mode 100644
index fd82255..0000000
--- a/chrome/browser/performance_manager/page_resource_coordinator.h
+++ /dev/null
@@ -1,56 +0,0 @@
-// Copyright 2017 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_PAGE_RESOURCE_COORDINATOR_H_
-#define CHROME_BROWSER_PERFORMANCE_MANAGER_PAGE_RESOURCE_COORDINATOR_H_
-
-#include "base/memory/weak_ptr.h"
-#include "base/threading/thread_checker.h"
-#include "base/time/time.h"
-#include "chrome/browser/performance_manager/frame_resource_coordinator.h"
-#include "chrome/browser/performance_manager/resource_coordinator_interface.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
-
-namespace performance_manager {
-
-class PageResourceCoordinator
- : public ResourceCoordinatorInterface<
- resource_coordinator::mojom::PageCoordinationUnitPtr,
- resource_coordinator::mojom::PageCoordinationUnitRequest> {
- public:
- explicit PageResourceCoordinator(PerformanceManager* performance_manager);
- ~PageResourceCoordinator() override;
-
- void SetIsLoading(bool is_loading);
- void SetVisibility(bool visible);
- void SetUKMSourceId(int64_t ukm_source_id);
- void OnFaviconUpdated();
- void OnTitleUpdated();
- void OnMainFrameNavigationCommitted(base::TimeTicks navigation_committed_time,
- uint64_t navigation_id,
- const std::string& url);
-
- void AddFrame(const FrameResourceCoordinator& frame);
- void RemoveFrame(const FrameResourceCoordinator& frame);
-
- private:
- void ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) override;
-
- void AddFrameByID(const resource_coordinator::CoordinationUnitID& cu_id);
- void RemoveFrameByID(const resource_coordinator::CoordinationUnitID& cu_id);
-
- THREAD_CHECKER(thread_checker_);
-
- // The WeakPtrFactory should come last so the weak ptrs are invalidated
- // before the rest of the member variables.
- base::WeakPtrFactory<PageResourceCoordinator> weak_ptr_factory_;
-
- DISALLOW_COPY_AND_ASSIGN(PageResourceCoordinator);
-};
-
-} // namespace performance_manager
-
-#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_PAGE_RESOURCE_COORDINATOR_H_
diff --git a/chrome/browser/performance_manager/performance_manager.cc b/chrome/browser/performance_manager/performance_manager.cc
index 7460c46..e1523e4 100644
--- a/chrome/browser/performance_manager/performance_manager.cc
+++ b/chrome/browser/performance_manager/performance_manager.cc
@@ -14,6 +14,7 @@
#include "base/task/task_traits.h"
#include "build/build_config.h"
#include "chrome/browser/performance_manager/decorators/page_almost_idle_decorator.h"
+#include "chrome/browser/performance_manager/graph/page_node_impl.h"
#include "chrome/browser/performance_manager/graph/system_node_impl.h"
#include "chrome/browser/performance_manager/observers/metrics_collector.h"
#include "chrome/browser/performance_manager/observers/page_signal_generator_impl.h"
@@ -75,7 +76,19 @@
base::Unretained(this), std::move(batch)));
}
-void PerformanceManager::BindInterface(
+std::unique_ptr<FrameNodeImpl> PerformanceManager::CreateFrameNode() {
+ return CreateNodeImpl<FrameNodeImpl>();
+}
+
+std::unique_ptr<PageNodeImpl> PerformanceManager::CreatePageNode() {
+ return CreateNodeImpl<PageNodeImpl>();
+}
+
+std::unique_ptr<ProcessNodeImpl> PerformanceManager::CreateProcessNode() {
+ return CreateNodeImpl<ProcessNodeImpl>();
+}
+
+void PerformanceManager::PostBindInterface(
const std::string& interface_name,
mojo::ScopedMessagePipeHandle message_pipe) {
task_runner_->PostTask(FROM_HERE,
@@ -84,6 +97,30 @@
std::move(message_pipe)));
}
+template <typename NodeType>
+std::unique_ptr<NodeType> PerformanceManager::CreateNodeImpl() {
+ resource_coordinator::CoordinationUnitID id(
+ NodeType::Type(), resource_coordinator::CoordinationUnitID::RANDOM_ID);
+ std::unique_ptr<NodeType> new_node = std::make_unique<NodeType>(id, &graph_);
+ task_runner_->PostTask(
+ FROM_HERE, base::BindOnce(&Graph::AddNewNode, base::Unretained(&graph_),
+ base::Unretained(new_node.get())));
+
+ return new_node;
+}
+
+void PerformanceManager::PostDeleteNode(std::unique_ptr<NodeBase> node) {
+ task_runner_->PostTask(
+ FROM_HERE, base::BindOnce(&PerformanceManager::DeleteNodeImpl,
+ base::Unretained(this), std::move(node)));
+}
+
+void PerformanceManager::DeleteNodeImpl(std::unique_ptr<NodeBase> node) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+ graph_.DestroyNode(node.get());
+}
+
void PerformanceManager::OnStart() {
// Some tests don't initialize the service manager connection, so this class
// tolerates its absence for tests.
@@ -125,8 +162,6 @@
ukm_recorder_ = ukm::MojoUkmRecorder::Create(connector.get());
graph_.set_ukm_recorder(ukm_recorder_.get());
}
-
- graph_.OnStart(&interface_registry_);
}
void PerformanceManager::BindInterfaceImpl(
diff --git a/chrome/browser/performance_manager/performance_manager.h b/chrome/browser/performance_manager/performance_manager.h
index ea4063c..709cf40 100644
--- a/chrome/browser/performance_manager/performance_manager.h
+++ b/chrome/browser/performance_manager/performance_manager.h
@@ -7,6 +7,7 @@
#include <memory>
#include <string>
+#include <utility>
#include <vector>
#include "base/sequence_checker.h"
@@ -27,6 +28,8 @@
namespace performance_manager {
+class PageNodeImpl;
+
// The performance manager is a rendezvous point for binding to performance
// manager interfaces.
// TODO(https://crbug.com/910288): Refactor this along with the
@@ -59,14 +62,36 @@
void DistributeMeasurementBatch(
resource_coordinator::mojom::ProcessResourceMeasurementBatchPtr batch);
+ // Creates a new node of the requested type and adds it to the graph.
+ // May be called from any sequence.
+ std::unique_ptr<FrameNodeImpl> CreateFrameNode();
+ std::unique_ptr<PageNodeImpl> CreatePageNode();
+ std::unique_ptr<ProcessNodeImpl> CreateProcessNode();
+
+ // Destroys a node returned from the creation functions above.
+ // May be called from any sequence.
+ template <typename NodeType>
+ void DeleteNode(std::unique_ptr<NodeType> node);
+
+ // TODO(siggi): Can this be hidden away?
+ scoped_refptr<base::SequencedTaskRunner> task_runner() const {
+ return task_runner_;
+ }
+
private:
using InterfaceRegistry = service_manager::BinderRegistryWithArgs<
const service_manager::BindSourceInfo&>;
PerformanceManager();
- void BindInterface(const std::string& interface_name,
- mojo::ScopedMessagePipeHandle message_pipe);
+ void PostBindInterface(const std::string& interface_name,
+ mojo::ScopedMessagePipeHandle message_pipe);
+
+ template <typename NodeType>
+ std::unique_ptr<NodeType> CreateNodeImpl();
+
+ void PostDeleteNode(std::unique_ptr<NodeBase> node);
+ void DeleteNodeImpl(std::unique_ptr<NodeBase> node);
void OnStart();
void OnStartImpl(std::unique_ptr<service_manager::Connector> connector);
@@ -102,7 +127,12 @@
template <typename Interface>
void PerformanceManager::BindInterface(
mojo::InterfaceRequest<Interface> request) {
- BindInterface(Interface::Name_, request.PassMessagePipe());
+ PostBindInterface(Interface::Name_, request.PassMessagePipe());
+}
+
+template <typename NodeType>
+void PerformanceManager::DeleteNode(std::unique_ptr<NodeType> node) {
+ PostDeleteNode(std::move(node));
}
} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/performance_manager_tab_helper.cc b/chrome/browser/performance_manager/performance_manager_tab_helper.cc
index c98fb4a..debc45d2 100644
--- a/chrome/browser/performance_manager/performance_manager_tab_helper.cc
+++ b/chrome/browser/performance_manager/performance_manager_tab_helper.cc
@@ -7,8 +7,11 @@
#include <utility>
#include <vector>
+#include "base/bind.h"
#include "base/stl_util.h"
-#include "chrome/browser/performance_manager/frame_resource_coordinator.h"
+#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/performance_manager.h"
#include "chrome/browser/performance_manager/render_process_user_data.h"
#include "content/public/browser/navigation_handle.h"
@@ -26,7 +29,7 @@
PerformanceManagerTabHelper* helper = FromWebContents(web_contents);
if (!helper)
return false;
- *id = helper->page_resource_coordinator_.id();
+ *id = helper->page_node_->id();
return true;
}
@@ -41,7 +44,7 @@
content::WebContents* web_contents)
: content::WebContentsObserver(web_contents),
performance_manager_(PerformanceManager::GetInstance()),
- page_resource_coordinator_(PerformanceManager::GetInstance()) {
+ page_node_(performance_manager_->CreatePageNode()) {
// Make sure to set the visibility property when we create
// |page_resource_coordinator_|.
UpdatePageNodeVisibility(web_contents->GetVisibility());
@@ -68,6 +71,10 @@
}
PerformanceManagerTabHelper::~PerformanceManagerTabHelper() {
+ performance_manager_->DeleteNode(std::move(page_node_));
+ for (auto& kv : frames_)
+ performance_manager_->DeleteNode(std::move(kv.second));
+
if (first_ == this)
first_ = next_;
@@ -87,13 +94,16 @@
// This must not exist in the map yet.
DCHECK(!base::ContainsKey(frames_, render_frame_host));
- std::unique_ptr<FrameResourceCoordinator> frame =
- std::make_unique<FrameResourceCoordinator>(performance_manager_);
+ std::unique_ptr<FrameNodeImpl> frame =
+ performance_manager_->CreateFrameNode();
content::RenderFrameHost* parent = render_frame_host->GetParent();
if (parent) {
DCHECK(base::ContainsKey(frames_, parent));
auto& parent_frame_node = frames_[parent];
- parent_frame_node->AddChildFrame(*frame.get());
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&FrameNodeImpl::AddChildFrame,
+ base::Unretained(parent_frame_node.get()), frame->id()));
}
RenderProcessUserData* user_data =
@@ -103,24 +113,35 @@
// is not in play.
// TODO(siggi): Figure out how to assert on this when the main parts are
// registered with the content browser client.
- if (user_data)
- frame->SetProcess(*user_data->process_resource_coordinator());
+ if (user_data) {
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&FrameNodeImpl::SetProcess,
+ base::Unretained(frame.get()),
+ user_data->process_node()->id()));
+ }
frames_[render_frame_host] = std::move(frame);
}
void PerformanceManagerTabHelper::RenderFrameDeleted(
content::RenderFrameHost* render_frame_host) {
- DCHECK(base::ContainsKey(frames_, render_frame_host));
- frames_.erase(render_frame_host);
+ auto it = frames_.find(render_frame_host);
+ DCHECK(it != frames_.end());
+
+ performance_manager_->DeleteNode(std::move(it->second));
+ frames_.erase(it);
}
void PerformanceManagerTabHelper::DidStartLoading() {
- page_resource_coordinator_.SetIsLoading(true);
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&PageNodeImpl::SetIsLoading,
+ base::Unretained(page_node_.get()), true));
}
void PerformanceManagerTabHelper::DidStopLoading() {
- page_resource_coordinator_.SetIsLoading(false);
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&PageNodeImpl::SetIsLoading,
+ base::Unretained(page_node_.get()), false));
}
void PerformanceManagerTabHelper::OnVisibilityChanged(
@@ -149,13 +170,20 @@
auto it = frames_.find(render_frame_host);
if (it != frames_.end()) {
// TODO(siggi): See whether this can be done in RenderFrameCreated.
- page_resource_coordinator_.AddFrame(*(it->second));
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&PageNodeImpl::AddFrame,
+ base::Unretained(page_node_.get()), it->second->id()));
if (navigation_handle->IsInMainFrame()) {
OnMainFrameNavigation(navigation_handle->GetNavigationId());
- page_resource_coordinator_.OnMainFrameNavigationCommitted(
- navigation_committed_time, navigation_handle->GetNavigationId(),
- navigation_handle->GetURL().spec());
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&PageNodeImpl::OnMainFrameNavigationCommitted,
+ base::Unretained(page_node_.get()),
+ navigation_committed_time,
+ navigation_handle->GetNavigationId(),
+ navigation_handle->GetURL().spec()));
}
}
}
@@ -166,7 +194,9 @@
first_time_title_set_ = true;
return;
}
- page_resource_coordinator_.OnTitleUpdated();
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&PageNodeImpl::OnTitleUpdated,
+ base::Unretained(page_node_.get())));
}
void PerformanceManagerTabHelper::DidUpdateFaviconURL(
@@ -176,7 +206,9 @@
first_time_favicon_set_ = true;
return;
}
- page_resource_coordinator_.OnFaviconUpdated();
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&PageNodeImpl::OnFaviconUpdated,
+ base::Unretained(page_node_.get())));
}
void PerformanceManagerTabHelper::OnInterfaceRequestFromFrame(
@@ -189,15 +221,21 @@
auto it = frames_.find(render_frame_host);
DCHECK(it != frames_.end());
- it->second->AddBinding(
- resource_coordinator::mojom::FrameCoordinationUnitRequest(
- std::move(*interface_pipe)));
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&FrameNodeImpl::AddBinding,
+ base::Unretained(it->second.get()),
+ resource_coordinator::mojom::FrameCoordinationUnitRequest(
+ std::move(*interface_pipe))));
}
void PerformanceManagerTabHelper::OnMainFrameNavigation(int64_t navigation_id) {
ukm_source_id_ =
ukm::ConvertToSourceId(navigation_id, ukm::SourceIdType::NAVIGATION_ID);
- page_resource_coordinator_.SetUKMSourceId(ukm_source_id_);
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&PageNodeImpl::SetUKMSourceId,
+ base::Unretained(page_node_.get()), ukm_source_id_));
first_time_title_set_ = false;
first_time_favicon_set_ = false;
@@ -207,7 +245,10 @@
content::Visibility visibility) {
// TODO(fdoray): An OCCLUDED tab should not be considered visible.
const bool is_visible = visibility != content::Visibility::HIDDEN;
- page_resource_coordinator_.SetVisibility(is_visible);
+ performance_manager_->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&PageNodeImpl::SetVisibility,
+ base::Unretained(page_node_.get()), is_visible));
}
WEB_CONTENTS_USER_DATA_KEY_IMPL(PerformanceManagerTabHelper)
diff --git a/chrome/browser/performance_manager/performance_manager_tab_helper.h b/chrome/browser/performance_manager/performance_manager_tab_helper.h
index 1833612..b4d2861 100644
--- a/chrome/browser/performance_manager/performance_manager_tab_helper.h
+++ b/chrome/browser/performance_manager/performance_manager_tab_helper.h
@@ -11,7 +11,6 @@
#include <vector>
#include "base/macros.h"
-#include "chrome/browser/performance_manager/page_resource_coordinator.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
@@ -19,8 +18,8 @@
namespace performance_manager {
-class FrameResourceCoordinator;
-class PageResourceCoordinator;
+class FrameNodeImpl;
+class PageNodeImpl;
class PerformanceManager;
// This tab helper maintains a page node, and its associated tree of frame nodes
@@ -42,9 +41,7 @@
~PerformanceManagerTabHelper() override;
- PageResourceCoordinator* page_resource_coordinator() {
- return &page_resource_coordinator_;
- }
+ PageNodeImpl* page_node() { return page_node_.get(); }
// WebContentsObserver overrides.
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
@@ -74,8 +71,7 @@
// The performance manager for this process, if any.
PerformanceManager* const performance_manager_;
-
- PageResourceCoordinator page_resource_coordinator_;
+ std::unique_ptr<PageNodeImpl> page_node_;
ukm::SourceId ukm_source_id_ = ukm::kInvalidSourceId;
// Favicon and title are set when a page is loaded, we only want to send
@@ -85,9 +81,8 @@
bool first_time_favicon_set_ = false;
bool first_time_title_set_ = false;
- // Maps from RenderFrameHost to the associated RC node.
- std::map<content::RenderFrameHost*, std::unique_ptr<FrameResourceCoordinator>>
- frames_;
+ // Maps from RenderFrameHost to the associated PM node.
+ std::map<content::RenderFrameHost*, std::unique_ptr<FrameNodeImpl>> frames_;
// All instances are linked together in a doubly linked list to allow orderly
// destruction at browser shutdown time.
diff --git a/chrome/browser/performance_manager/performance_manager_unittest.cc b/chrome/browser/performance_manager/performance_manager_unittest.cc
index adca71e..57c5870 100644
--- a/chrome/browser/performance_manager/performance_manager_unittest.cc
+++ b/chrome/browser/performance_manager/performance_manager_unittest.cc
@@ -8,13 +8,9 @@
#include "base/test/bind_test_util.h"
#include "base/test/scoped_task_environment.h"
-#include "chrome/browser/performance_manager/frame_resource_coordinator.h"
-#include "chrome/browser/performance_manager/page_resource_coordinator.h"
-#include "chrome/browser/performance_manager/process_resource_coordinator.h"
-#include "chrome/browser/performance_manager/system_resource_coordinator.h"
-#include "services/resource_coordinator/public/cpp/coordination_unit_id.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit_provider.mojom.h"
+#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 "testing/gtest/include/gtest/gtest.h"
namespace performance_manager {
@@ -42,31 +38,6 @@
task_environment_.RunUntilIdle();
}
- // Given a CU, tests that it works by invoking GetID and waiting for the
- // response. This test will hang (and eventually fail) if the response does
- // not come back from the remote endpoint.
- template <typename CoordinationUnitPtrType>
- void TestCUImpl(CoordinationUnitPtrType cu) {
- base::RunLoop loop;
- cu->GetID(base::BindLambdaForTesting(
- [&loop](const resource_coordinator::CoordinationUnitID& cu_id) {
- loop.Quit();
- }));
- loop.Run();
- }
-
- // Variant that works with mojo interface pointers.
- template <typename CoordinationUnitPtrType>
- void TestCU(CoordinationUnitPtrType& cu) {
- TestCUImpl<CoordinationUnitPtrType&>(cu);
- }
-
- // Variant that works with pointers to FooResourceCoordinator wrappers.
- template <typename CoordinationUnitPtrType>
- void TestCU(CoordinationUnitPtrType* cu) {
- TestCUImpl<CoordinationUnitPtrType*>(cu);
- }
-
protected:
PerformanceManager* performance_manager() {
return performance_manager_.get();
@@ -79,50 +50,27 @@
DISALLOW_COPY_AND_ASSIGN(PerformanceManagerTest);
};
-TEST_F(PerformanceManagerTest, ResourceCoordinatorInstantiate) {
- // Get the CU provider interface.
- resource_coordinator::mojom::CoordinationUnitProviderPtr provider;
- performance_manager()->BindInterface(mojo::MakeRequest(&provider));
+TEST_F(PerformanceManagerTest, InstantiateNodes) {
+ // Create a node of each type.
+ std::unique_ptr<FrameNodeImpl> frame_node =
+ performance_manager()->CreateFrameNode();
+ EXPECT_NE(nullptr, frame_node.get());
- // Create and test a dummy FrameCU.
- resource_coordinator::CoordinationUnitID frame_id(
- resource_coordinator::CoordinationUnitType::kFrame,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- resource_coordinator::mojom::FrameCoordinationUnitPtr frame_cu;
- provider->CreateFrameCoordinationUnit(mojo::MakeRequest(&frame_cu), frame_id);
- TestCU(frame_cu);
+ std::unique_ptr<PageNodeImpl> page_node =
+ performance_manager()->CreatePageNode();
+ EXPECT_NE(nullptr, page_node.get());
- // Create and test a dummy PageCU.
- resource_coordinator::CoordinationUnitID page_id(
- resource_coordinator::CoordinationUnitType::kPage,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- resource_coordinator::mojom::PageCoordinationUnitPtr page_cu;
- provider->CreatePageCoordinationUnit(mojo::MakeRequest(&page_cu), page_id);
- TestCU(page_cu);
+ std::unique_ptr<ProcessNodeImpl> process_node =
+ performance_manager()->CreateProcessNode();
+ EXPECT_NE(nullptr, process_node.get());
- // Create and test a dummy SystemCU.
- resource_coordinator::mojom::SystemCoordinationUnitPtr system_cu;
- provider->GetSystemCoordinationUnit(mojo::MakeRequest(&system_cu));
- TestCU(system_cu);
-
- // Create and test a dummy ProcessCU.
- resource_coordinator::CoordinationUnitID process_id(
- resource_coordinator::CoordinationUnitType::kProcess,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- resource_coordinator::mojom::ProcessCoordinationUnitPtr process_cu;
- provider->CreateProcessCoordinationUnit(mojo::MakeRequest(&process_cu),
- process_id);
- TestCU(process_cu);
-
- // Also test the convenience headers for creating and communicating with CUs.
- FrameResourceCoordinator frame_rc(performance_manager());
- TestCU(&frame_rc);
- PageResourceCoordinator page_rc(performance_manager());
- TestCU(&page_rc);
- ProcessResourceCoordinator process_rc(performance_manager());
- TestCU(&process_rc);
- SystemResourceCoordinator system_rc(performance_manager());
- TestCU(&system_rc);
+ performance_manager()->DeleteNode(std::move(process_node));
+ performance_manager()->DeleteNode(std::move(page_node));
+ performance_manager()->DeleteNode(std::move(frame_node));
}
+// TODO(siggi): More tests!
+// - Test the WebUI interface.
+// - Test the graph introspector interface.
+
} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/process_resource_coordinator.cc b/chrome/browser/performance_manager/process_resource_coordinator.cc
deleted file mode 100644
index f213837..0000000
--- a/chrome/browser/performance_manager/process_resource_coordinator.cc
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright 2017 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/process_resource_coordinator.h"
-
-#include "base/time/time.h"
-#include "build/build_config.h"
-
-namespace performance_manager {
-
-ProcessResourceCoordinator::ProcessResourceCoordinator(
- PerformanceManager* performance_manager)
- : ResourceCoordinatorInterface(), weak_ptr_factory_(this) {
- resource_coordinator::CoordinationUnitID new_cu_id(
- resource_coordinator::CoordinationUnitType::kProcess,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- ResourceCoordinatorInterface::ConnectToService(performance_manager,
- new_cu_id);
-}
-
-ProcessResourceCoordinator::~ProcessResourceCoordinator() = default;
-
-void ProcessResourceCoordinator::OnProcessLaunched(
- const base::Process& process) {
- if (!service_)
- return;
-
- // TODO(fdoray): Merge ProcessCoordinationUnit::SetPID/SetLaunchTime().
- service_->SetPID(process.Pid());
- service_->SetLaunchTime(
-#if defined(OS_ANDROID)
- // Process::CreationTime() is not available on Android. Since this method
- // is called immediately after the process is launched, the process launch
- // time can be approximated with the current time.
- base::Time::Now()
-#else
- process.CreationTime()
-#endif
- );
-}
-
-void ProcessResourceCoordinator::SetCPUUsage(double cpu_usage) {
- if (!service_)
- return;
-
- service_->SetCPUUsage(cpu_usage);
-}
-
-void ProcessResourceCoordinator::SetProcessExitStatus(int32_t exit_status) {
- if (!service_)
- return;
-
- service_->SetProcessExitStatus(exit_status);
-}
-
-void ProcessResourceCoordinator::ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) {
- provider->CreateProcessCoordinationUnit(mojo::MakeRequest(&service_), cu_id);
-}
-
-} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/process_resource_coordinator.h b/chrome/browser/performance_manager/process_resource_coordinator.h
deleted file mode 100644
index fe5475c..0000000
--- a/chrome/browser/performance_manager/process_resource_coordinator.h
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright 2017 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_PROCESS_RESOURCE_COORDINATOR_H_
-#define CHROME_BROWSER_PERFORMANCE_MANAGER_PROCESS_RESOURCE_COORDINATOR_H_
-
-#include "base/memory/weak_ptr.h"
-#include "base/process/process.h"
-#include "base/threading/thread_checker.h"
-#include "chrome/browser/performance_manager/frame_resource_coordinator.h"
-#include "chrome/browser/performance_manager/resource_coordinator_interface.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
-
-namespace performance_manager {
-
-class ProcessResourceCoordinator
- : public ResourceCoordinatorInterface<
- resource_coordinator::mojom::ProcessCoordinationUnitPtr,
- resource_coordinator::mojom::ProcessCoordinationUnitRequest> {
- public:
- explicit ProcessResourceCoordinator(PerformanceManager* performance_manager);
- ~ProcessResourceCoordinator() override;
-
- // Must be called immediately after the process is launched.
- void OnProcessLaunched(const base::Process& process);
-
- void SetCPUUsage(double usage);
- void SetProcessExitStatus(int32_t exit_status);
-
- private:
- void ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) override;
-
- THREAD_CHECKER(thread_checker_);
-
- // The WeakPtrFactory should come last so the weak ptrs are invalidated
- // before the rest of the member variables.
- base::WeakPtrFactory<ProcessResourceCoordinator> weak_ptr_factory_;
-
- DISALLOW_COPY_AND_ASSIGN(ProcessResourceCoordinator);
-};
-
-} // namespace performance_manager
-
-#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_PROCESS_RESOURCE_COORDINATOR_H_
diff --git a/chrome/browser/performance_manager/render_process_user_data.cc b/chrome/browser/performance_manager/render_process_user_data.cc
index 1ec7eaf..cb10c81 100644
--- a/chrome/browser/performance_manager/render_process_user_data.cc
+++ b/chrome/browser/performance_manager/render_process_user_data.cc
@@ -7,8 +7,12 @@
#include <memory>
#include <utility>
+#include "base/bind.h"
#include "base/command_line.h"
+#include "base/logging.h"
#include "base/memory/ptr_util.h"
+#include "build/build_config.h"
+#include "chrome/browser/performance_manager/graph/process_node_impl.h"
#include "chrome/browser/performance_manager/performance_manager.h"
#include "content/public/browser/child_process_termination_info.h"
#include "content/public/browser/render_process_host.h"
@@ -27,8 +31,7 @@
RenderProcessUserData::RenderProcessUserData(
content::RenderProcessHost* render_process_host)
: host_(render_process_host),
- process_resource_coordinator_(
- performance_manager::PerformanceManager::GetInstance()) {
+ process_node_(PerformanceManager::GetInstance()->CreateProcessNode()) {
// The process itself shouldn't have been created at this point.
DCHECK(!host_->GetProcess().IsValid() ||
base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -44,6 +47,7 @@
}
RenderProcessUserData::~RenderProcessUserData() {
+ PerformanceManager::GetInstance()->DeleteNode(std::move(process_node_));
host_->RemoveObserver(this);
if (first_ == this)
@@ -81,14 +85,38 @@
void RenderProcessUserData::RenderProcessReady(
content::RenderProcessHost* host) {
- // TODO(siggi): Rename OnProcessLaunched->OnProcessReady.
- process_resource_coordinator_.OnProcessLaunched(host->GetProcess());
+ PerformanceManager* performance_manager = PerformanceManager::GetInstance();
+
+ // TODO(siggi): Change this to pass the process into the graph node.
+ const base::ProcessId pid = host->GetProcess().Pid();
+ const base::Time launch_time =
+#if defined(OS_ANDROID)
+ // Process::CreationTime() is not available on Android. Since this
+ // method is called immediately after the process is launched, the
+ // process launch time can be approximated with the current time.
+ base::Time::Now();
+#else
+ host->GetProcess().CreationTime();
+#endif
+
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE, base::BindOnce(&ProcessNodeImpl::SetPID,
+ base::Unretained(process_node_.get()), pid));
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&ProcessNodeImpl::SetLaunchTime,
+ base::Unretained(process_node_.get()), launch_time));
}
void RenderProcessUserData::RenderProcessExited(
content::RenderProcessHost* host,
const content::ChildProcessTerminationInfo& info) {
- process_resource_coordinator_.SetProcessExitStatus(info.exit_code);
+ PerformanceManager* performance_manager = PerformanceManager::GetInstance();
+
+ performance_manager->task_runner()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&ProcessNodeImpl::SetProcessExitStatus,
+ base::Unretained(process_node_.get()), info.exit_code));
}
} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/render_process_user_data.h b/chrome/browser/performance_manager/render_process_user_data.h
index 5152dcc..18a2554 100644
--- a/chrome/browser/performance_manager/render_process_user_data.h
+++ b/chrome/browser/performance_manager/render_process_user_data.h
@@ -5,9 +5,10 @@
#ifndef CHROME_BROWSER_PERFORMANCE_MANAGER_RENDER_PROCESS_USER_DATA_H_
#define CHROME_BROWSER_PERFORMANCE_MANAGER_RENDER_PROCESS_USER_DATA_H_
+#include <memory>
+
#include "base/macros.h"
#include "base/supports_user_data.h"
-#include "chrome/browser/performance_manager/process_resource_coordinator.h"
#include "content/public/browser/render_process_host_observer.h"
namespace content {
@@ -18,6 +19,8 @@
namespace performance_manager {
+class ProcessNodeImpl;
+
// Attached to RenderProcessHost as user data, associates the RenderProcessHost
// with the Resource Coordinator process node.
class RenderProcessUserData : public base::SupportsUserData::Data,
@@ -32,10 +35,7 @@
// Detaches all instances from their RenderProcessHosts and destroys them.
static void DetachAndDestroyAll();
- performance_manager::ProcessResourceCoordinator*
- process_resource_coordinator() {
- return &process_resource_coordinator_;
- }
+ ProcessNodeImpl* process_node() { return process_node_.get(); }
private:
explicit RenderProcessUserData(
@@ -56,7 +56,7 @@
content::RenderProcessHost* const host_;
- performance_manager::ProcessResourceCoordinator process_resource_coordinator_;
+ std::unique_ptr<ProcessNodeImpl> process_node_;
DISALLOW_COPY_AND_ASSIGN(RenderProcessUserData);
};
diff --git a/chrome/browser/performance_manager/resource_coordinator_interface.h b/chrome/browser/performance_manager/resource_coordinator_interface.h
deleted file mode 100644
index 0a182bb..0000000
--- a/chrome/browser/performance_manager/resource_coordinator_interface.h
+++ /dev/null
@@ -1,70 +0,0 @@
-// Copyright 2017 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_RESOURCE_COORDINATOR_INTERFACE_H_
-#define CHROME_BROWSER_PERFORMANCE_MANAGER_RESOURCE_COORDINATOR_INTERFACE_H_
-
-#include <stdint.h>
-
-#include "base/component_export.h"
-#include "base/macros.h"
-#include "chrome/browser/performance_manager/performance_manager.h"
-#include "services/resource_coordinator/public/cpp/coordination_unit_id.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit_provider.mojom.h"
-#include "services/resource_coordinator/public/mojom/service_constants.mojom.h"
-
-namespace performance_manager {
-
-template <class CoordinationUnitMojoPtr, class CoordinationUnitMojoRequest>
-class ResourceCoordinatorInterface {
- public:
- ResourceCoordinatorInterface() = default;
- virtual ~ResourceCoordinatorInterface() = default;
-
- void AddBinding(CoordinationUnitMojoRequest request) {
- if (!service_)
- return;
- service_->AddBinding(std::move(request));
- }
-
- // Returns the ID. Note that this is meaningless for a singleton CU.
- resource_coordinator::CoordinationUnitID id() const { return cu_id_; }
-
- // Returns the remote endpoint interface.
- const CoordinationUnitMojoPtr& service() const { return service_; }
-
- // Expose the GetID function for testing.
- using CoordinationUnitMojo = typename CoordinationUnitMojoPtr::InterfaceType;
- using GetIDCallback = typename CoordinationUnitMojo::GetIDCallback;
- void GetID(GetIDCallback callback) {
- if (!service_)
- std::move(callback).Run(cu_id_);
- service_->GetID(std::move(callback));
- }
-
- protected:
- virtual void ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) = 0;
-
- void ConnectToService(PerformanceManager* performance_manager,
- const resource_coordinator::CoordinationUnitID& cu_id) {
- if (!performance_manager)
- return;
- cu_id_ = cu_id;
- resource_coordinator::mojom::CoordinationUnitProviderPtr provider;
- performance_manager->BindInterface(mojo::MakeRequest(&provider));
- ConnectToService(provider, cu_id);
- }
-
- CoordinationUnitMojoPtr service_;
- resource_coordinator::CoordinationUnitID cu_id_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(ResourceCoordinatorInterface);
-};
-
-} // namespace performance_manager
-
-#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_RESOURCE_COORDINATOR_INTERFACE_H_
diff --git a/chrome/browser/performance_manager/system_resource_coordinator.cc b/chrome/browser/performance_manager/system_resource_coordinator.cc
deleted file mode 100644
index eb92375..0000000
--- a/chrome/browser/performance_manager/system_resource_coordinator.cc
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2018 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/system_resource_coordinator.h"
-
-namespace performance_manager {
-
-SystemResourceCoordinator::SystemResourceCoordinator(
- PerformanceManager* performance_manager)
- : ResourceCoordinatorInterface() {
- resource_coordinator::CoordinationUnitID new_cu_id(
- resource_coordinator::CoordinationUnitType::kSystem,
- resource_coordinator::CoordinationUnitID::RANDOM_ID);
- ResourceCoordinatorInterface::ConnectToService(performance_manager,
- new_cu_id);
-}
-
-SystemResourceCoordinator::~SystemResourceCoordinator() = default;
-
-void SystemResourceCoordinator::DistributeMeasurementBatch(
- resource_coordinator::mojom::ProcessResourceMeasurementBatchPtr batch) {
- if (!service_)
- return;
- service_->DistributeMeasurementBatch(std::move(batch));
-}
-
-void SystemResourceCoordinator::ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) {
- provider->GetSystemCoordinationUnit(mojo::MakeRequest(&service_));
-}
-
-} // namespace performance_manager
diff --git a/chrome/browser/performance_manager/system_resource_coordinator.h b/chrome/browser/performance_manager/system_resource_coordinator.h
deleted file mode 100644
index 672ce8b..0000000
--- a/chrome/browser/performance_manager/system_resource_coordinator.h
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2018 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_SYSTEM_RESOURCE_COORDINATOR_H_
-#define CHROME_BROWSER_PERFORMANCE_MANAGER_SYSTEM_RESOURCE_COORDINATOR_H_
-
-#include "chrome/browser/performance_manager/resource_coordinator_interface.h"
-#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
-
-namespace performance_manager {
-
-class SystemResourceCoordinator
- : public ResourceCoordinatorInterface<
- resource_coordinator::mojom::SystemCoordinationUnitPtr,
- resource_coordinator::mojom::SystemCoordinationUnitRequest> {
- public:
- explicit SystemResourceCoordinator(PerformanceManager* performance_manager);
- ~SystemResourceCoordinator() override;
-
- void DistributeMeasurementBatch(
- resource_coordinator::mojom::ProcessResourceMeasurementBatchPtr batch);
-
- private:
- void ConnectToService(
- resource_coordinator::mojom::CoordinationUnitProviderPtr& provider,
- const resource_coordinator::CoordinationUnitID& cu_id) override;
-
- DISALLOW_COPY_AND_ASSIGN(SystemResourceCoordinator);
-};
-
-} // namespace performance_manager
-
-#endif // CHROME_BROWSER_PERFORMANCE_MANAGER_SYSTEM_RESOURCE_COORDINATOR_H_
diff --git a/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc b/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc
index 0205b75..d05f4764 100644
--- a/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc
+++ b/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc
@@ -10,7 +10,6 @@
#include "chrome/browser/performance_manager/browser_child_process_watcher.h"
#include "chrome/browser/performance_manager/performance_manager.h"
#include "chrome/browser/performance_manager/performance_manager_tab_helper.h"
-#include "chrome/browser/performance_manager/process_resource_coordinator.h"
#include "chrome/browser/performance_manager/render_process_user_data.h"
#include "chrome/browser/resource_coordinator/page_signal_receiver.h"
#include "chrome/browser/resource_coordinator/render_process_probe.h"
