Make sure mRaceCacheWithNetwork is only set when we are actually racing

RESOLVED FIXED in Firefox 56

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: valentin, Assigned: valentin)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
This flag should not be set when TriggerNetwork is called because we are revalidating, or in other circumstances when we are not actually racing.
(Assignee)

Comment 1

2 years ago
Created attachment 8881993 [details] [diff] [review]
bug1376970.patch
Attachment #8881993 - Flags: review?(michal.novotny)
Attachment #8881993 - Flags: review?(michal.novotny) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 2

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e70da3068df
Make sure mRaceCacheWithNetwork is only set when we are actually racing. r=michal
Keywords: checkin-needed

Comment 3

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3e70da3068df
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
This is IMO wrong. In case of delayed race, mRaceCacheWithNetwork is false until we hit the network, but what if it happens after OnCacheEntryAvailable is called? Imagine following scenario:

1) MaybeRaceCacheWithNetwork triggers the network with a delay, so mRaceCacheWithNetwork==false, mRaceDelay!=0
2) cache wins before the mRaceDelay time elapsed, so mRaceCacheWithNetwork is still false and we won't set mFirstResponseSource in ReadFromCache
3) mRaceDelay time elapsed and nsHttpChannel was notified by the timer, TriggerNetwork(0) is called but mRaceCacheWithNetwork is not set again, because mOnCacheAvailableCalled is already true.
4) Now we start network request when cache already started delivering data, mRaceCacheWithNetwork is false and mFirstResponseSource is RESPONSE_PENDING :-/

We should either change what mRaceCacheWithNetwork means or check it together with mRaceDelay.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 5

a year ago
This is kind of a new bug. Filed bug 1382845 for it.
Status: REOPENED → RESOLVED
Last Resolved: 2 years agoa year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.