Share a user gesture indicator across touch event sequences

Share a user gesture indicator across touch event sequences

BUG=379271
R=rbyers@chromium.org

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

[Rick Byers, 2014-06-12 20:45:53]

Thanks, looks good - just a couple minor comments.

https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
File LayoutTests/fast/events/popup-allowed-from-touch-only-once.html (right):

https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:16: function
popup(e)
If you wanted to be a little more thorough here you could also invoke this for
touchstart and touchend.  Also trigger a second finger to go down - a pinch-like
gesture should permit one pop-up not two.

https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:49: var button =
document.getElementById("button");
what's this for?  delete it?

https://codereview.chromium.org/329393006/diff/1/Source/core/page/EventHandle...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/329393006/diff/1/Source/core/page/EventHandle...
Source/core/page/EventHandler.cpp:3432: gestureIndicator = adoptPtr(new
UserGestureIndicator(DefinitelyProcessingUserGesture));
Looks like the mouse code takes an indicator only for the press and release
events, not the movement events.  Is there any reason for mouse and touch to be
inconsistent here?

[Rick Byers, 2014-06-12 20:46:28]

On 2014/06/12 20:45:53, Rick Byers wrote:
> Thanks, looks good - just a couple minor comments.
> 
>
https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
> File LayoutTests/fast/events/popup-allowed-from-touch-only-once.html (right):
> 
>
https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
> LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:16: function
> popup(e)
> If you wanted to be a little more thorough here you could also invoke this for
> touchstart and touchend.  Also trigger a second finger to go down - a
pinch-like
> gesture should permit one pop-up not two.
> 
>
https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
> LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:49: var button
=
> document.getElementById("button");
> what's this for?  delete it?
> 
>
https://codereview.chromium.org/329393006/diff/1/Source/core/page/EventHandle...
> File Source/core/page/EventHandler.cpp (right):
> 
>
https://codereview.chromium.org/329393006/diff/1/Source/core/page/EventHandle...
> Source/core/page/EventHandler.cpp:3432: gestureIndicator = adoptPtr(new
> UserGestureIndicator(DefinitelyProcessingUserGesture));
> Looks like the mouse code takes an indicator only for the press and release
> events, not the movement events.  Is there any reason for mouse and touch to
be
> inconsistent here?

Also fix typo in CL title / description ("sequencesn").

[jochen (gone - plz use gerrit), 2014-06-12 21:09:55]

https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
File LayoutTests/fast/events/popup-allowed-from-touch-only-once.html (right):

https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:16: function
popup(e)
On 2014/06/12 20:45:53, Rick Byers wrote:
> If you wanted to be a little more thorough here you could also invoke this for
> touchstart and touchend.  Also trigger a second finger to go down - a
pinch-like
> gesture should permit one pop-up not two.

Done.

https://codereview.chromium.org/329393006/diff/1/LayoutTests/fast/events/popu...
LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:49: var button =
document.getElementById("button");
On 2014/06/12 20:45:53, Rick Byers wrote:
> what's this for?  delete it?

Done.

https://codereview.chromium.org/329393006/diff/1/Source/core/page/EventHandle...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/329393006/diff/1/Source/core/page/EventHandle...
Source/core/page/EventHandler.cpp:3432: gestureIndicator = adoptPtr(new
UserGestureIndicator(DefinitelyProcessingUserGesture));
On 2014/06/12 20:45:53, Rick Byers wrote:
> Looks like the mouse code takes an indicator only for the press and release
> events, not the movement events.  Is there any reason for mouse and touch to
be
> inconsistent here?

no idea, however, if we want to change that, I think it should be a different CL

[Rick Byers, 2014-06-12 21:14:23]

LGTM with nit

https://codereview.chromium.org/329393006/diff/20001/LayoutTests/fast/events/...
File LayoutTests/fast/events/popup-allowed-from-touch-only-once.html (right):

https://codereview.chromium.org/329393006/diff/20001/LayoutTests/fast/events/...
LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:68:
eventSender.addTouchPoint(75, 50);
nit: please add one more touchStart above here - that way we're actually testing
the 2nd touchstart event path (this is just one touchstart with two touch points
in it).

[jochen (gone - plz use gerrit), 2014-06-12 21:17:06]

https://codereview.chromium.org/329393006/diff/20001/LayoutTests/fast/events/...
File LayoutTests/fast/events/popup-allowed-from-touch-only-once.html (right):

https://codereview.chromium.org/329393006/diff/20001/LayoutTests/fast/events/...
LayoutTests/fast/events/popup-allowed-from-touch-only-once.html:68:
eventSender.addTouchPoint(75, 50);
On 2014/06/12 21:14:23, Rick Byers wrote:
> nit: please add one more touchStart above here - that way we're actually
testing
> the 2nd touchstart event path (this is just one touchstart with two touch
points
> in it).

Done.

[jochen (gone - plz use gerrit), 2014-06-12 21:18:23]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-06-12 21:19:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jochen@chromium.org/329393006/40001

[commit-bot: I haz the power, 2014-06-12 22:47:07]

Change committed as 176059