Define a --force-gpu-rasterization flag

Define a --force-gpu-rasterization flag

This flag forces gpu rasterization for all layers (that is, it behaves the
way --enable-gpu-rasterization behaved previously).

--enable-gpu-rasterization now enables gpu rasterization only on layers
that have a GPU rasterization hint. Since this hint will be added in a
later CL, this flag is currently a no-op.

BUG=329722
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256143

[ajuma, 2014-03-08 01:20:13]

This is split off from https://codereview.chromium.org/181063022/, and uses
enable/disable/force flags instead of a single rasterization-site flag.

[piman, 2014-03-08 01:22:21]

LGTM+nits

https://codereview.chromium.org/189883009/diff/40001/content/public/common/co...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/189883009/diff/40001/content/public/common/co...
content/public/common/content_switches.cc:466: const char
kEnableGpuRasterization[]        = "-enable-gpu-rasterization";
nit: remove leading -

https://codereview.chromium.org/189883009/diff/40001/content/public/common/co...
content/public/common/content_switches.cc:652: const char
kForceGpuRasterization[]        = "-force-gpu-rasterization";
nit: remove leading -

[ajuma, 2014-03-08 01:25:25]

https://codereview.chromium.org/189883009/diff/40001/content/public/common/co...
File content/public/common/content_switches.cc (right):

https://codereview.chromium.org/189883009/diff/40001/content/public/common/co...
content/public/common/content_switches.cc:466: const char
kEnableGpuRasterization[]        = "-enable-gpu-rasterization";
On 2014/03/08 01:22:21, piman wrote:
> nit: remove leading -

Done.

https://codereview.chromium.org/189883009/diff/40001/content/public/common/co...
content/public/common/content_switches.cc:652: const char
kForceGpuRasterization[]        = "-force-gpu-rasterization";
On 2014/03/08 01:22:21, piman wrote:
> nit: remove leading -

Done.

[nduca, 2014-03-08 04:18:01]

lgtm thank you!!

https://codereview.chromium.org/189883009/diff/70001/cc/trees/layer_tree_sett...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/189883009/diff/70001/cc/trees/layer_tree_sett...
cc/trees/layer_tree_settings.h:35: GpuRasterizationEnabled,
Once you get into cc, you can use sw/hw/forced so that its clear what mode we're
in. That part of the previous review discussion seemed solid.

[ernstm, 2014-03-08 05:13:53]

https://codereview.chromium.org/189883009/diff/70001/cc/trees/layer_tree_sett...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/189883009/diff/70001/cc/trees/layer_tree_sett...
cc/trees/layer_tree_settings.h:35: GpuRasterizationEnabled,
On 2014/03/08 04:18:02, nduca wrote:
> Once you get into cc, you can use sw/hw/forced so that its clear what mode
we're
> in. That part of the previous review discussion seemed solid.

In the previous review we settled on cpu/hybrid/gpu.

[ajuma, 2014-03-08 19:04:02]

https://codereview.chromium.org/189883009/diff/70001/cc/trees/layer_tree_sett...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/189883009/diff/70001/cc/trees/layer_tree_sett...
cc/trees/layer_tree_settings.h:35: GpuRasterizationEnabled,
On 2014/03/08 05:13:53, ernstm wrote:
> On 2014/03/08 04:18:02, nduca wrote:
> > Once you get into cc, you can use sw/hw/forced so that its clear what mode
> we're
> > in. That part of the previous review discussion seemed solid.
> 
> In the previous review we settled on cpu/hybrid/gpu.

Switched cc back to using cpu/hybrid/gpu.

[ajuma, 2014-03-10 14:17:54]

TBRing boliu for mechanical change to android_webview/ (replacing "cc::switches"
with "switches").

[ajuma, 2014-03-10 14:18:03]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2014-03-10 14:30:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/100001

[boliu, 2014-03-10 15:40:38]

android_webview lgtm

And one question about default value of impl-side-painting on android

https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
content/renderer/render_thread_impl.cc:397:
!command_line.HasSwitch(switches::kDisableImplSidePainting);
impl_side_painting was enabled on android if no switch is defined, does this
reverse the default on android?

[ajuma, 2014-03-10 15:53:55]

https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
content/renderer/render_thread_impl.cc:397:
!command_line.HasSwitch(switches::kDisableImplSidePainting);
On 2014/03/10 15:40:38, boliu wrote:
> impl_side_painting was enabled on android if no switch is defined, does this
> reverse the default on android?

The Android default is now handled in IsImplSidePaintingEnabled() in
content/browser/gpu/compositor_util.cc, which causes kEnableImplSidePainting to
get added to the renderer command line in AppendGpuCommandLineFlags() in
content/browser/renderer_host/render_process_host_impl.cc. So by the time the
renderer performs the check here, the switch should be defined.

[commit-bot: I haz the power, 2014-03-10 15:54:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/100001

[ajuma, 2014-03-10 15:59:34]

https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
content/renderer/render_thread_impl.cc:397:
!command_line.HasSwitch(switches::kDisableImplSidePainting);
On 2014/03/10 15:53:56, ajuma wrote:
> On 2014/03/10 15:40:38, boliu wrote:
> > impl_side_painting was enabled on android if no switch is defined, does this
> > reverse the default on android?
> 
> The Android default is now handled in IsImplSidePaintingEnabled() in
> content/browser/gpu/compositor_util.cc, which causes kEnableImplSidePainting
to
> get added to the renderer command line in AppendGpuCommandLineFlags() in
> content/browser/renderer_host/render_process_host_impl.cc. So by the time the
> renderer performs the check here, the switch should be defined.

Actually, this code path will only get followed for out-of-process renderers. Do
we use in-process renderers anywhere on Android (e.g. for webview)?

[ajuma, 2014-03-10 15:59:40]

The CQ bit was unchecked by ajuma@chromium.org

[boliu, 2014-03-10 16:02:30]

On 2014/03/10 15:53:55, ajuma wrote:
>
https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
> File content/renderer/render_thread_impl.cc (right):
> 
>
https://codereview.chromium.org/189883009/diff/100001/content/renderer/render...
> content/renderer/render_thread_impl.cc:397:
> !command_line.HasSwitch(switches::kDisableImplSidePainting);
> On 2014/03/10 15:40:38, boliu wrote:
> > impl_side_painting was enabled on android if no switch is defined, does this
> > reverse the default on android?
> 
> The Android default is now handled in IsImplSidePaintingEnabled() in
> content/browser/gpu/compositor_util.cc, which causes kEnableImplSidePainting
to
> get added to the renderer command line in AppendGpuCommandLineFlags() in
> content/browser/renderer_host/render_process_host_impl.cc. So by the time the
> renderer performs the check here, the switch should be defined.

Ok thanks for clarifying.

From cs, looks like it's going to be a problem for android webview which runs in
single-process mode. I think AppendGpuCommandLineFlags is never called in
single-process mode. I guess easiest way is just add it to aw_main_delegate.cc?

[ajuma, 2014-03-10 16:41:49]

boliu, PTAL.

> From cs, looks like it's going to be a problem for android webview which runs
in
> single-process mode. I think AppendGpuCommandLineFlags is never called in
> single-process mode. I guess easiest way is just add it to
aw_main_delegate.cc?

Added it to aw_main_delegate.cc.

[boliu, 2014-03-10 16:43:13]

lgtm

[ajuma, 2014-03-10 16:43:50]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2014-03-10 17:41:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/120001

[commit-bot: I haz the power, 2014-03-10 19:47:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-10 19:47:51]

Try jobs failed on following builders: linux_chromium_rel

[ajuma, 2014-03-10 19:48:57]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2014-03-10 19:53:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/120001

[commit-bot: I haz the power, 2014-03-10 21:42:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/120001

[commit-bot: I haz the power, 2014-03-10 22:21:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/120001

[ajuma, 2014-03-10 23:20:03]

piman, PTAL.

Since RenderThreadImpl::current() can be null in during tests, I've added null
checks to render_view_impl.cc. (I haven't added null checks to
render_widget_compositor.cc, since it seems this is never used during tests
where RenderThreadImpl::current() is null -- for example, 
RenderThreadImpl::current() is used without a null check in
RenderWidgetCompositor::Initialize).

[piman, 2014-03-10 23:21:03]

lgtm

[ajuma, 2014-03-10 23:38:28]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2014-03-11 00:19:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/189883009/140001

[commit-bot: I haz the power, 2014-03-11 06:04:18]

Change committed as 256143