Glue OfflineContentProvider to the download service

Glue OfflineContentProvider to the download service

Add support for OfflineItems showing up in the DownloadNotificationService.
This consists of a few changes:
- Building a glue class between the OfflineContentAggregator and the
SystemDownloadNotifier.
- Adding some properties to DownloadInfo to support the properties
OfflineItem needs ('isTransient' and 'isOpenable').
- Update the SharedPrefs to store the isTransient property.

BUG=691805
Review-Url: https://codereview.chromium.org/2771803004
Cr-Commit-Position: refs/heads/master@{#460636}
Committed: https://chromium.googlesource.com/chromium/src/+/e872b7528ab479490c19761d64a3784d68ae271f

[David Trainor- moved to gerrit, 2017-03-24 00:52:03]

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

[David Trainor- moved to gerrit, 2017-03-24 00:52:04]

ptal thanks!  Some of this might roll back into the content id CL (I found a few
mistakes there).

[qinmin, 2017-03-27 17:38:51]

lgtm % nits

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java
(right):

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java:99:
DownloadInfo info = new DownloadInfo.Builder()
nit: maybe downloadInfo can have a fromeOfflineItem() method to construct this

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java
(right):

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java:24:
public static OfflineContentAggregatorNotificationBridgeUi instance() {
nit: do an assert that this is called on UI thread only? singleton is not thread
safe.

[gone, 2017-03-27 20:44:47]

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

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:696:
* @param isTransient Whether or not clicking on the download should launch
downloads home.
I'm not sure I can logically connect "transient" to "open download home"

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

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java:70:
if (version == 1) return parseFromVersion1(sharedPrefString);
Feels like this should be a switch statement.

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java:256:
static DownloadSharedPreferenceEntry parseFromVersion6(String string) {
Er, is there any reason why this is just copy pastaed basically?  Is it unsafe
to combine them all?

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java
(right):

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java:24:
public static OfflineContentAggregatorNotificationBridgeUi instance() {
Should be synchronized or assert that it's on the UI thread.

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java:42:
if (sBridgeUi == null) return;
Also should be synchronized.

[David Trainor- moved to gerrit, 2017-03-28 18:43:52]

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

[commit-bot: I haz the power, 2017-03-28 18:44:23]

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-03-28 19:02:15]

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

[commit-bot: I haz the power, 2017-03-28 19:02:16]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[David Trainor- moved to gerrit, 2017-03-28 20:04:09]

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

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java:696:
* @param isTransient Whether or not clicking on the download should launch
downloads home.
On 2017/03/27 20:44:47, dfalcantara (load balance plz) wrote:
> I'm not sure I can logically connect "transient" to "open download home"

It's meant to say the download won't live beyond the 'downloading' phase.  I'm
happy to change if you have suggestions though!

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

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java:70:
if (version == 1) return parseFromVersion1(sharedPrefString);
On 2017/03/27 20:44:47, dfalcantara (load balance plz) wrote:
> Feels like this should be a switch statement.

Done.

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntry.java:256:
static DownloadSharedPreferenceEntry parseFromVersion6(String string) {
On 2017/03/27 20:44:47, dfalcantara (load balance plz) wrote:
> Er, is there any reason why this is just copy pastaed basically?  Is it unsafe
> to combine them all?

Pulling out the serialization code for each version makes it really easy to
avoid bugs.  It was getting unwieldy.

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java
(right):

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java:99:
DownloadInfo info = new DownloadInfo.Builder()
On 2017/03/27 17:38:50, qinmin wrote:
> nit: maybe downloadInfo can have a fromeOfflineItem() method to construct this

That's a good idea.  Do you want me to pull over all of the fields that I can or
are you ok with me just porting the ones that I need for the UI?  If I put it on
DownloadInfo it feels like I should build the conversion code for as much as
possible.

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java
(right):

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java:24:
public static OfflineContentAggregatorNotificationBridgeUi instance() {
On 2017/03/27 20:44:47, dfalcantara (load balance plz) wrote:
> Should be synchronized or assert that it's on the UI thread.

Good call guys thanks.  Done.

https://codereview.chromium.org/2771803004/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java:42:
if (sBridgeUi == null) return;
On 2017/03/27 20:44:47, dfalcantara (load balance plz) wrote:
> Also should be synchronized.

Done.

[gone, 2017-03-28 22:36:09]

lgtm

[David Trainor- moved to gerrit, 2017-03-29 00:36:19]

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

[commit-bot: I haz the power, 2017-03-29 00:37:01]

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-03-29 01:11:42]

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

[commit-bot: I haz the power, 2017-03-29 01:11:43]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[David Trainor- moved to gerrit, 2017-03-29 06:19:43]

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

[commit-bot: I haz the power, 2017-03-29 06:20:06]

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-03-29 07:02:48]

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

[commit-bot: I haz the power, 2017-03-29 07:02:49]

Dry run: 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...)

[David Trainor- moved to gerrit, 2017-03-29 16:26:00]

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

[commit-bot: I haz the power, 2017-03-29 16:27:03]

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-03-29 16:27:10]

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-03-29 18:23:40]

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

[commit-bot: I haz the power, 2017-03-29 18:23:41]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[David Trainor- moved to gerrit, 2017-03-29 19:00:27]

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

[commit-bot: I haz the power, 2017-03-29 19:01:36]

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-03-29 19:34:17]

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

[commit-bot: I haz the power, 2017-03-29 19:34:18]

Dry run: 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_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[David Trainor- moved to gerrit, 2017-03-29 19:39:47]

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

[commit-bot: I haz the power, 2017-03-29 19:40:33]

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-03-29 20:04:31]

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

[commit-bot: I haz the power, 2017-03-29 20:04:32]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[David Trainor- moved to gerrit, 2017-03-29 22:08:40]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-29 22:08:41]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org,
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2771803004/#ps150001
(title: "Fix context issue")

[commit-bot: I haz the power, 2017-03-29 22:09:09]

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-03-29 22:42:03]

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

[commit-bot: I haz the power, 2017-03-29 22:42:04]

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

[David Trainor- moved to gerrit, 2017-03-29 23:55:59]

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

[commit-bot: I haz the power, 2017-03-29 23:56:47]

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

[David Trainor- moved to gerrit, 2017-03-30 01:16:16]

The CQ bit was unchecked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-30 01:16:17]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-30 01:16:18]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org,
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2771803004/#ps170001
(title: "Fixing test failure")

[commit-bot: I haz the power, 2017-03-30 01:16:46]

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-03-30 01:59:05]

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

[commit-bot: I haz the power, 2017-03-30 01:59:06]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[David Trainor- moved to gerrit, 2017-03-30 02:56:14]

The CQ bit was checked by dtrainor@chromium.org

[commit-bot: I haz the power, 2017-03-30 02:57: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, 2017-03-30 03:09:52]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1490842574546690,
"parent_rev": "817ab0f404dc89f75187b1a23cd6f5fcc5b93a3a", "commit_rev":
"e872b7528ab479490c19761d64a3784d68ae271f"}

[commit-bot: I haz the power, 2017-03-30 03:11:05]

Description was changed from

==========
Glue OfflineContentProvider to the download service

Add support for OfflineItems showing up in the DownloadNotificationService.
This consists of a few changes:
- Building a glue class between the OfflineContentAggregator and the
SystemDownloadNotifier.
- Adding some properties to DownloadInfo to support the properties
OfflineItem needs ('isTransient' and 'isOpenable').
- Update the SharedPrefs to store the isTransient property.

BUG=691805
==========

to

==========
Glue OfflineContentProvider to the download service

Add support for OfflineItems showing up in the DownloadNotificationService.
This consists of a few changes:
- Building a glue class between the OfflineContentAggregator and the
SystemDownloadNotifier.
- Adding some properties to DownloadInfo to support the properties
OfflineItem needs ('isTransient' and 'isOpenable').
- Update the SharedPrefs to store the isTransient property.

BUG=691805

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

[commit-bot: I haz the power, 2017-03-30 03:11:06]

Committed patchset #10 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/e872b7528ab479490c19761d64a3...