Fix for pinch virtual viewport flag wasn't fully enabled.

Fix for pinch virtual viewport flag wasn't fully enabled.

Missed a case when enabling the flag on CrOS in r266119.

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

[wjmaclean, 2014-04-25 15:00:00]

lgtm

[bokan, 2014-04-25 15:01:43]

piman@, forgot this in r266119, PTAL.

[piman, 2014-04-25 20:53:05]

https://codereview.chromium.org/257903002/diff/20001/content/renderer/gpu/ren...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/257903002/diff/20001/content/renderer/gpu/ren...
content/renderer/gpu/render_widget_compositor.cc:195: #endif
I would prefer if this kind of per-platform decision was in one place, in the
browser, rather than littered all over the code, with different logic - as this
CL shows, it's easy to miss one place and have the wrong behavior.

See for example what we do for IsThreadedCompositingEnabled or
IsDelegatedRendererEnabled or IsImplSidePaintingEnabled etc. in
content/browser/gpu/compositor_util.cc

We have one single place that makes the decision, and then this decision is used
on the browser side, and passed (as a command line flag, for lack of better way)
to the renderer. We can even test that logic.

I think we should be able to easily add a IsPinchVirtualViewportEnabled in
compositor_util.cc, with all the logic there, and use that information to pass a
single enable flag to the renderer (see render_process_host_impl.cc) and use
that flag to make all the decisions.

[bokan, 2014-04-25 22:07:24]

Thanks for the guidance, much better.

https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:943: if
(IsPinchVirtualViewportEnabled())
Not sure if this qualifies as a GPU command line (it's a compositor feature), is
this ok here or should I move it directly into AppendRendererCommandLine?

[bokan, 2014-04-25 22:07:56]

On 2014/04/25 22:07:24, bokan wrote:
> Thanks for the guidance, much better.
> 
>
https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
> content/browser/renderer_host/render_process_host_impl.cc:943: if
> (IsPinchVirtualViewportEnabled())
> Not sure if this qualifies as a GPU command line (it's a compositor feature),
is
> this ok here or should I move it directly into AppendRendererCommandLine?

I also took the opportunity to make the flag an enable/disable/default.

[piman, 2014-04-25 22:44:39]

lgtm

https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:943: if
(IsPinchVirtualViewportEnabled())
On 2014/04/25 22:07:25, bokan wrote:
> Not sure if this qualifies as a GPU command line (it's a compositor feature),
is
> this ok here or should I move it directly into AppendRendererCommandLine?

Maybe we can rename as AppendCompositorCommandLineFlags?

[bokan, 2014-04-25 23:03:33]

The CQ bit was checked by bokan@chromium.org

[bokan, 2014-04-25 23:03:46]

https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/257903002/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:943: if
(IsPinchVirtualViewportEnabled())
On 2014/04/25 22:44:40, piman wrote:
> On 2014/04/25 22:07:25, bokan wrote:
> > Not sure if this qualifies as a GPU command line (it's a compositor
feature),
> is
> > this ok here or should I move it directly into AppendRendererCommandLine?
> 
> Maybe we can rename as AppendCompositorCommandLineFlags?

Done.

[piman, 2014-04-25 23:05:43]

LGTM, thanks!

[commit-bot: I haz the power, 2014-04-25 23:15:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bokan@chromium.org/257903002/90001

[commit-bot: I haz the power, 2014-04-28 10:29:06]

Change committed as 266504