Expose a low-end device mode override flags for non-android OSs as well

Expose a low-end device mode override flags for non-android OSs as well

BUG=324824

[ostap, 2013-12-20 22:53:20]

Script recommended me those reviewers:

tony:
  webkit/child/webkitplatformsupport_impl.cc
yfriedman:
  base/android/base_jni_registrar.cc
  base/android/sys_utils.cc
  base/android/sys_utils.h
  base/android/sys_utils_android.cc
  base/android/sys_utils_android.h
  base/android/sys_utils_unittest.cc
  chrome/browser/android/chrome_startup_flags.cc
jochen:
  chrome/browser/android/chrome_startup_flags.cc
  chrome/browser/prerender/prerender_manager_factory.cc
  chrome/browser/renderer_host/web_cache_manager.cc
  content/browser/renderer_host/render_process_host_impl.cc
  content/renderer/gpu/render_widget_compositor.cc
  content/renderer/render_process_impl.cc
  content/renderer/render_widget.cc
  content/renderer/renderer_main.cc
willchan:
  base/android/base_jni_registrar.cc
  base/android/sys_utils.cc
  base/android/sys_utils.h
  base/android/sys_utils_android.cc
  base/android/sys_utils_android.h
  base/android/sys_utils_unittest.cc
  base/base.gypi
  base/base_switches.cc
  base/base_switches.h
  base/sys_utils.cc
  base/sys_utils.h
piman:
  cc/resources/pixel_buffer_raster_worker_pool.cc
  content/browser/renderer_host/render_process_host_impl.cc
  content/renderer/gpu/render_widget_compositor.cc
  content/renderer/render_process_impl.cc
  content/renderer/render_widget.cc
  content/renderer/renderer_main.cc
  gpu/command_buffer/service/async_pixel_transfer_manager_android.cc
  ui/gl/gl_context_android.cc

[willchan no longer on Chromium, 2013-12-26 19:02:38]

https://chromiumcodereview.appspot.com/119493005/diff/1/base/android/sys_util...
File base/android/sys_utils_android.cc (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/base/android/sys_util...
base/android/sys_utils_android.cc:26: static bool is_low_end =
android::SysUtilsAndroid::IsLowEndDeviceFromJni();
This is racy too

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.cc
File base/sys_utils.cc (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.cc#new...
base/sys_utils.cc:25: int ramSizeKB = SysUtils::AmountOfPhysicalMemoryKB();
ram_size_kb

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.cc#new...
base/sys_utils.cc:31: static bool isLowEndDevice = detectLowEndDevice();
We disable the compiler generated locks around function local statics. This is
racy. You should use LazyInstance to fix the race.

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h
File base/sys_utils.h (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h#newc...
base/sys_utils.h:15: // Returns true iff this is a low-end device.
This is too vague. It was kinda-OK when it was just Android, but for other
platforms? What does "low-end" mean?

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h#newc...
base/sys_utils.h:19: static size_t AmountOfPhysicalMemoryKB();
Why does this even exist? Why aren't we using SysInfo? How is SysUtils supposed
to be different from SysInfo?

[lgombos, 2013-12-27 05:31:44]

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h
File base/sys_utils.h (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h#newc...
base/sys_utils.h:15: // Returns true iff this is a low-end device.
On 2013/12/26 19:02:39, willchan wrote:
> This is too vague. It was kinda-OK when it was just Android, but for other
> platforms? What does "low-end" mean?

I have to agree that it is vague, but fwiw I think making it available to other
OSes does not make it more vague. I expect that the flag could be used the exact
same way as it is on Android for platforms where 'embedded==1'. 

I personally do not feel strongly about "auto-detecting" if chromium runs on
"low-end". Just allowing the command line option to initialize this bit is very
valuable by itself, without the additional runtime detection.

[ostap, 2013-12-30 16:36:49]

On 2013/12/26 19:02:38, willchan wrote:
>
https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h#newc...
> base/sys_utils.h:15: // Returns true iff this is a low-end device.
> This is too vague. It was kinda-OK when it was just Android, but for other
> platforms? What does "low-end" mean?

Currently it is simple threshold of 512MB of RAM. Everything of 512MB and below
is considered low end (as it taken from Android).
Nothing prevents embedder to implement other low end device detection.
Also, command line switches could be used to enable low end device mode.

This patch is also reduces amount of ANDROID ifdefs and makes it possible to
develop and test some related code on linux desktop.

>
https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.h#newc...
> base/sys_utils.h:19: static size_t AmountOfPhysicalMemoryKB();
> Why does this even exist? Why aren't we using SysInfo? How is SysUtils
supposed
> to be different from SysInfo?

I think also that it is pretty strange to have separate SysUtils class just for
LowEndDevice thing. Do you think it is good idea to merge LowEndDevice detection
into SysInfo?

[piman, 2014-01-08 04:41:50]

I'm not convinced the code/settings for low-end android devices has generally
been tested on low-end non-android devices. There's a lot of assumptions baked.

I would suggest starting with making these changes behind a flag. Right now,
they're enabled by default.

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.cc
File base/sys_utils.cc (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/base/sys_utils.cc#new...
base/sys_utils.cc:37: return SysInfo::AmountOfPhysicalMemory() / 1024;
I'm not sure SysInfo::AmountOfPhysicalMemory() works in the renderer / in the
sandbox.

https://chromiumcodereview.appspot.com/119493005/diff/1/content/renderer/rend...
File content/renderer/render_widget.cc (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/content/renderer/rend...
content/renderer/render_widget.cc:2855: max_transfer_buffer_usage_mb *
kBytesPerMegabyte;
This changes behavior for non-low-end devices too. (previously we were using the
defaults for mapped_memory_reclaim_limit).

https://chromiumcodereview.appspot.com/119493005/diff/1/content/renderer/rend...
File content/renderer/renderer_main.cc (right):

https://chromiumcodereview.appspot.com/119493005/diff/1/content/renderer/rend...
content/renderer/renderer_main.cc:140:
SkGraphics::SetFontCacheLimit(font_cache_limit);
This changes behavior for non-low-end devices too. (previously we were not
setting the font cache limit).

[ostap, 2014-01-20 08:12:23]

https://codereview.chromium.org/119493005/diff/1/base/android/sys_utils_andro...
File base/android/sys_utils_android.cc (right):

https://codereview.chromium.org/119493005/diff/1/base/android/sys_utils_andro...
base/android/sys_utils_android.cc:26: static bool is_low_end =
android::SysUtilsAndroid::IsLowEndDeviceFromJni();
On 2013/12/26 19:02:39, willchan wrote:
> This is racy too

Done.

https://codereview.chromium.org/119493005/diff/1/base/sys_utils.cc
File base/sys_utils.cc (right):

https://codereview.chromium.org/119493005/diff/1/base/sys_utils.cc#newcode25
base/sys_utils.cc:25: int ramSizeKB = SysUtils::AmountOfPhysicalMemoryKB();
On 2013/12/26 19:02:39, willchan wrote:
> ram_size_kb

Done.

https://codereview.chromium.org/119493005/diff/1/base/sys_utils.cc#newcode31
base/sys_utils.cc:31: static bool isLowEndDevice = detectLowEndDevice();
On 2013/12/26 19:02:39, willchan wrote:
> We disable the compiler generated locks around function local statics. This is
> racy. You should use LazyInstance to fix the race.

Done.

https://codereview.chromium.org/119493005/diff/1/base/sys_utils.cc#newcode37
base/sys_utils.cc:37: return SysInfo::AmountOfPhysicalMemory() / 1024;
On 2014/01/08 04:41:50, piman wrote:
> I'm not sure SysInfo::AmountOfPhysicalMemory() works in the renderer / in the
> sandbox.

Should work.
At least initializeV8Common() in WebKit/Source/bindings/v8/V8Initializer.cpp
calls blink::Platform::current()->physicalMemoryMB() which calls
SysInfo::AmountOfPhysicalMemoryMB() .

https://codereview.chromium.org/119493005/diff/1/content/renderer/render_widg...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/119493005/diff/1/content/renderer/render_widg...
content/renderer/render_widget.cc:2855: max_transfer_buffer_usage_mb *
kBytesPerMegabyte;
On 2014/01/08 04:41:50, piman wrote:
> This changes behavior for non-low-end devices too. (previously we were using
the
> defaults for mapped_memory_reclaim_limit).

Done.

https://codereview.chromium.org/119493005/diff/1/content/renderer/renderer_ma...
File content/renderer/renderer_main.cc (right):

https://codereview.chromium.org/119493005/diff/1/content/renderer/renderer_ma...
content/renderer/renderer_main.cc:140:
SkGraphics::SetFontCacheLimit(font_cache_limit);
On 2014/01/08 04:41:50, piman wrote:
> This changes behavior for non-low-end devices too. (previously we were not
> setting the font cache limit).

Done.

[piman, 2014-01-21 21:23:49]

https://codereview.chromium.org/119493005/diff/310001/base/sys_utils.cc
File base/sys_utils.cc (right):

https://codereview.chromium.org/119493005/diff/310001/base/sys_utils.cc#newco...
base/sys_utils.cc:15: static const int LOW_MEMORY_DEVICE_THRESHOLD_MB = 512;
nit: kLowMemoryDeviceThresholdMB.

https://codereview.chromium.org/119493005/diff/310001/base/sys_utils.cc#newco...
base/sys_utils.cc:25: #if defined(ENABLE_LOW_END_DEVICE_DETECTION)
Command-line flag instead of compile-time flag?

https://codereview.chromium.org/119493005/diff/310001/content/renderer/render...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/119493005/diff/310001/content/renderer/render...
content/renderer/render_widget.cc:2862: if (base::SysUtils::IsLowEndDevice())
needs braces

[gavinp, 2014-01-22 00:35:52]

chrome/browser/prerender LGTM (mostly a rubber stamp)

[kbalazs, 2014-06-11 17:01:38]

On 2014/01/22 00:35:52, gavinp wrote:
> chrome/browser/prerender LGTM (mostly a rubber stamp)

FYI this is continuing in https://codereview.chromium.org/258663002/