Have the VariationsService attempt to fetch the seed when an update is ready.

Includes a unit test to exercise this functionality.

BUG=138384
TEST=Ensure that Chrome makes an HTTP request to the Variations server when it receives an auto update.


Review URL: https://chromiumcodereview.appspot.com/10790116

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

chrome/browser/metrics/variations_service.cc

@@ -16,10 +16,13 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/upgrade_detector.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/metrics/experiments_helper.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/common/url_fetcher.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/load_flags.h"
@@ -32,7 +35,7 @@ namespace chrome_variations {
 
 namespace {
 
-// Default server of Variations seed info.
+// Default server of variations seed info.
 const char kDefaultVariationsServerURL[] =
     "https://clients4.google.com/chrome-variations/seed";
 const int kMaxRetrySeedFetch = 5;
@@ -84,7 +87,7 @@ base::Time ConvertStudyDateToBaseTime(int64 date_time) {
   return base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(date_time);
 }
 
-// Determine and return the variations server URL.
+// Determine and return the Variations server URL.
 GURL GetVariationsServerURL() {
   std::string server_url(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
       switches::kVariationsServerURL));
@@ -100,6 +103,8 @@ GURL GetVariationsServerURL() {
 VariationsService::VariationsService()
     : variations_server_url_(GetVariationsServerURL()),
       create_trials_from_seed_called_(false) {
+  registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
+      content::Source<UpgradeDetector>(UpgradeDetector::GetInstance()));
 }
 
 VariationsService::~VariationsService() {}
@@ -133,7 +138,7 @@ bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) {
 }
 
 void VariationsService::StartRepeatedVariationsSeedFetch() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  DCHECK(CalledOnValidThread());
 
   // Check that |CreateTrialsFromSeed| was called, which is necessary to
   // retrieve the serial number that will be sent to the server.
@@ -147,8 +152,15 @@ void VariationsService::StartRepeatedVariationsSeedFetch() {
                this, &VariationsService::FetchVariationsSeed);
 }
 
+// static
+void VariationsService::RegisterPrefs(PrefService* prefs) {
+  prefs->RegisterStringPref(prefs::kVariationsSeed, std::string());
+  prefs->RegisterInt64Pref(prefs::kVariationsSeedDate,
+                           base::Time().ToInternalValue());
+}
+
 void VariationsService::FetchVariationsSeed() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  DCHECK(CalledOnValidThread());
 
   const bool is_offline = net::NetworkChangeNotifier::IsOffline();
   UMA_HISTOGRAM_BOOLEAN("Variations.NetworkAvailability", !is_offline);
@@ -171,6 +183,22 @@ void VariationsService::FetchVariationsSeed() {
   pending_seed_request_->Start();
 }
 
+void VariationsService::Observe(int type,
+                                const content::NotificationSource& source,
+                                const content::NotificationDetails& details) {
+  DCHECK(CalledOnValidThread());
+  DCHECK_EQ(chrome::NOTIFICATION_UPGRADE_RECOMMENDED, type);
+
+  // An upgrade is ready, so attempt to fetch the Variations seed in case there
+  // were updates.
+  FetchVariationsSeed();
+
+  // Since we explicitly call FetchVariationsSeed here, we can reset the timer
+  // so that we don't retry for another full period.
+  if (timer_.IsRunning())
+    timer_.Reset();
+}
+
 void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK_EQ(pending_seed_request_.get(), source);
   // When we're done handling the request, the fetcher will be deleted.
@@ -198,13 +226,6 @@ void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
   StoreSeedData(seed_data, response_date, g_browser_process->local_state());
 }
 
-// static
-void VariationsService::RegisterPrefs(PrefService* prefs) {
-  prefs->RegisterStringPref(prefs::kVariationsSeed, std::string());
-  prefs->RegisterInt64Pref(prefs::kVariationsSeedDate,
-                           base::Time().ToInternalValue());
-}
-
 bool VariationsService::StoreSeedData(const std::string& seed_data,
                                       const base::Time& seed_date,
                                       PrefService* local_prefs) {

chrome/browser/metrics/variations_service.h

@@ -11,11 +11,14 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
+#include "base/threading/non_thread_safe.h"
 #include "base/time.h"
 #include "base/timer.h"
 #include "chrome/browser/metrics/proto/study.pb.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
 #include "chrome/common/chrome_version_info.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
@@ -29,7 +32,9 @@ namespace chrome_variations {
 
 // Used to setup field trials based on stored variations seed data, and fetch
 // new seed data from the variations server.
-class VariationsService : public net::URLFetcherDelegate {
+class VariationsService : public net::URLFetcherDelegate,
+                          public content::NotificationObserver,
+                          public base::NonThreadSafe {
  public:
   VariationsService();
   virtual ~VariationsService();
@@ -44,16 +49,15 @@ class VariationsService : public net::URLFetcherDelegate {
   // |CreateTrialsFromSeed|.
   void StartRepeatedVariationsSeedFetch();
 
-  // Starts the fetching process once, where |OnURLFetchComplete| is called with
-  // the response.
-  void FetchVariationsSeed();
-
-  // net::URLFetcherDelegate implementation:
-  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
-
   // Register Variations related prefs in Local State.
   static void RegisterPrefs(PrefService* prefs);
 
+ protected:
+  // Starts the fetching process once, where |OnURLFetchComplete| is called with
+  // the response. This is protected so we can override this for testing
+  // purposes.
+  virtual void FetchVariationsSeed();
+
  private:
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyChannel);
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyLocale);
@@ -66,6 +70,14 @@ class VariationsService : public net::URLFetcherDelegate {
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, StoreSeed);
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, ValidateStudy);
 
+  // Overridden from content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // Overridden from net::URLFetcherDelegate:
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
   // Store the given seed data to the given local prefs. Note that |seed_data|
   // is assumed to be the raw serialized protobuf data stored in a string. It
   // will be Base64Encoded for storage. If the string is invalid or the encoding
@@ -125,10 +137,10 @@ class VariationsService : public net::URLFetcherDelegate {
   // is pending, and will be reset by |OnURLFetchComplete|.
   scoped_ptr<net::URLFetcher> pending_seed_request_;
 
-  // The URL to use for querying the variations server.
+  // The URL to use for querying the Variations server.
   GURL variations_server_url_;
 
-  // Cached serial number from the most recently fetched variations seed.
+  // Cached serial number from the most recently fetched Variations seed.
   std::string variations_serial_number_;
 
   // Tracks whether |CreateTrialsFromSeed| has been called, to ensure that
@@ -139,6 +151,9 @@ class VariationsService : public net::URLFetcherDelegate {
   // member so if VariationsService goes out of scope, the timer is
   // automatically canceled.
   base::RepeatingTimer<VariationsService> timer_;
+
+  // The registrar used to manage our Notification registrations.
+  content::NotificationRegistrar registrar_;
 };
 
 }  // namespace chrome_variations

chrome/browser/metrics/variations_service_unittest.cc

@@ -6,15 +6,38 @@
 #include "base/string_split.h"
 #include "chrome/browser/metrics/proto/study.pb.h"
 #include "chrome/browser/metrics/variations_service.h"
+#include "chrome/browser/upgrade_detector.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chrome_variations {
 
 namespace {
 
+// A test class used to validate expected functionality in VariationsService.
+class TestVariationsService : public VariationsService {
+ public:
+  TestVariationsService() : fetch_attempted_(false) {}
+  virtual ~TestVariationsService() {}
+
+  bool fetch_attempted() { return fetch_attempted_; }
+
+ protected:
+  virtual void FetchVariationsSeed() OVERRIDE {
+    fetch_attempted_ = true;
+  }
+
+ private:
+  bool fetch_attempted_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestVariationsService);
+};
+
 // Converts |time| to Study proto format.
 int64 TimeToProtoTime(const base::Time& time) {
   return (time - base::Time::UnixEpoch()).InSeconds();
@@ -37,6 +60,21 @@ chrome_variations::TrialsSeed CreateTestSeed() {
 
 }  // namespace
 
+TEST(VariationsServiceTest, AttemptFetchOnAutoUpdate) {
+  // Simulate an auto-update and ensure that the VariationsService attempts
+  // to fetch the Variations seed.
+  MessageLoopForUI message_loop;
+  content::TestBrowserThread ui_thread(content::BrowserThread::UI,
+                                       &message_loop);
+  TestVariationsService test_service;
+  EXPECT_FALSE(test_service.fetch_attempted());
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
+      content::Source<UpgradeDetector>(UpgradeDetector::GetInstance()),
+      content::NotificationService::NoDetails());
+  EXPECT_TRUE(test_service.fetch_attempted());
+}
+
 TEST(VariationsServiceTest, CheckStudyChannel) {
   const chrome::VersionInfo::Channel channels[] = {
     chrome::VersionInfo::CHANNEL_CANARY,