Record the tab state during data use

components/data_use_measurement/core/data_use_measurement.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/data_use_measurement/core/data_use_measurement.h"
 
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
+#include "components/data_use_measurement/core/data_use_ascriber.h"
+#include "components/data_use_measurement/core/data_use_recorder.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/data_use_measurement/core/url_request_classifier.h"
 #include "components/domain_reliability/uploader.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/upload_data_stream.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 
 #if defined(OS_ANDROID)
 #include "net/android/traffic_stats.h"
@@ skipping 37 matching lines @@
       base::TimeDelta::FromHours(1),         // Maximum sample
       50,                                    // Bucket count.
       base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram_pointer->AddCount(latency.InMilliseconds(), count);
 }
 #endif
 
 }  // namespace
 
 DataUseMeasurement::DataUseMeasurement(
+    DataUseAscriber* ascriber,

[RyanSturm, 2016/12/16 16:26:10]

nit: can you order the arguments in the same order as the initialization list;
i.e., move ascriber to be the last argument.

[Raj, 2016/12/16 19:59:10]

Done.

     std::unique_ptr<URLRequestClassifier> url_request_classifier,
     const metrics::UpdateUsagePrefCallbackType& metrics_data_use_forwarder)
     : url_request_classifier_(std::move(url_request_classifier)),
-      metrics_data_use_forwarder_(metrics_data_use_forwarder)
+      metrics_data_use_forwarder_(metrics_data_use_forwarder),
+      ascriber_(ascriber)
 #if defined(OS_ANDROID)
       ,
       app_state_(base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES),
       app_listener_(new base::android::ApplicationStatusListener(
           base::Bind(&DataUseMeasurement::OnApplicationStateChange,
                      base::Unretained(this)))),
       rx_bytes_os_(0),
       tx_bytes_os_(0),
       bytes_transferred_since_last_traffic_stats_query_(0),
       no_reads_since_background_(false)
 #endif
 {
+  DCHECK(ascriber_);
   DCHECK(url_request_classifier_);
 }
 
 DataUseMeasurement::~DataUseMeasurement(){};
 
 void DataUseMeasurement::OnBeforeURLRequest(net::URLRequest* request) {
   DataUseUserData* data_use_user_data = reinterpret_cast<DataUseUserData*>(
       request->GetUserData(DataUseUserData::kUserDataKey));
   if (!data_use_user_data) {
     DataUseUserData::ServiceName service_name =
@@ skipping 89 matching lines @@
         time_since_background, bytes);
     if (no_reads_since_background_) {
       no_reads_since_background_ = false;
       IncrementLatencyHistogramByCount(
           is_user_traffic ? "DataUse.BackgroundToFirstDownstream.User"
                           : "DataUse.BackgroundToFirstDownstream.System",
           time_since_background, 1);
     }
   }
 #endif
+
+  if (is_user_traffic) {
+    const auto recorder = ascriber_->GetDataUseRecorderIfExists(&request);

[RyanSturm, 2016/12/16 16:26:10]

nit: s/auto/DataUseRecorder*/

It's a little fuzzy in the style guide whether the method name Get<type>IfExists
is enough to allow auto:
https://google.github.io/styleguide/cppguide.html#auto

[Raj, 2016/12/16 19:59:10]

Done.

+    if (recorder) {
+      RecordTabStateHistogram(
+          dir, new_app_state,
+          recorder->is_visible() ? TabState::FOREGROUND : TabState::BACKGROUND,
+          bytes);
+    }
+  }
 }
 
 void DataUseMeasurement::UpdateDataUsePrefs(
     const net::URLRequest& request) const {
   bool is_connection_cellular =
       net::NetworkChangeNotifier::IsConnectionCellular(
           net::NetworkChangeNotifier::GetConnectionType());
 
   DataUseUserData* attached_service_data = static_cast<DataUseUserData*>(
       request.GetUserData(DataUseUserData::kUserDataKey));
@@ skipping 120 matching lines @@
       "DataUse.MessageSize." + DataUseUserData::GetServiceNameAsString(service),
       message_size);
   if (message_size > 0) {
     IncreaseSparseHistogramByValue(
         GetHistogramName("DataUse.MessageSize.AllServices", dir, app_state,
                          is_connection_cellular),
         service, message_size);
   }
 }
 
+void DataUseMeasurement::RecordTabStateHistogram(
+    TrafficDirection dir,
+    DataUseUserData::AppState app_state,
+    TabState tab_state,
+    int64_t bytes) {
+  if (app_state == DataUseUserData::UNKNOWN || tab_state == TabState::UNKNOWN)
+    return;
+
+  RecordUMAHistogramCount(
+      base::StringPrintf("DataUse.AppTabState.%s.%s",

[RyanSturm, 2016/12/16 16:26:10]

This expression might be a little more readable if you built the histogram
string in steps.

std::string histogram_name = "DataUse.AppTabState.";
histogram_name.append(...)
if (app_state == brackground)
{...}
else
{...}

RecordUMAHistogramCount(histogram_name...)

[Raj, 2016/12/16 19:59:10]

Done.

[RyanSturm, 2016/12/19 19:31:49]

Thanks.

+                         dir == UPSTREAM ? "Upstream" : "Downstream",
+                         app_state == DataUseUserData::BACKGROUND
+                             ? "AppBackground"
+                             : tab_state == TabState::FOREGROUND
+                                   ? "AppForeground.TabForeground"
+                                   : "AppForeground.TabBackground"),
+      bytes);
+}
+
 }  // namespace data_use_measurement