Commit ff77ee2e authored by Jon Mann's avatar Jon Mann Committed by Commit Bot

Wi-Fi Sync: Prevent crash on logout when connected to Instant Tethering.

Previously during logout, when the notification came in to
SyncedNetworkMetricsLogger that the tether network status changed to
disconnected, the pref service was already destroyed.  When the metrics
logger attempted to check the network's metadata to see if it was
eligible for logging, it caused a crash.  This fix prevents querying the
metadata store for non-wifi or tether networks in the first place, it
also performs a null check on the metadata store before querying for
information about eligible networks in case something changes with the
timing of teardown during logout.

Fixed: 1122678
Change-Id: I8d273b4f7c0d1d0ee5f58fcbc6b946743d28456a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2380083
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: default avatarAzeem Arshad <azeemarshad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802415}
parent f33811a2
......@@ -166,9 +166,19 @@ void SyncedNetworkMetricsLogger::NetworkConnectionStateChanged(
}
bool SyncedNetworkMetricsLogger::IsEligible(const NetworkState* network) {
return network &&
NetworkHandler::Get()->network_metadata_store()->GetIsConfiguredBySync(
network->guid());
// Only non-tether Wi-Fi networks are eligible for logging.
if (!network || network->type() != shill::kTypeWifi ||
!network->tether_guid().empty()) {
return false;
}
NetworkMetadataStore* metadata_store =
NetworkHandler::Get()->network_metadata_store();
if (!metadata_store) {
return false;
}
return metadata_store->GetIsConfiguredBySync(network->guid());
}
void SyncedNetworkMetricsLogger::OnConnectErrorGetProperties(
......
......@@ -235,6 +235,18 @@ TEST_F(SyncedNetworkMetricsLoggerTest, RecordTotalCount) {
testing::ElementsAre(base::Bucket(/*min=*/10, /*count=*/1)));
}
TEST_F(SyncedNetworkMetricsLoggerTest, NetworkStatusChange_DuringLogout) {
base::HistogramTester histogram_tester;
const NetworkState* network = CreateNetwork(/*from_sync=*/true);
NetworkHandler::Get()->ShutdownPrefServices();
synced_network_metrics_logger()->NetworkConnectionStateChanged(network);
base::RunLoop().RunUntilIdle();
// Expect that there is no crash, and no Wi-Fi sync histograms recorded.
EXPECT_THAT(histogram_tester.GetTotalCountsForPrefix("Network.Wifi.Synced"),
testing::ContainerEq(base::HistogramTester::CountsMap()));
}
} // namespace sync_wifi
} // namespace chromeos
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment