Render process count should not be forced from java unconditionally

content/browser/android/content_startup_flags.cc

Index: content/browser/android/content_startup_flags.cc
diff --git a/content/browser/android/content_startup_flags.cc b/content/browser/android/content_startup_flags.cc
index 7d9503434434ae5830345e6dfbde40d574f8a911..3ce2bd657b0c2d066f1bc433537b8d47848a9cb9 100644
--- a/content/browser/android/content_startup_flags.cc
+++ b/content/browser/android/content_startup_flags.cc
@@ -27,25 +27,31 @@ void SetContentCommandLineFlags(int max_render_process_count,
 
   CommandLine* parsed_command_line = CommandLine::ForCurrentProcess();
 
+  int command_line_renderer_limit = -1;
   if (parsed_command_line->HasSwitch(switches::kRendererProcessLimit)) {
     std::string limit = parsed_command_line->GetSwitchValueASCII(
         switches::kRendererProcessLimit);
     int value;
     if (base::StringToInt(limit, &value))
-      max_render_process_count = value;
+      command_line_renderer_limit = value;
   }
 
-  if (max_render_process_count <= 0) {
+  if (command_line_renderer_limit != -1) {

[aberent, 2014/01/24 21:29:09]

What happens if command_line_render_limit is set but max_render_process_count <=
0?
It seems that that setting the render limit on the command line shouldn't
override single process mode, so that should execute the next block, but with
this code it won't.
Also what happens if command_line_render_limit is 0 (or < -1)?

[kbalazs, 2014/01/29 16:55:43]

I think the command line should override everything if it's >= 1. Good point
about 0 or < -1, I will turn the condition into |command_line_renderer_limit >
0|.

+    int limit = std::min(command_line_renderer_limit,
+                         static_cast<int>(kMaxRendererProcessCount));
+    RenderProcessHost::SetMaxRendererProcessCount(limit);
+  } else if (max_render_process_count <= 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 {
-    max_render_process_count =
-        std::min(max_render_process_count,
-                 static_cast<int>(content::kMaxRendererProcessCount));
-    content::RenderProcessHost::SetMaxRendererProcessCount(
-        max_render_process_count);
+    int default_maximum = RenderProcessHost::GetMaxRendererProcessCount();

[aberent, 2014/01/24 21:29:09]

If I am reading this right then GetMaxRendererProcessCount() isn't called if the
process count is set on the command line, so the available physical memory etc.
is ignored. This seems to contradict the description of the CL. Is this what you
intend?

[kbalazs, 2014/01/29 16:55:43]

Yes, this was my intent. This is consistent with this:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br....

+    DCHECK(default_maximum <= kMaxRendererProcessCount);
+    if (max_render_process_count < default_maximum) {
+      content::RenderProcessHost::SetMaxRendererProcessCount(

[aberent, 2014/01/24 21:29:09]

Why content::RenderProcessHost here, but simply RenderProcessHost on lines 42
and 49?

[kbalazs, 2014/01/29 16:55:43]

by mistake

+          max_render_process_count);
+    }
   }
 
   parsed_command_line->AppendSwitch(switches::kForceCompositingMode);