Support needed to measure user and service traffic in Chrome.

components/data_use_measurement/content/data_use_measurement.h

+// 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.
+
+#ifndef COMPONENTS_DATA_USE_MEASUREMENT_CONTENT_DATA_USE_MEASUREMENT_H_
+#define COMPONENTS_DATA_USE_MEASUREMENT_CONTENT_DATA_USE_MEASUREMENT_H_
+
+#include <stdint.h>
+
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "build/build_config.h"

[bengr, 2015/09/01 17:00:11]

Why do you need this?

[amohammadkhan, 2015/09/01 19:38:14]

I was getting some errors, because OS_ANDROID  was not defined here when it
should have been defined and the problem was fixed by adding this.

+#include "components/data_use_measurement/core/data_use_user_data.h"
+
+#if defined(OS_ANDROID)
+#include "base/android/application_status_listener.h"
+#endif
+
+namespace net {
+class URLRequest;
+}
+
+namespace data_use_measurement {
+
+// The class records the data use of different services and user in according

[bengr, 2015/09/01 17:00:11]

// Records the data use of user traffic and various services in UMA histograms.
The UMA is broken down by network technology used (Wi-Fi vs cellular). On
Android, the UMA is further broken down by whether the app was in the background
or foreground during the request. 


Is the foreground check at the start or end of the request?

[amohammadkhan, 2015/09/01 19:38:15]

At the end of request.

+// UMA histograms. It also considers application state and connection state when
+// it records a usage.
+class DataUseMeasurement {

[bengr, 2015/09/01 17:00:11]

Is this an IO-thread object?

[amohammadkhan, 2015/09/01 19:38:14]

I think it is (If ChromeNetworkDelegate is IO-thread too)

+ public:
+  DataUseMeasurement();
+
+  ~DataUseMeasurement();
+
+  // Records the data use of the |request|.

[bengr, 2015/09/01 17:00:11]

, thus |request| must be non-null.

[mmenke, 2015/09/01 19:26:17]

Or could take it as const net::URLRequest& request, which implies it can't be
NULL.

[amohammadkhan, 2015/09/01 19:38:14]

Done.

+  void ReportDataUseUMA(const net::URLRequest* request);
+
+#if defined(OS_ANDROID)
+  // If a change in application status happens, this function is called and this
+  // function sets app_state_ appropriately.
+  void OnApplicationStateChangeForTesting(

[bengr, 2015/09/01 17:00:11]

// Called whenever the application transitions from foreground to background and
vice versa.

[amohammadkhan, 2015/09/01 19:38:15]

Done.

+      base::android::ApplicationState application_state);
+#endif
+
+ private:
+#if defined(OS_ANDROID)
+  // If a change in application status happens, this function is called and this
+  // function sets app_state_ appropriately.
+  void OnApplicationStateChange(

[bengr, 2015/09/01 17:00:11]

Why is there a separate method for testing?

[amohammadkhan, 2015/09/01 19:38:14]

To have the testing method public while this method is not public and it should
not be used by other classes. (Based on one of reviews)

+      base::android::ApplicationState application_state);
+#endif
+
+  // Specifies that data is received or sent, respectively.
+  enum TrafficDirection { DOWNSTREAM, UPSTREAM };
+
+  // The state of the application. Only available on Android and on other
+  // platforms it is always FOREGROUND.
+  enum AppState { BACKGROUND, FOREGROUND };

[bengr, 2015/09/01 17:00:11]

Does this really need to be an enum? 

[amohammadkhan, 2015/09/01 19:38:15]

Basically apps have 4 states, but we are using 2 of them here. So having this as
an enum make it possible to change it later based on later needs.

+
+  // The reason a request was sent. Whether the traffic originates from

[bengr, 2015/09/01 17:00:11]

The comment is more confusing than the enum.

// The request is from a user or from a service.

[amohammadkhan, 2015/09/01 19:38:14]

Done.

+  // user's request for a web site or a service is the source of the traffic.
+  enum ChromeTrafficType { USER_TRAFFIC, SERVICE_TRAFFIC };

[bengr, 2015/09/01 17:00:11]

Can you name this RequestInitiator { REQUEST_INITIATOR_USER,
REQUEST_INITIATOR_SERVICE }

[amohammadkhan, 2015/09/01 19:38:14]

Done.

+
+  // Returns the current application state (Foreground or Background). It always
+  // returns Foreground if Chrome is not running on Android.
+  AppState CurrentAppState();

[bengr, 2015/09/01 17:00:11]

Can't this just return a bool? IsAppInBackground()

[amohammadkhan, 2015/09/01 19:38:14]

similar reason to having the enum for it.

+
+  // The function produces a string representing the status of the Chrome when

[bengr, 2015/09/01 17:00:11]

// Produces a string representing whether the data use was on the send
("Upload") or receive ("Download") path, whether the app was in the "Foreground"
or "Background", and whether a "Cellular" or "WiFi" network was use. For
example, "Upload.Foreground.Cellular" is one possible output.

[amohammadkhan, 2015/09/01 19:38:14]

Done.

+  // recording is done. Its output is in following form: dir.appState.ConnState
+  // for example: Upload.Foreground.Cellular is one of its possible outputs.
+  std::string GetSuffixForHistogramName(TrafficDirection dir);
+
+  // Helper function used to record data use of services.

[bengr, 2015/09/01 17:00:11]

Describe what these helper functions do and what their inputs are.

[amohammadkhan, 2015/09/01 19:38:14]

Done.

+  void ReportDataUsage(
+      data_use_measurement::DataUseUserData::ServiceType service,
+      TrafficDirection dir,
+      int64_t message_size);
+
+  // Helper function used for reporting data use of URLRequests.
+  void ReportDataUsageURLRequest(ChromeTrafficType service_type,
+                                 TrafficDirection dir,
+                                 int64_t message_size);
+
+#if defined(OS_ANDROID)
+  // Application listener store the last known state of the application in this
+  // field.
+  base::android::ApplicationState app_state_;
+
+  // ApplicationStatusListener used to monitor that application is in foreground

[bengr, 2015/09/01 17:00:11]

that -> whether the
in foreground or in background -> in the foreground or in the background

[amohammadkhan, 2015/09/01 19:38:14]

Done.

+  // or in background. It is owned by DataUseMeasurement
+  scoped_ptr<base::android::ApplicationStatusListener> app_listener_;
+#endif
+
+  DISALLOW_COPY_AND_ASSIGN(DataUseMeasurement);
+};
+
+}  // namespace data_use_measurement
+
+#endif  // COMPONENTS_DATA_USE_MEASUREMENT_CONTENT_DATA_USE_MEASUREMENT_H_