Move first call to RecomputeCurrentConnectionType to background thread [chromium/src : main]
Chris Davis (Gerrit)
Message from Chris Davis![]( "Open in Gerrit")
Set Ready For Review
Related details
- Chris Davis
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nick Harper (Gerrit)
Nick Harper added 1 comment![]( "Open in Gerrit")
(I'm a Security Shepherd today thru Friday, so I probably won't have time to look at this until next week. The other reviewer, davidben, is also Security Shepherd on the same shift and might also have the same review latency/delay.)
Related details
- Chris Davis
- David Benjamin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chris Davis (Gerrit)
Chris Davis added 1 comment![]( "Open in Gerrit")
Updated code reviewers since Davide and Nick are on Security Shepherd this week.
Related details
- Matt Mueller
- mmenke
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
mmenke (Gerrit)
mmenke added 1 comment![]( "Open in Gerrit")
Is there a chance this could make requests fail with ERR_NETWORK_CHANGED at startup, if we managed to send a request over the wire before we get the list of networks?
Even if we don't, this does mean we'll be broadcasting the change to all observers, and they may restart requests as a result.
Related details
- Chris Davis
- Matt Mueller
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chris Davis (Gerrit)
Chris Davis added 1 comment![]( "Open in Gerrit")
Is there a chance this could make requests fail with ERR_NETWORK_CHANGED at startup, if we managed to send a request over the wire before we get the list of networks?
Even if we don't, this does mean we'll be broadcasting the change to all observers, and they may restart requests as a result.
It does introduce a small window in early startup where the network state is unknown until we calculate the state on a background thread. When the state is known we will notify observers. I can take a look at startup traces to see how impactful this is.
Related details
- Matt Mueller
- mmenke
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chris Davis (Gerrit)
Chris Davis added 1 comment![]( "Open in Gerrit")
Chris DavisIs there a chance this could make requests fail with ERR_NETWORK_CHANGED at startup, if we managed to send a request over the wire before we get the list of networks?
Even if we don't, this does mean we'll be broadcasting the change to all observers, and they may restart requests as a result.
It does introduce a small window in early startup where the network state is unknown until we calculate the state on a background thread. When the state is known we will notify observers. I can take a look at startup traces to see how impactful this is.
The initial computation intentionally uses SetCurrentConnectionType (not NotifyObservers) as the callback. SetCurrentConnectionType only updates the cached value under a lock without notifying any observers, so there's no broadcast and no risk of ERR_NETWORK_CHANGED. Requests in flight will see the type transition silently from CONNECTION_UNKNOWN to the actual type without any observer-driven disruption.