ApplicationManager: Use callback to get notified on application shutdown.

shell/application_manager/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 "shell/application_manager/application_manager.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/stl_util.h"
@@ skipping 13 matching lines @@
 namespace {
 
 // Used by TestAPI.
 bool has_created_instance = false;
 
 }  // namespace
 
 ApplicationManager::Delegate::~Delegate() {
 }
 
-void ApplicationManager::Delegate::OnApplicationError(const GURL& url) {
-  LOG(ERROR) << "Communication error with application: " << url.spec();
-}
-
 GURL ApplicationManager::Delegate::ResolveURL(const GURL& url) {
   return url;
 }
 
 GURL ApplicationManager::Delegate::ResolveMappings(const GURL& url) {
   return url;
 }
 
 class ApplicationManager::ContentHandlerConnection : public ErrorHandler {
  public:
   ContentHandlerConnection(ApplicationManager* manager,
                            const GURL& content_handler_url)
       : manager_(manager), content_handler_url_(content_handler_url) {
     ServiceProviderPtr services;
     manager->ConnectToApplication(content_handler_url, GURL(),
-                                  GetProxy(&services), nullptr);
+                                  GetProxy(&services), nullptr,
+                                  base::Closure());
     MessagePipe pipe;
     content_handler_.Bind(pipe.handle0.Pass());
     services->ConnectToService(ContentHandler::Name_, pipe.handle1.Pass());
     content_handler_.set_error_handler(this);
   }
 
   ContentHandler* content_handler() { return content_handler_.get(); }
 
   GURL content_handler_url() { return content_handler_url_; }
 
@@ skipping 37 matching lines @@
 }
 
 void ApplicationManager::TerminateShellConnections() {
   STLDeleteValues(&url_to_shell_impl_);
 }
 
 void ApplicationManager::ConnectToApplication(
     const GURL& requested_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider> services,
-    ServiceProviderPtr exposed_services) {
+    ServiceProviderPtr exposed_services,
+    const base::Closure& on_application_end) {
   DCHECK(requested_url.is_valid());
 
   // We check both the mapped and resolved urls for existing shell_impls because
   // 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)) {
     return;
   }
 
   GURL resolved_url = delegate_->ResolveURL(mapped_url);
   if (ConnectToRunningApplication(resolved_url, requestor_url, &services,
                                   &exposed_services)) {
     return;
   }
 
-  if (ConnectToApplicationWithLoader(requested_url, mapped_url, requestor_url,
-                                     &services, &exposed_services,
+  if (ConnectToApplicationWithLoader(mapped_url, requestor_url, &services,
+                                     &exposed_services, on_application_end,
                                      GetLoaderForURL(mapped_url))) {
     return;
   }
 
-  if (ConnectToApplicationWithLoader(requested_url, resolved_url, requestor_url,
-                                     &services, &exposed_services,
+  if (ConnectToApplicationWithLoader(resolved_url, requestor_url, &services,
+                                     &exposed_services, on_application_end,
                                      GetLoaderForURL(resolved_url))) {
     return;
   }
 
-  if (ConnectToApplicationWithLoader(requested_url, resolved_url, requestor_url,
-                                     &services, &exposed_services,
+  if (ConnectToApplicationWithLoader(resolved_url, requestor_url, &services,
+                                     &exposed_services, on_application_end,
                                      default_loader_.get())) {
     return;
   }
 
-  auto callback = base::Bind(&ApplicationManager::HandleFetchCallback,
-                             weak_ptr_factory_.GetWeakPtr(), requested_url,
-                             requestor_url, base::Passed(services.Pass()),
-                             base::Passed(exposed_services.Pass()));
+  auto callback = base::Bind(
+      &ApplicationManager::HandleFetchCallback, weak_ptr_factory_.GetWeakPtr(),
+      requestor_url, base::Passed(services.Pass()),
+      base::Passed(exposed_services.Pass()), on_application_end);
 
   if (resolved_url.SchemeIsFile()) {
     new LocalFetcher(resolved_url, GetBaseURLAndQuery(resolved_url, nullptr),
                      base::Bind(callback, NativeRunner::DontDeleteAppPath));
     return;
   }
 
   if (!network_service_)
     ConnectToService(GURL("mojo:network_service"), &network_service_);
 
@@ skipping 10 matching lines @@
   ShellImpl* shell_impl = GetShellImpl(application_url);
   if (!shell_impl)
     return false;
 
   ConnectToClient(shell_impl, resolved_url, requestor_url, services->Pass(),
                   exposed_services->Pass());
   return true;
 }
 
 bool ApplicationManager::ConnectToApplicationWithLoader(
-    const GURL& requested_url,
     const GURL& resolved_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider>* services,
     ServiceProviderPtr* exposed_services,
+    const base::Closure& on_application_end,
     ApplicationLoader* loader) {
   if (!loader)
     return false;
 
   loader->Load(resolved_url,
-               RegisterShell(requested_url, resolved_url, requestor_url,
-                             services->Pass(), exposed_services->Pass()));
+               RegisterShell(resolved_url, requestor_url, services->Pass(),
+                             exposed_services->Pass(), on_application_end));
   return true;
 }
 
 InterfaceRequest<Application> ApplicationManager::RegisterShell(
-    const GURL& original_url,
     const GURL& resolved_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider> services,
-    ServiceProviderPtr exposed_services) {
+    ServiceProviderPtr exposed_services,
+    const base::Closure& on_application_end) {
   GURL app_url = GetBaseURLAndQuery(resolved_url, nullptr);
 
   ApplicationPtr application;
   InterfaceRequest<Application> application_request = GetProxy(&application);
   ShellImpl* shell =
-      new ShellImpl(application.Pass(), this, original_url, app_url);
+      new ShellImpl(application.Pass(), this, app_url, on_application_end);
   url_to_shell_impl_[app_url] = shell;
-  shell->InitializeApplication(GetArgsForURL(original_url));
+  shell->InitializeApplication(GetArgsForURL(app_url));

[qsr, 2015/03/09 16:50:21]

I cannot push this until this is fixed -> this changed the behavior from using
the requested url to using the app url.

Using the requested url seems bad to me (because the app won't receive the same
arguments depending on which URL is used the first time it is connected to), but
using the real app url is not very good either because it will now be depending
on all the mapping, and redirect, some that cannot be known in advance.

I'm proposing the following, that I will implement in another CL if that's fine
with you:

If an URL A is mapped to B, and that both A and B have arguments, we should
concatenate all arguments and pass it to the app whether it is called with URL A
or B.

Now, we still have an issue with http redirect. Unfortunately I do not have a
very good solution, and I propose we continue to send the arguments that can be
found by using the given URL, it means that if an URL A is mapped to B, it will
not get the same arguments whether it is called the first time with A, or
directly with B.

 We kind of need this, because when we will map URL to the cloud to get the last
version of the app, we will still want to pass argument using the canonical name
for those Apps.

 WDYT?

   ConnectToClient(shell, resolved_url, requestor_url, services.Pass(),
                   exposed_services.Pass());
   return application_request.Pass();
 }
 
 ShellImpl* ApplicationManager::GetShellImpl(const GURL& url) {
   const auto& shell_it = url_to_shell_impl_.find(url);
   if (shell_it != url_to_shell_impl_.end())
     return shell_it->second;
   return nullptr;
 }
 
 void ApplicationManager::ConnectToClient(
     ShellImpl* shell_impl,
     const GURL& resolved_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider> services,
     ServiceProviderPtr exposed_services) {
   shell_impl->ConnectToClient(resolved_url, requestor_url, services.Pass(),
                               exposed_services.Pass());
 }
 
 void ApplicationManager::HandleFetchCallback(
-    const GURL& requested_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider> services,
     ServiceProviderPtr exposed_services,
+    const base::Closure& on_application_end,
     NativeRunner::CleanupBehavior cleanup_behavior,
     scoped_ptr<Fetcher> fetcher) {
   if (!fetcher) {
     // Network error. Drop |application_request| to tell requestor.
     return;
   }
 
   GURL redirect_url = fetcher->GetRedirectURL();
   if (!redirect_url.is_empty()) {
     // And around we go again... Whee!
     ConnectToApplication(redirect_url, requestor_url, services.Pass(),
-                         exposed_services.Pass());
+                         exposed_services.Pass(), on_application_end);
     return;
   }
 
   // We already checked if the application was running before we fetched it, but
   // it might have started while the fetch was outstanding. We don't want to
   // have two copies of the app running, so check again.
   //
   // 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)) {
     return;
   }
 
   InterfaceRequest<Application> request(
-      RegisterShell(requested_url, fetcher->GetURL(), requestor_url,
-                    services.Pass(), exposed_services.Pass()));
+      RegisterShell(fetcher->GetURL(), requestor_url, services.Pass(),
+                    exposed_services.Pass(), on_application_end));
 
   // If the response begins with a #!mojo <content-handler-url>, use it.
   GURL content_handler_url;
   std::string shebang;
   if (fetcher->PeekContentHandler(&shebang, &content_handler_url)) {
     LoadWithContentHandler(
         content_handler_url, request.Pass(),
         fetcher->AsURLResponse(blocking_pool_,
                                static_cast<int>(shebang.size())));
     return;
   }
 
   MimeTypeToURLMap::iterator iter = mime_type_to_url_.find(fetcher->MimeType());
   if (iter != mime_type_to_url_.end()) {
     LoadWithContentHandler(iter->second, request.Pass(),
                            fetcher->AsURLResponse(blocking_pool_, 0));
     return;
   }
 
   // TODO(aa): Sanity check that the thing we got looks vaguely like a mojo
   // application. That could either mean looking for the platform-specific dll
   // header, or looking for some specific mojo signature prepended to the
   // library.
   // TODO(vtl): (Maybe this should be done by the factory/runner?)
 
   NativeRunnerFactory::Options options;
   if (url_to_native_options_.find(fetcher->GetURL()) !=
       url_to_native_options_.end()) {
     DVLOG(2) << "Applying stored native options to resolved URL "
-             << fetcher->GetURL() << " (requested URL " << requested_url << ")";
+             << fetcher->GetURL();
     options = url_to_native_options_[fetcher->GetURL()];
   }
 
   fetcher->AsPath(
       blocking_pool_,
       base::Bind(&ApplicationManager::RunNativeApplication,
                  weak_ptr_factory_.GetWeakPtr(), base::Passed(request.Pass()),
                  options, cleanup_behavior, base::Passed(fetcher.Pass())));
 }
 
@@ skipping 24 matching lines @@
 
 void ApplicationManager::RegisterExternalApplication(
     const GURL& url,
     const std::vector<std::string>& args,
     ApplicationPtr application) {
   const auto& args_it = url_to_args_.find(url);
   if (args_it != url_to_args_.end()) {
     LOG(WARNING) << "--args-for provided for external application " << url
                  << " <ignored>";
   }
-  ShellImpl* shell_impl = new ShellImpl(application.Pass(), this, url, url);
+  ShellImpl* shell_impl =
+      new ShellImpl(application.Pass(), this, url, base::Closure());
   url_to_shell_impl_[url] = shell_impl;
   shell_impl->InitializeApplication(Array<String>::From(args));
 }
 
 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 57 matching lines @@
     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;
 }
 
 void ApplicationManager::OnShellImplError(ShellImpl* shell_impl) {
   // Called from ~ShellImpl, so we do not need to call Destroy here.
   const GURL url = shell_impl->url();
-  const GURL requested_url = shell_impl->requested_url();
+  base::Closure on_application_end = shell_impl->on_application_end();
   // Remove the shell.
   URLToShellImplMap::iterator it = url_to_shell_impl_.find(url);
   DCHECK(it != url_to_shell_impl_.end());
   delete it->second;
   url_to_shell_impl_.erase(it);
-  delegate_->OnApplicationError(requested_url);
+  if (!on_application_end.is_null())
+    on_application_end.Run();
 }
 
 void ApplicationManager::OnContentHandlerError(
     ContentHandlerConnection* content_handler) {
   // Remove the mapping to the content handler.
   auto it =
       url_to_content_handler_.find(content_handler->content_handler_url());
   DCHECK(it != url_to_content_handler_.end());
   delete it->second;
   url_to_content_handler_.erase(it);
 }
 
 ScopedMessagePipeHandle ApplicationManager::ConnectToServiceByName(
     const GURL& application_url,
     const std::string& interface_name) {
   ServiceProviderPtr services;
-  ConnectToApplication(application_url, GURL(), GetProxy(&services), nullptr);
+  ConnectToApplication(application_url, GURL(), GetProxy(&services), nullptr,
+                       base::Closure());
   MessagePipe pipe;
   services->ConnectToService(interface_name, pipe.handle1.Pass());
   return pipe.handle0.Pass();
 }
 
 Array<String> ApplicationManager::GetArgsForURL(const GURL& url) {
   const auto& args_it = url_to_args_.find(url);
   if (args_it != url_to_args_.end())
     return Array<String>::From(args_it->second);
   return Array<String>();
 }
 
 void ApplicationManager::CleanupRunner(NativeRunner* runner) {
   native_runners_.erase(
       std::find(native_runners_.begin(), native_runners_.end(), runner));
 }
 
 }  // namespace shell
 }  // namespace mojo