1376970 - Make sure mRaceCacheWithNetwork is only set when we are actually racing

Status: RESOLVED FIXED

(Core :: Networking, enhancement)

Description

[Valentin Gosu [:valentin] (he/him)]

This flag should not be set when TriggerNetwork is called because we are revalidating, or in other circumstances when we are not actually racing.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: bug1376970.patch

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e70da3068df
Make sure mRaceCacheWithNetwork is only set when we are actually racing. r=michal

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3e70da3068df

Comment 4

[Michal Novotny [:michal]]

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.

Comment 5

[Valentin Gosu [:valentin] (he/him)]

This is kind of a new bug. Filed bug 1382845 for it.