[Android] Support double-tap selection

[Android] Support double-tap selection

Add single tap repeat count detection to GestureDetector for cases when
double-tap detection is disabled. Refactor Aura to use this path, and
also update the TouchSelectionController to support selection handle
display when the user's double (or triple) tap gesture selects text.

BUG=234986, 453476, 522268
Committed: https://crrev.com/b1c7ccf1e392ea2b3bef380148a86978f43a2967
Cr-Commit-Position: refs/heads/master@{#350902}

[jdduke (slow), 2015-09-22 20:12:30]

jdduke@chromium.org changed reviewers:
+ tdresser@chromium.org

[jdduke (slow), 2015-09-22 20:12:31]

tdresser@: PTAL, thanks.

[jdduke (slow), 2015-09-22 20:17:57]

Patchset #2 (id:20001) has been deleted

[tdresser, 2015-09-23 14:10:46]

LGTM, though it looks like there's some test flake.

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.h:142: int single_tap_count_;
The semantics of this variable feel a bit confusing.
Would it be clearer if this was the number of taps that have already happened
during the current tap sequence, instead of the tap count of the next tap to
occur?

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_provider_unittest.cc (right):

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_provider_unittest.cc:2533:
SetSingleTapRepeatLength(3);
Do we have adequate coverage of when the repeat length is 1?

[jdduke (slow), 2015-09-23 19:56:04]

I went ahead and removed the "min" double-tap check when we're not detecting
double-taps. It's really only necessary to avoid entry into double-tap drag
sequences.

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.h:142: int single_tap_count_;
On 2015/09/23 14:10:46, tdresser wrote:
> The semantics of this variable feel a bit confusing.
> Would it be clearer if this was the number of taps that have already happened
> during the current tap sequence, instead of the tap count of the next tap to
> occur?

Yeah, though I think we'll need an auxiliary variable to indicate that the
current touch sequence is possibly a repeated one (the IsConsideredDoubleTap
check needs to happen on DOWN for several reasons).

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_provider_unittest.cc (right):

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_provider_unittest.cc:2533:
SetSingleTapRepeatLength(3);
On 2015/09/23 14:10:46, tdresser wrote:
> Do we have adequate coverage of when the repeat length is 1?

Probably not, I'll add a test.

[tdresser, 2015-09-24 12:05:00]

Still LGTM.

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
File ui/events/gesture_detection/gesture_detector.h (right):

https://codereview.chromium.org/1358263002/diff/80001/ui/events/gesture_detec...
ui/events/gesture_detection/gesture_detector.h:142: int single_tap_count_;
On 2015/09/23 19:56:04, jdduke wrote:
> On 2015/09/23 14:10:46, tdresser wrote:
> > The semantics of this variable feel a bit confusing.
> > Would it be clearer if this was the number of taps that have already
happened
> > during the current tap sequence, instead of the tap count of the next tap to
> > occur?
> 
> Yeah, though I think we'll need an auxiliary variable to indicate that the
> current touch sequence is possibly a repeated one (the IsConsideredDoubleTap
> check needs to happen on DOWN for several reasons).

Thanks - I find this easier to follow.

[jdduke (slow), 2015-09-25 16:50:44]

jdduke@chromium.org changed reviewers:
+ donnd@chromium.org

[jdduke (slow), 2015-09-25 16:50:45]

donnd@: Could you take a look at the contextual search changes? I think we're
actually OK here for now, I played with the patch locally and it seems to work
fine, even for nearby taps. If you double-tap nearby an existing CS selection,
you can get into the state where the double-tap shows the pins instead of
refining, but I think that's OK. We still want to proceed with your work of
getting the ACK though.

[Donn Denman, 2015-09-25 19:58:06]

On 2015/09/25 16:50:45, jdduke wrote:
> donnd@: Could you take a look at the contextual search changes? I think we're
> actually OK here for now, I played with the patch locally and it seems to work
> fine, even for nearby taps. If you double-tap nearby an existing CS selection,
> you can get into the state where the double-tap shows the pins instead of
> refining, but I think that's OK. We still want to proceed with your work of
> getting the ACK though.

LGTM for the Contextual Search changes.

[jdduke (slow), 2015-09-25 20:00:03]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2015-09-25 20:00:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1358263002/#ps120001
(title: "Fix contextual search")

[commit-bot: I haz the power, 2015-09-25 20:01:27]

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

[commit-bot: I haz the power, 2015-09-25 20:41:50]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-09-25 20:42:42]

Patchset 6 (id:??) landed as
https://crrev.com/b1c7ccf1e392ea2b3bef380148a86978f43a2967
Cr-Commit-Position: refs/heads/master@{#350902}