Added anti-singleton services (new process/thread per connection).

shell/application_manager/application_manager.cc

Index: shell/application_manager/application_manager.cc
diff --git a/shell/application_manager/application_manager.cc b/shell/application_manager/application_manager.cc
index 6e7b9657786238f255fcfc1039c404c548eaafb0..4e5c947e767a83d4c7cd2ad8898a9202b5038e34 100644
--- a/shell/application_manager/application_manager.cc
+++ b/shell/application_manager/application_manager.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <inttypes.h>
+
 #include "shell/application_manager/application_manager.h"

[viettrungluu, 2015/11/13 18:41:26]

This include should go *first* (see the style guide).

[Sean Klein, 2015/11/16 02:24:26]

Done (well, removed, actually, since no longer used).

 
 #include "base/bind.h"
@@ -44,17 +46,22 @@ std::vector<std::string> Concatenate(const std::vector<std::string>& v1,
   return result;
 }
 
+std::string create_id_string(uint64_t uid) {

[viettrungluu, 2015/11/13 18:41:26]

Nit: Functions should be NamedLikeThis.

[Sean Klein, 2015/11/16 02:24:26]

Done (well, also removed)

+  // 20 == Max number of digits in a 64 bit integer.
+  char buffer[21];
+  CHECK_EQ(20, snprintf(buffer, sizeof(buffer), "%020" PRIu64, uid));

[viettrungluu, 2015/11/13 18:41:26]

Probably you should just use one of the functions in
base/strings/string_number_conversions.h. I wouldn't worry too much about it
having a fixed length, but if you really want you could use HexEncode().

(Note that using snprintf() might make you sad, since its output is locale
dependent!)

[Sean Klein, 2015/11/16 02:24:26]

Updating to "Uint64ToString", which eliminates the need for the "CreateIdString"
function. Good call on using base.

+  return std::string(buffer);
+}
+
 }  // namespace
 
 class ApplicationManager::ContentHandlerConnection {
  public:
-  ContentHandlerConnection(ApplicationManager* manager,
-                           const GURL& content_handler_url)
-      : manager_(manager), content_handler_url_(content_handler_url) {
+  ContentHandlerConnection(ApplicationManager* manager, Identity id)
+      : manager_(manager), id_(id) {
     ServiceProviderPtr services;
-    manager->ConnectToApplication(content_handler_url, GURL(),
-                                  mojo::GetProxy(&services), nullptr,
-                                  base::Closure());
+    manager->ConnectToApplication(id_.url, GURL(), mojo::GetProxy(&services),
+                                  nullptr, base::Closure());
     mojo::MessagePipe pipe;
     content_handler_.Bind(
         mojo::InterfacePtrInfo<mojo::ContentHandler>(pipe.handle0.Pass(), 0u));
@@ -67,11 +74,13 @@ class ApplicationManager::ContentHandlerConnection {
 
   mojo::ContentHandler* content_handler() { return content_handler_.get(); }
 
-  GURL content_handler_url() { return content_handler_url_; }
+  GURL content_handler_url() { return id_.url; }
+
+  Identity get_identity() { return id_; }

[viettrungluu, 2015/11/13 18:41:26]

nit: Getters of this type usually omit the word "get", so this should be
|identity()|. But since getters of this type are usually named in fairly direct
correspondence to the member variable, you should probably either rename |id_|
to |identity_| or this to |id()|.

Also, this method should be const, and return a |const Identity&|. (Ditto for
|content_handler_url()|, I guess.

[Sean Klein, 2015/11/16 02:24:26]

Done (renamed to identity, added const, returning references).

 
  private:
   ApplicationManager* manager_;
-  GURL content_handler_url_;
+  Identity id_;

[viettrungluu, 2015/11/13 18:41:26]

nit: Maybe this should be const.

(And I guess the above should be |ApplicationManager* const manager_;|.)

[Sean Klein, 2015/11/16 02:24:26]

Done.

   mojo::ContentHandlerPtr content_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentHandlerConnection);
@@ -230,6 +239,9 @@ bool ApplicationManager::ConnectToRunningApplication(
   if (!shell_impl)
     return false;
 
+  DCHECK(!this->GetNativeApplicationOptionsForURL(application_url)

[viettrungluu, 2015/11/13 18:41:26]

No need for "this->" (here and elsewhere).

[Sean Klein, 2015/11/16 02:24:26]

Done.

+              ->force_unique_process);
+
   ConnectToClient(shell_impl, resolved_url, requestor_url, services->Pass(),
                   exposed_services->Pass());
   return true;
@@ -253,6 +265,18 @@ bool ApplicationManager::ConnectToApplicationWithLoader(
   return true;
 }
 
+Identity ApplicationManager::MakeApplicationIdentity(const GURL& resolved_url) {
+  static uint64_t uid = 1;

[viettrungluu, 2015/11/13 18:41:26]

nit: Calling it "uid" is a little strange.

[Sean Klein, 2015/11/16 02:24:26]

Renaming to "unique_id_number", since it is intended to be a unique
identification number.

+  bool force_unique_process = this->GetNativeApplicationOptionsForURL(
+                                      GetBaseURLAndQuery(resolved_url, nullptr))
+                                  ->force_unique_process;
+  if (force_unique_process) {

[viettrungluu, 2015/11/13 18:41:26]

Style nits:
- In this file/directory, we omit braces for single-line blocks.
- It's fairly conventional to not have an |else| if the |if| block ends with a
|return|.
- But in this case, I'd just do |return force_unique_process ? Identity(...) :
Identity(...);|.

[Sean Klein, 2015/11/16 02:24:26]

Done.

+    return Identity(resolved_url, create_id_string(uid++));
+  } else {
+    return Identity(resolved_url);
+  }
+}
+
 InterfaceRequest<Application> ApplicationManager::RegisterShell(
     const GURL& resolved_url,
     const GURL& requestor_url,
@@ -260,7 +284,7 @@ InterfaceRequest<Application> ApplicationManager::RegisterShell(
     ServiceProviderPtr exposed_services,
     const base::Closure& on_application_end,
     const std::vector<std::string>& parameters) {
-  Identity app_identity(resolved_url);
+  Identity app_identity = MakeApplicationIdentity(resolved_url);
 
   mojo::ApplicationPtr application;
   InterfaceRequest<Application> application_request =
@@ -274,6 +298,9 @@ InterfaceRequest<Application> ApplicationManager::RegisterShell(
   return application_request;
 }
 
+// Note: If a service was created with a unique ID, intending to be unique
+// (such that multiple requests for a service result in unique processes), then
+// 'GetShellImpl' should return nullptr.
 ShellImpl* ApplicationManager::GetShellImpl(const GURL& url) {
   const auto& shell_it = identity_to_shell_impl_.find(Identity(url));
   if (shell_it != identity_to_shell_impl_.end())
@@ -410,12 +437,14 @@ void ApplicationManager::LoadWithContentHandler(
     InterfaceRequest<Application> application_request,
     mojo::URLResponsePtr url_response) {
   ContentHandlerConnection* connection = nullptr;
-  auto it = url_to_content_handler_.find(content_handler_url);
-  if (it != url_to_content_handler_.end()) {
+  Identity content_handler_id = MakeApplicationIdentity(content_handler_url);
+  auto it = identity_to_content_handler_.find(content_handler_id);
+  if (it != identity_to_content_handler_.end()) {
     connection = it->second.get();
   } else {
-    connection = new ContentHandlerConnection(this, content_handler_url);
-    url_to_content_handler_[content_handler_url] = make_scoped_ptr(connection);
+    connection = new ContentHandlerConnection(this, content_handler_id);
+    identity_to_content_handler_[content_handler_id] =
+        make_scoped_ptr(connection);
   }
 
   connection->content_handler()->StartApplication(application_request.Pass(),
@@ -485,10 +514,9 @@ void ApplicationManager::OnShellImplError(ShellImpl* shell_impl) {
 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());
-  url_to_content_handler_.erase(it);
+  auto it = identity_to_content_handler_.find(content_handler->get_identity());
+  DCHECK(it != identity_to_content_handler_.end());
+  identity_to_content_handler_.erase(it);
 }
 
 mojo::ScopedMessagePipeHandle ApplicationManager::ConnectToServiceByName(