Commit dba2dc05 authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

[ios] Pass FieldTrial::EntropyProvider Pointer into CreateTrialsFromSeed

Now that |low_entropy_provider| is used later in
VariationsFieldTrialCreator::SetupFieldTrials(),
VariationsFieldTrialCreator::CreateTrialsFromSeed() shouldn't
own |low_entropy_provider|.

Bug: 1138603
Change-Id: I53ba8d2b7160bd3c9f02e92af4bd53ca72c63f25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2525856
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826023}
parent c43cab37
...@@ -163,8 +163,7 @@ std::string VariationsFieldTrialCreator::GetLatestCountry() const { ...@@ -163,8 +163,7 @@ std::string VariationsFieldTrialCreator::GetLatestCountry() const {
} }
bool VariationsFieldTrialCreator::CreateTrialsFromSeed( bool VariationsFieldTrialCreator::CreateTrialsFromSeed(
std::unique_ptr<const base::FieldTrial::EntropyProvider> const base::FieldTrial::EntropyProvider& low_entropy_provider,
low_entropy_provider,
base::FeatureList* feature_list, base::FeatureList* feature_list,
SafeSeedManager* safe_seed_manager) { SafeSeedManager* safe_seed_manager) {
TRACE_EVENT0("startup", "VariationsFieldTrialCreator::CreateTrialsFromSeed"); TRACE_EVENT0("startup", "VariationsFieldTrialCreator::CreateTrialsFromSeed");
...@@ -206,7 +205,7 @@ bool VariationsFieldTrialCreator::CreateTrialsFromSeed( ...@@ -206,7 +205,7 @@ bool VariationsFieldTrialCreator::CreateTrialsFromSeed(
seed, *client_filterable_state, seed, *client_filterable_state,
base::BindRepeating(&VariationsFieldTrialCreator::OverrideUIString, base::BindRepeating(&VariationsFieldTrialCreator::OverrideUIString,
base::Unretained(this)), base::Unretained(this)),
low_entropy_provider.get(), feature_list); low_entropy_provider, feature_list);
// Store into the |safe_seed_manager| the combined server and client data used // Store into the |safe_seed_manager| the combined server and client data used
// to create the field trials. But, as an optimization, skip this step when // to create the field trials. But, as an optimization, skip this step when
...@@ -522,16 +521,14 @@ bool VariationsFieldTrialCreator::SetupFieldTrials( ...@@ -522,16 +521,14 @@ bool VariationsFieldTrialCreator::SetupFieldTrials(
used_testing_config = true; used_testing_config = true;
} }
#endif // BUILDFLAG(FIELDTRIAL_TESTING_ENABLED) #endif // BUILDFLAG(FIELDTRIAL_TESTING_ENABLED)
const base::FieldTrial::EntropyProvider& low_entry_provider_ref =
*low_entropy_provider.get();
bool used_seed = false; bool used_seed = false;
if (!used_testing_config) { if (!used_testing_config) {
used_seed = CreateTrialsFromSeed(std::move(low_entropy_provider), used_seed = CreateTrialsFromSeed(*low_entropy_provider, feature_list.get(),
feature_list.get(), safe_seed_manager); safe_seed_manager);
} }
platform_field_trials->SetupFeatureControllingFieldTrials( platform_field_trials->SetupFeatureControllingFieldTrials(
used_seed, low_entry_provider_ref, feature_list.get()); used_seed, *low_entropy_provider, feature_list.get());
base::FeatureList::SetInstance(std::move(feature_list)); base::FeatureList::SetInstance(std::move(feature_list));
......
...@@ -154,8 +154,7 @@ class VariationsFieldTrialCreator { ...@@ -154,8 +154,7 @@ class VariationsFieldTrialCreator {
// successfully; and if so, stores the loaded variations state into the // successfully; and if so, stores the loaded variations state into the
// |safe_seed_manager|. // |safe_seed_manager|.
bool CreateTrialsFromSeed( bool CreateTrialsFromSeed(
std::unique_ptr<const base::FieldTrial::EntropyProvider> const base::FieldTrial::EntropyProvider& low_entropy_provider,
low_entropy_provider,
base::FeatureList* feature_list, base::FeatureList* feature_list,
SafeSeedManager* safe_seed_manager); SafeSeedManager* safe_seed_manager);
......
...@@ -197,7 +197,7 @@ void VariationsSeedProcessor::CreateTrialsFromSeed( ...@@ -197,7 +197,7 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
const VariationsSeed& seed, const VariationsSeed& seed,
const ClientFilterableState& client_state, const ClientFilterableState& client_state,
const UIStringOverrideCallback& override_callback, const UIStringOverrideCallback& override_callback,
const base::FieldTrial::EntropyProvider* low_entropy_provider, const base::FieldTrial::EntropyProvider& low_entropy_provider,
base::FeatureList* feature_list) { base::FeatureList* feature_list) {
std::vector<ProcessedStudy> filtered_studies; std::vector<ProcessedStudy> filtered_studies;
FilterAndValidateStudies(seed, client_state, &filtered_studies); FilterAndValidateStudies(seed, client_state, &filtered_studies);
...@@ -225,7 +225,7 @@ bool VariationsSeedProcessor::ShouldStudyUseLowEntropy(const Study& study) { ...@@ -225,7 +225,7 @@ bool VariationsSeedProcessor::ShouldStudyUseLowEntropy(const Study& study) {
void VariationsSeedProcessor::CreateTrialFromStudy( void VariationsSeedProcessor::CreateTrialFromStudy(
const ProcessedStudy& processed_study, const ProcessedStudy& processed_study,
const UIStringOverrideCallback& override_callback, const UIStringOverrideCallback& override_callback,
const base::FieldTrial::EntropyProvider* low_entropy_provider, const base::FieldTrial::EntropyProvider& low_entropy_provider,
base::FeatureList* feature_list) { base::FeatureList* feature_list) {
const Study& study = *processed_study.study(); const Study& study = *processed_study.study();
...@@ -290,7 +290,7 @@ void VariationsSeedProcessor::CreateTrialFromStudy( ...@@ -290,7 +290,7 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
study.name(), processed_study.total_probability(), study.name(), processed_study.total_probability(),
processed_study.GetDefaultExperimentName(), randomization_type, processed_study.GetDefaultExperimentName(), randomization_type,
randomization_seed, nullptr, randomization_seed, nullptr,
ShouldStudyUseLowEntropy(study) ? low_entropy_provider : nullptr)); ShouldStudyUseLowEntropy(study) ? &low_entropy_provider : nullptr));
bool has_overrides = false; bool has_overrides = false;
bool enables_or_disables_features = false; bool enables_or_disables_features = false;
......
...@@ -47,7 +47,7 @@ class VariationsSeedProcessor { ...@@ -47,7 +47,7 @@ class VariationsSeedProcessor {
const VariationsSeed& seed, const VariationsSeed& seed,
const ClientFilterableState& client_state, const ClientFilterableState& client_state,
const UIStringOverrideCallback& override_callback, const UIStringOverrideCallback& override_callback,
const base::FieldTrial::EntropyProvider* low_entropy_provider, const base::FieldTrial::EntropyProvider& low_entropy_provider,
base::FeatureList* feature_list); base::FeatureList* feature_list);
// If the given |study| should alwoys use low entropy. This is true for any // If the given |study| should alwoys use low entropy. This is true for any
...@@ -85,7 +85,7 @@ class VariationsSeedProcessor { ...@@ -85,7 +85,7 @@ class VariationsSeedProcessor {
void CreateTrialFromStudy( void CreateTrialFromStudy(
const ProcessedStudy& processed_study, const ProcessedStudy& processed_study,
const UIStringOverrideCallback& override_callback, const UIStringOverrideCallback& override_callback,
const base::FieldTrial::EntropyProvider* low_entropy_provider, const base::FieldTrial::EntropyProvider& low_entropy_provider,
base::FeatureList* feature_list); base::FeatureList* feature_list);
DISALLOW_COPY_AND_ASSIGN(VariationsSeedProcessor); DISALLOW_COPY_AND_ASSIGN(VariationsSeedProcessor);
......
...@@ -129,26 +129,28 @@ class VariationsSeedProcessorTest : public ::testing::Test { ...@@ -129,26 +129,28 @@ class VariationsSeedProcessorTest : public ::testing::Test {
} }
bool CreateTrialFromStudy(const Study& study) { bool CreateTrialFromStudy(const Study& study) {
base::MockEntropyProvider mock_low_entropy_provider(0.9);
return CreateTrialFromStudyWithFeatureListAndEntropyOverride( return CreateTrialFromStudyWithFeatureListAndEntropyOverride(
study, nullptr, base::FeatureList::GetInstance()); study, mock_low_entropy_provider, base::FeatureList::GetInstance());
} }
bool CreateTrialFromStudyWithEntropyOverride( bool CreateTrialFromStudyWithEntropyOverride(
const Study& study, const Study& study,
const base::FieldTrial::EntropyProvider* override_entropy_provider) { const base::FieldTrial::EntropyProvider& override_entropy_provider) {
return CreateTrialFromStudyWithFeatureListAndEntropyOverride( return CreateTrialFromStudyWithFeatureListAndEntropyOverride(
study, override_entropy_provider, base::FeatureList::GetInstance()); study, override_entropy_provider, base::FeatureList::GetInstance());
} }
bool CreateTrialFromStudyWithFeatureList(const Study& study, bool CreateTrialFromStudyWithFeatureList(const Study& study,
base::FeatureList* feature_list) { base::FeatureList* feature_list) {
return CreateTrialFromStudyWithFeatureListAndEntropyOverride(study, nullptr, base::MockEntropyProvider mock_low_entropy_provider(0.9);
feature_list); return CreateTrialFromStudyWithFeatureListAndEntropyOverride(
study, mock_low_entropy_provider, feature_list);
} }
bool CreateTrialFromStudyWithFeatureListAndEntropyOverride( bool CreateTrialFromStudyWithFeatureListAndEntropyOverride(
const Study& study, const Study& study,
const base::FieldTrial::EntropyProvider* override_entropy_provider, const base::FieldTrial::EntropyProvider& override_entropy_provider,
base::FeatureList* feature_list) { base::FeatureList* feature_list) {
ProcessedStudy processed_study; ProcessedStudy processed_study;
const bool is_expired = internal::IsStudyExpired(study, base::Time::Now()); const bool is_expired = internal::IsStudyExpired(study, base::Time::Now());
...@@ -289,9 +291,10 @@ TEST_F(VariationsSeedProcessorTest, ...@@ -289,9 +291,10 @@ TEST_F(VariationsSeedProcessorTest,
base::FeatureList feature_list; base::FeatureList feature_list;
study1->set_expiry_date(TimeToProtoTime(year_ago)); study1->set_expiry_date(TimeToProtoTime(year_ago));
seed_processor.CreateTrialsFromSeed(seed, client_state, base::MockEntropyProvider mock_low_entropy_provider(0.9);
override_callback_.callback(), nullptr, seed_processor.CreateTrialsFromSeed(
&feature_list); seed, client_state, override_callback_.callback(),
mock_low_entropy_provider, &feature_list);
EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName)); EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName));
} }
...@@ -304,9 +307,10 @@ TEST_F(VariationsSeedProcessorTest, ...@@ -304,9 +307,10 @@ TEST_F(VariationsSeedProcessorTest,
base::FeatureList feature_list; base::FeatureList feature_list;
study1->clear_expiry_date(); study1->clear_expiry_date();
study2->set_expiry_date(TimeToProtoTime(year_ago)); study2->set_expiry_date(TimeToProtoTime(year_ago));
seed_processor.CreateTrialsFromSeed(seed, client_state, base::MockEntropyProvider mock_low_entropy_provider(0.9);
override_callback_.callback(), nullptr, seed_processor.CreateTrialsFromSeed(
&feature_list); seed, client_state, override_callback_.callback(),
mock_low_entropy_provider, &feature_list);
EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName)); EXPECT_EQ(kGroup1Name, base::FieldTrialList::FindFullName(kTrialName));
} }
} }
...@@ -560,9 +564,10 @@ TEST_F(VariationsSeedProcessorTest, StartsActive) { ...@@ -560,9 +564,10 @@ TEST_F(VariationsSeedProcessorTest, StartsActive) {
client_state.platform = Study::PLATFORM_ANDROID; client_state.platform = Study::PLATFORM_ANDROID;
VariationsSeedProcessor seed_processor; VariationsSeedProcessor seed_processor;
seed_processor.CreateTrialsFromSeed(seed, client_state, base::MockEntropyProvider mock_low_entropy_provider(0.9);
override_callback_.callback(), nullptr, seed_processor.CreateTrialsFromSeed(
base::FeatureList::GetInstance()); seed, client_state, override_callback_.callback(),
mock_low_entropy_provider, base::FeatureList::GetInstance());
// Non-specified and ACTIVATE_ON_QUERY should not start active, but // Non-specified and ACTIVATE_ON_QUERY should not start active, but
// ACTIVATE_ON_STARTUP should. // ACTIVATE_ON_STARTUP should.
...@@ -979,9 +984,9 @@ TEST_F(VariationsSeedProcessorTest, LowEntropyStudyTest) { ...@@ -979,9 +984,9 @@ TEST_F(VariationsSeedProcessorTest, LowEntropyStudyTest) {
base::MockEntropyProvider mock_low_entropy_provider(0.9); base::MockEntropyProvider mock_low_entropy_provider(0.9);
EXPECT_TRUE(CreateTrialFromStudyWithEntropyOverride( EXPECT_TRUE(CreateTrialFromStudyWithEntropyOverride(
*study1, &mock_low_entropy_provider)); *study1, mock_low_entropy_provider));
EXPECT_TRUE(CreateTrialFromStudyWithEntropyOverride( EXPECT_TRUE(CreateTrialFromStudyWithEntropyOverride(
*study2, &mock_low_entropy_provider)); *study2, mock_low_entropy_provider));
// Since no experiment in study1 sends experiment IDs, it will use the high // Since no experiment in study1 sends experiment IDs, it will use the high
// entropy provider, which selects the non-default group. // entropy provider, which selects the non-default group.
......
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