1362384 - Remove code to import data from "downloads.sqlite"

Status: RESOLVED FIXED

(Toolkit :: Downloads API, enhancement, P5)

Description

[:Paolo Amadini]

The migration from "downloads.sqlite" to "downloads.json" was implemented in bug 851466 for Firefox 26. Presumably, this is not needed anymore.

I don't think there is an automatic update path directly from these old versions, without offering intermediate versions first, but regardless I think it's acceptable that in-progress downloads from a four years old profile may be lost when upgrading directly.

Comment 1

[:Paolo Amadini]

Attachment: Bug 1362384 - Remove code to import data from "downloads.sqlite".

When upgrading an old profile that still uses "downloads.sqlite", information about in-progress and paused downloads will be lost. The history of completed downloads will be preserved because it is stored in the Places database, although it may be affected by history expiration.

Review commit: https://reviewboard.mozilla.org/r/136546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136546/

Comment 2

[Marco Bonardo [:mak]]

you said Seamonkey plans to move to jsdownloads, it sounds like they may want to have this migration around when that will happen.
Maybe we should make this depend on bug 888915 and wait for that. It should not be a long time if bug 851471 lands.

Comment 3

[:Paolo Amadini]

I thought about whether we should do that, but I noticed that the module that performs the migration is actually self-contained, and can be easily restored as an upgrade step in comm-central.

Given that future SeaMonkey versions might track ESR releases, which makes it unclear for how long we'd need to retain this code, I think it's simpler if we remove the module now and let it be readded to comm-central as needed.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8864855 [details]
Bug 1362384 - Remove code to import data from "downloads.sqlite".

https://reviewboard.mozilla.org/r/136546/#review140174

(In reply to :Paolo Amadini from comment #3)
> I thought about whether we should do that, but I noticed that the module
> that performs the migration is actually self-contained, and can be easily
> restored as an upgrade step in comm-central.

It's unlikely to happen, the SM resources from what I got are _very_ limited and mostly trying to keep the product alive.

> Given that future SeaMonkey versions might track ESR releases, which makes
> it unclear for how long we'd need to retain this code, I think it's simpler
> if we remove the module now and let it be readded to comm-central as needed.


On the other side, our cost here is very small, and their code is likely to stop at ~56, considered 57 is getting rid of xul add-ons.
Personally I'd prefer to retain DownloadImport.jsm and the code in DownloadIntegration.jsm (except for downloads.rdf references!) at least until 57. All the rest can go even now.

::: toolkit/xre/nsXREDirProvider.cpp
(Diff revision 1)
>        else {
>          rv = file->AppendNative(NS_LITERAL_CSTRING("localstore.rdf"));
>          ensureFilePermissions = true;
>        }
>      }
> -    else if (!strcmp(aProperty, NS_APP_DOWNLOADS_50_FILE)) {

This depends on bug 851471 landing first since it's used in nsDownloadManager.cpp, please clarify the dependencies.

Comment 5

[:Paolo Amadini]

(In reply to Marco Bonardo [::mak] from comment #4)
> This depends on bug 851471 landing first since it's used in
> nsDownloadManager.cpp, please clarify the dependencies.

Good catch, I didn't actually notice that. I'll add the dependency.

Comment 6

[:Paolo Amadini]

Frank, how would you feel about adding DownloadImport.jsm to your WIP patch for bug 888915? As I said in comment 2, I'd like to remove the migration code from central. Marco expressed some concerns, but honestly I think it would be quite easy... if it turns out to be difficult then I can keep it around, but I'd like to hear what you think.

Comment 7

[Frank-Rainer Grahl (:frg)]

Paolo,

thanks for asking. Just remove it. I will put the import in suite. Better do not block anything on bug 888915. I will see that I resolve it as fast as I can after the old download manager has been removed so that SeaMonkey is not broken too long. Makes testing easier in this case if no old parts remain in m-c and I don't have to juggle with private patches which removes stuff there too.

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 8864855 [details]
Bug 1362384 - Remove code to import data from "downloads.sqlite".

https://reviewboard.mozilla.org/r/136546/#review141168

Considered the status of bug 851471, I'd be fine to land this if you strip the code used by nsDownloadManager and move that removal to bug 851471.

Comment 9

[:Paolo Amadini]

Comment on attachment 8864855 [details]
Bug 1362384 - Remove code to import data from "downloads.sqlite".

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136546/diff/1-2/

Comment 10

[Marco Bonardo [:mak]]

Comment on attachment 8864855 [details]
Bug 1362384 - Remove code to import data from "downloads.sqlite".

https://reviewboard.mozilla.org/r/136546/#review141546

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec3c143e890
Remove code to import data from "downloads.sqlite". r=mak

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ec3c143e890