🔍 Add basic search widget test

🔍 Add basic search widget test

* Adds a basic test that confirms that what is displayed by the
  widget accurately reflects SharedPreference state.

* Adds a pair of tests that confirm that clicking on the widget
  either starts a text or voice query.

* Adds a Delegate to the SearchWidgetProvider to control what it
  interacts with and how.

* Fixes the IntentFilters for the SearchWidgetProvider.  Not sure
  they're actually necessary, given that we directly send the
  Intent to the class (it was working while this was wrong).

BUG=708844
Review-Url: https://codereview.chromium.org/2819083002
Cr-Commit-Position: refs/heads/master@{#465328}
Committed: https://chromium.googlesource.com/chromium/src/+/493c47dd802534fa89c50c48fc9c6f72622b067d

[gone, 2017-04-15 01:50:16]

dfalcantara@chromium.org changed reviewers:
+ tedchoc@chromium.org

[gone, 2017-04-15 01:50:22]

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

[commit-bot: I haz the power, 2017-04-15 01:50:32]

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-04-15 02:51:32]

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

[commit-bot: I haz the power, 2017-04-15 02:51:32]

Dry run: This issue passed the CQ dry run.

[gone, 2017-04-16 07:09:18]

Description was changed from

==========
🔍 Add basic search widget test

Adds a basic test that confirms that what is displayed by the widget
accurately reflects SharedPreference state.  Adds a Delegate to the
SearchWidgetProvider to control what it interacts with and how.

BUG=708844
==========

to

==========
🔍 Add basic search widget test

* Adds a basic test that confirms that what is displayed by the
  widget accurately reflects SharedPreference state.

* Adds a pair of tests that confirm that clicking on the widget
  either starts a text or voice query.

* Adds a Delegate to the SearchWidgetProvider to control what it
  interacts with and how.

* Fixes the IntentFilters for the SearchWidgetProvider.  Not sure
  they're actually necessary, given that we directly send the
  Intent to the class (it was working while this was wrong).

BUG=708844
==========

[gone, 2017-04-16 07:09:42]

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

[commit-bot: I haz the power, 2017-04-16 07:09:46]

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-04-16 09:00:47]

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

[commit-bot: I haz the power, 2017-04-16 09:00:48]

Dry run: This issue passed the CQ dry run.

[gone, 2017-04-17 00:44:56]

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

[commit-bot: I haz the power, 2017-04-17 00:44:59]

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-04-17 02:37:29]

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

[commit-bot: I haz the power, 2017-04-17 02:37:30]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-04-18 04:33:02]

lgtm w/ style stuff and thangs

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
(right):

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:44:
static class Delegate {
I prefer the names to be fully qualified even internal ones as I find them
imported in other places to be hard to grok with it like this (i.e.
SearchWidgetDelegate)

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:45:
protected final Context mContext;
why protected but provide an accessor?

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:164:
Log.e(TAG, "Caught exception when attempting to launch widget.", e);
Here and below, we should try to find a way to hook into:
https://cs.chromium.org/chromium/src/base/android/java/src/org/chromium/base/...

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:179:
/** Delegates out calls to the correct functions. */
Nit of nits, the use of "Delegates" here makes me think it calls into the
delegate.  I would just say, "Handles the intent actions to the widget" or
something of that sort.\\

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java
(right):

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java:44:
new ArrayList<Pair<Integer, RemoteViews>>();
I think the second part can just be "new ArrayList<>();"

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java:66:
.getTargetContext()
this indentation looks quite wonky...clang format strikes again?

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java:116:
assert mDelegate.mViews.size() == TestDelegate.ALL_IDS.length;
s/assert/assertEquals

[gone, 2017-04-18 17:14:33]

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java
(right):

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:44:
static class Delegate {
On 2017/04/18 04:33:02, Ted C wrote:
> I prefer the names to be fully qualified even internal ones as I find them
> imported in other places to be hard to grok with it like this (i.e.
> SearchWidgetDelegate)

Done.

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:45:
protected final Context mContext;
On 2017/04/18 04:33:02, Ted C wrote:
> why protected but provide an accessor?

Don't even remember at this point.  Fixed.

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:164:
Log.e(TAG, "Caught exception when attempting to launch widget.", e);
On 2017/04/18 04:33:01, Ted C wrote:
> Here and below, we should try to find a way to hook into:
>
https://cs.chromium.org/chromium/src/base/android/java/src/org/chromium/base/...

This is part of the CL it depends on; could you comment there?

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProvider.java:179:
/** Delegates out calls to the correct functions. */
On 2017/04/18 04:33:02, Ted C wrote:
> Nit of nits, the use of "Delegates" here makes me think it calls into the
> delegate.  I would just say, "Handles the intent actions to the widget" or
> something of that sort.\\

Done.

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java
(right):

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java:44:
new ArrayList<Pair<Integer, RemoteViews>>();
On 2017/04/18 04:33:02, Ted C wrote:
> I think the second part can just be "new ArrayList<>();"

Done.

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java:66:
.getTargetContext()
On 2017/04/18 04:33:02, Ted C wrote:
> this indentation looks quite wonky...clang format strikes again?

Yep.  Rejoice.

https://codereview.chromium.org/2819083002/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java:116:
assert mDelegate.mViews.size() == TestDelegate.ALL_IDS.length;
On 2017/04/18 04:33:02, Ted C wrote:
> s/assert/assertEquals

Done.

[gone, 2017-04-18 17:18:56]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2017-04-18 17:18:56]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2819083002/#ps80002
(title: "🔍 Tests")

[commit-bot: I haz the power, 2017-04-18 17:19:28]

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-04-18 19:37:17]

CQ is committing da patch.

Bot data: {"patchset_id": 80002, "attempt_start_ts": 1492535936411920,
"parent_rev": "a53f68744f93a8b322d5b73ed028491dcdd9612e", "commit_rev":
"493c47dd802534fa89c50c48fc9c6f72622b067d"}

[commit-bot: I haz the power, 2017-04-18 19:37:34]

Description was changed from

==========
🔍 Add basic search widget test

* Adds a basic test that confirms that what is displayed by the
  widget accurately reflects SharedPreference state.

* Adds a pair of tests that confirm that clicking on the widget
  either starts a text or voice query.

* Adds a Delegate to the SearchWidgetProvider to control what it
  interacts with and how.

* Fixes the IntentFilters for the SearchWidgetProvider.  Not sure
  they're actually necessary, given that we directly send the
  Intent to the class (it was working while this was wrong).

BUG=708844
==========

to

==========
🔍 Add basic search widget test

* Adds a basic test that confirms that what is displayed by the
  widget accurately reflects SharedPreference state.

* Adds a pair of tests that confirm that clicking on the widget
  either starts a text or voice query.

* Adds a Delegate to the SearchWidgetProvider to control what it
  interacts with and how.

* Fixes the IntentFilters for the SearchWidgetProvider.  Not sure
  they're actually necessary, given that we directly send the
  Intent to the class (it was working while this was wrong).

BUG=708844

Review-Url: https://codereview.chromium.org/2819083002
Cr-Commit-Position: refs/heads/master@{#465328}
Committed:
https://chromium.googlesource.com/chromium/src/+/493c47dd802534fa89c50c48fc9c...
==========

[commit-bot: I haz the power, 2017-04-18 19:37:34]

Committed patchset #8 (id:80002) as
https://chromium.googlesource.com/chromium/src/+/493c47dd802534fa89c50c48fc9c...