Skip to content

Commit

Permalink
[chrome://flags] Rename FEATURE_WITH_VARIATIONS* to FEATURE_WITH_PARAMS*
Browse files Browse the repository at this point in the history
This CL aligns the naming for feature entries to the new naming for
field trials. It satisfies a request that has been raised in the review
of CL 2707013002.

BUG=none

Review-Url: https://codereview.chromium.org/2764973002
Cr-Commit-Position: refs/heads/master@{#458734}
  • Loading branch information
jkrcal authored and Commit bot committed Mar 22, 2017
1 parent 7706681 commit 531f367
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 43 deletions.
27 changes: 13 additions & 14 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1911,14 +1911,14 @@ const FeatureEntry kFeatureEntries[] = {
{"content-suggestions-category-order",
IDS_FLAGS_CONTENT_SUGGESTIONS_CATEGORY_ORDER_NAME,
IDS_FLAGS_CONTENT_SUGGESTIONS_CATEGORY_ORDER_DESCRIPTION, kOsAndroid,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
ntp_snippets::kCategoryOrder,
kContentSuggestionsCategoryOrderFeatureVariations,
ntp_snippets::kStudyName)},
{"content-suggestions-category-ranker",
IDS_FLAGS_CONTENT_SUGGESTIONS_CATEGORY_RANKER_NAME,
IDS_FLAGS_CONTENT_SUGGESTIONS_CATEGORY_RANKER_DESCRIPTION, kOsAndroid,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
ntp_snippets::kCategoryRanker,
kContentSuggestionsCategoryRankerFeatureVariations,
ntp_snippets::kStudyName)},
Expand All @@ -1935,10 +1935,9 @@ const FeatureEntry kFeatureEntries[] = {
{"enable-ntp-remote-suggestions",
IDS_FLAGS_ENABLE_NTP_REMOTE_SUGGESTIONS_NAME,
IDS_FLAGS_ENABLE_NTP_REMOTE_SUGGESTIONS_DESCRIPTION, kOsAndroid,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
ntp_snippets::kArticleSuggestionsFeature,
kRemoteSuggestionsFeatureVariations,
ntp_snippets::kStudyName)},
FEATURE_WITH_PARAMS_VALUE_TYPE(ntp_snippets::kArticleSuggestionsFeature,
kRemoteSuggestionsFeatureVariations,
ntp_snippets::kStudyName)},
{"enable-ntp-recent-offline-tab-suggestions",
IDS_FLAGS_ENABLE_NTP_RECENT_OFFLINE_TAB_SUGGESTIONS_NAME,
IDS_FLAGS_ENABLE_NTP_RECENT_OFFLINE_TAB_SUGGESTIONS_DESCRIPTION,
Expand Down Expand Up @@ -1968,7 +1967,7 @@ const FeatureEntry kFeatureEntries[] = {
{"enable-ntp-suggestions-notifications",
IDS_FLAGS_ENABLE_NTP_SUGGESTIONS_NOTIFICATIONS_NAME,
IDS_FLAGS_ENABLE_NTP_SUGGESTIONS_NOTIFICATIONS_DESCRIPTION, kOsAndroid,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
params::ntp_snippets::kNotificationsFeature,
kContentSuggestionsNotificationsFeatureVariations,
ntp_snippets::kStudyName)},
Expand All @@ -1977,7 +1976,7 @@ const FeatureEntry kFeatureEntries[] = {
FEATURE_VALUE_TYPE(chrome::android::kNTPCondensedLayoutFeature)},
{"ntp-condensed-tile-layout", IDS_FLAGS_NTP_CONDENSED_TILE_LAYOUT_NAME,
IDS_FLAGS_NTP_CONDENSED_TILE_LAYOUT_DESCRIPTION, kOsAndroid,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
chrome::android::kNTPCondensedTileLayoutFeature,
kNTPCondensedTileLayoutFeatureVariations,
ntp_snippets::kStudyName)},
Expand Down Expand Up @@ -2238,9 +2237,9 @@ const FeatureEntry kFeatureEntries[] = {
#endif
{"enable-nostate-prefetch", IDS_FLAGS_NOSTATE_PREFETCH,
IDS_FLAGS_NOSTATE_PREFETCH_DESCRIPTION, kOsAll,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(prerender::kNoStatePrefetchFeature,
kNoStatePrefetchFeatureVariations,
"NoStatePrefetchValidation")},
FEATURE_WITH_PARAMS_VALUE_TYPE(prerender::kNoStatePrefetchFeature,
kNoStatePrefetchFeatureVariations,
"NoStatePrefetchValidation")},
#if defined(OS_CHROMEOS)
{"cros-comp-updates", IDS_FLAGS_CROS_COMP_UPDATES_NAME,
IDS_FLAGS_CROS_COMP_UPDATES_DESCRIPTION, kOsCrOS,
Expand All @@ -2262,7 +2261,7 @@ const FeatureEntry kFeatureEntries[] = {
IDS_FLAGS_ENABLE_EXPANDED_AUTOFILL_CREDIT_CARD_POPUP_LAYOUT,
IDS_FLAGS_ENABLE_EXPANDED_AUTOFILL_CREDIT_CARD_POPUP_LAYOUT_DESCRIPTION,
kOsAndroid,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
autofill::kAutofillCreditCardPopupLayout,
kAutofillCreditCardPopupLayoutFeatureVariations,
"AutofillCreditCardPopupLayout")},
Expand All @@ -2274,7 +2273,7 @@ const FeatureEntry kFeatureEntries[] = {
IDS_FLAGS_ENABLE_AUTOFILL_CREDIT_CARD_LAST_USED_DATE_DISPLAY,
IDS_FLAGS_ENABLE_AUTOFILL_CREDIT_CARD_LAST_USED_DATE_DISPLAY_DESCRIPTION,
kOsAll,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
autofill::kAutofillCreditCardLastUsedDateDisplay,
kAutofillCreditCardLastUsedDateFeatureVariations,
"AutofillCreditCardLastUsedDate")},
Expand Down Expand Up @@ -2374,7 +2373,7 @@ const FeatureEntry kFeatureEntries[] = {

{"enable-resource-prefetch", IDS_FLAGS_SPECULATIVE_PREFETCH_NAME,
IDS_FLAGS_SPECULATIVE_PREFETCH_DESCRIPTION, kOsAll,
FEATURE_WITH_VARIATIONS_VALUE_TYPE(
FEATURE_WITH_PARAMS_VALUE_TYPE(
predictors::kSpeculativeResourcePrefetchingFeature,
kSpeculativeResourcePrefetchingFeatureVariations,
"SpeculativeResourcePrefetchingValidation")},
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/about_flags_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ std::set<std::string> GetAllSwitchesAndFeaturesForTesting() {
result.insert(entry.disable_command_line_switch);
break;
case flags_ui::FeatureEntry::FEATURE_VALUE:
case flags_ui::FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
case flags_ui::FeatureEntry::FEATURE_WITH_PARAMS_VALUE:
result.insert(std::string(entry.feature->name) + ":enabled");
result.insert(std::string(entry.feature->name) + ":disabled");
break;
Expand Down
12 changes: 6 additions & 6 deletions components/flags_ui/feature_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ std::string FeatureEntry::NameForOption(int index) const {
DCHECK(type == FeatureEntry::MULTI_VALUE ||
type == FeatureEntry::ENABLE_DISABLE_VALUE ||
type == FeatureEntry::FEATURE_VALUE ||
type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE);
DCHECK_LT(index, num_options);
return std::string(internal_name) + testing::kMultiSeparator +
base::IntToString(index);
Expand All @@ -26,7 +26,7 @@ base::string16 FeatureEntry::DescriptionForOption(int index) const {
DCHECK(type == FeatureEntry::MULTI_VALUE ||
type == FeatureEntry::ENABLE_DISABLE_VALUE ||
type == FeatureEntry::FEATURE_VALUE ||
type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE);
DCHECK_LT(index, num_options);
int description_id;
if (type == FeatureEntry::ENABLE_DISABLE_VALUE ||
Expand All @@ -37,7 +37,7 @@ base::string16 FeatureEntry::DescriptionForOption(int index) const {
IDS_GENERIC_EXPERIMENT_CHOICE_DISABLED,
};
description_id = kEnableDisableDescriptionIds[index];
} else if (type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) {
} else if (type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE) {
if (index == 0) {
description_id = IDS_GENERIC_EXPERIMENT_CHOICE_DEFAULT;
} else if (index == 1) {
Expand Down Expand Up @@ -68,7 +68,7 @@ const FeatureEntry::Choice& FeatureEntry::ChoiceForOption(int index) const {

FeatureEntry::FeatureState FeatureEntry::StateForOption(int index) const {
DCHECK(type == FeatureEntry::FEATURE_VALUE ||
type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE);
DCHECK_LT(index, num_options);

if (index == 0)
Expand All @@ -82,10 +82,10 @@ FeatureEntry::FeatureState FeatureEntry::StateForOption(int index) const {
const FeatureEntry::FeatureVariation* FeatureEntry::VariationForOption(
int index) const {
DCHECK(type == FeatureEntry::FEATURE_VALUE ||
type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE);
DCHECK_LT(index, num_options);

if (type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE && index > 1 &&
if (type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE && index > 1 &&
index < num_options - 1) {
// We have no variations for FEATURE_VALUE type. Option at |index|
// corresponds to variation at |index| - 2 as the list starts with "Default"
Expand Down
16 changes: 8 additions & 8 deletions components/flags_ui/feature_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ struct FeatureEntry {
FEATURE_VALUE,

// Corresponds to a base::Feature and additional options [O_1, ..., O_n]
// that specify variation parameters. Each of the options can specify a set
// of variation parameters. The entry will have n+3 states: Default,
// Enabled, Enabled V_1, ..., Enabled: V_n, Disabled. When set to Default,
// the normal default values of the feature and of the parameters are used
// (possibly passed from the server in a trial config). When set to Enabled,
// the feature is overriden to be enabled and empty set of parameters is
// used boiling down to the default behavior in the code.
FEATURE_WITH_VARIATIONS_VALUE,
// that specify field trial params. Each of the options can specify a set
// of field trial params. The entry will have n+3 states: Default, Enabled,
// Enabled V_1, ..., Enabled: V_n, Disabled. When set to Default, the normal
// default values of the feature and of the parameters are used (possibly
// passed from the server in a trial config). When set to Enabled, the
// feature is overriden to be enabled and empty set of parameters is used
// boiling down to the default behavior in the code.
FEATURE_WITH_PARAMS_VALUE,
};

// Describes state of a feature.
Expand Down
7 changes: 3 additions & 4 deletions components/flags_ui/feature_entry_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@
#define FEATURE_VALUE_TYPE(feature) \
flags_ui::FeatureEntry::FEATURE_VALUE, nullptr, nullptr, nullptr, nullptr, \
&feature, 3, nullptr, nullptr, nullptr
// TODO(jkrcal): Rename FEATURE_WITH_VARIATIONS* to FEATURE_WITH_PARAMS*.
#define FEATURE_WITH_VARIATIONS_VALUE_TYPE(feature, feature_variations, \
feature_trial) \
flags_ui::FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE, nullptr, nullptr, \
#define FEATURE_WITH_PARAMS_VALUE_TYPE(feature, feature_variations, \
feature_trial) \
flags_ui::FeatureEntry::FEATURE_WITH_PARAMS_VALUE, nullptr, nullptr, \
nullptr, nullptr, &feature, 3 + arraysize(feature_variations), nullptr, \
feature_variations, feature_trial

Expand Down
14 changes: 7 additions & 7 deletions components/flags_ui/flags_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void AddInternalName(const FeatureEntry& e, std::set<std::string>* names) {
case FeatureEntry::MULTI_VALUE:
case FeatureEntry::ENABLE_DISABLE_VALUE:
case FeatureEntry::FEATURE_VALUE:
case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
case FeatureEntry::FEATURE_WITH_PARAMS_VALUE:
for (int i = 0; i < e.num_options; ++i)
names->insert(e.NameForOption(i));
break;
Expand Down Expand Up @@ -140,7 +140,7 @@ bool ValidateFeatureEntry(const FeatureEntry& e) {
DCHECK(!e.choices);
DCHECK(e.feature);
return true;
case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
case FeatureEntry::FEATURE_WITH_PARAMS_VALUE:
DCHECK_GT(e.num_options, 2);
DCHECK(!e.choices);
DCHECK(e.feature);
Expand All @@ -162,7 +162,7 @@ bool IsDefaultValue(const FeatureEntry& entry,
case FeatureEntry::MULTI_VALUE:
case FeatureEntry::ENABLE_DISABLE_VALUE:
case FeatureEntry::FEATURE_VALUE:
case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
case FeatureEntry::FEATURE_WITH_PARAMS_VALUE:
for (int i = 0; i < entry.num_options; ++i) {
if (enabled_entries.count(entry.NameForOption(i)) > 0)
return false;
Expand All @@ -179,7 +179,7 @@ base::Value* CreateOptionsData(const FeatureEntry& entry,
DCHECK(entry.type == FeatureEntry::MULTI_VALUE ||
entry.type == FeatureEntry::ENABLE_DISABLE_VALUE ||
entry.type == FeatureEntry::FEATURE_VALUE ||
entry.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
entry.type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE);
base::ListValue* result = new base::ListValue;
for (int i = 0; i < entry.num_options; ++i) {
std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue);
Expand Down Expand Up @@ -440,7 +440,7 @@ std::vector<std::string> FlagsState::RegisterAllFeatureVariationParameters(
// First collect all the data for each trial.
for (size_t i = 0; i < num_feature_entries_; ++i) {
const FeatureEntry& e = feature_entries_[i];
if (e.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) {
if (e.type == FeatureEntry::FEATURE_WITH_PARAMS_VALUE) {
for (int j = 0; j < e.num_options; ++j) {
if (e.StateForOption(j) == FeatureEntry::FeatureState::ENABLED &&
enabled_entries.count(e.NameForOption(j))) {
Expand Down Expand Up @@ -533,7 +533,7 @@ void FlagsState::GetFlagFeatureEntries(
case FeatureEntry::MULTI_VALUE:
case FeatureEntry::ENABLE_DISABLE_VALUE:
case FeatureEntry::FEATURE_VALUE:
case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
case FeatureEntry::FEATURE_WITH_PARAMS_VALUE:
data->Set("options", CreateOptionsData(entry, enabled_entries));
break;
}
Expand Down Expand Up @@ -783,7 +783,7 @@ void FlagsState::GenerateFlagsToSwitchesMapping(
e.disable_command_line_value, name_to_switch_map);
break;
case FeatureEntry::FEATURE_VALUE:
case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
case FeatureEntry::FEATURE_WITH_PARAMS_VALUE:
for (int j = 0; j < e.num_options; ++j) {
FeatureEntry::FeatureState state = e.StateForOption(j);
if (state == FeatureEntry::FeatureState::DEFAULT) {
Expand Down
6 changes: 3 additions & 3 deletions components/flags_ui/flags_state_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,15 @@ static FeatureEntry kEntries[] = {
&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,
FeatureEntry::FEATURE_WITH_PARAMS_VALUE, nullptr, nullptr, nullptr,
nullptr, &kTestFeature1, 4, nullptr, kTestVariations1, kTestTrial},
{kFlags9, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE, nullptr, nullptr, nullptr,
FeatureEntry::FEATURE_WITH_PARAMS_VALUE, nullptr, nullptr, nullptr,
nullptr, &kTestFeature1, 4, nullptr, kTestVariations1, kTestTrial},
{kFlags10, kDummyNameId, kDummyDescriptionId,
0, // Ends up being mapped to the current platform.
FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE, nullptr, nullptr, nullptr,
FeatureEntry::FEATURE_WITH_PARAMS_VALUE, nullptr, nullptr, nullptr,
nullptr, &kTestFeature2, 4, nullptr, kTestVariations2, kTestTrial},
};

Expand Down

0 comments on commit 531f367

Please sign in to comment.