Move fetching logic out of ApplicationManager, eliminate url mappings.

mojo/shell/application_manager.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "mojo/shell/application_manager.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/trace_event/trace_event.h"
 #include "mojo/application/public/interfaces/content_handler.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/shell/application_fetcher.h"
 #include "mojo/shell/application_instance.h"
 #include "mojo/shell/content_handler_connection.h"
 #include "mojo/shell/fetcher.h"
-#include "mojo/shell/local_fetcher.h"
-#include "mojo/shell/network_fetcher.h"
 #include "mojo/shell/query_util.h"
 #include "mojo/shell/switches.h"
-#include "mojo/shell/update_fetcher.h"
 
 namespace mojo {
 namespace shell {
 
 namespace {
 
 // Used by TestAPI.
 bool has_created_instance = false;
 
 void OnEmptyOnConnectCallback(uint32_t content_handler_id) {}
@@ skipping 11 matching lines @@
 bool ApplicationManager::TestAPI::HasCreatedInstance() {
   return has_created_instance;
 }
 
 bool ApplicationManager::TestAPI::HasRunningInstanceForURL(
     const GURL& url) const {
   return manager_->identity_to_instance_.find(Identity(url)) !=
          manager_->identity_to_instance_.end();
 }
 
-ApplicationManager::ApplicationManager(Delegate* delegate)
-    : delegate_(delegate),
-      disable_cache_(false),
+ApplicationManager::ApplicationManager(scoped_ptr<ApplicationFetcher> fetcher)
+    : fetcher_(fetcher.Pass()),
       content_handler_id_counter_(0u),
-      weak_ptr_factory_(this) {}
+      weak_ptr_factory_(this) {
+  fetcher_->SetApplicationManager(this);
+}
 
 ApplicationManager::~ApplicationManager() {
   URLToContentHandlerMap url_to_content_handler(url_to_content_handler_);
   for (auto& pair : url_to_content_handler)
     pair.second->CloseConnection();
   TerminateShellConnections();
   STLDeleteValues(&url_to_loader_);
   STLDeleteValues(&scheme_to_loader_);
 }
 
@@ skipping 27 matching lines @@
     scoped_ptr<ConnectToApplicationParams> params) {
   GURL original_url = params->app_url();
   URLRequestPtr original_url_request = params->TakeAppURLRequest();
 
   TRACE_EVENT_INSTANT1("mojo_shell", "ApplicationManager::ConnectToApplication",
                        TRACE_EVENT_SCOPE_THREAD, "original_url",
                        original_url.spec());
   DCHECK(original_url.is_valid());
   DCHECK(original_url_request);
 
-  // We check both the mapped and resolved urls for existing instances because
-  // external applications can be registered for the unresolved mojo:foo urls.
-
-  GURL mapped_url = delegate_->ResolveMappings(original_url);
-  params->SetURLInfo(mapped_url);
-  if (ConnectToRunningApplication(&params))
-    return;
-
-  GURL resolved_url = delegate_->ResolveMojoURL(mapped_url);
+  GURL resolved_url = fetcher_->ResolveURL(original_url);

[yzshen1, 2015/09/14 19:06:15]

One thing that seems like an issue:
Currently for mojo: URLs, we use the pre-resolved URL in the identity (line
143).
That means we won't find a running instance on line 108 even if there is one,
because it uses the resolved URL to look up. So the code proceeds to fetch, and
finally uses the pre-resolved URL on line 208 to look up and find the running
instance.

What do you think? Thanks!

[Ben Goodger (Google), 2015/09/14 23:27:16]

Good catch! WDYT about uniquifying instances based on the resolved URL?

   params->SetURLInfo(resolved_url);
   if (ConnectToRunningApplication(&params))
     return;
 
   // The application is not running, let's compute the parameters.
   // NOTE: Set URL info using |original_url_request| instead of |original_url|
   // because it may contain more information (e.g., it is a POST request).
   params->SetURLInfo(original_url_request.Pass());
-  if (ConnectToApplicationWithLoader(&params, mapped_url,
-                                     GetLoaderForURL(mapped_url))) {
+  ApplicationLoader* loader = GetLoaderForURL(resolved_url);
+  if (loader) {
+    ConnectToApplicationWithLoader(&params, resolved_url, loader);
     return;
   }
 
-  if (ConnectToApplicationWithLoader(&params, resolved_url,
-                                     GetLoaderForURL(resolved_url))) {
-    return;
-  }
-
-  if (ConnectToApplicationWithLoader(&params, resolved_url,
-                                     default_loader_.get())) {
-    return;
-  }
-
   original_url_request = params->TakeAppURLRequest();
   auto callback =
       base::Bind(&ApplicationManager::HandleFetchCallback,
                  weak_ptr_factory_.GetWeakPtr(), base::Passed(&params));
-
-  if (delegate_->CreateFetcher(
-          resolved_url,
-          base::Bind(callback, NativeApplicationCleanup::DONT_DELETE))) {
-    return;
-  }
-
-  if (resolved_url.SchemeIsFile()) {
-    // 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_ && !original_url.SchemeIs("mojo"))
-      ConnectToService(GURL("mojo:network_service"), &network_service_);
-    new LocalFetcher(
-        network_service_.get(), resolved_url,
-        GetBaseURLAndQuery(resolved_url, nullptr),
-        base::Bind(callback, NativeApplicationCleanup::DONT_DELETE));
-    return;
-  }
-
-  if (mapped_url.SchemeIs("mojo") &&
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kUseUpdater)) {
-    ConnectToService(GURL("mojo:updater"), &updater_);
-    new UpdateFetcher(
-        mapped_url, updater_.get(),
-        base::Bind(callback, NativeApplicationCleanup::DONT_DELETE));
-    return;
-  }
-
-  if (!url_loader_factory_)
-    ConnectToService(GURL("mojo:network_service"), &url_loader_factory_);
-
-  const NativeApplicationCleanup cleanup =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDontDeleteOnDownload)
-          ? NativeApplicationCleanup::DONT_DELETE
-          : NativeApplicationCleanup::DELETE;
-
-  if (original_url.SchemeIs("mojo")) {
-    // Use the resolved mojo URL in the request to support origin mapping, etc.
-    URLRequestPtr resolved_url_request(URLRequest::New());
-    resolved_url_request->url = resolved_url.spec();
-    new NetworkFetcher(disable_cache_, resolved_url_request.Pass(),
-                       url_loader_factory_.get(),
-                       base::Bind(callback, cleanup));
-    return;
-  }
-
-  new NetworkFetcher(disable_cache_, original_url_request.Pass(),
-                     url_loader_factory_.get(), base::Bind(callback, cleanup));
+  fetcher_->FetchRequest(original_url_request.Pass(), callback);
 }
 
 bool ApplicationManager::ConnectToRunningApplication(
     scoped_ptr<ConnectToApplicationParams>* params) {
   ApplicationInstance* instance = GetApplicationInstance(
       Identity((*params)->app_url(), (*params)->qualifier()));
   if (!instance)
     return false;
 
   instance->ConnectToClient(params->Pass());
   return true;
 }
 
-bool ApplicationManager::ConnectToApplicationWithLoader(
+void ApplicationManager::ConnectToApplicationWithLoader(
     scoped_ptr<ConnectToApplicationParams>* params,
     const GURL& resolved_url,
     ApplicationLoader* loader) {
-  if (!loader)
-    return false;
-
   if (!(*params)->app_url().SchemeIs("mojo"))
     (*params)->SetURLInfo(resolved_url);
 
   loader->Load(resolved_url, RegisterInstance(params->Pass(), nullptr));
-  return true;
 }
 
 InterfaceRequest<Application> ApplicationManager::RegisterInstance(
     scoped_ptr<ConnectToApplicationParams> params,
     ApplicationInstance** resulting_instance) {
   Identity app_identity(params->app_url(), params->qualifier());
 
   ApplicationPtr application;
   InterfaceRequest<Application> application_request = GetProxy(&application);
   ApplicationInstance* instance = new ApplicationInstance(
@@ skipping 13 matching lines @@
 ApplicationInstance* ApplicationManager::GetApplicationInstance(
     const Identity& identity) const {
   const auto& instance_it = identity_to_instance_.find(identity);
   if (instance_it != identity_to_instance_.end())
     return instance_it->second;
   return nullptr;
 }
 
 void ApplicationManager::HandleFetchCallback(
     scoped_ptr<ConnectToApplicationParams> params,
-    NativeApplicationCleanup cleanup,
     scoped_ptr<Fetcher> fetcher) {
   if (!fetcher) {
     // Network error. Drop |params| to tell the requestor.
     params->connect_callback().Run(Shell::kInvalidContentHandlerID);
     return;
   }
 
   GURL redirect_url = fetcher->GetRedirectURL();
   if (!redirect_url.is_empty()) {
     // And around we go again... Whee!
@@ skipping 31 matching lines @@
   params->set_connect_callback(EmptyConnectCallback());
   ApplicationInstance* app = nullptr;
   InterfaceRequest<Application> request(RegisterInstance(params.Pass(), &app));
 
   // For resources that are loaded with content handlers, we group app instances
   // by site.
 
   // If the response begins with a #!mojo <content-handler-url>, use it.
   GURL content_handler_url;
   std::string shebang;
+  // TODO(beng): it seems like some delegate should/would want to have a say in
+  //             configuring the qualifier also.
   bool enable_multi_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableMultiprocess);
 
   if (fetcher->PeekContentHandler(&shebang, &content_handler_url)) {
     URLResponsePtr response(fetcher->AsURLResponse(
         blocking_pool_, static_cast<int>(shebang.size())));
     std::string site =
         enable_multi_process ? response->site.To<std::string>() : std::string();
     LoadWithContentHandler(originator_identity, originator_filter,
                            content_handler_url, site, filter, connect_callback,
@@ skipping 65 matching lines @@
     else if (app_url == GURL("mojo://html_viewer/"))
       start_sandboxed = true;
   }
 
   connect_callback.Run(Shell::kInvalidContentHandlerID);
 
   fetcher->AsPath(blocking_pool_,
                   base::Bind(&ApplicationManager::RunNativeApplication,
                              weak_ptr_factory_.GetWeakPtr(),
                              base::Passed(request.Pass()), start_sandboxed,
-                             options, cleanup, base::Passed(fetcher.Pass())));
+                             options, base::Passed(fetcher.Pass())));
 }
 
 void ApplicationManager::RunNativeApplication(
     InterfaceRequest<Application> application_request,
     bool start_sandboxed,
     const NativeRunnerFactory::Options& options,
-    NativeApplicationCleanup cleanup,
     scoped_ptr<Fetcher> fetcher,
     const base::FilePath& path,
     bool path_exists) {
   // We only passed fetcher to keep it alive. Done with it now.
   fetcher.reset();
 
   DCHECK(application_request.is_pending());
 
   if (!path_exists) {
     LOG(ERROR) << "Library not started because library path '" << path.value()
                << "' does not exist.";
     return;
   }
 
   TRACE_EVENT1("mojo_shell", "ApplicationManager::RunNativeApplication", "path",
                path.AsUTF8Unsafe());
   NativeRunner* runner = native_runner_factory_->Create(options).release();
   native_runners_.push_back(runner);
-  runner->Start(path, start_sandboxed, cleanup, application_request.Pass(),
+  runner->Start(path, start_sandboxed, NativeApplicationCleanup::DONT_DELETE,
+                application_request.Pass(),
                 base::Bind(&ApplicationManager::CleanupRunner,
                            weak_ptr_factory_.GetWeakPtr(), runner));
 }
 
 void ApplicationManager::RegisterContentHandler(
     const std::string& mime_type,
     const GURL& content_handler_url) {
   DCHECK(content_handler_url.is_valid())
       << "Content handler URL is invalid for mime type " << mime_type;
   mime_type_to_url_[mime_type] = content_handler_url;
@@ skipping 52 matching lines @@
   if (it != scheme_to_loader_.end())
     delete it->second;
   scheme_to_loader_[scheme] = loader.release();
 }
 
 void ApplicationManager::SetNativeOptionsForURL(
     const NativeRunnerFactory::Options& options,
     const GURL& url) {
   DCHECK(!url.has_query());  // Precondition.
   // Apply mappings and resolution to get the resolved URL.
-  GURL resolved_url =
-      delegate_->ResolveMojoURL(delegate_->ResolveMappings(url));
+  GURL resolved_url = fetcher_->ResolveURL(url);
   DCHECK(!resolved_url.has_query());  // Still shouldn't have query.
   // TODO(vtl): We should probably also remove/disregard the query string (and
   // maybe canonicalize in other ways).
   DVLOG(2) << "Storing native options for resolved URL " << resolved_url
            << " (original URL " << url << ")";
   url_to_native_options_[resolved_url] = options;
 }
 
 ApplicationLoader* ApplicationManager::GetLoaderForURL(const GURL& url) {
   auto url_it = url_to_loader_.find(GetBaseURLAndQuery(url, nullptr));
   if (url_it != url_to_loader_.end())
     return url_it->second;
   auto scheme_it = scheme_to_loader_.find(url.scheme());
   if (scheme_it != scheme_to_loader_.end())
     return scheme_it->second;
-  return nullptr;
+  return default_loader_.get();
 }
 
 void ApplicationManager::OnApplicationInstanceError(
     ApplicationInstance* instance) {
   // Called from ~ApplicationInstance, so we do not need to call Destroy here.
   const Identity identity = instance->identity();
   base::Closure on_application_end = instance->on_application_end();
   // Remove the shell.
   auto it = identity_to_instance_.find(identity);
   DCHECK(it != identity_to_instance_.end());
@@ skipping 31 matching lines @@
   services->ConnectToService(interface_name, pipe.handle1.Pass());
   return pipe.handle0.Pass();
 }
 
 Shell::ConnectToApplicationCallback EmptyConnectCallback() {
   return base::Bind(&OnEmptyOnConnectCallback);
 }
 
 }  // namespace shell
 }  // namespace mojo