Global Resource Coordinator: Basic service internals

services/resource_coordinator/public/cpp/resource_coordinator_interface.cc

Index: services/resource_coordinator/public/cpp/resource_coordinator_interface.cc
diff --git a/services/resource_coordinator/public/cpp/resource_coordinator_interface.cc b/services/resource_coordinator/public/cpp/resource_coordinator_interface.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d681a6a1da52215132db6f027781f5334e7e4dae
--- /dev/null
+++ b/services/resource_coordinator/public/cpp/resource_coordinator_interface.cc
@@ -0,0 +1,74 @@
+// 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 "services/resource_coordinator/public/cpp/resource_coordinator_interface.h"
+
+#include "base/process/process_handle.h"
+#include "base/strings/string_number_conversions.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom.h"
+#include "services/resource_coordinator/public/interfaces/service_constants.mojom.h"
+#include "services/service_manager/public/cpp/connector.h"
+
+namespace {
+
+void OnConnectionError() {
+  CHECK(false);
+}
+
+}  // namespace
+
+namespace resource_coordinator {
+
+ResourceCoordinatorInterface::ResourceCoordinatorInterface(
+    service_manager::Connector* connector,
+    const CoordinationUnitType& type)
+    : weak_ptr_factory_(this) {
+  static int next_id = 0;

[Primiano Tucci (use gerrit), 2017/04/06 18:09:47]

you might be bitten by this in component builds similar to Ehsan.

resource_coordinator_interface.cc is part of the cpp source_set. If that
source_set gets included by different target, each target will have a local
next_id instance, so you will get duplicated ids.

On top of this, don't you ned this ID to be generated in a thread safe way?
In other words, can a ResourceCoordinatorInterface be created by multiple
threads at the same time? If so  there is StaticAtomicSequenceNumber  in base
for this.

[oystein (OOO til 10th of July), 2017/04/10 20:03:00]

Yikes. Done.

+  std::string id = base::IntToString(base::GetCurrentProcId()) + "." +
+                   base::IntToString(++next_id);
+  ConnectToService(connector, type, id);
+}
+
+ResourceCoordinatorInterface::~ResourceCoordinatorInterface() = default;
+
+void ResourceCoordinatorInterface::ConnectToService(
+    service_manager::Connector* connector,
+    const CoordinationUnitType& type,
+    const std::string& id) {
+  DCHECK(connector);
+  mojom::CoordinationUnitProviderPtr provider;
+  connector->BindInterface(mojom::kServiceName, mojo::MakeRequest(&provider));
+
+  CoordinationUnitID new_cu_id(type, id);
+
+  provider->CreateCoordinationUnit(mojo::MakeRequest(&service_), new_cu_id);
+
+  service_.set_connection_error_handler(base::Bind(&OnConnectionError));
+}
+
+void ResourceCoordinatorInterface::SendEvent(
+    const resource_coordinator::EventType& event_type) {
+  DCHECK(service_);

[Primiano Tucci (use gerrit), 2017/04/06 18:09:47]

I think the patttern in chrome is to not DCHECK for nullpointers if you
dereference the nullptr in the same scope, because that will fail anyways on
line 57.

[oystein (OOO til 10th of July), 2017/04/10 20:03:00]

Done.

+  mojom::EventPtr event = mojom::Event::New();
+  event->type = event_type;
+
+  service_->SendEvent(std::move(event));
+}
+
+void ResourceCoordinatorInterface::AddChild(
+    const ResourceCoordinatorInterface& child) {
+  DCHECK(service_);
+  // 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::Bind(&ResourceCoordinatorInterface::AddChildByID,
+                                    weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ResourceCoordinatorInterface::AddChildByID(
+    const CoordinationUnitID& child_id) {
+  service_->AddChild(child_id);
+}
+
+}  // namespace resource_coordinator