Revert "[service-manager] Introduce ServiceBinding"

This reverts commit cf9baf41.

Reason for revert: Seemed to cause test failures on Fuchsia https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Fuchsia%20x64/20283

Original change's description:
> [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/1259485
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Commit-Queue: Ken Rockot <rockot@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597324}

TBR=rockot@chromium.org,reillyg@chromium.org

Change-Id: Ib638e97e13b4d0ab24b0123323ff9941da6eb1fc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 891780
Reviewed-on: https://chromium-review.googlesource.com/c/1266135Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597362}

services/service_manager/public/cpp/BUILD.gn

@@ -17,8 +17,6 @@ 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",
@@ -39,12 +37,7 @@ component("cpp") {
     "//url",
   ]
 
-  defines = [
-    "IS_SERVICE_MANAGER_CPP_IMPL",
-
-    # TODO: Use COMPONENT_EXPORT everywhere here and remove this.
-    "SERVICE_MANAGER_PUBLIC_CPP_IMPL",
-  ]
+  defines = [ "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,8 +19,6 @@ 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 COMPONENT_EXPORT(SERVICE_MANAGER_CPP) Service {
+class SERVICE_MANAGER_PUBLIC_CPP_EXPORT Service {
  public:
   Service();
   virtual ~Service();
@@ -37,14 +37,6 @@ class COMPONENT_EXPORT(SERVICE_MANAGER_CPP) 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.
@@ -57,9 +49,6 @@ class COMPONENT_EXPORT(SERVICE_MANAGER_CPP) 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:
@@ -83,7 +72,7 @@ class COMPONENT_EXPORT(SERVICE_MANAGER_CPP) 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 COMPONENT_EXPORT(SERVICE_MANAGER_CPP) ForwardingService : public Service {
+class SERVICE_MANAGER_PUBLIC_CPP_EXPORT 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,7 +10,6 @@
 
 #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"
@@ -18,7 +17,8 @@
 #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_binding.h"
+#include "services/service_manager/public/cpp/service_context.h"
+#include "services/service_manager/public/cpp/service_runner.h"
 #include "services/service_manager/public/mojom/service_factory.mojom.h"
 #include "services/service_manager/tests/connect/connect_test.mojom.h"
 
@@ -74,7 +74,6 @@ 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 {
@@ -82,11 +81,6 @@ 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) {
@@ -94,11 +88,10 @@ 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 = service_binding_.identity().name();
-    state->initialize_userid = service_binding_.identity().user_id();
+    state->initialize_local_name = context()->identity().name();
+    state->initialize_userid = context()->identity().user_id();
 
-    service_binding_.GetConnector()->BindInterface(source_info.identity,
-                                                   &caller_);
+    context()->connector()->BindInterface(source_info.identity, &caller_);
     caller_->ConnectionAccepted(std::move(state));
   }
 
@@ -118,7 +111,7 @@ class ProvidedService : public Service,
   }
 
   void GetInstance(GetInstanceCallback callback) override {
-    std::move(callback).Run(service_binding_.identity().instance());
+    std::move(callback).Run(context()->identity().instance());
   }
 
   // test::mojom::BlockedInterface:
@@ -130,12 +123,12 @@ class ProvidedService : public Service,
   void ConnectToClassAppAsDifferentUser(
       const service_manager::Identity& target,
       ConnectToClassAppAsDifferentUserCallback callback) override {
-    service_binding_.GetConnector()->StartService(target);
+    context()->connector()->StartService(target);
     mojom::ConnectResult result;
     Identity resolved_identity;
     {
       base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
-      Connector::TestApi test_api(service_binding_.GetConnector());
+      Connector::TestApi test_api(context()->connector());
       test_api.SetStartServiceCallback(
           base::Bind(&QuitLoop, &loop, &result, &resolved_identity));
       loop.Run();
@@ -145,13 +138,8 @@ class ProvidedService : public Service,
 
   // base::SimpleThread:
   void Run() override {
-    base::MessageLoop message_loop;
-    base::RunLoop run_loop;
-    run_loop_ = &run_loop;
-    service_binding_.Bind(std::move(request_));
-    run_loop.Run();
-    run_loop_ = nullptr;
-
+    ServiceRunner(new ForwardingService(this)).Run(
+        request_.PassMessagePipe().release().value(), false);
     caller_.reset();
     bindings_.CloseAllBindings();
     blocked_bindings_.CloseAllBindings();
@@ -159,15 +147,10 @@ class ProvidedService : public Service,
   }
 
   void OnConnectionError() {
-    if (bindings_.empty()) {
-      if (service_binding_.is_bound())
-        service_binding_.Close();
-      run_loop_->Quit();
-    }
+    if (bindings_.empty())
+      context()->QuitNow();
   }
 
-  base::RunLoop* run_loop_;
-  service_manager::ServiceBinding service_binding_{this};
   const std::string title_;
   mojom::ServiceRequest request_;
   test::mojom::ExposedInterfacePtr caller_;
@@ -183,11 +166,8 @@ class ConnectTestService : public Service,
                            public mojom::ServiceFactory,
                            public test::mojom::ConnectTestService {
  public:
-  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;
+  ConnectTestService() {}
+  ~ConnectTestService() override {}
 
  private:
   // service_manager::Service:
@@ -204,16 +184,15 @@ 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));
   }
 
-  void OnDisconnected() override {
+  bool OnServiceManagerConnectionLost() override {
     provided_services_.clear();
-    std::move(quit_closure_).Run();
+    return true;
   }
 
   void BindServiceFactoryRequest(mojom::ServiceFactoryRequest request) {
@@ -244,16 +223,14 @@ class ConnectTestService : public Service,
   }
 
   void GetInstance(GetInstanceCallback callback) override {
-    std::move(callback).Run(service_binding_.identity().instance());
+    std::move(callback).Run(context()->identity().instance());
   }
 
   void OnConnectionError() {
     if (bindings_.empty() && service_factory_bindings_.empty())
-      service_binding_.RequestClose();
+      context()->CreateQuitClosure().Run();
   }
 
-  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_;
@@ -266,12 +243,7 @@ class ConnectTestService : public Service,
 }  // namespace service_manager
 
 MojoResult ServiceMain(MojoHandle 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;
+  service_manager::ServiceRunner runner(
+      new service_manager::ConnectTestService);
+  return runner.Run(service_request_handle);
 }

services/service_manager/tests/lifecycle/app.cc

@@ -2,19 +2,11 @@
 // 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/mojom/service.mojom.h"
+#include "services/service_manager/public/cpp/service_runner.h"
 #include "services/service_manager/tests/lifecycle/app_client.h"
 
 MojoResult ServiceMain(MojoHandle 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;
+  service_manager::ServiceRunner runner(new service_manager::test::AppClient);
+  return runner.Run(service_request_handle);
 }

services/service_manager/tests/lifecycle/app_client.cc

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

services/service_manager/tests/lifecycle/app_client.h

@@ -8,12 +8,11 @@
 #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_binding.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/lifecycle_unittest.mojom.h"
 
@@ -23,15 +22,16 @@ namespace test {
 class AppClient : public Service,
                   public mojom::LifecycleControl {
  public:
-  explicit AppClient(service_manager::mojom::ServiceRequest request,
-                     base::OnceClosure quit_closure);
+  AppClient();
   ~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;
-  void OnDisconnected() override;
+  bool OnServiceManagerConnectionLost() override;
 
   void Create(mojom::LifecycleControlRequest request);
 
@@ -44,11 +44,9 @@ class AppClient : public Service,
  private:
   class ServiceImpl;
 
-  void LifecycleControlBindingLost();
-
-  ServiceBinding service_binding_;
-  base::OnceClosure quit_closure_;
+  void BindingLost();
 
+  ServiceRunner* runner_ = nullptr;
   BinderRegistry registry_;
   mojo::BindingSet<mojom::LifecycleControl> bindings_;
 

services/service_manager/tests/lifecycle/lifecycle_exe.cc

@@ -2,19 +2,11 @@
 // 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/mojom/service.mojom.h"
+#include "services/service_manager/public/cpp/service_runner.h"
 #include "services/service_manager/tests/lifecycle/app_client.h"
 
 MojoResult ServiceMain(MojoHandle 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;
+  service_manager::ServiceRunner runner(new service_manager::test::AppClient);
+  return runner.Run(service_request_handle);
 }

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_binding.h"
+#include "services/service_manager/public/cpp/service_context.h"
+#include "services/service_manager/public/cpp/service_runner.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,20 +22,19 @@ namespace {
 class PackagedApp : public service_manager::Service,
                     public service_manager::test::mojom::LifecycleControl {
  public:
-  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)));
+  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)));
     registry_.AddInterface<service_manager::test::mojom::LifecycleControl>(
         base::Bind(&PackagedApp::Create, base::Unretained(this)));
   }
 
-  ~PackagedApp() override = default;
+  ~PackagedApp() override {}
 
  private:
   // service_manager::Service:
@@ -45,11 +44,6 @@ 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));
   }
@@ -57,7 +51,12 @@ class PackagedApp : public service_manager::Service,
   // LifecycleControl:
   void Ping(PingCallback callback) override { std::move(callback).Run(); }
 
-  void GracefulQuit() override { service_binding_.RequestClose(); }
+  void GracefulQuit() override {
+    service_manager_connection_closed_callback_.Run();
+
+    // Deletes |this|.
+    destruct_callback_.Run();
+  }
 
   void Crash() override {
     // When multiple instances are vended from the same package instance, this
@@ -66,68 +65,50 @@ class PackagedApp : public service_manager::Service,
   }
 
   void CloseServiceManagerConnection() override {
-    std::move(service_manager_connection_closed_callback_).Run();
-
-    if (service_binding_.is_bound())
-      service_binding_.Close();
-
+    service_manager_connection_closed_callback_.Run();
+    context()->QuitNow();
     // This only closed our relationship with the service manager, existing
     // |bindings_| remain active.
-    MaybeQuit();
   }
 
-  void MaybeQuit() {
-    if (service_binding_.is_bound() || !bindings_.empty())
-      return;
-
-    // Deletes |this|.
-    std::move(destruct_callback_).Run();
+  void BindingLost() {
+    if (bindings_.empty()) {
+      // Deletes |this|.
+      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::OnceClosure service_manager_connection_closed_callback_;
+  base::Closure service_manager_connection_closed_callback_;
   // Run when this object is destructed.
-  base::OnceClosure destruct_callback_;
+  base::Closure destruct_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(PackagedApp);
 };
 
-class Package : public service_manager::Service,
+class Package : public service_manager::ForwardingService,
                 public service_manager::mojom::ServiceFactory {
  public:
-  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))) {
+  Package() : ForwardingService(&app_client_) {
     registry_.AddInterface<service_manager::mojom::ServiceFactory>(
-        base::BindRepeating(&Package::Create, base::Unretained(this)));
+        base::Bind(&Package::Create, base::Unretained(this)));
   }
-
-  ~Package() override = default;
+  ~Package() override {}
 
  private:
-  void QuitFromAppClient() { std::move(quit_closure_).Run(); }
-
-  // service_manager::Service:
+  // ForwardingService:
   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)) {
-      app_client_.OnBindInterface(source_info, interface_name,
-                                  std::move(interface_pipe));
+      ForwardingService::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));
   }
@@ -139,35 +120,48 @@ class Package : public service_manager::Service,
       service_manager::mojom::PIDReceiverPtr pid_receiver) override {
     ++service_manager_connection_refcount_;
     int id = next_id_++;
-    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));
+    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));
   }
 
-  void OnAppInstanceDisconnected() {
-    if (--service_manager_connection_refcount_ == 0)
-      service_binding_.RequestClose();
+  void AppServiceManagerConnectionClosed() {
+    if (!--service_manager_connection_refcount_)
+      app_client_.CloseServiceManagerConnection();
   }
 
-  void DestroyAppInstance(int id) {
-    app_instances_.erase(id);
-    if (app_instances_.empty())
-      std::move(quit_closure_).Run();
+  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();
   }
 
-  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, std::unique_ptr<PackagedApp>> app_instances_;
+  std::map<int, service_manager::ServiceContext*> id_to_context_;
 
   DISALLOW_COPY_AND_ASSIGN(Package);
 };
@@ -175,11 +169,6 @@ class Package : public service_manager::Service,
 }  // namespace
 
 MojoResult ServiceMain(MojoHandle 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;
+  service_manager::ServiceRunner runner(new Package);
+  return runner.Run(service_request_handle);
 }