Skip to content

Commit

Permalink
Make ScopedFeatureList Field Trial Name Unique.
Browse files Browse the repository at this point in the history
Make the name for the field trial created by ScopedFeatureList unique
so that different ScopedFeatureLists do not collide when the field
trial is stored globally.

Bug: none
Change-Id: Ibe0ebc6747868fd468e0c9455f21e990635aa1c4
Reviewed-on: https://chromium-review.googlesource.com/813102
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Gregory Chatzinoff <gchatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523360}
  • Loading branch information
Gregory Chatzinoff authored and Commit Bot committed Dec 12, 2017
1 parent 8193e87 commit cdfef07
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
12 changes: 11 additions & 1 deletion base/test/scoped_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/memory/ptr_util.h"
#include "base/metrics/field_trial_param_associator.h"
#include "base/stl_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"

Expand Down Expand Up @@ -191,8 +192,17 @@ void ScopedFeatureList::InitAndEnableFeatureWithParameters(
field_trial_list_ = std::make_unique<base::FieldTrialList>(nullptr);
}

std::string kTrialName = "scoped_feature_list_trial_name";
// TODO(crbug.com/794021) Remove this unique field trial name hack when there
// is a cleaner solution.
// Ensure that each call to this method uses a distinct field trial name.
// Otherwise, nested calls might fail due to the shared FieldTrialList
// already having the field trial registered.
static int num_calls = 0;
++num_calls;
std::string kTrialName =
"scoped_feature_list_trial_name" + base::NumberToString(num_calls);
std::string kTrialGroup = "scoped_feature_list_trial_group";

field_trial_override_ =
base::FieldTrialList::CreateFieldTrial(kTrialName, kTrialGroup);
DCHECK(field_trial_override_);
Expand Down
2 changes: 1 addition & 1 deletion base/test/scoped_feature_list_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ TEST_F(ScopedFeatureListTest, OverrideWithFeatureParameters) {
EXPECT_TRUE(FeatureList::IsEnabled(kTestFeature1));
EXPECT_TRUE(FeatureList::IsEnabled(kTestFeature2));
EXPECT_EQ("", GetFieldTrialParamValueByFeature(kTestFeature1, kParam));
EXPECT_EQ("", GetFieldTrialParamValueByFeature(kTestFeature2, kParam));
EXPECT_EQ(kValue, GetFieldTrialParamValueByFeature(kTestFeature2, kParam));
EXPECT_EQ(trial.get(), FeatureList::GetFieldTrial(kTestFeature1));
EXPECT_NE(nullptr, FeatureList::GetFieldTrial(kTestFeature2));
}
Expand Down

0 comments on commit cdfef07

Please sign in to comment.