Cascade shutdown of instances

BUG=

Review URL: https://codereview.chromium.org/1810713002

Cr-Commit-Position: refs/heads/master@{#381660}

ash/mus/sysui_application.cc

@@ -16,6 +16,7 @@
 #include "ash/shell_window_ids.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
+#include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "components/mus/public/cpp/property_type_converters.h"
@@ -288,5 +289,9 @@ bool SysUIApplication::AcceptConnection(mojo::Connection* connection) {
   return true;
 }
 
+void SysUIApplication::ShellConnectionLost() {
+  base::MessageLoop::current()->QuitWhenIdle();
+}
+
 }  // namespace sysui
 }  // namespace ash

ash/mus/sysui_application.h

@@ -26,6 +26,7 @@ class SysUIApplication : public mojo::ShellClient {
                   const mojo::Identity& identity,
                   uint32_t id) override;
   bool AcceptConnection(mojo::Connection* connection) override;
+  void ShellConnectionLost() override;
 
   mojo::TracingImpl tracing_;
   scoped_ptr<AshInit> ash_init_;

components/resource_provider/manifest.json

 {
+  "manifest_version":  1,
   "name": "mojo:resource_provider",
   "display_name": "Resource Provider",
-  "capabilities": { "*": [ "*" ] }
+  "capabilities": {
+    "required": {
+      "mojo:shell": { "classes": [ "all_users" ] }
+    }
+  }
 }

mash/browser_driver/browser_driver_application_delegate.cc

@@ -7,6 +7,7 @@
 #include <stdint.h>
 
 #include "base/bind.h"
+#include "base/message_loop/message_loop.h"
 #include "components/mus/public/cpp/event_matcher.h"
 #include "mojo/shell/public/cpp/connection.h"
 #include "mojo/shell/public/cpp/connector.h"
@@ -42,11 +43,14 @@ void AssertTrue(bool success) {
   DCHECK(success);
 }
 
+void DoNothing() {}
+
 }  // namespace
 
 BrowserDriverApplicationDelegate::BrowserDriverApplicationDelegate()
     : connector_(nullptr),
-      binding_(this) {}
+      binding_(this),
+      weak_factory_(this) {}
 
 BrowserDriverApplicationDelegate::~BrowserDriverApplicationDelegate() {}
 
@@ -63,6 +67,12 @@ bool BrowserDriverApplicationDelegate::AcceptConnection(
   return true;
 }
 
+void BrowserDriverApplicationDelegate::ShellConnectionLost() {
+  // Prevent the code in AddAccelerators() from keeping this app alive.
+  binding_.set_connection_error_handler(base::Bind(&DoNothing));
+  base::MessageLoop::current()->QuitWhenIdle();
+}
+
 void BrowserDriverApplicationDelegate::OnAccelerator(
     uint32_t id, mus::mojom::EventPtr event) {
   switch (static_cast<Accelerator>(id)) {
@@ -92,7 +102,7 @@ void BrowserDriverApplicationDelegate::AddAccelerators() {
   // to re-add our accelerators when the window manager comes back up.
   binding_.set_connection_error_handler(
       base::Bind(&BrowserDriverApplicationDelegate::AddAccelerators,
-                 base::Unretained(this)));
+                 weak_factory_.GetWeakPtr()));
 
   for (const AcceleratorSpec& spec : g_spec) {
     registrar->AddAccelerator(

mash/browser_driver/browser_driver_application_delegate.h

@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "components/mus/public/interfaces/accelerator_registrar.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/shell/public/cpp/shell_client.h"
@@ -30,6 +31,7 @@ class BrowserDriverApplicationDelegate : public mojo::ShellClient,
   void Initialize(mojo::Connector* connector, const mojo::Identity& identity,
                   uint32_t id) override;
   bool AcceptConnection(mojo::Connection* connection) override;
+  void ShellConnectionLost() override;
 
   // mus::mojom::AcceleratorHandler:
   void OnAccelerator(uint32_t id, mus::mojom::EventPtr event) override;
@@ -38,6 +40,7 @@ class BrowserDriverApplicationDelegate : public mojo::ShellClient,
 
   mojo::Connector* connector_;
   mojo::Binding<mus::mojom::AcceleratorHandler> binding_;
+  base::WeakPtrFactory<BrowserDriverApplicationDelegate> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserDriverApplicationDelegate);
 };

mash/example/window_type_launcher/BUILD.gn

@@ -24,7 +24,6 @@ executable("window_type_launcher") {
     "//base:base_static",
     "//build/config/sanitizers:deps",
     "//components/mus/public/interfaces",
-    "//mash/login/public/interfaces",
     "//mash/shell/public/interfaces",
     "//mojo/common:common_base",
     "//mojo/converters/geometry",

mash/example/window_type_launcher/window_type_launcher.cc

@@ -5,8 +5,8 @@
 #include "mash/example/window_type_launcher/window_type_launcher.h"
 
 #include "base/macros.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
-#include "mash/login/public/interfaces/login.mojom.h"
 #include "mash/shell/public/interfaces/shell.mojom.h"
 #include "mojo/converters/geometry/geometry_type_converters.h"
 #include "mojo/shell/public/cpp/connection.h"
@@ -283,13 +283,13 @@ class WindowTypeLauncherView : public views::WidgetDelegateView,
       connector_->ConnectToInterface("mojo:mash_shell", &shell);
       shell->LockScreen();
     } else if (sender == logout_button_) {
-      mash::login::mojom::LoginPtr login;
-      connector_->ConnectToInterface("mojo:login", &login);
-      login->Logout();
+      mash::shell::mojom::ShellPtr shell;
+      connector_->ConnectToInterface("mojo:mash_shell", &shell);
+      shell->Logout();
     } else if (sender == switch_user_button_) {
-      mash::login::mojom::LoginPtr login;
-      connector_->ConnectToInterface("mojo:login", &login);
-      login->SwitchUser();
+      mash::shell::mojom::ShellPtr shell;
+      connector_->ConnectToInterface("mojo:mash_shell", &shell);
+      shell->SwitchUser();
     } else if (sender == widgets_button_) {
       NOTIMPLEMENTED();
     }
@@ -383,3 +383,7 @@ void WindowTypeLauncher::Initialize(mojo::Connector* connector,
   widget->Init(params);
   widget->Show();
 }
+
+void WindowTypeLauncher::ShellConnectionLost() {
+  base::MessageLoop::current()->QuitWhenIdle();
+}

mash/example/window_type_launcher/window_type_launcher.h

@@ -22,6 +22,7 @@ class WindowTypeLauncher : public mojo::ShellClient {
   // mojo::ShellClient:
   void Initialize(mojo::Connector* connector, const mojo::Identity& identity,
                   uint32_t id) override;
+  void ShellConnectionLost() override;
 
   scoped_ptr<views::AuraInit> aura_init_;
 

mash/init/init.cc

@@ -14,7 +14,8 @@
 namespace mash {
 namespace init {
 
-Init::Init() : connector_(nullptr), login_user_id_(base::GenerateGUID()) {}
+Init::Init()
+    : connector_(nullptr) {}
 Init::~Init() {}
 
 void Init::Initialize(mojo::Connector* connector,
@@ -22,18 +23,23 @@ void Init::Initialize(mojo::Connector* connector,
                       uint32_t id) {
   connector_ = connector;
   connector_->Connect("mojo:mus");
+  StartTracing();
+  StartResourceProvider();
   StartLogin();
 }
 
 void Init::StartService(const mojo::String& name,
                         const mojo::String& user_id) {
-  DCHECK(user_services_.find(user_id) == user_services_.end());
-  mojo::Connector::ConnectParams params(mojo::Identity(name, user_id));
-  user_services_[user_id] = connector_->Connect(&params);
+  if (user_services_.find(user_id) == user_services_.end()) {
+    mojo::Connector::ConnectParams params(mojo::Identity(name, user_id));
+    scoped_ptr<mojo::Connection> connection = connector_->Connect(&params);
+    connection->SetConnectionLostClosure(
+        base::Bind(&Init::UserServiceQuit, base::Unretained(this), user_id));
+    user_services_[user_id] = std::move(connection);
+  }
 }
 
 void Init::StopServicesForUser(const mojo::String& user_id) {
-  // TODO(beng): Make shell cascade shutdown of services.
   auto it = user_services_.find(user_id);
   if (it != user_services_.end())
     user_services_.erase(it);
@@ -43,10 +49,22 @@ void Init::Create(mojo::Connection* connection, mojom::InitRequest request) {
   init_bindings_.AddBinding(this, std::move(request));
 }
 
+void Init::UserServiceQuit(const std::string& user_id) {
+  auto it = user_services_.find(user_id);
+  DCHECK(it != user_services_.end());
+  user_services_.erase(it);
+}
+
+void Init::StartTracing() {
+  connector_->Connect("mojo:tracing");
+}
+
+void Init::StartResourceProvider() {
+  connector_->Connect("mojo:resource_provider");
+}
+
 void Init::StartLogin() {
-  mojo::Connector::ConnectParams params(
-      mojo::Identity("mojo:login", login_user_id_));
-  login_connection_ = connector_->Connect(&params);
+  login_connection_ = connector_->Connect("mojo:login");
   login_connection_->AddInterface<mojom::Init>(this);
   login_connection_->SetConnectionLostClosure(
       base::Bind(&Init::StartLogin, base::Unretained(this)));

mash/init/init.h

@@ -43,13 +43,16 @@ class Init : public mojo::ShellClient,
                     const mojo::String& user_id) override;
   void StopServicesForUser(const mojo::String& user_id) override;
 
+  void UserServiceQuit(const std::string& user_id);
+
+  void StartTracing();
+  void StartResourceProvider();
   void StartLogin();
 
   mojo::Connector* connector_;
   scoped_ptr<mojo::Connection> login_connection_;
   mojo::BindingSet<mojom::Init> init_bindings_;
   std::map<std::string, scoped_ptr<mojo::Connection>> user_services_;
-  const std::string login_user_id_;
 
   DISALLOW_COPY_AND_ASSIGN(Init);
 };

mash/login/login.cc

@@ -32,10 +32,6 @@ class Login : public mojom::Login {
   void ShowLoginUI() override {
     UI::Show(connector_, controller_);
   }
-  void Logout() override {
-    controller_->init()->StopServicesForUser(user_id_);
-    UI::Show(connector_, controller_);
-  }
   void SwitchUser() override {
     UI::Show(connector_, controller_);
   }

mash/login/public/interfaces/login.mojom

@@ -6,6 +6,5 @@ module mash.login.mojom;
 
 interface Login {
   ShowLoginUI();
-  Logout();
   SwitchUser();
 };

mash/login/ui.cc

@@ -79,6 +79,7 @@ UI::~UI() {
   // Prevent the window manager from restarting during graceful shutdown.
   window_manager_connection_->SetConnectionLostClosure(base::Closure());
   is_showing_ = false;
+  // TODO(beng): we should be terminating this app at this point.
 }
 
 views::View* UI::GetContentsView() { return this; }

mash/quick_launch/quick_launch_application.cc

@@ -5,6 +5,7 @@
 #include "mash/quick_launch/quick_launch_application.h"
 
 #include "base/macros.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "mojo/public/c/system/main.h"
@@ -105,5 +106,9 @@ bool QuickLaunchApplication::AcceptConnection(mojo::Connection* connection) {
   return true;
 }
 
+void QuickLaunchApplication::ShellConnectionLost() {
+  base::MessageLoop::current()->QuitWhenIdle();
+}
+
 }  // namespace quick_launch
 }  // namespace mash

mash/quick_launch/quick_launch_application.h

@@ -28,6 +28,7 @@ class QuickLaunchApplication : public mojo::ShellClient {
                   const mojo::Identity& identity,
                   uint32_t id) override;
   bool AcceptConnection(mojo::Connection* connection) override;
+  void ShellConnectionLost() override;
 
   mojo::TracingImpl tracing_;
   scoped_ptr<views::AuraInit> aura_init_;

mash/shell/BUILD.gn

@@ -16,6 +16,7 @@ source_set("lib") {
 
   deps = [
     "//base",
+    "//mash/login/public/interfaces",
     "//mash/shell/public/interfaces",
     "//mojo/common",
     "//mojo/public/cpp/bindings",

mash/shell/public/interfaces/shell.mojom

@@ -11,6 +11,8 @@ interface ScreenlockStateListener {
 };
 
 interface Shell {
+  Logout();
+  SwitchUser();
   LockScreen();
   UnlockScreen();
   AddScreenlockStateListener(ScreenlockStateListener listener);

mash/shell/shell_application_delegate.cc

@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/message_loop/message_loop.h"
+#include "mash/login/public/interfaces/login.mojom.h"
 #include "mojo/shell/public/cpp/connection.h"
 #include "mojo/shell/public/cpp/connector.h"
 
@@ -32,6 +34,22 @@ bool ShellApplicationDelegate::AcceptConnection(mojo::Connection* connection) {
   return true;
 }
 
+void ShellApplicationDelegate::Logout() {
+  // TODO(beng): Notify connected listeners that login is happening, potentially
+  // give them the option to stop it.
+  mash::login::mojom::LoginPtr login;
+  connector_->ConnectToInterface("mojo:login", &login);
+  login->ShowLoginUI();
+  // This kills the user environment.
+  base::MessageLoop::current()->QuitWhenIdle();
+}
+
+void ShellApplicationDelegate::SwitchUser() {
+  mash::login::mojom::LoginPtr login;
+  connector_->ConnectToInterface("mojo:login", &login);
+  login->SwitchUser();
+}
+
 void ShellApplicationDelegate::AddScreenlockStateListener(
     mojom::ScreenlockStateListenerPtr listener) {
   listener->ScreenlockStateChanged(screen_locked_);

mash/shell/shell_application_delegate.h

@@ -39,6 +39,8 @@ class ShellApplicationDelegate
   bool AcceptConnection(mojo::Connection* connection) override;
 
   // mash::shell::mojom::Shell:
+  void Logout() override;
+  void SwitchUser() override;
   void AddScreenlockStateListener(
       mojom::ScreenlockStateListenerPtr listener) override;
   void LockScreen() override;

mojo/services/catalog/manifest.json

 {
+  "manifest_version":  1,
   "name": "mojo:catalog",
   "display_name": "Catalog",
-  "capabilities": {}
+  "capabilities": {
+    "required": {
+      "mojo:shell": { "classes": [ "all_users" ] }      
+    }
+  }
 }

mojo/services/tracing/manifest.json

 {
+  "manifest_version":  1,
   "name": "mojo:tracing",
   "display_name": "Tracing Collector",
-  "capabilities": {}
+  "capabilities": {
+    "required": {
+      "mojo:shell": { "classes": [ "all_users" ] }
+    }
+  }
 }

mojo/shell/manifest.json

@@ -6,7 +6,11 @@
     "provided": {
       "user_id": [ ],
       "client_process": [ ],
-      "instance_name": [ ]
+      "instance_name": [ ],
+      "all_users":  [ ]
+    },
+    "required": {
+      "mojo:shell": { "classes": [ "all_users" ] }
     }
   }
 }

mojo/shell/shell.cc

@@ -129,28 +129,25 @@ class Shell::Instance : public mojom::Connector,
     DCHECK_NE(mojom::kInvalidInstanceID, id_);
   }
 
-  ~Instance() override {}
-
-  void OnShellClientLost() {
-    shell_client_.reset();
-    OnConnectionLost();
+  ~Instance() override {
+    if (parent_)
+      parent_->RemoveChild(this);
+    // |children_| will be modified during destruction.
+    std::set<Instance*> children = children_;
+    for (auto child : children)
+      shell_->OnInstanceError(child);
   }
 
-  void OnConnectionLost() {
-    // Any time a Connector is lost or we lose the ShellClient connection, it
-    // may have been the last pipe using this Instance. If so, clean up.
-    if (connectors_.empty() && !shell_client_) {
-      // Deletes |this|.
-      shell_->OnInstanceError(this);
-    }
+  Instance* parent() { return parent_; }
+  void AddChild(Instance* child) {
+    children_.insert(child);
+    child->parent_ = 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)));
-    }
+  void RemoveChild(Instance* child) {
+    auto it = children_.find(child);
+    DCHECK(it != children_.end());
+    children_.erase(it);
+    child->parent_ = nullptr;
   }
 
   void ConnectToClient(scoped_ptr<ConnectParams> params) {
@@ -176,7 +173,8 @@ class Shell::Instance : public mojom::Connector,
     CHECK(!shell_client_);
     shell_client_ = std::move(client);
     shell_client_.set_connection_error_handler(
-        base::Bind(&Instance::OnShellClientLost, base::Unretained(this)));
+        base::Bind(&Instance::OnShellClientLost, base::Unretained(this),
+                   shell_->GetWeakPtr()));
     shell_client_->Initialize(mojom::Identity::From(identity_), id_,
                               base::Bind(&Instance::OnInitializeResponse,
                                          base::Unretained(this)));
@@ -384,6 +382,29 @@ class Shell::Instance : public mojom::Connector,
     shell_->NotifyPIDAvailable(id_, pid_);
   }
 
+  void OnShellClientLost(base::WeakPtr<mojo::shell::Shell> shell) {
+    shell_client_.reset();
+    OnConnectionLost(shell);
+  }
+
+  void OnConnectionLost(base::WeakPtr<mojo::shell::Shell> shell) {
+    // Any time a Connector is lost or we lose the ShellClient connection, it
+    // may have been the last pipe using this Instance. If so, clean up.
+    if (shell && connectors_.empty() && !shell_client_) {
+      // Deletes |this|.
+      shell->OnInstanceError(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),
+                     shell_->GetWeakPtr()));
+    }
+  }
+
   mojo::shell::Shell* const shell_;
 
   // An id that identifies this instance. Distinct from pid, as a single process
@@ -399,6 +420,8 @@ class Shell::Instance : public mojom::Connector,
   BindingSet<mojom::Shell> shell_bindings_;
   NativeRunner* runner_ = nullptr;
   base::ProcessId pid_ = base::kNullProcessId;
+  Instance* parent_ = nullptr;
+  std::set<Instance*> children_;
   base::WeakPtrFactory<Instance> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Instance);
@@ -425,9 +448,10 @@ Shell::Shell(scoped_ptr<NativeRunnerFactory> native_runner_factory,
       weak_ptr_factory_(this) {
   mojom::ShellClientPtr client;
   mojom::ShellClientRequest request = GetProxy(&client);
-  Instance* instance = CreateInstance(CreateShellIdentity(),
+  Instance* instance = CreateInstance(Identity(), CreateShellIdentity(),
                                       GetPermissiveCapabilities());
   instance->StartWithClient(std::move(client));
+  singletons_.insert(kShellName);
   shell_connection_.reset(new ShellConnection(this, std::move(request)));
 
   if (catalog)
@@ -467,7 +491,7 @@ mojom::ShellClientRequest Shell::InitInstanceForEmbedder(
 
 void Shell::SetLoaderForName(scoped_ptr<Loader> loader,
                              const std::string& name) {
-  NameToLoaderMap::iterator it = name_to_loader_.find(name);
+  auto it = name_to_loader_.find(name);
   if (it != name_to_loader_.end())
     delete it->second;
   name_to_loader_[name] = loader.release();
@@ -496,8 +520,10 @@ bool Shell::AcceptConnection(Connection* connection) {
 // Shell, private:
 
 void Shell::InitCatalog(mojom::ShellClientPtr catalog) {
-  Instance* instance =
-      CreateInstance(CreateCatalogIdentity(), CapabilitySpec());
+  Instance* instance = CreateInstance(CreateShellIdentity(),
+                                      CreateCatalogIdentity(),
+                                      CapabilitySpec());
+  singletons_.insert(kCatalogName);
   instance->StartWithClient(std::move(catalog));
 
   // TODO(beng): this doesn't work anymore.
@@ -509,7 +535,9 @@ void Shell::InitCatalog(mojom::ShellClientPtr catalog) {
 }
 
 void Shell::TerminateShellConnections() {
-  STLDeleteValues(&identity_to_instance_);
+  Instance* instance = GetExistingInstance(CreateShellIdentity());
+  DCHECK(instance);
+  OnInstanceError(instance);
 }
 
 void Shell::OnInstanceError(Instance* instance) {
@@ -588,12 +616,16 @@ bool Shell::ConnectToExistingInstance(scoped_ptr<ConnectParams>* params) {
   return !!instance;
 }
 
-Shell::Instance* Shell::CreateInstance(const Identity& target,
+Shell::Instance* Shell::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());
+  Instance* source_instance = GetExistingInstance(source);
+  if (source_instance)
+    source_instance->AddChild(instance);
   identity_to_instance_[target] = instance;
   mojom::InstanceInfoPtr info = instance->CreateInstanceInfo();
   instance_listeners_.ForAllPtrs(
@@ -678,13 +710,16 @@ void Shell::OnGotResolvedName(mojom::ShellResolverPtr resolver,
     capabilities = capabilities_ptr.To<CapabilitySpec>();
 
   // Clients that request "all_users" class from the shell are allowed to
-  // field connection requests from any user.
-  if (HasClass(capabilities, kCapabilityClass_AllUsers))
+  // field connection requests from any user. They also run with a synthetic
+  // user id generated here. The user id provided via Connect() is ignored.
+  if (HasClass(capabilities, kCapabilityClass_AllUsers)) {
     singletons_.insert(target.name());
+    target.set_user_id(base::GenerateGUID());
+  }
 
   mojom::ClientProcessConnectionPtr client_process_connection =
       params->TakeClientProcessConnection();
-  Instance* instance = CreateInstance(target, capabilities);
+  Instance* instance = CreateInstance(params->source(), target, capabilities);
 
   // Below are various paths through which a new Instance can be bound to a
   // ShellClient proxy.
@@ -736,6 +771,10 @@ Loader* Shell::GetLoaderForName(const std::string& name) {
   return default_loader_.get();
 }
 
+base::WeakPtr<Shell> Shell::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void Shell::CleanupRunner(NativeRunner* runner) {
   for (auto it = native_runners_.begin(); it != native_runners_.end(); ++it) {
     if (it->get() == runner) {

mojo/shell/shell.h

@@ -87,11 +87,6 @@ class Shell : public ShellClient {
  private:
   class Instance;
 
-  using IdentityToInstanceMap = std::map<Identity, Instance*>;
-  using NameToLoaderMap = std::map<std::string, Loader*>;
-  using IdentityToShellClientFactoryMap =
-      std::map<Identity, mojom::ShellClientFactoryPtr>;
-
   // ShellClient:
   bool AcceptConnection(Connection* connection) override;
 
@@ -125,7 +120,8 @@ class Shell : public ShellClient {
   // and this function returns true.
   bool ConnectToExistingInstance(scoped_ptr<ConnectParams>* params);
 
-  Instance* CreateInstance(const Identity& target,
+  Instance* CreateInstance(const Identity& source,
+                           const Identity& target,
                            const CapabilitySpec& spec);
 
   // Called from the instance implementing mojom::Shell.
@@ -168,20 +164,22 @@ class Shell : public ShellClient {
   // is no loader configured for the name.
   Loader* GetLoaderForName(const std::string& name);
 
+  base::WeakPtr<Shell> GetWeakPtr();
+
   void CleanupRunner(NativeRunner* runner);
 
   // Loader management.
   // Loaders are chosen in the order they are listed here.
-  NameToLoaderMap name_to_loader_;
+  std::map<std::string, Loader*> name_to_loader_;
   scoped_ptr<Loader> default_loader_;
 
-  IdentityToInstanceMap identity_to_instance_;
+  std::map<Identity, Instance*> identity_to_instance_;
 
   // Tracks the names of instances that are allowed to field connection requests
   // from all users.
   std::set<std::string> singletons_;
 
-  IdentityToShellClientFactoryMap shell_client_factories_;
+  std::map<Identity, mojom::ShellClientFactoryPtr> shell_client_factories_;
   // Counter used to assign ids to client factories.
   uint32_t shell_client_factory_id_counter_;
 

mojo/shell/tests/connect/BUILD.gn

@@ -24,6 +24,7 @@ source_set("connect") {
   data_deps = [
     ":connect_test_app",
     ":connect_test_class_app",
+    ":connect_test_singleton_app",
     ":connect_test_driver",
     ":connect_test_exe",
     ":connect_test_package",
@@ -123,6 +124,24 @@ mojo_application_manifest("connect_test_class_app_manifest") {
   source = "connect_test_class_app_manifest.json"
 }
 
+mojo_native_application("connect_test_singleton_app") {
+  testonly = true
+  sources = [
+    "connect_test_singleton_app.cc",
+  ]
+  deps = [
+    ":connect_test_singleton_app_manifest",
+    "//base",
+    "//mojo/common:common_base",
+    "//mojo/shell/public/cpp:sources",
+  ]
+}
+
+mojo_application_manifest("connect_test_singleton_app_manifest") {
+  application_name = "connect_test_singleton_app"
+  source = "connect_test_singleton_app_manifest.json"
+}
+
 executable("connect_test_driver") {
   testonly = true
 

mojo/shell/tests/connect/connect_test_app_manifest.json

@@ -9,7 +9,7 @@
         "classes": [ "class" ],
         "interfaces": ["mojo::shell::test::mojom::ConnectTestService"]
       },
-      "mojo:shell": { "classes": [ "user_id", "all_users" ] }
+      "mojo:shell": { "classes": [ "user_id" ] }
     }
   }
 }

mojo/shell/tests/connect/connect_test_singleton_app.cc

+// Copyright 2016 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 "base/macros.h"
+#include "mojo/public/c/system/main.h"
+#include "mojo/shell/public/cpp/application_runner.h"
+#include "mojo/shell/public/cpp/shell_client.h"
+
+namespace mojo {
+namespace shell {
+
+class ConnectTestSingletonApp : public ShellClient {
+ public:
+  ConnectTestSingletonApp() {}
+  ~ConnectTestSingletonApp() override {}
+
+ private:
+  // mojo::ShellClient:
+  void Initialize(Connector* connector, const Identity& identity,
+                  uint32_t id) override {}
+  bool AcceptConnection(Connection* connection) override {
+    return true;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ConnectTestSingletonApp);
+};
+
+}  // namespace shell
+}  // namespace mojo
+
+
+MojoResult MojoMain(MojoHandle shell_handle) {
+  return mojo::ApplicationRunner(
+      new mojo::shell::ConnectTestSingletonApp).Run(shell_handle);
+}

mojo/shell/tests/connect/connect_test_singleton_app_manifest.json

+{
+  "manifest_version": 1,
+  "name": "mojo:connect_test_singleton_app",
+  "display_name": "Connect Test Singleton App",
+  "capabilities": {
+    "required": {
+      "mojo:shell": { "classes": [ "all_users" ] }
+    }
+  }
+}

mojo/shell/tests/connect/connect_unittest.cc

@@ -30,6 +30,7 @@ const char kTestAppName[] = "mojo:connect_test_app";
 const char kTestAppAName[] = "mojo:connect_test_a";
 const char kTestAppBName[] = "mojo:connect_test_b";
 const char kTestClassAppName[] = "mojo:connect_test_class_app";
+const char kTestSingletonAppName[] = "mojo:connect_test_singleton_app";
 const char kTestDriverName[] = "exe:connect_test_driver";
 
 void ReceiveOneString(std::string* out_string,
@@ -355,26 +356,30 @@ TEST_F(ConnectTest, ConnectToClientProcess_Blocked) {
 // Verifies that a client with the "all_users" capability class can receive
 // connections from clients run as other users.
 TEST_F(ConnectTest, AllUsersSingleton) {
-  // Connect to an instance with an explicitly different user_id.
+  // Connect to an instance with an explicitly different user_id. This supplied
+  // user id should be ignored by the shell (which will generate its own
+  // synthetic user id for all-user singleton instances).
   const std::string singleton_userid = base::GenerateGUID();
-  Connector::ConnectParams params(Identity(kTestAppName, singleton_userid));
+  Connector::ConnectParams params(
+      Identity(kTestSingletonAppName, singleton_userid));
   scoped_ptr<Connection> connection = connector()->Connect(&params);
   {
     base::RunLoop loop;
     connection->AddConnectionCompletedClosure(base::Bind(&QuitLoop, &loop));
     loop.Run();
-    EXPECT_EQ(connection->GetRemoteIdentity().user_id(), singleton_userid);
+    EXPECT_NE(connection->GetRemoteIdentity().user_id(), singleton_userid);
   }
-  // This connects using the current client's user_id, but should be bound to
-  // the instance run as |singleton_userid|.
+  // This connects using the current client's user_id. It should be bound to the
+  // same service started above, with the same shell-generated user id.
   scoped_ptr<Connection> inherit_connection =
-      connector()->Connect(kTestAppName);
+      connector()->Connect(kTestSingletonAppName);
   {
     base::RunLoop loop;
     inherit_connection->AddConnectionCompletedClosure(
         base::Bind(&QuitLoop, &loop));
     loop.Run();
-    EXPECT_EQ(connection->GetRemoteIdentity().user_id(), singleton_userid);
+    EXPECT_EQ(inherit_connection->GetRemoteIdentity().user_id(),
+              connection->GetRemoteIdentity().user_id());
   }
 }
 

mojo/shell/tests/lifecycle/BUILD.gn

@@ -25,6 +25,7 @@ source_set("lifecycle") {
 
   data_deps = [
     ":lifecycle_unittest_app",
+    ":lifecycle_unittest_parent",
     ":lifecycle_unittest_exe",
     ":lifecycle_unittest_package",
     ":manifest",
@@ -115,6 +116,27 @@ mojo_application_manifest("lifecycle_unittest_app_manifest") {
   source = "app_manifest.json"
 }
 
+mojo_native_application("lifecycle_unittest_parent") {
+  testonly = true
+  sources = [
+    "parent.cc",
+  ]
+  deps = [
+    ":interfaces",
+    "//base",
+    "//mojo/shell/public/cpp:sources",
+  ]
+
+  data_deps = [
+    ":lifecycle_unittest_parent_manifest",
+  ]
+}
+
+mojo_application_manifest("lifecycle_unittest_parent_manifest") {
+  application_name = "lifecycle_unittest_parent"
+  source = "parent_manifest.json"
+}
+
 executable("lifecycle_unittest_exe") {
   testonly = true
   sources = [

mojo/shell/tests/lifecycle/app_client.cc

@@ -20,6 +20,10 @@ bool AppClient::AcceptConnection(mojo::Connection* connection) {
   return true;
 }
 
+void AppClient::ShellConnectionLost() {
+  GracefulQuit();
+}
+
 void AppClient::Create(mojo::Connection* connection,
                        LifecycleControlRequest request) {
   bindings_.AddBinding(this, std::move(request));

mojo/shell/tests/lifecycle/app_client.h

@@ -37,6 +37,7 @@ class AppClient : public ShellClient,
 
   // ShellClient:
   bool AcceptConnection(Connection* connection) override;
+  void ShellConnectionLost() override;
 
   // InterfaceFactory<LifecycleControl>:
   void Create(Connection* connection, LifecycleControlRequest request) override;

mojo/shell/tests/lifecycle/lifecycle_unittest.cc

@@ -18,6 +18,7 @@ namespace shell {
 namespace {
 
 const char kTestAppName[] = "mojo:lifecycle_unittest_app";
+const char kTestParentName[] = "mojo:lifecycle_unittest_parent";
 const char kTestExeName[] = "exe:lifecycle_unittest_exe";
 const char kTestPackageName[] = "mojo:lifecycle_unittest_package";
 const char kTestPackageAppNameA[] = "mojo:lifecycle_unittest_package_app_a";
@@ -438,6 +439,34 @@ TEST_F(LifecycleTest, Exe_TerminateProcess) {
   EXPECT_EQ(0u, instances()->GetNewInstanceCount());
 }
 
+TEST_F(LifecycleTest, ShutdownTree) {
+  // Verifies that Instances are destroyed when their creator is.
+  scoped_ptr<Connection> parent_connection =
+      connector()->Connect(kTestParentName);
+  test::mojom::ParentPtr parent;
+  parent_connection->GetInterface(&parent);
+
+  // This asks kTestParentName to open a connection to kTestAppName and blocks
+  // on a response from a Ping().
+  {
+    base::RunLoop loop;
+    parent->ConnectToChild(base::Bind(&QuitLoop, &loop));
+    loop.Run();
+  }
+
+  // Should now have two new instances (parent and child).
+  EXPECT_EQ(2u, instances()->GetNewInstanceCount());
+  EXPECT_TRUE(instances()->HasInstanceForName(kTestParentName));
+  EXPECT_TRUE(instances()->HasInstanceForName(kTestAppName));
+
+  parent->Quit();
+
+  // Quitting the parent should cascade-quit the child.
+  WaitForInstanceDestruction();
+  EXPECT_EQ(0u, instances()->GetNewInstanceCount());
+  EXPECT_FALSE(instances()->HasInstanceForName(kTestParentName));
+  EXPECT_FALSE(instances()->HasInstanceForName(kTestAppName));
+}
 
 }  // namespace shell
 }  // namespace mojo

mojo/shell/tests/lifecycle/lifecycle_unittest.mojom

@@ -21,3 +21,12 @@ interface LifecycleControl {
   // longer tracking this application.
   CloseShellConnection();
 };
+
+// Implemented by an app that connects to another app, thereby creating an
+// instance for it in the shell.
+interface Parent {
+  // Connects to another app and runs the callback when that app has acked a
+  // Ping.
+  ConnectToChild() => ();
+  Quit();
+};

mojo/shell/tests/lifecycle/parent.cc

+// Copyright 2016 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 "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "mojo/public/c/system/main.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/shell/public/cpp/application_runner.h"
+#include "mojo/shell/public/cpp/connector.h"
+#include "mojo/shell/public/cpp/shell_client.h"
+#include "mojo/shell/tests/lifecycle/lifecycle_unittest.mojom.h"
+
+namespace {
+
+void QuitLoop(base::RunLoop* loop) {
+  loop->Quit();
+}
+
+class Parent
+    : public mojo::ShellClient,
+      public mojo::InterfaceFactory<mojo::shell::test::mojom::Parent>,
+      public mojo::shell::test::mojom::Parent {
+ public:
+  Parent() {}
+  ~Parent() override {
+    connector_ = nullptr;
+    child_connection_.reset();
+    parent_bindings_.CloseAllBindings();
+  }
+
+ private:
+  // ShellClient:
+  void Initialize(mojo::Connector* connector, const mojo::Identity& identity,
+                  uint32_t id) override {
+    connector_ = connector;
+  }
+  bool AcceptConnection(mojo::Connection* connection) override {
+    connection->AddInterface<mojo::shell::test::mojom::Parent>(this);
+    return true;
+  }
+
+  // InterfaceFactory<mojo::shell::test::mojom::Parent>:
+  void Create(mojo::Connection* connection,
+              mojo::shell::test::mojom::ParentRequest request) override {
+    parent_bindings_.AddBinding(this, std::move(request));
+  }
+
+  // Parent:
+  void ConnectToChild(const ConnectToChildCallback& callback) override {
+    child_connection_ = connector_->Connect("mojo:lifecycle_unittest_app");
+    mojo::shell::test::mojom::LifecycleControlPtr lifecycle;
+    child_connection_->GetInterface(&lifecycle);
+    {
+      base::RunLoop loop;
+      lifecycle->Ping(base::Bind(&QuitLoop, &loop));
+      base::MessageLoop::ScopedNestableTaskAllower allow(
+          base::MessageLoop::current());
+      loop.Run();
+    }
+    callback.Run();
+  }
+  void Quit() override {
+    base::MessageLoop::current()->QuitWhenIdle();
+  }
+
+  mojo::Connector* connector_;
+  scoped_ptr<mojo::Connection> child_connection_;
+  mojo::BindingSet<mojo::shell::test::mojom::Parent> parent_bindings_;
+
+  DISALLOW_COPY_AND_ASSIGN(Parent);
+};
+
+}  // namespace
+
+MojoResult MojoMain(MojoHandle shell_handle) {
+  Parent* parent = new Parent;
+  return mojo::ApplicationRunner(parent).Run(shell_handle);
+}

mojo/shell/tests/lifecycle/parent_manifest.json

+{
+  "manifest_version":  1,
+  "name": "mojo:lifecycle_unittest_parent",
+  "display_name": "Lifecycle Unittest Parent",
+  "capabilities": {
+    "required": {
+      "mojo:lifecycle_unittest_app": {
+        "interfaces": [ "mojo::shell::test::mojom::LifecycleControl" ]
+      }
+    }
+  }
+}