[TTS] Fix for tap on a previous tap-selection.

[TTS] Fix tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the second tap happens before we get notification
that the selection pins have been shown.  TTS starts the process
of handling the tap unless it's fully handled the previous one,
which is flawed logic.  This flawed logic may have been responsible
for some taps being ignored.

This CL introduces a new state that's used to wait for the tap
gesture to be converted to a long-press gesture.  Unless we're
in the IDLE state when a tap is recognized we'll enter this new
wait state and usually timeout and advance our normal tap
processing.  In the case of the user doing a second tap the
recognition of the long-press abandons the previous tap processing.
    
Also clean up our @see directives in CSInternalStateHandler.java.
    
BUG=736237, 717942
Review-Url: https://codereview.chromium.org/2963013002
Cr-Commit-Position: refs/heads/master@{#483510}
Committed: https://chromium.googlesource.com/chromium/src/+/04a6e962e89fbab7754fc4c796b25b6ddc134985

[Donn Denman, 2017-06-28 21:37:54]

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

[commit-bot: I haz the power, 2017-06-28 21:38:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Donn Denman, 2017-06-28 21:39:39]

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

[Donn Denman, 2017-06-28 21:39:39]

Theresa, PTAL when convenient.  Thanks!

[commit-bot: I haz the power, 2017-06-28 22:50:59]

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

[commit-bot: I haz the power, 2017-06-28 22:50:59]

Dry run: This issue passed the CQ dry run.

[Donn Denman, 2017-06-28 22:57:18]

Description was changed from

==========
[TTS] Fix tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the process of gathering surrounding text
fails when the selection is changed to a user-controlled selection
and our current code always aborts when it cannot get surrounding
text.

The fix is to only abort when the selection type is Tap but to
allow processing to continue when it's Long-press.

BUG=736237
==========

to

==========
[TTS] Fix for tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the process of gathering surrounding text
fails when the selection is changed to a user-controlled selection
and our current code always aborts when it cannot get surrounding
text.

The fix is to only abort when the selection type is Tap but to
allow processing to continue when it's Long-press.

This fixes the most common way that this problem occurs.

BUG=736237
==========

[Theresa, 2017-06-28 23:41:29]

On 2017/06/28 21:39:39, Donn Denman wrote:
> Theresa, PTAL when convenient.  Thanks!

I think these are the state transitions that are happening:

1) TAP_RECOGNIZED -> 2) GATHERING_SURROUNDINGS -> 3) LONG_PRESS_RECOGNIZED -> 4)
GATHERING_SURROUNDINGS

When onTextSurroundingSelectionAvailable is called, is it in response to #4 or
#2?  Do we have an idea of why gathering text fails when the selection changes?

Perhaps we should always fall back to the selected word and gracefully handle
surroundings not being available whether triggered from tap or long-press?

[Donn Denman, 2017-06-29 01:33:45]

On 2017/06/28 23:41:29, Theresa wrote:
> On 2017/06/28 21:39:39, Donn Denman wrote:
> > Theresa, PTAL when convenient.  Thanks!
> 
> I think these are the state transitions that are happening:
> 
> 1) TAP_RECOGNIZED -> 2) GATHERING_SURROUNDINGS -> 3) LONG_PRESS_RECOGNIZED ->
4)
> GATHERING_SURROUNDINGS
I initially thought this too, but after you asked I realized the we gather the
surroundings for tap BEFORE we select the word, so we've already gotten the
surrounding text
before the second tap can trigger the pins to show.

However, I investigated further and I see now that there's a clear difference in
state-sequences
when tapping slowly versus tapping quickly.  When you tap slowly everything
works
as I just described.  But when you tap quickly the second tap DOES enter the
TAP_RECOGNIZED
state and returns the surroundings. Then the pins show and LONG_PRESS_RECOGNIZED
immediately
gathers the surroundings again and that gathering fails.

> 
> When onTextSurroundingSelectionAvailable is called, is it in response to #4 or
> #2?  Do we have an idea of why gathering text fails when the selection
changes?
> 
> Perhaps we should always fall back to the selected word and gracefully handle
> surroundings not being available whether triggered from tap or long-press?

I'll try to figure out why that second tap on the selection is sometimes
considered a tap
and sometimes ignored in favor of the longpress.  Fixing that so we only get one
sequence
in response to the user gesture should help us get a better solution.

[Donn Denman, 2017-06-29 01:36:10]

donnd@chromium.org changed reviewers:
- twellington@chromium.org

[Donn Denman, 2017-06-29 01:36:11]

Theresa, Thanks for the questions and gentle push-back!

I'll work on getting that tap on the existing selection to always register as a
long-press even when done quickly.

I'll put you back on the reviewer list when ready.

[Donn Denman, 2017-06-29 17:11:56]

Description was changed from

==========
[TTS] Fix for tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the process of gathering surrounding text
fails when the selection is changed to a user-controlled selection
and our current code always aborts when it cannot get surrounding
text.

The fix is to only abort when the selection type is Tap but to
allow processing to continue when it's Long-press.

This fixes the most common way that this problem occurs.

BUG=736237
==========

to

==========
[TTS] Fix tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the second tap happens before we get notification
that the selection pins have been shown.  TTS starts the process
of handling the tap unless it's fully handled the previous one,
which is flawed logic.  This flawed logic may have been responsible
for some taps being ignored.

This CL introduces a new state that's used to wait for the tap
gesture to be converted to a long-press gesture.  Unless we're
in the IDLE state when a tap is recognized we'll enter this new
wait state and usually timeout and advance our normal tap
processing.  In the case of the user doing a second tap the
recognition of the long-press abandons the previous tap processing.
    
Also clean up our @see directives in CSInternalStateHandler.java.
    
BUG=736237,717942
==========

[Donn Denman, 2017-06-29 17:12:55]

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

[Donn Denman, 2017-06-29 17:12:55]

Theresa, PTAL when convenient.  

This version works much better.

Thanks!

[Theresa, 2017-06-29 17:15:52]

lgtm

[Donn Denman, 2017-06-29 17:19:13]

The CQ bit was checked by donnd@chromium.org

[commit-bot: I haz the power, 2017-06-29 17:19:24]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 17:54:24]

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

[commit-bot: I haz the power, 2017-06-29 17:54:25]

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

[Donn Denman, 2017-06-29 21:08:04]

The CQ bit was checked by donnd@chromium.org

[Donn Denman, 2017-06-29 21:08:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org
Link to the patchset: https://codereview.chromium.org/2963013002/#ps40001
(title: "Updated the CSInternalStateTest.")

[commit-bot: I haz the power, 2017-06-29 21:08:56]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 22:08:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498770484893160,
"parent_rev": "24046fd36c1a132243a7ffd279f59734f947d223", "commit_rev":
"04a6e962e89fbab7754fc4c796b25b6ddc134985"}

[commit-bot: I haz the power, 2017-06-29 22:08:37]

Description was changed from

==========
[TTS] Fix tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the second tap happens before we get notification
that the selection pins have been shown.  TTS starts the process
of handling the tap unless it's fully handled the previous one,
which is flawed logic.  This flawed logic may have been responsible
for some taps being ignored.

This CL introduces a new state that's used to wait for the tap
gesture to be converted to a long-press gesture.  Unless we're
in the IDLE state when a tap is recognized we'll enter this new
wait state and usually timeout and advance our normal tap
processing.  In the case of the user doing a second tap the
recognition of the long-press abandons the previous tap processing.
    
Also clean up our @see directives in CSInternalStateHandler.java.
    
BUG=736237,717942
==========

to

==========
[TTS] Fix tap on a previous tap-selection.

When a tap-select is later tapped upon, it should always show
the selection pins and the CS Bar.  However this isn't working
when the later tap happens very quickly after the first tap.

The problem is that the second tap happens before we get notification
that the selection pins have been shown.  TTS starts the process
of handling the tap unless it's fully handled the previous one,
which is flawed logic.  This flawed logic may have been responsible
for some taps being ignored.

This CL introduces a new state that's used to wait for the tap
gesture to be converted to a long-press gesture.  Unless we're
in the IDLE state when a tap is recognized we'll enter this new
wait state and usually timeout and advance our normal tap
processing.  In the case of the user doing a second tap the
recognition of the long-press abandons the previous tap processing.
    
Also clean up our @see directives in CSInternalStateHandler.java.
    
BUG=736237,717942

Review-Url: https://codereview.chromium.org/2963013002
Cr-Commit-Position: refs/heads/master@{#483510}
Committed:
https://chromium.googlesource.com/chromium/src/+/04a6e962e89fbab7754fc4c796b2...
==========

[commit-bot: I haz the power, 2017-06-29 22:08:39]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/04a6e962e89fbab7754fc4c796b2...