Some more minor cleanup to ApplicationManager.

R=yzshen@chromium.org
http://crbug.com/533085

Committed: https://crrev.com/c6bde172a35b65b5a735675e840fab55921d28f8
Cr-Commit-Position: refs/heads/master@{#349515}

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

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

mandoline/app/android/BUILD.gn

@@ -27,6 +27,7 @@ executable("mandoline_runner") {
     "//mojo/package_manager",
     "//mojo/runner:lib",
     "//mojo/runner:mojo_runner_lib",
+    "//mojo/shell",
   ]
 
   sources = [

mandoline/app/android/mandoline_activity.cc

@@ -7,15 +7,17 @@
 #include "mandoline/ui/desktop_ui/public/interfaces/launch_handler.mojom.h"
 #include "mojo/runner/android/main.h"
 #include "mojo/runner/context.h"
+#include "mojo/shell/connect_util.h"
 
 namespace mandoline {
 
 static void LaunchURL(JNIEnv* env,
                       const JavaParamRef<jclass>& clazz,
                       const JavaParamRef<jstring>& jurl) {
   LaunchHandlerPtr launch_handler;
-  mojo::runner::GetContext()->application_manager()->ConnectToService(
-      GURL("mojo:phone_ui"), &launch_handler);
+  mojo::shell::ConnectToService(
+      mojo::runner::GetContext()->application_manager(), GURL("mojo:phone_ui"),
+      &launch_handler);
   launch_handler->LaunchURL(
       base::android::ConvertJavaStringToUTF8(env, jurl));
 }

mojo/fetcher/about_fetcher_unittest.cc

@@ -112,10 +112,12 @@ class AboutFetcherTest : public testing::Test {
 
     URLRequestPtr request(URLRequest::New());
     request->url = url;
-    application_manager_->ConnectToApplication(
-        nullptr, request.Pass(), std::string(), service_provider_request.Pass(),
-        nullptr, shell::CapabilityFilter(), base::Closure(),
-        shell::EmptyConnectCallback());
+
+    scoped_ptr<shell::ConnectToApplicationParams> params(
+        new shell::ConnectToApplicationParams);
+    params->SetURLInfo(request.Pass());
+    params->set_services(service_provider_request.Pass());
+    application_manager_->ConnectToApplication(params.Pass());
 
     run_loop.Run();
   }

mojo/mojo_shell.gyp

@@ -16,6 +16,8 @@
       'shell/capability_filter.h',
       'shell/connect_to_application_params.cc',
       'shell/connect_to_application_params.h',
+      'shell/connect_util.cc',
+      'shell/connect_util.h',
       'shell/content_handler_connection.cc',
       'shell/content_handler_connection.h',
       'shell/data_pipe_peek.cc',

mojo/package_manager/package_manager_impl.cc

@@ -11,6 +11,7 @@
 #include "mojo/fetcher/switches.h"
 #include "mojo/fetcher/update_fetcher.h"
 #include "mojo/shell/application_manager.h"
+#include "mojo/shell/connect_util.h"
 #include "mojo/shell/query_util.h"
 #include "mojo/shell/switches.h"
 #include "mojo/util/filename_util.h"
@@ -79,8 +80,8 @@ void PackageManagerImpl::FetchRequest(
     // LocalFetcher uses the network service to infer MIME types from URLs.
     // Skip this for mojo URLs to avoid recursively loading the network service.
     if (!network_service_ && !url.SchemeIs("mojo")) {
-      application_manager_->ConnectToService(GURL("mojo:network_service"),
-                                            &network_service_);
+      shell::ConnectToService(application_manager_,
+                              GURL("mojo:network_service"), &network_service_);
     }
     // Ownership of this object is transferred to |loader_callback|.
     // TODO(beng): this is eff'n weird.
@@ -99,7 +100,8 @@ void PackageManagerImpl::FetchRequest(
   if (url.SchemeIs("mojo") &&
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kUseUpdater)) {
-    application_manager_->ConnectToService(GURL("mojo:updater"), &updater_);
+    shell::ConnectToService(application_manager_, GURL("mojo:updater"),
+                            &updater_);
     // Ownership of this object is transferred to |loader_callback|.
     // TODO(beng): this is eff'n weird.
     new fetcher::UpdateFetcher(url, updater_.get(), loader_callback);
@@ -108,8 +110,8 @@ void PackageManagerImpl::FetchRequest(
 #endif
 
   if (!url_loader_factory_) {
-    application_manager_->ConnectToService(GURL("mojo:network_service"),
-                                           &url_loader_factory_);
+    shell::ConnectToService(application_manager_, GURL("mojo:network_service"),
+                            &url_loader_factory_);
   }
 
   // Ownership of this object is transferred to |loader_callback|.

mojo/runner/context.cc

@@ -209,11 +209,14 @@ bool Context::Init() {
   new TracingServiceProvider(GetProxy(&tracing_service_provider_ptr));
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From("mojo:tracing");
-  application_manager_->ConnectToApplication(
-      nullptr, request.Pass(), std::string(), GetProxy(&service_provider_ptr),
-      tracing_service_provider_ptr.Pass(),
-      shell::GetPermissiveCapabilityFilter(), base::Closure(),
-      shell::EmptyConnectCallback());
+
+  scoped_ptr<shell::ConnectToApplicationParams> params(
+      new shell::ConnectToApplicationParams);
+  params->SetURLInfo(request.Pass());
+  params->set_services(GetProxy(&service_provider_ptr));
+  params->set_exposed_services(tracing_service_provider_ptr.Pass());
+  params->set_filter(shell::GetPermissiveCapabilityFilter());
+  application_manager_->ConnectToApplication(params.Pass());
 
   // Record the shell startup metrics used for performance testing.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -263,11 +266,16 @@ void Context::Run(const GURL& url) {
   app_urls_.insert(url);
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(url.spec());
-  application_manager_->ConnectToApplication(
-      nullptr, request.Pass(), std::string(), GetProxy(&services),
-      exposed_services.Pass(), shell::GetPermissiveCapabilityFilter(),
-      base::Bind(&Context::OnApplicationEnd, base::Unretained(this), url),
-      shell::EmptyConnectCallback());
+
+  scoped_ptr<shell::ConnectToApplicationParams> params(
+      new shell::ConnectToApplicationParams);
+  params->SetURLInfo(request.Pass());
+  params->set_services(GetProxy(&services));
+  params->set_exposed_services(exposed_services.Pass());
+  params->set_filter(shell::GetPermissiveCapabilityFilter());
+  params->set_on_application_end(
+      base::Bind(&Context::OnApplicationEnd, base::Unretained(this), url));
+  application_manager_->ConnectToApplication(params.Pass());
 }
 
 void Context::RunCommandLineApplication(const base::Closure& callback) {

mojo/runner/native_runner_unittest.cc

@@ -94,10 +94,14 @@ TEST_F(NativeApplicationLoaderTest, DoesNotExist) {
   ServiceProviderPtr service_provider;
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(url.spec());
-  context_->application_manager()->ConnectToApplication(
-      nullptr, request.Pass(), std::string(), services.Pass(),
-      service_provider.Pass(), shell::GetPermissiveCapabilityFilter(),
-      base::Closure(), shell::EmptyConnectCallback());
+
+  scoped_ptr<shell::ConnectToApplicationParams> params(
+      new shell::ConnectToApplicationParams);
+  params->SetURLInfo(request.Pass());
+  params->set_services(services.Pass());
+  params->set_exposed_services(service_provider.Pass());
+  params->set_filter(shell::GetPermissiveCapabilityFilter());
+  context_->application_manager()->ConnectToApplication(params.Pass());
   EXPECT_FALSE(state_.runner_was_created);
   EXPECT_FALSE(state_.runner_was_started);
   EXPECT_FALSE(state_.runner_was_destroyed);

mojo/runner/shell_test_base.cc

@@ -49,10 +49,14 @@ ScopedMessagePipeHandle ShellTestBase::ConnectToService(
   ServiceProviderPtr services;
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(application_url.spec());
-  shell_context_.application_manager()->ConnectToApplication(
-      nullptr, request.Pass(), std::string(), GetProxy(&services), nullptr,
-      shell::GetPermissiveCapabilityFilter(), base::Bind(&QuitIfRunning),
-      shell::EmptyConnectCallback());
+
+  scoped_ptr<shell::ConnectToApplicationParams> params(
+      new shell::ConnectToApplicationParams);
+  params->SetURLInfo(request.Pass());
+  params->set_services(GetProxy(&services));
+  params->set_filter(shell::GetPermissiveCapabilityFilter());
+  params->set_on_application_end(base::Bind(&QuitIfRunning));
+  shell_context_.application_manager()->ConnectToApplication(params.Pass());
   MessagePipe pipe;
   services->ConnectToService(service_name, pipe.handle1.Pass());
   return pipe.handle0.Pass();

mojo/shell/BUILD.gn

@@ -17,6 +17,8 @@ source_set("shell") {
     "capability_filter.h",
     "connect_to_application_params.cc",
     "connect_to_application_params.h",
+    "connect_util.cc",
+    "connect_util.h",
     "content_handler_connection.cc",
     "content_handler_connection.h",
     "data_pipe_peek.cc",

mojo/shell/application_instance.cc

@@ -94,9 +94,16 @@ void ApplicationInstance::ConnectToApplication(
     CapabilityFilter capability_filter = GetPermissiveCapabilityFilter();
     if (!filter.is_null())
       capability_filter = filter->filter.To<CapabilityFilter>();
-    manager_->ConnectToApplication(
-        this, app_request.Pass(), std::string(), services.Pass(),
-        exposed_services.Pass(), capability_filter, base::Closure(), callback);
+
+    scoped_ptr<ConnectToApplicationParams> params(
+        new ConnectToApplicationParams);
+    params->SetOriginatorInfo(this);
+    params->SetURLInfo(app_request.Pass());
+    params->set_services(services.Pass());
+    params->set_exposed_services(exposed_services.Pass());
+    params->set_filter(capability_filter);
+    params->set_connect_callback(callback);
+    manager_->ConnectToApplication(params.Pass());
   } else {
     LOG(WARNING) << "CapabilityFilter prevented connection from: " <<
         identity_.url << " to: " << url.spec();

mojo/shell/application_manager.cc

@@ -59,8 +59,9 @@ ApplicationManager::ApplicationManager(
 }
 
 ApplicationManager::~ApplicationManager() {
-  URLToContentHandlerMap url_to_content_handler(url_to_content_handler_);
-  for (auto& pair : url_to_content_handler)
+  IdentityToContentHandlerMap identity_to_content_handler(
+      identity_to_content_handler_);
+  for (auto& pair : identity_to_content_handler)
     pair.second->CloseConnection();
   TerminateShellConnections();
   STLDeleteValues(&url_to_loader_);
@@ -70,28 +71,6 @@ void ApplicationManager::TerminateShellConnections() {
   STLDeleteValues(&identity_to_instance_);
 }
 
-void ApplicationManager::ConnectToApplication(
-    ApplicationInstance* originator,
-    URLRequestPtr app_url_request,
-    const std::string& qualifier,
-    InterfaceRequest<ServiceProvider> services,
-    ServiceProviderPtr exposed_services,
-    const CapabilityFilter& filter,
-    const base::Closure& on_application_end,
-    const Shell::ConnectToApplicationCallback& connect_callback) {
-  scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams);
-  params->SetOriginatorInfo(originator);
-  params->SetURLInfo(app_url_request.Pass());
-  params->set_qualifier(qualifier);
-  params->set_services(services.Pass());
-  params->set_exposed_services(exposed_services.Pass());
-  params->set_filter(filter);
-  params->set_on_application_end(on_application_end);
-  params->set_connect_callback(connect_callback);
-
-  ConnectToApplication(params.Pass());
-}
-
 void ApplicationManager::ConnectToApplication(
     scoped_ptr<ConnectToApplicationParams> params) {
   GURL original_url = params->app_url();
@@ -148,10 +127,10 @@ void ApplicationManager::ConnectToApplicationWithLoader(
   if (!(*params)->app_url().SchemeIs("mojo"))
     (*params)->SetURLInfo(resolved_url);
 
-  loader->Load(resolved_url, RegisterInstance(params->Pass(), nullptr));
+  loader->Load(resolved_url, CreateInstance(params->Pass(), nullptr));
 }
 
-InterfaceRequest<Application> ApplicationManager::RegisterInstance(
+InterfaceRequest<Application> ApplicationManager::CreateInstance(
     scoped_ptr<ConnectToApplicationParams> params,
     ApplicationInstance** resulting_instance) {
   Identity app_identity(params->app_url(), params->qualifier());
@@ -225,7 +204,7 @@ void ApplicationManager::HandleFetchCallback(
       params->connect_callback();
   params->set_connect_callback(EmptyConnectCallback());
   ApplicationInstance* app = nullptr;
-  InterfaceRequest<Application> request(RegisterInstance(params.Pass(), &app));
+  InterfaceRequest<Application> request(CreateInstance(params.Pass(), &app));
 
 
   GURL content_handler_url;
@@ -299,17 +278,18 @@ void ApplicationManager::LoadWithContentHandler(
     InterfaceRequest<Application> application_request,
     URLResponsePtr url_response) {
   ContentHandlerConnection* connection = nullptr;
-  std::pair<GURL, std::string> key(content_handler_url, qualifier);
+  Identity content_handler_identity(content_handler_url, qualifier);
   // TODO(beng): Figure out the extent to which capability filter should be
   //             factored into handler identity.
-  URLToContentHandlerMap::iterator iter = url_to_content_handler_.find(key);
-  if (iter != url_to_content_handler_.end()) {
+  IdentityToContentHandlerMap::iterator iter =
+      identity_to_content_handler_.find(content_handler_identity);
+  if (iter != identity_to_content_handler_.end()) {
     connection = iter->second;
   } else {
     connection = new ContentHandlerConnection(
         this, originator_identity, originator_filter, content_handler_url,
         qualifier, filter, ++content_handler_id_counter_);
-    url_to_content_handler_[key] = connection;
+    identity_to_content_handler_[content_handler_identity] = connection;
   }
 
   app->set_requesting_content_handler_id(connection->id());
@@ -350,32 +330,18 @@ void ApplicationManager::OnApplicationInstanceError(
 void ApplicationManager::OnContentHandlerConnectionClosed(
     ContentHandlerConnection* content_handler) {
   // Remove the mapping to the content handler.
-  auto it = url_to_content_handler_.find(
-      std::make_pair(content_handler->content_handler_url(),
-                     content_handler->content_handler_qualifier()));
-  DCHECK(it != url_to_content_handler_.end());
-  url_to_content_handler_.erase(it);
+  auto it = identity_to_content_handler_.find(
+      Identity(content_handler->content_handler_url(),
+               content_handler->content_handler_qualifier()));
+  DCHECK(it != identity_to_content_handler_.end());
+  identity_to_content_handler_.erase(it);
 }
 
 void ApplicationManager::CleanupRunner(NativeRunner* runner) {
   native_runners_.erase(
       std::find(native_runners_.begin(), native_runners_.end(), runner));
 }
 
-ScopedMessagePipeHandle ApplicationManager::ConnectToServiceByName(
-    const GURL& application_url,
-    const std::string& interface_name) {
-  ServiceProviderPtr services;
-  scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams);
-  params->SetURLInfo(application_url);
-  params->set_services(GetProxy(&services));
-  params->set_filter(GetPermissiveCapabilityFilter());
-  ConnectToApplication(params.Pass());
-  MessagePipe pipe;
-  services->ConnectToService(interface_name, pipe.handle1.Pass());
-  return pipe.handle0.Pass();
-}
-
 Shell::ConnectToApplicationCallback EmptyConnectCallback() {
   return base::Bind(&OnEmptyOnConnectCallback);
 }