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

Unified Diff: components/flags_ui/flags_state_unittest.cc

Issue 2036193002: Allow overriding variation parameter via chrome://flags. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Build deps #2 Created 4 years, 6 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
« no previous file with comments | « components/flags_ui/flags_state.cc ('k') | components/flags_ui/resources/flags.html » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/flags_ui/flags_state_unittest.cc
diff --git a/components/flags_ui/flags_state_unittest.cc b/components/flags_ui/flags_state_unittest.cc
index 628331b5a4f56a5c7e7731a59744ef294b705a5c..b95373a5f4ba8775df577b6b104c9dd971de43c7 100644
--- a/components/flags_ui/flags_state_unittest.cc
+++ b/components/flags_ui/flags_state_unittest.cc
@@ -15,6 +15,7 @@
#include "base/feature_list.h"
#include "base/format_macros.h"
#include "base/macros.h"
+#include "base/metrics/field_trial.h"
#include "base/stl_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
@@ -26,6 +27,7 @@
#include "components/flags_ui/pref_service_flags_storage.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
+#include "components/variations/variations_associated_data.h"
#include "grit/components_strings.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -40,6 +42,7 @@ const char kFlags4[] = "flag4";
const char kFlags5[] = "flag5";
const char kFlags6[] = "flag6";
const char kFlags7[] = "flag7";
+const char kFlags8[] = "flag8";
const char kSwitch1[] = "switch";
const char kSwitch2[] = "switch2";
@@ -57,6 +60,26 @@ const char kEnableDisableValue2[] = "value2";
const char kEnableFeatures[] = "dummy-enable-features";
const char kDisableFeatures[] = "dummy-disable-features";
+const char kTestTrial[] = "TestTrial";
+const char kTestParam[] = "param";
+const char kTestParamValue1[] = "value1";
+const char kTestParamValue2[] = "value2";
+
+const base::Feature kTestFeature1{"FeatureName1",
+ base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kTestFeature2{"FeatureName2",
+ base::FEATURE_ENABLED_BY_DEFAULT};
+
+const FeatureEntry::FeatureParam kTestVariationDefault[] = {
+ {kTestParam, kTestParamValue1}};
+
+const FeatureEntry::FeatureParam kTestVariationOther[] = {
+ {kTestParam, kTestParamValue2}};
+
+const FeatureEntry::FeatureVariation kTestVariations[] = {
+ {"", kTestVariationDefault, 1},
+ {"dummy description", kTestVariationOther, 1}};
+
// Those have to be valid ids for the translation system but the value are
// never used, so pick one at random from the current component.
const int kDummyNameId = IDS_FLAGS_UI_WARNING_HEADER;
@@ -74,44 +97,45 @@ const FeatureEntry::Choice kMultiChoices[] = {
{kDummyDescriptionId, kMultiSwitch2, kValueForMultiSwitch2},
};
-const base::Feature kTestFeature{"FeatureName",
- base::FEATURE_ENABLED_BY_DEFAULT};
-
// The entries that are set for these tests. The 3rd entry is not supported on
// the current platform, all others are.
static FeatureEntry kEntries[] = {
{kFlags1, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
- FeatureEntry::SINGLE_VALUE, kSwitch1, "", nullptr, nullptr, nullptr,
- nullptr, 0},
+ FeatureEntry::SINGLE_VALUE, kSwitch1, "", nullptr, nullptr, nullptr, 0,
+ nullptr, nullptr, nullptr},
{kFlags2, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
FeatureEntry::SINGLE_VALUE, kSwitch2, kValueForSwitch2, nullptr, nullptr,
- nullptr, nullptr, 0},
+ nullptr, 0, nullptr, nullptr, nullptr},
{kFlags3, kDummyNameId, kDummyDescriptionId,
0, // This ends up enabling for an OS other than the current.
- FeatureEntry::SINGLE_VALUE, kSwitch3, "", nullptr, nullptr, nullptr,
- nullptr, 0},
+ FeatureEntry::SINGLE_VALUE, kSwitch3, "", nullptr, nullptr, nullptr, 0,
+ nullptr, nullptr, nullptr},
{kFlags4, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
- FeatureEntry::MULTI_VALUE, "", "", "", "", nullptr, kMultiChoices,
- arraysize(kMultiChoices)},
+ FeatureEntry::MULTI_VALUE, "", "", "", "", nullptr,
+ arraysize(kMultiChoices), kMultiChoices, nullptr, nullptr},
{kFlags5, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
FeatureEntry::ENABLE_DISABLE_VALUE, kSwitch1, kEnableDisableValue1,
- kSwitch2, kEnableDisableValue2, nullptr, nullptr, 3},
+ kSwitch2, kEnableDisableValue2, nullptr, 3, nullptr, nullptr, nullptr},
{kFlags6, kDummyNameId, kDummyDescriptionId, 0,
FeatureEntry::SINGLE_DISABLE_VALUE, kSwitch6, "", nullptr, nullptr,
- nullptr, nullptr, 0},
+ nullptr, 0, nullptr, nullptr, nullptr},
{kFlags7, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
FeatureEntry::FEATURE_VALUE, nullptr, nullptr, nullptr, nullptr,
- &kTestFeature, nullptr, 3},
+ &kTestFeature1, 3, nullptr, nullptr, nullptr},
+ {kFlags8, kDummyNameId, kDummyDescriptionId,
+ 0, // Ends up being mapped to the current platform.
+ FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE, nullptr, nullptr, nullptr,
+ nullptr, &kTestFeature2, 4, nullptr, kTestVariations, kTestTrial},
};
class FlagsStateTest : public ::testing::Test {
protected:
- FlagsStateTest() : flags_storage_(&prefs_) {
+ FlagsStateTest() : flags_storage_(&prefs_), trial_list_(nullptr) {
prefs_.registry()->RegisterListPref(prefs::kEnabledLabsExperiments);
for (size_t i = 0; i < arraysize(kEntries); ++i)
@@ -127,6 +151,7 @@ class FlagsStateTest : public ::testing::Test {
TestingPrefServiceSimple prefs_;
PrefServiceFlagsStorage flags_storage_;
std::unique_ptr<FlagsState> flags_state_;
+ base::FieldTrialList trial_list_;
};
TEST_F(FlagsStateTest, NoChangeNoRestart) {
@@ -157,13 +182,13 @@ TEST_F(FlagsStateTest, MultiFlagChangeNeedsRestart) {
ASSERT_EQ(kFlags4, entry.internal_name);
EXPECT_FALSE(flags_state_->IsRestartNeededToCommitChanges());
// Enable the 2nd choice of the multi-value.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(2),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(2),
true);
EXPECT_TRUE(flags_state_->IsRestartNeededToCommitChanges());
flags_state_->Reset();
EXPECT_FALSE(flags_state_->IsRestartNeededToCommitChanges());
// Enable the default choice now.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(0),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(0),
true);
EXPECT_TRUE(flags_state_->IsRestartNeededToCommitChanges());
}
@@ -241,6 +266,42 @@ TEST_F(FlagsStateTest, ConvertFlagsToSwitches) {
EXPECT_FALSE(command_line2.HasSwitch(switches::kFlagSwitchesEnd));
}
+TEST_F(FlagsStateTest, RegisterAllFeatureVariationParameters) {
+ const FeatureEntry& entry = kEntries[7];
+ // Select the "Disabled" variation.
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(0),
+ true);
+ flags_state_->RegisterAllFeatureVariationParameters(&flags_storage_);
+ // No value should be associated.
+ EXPECT_EQ("", variations::GetVariationParamValue(kTestTrial, kTestParam));
+ // The trial should not be created.
+ base::FieldTrial* trial = base::FieldTrialList::Find(kTestTrial);
+ EXPECT_EQ(nullptr, trial);
+
+ // Select the first "Enabled" variation.
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(1),
+ true);
+
+ flags_state_->RegisterAllFeatureVariationParameters(&flags_storage_);
+ // The value should be associated.
+ EXPECT_EQ(kTestParamValue1,
+ variations::GetVariationParamValue(kTestTrial, kTestParam));
+
+ // The trial should be created.
+ trial = base::FieldTrialList::Find(kTestTrial);
+ EXPECT_NE(nullptr, trial);
+ // The about:flags group should be selected for the trial.
+ EXPECT_EQ(internal::kTrialGroupAboutFlags, trial->group_name());
+
+ // Select the second "Enabled" variation.
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(2),
+ true);
+ flags_state_->RegisterAllFeatureVariationParameters(&flags_storage_);
+ // Associating for the second time should not change the value.
+ EXPECT_EQ(kTestParamValue1,
+ variations::GetVariationParamValue(kTestTrial, kTestParam));
+}
+
base::CommandLine::StringType CreateSwitch(const std::string& value) {
#if defined(OS_WIN)
return base::ASCIIToUTF16(value);
@@ -349,11 +410,11 @@ TEST_F(FlagsStateTest, RemoveFlagSwitches_Features) {
{0, nullptr, nullptr, nullptr, nullptr},
{0, "A,B", "C", "A,B", "C"},
// "Enable" option: should only affect enabled list.
- {1, nullptr, nullptr, "FeatureName", nullptr},
- {1, "A,B", "C", "A,B,FeatureName", "C"},
+ {1, nullptr, nullptr, "FeatureName1", nullptr},
+ {1, "A,B", "C", "A,B,FeatureName1", "C"},
// "Disable" option: should only affect disabled list.
- {2, nullptr, nullptr, nullptr, "FeatureName"},
- {2, "A,B", "C", "A,B", "C,FeatureName"},
+ {2, nullptr, nullptr, nullptr, "FeatureName1"},
+ {2, "A,B", "C", "A,B", "C,FeatureName1"},
};
for (size_t i = 0; i < arraysize(cases); ++i) {
@@ -515,7 +576,7 @@ TEST_F(FlagsStateTest, MultiValues) {
}
// Enable the 2nd choice of the multi-value.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(2),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(2),
true);
{
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
@@ -529,7 +590,7 @@ TEST_F(FlagsStateTest, MultiValues) {
}
// Disable the multi-value entry.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(0),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(0),
true);
{
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
@@ -588,7 +649,7 @@ TEST_F(FlagsStateTest, EnableDisableValues) {
}
// "Enable" option selected.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(1),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(1),
true);
{
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
@@ -601,7 +662,7 @@ TEST_F(FlagsStateTest, EnableDisableValues) {
}
// "Disable" option selected.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(2),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(2),
true);
{
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
@@ -614,7 +675,7 @@ TEST_F(FlagsStateTest, EnableDisableValues) {
}
// "Default" option selected, same as nothing selected.
- flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForChoice(0),
+ flags_state_->SetFeatureEntryEnabled(&flags_storage_, entry.NameForOption(0),
true);
{
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
@@ -642,13 +703,13 @@ TEST_F(FlagsStateTest, FeatureValues) {
// "Default" option selected, same as nothing selected.
{0, nullptr, nullptr, "", ""},
// "Enable" option selected.
- {1, nullptr, nullptr, "FeatureName", ""},
+ {1, nullptr, nullptr, "FeatureName1", ""},
// "Disable" option selected.
- {2, nullptr, nullptr, "", "FeatureName"},
+ {2, nullptr, nullptr, "", "FeatureName1"},
// "Enable" option should get added to the existing list.
- {1, "Foo,Bar", nullptr, "Foo,Bar,FeatureName", ""},
+ {1, "Foo,Bar", nullptr, "Foo,Bar,FeatureName1", ""},
// "Disable" option should get added to the existing list.
- {2, nullptr, "Foo,Bar", "", "Foo,Bar,FeatureName"},
+ {2, nullptr, "Foo,Bar", "", "Foo,Bar,FeatureName1"},
};
for (size_t i = 0; i < arraysize(cases); ++i) {
@@ -661,7 +722,7 @@ TEST_F(FlagsStateTest, FeatureValues) {
if (cases[i].enabled_choice != -1) {
flags_state_->SetFeatureEntryEnabled(
- &flags_storage_, entry.NameForChoice(cases[i].enabled_choice), true);
+ &flags_storage_, entry.NameForOption(cases[i].enabled_choice), true);
}
base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
@@ -691,7 +752,7 @@ TEST_F(FlagsStateTest, GetFlagFeatureEntries) {
&supported_entries, &unsupported_entries,
base::Bind(&SkipFeatureEntry));
// All |kEntries| except for |kFlags3| should be supported.
- EXPECT_EQ(6u, supported_entries.GetSize());
+ EXPECT_EQ(7u, supported_entries.GetSize());
EXPECT_EQ(1u, unsupported_entries.GetSize());
EXPECT_EQ(arraysize(kEntries),
supported_entries.GetSize() + unsupported_entries.GetSize());
« no previous file with comments | « components/flags_ui/flags_state.cc ('k') | components/flags_ui/resources/flags.html » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698