From 8629ce72e06a8e4dae428f0fbc3ba83cc5555407 Mon Sep 17 00:00:00 2001 From: ben Date: Tue, 8 Mar 2016 07:18:21 -0800 Subject: 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} --- mojo/shell/shell.cc | 6 ++- mojo/shell/tests/connect/connect_test.mojom | 3 +- mojo/shell/tests/connect/connect_test_app.cc | 6 +-- mojo/shell/tests/connect/connect_test_package.cc | 18 ++++++--- mojo/shell/tests/connect/connect_unittest.cc | 48 ++++++++++++++++++------ 5 files changed, 56 insertions(+), 25 deletions(-) (limited to 'mojo') 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 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 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> delegates_; BindingSet shell_client_factory_bindings_; BindingSet 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_a1 = ConnectTo(¶ms_a); scoped_ptr connection_a2 = ConnectTo(¶ms_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_b = ConnectTo(¶ms_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 = 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) { -- cgit v1.1