1382845 - We should cancel mNetworkTriggerTimer when we start reading data from the cache

Status: RESOLVED FIXED

(Core :: Networking, enhancement)

Description

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

(In reply to Michal Novotny (:michal) from bug 1376970 comment #4)
> 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.

To avoid this situation occurring, we should mNetworkTriggerTimer when we start reading from the cache.

Comment 1

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

Attachment: We should cancel mNetworkTriggerTimer when we start reading data from the cache

I also changed ReportRcwnStats as mRaceCacheWithNetwork isn't necessarily true when mRaceDelay is.
MozReview-Commit-ID: HOnWemy2YCY

Comment 2

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

Attachment: We should cancel mNetworkTriggerTimer when we start reading data from the cache

Your patch in bug 1382852 has a better fix for the telemetry issue

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4c46cc48c78
We should cancel mNetworkTriggerTimer when we start reading data from the cache. r=michal

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a4c46cc48c78