Fix chrome --mash

Reduce the number of cases in which the resolved_qualifier is used. It should only be used in lieu of the one provided via params when it differs from the default qualifier for the resolved name.

TBR=sky@chromium.org
BUG=

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

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

5 files changed, 56 insertions, 25 deletions

diff --git a/mojo/shell/shell.cc b/mojo/shell/shell.cc
index 4955c97..277379f 100644
--- a/mojo/shell/shell.cc
+++ b/mojo/shell/shell.cc
@@ -529,8 +529,10 @@ void Shell::OnGotResolvedName(scoped_ptr<ConnectParams> params,
                               mojom::CapabilityFilterPtr base_filter,
                               const String& file_url) {
   std::string instance_name = params->target().instance();
-  if (instance_name == GetNamePath(params->target().name()))
+  if (instance_name == GetNamePath(params->target().name()) &&
+      resolved_instance != GetNamePath(resolved_name)) {
     instance_name = resolved_instance;
+  }
   Identity target(params->target().name(), params->target().user_id(),
                   instance_name);
   params->set_target(target);
@@ -562,7 +564,7 @@ void Shell::OnGotResolvedName(scoped_ptr<ConnectParams> params,
     // from the original request rather than for the package itself, which will
     // always be the same.
     CreateShellClient(
-        source, Identity(resolved_name, target.user_id(), resolved_instance),
+        source, Identity(resolved_name, target.user_id(), instance_name),
         target.name(), std::move(request));
   } else {
     bool start_sandboxed = false;
diff --git a/mojo/shell/tests/connect/connect_test.mojom b/mojo/shell/tests/connect/connect_test.mojom
index 3eeaa12..ba29865 100644
--- a/mojo/shell/tests/connect/connect_test.mojom
+++ b/mojo/shell/tests/connect/connect_test.mojom
@@ -6,6 +6,7 @@ module mojo.shell.test.mojom;
 
 interface ConnectTestService {
   GetTitle() => (string title);
+  GetInstance() => (string instance);
 };
 
 // Interface implemented by a standalone (non-package) app.
@@ -14,8 +15,6 @@ interface StandaloneApp {
   // the standalone app's CapabilityFilter, but whose enclosing package is not.
   // The connection should be blocked and title should be "uninitialized".
   ConnectToAllowedAppInBlockedPackage() => (string title);
-
-  GetInstance() => (string instance);
 };
 
 struct ConnectionState {
diff --git a/mojo/shell/tests/connect/connect_test_app.cc b/mojo/shell/tests/connect/connect_test_app.cc
index 205ddcf..86ae235 100644
--- a/mojo/shell/tests/connect/connect_test_app.cc
+++ b/mojo/shell/tests/connect/connect_test_app.cc
@@ -93,6 +93,9 @@ class ConnectTestApp : public ShellClient,
   void GetTitle(const GetTitleCallback& callback) override {
     callback.Run("APP");
   }
+  void GetInstance(const GetInstanceCallback& callback) override {
+    callback.Run(identity_.instance());
+  }
 
   // test::mojom::StandaloneApp:
   void ConnectToAllowedAppInBlockedPackage(
@@ -116,9 +119,6 @@ class ConnectTestApp : public ShellClient,
       run_loop.Run();
     }
   }
-  void GetInstance(const GetInstanceCallback& callback) override {
-    callback.Run(identity_.instance());
-  }
 
   // test::mojom::BlockedInterface:
   void GetTitleBlocked(const GetTitleBlockedCallback& callback) override {
diff --git a/mojo/shell/tests/connect/connect_test_package.cc b/mojo/shell/tests/connect/connect_test_package.cc
index 7956425..6f365e1 100644
--- a/mojo/shell/tests/connect/connect_test_package.cc
+++ b/mojo/shell/tests/connect/connect_test_package.cc
@@ -52,8 +52,7 @@ class ProvidedShellClient
   // mojo::ShellClient:
   void Initialize(Connector* connector, const Identity& identity,
                   uint32_t id) override {
-    name_ = identity.name();
-    userid_ = identity.user_id();
+    identity_ = identity;
     id_ = id;
     bindings_.set_connection_error_handler(
         base::Bind(&ProvidedShellClient::OnConnectionError,
@@ -69,9 +68,9 @@ class ProvidedShellClient
     state->connection_remote_name = connection->GetRemoteIdentity().name();
     state->connection_remote_userid = connection->GetRemoteIdentity().user_id();
     state->connection_remote_id = remote_id;
-    state->initialize_local_name = name_;
+    state->initialize_local_name = identity_.name();
     state->initialize_id = id_;
-    state->initialize_userid = userid_;
+    state->initialize_userid = identity_.user_id();
     connection->GetInterface(&caller_);
     caller_->ConnectionAccepted(std::move(state));
 
@@ -94,6 +93,9 @@ class ProvidedShellClient
   void GetTitle(const GetTitleCallback& callback) override {
     callback.Run(title_);
   }
+  void GetInstance(const GetInstanceCallback& callback) override {
+    callback.Run(identity_.instance());
+  }
 
   // test::mojom::BlockedInterface:
   void GetTitleBlocked(const GetTitleBlockedCallback& callback) override {
@@ -112,9 +114,8 @@ class ProvidedShellClient
       base::MessageLoop::current()->QuitWhenIdle();
   }
 
-  std::string name_;
+  Identity identity_;
   uint32_t id_ = shell::mojom::kInvalidInstanceID;
-  std::string userid_ = mojom::kRootUserID;
   const std::string title_;
   mojom::ShellClientRequest request_;
   test::mojom::ExposedInterfacePtr caller_;
@@ -138,6 +139,7 @@ class ConnectTestShellClient
   // mojo::ShellClient:
   void Initialize(Connector* connector, const Identity& identity,
                   uint32_t id) override {
+    identity_ = identity;
     bindings_.set_connection_error_handler(
         base::Bind(&ConnectTestShellClient::OnConnectionError,
                    base::Unretained(this)));
@@ -179,12 +181,16 @@ class ConnectTestShellClient
   void GetTitle(const GetTitleCallback& callback) override {
     callback.Run("ROOT");
   }
+  void GetInstance(const GetInstanceCallback& callback) override {
+    callback.Run(identity_.instance());
+  }
 
   void OnConnectionError() {
     if (bindings_.empty())
       base::MessageLoop::current()->QuitWhenIdle();
   }
 
+  Identity identity_;
   std::vector<scoped_ptr<ShellClient>> delegates_;
   BindingSet<mojom::ShellClientFactory> shell_client_factory_bindings_;
   BindingSet<test::mojom::ConnectTestService> bindings_;
diff --git a/mojo/shell/tests/connect/connect_unittest.cc b/mojo/shell/tests/connect/connect_unittest.cc
index e28a635..38aee7a 100644
--- a/mojo/shell/tests/connect/connect_unittest.cc
+++ b/mojo/shell/tests/connect/connect_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "base/test/test_suite.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/shell/public/cpp/names.h"
 #include "mojo/shell/public/cpp/shell_test.h"
 #include "mojo/shell/public/interfaces/shell.mojom.h"
 #include "mojo/shell/tests/connect/connect_test.mojom.h"
@@ -136,20 +137,18 @@ TEST_F(ConnectTest, Instances) {
   scoped_ptr<Connection> connection_a1 = ConnectTo(&params_a);
   scoped_ptr<Connection> connection_a2 = ConnectTo(&params_a);
   std::string instance_a1, instance_a2;
-  test::mojom::StandaloneAppPtr standalone_app_a1;
+  test::mojom::ConnectTestServicePtr service_a1;
   {
-    connection_a1->GetInterface(&standalone_app_a1);
+    connection_a1->GetInterface(&service_a1);
     base::RunLoop loop;
-    standalone_app_a1->GetInstance(
-        base::Bind(&ReceiveTitle, &instance_a1, &loop));
+    service_a1->GetInstance(base::Bind(&ReceiveTitle, &instance_a1, &loop));
     loop.Run();
   }
-  test::mojom::StandaloneAppPtr standalone_app_a2;
+  test::mojom::ConnectTestServicePtr service_a2;
   {
-    connection_a2->GetInterface(&standalone_app_a2);
+    connection_a2->GetInterface(&service_a2);
     base::RunLoop loop;
-    standalone_app_a2->GetInstance(
-        base::Bind(&ReceiveTitle, &instance_a2, &loop));
+    service_a2->GetInstance(base::Bind(&ReceiveTitle, &instance_a2, &loop));
     loop.Run();
   }
   EXPECT_EQ(instance_a1, instance_a2);
@@ -158,18 +157,43 @@ TEST_F(ConnectTest, Instances) {
       Identity(kTestAppName, mojom::kInheritUserID, "B"));
   scoped_ptr<Connection> connection_b = ConnectTo(&params_b);
   std::string instance_b;
-  test::mojom::StandaloneAppPtr standalone_app_b;
+  test::mojom::ConnectTestServicePtr service_b;
   {
-    connection_b->GetInterface(&standalone_app_b);
+    connection_b->GetInterface(&service_b);
     base::RunLoop loop;
-    standalone_app_b->GetInstance(
-        base::Bind(&ReceiveTitle, &instance_b, &loop));
+    service_b->GetInstance(base::Bind(&ReceiveTitle, &instance_b, &loop));
     loop.Run();
   }
 
   EXPECT_NE(instance_a1, instance_b);
 }
 
+// When both the unresolved and resolved instance names are their default
+// values, the instance name from the unresolved name must be used.
+// (The case where the instance names differ is covered by
+// LifecycleTest.PackagedApp_CrashCrashesOtherProvidedApp).
+TEST_F(ConnectTest, PreferUnresolvedDefaultInstanceName) {
+  // Connect to an app with no manifest-supplied instance name provided by a
+  // package, the instance name must be derived from the application instance
+  // name, not the package.
+  scoped_ptr<Connection> connection = connector()->Connect(kTestAppName);
+  {
+    base::RunLoop loop;
+    connection->AddConnectionCompletedClosure(base::Bind(&QuitLoop, &loop));
+    loop.Run();
+  }
+
+  std::string instance;
+  {
+    test::mojom::ConnectTestServicePtr service;
+    connection->GetInterface(&service);
+    base::RunLoop loop;
+    service->GetInstance(base::Bind(&ReceiveTitle, &instance, &loop));
+    loop.Run();
+  }
+  EXPECT_EQ(GetNamePath(kTestAppName), instance);
+}
+
 // BlockedInterface should not be exposed to this application because it is not
 // in our CapabilityFilter whitelist.
 TEST_F(ConnectTest, BlockedInterface) {