Remove ResourceCoordinatorService.

This class added an unnecessary layer of complexity to access
memory instrumentation functionality.

Follow-up: Rename //services/resource_coordinator/ to
//services/memory_instrumentation and remove "memory_instrumentation"
subdirectories.

Change-Id: I556423d2004ab0d2b2462ae5b0cac529712d50de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593829
Reviewed-by: Kinuko Yasuda <[email protected]>
Reviewed-by: Erik Chen <[email protected]>
Reviewed-by: ssid <[email protected]>
Commit-Queue: Erik Chen <[email protected]>
Auto-Submit: François Doray <[email protected]>
Cr-Commit-Position: refs/heads/master@{#840316}

components/heap_profiling/multi_process/supervisor.cc

@@ -86,7 +86,7 @@ void Supervisor::StartProfilingOnMemoryInfraThread(Mode mode,
   mojo::PendingRemote<memory_instrumentation::mojom::HeapProfilerHelper> helper;
   mojo::PendingRemote<memory_instrumentation::mojom::HeapProfiler> profiler;
   auto profiler_receiver = profiler.InitWithNewPipeAndPassReceiver();
-  content::GetResourceCoordinatorService()->RegisterHeapProfiler(
+  content::GetMemoryInstrumentationRegistry()->RegisterHeapProfiler(
       std::move(profiler), helper.InitWithNewPipeAndPassReceiver());
   content::GetIOThreadTaskRunner({})->PostTask(
       FROM_HERE,

content/browser/browser_child_process_host_impl.cc

@@ -680,10 +680,9 @@ void BrowserChildProcessHostImpl::RegisterCoordinatorClient(
                  memory_instrumentation::mojom::ProcessType process_type,
                  base::ProcessId process_id,
                  base::Optional<std::string> service_name) {
-                GetMemoryInstrumentationCoordinatorController()
-                    ->RegisterClientProcess(
-                        std::move(receiver), std::move(client_process),
-                        process_type, process_id, std::move(service_name));
+                GetMemoryInstrumentationRegistry()->RegisterClientProcess(
+                    std::move(receiver), std::move(client_process),
+                    process_type, process_id, std::move(service_name));
               },
               std::move(receiver), std::move(client_process),
               GetCoordinatorClientProcessType(

content/browser/renderer_host/render_process_host_impl.cc

@@ -2597,12 +2597,10 @@ void RenderProcessHostImpl::RegisterCoordinatorClient(
                      memory_instrumentation::mojom::ClientProcess>
                      client_process,
                  base::ProcessId pid) {
-                GetMemoryInstrumentationCoordinatorController()
-                    ->RegisterClientProcess(
-                        std::move(receiver), std::move(client_process),
-                        memory_instrumentation::mojom::ProcessType::RENDERER,
-                        pid,
-                        /*service_name=*/base::nullopt);
+                GetMemoryInstrumentationRegistry()->RegisterClientProcess(
+                    std::move(receiver), std::move(client_process),
+                    memory_instrumentation::mojom::ProcessType::RENDERER, pid,
+                    /*service_name=*/base::nullopt);
               },
               std::move(receiver), std::move(client_process),
               GetProcess().Pid()));

content/browser/resource_coordinator_service.cc

@@ -4,42 +4,19 @@
 
 #include "content/public/browser/resource_coordinator_service.h"
 
-#include "base/no_destructor.h"
 #include "base/trace_event/memory_dump_manager.h"
-#include "content/public/browser/browser_thread.h"
-#include "services/resource_coordinator/public/mojom/resource_coordinator_service.mojom.h"
-#include "services/resource_coordinator/resource_coordinator_service.h"
+#include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/registry.h"
 
 namespace content {
 
-resource_coordinator::mojom::ResourceCoordinatorService*
-GetResourceCoordinatorService() {
+memory_instrumentation::Registry* GetMemoryInstrumentationRegistry() {
   DCHECK(base::trace_event::MemoryDumpManager::GetInstance()
              ->GetDumpThreadTaskRunner()
              ->RunsTasksInCurrentSequence());
-  static base::NoDestructor<
-      mojo::Remote<resource_coordinator::mojom::ResourceCoordinatorService>>
-      remote;
-  static base::NoDestructor<resource_coordinator::ResourceCoordinatorService>
-      service(remote->BindNewPipeAndPassReceiver());
-  return remote->get();
-}
-
-memory_instrumentation::mojom::CoordinatorController*
-GetMemoryInstrumentationCoordinatorController() {
-  DCHECK(base::trace_event::MemoryDumpManager::GetInstance()
-             ->GetDumpThreadTaskRunner()
-             ->RunsTasksInCurrentSequence());
-  static base::NoDestructor<
-      mojo::Remote<memory_instrumentation::mojom::CoordinatorController>>
-      controller([] {
-        mojo::Remote<memory_instrumentation::mojom::CoordinatorController> c;
-        GetResourceCoordinatorService()
-            ->BindMemoryInstrumentationCoordinatorController(
-                c.BindNewPipeAndPassReceiver());
-        return c;
-      }());
-  return controller->get();
+  static memory_instrumentation::Registry* registry =
+      new memory_instrumentation::CoordinatorImpl();
+  return registry;
 }
 
 }  // namespace content

content/browser/tracing/memory_instrumentation_util.cc

@@ -24,7 +24,7 @@ void InitializeBrowserMemoryInstrumentationClient() {
   mojo::PendingRemote<memory_instrumentation::mojom::Coordinator> coordinator;
   mojo::PendingRemote<memory_instrumentation::mojom::ClientProcess> process;
   auto process_receiver = process.InitWithNewPipeAndPassReceiver();
-  GetMemoryInstrumentationCoordinatorController()->RegisterClientProcess(
+  GetMemoryInstrumentationRegistry()->RegisterClientProcess(
       coordinator.InitWithNewPipeAndPassReceiver(), std::move(process),
       memory_instrumentation::mojom::ProcessType::BROWSER,
       base::GetCurrentProcId(), /*service_name=*/base::nullopt);

content/public/browser/resource_coordinator_service.h

@@ -6,19 +6,12 @@
 #define CONTENT_PUBLIC_BROWSER_RESOURCE_COORDINATOR_SERVICE_H_
 
 #include "content/common/content_export.h"
-#include "services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom.h"
-#include "services/resource_coordinator/public/mojom/resource_coordinator_service.mojom.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/registry.h"
 
 namespace content {
 
-// Gets the browser's connection to the in-process Resource Coordinator service.
-CONTENT_EXPORT resource_coordinator::mojom::ResourceCoordinatorService*
-GetResourceCoordinatorService();
-
-// Gets the browser's connection to the Resource Coordinator's
-// memory instrumentation CoordinatorController.
-CONTENT_EXPORT memory_instrumentation::mojom::CoordinatorController*
-GetMemoryInstrumentationCoordinatorController();
+CONTENT_EXPORT memory_instrumentation::Registry*
+GetMemoryInstrumentationRegistry();
 
 }  // namespace content
 

services/resource_coordinator/BUILD.gn

@@ -25,8 +25,6 @@ source_set("lib") {
     "memory_instrumentation/queued_request_dispatcher.h",
     "memory_instrumentation/switches.cc",
     "memory_instrumentation/switches.h",
-    "resource_coordinator_service.cc",
-    "resource_coordinator_service.h",
   ]
 
   configs += [ "//build/config/compiler:wexit_time_destructors" ]

services/resource_coordinator/memory_instrumentation/coordinator_impl.cc

@@ -89,11 +89,6 @@ CoordinatorImpl* CoordinatorImpl::GetInstance() {
   return g_coordinator_impl;
 }
 
-void CoordinatorImpl::BindController(
-    mojo::PendingReceiver<mojom::CoordinatorController> receiver) {
-  controller_receiver_.Bind(std::move(receiver));
-}
-
 void CoordinatorImpl::RegisterHeapProfiler(
     mojo::PendingRemote<mojom::HeapProfiler> profiler,
     mojo::PendingReceiver<mojom::HeapProfilerHelper> helper_receiver) {

services/resource_coordinator/memory_instrumentation/coordinator_impl.h

@@ -19,6 +19,7 @@
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "services/resource_coordinator/memory_instrumentation/queued_request.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/registry.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.h"
 #include "services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom.h"
 
@@ -30,7 +31,7 @@ namespace memory_instrumentation {
 // - Provides global (i.e. for all processes) memory snapshots on demand.
 //   Global snapshots are obtained by requesting in-process snapshots from each
 //   registered client and aggregating them.
-class CoordinatorImpl : public mojom::CoordinatorController,
+class CoordinatorImpl : public Registry,
                         public mojom::Coordinator,
                         public mojom::HeapProfilerHelper {
  public:
@@ -40,14 +41,10 @@ class CoordinatorImpl : public mojom::CoordinatorController,
   // The getter of the unique instance.
   static CoordinatorImpl* GetInstance();
 
-  void BindController(
-      mojo::PendingReceiver<mojom::CoordinatorController> receiver);
-
-  void RegisterHeapProfiler(
-      mojo::PendingRemote<mojom::HeapProfiler> profiler,
-      mojo::PendingReceiver<mojom::HeapProfilerHelper> helper_receiver);
-
-  // mojom::CoordinatorController implementation.
+  // Registry:
+  void RegisterHeapProfiler(mojo::PendingRemote<mojom::HeapProfiler> profiler,
+                            mojo::PendingReceiver<mojom::HeapProfilerHelper>
+                                helper_receiver) override;
   void RegisterClientProcess(
       mojo::PendingReceiver<mojom::Coordinator> receiver,
       mojo::PendingRemote<mojom::ClientProcess> client_process,
@@ -171,9 +168,6 @@ class CoordinatorImpl : public mojom::CoordinatorController,
   std::map<uint64_t, std::unique_ptr<QueuedVmRegionRequest>>
       in_progress_vm_region_requests_;
 
-  // Receives control messages from the single privileged client of this object.
-  mojo::Receiver<mojom::CoordinatorController> controller_receiver_{this};
-
   // There may be extant callbacks in |queued_memory_dump_requests_|. These
   // receivers must be closed before destroying the un-run callbacks.
   mojo::ReceiverSet<mojom::Coordinator, base::ProcessId> coordinator_receivers_;

services/resource_coordinator/public/cpp/memory_instrumentation/BUILD.gn

@@ -12,6 +12,7 @@ component("memory_instrumentation") {
     "memory_instrumentation.h",
     "os_metrics.cc",
     "os_metrics.h",
+    "registry.h",
     "tracing_observer.cc",
     "tracing_observer.h",
     "tracing_observer_proto.cc",

services/resource_coordinator/public/cpp/memory_instrumentation/registry.h

@@ -0,0 +1,44 @@
+// Copyright 2020 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 SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_INSTRUMENTATION_REGISTRY_H_
+#define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_INSTRUMENTATION_REGISTRY_H_
+
+#include <string>
+
+#include "base/component_export.h"
+#include "base/optional.h"
+#include "base/process/process_handle.h"
+#include "services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom.h"
+
+namespace memory_instrumentation {
+
+// Interface to register client processes and heap profilers with the memory
+// instrumentation coordinator. This is considered privileged and the browser
+// should be the only client.
+class COMPONENT_EXPORT(
+    RESOURCE_COORDINATOR_PUBLIC_MEMORY_INSTRUMENTATION) Registry {
+ public:
+  virtual ~Registry() = default;
+
+  virtual void RegisterHeapProfiler(
+      mojo::PendingRemote<mojom::HeapProfiler> profiler,
+      mojo::PendingReceiver<mojom::HeapProfilerHelper> helper_receiver) = 0;
+
+  // Must be called once for each client process, including the browser process.
+  // |client_process| is an endpoint the service can use to push client events
+  // to the process. |process_type|, |process_id| and |service_name| are
+  // considered to be authoritative information about the client process
+  // (verified by the browser process).
+  virtual void RegisterClientProcess(
+      mojo::PendingReceiver<mojom::Coordinator> receiver,
+      mojo::PendingRemote<mojom::ClientProcess> client_process,
+      mojom::ProcessType process_type,
+      base::ProcessId process_id,
+      const base::Optional<std::string>& service_name) = 0;
+};
+
+}  // namespace memory_instrumentation
+
+#endif  // SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_INSTRUMENTATION_REGISTRY_H_

services/resource_coordinator/public/mojom/BUILD.gn

@@ -11,7 +11,6 @@ mojom_component("mojom") {
   sources = [
     "memory_instrumentation/constants.mojom",
     "memory_instrumentation/memory_instrumentation.mojom",
-    "resource_coordinator_service.mojom",
   ]
 
   public_deps = [

services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom

@@ -334,28 +334,12 @@ interface Coordinator {
       (bool success, uint64 dump_id);
 };
 
-// An interface which can be used by client processes to register themselves
-// with a CoordinatorController client (like the browser), who can then pass the
-// registration along to the service with authoritative information about the
-// process's identity.
+// An interface used by client processes to register themselves with a
+// memory_instrumentation::Registry. The implementation must already know the
+// calling process. It adds authoritative information about the calling process'
+// identity when forwarding the registration to a
+// memory_instrumentation::Registry.
 interface CoordinatorConnector {
-  // Registers the calling process. The implementation of this API must already
-  // know who the calling process is and should forward these arguments along to
-  // the CoordinatorController along with information about the process.
   RegisterCoordinatorClient(pending_receiver<Coordinator> receiver,
                             pending_remote<ClientProcess> client_process);
 };
-
-// Main interface to the memory instrumentation coordinator. This is considered
-// privileged and the browser should effectively be the only client.
-interface CoordinatorController {
-  // Binds a new Coordinator interface endpoint for a unique client process.
-  // Should be called once for each process, including the browser itself.
-  // |client_process| is an endpoint the service can use to push client events
-  // to the process.
-  RegisterClientProcess(pending_receiver<Coordinator> receiver,
-                        pending_remote<ClientProcess> client_process,
-                        ProcessType process_type,
-                        mojo_base.mojom.ProcessId process_id,
-                        string? service_name);
-};