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

chrome/browser/metrics/variations_service.cc

 // Copyright (c) 2012 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_service.h"
 
 #include <set>
 
 #include "base/base64.h"
 #include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
 #include "chrome/browser/prefs/pref_service.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"
 #include "net/base/network_change_notifier.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
 
 namespace chrome_variations {
 
@@ skipping 60 matching lines @@
   GURL url_as_gurl = GURL(server_url);
   DCHECK(url_as_gurl.is_valid());
   return url_as_gurl;
 }
 
 }  // namespace
 
 VariationsService::VariationsService()
     : variations_server_url_(GetVariationsServerURL()),
       create_trials_from_seed_called_(false) {
+  registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
+                 content::NotificationService::AllSources());

[Ilya Sherman, 2012/08/07 20:31:51]

NotificationService::AllSources() is generally discouraged.  Do you know what
the source for this notification is?

[SteveT, 2012/08/08 18:11:39]

This is an UpgradeDetector, which conveniently is a Singleton, so I'll call
GetInstance here.

Note that none of the other listeners of NOTIFICATION_UPGRADE_RECOMMENDED have a
specific source... Do you think it'd be wise to (in a separate patch) change
them all to listen to UpgradeDetector::GetInstance() as a source?

[Ilya Sherman, 2012/08/08 21:33:47]

Lovely :)
Yes, that would be great -- thanks for offering to clean that up! :)

 }
 
 VariationsService::~VariationsService() {}
 
 bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) {
   create_trials_from_seed_called_ = true;
 
   TrialsSeed seed;
   if (!LoadTrialsSeedFromPref(local_prefs, &seed))
     return false;
@@ skipping 27 matching lines @@
   DCHECK(create_trials_from_seed_called_);
 
   // Perform the first fetch.
   FetchVariationsSeed();
 
   // Repeat this periodically.
   timer_.Start(FROM_HERE, base::TimeDelta::FromHours(kSeedFetchPeriodHours),
                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));
 
   const bool is_offline = net::NetworkChangeNotifier::IsOffline();
   UMA_HISTOGRAM_BOOLEAN("Variations.NetworkAvailability", !is_offline);
   if (is_offline) {
     DVLOG(1) << "Network was offline.";
     return;
   }
 
   pending_seed_request_.reset(net::URLFetcher::Create(
       variations_server_url_, net::URLFetcher::GET, this));
   pending_seed_request_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                                       net::LOAD_DO_NOT_SAVE_COOKIES);
   pending_seed_request_->SetRequestContext(
       g_browser_process->system_request_context());
   pending_seed_request_->SetMaxRetries(kMaxRetrySeedFetch);
   if (!variations_serial_number_.empty()) {
     pending_seed_request_->AddExtraRequestHeader("If-Match:" +
                                                  variations_serial_number_);
   }
   pending_seed_request_->Start();
 }
 
+void VariationsService::Observe(int type,
+                                const content::NotificationSource& source,
+                                const content::NotificationDetails& details) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  DCHECK(type == chrome::NOTIFICATION_UPGRADE_RECOMMENDED);

[Ilya Sherman, 2012/08/07 20:31:51]

nit: DCHECK_EQ(chrome::NOTIFICATION_UPGRADE_RECOMMENDED, type);

[SteveT, 2012/08/08 18:11:39]

Done.

+
+  // An upgrade is ready, so attempt to fetch the Variations seed in case there

[Ilya Sherman, 2012/08/07 20:31:51]

Out of curiosity, when do we capitalize jargon like "variations", "seed", etc.? 
(Obviously not a big deal, I'm just curious.)

[SteveT, 2012/08/08 18:11:39]

Hm, good question. I think in this file we're sort of using it as the name for
the feature (as opposed to the internal code name), so that's why we capitalize
it. This is not consistent, though, so I've capitalized "variations" across this
file.

Do push back if you think we're breaking some important style guide/grammar rule
:)

[Ilya Sherman, 2012/08/08 21:33:47]

Nope, was just curious.  Thanks :)

+  // 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.
   scoped_ptr<const net::URLFetcher> request(
       pending_seed_request_.release());
   if (request->GetStatus().status() != net::URLRequestStatus::SUCCESS) {
     DVLOG(1) << "Variations server request failed.";
     return;
   }
 
   if (request->GetResponseCode() != 200) {
     DVLOG(1) << "Variations server request returned non-200 response code: "
              << request->GetResponseCode();
     return;
   }
 
   std::string seed_data;
   bool success = request->GetResponseAsString(&seed_data);
   DCHECK(success);
 
   base::Time response_date;
   success = request->GetResponseHeaders()->GetDateValue(&response_date);
   DCHECK(success || response_date.is_null());
 
   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) {
   // Only store the seed data if it parses correctly.
   TrialsSeed seed;
   if (!seed.ParseFromString(seed_data)) {
     VLOG(1) << "Variations Seed data from server is not in valid proto format, "
             << "rejecting the seed.";
     return false;
   }
@@ skipping 246 matching lines @@
                                                           variation_id);
     }
   }
 
   trial->SetForced();
   if (IsStudyExpired(study, reference_date))
     trial->Disable();
 }
 
 }  // namespace chrome_variations