Allow overriding variation parameter via chrome://flags.

components/flags_ui/flags_state.cc

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..669f4bfa9e2e19e8bf433162d10ab8ecf8333c51 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,6 +20,7 @@
 #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 {
@@ -99,8 +101,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 +114,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,10 +132,17 @@ 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();
   return false;
@@ -148,8 +158,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 +170,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, 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.

[Alexei Svitkine (slow), 2016/06/16 09:40:57]

Nit: Does "selected" fit on line above?

[jkrcal, 2016/06/16 14:49:17]

Done (you were right, it fits).

+    // This way, the parameters cannot get overwritten in later phases
+    // (such as from the server).
+    base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
+        feature_trial_name, kTrialGroupAboutFlags);
+    if (!trial) {
+      DLOG(WARNING) << "Could not create the trial " << feature_trial_name
+                    << " with group " << kTrialGroupAboutFlags;
+    }
+  }
+}
+
 }  // namespace
 
 // Keeps track of affected switches for each FeatureEntry, based on which
@@ -224,27 +266,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 +353,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 +428,31 @@ 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.

[Alexei Svitkine (slow), 2016/06/16 09:40:57]

Please also mention what this means in practice.

Specifically, it means that:  If the feature state is queried, it won't
automatically activate the field trial. So the related field trial will not be
reported to UMA/crash, etc.

[jkrcal, 2016/06/16 14:49:17]

Done.

+        }
+      }
+    }
+  }
+}
+
 void FlagsState::GetFlagFeatureEntries(
     FlagsStorage* flags_storage,
     FlagAccess access,
@@ -421,7 +494,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;
     }