Suppressed MEs for gestures from cancelled PEs

Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
however, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs, when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a unique-touch-id queue in PointerEventManager
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938, 607588, 507408
Committed: https://crrev.com/1904837ebb6f8f352572e0f833e14640a0db6d54
Cr-Commit-Position: refs/heads/master@{#400264}

[mustaq, 2016-05-26 21:05:09]

Description was changed from

==========
Experimental patch: added WebGestureEvent.uniqueTouchEventId, to blink.

BUG=
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

BUG=607588,606938,507408
==========

[mustaq, 2016-05-26 21:46:00]

Patchset #3 (id:40001) has been deleted

[mustaq, 2016-05-26 22:11:32]

Description was changed from

==========
Suppressed MEs for gestures from cancelled PEs

BUG=607588,606938,507408
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points for which the
pointerevents have been canceled, to mimic the ME suppression behavior of
PointerEvents. In Edge, TEs are fired as-is without considering the canceling of
pointerdowns. We need to follow the Edge behavior here.

Moreover, since preventing a pointerdown suppresses MEs, this should be true for
the MEs from gestures that are ultimately triggered by TEs when the PEs
corresponding to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing touch-points for canceled
pointerevents) has a few bad side-effects:
- TE sequence can become inconsistent (eg touchend w/o toushstart) for certain
sequences of PE canceling (crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch scroll starts, which
suppresses PEs only to "reveal" a TE sequence from midway (crbug.com/507408).

This CL adds a primary-pointerdown-status queue in Blink to properly suppress
MEs from GestureTap, and makes TE firing independent of PE handling in JS. The
MEs from GestureLongPress are left as-is because suppressing them could break
drag-drop, will be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

[mustaq, 2016-05-26 22:15:01]

Description was changed from

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points for which the
pointerevents have been canceled, to mimic the ME suppression behavior of
PointerEvents. In Edge, TEs are fired as-is without considering the canceling of
pointerdowns. We need to follow the Edge behavior here.

Moreover, since preventing a pointerdown suppresses MEs, this should be true for
the MEs from gestures that are ultimately triggered by TEs when the PEs
corresponding to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing touch-points for canceled
pointerevents) has a few bad side-effects:
- TE sequence can become inconsistent (eg touchend w/o toushstart) for certain
sequences of PE canceling (crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch scroll starts, which
suppresses PEs only to "reveal" a TE sequence from midway (crbug.com/507408).

This CL adds a primary-pointerdown-status queue in Blink to properly suppress
MEs from GestureTap, and makes TE firing independent of PE handling in JS. The
MEs from GestureLongPress are left as-is because suppressing them could break
drag-drop, will be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
hoever, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a primary-pointerdown-status queue in Blink
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. The MEs from
GestureLongPress are left as-is because suppressing them
could break drag-drop; this will be done separately in a
follow-up CL.

BUG=606938,607588,507408
==========

[mustaq, 2016-05-26 22:16:05]

mustaq@chromium.org changed reviewers:
+ rbyers@chromium.org, tdresser@chromium.org

[mustaq, 2016-05-26 22:16:06]

ptal. Dropped the uniqueTouchId idea in favor of a simpler solution.

[tdresser, 2016-05-27 18:26:32]

LGTM, this is definitely cleaner.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:470: // firing for the
current gesure sequence (i.e. until next GestureTapDown).
gesure -> gesture.

[Rick Byers, 2016-05-27 20:39:27]

Would love to be proven wrong about my concern (or convinced it doesn't matter) 
:-)

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-mouse-expected.txt
(right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-mouse-expected.txt:18:
Received touchend
nit: add contextmenu to the list of events to watch (since it's really what
brackets the mousedown)

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/touch-pointer-mouse-expected.txt
(right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/touch-pointer-mouse-expected.txt:35:
Received mousemove
Your CL description says that you're still firing mousedown on GestureLongPress,
but I don't see it here...

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:2233: mouseDownEventResult
= handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown,
currentHitTest));
In addition to suppressing the generation of mouse events, I believe this change
also suppresses some/all of the default behavior associated with a tap (eg.
focusing an input field).  I'm pretty sure this is what you want (since
cancelling the pointerdown for a mouse would do the same), but there may be some
subtle cases here.  Anyway maybe just expand the CL description to describe this
broader impact, ideally also add at least a little test coverage (we really
should have interop tests for that sort of thing).

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:480: // single
touches only, it is enough to queue the
Not quite true - GestureTwoFingerTap behaves similarly (but not identically) to
LongPress on desktop IIRC.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:484:
m_primaryPointerdownResults.append(result);
Unfortunately I don't think we can currently guarantee a 1:1 correspondence
between touch and gesture events.  Eg:
 - If the page has no touch/pointer listeners at all, blink will never get the
WebTouchEvents (and so I think your take_first will fail because the queue is
empty).  Similarly, can CC touch hit testing still cause touch events to be
hidden completely from blink or did Dave change that?
 - The page may change between when the touch event was received and when the
gesture event was received so that they go to different frames.  Moving to a
page-wide PointerEventManager should fix this except for the OOPIF scenario

One option is to make the code more robust but still a heuristic (eg. identify
failure cases - unexpected empty queue, timeout with something left in the queue
a long time), do something reasonable but log an UMA counter so we know how
often these edge cases are happening (to decide if it's worth fixing).

Another option would be to include some sort of identifier in both the touch and
gesture events to correlate them (and then use a map, but still need some expiry
mechanism).

Or perhaps we just need to flow the information back with the touch ACK and
bring it back with the gesture event.  We discussed this when considering the
"TouchEvent.preventMouseEvents()" API and I think this was our preferred choice
at the time.  Not sure how hard it is though.

[dtapuska, 2016-05-27 20:54:06]

dtapuska@chromium.org changed reviewers:
+ dtapuska@chromium.org

[dtapuska, 2016-05-27 20:54:07]

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:2169:
m_suppressMouseEventsFromGestures =
I see there is an ASSERT in debug mode for the Deque having enough elements. But
what happens in release? Is it ever a scenario when we might not have things in
the deque?

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:484:
m_primaryPointerdownResults.append(result);
On 2016/05/27 20:39:27, Rick Byers wrote:
> Unfortunately I don't think we can currently guarantee a 1:1 correspondence
> between touch and gesture events.  Eg:
>  - If the page has no touch/pointer listeners at all, blink will never get the
> WebTouchEvents (and so I think your take_first will fail because the queue is
> empty).  Similarly, can CC touch hit testing still cause touch events to be
> hidden completely from blink or did Dave change that?
>  - The page may change between when the touch event was received and when the
> gesture event was received so that they go to different frames.  Moving to a
> page-wide PointerEventManager should fix this except for the OOPIF scenario
> 
> One option is to make the code more robust but still a heuristic (eg. identify
> failure cases - unexpected empty queue, timeout with something left in the
queue
> a long time), do something reasonable but log an UMA counter so we know how
> often these edge cases are happening (to decide if it's worth fixing).
> 
> Another option would be to include some sort of identifier in both the touch
and
> gesture events to correlate them (and then use a map, but still need some
expiry
> mechanism).
> 
> Or perhaps we just need to flow the information back with the touch ACK and
> bring it back with the gesture event.  We discussed this when considering the
> "TouchEvent.preventMouseEvents()" API and I think this was our preferred
choice
> at the time.  Not sure how hard it is though.

I'd prefer if the information went up the stack and then flowed back down as
opposed to this side channel of data. Side channels can get out of sync. Is the
tap targeted at the same iframe if something moved between the events?

[mustaq, 2016-05-30 15:14:26]

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-mouse-expected.txt
(right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-mouse-expected.txt:18:
Received touchend
On 2016/05/27 20:39:26, Rick Byers wrote:
> nit: add contextmenu to the list of events to watch (since it's really what
> brackets the mousedown)

Will do.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/touch-pointer-mouse-expected.txt
(right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/touch-pointer-mouse-expected.txt:35:
Received mousemove
On 2016/05/27 20:39:27, Rick Byers wrote:
> Your CL description says that you're still firing mousedown on
GestureLongPress,
> but I don't see it here...

Yes, I noticed it too, that's why I added the long-press tests in this CL: the
mousedown is gone from here as well as from the next block of test where the
primary pointerdown not even canceled. I will debug it today.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:480: // single
touches only, it is enough to queue the
On 2016/05/27 20:39:27, Rick Byers wrote:
> Not quite true - GestureTwoFingerTap behaves similarly (but not identically)
to
> LongPress on desktop IIRC.

I will update the comment to clarify it: TwoFingerTaps also send MEs but since
they fire no touchstarts so we should be fine here.

We should perhaps fire PEs for TwoFingerTaps? I will file a bug if this may be
the case.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:484:
m_primaryPointerdownResults.append(result);
On 2016/05/27 20:54:07, dtapuska wrote:
> On 2016/05/27 20:39:27, Rick Byers wrote:
> > Unfortunately I don't think we can currently guarantee a 1:1 correspondence
> > between touch and gesture events.  Eg:
> >  - If the page has no touch/pointer listeners at all, blink will never get
the
> > WebTouchEvents (and so I think your take_first will fail because the queue
is
> > empty).  Similarly, can CC touch hit testing still cause touch events to be
> > hidden completely from blink or did Dave change that?
> >  - The page may change between when the touch event was received and when
the
> > gesture event was received so that they go to different frames.  Moving to a
> > page-wide PointerEventManager should fix this except for the OOPIF scenario
> > 
> > One option is to make the code more robust but still a heuristic (eg.
identify
> > failure cases - unexpected empty queue, timeout with something left in the
> queue
> > a long time), do something reasonable but log an UMA counter so we know how
> > often these edge cases are happening (to decide if it's worth fixing).
> > 
> > Another option would be to include some sort of identifier in both the touch
> and
> > gesture events to correlate them (and then use a map, but still need some
> expiry
> > mechanism).
> > 
> > Or perhaps we just need to flow the information back with the touch ACK and
> > bring it back with the gesture event.  We discussed this when considering
the
> > "TouchEvent.preventMouseEvents()" API and I think this was our preferred
> choice
> > at the time.  Not sure how hard it is though.
> 
> I'd prefer if the information went up the stack and then flowed back down as
> opposed to this side channel of data. Side channels can get out of sync. Is
the
> tap targeted at the same iframe if something moved between the events?

Local sync seems unavoidable. I had uniqueTouchIds plumbed all the way down to
Blink (see Patch#2) but because of brittleness in ID handling in GR (missing ids
on timer-fired GEs, GestureTapUnconfirmed breaking tap ids etc), I had to rely
on TE-vs-GE bracketing behavior to sync the outcome in Blink again.

Flowing the info back to GR was my first attempt but I had to rule out the
option because:
(1) Touch acks are not even sent back for passive listeners, so we can't piggy
back.
(2) Sending an IPC back would make it prone to race conditions: what if the IPC
reaches GR when the GR is processing either previous or next GE.

There are two scenarios that may break the TE/GE braketing behavior I am relying
on:
- Too many GEs: easy case, will ignore empty queues.
- Too many TEs: can it happen except for the OOPIF case?

[mustaq, 2016-06-02 19:26:27]

ptal. In short, pulled the unique-touch-ids from TouchDispositionGestureFilter
to Blink to handle unpaired TE/GE sequences. The Blink-side bookkeeping still
needs to be there because of the "instability" in unique_touch_id generation.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-mouse-expected.txt
(right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-mouse-expected.txt:18:
Received touchend
On 2016/05/30 15:14:26, mustaq wrote:
> On 2016/05/27 20:39:26, Rick Byers wrote:
> > nit: add contextmenu to the list of events to watch (since it's really what
> > brackets the mousedown)
> 
> Will do.

The latest patch is id based, not relying on bracketing behavior. Moreover,
firing context menu events through eventSender needs some work. So I am skipping
'contextmenu', let me know if I am missing something.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/touch-pointer-mouse-expected.txt
(right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/touch-pointer-mouse-expected.txt:35:
Received mousemove
On 2016/05/30 15:14:26, mustaq wrote:
> On 2016/05/27 20:39:27, Rick Byers wrote:
> > Your CL description says that you're still firing mousedown on
> GestureLongPress,
> > but I don't see it here...
> 
> Yes, I noticed it too, that's why I added the long-press tests in this CL: the
> mousedown is gone from here as well as from the next block of test where the
> primary pointerdown not even canceled. I will debug it today.

The way I was sending TE+GE sequence for long-tap thorough eventSender doesn't
match the reality in some way, causing some inconsistent state in EventSender.
Chrome behaves perfectly when I mimic the tap+LongTap sequence manually. I will
figure out the correct TE+GE firing sequence in my followup (LongPress) CL.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:2233: mouseDownEventResult
= handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown,
currentHitTest));
On 2016/05/27 20:39:27, Rick Byers wrote:
> In addition to suppressing the generation of mouse events, I believe this
change
> also suppresses some/all of the default behavior associated with a tap (eg.
> focusing an input field).  I'm pretty sure this is what you want (since
> cancelling the pointerdown for a mouse would do the same), but there may be
some
> subtle cases here.  Anyway maybe just expand the CL description to describe
this
> broader impact, ideally also add at least a little test coverage (we really
> should have interop tests for that sort of thing).

A milder approach seems to make more sense to me now: minimize the effect of
canceled pointerdown on GEs. In this particular case focusing was originally
triggered by gestureTap, and we are not suppressing gestureTaps, so let's keep
the behavior unchanged. Wdyt?

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:470: // firing for the
current gesure sequence (i.e. until next GestureTapDown).
On 2016/05/27 18:26:32, tdresser wrote:
> gesure -> gesture.

Done.

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:480: // single
touches only, it is enough to queue the
On 2016/05/30 15:14:26, mustaq wrote:
> On 2016/05/27 20:39:27, Rick Byers wrote:
> > Not quite true - GestureTwoFingerTap behaves similarly (but not identically)
> to
> > LongPress on desktop IIRC.
> 
> I will update the comment to clarify it: TwoFingerTaps also send MEs but since
> they fire no touchstarts so we should be fine here.
> 
> We should perhaps fire PEs for TwoFingerTaps? I will file a bug if this may be
> the case.

Clarified the comment. My last post seems wrong about touchstart :(

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:484:
m_primaryPointerdownResults.append(result);
On 2016/05/30 15:14:26, mustaq wrote:
> On 2016/05/27 20:54:07, dtapuska wrote:
> > On 2016/05/27 20:39:27, Rick Byers wrote:
> > > Unfortunately I don't think we can currently guarantee a 1:1
correspondence
> > > between touch and gesture events.  Eg:
> > >  - If the page has no touch/pointer listeners at all, blink will never get
> the
> > > WebTouchEvents (and so I think your take_first will fail because the queue
> is
> > > empty).  Similarly, can CC touch hit testing still cause touch events to
be
> > > hidden completely from blink or did Dave change that?
> > >  - The page may change between when the touch event was received and when
> the
> > > gesture event was received so that they go to different frames.  Moving to
a
> > > page-wide PointerEventManager should fix this except for the OOPIF
scenario
> > > 
> > > One option is to make the code more robust but still a heuristic (eg.
> identify
> > > failure cases - unexpected empty queue, timeout with something left in the
> > queue
> > > a long time), do something reasonable but log an UMA counter so we know
how
> > > often these edge cases are happening (to decide if it's worth fixing).
> > > 
> > > Another option would be to include some sort of identifier in both the
touch
> > and
> > > gesture events to correlate them (and then use a map, but still need some
> > expiry
> > > mechanism).
> > > 
> > > Or perhaps we just need to flow the information back with the touch ACK
and
> > > bring it back with the gesture event.  We discussed this when considering
> the
> > > "TouchEvent.preventMouseEvents()" API and I think this was our preferred
> > choice
> > > at the time.  Not sure how hard it is though.
> > 
> > I'd prefer if the information went up the stack and then flowed back down as
> > opposed to this side channel of data. Side channels can get out of sync. Is
> the
> > tap targeted at the same iframe if something moved between the events?
> 
> Local sync seems unavoidable. I had uniqueTouchIds plumbed all the way down to
> Blink (see Patch#2) but because of brittleness in ID handling in GR (missing
ids
> on timer-fired GEs, GestureTapUnconfirmed breaking tap ids etc), I had to rely
> on TE-vs-GE bracketing behavior to sync the outcome in Blink again.
> 
> Flowing the info back to GR was my first attempt but I had to rule out the
> option because:
> (1) Touch acks are not even sent back for passive listeners, so we can't piggy
> back.
> (2) Sending an IPC back would make it prone to race conditions: what if the
IPC
> reaches GR when the GR is processing either previous or next GE.
> 
> There are two scenarios that may break the TE/GE braketing behavior I am
relying
> on:
> - Too many GEs: easy case, will ignore empty queues.
> - Too many TEs: can it happen except for the OOPIF case?

As we suspected, both of these cases can happen, see the repro
output.jsbin.com/vipuca (touch the pink iframe, then touch at the same coord
again).

My current patch handles all such cases: it uses local sync between TE & GE
handler as before, but brings touch-id plumbing back from Patch#2 to handle
unpaired event streams correctly during the local sync.

[mustaq, 2016-06-02 19:38:17]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-02 19:39:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1989623002/120001

[commit-bot: I haz the power, 2016-06-02 19:44:08]

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

[commit-bot: I haz the power, 2016-06-02 19:44:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dtapuska, 2016-06-03 15:02:11]

lgtm % a few questions

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:784: while
(!m_touchIdsForCanceledPointerdowns.isEmpty()) {
Can we add a comment that we never expect wrap around; because the unique touch
event id always starts at 0 and I would be dead before someone got to 4 billion
touch events.

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/PlatformTouchEvent.h (right):

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/PlatformTouchEvent.h:46: uint32_t
uniqueTouchEventId() const
Can this fit on one line like the inlines above? (consistent styling).

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:480: 
Remove new line

https://codereview.chromium.org/1989623002/diff/120001/ui/events/gesture_dete...
File ui/events/gesture_detection/touch_disposition_gesture_filter.cc (right):

https://codereview.chromium.org/1989623002/diff/120001/ui/events/gesture_dete...
ui/events/gesture_detection/touch_disposition_gesture_filter.cc:310:
GestureEventData event(original_event);
Wouldn't it make more sense to set the touch_event_id on the push_back ?

https://cs.chromium.org/chromium/src/ui/events/gesture_detection/gesture_even...

[mustaq, 2016-06-03 15:05:22]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-03 15:05:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1989623002/140001

[commit-bot: I haz the power, 2016-06-03 15:12:40]

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

[commit-bot: I haz the power, 2016-06-03 15:12:42]

Dry run: Try jobs failed on following builders:
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Rick Byers, 2016-06-03 15:31:40]

The approach seems reasonable to me (it's unfortunate that we need this
complexity, but we've been dancing around it for awhile - it's not really just
PE that benefits from being able to connect gesture events and touch events).

But I think there are some issues with the details.  Where does the touch ID
actually ever make it into the WebGestureEvent?  I see 0 getting passed in a
lot, but unless I'm missing it, only EventSender ever sets a non-zero value.

This is all a little brittle (since the value starts at 0 and gets set later
during dispatch), I think it's important that it has tests (for both Android,
Aura, maybe also devtools touch emulation - or at least check manually that
devtools works without extra code).

I'll look at the WebKit-side changes in more details shortly (but wanted to send
this before running to a meeting).

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:2233: mouseDownEventResult
= handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown,
currentHitTest));
On 2016/06/02 19:26:26, mustaq wrote:
> On 2016/05/27 20:39:27, Rick Byers wrote:
> > In addition to suppressing the generation of mouse events, I believe this
> change
> > also suppresses some/all of the default behavior associated with a tap (eg.
> > focusing an input field).  I'm pretty sure this is what you want (since
> > cancelling the pointerdown for a mouse would do the same), but there may be
> some
> > subtle cases here.  Anyway maybe just expand the CL description to describe
> this
> > broader impact, ideally also add at least a little test coverage (we really
> > should have interop tests for that sort of thing).
> 
> A milder approach seems to make more sense to me now: minimize the effect of
> canceled pointerdown on GEs. In this particular case focusing was originally
> triggered by gestureTap, and we are not suppressing gestureTaps, so let's keep
> the behavior unchanged. Wdyt?

I think it's important that cancelling pointerdown prevents all behavior that
cancelling the associated mousedown originally would have in the GestureTap
scenario (otherwise switching from handling MEs to PEs may break some
scenarios).  So yes, keeping the behavior unchanged makes sense.

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebInputEvent.h (right):

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebInputEvent.h:500: // field contains the unique
identifier for the touch event that triggered
please define a bit more precisely WHICH touch event this corresponds to
("triggers" is pretty vague).  Eg. for a GestureScrollUpdate is it the
touchmove?  What about for a GestureLongPress?  Is it "the ID of the most recent
event which, if cancelled, would have suppressed the generation of this gesture
event"?

This might get a little hairy in the general case.  You could perhaps avoid the
confusion/complexity by saying it represents the entire sequence (not a specific
event) and does so by referring to the event ID for the touchstart of the first
finger involved in the gesture.  This might be more work to implement on the
content side though.

[Rick Byers, 2016-06-03 16:20:45]

blink code looks good too, except that I think with core now depending on these
IDs you'll have to also plumb them down into the DOM TouchEvent/GestureEvent
classes (and update the functions that generate WebInputEvents from DOM events)
so that they're preserved across WebView.  +wjmaclean who owns that.

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1906: WebInputEventResult
mouseDownEventResult = WebInputEventResult::NotHandled;
Back to the earlier discussion - as written this means that if you cancel the
pointerdown, you have no way to avoid the logic in handleMouseFocus and
handleMousePressEvent.  I expect that's not what Edge does (spec says the
default action of pointerdown includes the default actions of mousedown), but I
guess it's not completely clear for the touch case.  Feel free to file a
separate bug if you like.

But I think it would also be fine (and likely more correct) to just initialize
mouseDownEventResult to HandledSuppressed.

Same for mouseup case below of course.

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:787: if (firstId >
uniqueTouchEventId)
Oh, I didn't realize these are required to be monotonically increasing (and
small numbers) - not sure if we depend on this anywhere else.  The comment in
WebTouchEvent should be updated, as well as a comment somewhere on the content
side to make it clear we're depending on these semantics of the ID.

[Rick Byers, 2016-06-03 16:34:06]

On 2016/06/03 15:31:40, Rick Byers wrote:
> The approach seems reasonable to me (it's unfortunate that we need this
> complexity, but we've been dancing around it for awhile - it's not really just
> PE that benefits from being able to connect gesture events and touch events).
> 
> But I think there are some issues with the details.  Where does the touch ID
> actually ever make it into the WebGestureEvent?  I see 0 getting passed in a
> lot, but unless I'm missing it, only EventSender ever sets a non-zero value.

Oh I see now that CreateWebGestureEventFromGestureEventData plumbs it through
for the touch emulation and Android cases.  But I don't see a path for Aura
(with real touch).  Does MakeWebGestureEventFromUIEvent need to be updated
somehow?
 
> This is all a little brittle (since the value starts at 0 and gets set later
> during dispatch), I think it's important that it has tests (for both Android,
> Aura, maybe also devtools touch emulation - or at least check manually that
> devtools works without extra code).
> 
> I'll look at the WebKit-side changes in more details shortly (but wanted to
send
> this before running to a meeting).
> 
>
https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/input/EventHandler.cpp (right):
> 
>
https://codereview.chromium.org/1989623002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/input/EventHandler.cpp:2233:
mouseDownEventResult
> = handleMousePressEvent(MouseEventWithHitTestResults(fakeMouseDown,
> currentHitTest));
> On 2016/06/02 19:26:26, mustaq wrote:
> > On 2016/05/27 20:39:27, Rick Byers wrote:
> > > In addition to suppressing the generation of mouse events, I believe this
> > change
> > > also suppresses some/all of the default behavior associated with a tap
(eg.
> > > focusing an input field).  I'm pretty sure this is what you want (since
> > > cancelling the pointerdown for a mouse would do the same), but there may
be
> > some
> > > subtle cases here.  Anyway maybe just expand the CL description to
describe
> > this
> > > broader impact, ideally also add at least a little test coverage (we
really
> > > should have interop tests for that sort of thing).
> > 
> > A milder approach seems to make more sense to me now: minimize the effect of
> > canceled pointerdown on GEs. In this particular case focusing was originally
> > triggered by gestureTap, and we are not suppressing gestureTaps, so let's
keep
> > the behavior unchanged. Wdyt?
> 
> I think it's important that cancelling pointerdown prevents all behavior that
> cancelling the associated mousedown originally would have in the GestureTap
> scenario (otherwise switching from handling MEs to PEs may break some
> scenarios).  So yes, keeping the behavior unchanged makes sense.
> 
>
https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/pub...
> File third_party/WebKit/public/web/WebInputEvent.h (right):
> 
>
https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/pub...
> third_party/WebKit/public/web/WebInputEvent.h:500: // field contains the
unique
> identifier for the touch event that triggered
> please define a bit more precisely WHICH touch event this corresponds to
> ("triggers" is pretty vague).  Eg. for a GestureScrollUpdate is it the
> touchmove?  What about for a GestureLongPress?  Is it "the ID of the most
recent
> event which, if cancelled, would have suppressed the generation of this
gesture
> event"?
> 
> This might get a little hairy in the general case.  You could perhaps avoid
the
> confusion/complexity by saying it represents the entire sequence (not a
specific
> event) and does so by referring to the event ID for the touchstart of the
first
> finger involved in the gesture.  This might be more work to implement on the
> content side though.

[mustaq, 2016-06-03 19:03:29]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-03 19:03:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/160001

[commit-bot: I haz the power, 2016-06-03 19:23:04]

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

[commit-bot: I haz the power, 2016-06-03 19:23:06]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[mustaq, 2016-06-03 20:34:45]

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:784: while
(!m_touchIdsForCanceledPointerdowns.isEmpty()) {
On 2016/06/03 15:02:11, dtapuska wrote:
> Can we add a comment that we never expect wrap around; because the unique
touch
> event id always starts at 0 and I would be dead before someone got to 4
billion
> touch events.

Done (but didn't mention you would be dead :-P).

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/PlatformTouchEvent.h (right):

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/PlatformTouchEvent.h:46: uint32_t
uniqueTouchEventId() const
On 2016/06/03 15:02:11, dtapuska wrote:
> Can this fit on one line like the inlines above? (consistent styling).

Done.

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/1989623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:480: 
On 2016/06/03 15:02:11, dtapuska wrote:
> Remove new line

Done.

https://codereview.chromium.org/1989623002/diff/120001/ui/events/gesture_dete...
File ui/events/gesture_detection/touch_disposition_gesture_filter.cc (right):

https://codereview.chromium.org/1989623002/diff/120001/ui/events/gesture_dete...
ui/events/gesture_detection/touch_disposition_gesture_filter.cc:310:
GestureEventData event(original_event);
On 2016/06/03 15:02:11, dtapuska wrote:
> Wouldn't it make more sense to set the touch_event_id on the push_back ?
> 
>
https://cs.chromium.org/chromium/src/ui/events/gesture_detection/gesture_even...

Good catch, thanks. I originally got confused with
GestureEventDataPacket::FromTouchTimeout() setting the packet id to zero, but at
the end that zero id is never set later on in the packet, at least in the cases
I experimented. So it doesn't matter.

Done.

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1906: WebInputEventResult
mouseDownEventResult = WebInputEventResult::NotHandled;
On 2016/06/03 16:20:44, Rick Byers wrote:
> Back to the earlier discussion - as written this means that if you cancel the
> pointerdown, you have no way to avoid the logic in handleMouseFocus and
> handleMousePressEvent.  I expect that's not what Edge does (spec says the
> default action of pointerdown includes the default actions of mousedown), but
I
> guess it's not completely clear for the touch case.  Feel free to file a
> separate bug if you like.
> 
> But I think it would also be fine (and likely more correct) to just initialize
> mouseDownEventResult to HandledSuppressed.
> 
> Same for mouseup case below of course.

Looks like we were thinking in opposite directions: by "minimal" I meant keeping
TE side-effects unchanged. Moved back to "maximal" suppression.

Also filed crbug.com/617255 because it's not clear to me what would, say,
not-focusing, mean for uncanceled TEs.

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:787: if (firstId >
uniqueTouchEventId)
On 2016/06/03 16:20:44, Rick Byers wrote:
> Oh, I didn't realize these are required to be monotonically increasing (and
> small numbers) - not sure if we depend on this anywhere else.  The comment in
> WebTouchEvent should be updated, as well as a comment somewhere on the content
> side to make it clear we're depending on these semantics of the ID.

Clarified the existing comment in WebTouchEvent.

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebInputEvent.h (right):

https://codereview.chromium.org/1989623002/diff/140001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebInputEvent.h:500: // field contains the unique
identifier for the touch event that triggered
On 2016/06/03 15:31:40, Rick Byers wrote:
> please define a bit more precisely WHICH touch event this corresponds to
> ("triggers" is pretty vague).  Eg. for a GestureScrollUpdate is it the
> touchmove?  What about for a GestureLongPress?  Is it "the ID of the most
recent
> event which, if cancelled, would have suppressed the generation of this
gesture
> event"?
> 
> This might get a little hairy in the general case.  You could perhaps avoid
the
> confusion/complexity by saying it represents the entire sequence (not a
specific
> event) and does so by referring to the event ID for the touchstart of the
first
> finger involved in the gesture.  This might be more work to implement on the
> content side though.

Does the comment sound better now? This is set on releasing the GE from TDGF, so
"trigger" wasn't the right word.

[mustaq, 2016-06-03 21:04:06]

On 2016/06/03 16:34:06, Rick Byers wrote:
> Oh I see now that CreateWebGestureEventFromGestureEventData plumbs it through
> for the touch emulation and Android cases.  But I don't see a path for Aura
> (with real touch).  Does MakeWebGestureEventFromUIEvent need to be updated
> somehow?

You are right: Aura still needs some work.

Question: any chance TDGF is not used for all platforms? The DCHECK at
SendGesture is not firing on Pixel.

[mustaq, 2016-06-08 19:30:55]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-08 19:32:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/200001

[commit-bot: I haz the power, 2016-06-08 20:15:33]

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

[commit-bot: I haz the power, 2016-06-08 20:15:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mustaq, 2016-06-08 20:30:20]

ptal, Aura gesture path is now fixed.

[Rick Byers, 2016-06-08 20:42:48]

https://codereview.chromium.org/1989623002/diff/200001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/1989623002/diff/200001/ui/events/event.h#newc...
ui/events/event.h:1056: const GestureEventDetails& details);
Should we plan to eliminate this ctor variant eventually?  Have you spot checked
the existing callers to see if any look like they could be sources of bugs (code
paths where the unique_touch_event_id gets dropped)?

[mustaq, 2016-06-08 20:53:17]

https://codereview.chromium.org/1989623002/diff/200001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/1989623002/diff/200001/ui/events/event.h#newc...
ui/events/event.h:1056: const GestureEventDetails& details);
On 2016/06/08 20:42:48, Rick Byers wrote:
> Should we plan to eliminate this ctor variant eventually?  Have you spot
checked
> the existing callers to see if any look like they could be sources of bugs
(code
> paths where the unique_touch_event_id gets dropped)?

We have 83 calls to this ctor, only 4 of them are in non-test code:
src/ash/drag_drop/drag_drop_controller.cc
src/ui/events/gestures/gesture_provider_aura.cc
src/ui/events/ozone/evdev/event_factory_evdev.cc
src/ui/events/test/event_generator.cc

Only the third one is related to gestures released through touches which has
been fixed here.

Is there any way to mark the old ctor as test-only? Like Java
@VisibleForTesting?

[Rick Byers, 2016-06-08 21:02:05]

On 2016/06/08 20:53:17, mustaq wrote:
> https://codereview.chromium.org/1989623002/diff/200001/ui/events/event.h
> File ui/events/event.h (right):
> 
>
https://codereview.chromium.org/1989623002/diff/200001/ui/events/event.h#newc...
> ui/events/event.h:1056: const GestureEventDetails& details);
> On 2016/06/08 20:42:48, Rick Byers wrote:
> > Should we plan to eliminate this ctor variant eventually?  Have you spot
> checked
> > the existing callers to see if any look like they could be sources of bugs
> (code
> > paths where the unique_touch_event_id gets dropped)?
> 
> We have 83 calls to this ctor, only 4 of them are in non-test code:
> src/ash/drag_drop/drag_drop_controller.cc
> src/ui/events/gestures/gesture_provider_aura.cc
> src/ui/events/ozone/evdev/event_factory_evdev.cc
> src/ui/events/test/event_generator.cc
> 
> Only the third one is related to gestures released through touches which has
> been fixed here.
> 
> Is there any way to mark the old ctor as test-only? Like Java
> @VisibleForTesting?

Not really (although we do sometimes make methods private and make the tests
friend classes).  Doing the search and updating the cases that matter is good
enough for me - thanks!  Though an OWNER of this code might want more (eg. TODO
and plan to eventually remove?).

Also I think the coding style implies we should prefer a default argument to an
overload in this case (but not positive):
https://google.github.io/styleguide/cppguide.html#Default_Arguments

[mustaq, 2016-06-08 21:25:18]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-08 21:26:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/220001

[mustaq, 2016-06-08 21:52:48]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-08 21:53:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/240001

[commit-bot: I haz the power, 2016-06-08 23:15:46]

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

[commit-bot: I haz the power, 2016-06-08 23:15:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mustaq, 2016-06-09 15:50:21]

The CQ bit was checked by mustaq@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-09 15:50:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1989623002/280001

[mustaq, 2016-06-09 15:52:18]

ptal:
- Fixed all tests. All bots should be green now.
- Replaced the overloaded ctor in event.h with a default param & detailed
comment.

[Rick Byers, 2016-06-09 16:07:33]

LGTM with nits

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1909: WebInputEventResult
mouseDownEventResult = WebInputEventResult::NotHandled;
nit: this local isn't used outside the below scope - move the declaration into
the scope of the if.

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1958: if
(!m_suppressMouseEventsFromGestures && mouseUpEventResult ==
WebInputEventResult::NotHandled)
nit: you can simplify this slightly and have the exact same behavior by
initializing mouseUpEventResult to handledSuppressed, then you don't need to
re-check m_suppressMouseEventsFromGestures here (and logically it makes sense -
if m_suppressMouseEventsFromGestures is true, then the mouseUp event WASN'T "not
handled", it was never sent in the first place which is what "suppressed"
means).

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h#newc...
ui/events/event.h:1052: // The constructor takes a default unique_touch_id of
zero because test
nit replace "because test" with "for tests."

[mustaq, 2016-06-09 16:40:01]

Description was changed from

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
hoever, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a primary-pointerdown-status queue in Blink
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. The MEs from
GestureLongPress are left as-is because suppressing them
could break drag-drop; this will be done separately in a
follow-up CL.

BUG=606938,607588,507408
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
hoever, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a primary-pointerdown-status queue in Blink
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

[mustaq, 2016-06-09 16:41:01]

mustaq@chromium.org changed reviewers:
+ sadrul@chromium.org

[mustaq, 2016-06-09 16:41:03]

sadrul@chromium.org: Please review changes in ui/events/event.*

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1909: WebInputEventResult
mouseDownEventResult = WebInputEventResult::NotHandled;
On 2016/06/09 16:07:33, Rick Byers wrote:
> nit: this local isn't used outside the below scope - move the declaration into
> the scope of the if.

The mergeEventResult calls at the end combine all the individual results,
including this one, to get the "highest" result. If you think we can skip the
mousedown there, I will add a TODO to avoid changing any behavior in this CL).

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1958: if
(!m_suppressMouseEventsFromGestures && mouseUpEventResult ==
WebInputEventResult::NotHandled)
On 2016/06/09 16:07:33, Rick Byers wrote:
> nit: you can simplify this slightly and have the exact same behavior by
> initializing mouseUpEventResult to handledSuppressed, then you don't need to
> re-check m_suppressMouseEventsFromGestures here (and logically it makes sense
-
> if m_suppressMouseEventsFromGestures is true, then the mouseUp event WASN'T
"not
> handled", it was never sent in the first place which is what "suppressed"
> means).

Done.

Dave, are you okay with this? This only affects the canceled pointerdown case,
so no regression from caller's perspective.

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h#newc...
ui/events/event.h:1052: // The constructor takes a default unique_touch_id of
zero because test
On 2016/06/09 16:07:33, Rick Byers wrote:
> nit replace "because test" with "for tests."

Ooops, had dropped the long line accidentally.

[sadrul, 2016-06-09 16:49:15]

event* lgtm

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h#newc...
ui/events/event.h:1089: uint32_t unique_touch_event_id_;
"""The unique id of the touch event that released the gesture event.""" ==> this
initially lead me to think that this is valid only for gesture-end event-types.
But I think what you mean the touch-event that triggered the gesture event to be
dispatched, so I would change the phrasing a bit (e.g. 'The unique id of the
touch event that caused the gesture event to be dispatched.').

Also, for gesture events that aren't dispatched directly in response to
processing a touch-event (e.g. the long-press gesture, which is dispatched in
response to a timer running out), what does this id mean? The first touch-event
that caused this gesture-event to be generated? Can you clarify in the comment
here?

[dtapuska, 2016-06-09 16:55:02]

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1958: if
(!m_suppressMouseEventsFromGestures && mouseUpEventResult ==
WebInputEventResult::NotHandled)
On 2016/06/09 16:41:03, mustaq wrote:
> On 2016/06/09 16:07:33, Rick Byers wrote:
> > nit: you can simplify this slightly and have the exact same behavior by
> > initializing mouseUpEventResult to handledSuppressed, then you don't need to
> > re-check m_suppressMouseEventsFromGestures here (and logically it makes
sense
> -
> > if m_suppressMouseEventsFromGestures is true, then the mouseUp event WASN'T
> "not
> > handled", it was never sent in the first place which is what "suppressed"
> > means).
> 
> Done.
> 
> Dave, are you okay with this? This only affects the canceled pointerdown case,
> so no regression from caller's perspective.

That is alright. Like I said we could interpret it either way.

[Rick Byers, 2016-06-09 17:01:33]

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1909: WebInputEventResult
mouseDownEventResult = WebInputEventResult::NotHandled;
On 2016/06/09 16:41:03, mustaq wrote:
> On 2016/06/09 16:07:33, Rick Byers wrote:
> > nit: this local isn't used outside the below scope - move the declaration
into
> > the scope of the if.
> 
> The mergeEventResult calls at the end combine all the individual results,
> including this one, to get the "highest" result. If you think we can skip the
> mousedown there, I will add a TODO to avoid changing any behavior in this CL).

Oh sorry, my search missed it (due to wrapping in rietveld) - I was surprised
not to find that.  It's probably only contextual search that relies on the
return value from this function, and in that case cancelling the pointerDown
should definitely prevent CS UI from appearing (so you really don't want to
return NotHandled in that case).

https://codereview.chromium.org/1989623002/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1918: mouseDownEventResult
= WebInputEventResult::HandledSuppressed;
nit: if you want to use if/else (instead of just initializing to
HandledSuppressed and then an if without else), then I believe our style guide
says to avoid the double-negative by putting the positive condition first.

[mustaq, 2016-06-09 17:17:28]

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1909: WebInputEventResult
mouseDownEventResult = WebInputEventResult::NotHandled;
On 2016/06/09 17:01:33, Rick Byers wrote:
> On 2016/06/09 16:41:03, mustaq wrote:
> > On 2016/06/09 16:07:33, Rick Byers wrote:
> > > nit: this local isn't used outside the below scope - move the declaration
> into
> > > the scope of the if.
> > 
> > The mergeEventResult calls at the end combine all the individual results,
> > including this one, to get the "highest" result. If you think we can skip
the
> > mousedown there, I will add a TODO to avoid changing any behavior in this
CL).
> 
> Oh sorry, my search missed it (due to wrapping in rietveld) - I was surprised
> not to find that.  It's probably only contextual search that relies on the
> return value from this function, and in that case cancelling the pointerDown
> should definitely prevent CS UI from appearing (so you really don't want to
> return NotHandled in that case).

Acknowledged.

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h#newc...
ui/events/event.h:1089: uint32_t unique_touch_event_id_;
On 2016/06/09 16:49:15, sadrul wrote:
> """The unique id of the touch event that released the gesture event.""" ==>
this
> initially lead me to think that this is valid only for gesture-end
event-types.
> But I think what you mean the touch-event that triggered the gesture event to
be
> dispatched, so I would change the phrasing a bit (e.g. 'The unique id of the
> touch event that caused the gesture event to be dispatched.').
> 
> Also, for gesture events that aren't dispatched directly in response to
> processing a touch-event (e.g. the long-press gesture, which is dispatched in
> response to a timer running out), what does this id mean? The first
touch-event
> that caused this gesture-event to be generated? Can you clarify in the comment
> here?

Does it sound better?

https://codereview.chromium.org/1989623002/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1989623002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandler.cpp:1918: mouseDownEventResult
= WebInputEventResult::HandledSuppressed;
On 2016/06/09 17:01:33, Rick Byers wrote:
> nit: if you want to use if/else (instead of just initializing to
> HandledSuppressed and then an if without else), then I believe our style guide
> says to avoid the double-negative by putting the positive condition first.

Done.

[sadrul, 2016-06-09 17:22:14]

> https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h
> File ui/events/event.h (right):
> 
>
https://codereview.chromium.org/1989623002/diff/280001/ui/events/event.h#newc...
> ui/events/event.h:1089: uint32_t unique_touch_event_id_;
> On 2016/06/09 16:49:15, sadrul wrote:
> > """The unique id of the touch event that released the gesture event.""" ==>
> this
> > initially lead me to think that this is valid only for gesture-end
> event-types.
> > But I think what you mean the touch-event that triggered the gesture event
to
> be
> > dispatched, so I would change the phrasing a bit (e.g. 'The unique id of the
> > touch event that caused the gesture event to be dispatched.').
> > 
> > Also, for gesture events that aren't dispatched directly in response to
> > processing a touch-event (e.g. the long-press gesture, which is dispatched
in
> > response to a timer running out), what does this id mean? The first
> touch-event
> > that caused this gesture-event to be generated? Can you clarify in the
comment
> > here?
> 
> Does it sound better?

Yes, thanks! slgtm

[mustaq, 2016-06-09 17:29:58]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-06-09 17:29:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, dtapuska@chromium.org, rbyers@chromium.org,
sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/1989623002/#ps320001
(title: "Addressed final comments.")

[commit-bot: I haz the power, 2016-06-09 17:30:40]

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

[mustaq, 2016-06-09 17:34:03]

Description was changed from

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
hoever, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a primary-pointerdown-status queue in Blink
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
however, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs, when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a unique-touch-id queue in PointerEventManager
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

[commit-bot: I haz the power, 2016-06-09 18:22:32]

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

[commit-bot: I haz the power, 2016-06-09 18:22:34]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mustaq, 2016-06-09 19:41:34]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-06-09 19:41:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, sadrul@chromium.org, tdresser@chromium.org,
rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1989623002/#ps340001
(title: "Merge conflicts for Aura")

[commit-bot: I haz the power, 2016-06-09 19:41:55]

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

[commit-bot: I haz the power, 2016-06-09 22:45:44]

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

[commit-bot: I haz the power, 2016-06-09 22:45:46]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mustaq, 2016-06-15 15:53:00]

Discovered that the Windows test failure is caused by the change in
EventSender::GetMovedBeyondSlopRegionArg(). Still investigating why this is
Windows only.

[mustaq, 2016-06-15 17:32:14]

On 2016/06/15 15:53:00, mustaq wrote:
> Discovered that the Windows test failure is caused by the change in
> EventSender::GetMovedBeyondSlopRegionArg(). Still investigating why this is
> Windows only.

This was caused by function param evaluation order in MSVC vs gcc. Fixed.

[mustaq, 2016-06-15 17:37:20]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-06-15 17:37:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, sadrul@chromium.org, tdresser@chromium.org,
rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1989623002/#ps400001
(title: "Fixed event_sender parsing order in Windows.")

[commit-bot: I haz the power, 2016-06-15 17:37:51]

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

[commit-bot: I haz the power, 2016-06-15 17:47:29]

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

[commit-bot: I haz the power, 2016-06-15 17:47:31]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mustaq, 2016-06-15 18:46:10]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-06-15 18:46:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, sadrul@chromium.org, tdresser@chromium.org,
rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1989623002/#ps420001
(title: "Rebased")

[commit-bot: I haz the power, 2016-06-15 18:46:50]

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

[commit-bot: I haz the power, 2016-06-15 21:29:17]

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

[commit-bot: I haz the power, 2016-06-15 21:29:19]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mustaq, 2016-06-16 15:31:36]

The CQ bit was checked by mustaq@chromium.org

[commit-bot: I haz the power, 2016-06-16 15:32:45]

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

[commit-bot: I haz the power, 2016-06-16 21:39:46]

Description was changed from

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
however, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs, when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a unique-touch-id queue in PointerEventManager
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
however, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs, when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a unique-touch-id queue in PointerEventManager
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

[commit-bot: I haz the power, 2016-06-16 21:39:48]

Committed patchset #21 (id:420001)

[commit-bot: I haz the power, 2016-06-16 21:43:35]

Description was changed from

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
however, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs, when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a unique-touch-id queue in PointerEventManager
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408
==========

to

==========
Suppressed MEs for gestures from cancelled PEs

Our current implementation suppresses the touch-points
for which the pointerevents have been canceled, to mimic
the ME suppression behavior of PointerEvents. In Edge,
however, TEs are fired as-is without considering the
canceling status of pointerdowns. We need to follow the
Edge behavior here for better web-compat.

Moreover, since preventing a pointerdown suppresses MEs,
this should be true for the MEs from gestures that are
ultimately triggered by TEs, when the PEs corresponding
to the TEs are canceled (crbug.com/606938).

In addition, the current implementation (suppressing
touch-points for canceled pointerevents) has a few bad
side-effects:
- TE sequence can become inconsistent (eg touchend w/o
toushstart) for certain sequences of PE canceling
(crbug.com/607588).
- Preventing all PEs completely hides TEs but until touch
scroll starts, which suppresses PEs only to "reveal" the
hidden TE sequence from "middle" (crbug.com/507408).

This CL adds a unique-touch-id queue in PointerEventManager
to properly suppress MEs from GestureTap, and makes TE
firing independent of PE handling in JS. This CL also
suppresses some default behavior associated with a
tap on canceled pointerdowns (e.g. focusing an input
field).

The MEs from GestureLongPress are left as-is in this CL
because suppressing them could break drag-drop; this will
be done separately in a follow-up CL.

BUG=606938,607588,507408

Committed: https://crrev.com/1904837ebb6f8f352572e0f833e14640a0db6d54
Cr-Commit-Position: refs/heads/master@{#400264}
==========

[commit-bot: I haz the power, 2016-06-16 21:43:38]

Patchset 21 (id:??) landed as
https://crrev.com/1904837ebb6f8f352572e0f833e14640a0db6d54
Cr-Commit-Position: refs/heads/master@{#400264}