Remove support for Root identity singletons and rely on the all_users bit to be set. + test

. set this bit on mus so we don't try to create per-user instances of it now that running it as root has no superpower.

R=sky@chromium.org

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

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

5 files changed, 41 insertions, 25 deletions

diff --git a/mojo/shell/shell.cc b/mojo/shell/shell.cc
index 6fb4874..4cefbc3 100644
--- a/mojo/shell/shell.cc
+++ b/mojo/shell/shell.cc
@@ -319,7 +319,7 @@ class Shell::Instance : public mojom::Connector,
                      mojom::kInheritUserID, mojom::kInvalidInstanceID);
         return false;
       }
-      if (shell_->GetExistingOrRootInstance(target)) {
+      if (shell_->GetExistingInstance(target)) {
         LOG(ERROR) << "Error: Cannot client process matching existing identity:"
                    << "Name: " << target.name() << " User: " << target.user_id()
                    << " Instance: " << target.instance();
@@ -560,27 +560,19 @@ void Shell::Connect(scoped_ptr<ConnectParams> params,
 
 Shell::Instance* Shell::GetExistingInstance(const Identity& identity) const {
   const auto& it = identity_to_instance_.find(identity);
-  return it != identity_to_instance_.end() ? it->second : nullptr;
-}
+  Instance* instance = it != identity_to_instance_.end() ? it->second : nullptr;
+  if (instance)
+    return instance;
 
-Shell::Instance* Shell::GetExistingOrRootInstance(
-    const Identity& identity) const {
-  Instance* instance = GetExistingInstance(identity);
-  if (!instance) {
-    if (singletons_.find(identity.name()) != singletons_.end()) {
-      for (auto entry : identity_to_instance_) {
-        if (entry.first.name() == identity.name() &&
-            entry.first.instance() == identity.instance()) {
-          return entry.second;
-        }
+  if (singletons_.find(identity.name()) != singletons_.end()) {
+    for (auto entry : identity_to_instance_) {
+      if (entry.first.name() == identity.name() &&
+          entry.first.instance() == identity.instance()) {
+        return entry.second;
       }
     }
-
-    Identity root_identity = identity;
-    root_identity.set_user_id(mojom::kRootUserID);
-    instance = GetExistingInstance(root_identity);
   }
-  return instance;
+  return nullptr;
 }
 
 void Shell::NotifyPIDAvailable(uint32_t id, base::ProcessId pid) {
@@ -590,7 +582,7 @@ void Shell::NotifyPIDAvailable(uint32_t id, base::ProcessId pid) {
 }
 
 bool Shell::ConnectToExistingInstance(scoped_ptr<ConnectParams>* params) {
-  Instance* instance = GetExistingOrRootInstance((*params)->target());
+  Instance* instance = GetExistingInstance((*params)->target());
   if (instance)
     instance->ConnectToClient(std::move(*params));
   return !!instance;
diff --git a/mojo/shell/shell.h b/mojo/shell/shell.h
index 7128a24..8b9f975 100644
--- a/mojo/shell/shell.h
+++ b/mojo/shell/shell.h
@@ -114,11 +114,9 @@ class Shell : public ShellClient {
   // |client| to control it.
   void Connect(scoped_ptr<ConnectParams> params, mojom::ShellClientPtr client);
 
-  // Returns a running instance matching |identity|.
+  // Returns a running instance matching |identity|. This might be an instance
+  // running as a different user if one is available that services all users.
   Instance* GetExistingInstance(const Identity& identity) const;
-  // Like GetExistingInstance, but if no instance for |identity.user_id()| is
-  // found, looks for kRootUserID too.
-  Instance* GetExistingOrRootInstance(const Identity& identity) const;
 
   void NotifyPIDAvailable(uint32_t id, base::ProcessId pid);
 
diff --git a/mojo/shell/tests/connect/connect_test_app_manifest.json b/mojo/shell/tests/connect/connect_test_app_manifest.json
index 7c45922..93b9782 100644
--- a/mojo/shell/tests/connect/connect_test_app_manifest.json
+++ b/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" ] }
+      "mojo:shell": { "classes": [ "user_id", "all_users" ] }
     }
   }
 }
diff --git a/mojo/shell/tests/connect/connect_unittest.cc b/mojo/shell/tests/connect/connect_unittest.cc
index 8dd65ce..9cfb0a7 100644
--- a/mojo/shell/tests/connect/connect_unittest.cc
+++ b/mojo/shell/tests/connect/connect_unittest.cc
@@ -352,6 +352,32 @@ TEST_F(ConnectTest, ConnectToClientProcess_Blocked) {
   EXPECT_EQ(shell::mojom::ConnectResult::ACCESS_DENIED, result);
 }
 
+// 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.
+  const std::string singleton_userid = base::GenerateGUID();
+  Connector::ConnectParams params(Identity(kTestAppName, 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);
+  }
+  // This connects using the current client's user_id, but should be bound to
+  // the instance run as |singleton_userid|.
+  scoped_ptr<Connection> inherit_connection =
+      connector()->Connect(kTestAppName);
+  {
+    base::RunLoop loop;
+    inherit_connection->AddConnectionCompletedClosure(
+        base::Bind(&QuitLoop, &loop));
+    loop.Run();
+    EXPECT_EQ(connection->GetRemoteIdentity().user_id(), singleton_userid);
+  }
+}
+
 // Tests that we can expose an interface to targets on outbound connections.
 TEST_F(ConnectTest, LocalInterface) {
   // Connect to a standalone application.
diff --git a/mojo/shell/tests/connect/connect_unittests_manifest.json b/mojo/shell/tests/connect/connect_unittests_manifest.json
index e0b99ce..ea650b1 100644
--- a/mojo/shell/tests/connect/connect_unittests_manifest.json
+++ b/mojo/shell/tests/connect/connect_unittests_manifest.json
@@ -22,7 +22,7 @@
           "mojo::shell::test::mojom::UserIdTest"
         ]
       },
-      "mojo:shell": { "classes": [ "instance_name" ] }
+      "mojo:shell": { "classes": [ "instance_name", "user_id" ] }
     }
   }
 }