[Android Download] Show a progress bar before backend is loaded

[Android Download] Show a progress bar before backend is loaded

This CL implements a loading view (aka progress bar) in download manager
UI on Android. It is slightly trickier to handle state changes on
Tablets, because when the download ui is initialized, a filter is
already set in the URL. Therefore we cache the filter temporarily in the
manager, and will show it after we load everything.

There are three possible types of backend the UI needs to wait:
incognito downloads, downloads and offline apges. The UI will only claim
it is loaded if all of them are loaed.

TODO(ianwen): add a timeout mechanisim to either the
DownloadLoadingDelegate or to the backend so that if it takes forever to
load one of the backend, users are still able to see the other two.

BUG=616324
Committed: https://crrev.com/36e2d9412a3ec3dcd7b0efbb2e2ecb8b33c0f728
Cr-Commit-Position: refs/heads/master@{#414369}

[Ian Wen, 2016-08-24 21:41:29]

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

[Ian Wen, 2016-08-24 21:41:30]

PTAL

[gone, 2016-08-24 21:54:12]

Move that TODO in your CL message into the code itself.  It's lost, otherwise.

https://codereview.chromium.org/2269353004/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/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:114:
if (mLoadingDelegate.isLoaded()) filter(mLoadingDelegate.getPendingFilter());
swap the ordering of your line and the recordDownloadCountHistograms line below
to keep them together.

https://codereview.chromium.org/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:413:
*/
Eh, this sounds more like a CL comment than a class comment.

how about:
/**
 * Determines when the data from all of the backends has been loaded.
 */

https://codereview.chromium.org/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:415:
public static final int DOWNLOAD_HISTORY_LOADED = 1;
1) Use OffTheRecord to be consistent with the rest of the file.

2) maybe just be explicit?

regular_history_loaded = 0x0001;
incognito_history_loaded = 0x0010;
offline_pages_loaded = 0x0100;
all_loaded = 0x0111;

https://codereview.chromium.org/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:429:
mState |= showIncognito ? 0 : INCOGNITO_HISTORY_LOADED;
be explicit:

mState = showIncognito ? 0 : INCOGNITO_HISTORY_LOADED;

https://codereview.chromium.org/2269353004/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/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java:106:
}
RecyclerView

https://codereview.chromium.org/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java:108:
// long as the adapter changes, we show the recycler view, and hide the loading
view.
As soon as the adapter data changes?

[gone, 2016-08-24 21:55:59]

lgtm % comments and moving the TODO into code somewhere (maybe the
LoadingDelegate class comment)

https://codereview.chromium.org/2269353004/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/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:439:
public boolean isLoaded() {
Javadoc

https://codereview.chromium.org/2269353004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:445:
*/
Cache a filter for when the backends have loaded.

[Ian Wen, 2016-08-25 04:13:35]

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
(right):

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:114:
if (mLoadingDelegate.isLoaded()) filter(mLoadingDelegate.getPendingFilter());
On 2016/08/24 21:54:11, dfalcantara wrote:
> swap the ordering of your line and the recordDownloadCountHistograms line
below
> to keep them together.

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:413:
*/
On 2016/08/24 21:54:11, dfalcantara wrote:
> Eh, this sounds more like a CL comment than a class comment.
> 
> how about:
> /**
>  * Determines when the data from all of the backends has been loaded.
>  */

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:415:
public static final int DOWNLOAD_HISTORY_LOADED = 1;
On 2016/08/24 21:54:11, dfalcantara wrote:
> 1) Use OffTheRecord to be consistent with the rest of the file.
> 
> 2) maybe just be explicit?
> 
> regular_history_loaded = 0x0001;
> incognito_history_loaded = 0x0010;
> offline_pages_loaded = 0x0100;
> all_loaded = 0x0111;

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:429:
mState |= showIncognito ? 0 : INCOGNITO_HISTORY_LOADED;
On 2016/08/24 21:54:11, dfalcantara wrote:
> be explicit:
> 
> mState = showIncognito ? 0 : INCOGNITO_HISTORY_LOADED;

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:439:
public boolean isLoaded() {
On 2016/08/24 21:55:59, dfalcantara wrote:
> Javadoc

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:445:
*/
On 2016/08/24 21:55:59, dfalcantara wrote:
> Cache a filter for when the backends have loaded.

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java
(right):

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java:106:
}
On 2016/08/24 21:54:12, dfalcantara wrote:
> RecyclerView

Done.

https://chromiumcodereview.appspot.com/2269353004/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadManagerUi.java:108:
// long as the adapter changes, we show the recycler view, and hide the loading
view.
On 2016/08/24 21:54:11, dfalcantara wrote:
> As soon as the adapter data changes?

Done.

[Ian Wen, 2016-08-25 05:33:57]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-08-25 05:33:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2269353004/#ps20001
(title: "comments")

[commit-bot: I haz the power, 2016-08-25 05:34:24]

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

[Ian Wen, 2016-08-25 05:36:14]

The CQ bit was unchecked by ianwen@chromium.org

[Ian Wen, 2016-08-25 05:57:20]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-08-25 05:57:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2269353004/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-25 05:57:44]

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, 2016-08-25 07:23:01]

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

[commit-bot: I haz the power, 2016-08-25 07:23:02]

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

[Ian Wen, 2016-08-25 08:22:22]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-08-25 08:22:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2269353004/#ps60001
(title: "fix tests")

[commit-bot: I haz the power, 2016-08-25 08:23:01]

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, 2016-08-25 09:10:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-25 09:14:12]

Description was changed from

==========
[Android Download] Show a progress bar before backend is loaded

This CL implements a loading view (aka progress bar) in download manager
UI on Android. It is slightly trickier to handle state changes on
Tablets, because when the download ui is initialized, a filter is
already set in the URL. Therefore we cache the filter temporarily in the
manager, and will show it after we load everything.

There are three possible types of backend the UI needs to wait:
incognito downloads, downloads and offline apges. The UI will only claim
it is loaded if all of them are loaed.

TODO(ianwen): add a timeout mechanisim to either the
DownloadLoadingDelegate or to the backend so that if it takes forever to
load one of the backend, users are still able to see the other two.

BUG=616324
==========

to

==========
[Android Download] Show a progress bar before backend is loaded

This CL implements a loading view (aka progress bar) in download manager
UI on Android. It is slightly trickier to handle state changes on
Tablets, because when the download ui is initialized, a filter is
already set in the URL. Therefore we cache the filter temporarily in the
manager, and will show it after we load everything.

There are three possible types of backend the UI needs to wait:
incognito downloads, downloads and offline apges. The UI will only claim
it is loaded if all of them are loaed.

TODO(ianwen): add a timeout mechanisim to either the
DownloadLoadingDelegate or to the backend so that if it takes forever to
load one of the backend, users are still able to see the other two.

BUG=616324

Committed: https://crrev.com/36e2d9412a3ec3dcd7b0efbb2e2ecb8b33c0f728
Cr-Commit-Position: refs/heads/master@{#414369}
==========

[commit-bot: I haz the power, 2016-08-25 09:14:13]

Patchset 4 (id:??) landed as
https://crrev.com/36e2d9412a3ec3dcd7b0efbb2e2ecb8b33c0f728
Cr-Commit-Position: refs/heads/master@{#414369}