Implement UMA log throttling for cellular connections

components/metrics/metrics_service.h

Index: components/metrics/metrics_service.h
diff --git a/components/metrics/metrics_service.h b/components/metrics/metrics_service.h
index ce4102b8dfa36408fe037bd6772184d759a48a4e..c349d439da4dddf8466300362208f233b6a0621b 100644
--- a/components/metrics/metrics_service.h
+++ b/components/metrics/metrics_service.h
@@ -27,6 +27,7 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/metrics/clean_exit_beacon.h"
+#include "components/metrics/data_use_tracker.h"
 #include "components/metrics/metrics_log.h"
 #include "components/metrics/metrics_log_manager.h"
 #include "components/metrics/metrics_provider.h"
@@ -80,6 +81,12 @@ class MetricsService : public base::HistogramFlattener {
   MetricsService(MetricsStateManager* state_manager,
                  MetricsServiceClient* client,
                  PrefService* local_state);
+
+  // Creates the MetricsService as above with given |cellular_callback|

[Alexei Svitkine (slow), 2016/03/31 20:00:40]

Instead of having a callback as a ctor param, can't it just be a function of the
client interface? Isn't it already?

[gayane -on leave until 09-2017, 2016/03/31 21:55:10]

I think we tried to do this once and I think metrics service cannot depend on
client.

[Alexei Svitkine (slow), 2016/03/31 22:18:02]

I think it should work. MetricsService already depends on client for a number of
other things (e.g. client->GetRegistryBackupKey() right in its ctor).

+  MetricsService(MetricsStateManager* state_manager,
+                 MetricsServiceClient* client,
+                 PrefService* local_state,
+                 base::Callback<void(bool*)> cellular_callback);
   ~MetricsService() override;
 
   // Initializes metrics recording state. Updates various bookkeeping values in
@@ -214,6 +221,10 @@ class MetricsService : public base::HistogramFlattener {
   // Pushes a log that has been generated by an external component.
   void PushExternalLog(const std::string& log);
 
+  // Returns a callback to data use pref updating function attached to
+  // |data_use_tracker| weak pointer. Should be called on UI thread.

[Alexei Svitkine (slow), 2016/03/31 20:00:40]

This description reads like implementation detail. Can you instead talk about
the purpose of this. Also, mention that the callback should be safe to use from
any thread (don't just say its only for UI thread), although this function
should be called on UI thread.

[gayane -on leave until 09-2017, 2016/03/31 21:55:10]

Done.

+  UpdateUsagePrefCallbackType GetDataUseForwardingCallback();
+
  protected:
   // Exposed for testing.
   MetricsLogManager* log_manager() { return &log_manager_; }
@@ -468,7 +479,7 @@ class MetricsService : public base::HistogramFlattener {
   // Execution phase the browser is in.
   static ExecutionPhase execution_phase_;
 
-  // Reduntant marker to check that we completed our shutdown, and set the
+  // Redundant marker to check that we completed our shutdown, and set the
   // exited-cleanly bit in the prefs.
   static ShutdownCleanliness clean_shutdown_status_;
 
@@ -477,6 +488,13 @@ class MetricsService : public base::HistogramFlattener {
                            PermutedEntropyCacheClearedWhenLowEntropyReset);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, RegisterSyntheticTrial);
 
+  // Assigns true to output param if current connection type is cellular and
+  // user is assigned to experimental group for enabled cellular uploads.
+  base::Callback<void(bool*)> cellular_callback_;
+
+  // Pointer used for obtaining data use pref updater callback on above layers.
+  scoped_ptr<DataUseTracker> data_use_tracker_;
+
   // Weak pointers factory used to post task on different threads. All weak
   // pointers managed by this factory have the same lifetime as MetricsService.
   base::WeakPtrFactory<MetricsService> self_ptr_factory_;