Make "all user" services work when packaged.

services/service_manager/service_manager.cc

Index: services/service_manager/service_manager.cc
diff --git a/services/service_manager/service_manager.cc b/services/service_manager/service_manager.cc
index 59ab943054c01b9063faf89e94a788fa706a4e4b..e4c8155ee0568158ef64541a64902933debb082c 100644
--- a/services/service_manager/service_manager.cc
+++ b/services/service_manager/service_manager.cc
@@ -125,6 +125,7 @@ class ServiceManager::Instance
         connection_spec_(connection_spec),
         allow_any_application_(connection_spec.requires.count("*") == 1),
         pid_receiver_binding_(this),
+        state_(State::IDLE),
         weak_factory_(this) {
     if (identity_.name() == kServiceManagerName ||
         identity_.name() == kCatalogName) {
@@ -143,8 +144,12 @@ class ServiceManager::Instance
     connectors_.CloseAllBindings();
     service_manager_bindings_.CloseAllBindings();
 
-    // Notify the ServiceManager that this Instance is really going away.
-    service_manager_->OnInstanceStopped(identity_);
+    if (state_ == State::STARTING) {
+      service_manager_->NotifyServiceFailedToStart(identity_);
+    } else {
+      // Notify the ServiceManager that this Instance is really going away.
+      service_manager_->OnInstanceStopped(identity_);
+    }
 
     // Release |runner_| so that if we are called back to OnRunnerCompleted()
     // we know we're in the destructor.
@@ -152,7 +157,7 @@ class ServiceManager::Instance
     runner.reset();
   }
 
-  Instance* parent() { return parent_; }
+  Instance* parent() const { return parent_; }
 
   void AddChild(std::unique_ptr<Instance> child) {
     child->parent_ = this;
@@ -202,6 +207,7 @@ class ServiceManager::Instance
 
   void StartWithService(mojom::ServicePtr service) {
     CHECK(!service_);
+    state_ = State::STARTING;
     service_ = std::move(service);
     service_.set_connection_error_handler(
         base::Bind(&Instance::OnServiceLost, base::Unretained(this),
@@ -244,6 +250,7 @@ class ServiceManager::Instance
     return connection_spec_;
   }
   const Identity& identity() const { return identity_; }
+  void set_identity(const Identity& identity) { identity_ = identity; }
   uint32_t id() const { return id_; }
 
   // Service:
@@ -259,6 +266,17 @@ class ServiceManager::Instance
   }
 
  private:
+   enum class State {
+    // This connection was not started yet.

[Ken Rockot(use gerrit already), 2016/10/21 19:52:33]

nit: in these comments I think s/connection/instance/ is more accurate

[Jay Civelli, 2016/10/21 20:48:33]

Done.

+    IDLE,
+
+    // The connection was started but no success was reported.
+    STARTING,
+
+    // The connection was started successfully.
+    STARTED
+  };
+
   // mojom::Connector implementation:
   void Connect(const service_manager::Identity& in_target,
                mojom::InterfaceProviderRequest remote_interfaces,
@@ -416,7 +434,6 @@ class ServiceManager::Instance
       return;
     }
     pid_ = pid;
-    service_manager_->NotifyPIDAvailable(identity_, pid_);
   }
 
   void OnServiceLost(
@@ -438,12 +455,14 @@ class ServiceManager::Instance
   }
 
   void OnInitializeResponse(mojom::ConnectorRequest connector_request) {
+    state_ = State::STARTED;
     if (connector_request.is_pending()) {
       connectors_.AddBinding(this, std::move(connector_request));
       connectors_.set_connection_error_handler(
           base::Bind(&Instance::OnConnectionLost, base::Unretained(this),
                      service_manager_->GetWeakPtr()));
     }
+    service_manager_->NotifyServiceStarted(identity_, pid_);
   }
 
   // Callback when NativeRunner completes.
@@ -460,7 +479,7 @@ class ServiceManager::Instance
   // may vend multiple application instances, and this object may exist before a
   // process is launched.
   const uint32_t id_;
-  const Identity identity_;
+  Identity identity_;
   const InterfaceProviderSpec connection_spec_;
   const bool allow_any_application_;
   std::unique_ptr<NativeRunner> runner_;
@@ -471,6 +490,7 @@ class ServiceManager::Instance
   base::ProcessId pid_ = base::kNullProcessId;
   Instance* parent_ = nullptr;
   InstanceMap children_;
+  State state_;
   base::WeakPtrFactory<Instance> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Instance);
@@ -692,14 +712,21 @@ ServiceManager::Instance* ServiceManager::GetExistingInstance(
   return nullptr;
 }
 
-void ServiceManager::NotifyPIDAvailable(const Identity& identity,
-                                        base::ProcessId pid) {
+void ServiceManager::NotifyServiceStarted(const Identity& identity,
+                                          base::ProcessId pid) {
   listeners_.ForAllPtrs(
       [identity, pid](mojom::ServiceManagerListener* listener) {
         listener->OnServiceStarted(identity, pid);
       });
 }
 
+void ServiceManager::NotifyServiceFailedToStart(const Identity& identity) {
+  listeners_.ForAllPtrs(
+      [identity](mojom::ServiceManagerListener* listener) {
+        listener->OnServiceFailedToStart(identity);
+      });
+}
+
 bool ServiceManager::ConnectToExistingInstance(
     std::unique_ptr<ConnectParams>* params) {
   Instance* instance = GetExistingInstance((*params)->target());
@@ -801,6 +828,7 @@ void ServiceManager::OnGotResolvedName(std::unique_ptr<ConnectParams> params,
   if (result->connection_spec.has_value())
     connection_spec = result->connection_spec.value();
 
+  const Identity original_target(params->target());
   const std::string user_id =
       HasCapability(connection_spec, kCapability_AllUsers)
           ? base::GenerateGUID() : params->target().user_id();
@@ -851,8 +879,12 @@ void ServiceManager::OnGotResolvedName(std::unique_ptr<ConnectParams> params,
     CHECK(!result->package_path.empty() && result->connection_spec.has_value());
 
     if (target.name() != result->resolved_name) {
+      // When part of a packaged app, use the original user ID.

[Ken Rockot(use gerrit already), 2016/10/21 19:52:33]

nit: s/app/service/

[Jay Civelli, 2016/10/21 20:48:33]

Done.

+      Identity packaged_app_target(target);
+      packaged_app_target.set_user_id(original_target.user_id());
+      instance->set_identity(packaged_app_target);
       instance->StartWithService(std::move(service));
-      Identity factory(result->resolved_name, target.user_id(),
+      Identity factory(result->resolved_name, original_target.user_id(),
                        instance_name);
       CreateServiceWithFactory(factory, target.name(), std::move(request));
     } else {