Neuter the wrong-way ServiceProvider (a.k.a. exposed_services). You can still give it to ConnectToApplication(), but mojo_shell will drop it on the floor. R=vardhan@google.com BUG=#762 Review URL: https://codereview.chromium.org/1965563002 .
diff --git a/mojo/public/interfaces/application/application.mojom b/mojo/public/interfaces/application/application.mojom index 7f1ebe5..0ed5154 100644 --- a/mojo/public/interfaces/application/application.mojom +++ b/mojo/public/interfaces/application/application.mojom
@@ -50,6 +50,10 @@ // resolved_url is the URL that was requested to create this connection, after // all mappings, resolutions, and redirects. This will include any querystring // that was part of the request. + // + // WARNING: |exposed_services| is deprecated and non-functional, and will be + // removed. Note that this also means that |services| will become required. + // https://github.com/domokit/mojo/issues/762 AcceptConnection(string requestor_url, ServiceProvider&? services, ServiceProvider? exposed_services,
diff --git a/mojo/public/interfaces/application/application_connector.mojom b/mojo/public/interfaces/application/application_connector.mojom index 23df7f6..7cbb679 100644 --- a/mojo/public/interfaces/application/application_connector.mojom +++ b/mojo/public/interfaces/application/application_connector.mojom
@@ -31,6 +31,10 @@ // "mojo:{service}", it is up to the Mojo shell to select an appropriate // application for the service. Currently, the shell does this based on the // value of its --origin flag. + // + // WARNING: |exposed_services| is deprecated and non-functional, and will be + // removed. Note that this also means that |services| will become required. + // https://github.com/domokit/mojo/issues/762 ConnectToApplication(string application_url, ServiceProvider&? services, ServiceProvider? exposed_services);
diff --git a/mojo/public/interfaces/application/shell.mojom b/mojo/public/interfaces/application/shell.mojom index dc210c7..d00ac01 100644 --- a/mojo/public/interfaces/application/shell.mojom +++ b/mojo/public/interfaces/application/shell.mojom
@@ -12,7 +12,13 @@ // system and request connections to other applications. interface Shell { // See |ApplicationConnector.ConnectToApplication()|. - // TODO(vtl): Maybe this should be deprecated/removed. + // + // WARNING: |exposed_services| is deprecated and non-functional, and will be + // removed. Note that this also means that |services| will become required. + // https://github.com/domokit/mojo/issues/762 + // + // TODO(vtl): Maybe this should be deprecated/removed (in favor of just using + // |ApplicationConnector|). ConnectToApplication(string application_url, ServiceProvider&? services, ServiceProvider? exposed_services);
diff --git a/shell/application_manager/application_manager.cc b/shell/application_manager/application_manager.cc index f5560e5..2ee6c59 100644 --- a/shell/application_manager/application_manager.cc +++ b/shell/application_manager/application_manager.cc
@@ -64,8 +64,7 @@ : manager_(manager), identity_(identity) { ServiceProviderPtr services; manager->ConnectToApplication(identity_.url, GURL(), - mojo::GetProxy(&services), nullptr, - base::Closure()); + mojo::GetProxy(&services), base::Closure()); mojo::MessagePipe pipe; content_handler_.Bind( mojo::InterfaceHandle<mojo::ContentHandler>(pipe.handle0.Pass(), 0u)); @@ -128,18 +127,16 @@ const GURL& requested_url, const GURL& requestor_url, InterfaceRequest<ServiceProvider> services, - mojo::InterfaceHandle<ServiceProvider> exposed_services, const base::Closure& on_application_end) { - ConnectToApplicationWithParameters( - requested_url, requestor_url, services.Pass(), exposed_services.Pass(), - on_application_end, std::vector<std::string>()); + ConnectToApplicationWithParameters(requested_url, requestor_url, + services.Pass(), on_application_end, + std::vector<std::string>()); } void ApplicationManager::ConnectToApplicationWithParameters( const GURL& requested_url, const GURL& requestor_url, InterfaceRequest<ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& pre_redirect_parameters) { TRACE_EVENT_INSTANT2( @@ -152,43 +149,36 @@ // external applications can be registered for the unresolved mojo:foo urls. GURL mapped_url = delegate_->ResolveMappings(requested_url); - if (ConnectToRunningApplication(mapped_url, requestor_url, &services, - &exposed_services)) { + if (ConnectToRunningApplication(mapped_url, requestor_url, &services)) return; - } GURL resolved_url = delegate_->ResolveMojoURL(mapped_url); - if (ConnectToRunningApplication(resolved_url, requestor_url, &services, - &exposed_services)) { + if (ConnectToRunningApplication(resolved_url, requestor_url, &services)) return; - } // The application is not running, let's compute the parameters. std::vector<std::string> parameters = Concatenate(pre_redirect_parameters, GetArgsForURL(resolved_url)); if (ConnectToApplicationWithLoader(mapped_url, requestor_url, &services, - &exposed_services, on_application_end, - parameters, GetLoaderForURL(mapped_url))) { + on_application_end, parameters, + GetLoaderForURL(mapped_url))) return; - } - - if (ConnectToApplicationWithLoader( - resolved_url, requestor_url, &services, &exposed_services, - on_application_end, parameters, GetLoaderForURL(resolved_url))) { - return; - } if (ConnectToApplicationWithLoader(resolved_url, requestor_url, &services, - &exposed_services, on_application_end, - parameters, default_loader_.get())) { + on_application_end, parameters, + GetLoaderForURL(resolved_url))) return; - } - auto callback = base::Bind( - &ApplicationManager::HandleFetchCallback, weak_ptr_factory_.GetWeakPtr(), - requestor_url, base::Passed(services.Pass()), - base::Passed(exposed_services.Pass()), on_application_end, parameters); + if (ConnectToApplicationWithLoader(resolved_url, requestor_url, &services, + on_application_end, parameters, + default_loader_.get())) + return; + + auto callback = + base::Bind(&ApplicationManager::HandleFetchCallback, + weak_ptr_factory_.GetWeakPtr(), requestor_url, + base::Passed(services.Pass()), on_application_end, parameters); if (resolved_url.SchemeIsFile()) { new LocalFetcher(resolved_url, GetBaseURLAndQuery(resolved_url, nullptr), @@ -242,8 +232,7 @@ bool ApplicationManager::ConnectToRunningApplication( const GURL& resolved_url, const GURL& requestor_url, - InterfaceRequest<ServiceProvider>* services, - mojo::InterfaceHandle<mojo::ServiceProvider>* exposed_services) { + InterfaceRequest<ServiceProvider>* services) { GURL application_url = GetBaseURLAndQuery(resolved_url, nullptr); ShellImpl* shell_impl = GetShellImpl(application_url); if (!shell_impl) @@ -252,8 +241,7 @@ DCHECK(!GetNativeApplicationOptionsForURL(application_url) ->new_process_per_connection); - ConnectToClient(shell_impl, resolved_url, requestor_url, services->Pass(), - exposed_services->Pass()); + ConnectToClient(shell_impl, resolved_url, requestor_url, services->Pass()); return true; } @@ -261,17 +249,15 @@ const GURL& resolved_url, const GURL& requestor_url, InterfaceRequest<ServiceProvider>* services, - mojo::InterfaceHandle<mojo::ServiceProvider>* exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& parameters, ApplicationLoader* loader) { if (!loader) return false; - loader->Load( - resolved_url, - RegisterShell(resolved_url, requestor_url, services->Pass(), - exposed_services->Pass(), on_application_end, parameters)); + loader->Load(resolved_url, + RegisterShell(resolved_url, requestor_url, services->Pass(), + on_application_end, parameters)); return true; } @@ -292,7 +278,6 @@ const GURL& resolved_url, const GURL& requestor_url, InterfaceRequest<ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& parameters) { Identity app_identity = MakeApplicationIdentity(resolved_url); @@ -304,8 +289,7 @@ new ShellImpl(application.Pass(), this, app_identity, on_application_end); identity_to_shell_impl_[app_identity] = make_scoped_ptr(shell); shell->InitializeApplication(mojo::Array<mojo::String>::From(parameters)); - ConnectToClient(shell, resolved_url, requestor_url, services.Pass(), - exposed_services.Pass()); + ConnectToClient(shell, resolved_url, requestor_url, services.Pass()); return application_request; } @@ -324,16 +308,13 @@ ShellImpl* shell_impl, const GURL& resolved_url, const GURL& requestor_url, - InterfaceRequest<ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services) { - shell_impl->ConnectToClient(resolved_url, requestor_url, services.Pass(), - exposed_services.Pass()); + InterfaceRequest<ServiceProvider> services) { + shell_impl->ConnectToClient(resolved_url, requestor_url, services.Pass()); } void ApplicationManager::HandleFetchCallback( const GURL& requestor_url, InterfaceRequest<ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& parameters, scoped_ptr<Fetcher> fetcher) { @@ -346,8 +327,8 @@ if (!redirect_url.is_empty()) { // And around we go again... Whee! ConnectToApplicationWithParameters(redirect_url, requestor_url, - services.Pass(), exposed_services.Pass(), - on_application_end, parameters); + services.Pass(), on_application_end, + parameters); return; } @@ -357,14 +338,12 @@ // // Also, it's possible the original URL was redirected to an app that is // already running. - if (ConnectToRunningApplication(fetcher->GetURL(), requestor_url, &services, - &exposed_services)) { + if (ConnectToRunningApplication(fetcher->GetURL(), requestor_url, &services)) return; - } InterfaceRequest<Application> request( RegisterShell(fetcher->GetURL(), requestor_url, services.Pass(), - exposed_services.Pass(), on_application_end, parameters)); + on_application_end, parameters)); // If the response begins with a #!mojo <content-handler-url>, use it. GURL content_handler_url; @@ -538,7 +517,7 @@ const std::string& interface_name) { ServiceProviderPtr services; ConnectToApplication(application_url, GURL(), mojo::GetProxy(&services), - nullptr, base::Closure()); + base::Closure()); mojo::MessagePipe pipe; services->ConnectToService(interface_name, pipe.handle1.Pass()); return pipe.handle0.Pass();
diff --git a/shell/application_manager/application_manager.h b/shell/application_manager/application_manager.h index 72cc947..edfb46e 100644 --- a/shell/application_manager/application_manager.h +++ b/shell/application_manager/application_manager.h
@@ -81,7 +81,6 @@ const GURL& application_url, const GURL& requestor_url, mojo::InterfaceRequest<mojo::ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end); template <typename Interface> @@ -156,21 +155,18 @@ const GURL& application_url, const GURL& requestor_url, mojo::InterfaceRequest<mojo::ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& pre_redirect_parameters); bool ConnectToRunningApplication( const GURL& resolved_url, const GURL& requestor_url, - mojo::InterfaceRequest<mojo::ServiceProvider>* services, - mojo::InterfaceHandle<mojo::ServiceProvider>* exposed_services); + mojo::InterfaceRequest<mojo::ServiceProvider>* services); bool ConnectToApplicationWithLoader( const GURL& resolved_url, const GURL& requestor_url, mojo::InterfaceRequest<mojo::ServiceProvider>* services, - mojo::InterfaceHandle<mojo::ServiceProvider>* exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& parameters, ApplicationLoader* loader); @@ -188,23 +184,19 @@ const GURL& resolved_url, const GURL& requestor_url, mojo::InterfaceRequest<mojo::ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& parameters); ShellImpl* GetShellImpl(const GURL& url); - void ConnectToClient( - ShellImpl* shell_impl, - const GURL& resolved_url, - const GURL& requestor_url, - mojo::InterfaceRequest<mojo::ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services); + void ConnectToClient(ShellImpl* shell_impl, + const GURL& resolved_url, + const GURL& requestor_url, + mojo::InterfaceRequest<mojo::ServiceProvider> services); void HandleFetchCallback( const GURL& requestor_url, mojo::InterfaceRequest<mojo::ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services, const base::Closure& on_application_end, const std::vector<std::string>& parameters, scoped_ptr<Fetcher> fetcher);
diff --git a/shell/application_manager/application_manager_unittest.cc b/shell/application_manager/application_manager_unittest.cc index 60a708f..aa1f7ae 100644 --- a/shell/application_manager/application_manager_unittest.cc +++ b/shell/application_manager/application_manager_unittest.cc
@@ -789,7 +789,7 @@ bool called = false; application_manager_->ConnectToApplication( - GURL("test:test"), GURL(), nullptr, nullptr, + GURL("test:test"), GURL(), nullptr, base::Bind(&QuitClosure, base::Unretained(&called))); loop_.Run(); EXPECT_TRUE(called);
diff --git a/shell/application_manager/shell_impl.cc b/shell/application_manager/shell_impl.cc index b1c97c1..bdd38f1 100644 --- a/shell/application_manager/shell_impl.cc +++ b/shell/application_manager/shell_impl.cc
@@ -69,14 +69,12 @@ identity_.url.spec()); } -void ShellImpl::ConnectToClient( - const GURL& requested_url, - const GURL& requestor_url, - InterfaceRequest<ServiceProvider> services, - InterfaceHandle<ServiceProvider> exposed_services) { - application_->AcceptConnection( - String::From(requestor_url), std::move(services), - std::move(exposed_services), requested_url.spec()); +void ShellImpl::ConnectToClient(const GURL& requested_url, + const GURL& requestor_url, + InterfaceRequest<ServiceProvider> services) { + application_->AcceptConnection(String::From(requestor_url), + std::move(services), nullptr, + requested_url.spec()); } void ShellImpl::ConnectToApplication( @@ -88,8 +86,9 @@ LOG(ERROR) << "Error: invalid URL: " << app_url; return; } + LOG_IF(ERROR, exposed_services) << "exposed_services no longer supported!"; manager_->ConnectToApplication(app_gurl, identity_.url, std::move(services), - std::move(exposed_services), base::Closure()); + base::Closure()); } void ShellImpl::CreateApplicationConnector(
diff --git a/shell/application_manager/shell_impl.h b/shell/application_manager/shell_impl.h index c8af66b..e513367 100644 --- a/shell/application_manager/shell_impl.h +++ b/shell/application_manager/shell_impl.h
@@ -35,11 +35,9 @@ void InitializeApplication(mojo::Array<mojo::String> args); - void ConnectToClient( - const GURL& requested_url, - const GURL& requestor_url, - mojo::InterfaceRequest<mojo::ServiceProvider> services, - mojo::InterfaceHandle<mojo::ServiceProvider> exposed_services); + void ConnectToClient(const GURL& requested_url, + const GURL& requestor_url, + mojo::InterfaceRequest<mojo::ServiceProvider> services); mojo::Application* application() { return application_.get(); } const Identity& identity() const { return identity_; }
diff --git a/shell/context.cc b/shell/context.cc index 55aadb4..7e9e5dd 100644 --- a/shell/context.cc +++ b/shell/context.cc
@@ -311,7 +311,7 @@ ServiceProviderPtr tracing_services; application_manager_.ConnectToApplication(GURL("mojo:tracing"), GURL(""), GetProxy(&tracing_services), - nullptr, base::Closure()); + base::Closure()); if (tracer_) { tracing::TraceProviderRegistryPtr registry; mojo::ConnectToService(tracing_services.get(), GetProxy(®istry)); @@ -358,11 +358,10 @@ void Context::Run(const GURL& url) { ServiceProviderPtr services; - ServiceProviderPtr exposed_services; app_urls_.insert(url); application_manager_.ConnectToApplication( - url, GURL(), mojo::GetProxy(&services), exposed_services.Pass(), + url, GURL(), mojo::GetProxy(&services), base::Bind(&Context::OnApplicationEnd, base::Unretained(this), url)); }
diff --git a/shell/native_runner_unittest.cc b/shell/native_runner_unittest.cc index fe21447..6ab81aa 100644 --- a/shell/native_runner_unittest.cc +++ b/shell/native_runner_unittest.cc
@@ -88,10 +88,8 @@ ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); base::FilePath nonexistent_file(FILE_PATH_LITERAL("nonexistent.txt")); GURL url(FilePathToFileURL(temp_dir.path().Append(nonexistent_file))); - mojo::InterfaceRequest<mojo::ServiceProvider> services; - mojo::ServiceProviderPtr service_provider; - application_manager_.ConnectToApplication( - url, GURL(), services.Pass(), service_provider.Pass(), base::Closure()); + application_manager_.ConnectToApplication(url, GURL(), nullptr, + base::Closure()); EXPECT_FALSE(state_.runner_was_created); EXPECT_FALSE(state_.runner_was_started); EXPECT_FALSE(state_.runner_was_destroyed);
diff --git a/shell/shell_test_base.cc b/shell/shell_test_base.cc index 9be0abc..f698e6e 100644 --- a/shell/shell_test_base.cc +++ b/shell/shell_test_base.cc
@@ -28,11 +28,9 @@ } // namespace -ShellTestBase::ShellTestBase() { -} +ShellTestBase::ShellTestBase() {} -ShellTestBase::~ShellTestBase() { -} +ShellTestBase::~ShellTestBase() {} void ShellTestBase::SetUp() { CHECK(shell_context_.Init()); @@ -48,7 +46,7 @@ const std::string& service_name) { mojo::ServiceProviderPtr services; shell_context_.application_manager()->ConnectToApplication( - application_url, GURL(), mojo::GetProxy(&services), nullptr, + application_url, GURL(), mojo::GetProxy(&services), base::Bind(&QuitIfRunning)); mojo::MessagePipe pipe; services->ConnectToService(service_name, pipe.handle1.Pass());