Closed Bug 1455811 Opened 6 years ago Closed 6 years ago

ReadableStream.cancel() does not cancel the actual download

Categories

(Core :: DOM: Core & HTML, defect)

59 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: me, Assigned: baku)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180405092205

Steps to reproduce:

1) Open some kind of (external) network statistics monitor
2) Set dom.streams.enabled and javascript.options.streams to true in about:config.
3) Go to https://hg.mozilla.org/releases/mozilla-release/
4) Run the following sniplet in the console:
fetch('archive/tip.zip').then(response => response.body.cancel())


Actual results:

Firefox 59 will download the file regardless of the cancelled ReadableStream. Needlessly transfering a potentially large file.


Expected results:

The download should have been cancelled all together. Even if the Fetch specification does not require the browser to stop the actual download, it is most likely a waste of bandwith to let it continue.
With the same steps, Chrome will cancel the actual download, as expected.
Has Regression Range: --- → irrelevant
Has STR: --- → yes
Component: Untriaged → DOM
Product: Firefox → Core
What about AbortController?  That should work today even without the streams prefs.

We should leave this open on fetch body stream.
Please see my question in comment 1.
Flags: needinfo?(me)
(In reply to Ben Kelly [:bkelly] from comment #1)

Not sure what you are asking, using the abort controller is not related to the bug that Firefox will silently continue the unnecessary download, is it?

Anyway, using the abort controller does stops the download. (So does navigating away from the page by the way.)
Also, using the abort controller together with body.cancel() results in a total crash of the tab ("Gah. Your tab just crashed."), when done like this:
var controller = new AbortController();
fetch('archive/tip.zip', {signal: controller.signal}).then(response => response.body.cancel());
setTimeout(_ => controller.abort(), 1000);
Flags: needinfo?(me)
Andrea, any thoughts on the crashing steps in comment 3?
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
The issue here was that the mInputStream was not created yet, and FetchStream still have mOriginalInputStream when CancelCallback is executed. In order to stop the networking operation, the mOriginalInputStream (a pipe) must be closed.
Attachment #8970531 - Flags: review?(bkelly) → review+
When Fetch::Abort() is called, maybe the stream has been nullified already.
Attachment #8970540 - Flags: review?(bkelly)
Attachment #8970540 - Flags: review?(bkelly) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/21d58d5d3819
ReadableStream.cancel() must cancel the actual download - part 1, r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/1723fb2d2d83
ReadableStream.cancel() must cancel the actual download - part 2, r=bkelly
https://hg.mozilla.org/mozilla-central/rev/21d58d5d3819
https://hg.mozilla.org/mozilla-central/rev/1723fb2d2d83
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.