1344822 - downloads.onChanged notification is not firing when I download anything from browser

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P2)

Description

[Anu]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

Registered for downloads.onChanged notification.
Downloaded attachment from email, files from sourceforge.



Actual results:

Registered handler for downloads.onChanged is not getting invoked.


Expected results:

Registered handler should have been invoked.
On the other hand, if from within the extension if I do a downloads.download then the handler is getting invoked.

Comment 1

[Andy McKay]

Shane, could you investigate please?

Comment 2

[Shane Caraveo (:mixedpuppy)]

Tested in Fx52 (current release), Fx50 because I had it around, and Nightly.  Downloading files from sourceforge, the events worked in every version.  I did not test Fx51 (version reported against), so there is a chance something broke during that release.  Please try Fx52.  I tested with the following code in a background script:

browser.downloads.onCreated.addListener(data => {
  console.log("downloads.onCreated...");
  console.log(data);
});
browser.downloads.onChanged.addListener(data => {
  console.log("downloads.onChanged...");
  console.log(data);
});

Comment 3

[Shane Caraveo (:mixedpuppy)]

FYI, also just tested on Fx51 and it is still working.  Would need more details for any further investigation.

Comment 4

[nick]

Reporter here, just going to paste from my original email. Hope thats okay.



I'm having some trouble with WebExtensions in Firefox. I've got some code to track downloads, and it's working in Chrome, but not in FF.
I've boiled the code down to the minimal surface that works in Chrome but not FF:
    function handleChanged(delta) {
        console.log(delta);
    }
 
chrome.downloads.onChanged.addListener(handleChanged);
Whether I use chrome.* or browser.*, the behavior remains the same: it doesn't work. I've tried executing this line:
console.log(chrome.downloads.onChanged.hasListener(handleChanged));
After adding the listener, and it does log a value of true. Oddly enough, adding an onCreated listener works just fine:
    function handleCreated(delta) {
        console.log(delta);
    }
    chrome.downloads.onCreated.addListener(handleCreated);
This yields the expected object dump in the console when a download is started. Unless I'm missing something extremely obvious, I'm inclined to think this is a bug. It is not mentioned to the [incompatibilities list](https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities), and it is [documented by Mozilla](https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/downloads/onChanged). The thing is, I don't see anybody else asking this question online, so I'm inclined to think a bug is unlikely and I'm messing something up.
If it helps, I'm running Firefox 51.0.1 (32-bit) on Windows 7 Enterprise x64 inside of VMWare Workstation on a Windows 7 Enterprise x64 host. I'm loading the extension using the "Load Temporary Add-on" button. It's not a problem with the core manifest or add-on itself, because three other types of listeners are working just fine. The script is running as a background script.
I appreciate any guidance. Thanks!
 
To clarify, the workflow I’m using is: Load Extension -> Relaunch Debugger -> Start download -> Wait for download to complete


My original SO post: http://stackoverflow.com/questions/42545667/browser-downloads-onchanged-addlistener-doesnt-work-in-firefox

Comment 5

[nick]

As for which pages and URL exhibit the problem, I tried on multiple different sites to no avail. I can't remember exactly everything I tried, but I can be quite sure the behavior is not unique to a single page or a certain type of download.

Comment 6

[Shane Caraveo (:mixedpuppy)]

Attachment: download-test.xpi

This is the addon I'm testing with, can you test with it?  Data is dumped to the console.

Comment 7

[Shane Caraveo (:mixedpuppy)]

Nick, if you can test the addon as well, that would be great.

Comment 8

[Shane Caraveo (:mixedpuppy)]

Ok, I've come up with an STR.  This is all on OSX with Fx52.

- use a larger file to download to give you response time (e.g. firefox from the downloads page)

Test 1 (success case):
- initiate download
- click save on save-to dialog
- onCreated happens after the click
- onChanged happens at end of download

Test 2 (Failure case):
- initiate download
- save dialog appears, do not click
- watch a network monitor to see when download is finished
- click save on save-to dialog
- onCreated happens after the click
- onChanged never happens

Test 3 (success case):
- download something, but check the box to "always do this"
- now, initiate the download per above tests
- onCreated happens at download start
- onChanged happens at download finish

Comment 9

[Shane Caraveo (:mixedpuppy)]

aswan to investigate further as he now recalls the possible reason for this.

Nick, Anu, if one of you can verify that the above STRs are the cause of you're issue, that would be great.

Comment 10

[Anu]

Today I upgrade FF to version 52 and tested it and I am not seeing the issue. I could reproduce the issue with FF 51. I believe that as Shane mentioned above, something was broken in FF 51. Unless Nick is seeing the problem in 52 I believe the issue can be marked as resolved/closed.

Comment 11

[nick]

I'm still having the problem on FireFox 52.0. The problem happens for this workflow specifically, though possibly others:

Click Download Link (which opens in new tab) -> Hit "Save File" in dialog -> Wait to finish.

The created event fires, the changed event does not. If I can offer an observation, it's that you are testing on OSX and I'm on Windows. Is it possible that some Windows-specific abstraction is partially responsible for this? Would make sense if there's some code that was common to OSX/Windows in 51 but diverged in 52, explaining why I still see the issue but you don't.

Comment 12

[Andy McKay]

I've got Windows and can't reproduce this yet. But I also don't where you see "Download link" and why it opens in a new tab for you. Can you give me a URL of a page you were testing it on? Also do you have any add-ons currently installed?

Comment 13

[nick]

I don't see "Download Link", what I mean to say is that I click any link that leads to a download.

For instance, I'm doing this on my own website (http://xenobot.net) where I click "download for Windows".

When I say new tab, what I mean is that I'm middle clicking the link, as a force of habit. But, it also happens when I click it normally. New tab can also mean a href with target="_blank", which also exhibits the behavior.

The problem is appearing on all sorts of links, not just the site provided. Anything for sourceforge, for instance.

It is appearing with my own code, with the example code on the documentation pages, with any code I've been asked to try.

Everytime something new fails, I do confirm the same workflow is working in Chrome, which it does.

I'm not sure what else I can provide you with. I'm thinking to maybe just get a FF dev environment going so I can debug the issue myself; that's just such a pain.

Comment 14

[Andrew Swan [:aswan]]

Attachment: Bug 1344822 Remove downloads.onChanged hack

This bug highlights a difference between "legacy" downloads and those
created directly from Downloads.jsm, that the onDownloadCreated event for
legacy downloads comes when the download is already in progress while the
same event for non-legacy downloads comes before the actual transfer starts.
A clumsy workaround was originally used to avoid generating nonsense
onChanged events for non-legacy downloads but if we are just careful to
handle downloads that have not yet started, we don't need the workaround.

Review commit: https://reviewboard.mozilla.org/r/137214/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/137214/

Comment 15

[Shane Caraveo (:mixedpuppy)]

The patch has no test in it, what is the test coverage for this change?

Comment 16

[Andrew Swan [:aswan]]

Sorry, should have mentioned that.  The existing automated tests all pass for me (I'll run on try too, I got sidetracked by today's artifact builds snafu).  I manually tested your STR from comment 8 and the attached patch fixes that case.  We don't have any automated tests for using the downloads api to watch "legacy" downloads, but its a non-trivial task to do so, tracked in bug 1255629.

Comment 17

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8865584 [details]
Bug 1344822 Remove downloads.onChanged hack

https://reviewboard.mozilla.org/r/137214/#review140324

looks reasonable given passing try

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1464933ce7c8
Remove downloads.onChanged hack r=mixedpuppy

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1464933ce7c8