Consuming any touch move before SCROLL_START prevents the scroll from occuring in Aura

Consuming any touch move before SCROLL_START prevents the scroll from occuring in Aura

This patch does NOT prevent tap or long press when a touch move is consumed (see
crbug.com/327444).

An additional state is added to GestureState - GS_SYNTHETIC_CLICK_ABORTED. In addition,
the meaning of GS_PENDING_SYNTHETIC_CLICK_NO_SCROLL has changed.

GS_PENDING_SYNTHETIC_CLICK_NO_SCROLL indicates that one finger is down and no scrolling
can happen, but a tap or long press can still happen.

GS_SYNTHETIC_CLICK_ABORTED indicates that a finger is down, but no gestures can be
created until the number of active touch points changes.

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

[sadrul, 2014-01-15 20:22:38]

What is the desired behaviour in this case:

 1 First touch point is pressed.
 2 The first move of this touch point is consumed.
 3 The first touch point moves more. But no scroll is generated (because of step
2).
 4 Second touch point is pressed.
 5 Second touch point moves around.
 6 First touch point also moves around.

In steps 5 and 6, do we generate scroll/pinch events?

[tdresser, 2014-01-15 20:32:03]

On 2014/01/15 20:22:38, sadrul wrote:
> What is the desired behaviour in this case:
> 
>  1 First touch point is pressed.
>  2 The first move of this touch point is consumed.
>  3 The first touch point moves more. But no scroll is generated (because of
step
> 2).
>  4 Second touch point is pressed.
>  5 Second touch point moves around.
>  6 First touch point also moves around.
> 
> In steps 5 and 6, do we generate scroll/pinch events?

Rick and I decided we're okay if 5 and 6 generate scroll/pinch events, at least
for the moment.

[jdduke (slow), 2014-01-15 20:33:47]

On 2014/01/15 20:32:03, tdresser wrote:
> On 2014/01/15 20:22:38, sadrul wrote:
> > What is the desired behaviour in this case:
> > 
> >  1 First touch point is pressed.
> >  2 The first move of this touch point is consumed.
> >  3 The first touch point moves more. But no scroll is generated (because of
> step
> > 2).
> >  4 Second touch point is pressed.
> >  5 Second touch point moves around.
> >  6 First touch point also moves around.
> > 
> > In steps 5 and 6, do we generate scroll/pinch events?
> 
> Rick and I decided we're okay if 5 and 6 generate scroll/pinch events, at
least
> for the moment.

Hmm, under eager GR, 5 and 6 would not generate any events.  Will that be OK?

[tdresser, 2014-01-15 20:37:17]

On 2014/01/15 20:33:47, jdduke wrote:
> On 2014/01/15 20:32:03, tdresser wrote:
> > On 2014/01/15 20:22:38, sadrul wrote:
> > > What is the desired behaviour in this case:
> > > 
> > >  1 First touch point is pressed.
> > >  2 The first move of this touch point is consumed.
> > >  3 The first touch point moves more. But no scroll is generated (because
of
> > step
> > > 2).
> > >  4 Second touch point is pressed.
> > >  5 Second touch point moves around.
> > >  6 First touch point also moves around.
> > > 
> > > In steps 5 and 6, do we generate scroll/pinch events?
> > 
> > Rick and I decided we're okay if 5 and 6 generate scroll/pinch events, at
> least
> > for the moment.
> 
> Hmm, under eager GR, 5 and 6 would not generate any events.  Will that be OK?

That is where we want to end up, (I think).
Right now in Aura we're trying to make gradual changes which move us towards the
eventual model, so that we have the ability to roll back any changes that people
aren't happy with.

[sadrul, 2014-01-17 17:11:35]

LGTM with some comments below

https://codereview.chromium.org/133273013/diff/1/ui/aura/gestures/gesture_rec...
File ui/aura/gestures/gesture_recognizer_unittest.cc (right):

https://codereview.chromium.org/133273013/diff/1/ui/aura/gestures/gesture_rec...
ui/aura/gestures/gesture_recognizer_unittest.cc:3769:
EXPECT_FALSE(delegate->scroll_update());
EXPECT_FALSE(scroll_begin()) too?

https://codereview.chromium.org/133273013/diff/1/ui/aura/gestures/gesture_rec...
ui/aura/gestures/gesture_recognizer_unittest.cc:3770: }
Add a test as above, with the addition of a second finger, and show that
scroll/pinch happens.

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
File ui/events/gestures/gesture_sequence.cc (right):

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
ui/events/gestures/gesture_sequence.cc:767: state_ !=
GS_PENDING_SYNTHETIC_CLICK_NO_SCROLL &&
Can this be somewhat like this:

  if (last_state != state &&
      GestureStateSupportsActiveTimer(last_state) &&
      !GestureStateSupportsActiveTimer(state_)) {
    ...
  }

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
File ui/events/gestures/gesture_sequence.h (right):

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
ui/events/gestures/gesture_sequence.h:22: // One finger is down: tap could
occur, but scroll cannot
Mention that scroll/pinch can happen if a second touch-point is pressed.

[tdresser, 2014-01-17 19:48:42]

https://codereview.chromium.org/133273013/diff/1/ui/aura/gestures/gesture_rec...
File ui/aura/gestures/gesture_recognizer_unittest.cc (right):

https://codereview.chromium.org/133273013/diff/1/ui/aura/gestures/gesture_rec...
ui/aura/gestures/gesture_recognizer_unittest.cc:3769:
EXPECT_FALSE(delegate->scroll_update());
On 2014/01/17 17:11:35, sadrul wrote:
> EXPECT_FALSE(scroll_begin()) too?

Done.

https://codereview.chromium.org/133273013/diff/1/ui/aura/gestures/gesture_rec...
ui/aura/gestures/gesture_recognizer_unittest.cc:3770: }
On 2014/01/17 17:11:35, sadrul wrote:
> Add a test as above, with the addition of a second finger, and show that
> scroll/pinch happens.

Done.

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
File ui/events/gestures/gesture_sequence.cc (right):

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
ui/events/gestures/gesture_sequence.cc:767: state_ !=
GS_PENDING_SYNTHETIC_CLICK_NO_SCROLL &&
On 2014/01/17 17:11:35, sadrul wrote:
> Can this be somewhat like this:
> 
>   if (last_state != state &&
>       GestureStateSupportsActiveTimer(last_state) &&
>       !GestureStateSupportsActiveTimer(state_)) {
>     ...
>   }

We don't even need the "last_state != state" check! Definitely cleaner now.
Done.

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
File ui/events/gestures/gesture_sequence.h (right):

https://codereview.chromium.org/133273013/diff/1/ui/events/gestures/gesture_s...
ui/events/gestures/gesture_sequence.h:22: // One finger is down: tap could
occur, but scroll cannot
On 2014/01/17 17:11:35, sadrul wrote:
> Mention that scroll/pinch can happen if a second touch-point is pressed.

Done.

[commit-bot: I haz the power, 2014-01-17 19:50:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tdresser@chromium.org/133273013/210001

[commit-bot: I haz the power, 2014-01-17 22:11:13]

Change committed as 245635