mojo: Update content handler API

mojo/application_manager/application_manager.cc

Index: mojo/application_manager/application_manager.cc
diff --git a/mojo/application_manager/application_manager.cc b/mojo/application_manager/application_manager.cc
index 75ef7a76694b24773a65b38e644d83bd901d1ce9..72f3212e6f1b7356d18cec129cb5cb6a5f0ccee7 100644
--- a/mojo/application_manager/application_manager.cc
+++ b/mojo/application_manager/application_manager.cc
@@ -161,6 +161,7 @@ ApplicationManager::~ApplicationManager() {
 
 void ApplicationManager::TerminateShellConnections() {
   STLDeleteValues(&url_to_shell_impl_);
+  STLDeleteElements(&content_handler_shells_);
 }
 
 // static
@@ -224,14 +225,10 @@ void ApplicationManager::RegisterLoadedApplication(
     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());
@@ -253,10 +250,16 @@ void ApplicationManager::LoadWithContentHandler(
     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,
@@ -298,16 +301,22 @@ ApplicationLoader* ApplicationManager::GetLoaderForURL(const GURL& url) {
 
 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(
@@ -322,4 +331,11 @@ ScopedMessagePipeHandle ApplicationManager::ConnectToServiceByName(
                                                         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