Support needed to measure user and service traffic in Chrome.

chrome/browser/net/data_use_measurement.cc

Index: chrome/browser/net/data_use_measurement.cc
diff --git a/chrome/browser/net/data_use_measurement.cc b/chrome/browser/net/data_use_measurement.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6d098526a855c0e46256f15533965b3de806c168
--- /dev/null
+++ b/chrome/browser/net/data_use_measurement.cc
@@ -0,0 +1,168 @@
+// 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 "chrome/browser/net/data_use_measurement.h"
+
+#include <string>
+
+#include "base/hash.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/net/data_use_user_data.h"
+#include "content/public/browser/resource_request_info.h"
+#include "net/base/network_change_notifier.h"
+#include "net/base/upload_data_stream.h"
+#include "net/url_request/url_request.h"
+
+#if defined(OS_ANDROID)
+#include "base/android/application_status_listener.h"
+#endif
+
+// This macro increases the value of |sample| bucket in |name| sparse histogram
+// by |value|. This Macro can be called with different names.
+#define UMA_HISTOGRAM_SPARSE_SLOWLY_WITH_VALUE(name, sample, value)     \
+  do {                                                                  \

[sclittle, 2015/08/19 23:32:39]

Why does this need to be a macro? Could this just be a function?

[amohammadkhan, 2015/08/21 23:06:31]

Done.

+    base::HistogramBase* histogram = base::SparseHistogram::FactoryGet( \
+        name, base::HistogramBase::kUmaTargetedHistogramFlag);          \
+    histogram->AddCount(sample, value);                                 \
+  } while (0)
+
+// Copied from chromecast/base/metrics/cast_histograms.h
+// TODO: This macro can be unified with the cast_histogram.h macros.
+#define STATIC_HISTOGRAM_POINTER_BLOCK_NO_CACHE(                               \

[sclittle, 2015/08/19 23:32:39]

Same here, why a macro and not just a function?

[amohammadkhan, 2015/08/21 23:06:31]

Done.

+    constant_histogram_name, histogram_add_method_invocation,                  \
+    histogram_factory_get_invocation)                                          \
+  do {                                                                         \
+    base::HistogramBase* histogram_pointer = histogram_factory_get_invocation; \
+    if (DCHECK_IS_ON())                                                        \
+      histogram_pointer->CheckName(constant_histogram_name);                   \
+    histogram_pointer->histogram_add_method_invocation;                        \
+  } while (0)
+
+// Copied from chromecast/base/metrics/cast_histograms.h
+// TODO: This macro can be unified with the cast_histogram.h macros.
+#define UMA_HISTOGRAM_CUSTOM_COUNTS_NO_CACHE(name, sample, min, max, \
+                                             bucket_count)           \
+  STATIC_HISTOGRAM_POINTER_BLOCK_NO_CACHE(                           \
+      name, Add(sample), base::Histogram::FactoryGet(                \
+                             name, min, max, bucket_count,           \
+                             base::HistogramBase::kUmaTargetedHistogramFlag))
+
+// In this macro UMA_HISTOGRAM_CUSTOM_COUNTS_NO_CACHE instead of the common
+// UMA_HISTOGRAM_CUSTOM_COUNTS is used, because this macro is called with
+// variable |name| and UMA_HISTOGRAM_CUSTOM_COUNTS does not support it.
+// In this macro the maximum size of request is 5000000 (about 5MB), which may
+// not be enough for large requests but it should cover the big majority of the
+// requests.
+#define TRAFFIC_MEASUREMENT_UMA_HISTOGRAM(name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS_NO_CACHE(name, sample, 1, 5000000, 50)
+
+#if defined(OS_ANDROID)
+// When Chrome starts working it is in foreground.
+base::android::ApplicationState app_state_ =
+    base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES;
+
+void DataUseMeasurement::OnApplicationStateChange(
+    base::android::ApplicationState application_state) {
+  app_state_ = application_state;
+}
+#endif
+
+AppState DataUseMeasurement::CurrentAppState() {
+#if defined(OS_ANDROID)
+  if (app_state_ == base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES)
+    return FOREGROUND;
+  else
+    return BACKGROUND;
+#endif
+  // If the OS is not Android, all the requests are considered foreground so for
+  // fair comparison between foreground and background data use, -A histograms

[sclittle, 2015/08/19 23:32:39]

Just say "If the OS is not Android, all the requests are considered foreground."
You could just mention in the histogram description or enum value description in
histograms.xml that BACKGROUND only applies to android.

[amohammadkhan, 2015/08/21 23:06:31]

Done.

+  // should be chosen in UMA.
+  return FOREGROUND;
+}
+
+ConnectionType DataUseMeasurement::CurrentConnectionType() {
+  if (net::NetworkChangeNotifier::IsConnectionCellular(
+          net::NetworkChangeNotifier::GetConnectionType())) {
+    return CELLULAR;
+  }
+  return NOT_CELLULAR;
+}
+
+std::string DataUseMeasurement::DimensionMaker(TrafficDirection dir) {
+  AppState app_state = CurrentAppState();
+  ConnectionType conn_type = CurrentConnectionType();
+  return base::StringPrintf(
+      ".%s.%s.%s", dir == UPLOAD ? "Upload" : "Download",
+      app_state == BACKGROUND ? "Background" : "Foreground",
+      conn_type == NOT_CELLULAR ? "NotCellular" : "Cellular");
+}
+
+void DataUseMeasurement::ReportDataUsage(const std::string& service_name,
+                                         TrafficDirection dir,
+                                         int message_size) {
+  std::string serviceHistogramName = "DataUse.Service." + service_name;
+  std::string combinedHistogramName = "DataUse.Services" + DimensionMaker(dir);
+  TRAFFIC_MEASUREMENT_UMA_HISTOGRAM(serviceHistogramName, message_size);
+  if (message_size > 0) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY_WITH_VALUE(
+          combinedHistogramName, base::Hash(service_name), message_size);
+  }
+}
+
+void DataUseMeasurement::ReportDataUsageURLRequest(
+    ChromeTrafficType service_type,
+    TrafficDirection dir,
+    int message_size) {
+  std::string histogram_name = base::StringPrintf(
+      "DataUse.%s%s", service_type == USER_TRAFFIC ? "User" : "NotUser",
+      DimensionMaker(dir).c_str());
+  TRAFFIC_MEASUREMENT_UMA_HISTOGRAM(histogram_name, message_size);
+}
+
+DataUseMeasurement::DataUseMeasurement() {
+#if defined(OS_ANDROID)
+  app_listener_ = new base::android::ApplicationStatusListener(
+      base::Bind(&DataUseMeasurement::OnApplicationStateChange, this));

[sclittle, 2015/08/19 23:32:39]

Instead of posting this as a scoped_refptr, it might make more sense to post
this as a WeakPtr instead. See weak_ptr.h, you could add a WeakPtrFactory
attribute to this object.

[amohammadkhan, 2015/08/21 23:06:31]

I checked the examples of this class again, it seems it is even fine to use
base::Unretained, so I used the same approach.

+#endif
+}
+
+void DataUseMeasurement::ReportDataUseUMA(const net::URLRequest* request) {

[sclittle, 2015/08/19 23:32:39]

nit: please add blank lines where appropriate in this method for readability.
It's kinda dense right now.

[amohammadkhan, 2015/08/21 23:06:31]

Done.

+  bool user_traffic_flag = false;
+  const content::ResourceRequestInfo* info =
+      content::ResourceRequestInfo::ForRequest(request);
+  // Having |info| is the sign of a request for a web content from user. For now
+  // we could add a condition to check ProcessType in info is
+  // content::PROCESS_TYPE_RENDERER, but it won't be compatible with upcoming
+  // PlzNavigate architecture. So just existence of |info| is verified, and the
+  // current check should be compatible with upcoming changes in PlzNavigate.
+  if (info) {
+    user_traffic_flag = true;
+  }
+  // These number won't be the number of bytes handed to the
+  // kernel because session layer framing and compression is not being accounted
+  // for.
+  // TODO: Make these measured bytes more accurate based on above explanation.
+  int request_body_bytes = 0;
+  int request_header_bytes = 0;
+  int total_upload_bytes = 0;
+  if (request->has_upload())
+    request_body_bytes = request->get_upload()->size();
+  net::HttpRequestHeaders request_headers;
+  if (request->GetFullRequestHeaders(&request_headers))
+    request_header_bytes = request_headers.ToString().length();
+  total_upload_bytes = request_body_bytes + request_header_bytes;
+  int total_received_bytes = request->GetTotalReceivedBytes();

[sclittle, 2015/08/19 23:32:39]

Always use int64_t for byte counts unless you have a really good reason not to.
This would overflow if the user downloads a 5 GB file.

[amohammadkhan, 2015/08/21 23:06:31]

I see your point. But the problem is that UMA is using int32 everywhere, (For
the sample and for count) and I am worried about it too. So I need to find a way
to solve that problem, and then I can change my variable to int64 too, but if I
cannot, I have to find a trick to solve the problem, maybe changing the minimum
or storing the log() of values instead of their original values.

+  ReportDataUsageURLRequest(user_traffic_flag ? USER_TRAFFIC : SERVICE_TRAFFIC,
+                            UPLOAD, total_upload_bytes);
+  ReportDataUsageURLRequest(user_traffic_flag ? USER_TRAFFIC : SERVICE_TRAFFIC,
+                            DOWNLOAD, total_received_bytes);
+  DataUseUserData* sData = static_cast<DataUseUserData*>(

[sclittle, 2015/08/19 23:32:39]

nit: use reinterpret_cast here instead, since it's just a pointer

[amohammadkhan, 2015/08/21 23:06:31]

Done.

+      request->GetUserData(DataUseUserData::kUserDataKey));
+  if (sData) {
+    ReportDataUsage(sData->service_name(), UPLOAD, total_upload_bytes);
+    ReportDataUsage(sData->service_name(), DOWNLOAD, total_received_bytes);
+  }
+}