Add support for touch-action: manipulation

Add support for touch-action: manipulation

"manipulation" is used to disable double-tap-zoom without also disabling pinch-zoom.

Depends on blink CL:  http://src.chromium.org/viewvc/blink?view=rev&rev=168752

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

[Rick Byers, 2014-03-12 15:34:18]

Jared, please review

[jdduke (slow), 2014-03-12 17:07:00]

lgtm

[jdduke (slow), 2014-03-12 17:08:24]

Actually, I think there's a little more work needed here.

https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/input/touch_action_filter.cc (right):

https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/input/touch_action_filter.cc:176: return
!(allowed_touch_action_ & TOUCH_ACTION_PAN_Y);
Hmm, I think you need to allow the GestureScroll{Begin,End} through for
PINCH_ZOOM to work.

[jdduke (slow), 2014-03-12 17:09:49]

https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/input/touch_action_filter_unittest.cc
(right):

https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/input/touch_action_filter_unittest.cc:418: //
Pinch is allowed with touch-action: manipulation.
Please add an explicit test for PINCH_ZOOM, ensuring that GSB/GSE are allowed,
as well as pinch events, but I guess GSU's are dropped?

[Rick Byers, 2014-03-12 21:03:38]

On 2014/03/12 17:09:49, jdduke wrote:
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> File content/browser/renderer_host/input/touch_action_filter_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> content/browser/renderer_host/input/touch_action_filter_unittest.cc:418: //
> Pinch is allowed with touch-action: manipulation.
> Please add an explicit test for PINCH_ZOOM, ensuring that GSB/GSE are allowed,
> as well as pinch events, but I guess GSU's are dropped?

TOUCH_ACTION_PINCH_ZOOM isn't supported without also PAN_X | PAN_Y (i.e.
MANIPULATION is the only valid combination that includes PINCH_ZOOM).  See the
comment on line 66.  I added some DHECKs to SetTouchAction to enforce this.  So
I believe I'm testing all the valid (non-DCHECK-triggering) combinations now.

[jdduke (slow), 2014-03-12 21:09:37]

On 2014/03/12 21:03:38, Rick Byers wrote:
> On 2014/03/12 17:09:49, jdduke wrote:
> >
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> > File content/browser/renderer_host/input/touch_action_filter_unittest.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> > content/browser/renderer_host/input/touch_action_filter_unittest.cc:418: //
> > Pinch is allowed with touch-action: manipulation.
> > Please add an explicit test for PINCH_ZOOM, ensuring that GSB/GSE are
allowed,
> > as well as pinch events, but I guess GSU's are dropped?
> 
> TOUCH_ACTION_PINCH_ZOOM isn't supported without also PAN_X | PAN_Y (i.e.
> MANIPULATION is the only valid combination that includes PINCH_ZOOM).  See the
> comment on line 66.  I added some DHECKs to SetTouchAction to enforce this. 
So
> I believe I'm testing all the valid (non-DCHECK-triggering) combinations now.

Makes sense, lgtm.

[Rick Byers, 2014-03-12 21:13:51]

On 2014/03/12 21:09:37, jdduke wrote:
> On 2014/03/12 21:03:38, Rick Byers wrote:
> > On 2014/03/12 17:09:49, jdduke wrote:
> > >
> >
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> > > File content/browser/renderer_host/input/touch_action_filter_unittest.cc
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> > > content/browser/renderer_host/input/touch_action_filter_unittest.cc:418:
//
> > > Pinch is allowed with touch-action: manipulation.
> > > Please add an explicit test for PINCH_ZOOM, ensuring that GSB/GSE are
> allowed,
> > > as well as pinch events, but I guess GSU's are dropped?
> > 
> > TOUCH_ACTION_PINCH_ZOOM isn't supported without also PAN_X | PAN_Y (i.e.
> > MANIPULATION is the only valid combination that includes PINCH_ZOOM).  See
the
> > comment on line 66.  I added some DHECKs to SetTouchAction to enforce this. 
> So
> > I believe I'm testing all the valid (non-DCHECK-triggering) combinations
now.
> 
> Makes sense, lgtm.

Actually, since this comes from a (potentially compromised) renderer I think a
CHECK is better than a DCHECK.  Also it's worth checking again after Intersect
in case it gets more complicated in the future and intersection doesn't so
obviously preserve the validity property.  Sorry I didn't do this in the last
patch.  Still look ok?

[Rick Byers, 2014-03-12 21:48:00]

On 2014/03/12 21:13:51, Rick Byers wrote:
> On 2014/03/12 21:09:37, jdduke wrote:
> > On 2014/03/12 21:03:38, Rick Byers wrote:
> > > On 2014/03/12 17:09:49, jdduke wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> > > > File content/browser/renderer_host/input/touch_action_filter_unittest.cc
> > > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/197273003/diff/1/content/browser/renderer_hos...
> > > > content/browser/renderer_host/input/touch_action_filter_unittest.cc:418:
> //
> > > > Pinch is allowed with touch-action: manipulation.
> > > > Please add an explicit test for PINCH_ZOOM, ensuring that GSB/GSE are
> > allowed,
> > > > as well as pinch events, but I guess GSU's are dropped?
> > > 
> > > TOUCH_ACTION_PINCH_ZOOM isn't supported without also PAN_X | PAN_Y (i.e.
> > > MANIPULATION is the only valid combination that includes PINCH_ZOOM).  See
> the
> > > comment on line 66.  I added some DHECKs to SetTouchAction to enforce
this. 
> > So
> > > I believe I'm testing all the valid (non-DCHECK-triggering) combinations
> now.
> > 
> > Makes sense, lgtm.
> 
> Actually, since this comes from a (potentially compromised) renderer I think a
> CHECK is better than a DCHECK.  Also it's worth checking again after Intersect
> in case it gets more complicated in the future and intersection doesn't so
> obviously preserve the validity property.  Sorry I didn't do this in the last
> patch.  Still look ok?

As discussed offline, I've moved the validation to the IPC system.  I've left
one simple DCHECK in (eg. for the unit test case where we don't go through an
IPC message).

[jdduke (slow), 2014-03-12 21:52:23]

On 2014/03/12 21:48:00, Rick Byers wrote:
> As discussed offline, I've moved the validation to the IPC system.  I've left
> one simple DCHECK in (eg. for the unit test case where we don't go through an
> IPC message).

Thanks, content/browser/renderer_host/input and content/common/input still lgtm.

[Rick Byers, 2014-03-12 21:59:30]

Thanks

+kenrb for OWNER in content/common/input_messages.h

+jochen for trivial OWNERs review of content/renderer/render_widget.cc

[kenrb, 2014-03-12 22:14:00]

lgtm

[jochen (gone - plz use gerrit), 2014-03-13 08:20:48]

lgtm

[jochen (gone - plz use gerrit), 2014-03-13 08:20:50]

lgtm

lgtm

[Rick Byers, 2014-03-13 14:08:52]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-13 14:09:11]

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

[commit-bot: I haz the power, 2014-03-13 14:36:52]

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

[commit-bot: I haz the power, 2014-03-13 14:36:52]

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

[Rick Byers, 2014-03-13 14:50:41]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-13 14:51:10]

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

[commit-bot: I haz the power, 2014-03-13 19:36:20]

Change committed as 256885