Commit 2955e423 authored by asvitkine's avatar asvitkine Committed by Commit bot

Fix crash with forcing variations when the trial exists.

This can happen, for example, when a user used the
--force-fieldtrials= command-line to set a different
group than what they have selected from chrome://flags.

Includes a test.

BUG=409976

Review URL: https://codereview.chromium.org/529183002

Cr-Commit-Position: refs/heads/master@{#293173}
parent ca100d59
...@@ -111,6 +111,13 @@ void VariationsSeedProcessor::CreateTrialFromStudy( ...@@ -111,6 +111,13 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
scoped_refptr<base::FieldTrial> trial( scoped_refptr<base::FieldTrial> trial(
base::FieldTrialList::CreateFieldTrial(study.name(), base::FieldTrialList::CreateFieldTrial(study.name(),
experiment.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)
break;
RegisterExperimentParams(study, experiment); RegisterExperimentParams(study, experiment);
RegisterVariationIds(experiment, study.name()); RegisterVariationIds(experiment, study.name());
if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) { if (study.activation_type() == Study_ActivationType_ACTIVATION_AUTO) {
......
...@@ -463,4 +463,28 @@ TEST_F(VariationsSeedProcessorTest, StartsActiveWithFlag) { ...@@ -463,4 +463,28 @@ TEST_F(VariationsSeedProcessorTest, StartsActiveWithFlag) {
base::FieldTrialList::FindFullName(kFlagStudyName)); base::FieldTrialList::FindFullName(kFlagStudyName));
} }
TEST_F(VariationsSeedProcessorTest, ForcingFlagAlreadyForced) {
Study study = CreateStudyWithFlagGroups(100, 0, 0);
ASSERT_EQ(kNonFlagGroupName, study.experiment(0).name());
Study_Experiment_Param* param = study.mutable_experiment(0)->add_param();
param->set_name("x");
param->set_value("y");
study.mutable_experiment(0)->set_google_web_experiment_id(kExperimentId);
base::FieldTrialList field_trial_list(NULL);
base::FieldTrialList::CreateFieldTrial(kFlagStudyName, kNonFlagGroupName);
CommandLine::ForCurrentProcess()->AppendSwitch(kForcingFlag1);
EXPECT_TRUE(CreateTrialFromStudy(&study));
// The previously forced experiment should still hold.
EXPECT_EQ(kNonFlagGroupName,
base::FieldTrialList::FindFullName(study.name()));
// Check that params and experiment ids correspond.
EXPECT_EQ("y", GetVariationParamValue(study.name(), "x"));
VariationID id = GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, kFlagStudyName,
kNonFlagGroupName);
EXPECT_EQ(kExperimentId, id);
}
} // namespace variations } // namespace variations
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