1439656 - Doom the cache entry from nsHttpChannel::Cancel under the same conditions as within CloseCacheEntry

Status: RESOLVED INVALID

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

Description

[Honza Bambas (:mayhemer)]

This is files as part (1) from https://bugzilla.mozilla.org/show_bug.cgi?id=1395760#c8

I want to keep the entry in the channel (despite doomed), since some code may expect an entry be present on the channel inside OnStopRequest.

Note that we do close and doom the entry when it's been written to (=initialized) and is not resumable on mStatus of the channel being a failure after we have called OnStopRequest on the listener.

But for sake of (1) we need to doom the entry a bit sooner, since following consumers for the same entry (a new channel) can be created and asking for an entry sooner the canceled channel reaches its onstoprequest call.

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: wip (breaks a lot of stuff)

the comment in nsHttpChannel::Cancel is likely outdated and imprecise.  I'm not sure it's that easy to doom in cancel...

I'm no longer sure the concern (1) at https://bugzilla.mozilla.org/show_bug.cgi?id=1395760#c8 is valid, because the scenario happens quite often and we would probably already know we serve a broken content on charset reload.  not sure, tho...

Comment 2

[Honza Bambas (:mayhemer)]

(In reply to Honza Bambas (:mayhemer) from comment #0)
> This is files as part (1) from
> https://bugzilla.mozilla.org/show_bug.cgi?id=1395760#c8
> 
> I want to keep the entry in the channel (despite doomed), since some code
> may expect an entry be present on the channel inside OnStopRequest.
> 
> Note that we do close and doom the entry when it's been written to
> (=initialized) and is not resumable on mStatus of the channel being a
> failure after we have called OnStopRequest on the listener.
> 
> But for sake of (1) we need to doom the entry a bit sooner, since following
> consumers for the same entry (a new channel) can be created and asking for
> an entry sooner the canceled channel reaches its onstoprequest call.

OK, this IS INVALID.  the second channel will see that the entry is being written and not resumable and will wait for the entry completion prior continuing, https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/netwerk/protocol/http/nsHttpChannel.cpp#4043, or bypass for top level loads (charset reload falls to that category), https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/netwerk/protocol/http/nsHttpChannel.cpp#4036.  

the entry will be doomed prior releasing from the first channel, hence, the follower will not get an entry at all (bug 923688) in the wait-for-complete case

Comment 3

[Honza Bambas (:mayhemer)]

logs in bug 1395760 prove this.