Fixes race in Shell

services/shell/shell.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/shell.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 458 matching lines @@
   instance->StartWithClient(std::move(client));
   singletons_.insert(kShellName);
   shell_connection_.reset(new ShellConnection(this, std::move(request)));
 
   if (catalog)
     InitCatalog(std::move(catalog));
 }
 
 Shell::~Shell() {
   TerminateShellConnections();
+  // Terminate any remaining instances.
+  while (!identity_to_instance_.empty())
+    OnInstanceError(identity_to_instance_.begin()->second);
+  identity_to_resolver_.clear();
   for (auto& runner : native_runners_)
     runner.reset();
 }
 
 void Shell::SetInstanceQuitCallback(
     base::Callback<void(const Identity&)> callback) {
   instance_quit_callback_ = callback;
 }
 
 void Shell::Connect(std::unique_ptr<ConnectParams> params) {
@@ skipping 45 matching lines @@
   instance->StartWithClient(std::move(catalog));
 
   // TODO(beng): this doesn't work anymore.
   // Seed the catalog with manifest info for the shell & catalog.
   mojom::ShellResolverPtr resolver;
   shell_connection_->connector()->ConnectToInterface(kCatalogName, &resolver);
   resolver->ResolveMojoName(kCatalogName, base::Bind(&EmptyResolverCallback));
   resolver->ResolveMojoName(kShellName, base::Bind(&EmptyResolverCallback));
 }
 
+mojom::ShellResolver* Shell::GetResolver(const Identity& identity) {
+  auto iter = identity_to_resolver_.find(identity);
+  if (iter != identity_to_resolver_.end())
+    return iter->second.get();
+
+  mojom::ShellResolverPtr resolver_ptr;
+  ConnectToInterface(this, identity, CreateCatalogIdentity(), &resolver_ptr);
+  mojom::ShellResolver* resolver = resolver_ptr.get();
+  identity_to_resolver_[identity] = std::move(resolver_ptr);
+  return resolver;
+}
+
 void Shell::TerminateShellConnections() {
   Instance* instance = GetExistingInstance(CreateShellIdentity());
   DCHECK(instance);
   OnInstanceError(instance);
 }
 
 void Shell::OnInstanceError(Instance* instance) {
   const Identity identity = instance->identity();
   // Remove the shell.
   auto it = identity_to_instance_.find(identity);
@@ skipping 19 matching lines @@
   DCHECK(!client.is_bound() || !identity_to_instance_.count(params->target()));
 
   // Connect to an existing matching instance, if possible.
   if (!client.is_bound() && ConnectToExistingInstance(&params))
     return;
 
   // The catalog needs to see the source identity as that of the originating
   // app so it loads the correct store. Since the catalog is itself run as root
   // when this re-enters Connect() it'll be handled by
   // ConnectToExistingInstance().
-  mojom::ShellResolverPtr resolver;
-  ConnectToInterface(this, Identity(kShellName, params->target().user_id()),
-                     CreateCatalogIdentity(), &resolver);
+  mojom::ShellResolver* resolver =
+      GetResolver(Identity(kShellName, params->target().user_id()));
 
   std::string name = params->target().name();
-  mojom::ShellResolver* resolver_raw = resolver.get();
-  resolver_raw->ResolveMojoName(
+  resolver->ResolveMojoName(
       name, base::Bind(&shell::Shell::OnGotResolvedName,
-                       weak_ptr_factory_.GetWeakPtr(),
-                       base::Passed(std::move(resolver)), base::Passed(&params),
+                       weak_ptr_factory_.GetWeakPtr(), base::Passed(&params),
                        base::Passed(&client)));
 }
 
 Shell::Instance* Shell::GetExistingInstance(const Identity& identity) const {
   const auto& it = identity_to_instance_.find(identity);
   Instance* instance = it != identity_to_instance_.end() ? it->second : nullptr;
   if (instance)
     return instance;
 
   if (singletons_.find(identity.name()) != singletons_.end()) {
@@ skipping 75 matching lines @@
   return factory_interface;
 }
 
 void Shell::OnShellClientFactoryLost(const Identity& which) {
   // Remove the mapping.
   auto it = shell_client_factories_.find(which);
   DCHECK(it != shell_client_factories_.end());
   shell_client_factories_.erase(it);
 }
 
-void Shell::OnGotResolvedName(mojom::ShellResolverPtr resolver,
-                              std::unique_ptr<ConnectParams> params,
+void Shell::OnGotResolvedName(std::unique_ptr<ConnectParams> params,
                               mojom::ShellClientPtr client,
                               mojom::ResolveResultPtr result) {
   std::string instance_name = params->target().instance();
   if (instance_name == GetNamePath(params->target().name()) &&
       result->qualifier != GetNamePath(result->resolved_name)) {
     instance_name = result->qualifier;
   }
   Identity target(params->target().name(), params->target().user_id(),
                   instance_name);
   params->set_target(target);
@@ skipping 68 matching lines @@
 void Shell::CleanupRunner(NativeRunner* runner) {
   for (auto it = native_runners_.begin(); it != native_runners_.end(); ++it) {
     if (it->get() == runner) {
       native_runners_.erase(it);
       return;
     }
   }
 }
 
 }  // namespace shell