diff options
Diffstat (limited to 'components')
-rw-r--r-- | components/variations/service/variations_service.cc | 75 | ||||
-rw-r--r-- | components/variations/service/variations_service.h | 5 | ||||
-rw-r--r-- | components/variations/service/variations_service_unittest.cc | 115 |
3 files changed, 158 insertions, 37 deletions
diff --git a/components/variations/service/variations_service.cc b/components/variations/service/variations_service.cc index 20ba860..2607ead 100644 --- a/components/variations/service/variations_service.cc +++ b/components/variations/service/variations_service.cc @@ -48,6 +48,9 @@ const int kMaxRetrySeedFetch = 5; // For the HTTP date headers, the resolution of the server time is 1 second. const int64 kServerTimeResolutionMs = 1000; +// Maximum age permitted for a variations seed, in days. +const int kMaxVariationsSeedAgeDays = 30; + // Wrapper around channel checking, used to enable channel mocking for // testing. If the current browser channel is not UNKNOWN, this will return // that channel value. Otherwise, if the fake channel flag is provided, this @@ -130,6 +133,20 @@ void RecordRequestsAllowedHistogram(ResourceRequestsAllowedState state) { RESOURCE_REQUESTS_ALLOWED_ENUM_SIZE); } +enum VariationsSeedExpiry { + VARIATIONS_SEED_EXPIRY_NOT_EXPIRED, + VARIATIONS_SEED_EXPIRY_FETCH_TIME_MISSING, + VARIATIONS_SEED_EXPIRY_EXPIRED, + VARIATIONS_SEED_EXPIRY_ENUM_SIZE, +}; + +// Records UMA histogram with the result of the variations seed expiry check. +void RecordCreateTrialsSeedExpiry(VariationsSeedExpiry expiry_check_result) { + UMA_HISTOGRAM_ENUMERATION("Variations.CreateTrials.SeedExpiry", + expiry_check_result, + VARIATIONS_SEED_EXPIRY_ENUM_SIZE); +} + // Converts ResourceRequestAllowedNotifier::State to the corresponding // ResourceRequestsAllowedState value. ResourceRequestsAllowedState ResourceRequestStateToHistogramValue( @@ -217,6 +234,9 @@ VariationsService::VariationsService( resource_request_allowed_notifier_(notifier.Pass()), request_count_(0), weak_ptr_factory_(this) { + DCHECK(client_.get()); + DCHECK(resource_request_allowed_notifier_.get()); + resource_request_allowed_notifier_->Init(this); } @@ -229,9 +249,28 @@ bool VariationsService::CreateTrialsFromSeed(base::FeatureList* feature_list) { create_trials_from_seed_called_ = true; variations::VariationsSeed seed; - if (!seed_store_.LoadSeed(&seed)) + if (!LoadSeed(&seed)) return false; + const int64 last_fetch_time_internal = + local_state_->GetInt64(prefs::kVariationsLastFetchTime); + const base::Time last_fetch_time = + base::Time::FromInternalValue(last_fetch_time_internal); + if (last_fetch_time.is_null()) { + // If the last fetch time is missing and we have a seed, then this must be + // the first run of Chrome. Store the current time as the last fetch time. + RecordLastFetchTime(); + RecordCreateTrialsSeedExpiry(VARIATIONS_SEED_EXPIRY_FETCH_TIME_MISSING); + } else { + // Reject the seed if it is more than 30 days old. + const base::TimeDelta seed_age = base::Time::Now() - last_fetch_time; + if (seed_age.InDays() > kMaxVariationsSeedAgeDays) { + RecordCreateTrialsSeedExpiry(VARIATIONS_SEED_EXPIRY_EXPIRED); + return false; + } + RecordCreateTrialsSeedExpiry(VARIATIONS_SEED_EXPIRY_NOT_EXPIRED); + } + const base::Version current_version(version_info::GetVersionNumber()); if (!current_version.IsValid()) return false; @@ -259,8 +298,6 @@ bool VariationsService::CreateTrialsFromSeed(base::FeatureList* feature_list) { // Log the "freshness" of the seed that was just used. The freshness is the // time between the last successful seed download and now. - const int64 last_fetch_time_internal = - local_state_->GetInt64(prefs::kVariationsLastFetchTime); if (last_fetch_time_internal) { const base::TimeDelta delta = now - base::Time::FromInternalValue(last_fetch_time_internal); @@ -269,34 +306,6 @@ bool VariationsService::CreateTrialsFromSeed(base::FeatureList* feature_list) { 1, base::TimeDelta::FromDays(30).InMinutes(), 50); } - // Log the skew between the seed date and the system clock/build time to - // analyze whether either could be used to make old variations seeds expire - // after some time. - const int64 seed_date_internal = - local_state_->GetInt64(prefs::kVariationsSeedDate); - if (seed_date_internal) { - const base::Time seed_date = - base::Time::FromInternalValue(seed_date_internal); - const int system_clock_delta_days = (now - seed_date).InDays(); - if (system_clock_delta_days < 0) { - UMA_HISTOGRAM_COUNTS_100("Variations.SeedDateSkew.SystemClockBehindBy", - -system_clock_delta_days); - } else { - UMA_HISTOGRAM_COUNTS_100("Variations.SeedDateSkew.SystemClockAheadBy", - system_clock_delta_days); - } - - const int build_time_delta_days = - (base::GetBuildTime() - seed_date).InDays(); - if (build_time_delta_days < 0) { - UMA_HISTOGRAM_COUNTS_100("Variations.SeedDateSkew.BuildTimeBehindBy", - -build_time_delta_days); - } else { - UMA_HISTOGRAM_COUNTS_100("Variations.SeedDateSkew.BuildTimeAheadBy", - build_time_delta_days); - } - } - return true; } @@ -525,6 +534,10 @@ bool VariationsService::StoreSeed(const std::string& seed_data, return true; } +bool VariationsService::LoadSeed(VariationsSeed* seed) { + return seed_store_.LoadSeed(seed); +} + void VariationsService::FetchVariationsSeed() { DCHECK(thread_checker_.CalledOnValidThread()); diff --git a/components/variations/service/variations_service.h b/components/variations/service/variations_service.h index 90f7c97..e9b786f 100644 --- a/components/variations/service/variations_service.h +++ b/components/variations/service/variations_service.h @@ -207,6 +207,11 @@ class VariationsService LOAD_COUNTRY_MAX, }; + // Loads the seed from the variations store into |seed|. If successfull, + // |seed| will contain the loaded data and true is returned. Set as virtual + // so that it can be overridden by tests. + virtual bool LoadSeed(VariationsSeed* seed); + // Checks if prerequisites for fetching the Variations seed are met, and if // so, performs the actual fetch using |DoActualFetch|. void FetchVariationsSeed(); diff --git a/components/variations/service/variations_service_unittest.cc b/components/variations/service/variations_service_unittest.cc index 5f2382b..1b85ce7 100644 --- a/components/variations/service/variations_service_unittest.cc +++ b/components/variations/service/variations_service_unittest.cc @@ -7,6 +7,7 @@ #include <vector> #include "base/base64.h" +#include "base/feature_list.h" #include "base/json/json_string_value_serializer.h" #include "base/message_loop/message_loop.h" #include "base/prefs/testing_pref_service.h" @@ -106,21 +107,28 @@ class TestVariationsService : public VariationsService { VariationsService::DoActualFetch(); } - protected: bool StoreSeed(const std::string& seed_data, const std::string& seed_signature, const std::string& country_code, const base::Time& date_fetched, bool is_delta_compressed) override { seed_stored_ = true; + stored_seed_data_ = seed_data; stored_country_ = country_code; return true; } private: + bool LoadSeed(VariationsSeed* seed) override { + if (!seed_stored_) + return false; + return seed->ParseFromString(stored_seed_data_); + } + bool intercepts_fetch_; bool fetch_attempted_; bool seed_stored_; + std::string stored_seed_data_; std::string stored_country_; DISALLOW_COPY_AND_ASSIGN(TestVariationsService); @@ -163,6 +171,12 @@ class TestVariationsServiceObserver : public VariationsService::Observer { DISALLOW_COPY_AND_ASSIGN(TestVariationsServiceObserver); }; +// Constants used to create the test seed. +const char kTestSeedStudyName[] = "test"; +const char kTestSeedExperimentName[] = "abc"; +const int kTestSeedExperimentProbability = 100; +const char kTestSeedSerialNumber[] = "123"; + // Populates |seed| with simple test data. The resulting seed will contain one // study called "test", which contains one experiment called "abc" with // probability weight 100. |seed|'s study field will be cleared before adding @@ -170,12 +184,12 @@ class TestVariationsServiceObserver : public VariationsService::Observer { variations::VariationsSeed CreateTestSeed() { variations::VariationsSeed seed; variations::Study* study = seed.add_study(); - study->set_name("test"); - study->set_default_experiment_name("abc"); + study->set_name(kTestSeedStudyName); + study->set_default_experiment_name(kTestSeedExperimentName); variations::Study_Experiment* experiment = study->add_experiment(); - experiment->set_name("abc"); - experiment->set_probability_weight(100); - seed.set_serial_number("123"); + experiment->set_name(kTestSeedExperimentName); + experiment->set_probability_weight(kTestSeedExperimentProbability); + seed.set_serial_number(kTestSeedSerialNumber); return seed; } @@ -220,6 +234,95 @@ class VariationsServiceTest : public ::testing::Test { DISALLOW_COPY_AND_ASSIGN(VariationsServiceTest); }; +TEST_F(VariationsServiceTest, CreateTrialsFromSeed) { + TestingPrefServiceSimple prefs; + VariationsService::RegisterPrefs(prefs.registry()); + + // Setup base::FeatureList. + base::FeatureList::ClearInstanceForTesting(); + base::FeatureList::SetInstance(make_scoped_ptr(new base::FeatureList())); + + // Create a local base::FieldTrialList, to hold the field trials created in + // this test. + base::FieldTrialList field_trial_list(nullptr); + + // Create a variations service. + TestVariationsService service( + make_scoped_ptr(new web_resource::TestRequestAllowedNotifier(&prefs)), + &prefs); + + // Store a seed. + service.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(), + std::string(), base::Time::Now(), false); + prefs.SetInt64(prefs::kVariationsLastFetchTime, + base::Time::Now().ToInternalValue()); + + // Check that field trials are created from the seed. Since the test study has + // only 1 experiment with 100% probability weight, we must be part of it. + EXPECT_TRUE(service.CreateTrialsFromSeed(base::FeatureList::GetInstance())); + EXPECT_EQ(base::FieldTrialList::FindFullName(kTestSeedStudyName), + kTestSeedExperimentName); +} + +TEST_F(VariationsServiceTest, CreateTrialsFromSeedNoLastFetchTime) { + TestingPrefServiceSimple prefs; + VariationsService::RegisterPrefs(prefs.registry()); + + // Setup base::FeatureList. + base::FeatureList::ClearInstanceForTesting(); + base::FeatureList::SetInstance(make_scoped_ptr(new base::FeatureList())); + + // Create a local base::FieldTrialList, to hold the field trials created in + // this test. + base::FieldTrialList field_trial_list(nullptr); + + // Create a variations service + TestVariationsService service( + make_scoped_ptr(new web_resource::TestRequestAllowedNotifier(&prefs)), + &prefs); + + // Store a seed. To simulate a first run, |prefs::kVariationsLastFetchTime| + // is left empty. + service.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(), + std::string(), base::Time::Now(), false); + EXPECT_EQ(0, prefs.GetInt64(prefs::kVariationsLastFetchTime)); + + // Check that field trials are created from the seed. Since the test study has + // only 1 experiment with 100% probability weight, we must be part of it. + EXPECT_TRUE(service.CreateTrialsFromSeed(base::FeatureList::GetInstance())); + EXPECT_EQ(base::FieldTrialList::FindFullName(kTestSeedStudyName), + kTestSeedExperimentName); +} + +TEST_F(VariationsServiceTest, CreateTrialsFromOutdatedSeed) { + TestingPrefServiceSimple prefs; + VariationsService::RegisterPrefs(prefs.registry()); + + // Setup base::FeatureList. + base::FeatureList::ClearInstanceForTesting(); + base::FeatureList::SetInstance(make_scoped_ptr(new base::FeatureList())); + + // Create a local base::FieldTrialList, to hold the field trials created in + // this test. + base::FieldTrialList field_trial_list(nullptr); + + // Create a variations service. + TestVariationsService service( + make_scoped_ptr(new web_resource::TestRequestAllowedNotifier(&prefs)), + &prefs); + + // Store a seed, with a fetch time 31 days in the past. + const base::Time seed_date = + base::Time::Now() - base::TimeDelta::FromDays(31); + service.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(), + std::string(), seed_date, false); + prefs.SetInt64(prefs::kVariationsLastFetchTime, seed_date.ToInternalValue()); + + // Check that field trials are not created from the seed. + EXPECT_FALSE(service.CreateTrialsFromSeed(base::FeatureList::GetInstance())); + EXPECT_TRUE(base::FieldTrialList::FindFullName(kTestSeedStudyName).empty()); +} + TEST_F(VariationsServiceTest, GetVariationsServerURL) { TestingPrefServiceSimple prefs; VariationsService::RegisterPrefs(prefs.registry()); |