[service-manager] Introduce ServiceBinding

Adds a new ServiceBinding helper to the Service Manager's client
library. This is intended as a replacement for ServiceContext, because
ServiceContext is cumbersome in its insistence to own the
corresponding Service object.

ServiceBinding, like ServiceContext, deals with the IPC interface
between a local Service instance and the Service Manager, exposing
interesting events to the Service implementation while also providing
it access to its Connector and assigned Identity.

Some particularly gnarly tests have been updated to use ServiceBinding
in lieu of ServiceContext. Furthermore, without ServiceContext and with
a lot of its legacy motivations gone, ServiceRunner isn't terribly
useful either. The tests have also been trivially updated to manage
their own MessageLoop instead of using ServiceRunner.

This is the first in a series of patches which aim to completely
eliminate ServiceContext, ForwardingService (in service.h), and
ServiceRunner, requiring Service implementations to write a little more
boilerplate in exchange for flexibility with ownership semantics.

Bug: 891780
Change-Id: Ib0c1d3263f34a5e33a167a8187edad1d52413f73
Reviewed-on: https://chromium-review.googlesource.com/c/1259485Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597324}

services/service_manager/public/cpp/BUILD.gn

@@ -17,6 +17,8 @@ component("cpp") {
     "local_interface_provider.h",
     "service.cc",
     "service.h",
+    "service_binding.cc",
+    "service_binding.h",
     "service_context.cc",
     "service_context.h",
     "service_context_ref.cc",
@@ -37,7 +39,12 @@ component("cpp") {
     "//url",
   ]
 
-  defines = [ "SERVICE_MANAGER_PUBLIC_CPP_IMPL" ]
+  defines = [
+    "IS_SERVICE_MANAGER_CPP_IMPL",
+
+    # TODO: Use COMPONENT_EXPORT everywhere here and remove this.
+    "SERVICE_MANAGER_PUBLIC_CPP_IMPL",
+  ]
 }
 
 # A component for types which the public interfaces depend on for typemapping.

services/service_manager/public/cpp/service.cc

@@ -19,6 +19,8 @@ void Service::OnBindInterface(const BindSourceInfo& source,
                               const std::string& interface_name,
                               mojo::ScopedMessagePipeHandle interface_pipe) {}
 
+void Service::OnDisconnected() {}
+
 bool Service::OnServiceManagerConnectionLost() {
   return true;
 }

services/service_manager/public/cpp/service.h

@@ -7,9 +7,9 @@
 
 #include <string>
 
+#include "base/component_export.h"
 #include "base/macros.h"
 #include "mojo/public/cpp/system/message_pipe.h"
-#include "services/service_manager/public/cpp/export.h"
 
 namespace service_manager {
 
@@ -18,7 +18,7 @@ struct BindSourceInfo;
 
 // The primary contract between a Service and the Service Manager, receiving
 // lifecycle notifications and connection requests.
-class SERVICE_MANAGER_PUBLIC_CPP_EXPORT Service {
+class COMPONENT_EXPORT(SERVICE_MANAGER_CPP) Service {
  public:
   Service();
   virtual ~Service();
@@ -37,6 +37,14 @@ class SERVICE_MANAGER_PUBLIC_CPP_EXPORT Service {
                                const std::string& interface_name,
                                mojo::ScopedMessagePipeHandle interface_pipe);
 
+  // Called when the Service Manager has stopped tracking this instance. Once
+  // invoked, no further Service interface methods will be called on this
+  // Service, and no further communication with the Service Manager is possible.
+  //
+  // The Service may continue to operate and service existing client connections
+  // as it deems appropriate.
+  virtual void OnDisconnected();
+
   // Called when the Service Manager has stopped tracking this instance. The
   // service should use this as a signal to shut down, and in fact its process
   // may be reaped shortly afterward if applicable.
@@ -49,6 +57,9 @@ class SERVICE_MANAGER_PUBLIC_CPP_EXPORT Service {
   //
   // NOTE: This may be called at any time, and once it's been called, none of
   // the other public Service methods will be invoked by the ServiceContext.
+  //
+  // This is ONLY invoked when using a ServiceContext and is therefore
+  // deprecated.
   virtual bool OnServiceManagerConnectionLost();
 
  protected:
@@ -72,7 +83,7 @@ class SERVICE_MANAGER_PUBLIC_CPP_EXPORT Service {
 
 // TODO(rockot): Remove this. It's here to satisfy a few remaining use cases
 // where a Service impl is owned by something other than its ServiceContext.
-class SERVICE_MANAGER_PUBLIC_CPP_EXPORT ForwardingService : public Service {
+class COMPONENT_EXPORT(SERVICE_MANAGER_CPP) ForwardingService : public Service {
  public:
   // |target| must outlive this object.
   explicit ForwardingService(Service* target);

services/service_manager/public/cpp/service_binding.cc

+// Copyright 2018 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 "services/service_manager/public/cpp/service_binding.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "services/service_manager/public/cpp/service.h"
+
+#include "base/debug/stack_trace.h"
+
+namespace service_manager {
+
+ServiceBinding::ServiceBinding(service_manager::Service* service)
+    : service_(service), binding_(this) {
+  DCHECK(service_);
+}
+
+ServiceBinding::ServiceBinding(service_manager::Service* service,
+                               mojom::ServiceRequest request)
+    : ServiceBinding(service) {
+  if (request.is_pending())
+    Bind(std::move(request));
+}
+
+ServiceBinding::~ServiceBinding() = default;
+
+Connector* ServiceBinding::GetConnector() {
+  if (!connector_)
+    connector_ = Connector::Create(&pending_connector_request_);
+  return connector_.get();
+}
+
+void ServiceBinding::Bind(mojom::ServiceRequest request) {
+  DCHECK(!is_bound());
+  binding_.Bind(std::move(request));
+  binding_.set_connection_error_handler(base::BindOnce(
+      &ServiceBinding::OnConnectionError, base::Unretained(this)));
+}
+
+void ServiceBinding::RequestClose() {
+  DCHECK(is_bound());
+  if (service_control_.is_bound()) {
+    service_control_->RequestQuit();
+  } else {
+    // It's possible that the service may request closure before receiving the
+    // initial |OnStart()| event, in which case there is not yet a control
+    // interface on which to request closure. In that case we defer until
+    // |OnStart()| is received.
+    request_closure_on_start_ = true;
+  }
+}
+
+void ServiceBinding::Close() {
+  DCHECK(is_bound());
+  binding_.Close();
+  service_control_.reset();
+  connector_.reset();
+}
+
+void ServiceBinding::OnConnectionError() {
+  service_->OnDisconnected();
+}
+
+void ServiceBinding::OnStart(const Identity& identity,
+                             OnStartCallback callback) {
+  identity_ = identity;
+  service_->OnStart();
+
+  if (!pending_connector_request_.is_pending())
+    connector_ = Connector::Create(&pending_connector_request_);
+  std::move(callback).Run(std::move(pending_connector_request_),
+                          mojo::MakeRequest(&service_control_));
+
+  // Execute any prior |RequestClose()| request on the service's behalf.
+  if (request_closure_on_start_)
+    service_control_->RequestQuit();
+}
+
+void ServiceBinding::OnBindInterface(
+    const BindSourceInfo& source_info,
+    const std::string& interface_name,
+    mojo::ScopedMessagePipeHandle interface_pipe,
+    OnBindInterfaceCallback callback) {
+  // Acknowledge this request.
+  std::move(callback).Run();
+
+  service_->OnBindInterface(source_info, interface_name,
+                            std::move(interface_pipe));
+}
+
+}  // namespace service_manager

services/service_manager/public/cpp/service_binding.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SERVICES_SERVICE_MANAGER_PUBLIC_CPP_SERVICE_BINDING_H_
+#define SERVICES_SERVICE_MANAGER_PUBLIC_CPP_SERVICE_BINDING_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "services/service_manager/public/cpp/connector.h"
+#include "services/service_manager/public/mojom/connector.mojom.h"
+#include "services/service_manager/public/mojom/service.mojom.h"
+#include "services/service_manager/public/mojom/service_control.mojom.h"
+
+namespace service_manager {
+
+class Service;
+
+// Encapsulates service-side bindings to Service Manager interfaces. Namely,
+// this helps receive and dispatch Service interface events to a service
+// implementation, while also exposing a working Connector interface the service
+// can use to make outgoing interface requests.
+//
+// A ServiceBinding is considered to be "bound" after |Bind()| is invoked with a
+// valid ServiceRequest (or the equivalent constructor is used -- see below).
+// Upon connection error or an explicit call to |Close()|, the ServiceBinding
+// will be considered "unbound" until another call to |Bind()| is made.
+//
+// NOTE: A well-behaved service should aim to always close its ServiceBinding
+// gracefully by calling |RequestClose()|. Closing a ServiceBinding abruptly
+// (by either destroying it or explicitly calling |Close()|) introduces inherent
+// flakiness into the system unless the Service's |OnDisconnected()| has already
+// been invoked, because otherwise the Service Manager may have in-flight
+// interface requests directed at your service instance and these will be
+// dropped to the dismay of the service instance which issued them. Exceptions
+// can reasonably be made for system-wide shutdown situations where even the
+// Service Manager itself will be imminently torn down.
+class COMPONENT_EXPORT(SERVICE_MANAGER_CPP) ServiceBinding
+    : public mojom::Service {
+ public:
+  // Creates a new ServiceBinding bound to |service|. The service will not
+  // receive any Service interface calls until |Bind()| is called, but its
+  // |connector()| is usable immediately upon construction.
+  //
+  // |service| is not owned and must outlive this ServiceBinding.
+  explicit ServiceBinding(service_manager::Service* service);
+
+  // Same as above, but behaves as if |Bind(request)| is also called immediately
+  // after construction. See below.
+  ServiceBinding(service_manager::Service* service,
+                 mojom::ServiceRequest request);
+
+  ~ServiceBinding() override;
+
+  bool is_bound() const { return binding_.is_bound(); }
+
+  Identity identity() const { return identity_; }
+
+  // Returns a usable Connector which can make outgoing interface requests
+  // identifying as the service to which this ServiceBinding is bound.
+  Connector* GetConnector();
+
+  // Binds this ServiceBinding to a new ServiceRequest. Once a ServiceBinding
+  // is bound, its target Service will begin receiving Service events. The
+  // order of events received is:
+  //
+  //   - OnStart() exactly once
+  //   - OnIdentityKnown() exactly once
+  //   - OnBindInterface() zero or more times
+  //
+  // The target Service will be able to receive these events until this
+  // ServiceBinding is either unbound or destroyed.
+  //
+  // If |request| is invalid, this call does nothing.
+  //
+  // Must only be called on an unbound ServiceBinding.
+  void Bind(mojom::ServiceRequest request);
+
+  // Asks the Service Manager nicely if it's OK for this service instance to
+  // disappear now. If the Service Manager thinks it's OK, it will sever the
+  // binding's connection, ultimately triggering an |OnDisconnected()| call on
+  // the bound Service object.
+  //
+  // Must only be called on a bound ServiceBinding.
+  void RequestClose();
+
+  // Immediately severs the connection to the Service Manager. No further
+  // incoming interface requests will be received until this ServiceBinding is
+  // bound again. Always prefer |RequestClose()| under normal circumstances,
+  // unless |OnDisconnected()| has already been invoked on the Service. See the
+  // note in the class documentation above regarding graceful binding closure.
+  //
+  // Must only be called on a bound ServiceBinding.
+  void Close();
+
+ private:
+  void OnConnectionError();
+
+  // mojom::Service:
+  void OnStart(const Identity& identity, OnStartCallback callback) override;
+  void OnBindInterface(const BindSourceInfo& source_info,
+                       const std::string& interface_name,
+                       mojo::ScopedMessagePipeHandle interface_pipe,
+                       OnBindInterfaceCallback callback) override;
+
+  // The Service instance to which all incoming events from the Service Manager
+  // should be directed. Typically this is the object which owns this
+  // ServiceBinding.
+  service_manager::Service* const service_;
+
+  // A pending Connector request which will eventually be passed to the Service
+  // Manager. Created preemptively by every unbound ServiceBinding so that
+  // |connector()| may begin pipelining outgoing requests even before the
+  // ServiceBinding is bound to a ServiceRequest.
+  mojom::ConnectorRequest pending_connector_request_;
+
+  mojo::Binding<mojom::Service> binding_;
+  Identity identity_;
+  std::unique_ptr<Connector> connector_;
+
+  // This instance's control interface to the service manager. Note that this
+  // is unbound and therefore invalid until OnStart() is called.
+  mojom::ServiceControlAssociatedPtr service_control_;
+
+  // Tracks whether |RequestClose()| has been called at least once prior to
+  // receiving |OnStart()| on a bound ServiceBinding. This ensures that the
+  // closure request is actually issued once |OnStart()| is invoked.
+  bool request_closure_on_start_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(ServiceBinding);
+};
+
+}  // namespace service_manager
+
+#endif  // SERVICES_SERVICE_MANAGER_PUBLIC_CPP_SERVICE_CONTEXT_H_

services/service_manager/tests/connect/connect_test_package.cc

@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/macros.h"
+#include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/threading/simple_thread.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
@@ -17,8 +18,7 @@
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/cpp/service.h"
-#include "services/service_manager/public/cpp/service_context.h"
-#include "services/service_manager/public/cpp/service_runner.h"
+#include "services/service_manager/public/cpp/service_binding.h"
 #include "services/service_manager/public/mojom/service_factory.mojom.h"
 #include "services/service_manager/tests/connect/connect_test.mojom.h"
 
@@ -74,6 +74,7 @@ class ProvidedService : public Service,
     registry_.AddInterface<test::mojom::UserIdTest>(base::Bind(
         &ProvidedService::BindUserIdTestRequest, base::Unretained(this)));
   }
+
   void OnBindInterface(const BindSourceInfo& source_info,
                        const std::string& interface_name,
                        mojo::ScopedMessagePipeHandle interface_pipe) override {
@@ -81,6 +82,11 @@ class ProvidedService : public Service,
                             source_info);
   }
 
+  void OnDisconnected() override {
+    service_binding_.Close();
+    run_loop_->Quit();
+  }
+
   void BindConnectTestServiceRequest(
       test::mojom::ConnectTestServiceRequest request,
       const BindSourceInfo& source_info) {
@@ -88,10 +94,11 @@ class ProvidedService : public Service,
     test::mojom::ConnectionStatePtr state(test::mojom::ConnectionState::New());
     state->connection_remote_name = source_info.identity.name();
     state->connection_remote_userid = source_info.identity.user_id();
-    state->initialize_local_name = context()->identity().name();
-    state->initialize_userid = context()->identity().user_id();
+    state->initialize_local_name = service_binding_.identity().name();
+    state->initialize_userid = service_binding_.identity().user_id();
 
-    context()->connector()->BindInterface(source_info.identity, &caller_);
+    service_binding_.GetConnector()->BindInterface(source_info.identity,
+                                                   &caller_);
     caller_->ConnectionAccepted(std::move(state));
   }
 
@@ -111,7 +118,7 @@ class ProvidedService : public Service,
   }
 
   void GetInstance(GetInstanceCallback callback) override {
-    std::move(callback).Run(context()->identity().instance());
+    std::move(callback).Run(service_binding_.identity().instance());
   }
 
   // test::mojom::BlockedInterface:
@@ -123,12 +130,12 @@ class ProvidedService : public Service,
   void ConnectToClassAppAsDifferentUser(
       const service_manager::Identity& target,
       ConnectToClassAppAsDifferentUserCallback callback) override {
-    context()->connector()->StartService(target);
+    service_binding_.GetConnector()->StartService(target);
     mojom::ConnectResult result;
     Identity resolved_identity;
     {
       base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
-      Connector::TestApi test_api(context()->connector());
+      Connector::TestApi test_api(service_binding_.GetConnector());
       test_api.SetStartServiceCallback(
           base::Bind(&QuitLoop, &loop, &result, &resolved_identity));
       loop.Run();
@@ -138,8 +145,13 @@ class ProvidedService : public Service,
 
   // base::SimpleThread:
   void Run() override {
-    ServiceRunner(new ForwardingService(this)).Run(
-        request_.PassMessagePipe().release().value(), false);
+    base::MessageLoop message_loop;
+    base::RunLoop run_loop;
+    run_loop_ = &run_loop;
+    service_binding_.Bind(std::move(request_));
+    run_loop.Run();
+    run_loop_ = nullptr;
+
     caller_.reset();
     bindings_.CloseAllBindings();
     blocked_bindings_.CloseAllBindings();
@@ -147,10 +159,15 @@ class ProvidedService : public Service,
   }
 
   void OnConnectionError() {
-    if (bindings_.empty())
-      context()->QuitNow();
+    if (bindings_.empty()) {
+      if (service_binding_.is_bound())
+        service_binding_.Close();
+      run_loop_->Quit();
+    }
   }
 
+  base::RunLoop* run_loop_;
+  service_manager::ServiceBinding service_binding_{this};
   const std::string title_;
   mojom::ServiceRequest request_;
   test::mojom::ExposedInterfacePtr caller_;
@@ -166,8 +183,11 @@ class ConnectTestService : public Service,
                            public mojom::ServiceFactory,
                            public test::mojom::ConnectTestService {
  public:
-  ConnectTestService() {}
-  ~ConnectTestService() override {}
+  ConnectTestService(service_manager::mojom::ServiceRequest request,
+                     base::OnceClosure quit_closure)
+      : service_binding_(this, std::move(request)),
+        quit_closure_(std::move(quit_closure)) {}
+  ~ConnectTestService() override = default;
 
  private:
   // service_manager::Service:
@@ -184,15 +204,16 @@ class ConnectTestService : public Service,
         base::Bind(&ConnectTestService::BindConnectTestServiceRequest,
                    base::Unretained(this)));
   }
+
   void OnBindInterface(const BindSourceInfo& source_info,
                        const std::string& interface_name,
                        mojo::ScopedMessagePipeHandle interface_pipe) override {
     registry_.BindInterface(interface_name, std::move(interface_pipe));
   }
 
-  bool OnServiceManagerConnectionLost() override {
+  void OnDisconnected() override {
     provided_services_.clear();
-    return true;
+    std::move(quit_closure_).Run();
   }
 
   void BindServiceFactoryRequest(mojom::ServiceFactoryRequest request) {
@@ -223,14 +244,16 @@ class ConnectTestService : public Service,
   }
 
   void GetInstance(GetInstanceCallback callback) override {
-    std::move(callback).Run(context()->identity().instance());
+    std::move(callback).Run(service_binding_.identity().instance());
   }
 
   void OnConnectionError() {
     if (bindings_.empty() && service_factory_bindings_.empty())
-      context()->CreateQuitClosure().Run();
+      service_binding_.RequestClose();
   }
 
+  service_manager::ServiceBinding service_binding_;
+  base::OnceClosure quit_closure_;
   std::vector<std::unique_ptr<Service>> delegates_;
   mojo::BindingSet<mojom::ServiceFactory> service_factory_bindings_;
   BinderRegistry registry_;
@@ -243,7 +266,12 @@ class ConnectTestService : public Service,
 }  // namespace service_manager
 
 MojoResult ServiceMain(MojoHandle service_request_handle) {
-  service_manager::ServiceRunner runner(
-      new service_manager::ConnectTestService);
-  return runner.Run(service_request_handle);
+  base::MessageLoop message_loop;
+  base::RunLoop run_loop;
+  service_manager::ConnectTestService service(
+      service_manager::mojom::ServiceRequest(mojo::MakeScopedHandle(
+          mojo::MessagePipeHandle(service_request_handle))),
+      run_loop.QuitClosure());
+  run_loop.Run();
+  return MOJO_RESULT_OK;
 }

services/service_manager/tests/lifecycle/app.cc

@@ -2,11 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "services/service_manager/public/c/main.h"
-#include "services/service_manager/public/cpp/service_runner.h"
+#include "services/service_manager/public/mojom/service.mojom.h"
 #include "services/service_manager/tests/lifecycle/app_client.h"
 
 MojoResult ServiceMain(MojoHandle service_request_handle) {
-  service_manager::ServiceRunner runner(new service_manager::test::AppClient);
-  return runner.Run(service_request_handle);
+  base::MessageLoop message_loop;
+  base::RunLoop run_loop;
+  service_manager::test::AppClient app_client(
+      service_manager::mojom::ServiceRequest(mojo::MakeScopedHandle(
+          mojo::MessagePipeHandle(service_request_handle))),
+      run_loop.QuitClosure());
+  run_loop.Run();
+  return MOJO_RESULT_OK;
 }

services/service_manager/tests/lifecycle/app_client.cc

@@ -6,14 +6,20 @@
 
 #include "base/macros.h"
 #include "base/run_loop.h"
-#include "services/service_manager/public/cpp/service_context.h"
+#include "services/service_manager/public/cpp/service_binding.h"
 
 namespace service_manager {
 namespace test {
 
-AppClient::AppClient() {
+AppClient::AppClient(service_manager::mojom::ServiceRequest request,
+                     base::OnceClosure quit_closure)
+    : service_binding_(this, std::move(request)),
+      quit_closure_(std::move(quit_closure)) {
+  bindings_.set_connection_error_handler(base::BindRepeating(
+      &AppClient::LifecycleControlBindingLost, base::Unretained(this)));
+
   registry_.AddInterface<mojom::LifecycleControl>(
-      base::Bind(&AppClient::Create, base::Unretained(this)));
+      base::BindRepeating(&AppClient::Create, base::Unretained(this)));
 }
 
 AppClient::~AppClient() {}
@@ -24,9 +30,12 @@ void AppClient::OnBindInterface(const BindSourceInfo& source_info,
   registry_.BindInterface(interface_name, std::move(interface_pipe));
 }
 
-bool AppClient::OnServiceManagerConnectionLost() {
-  context()->QuitNow();
-  return true;
+void AppClient::OnDisconnected() {
+  DCHECK(service_binding_.is_bound());
+  service_binding_.Close();
+
+  if (quit_closure_)
+    std::move(quit_closure_).Run();
 }
 
 void AppClient::Create(mojom::LifecycleControlRequest request) {
@@ -38,7 +47,10 @@ void AppClient::Ping(PingCallback callback) {
 }
 
 void AppClient::GracefulQuit() {
-  context()->CreateQuitClosure().Run();
+  if (service_binding_.is_bound())
+    service_binding_.RequestClose();
+  else if (quit_closure_)
+    std::move(quit_closure_).Run();
 }
 
 void AppClient::Crash() {
@@ -49,14 +61,13 @@ void AppClient::Crash() {
 }
 
 void AppClient::CloseServiceManagerConnection() {
-  context()->DisconnectFromServiceManager();
-  bindings_.set_connection_error_handler(
-      base::Bind(&AppClient::BindingLost, base::Unretained(this)));
+  if (service_binding_.is_bound())
+    service_binding_.Close();
 }
 
-void AppClient::BindingLost() {
-  if (bindings_.empty())
-    OnServiceManagerConnectionLost();
+void AppClient::LifecycleControlBindingLost() {
+  if (!service_binding_.is_bound() && bindings_.empty() && quit_closure_)
+    std::move(quit_closure_).Run();
 }
 
 }  // namespace test

services/service_manager/tests/lifecycle/app_client.h

@@ -8,11 +8,12 @@
 #include <memory>
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/macros.h"
 #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_runner.h"
+#include "services/service_manager/public/cpp/service_binding.h"
 #include "services/service_manager/public/mojom/service.mojom.h"
 #include "services/service_manager/tests/lifecycle/lifecycle_unittest.mojom.h"
 
@@ -22,16 +23,15 @@ namespace test {
 class AppClient : public Service,
                   public mojom::LifecycleControl {
  public:
-  AppClient();
+  explicit AppClient(service_manager::mojom::ServiceRequest request,
+                     base::OnceClosure quit_closure);
   ~AppClient() override;
 
-  void set_runner(ServiceRunner* runner) { runner_ = runner; }
-
   // Service:
   void OnBindInterface(const BindSourceInfo& source_info,
                        const std::string& interface_name,
                        mojo::ScopedMessagePipeHandle interface_pipe) override;
-  bool OnServiceManagerConnectionLost() override;
+  void OnDisconnected() override;
 
   void Create(mojom::LifecycleControlRequest request);
 
@@ -44,9 +44,11 @@ class AppClient : public Service,
  private:
   class ServiceImpl;
 
-  void BindingLost();
+  void LifecycleControlBindingLost();
+
+  ServiceBinding service_binding_;
+  base::OnceClosure quit_closure_;
 
-  ServiceRunner* runner_ = nullptr;
   BinderRegistry registry_;
   mojo::BindingSet<mojom::LifecycleControl> bindings_;
 

services/service_manager/tests/lifecycle/lifecycle_exe.cc

@@ -2,11 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "services/service_manager/public/c/main.h"
-#include "services/service_manager/public/cpp/service_runner.h"
+#include "services/service_manager/public/mojom/service.mojom.h"
 #include "services/service_manager/tests/lifecycle/app_client.h"
 
 MojoResult ServiceMain(MojoHandle service_request_handle) {
-  service_manager::ServiceRunner runner(new service_manager::test::AppClient);
-  return runner.Run(service_request_handle);
+  base::MessageLoop message_loop;
+  base::RunLoop run_loop;
+  service_manager::test::AppClient app_client(
+      service_manager::mojom::ServiceRequest(mojo::MakeScopedHandle(
+          mojo::MessagePipeHandle(service_request_handle))),
+      run_loop.QuitClosure());
+  run_loop.Run();
+  return MOJO_RESULT_OK;
 }

services/service_manager/tests/lifecycle/package.cc

@@ -7,12 +7,12 @@
 
 #include "base/bind.h"
 #include "base/macros.h"
+#include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "services/service_manager/public/c/main.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
-#include "services/service_manager/public/cpp/service_context.h"
-#include "services/service_manager/public/cpp/service_runner.h"
+#include "services/service_manager/public/cpp/service_binding.h"
 #include "services/service_manager/public/mojom/service_factory.mojom.h"
 #include "services/service_manager/tests/lifecycle/app_client.h"
 #include "services/service_manager/tests/lifecycle/lifecycle_unittest.mojom.h"
@@ -22,19 +22,20 @@ namespace {
 class PackagedApp : public service_manager::Service,
                     public service_manager::test::mojom::LifecycleControl {
  public:
-  PackagedApp(
-      const base::Closure& service_manager_connection_closed_callback,
-      const base::Closure& destruct_callback)
-      : service_manager_connection_closed_callback_(
-            service_manager_connection_closed_callback),
-        destruct_callback_(destruct_callback) {
-    bindings_.set_connection_error_handler(base::Bind(&PackagedApp::BindingLost,
-                                                      base::Unretained(this)));
+  PackagedApp(service_manager::mojom::ServiceRequest request,
+              base::OnceClosure service_manager_connection_closed_callback,
+              base::OnceClosure destruct_callback)
+      : service_binding_(this, std::move(request)),
+        service_manager_connection_closed_callback_(
+            std::move(service_manager_connection_closed_callback)),
+        destruct_callback_(std::move(destruct_callback)) {
+    bindings_.set_connection_error_handler(
+        base::BindRepeating(&PackagedApp::MaybeQuit, base::Unretained(this)));
     registry_.AddInterface<service_manager::test::mojom::LifecycleControl>(
         base::Bind(&PackagedApp::Create, base::Unretained(this)));
   }
 
-  ~PackagedApp() override {}
+  ~PackagedApp() override = default;
 
  private:
   // service_manager::Service:
@@ -44,6 +45,11 @@ class PackagedApp : public service_manager::Service,
     registry_.BindInterface(interface_name, std::move(interface_pipe));
   }
 
+  void OnDisconnected() override {
+    std::move(service_manager_connection_closed_callback_).Run();
+    std::move(destruct_callback_).Run();
+  }
+
   void Create(service_manager::test::mojom::LifecycleControlRequest request) {
     bindings_.AddBinding(this, std::move(request));
   }
@@ -51,12 +57,7 @@ class PackagedApp : public service_manager::Service,
   // LifecycleControl:
   void Ping(PingCallback callback) override { std::move(callback).Run(); }
 
-  void GracefulQuit() override {
-    service_manager_connection_closed_callback_.Run();
-
-    // Deletes |this|.
-    destruct_callback_.Run();
-  }
+  void GracefulQuit() override { service_binding_.RequestClose(); }
 
   void Crash() override {
     // When multiple instances are vended from the same package instance, this
@@ -65,50 +66,68 @@ class PackagedApp : public service_manager::Service,
   }
 
   void CloseServiceManagerConnection() override {
-    service_manager_connection_closed_callback_.Run();
-    context()->QuitNow();
+    std::move(service_manager_connection_closed_callback_).Run();
+
+    if (service_binding_.is_bound())
+      service_binding_.Close();
+
     // This only closed our relationship with the service manager, existing
     // |bindings_| remain active.
+    MaybeQuit();
   }
 
-  void BindingLost() {
-    if (bindings_.empty()) {
-      // Deletes |this|.
-      destruct_callback_.Run();
-    }
+  void MaybeQuit() {
+    if (service_binding_.is_bound() || !bindings_.empty())
+      return;
+
+    // Deletes |this|.
+    std::move(destruct_callback_).Run();
   }
 
+  service_manager::ServiceBinding service_binding_;
+
   service_manager::BinderRegistry registry_;
   mojo::BindingSet<service_manager::test::mojom::LifecycleControl> bindings_;
 
   // Run when this object's connection to the service manager is closed.
-  base::Closure service_manager_connection_closed_callback_;
+  base::OnceClosure service_manager_connection_closed_callback_;
   // Run when this object is destructed.
-  base::Closure destruct_callback_;
+  base::OnceClosure destruct_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(PackagedApp);
 };
 
-class Package : public service_manager::ForwardingService,
+class Package : public service_manager::Service,
                 public service_manager::mojom::ServiceFactory {
  public:
-  Package() : ForwardingService(&app_client_) {
+  explicit Package(service_manager::mojom::ServiceRequest request,
+                   base::OnceClosure quit_closure)
+      : service_binding_(this, std::move(request)),
+        quit_closure_(std::move(quit_closure)),
+        app_client_(service_manager::mojom::ServiceRequest(),
+                    base::BindOnce(&Package::QuitFromAppClient,
+                                   base::Unretained(this))) {
     registry_.AddInterface<service_manager::mojom::ServiceFactory>(
-        base::Bind(&Package::Create, base::Unretained(this)));
+        base::BindRepeating(&Package::Create, base::Unretained(this)));
   }
-  ~Package() override {}
+
+  ~Package() override = default;
 
  private:
-  // ForwardingService:
+  void QuitFromAppClient() { std::move(quit_closure_).Run(); }
+
+  // service_manager::Service:
   void OnBindInterface(const service_manager::BindSourceInfo& source_info,
                        const std::string& interface_name,
                        mojo::ScopedMessagePipeHandle interface_pipe) override {
     if (!registry_.TryBindInterface(interface_name, &interface_pipe)) {
-      ForwardingService::OnBindInterface(source_info, interface_name,
-                                         std::move(interface_pipe));
+      app_client_.OnBindInterface(source_info, interface_name,
+                                  std::move(interface_pipe));
     }
   }
 
+  void OnDisconnected() override { std::move(quit_closure_).Run(); }
+
   void Create(service_manager::mojom::ServiceFactoryRequest request) {
     bindings_.AddBinding(this, std::move(request));
   }
@@ -120,48 +139,35 @@ class Package : public service_manager::ForwardingService,
       service_manager::mojom::PIDReceiverPtr pid_receiver) override {
     ++service_manager_connection_refcount_;
     int id = next_id_++;
-    std::unique_ptr<service_manager::ServiceContext> context =
-        std::make_unique<service_manager::ServiceContext>(
-            std::make_unique<PackagedApp>(
-                base::Bind(&Package::AppServiceManagerConnectionClosed,
-                           base::Unretained(this)),
-                base::Bind(&Package::DestroyService, base::Unretained(this),
-                           id)),
-            std::move(request));
-    service_manager::ServiceContext* raw_context = context.get();
-    contexts_.insert(std::make_pair(raw_context, std::move(context)));
-    id_to_context_.insert(std::make_pair(id, raw_context));
+    auto app = std::make_unique<PackagedApp>(
+        std::move(request),
+        base::BindOnce(&Package::OnAppInstanceDisconnected,
+                       base::Unretained(this)),
+        base::BindOnce(&Package::DestroyAppInstance, base::Unretained(this),
+                       id));
+    app_instances_.emplace(id, std::move(app));
   }
 
-  void AppServiceManagerConnectionClosed() {
-    if (!--service_manager_connection_refcount_)
-      app_client_.CloseServiceManagerConnection();
+  void OnAppInstanceDisconnected() {
+    if (--service_manager_connection_refcount_ == 0)
+      service_binding_.RequestClose();
   }
 
-  void DestroyService(int id) {
-    auto id_it = id_to_context_.find(id);
-    DCHECK(id_it != id_to_context_.end());
-
-    auto it = contexts_.find(id_it->second);
-    DCHECK(it != contexts_.end());
-    contexts_.erase(it);
-    id_to_context_.erase(id_it);
-    if (contexts_.empty() && base::RunLoop::IsRunningOnCurrentThread())
-      context()->QuitNow();
+  void DestroyAppInstance(int id) {
+    app_instances_.erase(id);
+    if (app_instances_.empty())
+      std::move(quit_closure_).Run();
   }
 
+  service_manager::ServiceBinding service_binding_;
+  base::OnceClosure quit_closure_;
   service_manager::test::AppClient app_client_;
   int service_manager_connection_refcount_ = 0;
   service_manager::BinderRegistry registry_;
   mojo::BindingSet<service_manager::mojom::ServiceFactory> bindings_;
 
-  using ServiceContextMap =
-      std::map<service_manager::ServiceContext*,
-               std::unique_ptr<service_manager::ServiceContext>>;
-  ServiceContextMap contexts_;
-
   int next_id_ = 0;
-  std::map<int, service_manager::ServiceContext*> id_to_context_;
+  std::map<int, std::unique_ptr<PackagedApp>> app_instances_;
 
   DISALLOW_COPY_AND_ASSIGN(Package);
 };
@@ -169,6 +175,11 @@ class Package : public service_manager::ForwardingService,
 }  // namespace
 
 MojoResult ServiceMain(MojoHandle service_request_handle) {
-  service_manager::ServiceRunner runner(new Package);
-  return runner.Run(service_request_handle);
+  base::MessageLoop message_loop;
+  base::RunLoop run_loop;
+  Package package(service_manager::mojom::ServiceRequest(mojo::MakeScopedHandle(
+                      mojo::MessagePipeHandle(service_request_handle))),
+                  run_loop.QuitClosure());
+  run_loop.Run();
+  return MOJO_RESULT_OK;
 }