Render process count should not be forced from java unconditionally

content/browser/android/content_startup_flags.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.
 
 #include "content/browser/android/content_startup_flags.h"
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "cc/base/switches.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "ui/base/ui_base_switches.h"
 
 namespace content {
 
 void SetContentCommandLineFlags(int max_render_process_count,
                                 const std::string& plugin_descriptor) {
   // May be called multiple times, to cover all possible program entry points.
   static bool already_initialized = false;
   if (already_initialized)
     return;
   already_initialized = true;
 
   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);
   parsed_command_line->AppendSwitch(switches::kAllowWebUICompositing);
   parsed_command_line->AppendSwitch(switches::kEnableThreadedCompositing);
   parsed_command_line->AppendSwitch(
       switches::kEnableCompositingForFixedPosition);
   parsed_command_line->AppendSwitch(switches::kEnableAcceleratedOverflowScroll);
   parsed_command_line->AppendSwitch(
       switches::kEnableAcceleratedScrollableFrames);
@@ skipping 33 matching lines @@
   }
 
   // Disable profiler timing by default.
   if (!parsed_command_line->HasSwitch(switches::kProfilerTiming)) {
     parsed_command_line->AppendSwitchASCII(
         switches::kProfilerTiming, switches::kProfilerTimingDisabledValue);
   }
 }
 
 }  // namespace content