Commit f710f276 authored by Sigurdur Asgeirsson's avatar Sigurdur Asgeirsson Committed by Commit Bot

RC: Ditch the legacy service_keepalive in PerformanceManager.

Bug: 910288
Change-Id: I1381260a1001c236e9d65d5fb28778436268e61a
Reviewed-on: https://chromium-review.googlesource.com/c/1483693Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635108}
parent cbfeebea
......@@ -11,11 +11,9 @@
namespace performance_manager {
FrameNodeImpl::FrameNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref)
: CoordinationUnitInterface(id, graph, std::move(keepalive_ref)),
FrameNodeImpl::FrameNodeImpl(const resource_coordinator::CoordinationUnitID& id,
Graph* graph)
: CoordinationUnitInterface(id, graph),
parent_frame_coordination_unit_(nullptr),
page_coordination_unit_(nullptr),
process_coordination_unit_(nullptr) {
......
......@@ -29,10 +29,8 @@ class FrameNodeImpl
return resource_coordinator::CoordinationUnitType::kFrame;
}
FrameNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref);
FrameNodeImpl(const resource_coordinator::CoordinationUnitID& id,
Graph* graph);
~FrameNodeImpl() override;
// FrameNode implementation.
......
......@@ -48,10 +48,9 @@ Graph::~Graph() {
}
void Graph::OnStart(service_manager::BinderRegistryWithArgs<
const service_manager::BindSourceInfo&>* registry,
service_manager::ServiceKeepalive* keepalive) {
const service_manager::BindSourceInfo&>* registry) {
// Create the singleton CoordinationUnitProvider.
provider_ = std::make_unique<GraphNodeProviderImpl>(keepalive, this);
provider_ = std::make_unique<GraphNodeProviderImpl>(this);
registry->AddInterface(base::BindRepeating(
&GraphNodeProviderImpl::Bind, base::Unretained(provider_.get())));
}
......@@ -75,32 +74,27 @@ void Graph::OnBeforeNodeDestroyed(NodeBase* coordination_unit) {
}
FrameNodeImpl* Graph::CreateFrameNode(
const resource_coordinator::CoordinationUnitID& id,
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref) {
return FrameNodeImpl::Create(id, this, std::move(service_ref));
const resource_coordinator::CoordinationUnitID& id) {
return FrameNodeImpl::Create(id, this);
}
PageNodeImpl* Graph::CreatePageNode(
const resource_coordinator::CoordinationUnitID& id,
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref) {
return PageNodeImpl::Create(id, this, std::move(service_ref));
const resource_coordinator::CoordinationUnitID& id) {
return PageNodeImpl::Create(id, this);
}
ProcessNodeImpl* Graph::CreateProcessNode(
const resource_coordinator::CoordinationUnitID& id,
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref) {
return ProcessNodeImpl::Create(id, this, std::move(service_ref));
const resource_coordinator::CoordinationUnitID& id) {
return ProcessNodeImpl::Create(id, this);
}
SystemNodeImpl* Graph::FindOrCreateSystemNode(
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref) {
SystemNodeImpl* Graph::FindOrCreateSystemNode() {
NodeBase* system_cu = GetNodeByID(system_coordination_unit_id_);
if (system_cu)
return SystemNodeImpl::FromNodeBase(system_cu);
// Create the singleton SystemCU instance. Ownership is taken by the graph.
return SystemNodeImpl::Create(system_coordination_unit_id_, this,
std::move(service_ref));
return SystemNodeImpl::Create(system_coordination_unit_id_, this);
}
NodeBase* Graph::GetNodeByID(
......
......@@ -17,7 +17,6 @@
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/resource_coordinator/public/cpp/coordination_unit_id.h"
#include "services/resource_coordinator/public/cpp/coordination_unit_types.h"
#include "services/service_manager/public/cpp/service_keepalive.h"
namespace service_manager {
template <typename... BinderArgs>
......@@ -50,23 +49,18 @@ class Graph {
ukm::UkmRecorder* ukm_recorder() const { return ukm_recorder_; }
void OnStart(service_manager::BinderRegistryWithArgs<
const service_manager::BindSourceInfo&>* registry,
service_manager::ServiceKeepalive* keepalive);
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,
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref);
const resource_coordinator::CoordinationUnitID& id);
PageNodeImpl* CreatePageNode(
const resource_coordinator::CoordinationUnitID& id,
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref);
const resource_coordinator::CoordinationUnitID& id);
ProcessNodeImpl* CreateProcessNode(
const resource_coordinator::CoordinationUnitID& id,
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref);
SystemNodeImpl* FindOrCreateSystemNode(
std::unique_ptr<service_manager::ServiceKeepaliveRef> service_ref);
const resource_coordinator::CoordinationUnitID& id);
SystemNodeImpl* FindOrCreateSystemNode();
std::vector<ProcessNodeImpl*> GetAllProcessNodes();
std::vector<FrameNodeImpl*> GetAllFrameNodes();
......@@ -105,7 +99,7 @@ class Graph {
ukm::UkmRecorder* ukm_recorder_ = nullptr;
std::unique_ptr<GraphNodeProviderImpl> provider_;
static void Create(service_manager::ServiceKeepalive* keepalive);
static void Create();
DISALLOW_COPY_AND_ASSIGN(Graph);
};
......
......@@ -17,14 +17,8 @@
namespace performance_manager {
GraphNodeProviderImpl::GraphNodeProviderImpl(
service_manager::ServiceKeepalive* service_keepalive,
Graph* coordination_unit_graph)
: service_keepalive_(service_keepalive),
coordination_unit_graph_(coordination_unit_graph) {
DCHECK(service_keepalive_);
keepalive_ref_ = service_keepalive_->CreateRef();
}
GraphNodeProviderImpl::GraphNodeProviderImpl(Graph* coordination_unit_graph)
: coordination_unit_graph_(coordination_unit_graph) {}
GraphNodeProviderImpl::~GraphNodeProviderImpl() = default;
......@@ -35,8 +29,7 @@ void GraphNodeProviderImpl::OnConnectionError(NodeBase* coordination_unit) {
void GraphNodeProviderImpl::CreateFrameCoordinationUnit(
resource_coordinator::mojom::FrameCoordinationUnitRequest request,
const resource_coordinator::CoordinationUnitID& id) {
FrameNodeImpl* frame_cu = coordination_unit_graph_->CreateFrameNode(
id, service_keepalive_->CreateRef());
FrameNodeImpl* frame_cu = coordination_unit_graph_->CreateFrameNode(id);
frame_cu->Bind(std::move(request));
auto& frame_cu_binding = frame_cu->binding();
......@@ -49,8 +42,7 @@ void GraphNodeProviderImpl::CreateFrameCoordinationUnit(
void GraphNodeProviderImpl::CreatePageCoordinationUnit(
resource_coordinator::mojom::PageCoordinationUnitRequest request,
const resource_coordinator::CoordinationUnitID& id) {
PageNodeImpl* page_cu = coordination_unit_graph_->CreatePageNode(
id, service_keepalive_->CreateRef());
PageNodeImpl* page_cu = coordination_unit_graph_->CreatePageNode(id);
page_cu->Bind(std::move(request));
auto& page_cu_binding = page_cu->binding();
......@@ -63,8 +55,7 @@ void GraphNodeProviderImpl::CreatePageCoordinationUnit(
void GraphNodeProviderImpl::CreateProcessCoordinationUnit(
resource_coordinator::mojom::ProcessCoordinationUnitRequest request,
const resource_coordinator::CoordinationUnitID& id) {
ProcessNodeImpl* process_cu = coordination_unit_graph_->CreateProcessNode(
id, service_keepalive_->CreateRef());
ProcessNodeImpl* process_cu = coordination_unit_graph_->CreateProcessNode(id);
process_cu->Bind(std::move(request));
auto& process_cu_binding = process_cu->binding();
......@@ -77,9 +68,8 @@ void GraphNodeProviderImpl::CreateProcessCoordinationUnit(
void GraphNodeProviderImpl::GetSystemCoordinationUnit(
resource_coordinator::mojom::SystemCoordinationUnitRequest request) {
// Simply fetch the existing SystemCU and add an additional binding to it.
coordination_unit_graph_
->FindOrCreateSystemNode(service_keepalive_->CreateRef())
->AddBinding(std::move(request));
coordination_unit_graph_->FindOrCreateSystemNode()->AddBinding(
std::move(request));
}
void GraphNodeProviderImpl::Bind(
......
......@@ -12,7 +12,6 @@
#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"
#include "services/service_manager/public/cpp/service_keepalive.h"
namespace service_manager {
struct BindSourceInfo;
......@@ -23,8 +22,7 @@ namespace performance_manager {
class GraphNodeProviderImpl
: public resource_coordinator::mojom::CoordinationUnitProvider {
public:
GraphNodeProviderImpl(service_manager::ServiceKeepalive* service_keepalive,
Graph* coordination_unit_graph);
explicit GraphNodeProviderImpl(Graph* coordination_unit_graph);
~GraphNodeProviderImpl() override;
void Bind(
......@@ -48,8 +46,6 @@ class GraphNodeProviderImpl
override;
private:
service_manager::ServiceKeepalive* const service_keepalive_;
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref_;
Graph* coordination_unit_graph_;
mojo::BindingSet<resource_coordinator::mojom::CoordinationUnitProvider>
bindings_;
......
......@@ -12,9 +12,7 @@ namespace performance_manager {
GraphTestHarness::GraphTestHarness()
: task_env_(base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME,
base::test::ScopedTaskEnvironment::ExecutionMode::QUEUED),
service_keepalive_(static_cast<service_manager::ServiceBinding*>(nullptr),
base::nullopt /* idle_timeout */),
provider_(&service_keepalive_, &coordination_unit_graph_) {}
provider_(&coordination_unit_graph_) {}
GraphTestHarness::~GraphTestHarness() = default;
......
......@@ -13,7 +13,6 @@
#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/system_node_impl.h"
#include "services/service_manager/public/cpp/service_keepalive.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace resource_coordinator {
......@@ -30,7 +29,7 @@ class TestNodeWrapper {
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, nullptr));
return TestNodeWrapper<NodeClass>(NodeClass::Create(cu_id, graph));
}
explicit TestNodeWrapper(NodeClass* impl) : impl_(impl) { DCHECK(impl); }
......@@ -63,8 +62,8 @@ class GraphTestHarness : public testing::Test {
template <class NodeClass>
TestNodeWrapper<NodeClass> CreateCoordinationUnit(
resource_coordinator::CoordinationUnitID cu_id) {
return TestNodeWrapper<NodeClass>(NodeClass::Create(
cu_id, coordination_unit_graph(), service_keepalive_.CreateRef()));
return TestNodeWrapper<NodeClass>(
NodeClass::Create(cu_id, coordination_unit_graph()));
}
template <class NodeClass>
......@@ -76,8 +75,7 @@ class GraphTestHarness : public testing::Test {
TestNodeWrapper<SystemNodeImpl> GetSystemCoordinationUnit() {
return TestNodeWrapper<SystemNodeImpl>(
coordination_unit_graph()->FindOrCreateSystemNode(
service_keepalive_.CreateRef()));
coordination_unit_graph()->FindOrCreateSystemNode());
}
// testing::Test:
......@@ -90,7 +88,6 @@ class GraphTestHarness : public testing::Test {
private:
base::test::ScopedTaskEnvironment task_env_;
service_manager::ServiceKeepalive service_keepalive_;
Graph coordination_unit_graph_;
GraphNodeProviderImpl provider_;
};
......
......@@ -14,11 +14,9 @@ namespace performance_manager {
namespace {
ProcessNodeImpl* CreateProcessNode(Graph* graph) {
return graph->CreateProcessNode(
resource_coordinator::CoordinationUnitID(
resource_coordinator::CoordinationUnitType::kProcess,
resource_coordinator::CoordinationUnitID::RANDOM_ID),
nullptr);
return graph->CreateProcessNode(resource_coordinator::CoordinationUnitID(
resource_coordinator::CoordinationUnitType::kProcess,
resource_coordinator::CoordinationUnitID::RANDOM_ID));
}
} // namespace
......@@ -33,7 +31,7 @@ TEST(GraphTest, DestructionWhileCUSOutstanding) {
process->SetPID(i + 100);
}
EXPECT_NE(nullptr, graph->FindOrCreateSystemNode(nullptr));
EXPECT_NE(nullptr, graph->FindOrCreateSystemNode());
// This should destroy all the CUs without incident.
graph.reset();
......@@ -42,15 +40,15 @@ TEST(GraphTest, DestructionWhileCUSOutstanding) {
TEST(GraphTest, FindOrCreateSystemNode) {
Graph graph;
SystemNodeImpl* system_cu = graph.FindOrCreateSystemNode(nullptr);
SystemNodeImpl* system_cu = graph.FindOrCreateSystemNode();
// A second request should return the same instance.
EXPECT_EQ(system_cu, graph.FindOrCreateSystemNode(nullptr));
EXPECT_EQ(system_cu, graph.FindOrCreateSystemNode());
// Destructing the system CU should be allowed.
system_cu->Destruct();
system_cu = graph.FindOrCreateSystemNode(nullptr);
system_cu = graph.FindOrCreateSystemNode();
EXPECT_NE(nullptr, system_cu);
}
......
......@@ -19,7 +19,6 @@
#include "services/resource_coordinator/public/cpp/coordination_unit_types.h"
#include "services/resource_coordinator/public/mojom/coordination_unit.mojom.h"
#include "services/resource_coordinator/public/mojom/coordination_unit_provider.mojom.h"
#include "services/service_manager/public/cpp/service_keepalive.h"
namespace performance_manager {
......@@ -90,11 +89,9 @@ class CoordinationUnitInterface : public NodeBase, public MojoInterfaceClass {
public:
static CoordinationUnitClass* Create(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref) {
Graph* graph) {
std::unique_ptr<CoordinationUnitClass> new_cu =
std::make_unique<CoordinationUnitClass>(id, graph,
std::move(keepalive_ref));
std::make_unique<CoordinationUnitClass>(id, graph);
return static_cast<CoordinationUnitClass*>(
PassOwnershipToGraph(std::move(new_cu)));
}
......@@ -109,14 +106,9 @@ class CoordinationUnitInterface : public NodeBase, public MojoInterfaceClass {
return static_cast<CoordinationUnitClass*>(cu);
}
CoordinationUnitInterface(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref)
: NodeBase(id, graph), binding_(this) {
keepalive_ref_ = std::move(keepalive_ref);
}
CoordinationUnitInterface(const resource_coordinator::CoordinationUnitID& id,
Graph* graph)
: NodeBase(id, graph), binding_(this) {}
~CoordinationUnitInterface() override = default;
......@@ -146,7 +138,6 @@ class CoordinationUnitInterface : public NodeBase, public MojoInterfaceClass {
private:
mojo::BindingSet<MojoInterfaceClass> bindings_;
mojo::Binding<MojoInterfaceClass> binding_;
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref_;
DISALLOW_COPY_AND_ASSIGN(CoordinationUnitInterface);
};
......
......@@ -27,11 +27,9 @@ size_t ToIndex(
} // namespace
PageNodeImpl::PageNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref)
: CoordinationUnitInterface(id, graph, std::move(keepalive_ref)) {
PageNodeImpl::PageNodeImpl(const resource_coordinator::CoordinationUnitID& id,
Graph* graph)
: CoordinationUnitInterface(id, graph) {
InvalidateAllInterventionPolicies();
}
......
......@@ -24,10 +24,8 @@ class PageNodeImpl
return resource_coordinator::CoordinationUnitType::kPage;
}
PageNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref);
PageNodeImpl(const resource_coordinator::CoordinationUnitID& id,
Graph* graph);
~PageNodeImpl() override;
// resource_coordinator::mojom::PageCoordinationUnit implementation.
......
......@@ -12,9 +12,8 @@ namespace performance_manager {
ProcessNodeImpl::ProcessNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref)
: CoordinationUnitInterface(id, graph, std::move(keepalive_ref)) {}
Graph* graph)
: CoordinationUnitInterface(id, graph) {}
ProcessNodeImpl::~ProcessNodeImpl() {
// Make as if we're transitioning to the null PID before we die to clear this
......
......@@ -37,10 +37,8 @@ class ProcessNodeImpl
return resource_coordinator::CoordinationUnitType::kProcess;
}
ProcessNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref);
ProcessNodeImpl(const resource_coordinator::CoordinationUnitID& id,
Graph* graph);
~ProcessNodeImpl() override;
// resource_coordinator::mojom::ProcessCoordinationUnit implementation.
......
......@@ -18,9 +18,8 @@ namespace performance_manager {
SystemNodeImpl::SystemNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref)
: CoordinationUnitInterface(id, graph, std::move(keepalive_ref)) {}
Graph* graph)
: CoordinationUnitInterface(id, graph) {}
SystemNodeImpl::~SystemNodeImpl() = default;
......
......@@ -21,10 +21,8 @@ class SystemNodeImpl
return resource_coordinator::CoordinationUnitType::kSystem;
}
SystemNodeImpl(
const resource_coordinator::CoordinationUnitID& id,
Graph* graph,
std::unique_ptr<service_manager::ServiceKeepaliveRef> keepalive_ref);
SystemNodeImpl(const resource_coordinator::CoordinationUnitID& id,
Graph* graph);
~SystemNodeImpl() override;
// resource_coordinator::mojom::SystemCoordinationUnit implementation:
......
......@@ -38,10 +38,7 @@ PerformanceManager* PerformanceManager::GetInstance() {
}
PerformanceManager::PerformanceManager()
: service_keepalive_(static_cast<service_manager::ServiceBinding*>(nullptr),
base::nullopt),
task_runner_(CreateTaskRunner()),
introspector_(&graph_) {
: task_runner_(CreateTaskRunner()), introspector_(&graph_) {
DETACH_FROM_SEQUENCE(sequence_checker_);
}
......@@ -127,7 +124,7 @@ void PerformanceManager::OnStartImpl(
graph_.set_ukm_recorder(ukm_recorder_.get());
}
graph_.OnStart(&interface_registry_, &service_keepalive_);
graph_.OnStart(&interface_registry_);
}
void PerformanceManager::BindInterfaceImpl(
......@@ -140,7 +137,7 @@ void PerformanceManager::BindInterfaceImpl(
void PerformanceManager::DistributeMeasurementBatchImpl(
resource_coordinator::mojom::ProcessResourceMeasurementBatchPtr batch) {
SystemNodeImpl* system_node = graph_.FindOrCreateSystemNode(nullptr);
SystemNodeImpl* system_node = graph_.FindOrCreateSystemNode();
DCHECK(system_node);
system_node->DistributeMeasurementBatch(std::move(batch));
......
......@@ -20,7 +20,6 @@
#include "services/service_manager/public/cpp/bind_source_info.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/connector.h"
#include "services/service_manager/public/cpp/service_keepalive.h"
namespace ukm {
class MojoUkmRecorder;
......@@ -81,9 +80,6 @@ class PerformanceManager {
const service_manager::BindSourceInfo& source_info);
void OnGraphDumpConnectionError(WebUIGraphDumpImpl* graph_dump);
// TODO(siggi): Remove this as it's only here to maintain compatibility
// with the current interface of the CoordinationUnits.
service_manager::ServiceKeepalive service_keepalive_;
InterfaceRegistry interface_registry_;
// The performance task runner.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment