Closed Bug 1276673 Opened 7 years ago Closed 7 years ago

Adjust |doomOnFailure| argument when calling CloseCacheEntry properly


(Core :: Networking: Cache, defect)

Not set



Tracking Status
firefox49 --- fixed


(Reporter: mayhemer, Assigned: mayhemer)


(Whiteboard: [necko-active])


(1 file)

The argument only has effect when we are past processing the response, the channel has been canceled, the entry is new, and the response is non-resumable.  

When set to false it apparently tries to keep entries that are non-resumable in the cache even they could potentially be partial because of premature cancellation of the loading channel.  Those are definitely 304 and redirects.

nsHttpChannel::ContinueHandleAsyncRedirect (false)
 - we can potentially end up with a non-resumable entry here, should be true

nsHttpChannel::HandleAsyncNotModified (true)
 - can be called either after response or before and we should IMO preserve the entry in the cache in what ever state it is
 - here we usually reuse an existing entry (not new) so one of the conditions to doom on true is not met anyway
 - turn to false

nsHttpChannel::OnCacheEntryAvailable (true)
 - we are definitely not past InitCacheEntry()
 - hence it has no effect and should be IMO turned to false since it just cases confusion

nsHttpChannel::ContinueBeginConnect (true)
 - called when ContinueBeginConnectWithResult returns a failure
 - the channel is definitely not past InitCacheEntry(), 
 - hence same case as OnCacheEntryAvailable -> turn to false

nsHttpChannel::OnPreflightFailed (true)
 - same case, actually this is called when ContinueConnect() is interrupted with nsCORSListenerProxy::StartCORSPreflight call
 - turn to false
Assignee: nobody → honzab.moz
Whiteboard: [necko-active]
(applies over bug 1271987)
Comment on attachment 8758364 [details] [diff] [review]

Review of attachment 8758364 [details] [diff] [review]:

makes sense
Attachment #8758364 - Flags: review?(mcmanus) → review+
Keywords: checkin-needed
Pushed by
Adjust |doomOnFailure| argument when calling CloseCacheEntry properly, r=michal
Keywords: checkin-needed
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.