Refactor CronetAdaptiveRequestContext to do everything in one method that just returns the network handles (primary and fallback) that should be use for a request. [chromium/src : main]
Erik Ovelius (Gerrit)
Erik Ovelius added 2 comments![]( "Open in Gerrit")
return null;Erik OveliusCould we inline AdaptiveRequestContextcomputeAlternativeNetworkHandle here?
That way we can re-use the call to `mConnectivityManagerWrapper.getAllNetworks` above and we won't have to call into connectivity manager twice. This is preferable because the time it takes to query connectivity manager can have a long tail.
Done
Long memorizedFallback = mAdaptiveRequestContext.getFallbackNetworkHandle(url);Erik OveliusAlongside my other comment, we could go as far as making `AdaptiveRequestContext.getFallbackNetworkHandle` (renaming it to `getNetworkHandles`/`getPrimaryAndFallbackNetworkHandles`) return a pair of network handles: primary and fallback.
Though, this fells mainly editorial, so it's up to you.
Excellent comment, this simplifies things a lot. Done.
getPrimaryAndFallbackNetworkHandles ->computeStreamNetworkHandles though.
(I general a think a get* method should be a vanilla getter and not do too much work).
PTAL.
Related details
- Stefano Duo
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |