Move some Chrome OS services off ServiceContext

This switches the IME and Assistant services to use ServiceBinding
instead of deprecated ServiceContext & EmbeddedService APIs. Also moves
their tests off of the deprecated ServiceTest framework and changes them
to be simple unit tests.

TBR=dcheng@chromium.org

Bug: 891780,906239
Change-Id: I047fd3879da1c4eb89643717cd3d247cf638ef9b
Reviewed-on: https://chromium-review.googlesource.com/c/1343037Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Leo Zhang <googleo@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#609812}

chrome/browser/profiles/profile_impl.cc

@@ -1170,23 +1170,6 @@ void ProfileImpl::RegisterInProcessServices(StaticServiceMap* services) {
   }
 
 #if defined(OS_CHROMEOS)
-#if BUILDFLAG(ENABLE_CROS_ASSISTANT)
-  {
-    service_manager::EmbeddedServiceInfo info;
-    info.factory = base::BindRepeating([] {
-      network::NetworkConnectionTracker* network_connection_tracker =
-          content::GetNetworkConnectionTracker();
-      return std::unique_ptr<service_manager::Service>(
-          std::make_unique<chromeos::assistant::Service>(
-              network_connection_tracker));
-    });
-    info.task_runner = base::CreateSingleThreadTaskRunnerWithTraits(
-        {content::BrowserThread::UI});
-    services->insert(
-        std::make_pair(chromeos::assistant::mojom::kServiceName, info));
-  }
-#endif
-
   if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
     service_manager::EmbeddedServiceInfo info;
     info.task_runner = base::ThreadTaskRunnerHandle::Get();
@@ -1233,6 +1216,21 @@ void ProfileImpl::RegisterInProcessServices(StaticServiceMap* services) {
       std::make_pair(identity::mojom::kServiceName, identity_service_info));
 }
 
+std::unique_ptr<service_manager::Service> ProfileImpl::HandleServiceRequest(
+    const std::string& service_name,
+    service_manager::mojom::ServiceRequest request) {
+#if defined(OS_CHROMEOS)
+#if BUILDFLAG(ENABLE_CROS_ASSISTANT)
+  if (service_name == chromeos::assistant::mojom::kServiceName) {
+    return std::make_unique<chromeos::assistant::Service>(
+        std::move(request), content::GetNetworkConnectionTracker());
+  }
+#endif
+#endif
+
+  return nullptr;
+}
+
 std::string ProfileImpl::GetMediaDeviceIDSalt() {
   return media_device_id_salt_->GetSalt();
 }

chrome/browser/profiles/profile_impl.h

@@ -99,6 +99,9 @@ class ProfileImpl : public Profile {
       const base::FilePath& partition_path,
       bool in_memory) override;
   void RegisterInProcessServices(StaticServiceMap* services) override;
+  std::unique_ptr<service_manager::Service> HandleServiceRequest(
+      const std::string& service_name,
+      service_manager::mojom::ServiceRequest request) override;
   std::string GetMediaDeviceIDSalt() override;
   download::InProgressDownloadManager* RetriveInProgressDownloadManager()
       override;

chrome/utility/chrome_content_utility_client.cc

@@ -300,13 +300,6 @@ void ChromeContentUtilityClient::RegisterServices(
 #if defined(OS_CHROMEOS)
   // TODO(jamescook): Figure out why we have to do this when not using mash.
   mash_service_factory_->RegisterOutOfProcessServices(services);
-
-  {
-    service_manager::EmbeddedServiceInfo service_info;
-    service_info.factory =
-        base::BindRepeating(&chromeos::ime::CreateImeService);
-    services->emplace(chromeos::ime::mojom::kServiceName, service_info);
-  }
 #endif
 
 #if BUILDFLAG(ENABLE_SIMPLE_BROWSER_SERVICE_OUT_OF_PROCESS)
@@ -335,6 +328,11 @@ ChromeContentUtilityClient::HandleServiceRequest(
     return std::make_unique<patch::PatchService>(std::move(request));
 #endif
 
+#if defined(OS_CHROMEOS)
+  if (service_name == chromeos::ime::mojom::kServiceName)
+    return std::make_unique<chromeos::ime::ImeService>(std::move(request));
+#endif
+
   return nullptr;
 }
 

chromeos/services/BUILD.gn

@@ -5,8 +5,6 @@
 import("//build/config/ui.gni")
 import("//chromeos/assistant/assistant.gni")
 import("//chromeos/services/ime/public/features.gni")
-import("//services/catalog/public/tools/catalog.gni")
-import("//services/service_manager/public/tools/test/service_test.gni")
 import("//testing/test.gni")
 
 assert(is_chromeos, "Non-ChromeOS builds cannot depend on //chromeos")
@@ -32,28 +30,13 @@ source_set("unit_tests") {
 # Use this target for adding new service tests. To add a unit test to this
 # target, create a "tests" source_set in your service and add it as a dependency
 # here.
-#
-# If your unit tests use the ServiceTest framework, you must also include
-# corresponding catalog entries in the "chromeos_services_unittests_catalog"
-# target below.
-service_test("chromeos_services_unittests") {
+test("chromeos_services_unittests") {
   deps = [
     "//chromeos/services/ime:services_unittests",
+    "//mojo/core/test:run_all_unittests",
   ]
 
   if (enable_cros_assistant) {
     deps += [ "//chromeos/services/assistant:tests" ]
   }
-
-  catalog = ":chromeos_services_unittests_catalog"
-}
-
-catalog("chromeos_services_unittests_catalog") {
-  testonly = true
-
-  catalog_deps = [ "//chromeos/services/ime:tests_catalog" ]
-
-  if (enable_cros_assistant) {
-    catalog_deps += [ "//chromeos/services/assistant:tests_catalog" ]
-  }
 }

chromeos/services/assistant/BUILD.gn

@@ -3,8 +3,6 @@
 # found in the LICENSE file.
 
 import("//chromeos/assistant/assistant.gni")
-import("//services/catalog/public/tools/catalog.gni")
-import("//services/service_manager/public/cpp/service.gni")
 import("//services/service_manager/public/service_manifest.gni")
 
 assert(is_chromeos)
@@ -115,7 +113,7 @@ source_set("tests") {
     "//services/device/public/mojom",
     "//services/identity/public/mojom",
     "//services/service_manager/public/cpp",
-    "//services/service_manager/public/cpp:service_test_support",
+    "//services/service_manager/public/cpp/test:test_support",
     "//testing/gmock",
     "//testing/gtest",
   ]
@@ -133,17 +131,6 @@ source_set("tests") {
   }
 }
 
-service_manifest("unittest_manifest") {
-  name = "assistant_unittests"
-  source = "unittest_manifest.json"
-  packaged_services = [ ":manifest" ]
-}
-
-catalog("tests_catalog") {
-  testonly = true
-  embedded_services = [ ":unittest_manifest" ]
-}
-
 static_library("test_support") {
   testonly = true
   sources = [

chromeos/services/assistant/OWNERS

@@ -4,6 +4,3 @@ file://chromeos/assistant/OWNERS
 
 per-file manifest.json=set noparent
 per-file manifest.json=file://ipc/SECURITY_OWNERS
-
-per-file unittest_manifest.json=set noparent
-per-file unittest_manifest.json=file://ipc/SECURITY_OWNERS

chromeos/services/assistant/service.cc

@@ -52,8 +52,10 @@ constexpr base::TimeDelta kMaxTokenRefreshDelay =
 
 }  // namespace
 
-Service::Service(network::NetworkConnectionTracker* network_connection_tracker)
-    : platform_binding_(this),
+Service::Service(service_manager::mojom::ServiceRequest request,
+                 network::NetworkConnectionTracker* network_connection_tracker)
+    : service_binding_(this, std::move(request)),
+      platform_binding_(this),
       session_observer_binding_(this),
       token_refresh_timer_(std::make_unique<base::OneShotTimer>()),
       main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
@@ -211,14 +213,14 @@ void Service::Init(mojom::ClientPtr client,
                    mojom::DeviceActionsPtr device_actions) {
   client_ = std::move(client);
   device_actions_ = std::move(device_actions);
-  assistant_state_.Init(context()->connector());
+  assistant_state_.Init(service_binding_.GetConnector());
   assistant_state_.AddObserver(this);
   RequestAccessToken();
 }
 
 identity::mojom::IdentityManager* Service::GetIdentityManager() {
   if (!identity_manager_) {
-    context()->connector()->BindInterface(
+    service_binding_.GetConnector()->BindInterface(
         identity::mojom::kServiceName, mojo::MakeRequest(&identity_manager_));
   }
   return identity_manager_.get();
@@ -273,15 +275,15 @@ void Service::RetryRefreshToken() {
 void Service::CreateAssistantManagerService() {
 #if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
   device::mojom::BatteryMonitorPtr battery_monitor;
-  context()->connector()->BindInterface(device::mojom::kServiceName,
-                                        mojo::MakeRequest(&battery_monitor));
+  service_binding_.GetConnector()->BindInterface(
+      device::mojom::kServiceName, mojo::MakeRequest(&battery_monitor));
   assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>(
-      context()->connector(), std::move(battery_monitor), this,
+      service_binding_.GetConnector(), std::move(battery_monitor), this,
       network_connection_tracker_);
 
   // Bind to Assistant controller in ash.
-  context()->connector()->BindInterface(ash::mojom::kServiceName,
-                                        &assistant_controller_);
+  service_binding_.GetConnector()->BindInterface(ash::mojom::kServiceName,
+                                                 &assistant_controller_);
   assistant_settings_manager_ =
       assistant_manager_service_.get()->GetAssistantSettingsManager();
 #else
@@ -322,8 +324,8 @@ void Service::StopAssistantManagerService() {
 
 void Service::AddAshSessionObserver() {
   ash::mojom::SessionControllerPtr session_controller;
-  context()->connector()->BindInterface(ash::mojom::kServiceName,
-                                        &session_controller);
+  service_binding_.GetConnector()->BindInterface(ash::mojom::kServiceName,
+                                                 &session_controller);
   ash::mojom::SessionActivationObserverPtr observer;
   session_observer_binding_.Bind(mojo::MakeRequest(&observer));
   session_controller->AddSessionActivationObserverForAccountId(

chromeos/services/assistant/service.h

@@ -28,6 +28,8 @@
 #include "services/identity/public/mojom/identity_manager.mojom.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_binding.h"
+#include "services/service_manager/public/mojom/service.mojom.h"
 
 class GoogleServiceAuthError;
 
@@ -51,8 +53,8 @@ class Service : public service_manager::Service,
                 public mojom::AssistantPlatform,
                 public ash::DefaultVoiceInteractionObserver {
  public:
-  explicit Service(
-      network::NetworkConnectionTracker* network_connection_tracker);
+  Service(service_manager::mojom::ServiceRequest request,
+          network::NetworkConnectionTracker* network_connection_tracker);
   ~Service() override;
 
   mojom::Client* client() { return client_.get(); }
@@ -123,6 +125,7 @@ class Service : public service_manager::Service,
 
   void UpdateListeningState();
 
+  service_manager::ServiceBinding service_binding_;
   service_manager::BinderRegistry registry_;
 
   mojo::BindingSet<mojom::Assistant> bindings_;

chromeos/services/assistant/service_unittest.cc

@@ -17,10 +17,10 @@
 #include "chromeos/dbus/fake_power_manager_client.h"
 #include "chromeos/services/assistant/fake_assistant_manager_service_impl.h"
 #include "chromeos/services/assistant/public/mojom/constants.mojom.h"
+#include "chromeos/services/assistant/service.h"
 #include "services/identity/public/mojom/identity_manager.mojom.h"
-#include "services/service_manager/public/cpp/service_context.h"
-#include "services/service_manager/public/cpp/service_test.h"
-#include "services/service_manager/public/mojom/service_factory.mojom.h"
+#include "services/service_manager/public/cpp/service_binding.h"
+#include "services/service_manager/public/cpp/test/test_connector_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
@@ -149,119 +149,60 @@ class FakeDeviceActions : mojom::DeviceActions {
   DISALLOW_COPY_AND_ASSIGN(FakeDeviceActions);
 };
 
-class ServiceTestClient : public service_manager::test::ServiceTestClient,
-                          public service_manager::mojom::ServiceFactory {
- public:
-  ServiceTestClient(service_manager::test::ServiceTest* test,
-                    FakeIdentityManager* fake_identity_manager,
-                    FakeAssistantManagerServiceImpl* fake_assistant_manager_ptr,
-                    base::TestMockTimeTaskRunner* mock_task_runner)
-      : service_manager::test::ServiceTestClient(test),
-        fake_identity_manager_(fake_identity_manager),
-        fake_assistant_manager_ptr_(fake_assistant_manager_ptr),
-        mock_task_runner_(mock_task_runner) {
-    registry_.AddInterface<service_manager::mojom::ServiceFactory>(
-        base::BindRepeating(&ServiceTestClient::Create,
-                            base::Unretained(this)));
-  }
-
- protected:
-  void OnBindInterface(const service_manager::BindSourceInfo& source_info,
-                       const std::string& interface_name,
-                       mojo::ScopedMessagePipeHandle interface_pipe) override {
-    registry_.BindInterface(interface_name, std::move(interface_pipe));
-  }
-
-  void CreateService(
-      service_manager::mojom::ServiceRequest request,
-      const std::string& name,
-      service_manager::mojom::PIDReceiverPtr pid_receiver) override {
-    if (name == mojom::kServiceName) {
-      auto service = std::make_unique<chromeos::assistant::Service>(
-          nullptr /* network_connection_tracker */);
-
-      auto mock_timer = std::make_unique<base::OneShotTimer>(
-          mock_task_runner_->GetMockTickClock());
-      mock_timer->SetTaskRunner(mock_task_runner_);
-      service->SetTimerForTesting(std::move(mock_timer));
-
-      service->SetIdentityManagerForTesting(
-          fake_identity_manager_->CreateInterfacePtrAndBind());
-      service->SetAssistantManagerForTesting(
-          base::WrapUnique(fake_assistant_manager_ptr_));
-
-      service_context_ = std::make_unique<service_manager::ServiceContext>(
-          std::move(service), std::move(request));
-    }
-  }
-
-  void Create(service_manager::mojom::ServiceFactoryRequest request) {
-    service_factory_bindings_.AddBinding(this, std::move(request));
-  }
-
- private:
-  service_manager::BinderRegistry registry_;
-  mojo::BindingSet<service_manager::mojom::ServiceFactory>
-      service_factory_bindings_;
-
-  FakeIdentityManager* fake_identity_manager_;
-  FakeAssistantManagerServiceImpl* fake_assistant_manager_ptr_;
-  base::TestMockTimeTaskRunner* mock_task_runner_;
-
-  std::unique_ptr<service_manager::ServiceContext> service_context_;
-  DISALLOW_COPY_AND_ASSIGN(ServiceTestClient);
-};
-
-class ServiceTest : public service_manager::test::ServiceTest {
+class AssistantServiceTest : public testing::Test {
  public:
-  ServiceTest() : service_manager::test::ServiceTest("assistant_unittests") {}
-
-  void SetUp() override {
-    service_manager::test::ServiceTest::SetUp();
-
-    GetService()->Init(fake_assistant_client_->CreateInterfacePtrAndBind(),
-                       fake_device_actions_->CreateInterfacePtrAndBind());
-    platform_service_.FlushForTesting();
-    base::RunLoop().RunUntilIdle();
-  }
-
-  // service_manager::test::ServiceTest:
-  std::unique_ptr<service_manager::Service> CreateService() override {
-    fake_identity_manager_ = std::make_unique<FakeIdentityManager>();
-    fake_assistant_client_ = std::make_unique<FakeAssistantClient>();
-    fake_device_actions_ = std::make_unique<FakeDeviceActions>();
-    fake_assistant_manager_ptr_ = new FakeAssistantManagerServiceImpl();
-
-    std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
-        chromeos::DBusThreadManager::GetSetterForTesting();
+  AssistantServiceTest()
+      : connector_(test_connector_factory_.CreateConnector()) {
+    // The assistant service may attempt to connect to a number of services
+    // which are irrelevant for these tests.
+    test_connector_factory_.set_ignore_unknown_service_requests(true);
 
     auto power_manager_client =
         std::make_unique<chromeos::FakePowerManagerClient>();
     power_manager_client->SetTabletMode(
         chromeos::PowerManagerClient::TabletMode::OFF, base::TimeTicks());
     power_manager_client_ = power_manager_client.get();
+
+    auto dbus_setter = chromeos::DBusThreadManager::GetSetterForTesting();
     dbus_setter->SetPowerManagerClient(std::move(power_manager_client));
 
+    service_ = std::make_unique<Service>(
+        test_connector_factory_.RegisterInstance(mojom::kServiceName),
+        nullptr /* network_connection_tracker */);
+
     mock_task_runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
         base::Time::Now(), base::TimeTicks::Now());
-
-    return std::make_unique<ServiceTestClient>(this, identity_manager(),
-                                               assistant_manager_service(),
-                                               mock_task_runner());
+    auto mock_timer = std::make_unique<base::OneShotTimer>(
+        mock_task_runner_->GetMockTickClock());
+    mock_timer->SetTaskRunner(mock_task_runner_);
+    service_->SetTimerForTesting(std::move(mock_timer));
+
+    service_->SetIdentityManagerForTesting(
+        fake_identity_manager_.CreateInterfacePtrAndBind());
+
+    auto fake_assistant_manager =
+        std::make_unique<FakeAssistantManagerServiceImpl>();
+    fake_assistant_manager_ = fake_assistant_manager.get();
+    service_->SetAssistantManagerForTesting(std::move(fake_assistant_manager));
   }
 
-  mojom::AssistantPlatform* GetService() {
-    if (!platform_service_)
-      connector()->BindInterface(mojom::kServiceName, &platform_service_);
-    return platform_service_.get();
+  void SetUp() override {
+    GetPlatform()->Init(fake_assistant_client_.CreateInterfacePtrAndBind(),
+                        fake_device_actions_.CreateInterfacePtrAndBind());
+    platform_.FlushForTesting();
+    base::RunLoop().RunUntilIdle();
   }
 
-  FakeIdentityManager* identity_manager() {
-    return fake_identity_manager_.get();
+  mojom::AssistantPlatform* GetPlatform() {
+    if (!platform_)
+      connector_->BindInterface(mojom::kServiceName, &platform_);
+    return platform_.get();
   }
 
+  FakeIdentityManager* identity_manager() { return &fake_identity_manager_; }
+
   FakeAssistantManagerServiceImpl* assistant_manager_service() {
-    return fake_assistant_manager_ptr_;
+    return fake_assistant_manager_;
   }
 
   chromeos::FakePowerManagerClient* power_manager_client() {
@@ -273,24 +214,27 @@ class ServiceTest : public service_manager::test::ServiceTest {
   }
 
  private:
-  mojom::AssistantPlatformPtr platform_service_;
-
-  std::unique_ptr<FakeIdentityManager> fake_identity_manager_;
-  std::unique_ptr<FakeAssistantClient> fake_assistant_client_;
+  base::test::ScopedTaskEnvironment task_environment_;
+  service_manager::TestConnectorFactory test_connector_factory_;
+  std::unique_ptr<service_manager::Connector> connector_;
 
-  std::unique_ptr<FakeDeviceActions> fake_device_actions_;
+  std::unique_ptr<chromeos::assistant::Service> service_;
+  mojom::AssistantPlatformPtr platform_;
 
-  FakeAssistantManagerServiceImpl* fake_assistant_manager_ptr_;
+  FakeIdentityManager fake_identity_manager_;
+  FakeAssistantClient fake_assistant_client_;
+  FakeDeviceActions fake_device_actions_;
 
+  FakeAssistantManagerServiceImpl* fake_assistant_manager_;
   chromeos::FakePowerManagerClient* power_manager_client_;
 
   scoped_refptr<base::TestMockTimeTaskRunner> mock_task_runner_;
   std::unique_ptr<base::OneShotTimer> mock_timer_;
 
-  DISALLOW_COPY_AND_ASSIGN(ServiceTest);
+  DISALLOW_COPY_AND_ASSIGN(AssistantServiceTest);
 };
 
-TEST_F(ServiceTest, RefreshTokenAfterExpire) {
+TEST_F(AssistantServiceTest, RefreshTokenAfterExpire) {
   auto current_count = identity_manager()->get_access_token_count();
   mock_task_runner()->FastForwardBy(kDefaultTokenExpirationDelay / 2);
   base::RunLoop().RunUntilIdle();
@@ -305,7 +249,7 @@ TEST_F(ServiceTest, RefreshTokenAfterExpire) {
   EXPECT_EQ(identity_manager()->get_access_token_count(), ++current_count);
 }
 
-TEST_F(ServiceTest, RetryRefreshTokenAfterFailure) {
+TEST_F(AssistantServiceTest, RetryRefreshTokenAfterFailure) {
   auto current_count = identity_manager()->get_access_token_count();
   identity_manager()->SetShouldFail(true);
   mock_task_runner()->FastForwardBy(kDefaultTokenExpirationDelay);
@@ -325,7 +269,7 @@ TEST_F(ServiceTest, RetryRefreshTokenAfterFailure) {
   EXPECT_EQ(identity_manager()->get_access_token_count(), ++current_count);
 }
 
-TEST_F(ServiceTest, RetryRefreshTokenAfterDeviceWakeup) {
+TEST_F(AssistantServiceTest, RetryRefreshTokenAfterDeviceWakeup) {
   auto current_count = identity_manager()->get_access_token_count();
   power_manager_client()->SendSuspendDone();
   base::RunLoop().RunUntilIdle();

chromeos/services/assistant/unittest_manifest.json

-{
-  "name": "assistant_unittests",
-  "display_name": "Assistant Unittests",
-  "interface_provider_specs": {
-    "service_manager:connector": {
-      "provides": {
-        "service_manager:service_factory": [
-          "service_manager.mojom.ServiceFactory"
-        ]
-      },
-      "requires": {
-        "assistant": [
-            "assistant"
-        ]
-      }
-    }
-  }
-}

chromeos/services/ime/BUILD.gn

@@ -3,8 +3,6 @@
 # found in the LICENSE file.
 
 import("//chromeos/services/ime/public/features.gni")
-import("//services/catalog/public/tools/catalog.gni")
-import("//services/service_manager/public/cpp/service.gni")
 import("//services/service_manager/public/service_manifest.gni")
 
 assert(is_chromeos, "Non-ChromeOS builds cannot depend on //chromeos")
@@ -51,24 +49,15 @@ source_set("services_unittests") {
   deps = [
     ":lib",
     "//base",
+    "//base/test:test_support",
     "//chromeos/services/ime/public/mojom",
     "//mojo/public/cpp/bindings",
     "//services/service_manager/public/cpp",
-    "//services/service_manager/public/cpp:service_test_support",
+    "//services/service_manager/public/cpp/test:test_support",
     "//testing/gmock",
+    "//testing/gtest",
   ]
   sources = [
     "ime_service_unittest.cc",
   ]
 }
-
-service_manifest("unittest_manifest") {
-  name = "ime_unittests"
-  source = "test_manifest.json"
-  packaged_services = [ ":manifest" ]
-}
-
-catalog("tests_catalog") {
-  testonly = true
-  embedded_services = [ ":unittest_manifest" ]
-}

chromeos/services/ime/OWNERS

@@ -3,6 +3,3 @@ shuchen@chromium.org
 
 per-file manifest.json=set noparent
 per-file manifest.json=file://ipc/SECURITY_OWNERS
-
-per-file test_manifest.json=set noparent
-per-file test_manifest.json=file://ipc/SECURITY_OWNERS

chromeos/services/ime/ime_service.cc

@@ -10,19 +10,15 @@
 
 #if BUILDFLAG(ENABLE_CROS_IME_DECODER)
 #include "chromeos/services/ime/decoder/decoder_engine.h"
-#include "services/service_manager/public/cpp/service_context.h"
 #endif
 
 namespace chromeos {
 namespace ime {
 
-std::unique_ptr<service_manager::Service> CreateImeService() {
-  return std::make_unique<ImeService>();
-}
-
-ImeService::ImeService() {}
+ImeService::ImeService(service_manager::mojom::ServiceRequest request)
+    : service_binding_(this, std::move(request)) {}
 
-ImeService::~ImeService() {}
+ImeService::~ImeService() = default;
 
 void ImeService::OnStart() {
   binder_registry_.AddInterface<mojom::InputEngineManager>(base::BindRepeating(
@@ -33,7 +29,7 @@ void ImeService::OnStart() {
 
 #if BUILDFLAG(ENABLE_CROS_IME_DECODER)
   input_engine_ = std::make_unique<DecoderEngine>(
-      context()->connector(), base::SequencedTaskRunnerHandle::Get());
+      service_binding_.GetConnector(), base::SequencedTaskRunnerHandle::Get());
 #else
   input_engine_ = std::make_unique<InputEngine>();
 #endif

chromeos/services/ime/ime_service.h

@@ -10,16 +10,16 @@
 #include "mojo/public/cpp/bindings/binding_set.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_binding.h"
+#include "services/service_manager/public/mojom/service.mojom.h"
 
 namespace chromeos {
 namespace ime {
 
-std::unique_ptr<service_manager::Service> CreateImeService();
-
 class ImeService : public service_manager::Service,
                    public mojom::InputEngineManager {
  public:
-  ImeService();
+  explicit ImeService(service_manager::mojom::ServiceRequest request);
   ~ImeService() override;
 
  private:
@@ -41,6 +41,8 @@ class ImeService : public service_manager::Service,
 
   void OnConnectionLost();
 
+  service_manager::ServiceBinding service_binding_;
+
   // For the duration of this service lifetime, there should be only one
   // input engine instance.
   std::unique_ptr<InputEngine> input_engine_;

chromeos/services/ime/ime_service_unittest.cc

@@ -2,20 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chromeos/services/ime/ime_service.h"
 #include "base/bind.h"
 #include "base/strings/utf_string_conversions.h"
-
-#include "chromeos/services/ime/ime_service.h"
+#include "base/test/scoped_task_environment.h"
 #include "chromeos/services/ime/public/mojom/constants.mojom.h"
 #include "chromeos/services/ime/public/mojom/input_engine.mojom.h"
-
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
-#include "services/service_manager/public/cpp/service_context.h"
-#include "services/service_manager/public/cpp/service_test.h"
+#include "services/service_manager/public/cpp/service_binding.h"
+#include "services/service_manager/public/cpp/test/test_connector_factory.h"
 #include "services/service_manager/public/mojom/service_factory.mojom.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 using testing::_;
 
@@ -23,7 +23,7 @@ namespace chromeos {
 namespace ime {
 
 namespace {
-const char kTestServiceName[] = "ime_unittests";
+
 const char kInvalidImeSpec[] = "ime_spec_never_support";
 const std::vector<uint8_t> extra{0x66, 0x77, 0x88};
 
@@ -58,78 +58,33 @@ class TestClientChannel : mojom::InputChannel {
   DISALLOW_COPY_AND_ASSIGN(TestClientChannel);
 };
 
-class ImeServiceTestClient : public service_manager::test::ServiceTestClient,
-                             public service_manager::mojom::ServiceFactory {
- public:
-  ImeServiceTestClient(service_manager::test::ServiceTest* test)
-      : service_manager::test::ServiceTestClient(test) {
-    registry_.AddInterface<service_manager::mojom::ServiceFactory>(
-        base::BindRepeating(&ImeServiceTestClient::Create,
-                            base::Unretained(this)));
-  }
-
- protected:
-  void OnBindInterface(const service_manager::BindSourceInfo& source_info,
-                       const std::string& interface_name,
-                       mojo::ScopedMessagePipeHandle interface_pipe) override {
-    registry_.BindInterface(interface_name, std::move(interface_pipe));
-  }
-
-  // service_manager::mojom::ServiceFactory
-  void CreateService(
-      service_manager::mojom::ServiceRequest request,
-      const std::string& name,
-      service_manager::mojom::PIDReceiverPtr pid_receiver) override {
-    if (name == mojom::kServiceName) {
-      service_context_.reset(new service_manager::ServiceContext(
-          CreateImeService(), std::move(request)));
-    }
-  }
-
-  void Create(service_manager::mojom::ServiceFactoryRequest request) {
-    service_factory_bindings_.AddBinding(this, std::move(request));
-  }
-
- private:
-  service_manager::BinderRegistry registry_;
-  mojo::BindingSet<service_manager::mojom::ServiceFactory>
-      service_factory_bindings_;
-
-  std::unique_ptr<service_manager::ServiceContext> service_context_;
-  DISALLOW_COPY_AND_ASSIGN(ImeServiceTestClient);
-};
-
-class ImeServiceTest : public service_manager::test::ServiceTest {
+class ImeServiceTest : public testing::Test {
  public:
-  ImeServiceTest() : service_manager::test::ServiceTest(kTestServiceName) {}
-  ~ImeServiceTest() override {}
+  ImeServiceTest()
+      : service_(test_connector_factory_.RegisterInstance(mojom::kServiceName)),
+        connector_(test_connector_factory_.CreateConnector()) {}
+  ~ImeServiceTest() override = default;
 
   MOCK_METHOD1(SentTextCallback, void(const std::string&));
   MOCK_METHOD1(SentMessageCallback, void(const std::vector<uint8_t>&));
 
  protected:
   void SetUp() override {
-    ServiceTest::SetUp();
-    connector()->BindInterface(mojom::kServiceName,
-                               mojo::MakeRequest(&ime_manager_));
+    connector_->BindInterface(mojom::kServiceName,
+                              mojo::MakeRequest(&ime_manager_));
 
     // TODO(https://crbug.com/837156): Start or bind other services used.
     // Eg.  connector()->StartService(mojom::kSomeServiceName);
   }
 
-  // service_manager::test::ServiceTest
-  std::unique_ptr<service_manager::Service> CreateService() override {
-    return std::make_unique<ImeServiceTestClient>(this);
-  }
-
-  void TearDown() override {
-    ime_manager_.reset();
-    ServiceTest::TearDown();
-  }
-
   mojom::InputEngineManagerPtr ime_manager_;
 
  private:
+  base::test::ScopedTaskEnvironment task_environment_;
+  service_manager::TestConnectorFactory test_connector_factory_;
+  ImeService service_;
+  std::unique_ptr<service_manager::Connector> connector_;
+
   DISALLOW_COPY_AND_ASSIGN(ImeServiceTest);
 };
 

chromeos/services/ime/test_manifest.json

-{
-  "name": "ime_unittests",
-  "display_name": "IME Unittests",
-  "interface_provider_specs": {
-    "service_manager:connector": {
-      "provides": {
-        "service_manager:service_factory": [
-          "service_manager.mojom.ServiceFactory"
-        ]
-      },
-      "requires": {
-        "ime": [ "input_engine" ]
-      }
-    }
-  }
-}

content/browser/browser_context.cc

@@ -23,6 +23,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
 #include "base/no_destructor.h"
 #include "base/rand_util.h"
 #include "base/supports_user_data.h"
@@ -66,6 +67,7 @@
 #include "services/file/user_id_map.h"
 #include "services/network/public/cpp/features.h"
 #include "services/service_manager/public/cpp/connector.h"
+#include "services/service_manager/public/cpp/service.h"
 #include "services/service_manager/public/mojom/service.mojom.h"
 #include "storage/browser/blob/blob_storage_context.h"
 #include "storage/browser/database/database_tracker.h"
@@ -247,11 +249,18 @@ class BrowserContextServiceManagerConnectionHolder
     : public base::SupportsUserData::Data {
  public:
   explicit BrowserContextServiceManagerConnectionHolder(
+      BrowserContext* browser_context,
       service_manager::mojom::ServiceRequest request)
-      : service_manager_connection_(ServiceManagerConnection::Create(
+      : browser_context_(browser_context),
+        service_manager_connection_(ServiceManagerConnection::Create(
             std::move(request),
             base::CreateSingleThreadTaskRunnerWithTraits(
-                {BrowserThread::IO}))) {}
+                {BrowserThread::IO}))) {
+    service_manager_connection_->SetDefaultServiceRequestHandler(
+        base::BindRepeating(
+            &BrowserContextServiceManagerConnectionHolder::OnServiceRequest,
+            weak_ptr_factory_.GetWeakPtr()));
+  }
   ~BrowserContextServiceManagerConnectionHolder() override {}
 
   ServiceManagerConnection* service_manager_connection() {
@@ -259,7 +268,35 @@ class BrowserContextServiceManagerConnectionHolder
   }
 
  private:
+  void OnServiceRequest(const std::string& service_name,
+                        service_manager::mojom::ServiceRequest request) {
+    std::unique_ptr<service_manager::Service> service =
+        browser_context_->HandleServiceRequest(service_name,
+                                               std::move(request));
+    if (!service) {
+      LOG(ERROR) << "Ignoring request for unknown per-browser-context service:"
+                 << service_name;
+      return;
+    }
+
+    auto* raw_service = service.get();
+    service->set_termination_closure(base::BindOnce(
+        &BrowserContextServiceManagerConnectionHolder::OnServiceQuit,
+        base::Unretained(this), raw_service));
+    running_services_.emplace(raw_service, std::move(service));
+  }
+
+  void OnServiceQuit(service_manager::Service* service) {
+    running_services_.erase(service);
+  }
+
+  BrowserContext* const browser_context_;
   std::unique_ptr<ServiceManagerConnection> service_manager_connection_;
+  std::map<service_manager::Service*, std::unique_ptr<service_manager::Service>>
+      running_services_;
+
+  base::WeakPtrFactory<BrowserContextServiceManagerConnectionHolder>
+      weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(BrowserContextServiceManagerConnectionHolder);
 };
@@ -591,7 +628,7 @@ void BrowserContext::Initialize(
 
     BrowserContextServiceManagerConnectionHolder* connection_holder =
         new BrowserContextServiceManagerConnectionHolder(
-            std::move(service_request));
+            browser_context, std::move(service_request));
     browser_context->SetUserData(kServiceManagerConnection,
                                  base::WrapUnique(connection_holder));
 
@@ -739,6 +776,12 @@ std::string BrowserContext::CreateRandomMediaDeviceIDSalt() {
   return base::UnguessableToken::Create().ToString();
 }
 
+std::unique_ptr<service_manager::Service> BrowserContext::HandleServiceRequest(
+    const std::string& service_name,
+    service_manager::mojom::ServiceRequest request) {
+  return nullptr;
+}
+
 const std::string& BrowserContext::UniqueId() const {
   return unique_id_;
 }

content/common/service_manager/service_manager_connection_impl.cc

@@ -69,6 +69,12 @@ class ServiceManagerConnectionImpl::IOThreadContext
     io_thread_checker_.DetachFromThread();
   }
 
+  void SetDefaultServiceRequestHandler(
+      const ServiceManagerConnection::DefaultServiceRequestHandler& handler) {
+    DCHECK(!started_);
+    default_request_handler_ = handler;
+  }
+
   // Safe to call from any thread.
   void Start(const base::Closure& stop_callback) {
     DCHECK(!started_);
@@ -312,15 +318,24 @@ class ServiceManagerConnectionImpl::IOThreadContext
     DCHECK(io_thread_checker_.CalledOnValidThread());
     auto it = request_handlers_.find(name);
     if (it == request_handlers_.end()) {
-      LOG(ERROR) << "Can't create service " << name << ". No handler found.";
-      return;
+      if (default_request_handler_) {
+        callback_task_runner_->PostTask(
+            FROM_HERE,
+            base::BindOnce(default_request_handler_, name, std::move(request)));
+      } else {
+        LOG(ERROR) << "Can't create service " << name << ". No handler found.";
+      }
+    } else {
+      it->second.Run(std::move(request), std::move(pid_receiver));
     }
-    it->second.Run(std::move(request), std::move(pid_receiver));
   }
 
   base::ThreadChecker io_thread_checker_;
   bool started_ = false;
 
+  ServiceManagerConnection::DefaultServiceRequestHandler
+      default_request_handler_;
+
   // Temporary state established on construction and consumed on the IO thread
   // once the connection is started.
   service_manager::mojom::ServiceRequest pending_service_request_;
@@ -475,6 +490,11 @@ void ServiceManagerConnectionImpl::AddServiceRequestHandlerWithPID(
   context_->AddServiceRequestHandlerWithPID(name, handler);
 }
 
+void ServiceManagerConnectionImpl::SetDefaultServiceRequestHandler(
+    const DefaultServiceRequestHandler& handler) {
+  context_->SetDefaultServiceRequestHandler(handler);
+}
+
 void ServiceManagerConnectionImpl::OnConnectionLost() {
   if (!connection_lost_handler_.is_null())
     connection_lost_handler_.Run();

content/common/service_manager/service_manager_connection_impl.h

@@ -49,6 +49,8 @@ class CONTENT_EXPORT ServiceManagerConnectionImpl
   void AddServiceRequestHandlerWithPID(
       const std::string& name,
       const ServiceRequestHandlerWithPID& handler) override;
+  void SetDefaultServiceRequestHandler(
+      const DefaultServiceRequestHandler& handler) override;
 
   void OnConnectionLost();
   void GetInterface(service_manager::mojom::InterfaceProvider* provider,

content/public/browser/browser_context.h

@@ -22,6 +22,7 @@
 #include "net/url_request/url_request_job_factory.h"
 #include "services/network/public/mojom/cors_origin_pattern.mojom.h"
 #include "services/service_manager/public/cpp/embedded_service_info.h"
+#include "services/service_manager/public/mojom/service.mojom.h"
 #include "third_party/blink/public/mojom/blob/blob.mojom.h"
 
 #if !defined(OS_ANDROID)
@@ -41,6 +42,7 @@ class InProgressDownloadManager;
 
 namespace service_manager {
 class Connector;
+class Service;
 }
 
 namespace storage {
@@ -338,6 +340,12 @@ class CONTENT_EXPORT BrowserContext : public base::SupportsUserData {
   // by the Service Manager.
   virtual void RegisterInProcessServices(StaticServiceMap* services) {}
 
+  // Handles a service request for a service expected to run an instance per
+  // BrowserContext.
+  virtual std::unique_ptr<service_manager::Service> HandleServiceRequest(
+      const std::string& service_name,
+      service_manager::mojom::ServiceRequest request);
+
   // Returns a unique string associated with this browser context.
   virtual const std::string& UniqueId() const;
 

content/public/common/service_manager_connection.h

@@ -125,6 +125,14 @@ class CONTENT_EXPORT ServiceManagerConnection {
   virtual void AddServiceRequestHandlerWithPID(
       const std::string& name,
       const ServiceRequestHandlerWithPID& handler) = 0;
+
+  // Sets a request handler to use if no registered handlers were interested in
+  // an incoming service request. Must be called before |Start()|.
+  using DefaultServiceRequestHandler =
+      base::RepeatingCallback<void(const std::string& service_name,
+                                   service_manager::mojom::ServiceRequest)>;
+  virtual void SetDefaultServiceRequestHandler(
+      const DefaultServiceRequestHandler& handler) = 0;
 };
 
 }  // namespace content

services/service_manager/public/cpp/test/test_connector_factory.cc

@@ -196,10 +196,12 @@ class MultipleServiceConnector : public TestConnectorImplBase {
 class ProxiedServiceConnector : public mojom::Connector {
  public:
   ProxiedServiceConnector(
+      TestConnectorFactory* factory,
       TestConnectorFactory::NameToServiceProxyMap* proxies,
       TestConnectorFactory::NameToServiceHandlerMap* handlers,
       const base::Token& test_instance_group)
       : fake_guid_(base::Token::CreateRandom()),
+        factory_(factory),
         proxies_(proxies),
         handlers_(handlers),
         test_instance_group_(test_instance_group) {}
@@ -230,6 +232,12 @@ class ProxiedServiceConnector : public mojom::Connector {
                      mojo::ScopedMessagePipeHandle interface_pipe,
                      BindInterfaceCallback callback) override {
     auto* proxy = GetServiceProxy(service_filter.service_name());
+    if (!proxy && factory_->ignore_unknown_service_requests()) {
+      std::move(callback).Run(mojom::ConnectResult::ACCESS_DENIED,
+                              base::nullopt);
+      return;
+    }
+
     CHECK(proxy)
         << "TestConnectorFactory received a BindInterface request for an "
         << "unregistered service '" << service_filter.service_name() << "'";
@@ -271,6 +279,7 @@ class ProxiedServiceConnector : public mojom::Connector {
   }
 
   const base::Token fake_guid_;
+  TestConnectorFactory* const factory_;
   TestConnectorFactory::NameToServiceProxyMap* const proxies_;
   TestConnectorFactory::NameToServiceHandlerMap* const handlers_;
   const base::Token test_instance_group_;
@@ -284,7 +293,7 @@ class ProxiedServiceConnector : public mojom::Connector {
 TestConnectorFactory::TestConnectorFactory() {
   test_instance_group_ = base::Token::CreateRandom();
   impl_ = std::make_unique<ProxiedServiceConnector>(
-      &service_proxies_, &service_handlers_, test_instance_group_);
+      this, &service_proxies_, &service_handlers_, test_instance_group_);
 }
 
 TestConnectorFactory::TestConnectorFactory(

services/service_manager/public/cpp/test/test_connector_factory.h

@@ -111,6 +111,16 @@ class TestConnectorFactory : public mojom::ServiceControl {
     return test_instance_group_;
   }
 
+  // Normally a TestConnectorFactory will assert if asked to route a request to
+  // an unregistered service. If this is set to |true|, such requests will be
+  // silently ignored instead.
+  bool ignore_unknown_service_requests() const {
+    return ignore_unknown_service_requests_;
+  }
+  void set_ignore_unknown_service_requests(bool ignore) {
+    ignore_unknown_service_requests_ = ignore;
+  }
+
   // Normally when a service instance registered via either |RegisterInstance()|
   // or |RegisterServiceHandler()| requests termination from the Service
   // Manager, TestConnectorFactory immediately severs the service instance's
@@ -152,6 +162,7 @@ class TestConnectorFactory : public mojom::ServiceControl {
   mojo::AssociatedBindingSet<mojom::ServiceControl, std::string>
       service_control_bindings_;
 
+  bool ignore_unknown_service_requests_ = false;
   bool ignore_quit_requests_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TestConnectorFactory);