1650082 - `HttpChannelChild::OverrideWithSynthesizedResponse` doesn't handle error of `ContinueAsyncOpen()`

Status: RESOLVED INVALID

(Core :: Networking: HTTP, defect, P2)

Description

[Honza Bambas (:mayhemer)]

This has been introduced in Bug 1204254 reviewed by a non-necko peer (!). Ignoring the result can lead (unless I'm missing wider context) to non delivery of OnStartRequest/OnStopRequest which is a violation of the nsIStreamListener contract.

:asuth, do you remember why exactly this (wrong) change has been made?
https://searchfox.org/mozilla-central/diff/33afdbe632455fa9a073f8ba9284ebc0c146977a/netwerk/protocol/http/HttpChannelChild.cpp#3589-3590

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

The MakeScopeExit block in the patch took on the responsibility for dealing with the return value. It did change to call Cancel() instead of AsyncAbort(), but this was explicitly intentional per bkelly's comment at https://bugzilla.mozilla.org/show_bug.cgi?id=1204254#c99 and my understanding of this change is documented at https://bugzilla.mozilla.org/show_bug.cgi?id=1204254#c104.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

I should probably also note that we would like to remove all support for child intercept (which should no longer be used anywhere) from the codebase, which this logic is part of. Bug 1496997 is the tracking bug for this, and :perry is currently slated to work this. It sounds like you might be the right reviewer for this? (Or, if you'd like to help with creating the patches, both Perry and I would glad to help review!)

Comment 3

[Honza Bambas (:mayhemer)]

Andrew, thanks for providing the details! I missed that scopeexit, it's fine then and this is INVALID.