move memory_infra and tracing to the GRC service

memory_instrumentation::Coordinator is exposed by the browser service
right now, which was a temporary solution before the
resource_coordinator service was created. This CL moves it to the
resource_coordinator service.

Also, this CL exposes the tracing service, too. Later it will be used
instead of the tracing controller that lives in the browser.

Moving memory instrumentation and tracing to the resource_coordinator
service will let the service manager to register all "service"
processes to report memory usage and trace data. This is not possible
if memory instrumentation and tracing live in content_browser since it
will be a layering violation to have a dependency from
//services/service_manager to content.

BUG=687020,640235,733165

Change-Id: I7ea9e61430f4406fd115abd747b35a806a392469
Reviewed-on: https://chromium-review.googlesource.com/628658Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501668}

chrome/browser/metrics/process_memory_metrics_emitter.cc

@@ -142,7 +142,7 @@ void ProcessMemoryMetricsEmitter::FetchAndEmitProcessMemoryMetrics() {
 
   service_manager::Connector* connector =
       content::ServiceManagerConnection::GetForProcess()->GetConnector();
-  connector->BindInterface(content::mojom::kBrowserServiceName,
+  connector->BindInterface(resource_coordinator::mojom::kServiceName,
                            mojo::MakeRequest(&coordinator_));
 
   // The callback keeps this object alive until the callback is invoked..

content/browser/browser_main_loop.cc

@@ -115,8 +115,9 @@
 #include "net/socket/client_socket_factory.h"
 #include "net/ssl/ssl_config_service.h"
 #include "ppapi/features/features.h"
-#include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.h"
+#include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
+#include "services/resource_coordinator/public/interfaces/service_constants.mojom.h"
 #include "services/service_manager/runner/common/client_util.h"
 #include "skia/ext/event_tracer_impl.h"
 #include "skia/ext/skia_memory_dump_provider.h"
@@ -1430,19 +1431,12 @@ int BrowserMainLoop::BrowserThreadsStarted() {
   // so this cannot happen any earlier than now.
   InitializeMojo();
 
-  // Create the memory instrumentation service. It will initialize the memory
-  // dump manager, too. It makes sense that BrowserMainLoop owns the service;
-  // this way, the service is alive for the lifetime of Mojo. Mojo is shutdown
-  // in BrowserMainLoop::ShutdownThreadsAndCleanupIO.
-  service_manager::Connector* connector =
-      content::ServiceManagerConnection::GetForProcess()->GetConnector();
-  memory_instrumentation_coordinator_ =
-      base::MakeUnique<memory_instrumentation::CoordinatorImpl>(connector);
-
   // Registers the browser process as a memory-instrumentation client, so
   // that data for the browser process will be available in memory dumps.
+  service_manager::Connector* connector =
+      content::ServiceManagerConnection::GetForProcess()->GetConnector();
   memory_instrumentation::ClientProcessImpl::Config config(
-      connector, mojom::kBrowserServiceName,
+      connector, resource_coordinator::mojom::kServiceName,
       memory_instrumentation::mojom::ProcessType::BROWSER);
   memory_instrumentation::ClientProcessImpl::CreateInstance(config);
 

content/browser/browser_main_loop.h

@@ -58,10 +58,6 @@ class DeviceMonitorMac;
 #endif
 }  // namespace media
 
-namespace memory_instrumentation {
-class CoordinatorImpl;
-}  // memory_instrumentation
-
 namespace midi {
 class MidiService;
 }  // namespace midi
@@ -356,8 +352,6 @@ class CONTENT_EXPORT BrowserMainLoop {
   std::unique_ptr<discardable_memory::DiscardableSharedMemoryManager>
       discardable_shared_memory_manager_;
   scoped_refptr<SaveFileManager> save_file_manager_;
-  std::unique_ptr<memory_instrumentation::CoordinatorImpl>
-      memory_instrumentation_coordinator_;
 #if !defined(OS_ANDROID)
   std::unique_ptr<viz::HostFrameSinkManager> host_frame_sink_manager_;
   // This is owned here so that SurfaceManager will be accessible in process

content/browser/service_manager/common_browser_interfaces.cc

@@ -17,27 +17,17 @@
 #include "content/public/common/service_manager_connection.h"
 #include "device/geolocation/geolocation_config.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
-#include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 
 namespace content {
 
 namespace {
 
-void BindMemoryCoordinatorRequest(
-    memory_instrumentation::mojom::CoordinatorRequest request,
-    const service_manager::BindSourceInfo& source_info) {
-  auto* coordinator = memory_instrumentation::CoordinatorImpl::GetInstance();
-  if (coordinator)
-    coordinator->BindCoordinatorRequest(std::move(request), source_info);
-}
-
 class ConnectionFilterImpl : public ConnectionFilter {
  public:
   ConnectionFilterImpl()
       : main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
     RegisterMainThreadInterface(base::Bind(&device::GeolocationConfig::Create));
-    RegisterMainThreadInterface(base::Bind(&BindMemoryCoordinatorRequest));
 
     auto* browser_main_loop = BrowserMainLoop::GetInstance();
     if (browser_main_loop) {

content/child/child_thread_impl.cc

@@ -72,6 +72,7 @@
 #include "services/device/public/cpp/power_monitor/power_monitor_broadcast_source.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.h"
 #include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
+#include "services/resource_coordinator/public/interfaces/service_constants.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "services/service_manager/runner/common/client_util.h"
@@ -525,7 +526,8 @@ void ChildThreadImpl::Init(const Options& options) {
         process_type = memory_instrumentation::mojom::ProcessType::PLUGIN;
 
       memory_instrumentation::ClientProcessImpl::Config config(
-          GetConnector(), mojom::kBrowserServiceName, process_type);
+          GetConnector(), resource_coordinator::mojom::kServiceName,
+          process_type);
       memory_instrumentation::ClientProcessImpl::CreateInstance(config);
     }
   }

content/public/app/mojo/content_packaged_services_manifest.json

@@ -16,6 +16,7 @@
         ]
       },
       "requires": {
+        "*": [ "app" ],
         "content_browser": [],
         "service_manager": [
           "service_manager:all_users",

content/public/app/mojo/content_renderer_manifest.json

@@ -50,7 +50,7 @@
         ]
       },
       "requires": {
-        "content_browser": [ "memory_instrumentation", "renderer" ]
+        "content_browser": [ "renderer" ]
       }
     }
   },

services/resource_coordinator/BUILD.gn

@@ -49,6 +49,7 @@ source_set("lib") {
     "//mojo/public/cpp/bindings",
     "//services/metrics/public/cpp:ukm_builders",
     "//services/resource_coordinator/public/cpp:resource_coordinator_cpp",
+    "//services/resource_coordinator/tracing:lib",
     "//services/service_manager/public/cpp/standalone_service:standalone_service",
   ]
 }

services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.cc

@@ -9,6 +9,7 @@
 #include "base/process/process_handle.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_impl.h"
 #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h"
+#include "services/service_manager/public/cpp/bind_source_info.h"
 
 namespace resource_coordinator {
 
@@ -48,7 +49,8 @@ void CoordinationUnitIntrospectorImpl::GetProcessToURLMap(
 }
 
 void CoordinationUnitIntrospectorImpl::BindToInterface(
-    resource_coordinator::mojom::CoordinationUnitIntrospectorRequest request) {
+    resource_coordinator::mojom::CoordinationUnitIntrospectorRequest request,
+    const service_manager::BindSourceInfo& source_info) {
   bindings_.AddBinding(this, std::move(request));
 }
 

services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.h

@@ -9,6 +9,10 @@
 #include "services/resource_coordinator/public/interfaces/coordination_unit_introspector.mojom.h"
 #include "services/service_manager/public/cpp/bind_source_info.h"
 
+namespace service_manager {
+struct BindSourceInfo;
+}  // namespace service_manager
+
 namespace resource_coordinator {
 
 class CoordinationUnitIntrospectorImpl
@@ -18,7 +22,8 @@ class CoordinationUnitIntrospectorImpl
   ~CoordinationUnitIntrospectorImpl() override;
 
   void BindToInterface(
-      resource_coordinator::mojom::CoordinationUnitIntrospectorRequest request);
+      resource_coordinator::mojom::CoordinationUnitIntrospectorRequest request,
+      const service_manager::BindSourceInfo& source_info);
 
   // Overridden from mojom::CoordinationUnitIntrospector:
   void GetProcessToURLMap(const GetProcessToURLMapCallback& callback) override;

services/resource_coordinator/coordination_unit/coordination_unit_manager.cc

@@ -13,6 +13,7 @@
 #include "services/resource_coordinator/coordination_unit/coordination_unit_impl.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.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 {
@@ -33,7 +34,8 @@ CoordinationUnitManager::~CoordinationUnitManager() {
 }
 
 void CoordinationUnitManager::OnStart(
-    service_manager::BinderRegistry* registry,
+    service_manager::BinderRegistryWithArgs<
+        const service_manager::BindSourceInfo&>* registry,
     service_manager::ServiceContextRefFactory* service_ref_factory) {
   provider_ =
       base::MakeUnique<CoordinationUnitProviderImpl>(service_ref_factory, this);

services/resource_coordinator/coordination_unit/coordination_unit_manager.h

@@ -13,9 +13,11 @@
 #include "base/macros.h"
 #include "services/metrics/public/cpp/mojo_ukm_recorder.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
 
 namespace service_manager {
+template <typename... BinderArgs>
+class BinderRegistryWithArgs;
+struct BindSourceInfo;
 class ServiceContextRefFactory;
 }  // service_manager
 
@@ -39,7 +41,8 @@ class CoordinationUnitManager {
   }
   ukm::MojoUkmRecorder* ukm_recorder() const { return ukm_recorder_; }
 
-  void OnStart(service_manager::BinderRegistry* registry,
+  void OnStart(service_manager::BinderRegistryWithArgs<
+                   const service_manager::BindSourceInfo&>* registry,
                service_manager::ServiceContextRefFactory* service_ref_factory);
   void RegisterObserver(
       std::unique_ptr<CoordinationUnitGraphObserver> observer);

services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.cc

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_graph_observer.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_impl.h"
+#include "services/service_manager/public/cpp/bind_source_info.h"
 #include "services/service_manager/public/cpp/service_context_ref.h"
 
 namespace resource_coordinator {
@@ -49,7 +50,8 @@ void CoordinationUnitProviderImpl::CreateCoordinationUnit(
 }
 
 void CoordinationUnitProviderImpl::Bind(
-    resource_coordinator::mojom::CoordinationUnitProviderRequest request) {
+    resource_coordinator::mojom::CoordinationUnitProviderRequest request,
+    const service_manager::BindSourceInfo& source_info) {
   bindings_.AddBinding(this, std::move(request));
 }
 

services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h

@@ -14,6 +14,7 @@
 #include "services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom.h"
 
 namespace service_manager {
+struct BindSourceInfo;
 class ServiceContextRefFactory;
 class ServiceContextRef;
 }  // service_manager
@@ -28,7 +29,8 @@ class CoordinationUnitProviderImpl : public mojom::CoordinationUnitProvider {
   ~CoordinationUnitProviderImpl() override;
 
   void Bind(
-      resource_coordinator::mojom::CoordinationUnitProviderRequest request);
+      resource_coordinator::mojom::CoordinationUnitProviderRequest request,
+      const service_manager::BindSourceInfo& source_info);
 
   void OnConnectionError(CoordinationUnitImpl* coordination_unit);
 

services/resource_coordinator/coordination_unit/tab_signal_generator_impl.cc

@@ -10,6 +10,7 @@
 #include "services/resource_coordinator/coordination_unit/coordination_unit_impl.h"
 #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h"
 #include "services/resource_coordinator/coordination_unit/web_contents_coordination_unit_impl.h"
+#include "services/service_manager/public/cpp/bind_source_info.h"
 
 namespace resource_coordinator {
 
@@ -63,7 +64,8 @@ void TabSignalGeneratorImpl::OnWebContentsPropertyChanged(
 }
 
 void TabSignalGeneratorImpl::BindToInterface(
-    resource_coordinator::mojom::TabSignalGeneratorRequest request) {
+    resource_coordinator::mojom::TabSignalGeneratorRequest request,
+    const service_manager::BindSourceInfo& source_info) {
   bindings_.AddBinding(this, std::move(request));
 }
 

services/resource_coordinator/coordination_unit/tab_signal_generator_impl.h

@@ -11,6 +11,10 @@
 #include "services/resource_coordinator/coordination_unit/coordination_unit_graph_observer.h"
 #include "services/resource_coordinator/public/interfaces/tab_signal.mojom.h"
 
+namespace service_manager {
+struct BindSourceInfo;
+}  // namespace service_manager
+
 namespace resource_coordinator {
 
 class CoordinationUnitImpl;
@@ -41,7 +45,8 @@ class TabSignalGeneratorImpl : public CoordinationUnitGraphObserver,
       int64_t value) override;
 
   void BindToInterface(
-      resource_coordinator::mojom::TabSignalGeneratorRequest request);
+      resource_coordinator::mojom::TabSignalGeneratorRequest request,
+      const service_manager::BindSourceInfo& source_info);
 
  private:
   mojo::BindingSet<mojom::TabSignalGenerator> bindings_;

services/resource_coordinator/manifest.json

@@ -4,16 +4,24 @@
   "interface_provider_specs": {
     "service_manager:connector": {
       "provides": {
+        "app": [
+          "memory_instrumentation::mojom::Coordinator",
+          "tracing::mojom::AgentRegistry"
+        ],
         "coordination_unit_introspector": [
           "resource_coordinator::mojom::CoordinationUnitIntrospector"
         ],
         "coordination_unit": [ "resource_coordinator::mojom::CoordinationUnitProvider" ],
         "service_callbacks": [ "resource_coordinator::mojom::ServiceCallbacks" ],
         "tab_signal": [ "resource_coordinator::mojom::TabSignalGenerator" ],
+        "tracing": [ "tracing::mojom::Coordinator" ],
         "tests": [ "*" ]
       },
       "requires": {
-        "service_manager": [ "service_manager:all_users" ]
+        "service_manager": [
+          "service_manager:all_users",
+          "service_manager:service_manager"
+        ]
       }
     }
   }

services/resource_coordinator/memory_instrumentation/coordinator_impl.cc

@@ -103,12 +103,7 @@ CoordinatorImpl::~CoordinatorImpl() {
 
 base::ProcessId CoordinatorImpl::GetProcessIdForClientIdentity(
     service_manager::Identity identity) const {
-  // TODO(primiano): the browser process registers bypassing mojo and ends up
-  // with an invalid identity. Fix that (crbug.com/733165) and remove the
-  // special case in the else branch below.
-  if (!identity.IsValid()) {
-    return base::GetCurrentProcId();
-  }
+  DCHECK(identity.IsValid());
   return process_map_->GetProcessId(identity);
 }
 

services/resource_coordinator/memory_instrumentation/coordinator_impl.h

@@ -20,8 +20,10 @@
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/coordinator.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.h"
 #include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
+#include "services/service_manager/public/cpp/identity.h"
 
 namespace service_manager {
+struct BindSourceInfo;
 class Connector;
 }
 

services/resource_coordinator/resource_coordinator_service.cc

@@ -9,7 +9,10 @@
 #include "base/memory/ptr_util.h"
 #include "services/resource_coordinator/coordination_unit/metrics_collector.h"
 #include "services/resource_coordinator/coordination_unit/tab_signal_generator_impl.h"
+#include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
 #include "services/resource_coordinator/service_callbacks_impl.h"
+#include "services/resource_coordinator/tracing/agent_registry.h"
+#include "services/resource_coordinator/tracing/coordinator.h"
 #include "services/service_manager/public/cpp/service_context.h"
 
 namespace resource_coordinator {
@@ -53,13 +56,36 @@ void ResourceCoordinatorService::OnStart() {
       base::MakeUnique<MetricsCollector>());
 
   coordination_unit_manager_.OnStart(&registry_, ref_factory_.get());
+
+  // TODO(chiniforooshan): The abstract class Coordinator in the
+  // public/cpp/memory_instrumentation directory should not be needed anymore.
+  // We should be able to delete that and rename
+  // memory_instrumentation::CoordinatorImpl to
+  // memory_instrumentation::Coordinator.
+  memory_instrumentation_coordinator_ =
+      base::MakeUnique<memory_instrumentation::CoordinatorImpl>(
+          context()->connector());
+  registry_.AddInterface(base::BindRepeating(
+      &memory_instrumentation::CoordinatorImpl::BindCoordinatorRequest,
+      base::Unretained(memory_instrumentation_coordinator_.get())));
+
+  tracing_agent_registry_ = base::MakeUnique<tracing::AgentRegistry>();
+  registry_.AddInterface(
+      base::BindRepeating(&tracing::AgentRegistry::BindAgentRegistryRequest,
+                          base::Unretained(tracing_agent_registry_.get())));
+
+  tracing_coordinator_ = base::MakeUnique<tracing::Coordinator>();
+  registry_.AddInterface(
+      base::BindRepeating(&tracing::Coordinator::BindCoordinatorRequest,
+                          base::Unretained(tracing_coordinator_.get())));
 }
 
 void ResourceCoordinatorService::OnBindInterface(
     const service_manager::BindSourceInfo& source_info,
     const std::string& interface_name,
     mojo::ScopedMessagePipeHandle interface_pipe) {
-  registry_.BindInterface(interface_name, std::move(interface_pipe));
+  registry_.BindInterface(interface_name, std::move(interface_pipe),
+                          source_info);
 }
 
 void ResourceCoordinatorService::SetUkmRecorder(

services/resource_coordinator/resource_coordinator_service.h

@@ -14,6 +14,9 @@
 #include "services/metrics/public/cpp/mojo_ukm_recorder.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_manager.h"
+#include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
+#include "services/resource_coordinator/tracing/agent_registry.h"
+#include "services/resource_coordinator/tracing/coordinator.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/service.h"
 #include "services/service_manager/public/cpp/service_context_ref.h"
@@ -37,7 +40,6 @@ class ResourceCoordinatorService : public service_manager::Service {
 
   void SetUkmRecorder(std::unique_ptr<ukm::MojoUkmRecorder> ukm_recorder);
 
-  service_manager::BinderRegistry& registry() { return registry_; }
   service_manager::ServiceContextRefFactory* ref_factory() {
     return ref_factory_.get();
   }
@@ -47,11 +49,17 @@ class ResourceCoordinatorService : public service_manager::Service {
   }
 
  private:
-  service_manager::BinderRegistry registry_;
+  service_manager::BinderRegistryWithArgs<
+      const service_manager::BindSourceInfo&>
+      registry_;
   std::unique_ptr<service_manager::ServiceContextRefFactory> ref_factory_;
   CoordinationUnitManager coordination_unit_manager_;
   CoordinationUnitIntrospectorImpl introspector_;
   std::unique_ptr<ukm::MojoUkmRecorder> ukm_recorder_;
+  std::unique_ptr<memory_instrumentation::CoordinatorImpl>
+      memory_instrumentation_coordinator_;
+  std::unique_ptr<tracing::AgentRegistry> tracing_agent_registry_;
+  std::unique_ptr<tracing::Coordinator> tracing_coordinator_;
 
   // WeakPtrFactory members should always come last so WeakPtrs are destructed
   // before other members.

services/resource_coordinator/service_callbacks_impl.cc

@@ -9,6 +9,7 @@
 #include "base/memory/ptr_util.h"
 #include "services/metrics/public/cpp/mojo_ukm_recorder.h"
 #include "services/resource_coordinator/resource_coordinator_service.h"
+#include "services/service_manager/public/cpp/bind_source_info.h"
 #include "services/service_manager/public/cpp/service_context_ref.h"
 
 namespace resource_coordinator {
@@ -27,7 +28,8 @@ ServiceCallbacksImpl::~ServiceCallbacksImpl() = default;
 void ServiceCallbacksImpl::Create(
     service_manager::ServiceContextRefFactory* service_ref_factory,
     ResourceCoordinatorService* resource_coordinator_service,
-    resource_coordinator::mojom::ServiceCallbacksRequest request) {
+    resource_coordinator::mojom::ServiceCallbacksRequest request,
+    const service_manager::BindSourceInfo& source_info) {
   mojo::MakeStrongBinding(
       base::MakeUnique<ServiceCallbacksImpl>(service_ref_factory,
                                              resource_coordinator_service),

services/resource_coordinator/service_callbacks_impl.h

@@ -13,6 +13,7 @@
 #include "services/resource_coordinator/public/interfaces/service_callbacks.mojom.h"
 
 namespace service_manager {
+struct BindSourceInfo;
 class ServiceContextRefFactory;
 class ServiceContextRef;
 }  // namespace service_manager
@@ -31,7 +32,8 @@ class ServiceCallbacksImpl : public mojom::ServiceCallbacks {
   static void Create(
       service_manager::ServiceContextRefFactory* service_ref_factory,
       ResourceCoordinatorService* resource_coordinator_service,
-      resource_coordinator::mojom::ServiceCallbacksRequest request);
+      resource_coordinator::mojom::ServiceCallbacksRequest request,
+      const service_manager::BindSourceInfo& source_info);
 
   void IsUkmRecorderInterfaceInitialized(
       const IsUkmRecorderInterfaceInitializedCallback& callback) override;