Don't share renderers between unrelated tabs on Android.

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"
 
 #include <algorithm>
@@ skipping 376 matching lines @@
 
 // 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;
 
-  // Defines the maximum number of renderer processes according to the
-  // amount of installed memory as reported by the OS. The calculation
-  // assumes that you want the renderers to use half of the installed
-  // RAM and assuming that each WebContents uses ~40MB.
-  // If you modify this assumption, you need to adjust the
-  // ThirtyFourTabs test to match the expected number of processes.
+  // On Android we don't maintain a limit on the number of renderer process
+  // hosts we keep around - we are happy with keeping a lot of these, as long as
+  // the number of live renderer processes remains reasonable, and on Android
+  // the OS takes care of that.
+#if defined(OS_ANDROID)

[klobag.chromium, 2014/06/27 16:25:42]

hmm, I don't think this is a good way. It makes this api very subtle.

[ppi, 2014/06/27 16:40:32]

But it doesn't diverge too much from the codepaths of the other platforms. We
still have all of the code working the same way, just returning infinity (-> no
limit) here (unless overriden) and a finite number on other platforms.

Alternative would be to make the change in
"ShouldTryToUseExistingProcessHost()", making it always return false on Android.
But this breaks the connection between GetMaxRendererProcessCount() and
ShouldTryToUseExistingProcessHos() and takes away the useful ability of
overriding the limit to have the old behavior back (e.g. for local
experimentation).

Wdyt?

[ppi, 2014/06/27 16:44:32]

Should we add a separate boolean function to the API like "bool
IsRendererProcessCountLimited()" to make this explicit / less tricky?

[jam, 2014/07/14 22:45:56]

I agree with Grace that it's confusing to return 0 to mean no limit. Why not
return SIZE_MAX?

[ppi, 2014/07/16 13:26:54]

Done.

+  return 0u;
+#endif
+
+  // On other platforms, we calculate the maximum number of renderer process
+  // hosts according to the amount of installed memory as reported by the OS.
+  // The calculation assumes that you want the renderers to use half of the
+  // installed RAM and assuming that each WebContents uses ~40MB.  If you modify
+  // this assumption, you need to adjust the ThirtyFourTabs test to match the
+  // expected number of processes.
   //
-  // With the given amounts of installed memory below on a 32-bit CPU,
-  // the maximum renderer count will roughly be as follows:
+  // With the given amounts of installed memory below on a 32-bit CPU, the
+  // maximum renderer count will roughly be as follows:
   //
   //   128 MB -> 3
   //   512 MB -> 6
   //  1024 MB -> 12
   //  4096 MB -> 51
   // 16384 MB -> 82 (kMaxRendererProcessCount)
 
   static size_t max_count = 0;
   if (!max_count) {
     const size_t kEstimatedWebContentsMemoryUsage =
@@ skipping 1363 matching lines @@
     return false;
 
   if (run_renderer_in_process())
     return true;
 
   // NOTE: Sometimes it's necessary to create more render processes than
   //       GetMaxRendererProcessCount(), for instance when we want to create
   //       a renderer process for a browser context that has no existing
   //       renderers. This is OK in moderation, since the
   //       GetMaxRendererProcessCount() is conservative.
-  if (g_all_hosts.Get().size() >= GetMaxRendererProcessCount())
+  size_t render_host_limit = GetMaxRendererProcessCount();
+  if (render_host_limit && g_all_hosts.Get().size() >= render_host_limit)

[klobag.chromium, 2014/06/27 16:25:42]

Should we check the live process count?

[ppi, 2014/06/27 16:40:32]

Dead process hosts can come back (re-spawn their processes) without going
through this path. So for other platforms checking the number of process hosts
can make sense I think.

     return true;
 
   return GetContentClient()->browser()->
       ShouldTryToUseExistingProcessHost(browser_context, url);
 }
 
 // static
 RenderProcessHost* RenderProcessHost::GetExistingProcessHost(
     BrowserContext* browser_context,
     const GURL& site_url) {
@@ skipping 561 matching lines @@
 void RenderProcessHostImpl::GpuMemoryBufferAllocated(
     IPC::Message* reply,
     const gfx::GpuMemoryBufferHandle& handle) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   ChildProcessHostMsg_SyncAllocateGpuMemoryBuffer::WriteReplyParams(reply,
                                                                     handle);
   Send(reply);
 }
 
 }  // namespace content