Move process-per-site logic from BrowsingInstance to RenderProcessHost.

content/browser/renderer_host/render_process_host_impl.cc

 // Copyright (c) 2012 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.
 
 // Represents the browser side of the browser <--> renderer communication
 // channel. There will be one RenderProcessHost per renderer process.
 
 #include "content/browser/renderer_host/render_process_host_impl.h"
 
 #if defined(OS_WIN)
@@ skipping 73 matching lines @@
 #include "content/browser/trace_message_filter.h"
 #include "content/browser/worker_host/worker_message_filter.h"
 #include "content/common/child_process_host_impl.h"
 #include "content/common/child_process_messages.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "content/common/resource_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_process_host_factory.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_ui_controller_factory.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/process_type.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "content/renderer/render_process_impl.h"
 #include "content/renderer/render_thread_impl.h"
@@ skipping 97 matching lines @@
   virtual ~RendererURLRequestContextSelector() {}
 
   scoped_refptr<net::URLRequestContextGetter> request_context_;
   scoped_refptr<net::URLRequestContextGetter> media_request_context_;
 };
 
 // the global list of all renderer processes
 base::LazyInstance<IDMap<RenderProcessHost> >::Leaky
     g_all_hosts = LAZY_INSTANCE_INITIALIZER;
 
+// The global map of BrowsingContexts to SiteProcessMaps, allowing us to
+// implement process-per-site mode within each BrowsingContext.
+base::LazyInstance<RenderProcessHostImpl::ContextSiteProcessMap>::Leaky
+    g_context_site_process_map_ = LAZY_INSTANCE_INITIALIZER;
+
 }  // namespace
 
 // Stores the maximum number of renderer processes the content module can
 // create.
 static size_t g_max_renderer_count_override = 0;
 
 // static
 size_t RenderProcessHost::GetMaxRendererProcessCount() {
   if (g_max_renderer_count_override)
     return g_max_renderer_count_override;
@@ skipping 110 matching lines @@
   ChildProcessSecurityPolicyImpl::GetInstance()->Remove(GetID());
 
   // We may have some unsent messages at this point, but that's OK.
   channel_.reset();
   while (!queued_messages_.empty()) {
     delete queued_messages_.front();
     queued_messages_.pop();
   }
 
   ClearTransportDIBCache();
-  UnregisterHost(GetID());
+  UnregisterHost(GetID(), this);
 }
 
 void RenderProcessHostImpl::EnableSendQueue() {
   is_initialized_ = false;
 }
 
 bool RenderProcessHostImpl::Init() {
   // calling Init() more than once does nothing, this makes it more convenient
   // for the view host which may not be sure in some cases
   if (channel_.get())
@@ skipping 712 matching lines @@
     // It's important not to wait for the DeleteTask to delete the channel
     // proxy. Kill it off now. That way, in case the profile is going away, the
     // rest of the objects attached to this RenderProcessHost start going
     // away first, since deleting the channel proxy will post a
     // OnChannelClosed() to IPC::ChannelProxy::Context on the IO thread.
     channel_.reset();
     gpu_message_filter_ = NULL;
 
     // Remove ourself from the list of renderer processes so that we can't be
     // reused in between now and when the Delete task runs.
-    g_all_hosts.Get().Remove(GetID());
+    UnregisterHost(GetID(), this);
   }
 }
 
 void RenderProcessHostImpl::AddPendingView() {
   pending_views_++;
 }
 
 void RenderProcessHostImpl::RemovePendingView() {
   DCHECK(pending_views_);
   pending_views_--;
@@ skipping 38 matching lines @@
 bool RenderProcessHostImpl::FastShutdownStarted() const {
   return fast_shutdown_started_;
 }
 
 // static
 void RenderProcessHostImpl::RegisterHost(int host_id, RenderProcessHost* host) {
   g_all_hosts.Get().AddWithID(host, host_id);
 }
 
 // static
-void RenderProcessHostImpl::UnregisterHost(int host_id) {
+void RenderProcessHostImpl::UnregisterHost(int host_id,
+                                           RenderProcessHost* host) {
   if (g_all_hosts.Get().Lookup(host_id))
     g_all_hosts.Get().Remove(host_id);
+
+  // Look up the map of site to process for the given browser_context,
+  // in case we need to remove this process from it.  It will be registered
+  // under any sites it rendered that use process-per-site mode.
+  RenderProcessHostImpl::SiteProcessMap* map =
+      &g_context_site_process_map_.Get()[host->GetBrowserContext()];

[awong, 2012/06/27 00:26:54]

Having a global map of BrowserContext objects -> data seems odd.  Would it be
better to user browser_context->SetUserData()? I'm not sure what the rules are
around adding more keys into the context though.

[Charlie Reis, 2012/06/27 20:53:43]

That would be possible, but it also requires adding a new class that inherits
from SupportsUserData::Data.  At the moment, I'm just moving the global state
from BrowsingInstance to RenderProcessHostImpl.

I'm inclined to leave it as is for the moment, but I can switch it if others
prefer.

[awong, 2012/06/27 21:24:40]

sgtm

+  // Find all instances of this process in the map, to remove.
+  std::set<std::string> sites;
+  for (RenderProcessHostImpl::SiteProcessMap::const_iterator i = map->begin();
+      i != map->end();
+      i++) {
+    if (i->second == host)
+      sites.insert(i->first);
+  }
+  // Remove them from the map.

[awong, 2012/06/27 00:26:54]

I'd just merge this with the previous comment. This is getting a bit too close
to redescribing what the code says.

[Charlie Reis, 2012/06/27 20:53:43]

Done.

+  for (std::set<std::string>::iterator i = sites.begin();
+      i != sites.end();
+      i++) {
+    RenderProcessHostImpl::SiteProcessMap::iterator iter = map->find(*i);
+    if (iter != map->end()) {
+      DCHECK_EQ(iter->second, host);
+      map->erase(iter);
+    }
+  }
 }
 
 // static
 bool RenderProcessHostImpl::IsSuitableHost(
     RenderProcessHost* host,
     BrowserContext* browser_context,
     const GURL& site_url) {
   if (run_renderer_in_process())
     return true;
 
@@ skipping 82 matching lines @@
   // Now pick a random suitable renderer, if we have any.
   if (!suitable_renderers.empty()) {
     int suitable_count = static_cast<int>(suitable_renderers.size());
     int random_index = base::RandInt(0, suitable_count - 1);
     return suitable_renderers[random_index];
   }
 
   return NULL;
 }
 
+// static
+bool RenderProcessHost::ShouldUseProcessPerSite(BrowserContext* browser_context,
+                                                const GURL& url) {
+  // Returns true if we should use the process-per-site model.  This will be
+  // the case if the --process-per-site switch is specified, or in
+  // process-per-site-instance for particular sites (e.g., WebUI).
+
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kProcessPerSite))
+    return true;
+
+  // We want to consolidate particular sites like WebUI when we are using
+  // process-per-tab or process-per-site-instance models.
+  // Note that --single-process is handled in ShouldTryToUseExistingProcessHost.
+
+  if (content::GetContentClient()->browser()->
+          ShouldUseProcessPerSite(browser_context, url)) {
+    return true;
+  }
+
+  // DevTools pages have WebUI type but should not reuse the same host.
+  WebUIControllerFactory* factory =
+      content::GetContentClient()->browser()->GetWebUIControllerFactory();
+  if (factory &&
+      factory->UseWebUIForURL(browser_context, url) &&
+      !url.SchemeIs(chrome::kChromeDevToolsScheme)) {
+    return true;
+  }
+
+  // In all other cases, don't use process-per-site logic.
+  return false;
+}
+
+// static
+RenderProcessHost* RenderProcessHost::GetProcessHostForSite(
+    BrowserContext* browser_context,
+    const GURL& url) {
+  // Look up the map of site to process for the given browser_context.
+  RenderProcessHostImpl::SiteProcessMap* map =
+      &g_context_site_process_map_.Get()[browser_context];
+
+  // See if we have an existing process for this site.
+  GURL site_url = SiteInstanceImpl::GetSiteForURL(browser_context, url);

[awong, 2012/06/27 00:26:54]

std::string site = SiteInstanceImpl::GetSiteForURL(browser_context, url)
    .possibly_invalid_spec();

is what is used in browsing_instance.cc.  Make them consistent here and
elsewhere?  (I personally prefer not introducing the extra var, but up to you).

[Charlie Reis, 2012/06/27 20:53:43]

Done.

+  std::string site = site_url.possibly_invalid_spec();
+  RenderProcessHostImpl::SiteProcessMap::iterator i = map->find(site);
+  if (i != map->end())
+    return i->second;
+
+  // Otherwise, the caller should create a new process and register it.
+  return NULL;
+}
+
+void RenderProcessHost::RegisterProcessHostForSite(
+    BrowserContext* browser_context,
+    RenderProcessHost* process,
+    const GURL& url) {
+  // Look up the map of site to process for the given browser_context.
+  RenderProcessHostImpl::SiteProcessMap* map =
+      &g_context_site_process_map_.Get()[browser_context];
+
+  GURL site_url = SiteInstanceImpl::GetSiteForURL(browser_context, url);
+  std::string site = site_url.possibly_invalid_spec();
+  if (!site.empty())
+    (*map)[site] = process;

[awong, 2012/06/27 00:26:54]

What happens if the site is empty?  The process is just never recorded?

[Charlie Reis, 2012/06/27 20:53:43]

Right.  I didn't think we would want to group all degenerate cases into the same
process.

Then again, we didn't have that check before in BrowsingInstance.  Perhaps (in
the interest of having a minimal functional change) I should leave the empty
check out.  Updated.

[awong, 2012/06/27 21:24:40]

Cool.  Also maybe a comment explaining why the behavior makes sense if
site.empty() would be helpful to future readers?

[Charlie Reis, 2012/06/27 21:42:30]

Not sure there's a clear answer, so I added a TODO comment to investigate.

+}
+
 void RenderProcessHostImpl::ProcessDied(base::ProcessHandle handle,
-                                           base::TerminationStatus status,
-                                           int exit_code,
-                                           bool was_alive) {
+                                        base::TerminationStatus status,
+                                        int exit_code,
+                                        bool was_alive) {
   // Our child process has died.  If we didn't expect it, it's a crash.
   // In any case, we need to let everyone know it's gone.
   // The OnChannelError notification can fire multiple times due to nested sync
   // calls to a renderer. If we don't have a valid channel here it means we
   // already handled the error.
 
   RendererClosedDetails details(handle, status, exit_code, was_alive);
   NotificationService::current()->Notify(
       NOTIFICATION_RENDERER_PROCESS_CLOSED,
       Source<RenderProcessHost>(this),
@@ skipping 139 matching lines @@
       uint64 surface_handle,
       int32 route_id,
       int32 gpu_process_host_id) {
   TRACE_EVENT0("renderer_host",
                "RenderWidgetHostImpl::OnCompositorSurfaceBuffersSwappedNoHost");
   RenderWidgetHostImpl::AcknowledgeSwapBuffers(route_id,
                                                gpu_process_host_id);
 }
 
 }  // namespace content