mojo: Update content handler API

mojo/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 "mojo/application_manager/application_manager.h"
 
 #include <stdio.h>
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
@@ skipping 143 matching lines @@
 
 ApplicationManager::~ApplicationManager() {
   STLDeleteValues(&url_to_content_handler_);
   TerminateShellConnections();
   STLDeleteValues(&url_to_loader_);
   STLDeleteValues(&scheme_to_loader_);
 }
 
 void ApplicationManager::TerminateShellConnections() {
   STLDeleteValues(&url_to_shell_impl_);
+  STLDeleteElements(&content_handler_shells_);
 }
 
 // static
 ApplicationManager* ApplicationManager::GetInstance() {
   static base::LazyInstance<ApplicationManager> instance =
       LAZY_INSTANCE_INITIALIZER;
   has_created_instance = true;
   return &instance.Get();
 }
 
@@ skipping 43 matching lines @@
     ScopedMessagePipeHandle* shell_handle) {
   ShellImpl* shell_impl = NULL;
   URLToShellImplMap::iterator iter = url_to_shell_impl_.find(url);
   if (iter != url_to_shell_impl_.end()) {
     // This can happen because services are loaded asynchronously. So if we get
     // two requests for the same service close to each other, we might get here
     // and find that we already have it.
     shell_impl = iter->second;
   } else {
     MessagePipe pipe;
-    URLToArgsMap::const_iterator args_it = url_to_args_.find(url);
-    Array<String> args;
-    if (args_it != url_to_args_.end())
-      args = Array<String>::From(args_it->second);
     shell_impl = WeakBindToPipe(new ShellImpl(this, url), pipe.handle1.Pass());
     url_to_shell_impl_[url] = shell_impl;
     *shell_handle = pipe.handle0.Pass();
-    shell_impl->client()->Initialize(args.Pass());
+    shell_impl->client()->Initialize(GetArgsForURL(url));
   }
 
   ConnectToClient(shell_impl, url, requestor_url, service_provider.Pass());
 }
 
 void ApplicationManager::LoadWithContentHandler(
     const GURL& content_url,
     const GURL& requestor_url,
     const GURL& content_handler_url,
     URLResponsePtr url_response,
     ServiceProviderPtr service_provider) {
   ContentHandlerConnection* connection = NULL;
   URLToContentHandlerMap::iterator iter =
       url_to_content_handler_.find(content_handler_url);
   if (iter != url_to_content_handler_.end()) {
     connection = iter->second;
   } else {
     connection = new ContentHandlerConnection(this, content_handler_url);
     url_to_content_handler_[content_handler_url] = connection;
   }
 
-  InterfaceRequest<ServiceProvider> spir;
-  spir.Bind(service_provider.PassMessagePipe());
-  connection->content_handler->OnConnect(
-      requestor_url.spec(), url_response.Pass(), spir.Pass());
+  ShellPtr shell_proxy;
+  ShellImpl* shell_impl =
+      WeakBindToProxy(new ShellImpl(this, content_url), &shell_proxy);
+  content_handler_shells_.insert(shell_impl);

[Aaron Boodman, 2014/10/31 08:24:01]

How about the name 'content_shell_impls'?

1. it's parallel to url_to_shell_impl_
2. helps reinforce that this is not the shell to the contenthandler, but to the
content

[qsr, 2014/10/31 12:10:44]

Good idea. Done.

+  shell_impl->client()->Initialize(GetArgsForURL(content_url));
+
+  connection->content_handler->StartApplication(shell_proxy.Pass(),
+                                                url_response.Pass());
+  ConnectToClient(
+      shell_impl, content_url, requestor_url, service_provider.Pass());
 }
 
 void ApplicationManager::SetLoaderForURL(scoped_ptr<ApplicationLoader> loader,
                                          const GURL& url) {
   URLToLoaderMap::iterator it = url_to_loader_.find(url);
   if (it != url_to_loader_.end())
     delete it->second;
   url_to_loader_[url] = loader.release();
 }
 
@@ skipping 21 matching lines @@
     return url_it->second;
   SchemeToLoaderMap::const_iterator scheme_it =
       scheme_to_loader_.find(url.scheme());
   if (scheme_it != scheme_to_loader_.end())
     return scheme_it->second;
   return default_loader_.get();
 }
 
 void ApplicationManager::OnShellImplError(ShellImpl* shell_impl) {
   // Called from ~ShellImpl, so we do not need to call Destroy here.
-  const GURL url = shell_impl->url();
-  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);
-  ApplicationLoader* loader = GetLoaderForURL(url);
-  if (loader)
-    loader->OnApplicationError(this, url);
-  if (delegate_)
-    delegate_->OnApplicationError(url);
+  auto it = content_handler_shells_.find(shell_impl);
+  if (it != content_handler_shells_.end()) {
+    delete (*it);
+    content_handler_shells_.erase(it);

[Aaron Boodman, 2014/10/31 08:24:01]

return early.

[qsr, 2014/10/31 12:10:44]

Done.

+  } else {
+    const GURL url = shell_impl->url();

[Aaron Boodman, 2014/10/31 08:24:01]

Can't you remove the entry from url_to_content_handlers_ here?

[qsr, 2014/10/31 12:10:44]

More than can, I must. That was part of the reason I couldn't close the content
handler when it has not content anymore (the other part is on line 122 -> This
must be a weak bind as we keep the InterfacePtr.

+    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);
+    ApplicationLoader* loader = GetLoaderForURL(url);
+    if (loader)
+      loader->OnApplicationError(this, url);
+    if (delegate_)
+      delegate_->OnApplicationError(url);
+  }
 }
 
 ScopedMessagePipeHandle ApplicationManager::ConnectToServiceByName(
     const GURL& application_url,
     const std::string& interface_name) {
   StubServiceProvider* stub_sp = new StubServiceProvider;
   ServiceProviderPtr spp;
   BindToProxy(stub_sp, &spp);
   ConnectToApplication(application_url, GURL(), spp.Pass());
   MessagePipe pipe;
   stub_sp->GetRemoteServiceProvider()->ConnectToService(interface_name,
                                                         pipe.handle1.Pass());
   return pipe.handle0.Pass();
 }
+
+Array<String> ApplicationManager::GetArgsForURL(const GURL& url) {
+  URLToArgsMap::const_iterator args_it = url_to_args_.find(url);
+  if (args_it != url_to_args_.end())
+    return Array<String>::From(args_it->second);
+  return Array<String>();
+}
 }  // namespace mojo