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

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 <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"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string_util.h"
 #include "base/trace_event/trace_event.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/services/authenticating_url_loader_interceptor/interfaces/authenticating_url_loader_interceptor_meta_factory.mojom.h"
 #include "mojo/services/authentication/interfaces/authentication.mojom.h"
@@ skipping 21 matching lines @@
                                      const std::vector<std::string>& v2) {
   if (!v1.size())
     return v2;
   if (!v2.size())
     return v1;
   std::vector<std::string> result(v1);
   result.insert(result.end(), v1.begin(), v1.end());
   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));
     services->ConnectToService(mojo::ContentHandler::Name_,
                                pipe.handle1.Pass());
     content_handler_.set_connection_error_handler(
         [this]() { manager_->OnContentHandlerError(this); });
   }
   ~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);
 };
 
 // static
 ApplicationManager::TestAPI::TestAPI(ApplicationManager* manager)
     : manager_(manager) {
 }
 
@@ skipping 138 matching lines @@
 bool ApplicationManager::ConnectToRunningApplication(
     const GURL& resolved_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider>* services,
     ServiceProviderPtr* exposed_services) {
   GURL application_url = GetBaseURLAndQuery(resolved_url, nullptr);
   ShellImpl* shell_impl = GetShellImpl(application_url);
   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;
 }
 
 bool ApplicationManager::ConnectToApplicationWithLoader(
     const GURL& resolved_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider>* services,
     ServiceProviderPtr* exposed_services,
     const base::Closure& on_application_end,
     const std::vector<std::string>& parameters,
     ApplicationLoader* loader) {
   if (!loader)
     return false;
 
   loader->Load(
       resolved_url,
       RegisterShell(resolved_url, requestor_url, services->Pass(),
                     exposed_services->Pass(), on_application_end, parameters));
   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,
     InterfaceRequest<ServiceProvider> services,
     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 =
       mojo::GetProxy(&application);
   ShellImpl* shell =
       new ShellImpl(application.Pass(), this, app_identity, on_application_end);
   identity_to_shell_impl_[app_identity] = make_scoped_ptr(shell);
   shell->InitializeApplication(mojo::Array<mojo::String>::From(parameters));
   ConnectToClient(shell, resolved_url, requestor_url, services.Pass(),
                   exposed_services.Pass());
   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())
     return shell_it->second.get();
   return nullptr;
 }
 
 void ApplicationManager::ConnectToClient(
     ShellImpl* shell_impl,
     const GURL& resolved_url,
@@ skipping 116 matching lines @@
   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;
 }
 
 void ApplicationManager::LoadWithContentHandler(
     const GURL& content_handler_url,
     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(),
                                                   url_response.Pass());
 }
 
 void ApplicationManager::SetLoaderForURL(scoped_ptr<ApplicationLoader> loader,
                                          const GURL& url) {
   url_to_loader_[url] = loader.Pass();
 }
@@ skipping 49 matching lines @@
   auto it = identity_to_shell_impl_.find(identity);
   DCHECK(it != identity_to_shell_impl_.end());
   identity_to_shell_impl_.erase(it);
   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());
-  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(
     const GURL& application_url,
     const std::string& interface_name) {
   ServiceProviderPtr services;
   ConnectToApplication(application_url, GURL(), mojo::GetProxy(&services),
                        nullptr, base::Closure());
   mojo::MessagePipe pipe;
   services->ConnectToService(interface_name, pipe.handle1.Pass());
   return pipe.handle0.Pass();
 }
 
 std::vector<std::string> ApplicationManager::GetArgsForURL(const GURL& url) {
   const auto& args_it = url_to_args_.find(url);
   if (args_it != url_to_args_.end())
     return args_it->second;
   return std::vector<std::string>();
 }
 
 void ApplicationManager::CleanupRunner(NativeRunner* runner) {
   native_runners_.erase(
       std::find(native_runners_.begin(), native_runners_.end(), runner));
 }
 
 }  // namespace shell