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

Unified Diff: components/flags_ui/flags_state.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.h ('k') | components/flags_ui/flags_state_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/flags_ui/flags_state.cc
diff --git a/components/flags_ui/flags_state.cc b/components/flags_ui/flags_state.cc
index 8206ef077ad18a3c6878f2d7261f0c050c9cccd9..cadb8ee140137388db6ffb2bd33d001dad07430c 100644
--- a/components/flags_ui/flags_state.cc
+++ b/components/flags_ui/flags_state.cc
@@ -11,6 +11,7 @@
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/macros.h"
+#include "base/metrics/field_trial.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
@@ -19,10 +20,15 @@
#include "components/flags_ui/feature_entry.h"
#include "components/flags_ui/flags_storage.h"
#include "components/flags_ui/flags_ui_switches.h"
+#include "components/variations/variations_associated_data.h"
#include "ui/base/l10n/l10n_util.h"
namespace flags_ui {
+namespace internal {
+const char kTrialGroupAboutFlags[] = "AboutFlags";
+} // namespace internal
+
namespace {
// Convert switch constants to proper CommandLine::StringType strings.
@@ -99,8 +105,9 @@ void AddInternalName(const FeatureEntry& e, std::set<std::string>* names) {
case FeatureEntry::MULTI_VALUE:
case FeatureEntry::ENABLE_DISABLE_VALUE:
case FeatureEntry::FEATURE_VALUE:
- for (int i = 0; i < e.num_choices; ++i)
- names->insert(e.NameForChoice(i));
+ case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
+ for (int i = 0; i < e.num_options; ++i)
+ names->insert(e.NameForOption(i));
break;
}
}
@@ -111,17 +118,17 @@ bool ValidateFeatureEntry(const FeatureEntry& e) {
switch (e.type) {
case FeatureEntry::SINGLE_VALUE:
case FeatureEntry::SINGLE_DISABLE_VALUE:
- DCHECK_EQ(0, e.num_choices);
+ DCHECK_EQ(0, e.num_options);
DCHECK(!e.choices);
return true;
case FeatureEntry::MULTI_VALUE:
- DCHECK_GT(e.num_choices, 0);
+ DCHECK_GT(e.num_options, 0);
DCHECK(e.choices);
- DCHECK(e.choices[0].command_line_switch);
- DCHECK_EQ('\0', e.choices[0].command_line_switch[0]);
+ DCHECK(e.ChoiceForOption(0).command_line_switch);
+ DCHECK_EQ('\0', e.ChoiceForOption(0).command_line_switch[0]);
return true;
case FeatureEntry::ENABLE_DISABLE_VALUE:
- DCHECK_EQ(3, e.num_choices);
+ DCHECK_EQ(3, e.num_options);
DCHECK(!e.choices);
DCHECK(e.command_line_switch);
DCHECK(e.command_line_value);
@@ -129,9 +136,16 @@ bool ValidateFeatureEntry(const FeatureEntry& e) {
DCHECK(e.disable_command_line_value);
return true;
case FeatureEntry::FEATURE_VALUE:
- DCHECK_EQ(3, e.num_choices);
+ DCHECK_EQ(3, e.num_options);
+ DCHECK(!e.choices);
+ DCHECK(e.feature);
+ return true;
+ case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
+ DCHECK_GT(e.num_options, 2);
DCHECK(!e.choices);
DCHECK(e.feature);
+ DCHECK(e.feature_variations);
+ DCHECK(e.feature_trial_name);
return true;
}
NOTREACHED();
@@ -148,8 +162,9 @@ bool IsDefaultValue(const FeatureEntry& entry,
case FeatureEntry::MULTI_VALUE:
case FeatureEntry::ENABLE_DISABLE_VALUE:
case FeatureEntry::FEATURE_VALUE:
- for (int i = 0; i < entry.num_choices; ++i) {
- if (enabled_entries.count(entry.NameForChoice(i)) > 0)
+ case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
+ for (int i = 0; i < entry.num_options; ++i) {
+ if (enabled_entries.count(entry.NameForOption(i)) > 0)
return false;
}
return true;
@@ -159,23 +174,54 @@ bool IsDefaultValue(const FeatureEntry& entry,
}
// Returns the Value representing the choice data in the specified entry.
-base::Value* CreateChoiceData(const FeatureEntry& entry,
- const std::set<std::string>& enabled_entries) {
+base::Value* CreateOptionsData(const FeatureEntry& entry,
+ const std::set<std::string>& enabled_entries) {
DCHECK(entry.type == FeatureEntry::MULTI_VALUE ||
entry.type == FeatureEntry::ENABLE_DISABLE_VALUE ||
- entry.type == FeatureEntry::FEATURE_VALUE);
+ entry.type == FeatureEntry::FEATURE_VALUE ||
+ entry.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
base::ListValue* result = new base::ListValue;
- for (int i = 0; i < entry.num_choices; ++i) {
+ for (int i = 0; i < entry.num_options; ++i) {
std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue);
- const std::string name = entry.NameForChoice(i);
+ const std::string name = entry.NameForOption(i);
value->SetString("internal_name", name);
- value->SetString("description", entry.DescriptionForChoice(i));
+ value->SetString("description", entry.DescriptionForOption(i));
value->SetBoolean("selected", enabled_entries.count(name) > 0);
result->Append(std::move(value));
}
return result;
}
+// Registers variation parameters specified by |feature_variation| for the field
+// trial named |feature_trial_name|, unless a group for this trial has already
+// been created (e.g. via command-line switches that take precedence over
+// about:flags). In the trial, the function creates a new constant group called
+// |kTrialGroupAboutFlags|.
+void RegisterFeatureVariationParameters(
+ const std::string& feature_trial_name,
+ const FeatureEntry::FeatureVariation& feature_variation) {
+ std::map<std::string, std::string> params;
+ for (int i = 0; i < feature_variation.num_params; ++i) {
+ params[feature_variation.params[i].param_name] =
+ feature_variation.params[i].param_value;
+ }
+
+ bool success = variations::AssociateVariationParams(
+ feature_trial_name, internal::kTrialGroupAboutFlags, params);
+ if (success) {
+ // Successful association also means that no group is created and selected
+ // for the trial, yet. Thus, create the trial to select the group. This way,
+ // the parameters cannot get overwritten in later phases (such as from the
+ // server).
+ base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
+ feature_trial_name, internal::kTrialGroupAboutFlags);
+ if (!trial) {
+ DLOG(WARNING) << "Could not create the trial " << feature_trial_name
+ << " with group " << internal::kTrialGroupAboutFlags;
+ }
+ }
+}
+
} // namespace
// Keeps track of affected switches for each FeatureEntry, based on which
@@ -224,27 +270,33 @@ void FlagsState::ConvertFlagsToSwitches(
e.command_line_value, &name_to_switch_map);
break;
case FeatureEntry::MULTI_VALUE:
- for (int j = 0; j < e.num_choices; ++j) {
- AddSwitchMapping(e.NameForChoice(j), e.choices[j].command_line_switch,
- e.choices[j].command_line_value,
- &name_to_switch_map);
+ for (int j = 0; j < e.num_options; ++j) {
+ AddSwitchMapping(
+ e.NameForOption(j), e.ChoiceForOption(j).command_line_switch,
+ e.ChoiceForOption(j).command_line_value, &name_to_switch_map);
}
break;
case FeatureEntry::ENABLE_DISABLE_VALUE:
- AddSwitchMapping(e.NameForChoice(0), std::string(), std::string(),
+ AddSwitchMapping(e.NameForOption(0), std::string(), std::string(),
&name_to_switch_map);
- AddSwitchMapping(e.NameForChoice(1), e.command_line_switch,
+ AddSwitchMapping(e.NameForOption(1), e.command_line_switch,
e.command_line_value, &name_to_switch_map);
- AddSwitchMapping(e.NameForChoice(2), e.disable_command_line_switch,
+ AddSwitchMapping(e.NameForOption(2), e.disable_command_line_switch,
e.disable_command_line_value, &name_to_switch_map);
break;
case FeatureEntry::FEATURE_VALUE:
- AddFeatureMapping(e.NameForChoice(0), std::string(), false,
- &name_to_switch_map);
- AddFeatureMapping(e.NameForChoice(1), e.feature->name, true,
- &name_to_switch_map);
- AddFeatureMapping(e.NameForChoice(2), e.feature->name, false,
- &name_to_switch_map);
+ case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
+ for (int j = 0; j < e.num_options; ++j) {
+ FeatureEntry::FeatureState state = e.StateForOption(j);
+ if (state == FeatureEntry::FeatureState::DEFAULT) {
+ AddFeatureMapping(e.NameForOption(j), std::string(), false,
+ &name_to_switch_map);
+ } else {
+ AddFeatureMapping(e.NameForOption(j), e.feature->name,
+ state == FeatureEntry::FeatureState::ENABLED,
+ &name_to_switch_map);
+ }
+ }
break;
}
}
@@ -305,11 +357,11 @@ void FlagsState::SetFeatureEntryEnabled(FlagsStorage* flags_storage,
} else {
if (enable) {
// Enable the first choice.
- needs_restart_ |= enabled_entries.insert(e->NameForChoice(0)).second;
+ needs_restart_ |= enabled_entries.insert(e->NameForOption(0)).second;
} else {
// Find the currently enabled choice and disable it.
- for (int i = 0; i < e->num_choices; ++i) {
- std::string choice_name = e->NameForChoice(i);
+ for (int i = 0; i < e->num_options; ++i) {
+ std::string choice_name = e->NameForOption(i);
if (enabled_entries.find(choice_name) != enabled_entries.end()) {
needs_restart_ = true;
enabled_entries.erase(choice_name);
@@ -380,6 +432,38 @@ void FlagsState::Reset() {
appended_switches_.clear();
}
+void FlagsState::RegisterAllFeatureVariationParameters(
+ FlagsStorage* flags_storage) {
+ std::set<std::string> enabled_entries;
+ GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, &enabled_entries);
+
+ for (size_t i = 0; i < num_feature_entries_; ++i) {
+ const FeatureEntry& e = feature_entries_[i];
+ if (e.type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) {
+ for (int j = 0; j < e.num_options; ++j) {
+ const FeatureEntry::FeatureVariation* variation =
+ e.VariationForOption(j);
+ if (variation != nullptr && enabled_entries.count(e.NameForOption(j))) {
+ // If the option is selected by the user & has variation, register it.
+ RegisterFeatureVariationParameters(e.feature_trial_name, *variation);
+ // TODO(jkrcal) The code does not associate the feature with the field
+ // trial |e.feature_trial_name|. The reason is that features
+ // overridden in chrome://flags are translated to command-line flags
+ // and thus treated earlier in the initialization. The fix requires
+ // larger changes. As a result:
+ // - the API calls to variations::GetVariationParamValueByFeature and
+ // to variations::GetVariationParamsByFeature do not work; and
+ // - the API call to base::FeatureList::IsEnabled does not mark the
+ // field trial as active (and the trial does not appear in UMA).
+ // If the code calls variations::GetVariationParamValue or
+ // variations::GetVariationParams providing the trial name, everything
+ // should work fine.
+ }
+ }
+ }
+ }
+}
+
void FlagsState::GetFlagFeatureEntries(
FlagsStorage* flags_storage,
FlagAccess access,
@@ -421,7 +505,8 @@ void FlagsState::GetFlagFeatureEntries(
case FeatureEntry::MULTI_VALUE:
case FeatureEntry::ENABLE_DISABLE_VALUE:
case FeatureEntry::FEATURE_VALUE:
- data->Set("choices", CreateChoiceData(entry, enabled_entries));
+ case FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
+ data->Set("options", CreateOptionsData(entry, enabled_entries));
break;
}
« no previous file with comments | « components/flags_ui/flags_state.h ('k') | components/flags_ui/flags_state_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698