UMA: use a more frequent upload interval for Chromecast.

R=asvitkine@chromium.org
BUG=464554

Review URL: https://codereview.chromium.org/979363003

Cr-Commit-Position: refs/heads/master@{#320515}

12 files changed, 75 insertions, 75 deletions

diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index 8954197..c68d2bf 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -11,6 +11,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
@@ -98,6 +99,24 @@ metrics::SystemProfileProto::Channel AsProtobufChannel(
   return metrics::SystemProfileProto::CHANNEL_UNKNOWN;
 }
 
+// Standard interval between log uploads, in seconds.
+#if defined(OS_ANDROID) || defined(OS_IOS)
+const int kStandardUploadIntervalSeconds = 5 * 60;  // Five minutes.
+const int kStandardUploadIntervalCellularSeconds = 15 * 60;  // Fifteen minutes.
+#else
+const int kStandardUploadIntervalSeconds = 30 * 60;  // Thirty minutes.
+#endif
+
+#if defined(OS_ANDROID) || defined(OS_IOS)
+// Returns true if the user is assigned to the experiment group for enabled
+// cellular uploads.
+bool IsCellularEnabledByExperiment() {
+  const std::string group_name =
+      base::FieldTrialList::FindFullName("UMA_EnableCellularLogUpload");
+  return group_name == "Enabled";
+}
+#endif
+
 }  // namespace
 
 ChromeMetricsServiceClient::ChromeMetricsServiceClient(
@@ -246,6 +265,17 @@ ChromeMetricsServiceClient::CreateUploader(
           on_upload_complete));
 }
 
+base::TimeDelta ChromeMetricsServiceClient::GetStandardUploadInterval() {
+#if defined(OS_ANDROID) || defined(OS_IOS)
+  bool is_cellular = false;
+  cellular_callback_.Run(&is_cellular);
+
+  if (is_cellular && IsCellularEnabledByExperiment())
+    return base::TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds);
+#endif
+  return base::TimeDelta::FromSeconds(kStandardUploadIntervalSeconds);
+}
+
 base::string16 ChromeMetricsServiceClient::GetRegistryBackupKey() {
 #if defined(OS_WIN)
   return L"Software\\" PRODUCT_STRING_PATH L"\\StabilityMetrics";
@@ -276,9 +306,7 @@ void ChromeMetricsServiceClient::Initialize() {
   scoped_ptr<metrics::NetworkMetricsProvider> network_metrics_provider(
       new metrics::NetworkMetricsProvider(
           content::BrowserThread::GetBlockingPool()));
-  base::Callback<void(bool*)> cellular_callback =
-      network_metrics_provider->GetConnectionCallback();
-  metrics_service_->SetConnectionTypeCallback(cellular_callback);
+  cellular_callback_ = network_metrics_provider->GetConnectionCallback();
   metrics_service_->RegisterMetricsProvider(network_metrics_provider.Pass());
 
   metrics_service_->RegisterMetricsProvider(
@@ -289,7 +317,7 @@ void ChromeMetricsServiceClient::Initialize() {
       scoped_ptr<metrics::MetricsProvider>(new metrics::GPUMetricsProvider()));
 
   profiler_metrics_provider_ =
-      new metrics::ProfilerMetricsProvider(cellular_callback);
+      new metrics::ProfilerMetricsProvider(cellular_callback_);
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(profiler_metrics_provider_));
 
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.h b/chrome/browser/metrics/chrome_metrics_service_client.h
index 2b95742..c2f1c6e 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.h
+++ b/chrome/browser/metrics/chrome_metrics_service_client.h
@@ -68,6 +68,7 @@ class ChromeMetricsServiceClient
   void CollectFinalMetrics(const base::Closure& done_callback) override;
   scoped_ptr<metrics::MetricsLogUploader> CreateUploader(
       const base::Callback<void(int)>& on_upload_complete) override;
+  base::TimeDelta GetStandardUploadInterval() override;
   base::string16 GetRegistryBackupKey() override;
 
   metrics::MetricsService* metrics_service() { return metrics_service_.get(); }
@@ -171,6 +172,10 @@ class ChromeMetricsServiceClient
   // MemoryDetails.
   MemoryGrowthTracker memory_growth_tracker_;
 
+  // Callback to determine whether or not a cellular network is currently being
+  // used.
+  base::Callback<void(bool*)> cellular_callback_;
+
   base::WeakPtrFactory<ChromeMetricsServiceClient> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeMetricsServiceClient);
diff --git a/chromecast/browser/metrics/cast_metrics_service_client.cc b/chromecast/browser/metrics/cast_metrics_service_client.cc
index 3e97217..48bf45b 100644
--- a/chromecast/browser/metrics/cast_metrics_service_client.cc
+++ b/chromecast/browser/metrics/cast_metrics_service_client.cc
@@ -31,6 +31,10 @@
 namespace chromecast {
 namespace metrics {
 
+namespace {
+const int kStandardUploadIntervalMinutes = 5;
+}  // namespace
+
 // static
 scoped_ptr<CastMetricsServiceClient> CastMetricsServiceClient::Create(
     base::TaskRunner* io_task_runner,
@@ -150,6 +154,10 @@ CastMetricsServiceClient::CreateUploader(
           on_upload_complete));
 }
 
+base::TimeDelta CastMetricsServiceClient::GetStandardUploadInterval() {
+  return base::TimeDelta::FromMinutes(kStandardUploadIntervalMinutes);
+}
+
 void CastMetricsServiceClient::EnableMetricsService(bool enabled) {
   if (!metrics_service_loop_->BelongsToCurrentThread()) {
     metrics_service_loop_->PostTask(
diff --git a/chromecast/browser/metrics/cast_metrics_service_client.h b/chromecast/browser/metrics/cast_metrics_service_client.h
index 72ceede..ffa17a3 100644
--- a/chromecast/browser/metrics/cast_metrics_service_client.h
+++ b/chromecast/browser/metrics/cast_metrics_service_client.h
@@ -62,6 +62,7 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient {
   void CollectFinalMetrics(const base::Closure& done_callback) override;
   scoped_ptr< ::metrics::MetricsLogUploader> CreateUploader(
       const base::Callback<void(int)>& on_upload_complete) override;
+  base::TimeDelta GetStandardUploadInterval() override;
 
   // Starts/stops the metrics service.
   void EnableMetricsService(bool enabled);
diff --git a/components/metrics/metrics_reporting_scheduler.cc b/components/metrics/metrics_reporting_scheduler.cc
index fb5461b..1318b22 100644
--- a/components/metrics/metrics_reporting_scheduler.cc
+++ b/components/metrics/metrics_reporting_scheduler.cc
@@ -5,7 +5,6 @@
 #include "components/metrics/metrics_reporting_scheduler.h"
 
 #include "base/compiler_specific.h"
-#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/variations/variations_associated_data.h"
@@ -35,14 +34,6 @@ const int kUnsentLogsIntervalSeconds = 3;
 const int kUnsentLogsIntervalSeconds = 15;
 #endif
 
-// Standard interval between log uploads, in seconds.
-#if defined(OS_ANDROID) || defined(OS_IOS)
-const int kStandardUploadIntervalSeconds = 5 * 60;  // Five minutes.
-const int kStandardUploadIntervalCellularSeconds = 15 * 60;  // Fifteen minutes.
-#else
-const int kStandardUploadIntervalSeconds = 30 * 60;  // Thirty minutes.
-#endif
-
 // When uploading metrics to the server fails, we progressively wait longer and
 // longer before sending the next log. This backoff process helps reduce load
 // on a server that is having issues.
@@ -71,47 +62,23 @@ void LogActualUploadInterval(TimeDelta interval) {
                              50);
 }
 
-// Returns upload interval specified for the current experiment running.
-// TODO(gayane): Only for experimenting with upload interval for Android
-// (bug: 17391128). Should be removed once the experiments are done.
-base::TimeDelta GetUploadIntervalFromExperiment() {
-  std::string interval_str = variations::GetVariationParamValue(
-      "UMALogUploadInterval", "interval");
-  int interval;
-  if (interval_str.empty() || !base::StringToInt(interval_str, &interval))
-    return TimeDelta::FromSeconds(kStandardUploadIntervalSeconds);
-
-  return TimeDelta::FromMinutes(interval);
-}
-
-#if defined(OS_ANDROID) || defined(OS_IOS)
-// Returns true if the user is assigned to the experiment group for enabled
-// cellular uploads.
-bool IsCellularEnabledByExperiment() {
-  const std::string group_name =
-      base::FieldTrialList::FindFullName("UMA_EnableCellularLogUpload");
-  return group_name == "Enabled";
-}
-#endif
-
 }  // anonymous namespace
 
 MetricsReportingScheduler::MetricsReportingScheduler(
     const base::Closure& upload_callback,
-    const base::Callback<void(bool*)>& cellular_callback)
+    const base::Callback<base::TimeDelta(void)>& upload_interval_callback)
     : upload_callback_(upload_callback),
       upload_interval_(TimeDelta::FromSeconds(kInitialUploadIntervalSeconds)),
       running_(false),
       callback_pending_(false),
       init_task_complete_(false),
       waiting_for_init_task_complete_(false),
-      cellular_callback_(cellular_callback) {
+      upload_interval_callback_(upload_interval_callback) {
 }
 
 MetricsReportingScheduler::~MetricsReportingScheduler() {}
 
 void MetricsReportingScheduler::Start() {
-  GetUploadIntervalFromExperiment();
   running_ = true;
   ScheduleNextUpload();
 }
@@ -206,15 +173,7 @@ void MetricsReportingScheduler::BackOffUploadInterval() {
 }
 
 base::TimeDelta MetricsReportingScheduler::GetStandardUploadInterval() {
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  bool is_cellular = false;
-  if (!cellular_callback_.is_null())
-    cellular_callback_.Run(&is_cellular);
-
-  if (is_cellular && IsCellularEnabledByExperiment())
-    return TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds);
-#endif
-  return TimeDelta::FromSeconds(kStandardUploadIntervalSeconds);
+  return upload_interval_callback_.Run();
 }
 
 }  // namespace metrics
diff --git a/components/metrics/metrics_reporting_scheduler.h b/components/metrics/metrics_reporting_scheduler.h
index ddfe651..4d6102a 100644
--- a/components/metrics/metrics_reporting_scheduler.h
+++ b/components/metrics/metrics_reporting_scheduler.h
@@ -18,11 +18,12 @@ namespace metrics {
 class MetricsReportingScheduler {
  public:
   // Creates MetricsServiceScheduler object with the given |upload_callback|
-  // callback to call when uploading should happen and |cellular_callback|
-  // callback to get current network connection type.
+  // callback to call when uploading should happen and
+  // |upload_interval_callback| to determine the interval between uploads
+  // in steady state.
   MetricsReportingScheduler(
       const base::Closure& upload_callback,
-      const base::Callback<void(bool*)>& cellular_callback);
+      const base::Callback<base::TimeDelta(void)>& upload_interval_callback);
   ~MetricsReportingScheduler();
 
   // Starts scheduling uploads. This in a no-op if the scheduler is already
@@ -57,10 +58,7 @@ class MetricsReportingScheduler {
   // where the server is having issues.
   void BackOffUploadInterval();
 
-  // Returns upload interval based on the system and experiment that the user is
-  // assigned to.
-  // TODO(gayane): Only for experimenting with upload interval for Android
-  // (bug: 17391128). Should be removed once the experiments are done.
+  // Returns upload interval to use in steady state.
   base::TimeDelta GetStandardUploadInterval();
 
   // The MetricsService method to call when uploading should happen.
@@ -90,8 +88,8 @@ class MetricsReportingScheduler {
   // has been completed.
   bool waiting_for_init_task_complete_;
 
-  // Callback function used to get current network connection type.
-  base::Callback<void(bool*)> cellular_callback_;
+  // Callback function used to get the standard upload time.
+  base::Callback<base::TimeDelta(void)> upload_interval_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MetricsReportingScheduler);
 };
diff --git a/components/metrics/metrics_reporting_scheduler_unittest.cc b/components/metrics/metrics_reporting_scheduler_unittest.cc
index 2a8e7fa..630fc5c 100644
--- a/components/metrics/metrics_reporting_scheduler_unittest.cc
+++ b/components/metrics/metrics_reporting_scheduler_unittest.cc
@@ -22,8 +22,8 @@ class MetricsReportingSchedulerTest : public testing::Test {
                       base::Unretained(this));
   }
 
-  base::Callback<void(bool*)> GetConnectionCallback() {
-    return base::Bind(&MetricsReportingSchedulerTest::SetConnectionTypeCallback,
+  base::Callback<base::TimeDelta(void)> GetConnectionCallback() {
+    return base::Bind(&MetricsReportingSchedulerTest::GetStandardUploadInterval,
                       base::Unretained(this));
   }
 
@@ -34,8 +34,8 @@ class MetricsReportingSchedulerTest : public testing::Test {
     ++callback_call_count_;
   }
 
-  void SetConnectionTypeCallback(bool* is_cellular_out) {
-    *is_cellular_out = false;
+  base::TimeDelta GetStandardUploadInterval() {
+    return base::TimeDelta::FromMinutes(5);
   }
 
   int callback_call_count_;
diff --git a/components/metrics/metrics_service.cc b/components/metrics/metrics_service.cc
index b4ee77a..f86f33a 100644
--- a/components/metrics/metrics_service.cc
+++ b/components/metrics/metrics_service.cc
@@ -301,7 +301,12 @@ void MetricsService::InitializeMetricsRecordingState() {
       base::Bind(&MetricsService::StartScheduledUpload,
                  self_ptr_factory_.GetWeakPtr());
   scheduler_.reset(
-      new MetricsReportingScheduler(upload_callback, is_cellular_callback_));
+      new MetricsReportingScheduler(
+          upload_callback,
+          // MetricsServiceClient outlives MetricsService, and
+          // MetricsReportingScheduler is tied to the lifetime of |this|.
+          base::Bind(&MetricsServiceClient::GetStandardUploadInterval,
+                     base::Unretained(client_))));
 }
 
 void MetricsService::Start() {
@@ -1130,10 +1135,4 @@ void MetricsService::RecordCurrentState(PrefService* pref) {
                  base::Time::Now().ToTimeT());
 }
 
-void MetricsService::SetConnectionTypeCallback(
-    base::Callback<void(bool*)> is_cellular_callback) {
-  DCHECK(!scheduler_);
-  is_cellular_callback_ = is_cellular_callback;
-}
-
 }  // namespace metrics
diff --git a/components/metrics/metrics_service.h b/components/metrics/metrics_service.h
index 7216df1..d7a612c 100644
--- a/components/metrics/metrics_service.h
+++ b/components/metrics/metrics_service.h
@@ -243,10 +243,6 @@ class MetricsService : public base::HistogramFlattener {
   // Clears the stability metrics that are saved in local state.
   void ClearSavedStabilityMetrics();
 
-  // Sets the connection type callback used to pass to the scheduler.
-  void SetConnectionTypeCallback(
-      base::Callback<void(bool*)> is_cellular_callback);
-
  protected:
   // Exposed for testing.
   MetricsLogManager* log_manager() { return &log_manager_; }
@@ -480,9 +476,6 @@ class MetricsService : public base::HistogramFlattener {
   // exited-cleanly bit in the prefs.
   static ShutdownCleanliness clean_shutdown_status_;
 
-  // Callback function used to get current network connection type.
-  base::Callback<void(bool*)> is_cellular_callback_;
-
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, IsPluginProcess);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest,
                            PermutedEntropyCacheClearedWhenLowEntropyReset);
diff --git a/components/metrics/metrics_service_client.h b/components/metrics/metrics_service_client.h
index fa31c5e..1b481f9 100644
--- a/components/metrics/metrics_service_client.h
+++ b/components/metrics/metrics_service_client.h
@@ -12,6 +12,7 @@
 #include "base/callback_forward.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
+#include "base/time/time.h"
 #include "components/metrics/proto/system_profile.pb.h"
 
 namespace metrics {
@@ -66,6 +67,9 @@ class MetricsServiceClient {
   virtual scoped_ptr<MetricsLogUploader> CreateUploader(
       const base::Callback<void(int)>& on_upload_complete) = 0;
 
+  // Returns the standard interval between upload attempts.
+  virtual base::TimeDelta GetStandardUploadInterval() = 0;
+
   // Returns the name of a key under HKEY_CURRENT_USER that can be used to store
   // backups of metrics data. Unused except on Windows.
   virtual base::string16 GetRegistryBackupKey();
diff --git a/components/metrics/test_metrics_service_client.cc b/components/metrics/test_metrics_service_client.cc
index 53cb1d9..9dcb370 100644
--- a/components/metrics/test_metrics_service_client.cc
+++ b/components/metrics/test_metrics_service_client.cc
@@ -69,4 +69,8 @@ scoped_ptr<MetricsLogUploader> TestMetricsServiceClient::CreateUploader(
   return scoped_ptr<MetricsLogUploader>();
 }
 
+base::TimeDelta TestMetricsServiceClient::GetStandardUploadInterval() {
+  return base::TimeDelta::FromMinutes(5);
+}
+
 }  // namespace metrics
diff --git a/components/metrics/test_metrics_service_client.h b/components/metrics/test_metrics_service_client.h
index 14c5518..973d6e3 100644
--- a/components/metrics/test_metrics_service_client.h
+++ b/components/metrics/test_metrics_service_client.h
@@ -33,6 +33,7 @@ class TestMetricsServiceClient : public MetricsServiceClient {
   void CollectFinalMetrics(const base::Closure& done_callback) override;
   scoped_ptr<MetricsLogUploader> CreateUploader(
       const base::Callback<void(int)>& on_upload_complete) override;
+  base::TimeDelta GetStandardUploadInterval() override;
 
   const std::string& get_client_id() const { return client_id_; }
   void set_version_string(const std::string& str) { version_string_ = str; }