[Chromecast] Prepare BrokerService as a replacement for Service Manager

This allows us to eventually unify both internal and external service
interconnection under a single framework (BrokerService). This patch
creates some temporary hooks which make it easier for us to migrate from
Service Manager to BrokerService. As part of the transition, the
lifetime of the Cast BrokerService is no long scoped by the Service
Manager; it will instead by created eagerly in the Cast main process and
expose a service request handler so that clients can still use the
Service Manager to connect to it.

Merge-With: eureka-internal/513086

Bug: internal b/169098077
Test: CQ

Change-Id: I20960c2b57d2b2481c3fcac44b20ca7e3a658786
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622577Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Commit-Queue: Sean Topping <seantopping@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845910}

chromecast/browser/cast_content_browser_client_receiver_bindings.cc

@@ -35,10 +35,6 @@
 #include "media/mojo/services/media_service.h"  // nogncheck
 #endif  // BUILDFLAG(ENABLE_CAST_RENDERER)
 
-#if BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)
-#include "chromecast/external_mojo/broker_service/broker_service.h"  // nogncheck
-#endif
-
 #if (defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(USE_OZONE)
 #include "chromecast/browser/webview/js_channel_service.h"
 #include "chromecast/common/mojom/js_channel.mojom.h"
@@ -80,14 +76,6 @@ void CreateMediaDrmStorage(
       base::BindRepeating(&AllowEmptyOriginIdCB), std::move(receiver));
 }
 
-#if BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)
-void StartExternalMojoBrokerService(
-    mojo::PendingReceiver<service_manager::mojom::Service> receiver) {
-  service_manager::Service::RunAsyncUntilTermination(
-      std::make_unique<external_mojo::BrokerService>(std::move(receiver)));
-}
-#endif  // BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)
-
 }  // namespace
 
 void CastContentBrowserClient::ExposeInterfacesToRenderer(
@@ -212,14 +200,7 @@ void CastContentBrowserClient::BindGpuHostReceiver(
 
 void CastContentBrowserClient::RunServiceInstance(
     const service_manager::Identity& identity,
-    mojo::PendingReceiver<service_manager::mojom::Service>* receiver) {
-#if BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)
-  if (identity.name() == external_mojo::BrokerService::kServiceName) {
-    StartExternalMojoBrokerService(std::move(*receiver));
-    return;
-  }
-#endif  // BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)
-}
+    mojo::PendingReceiver<service_manager::mojom::Service>* receiver) {}
 
 void CastContentBrowserClient::BindHostReceiverForRenderer(
     content::RenderProcessHost* render_process_host,

chromecast/external_mojo/broker_service/BUILD.gn

@@ -11,6 +11,8 @@ source_set("broker_service") {
     "//base",
     "//chromecast/external_mojo/public/cpp:common",
     "//chromecast/external_mojo/public/cpp:external_mojo_broker",
+    "//chromecast/external_mojo/public/mojom",
+    "//chromecast/mojo",
     "//services/service_manager/public/cpp",
     "//services/service_manager/public/mojom",
   ]

chromecast/external_mojo/broker_service/broker_service.cc

@@ -16,6 +16,7 @@
 #include "base/threading/thread.h"
 #include "chromecast/external_mojo/public/cpp/common.h"
 #include "chromecast/external_mojo/public/cpp/external_mojo_broker.h"
+#include "chromecast/external_mojo/public/mojom/connector.mojom.h"
 #include "services/service_manager/public/cpp/manifest_builder.h"
 
 namespace chromecast {
@@ -23,6 +24,8 @@ namespace external_mojo {
 
 namespace {
 
+BrokerService* g_instance = nullptr;
+
 std::vector<service_manager::Manifest>& GetExternalManifests() {
   static std::vector<service_manager::Manifest> manifests;
   return manifests;
@@ -58,13 +61,14 @@ service_manager::Manifest MakePackagedServices(
 
 }  // namespace
 
-BrokerService::BrokerService(
-    mojo::PendingReceiver<service_manager::mojom::Service> receiver)
-    : service_receiver_(this, std::move(receiver)) {
+BrokerService::BrokerService(service_manager::Connector* connector) {
+  DCHECK(!g_instance);
+  g_instance = this;
   io_thread_ = std::make_unique<base::Thread>("external_mojo");
   io_thread_->StartWithOptions(
       base::Thread::Options(base::MessagePumpType::IO, 0));
 
+  bundle_.AddInterface(this);
   std::vector<std::string> external_services_to_proxy;
   const service_manager::Manifest& manifest = GetManifest();
   for (const auto& sub_manifest : manifest.packaged_services) {
@@ -73,13 +77,27 @@ BrokerService::BrokerService(
   broker_ = base::SequenceBound<ExternalMojoBroker>(io_thread_->task_runner(),
                                                     GetBrokerPath());
   broker_.Post(FROM_HERE, &ExternalMojoBroker::InitializeChromium,
-               service_receiver_.GetConnector()->Clone(),
-               external_services_to_proxy);
+               connector->Clone(), external_services_to_proxy);
 }
 
 BrokerService::~BrokerService() {
   broker_.Reset();
   io_thread_.reset();
+  g_instance = nullptr;
+}
+
+// static
+BrokerService* BrokerService::GetInstance() {
+  return g_instance;
+}
+
+// static
+void BrokerService::ServiceRequestHandler(
+    mojo::PendingReceiver<service_manager::mojom::Service> receiver) {
+  if (!g_instance) {
+    return;
+  }
+  g_instance->BindServiceRequest(std::move(receiver));
 }
 
 // static
@@ -98,10 +116,38 @@ const service_manager::Manifest& BrokerService::GetManifest() {
                            .WithSandboxType("none")
                            .CanRegisterOtherServiceInstances(true)
                            .Build())
+          .ExposeCapability(
+              "connector_factory",
+              std::set<const char*>{
+                  "chromecast.external_mojo.mojom.ConnectorFactory",
+              })
           .Build()
           .Amend(MakePackagedServices(GetExternalManifests()))};
   return *manifest;
 }
 
+void BrokerService::OnConnect(const service_manager::BindSourceInfo& source,
+                              const std::string& interface_name,
+                              mojo::ScopedMessagePipeHandle interface_pipe) {
+  bundle_.BindInterface(interface_name, std::move(interface_pipe));
+}
+
+void BrokerService::BindConnector(
+    mojo::PendingReceiver<mojom::ExternalConnector> receiver) {
+  broker_.Post(FROM_HERE, &ExternalMojoBroker::BindConnector,
+               std::move(receiver));
+}
+
+mojo::PendingRemote<mojom::ExternalConnector> BrokerService::CreateConnector() {
+  mojo::PendingRemote<mojom::ExternalConnector> connector_remote;
+  BindConnector(connector_remote.InitWithNewPipeAndPassReceiver());
+  return connector_remote;
+}
+
+void BrokerService::BindServiceRequest(
+    mojo::PendingReceiver<service_manager::mojom::Service> receiver) {
+  service_receiver_.Bind(std::move(receiver));
+}
+
 }  // namespace external_mojo
 }  // namespace chromecast

chromecast/external_mojo/broker_service/broker_service.h

@@ -11,6 +11,8 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/threading/sequence_bound.h"
+#include "chromecast/external_mojo/public/mojom/connector.mojom.h"
+#include "chromecast/mojo/interface_bundle.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "services/service_manager/public/cpp/manifest.h"
 #include "services/service_manager/public/cpp/service.h"
@@ -22,6 +24,10 @@ class SequencedTaskRunner;
 class Thread;
 }  // namespace base
 
+namespace service_manager {
+class Connector;
+}  // namespace service_manager
+
 namespace chromecast {
 namespace external_mojo {
 class ExternalMojoBroker;
@@ -29,8 +35,13 @@ class ExternalMojoBroker;
 // A Mojo service (intended to run within cast_shell or some other Chromium
 // ServiceManager environment) that allows Mojo services built into external
 // processes to interoperate with the Mojo services within cast_shell.
-class BrokerService : public ::service_manager::Service {
+class BrokerService : public ::service_manager::Service,
+                      public mojom::ConnectorFactory {
  public:
+  static BrokerService* GetInstance();
+  static void ServiceRequestHandler(
+      mojo::PendingReceiver<service_manager::mojom::Service> receiver);
+
   static constexpr char const* kServiceName = "external_mojo_broker";
 
   // Adds a manifest for an external Mojo service (ie, one that is running in
@@ -45,16 +56,33 @@ class BrokerService : public ::service_manager::Service {
   // Returns the manifest for this service.
   static const service_manager::Manifest& GetManifest();
 
-  explicit BrokerService(
-      mojo::PendingReceiver<service_manager::mojom::Service> receiver);
+  explicit BrokerService(service_manager::Connector* connector);
   ~BrokerService() override;
 
+  // ::service_manager::Service implementation:
+  void OnConnect(const service_manager::BindSourceInfo& source,
+                 const std::string& interface_name,
+                 mojo::ScopedMessagePipeHandle interface_pipe) override;
+
+  // mojom::ConnectorFactory implementation:
+  void BindConnector(
+      mojo::PendingReceiver<mojom::ExternalConnector> receiver) override;
+
+  // Dispenses a connector for use in a remote process. The remote process must
+  // already belong to the same process network as the BrokerService.
+  mojo::PendingRemote<mojom::ExternalConnector> CreateConnector();
+
  private:
-  service_manager::ServiceReceiver service_receiver_;
+  void BindServiceRequest(
+      mojo::PendingReceiver<service_manager::mojom::Service> receiver);
+
+  service_manager::ServiceReceiver service_receiver_{this};
 
   std::unique_ptr<base::Thread> io_thread_;
   scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
 
+  InterfaceBundle bundle_;
+
   base::SequenceBound<ExternalMojoBroker> broker_;
 
   DISALLOW_COPY_AND_ASSIGN(BrokerService);

chromecast/external_mojo/external_service_support/BUILD.gn

@@ -14,12 +14,29 @@ source_set("external_service") {
   ]
   deps = [
     "//base",
+    "//chromecast/external_mojo/broker_service",
     "//chromecast/external_mojo/public/mojom",
     "//chromecast/mojo",
     "//mojo/core/embedder",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/platform",
     "//mojo/public/cpp/system",
+    "//services/service_manager/public/cpp",
+  ]
+}
+
+source_set("test_support") {
+  testonly = true
+
+  sources = [
+    "fake_external_connector.cc",
+    "fake_external_connector.h",
+  ]
+
+  deps = [
+    ":external_service",
+    "//base",
+    "//mojo/public/cpp/bindings",
   ]
 }
 

chromecast/external_mojo/external_service_support/external_connector.h

@@ -17,6 +17,10 @@
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/system/message_pipe.h"
 
+namespace service_manager {
+class Connector;
+}  // namespace service_manager
+
 namespace chromecast {
 namespace external_service_support {
 class ExternalService;
@@ -36,6 +40,14 @@ class ExternalConnector {
   static std::unique_ptr<ExternalConnector> Create(
       const std::string& broker_path);
 
+  static std::unique_ptr<ExternalConnector> Create(
+      mojo::PendingRemote<external_mojo::mojom::ExternalConnector> remote);
+
+  // Acquires a connector from the BrokerService via the Chromium service
+  // manager.
+  static std::unique_ptr<ExternalConnector> Create(
+      ::service_manager::Connector* connector);
+
   virtual ~ExternalConnector() = default;
 
   // Adds a callback that will be called if this class loses its connection to

chromecast/external_mojo/external_service_support/external_connector_impl.cc

@@ -15,10 +15,13 @@
 #include "base/thread_annotations.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/time.h"
+#include "chromecast/external_mojo/broker_service/broker_service.h"
 #include "chromecast/external_mojo/external_service_support/external_service.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/platform/named_platform_channel.h"
 #include "mojo/public/cpp/platform/platform_channel_endpoint.h"
 #include "mojo/public/cpp/system/invitation.h"
+#include "services/service_manager/public/cpp/connector.h"
 
 namespace chromecast {
 namespace external_service_support {
@@ -141,6 +144,21 @@ std::unique_ptr<ExternalConnector> ExternalConnector::Create(
   return std::make_unique<ExternalConnectorImpl>(broker_path);
 }
 
+// static
+std::unique_ptr<ExternalConnector> ExternalConnector::Create(
+    mojo::PendingRemote<external_mojo::mojom::ExternalConnector> remote) {
+  return std::make_unique<ExternalConnectorImpl>(std::move(remote));
+}
+
+// static
+std::unique_ptr<ExternalConnector> ExternalConnector::Create(
+    service_manager::Connector* connector) {
+  mojo::PendingRemote<external_mojo::mojom::ExternalConnector> pending_remote;
+  connector->BindInterface(external_mojo::BrokerService::kServiceName,
+                           pending_remote.InitWithNewPipeAndPassReceiver());
+  return std::make_unique<ExternalConnectorImpl>(std::move(pending_remote));
+}
+
 ExternalConnectorImpl::ExternalConnectorImpl(const std::string& broker_path)
     : broker_connection_(base::MakeRefCounted<BrokerConnection>(broker_path)) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
@@ -247,11 +265,13 @@ void ExternalConnectorImpl::BindInterfaceImmediately(
 std::unique_ptr<ExternalConnector> ExternalConnectorImpl::Clone() {
   if (broker_connection_) {
     return std::make_unique<ExternalConnectorImpl>(broker_connection_);
-  } else {
-    mojo::PendingRemote<external_mojo::mojom::ExternalConnector> remote;
-    connector_->Clone(remote.InitWithNewPipeAndPassReceiver());
-    return std::make_unique<ExternalConnectorImpl>(std::move(remote));
   }
+  DCHECK(connector_.is_bound())
+      << "Cannot clone an ExternalConnector before it "
+      << "is bound to a sequence.";
+  mojo::PendingRemote<external_mojo::mojom::ExternalConnector> remote;
+  connector_->Clone(remote.InitWithNewPipeAndPassReceiver());
+  return std::make_unique<ExternalConnectorImpl>(std::move(remote));
 }
 
 void ExternalConnectorImpl::SendChromiumConnectorRequest(

chromecast/external_mojo/external_service_support/external_service.h

@@ -60,6 +60,8 @@ class ExternalService : public external_mojo::mojom::ExternalService {
     bundle_.RemoveInterface<Interface>();
   }
 
+  InterfaceBundle* bundle() { return &bundle_; }
+
  private:
   // external_mojo::mojom::ExternalService implementation:
   void OnBindInterface(const std::string& interface_name,

chromecast/external_mojo/external_service_support/fake_external_connector.cc

+// 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.
+
+#include "chromecast/external_mojo/external_service_support/fake_external_connector.h"
+
+#include <utility>
+
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "chromecast/external_mojo/external_service_support/external_service.h"
+
+namespace chromecast {
+namespace external_service_support {
+
+FakeExternalConnector::FakeExternalConnector() {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+}
+
+FakeExternalConnector::~FakeExternalConnector() = default;
+
+std::unique_ptr<base::CallbackList<void()>::Subscription>
+FakeExternalConnector::AddConnectionErrorCallback(
+    base::RepeatingClosure callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return nullptr;
+}
+
+void FakeExternalConnector::RegisterService(const std::string& service_name,
+                                            ExternalService* service) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  RegisterService(service_name, service->GetReceiver());
+}
+
+void FakeExternalConnector::RegisterService(
+    const std::string& service_name,
+    mojo::PendingRemote<external_mojo::mojom::ExternalService> service_remote) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  services_.emplace(service_name, std::move(service_remote));
+}
+
+void FakeExternalConnector::RegisterServices(
+    const std::vector<std::string>& service_names,
+    const std::vector<ExternalService*>& services) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  CHECK(service_names.size() == services.size());
+  for (size_t i = 0; i < services.size(); ++i) {
+    RegisterService(service_names[i], services[i]);
+  }
+}
+
+void FakeExternalConnector::RegisterServices(
+    std::vector<chromecast::external_mojo::mojom::ServiceInstanceInfoPtr>
+        service_instances_info) {}
+
+void FakeExternalConnector::QueryServiceList(
+    base::OnceCallback<void(
+        std::vector<chromecast::external_mojo::mojom::ExternalServiceInfoPtr>)>
+        callback) {}
+
+void FakeExternalConnector::BindInterface(
+    const std::string& service_name,
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle interface_pipe,
+    bool async) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (!services_.count(service_name)) {
+    return;
+  }
+  services_[service_name]->OnBindInterface(interface_name,
+                                           std::move(interface_pipe));
+}
+
+std::unique_ptr<ExternalConnector> FakeExternalConnector::Clone() {
+  return nullptr;
+}
+
+void FakeExternalConnector::SendChromiumConnectorRequest(
+    mojo::ScopedMessagePipeHandle request) {}
+
+}  // namespace external_service_support
+}  // namespace chromecast

chromecast/external_mojo/external_service_support/fake_external_connector.h

+// 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 CHROMECAST_EXTERNAL_MOJO_EXTERNAL_SERVICE_SUPPORT_FAKE_EXTERNAL_CONNECTOR_H_
+#define CHROMECAST_EXTERNAL_MOJO_EXTERNAL_SERVICE_SUPPORT_FAKE_EXTERNAL_CONNECTOR_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/sequence_checker.h"
+#include "chromecast/external_mojo/external_service_support/external_connector.h"
+#include "mojo/public/cpp/bindings/remote.h"
+
+namespace chromecast {
+namespace external_service_support {
+
+// Local, single-process ExternalConnector for testing. Clients can register
+// mock services with FakeExternalConnector and verify that tested code makes
+// the expected service requests.
+class FakeExternalConnector : public ExternalConnector {
+ public:
+  FakeExternalConnector();
+  FakeExternalConnector(const FakeExternalConnector&) = delete;
+  FakeExternalConnector& operator=(const FakeExternalConnector&) = delete;
+  ~FakeExternalConnector() override;
+
+  // ExternalConnector implementation:
+  void RegisterService(const std::string& service_name,
+                       ExternalService* service) override;
+  void RegisterService(
+      const std::string& service_name,
+      mojo::PendingRemote<external_mojo::mojom::ExternalService> service_remote)
+      override;
+  void RegisterServices(const std::vector<std::string>& service_names,
+                        const std::vector<ExternalService*>& services) override;
+  void BindInterface(const std::string& service_name,
+                     const std::string& interface_name,
+                     mojo::ScopedMessagePipeHandle interface_pipe,
+                     bool async = true) override;
+  // Note: These methods are unimplemented.
+  std::unique_ptr<base::CallbackList<void()>::Subscription>
+  AddConnectionErrorCallback(base::RepeatingClosure callback) override;
+  void RegisterServices(
+      std::vector<chromecast::external_mojo::mojom::ServiceInstanceInfoPtr>
+          service_instances_info) override;
+  std::unique_ptr<ExternalConnector> Clone() override;
+  void SendChromiumConnectorRequest(
+      mojo::ScopedMessagePipeHandle request) override;
+  void QueryServiceList(
+      base::OnceCallback<
+          void(std::vector<
+               chromecast::external_mojo::mojom::ExternalServiceInfoPtr>)>
+          callback) override;
+
+ private:
+  base::flat_map<std::string,
+                 mojo::Remote<external_mojo::mojom::ExternalService>>
+      services_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+};
+
+}  // namespace external_service_support
+}  // namespace chromecast
+
+#endif  // CHROMECAST_EXTERNAL_MOJO_EXTERNAL_SERVICE_SUPPORT_FAKE_EXTERNAL_CONNECTOR_H_

chromecast/external_mojo/public/cpp/external_mojo_broker.cc

@@ -441,6 +441,11 @@ ExternalMojoBroker::CreateConnector() {
   return remote;
 }
 
+void ExternalMojoBroker::BindConnector(
+    mojo::PendingReceiver<mojom::ExternalConnector> receiver) {
+  connector_->AddReceiver(std::move(receiver));
+}
+
 ExternalMojoBroker::~ExternalMojoBroker() = default;
 
 }  // namespace external_mojo

chromecast/external_mojo/public/cpp/external_mojo_broker.h

@@ -11,6 +11,7 @@
 
 #include "base/macros.h"
 #include "chromecast/external_mojo/public/mojom/connector.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 
 namespace service_manager {
@@ -38,6 +39,8 @@ class ExternalMojoBroker {
 
   mojo::PendingRemote<mojom::ExternalConnector> CreateConnector();
 
+  void BindConnector(mojo::PendingReceiver<mojom::ExternalConnector> receiver);
+
  private:
   class ConnectorImpl;
   class ReadWatcher;

chromecast/external_mojo/public/mojom/connector.mojom

@@ -57,3 +57,13 @@ interface ExternalConnector {
   // Query services that are available from this connector.
   QueryServiceList() => (array<ExternalServiceInfo> services);
 };
+
+// Simple factory for clients to bind an ExternalConnector. This interface is
+// exposed via the Chromium service manager so clients in the Chromium process
+// tree can easily use the External Service Broker. This is a temporary
+// interface which will be deleted once we are no longer using the Service
+// Manager.
+interface ConnectorFactory {
+  // Binds a request for an ExternalConnector.
+  BindConnector(pending_receiver<ExternalConnector> receiver);
+};