Remove code to import data from "downloads.sqlite"

RESOLVED FIXED in Firefox 55

Status

()

P5
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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)

Updated

2 years ago
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Priority: -- → P5
Comment hidden (mozreview-request)
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)
(Assignee)

Comment 3

2 years ago
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 4

2 years ago
mozreview-review
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.
(Assignee)

Comment 5

2 years ago
(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.
(Assignee)

Updated

2 years ago
Depends on: 851471
(Assignee)

Comment 6

2 years ago
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 8

2 years ago
mozreview-review
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 hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
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+

Comment 11

2 years ago
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

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8ec3c143e890
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Updated

a year ago
Depends on: 1428042
Blocks: 890702
You need to log in before you can comment on or make changes to this bug.