Closed Bug 1276673 Opened 3 years ago Closed 3 years ago

Adjust |doomOnFailure| argument when calling CloseCacheEntry properly

Categories

(Core :: Networking: Cache, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

Details

(Whiteboard: [necko-active])

Attachments

(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]
v1

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

makes sense
Attachment #8758364 - Flags: review?(mcmanus) → review+
Keywords: checkin-needed
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/86ca8a55d59e
Adjust |doomOnFailure| argument when calling CloseCacheEntry properly, r=michal
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/86ca8a55d59e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.