Implement UMA log throttling for cellular connections

components/data_use_measurement/content/data_use_measurement.cc

Index: components/data_use_measurement/content/data_use_measurement.cc
diff --git a/components/data_use_measurement/content/data_use_measurement.cc b/components/data_use_measurement/content/data_use_measurement.cc
index 23f916c6ae054c38efb296be694b623aa91cd5ab..8e6ccb1abf59c4e746ccd8409fb8446601c60e09 100644
--- a/components/data_use_measurement/content/data_use_measurement.cc
+++ b/components/data_use_measurement/content/data_use_measurement.cc
@@ -44,9 +44,12 @@ void IncreaseSparseHistogramByValue(const std::string& name,
 
 }  // namespace
 
-DataUseMeasurement::DataUseMeasurement()
+DataUseMeasurement::DataUseMeasurement(
+    const metrics::UpdateUsagePrefCallbackType& metrics_data_use_forwarder)
+    : metrics_data_use_forwarder_(metrics_data_use_forwarder)
 #if defined(OS_ANDROID)
-    : app_state_(base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES),
+      ,
+      app_state_(base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES),
       app_listener_(new base::android::ApplicationStatusListener(
           base::Bind(&DataUseMeasurement::OnApplicationStateChange,
                      base::Unretained(this))))
@@ -75,26 +78,36 @@ void DataUseMeasurement::ReportDataUseUMA(
   int64_t total_upload_bytes = request->GetTotalSentBytes();
   int64_t total_received_bytes = request->GetTotalReceivedBytes();
 
+  bool is_conn_cellular = net::NetworkChangeNotifier::IsConnectionCellular(
+      net::NetworkChangeNotifier::GetConnectionType());
   RecordUMAHistogramCount(
       GetHistogramName(is_user_traffic ? "DataUse.TrafficSize.User"
                                        : "DataUse.TrafficSize.System",
-                       UPSTREAM),
+                       UPSTREAM, is_conn_cellular),
       total_upload_bytes);
   RecordUMAHistogramCount(
       GetHistogramName(is_user_traffic ? "DataUse.TrafficSize.User"
                                        : "DataUse.TrafficSize.System",
-                       DOWNSTREAM),
+                       DOWNSTREAM, is_conn_cellular),
       total_received_bytes);
 
   DataUseUserData* attached_service_data = reinterpret_cast<DataUseUserData*>(
       request->GetUserData(DataUseUserData::kUserDataKey));
-
+  DataUseUserData::ServiceName service_name =
+      attached_service_data ? attached_service_data->service_name()
+                            : DataUseUserData::NOT_TAGGED;
   if (!is_user_traffic) {
-    DataUseUserData::ServiceName service_name =
-        attached_service_data ? attached_service_data->service_name()
-                              : DataUseUserData::NOT_TAGGED;
-    ReportDataUsageServices(service_name, UPSTREAM, total_upload_bytes);
-    ReportDataUsageServices(service_name, DOWNSTREAM, total_received_bytes);
+    ReportDataUsageServices(service_name, UPSTREAM, is_conn_cellular,
+                            total_upload_bytes);
+    ReportDataUsageServices(service_name, DOWNSTREAM, is_conn_cellular,
+                            total_received_bytes);
+  }
+
+  // Update data use prefs for cellular connections.
+  if (is_conn_cellular && !metrics_data_use_forwarder_.is_null()) {
+    metrics_data_use_forwarder_.Run(
+        attached_service_data->GetServiceNameAsString(service_name),
+        total_upload_bytes + total_received_bytes);

[mmenke, 2016/04/01 20:59:35]

This seems like way too many posted tasks for too little gain - one for every
redirect, one for every URLRequest.  Can we just jump over to the IOThread and
grab this whenever we need it, or accumulate data and only periodically post a
task or something?  The are certainly issues related to the fact that this is a
per-profile class.

[Alexei Svitkine (slow), 2016/04/01 21:07:30]

Is that really a lot? I suspect that the ratio of URL requests to posted tasks
in general is already something tiny (i.e. URL requests are very infrequent
compared to posted tasks).

[Alexei Svitkine (slow), 2016/04/04 16:01:00]

(mmenke: Not sure if you saw this reply - as I think you accidentally got
removed from the R list earlier when I sent it.)

[mmenke, 2016/04/04 17:26:23]

You may be right.  I'm generally fairly skeptical of doing significant work for
purely metrics-related reasons.

   }
 }
 
@@ -115,10 +128,9 @@ DataUseMeasurement::AppState DataUseMeasurement::CurrentAppState() const {
 }
 
 std::string DataUseMeasurement::GetHistogramName(const char* prefix,
-                                                 TrafficDirection dir) const {
+                                                 TrafficDirection dir,
+                                                 bool is_conn_cellular) const {
   AppState app_state = CurrentAppState();
-  bool is_conn_cellular = net::NetworkChangeNotifier::IsConnectionCellular(
-      net::NetworkChangeNotifier::GetConnectionType());
   return base::StringPrintf(
       "%s.%s.%s.%s", prefix, dir == UPSTREAM ? "Upstream" : "Downstream",
       app_state == BACKGROUND ? "Background" : "Foreground",
@@ -135,14 +147,16 @@ void DataUseMeasurement::OnApplicationStateChange(
 void DataUseMeasurement::ReportDataUsageServices(
     DataUseUserData::ServiceName service,
     TrafficDirection dir,
+    bool is_conn_cellular,
     int64_t message_size) const {
   RecordUMAHistogramCount(
       "DataUse.MessageSize." + DataUseUserData::GetServiceNameAsString(service),
       message_size);
   if (message_size > 0) {
     IncreaseSparseHistogramByValue(
-        GetHistogramName("DataUse.MessageSize.AllServices", dir), service,
-        message_size);
+        GetHistogramName("DataUse.MessageSize.AllServices", dir,
+                         is_conn_cellular),
+        service, message_size);
   }
 }