Closed Bug 1362384 Opened 3 years ago Closed 3 years ago

Remove code to import data from "downloads.sqlite"

Categories

(Toolkit :: Downloads API, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: Paolo, Assigned: Paolo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Priority: -- → P5
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.
Flags: needinfo?(paolo.mozmail)
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.
Flags: needinfo?(paolo.mozmail)
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.
(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.
Depends on: 851471
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.
Flags: needinfo?(frgrahl)
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.
Flags: needinfo?(frgrahl)
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.
Attachment #8864855 - Flags: review?(mak77)
Comment on attachment 8864855 [details]
Bug 1362384 - Remove code to import data from "downloads.sqlite".

https://reviewboard.mozilla.org/r/136546/#review141546
Attachment #8864855 - Flags: review?(mak77) → review+
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec3c143e890
Remove code to import data from "downloads.sqlite". r=mak
https://hg.mozilla.org/mozilla-central/rev/8ec3c143e890
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1428042
Blocks: 890702
You need to log in before you can comment on or make changes to this bug.