[Downloads UI] Implement search

[Downloads UI] Implement search

Adds search functionality for the downloads UI. Also refactors code
that is shared with the history UI into the super classes both UIs
rely on.

BUG=658750
Review-Url: https://codereview.chromium.org/2640333003
Cr-Commit-Position: refs/heads/master@{#445190}
Committed: https://chromium.googlesource.com/chromium/src/+/02e089f068c6aef4fb7341e9502211f953406ff2

[Theresa, 2017-01-19 17:55:03]

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

[Theresa, 2017-01-19 17:55:15]

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

[Theresa, 2017-01-19 17:55:15]

ptal (no rush)

[commit-bot: I haz the power, 2017-01-19 17:55:34]

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-01-19 19:09:08]

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

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

Dry run: This issue passed the CQ dry run.

[gone, 2017-01-20 20:13:00]

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/search_toolbar.xml (right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/search_toolbar.xml:22:
android:inputType="textNoSuggestions"
I guess suggestions wouldn't like the domain matching?

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java:36:
* TODO(dfalcantara): Show all non-cancelled downloads.
While you're here, can you remove this TODO?

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:350:
void search(String query) {
Javadoc

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:359:
void onEndSearch() {
Guess this is called when a search is canceled?

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java:312:
RecordUserAction.record(METRICS_PREFIX + "Search");
For native code, PRESUMBIT.py culls the codebase for strings that
RecordUserAction is called for.  We don't currently do that in Java, but it
seems we should at least keep convention and not break apart the metrics with a
prefix.  WDYT?

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java:244:
if (mIsSearching) UiUtils.hideKeyboard(mSearchEditText);
Hiding the keyboard seems like something that should be done when either the
normal view or the selection view are shown.  Would it make sense to call it
from showSelectionView/showNormalView?  It already looks like it's called from
hideSearchView().

https://codereview.chromium.org/2640333003/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapterTest.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapterTest.java:417:
mAdapter.onEndSearch();
onEndSearch and other things like that should be forced to be on the UI thread
to prevent thread thrashing.

[Theresa, 2017-01-20 21:15:14]

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

[commit-bot: I haz the power, 2017-01-20 21:15:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Theresa, 2017-01-20 21:16:18]

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/res/lay...
File chrome/android/java/res/layout/search_toolbar.xml (right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/res/lay...
chrome/android/java/res/layout/search_toolbar.xml:22:
android:inputType="textNoSuggestions"
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> I guess suggestions wouldn't like the domain matching?

Changed this to plain old text as discussed offline.

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java:36:
* TODO(dfalcantara): Show all non-cancelled downloads.
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> While you're here, can you remove this TODO?

Done.

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:350:
void search(String query) {
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> Javadoc

Done.

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:359:
void onEndSearch() {
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> Guess this is called when a search is canceled?

Yes, when the user hits the navigation back button to exit the search view.

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java:312:
RecordUserAction.record(METRICS_PREFIX + "Search");
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> For native code, PRESUMBIT.py culls the codebase for strings that
> RecordUserAction is called for.  We don't currently do that in Java, but it
> seems we should at least keep convention and not break apart the metrics with
a
> prefix.  WDYT?

Ah, I didn't realize it wouldn't check if the strings were concatenated. I
changed it back.

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/selection/SelectableListToolbar.java:244:
if (mIsSearching) UiUtils.hideKeyboard(mSearchEditText);
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> Hiding the keyboard seems like something that should be done when either the
> normal view or the selection view are shown.  Would it make sense to call it
> from showSelectionView/showNormalView?  It already looks like it's called from
> hideSearchView().

I was trying to avoid hiding it multiple times unnecessary, but I don't feel
strongly about leaving it here. Moved to showSelectionView()

https://codereview.chromium.org/2640333003/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapterTest.java
(right):

https://codereview.chromium.org/2640333003/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapterTest.java:417:
mAdapter.onEndSearch();
On 2017/01/20 20:13:00, dfalcantara (load balance plz) wrote:
> onEndSearch and other things like that should be forced to be on the UI thread
> to prevent thread thrashing.

Done.

[Theresa, 2017-01-20 21:24:41]

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

[Theresa, 2017-01-20 21:24:46]

Explicitly handling empty query to in DonwloadsHistoryAdapter now.

[commit-bot: I haz the power, 2017-01-20 21:25:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gone, 2017-01-20 21:45:13]

lgtm

[Theresa, 2017-01-20 21:46:07]

The CQ bit was unchecked by twellington@chromium.org

[Theresa, 2017-01-20 21:46:27]

twellington@chromium.org changed reviewers:
+ holte@google.com

[Theresa, 2017-01-20 21:46:28]

+holte@ for actions.xml

[Steven Holte, 2017-01-20 22:17:26]

holte@chromium.org changed reviewers:
+ holte@chromium.org

[Steven Holte, 2017-01-20 22:17:27]

lgtm

[Theresa, 2017-01-20 22:31:46]

The CQ bit was checked by twellington@chromium.org

[commit-bot: I haz the power, 2017-01-20 22:32:23]

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-01-20 22:39:41]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484951506793980,
"parent_rev": "6e0635ee33f5876d1735a5a49ff88fba5366e245", "commit_rev":
"02e089f068c6aef4fb7341e9502211f953406ff2"}

[commit-bot: I haz the power, 2017-01-20 22:40:12]

Description was changed from

==========
[Downloads UI] Implement search

Adds search functionality for the downloads UI. Also refactors code
that is shared with the history UI into the super classes both UIs
rely on.

BUG=658750
==========

to

==========
[Downloads UI] Implement search

Adds search functionality for the downloads UI. Also refactors code
that is shared with the history UI into the super classes both UIs
rely on.

BUG=658750

Review-Url: https://codereview.chromium.org/2640333003
Cr-Commit-Position: refs/heads/master@{#445190}
Committed:
https://chromium.googlesource.com/chromium/src/+/02e089f068c6aef4fb7341e95022...
==========

[commit-bot: I haz the power, 2017-01-20 22:40:13]

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