1474637 - Make DownloadCore.jsm responsible for adding all downloads, rather than sharing with nsExternalHelperAppService

Status: RESOLVED FIXED

(Toolkit :: Downloads API, enhancement)

Description

[Mark Banner (:standard8)]

Currently DownloadCore.jsm adds downloads to the history, except when they are via nsExternalHelperAppService, in which case, the service does it.

We should just consolidate this so that all download saving to history happens in DownloadCore.jsm. This also allows us to remove the only c++ consumer of nsIDownloadHistory::addDownload which will help with bug 1468980.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1474637 - Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService.

MozReview-Commit-ID: GRzLlDPPsCU

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

(FWIW, not asking review in bugzilla is easy for me to not ever see the review request.)

Comment 3

[Phabricator Automation]

Comment on attachment 8991056 [details]
Bug 1474637 - Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService.

Olli Pettay [:smaug] has approved the revision.

https://phabricator.services.mozilla.com/D2053

Comment 4

[Phabricator Automation]

Comment on attachment 8991056 [details]
Bug 1474637 - Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService.

Olli Pettay [:smaug] has been removed from the revision.

https://phabricator.services.mozilla.com/D2053

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8991056 [details]
Bug 1474637 - Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService.

this had r+ from me, but phabricator decided to remove it :/

Comment 6

[Phabricator Automation]

Comment on attachment 8991056 [details]
Bug 1474637 - Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService.

Olli Pettay [:smaug] has been removed from the revision.

https://phabricator.services.mozilla.com/D2053

Comment 7

[Phabricator Automation]

Comment on attachment 8991056 [details]
Bug 1474637 - Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService.

:Paolo Amadini has approved the revision.

https://phabricator.services.mozilla.com/D2053

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73ddd57de2c0
Make DownloadCore.jsm responsible for adding all downloads to history, rather than sharing with nsExternalHelperAppService. r=paolo

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/73ddd57de2c0