593553 - nsIDownloadManager.DOWNLOAD_FINISHED onStateChange not triggered when browser.download.manager.scanWhenDone is true

Status: RESOLVED INVALID

(Toolkit :: Downloads API, defect)

Description

[Matthew Turnbull [Bluefang]]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b6pre) Gecko/20100903 Firefox/4.0b6pre
Build Identifier: 

I've been working on an extension that preforms an action (change the files modification time) on the nsIDownloadManager.DOWNLOAD_FINISHED onStateChange event. I had this working on Linux, but (unknowingly) not on Windows.

After some poking around, I noticed that instead of seeing DOWNLOAD_SCANNING instead of DOWNLOAD_FINISHED. So I tried making it so the extension worked for both DOWNLOAD_FINISHED and DOWNLOAD_SCANNING. However, during the DOWNLOAD_SCANNING phase, something was changing the modification time on the file, thus negating the desired effect.

The only work around I can see, currently, is setting browser.download.manager.scanWhenDone to false.

Reproducible: Always

Steps to Reproduce:
1. Install the extension
2. In extension preferences, change logging verbosity to Debug
3. Open the error console
4. Set browser.download.manager.scanWhenDone to true
5. Download a file
6. Set browser.download.manager.scanWhenDone to false
7. Download a file
Actual Results:  
Downloading file http://b.fanfiction.net/static/ficons/feed.png

When browser.download.manager.scanWhenDone is true

PDMTS: Download status changed - 5 (DOWNLOAD_QUEUED)
PDMTS: Download status changed - 7 (DOWNLOAD_SCANNING)

When browser.download.manager.scanWhenDone is false

PDMTS: Download status changed - 5 (DOWNLOAD_QUEUED)
PDMTS: Download status changed - 1 (DOWNLOAD_FINISHED)
PDMTS: Download finished
...

Expected Results:  
When browser.download.manager.scanWhenDone is true, I would expect the following events:

PDMTS: Download status changed - 5 (DOWNLOAD_QUEUED)
PDMTS: Download status changed - 7 (DOWNLOAD_SCANNING)
PDMTS: Download status changed - 1 (DOWNLOAD_FINISHED)
PDMTS: Download finished
...

Comment 1

[Biju]

Conforming...

Comment 2

[Biju]

see issue at bug 178506 comment 239

Comment 3

[Shawn Wilsher :sdwilsh]

The problem here is that your code is using the wrong listener.  onStateChange is about network state.  onDownloadStateChange is about download state changing:
	onDownloadStateChange: function(state, dl) {},
	onStateChange: function(prog, req, flags, status, dl)
	{
		this.log(LOG_DEBUG, "Download status changed - " + dl.state);

onStateChange will happen before onDownloadStateChange, so you'll have to save the request (which you use) until onDownloadStateChange is called.

You also probably want to change lines like this:
if(this.do_QueryInterface(req, CI.nsIHttpChannel))
to just this:
if (req instanceof CI.nsIHttpChannel)
The latter won't throw (which is why I suspect you have the try-catch in the first place).

Comment 4

[Matthew Turnbull [Bluefang]]

From MDC nsIDownloadProgressListener

onDownloadStateChange() : Called when the status of a particular download changes.
onStateChange() : Called when the state of a particular download changes.

It would be nice if the documentation elaborated on the difference between these two and the difference between "status" and "state". I was under the impression that onStateChange was a generic interface that got both network and download events.

And if it is expected that browser.download.manager.scanWhenDone will change the events that are triggered, that should be documented too.


But thanks for the input. I appreciate it.

Comment 5

[Shawn Wilsher :sdwilsh]

(In reply to comment #4)
> It would be nice if the documentation elaborated on the difference between
> these two and the difference between "status" and "state". I was under the
> impression that onStateChange was a generic interface that got both network and
> download events.
It's often hard to write good documentation when you also write or work with the code.  Your brain automatically makes assumptions about things that others may not.  This happens to be why we make MDC a wiki so people can improve the documentation when it's not clear.  It'd be great if you could make that more clear.

Also note that the IDL says to see nsIWebProgressListener, which seems to be missing from the MDC page:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/public/nsIDownloadProgressListener.idl#39

> And if it is expected that browser.download.manager.scanWhenDone will change
> the events that are triggered, that should be documented too.
onStateChange is related to nsIWebProgressListener, whereas onDownloadStateChange is specifically related to downloads.  That whole listener interface is confusing though, so I can see why the disconnect came up.