Remove ContentClient::OnServiceManagerConnected

This API is only used to support binding process-wide interface
receivers sent from the browser to child processes; specifically only
for heap profiling today.

In the interest of eliminating ServiceManagerConnection, this CL focuses
the ContentClient API specifically on binding incoming interface pipes
and eliminates the dependency on ServiceManagerConnection.

For in-browser-process heap profiling clients, it's no longer
necessary to go through Service Manager at all, hence the
ClientConnectionManager directly uses a shared lazy
ProfilingClient instance in such cases.

TBR=boliu@chromium.org
TBR=halliwell@chromium.org

Bug: 904240
Change-Id: Ie0332d918ba94d8a2a4bc8978d5b693c73328d13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660841
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670655}

android_webview/browser/aw_content_browser_overlay_manifest.cc

@@ -22,11 +22,6 @@ const service_manager::Manifest& GetAWContentBrowserOverlayManifest() {
                             service_manager::Manifest::InterfaceList<
                                 safe_browsing::mojom::SafeBrowsing,
                                 spellcheck::mojom::SpellCheckHost>())
-          .ExposeCapability("profiling_client",
-                            service_manager::Manifest::InterfaceList<
-                                heap_profiling::mojom::ProfilingClient>())
-          .RequireCapability(content::mojom::kSystemServiceName,
-                             "profiling_client")
           .RequireCapability("heap_profiling", "profiling")
           .RequireCapability("heap_profiling", "heap_profiler")
           .ExposeInterfaceFilterCapability_Deprecated(

android_webview/common/aw_content_client.cc

@@ -15,12 +15,10 @@
 #include "components/services/heap_profiling/public/cpp/profiling_client.h"
 #include "components/version_info/version_info.h"
 #include "content/public/common/content_switches.h"
-#include "content/public/common/service_manager_connection.h"
-#include "content/public/common/simple_connection_filter.h"
 #include "gpu/config/gpu_info.h"
 #include "gpu/config/gpu_util.h"
 #include "ipc/ipc_message.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
@@ -84,19 +82,18 @@ media::MediaDrmBridgeClient* AwContentClient::GetMediaDrmBridgeClient() {
       AwResource::GetConfigKeySystemUuidMapping());
 }
 
-void AwContentClient::OnServiceManagerConnected(
-    content::ServiceManagerConnection* connection) {
+void AwContentClient::BindChildProcessInterface(
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle* receiving_handle) {
   // This creates a process-wide heap_profiling::ProfilingClient that listens
   // for requests from the HeapProfilingService to start profiling the current
   // process.
   static base::NoDestructor<heap_profiling::ProfilingClient> profiling_client;
-
-  auto registry = std::make_unique<service_manager::BinderRegistry>();
-  registry->AddInterface(
-      base::BindRepeating(&heap_profiling::ProfilingClient::BindToInterface,
-                          base::Unretained(profiling_client.get())));
-  connection->AddConnectionFilter(
-      std::make_unique<content::SimpleConnectionFilter>(std::move(registry)));
+  if (interface_name == heap_profiling::ProfilingClient::Name_) {
+    profiling_client->BindToInterface(
+        mojo::PendingReceiver<heap_profiling::mojom::ProfilingClient>(
+            std::move(*receiving_handle)));
+  }
 }
 
 }  // namespace android_webview

android_webview/common/aw_content_client.h

@@ -29,8 +29,9 @@ class AwContentClient : public content::ContentClient {
   void SetGpuInfo(const gpu::GPUInfo& gpu_info) override;
   bool UsingSynchronousCompositing() override;
   media::MediaDrmBridgeClient* GetMediaDrmBridgeClient() override;
-  void OnServiceManagerConnected(
-      content::ServiceManagerConnection* connection) override;
+  void BindChildProcessInterface(
+      const std::string& interface_name,
+      mojo::ScopedMessagePipeHandle* receiving_handle) override;
 
   const std::string& gpu_fingerprint() const { return gpu_fingerprint_; }
 

chrome/app/chrome_content_browser_overlay_manifest.cc

@@ -105,9 +105,6 @@ const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
         .ExposeCapability("gpu",
                           service_manager::Manifest::InterfaceList<
                               metrics::mojom::CallStackProfileCollector>())
-        .ExposeCapability("profiling_client",
-                          service_manager::Manifest::InterfaceList<
-                              heap_profiling::mojom::ProfilingClient>())
         .ExposeCapability("renderer",
                           service_manager::Manifest::InterfaceList<
                               chrome::mojom::AvailableOfflineContentProvider,
@@ -128,7 +125,6 @@ const service_manager::Manifest& GetChromeContentBrowserOverlayManifest() {
         // Only used in the classic Ash case
         .RequireCapability("chrome", "input_device_controller")
         .RequireCapability("chrome_printing", "converter")
-        .RequireCapability("content_system", "profiling_client")
         .RequireCapability("cups_ipp_parser", "ipp_parser")
         .RequireCapability("device", "device:fingerprint")
         .RequireCapability("device", "device:geolocation_config")

chrome/common/chrome_content_client.cc

@@ -43,8 +43,6 @@
 #include "content/public/common/cdm_info.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
-#include "content/public/common/service_manager_connection.h"
-#include "content/public/common/simple_connection_filter.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/buildflags/buildflags.h"
 #include "extensions/common/constants.h"
@@ -54,6 +52,7 @@
 #include "media/base/media_switches.h"
 #include "media/base/video_codecs.h"
 #include "media/media_buildflags.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "net/http/http_util.h"
 #include "pdf/buildflags.h"
 #include "ppapi/buildflags/buildflags.h"
@@ -736,14 +735,13 @@ media::MediaDrmBridgeClient* ChromeContentClient::GetMediaDrmBridgeClient() {
 }
 #endif  // OS_ANDROID
 
-void ChromeContentClient::OnServiceManagerConnected(
-    content::ServiceManagerConnection* connection) {
+void ChromeContentClient::BindChildProcessInterface(
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle* receiving_handle) {
   static base::NoDestructor<heap_profiling::ProfilingClient> profiling_client;
-
-  auto registry = std::make_unique<service_manager::BinderRegistry>();
-  registry->AddInterface(
-      base::BindRepeating(&heap_profiling::ProfilingClient::BindToInterface,
-                          base::Unretained(profiling_client.get())));
-  connection->AddConnectionFilter(
-      std::make_unique<content::SimpleConnectionFilter>(std::move(registry)));
+  if (interface_name == heap_profiling::ProfilingClient::Name_) {
+    profiling_client->BindToInterface(
+        mojo::PendingReceiver<heap_profiling::mojom::ProfilingClient>(
+            std::move(*receiving_handle)));
+  }
 }

chrome/common/chrome_content_client.h

@@ -106,9 +106,9 @@ class ChromeContentClient : public content::ContentClient {
   media::MediaDrmBridgeClient* GetMediaDrmBridgeClient() override;
 #endif  // OS_ANDROID
 
-  // This method isn't called by utility processes.
-  void OnServiceManagerConnected(
-      content::ServiceManagerConnection* connection) override;
+  void BindChildProcessInterface(
+      const std::string& interface_name,
+      mojo::ScopedMessagePipeHandle* receiving_handle) override;
 
  private:
   // Used to lock when |origin_trial_policy_| is initialized.

chromecast/browser/cast_overlay_manifests.cc

@@ -39,10 +39,6 @@ const service_manager::Manifest& GetCastContentBrowserOverlayManifest() {
 #if !defined(OS_FUCHSIA)
         .RequireCapability("heap_profiling", "heap_profiler")
         .RequireCapability("heap_profiling", "profiling")
-        .RequireCapability("content_system", "profiling_client")
-        .ExposeCapability("profiling_client",
-                          service_manager::Manifest::InterfaceList<
-                              heap_profiling::mojom::ProfilingClient>())
 #endif  // !defined(OS_FUCHSIA)
         .ExposeCapability("renderer",
                           service_manager::Manifest::InterfaceList<

chromecast/common/cast_content_client.cc

@@ -30,9 +30,7 @@
 #if !defined(OS_FUCHSIA)
 #include "base/no_destructor.h"
 #include "components/services/heap_profiling/public/cpp/profiling_client.h"  // nogncheck
-#include "content/public/common/service_manager_connection.h"
-#include "content/public/common/simple_connection_filter.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #endif
 
 namespace chromecast {
@@ -141,17 +139,16 @@ ::media::MediaDrmBridgeClient* CastContentClient::GetMediaDrmBridgeClient() {
 }
 #endif  // OS_ANDROID
 
-void CastContentClient::OnServiceManagerConnected(
-    content::ServiceManagerConnection* connection) {
+void CastContentClient::BindChildProcessInterface(
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle* receiving_handle) {
 #if !defined(OS_FUCHSIA)
   static base::NoDestructor<heap_profiling::ProfilingClient> profiling_client;
-
-  auto registry = std::make_unique<service_manager::BinderRegistry>();
-  registry->AddInterface(
-      base::BindRepeating(&heap_profiling::ProfilingClient::BindToInterface,
-                          base::Unretained(profiling_client.get())));
-  connection->AddConnectionFilter(
-      std::make_unique<content::SimpleConnectionFilter>(std::move(registry)));
+  if (interface_name == heap_profiling::ProfilingClient::Name_) {
+    profiling_client->BindToInterface(
+        mojo::PendingReceiver<heap_profiling::mojom::ProfilingClient>(
+            std::move(*receiving_handle)));
+  }
 #endif  // !defined(OS_FUCHSIA)
 }
 

chromecast/common/cast_content_client.h

@@ -30,8 +30,9 @@ class CastContentClient : public content::ContentClient {
 #if defined(OS_ANDROID)
   ::media::MediaDrmBridgeClient* GetMediaDrmBridgeClient() override;
 #endif  // OS_ANDROID
-  void OnServiceManagerConnected(
-      content::ServiceManagerConnection* connection) override;
+  void BindChildProcessInterface(
+      const std::string& interface_name,
+      mojo::ScopedMessagePipeHandle* receiving_handle) override;
 
  private:
   GURL last_active_url_;

components/heap_profiling/client_connection_manager.cc

@@ -5,9 +5,11 @@
 #include "components/heap_profiling/client_connection_manager.h"
 
 #include "base/bind.h"
+#include "base/no_destructor.h"
 #include "base/rand_util.h"
 #include "base/task/post_task.h"
 #include "components/services/heap_profiling/public/cpp/controller.h"
+#include "components/services/heap_profiling/public/cpp/profiling_client.h"
 #include "components/services/heap_profiling/public/cpp/settings.h"
 #include "components/services/heap_profiling/public/mojom/heap_profiling_client.mojom.h"
 #include "components/services/heap_profiling/public/mojom/heap_profiling_service.mojom.h"
@@ -46,13 +48,6 @@ class ProfilingClientBinder {
     content::BindInterface(host, std::move(request_));
   }
 
-  // Binds to the local connector to get the browser process' ProfilingClient.
-  explicit ProfilingClientBinder(service_manager::Connector* connector)
-      : ProfilingClientBinder() {
-    connector->BindInterface(content::mojom::kSystemServiceName,
-                             std::move(request_));
-  }
-
   mojom::ProfilingClientPtr take() { return std::move(memlog_client_); }
 
  private:
@@ -153,10 +148,11 @@ void StartProfilingBrowserProcessOnIOThread(
   if (!controller)
     return;
 
-  ProfilingClientBinder client(controller->GetConnector());
-  StartProfilingClientOnIOThread(controller, std::move(client),
-                                 base::GetCurrentProcId(),
-                                 mojom::ProcessType::BROWSER);
+  static base::NoDestructor<ProfilingClient> client;
+  mojom::ProfilingClientPtr proxy;
+  client->BindToInterface(mojo::MakeRequest(&proxy));
+  controller->StartProfilingClient(std::move(proxy), base::GetCurrentProcId(),
+                                   mojom::ProcessType::BROWSER);
 }
 
 void StartProfilingPidOnIOThread(base::WeakPtr<Controller> controller,
@@ -168,9 +164,7 @@ void StartProfilingPidOnIOThread(base::WeakPtr<Controller> controller,
 
   // Check if the request is for the current process.
   if (pid == base::GetCurrentProcId()) {
-    ProfilingClientBinder client(controller->GetConnector());
-    StartProfilingClientOnIOThread(controller, std::move(client), pid,
-                                   mojom::ProcessType::BROWSER);
+    StartProfilingBrowserProcessOnIOThread(std::move(controller));
     return;
   }
 

content/browser/browser_main_loop.cc

@@ -98,7 +98,6 @@
 #include "content/browser/webui/content_web_ui_controller_factory.h"
 #include "content/browser/webui/url_data_manager.h"
 #include "content/common/content_switches_internal.h"
-#include "content/common/service_manager/service_manager_connection_impl.h"
 #include "content/common/thread_pool_util.h"
 #include "content/public/browser/browser_main_parts.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -1554,9 +1553,6 @@ void BrowserMainLoop::InitializeMojo() {
     mojo::SyncCallRestrictions::DisallowSyncCall();
   }
 
-  GetContentClient()->OnServiceManagerConnected(
-      ServiceManagerConnection::GetForProcess());
-
   // Ensure that any NavigableContentsViews constructed in the browser process
   // know they're running in the same process as the service.
   content::NavigableContentsView::SetClientRunningInServiceProcess();

content/child/child_thread_impl.cc

@@ -260,6 +260,23 @@ class ChannelBootstrapFilter : public ConnectionFilter {
   DISALLOW_COPY_AND_ASSIGN(ChannelBootstrapFilter);
 };
 
+class ContentClientConnectionFilter : public ConnectionFilter {
+ public:
+  ContentClientConnectionFilter() = default;
+
+ private:
+  // ConnectionFilter:
+  void OnBindInterface(const service_manager::BindSourceInfo& source_info,
+                       const std::string& interface_name,
+                       mojo::ScopedMessagePipeHandle* interface_pipe,
+                       service_manager::Connector* connector) override {
+    GetContentClient()->BindChildProcessInterface(interface_name,
+                                                  interface_pipe);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ContentClientConnectionFilter);
+};
+
 }  // namespace
 
 ChildThread* ChildThread::Get() {
@@ -442,6 +459,9 @@ void ChildThreadImpl::Init(const Options& options) {
   thread_safe_sender_ =
       new ThreadSafeSender(main_thread_runner_, sync_message_filter_.get());
 
+  GetServiceManagerConnection()->AddConnectionFilter(
+      std::make_unique<ContentClientConnectionFilter>());
+
   auto registry = std::make_unique<service_manager::BinderRegistry>();
   registry->AddInterface(base::Bind(&ChildHistogramFetcherFactoryImpl::Create),
                          GetIOTaskRunner());
@@ -676,8 +696,6 @@ void ChildThreadImpl::OnAssociatedInterfaceRequest(
 
 void ChildThreadImpl::StartServiceManagerConnection() {
   DCHECK(service_manager_connection_);
-  GetContentClient()->OnServiceManagerConnected(
-      service_manager_connection_.get());
 
   // NOTE: You must register any ConnectionFilter instances on
   // |service_manager_connection_| *before* this call to |Start()|, otherwise

content/public/common/content_client.cc

@@ -129,7 +129,8 @@ media::MediaDrmBridgeClient* ContentClient::GetMediaDrmBridgeClient() {
 }
 #endif  // OS_ANDROID
 
-void ContentClient::OnServiceManagerConnected(
-    ServiceManagerConnection* connection) {}
+void ContentClient::BindChildProcessInterface(
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle* receiving_handle) {}
 
 }  // namespace content

content/public/common/content_client.h

@@ -13,6 +13,7 @@
 #include "base/strings/string_piece.h"
 #include "build/build_config.h"
 #include "content/common/content_export.h"
+#include "mojo/public/cpp/system/message_pipe.h"
 #include "ui/base/layout.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -51,7 +52,6 @@ class ContentClient;
 class ContentGpuClient;
 class ContentRendererClient;
 class ContentUtilityClient;
-class ServiceManagerConnection;
 struct CdmInfo;
 struct PepperPluginInfo;
 
@@ -205,7 +205,11 @@ class CONTENT_EXPORT ContentClient {
   virtual media::MediaDrmBridgeClient* GetMediaDrmBridgeClient();
 #endif  // OS_ANDROID
 
-  virtual void OnServiceManagerConnected(ServiceManagerConnection* connection);
+  // Allows the embedder to handle incoming interface binding requests from
+  // the browser process to any type of child process.
+  virtual void BindChildProcessInterface(
+      const std::string& interface_name,
+      mojo::ScopedMessagePipeHandle* receiving_handle);
 
  private:
   friend class ContentClientInitializer;  // To set these pointers.

content/utility/utility_thread_impl.cc

@@ -143,8 +143,6 @@ void UtilityThreadImpl::Init() {
   service_factory_.reset(new UtilityServiceFactory);
 
   if (connection) {
-    GetContentClient()->OnServiceManagerConnected(connection);
-
     // NOTE: You must register any ConnectionFilter instances on |connection|
     // *before* this call to |Start()|, otherwise incoming interface requests
     // may race with the registration.