Skip to content

GitLab

T
tangled
Commit f3aaa1c4 authored by Alexei Svitkine (slow)'s avatar Alexei Svitkine (slow) Committed by Commit Bot
Browse files
Options

Revert "Add the VariationsFieldTrialCreator class" 

This reverts commit 34b11e87.

Reason for revert: Causing crashes: crbug.com/742707

Original change's description:
> Add the VariationsFieldTrialCreator class
> 
> This is part 3 of 6 for FieldTrial refactoring for WebView
> 
> Part 1: https://chromium-review.googlesource.com/c/562098/
> Part 2: https://chromium-review.googlesource.com/c/561920/
> Part 3: https://chromium-review.googlesource.com/c/561922/
> Part 4: https://chromium-review.googlesource.com/c/561980/
> Part 5: https://chromium-review.googlesource.com/c/562417/
> Part 6: https://chromium-review.googlesource.com/c/562021/
> 
> This CL creates the VariationsFieldTrialCreator class. This class
> contains the code previously in VariationsService that
> is needed by both Chrome and WebView, mainly the portions
> needed to call CreateTrialsFromSeed.  The VariationsService
> class now contains an instance of this new class.
> 
> BUG=678288
> 
> Change-Id: Ic4340d5a0396cca56892a3b541e4cf6115bb8afb
> Reviewed-on: https://chromium-review.googlesource.com/561922
> Commit-Queue: Kyle Milka <kmilka@google.com>
> Reviewed-by: Alexei Svitkine (slow) <asvitkine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486590}

TBR=asvitkine@chromium.org,timav@chromium.org,paulmiller@chromium.org,kmilka@google.com

Change-Id: Idf0f12b5bd6d1bb7edf41cbdc6f767f8790de6e9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 678288
Reviewed-on: https://chromium-review.googlesource.com/571920Reviewed-by: default avatarAlexei Svitkine (slow) <asvitkine@chromium.org>
Commit-Queue: Alexei Svitkine (slow) <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486755}
parent 1a6a1f02
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 405 additions and 765 deletions
components/variations/service/BUILD.gn
View file @ f3aaa1c4
......@@ -6,8 +6,6 @@ static_library("service") {
sources = [
"ui_string_overrider.cc",
"ui_string_overrider.h",
"variations_field_trial_creator.cc",
"variations_field_trial_creator.h",
"variations_service.cc",
"variations_service.h",
"variations_service_client.h",
......@@ -32,7 +30,6 @@ static_library("service") {
source_set("unit_tests") {
testonly = true
sources = [
"field_trial_creator_unittest.cc",
"ui_string_overrider_unittest.cc",
"variations_service_unittest.cc",
]
......
components/variations/service/field_trial_creator_unittest.cc deleted 100644 → 0
View file @ 1a6a1f02
// Copyright 2017 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 "components/variations/service/variations_field_trial_creator.h"
#include <stddef.h>
#include "base/feature_list.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/message_loop/message_loop.h"
#include "base/version.h"
#include "components/metrics/clean_exit_beacon.h"
#include "components/metrics/client_info.h"
#include "components/metrics/metrics_state_manager.h"
#include "components/metrics/test_enabled_state_provider.h"
#include "components/prefs/testing_pref_service.h"
#include "components/variations/pref_names.h"
#include "components/variations/service/variations_service.h"
#include "components/web_resource/resource_request_allowed_notifier_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace variations {
namespace {
// A stub for the metrics state manager.
void StubStoreClientInfo(const metrics::ClientInfo& /* client_info */) {}
// A stub for the metrics state manager.
std::unique_ptr<metrics::ClientInfo> StubLoadClientInfo() {
return std::unique_ptr<metrics::ClientInfo>();
}
class TestVariationsServiceClient : public VariationsServiceClient {
public:
TestVariationsServiceClient() {}
~TestVariationsServiceClient() override {}
// VariationsServiceClient:
std::string GetApplicationLocale() override { return std::string(); }
base::Callback<base::Version(void)> GetVersionForSimulationCallback()
override {
return base::Callback<base::Version(void)>();
}
net::URLRequestContextGetter* GetURLRequestContext() override {
return nullptr;
}
network_time::NetworkTimeTracker* GetNetworkTimeTracker() override {
return nullptr;
}
version_info::Channel GetChannel() override {
return version_info::Channel::UNKNOWN;
}
bool OverridesRestrictParameter(std::string* parameter) override {
if (restrict_parameter_.empty())
return false;
*parameter = restrict_parameter_;
return true;
}
void set_restrict_parameter(const std::string& value) {
restrict_parameter_ = value;
}
private:
std::string restrict_parameter_;
DISALLOW_COPY_AND_ASSIGN(TestVariationsServiceClient);
};
class TestVariationsFieldTrialCreator : public VariationsFieldTrialCreator {
public:
TestVariationsFieldTrialCreator(
std::unique_ptr<web_resource::TestRequestAllowedNotifier> test_notifier,
PrefService* local_state,
TestVariationsServiceClient* client)
: VariationsFieldTrialCreator(local_state, client, UIStringOverrider()),
client_(client) {
SetCreateTrialsFromSeedCalledForTesting(true);
}
~TestVariationsFieldTrialCreator() {
delete client_;
client_ = 0;
}
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,
bool is_gzip_compressed) {
seed_stored_ = true;
stored_seed_data_ = seed_data;
stored_country_ = country_code;
delta_compressed_seed_ = is_delta_compressed;
gzip_compressed_seed_ = is_gzip_compressed;
return true;
}
private:
bool LoadSeed(VariationsSeed* seed) override {
if (!seed_stored_)
return false;
return seed->ParseFromString(stored_seed_data_);
}
bool seed_stored_;
std::string stored_seed_data_;
std::string stored_country_;
bool delta_compressed_seed_;
bool gzip_compressed_seed_;
TestVariationsServiceClient* client_;
DISALLOW_COPY_AND_ASSIGN(TestVariationsFieldTrialCreator);
};
// 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
// the new study.
VariationsSeed CreateTestSeed() {
VariationsSeed seed;
Study* study = seed.add_study();
study->set_name(kTestSeedStudyName);
study->set_default_experiment_name(kTestSeedExperimentName);
Study_Experiment* experiment = study->add_experiment();
experiment->set_name(kTestSeedExperimentName);
experiment->set_probability_weight(kTestSeedExperimentProbability);
seed.set_serial_number(kTestSeedSerialNumber);
return seed;
}
// Serializes |seed| to protobuf binary format.
std::string SerializeSeed(const VariationsSeed& seed) {
std::string serialized_seed;
seed.SerializeToString(&serialized_seed);
return serialized_seed;
}
} // namespace
class FieldTrialCreatorTest : public ::testing::Test {
protected:
FieldTrialCreatorTest()
: enabled_state_provider_(
new metrics::TestEnabledStateProvider(false, false)) {
VariationsService::RegisterPrefs(prefs_.registry());
metrics::CleanExitBeacon::RegisterPrefs(prefs_.registry());
metrics::MetricsStateManager::RegisterPrefs(prefs_.registry());
}
metrics::MetricsStateManager* GetMetricsStateManager() {
// Lazy-initialize the metrics_state_manager so that it correctly reads the
// stability state from prefs after tests have a chance to initialize it.
if (!metrics_state_manager_) {
metrics_state_manager_ = metrics::MetricsStateManager::Create(
&prefs_, enabled_state_provider_.get(), base::string16(),
base::Bind(&StubStoreClientInfo), base::Bind(&StubLoadClientInfo));
}
return metrics_state_manager_.get();
}
protected:
TestingPrefServiceSimple prefs_;
private:
base::MessageLoop message_loop_;
std::unique_ptr<metrics::TestEnabledStateProvider> enabled_state_provider_;
std::unique_ptr<metrics::MetricsStateManager> metrics_state_manager_;
DISALLOW_COPY_AND_ASSIGN(FieldTrialCreatorTest);
};
TEST_F(FieldTrialCreatorTest, CreateTrialsFromSeed) {
// Create a local base::FieldTrialList, to hold the field trials created in
// this test.
base::FieldTrialList field_trial_list(nullptr);
// Create a variations service.
TestVariationsFieldTrialCreator field_trial_creator(
base::MakeUnique<web_resource::TestRequestAllowedNotifier>(&prefs_),
&prefs_, new TestVariationsServiceClient());
field_trial_creator.SetCreateTrialsFromSeedCalledForTesting(false);
// Store a seed.
field_trial_creator.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(),
std::string(), base::Time::Now(), false, 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(field_trial_creator.CreateTrialsFromSeed(
std::unique_ptr<const base::FieldTrial::EntropyProvider>(nullptr),
base::FeatureList::GetInstance()));
EXPECT_EQ(kTestSeedExperimentName,
base::FieldTrialList::FindFullName(kTestSeedStudyName));
}
TEST_F(FieldTrialCreatorTest, CreateTrialsFromSeedNoLastFetchTime) {
// Create a local base::FieldTrialList, to hold the field trials created in
// this test.
base::FieldTrialList field_trial_list(nullptr);
// Create a variations service
TestVariationsFieldTrialCreator field_trial_creator(
base::MakeUnique<web_resource::TestRequestAllowedNotifier>(&prefs_),
&prefs_, new TestVariationsServiceClient());
field_trial_creator.SetCreateTrialsFromSeedCalledForTesting(false);
// Store a seed. To simulate a first run, |prefs::kVariationsLastFetchTime|
// is left empty.
field_trial_creator.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(),
std::string(), base::Time::Now(), false, 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(field_trial_creator.CreateTrialsFromSeed(
std::unique_ptr<const base::FieldTrial::EntropyProvider>(nullptr),
base::FeatureList::GetInstance()));
EXPECT_EQ(base::FieldTrialList::FindFullName(kTestSeedStudyName),
kTestSeedExperimentName);
}
TEST_F(FieldTrialCreatorTest, CreateTrialsFromOutdatedSeed) {
// Create a local base::FieldTrialList, to hold the field trials created in
// this test.
base::FieldTrialList field_trial_list(nullptr);
// Create a variations service.
TestVariationsFieldTrialCreator field_trial_creator(
base::MakeUnique<web_resource::TestRequestAllowedNotifier>(&prefs_),
&prefs_, new TestVariationsServiceClient());
field_trial_creator.SetCreateTrialsFromSeedCalledForTesting(false);
// Store a seed, with a fetch time 31 days in the past.
const base::Time seed_date =
base::Time::Now() - base::TimeDelta::FromDays(31);
field_trial_creator.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(),
std::string(), seed_date, false, false);
prefs_.SetInt64(prefs::kVariationsLastFetchTime, seed_date.ToInternalValue());
// Check that field trials are not created from the seed.
EXPECT_FALSE(field_trial_creator.CreateTrialsFromSeed(
std::unique_ptr<const base::FieldTrial::EntropyProvider>(nullptr),
base::FeatureList::GetInstance()));
EXPECT_TRUE(base::FieldTrialList::FindFullName(kTestSeedStudyName).empty());
}
} // namespace variations
components/variations/service/variations_field_trial_creator.h deleted 100644 → 0
View file @ 1a6a1f02
// Copyright 2017 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.
#ifndef COMPONENTS_VARIATIONS_FIELD_TRIAL_CREATOR_H_
#define COMPONENTS_VARIATIONS_FIELD_TRIAL_CREATOR_H_
#include <string>
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/metrics/field_trial.h"
#include "components/variations/client_filterable_state.h"
#include "components/variations/service/ui_string_overrider.h"
#include "components/variations/variations_seed_store.h"
namespace variations {
class VariationsServiceClient;
}
namespace variations {
// Used to setup field trials based on stored variations seed data.
class VariationsFieldTrialCreator {
public:
// Caller is responsible for ensuring that objects passed to the constructor
// stay valid for the lifetime of this object.
VariationsFieldTrialCreator(PrefService* local_state,
VariationsServiceClient* client,
const UIStringOverrider& ui_string_overrider);
~VariationsFieldTrialCreator();
// Returns what variations will consider to be the latest country. Returns
// empty if it is not available.
std::string GetLatestCountry() const;
// Creates field trials based on the variations seed loaded from local state.
// If there is a problem loading the seed data, all trials specified by the
// seed may not be created. Some field trials are configured to override or
// associate with (for reporting) specific features. These associations are
// registered with |feature_list|.
bool CreateTrialsFromSeed(
std::unique_ptr<const base::FieldTrial::EntropyProvider>
low_entropy_provider,
base::FeatureList* feature_list);
VariationsSeedStore& seed_store() { return seed_store_; }
const VariationsSeedStore& seed_store() const { return seed_store_; }
bool create_trials_from_seed_called() const {
return create_trials_from_seed_called_;
}
// Exposed for testing.
void SetCreateTrialsFromSeedCalledForTesting(bool called);
// Returns all of the client state used for filtering studies.
// As a side-effect, may update the stored permanent consistency country.
std::unique_ptr<ClientFilterableState> GetClientFilterableStateForVersion(
const base::Version& version);
// Loads the country code to use for filtering permanent consistency studies,
// updating the stored country code if the stored value was for a different
// Chrome version. The country used for permanent consistency studies is kept
// consistent between Chrome upgrades in order to avoid annoying the user due
// to experiment churn while traveling.
std::string LoadPermanentConsistencyCountry(
const base::Version& version,
const std::string& latest_country);
// Sets the stored permanent country pref for this client.
void StorePermanentCountry(const base::Version& version,
const std::string& country);
// Records the time of the most recent successful fetch.
void RecordLastFetchTime();
// 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);
private:
// Set of different possible values to report for the
// Variations.LoadPermanentConsistencyCountryResult histogram. This enum must
// be kept consistent with its counterpart in histograms.xml.
enum LoadPermanentConsistencyCountryResult {
LOAD_COUNTRY_NO_PREF_NO_SEED = 0,
LOAD_COUNTRY_NO_PREF_HAS_SEED,
LOAD_COUNTRY_INVALID_PREF_NO_SEED,
LOAD_COUNTRY_INVALID_PREF_HAS_SEED,
LOAD_COUNTRY_HAS_PREF_NO_SEED_VERSION_EQ,
LOAD_COUNTRY_HAS_PREF_NO_SEED_VERSION_NEQ,
LOAD_COUNTRY_HAS_BOTH_VERSION_EQ_COUNTRY_EQ,
LOAD_COUNTRY_HAS_BOTH_VERSION_EQ_COUNTRY_NEQ,
LOAD_COUNTRY_HAS_BOTH_VERSION_NEQ_COUNTRY_EQ,
LOAD_COUNTRY_HAS_BOTH_VERSION_NEQ_COUNTRY_NEQ,
LOAD_COUNTRY_MAX,
};
VariationsServiceClient* client_;
UIStringOverrider ui_string_overrider_;
// The pref service used to store persist the variations seed.
PrefService* local_state_;
VariationsSeedStore seed_store_;
// Tracks whether |CreateTrialsFromSeed| has been called, to ensure that
// it gets called prior to |StartRepeatedVariationsSeedFetch|.
bool create_trials_from_seed_called_;
SEQUENCE_CHECKER(sequence_checker_);
DISALLOW_COPY_AND_ASSIGN(VariationsFieldTrialCreator);
};
} // namespace variations
#endif // COMPONENTS_VARIATIONS_FIELD_TRIAL_CREATOR_H_
components/variations/service/variations_service.h
View file @ f3aaa1c4
......@@ -14,10 +14,10 @@
#include "base/memory/weak_ptr.h"
#include "base/metrics/field_trial.h"
#include "base/observer_list.h"
#include "base/threading/thread_checker.h"
#include "base/time/time.h"
#include "components/variations/client_filterable_state.h"
#include "components/variations/service/ui_string_overrider.h"
#include "components/variations/service/variations_field_trial_creator.h"
#include "components/variations/service/variations_service_client.h"
#include "components/variations/variations_request_scheduler.h"
#include "components/variations/variations_seed_simulator.h"
......@@ -230,6 +230,20 @@ 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);
// Returns all of the client state used for filtering studies.
// As a side-effect, may update the stored permanent consistency country.
std::unique_ptr<ClientFilterableState> GetClientFilterableStateForVersion(
const base::Version& version);
// Sets the stored permanent country pref for this client.
void StorePermanentCountry(const base::Version& version,
const std::string& country);
// Checks if prerequisites for fetching the Variations seed are met, and if
// so, performs the actual fetch using |DoActualFetch|.
void FetchVariationsSeed();
......@@ -250,6 +264,9 @@ class VariationsService
std::unique_ptr<variations::VariationsSeed> seed,
const base::Version& version);
// Record the time of the most recent successful fetch.
void RecordLastFetchTime();
// Loads the country code to use for filtering permanent consistency studies,
// updating the stored country code if the stored value was for a different
// Chrome version. The country used for permanent consistency studies is kept
......@@ -260,6 +277,7 @@ class VariationsService
const std::string& latest_country);
std::unique_ptr<VariationsServiceClient> client_;
UIStringOverrider ui_string_overrider_;
// The pref service used to store persist the variations seed.
PrefService* local_state_;
......@@ -272,6 +290,8 @@ class VariationsService
// either be Local State or Profile prefs.
PrefService* policy_pref_service_;
VariationsSeedStore seed_store_;
// Contains the scheduler instance that handles timing for requests to the
// server. Initially NULL and instantiated when the initial fetch is
// requested.
......@@ -289,6 +309,10 @@ class VariationsService
// The URL to use for querying the variations server.
GURL variations_server_url_;
// Tracks whether |CreateTrialsFromSeed| has been called, to ensure that
// it gets called prior to |StartRepeatedVariationsSeedFetch|.
bool create_trials_from_seed_called_;
// Tracks whether the initial request to the variations server had completed.
bool initial_request_completed_;
......@@ -312,10 +336,7 @@ class VariationsService
// List of observers of the VariationsService.
base::ObserverList<Observer> observer_list_;
// Member responsible for creating trials from a variations seed.
VariationsFieldTrialCreator field_trial_creator_;
SEQUENCE_CHECKER(sequence_checker_);
base::ThreadChecker thread_checker_;
base::WeakPtrFactory<VariationsService> weak_ptr_factory_;
......
components/variations/service/variations_service_unittest.cc
View file @ f3aaa1c4
......@@ -149,6 +149,11 @@ class TestVariationsService : public VariationsService {
}
private:
bool LoadSeed(VariationsSeed* seed) override {
if (!seed_stored_)
return false;
return seed->ParseFromString(stored_seed_data_);
}
bool intercepts_fetch_;
bool fetch_attempted_;
......@@ -293,6 +298,77 @@ class VariationsServiceTest : public ::testing::Test {
DISALLOW_COPY_AND_ASSIGN(VariationsServiceTest);
};
TEST_F(VariationsServiceTest, CreateTrialsFromSeed) {
// 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(
base::MakeUnique<web_resource::TestRequestAllowedNotifier>(&prefs_),
&prefs_, GetMetricsStateManager());
service.SetCreateTrialsFromSeedCalledForTesting(false);
// Store a seed.
service.StoreSeed(SerializeSeed(CreateTestSeed()), std::string(),
std::string(), base::Time::Now(), false, 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(kTestSeedExperimentName,
base::FieldTrialList::FindFullName(kTestSeedStudyName));
}
TEST_F(VariationsServiceTest, CreateTrialsFromSeedNoLastFetchTime) {
// 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(
base::MakeUnique<web_resource::TestRequestAllowedNotifier>(&prefs_),
&prefs_, GetMetricsStateManager());
service.SetCreateTrialsFromSeedCalledForTesting(false);
// 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, 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) {
// 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(
base::MakeUnique<web_resource::TestRequestAllowedNotifier>(&prefs_),
&prefs_, GetMetricsStateManager());
service.SetCreateTrialsFromSeedCalledForTesting(false);
// 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, 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) {
const std::string default_variations_url =
VariationsService::GetDefaultVariationsServerURLForTesting();
......
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