In HttpChannelChild.cpp, we're hitting the |return false;| here rv = mListener->OnDataAvailable(this, mListenerContext, stringStream, offset, count); stringStream->Close(); if (NS_FAILED(rv)) { // TODO: Cancel request: see OnStartRequest. Bug 536317 return false; } This is caused by rv = mImage->Init(this, mContentType.get(), containerFlags); if (NS_FAILED(rv)) { // Probably bad mimetype // There's no reason to keep the image around. Save memory. // // XXXbholley - This is also here because I'm not sure we've found // all the consumers who (incorrectly) check whether the container // is null to determine things like size availability (they should // be checking the image status instead). mImage = nsnull; this->Cancel(rv); return NS_BINDING_ABORTED; } in imgRequest.cpp. It apparently doesn't like (gdb) p mContentType $2 = { <nsACString_internal> = { mData = 0x28a91e8 "application/json", mLength = 16, mFlags = 5 }, <No data fields>} Can't say I blame it. bing.com looks to be at fault. However, HttpChannelChild shouldn't be returning false here; this isn't a protocol/transport error, it's just malformed web content. We have to deal with that gracefully. I think we should be returning true in this case.

I would agree, but I don't know if it's really worth making this change versus waiting for the Cancel work to finish up and land (bug 536317). Although I guess now that the child process aborts on any failure, this might have a larger impact.

If bug 540097 gets blocking+, this should as well, since we can't in good conscience turn on strong checking without this bug fixed.

(In reply to comment #1) > I would agree, but I don't know if it's really worth making this change versus > waiting for the Cancel work to finish up and land (bug 536317). Although I > guess now that the child process aborts on any failure, this might have a > larger impact. This is totally orthogonal to Cancel being implemented, unless those patches incidentally change this return value.

(In reply to comment #4) > This is totally orthogonal to Cancel being implemented, unless those patches > incidentally change this return value. Which in fact they do.

I'd rather land Cancel and fix these in that patch. Reopen if there winds up being a time constraint--I agree we should fix this in anything we ship.

IMHO this is a totally bogus dup. Please let's land Cancel() soon, in the passive-aggressive sense.

Reopening, marking as depending on Cancel. Is that better? At least this way we won't forget to test.

No, but we have bigger fish to fry, so let's not worry about it anymore. Onward ho!

I'm all for landing this, as Cancel doesn't seem in any danger of being finished up imminently. These errors are annoying and quite common on some pages.

Will give this another shot. In the interim, I found another |return false;| that this v2 fixes.

Comment on attachment 462828 [details] [diff] [review] HTTP channel listener errors shouldn't be reported as IPC protocol/transport errors, v2 forgot to mark +r