Don't share renderers between unrelated tabs on Android.

On Android we explicitly allow the OS to kill Chrome's background
renderers when under memory pressure and we don't try to control the
number of renderers ourselves.

The process limit logic in content causes process sharing
between unrelated tabs when the number of renderer process hosts
(not the number of actual live processes) is too high. Because on
Android the system adjusts the number of actual live processes for us,
we don't want to limit the number of process hosts or to ever share
renderers between unrelated tabs.

This patch:
- disables the renderer process host limit on Android. If not
  overridden, ShouldTryToUseExistingProcessHost() will always return
  false.
- drops the logic that sets the renderer limit based on the number of
  declared renderer services

BUG=325842

Review URL: https://codereview.chromium.org/356453003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284095 0039d316-1c4b-4281-b951-d872f2087c98

android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java

@@ -51,8 +51,7 @@ public abstract class AwBrowserProcess {
             @Override
             public void run() {
                 try {
-                    BrowserStartupController.get(context).startBrowserProcessesSync(
-                                BrowserStartupController.MAX_RENDERERS_SINGLE_PROCESS);
+                    BrowserStartupController.get(context).startBrowserProcessesSync(true);
                     initializePlatformKeySystem();
                 } catch (ProcessInitException e) {
                     throw new RuntimeException("Cannot initialize WebView", e);

chrome/android/java/src/org/chromium/chrome/browser/sync/ChromiumSyncAdapter.java

@@ -112,8 +112,7 @@ public abstract class ChromiumSyncAdapter extends AbstractThreadedSyncAdapter {
     private void startBrowserProcessesSync(
             final BrowserStartupController.StartupCallback callback) {
         try {
-            BrowserStartupController.get(mApplication).startBrowserProcessesSync(
-                    BrowserStartupController.MAX_RENDERERS_LIMIT);
+            BrowserStartupController.get(mApplication).startBrowserProcessesSync(false);
         } catch (ProcessInitException e) {
             Log.e(TAG, "Unable to load native library.", e);
             System.exit(-1);

chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java

@@ -42,8 +42,7 @@ public class ChromeShellTestBase extends ActivityInstrumentationTestCase2<Chrome
             public void run() {
                 CommandLine.initFromFile("/data/local/tmp/chrome-shell-command-line");
                 try {
-                    BrowserStartupController.get(targetContext).startBrowserProcessesSync(
-                            BrowserStartupController.MAX_RENDERERS_LIMIT);
+                    BrowserStartupController.get(targetContext).startBrowserProcessesSync(false);
                 } catch (ProcessInitException e) {
                     Log.e(TAG, "Unable to load native library.", e);
                     fail("Unable to load native library");

components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java

@@ -249,8 +249,7 @@ public class GCMDriver {
         // ChromeShellApplication.initCommandLine() as appropriate.
 
         try {
-            final int MAX_RENDERERS = 1;
-            BrowserStartupController.get(context).startBrowserProcessesSync(MAX_RENDERERS);
+            BrowserStartupController.get(context).startBrowserProcessesSync(false);
             if (sInstance != null) {
                 task.run();
             } else {

content/browser/android/browser_startup_controller.cc

@@ -28,13 +28,13 @@ bool RegisterBrowserStartupController(JNIEnv* env) {
 
 static void SetCommandLineFlags(JNIEnv* env,
                                 jclass clazz,
-                                jint max_render_process_count,
+                                jboolean single_process,
                                 jstring plugin_descriptor) {
   std::string plugin_str =
       (plugin_descriptor == NULL
            ? std::string()
            : base::android::ConvertJavaStringToUTF8(env, plugin_descriptor));
-  SetContentCommandLineFlags(max_render_process_count, plugin_str);
+  SetContentCommandLineFlags(static_cast<bool>(single_process), plugin_str);
 }
 
 static jboolean IsOfficialBuild(JNIEnv* env, jclass clazz) {

content/browser/android/content_startup_flags.cc

@@ -18,7 +18,7 @@
 
 namespace content {
 
-void SetContentCommandLineFlags(int max_render_process_count,
+void SetContentCommandLineFlags(bool single_process,
                                 const std::string& plugin_descriptor) {
   // May be called multiple times, to cover all possible program entry points.
   static bool already_initialized = false;
@@ -34,9 +34,7 @@ void SetContentCommandLineFlags(int max_render_process_count,
         switches::kRendererProcessLimit);
     int value;
     if (base::StringToInt(limit, &value)) {
-      command_line_renderer_limit = value;
-      if (value <= 0)
-        max_render_process_count = 0;
+      command_line_renderer_limit = std::max(0, value);
     }
   }
 
@@ -44,16 +42,13 @@ void SetContentCommandLineFlags(int max_render_process_count,
     int limit = std::min(command_line_renderer_limit,
                          static_cast<int>(kMaxRendererProcessCount));
     RenderProcessHost::SetMaxRendererProcessCount(limit);
-  } else if (max_render_process_count <= 0) {
+  }
+
+  if (single_process || command_line_renderer_limit == 0) {
     // Need to ensure the command line flag is consistent as a lot of chrome
     // internal code checks this directly, but it wouldn't normally get set when
     // we are implementing an embedded WebView.
     parsed_command_line->AppendSwitch(switches::kSingleProcess);
-  } else {
-    int default_maximum = RenderProcessHost::GetMaxRendererProcessCount();
-    DCHECK(default_maximum <= static_cast<int>(kMaxRendererProcessCount));
-    if (max_render_process_count < default_maximum)
-      RenderProcessHost::SetMaxRendererProcessCount(max_render_process_count);
   }
 
   parsed_command_line->AppendSwitch(

content/browser/android/content_startup_flags.h

@@ -12,7 +12,7 @@ namespace content {
 // Force-appends flags to the command line turning on Android-specific
 // features owned by Content. This is called as soon as possible during
 // initialization to make sure code sees the new flags.
-void SetContentCommandLineFlags(int max_render_process_count,
+void SetContentCommandLineFlags(bool single_process,
                                 const std::string& plugin_descriptor);
 
 }  // namespace content

content/browser/renderer_host/render_process_host_impl.cc

@@ -394,15 +394,22 @@ 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.
+#if defined(OS_ANDROID)
+  // On Android we don't maintain a limit of renderer process hosts - 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.
+  return std::numeric_limits<size_t>::max();
+#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

content/browser/renderer_host/render_process_host_unittest.cc

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <limits>
+
+#include "content/public/common/content_constants.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/test/test_render_view_host.h"
 
@@ -26,4 +29,51 @@ TEST_F(RenderProcessHostUnitTest, GuestsAreNotSuitableHosts) {
       RenderProcessHost::GetExistingProcessHost(browser_context(), test_url));
 }
 
+#if !defined(OS_ANDROID)
+TEST_F(RenderProcessHostUnitTest, RendererProcessLimit) {
+  // Disable any overrides.
+  RenderProcessHostImpl::SetMaxRendererProcessCount(0);
+
+  // Verify that the limit is between 1 and kMaxRendererProcessCount.
+  EXPECT_GT(RenderProcessHostImpl::GetMaxRendererProcessCount(), 0u);
+  EXPECT_LE(RenderProcessHostImpl::GetMaxRendererProcessCount(),
+      kMaxRendererProcessCount);
+
+  // Add dummy process hosts to saturate the limit.
+  ASSERT_NE(0u, kMaxRendererProcessCount);
+  ScopedVector<MockRenderProcessHost> hosts;
+  for (size_t i = 0; i < kMaxRendererProcessCount; ++i) {
+    hosts.push_back(new MockRenderProcessHost(browser_context()));
+  }
+
+  // Verify that the renderer sharing will happen.
+  GURL test_url("http://foo.com");
+  EXPECT_TRUE(RenderProcessHostImpl::ShouldTryToUseExistingProcessHost(
+        browser_context(), test_url));
+}
+#endif
+
+#if defined(OS_ANDROID)
+TEST_F(RenderProcessHostUnitTest, NoRendererProcessLimitOnAndroid) {
+  // Disable any overrides.
+  RenderProcessHostImpl::SetMaxRendererProcessCount(0);
+
+  // Verify that by default the limit on Android returns max size_t.
+  EXPECT_EQ(std::numeric_limits<size_t>::max(),
+      RenderProcessHostImpl::GetMaxRendererProcessCount());
+
+  // Add a few dummy process hosts.
+  ASSERT_NE(0u, kMaxRendererProcessCount);
+  ScopedVector<MockRenderProcessHost> hosts;
+  for (size_t i = 0; i < kMaxRendererProcessCount; ++i) {
+    hosts.push_back(new MockRenderProcessHost(browser_context()));
+  }
+
+  // Verify that the renderer sharing still won't happen.
+  GURL test_url("http://foo.com");
+  EXPECT_FALSE(RenderProcessHostImpl::ShouldTryToUseExistingProcessHost(
+        browser_context(), test_url));
+}
+#endif
+
 }  // namespace content

content/browser/site_instance_impl_unittest.cc

@@ -562,6 +562,11 @@ TEST_F(SiteInstanceTest, ProcessSharingByType) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess))
     return;
 
+  // On Android by default the number of renderer hosts is unlimited and process
+  // sharing doesn't happen. We set the override so that the test can run
+  // everywhere.
+  RenderProcessHost::SetMaxRendererProcessCount(kMaxRendererProcessCount);
+
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
@@ -607,6 +612,9 @@ TEST_F(SiteInstanceTest, ProcessSharingByType) {
   }
 
   DrainMessageLoops();
+
+  // Disable the process limit override.
+  RenderProcessHost::SetMaxRendererProcessCount(0u);
 }
 
 // Test to ensure that HasWrongProcessForURL behaves properly for different

content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java

@@ -94,20 +94,6 @@ public class BrowserStartupController {
     // Whether the async startup of the browser process is complete.
     private boolean mStartupDone;
 
-    // Use single-process mode that runs the renderer on a separate thread in
-    // the main application.
-    public static final int MAX_RENDERERS_SINGLE_PROCESS = 0;
-
-    // Cap on the maximum number of renderer processes that can be requested.
-    // This is currently set to account for:
-    //  13: The maximum number of sandboxed processes we have available
-    // - 1: The regular New Tab Page
-    // - 1: The incognito New Tab Page
-    // - 1: A regular incognito tab
-    // - 1: Safety buffer (http://crbug.com/251279)
-    public static final int MAX_RENDERERS_LIMIT =
-            ChildProcessLauncher.MAX_REGISTERED_SANDBOXED_SERVICES - 4;
-
     // This field is set after startup has been completed based on whether the startup was a success
     // or not. It is used when later requests to startup come in that happen after the initial set
     // of enqueued callbacks have been executed.
@@ -161,7 +147,7 @@ public class BrowserStartupController {
             // flag that indicates that we have kicked off starting the browser process.
             mHasStartedInitializingBrowserProcess = true;
 
-            prepareToStartBrowserProcess(MAX_RENDERERS_LIMIT);
+            prepareToStartBrowserProcess(false);
 
             setAsynchronousStartup(true);
             if (contentStart() > 0) {
@@ -178,15 +164,15 @@ public class BrowserStartupController {
      * <p/>
      * Note that this can only be called on the UI thread.
      *
-     * @param maxRenderers The maximum number of renderer processes the browser may
-     *                      create. Zero for single process mode.
+     * @param singleProcess true iff the browser should run single-process, ie. keep renderers in
+     *                      the browser process
      * @throws ProcessInitException
      */
-    public void startBrowserProcessesSync(int maxRenderers) throws ProcessInitException {
+    public void startBrowserProcessesSync(boolean singleProcess) throws ProcessInitException {
         // If already started skip to checking the result
         if (!mStartupDone) {
             if (!mHasStartedInitializingBrowserProcess) {
-                prepareToStartBrowserProcess(maxRenderers);
+                prepareToStartBrowserProcess(singleProcess);
             }
 
             setAsynchronousStartup(false);
@@ -260,8 +246,8 @@ public class BrowserStartupController {
     }
 
     @VisibleForTesting
-    void prepareToStartBrowserProcess(int maxRendererProcesses) throws ProcessInitException {
-        Log.i(TAG, "Initializing chromium process, renderers=" + maxRendererProcesses);
+    void prepareToStartBrowserProcess(boolean singleProcess) throws ProcessInitException {
+        Log.i(TAG, "Initializing chromium process, singleProcess=" + singleProcess);
 
         // Normally Main.java will have kicked this off asynchronously for Chrome. But other
         // ContentView apps like tests also need them so we make sure we've extracted resources
@@ -280,8 +266,7 @@ public class BrowserStartupController {
         // Now we really need to have the resources ready.
         resourceExtractor.waitForCompletion();
 
-        nativeSetCommandLineFlags(maxRendererProcesses,
-                nativeIsPluginEnabled() ? getPlugins() : null);
+        nativeSetCommandLineFlags(singleProcess, nativeIsPluginEnabled() ? getPlugins() : null);
         ContentMain.initApplicationContext(appContext);
     }
 
@@ -292,18 +277,15 @@ public class BrowserStartupController {
         ResourceExtractor resourceExtractor = ResourceExtractor.get(mContext);
         resourceExtractor.startExtractingResources();
         resourceExtractor.waitForCompletion();
-
-        // Having a single renderer should be sufficient for tests. We can't have more than
-        // MAX_RENDERERS_LIMIT.
-        nativeSetCommandLineFlags(1 /* maxRenderers */, null);
+        nativeSetCommandLineFlags(false, null);
     }
 
     private String getPlugins() {
         return PepperPluginManager.getPlugins(mContext);
     }
 
-    private static native void nativeSetCommandLineFlags(int maxRenderProcesses,
-            String pluginDescriptor);
+    private static native void nativeSetCommandLineFlags(
+            boolean singleProcess, String pluginDescriptor);
 
     // Is this an official build of Chrome? Only native code knows for sure. Official build
     // knowledge is needed very early in process startup.

content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java

@@ -92,7 +92,8 @@ public class ChildProcessLauncher {
                 ChromiumLinkerParams chromiumLinkerParams) {
             synchronized (mConnectionLock) {
                 if (mFreeConnectionIndices.isEmpty()) {
-                    Log.w(TAG, "Ran out of service.");
+                    Log.e(TAG, "Ran out of services to allocate.");
+                    assert false;
                     return null;
                 }
                 int slot = mFreeConnectionIndices.remove(0);

content/public/android/javatests/src/org/chromium/content/browser/BrowserStartupControllerTest.java

@@ -27,7 +27,7 @@ public class BrowserStartupControllerTest extends InstrumentationTestCase {
         private int mInitializedCounter = 0;
 
         @Override
-        void prepareToStartBrowserProcess(int numRenderers) throws ProcessInitException {
+        void prepareToStartBrowserProcess(boolean singleProcess) throws ProcessInitException {
             if (!mLibraryLoadSucceeds) {
                 throw new ProcessInitException(
                         LoaderErrors.LOADER_ERROR_NATIVE_LIBRARY_LOAD_FAILED);
@@ -360,7 +360,7 @@ public class BrowserStartupControllerTest extends InstrumentationTestCase {
             @Override
             public void run() {
                 try {
-                    mController.startBrowserProcessesSync(1);
+                    mController.startBrowserProcessesSync(false);
                 } catch (Exception e) {
                     fail("Browser should have started successfully");
                 }
@@ -392,7 +392,7 @@ public class BrowserStartupControllerTest extends InstrumentationTestCase {
                 // to do both these in a since Runnable instance. This avoids the
                 // unpredictable race that happens in real situations.
                 try {
-                    mController.startBrowserProcessesSync(1);
+                    mController.startBrowserProcessesSync(false);
                 } catch (Exception e) {
                     fail("Browser should have started successfully");
                 }
@@ -421,7 +421,7 @@ public class BrowserStartupControllerTest extends InstrumentationTestCase {
             @Override
             public void run() {
                 try {
-                    mController.startBrowserProcessesSync(1);
+                    mController.startBrowserProcessesSync(false);
                 } catch (Exception e) {
                     fail("Browser should have started successfully");
                 }

content/public/browser/render_process_host.h

@@ -279,8 +279,8 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
   // A value of zero means to use the default heuristic.
   static void SetMaxRendererProcessCount(size_t count);
 
-  // Returns the current max number of renderer processes used by the content
-  // module.
+  // Returns the current maximum number of renderer process hosts kept by the
+  // content module.
   static size_t GetMaxRendererProcessCount();
 };
 

content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellActivity.java

@@ -78,8 +78,7 @@ public class ContentShellActivity extends Activity {
 
         if (CommandLine.getInstance().hasSwitch(ContentSwitches.DUMP_RENDER_TREE)) {
             try {
-                BrowserStartupController.get(this).startBrowserProcessesSync(
-                       BrowserStartupController.MAX_RENDERERS_LIMIT);
+                BrowserStartupController.get(this).startBrowserProcessesSync(false);
             } catch (ProcessInitException e) {
                 Log.e(TAG, "Failed to load native library.", e);
                 System.exit(-1);