Cache in CastMediaSinkServiceImpl doesn't save sinks on network changes that aren't disconnected/unknown |
||
Issue descriptionCastMediaSinkServiceImpl only caches the current sink list if the network id is changing from known to disconnected or unknown. It's also possible to see network id changes directly between known networks, in which case CastMediaSinkServiceImpl should still cache the current list of sinks.
,
Nov 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4935983c64668fb4a3d0e1eb8a4fcc4d3933113f commit 4935983c64668fb4a3d0e1eb8a4fcc4d3933113f Author: btolsch <btolsch@chromium.org> Date: Thu Nov 16 18:34:35 2017 Fix cache bugs in CastMediaSinkServiceImpl This change fixes two bugs with caching of media sinks and a potential initialization race that could also affect the cache's performance. The first fix is to allow network ID changes of network1->network2 to cache sinks for network1 rather than only caching for network1->disconnected/unknown. The cache originally used the latter logic because net::NetworkChangeNotifier broadcasts a disconnect between any two network changes. However, DiscoveryNetworkMonitor doesn't necessarily forward this behavior to its observers because it actually calculates the current network ID on every net::NetworkChangeNotifier notification. Not caching in the former case could account for some of the cache's low utilization currently seen in metrics. The second fix is to add CastMediaSinkServiceImpl as a DiscoveryNetworkMonitor::Observer from the correct sequence so it receives notifications on the correct sequence. This in turn exacerbates a race between the first network ID update and CastMediaSinkServiceImpl's AddObserver call. The race was therefore also addressed in this change. Bug: 782788 , 782859 Change-Id: I064bd184508e66f1806e55dc5afe1eea907836ea Reviewed-on: https://chromium-review.googlesource.com/760667 Commit-Queue: Brandon Tolsch <btolsch@chromium.org> Reviewed-by: Derek Cheng <imcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#517134} [modify] https://crrev.com/4935983c64668fb4a3d0e1eb8a4fcc4d3933113f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc [modify] https://crrev.com/4935983c64668fb4a3d0e1eb8a4fcc4d3933113f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.h [modify] https://crrev.com/4935983c64668fb4a3d0e1eb8a4fcc4d3933113f/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
,
Nov 16 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by btolsch@chromium.org
, Nov 8 2017