Move DownloadSharedPreferenceEntry handling into another class

Move DownloadSharedPreferenceEntry handling into another class

We might need to consolidate java/native download status later
Moving the java download status into a separate class so we can handle it without UI knowledge

Review-Url: https://codereview.chromium.org/2625493004
Cr-Commit-Position: refs/heads/master@{#442995}
Committed: https://chromium.googlesource.com/chromium/src/+/b322c318a830e539f8ca1695d7f5e8e9fde6f5a7

[qinmin, 2017-01-09 23:45:33]

qinmin@chromium.org changed reviewers:
+ dfalcantara@chromium.org, dtrainor@chromium.org

[qinmin, 2017-01-09 23:45:34]

PTAL

[gone, 2017-01-10 00:41:06]

I don't think you added the class to this CL.

[qinmin, 2017-01-10 05:18:20]

On 2017/01/10 00:41:06, dfalcantara (check queue) wrote:
> I don't think you added the class to this CL.

Sorry, file is now added

[gone, 2017-01-10 19:44:42]

lgtm % comments

https://codereview.chromium.org/2625493004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceHelper.java
(right):

https://codereview.chromium.org/2625493004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceHelper.java:34:
@SuppressFBWarnings("LI_LAZY_INIT") // Findbugs doesn't see this is only UI
thread.
You can avoid the suppression by doing what EmptyTabModel.java does with its
LazyHolder.

https://codereview.chromium.org/2625493004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceHelper.java:36:
ThreadUtils.assertOnUiThread();
You should assert UI thread on everything, in case someone caches the
DownloadSharedPreferenceHelper.

[qinmin, 2017-01-10 22:58:07]

https://codereview.chromium.org/2625493004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceHelper.java
(right):

https://codereview.chromium.org/2625493004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceHelper.java:34:
@SuppressFBWarnings("LI_LAZY_INIT") // Findbugs doesn't see this is only UI
thread.
On 2017/01/10 19:44:42, dfalcantara (check queue) wrote:
> You can avoid the suppression by doing what EmptyTabModel.java does with its
> LazyHolder.

Done.

https://codereview.chromium.org/2625493004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceHelper.java:36:
ThreadUtils.assertOnUiThread();
On 2017/01/10 19:44:42, dfalcantara (check queue) wrote:
> You should assert UI thread on everything, in case someone caches the
> DownloadSharedPreferenceHelper.

Done.

[qinmin, 2017-01-10 22:58:28]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2017-01-10 22:58:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2625493004/#ps40001
(title: "change lazy instance impl")

[commit-bot: I haz the power, 2017-01-10 22:59:04]

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-10 23:01:35]

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

[commit-bot: I haz the power, 2017-01-10 23:01:36]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[qinmin, 2017-01-11 00:39:57]

Patchset #4 (id:60001) has been deleted

[qinmin, 2017-01-11 00:41:15]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2017-01-11 00:41:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2625493004/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-01-11 00:42:20]

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-11 02:06:51]

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

[commit-bot: I haz the power, 2017-01-11 02:06:52]

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

[qinmin, 2017-01-11 08:56:36]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2017-01-11 08:56:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2625493004/#ps100001
(title: "Simplify threading in SystemDownloadNotifier, remove lock")

[commit-bot: I haz the power, 2017-01-11 08:57:06]

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-11 10:59:07]

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

[commit-bot: I haz the power, 2017-01-11 10:59:08]

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...)

[qinmin, 2017-01-11 18:59:02]

Fixed the ui thread checks in java tests, will submit the CL as it is. 

If there are more comments, will fix it in later CLs.

[qinmin, 2017-01-11 18:59:07]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2017-01-11 18:59:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2625493004/#ps120001
(title: "fixing thread check in tests")

[commit-bot: I haz the power, 2017-01-11 18:59:38]

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-11 21:51:48]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1484161147598450,
"parent_rev": "1de27c2c277ada42068c38f4b25826061bd6bcc4", "commit_rev":
"b322c318a830e539f8ca1695d7f5e8e9fde6f5a7"}

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

Description was changed from

==========
Move DownloadSharedPreferenceEntry handling into another class

We might need to consolidate java/native download status later
Moving the java download status into a separate class so we can handle it
without UI knowledge
==========

to

==========
Move DownloadSharedPreferenceEntry handling into another class

We might need to consolidate java/native download status later
Moving the java download status into a separate class so we can handle it
without UI knowledge

Review-Url: https://codereview.chromium.org/2625493004
Cr-Commit-Position: refs/heads/master@{#442995}
Committed:
https://chromium.googlesource.com/chromium/src/+/b322c318a830e539f8ca1695d7f5...
==========

[commit-bot: I haz the power, 2017-01-11 21:52:22]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/b322c318a830e539f8ca1695d7f5...