A Nightly crash occurs when executing "fetch-event-network-error.https.html" wpt test such as |./mach web-platform-tests _mozilla/service-workers/service-worker/fetch-event-network-error.https.html| with today's (7/1) master build The assertion failure shown is as follows: "Assertion failure: ShouldIntercept() (This can only be called on channels that can be intercepted), at /Users/noef/Documents/mozilla-central/netwerk/protocol/http/HttpBaseChannel.cpp:1427" Please find attached the crash report corresponding to this

The issue here is that OverrideSecurityInfo/OverrideURI can be called after the content viewer of the docshell has been destroyed, so our attempt to get the document from the docshell here can always fail. <https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#14077> Therefore we cannot reliably say whether a channel is intercepted in that function, so there is no point in having these ShouldIntercept() checks.

Note that we have seen this in the wild too, see bug 1173378.

Comment on attachment 8628520 [details] [diff] [review] Part 1: Relax the ShouldIntercept checks when overriding JAR channel info https://hg.mozilla.org/integration/mozilla-inbound/rev/9ba6a55ec905

Comment on attachment 8628524 [details] [diff] [review] Part 2: Relax the ShouldIntercept checks when overriding HTTP channel info; r=michal The check was required by Honza (bug #1133763 comment #23).

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #1) > The issue here is that OverrideSecurityInfo/OverrideURI can be called after > the content viewer of the docshell has been destroyed, so our attempt to get > the document from the docshell here can always fail. > <https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell. > cpp#14077> > > Therefore we cannot reliably say whether a channel is intercepted in that > function, so there is no point in having these ShouldIntercept() checks. That's pretty sad.. :( Is there some other way so that we can at that point find out the channel is synthesized? I'd rather keep at least some check.. But if too complicated then let's remove the check/assert for the sake of the goal.

comment 8 ^^^

(In reply to Honza Bambas (:mayhemer) from comment #8) > (In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, > needinfo? me!) from comment #1) > > The issue here is that OverrideSecurityInfo/OverrideURI can be called after > > the content viewer of the docshell has been destroyed, so our attempt to get > > the document from the docshell here can always fail. > > <https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell. > > cpp#14077> > > > > Therefore we cannot reliably say whether a channel is intercepted in that > > function, so there is no point in having these ShouldIntercept() checks. > > That's pretty sad.. :( Is there some other way so that we can at that point > find out the channel is synthesized? I'd rather keep at least some check.. > But if too complicated then let's remove the check/assert for the sake of > the goal. I don't think there is a way to do that. Josh, can you confirm, please?

We could add a separate boolean to HttpBaseChannel and set it from appropriate places in nsHttpChannel and HttpChannelChild, but otherwise I can't think of anything.

Honza, please let me know which approach you would prefer. Thanks!

jdm, if you have time to add a flag then do so. But if that turns out too complicated, let's just remove the assertion.

Comment on attachment 8628524 [details] [diff] [review] Part 2: Relax the ShouldIntercept checks when overriding HTTP channel info; r=michal Review of attachment 8628524 [details] [diff] [review]: ----------------------------------------------------------------- r=honzab, if there is no better solution

(In reply to Honza Bambas (:mayhemer) from comment #13) > jdm, if you have time to add a flag then do so. But if that turns out too > complicated, let's just remove the assertion.

Josh: ping?

This should do the trick.

All yours. :-)

Comment on attachment 8637529 [details] [diff] [review] Add a flag to HttpBaseChannel indicating whether interception is occurring Review of attachment 8637529 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: netwerk/protocol/http/HttpBaseChannel.h @@ +378,5 @@ > > // True if this channel should skip any interception checks > uint32_t mForceNoIntercept : 1; > > + // True if this channel was intercepted could receive a synthesized response. intercepted _and_ could ?

Hi, I've just tested this in m-c (ba43a48d3c52 revision) and there is no a crash/assertion but the test still fails, please see below the details: Summary Harness status: OK Found 1 tests 1 Fail Details *Result **Fail *Test Name **Rejecting the fetch event or using preventDefault() causes a network error assert_equals: expected "PASS" but got "FAIL: prevent-default: expected network error but loaded" *Message **@https://web-platform.test:8443/_mozilla/service-workers/service-worker/fetch-event-network-error.https.html:37:11 promise callback*@https://web-platform.test:8443/_mozilla/service-workers/service-worker/fetch-event-network-error.https.html:24:1 Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1380:20 promise callback*promise_test@https://web-platform.test:8443/resources/testharness.js:526:31 @https://web-platform.test:8443/_mozilla/service-workers/service-worker/fetch-event-network-error.https.html:19:1 Josh, is that being tracked in other bug or should we open a new one?. Thanks!

That sounds like it's worth opening separately.

(In reply to Josh Matthews [:jdm] from comment #23) > That sounds like it's worth opening separately. ok, done. Please see bug 1198230. Thanks!