1182095 - Test downloads through Download Manager

Status: RESOLVED DUPLICATE of bug 1187300

(Core :: DOM: Service Workers, defect, P5)

Comment 1

[José Antonio Olivera Ortega [:jaoo]]

I've been poking around and this is what I saw. No call at https://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/nsDownloadManager.cpp#3558 when starting a download. At https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Downloads.jsm#Conversion_from_nsIDownloadManager you can see ""Starting in Firefox for Desktop version 26, the nsIDownloadManager and nsIDownload interfaces are not available anymore"" so the sensitive calls might be at:

https://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadCore.jsm#1910
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadCore.jsm#1954

Anyway I gave it a try manually (downloading an image from a page controlled by a service worker) and it seems there is no service worker interception when downloading anything. Ehsan, does it make sense? Thanks!

Comment 2

[(no longer active)]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #1)
> I've been poking around and this is what I saw. No call at
> https://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/
> nsDownloadManager.cpp#3558 when starting a download.

That code is in nsDownload::Resume.  I guess that means it runs when you resume a download.

> At
> https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/
> Downloads.jsm#Conversion_from_nsIDownloadManager you can see ""Starting in
> Firefox for Desktop version 26, the nsIDownloadManager and nsIDownload
> interfaces are not available anymore"" so the sensitive calls might be at:
> 
> https://mxr.mozilla.org/mozilla-central/source/toolkit/components/
> jsdownloads/src/DownloadCore.jsm#1910
> https://mxr.mozilla.org/mozilla-central/source/toolkit/components/
> jsdownloads/src/DownloadCore.jsm#1954

I don't know what we use for the download manager.  You want to talk to either mak or paolo.

> Anyway I gave it a try manually (downloading an image from a page controlled
> by a service worker) and it seems there is no service worker interception
> when downloading anything. Ehsan, does it make sense? Thanks!

That sounds like a bug which we should fix.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I'm leaving this to paolo that has far more knowledge about it, btw, the old cpp download manager code must be considered deprecated.
Off-hand looks like you're looking at the right place in jsdownloads...

Could you please just give more details about what's your goal and how you are moving to reach it? Cause it's not totally clear from comment 1, at least for me.

Comment 4

[:Paolo Amadini]

(In reply to Marco Bonardo [::mak] from comment #3)
> Could you please just give more details about what's your goal and how you
> are moving to reach it? Cause it's not totally clear from comment 1, at
> least for me.

Indeed, some context would be useful :-)

Comment 5

[:Paolo Amadini]

For downloads on Firefox OS we use Downloads.jsm, feel free to flag me again if you have any question.

Comment 6

[Josh Matthews [:jdm]]

José, the JS code you found is the right place to be investigating this.

Paolo, we're systematically investigating places in the browser that create channels and looking for requests that should be able to be intercepted by a ServiceWorker but aren't. What this means is that for some a.com, if the user requests a.com/file.zip and a.com has a registered ServiceWorker that can intercept the request, the worker is allowed to synthesize a response directly without the request ever reaching the network. This is implemented in the network code by checking each HTTP channel's notificationCallbacks and loadgroup's notificationCallbacks for the nsINetworkInterceptionController interface - if the callbacks/loadgroup are associated with the originating document, everything works as expected. The code at https://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadCore.jsm#1910 creates a channel that doesn't chain to any docshell or document and therefore this interception never occurs.

When is this code called? Is there always a document we can trace the download request to? What happens when we want to restart a download which was originally intercepted, but the user has closed the relevant tab? Can we do something like the special-casing for the ping listener in the docshell code: http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#724 ?

Comment 7

[:Paolo Amadini]

I'm not able to provide a detailed answer right now, but one of the conceptual differences between downloads and generic loads is exactly that the former can be set aside and executed later. They're the first step for a file to "exit" from the browser environment and reach the operating system level. In general, we can apply any logic we want when starting the download, but it may be more difficult later. Some downloads may be unable to be restarted without the original document or Service Worker code loaded.

Comment 8

[Josh Matthews [:jdm]]

The other thing to note about ServiceWorkers is that their lifetime is not tied to a particular document, so they have much more in common with SharedWorkers. Additionally, we have the ability to start a service worker at will if it isn't running, so maybe we could spin it up on download resumption.

Comment 9

[(no longer active)]

Hmm, if the originating document is unavailable when we restart/resume a download, then fixing this would be difficult, since nsINetworkInterceptController::SetInterceptController can only work if we have access to the docshell...  Perhaps we should mark such as downloads as non-resumable, even though that seems sub-optimal?

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

Thank you guys for your comments about this bug. Big stuff here. I filed a new bug for the fix we need first, the channels created by Downloads.jsm should be intercepted. That's bug 1187300. Once that bug is fixed I'll continue working this one.

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

Clearing target milestone until we have a fix for bug 1187300.

Comment 12

[:Paolo Amadini]

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #9)
> Hmm, if the originating document is unavailable when we restart/resume a
> download, then fixing this would be difficult, since
> nsINetworkInterceptController::SetInterceptController can only work if we
> have access to the docshell...  Perhaps we should mark such as downloads as
> non-resumable, even though that seems sub-optimal?

It seems to me that marking these downloads as non-resumable, and disallowing restart when we lose some context we need, would be the way to go. One existing example are attempts to save a complete page to disk, which we incorrectly restart as HTML-only downloads. If interrupted, we should restart them as complete downloads, but disallow that as soon as the document is not available anymore.

Comment 13

[Andrew Overholt [:overholt]]

Let's aim to fix this post-v1

Comment 14

[Jens Stutte [:jstutte]]

Let's track any remaining work here in bug 1187300.