Histogram for UMA log upload connection type.

chrome/browser/metrics/chrome_metrics_service_client.cc

Index: chrome/browser/metrics/chrome_metrics_service_client.cc
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index c68d2bf7b816bb4688c11c63d1dc4ad2a3f3648f..e7904d7792cce52eb289404df712c6c5ebb46ebf 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"

[Ilya Sherman, 2015/03/24 21:55:44]

nit: No longer needed?

[gayane -on leave until 09-2017, 2015/03/25 21:02:33]

Done.

 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string16.h"
@@ -123,6 +124,7 @@ ChromeMetricsServiceClient::ChromeMetricsServiceClient(
     metrics::MetricsStateManager* state_manager)
     : metrics_state_manager_(state_manager),
       chromeos_metrics_provider_(NULL),
+      network_metrics_provider_(NULL),
       waiting_for_collect_final_metrics_step_(false),
       num_async_histogram_fetches_in_progress_(0),
       weak_ptr_factory_(this) {
@@ -205,7 +207,15 @@ std::string ChromeMetricsServiceClient::GetVersionString() {
   return version;
 }
 
-void ChromeMetricsServiceClient::OnLogUploadComplete() {
+void ChromeMetricsServiceClient::OnLogUploadComplete(bool upload_succeeded) {
+  // Record the network connection type.
+  if (upload_succeeded) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "UMA.LogUpload.ConnetionType",
+        network_metrics_provider_->GetConnectionType(),
+        metrics::SystemProfileProto::Network::CONNECTION_BLUETOOTH + 1);

[Ilya Sherman, 2015/03/24 21:55:44]

Rather than using the enum type from the protocol buffer, which is really not
designed to be used in this way, how about skipping the
network_metrics_provider_ and just calling
net::NetworkChangeNotifier::GetConnectionType() directly?  In fact, you could
probably do this from the metrics component's code, rather than needing to
change the clients.

Actually, it might also be worth defining a separate enum specifically for
backing this histogram, since (a) the protobuf isn't guaranteed to preserve
CONNECTION_BLUETOOTH as the final element, and (b) the net layer code isn't
guaranteed to treat its enum as append-only.

[gayane -on leave until 09-2017, 2015/03/25 21:02:33]

For the enum eventually I have used net::NetworkChangeNotifier::ConnectionType.

+  }
+
   // Collect network stats after each UMA upload.
   network_stats_uploader_.CollectAndReportNetworkStats();
 }
@@ -267,10 +277,8 @@ ChromeMetricsServiceClient::CreateUploader(
 
 base::TimeDelta ChromeMetricsServiceClient::GetStandardUploadInterval() {
 #if defined(OS_ANDROID) || defined(OS_IOS)
-  bool is_cellular = false;
-  cellular_callback_.Run(&is_cellular);
-
-  if (is_cellular && IsCellularEnabledByExperiment())
+  if (network_metrics_provider_->IsCellularConnection() &&
+      IsCellularEnabledByExperiment())
     return base::TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds);
 #endif
   return base::TimeDelta::FromSeconds(kStandardUploadIntervalSeconds);
@@ -303,11 +311,11 @@ void ChromeMetricsServiceClient::Initialize() {
       scoped_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
-  scoped_ptr<metrics::NetworkMetricsProvider> network_metrics_provider(
-      new metrics::NetworkMetricsProvider(
-          content::BrowserThread::GetBlockingPool()));
-  cellular_callback_ = network_metrics_provider->GetConnectionCallback();
-  metrics_service_->RegisterMetricsProvider(network_metrics_provider.Pass());
+
+  network_metrics_provider_ = new metrics::NetworkMetricsProvider(
+      content::BrowserThread::GetBlockingPool());
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::NetworkMetricsProvider>(network_metrics_provider_));
 
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(new OmniboxMetricsProvider));
@@ -316,8 +324,8 @@ void ChromeMetricsServiceClient::Initialize() {
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(new metrics::GPUMetricsProvider()));
 
-  profiler_metrics_provider_ =
-      new metrics::ProfilerMetricsProvider(cellular_callback_);
+  profiler_metrics_provider_ = new metrics::ProfilerMetricsProvider(
+      network_metrics_provider_->GetConnectionCallback());
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(profiler_metrics_provider_));