Make Mac pinch thresholding apply only to zoom (Blink side)

Make Mac pinch thresholding apply only to zoom

Add a zoomDisabled flag (disabled so that it will default to false
everywhere except where we set it) to WebGestureEvent.

Note that the equivalent blink::Event does not have a corresponding
gesture sub-type that sends pinch scale information, so the
zoomDisabled information is not passed on either.

Chromium side:
https://codereview.chromium.org/1129693002

BUG=478981
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=195011

[ccameron, 2015-05-06 03:48:31]

ccameron@chromium.org changed reviewers:
+ aelias@chromium.org, rbyers@chromium.org

[ccameron, 2015-05-06 03:48:32]

I was surprised to see there was no blink::Event support pinch scale. Because
that was not there, I didn't update blink::PlatformEvent or blink::Event (as I
did for rails in https://codereview.chromium.org/1018183002). Let me know if I'm
missing something here.

https://codereview.chromium.org/1127013002/diff/1/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1127013002/diff/1/Source/web/WebViewImpl.cpp#...
Source/web/WebViewImpl.cpp:2187: return false;
Testing for this requires event sender support from the Chromium side.

[Rick Byers, 2015-05-06 20:28:25]

On 2015/05/06 03:48:32, ccameron wrote:
> I was surprised to see there was no blink::Event support pinch scale. Because
> that was not there, I didn't update blink::PlatformEvent or blink::Event (as I
> did for rails in https://codereview.chromium.org/1018183002). Let me know if
I'm
> missing something here.
> 
> https://codereview.chromium.org/1127013002/diff/1/Source/web/WebViewImpl.cpp
> File Source/web/WebViewImpl.cpp (right):
> 
>
https://codereview.chromium.org/1127013002/diff/1/Source/web/WebViewImpl.cpp#...
> Source/web/WebViewImpl.cpp:2187: return false;
> Testing for this requires event sender support from the Chromium side.

This basic approach is OK with me if we decide we need it.  But ideally pinch
zoom wouldn't need to be special in this way - let me know what you think of
https://code.google.com/p/chromium/issues/detail?id=478981#c21 first.

[Rick Byers, 2015-05-06 21:52:37]

On 2015/05/06 03:48:32, ccameron wrote:
> I was surprised to see there was no blink::Event support pinch scale. Because
> that was not there, I didn't update blink::PlatformEvent or blink::Event (as I
> did for rails in https://codereview.chromium.org/1018183002). Let me know if
I'm
> missing something here.

No, that makes sense.  Pinch is really entirely above blink core.  Even
<webview> tag ("browser plugin") doesn't need to pass pinch gestures down to the
guest AFAIK.

[Rick Byers, 2015-05-06 21:53:04]

LGTM with nit

Can you also please reference the chromium side CL in the CL description to tie
them together?

https://codereview.chromium.org/1127013002/diff/1/public/web/WebInputEvent.h
File public/web/WebInputEvent.h (right):

https://codereview.chromium.org/1127013002/diff/1/public/web/WebInputEvent.h#...
public/web/WebInputEvent.h:501: bool zoomDisabled;
nit: can we call this canZoom since it almost exactly parallels the
WebMouseWheelEvent::canScroll bit?  Also please add a comment saying why we need
this bit.

[ccameron, 2015-05-06 22:10:14]

wdyt WRT zoomDisabled?

https://codereview.chromium.org/1127013002/diff/1/public/web/WebInputEvent.h
File public/web/WebInputEvent.h (right):

https://codereview.chromium.org/1127013002/diff/1/public/web/WebInputEvent.h#...
public/web/WebInputEvent.h:501: bool zoomDisabled;
On 2015/05/06 21:53:04, Rick Byers wrote:
> nit: can we call this canZoom since it almost exactly parallels the
> WebMouseWheelEvent::canScroll bit?  Also please add a comment saying why we
need
> this bit.

I went with zoomDisabled so that the default value (the constructor memsets the
struct to 0) would match the default behavior. I can change it to canZoom, but
if I do, I'd like to update the constructor to initialize it to true then --
sound reasonable?

[Rick Byers, 2015-05-06 22:49:27]

On 2015/05/06 22:10:14, ccameron wrote:
> wdyt WRT zoomDisabled?
> 
> https://codereview.chromium.org/1127013002/diff/1/public/web/WebInputEvent.h
> File public/web/WebInputEvent.h (right):
> 
>
https://codereview.chromium.org/1127013002/diff/1/public/web/WebInputEvent.h#...
> public/web/WebInputEvent.h:501: bool zoomDisabled;
> On 2015/05/06 21:53:04, Rick Byers wrote:
> > nit: can we call this canZoom since it almost exactly parallels the
> > WebMouseWheelEvent::canScroll bit?  Also please add a comment saying why we
> need
> > this bit.
> 
> I went with zoomDisabled so that the default value (the constructor memsets
the
> struct to 0) would match the default behavior. I can change it to canZoom, but
> if I do, I'd like to update the constructor to initialize it to true then --
> sound reasonable?

We went through exactly the same debate with WebMouseWheelEvent::canScroll
(where I proposed 'disableScroll') and decided there was really no value to
false-initializing.

But in this case we have a union :-(, so I guess our options are limited.  Since
this is for M43 merge, let's keep it simple and stick with your zoomDisabled
plan.  Sorry I forgot about the stupid union...

[ccameron, 2015-05-06 22:57:36]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2015-05-06 22:57:36]

Thanks!

[commit-bot: I haz the power, 2015-05-06 22:57:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1127013002/1

[commit-bot: I haz the power, 2015-05-06 23:03:39]

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

[commit-bot: I haz the power, 2015-05-06 23:03:40]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)

[aelias_OOO_until_Jul13, 2015-05-06 23:17:02]

Source/web lgtm

[aelias_OOO_until_Jul13, 2015-05-06 23:17:17]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2015-05-06 23:18:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1127013002/1

[commit-bot: I haz the power, 2015-05-07 00:36:07]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195011