Flatten UpdateMetricsUsagePrefs - skip Metrics and Reporting service.

UpdateMetricsUsagePrefs() is a chain of calls:
- ChromeDataUseMeasurement::UpdateDataUseToMetricsService()
  -> UpdateMetricsUsagePrefs()
  -> MetricsService::UpdateMetricsUsagePrefs()
  -> ReportingService::UpdateMetricsUsagePrefs()
  -> DataUseTracker::UpdateMetricsUsagePrefs()

It can be reduced to:
- ChromeDataUseMeasurement::UpdateDataUseToMetricsService()
  -> UpdateMetricsUsagePrefs()
  -> DataUseTracker::UpdateMetricsUsagePrefs()

This removes the dependency from ChromeDataUseMeasurement to
MetricsService and ReportingService.

Bug: 902791
Change-Id: I38a6d22d1ff823134c79bc342ea32be0f89cda77
Reviewed-on: https://chromium-review.googlesource.com/c/1334267
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608567}

chrome/browser/data_use_measurement/chrome_data_use_measurement.cc

@@ -15,7 +15,7 @@
 #include "chrome/browser/browser_process.h"
 #include "components/data_use_measurement/core/data_use_ascriber.h"
 #include "components/data_use_measurement/core/url_request_classifier.h"
-#include "components/metrics/metrics_service.h"
+#include "components/metrics/data_use_tracker.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/network_service_instance.h"
@@ -34,14 +34,9 @@ void UpdateMetricsUsagePrefs(int64_t total_bytes,
                              bool is_cellular,
                              bool is_metrics_service_usage) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // Some unit tests use IOThread but do not initialize MetricsService. In that
-  // case it's fine to skip the update.
-  auto* metrics_service = g_browser_process->metrics_service();
-  if (metrics_service) {
-    metrics_service->UpdateMetricsUsagePrefs(
-        base::saturated_cast<int>(total_bytes), is_cellular,
-        is_metrics_service_usage);
-  }
+  metrics::DataUseTracker::UpdateMetricsUsagePrefs(
+      base::saturated_cast<int>(total_bytes), is_cellular,
+      is_metrics_service_usage, g_browser_process->local_state());
 }
 
 // This function is for forwarding metrics usage pref changes to the metrics

components/metrics/data_use_tracker.cc

@@ -8,6 +8,7 @@
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "build/build_config.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/variations/variations_associated_data.h"
@@ -33,6 +34,8 @@ DataUseTracker::~DataUseTracker() {}
 std::unique_ptr<DataUseTracker> DataUseTracker::Create(
     PrefService* local_state) {
   std::unique_ptr<DataUseTracker> data_use_tracker;
+// Instantiate DataUseTracker only on Android. UpdateMetricsUsagePrefs() honors
+// this rule too.
 #if defined(OS_ANDROID)
   data_use_tracker.reset(new DataUseTracker(local_state));
 #endif
@@ -45,9 +48,23 @@ void DataUseTracker::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(metrics::prefs::kUmaCellDataUse);
 }
 
+// static
 void DataUseTracker::UpdateMetricsUsagePrefs(int message_size,
                                              bool is_cellular,
-                                             bool is_metrics_service_usage) {
+                                             bool is_metrics_service_usage,
+                                             PrefService* local_state) {
+// Instantiate DataUseTracker only on Android. Create() honors this rule too.
+#if defined(OS_ANDROID)
+  metrics::DataUseTracker tracker(local_state);
+  tracker.UpdateMetricsUsagePrefsInternal(message_size, is_cellular,
+                                          is_metrics_service_usage);
+#endif  // defined(OS_ANDROID)
+}
+
+void DataUseTracker::UpdateMetricsUsagePrefsInternal(
+    int message_size,
+    bool is_cellular,
+    bool is_metrics_service_usage) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (!is_cellular)

components/metrics/data_use_tracker.h

@@ -17,9 +17,6 @@
 
 namespace metrics {
 
-typedef base::Callback<void(const std::string&, int, bool)>
-    UpdateUsagePrefCallbackType;
-
 // Records the data use of user traffic and UMA traffic in user prefs. Taking
 // into account those prefs it can verify whether certain UMA log upload is
 // allowed.
@@ -36,9 +33,10 @@ class DataUseTracker {
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
   // Updates data usage tracking prefs with the specified values.
-  void UpdateMetricsUsagePrefs(int message_size,
-                               bool is_cellular,
-                               bool is_metrics_service_usage);
+  static void UpdateMetricsUsagePrefs(int message_size,
+                                      bool is_cellular,
+                                      bool is_metrics_service_usage,
+                                      PrefService* local_state);
 
   // Returns whether a log with provided |log_bytes| can be uploaded according
   // to data use ratio and UMA quota provided by variations.
@@ -50,6 +48,11 @@ class DataUseTracker {
   FRIEND_TEST_ALL_PREFIXES(DataUseTrackerTest, CheckComputeTotalDataUse);
   FRIEND_TEST_ALL_PREFIXES(DataUseTrackerTest, CheckCanUploadUMALog);
 
+  // Updates data usage tracking prefs with the specified values.
+  void UpdateMetricsUsagePrefsInternal(int message_size,
+                                       bool is_cellular,
+                                       bool is_metrics_service_usage);
+
   // Updates provided |pref_name| for a current date with the given message
   // size.
   void UpdateUsagePref(const std::string& pref_name, int message_size);

components/metrics/data_use_tracker_unittest.cc

@@ -106,7 +106,7 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) {
   int user_pref_value = 0;
   int uma_pref_value = 0;
 
-  data_use_tracker.UpdateMetricsUsagePrefs(2 * 100, true, false);
+  data_use_tracker.UpdateMetricsUsagePrefsInternal(2 * 100, true, false);
   local_state.GetDictionary(prefs::kUserCellDataUse)
       ->GetInteger(kTodayStr, &user_pref_value);
   EXPECT_EQ(2 * 100, user_pref_value);
@@ -114,7 +114,7 @@ TEST(DataUseTrackerTest, CheckUpdateUsagePref) {
       ->GetInteger(kTodayStr, &uma_pref_value);
   EXPECT_EQ(0, uma_pref_value);
 
-  data_use_tracker.UpdateMetricsUsagePrefs(100, true, true);
+  data_use_tracker.UpdateMetricsUsagePrefsInternal(100, true, true);
   local_state.GetDictionary(prefs::kUserCellDataUse)
       ->GetInteger(kTodayStr, &user_pref_value);
   EXPECT_EQ(3 * 100, user_pref_value);

components/metrics/metrics_service.cc

@@ -452,14 +452,6 @@ void MetricsService::PushExternalLog(const std::string& log) {
   log_store()->StoreLog(log, MetricsLog::ONGOING_LOG);
 }
 
-void MetricsService::UpdateMetricsUsagePrefs(int message_size,
-                                             bool is_cellular,
-                                             bool is_metrics_service_usage) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  reporting_service_.UpdateMetricsUsagePrefs(message_size, is_cellular,
-                                             is_metrics_service_usage);
-}
-
 //------------------------------------------------------------------------------
 // private methods
 //------------------------------------------------------------------------------

components/metrics/metrics_service.h

@@ -169,11 +169,6 @@ class MetricsService : public base::HistogramFlattener {
   // Pushes a log that has been generated by an external component.
   void PushExternalLog(const std::string& log);
 
-  // Updates data usage tracking prefs with the specified values.
-  void UpdateMetricsUsagePrefs(int message_size,
-                               bool is_cellular,
-                               bool is_metrics_service_usage);
-
   variations::SyntheticTrialRegistry* synthetic_trial_registry() {
     return &synthetic_trial_registry_;
   }

components/metrics/reporting_service.cc

@@ -81,16 +81,6 @@ bool ReportingService::reporting_active() const {
   return reporting_active_;
 }
 
-void ReportingService::UpdateMetricsUsagePrefs(int message_size,
-                                               bool is_cellular,
-                                               bool is_metrics_service_usage) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (data_use_tracker_) {
-    data_use_tracker_->UpdateMetricsUsagePrefs(message_size, is_cellular,
-                                               is_metrics_service_usage);
-  }
-}
-
 //------------------------------------------------------------------------------
 // private methods
 //------------------------------------------------------------------------------

components/metrics/reporting_service.h

@@ -66,11 +66,6 @@ class ReportingService {
   // True iff reporting is currently enabled.
   bool reporting_active() const;
 
-  // Updates data usage tracking prefs with the specified values.
-  void UpdateMetricsUsagePrefs(int message_size,
-                               bool is_cellular,
-                               bool is_metrics_service_usage);
-
   // Registers local state prefs used by this class. This should only be called
   // once.
   static void RegisterPrefs(PrefRegistrySimple* registry);