Simplify pinch flags.

Simplify pinch flags.

At present, we have two flags to enable two different pinch models:

enable-pinch => enables Android style zoom
enable-pinch + enable-pinch-in-compositor => enables chrome linux/win accelerated zoom

This CL rationalizes the flags so that

enable-css-transform-pinch => enables Android style zoom
enable-pinch => enables chrome linux/win accelerated zoom

and the latter can be controlled via about:flags.

BUG=
NOTRY=TRUE


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

[wjmaclean, 2012-12-03 16:03:47]

Alexandre - before I mark this for review by other OWNERS, can you verify I've
got the right behaviour for Android? I've tested it on Linux desktop thus far.

[aelias_OOO_until_Jul13, 2012-12-03 18:34:35]

https://codereview.chromium.org/11348353/diff/1/content/renderer/render_view_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/11348353/diff/1/content/renderer/render_view_...
content/renderer/render_view_impl.cc:3180: bool enable_pinch = enable_viewport
||
This logic is broken because enable_viewport is also true on Android.

In general this whole method has accumulated a lot of cruft, please simplify it
as follows:

{
bool enable_viewport = command_line.HasSwitch(...);
bool enable_pinch = command_line.HasSwitch(...);

if (enable_viewport)
  return;

if (enable_pinch)
  webview()->setPageScaleFactorLimits(1, 4);
else
  webview()->setPageScaleFactorLimits(1, 1);
}

(I'm phrasing it this way because viewport tag support is the real reason we
don't need to make this call on Android.)

[wjmaclean, 2012-12-04 18:21:15]

Thanks Alexandre, I wasn't sure about the enable_viewport interaction. Will
upload a new patch momentarily.

On 2012/12/03 18:34:35, aelias wrote:
>
https://codereview.chromium.org/11348353/diff/1/content/renderer/render_view_...
> File content/renderer/render_view_impl.cc (right):
> 
>
https://codereview.chromium.org/11348353/diff/1/content/renderer/render_view_...
> content/renderer/render_view_impl.cc:3180: bool enable_pinch = enable_viewport
> ||
> This logic is broken because enable_viewport is also true on Android.
> 
> In general this whole method has accumulated a lot of cruft, please simplify
it
> as follows:
> 
> {
> bool enable_viewport = command_line.HasSwitch(...);
> bool enable_pinch = command_line.HasSwitch(...);
> 
> if (enable_viewport)
>   return;
> 
> if (enable_pinch)
>   webview()->setPageScaleFactorLimits(1, 4);
> else
>   webview()->setPageScaleFactorLimits(1, 1);
> }
> 
> (I'm phrasing it this way because viewport tag support is the real reason we
> don't need to make this call on Android.)

[wjmaclean, 2012-12-04 19:31:34]

Updated, please take a look?

[aelias_OOO_until_Jul13, 2012-12-04 20:30:38]

lgtm except I'm thinking we should also add "if
(!webview()->isAcceleratedCompositingActive()) return;" to the render_view_impl
function, in case something weird happens in the software case.

[wjmaclean, 2012-12-04 21:02:27]

On 2012/12/04 20:30:38, aelias wrote:
> lgtm except I'm thinking we should also add "if
> (!webview()->isAcceleratedCompositingActive()) return;" to the
render_view_impl
> function, in case something weird happens in the software case.

Done! Thanks.

[aelias_OOO_until_Jul13, 2012-12-04 21:32:23]

lgtm

[wjmaclean, 2012-12-04 22:15:56]

On 2012/12/04 21:32:23, aelias wrote:
> lgtm

+enne as cc OWNER

[enne (OOO), 2012-12-05 17:19:11]

lgtm

[darin (slow to review), 2012-12-05 17:50:59]

LGTM

[commit-bot: I haz the power, 2012-12-05 17:54:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/11348353/5002

[commit-bot: I haz the power, 2012-12-05 20:41:26]

Retried try job too often on linux_rel for step(s) content_unittests

[commit-bot: I haz the power, 2012-12-05 20:46:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/11348353/5002

[commit-bot: I haz the power, 2012-12-05 21:06:07]

Change committed as 171309