1485427 - Add two assertions ensuring that http-on-*request observers are (not) fired at the right times

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement, P3)

Comment 1

[(no longer active)]

Attachment: Add two assertions ensuring that request observers are (not) fired at the right times

Comment 2

[Honza Bambas (:mayhemer)]

Comment on attachment 9003220 [details] [diff] [review]
Add two assertions ensuring that request observers are (not) fired at the right times

Review of attachment 9003220 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +6042,5 @@
>      // Note that running these observers can itself result in the channel
>      // being canceled.  In that case, we accept that cancelation code as
>      // the cause of the cancelation, as if the classification of the channel
>      // would have occurred past this point!
> +    MOZ_ASSERT(!mRequestObserversCalled);

please put this inside CallOnModifyRequestObservers()

Comment 3

[Honza Bambas (:mayhemer)]

and note that needs a full try run before landing.

Comment 4

[(no longer active)]

Attachment: Add two assertions ensuring that request observers are (not) fired at the right times

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 9003239 [details] [diff] [review]
Add two assertions ensuring that request observers are (not) fired at the right times

Review of attachment 9003239 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!

Comment 6

[(no longer active)]

This assertion fires all over the place <https://treeherder.mozilla.org/#/jobs?repo=try&revision=4547cfaf9bcdd3acfa18ff306aefc5531072d9a6> from this call site:

https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/netwerk/protocol/http/nsHttpChannel.cpp#8174

Do you suggest resetting mRequestObserversCalled to false before calling CallOnModifyRequestObservers() there, given that we're effectively retrying there?

Comment 7

[Honza Bambas (:mayhemer)]

(In reply to :Ehsan Akhgari from comment #6)
> This assertion fires all over the place
> <https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=4547cfaf9bcdd3acfa18ff306aefc5531072d9a6> from this
> call site:
> 
> https://searchfox.org/mozilla-central/rev/
> f2ac80ab7dbde5400a3400d463e07331194dec94/netwerk/protocol/http/nsHttpChannel.
> cpp#8174
> 
> Do you suggest resetting mRequestObserversCalled to false before calling
> CallOnModifyRequestObservers() there, given that we're effectively retrying
> there?

Yes!  Thanks.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d6bf0312e08
Add two assertions ensuring that request observers are (not) fired at the right times; r=mayhemer

Comment 9

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/5d6bf0312e08