diff options
15 files changed, 105 insertions, 67 deletions
diff --git a/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java b/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java index 15c5edb..154179c 100644 --- a/android_webview/java/src/org/chromium/android_webview/AwBrowserProcess.java +++ b/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); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/sync/ChromiumSyncAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/sync/ChromiumSyncAdapter.java index 2c7510e..c56f560 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/sync/ChromiumSyncAdapter.java +++ b/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); diff --git a/chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java b/chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java index e3f8abc..01d160c 100644 --- a/chrome/android/shell/javatests/src/org/chromium/chrome/shell/ChromeShellTestBase.java +++ b/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"); diff --git a/components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java b/components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java index 579f181..f6596ee 100644 --- a/components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java +++ b/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 { diff --git a/content/browser/android/browser_startup_controller.cc b/content/browser/android/browser_startup_controller.cc index 502f198..74e38e6 100644 --- a/content/browser/android/browser_startup_controller.cc +++ b/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) { diff --git a/content/browser/android/content_startup_flags.cc b/content/browser/android/content_startup_flags.cc index f185691..d315371 100644 --- a/content/browser/android/content_startup_flags.cc +++ b/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( diff --git a/content/browser/android/content_startup_flags.h b/content/browser/android/content_startup_flags.h index 836ea3c..8f55a0c 100644 --- a/content/browser/android/content_startup_flags.h +++ b/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 diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index 61fb09e..6b6c3a1 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/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 diff --git a/content/browser/renderer_host/render_process_host_unittest.cc b/content/browser/renderer_host/render_process_host_unittest.cc index 474a72b..ca497a3 100644 --- a/content/browser/renderer_host/render_process_host_unittest.cc +++ b/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 diff --git a/content/browser/site_instance_impl_unittest.cc b/content/browser/site_instance_impl_unittest.cc index 646f4d0..f76be5e 100644 --- a/content/browser/site_instance_impl_unittest.cc +++ b/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 diff --git a/content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java b/content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java index 4b4420a..8b9f445 100644 --- a/content/public/android/java/src/org/chromium/content/browser/BrowserStartupController.java +++ b/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. diff --git a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java index b285e19..2a4bcd8 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java +++ b/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); diff --git a/content/public/android/javatests/src/org/chromium/content/browser/BrowserStartupControllerTest.java b/content/public/android/javatests/src/org/chromium/content/browser/BrowserStartupControllerTest.java index b9d9f26..7686f7e 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/BrowserStartupControllerTest.java +++ b/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"); } diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h index b050fd1..c1b5c47 100644 --- a/content/public/browser/render_process_host.h +++ b/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(); }; diff --git a/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellActivity.java b/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellActivity.java index 1703cf5..4e5edbc 100644 --- a/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellActivity.java +++ b/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); |