Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(500)

Unified Diff: components/metrics/net/network_metrics_provider.cc

Issue 1107613003: [Metrics, Cleanup] Replace a misleading TODO with documentation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/metrics/net/network_metrics_provider.cc
diff --git a/components/metrics/net/network_metrics_provider.cc b/components/metrics/net/network_metrics_provider.cc
index 4ec657435a60e2d5b4e3516cce826d2d42d39dad..776344ff86940cf6ffe356987e53aa39622df2c5 100644
--- a/components/metrics/net/network_metrics_provider.cc
+++ b/components/metrics/net/network_metrics_provider.cc
@@ -49,10 +49,13 @@ void NetworkMetricsProvider::ProvideSystemProfileMetrics(
wifi_phy_layer_protocol_is_ambiguous_);
network->set_wifi_phy_layer_protocol(GetWifiPHYLayerProtocol());
- // Resets the "ambiguous" flags, since a new metrics log session has started.
- connection_type_is_ambiguous_ = false;
- // TODO(isherman): This line seems unnecessary.
+ // Update the connection type. Note that this is necessary to set the network
+ // type to "none" if there is no network connection for an entire UMA logging
+ // window, since OnConnectionTypeChanged() ignores transitions to the "none"
+ // state.
connection_type_ = net::NetworkChangeNotifier::GetConnectionType();
+ // Reset the "ambiguous" flags, since a new metrics log session has started.
+ connection_type_is_ambiguous_ = false;
wifi_phy_layer_protocol_is_ambiguous_ = false;
if (!wifi_access_point_info_provider_.get()) {
@@ -73,8 +76,15 @@ void NetworkMetricsProvider::ProvideSystemProfileMetrics(
void NetworkMetricsProvider::OnConnectionTypeChanged(
net::NetworkChangeNotifier::ConnectionType type) {
+ // To avoid reporting an ambiguous connection type for users on flaky
+ // connections, ignore transitions to the "none" state. Note that the
+ // connection type is refreshed in ProvideSystemProfileMetrics() each time a
+ // new UMA logging window begins, so users who genuinely transition to offline
+ // mode for an extended duration will still be at least partially represented
+ // in the metrics logs.
if (type == net::NetworkChangeNotifier::CONNECTION_NONE)
return;
+
if (type != connection_type_ &&
connection_type_ != net::NetworkChangeNotifier::CONNECTION_NONE) {
connection_type_is_ambiguous_ = true;
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698