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 43185bc7c90a13dec1fe79c3f58ad6657718e10c..75c5dee31c38b8ce7f9e079f667fd135183be572 100644
--- a/components/flags_ui/flags_state.cc
+++ b/components/flags_ui/flags_state.cc
@@ -8,6 +8,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"
@@ -16,6 +17,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 {
@@ -96,8 +98,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;
   }
 }
@@ -108,17 +111,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]);
       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);
@@ -126,10 +129,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);
+      return true;
   }
   NOTREACHED();
   return false;
@@ -145,8 +155,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;
@@ -156,17 +167,18 @@ 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) {
     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(value);
   }
@@ -221,27 +233,34 @@ 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,
+        for (int j = 0; j < e.num_options; ++j) {
+          AddSwitchMapping(e.NameForOption(j), e.choices[j].command_line_switch,
                            e.choices[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::State state = e.StateForOption(j);
+          if (state == FeatureEntry::State::DEFAULT) {
+            AddFeatureMapping(e.NameForOption(j), std::string(), false,
+                              &name_to_switch_map);
+          } else {
+            const std::string& key = e.NameForOption(j);

[Alexei Svitkine (slow), 2016/06/08 18:54:35]

Nit: Just inline this below

[jkrcal, 2016/06/09 09:55:15]

Done.

+            AddFeatureMapping(key, e.feature->name,
+                              state == FeatureEntry::State::ENABLED,
+                              &name_to_switch_map);
+          }
+        }
         break;
     }
   }
@@ -302,11 +321,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);
@@ -377,6 +396,47 @@ void FlagsState::Reset() {
   appended_switches_.clear();
 }
 
+void FlagsState::RegisterFeatureVariationParameters(
+    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 std::string& key = e.NameForOption(j);
+        const FeatureEntry::FeatureVariation* variation =
+            e.VariationForOption(j);
+        if (enabled_entries.count(key) && variation != nullptr) {
+          // If the option is selected by the user & has variation, register it.
+          std::map<std::string, std::string> params;
+          for (int i = 0; i < variation->num_params; ++i) {
+            params[variation->params[i].param_name] =
+                variation->params[i].param_value;
+          }
+
+          bool success = variations::AssociateVariationParams(
+              e.feature_trial, kTrialGroupAboutFlags, params);
+          if (success) {
+            // Successful association also means that no group is created and
+            // selected for the trial. Thus, create the trial to make it active
+            // and 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(
+                e.feature_trial, kTrialGroupAboutFlags);
+            if (!trial) {
+              DLOG(WARNING) << "Could not create the trial " << e.feature_trial
+                            << " with group " << kTrialGroupAboutFlags;
+            }
+          }
+        }

[Alexei Svitkine (slow), 2016/06/08 18:54:35]

This is a lot of nesting. Can you make the refactor code that processes a given
option (i.e. the inner block) into a helper function so that its more readable?

[jkrcal, 2016/06/09 09:55:15]

Done. I moved it out one block deeper, I think it makes more sense so.
(the helper function is called only for the variations that are really
selected).

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