Eliminate Connector::Connect(), Connection, etc.

services/service_manager/public/interfaces/connector.mojom

Index: services/service_manager/public/interfaces/connector.mojom
diff --git a/services/service_manager/public/interfaces/connector.mojom b/services/service_manager/public/interfaces/connector.mojom
index 3c759cbbe918aed432832f5f394aaecb49319f67..7d5a9fb22ba8319bbab579d34bf29fff3846f94f 100644
--- a/services/service_manager/public/interfaces/connector.mojom
+++ b/services/service_manager/public/interfaces/connector.mojom
@@ -6,50 +6,51 @@ module service_manager.mojom;
 
 import "services/service_manager/public/interfaces/interface_provider.mojom";
 
+// TODO(beng): Determine who (if anyone) uses kRootUserID.
 const string kRootUserID = "505C0EE9-3013-43C0-82B0-A84F50CF8D84";
 const string kInheritUserID = "D26290E4-4485-4EAE-81A2-66D1EEB40A9D";
 
 const uint32 kInvalidInstanceID = 0;
 
+// TODO(beng): Evalute the utility of this enum. There are some inconsistencies
+//             in its use with BindInterface/StartService.
 enum ConnectResult {
-  // The connection was established successfully.
+  // The operation was established successfully.
   SUCCEEDED,
 
   // The name or user id supplied was malformed, or the service specified by
   // |name| could not be loaded.
   INVALID_ARGUMENT,
 
-  // The connection was blocked by policy. Either connections to |name| are
-  // forbidden from this app by the CapabilityFilter, or the service attempted
-  // to connect using a user id other than its own, |kInheritUserID| or
-  // |kRootUserID|.
+  // Policy prevented the successful completion of this operation. Either
+  // requests to bind to |name| are forbidden from the calling service by its
+  // manifest, or the service attempted to connect using a user id other than
+  // its own, |kInheritUserID| or |kRootUserID|.
   ACCESS_DENIED
 };
 
 // A collection of metadata that disambiguates instances in the service manager.
 struct Identity {
-  // A service: or exe: name identifying a service.
+  // A name identifying a service.
   string name;
 
-  // The user id of the target service instance to connect to. If no such
-  // instance exists, the service manager may start one. This user id will be
-  // passed to the new instance via Initialize().
-  //
-  // When connecting to other services, services must generally pass
-  // kInheritUserID for this value, and the service manager will either connect
-  // to an existing instance matching the caller's user id, create a new
-  // instance matching the caller's user id, or connect to an existing instance
-  // running as kRootUserID. By default, services do not have the ability to set
-  // arbitrary values to this field, and doing so will result in a connection
-  // error on the remote service provider.
+  // The user id of the target service instance to bind to. If no such instance
+  // exists, the service manager may start one. This user id will be passed to
+  // the new instance via Initialize().
+  //
+  // When binding to other services, services must generally pass kInheritUserID
+  // for this value, and the service manager will either bind to an existing
+  // instance matching the caller's user id, create a new instance matching the
+  // caller's user id, or bind to an existing instance running as kRootUserID.
+  // By default, services do not have the ability to set arbitrary values to
+  // this field, and doing so will result in an error response.
   //
   // A service with the ability to launch other services with arbitrary user ids
-  // (e.g. a login service) may set this  value to something meaningful to it.
-  // The user id string is a valid guid of the form
-  // "%08X-%04X-%04X-%04X-%012llX", and (aside from the root user whose
-  // guid is defined above) intended to be not-guessable.
+  // (e.g. a login service) may set this value. The user id string is a valid
+  // guid of the form "%08X-%04X-%04X-%04X-%012llX", and (aside from the root
+  // user whose guid is defined above) intended to be not-guessable.
   //
-  // When a service is initialized or receives a connection from another
+  // When a service is initialized or receives a bind request from another
   // service, this value is always the resolved user id, never |kInheritUserID|.
   string user_id;
 
@@ -67,76 +68,99 @@ interface PIDReceiver {
   SetPID(uint32 pid);
 };
 
-// Encapsulates establishing connections with other Services.
+// An interface that allows the holder to start other services & bind to
+// interfaces exposed by them.
 interface Connector {
-  // Typically, the service manager will start a process for a service the first
-  // time it receives a connection request for it. This struct allows a client
-  // to start the process itself and provide the service manager the pipes it
-  // needs to communicate with it. When this function is called, the client owns
-  // the lifetime of the child process it started, not the service manager. The
-  // service manager binds the |service| pipe, and when it closes destroys the
-  // associated  instance but the process stays alive.
+  // Asks the service manager to route a request to bind an implementation of
+  // the interface to a named service instance.
+  //
+  // A service's ability to bind interfaces exposed by another is controlled by
+  // policy set out in each service's manifest. See
+  // //services/service_manager/README.md for more information on manifests.
+  // If policy prevents the requesting service from binding the specified
+  // interface, the request pipe will be closed.
   //
   // Parameters:
   //
-  //  service
-  //    A pipe to an implementation of Service that the service manager can use
-  //    to communicate with the service.
+  //  target
+  //    The identity of the service instance to route the request to. If no
+  //    instance exists, the service will be started.
   //
-  //  pid_receiver_request
-  //   Allows the client process launcher to tell the service manager the PID of
-  //   the process it created (the pid isn't supplied directly here as the
-  //   process may not have been launched by the time Connect() is called.)
+  //  interface_name
+  //    The name of the interface to be bound. If the target service does not
+  //    expose an interface of this name, the request pipe will be closed.
   //
-  StartService(Identity name,
-               handle<message_pipe> service,
-               PIDReceiver& pid_receiver_request);
-
-  // Requests a connection with another service. The service originating the
-  // request is referred to as the "source" and the one receiving the "target".
+  //  interface_pipe
+  //    A message pipe endpoint encapsulating a request for an interface named
+  //    |interface_name|.
+  //
+  // Response parameters:
+  //
+  //  result
+  //    Indicates the result of the BindInterface() operation.
   //
-  // The connection is embodied by a pair of message pipes binding the
-  // InterfaceProvider interface, which allows both the source and target
-  // services to export interfaces to one another. The interfaces bound via
-  // these InterfaceProviders are brokered by the service manager according to
-  // the security policy defined by each service in its manifest.
+  //  identity
+  //    The fully resolved identity of the instance in the service manager, with
+  //    a resolved user id. Typically the client passes |kInheritUserID| as the
+  //    user id to BindInterface(), which will be resolved by the service manager

[Tom Sepez, 2017/04/12 16:18:48]

nit:80 cols

+  //    into a concrete user id.
   //
-  // If the target service is not running, the service manager will run it,
-  // calling its OnStart() method before completing the connection.
+  BindInterface(Identity target,
+                string interface_name,
+                handle<message_pipe> interface_pipe) =>
+      (ConnectResult result, Identity user_id);
+
+  // Asks the service manager to create an instance for a service. No action is
+  // taken if an instance is already present. If the service is not yet running,
+  // it will be initialized and its OnStart() method will be called. A process
+  // may be allocated.
   //
   // Parameters:
   //
   //  target
-  //    Identifies the target service instance to connect to.
-  //
-  //  remote_interfaces
-  //    Allows the source service access to interface implementations exposed by
-  //    the target service. The interfaces accessible via this InterfaceProvider
-  //    are filtered by the security policy described by the source and target
-  //    service manifests.
+  //    The identity of the service to start.
   //
   // Response parameters:
   //
   //  result
-  //    Indicates the result of the Connect() operation.
+  //    Indicates the result of the StartService() operation.
   //
-  //  user_id
-  //    The user id the service manager ran the target service as. Typically a
-  //    client passes |kInheritUserID| as the user id to Connect(), which is
-  //    resolved by the service manager into a valid user id returned through
-  //    this callback.
+  //  identity
+  //    The fully resolved identity of the instance in the service manager, with
+  //    a resolved user id. Typically the client passes |kInheritUserID| as the
+  //    user id to BindInterface(), which will be resolved by the service manager
+  //    into a concrete user id.
   //
-  Connect(Identity target, InterfaceProvider&? remote_interfaces) =>
-      (ConnectResult result, string user_id);
+  StartService(Identity target) => (ConnectResult result, Identity identity);
 
-  // Variant of Connect() above. Will (gradually) replace it. Think of this like
-  // a combination of Connect() and InterfaceProvider::GetInteface() - requests
-  // a connection to a service and binds an interface in one step.
-  // TODO(beng): Update this comment once the implementation is complete.
-  BindInterface(Identity target,
-                string interface_name,
-                handle<message_pipe> interface_pipe) =>
-      (ConnectResult result, string user_id);
+  // Typically, the service manager will start a process for a service the first
+  // time it receives a bind interface request for it, or when StartService() is
+  // called. This struct allows a client to start the process itself and provide
+  // the service manager the pipes it needs to communicate with it. When this
+  // function is called, the client owns the lifetime of the child process it
+  // started, not the service manager. The service manager binds the |service|
+  // pipe, and when it closes destroys the associated instance but the process
+  // stays alive.
+  //
+  // Parameters:
+  //
+  //  target
+  //    The identity of the service to create the instance for.
+  //
+  //  service
+  //    A pipe to an implementation of Service that the service manager can use
+  //    to communicate with the service.
+  //
+  //  pid_receiver_request
+  //   Allows the client process launcher to tell the service manager the PID of
+  //   the process it created (the pid isn't supplied directly here as the
+  //   process may not have been launched by the time BindInterface() is
+  //   called.)
+  //
+  StartServiceWithProcess(Identity target,
+                          handle<message_pipe> service,
+                          PIDReceiver& pid_receiver_request) =>
+      (ConnectResult result, Identity identity);
 
   // Clones this Connector so it can be passed to another thread.
   Clone(Connector& request);