Expose a method in VariationsService to trigger a seed fetch.

On mobile this will launch a (slightly delayed) request, iff the seed hasn't been refreshed in the past 5 hours. On desktop this will use the currently existing method on desktop, which does not do that check.

BUG=321379

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251532 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 210 insertions, 2 deletions

diff --git a/chrome/browser/metrics/variations/variations_request_scheduler.cc b/chrome/browser/metrics/variations/variations_request_scheduler.cc
index ff437a8..8f62c35 100644
--- a/chrome/browser/metrics/variations/variations_request_scheduler.cc
+++ b/chrome/browser/metrics/variations/variations_request_scheduler.cc
@@ -36,6 +36,10 @@ void VariationsRequestScheduler::ScheduleFetchShortly() {
                         task_);
 }
 
+void VariationsRequestScheduler::OnAppEnterForeground() {
+  NOTREACHED() << "Attempted to OnAppEnterForeground on non-mobile device";
+}
+
 base::Closure VariationsRequestScheduler::task() const {
   return task_;
 }
diff --git a/chrome/browser/metrics/variations/variations_request_scheduler.h b/chrome/browser/metrics/variations/variations_request_scheduler.h
index beb3bbf..7dd1a2c 100644
--- a/chrome/browser/metrics/variations/variations_request_scheduler.h
+++ b/chrome/browser/metrics/variations/variations_request_scheduler.h
@@ -19,7 +19,7 @@ class VariationsRequestScheduler {
  public:
   virtual ~VariationsRequestScheduler();
 
-  // Starts the task on a schedule.
+  // Starts the task. This can be a repeated event or a one-off.
   virtual void Start();
 
   // Resets the scheduler if it is currently on a timer.
@@ -29,6 +29,10 @@ class VariationsRequestScheduler {
   // may have failed.
   void ScheduleFetchShortly();
 
+  // Called when the application has been foregrounded. This may fetch a new
+  // seed.
+  virtual void OnAppEnterForeground();
+
   // Factory method for this class.
   static VariationsRequestScheduler* Create(const base::Closure& task,
                                             PrefService* local_state);
diff --git a/chrome/browser/metrics/variations/variations_request_scheduler_mobile.cc b/chrome/browser/metrics/variations/variations_request_scheduler_mobile.cc
index 7714861..9e028c0 100644
--- a/chrome/browser/metrics/variations/variations_request_scheduler_mobile.cc
+++ b/chrome/browser/metrics/variations/variations_request_scheduler_mobile.cc
@@ -11,6 +11,9 @@ namespace chrome_variations {
 
 namespace {
 
+// Time before attempting a seed fetch after a ScheduleFetch(), in seconds.
+const int kScheduleFetchDelaySeconds = 5;
+
 // Time between seed fetches, in hours.
 const int kSeedFetchPeriodHours = 5;
 
@@ -34,6 +37,7 @@ void VariationsRequestSchedulerMobile::Start() {
       local_state_->GetInt64(prefs::kVariationsLastFetchTime));
   if (base::Time::Now() >
       last_fetch_time + base::TimeDelta::FromHours(kSeedFetchPeriodHours)) {
+    last_request_time_ = base::Time::Now();
     task().Run();
   }
 }
@@ -41,6 +45,25 @@ void VariationsRequestSchedulerMobile::Start() {
 void VariationsRequestSchedulerMobile::Reset() {
 }
 
+void VariationsRequestSchedulerMobile::OnAppEnterForeground() {
+  // Verify we haven't just attempted a fetch (which has not completed). This
+  // is mainly used to verify we don't trigger a second fetch for the
+  // OnAppEnterForeground right after startup.
+  if (base::Time::Now() <
+      last_request_time_ + base::TimeDelta::FromHours(kSeedFetchPeriodHours)) {
+    return;
+  }
+
+  // Since Start() launches a one-off execution, we can reuse it here. Also
+  // note that since Start() verifies that the seed needs to be refreshed, we
+  // do not verify here.
+  schedule_fetch_timer_.Start(
+      FROM_HERE,
+      base::TimeDelta::FromSeconds(kScheduleFetchDelaySeconds),
+      this,
+      &VariationsRequestSchedulerMobile::Start);
+}
+
 // static
 VariationsRequestScheduler* VariationsRequestScheduler::Create(
     const base::Closure& task,
diff --git a/chrome/browser/metrics/variations/variations_request_scheduler_mobile.h b/chrome/browser/metrics/variations/variations_request_scheduler_mobile.h
index 56d4f4d..c222206 100644
--- a/chrome/browser/metrics/variations/variations_request_scheduler_mobile.h
+++ b/chrome/browser/metrics/variations/variations_request_scheduler_mobile.h
@@ -25,11 +25,25 @@ class VariationsRequestSchedulerMobile : public VariationsRequestScheduler {
   // Base class overrides.
   virtual void Start() OVERRIDE;
   virtual void Reset() OVERRIDE;
+  virtual void OnAppEnterForeground() OVERRIDE;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(VariationsRequestSchedulerMobileTest,
+                           OnAppEnterForegroundNoRun);
+  FRIEND_TEST_ALL_PREFIXES(VariationsRequestSchedulerMobileTest,
+                           OnAppEnterForegroundRun);
+  FRIEND_TEST_ALL_PREFIXES(VariationsRequestSchedulerMobileTest,
+                           OnAppEnterForegroundOnStartup);
+
   // The local state instance that provides the last fetch time.
   PrefService* local_state_;
 
+  // Timer used for triggering a delayed fetch for ScheduleFetch().
+  base::OneShotTimer<VariationsRequestSchedulerMobile> schedule_fetch_timer_;
+
+  // The time the last seed request was initiated.
+  base::Time last_request_time_;
+
   DISALLOW_COPY_AND_ASSIGN(VariationsRequestSchedulerMobile);
 };
 
diff --git a/chrome/browser/metrics/variations/variations_request_scheduler_mobile_unittest.cc b/chrome/browser/metrics/variations/variations_request_scheduler_mobile_unittest.cc
new file mode 100644
index 0000000..7abb923
--- /dev/null
+++ b/chrome/browser/metrics/variations/variations_request_scheduler_mobile_unittest.cc
@@ -0,0 +1,145 @@
+// Copyright 2014 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/metrics/variations/variations_request_scheduler_mobile.h"
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/testing_pref_service.h"
+#include "chrome/common/pref_names.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chrome_variations {
+
+namespace {
+
+// Simple method used to verify a Callback has been triggered.
+void Increment(int *n) {
+  (*n)++;
+}
+
+}  // namespace
+
+TEST(VariationsRequestSchedulerMobileTest, StartNoRun) {
+  TestingPrefServiceSimple prefs;
+  // Initialize to as if it was just fetched. This means it should not run.
+  prefs.registry()->RegisterInt64Pref(prefs::kVariationsLastFetchTime,
+                                      base::Time::Now().ToInternalValue());
+  int executed = 0;
+  const base::Closure task = base::Bind(&Increment, &executed);
+  VariationsRequestSchedulerMobile scheduler(task, &prefs);
+  scheduler.Start();
+  // We expect it the task to not have triggered.
+  EXPECT_EQ(0, executed);
+}
+
+TEST(VariationsRequestSchedulerMobileTest, StartRun) {
+  TestingPrefServiceSimple prefs;
+  // Verify it doesn't take more than a day.
+  base::Time old = base::Time::Now() - base::TimeDelta::FromHours(24);
+  prefs.registry()->RegisterInt64Pref(prefs::kVariationsLastFetchTime,
+                                      old.ToInternalValue());
+  int executed = 0;
+  const base::Closure task = base::Bind(&Increment, &executed);
+  VariationsRequestSchedulerMobile scheduler(task, &prefs);
+  scheduler.Start();
+  // We expect the task to have triggered.
+  EXPECT_EQ(1, executed);
+}
+
+TEST(VariationsRequestSchedulerMobileTest, OnAppEnterForegroundNoRun) {
+  base::MessageLoopForUI message_loop_;
+
+  TestingPrefServiceSimple prefs;
+
+  // Initialize to as if it was just fetched. This means it should not run.
+  prefs.registry()->RegisterInt64Pref(prefs::kVariationsLastFetchTime,
+                                      base::Time::Now().ToInternalValue());
+  int executed = 0;
+  const base::Closure task = base::Bind(&Increment, &executed);
+  VariationsRequestSchedulerMobile scheduler(task, &prefs);
+
+  // Verify timer not running.
+  EXPECT_FALSE(scheduler.schedule_fetch_timer_.IsRunning());
+  scheduler.OnAppEnterForeground();
+
+  // Timer now running.
+  EXPECT_TRUE(scheduler.schedule_fetch_timer_.IsRunning());
+
+  // Force execution of the task on this timer to verify that the correct task
+  // was added to the timer.
+  scheduler.schedule_fetch_timer_.user_task().Run();
+
+  // The task should not execute because the seed was fetched too recently.
+  EXPECT_EQ(0, executed);
+}
+
+TEST(VariationsRequestSchedulerMobileTest, OnAppEnterForegroundRun) {
+  base::MessageLoopForUI message_loop_;
+
+  TestingPrefServiceSimple prefs;
+
+  base::Time old = base::Time::Now() - base::TimeDelta::FromHours(24);
+  prefs.registry()->RegisterInt64Pref(prefs::kVariationsLastFetchTime,
+                                      old.ToInternalValue());
+  int executed = 0;
+  const base::Closure task = base::Bind(&Increment, &executed);
+  VariationsRequestSchedulerMobile scheduler(task, &prefs);
+
+  // Verify timer not running.
+  EXPECT_FALSE(scheduler.schedule_fetch_timer_.IsRunning());
+  scheduler.OnAppEnterForeground();
+
+  // Timer now running.
+  EXPECT_TRUE(scheduler.schedule_fetch_timer_.IsRunning());
+
+  // Force execution of the task on this timer to verify that the correct task
+  // was added to the timer - this will verify that the right task is running.
+  scheduler.schedule_fetch_timer_.user_task().Run();
+
+  // We expect the input task to have triggered.
+  EXPECT_EQ(1, executed);
+}
+
+TEST(VariationsRequestSchedulerMobileTest, OnAppEnterForegroundOnStartup) {
+  base::MessageLoopForUI message_loop_;
+
+  TestingPrefServiceSimple prefs;
+
+  base::Time old = base::Time::Now() - base::TimeDelta::FromHours(24);
+  prefs.registry()->RegisterInt64Pref(prefs::kVariationsLastFetchTime,
+                                      old.ToInternalValue());
+  int executed = 0;
+  const base::Closure task = base::Bind(&Increment, &executed);
+  VariationsRequestSchedulerMobile scheduler(task, &prefs);
+
+  scheduler.Start();
+
+  // We expect the task to have triggered.
+  EXPECT_EQ(1, executed);
+
+  scheduler.OnAppEnterForeground();
+
+  EXPECT_FALSE(scheduler.schedule_fetch_timer_.IsRunning());
+  // We expect the input task to not have triggered again, so executed stays
+  // at 1.
+  EXPECT_EQ(1, executed);
+
+  // Simulate letting time pass.
+  const base::Time last_fetch_time = base::Time::FromInternalValue(
+      prefs.GetInt64(prefs::kVariationsLastFetchTime));
+  prefs.SetInt64(
+      prefs::kVariationsLastFetchTime,
+      (last_fetch_time - base::TimeDelta::FromHours(24)).ToInternalValue());
+  scheduler.last_request_time_ -= base::TimeDelta::FromHours(24);
+
+  scheduler.OnAppEnterForeground();
+  EXPECT_TRUE(scheduler.schedule_fetch_timer_.IsRunning());
+  scheduler.schedule_fetch_timer_.user_task().Run();
+  // This time it should execute the task.
+  EXPECT_EQ(2, executed);
+}
+
+}  // namespace chrome_variations
diff --git a/chrome/browser/metrics/variations/variations_service.cc b/chrome/browser/metrics/variations/variations_service.cc
index 4bca443..5f1baac 100644
--- a/chrome/browser/metrics/variations/variations_service.cc
+++ b/chrome/browser/metrics/variations/variations_service.cc
@@ -263,6 +263,15 @@ void VariationsService::StartRepeatedVariationsSeedFetch() {
   request_scheduler_->Start();
 }
 
+// TODO(rkaplow): Handle this and the similar event in metrics_service by
+// observing an 'OnAppEnterForeground' event in RequestScheduler instead of
+// requiring the frontend code to notify each service individually. Since the
+// scheduler will handle it directly the VariationService shouldn't need to
+// know details of this anymore.
+void VariationsService::OnAppEnterForeground() {
+  request_scheduler_->OnAppEnterForeground();
+}
+
 // static
 GURL VariationsService::GetVariationsServerURL(
     PrefService* policy_pref_service) {
diff --git a/chrome/browser/metrics/variations/variations_service.h b/chrome/browser/metrics/variations/variations_service.h
index 0105081..bf76b04 100644
--- a/chrome/browser/metrics/variations/variations_service.h
+++ b/chrome/browser/metrics/variations/variations_service.h
@@ -57,6 +57,13 @@ class VariationsService
   // static for test purposes.
   static GURL GetVariationsServerURL(PrefService* local_prefs);
 
+  // Called when the application enters foreground. This may trigger a
+  // FetchVariationsSeed call.
+  // TODO(rkaplow): Handle this and the similar event in metrics_service by
+  // observing an 'OnAppEnterForeground' event instead of requiring the frontend
+  // code to notify each service individually.
+  void OnAppEnterForeground();
+
 #if defined(OS_WIN)
   // Starts syncing Google Update Variation IDs with the registry.
   void StartGoogleUpdateRegistrySync();
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 2051cda..a4c545d 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -1066,9 +1066,10 @@
         'browser/metrics/thread_watcher_unittest.cc',
         'browser/metrics/time_ticks_experiment_unittest.cc',
         'browser/metrics/variations/variations_http_header_provider_unittest.cc',
+        'browser/metrics/variations/variations_request_scheduler_mobile_unittest.cc',
+        'browser/metrics/variations/variations_request_scheduler_unittest.cc',
         'browser/metrics/variations/variations_seed_store_unittest.cc',
         'browser/metrics/variations/variations_service_unittest.cc',
-        'browser/metrics/variations/variations_request_scheduler_unittest.cc',
         'browser/net/chrome_fraudulent_certificate_reporter_unittest.cc',
         'browser/net/chrome_network_delegate_unittest.cc',
         'browser/net/client_hints_unittest.cc',
@@ -2546,6 +2547,7 @@
             'tools/profile_reset/jtl_compiler.gyp:jtl_compiler_lib',
           ],
           'sources!': [
+            'browser/metrics/variations/variations_request_scheduler_mobile_unittest.cc',
             'browser/net/spdyproxy/data_reduction_proxy_settings_unittest.cc',
             'browser/net/spdyproxy/data_reduction_proxy_settings_unittest.h',
             'browser/web_resource/promo_resource_service_mobile_ntp_unittest.cc',