Revert of Some more minor cleanup to ApplicationManager. (patchset ch…

…romium#4 id:60001 of https://codereview.chromium.org/1351963002/ ) Reason for revert: Broke Android GN. Original issue's description: > 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} TBR=yzshen@chromium.org,ben@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/1354043002 Cr-Commit-Position: refs/heads/master@{#349554}
hylschwy · Sep 18, 2015 · 4dff1d5 · 4dff1d5
1 parent 27bc389
commit 4dff1d5
Show file tree

Hide file tree

Showing 15 changed files with 142 additions and 188 deletions.
diff --git a/mandoline/app/android/mandoline_activity.cc b/mandoline/app/android/mandoline_activity.cc
@@ -7,17 +7,15 @@
 #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::shell::ConnectToService(
-      mojo::runner::GetContext()->application_manager(), GURL("mojo:phone_ui"),
-      &launch_handler);
+  mojo::runner::GetContext()->application_manager()->ConnectToService(
+      GURL("mojo:phone_ui"), &launch_handler);
   launch_handler->LaunchURL(
       base::android::ConvertJavaStringToUTF8(env, jurl));
 }

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

diff --git a/mojo/mojo_shell.gyp b/mojo/mojo_shell.gyp
@@ -16,8 +16,6 @@
       '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',

diff --git a/mojo/package_manager/package_manager_impl.cc b/mojo/package_manager/package_manager_impl.cc
@@ -11,7 +11,6 @@
 #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"
@@ -80,8 +79,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")) {
-      shell::ConnectToService(application_manager_,
-                              GURL("mojo:network_service"), &network_service_);
+      application_manager_->ConnectToService(GURL("mojo:network_service"),
+                                            &network_service_);
     }
     // Ownership of this object is transferred to |loader_callback|.
     // TODO(beng): this is eff'n weird.
@@ -100,8 +99,7 @@ void PackageManagerImpl::FetchRequest(
   if (url.SchemeIs("mojo") &&
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kUseUpdater)) {
-    shell::ConnectToService(application_manager_, GURL("mojo:updater"),
-                            &updater_);
+    application_manager_->ConnectToService(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);
@@ -110,8 +108,8 @@ void PackageManagerImpl::FetchRequest(
 #endif
 
   if (!url_loader_factory_) {
-    shell::ConnectToService(application_manager_, GURL("mojo:network_service"),
-                            &url_loader_factory_);
+    application_manager_->ConnectToService(GURL("mojo:network_service"),
+                                           &url_loader_factory_);
   }
 
   // Ownership of this object is transferred to |loader_callback|.

diff --git a/mojo/runner/context.cc b/mojo/runner/context.cc
@@ -209,14 +209,11 @@ bool Context::Init() {
   new TracingServiceProvider(GetProxy(&tracing_service_provider_ptr));
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From("mojo:tracing");
-
-  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());
+  application_manager_->ConnectToApplication(
+      nullptr, request.Pass(), std::string(), GetProxy(&service_provider_ptr),
+      tracing_service_provider_ptr.Pass(),
+      shell::GetPermissiveCapabilityFilter(), base::Closure(),
+      shell::EmptyConnectCallback());
 
   // Record the shell startup metrics used for performance testing.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -266,16 +263,11 @@ void Context::Run(const GURL& url) {
   app_urls_.insert(url);
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(url.spec());
-
-  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());
+  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());
 }
 
 void Context::RunCommandLineApplication(const base::Closure& callback) {

diff --git a/mojo/runner/native_runner_unittest.cc b/mojo/runner/native_runner_unittest.cc
@@ -94,14 +94,10 @@ TEST_F(NativeApplicationLoaderTest, DoesNotExist) {
   ServiceProviderPtr service_provider;
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(url.spec());
-
-  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());
+  context_->application_manager()->ConnectToApplication(
+      nullptr, request.Pass(), std::string(), services.Pass(),
+      service_provider.Pass(), shell::GetPermissiveCapabilityFilter(),
+      base::Closure(), shell::EmptyConnectCallback());
   EXPECT_FALSE(state_.runner_was_created);
   EXPECT_FALSE(state_.runner_was_started);
   EXPECT_FALSE(state_.runner_was_destroyed);

diff --git a/mojo/runner/shell_test_base.cc b/mojo/runner/shell_test_base.cc
@@ -49,14 +49,10 @@ ScopedMessagePipeHandle ShellTestBase::ConnectToService(
   ServiceProviderPtr services;
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(application_url.spec());
-
-  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());
+  shell_context_.application_manager()->ConnectToApplication(
+      nullptr, request.Pass(), std::string(), GetProxy(&services), nullptr,
+      shell::GetPermissiveCapabilityFilter(), base::Bind(&QuitIfRunning),
+      shell::EmptyConnectCallback());
   MessagePipe pipe;
   services->ConnectToService(service_name, pipe.handle1.Pass());
   return pipe.handle0.Pass();

diff --git a/mojo/shell/BUILD.gn b/mojo/shell/BUILD.gn
@@ -17,8 +17,6 @@ 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",

diff --git a/mojo/shell/application_instance.cc b/mojo/shell/application_instance.cc
@@ -94,16 +94,9 @@ void ApplicationInstance::ConnectToApplication(
     CapabilityFilter capability_filter = GetPermissiveCapabilityFilter();
     if (!filter.is_null())
       capability_filter = filter->filter.To<CapabilityFilter>();
-
-    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());
+    manager_->ConnectToApplication(
+        this, app_request.Pass(), std::string(), services.Pass(),
+        exposed_services.Pass(), capability_filter, base::Closure(), callback);
   } else {
     LOG(WARNING) << "CapabilityFilter prevented connection from: " <<
         identity_.url << " to: " << url.spec();

diff --git a/mojo/shell/application_manager.cc b/mojo/shell/application_manager.cc
@@ -59,9 +59,8 @@ ApplicationManager::ApplicationManager(
 }
 
 ApplicationManager::~ApplicationManager() {
-  IdentityToContentHandlerMap identity_to_content_handler(
-      identity_to_content_handler_);
-  for (auto& pair : identity_to_content_handler)
+  URLToContentHandlerMap url_to_content_handler(url_to_content_handler_);
+  for (auto& pair : url_to_content_handler)
     pair.second->CloseConnection();
   TerminateShellConnections();
   STLDeleteValues(&url_to_loader_);
@@ -71,6 +70,28 @@ 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();
@@ -127,10 +148,10 @@ void ApplicationManager::ConnectToApplicationWithLoader(
   if (!(*params)->app_url().SchemeIs("mojo"))
     (*params)->SetURLInfo(resolved_url);
 
-  loader->Load(resolved_url, CreateInstance(params->Pass(), nullptr));
+  loader->Load(resolved_url, RegisterInstance(params->Pass(), nullptr));
 }
 
-InterfaceRequest<Application> ApplicationManager::CreateInstance(
+InterfaceRequest<Application> ApplicationManager::RegisterInstance(
     scoped_ptr<ConnectToApplicationParams> params,
     ApplicationInstance** resulting_instance) {
   Identity app_identity(params->app_url(), params->qualifier());
@@ -204,7 +225,7 @@ void ApplicationManager::HandleFetchCallback(
       params->connect_callback();
   params->set_connect_callback(EmptyConnectCallback());
   ApplicationInstance* app = nullptr;
-  InterfaceRequest<Application> request(CreateInstance(params.Pass(), &app));
+  InterfaceRequest<Application> request(RegisterInstance(params.Pass(), &app));
 
 
   GURL content_handler_url;
@@ -278,18 +299,17 @@ void ApplicationManager::LoadWithContentHandler(
     InterfaceRequest<Application> application_request,
     URLResponsePtr url_response) {
   ContentHandlerConnection* connection = nullptr;
-  Identity content_handler_identity(content_handler_url, qualifier);
+  std::pair<GURL, std::string> key(content_handler_url, qualifier);
   // TODO(beng): Figure out the extent to which capability filter should be
   //             factored into handler identity.
-  IdentityToContentHandlerMap::iterator iter =
-      identity_to_content_handler_.find(content_handler_identity);
-  if (iter != identity_to_content_handler_.end()) {
+  URLToContentHandlerMap::iterator iter = url_to_content_handler_.find(key);
+  if (iter != url_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_);
-    identity_to_content_handler_[content_handler_identity] = connection;
+    url_to_content_handler_[key] = connection;
   }
 
   app->set_requesting_content_handler_id(connection->id());
@@ -330,18 +350,32 @@ void ApplicationManager::OnApplicationInstanceError(
 void ApplicationManager::OnContentHandlerConnectionClosed(
     ContentHandlerConnection* content_handler) {
   // Remove the mapping to the content handler.
-  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);
+  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);
 }
 
 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);
 }