Handle mac trackpad zoom via GesturePinch events

Handle mac trackpad zoom via GesturePinch events

Prior to this change, Mac handles pinch gestures directly in the UI layer (BrowserWindowController).  Here we instead have the RenderWidgetHost send GesturePinchUpdate events, and when those events go unhandled WebContentsImpl implements the browser zoom behavior (similar to how it does already for ctrl+mousewheel events on other platforms).

This lays the groundwork in the short term for giving the web page a chance to override the pinch behavior.  Longer term this will enable us to hook into the pinch-zoom codepath used for touchscreen so that we can get a similar effect (instead of just browser-zoom - which probably isn't really what the user wants when pinching).

The only observable behavior change in this CL is that pinching outside the content area (eg. in the tabstrip) no longer does anything.  It's not clear what the pinch origin should be in such a case, and it probably doesn't really make sense to try to handle it (pinching is supposed to zoom whatever is under the cursor).

This adds some basic unit test coverage for the pre-existing (but untested) logic for mapping pinch scales to zoom in/out actions.

Depends on blink CL: https://src.chromium.org/viewvc/blink?view=rev&revision=168482

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

[Rick Byers, 2014-03-26 21:01:58]

Hey Chris,
Can you please take a look at this change?  Mostly this just moves around the
existing code for mac pinch-zoom - making it more consistent with mousewheel
zoom on other platforms.

Thanks,
   Rick

[ccameron, 2014-03-26 21:07:21]

On 2014/03/26 21:01:58, Rick Byers wrote:
> Hey Chris,
> Can you please take a look at this change?  Mostly this just moves around the
> existing code for mac pinch-zoom - making it more consistent with mousewheel
> zoom on other platforms.
> 
> Thanks,
>    Rick

Sure -- taking a look.

[ccameron, 2014-03-26 23:02:23]

This lgtm (mod the rounding issues the Win bots hit).

I'm not the person to lg the behavior change though (not sure who is), so make
sure that's okay with the appropriate people.

[jdduke (slow), 2014-03-26 23:13:42]

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/gesture_event_queue.h (right):

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/gesture_event_queue.h:73: bool
ShouldForward(const GestureEventWithLatencyInfo&);
I've had a refactoring patch, for the longest time, that changes this to
|QueueEvent()|... this |ShouldForward| business is just confusing.

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/input_router_impl.cc:248: if
(gesture_event.event.sourceDevice == WebGestureEvent::Touchpad &&
I always hesitate to ack an event out-of-order.  Would it kill us to ack the
pinch *after* it's passed through the GEQ just before it's to be sent to the
renderer?  

Hmm, actually, a (perhaps better) alternative here would be to leverage the
|RenderWidgetHostView*::FilterInputEvent()| method, in this case we would have
|RenderWidgetHostViewMac| intercept the pinch and return
INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, and we wouldn't need to touch anything
here in the router.

[Rick Byers, 2014-03-27 01:44:09]

On 2014/03/26 23:02:23, ccameron1 wrote:
> This lgtm (mod the rounding issues the Win bots hit).

Thanks!  Rounding issue fixed.

> I'm not the person to lg the behavior change though (not sure who is), so make
> sure that's okay with the appropriate people.

I verified with glen@ that he's OK with the behavior change.

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/gesture_event_queue.h (right):

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/gesture_event_queue.h:73: bool
ShouldForward(const GestureEventWithLatencyInfo&);
On 2014/03/26 23:13:43, jdduke wrote:
> I've had a refactoring patch, for the longest time, that changes this to
> |QueueEvent()|... this |ShouldForward| business is just confusing.

Yeah, I get confused by this every time and have to keep reminding myself about
it.  Cleaning this up sounds great!

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/181723006/diff/40001/content/browser/renderer...
content/browser/renderer_host/input/input_router_impl.cc:248: if
(gesture_event.event.sourceDevice == WebGestureEvent::Touchpad &&
On 2014/03/26 23:13:43, jdduke wrote:
> I always hesitate to ack an event out-of-order.  Would it kill us to ack the
> pinch *after* it's passed through the GEQ just before it's to be sent to the
> renderer?  

Yeah that's probably better - done.

> Hmm, actually, a (perhaps better) alternative here would be to leverage the
> |RenderWidgetHostView*::FilterInputEvent()| method, in this case we would have
> |RenderWidgetHostViewMac| intercept the pinch and return
> INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, and we wouldn't need to touch
anything
> here in the router.

My next CL will involve turning these gestures into ctrl+mousewheel events, and
I _think_ the InputRouter (somewhere) is probably the right place to do that. 
In particular I'm going to need to hook things up so that when the SPECIFIC
wheel event ACK comes back from the renderer, it results in an ACK being sent
for the GesturePinchUpdate.  I haven't figured out exactly how I'm going to do
this yet, but I think it'll involve changes here.  So I'd rather land this CL
like this, then we can debate exactly that structure in the next CL (probably in
a week or two after I get through some other things).  Sound good?

[jdduke (slow), 2014-03-27 02:08:01]

content/browser/renderer_host/input lgtm.

https://codereview.chromium.org/181723006/diff/50001/content/browser/renderer...
File content/browser/renderer_host/input/input_router_impl.cc (right):

https://codereview.chromium.org/181723006/diff/50001/content/browser/renderer...
content/browser/renderer_host/input/input_router_impl.cc:423: // Trackpad pinch
gestures are not yet handled by the renderer.
Is that true? The compositor currently doesn't distinguish between source types
when dispatching GesturePinchUpdate
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/i...).
 However, maybe that's simply because we don't have any platforms (yet) that
provide such gestures via the trackpad?

As long as this is the behavior we want on *all* platforms, the router is
probably as good as any place for this.

[Rick Byers, 2014-03-27 02:20:02]

On 2014/03/27 02:08:01, jdduke wrote:
> content/browser/renderer_host/input lgtm.
> 
>
https://codereview.chromium.org/181723006/diff/50001/content/browser/renderer...
> File content/browser/renderer_host/input/input_router_impl.cc (right):
> 
>
https://codereview.chromium.org/181723006/diff/50001/content/browser/renderer...
> content/browser/renderer_host/input/input_router_impl.cc:423: // Trackpad
pinch
> gestures are not yet handled by the renderer.
> Is that true? The compositor currently doesn't distinguish between source
types
> when dispatching GesturePinchUpdate
>
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/i...).
>  However, maybe that's simply because we don't have any platforms (yet) that
> provide such gestures via the trackpad?

Right - it's never received them before, and wouldn't handle them properly if I
just sent them through (eg. these ones aren't bracketed by scroll or even
PinchBegin/PinchEnd events).

> As long as this is the behavior we want on *all* platforms, the router is
> probably as good as any place for this.

Well we'll eventually want common behavior for all platforms with trackpads (so
presumably not Android).  First step is Mac, then (hopefully still in M36)
Windows (which also has trackpad wired up to browser zoom today).  We could add
ChromeOS (which doesn't have pinch plumbed through at all right now), but I
suspect we'll wait and do that at the same time as full trackpad pinch-zoom
rather than enable browser zoom at all there.

[Rick Byers, 2014-03-27 02:21:33]

+jochen for OWNERS of content/browser/web_contents/ and rubber stamp for
deletions from chrome/browser/ui/cocoa/

[jochen (gone - plz use gerrit), 2014-03-27 14:24:24]

lgtm

[Rick Byers, 2014-03-27 14:31:18]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-27 14:31:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/181723006/50001

[commit-bot: I haz the power, 2014-03-27 14:54:58]

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

[commit-bot: I haz the power, 2014-03-27 14:54:58]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg

[Rick Byers, 2014-03-27 15:37:54]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-27 15:38:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/181723006/50001

[commit-bot: I haz the power, 2014-03-27 15:55:28]

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

[commit-bot: I haz the power, 2014-03-27 15:55:28]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg

[Rick Byers, 2014-03-27 23:10:44]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-27 23:12:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/181723006/50001

[Rick Byers, 2014-03-28 21:28:54]

The CQ bit was unchecked by rbyers@chromium.org

[Rick Byers, 2014-03-28 21:29:03]

The CQ bit was checked by rbyers@chromium.org

[cmp, 2014-03-30 17:00:14]

The CQ bit was unchecked by cmp@chromium.org

[cmp, 2014-03-30 17:02:26]

The CQ bit was checked by cmp@chromium.org

[cmp, 2014-03-30 17:03:46]

The CQ bit was unchecked by cmp@chromium.org

[cmp, 2014-03-30 17:07:26]

The CQ bit was checked by cmp@chromium.org

[commit-bot: I haz the power, 2014-03-30 17:08:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/181723006/50001

[commit-bot: I haz the power, 2014-03-30 17:37:36]

Change committed as 260452