[TTS] Robustness for VR: show with null selection.

[TTS] Robustness for VR: show with null selection.

There's a crash on VR that seems to be due to a lack of robustness
in the way Contextual Search builds a query with TemplateUrl.
This might also happen when tapping around a lot (speculation) so
improving robustness is worthwhile.

Adds a check to ContextualSearchManager.showContextualSearch that
the selection is valid before calling TemplateUrl.

Adds a test for this case that will expose this crash (without the
robustness change).  Also updates the test infrastructure for
Contextual Search to not mock out TemplateUrl since that does not
seem to be needed anymore.

BUG=728644
Review-Url: https://codereview.chromium.org/2928053002
Cr-Commit-Position: refs/heads/master@{#478446}
Committed: https://chromium.googlesource.com/chromium/src/+/06187f572930bf54ae294079baf9c8dad8a33926

[Donn Denman, 2017-06-09 00:17:49]

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

[commit-bot: I haz the power, 2017-06-09 00:18:21]

Dry run: 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-09 01:09:04]

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

[commit-bot: I haz the power, 2017-06-09 01:09:05]

Dry run: This issue passed the CQ dry run.

[Donn Denman, 2017-06-09 02:55:14]

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

[Donn Denman, 2017-06-09 02:55:15]

Theresa, PTAL at this VR crash fix that we want to merge into M60.  Thanks!

[Theresa, 2017-06-09 16:48:05]

https://codereview.chromium.org/2928053002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
(right):

https://codereview.chromium.org/2928053002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java:348:
Thread.sleep(500);
Are we waiting to make sure the panel doesn't open within .5 seconds, and if so,
is there another signal we may be able to rely on? This feels like it has the
potential to be flaky e.g. on some really slow devices maybe text extraction
takes more than .5 seconds.

[Donn Denman, 2017-06-09 16:48:16]

Theresa, Hold off on this for the moment -- I have another change to make to
this CL before it's ready for review.

[Donn Denman, 2017-06-09 22:22:22]

Theres, This is ready for a final review.  Thanks!

https://codereview.chromium.org/2928053002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
(right):

https://codereview.chromium.org/2928053002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java:348:
Thread.sleep(500);
On 2017/06/09 16:48:05, Theresa wrote:
> Are we waiting to make sure the panel doesn't open within .5 seconds, and if
so,
> is there another signal we may be able to rely on? This feels like it has the
> potential to be flaky e.g. on some really slow devices maybe text extraction
> takes more than .5 seconds.

Thanks for flagging this -- I forgot to finish of this test.

Now removed the sleep, validated that the test causes the crash (without the
fix), and cleaned up comments here.

[Donn Denman, 2017-06-09 22:22:36]

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

[commit-bot: I haz the power, 2017-06-09 22:22:58]

Dry run: CQ is trying da patch.

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

[Theresa, 2017-06-09 22:24:21]

lgtm

[Donn Denman, 2017-06-09 23:03:11]

The CQ bit was unchecked by donnd@chromium.org

[Donn Denman, 2017-06-09 23:03:14]

The CQ bit was checked by donnd@chromium.org

[commit-bot: I haz the power, 2017-06-09 23:03:51]

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-09 23:10:39]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497049394744520,
"parent_rev": "a81c5ee45efe465dbb6fb29720256d3e4787dc6b", "commit_rev":
"06187f572930bf54ae294079baf9c8dad8a33926"}

[commit-bot: I haz the power, 2017-06-09 23:10:49]

Description was changed from

==========
[TTS] Robustness for VR: show with null selection.

There's a crash on VR that seems to be due to a lack of robustness
in the way Contextual Search builds a query with TemplateUrl.
This might also happen when tapping around a lot (speculation) so
improving robustness is worthwhile.

Adds a check to ContextualSearchManager.showContextualSearch that
the selection is valid before calling TemplateUrl.

Adds a test for this case that will expose this crash (without the
robustness change).  Also updates the test infrastructure for
Contextual Search to not mock out TemplateUrl since that does not
seem to be needed anymore.

BUG=728644
==========

to

==========
[TTS] Robustness for VR: show with null selection.

There's a crash on VR that seems to be due to a lack of robustness
in the way Contextual Search builds a query with TemplateUrl.
This might also happen when tapping around a lot (speculation) so
improving robustness is worthwhile.

Adds a check to ContextualSearchManager.showContextualSearch that
the selection is valid before calling TemplateUrl.

Adds a test for this case that will expose this crash (without the
robustness change).  Also updates the test infrastructure for
Contextual Search to not mock out TemplateUrl since that does not
seem to be needed anymore.

BUG=728644

Review-Url: https://codereview.chromium.org/2928053002
Cr-Commit-Position: refs/heads/master@{#478446}
Committed:
https://chromium.googlesource.com/chromium/src/+/06187f572930bf54ae294079baf9...
==========

[commit-bot: I haz the power, 2017-06-09 23:10:51]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/06187f572930bf54ae294079baf9...