ServiceManager: Simplify Instance lifetime management

services/shell/service_manager.cc

 // Copyright 2014 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/shell/service_manager.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 115 matching lines @@
         pid_receiver_binding_(this),
         weak_factory_(this) {
     if (identity_.name() == kServiceManagerName ||
         identity_.name() == kCatalogName) {
       pid_ = base::Process::Current().Pid();
     }
     DCHECK_NE(mojom::kInvalidInstanceID, id_);
   }
 
   ~Instance() override {
-    if (parent_)
-      parent_->RemoveChild(this);
-    // |children_| will be modified during destruction.
-    std::set<Instance*> children = children_;
-    for (auto* child : children)
-      service_manager_->OnInstanceError(child, InstanceErrorType::DESTROY);
-
     // Shutdown all bindings before we close the runner. This way the process
     // should see the pipes closed and exit, as well as waking up any potential
     // sync/WaitForIncomingResponse().
     service_.reset();
     if (pid_receiver_binding_.is_bound())
       pid_receiver_binding_.Close();
     connectors_.CloseAllBindings();
     service_manager_bindings_.CloseAllBindings();
+
+    // 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.
     std::unique_ptr<NativeRunner> runner = std::move(runner_);
     runner.reset();
   }
 
   Instance* parent() { return parent_; }
 
-  void AddChild(Instance* child) {
-    children_.insert(child);
+  void AddChild(std::unique_ptr<Instance> child) {
     child->parent_ = this;
+    children_.insert(std::make_pair(child.get(), std::move(child)));
   }
 
   void RemoveChild(Instance* child) {
     auto it = children_.find(child);
     DCHECK(it != children_.end());
+
+    // Deletes |child|.
     children_.erase(it);
-    child->parent_ = nullptr;
   }
 
   bool ConnectToService(std::unique_ptr<ConnectParams>* connect_params) {
     if (!service_.is_bound())
       return false;
 
     std::unique_ptr<ConnectParams> params(std::move(*connect_params));
     if (!params->connect_callback().is_null()) {
         params->connect_callback().Run(mojom::ConnectResult::SUCCEEDED,
                                        identity_.user_id());
@@ skipping 216 matching lines @@
 
   uint32_t GenerateUniqueID() const {
     static uint32_t id = mojom::kInvalidInstanceID;
     ++id;
     CHECK_NE(mojom::kInvalidInstanceID, id);
     return id;
   }
 
   void PIDAvailable(base::ProcessId pid) {
     if (pid == base::kNullProcessId) {
-      service_manager_->OnInstanceError(this, InstanceErrorType::DESTROY);
+      service_manager_->OnInstanceError(this);
       return;
     }
     pid_ = pid;
     service_manager_->NotifyPIDAvailable(identity_, pid_);
   }
 
   void OnServiceLost(base::WeakPtr<shell::ServiceManager> service_manager) {
     service_.reset();
     OnConnectionLost(service_manager);
   }
 
   void OnConnectionLost(base::WeakPtr<shell::ServiceManager> service_manager) {
     // Any time a Connector is lost or we lose the Service connection, it
     // may have been the last pipe using this Instance. If so, clean up.
     if (service_manager && !service_) {
-      InstanceErrorType instance_error_type =
-          connectors_.empty() ? InstanceErrorType::DESTROY
-                              : InstanceErrorType::LOST_SERVICE;
-      service_manager->OnInstanceError(this, instance_error_type);
+      if (connectors_.empty())
+        service_manager->OnInstanceError(this);
+      else
+        service_manager->OnInstanceUnreachable(this);
     }
   }
 
   void OnInitializeResponse(mojom::ConnectorRequest connector_request) {
     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()));
     }
   }
 
   // Callback when NativeRunner completes.
   void OnRunnerCompleted() {
     if (!runner_.get())
       return;  // We're in the destructor.
 
-    service_manager_->OnInstanceError(this, InstanceErrorType::DESTROY);
+    service_manager_->OnInstanceError(this);
   }
 
   shell::ServiceManager* const service_manager_;
 
   // An id that identifies this instance. Distinct from pid, as a single process
   // may vend multiple application instances, and this object may exist before a
   // process is launched.
   const uint32_t id_;
   const Identity identity_;
   const CapabilitySpec capability_spec_;
   const bool allow_any_application_;
   std::unique_ptr<NativeRunner> runner_;
   mojom::ServicePtr service_;
   mojo::Binding<mojom::PIDReceiver> pid_receiver_binding_;
   mojo::BindingSet<mojom::Connector> connectors_;
   mojo::BindingSet<mojom::ServiceManager> service_manager_bindings_;
   base::ProcessId pid_ = base::kNullProcessId;
   Instance* parent_ = nullptr;
-  std::set<Instance*> children_;
+  InstanceMap children_;
   base::WeakPtrFactory<Instance> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Instance);
 };
 
 // static
 ServiceManager::TestAPI::TestAPI(ServiceManager* service_manager)
     : service_manager_(service_manager) {}
 ServiceManager::TestAPI::~TestAPI() {}
 
@@ skipping 21 matching lines @@
   instance->StartWithService(std::move(service));
   singletons_.insert(kServiceManagerName);
   service_context_.reset(new ServiceContext(this, std::move(request)));
 
   if (catalog)
     InitCatalog(std::move(catalog));
 }
 
 ServiceManager::~ServiceManager() {
   TerminateServiceManagerConnections();
-  // Terminate any remaining instances.
-  while (!identity_to_instance_.empty()) {
-    OnInstanceError(identity_to_instance_.begin()->second,
-                    InstanceErrorType::DESTROY);
-  }
-  while (!instances_without_service_.empty()) {
-    OnInstanceError(*instances_without_service_.begin(),
-                    InstanceErrorType::DESTROY);
-  }
-  identity_to_resolver_.clear();
 }
 
 void ServiceManager::SetInstanceQuitCallback(
     base::Callback<void(const Identity&)> callback) {
   instance_quit_callback_ = callback;
 }
 
 void ServiceManager::Connect(std::unique_ptr<ConnectParams> params) {
   Connect(std::move(params), nullptr, nullptr);
 }
@@ skipping 58 matching lines @@
   mojom::ResolverPtr resolver_ptr;
   ConnectToInterface(this, identity, CreateCatalogIdentity(), &resolver_ptr);
   mojom::Resolver* resolver = resolver_ptr.get();
   identity_to_resolver_[identity] = std::move(resolver_ptr);
   return resolver;
 }
 
 void ServiceManager::TerminateServiceManagerConnections() {
   Instance* instance = GetExistingInstance(CreateServiceManagerIdentity());
   if (instance)
-    OnInstanceError(instance, InstanceErrorType::DESTROY);
+    OnInstanceError(instance);
 }
 
-void ServiceManager::OnInstanceError(Instance* instance,
-                                     InstanceErrorType error_type) {
+void ServiceManager::OnInstanceError(Instance* instance) {
   const Identity identity = instance->identity();
+  identity_to_instance_.erase(identity);
 
-  const bool in_identity_to_instance =
-      identity_to_instance_.erase(identity) > 0;
-  const bool in_instances_without_services =
-      instances_without_service_.erase(instance) > 0;
-  DCHECK_NE(in_identity_to_instance, in_instances_without_services);
+  if (instance->parent()) {
+    // Deletes |instance|.
+    instance->parent()->RemoveChild(instance);
+  } else {
+    auto it = root_instances_.find(instance);
+    DCHECK(it != root_instances_.end());
 
-  if (error_type == InstanceErrorType::LOST_SERVICE) {
-    // |instance| has lost its service but still has connections to it, we need
-    // to keep it around. The instance is removed from |identity_to_instance_|
-    // so that if an attempt is made to connect to the same identity a new
-    // Instance is created.
-    instances_without_service_.insert(instance);
-    return;
+    // Deletes |instance|.
+    root_instances_.erase(it);
   }
+}
 
+void ServiceManager::OnInstanceUnreachable(Instance* instance) {
+  // If an Instance becomes unreachable, new connection requests for this
+  // identity will elicit a new Instance instantiation. The unreachable instance
+  // remains alive.
+  identity_to_instance_.erase(instance->identity());
+}
+
+void ServiceManager::OnInstanceStopped(const Identity& identity) {
   listeners_.ForAllPtrs([identity](mojom::ServiceManagerListener* listener) {
     listener->OnServiceStopped(identity);
   });
-  delete instance;
   if (!instance_quit_callback_.is_null())
     instance_quit_callback_.Run(identity);
 }
 
 void ServiceManager::Connect(std::unique_ptr<ConnectParams> params,
                              mojom::ServicePtr service,
                              base::WeakPtr<Instance> source_instance) {
   TRACE_EVENT_INSTANT1("mojo_shell", "ServiceManager::Connect",
                        TRACE_EVENT_SCOPE_THREAD, "original_name",
                        params->target().name());
@@ skipping 51 matching lines @@
     std::unique_ptr<ConnectParams>* params) {
   Instance* instance = GetExistingInstance((*params)->target());
   return instance && instance->ConnectToService(params);
 }
 
 ServiceManager::Instance* ServiceManager::CreateInstance(
     const Identity& source,
     const Identity& target,
     const CapabilitySpec& spec) {
   CHECK(target.user_id() != mojom::kInheritUserID);
-  Instance* instance = new Instance(this, target, spec);
-  DCHECK(identity_to_instance_.find(target) ==
-         identity_to_instance_.end());
+
+  std::unique_ptr<Instance> instance(new Instance(this, target, spec));
+  Instance* raw_instance = instance.get();
+
   Instance* source_instance = GetExistingInstance(source);
   if (source_instance)
-    source_instance->AddChild(instance);
-  identity_to_instance_[target] = instance;
-  mojom::ServiceInfoPtr info = instance->CreateServiceInfo();
+    source_instance->AddChild(std::move(instance));
+  else
+    root_instances_.insert(std::make_pair(raw_instance, std::move(instance)));
+
+  // NOTE: |instance| has been passed elsewhere. Use |raw_instance| from this
+  // point forward. It's safe for the extent of this method.
+
+  auto result =
+      identity_to_instance_.insert(std::make_pair(target, raw_instance));
+  DCHECK(result.second);
+
+  mojom::ServiceInfoPtr info = raw_instance->CreateServiceInfo();
   listeners_.ForAllPtrs([&info](mojom::ServiceManagerListener* listener) {
     listener->OnServiceCreated(info.Clone());
   });
-  return instance;
+
+  return raw_instance;
 }
 
 void ServiceManager::AddListener(mojom::ServiceManagerListenerPtr listener) {
   // TODO(beng): filter instances provided by those visible to this service.
   std::vector<mojom::ServiceInfoPtr> instances;
   instances.reserve(identity_to_instance_.size());
   for (auto& instance : identity_to_instance_)
     instances.push_back(instance.second->CreateServiceInfo());
   listener->OnInit(std::move(instances));
 
@@ skipping 115 matching lines @@
   // Now that the instance has a Service, we can connect to it.
   bool connected = instance->ConnectToService(&params);
   DCHECK(connected);
 }
 
 base::WeakPtr<ServiceManager> ServiceManager::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 }  // namespace shell