1000552 - about:downloads content no longer persists across restarts

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Download Manager, defect)

Description

[Bill Gianopoulos [:WG9s]]

2 or three weeks ago in Android nightlies downloads stopped persisting across browser restarts.  Is this an intentional behavior change?

Comment 1

[Aaron Train [:aaronmt]]

No.

Comment 2

[:Paolo Amadini]

The fix probably just involves tweaking shouldPersistDownload based on what you need:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadIntegration.jsm#305

Comment 3

[Christian Ascheberg]

The downloaded files are not accessible from the device anymore once their reference is removed from the Firefox download manager. That is why I think that this is a major problem.

Comment 4

[Aaron Train [:aaronmt]]

Agreed.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: downloadPersist

Agreed. Need to test this, so just setting feedback first. This (I think) persists everything forever. That's consistent with our old behavior and I think is probably our best first start.

I wonder, do you know if we need to migrate the old download data into the new files as well paolo?

Comment 6

[:Paolo Amadini]

Comment on attachment 8414519 [details] [diff] [review]
downloadPersist

(In reply to Wesley Johnston (:wesj) from comment #5)
> Agreed. Need to test this, so just setting feedback first. This (I think)
> persists everything forever. That's consistent with our old behavior and I
> think is probably our best first start.

The only thing to consider is that performance might be affected if the list becomes too long. I'm not familiar with the UI on Android, but on Desktop the download history tended to stay in place forever, as most users never cleared it.

> I wonder, do you know if we need to migrate the old download data into the
> new files as well paolo?

Good point, you'll need to update the import code to take existing completed downloads into account on Android:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadImport.jsm#120

This code runs only once, when the new API is initialized for the first time. You'll need to reset the "browser.download.importedFromSqlite" preference if testing again on the same profile.

Last thing, what is the difference between ANDROID and MOZ_WIDGET_ANDROID? I always forget, and keep asking hoping that someone will move this information to a wiki page sooner or later :-)

Comment 7

[Wesley Johnston (:wesj)]

> Last thing, what is the difference between ANDROID and MOZ_WIDGET_ANDROID? I
> always forget, and keep asking hoping that someone will move this
> information to a wiki page sooner or later :-)

Heh. Since b2g is based on the Android kernel, ANDROID will work for it as well. MOZ_WIDGET_ANDROID is FFAndroid only. I can put that on a wiki, but not sure what page that is....

Comment 8

[James Gilbertson]

FYI, notifications will come back after a restart. I do have a WIP patch that adds tracking to remember if a notification is dismissed. Should I attach it?

Comment 9

[:Paolo Amadini]

(In reply to James Gilbertson from comment #8)
> FYI, notifications will come back after a restart. I do have a WIP patch
> that adds tracking to remember if a notification is dismissed. Should I
> attach it?

I'd be interested in the approach, though this might be eventually solved in a separate bug. Note that if this happens only because of onDownloadAdded being triggered for existing downloads, maybe you can also choose to ignore onDownloadAdded notifications until the promise returned by addView is resolved.

Comment 10

[Wesley Johnston (:wesj)]

(In reply to James Gilbertson from comment #8)
> FYI, notifications will come back after a restart. I do have a WIP patch
> that adds tracking to remember if a notification is dismissed. Should I
> attach it?

I finally tested this today and saw that :) I'd love to see that patch too.

Comment 11

[Bill Gianopoulos [:WG9s]]

Another good idea here would be to check for the file still existing before persisting the about:downloads entry.

Comment 12

[Wesley Johnston (:wesj)]

Fixed by backout. Tracking for our next pass at Downloads.jsm, but these are cluttering up my list of bugs.

Comment 13

[:Paolo Amadini]

(In reply to Wesley Johnston (:wesj) from comment #12)
> Fixed by backout. Tracking for our next pass at Downloads.jsm, but these are
> cluttering up my list of bugs.

We've made substantial progress on WebAppRT, our last blocker, when do you think this next pass on Android will take place? As I understand it, all the regressions have a clear and simple solution.

Comment 14

[Wesley Johnston (:wesj)]

As soon as those patches are up and ready, I'd be happy to land it again.

Comment 15

[Flaviu Cos]

The downloads from about:downloads persist across browser restarts.
Verified as fixed in builds:
- 32.0a2 (2014-06-11);
- 31 Beta 1;
Device: Google Nexus 7 (Android 4.4.2).