Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(68)

Unified Diff: components/variations/variations_seed_processor.cc

Issue 1306653004: Expand FeatureList to support FieldTrial association. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase. Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/variations/variations_seed_processor.cc
diff --git a/components/variations/variations_seed_processor.cc b/components/variations/variations_seed_processor.cc
index 169ac63104b2e983b778fe00b9de0a3fa26630fb..8acfa88aa8f84e7aa5f3aab0ef46ace8e8572441 100644
--- a/components/variations/variations_seed_processor.cc
+++ b/components/variations/variations_seed_processor.cc
@@ -8,6 +8,7 @@
#include <vector>
#include "base/command_line.h"
+#include "base/feature_list.h"
#include "base/metrics/field_trial.h"
#include "base/strings/utf_string_conversions.h"
#include "components/variations/processed_study.h"
@@ -79,6 +80,72 @@ void ApplyUIStringOverrides(
}
}
+// Forces the specified |experiment| to be enabled in |study|.
+void ForceExperimentState(
+ const Study& study,
+ const Study_Experiment& experiment,
+ const VariationsSeedProcessor::UIStringOverrideCallback& override_callback,
+ base::FieldTrial* trial) {
+ RegisterExperimentParams(study, experiment);
+ RegisterVariationIds(experiment, study.name());
+ if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) {
+ trial->group();
+ // UI Strings can only be overridden from ACTIVATION_AUTO experiments.
+ ApplyUIStringOverrides(experiment, override_callback);
+ }
+}
+
+// Registers feature overrides for the chosen experiment in the specified study.
+void RegisterFeatureOverrides(const ProcessedStudy& processed_study,
+ base::FieldTrial* trial,
+ base::FeatureList* feature_list) {
+ const std::string& group_name = trial->GetGroupNameWithoutActivation();
+ int experiment_index = processed_study.GetExperimentIndexByName(group_name);
+ // The field trial was defined from |study|, so the active experiment's name
+ // must be in the |study|.
+ DCHECK_NE(-1, experiment_index);
+
+ const Study_Experiment& experiment =
+ processed_study.study()->experiment(experiment_index);
+
+ // Process all the features to enable.
+ int feature_count = experiment.feature_association().enable_feature_size();
+ for (int i = 0; i < feature_count; ++i) {
+ feature_list->RegisterFieldTrialOverride(
+ experiment.feature_association().enable_feature(i),
+ base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
+ }
+
+ // Process all the features to disable.
+ feature_count = experiment.feature_association().disable_feature_size();
+ for (int i = 0; i < feature_count; ++i) {
+ feature_list->RegisterFieldTrialOverride(
+ experiment.feature_association().disable_feature(i),
+ base::FeatureList::OVERRIDE_DISABLE_FEATURE, trial);
+ }
+}
+
+// Checks if |experiment| is associated with a forcing flag or feature and if it
+// is, returns whether it should be forced enabled based on the |command_line|
+// or |feature_list| state.
+bool ShouldForceExperiment(const Study_Experiment& experiment,
+ const base::CommandLine& command_line,
+ const base::FeatureList& feature_list) {
+ if (experiment.feature_association().has_forcing_feature_on()) {
+ return feature_list.IsFeatureOverriddenFromCommandLine(
+ experiment.feature_association().forcing_feature_on(),
+ base::FeatureList::OVERRIDE_ENABLE_FEATURE);
+ }
+ if (experiment.feature_association().has_forcing_feature_off()) {
+ return feature_list.IsFeatureOverriddenFromCommandLine(
+ experiment.feature_association().forcing_feature_off(),
+ base::FeatureList::OVERRIDE_DISABLE_FEATURE);
+ }
+ if (experiment.has_forcing_flag())
+ return command_line.HasSwitch(experiment.forcing_flag());
+ return false;
+}
+
} // namespace
VariationsSeedProcessor::VariationsSeedProcessor() {
@@ -97,7 +164,8 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
const std::string& hardware_class,
const std::string& session_consistency_country,
const std::string& permanent_consistency_country,
- const UIStringOverrideCallback& override_callback) {
+ const UIStringOverrideCallback& override_callback,
+ base::FeatureList* feature_list) {
std::vector<ProcessedStudy> filtered_studies;
FilterAndValidateStudies(seed, locale, reference_date, version, channel,
form_factor, hardware_class,
@@ -105,12 +173,13 @@ void VariationsSeedProcessor::CreateTrialsFromSeed(
permanent_consistency_country, &filtered_studies);
for (size_t i = 0; i < filtered_studies.size(); ++i)
- CreateTrialFromStudy(filtered_studies[i], override_callback);
+ CreateTrialFromStudy(filtered_studies[i], override_callback, feature_list);
}
void VariationsSeedProcessor::CreateTrialFromStudy(
const ProcessedStudy& processed_study,
- const UIStringOverrideCallback& override_callback) {
+ const UIStringOverrideCallback& override_callback,
+ base::FeatureList* feature_list) {
const Study& study = *processed_study.study();
// Check if any experiments need to be forced due to a command line
@@ -118,28 +187,26 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
for (int i = 0; i < study.experiment_size(); ++i) {
const Study_Experiment& experiment = study.experiment(i);
- if (experiment.has_forcing_flag() &&
- command_line->HasSwitch(experiment.forcing_flag())) {
- scoped_refptr<base::FieldTrial> trial(
- base::FieldTrialList::CreateFieldTrial(study.name(),
- experiment.name()));
- // If |trial| is NULL, then there might already be a trial forced to a
+ if (ShouldForceExperiment(experiment, *command_line, *feature_list)) {
+ base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
+ study.name(), experiment.name());
+ // If |trial| is null, then there might already be a trial forced to a
// different group (e.g. via --force-fieldtrials). Break out of the loop,
// but don't return, so that variation ids and params for the selected
// group will still be picked up.
- if (!trial.get())
+ if (!trial)
break;
- RegisterExperimentParams(study, experiment);
- RegisterVariationIds(experiment, study.name());
- if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) {
- trial->group();
- // UI Strings can only be overridden from ACTIVATION_AUTO experiments.
- ApplyUIStringOverrides(experiment, override_callback);
+ if (experiment.feature_association().has_forcing_feature_on()) {
+ feature_list->AssociateReportingFieldTrial(
+ experiment.feature_association().forcing_feature_on(),
+ base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial);
+ } else if (experiment.feature_association().has_forcing_feature_off()) {
+ feature_list->AssociateReportingFieldTrial(
+ experiment.feature_association().forcing_feature_off(),
+ base::FeatureList::OVERRIDE_DISABLE_FEATURE, trial);
}
-
- DVLOG(1) << "Trial " << study.name() << " forced by flag: "
- << experiment.forcing_flag();
+ ForceExperimentState(study, experiment, override_callback, trial);
return;
}
}
@@ -169,14 +236,18 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
randomization_seed, NULL));
bool has_overrides = false;
+ bool enables_or_disables_features = false;
for (int i = 0; i < study.experiment_size(); ++i) {
const Study_Experiment& experiment = study.experiment(i);
RegisterExperimentParams(study, experiment);
// Groups with forcing flags have probability 0 and will never be selected.
// Therefore, there's no need to add them to the field trial.
- if (experiment.has_forcing_flag())
+ if (experiment.has_forcing_flag() ||
+ experiment.feature_association().has_forcing_feature_on() ||
+ experiment.feature_association().has_forcing_feature_off()) {
continue;
+ }
if (experiment.name() != study.default_experiment_name())
trial->AppendGroup(experiment.name(), experiment.probability_weight());
@@ -184,9 +255,17 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
RegisterVariationIds(experiment, study.name());
has_overrides = has_overrides || experiment.override_ui_string_size() > 0;
+ if (experiment.feature_association().enable_feature_size() != 0 ||
+ experiment.feature_association().disable_feature_size() != 0) {
+ enables_or_disables_features = true;
+ }
}
trial->SetForced();
+
+ if (enables_or_disables_features)
+ RegisterFeatureOverrides(processed_study, trial.get(), feature_list);
+
if (processed_study.is_expired()) {
trial->Disable();
} else if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) {
« no previous file with comments | « components/variations/variations_seed_processor.h ('k') | components/variations/variations_seed_processor_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698