Skip to content

GitLab

T
tangled
Commit 6015537e authored by stevet@chromium.org's avatar stevet@chromium.org
Browse files
Options

Revert 150949 - 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

TBR=stevet@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10828255

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151031 0039d316-1c4b-4281-b951-d872f2087c98
parent cc4b9e12
Show whitespace changes
Inline Side-by-side
Showing with 21 additions and 95 deletions
chrome/browser/metrics/variations_service.cc
View file @ 6015537e
...@@ -16,13 +16,10 @@ ...@@ -16,13 +16,10 @@
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/metrics/proto/trials_seed.pb.h" #include "chrome/browser/metrics/proto/trials_seed.pb.h"
#include "chrome/browser/prefs/pref_service.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/chrome_switches.h"
#include "chrome/common/metrics/experiments_helper.h" #include "chrome/common/metrics/experiments_helper.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_service.h"
#include "content/public/common/url_fetcher.h" #include "content/public/common/url_fetcher.h"
#include "googleurl/src/gurl.h" #include "googleurl/src/gurl.h"
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
...@@ -35,7 +32,7 @@ namespace chrome_variations { ...@@ -35,7 +32,7 @@ namespace chrome_variations {
namespace { namespace {
// Default server of variations seed info. // Default server of Variations seed info.
const char kDefaultVariationsServerURL[] = const char kDefaultVariationsServerURL[] =
"https://clients4.google.com/chrome-variations/seed"; "https://clients4.google.com/chrome-variations/seed";
const int kMaxRetrySeedFetch = 5; const int kMaxRetrySeedFetch = 5;
...@@ -87,7 +84,7 @@ base::Time ConvertStudyDateToBaseTime(int64 date_time) { ...@@ -87,7 +84,7 @@ base::Time ConvertStudyDateToBaseTime(int64 date_time) {
return base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(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() { GURL GetVariationsServerURL() {
std::string server_url(CommandLine::ForCurrentProcess()->GetSwitchValueASCII( std::string server_url(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kVariationsServerURL)); switches::kVariationsServerURL));
...@@ -103,8 +100,6 @@ GURL GetVariationsServerURL() { ...@@ -103,8 +100,6 @@ GURL GetVariationsServerURL() {
VariationsService::VariationsService() VariationsService::VariationsService()
: variations_server_url_(GetVariationsServerURL()), : variations_server_url_(GetVariationsServerURL()),
create_trials_from_seed_called_(false) { create_trials_from_seed_called_(false) {
registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
content::Source<UpgradeDetector>(UpgradeDetector::GetInstance()));
} }
VariationsService::~VariationsService() {} VariationsService::~VariationsService() {}
...@@ -138,7 +133,7 @@ bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) { ...@@ -138,7 +133,7 @@ bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) {
} }
void VariationsService::StartRepeatedVariationsSeedFetch() { void VariationsService::StartRepeatedVariationsSeedFetch() {
DCHECK(CalledOnValidThread()); DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
// Check that |CreateTrialsFromSeed| was called, which is necessary to // Check that |CreateTrialsFromSeed| was called, which is necessary to
// retrieve the serial number that will be sent to the server. // retrieve the serial number that will be sent to the server.
...@@ -152,15 +147,8 @@ void VariationsService::StartRepeatedVariationsSeedFetch() { ...@@ -152,15 +147,8 @@ void VariationsService::StartRepeatedVariationsSeedFetch() {
this, &VariationsService::FetchVariationsSeed); 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() { void VariationsService::FetchVariationsSeed() {
DCHECK(CalledOnValidThread()); DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
const bool is_offline = net::NetworkChangeNotifier::IsOffline(); const bool is_offline = net::NetworkChangeNotifier::IsOffline();
UMA_HISTOGRAM_BOOLEAN("Variations.NetworkAvailability", !is_offline); UMA_HISTOGRAM_BOOLEAN("Variations.NetworkAvailability", !is_offline);
...@@ -183,22 +171,6 @@ void VariationsService::FetchVariationsSeed() { ...@@ -183,22 +171,6 @@ void VariationsService::FetchVariationsSeed() {
pending_seed_request_->Start(); 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) { void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK_EQ(pending_seed_request_.get(), source); DCHECK_EQ(pending_seed_request_.get(), source);
// When we're done handling the request, the fetcher will be deleted. // When we're done handling the request, the fetcher will be deleted.
...@@ -226,6 +198,13 @@ void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) { ...@@ -226,6 +198,13 @@ void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
StoreSeedData(seed_data, response_date, g_browser_process->local_state()); 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, bool VariationsService::StoreSeedData(const std::string& seed_data,
const base::Time& seed_date, const base::Time& seed_date,
PrefService* local_prefs) { PrefService* local_prefs) {
......
chrome/browser/metrics/variations_service.h
View file @ 6015537e
...@@ -11,14 +11,11 @@ ...@@ -11,14 +11,11 @@
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/threading/non_thread_safe.h"
#include "base/time.h" #include "base/time.h"
#include "base/timer.h" #include "base/timer.h"
#include "chrome/browser/metrics/proto/study.pb.h" #include "chrome/browser/metrics/proto/study.pb.h"
#include "chrome/browser/metrics/proto/trials_seed.pb.h" #include "chrome/browser/metrics/proto/trials_seed.pb.h"
#include "chrome/common/chrome_version_info.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 "googleurl/src/gurl.h"
#include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_fetcher_delegate.h"
...@@ -32,9 +29,7 @@ namespace chrome_variations { ...@@ -32,9 +29,7 @@ namespace chrome_variations {
// Used to setup field trials based on stored variations seed data, and fetch // Used to setup field trials based on stored variations seed data, and fetch
// new seed data from the variations server. // new seed data from the variations server.
class VariationsService : public net::URLFetcherDelegate, class VariationsService : public net::URLFetcherDelegate {
public content::NotificationObserver,
public base::NonThreadSafe {
public: public:
VariationsService(); VariationsService();
virtual ~VariationsService(); virtual ~VariationsService();
...@@ -49,15 +44,16 @@ class VariationsService : public net::URLFetcherDelegate, ...@@ -49,15 +44,16 @@ class VariationsService : public net::URLFetcherDelegate,
// |CreateTrialsFromSeed|. // |CreateTrialsFromSeed|.
void StartRepeatedVariationsSeedFetch(); 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. // Register Variations related prefs in Local State.
static void RegisterPrefs(PrefService* prefs); 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: private:
FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyChannel); FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyChannel);
FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyLocale); FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyLocale);
...@@ -70,14 +66,6 @@ class VariationsService : public net::URLFetcherDelegate, ...@@ -70,14 +66,6 @@ class VariationsService : public net::URLFetcherDelegate,
FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, StoreSeed); FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, StoreSeed);
FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, ValidateStudy); 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| // 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 // 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 // will be Base64Encoded for storage. If the string is invalid or the encoding
...@@ -137,10 +125,10 @@ class VariationsService : public net::URLFetcherDelegate, ...@@ -137,10 +125,10 @@ class VariationsService : public net::URLFetcherDelegate,
// is pending, and will be reset by |OnURLFetchComplete|. // is pending, and will be reset by |OnURLFetchComplete|.
scoped_ptr<net::URLFetcher> pending_seed_request_; 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_; 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_; std::string variations_serial_number_;
// Tracks whether |CreateTrialsFromSeed| has been called, to ensure that // Tracks whether |CreateTrialsFromSeed| has been called, to ensure that
...@@ -151,9 +139,6 @@ class VariationsService : public net::URLFetcherDelegate, ...@@ -151,9 +139,6 @@ class VariationsService : public net::URLFetcherDelegate,
// member so if VariationsService goes out of scope, the timer is // member so if VariationsService goes out of scope, the timer is
// automatically canceled. // automatically canceled.
base::RepeatingTimer<VariationsService> timer_; base::RepeatingTimer<VariationsService> timer_;
// The registrar used to manage our Notification registrations.
content::NotificationRegistrar registrar_;
}; };
} // namespace chrome_variations } // namespace chrome_variations
......
chrome/browser/metrics/variations_service_unittest.cc
View file @ 6015537e
...@@ -6,38 +6,15 @@ ...@@ -6,38 +6,15 @@
#include "base/string_split.h" #include "base/string_split.h"
#include "chrome/browser/metrics/proto/study.pb.h" #include "chrome/browser/metrics/proto/study.pb.h"
#include "chrome/browser/metrics/variations_service.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/chrome_version_info.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/test/base/testing_pref_service.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" #include "testing/gtest/include/gtest/gtest.h"
namespace chrome_variations { namespace chrome_variations {
namespace { 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. // Converts |time| to Study proto format.
int64 TimeToProtoTime(const base::Time& time) { int64 TimeToProtoTime(const base::Time& time) {
return (time - base::Time::UnixEpoch()).InSeconds(); return (time - base::Time::UnixEpoch()).InSeconds();
...@@ -60,21 +37,6 @@ chrome_variations::TrialsSeed CreateTestSeed() { ...@@ -60,21 +37,6 @@ chrome_variations::TrialsSeed CreateTestSeed() {
} // namespace } // 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) { TEST(VariationsServiceTest, CheckStudyChannel) {
const chrome::VersionInfo::Channel channels[] = { const chrome::VersionInfo::Channel channels[] = {
chrome::VersionInfo::CHANNEL_CANARY, chrome::VersionInfo::CHANNEL_CANARY,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment