Delete download in Android DownloadManager when download is deleted from download home

Delete download in Android DownloadManager when download is deleted from download home

This change adds a SharedPreference file to map download GUIDs to android download Ids.
When a download is removed from download home, also remove it from Android DM.

BUG=658829
Committed: https://crrev.com/b318140c9474e8e133bf3a403e94e7569c9bd4cd
Cr-Commit-Position: refs/heads/master@{#428495}

[qinmin, 2016-10-26 21:39:56]

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

[qinmin, 2016-10-26 21:39:57]

PTAL

[gone, 2016-10-26 21:49:58]

I'm not terribly familiar with how SQL databases work :/  Does it make sense to
try another reviewer?

[qinmin, 2016-10-26 22:01:11]

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

[qinmin, 2016-10-26 22:01:12]

ianwen@, are you ok with this sqldatabase code review?

[qinmin, 2016-10-28 18:25:17]

qinmin@chromium.org changed reviewers:
- ianwen@chromium.org

[qinmin, 2016-10-28 18:25:17]

Dan, PTAL again, switched to use SharedPreference instead of database

[qinmin, 2016-10-28 18:35:09]

Description was changed from

==========
Delete download in Android DownloadManager when download is deleted from
download home

This change adds a sqlite database to map download GUIDs to android
download Ids.
When a download is removed from download home, also remove it from
Android DM.

BUG=658829
==========

to

==========
Delete download in Android DownloadManager when download is deleted from
download home

This change adds a SharedPreference file to map download GUIDs to android
download Ids.
When a download is removed from download home, also remove it from Android DM.

BUG=658829
==========

[gone, 2016-10-28 18:51:03]

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java
(right):

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java:69:
return downloadId + "," + downloadGuid;
Worth using a StringBuilder?

Does it make sense to just override the toString() method?

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java:94:
private long removeDownloadIdMapping(String downloadGuid) {
Would it make more sense to just use Editor.putLong(downloadGuid, downloadId)
directly instead of abstracting it out with an in-between HashSet?

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java:119:
* frequent, don't leave the SharedPreference entries in the memory after each
use.
Where do we nuke the SharedPreferences from memory?

[qinmin, 2016-10-28 19:21:43]

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java
(right):

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java:69:
return downloadId + "," + downloadGuid;
On 2016/10/28 18:51:02, dfalcantara (slow 10.24) wrote:
> Worth using a StringBuilder?
> 
> Does it make sense to just override the toString() method?

this is no longer needed.

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java:94:
private long removeDownloadIdMapping(String downloadGuid) {
On 2016/10/28 18:51:02, dfalcantara (slow 10.24) wrote:
> Would it make more sense to just use Editor.putLong(downloadGuid, downloadId)
> directly instead of abstracting it out with an in-between HashSet?

Good point, the code looks much clean now

https://codereview.chromium.org/2456593002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java:119:
* frequent, don't leave the SharedPreference entries in the memory after each
use.
On 2016/10/28 18:51:02, dfalcantara (slow 10.24) wrote:
> Where do we nuke the SharedPreferences from memory?

Rephrased the description. We are not nuking the SharedPreferences, but just
don't load everything into a hashmap as did in DownloadNotificationService.

[qinmin, 2016-10-28 19:23:20]

Patchset #4 (id:60001) has been deleted

[gone, 2016-10-28 19:55:20]

lgtm

[qinmin, 2016-10-28 20:47:58]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2016-10-28 20:48:25]

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-10-28 21:19:20]

Description was changed from

==========
Delete download in Android DownloadManager when download is deleted from
download home

This change adds a SharedPreference file to map download GUIDs to android
download Ids.
When a download is removed from download home, also remove it from Android DM.

BUG=658829
==========

to

==========
Delete download in Android DownloadManager when download is deleted from
download home

This change adds a SharedPreference file to map download GUIDs to android
download Ids.
When a download is removed from download home, also remove it from Android DM.

BUG=658829
==========

[commit-bot: I haz the power, 2016-10-28 21:19:21]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-10-28 21:23:27]

Description was changed from

==========
Delete download in Android DownloadManager when download is deleted from
download home

This change adds a SharedPreference file to map download GUIDs to android
download Ids.
When a download is removed from download home, also remove it from Android DM.

BUG=658829
==========

to

==========
Delete download in Android DownloadManager when download is deleted from
download home

This change adds a SharedPreference file to map download GUIDs to android
download Ids.
When a download is removed from download home, also remove it from Android DM.

BUG=658829

Committed: https://crrev.com/b318140c9474e8e133bf3a403e94e7569c9bd4cd
Cr-Commit-Position: refs/heads/master@{#428495}
==========

[commit-bot: I haz the power, 2016-10-28 21:23:29]

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