Created attachment 8644547 [details] testcase ###!!! ASSERTION: cancel with non-failure status code: 'NS_FAILED(status)', file netwerk/base/nsInputStreamPump.cpp, line 210
The bug interestes me, and I investigate its history. The problem is here: https://dxr.mozilla.org/mozilla-central/source/dom/base/nsXMLHttpRequest.cpp#1965 When the response type is Blob or MozBlob, it cancel the request with NS_OK state. Which cause the assertion. It's done in Bug 689008. In comment five of that bug, it said that we support "Cancel(NS_OK)". But seems the assertion was exist long before that. I couldn't understand this. Some more info: 1. No one else did "Cancel(NS_OK)" on an XHR request: https://dxr.mozilla.org/mozilla-central/search?q=Cancel%28NS_OK%29 2.Seems that others saw this assertion before, and just ignore it: https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/chrome/test_video_discovery.html?offset=100#125
Duplicate of this bug: 714751
I think we should just drop support for moz-blob once we've implemented the Streams spec.
Depends on: 1128959
Could you check if the assertion is also triggered with "blob" rather than "moz-blob"?
Hm, you're right. But I don't know if the assertion is valid or not. Maybe netwerk peers would have more insight.
Flags: needinfo?(VYV03354) → needinfo?(jduell.mcbugs)
My understanding is that necko clients may use "Cancel(NS_OK)" when they want to tell necko that the data transfer could be stopped (and the network connection could be closed, etc), but that the channel was not to be considered a failure. (in the Blob case it looks like we don't need to serve the actual data immediately? Or we're getting it from elsewhere?) I've never been a huge fan of this semantic--it makes the necko code a little tricker (IIRC there are lots of places where we check mStatus and equate !NS_OK with cancelation/failure). But I'm willing to support it if it's needed. And from a glance at the code in question (now living in XMLHttpRequestMainThread::OnDataAvailable(), it looks like we're only reading from the channel because the nsIStreamListener contract mandates it, and then we cancel. So my hunch is that we should remove the assertion in nsInputStreamPump::Cancel(). Honza, what do you think?
Flags: needinfo?(jduell.mcbugs) → needinfo?(honzab.moz)
(In reply to Jason Duell [:jduell] (needinfo me) from comment #7) > So my hunch is that we should remove the assertion in > nsInputStreamPump::Cancel(). Honza, what do you think? Hard to say. But: https://dxr.mozilla.org/mozilla-central/rev/f4f374622111022d41dd8d5eb9220624135c534a/netwerk/base/nsInputStreamPump.cpp#207 And mAsyncStream->CloseWithStatus expects that too. So, no we can't. the caller has to migrate to NS_ERROR_SOMETHING and just ignore the error in OnStopRequest, if that is the reason why NS_OK was used. Apparently, there is no test for this, since it would be perma-crashing on try.... > now living in XMLHttpRequestMainThread::OnDataAvailable() https://dxr.mozilla.org/mozilla-central/rev/f4f374622111022d41dd8d5eb9220624135c534a/dom/xhr/XMLHttpRequestMainThread.cpp#1785
I mark this bug as duplicate of bug 1337746. I already wrote a patch for XHR. Important to know that this assertion is triggered only when the XHR opens a file: URL and the responseType is blob/moz-blob. About removing moz-blob, I added a telemetry ID in order to know if it's used or not. I'm planning to remove telemetry/moz-blob in 1 release.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1337746
OK, now that we're not going to support it, should we throw an error for Cancel(NS_OK)?
(In reply to Jason Duell [:jduell] (needinfo me) from comment #10) > OK, now that we're not going to support it, should we throw an error for > Cancel(NS_OK)? "throw an error" - like let Cancel(NS_OK) return some NS_ERROR_FAILURE and do nothing? Not sure... maybe add MOZ_ASSERT(NS_FAILED(aStatus)) there (and convert some NS_ASSERTIONS below to it too!) and we are done here.
You need to log in before you can comment on or make changes to this bug.