Allow overriding variation parameter via chrome://flags.

components/flags_ui/feature_entry.cc

Index: components/flags_ui/feature_entry.cc
diff --git a/components/flags_ui/feature_entry.cc b/components/flags_ui/feature_entry.cc
index b7556970cad778a9cef605e67d8c6bd5dcfa802e..4575c7146969c3de37133e8d5aee158ad0f89dd0 100644
--- a/components/flags_ui/feature_entry.cc
+++ b/components/flags_ui/feature_entry.cc
@@ -6,25 +6,28 @@
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace flags_ui {
 
-std::string FeatureEntry::NameForChoice(int index) const {
+std::string FeatureEntry::NameForOption(int index) const {
   DCHECK(type == FeatureEntry::MULTI_VALUE ||
          type == FeatureEntry::ENABLE_DISABLE_VALUE ||
-         type == FeatureEntry::FEATURE_VALUE);
-  DCHECK_LT(index, num_choices);
+         type == FeatureEntry::FEATURE_VALUE ||
+         type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
+  DCHECK_LT(index, num_options);
   return std::string(internal_name) + testing::kMultiSeparator +
          base::IntToString(index);
 }
 
-base::string16 FeatureEntry::DescriptionForChoice(int index) const {
+base::string16 FeatureEntry::DescriptionForOption(int index) const {
   DCHECK(type == FeatureEntry::MULTI_VALUE ||
          type == FeatureEntry::ENABLE_DISABLE_VALUE ||
-         type == FeatureEntry::FEATURE_VALUE);
-  DCHECK_LT(index, num_choices);
+         type == FeatureEntry::FEATURE_VALUE ||
+         type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
+  DCHECK_LT(index, num_options);
   int description_id;
   if (type == FeatureEntry::ENABLE_DISABLE_VALUE ||
       type == FeatureEntry::FEATURE_VALUE) {
@@ -34,12 +37,64 @@ base::string16 FeatureEntry::DescriptionForChoice(int index) const {
         IDS_GENERIC_EXPERIMENT_CHOICE_DISABLED,
     };
     description_id = kEnableDisableDescriptionIds[index];
+  } else if (type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE) {
+    if (index == 0) {
+      description_id = IDS_GENERIC_EXPERIMENT_CHOICE_DEFAULT;
+    } else if (index == 1) {
+      // Variation 1: the default Enabled (disregarding its description_id).
+      description_id = IDS_GENERIC_EXPERIMENT_CHOICE_ENABLED;
+    } else if (index < num_options - 1) {
+      // Variations 2 .. n
+      int variation_index = index - 1;
+      return l10n_util::GetStringUTF16(IDS_GENERIC_EXPERIMENT_CHOICE_ENABLED) +

[Alexei Svitkine (slow), 2016/06/10 15:46:48]

So the syntax here is: "Enabled <description_text>". Add a comment above this to
make it clearer.

Also, I wonder if this always right. Maybe some features don't want to say
"Enabled"? Or maybe they should have a ":" after them? e.g. "Enabled: <foo>".

Anyway, I don't have a very strong opinion right now, so I think it's fine to go
with this and it can be tweaked in the future if we find there's a desire for
different syntax.

[jkrcal, 2016/06/14 10:04:21]

Done.

+             base::ASCIIToUTF16(" ") +
+             base::ASCIIToUTF16(
+                 feature_variations[variation_index].description_text);
+    } else {
+      description_id = IDS_GENERIC_EXPERIMENT_CHOICE_DISABLED;

[Alexei Svitkine (slow), 2016/06/10 15:46:48]

Nit: Add a comment: // |index| == |num_options - 1|

Or actually can make it a DCHECK_EQ(num_options - 1, index);

[jkrcal, 2016/06/14 10:04:21]

Done.

+    }
   } else {
     description_id = choices[index].description_id;
   }
   return l10n_util::GetStringUTF16(description_id);
 }
 
+const FeatureEntry::Choice& FeatureEntry::ChoiceForOption(int index) const {
+  DCHECK(type == FeatureEntry::MULTI_VALUE);

[Alexei Svitkine (slow), 2016/06/10 15:46:48]

DCHECK_EQ() when it's just one option.

[jkrcal, 2016/06/14 10:04:21]

Done.

+  DCHECK_LT(index, num_options);
+
+  return choices[index];
+}
+
+FeatureEntry::State FeatureEntry::StateForOption(int index) const {
+  DCHECK(type == FeatureEntry::FEATURE_VALUE ||
+         type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
+  DCHECK_LT(index, num_options);
+
+  if (index == 0)
+    return FeatureEntry::State::DEFAULT;
+  else if (index == num_options - 1)
+    return FeatureEntry::State::DISABLED;
+  else
+    return FeatureEntry::State::ENABLED;
+}
+
+const FeatureEntry::FeatureVariation* FeatureEntry::VariationForOption(
+    int index) const {
+  DCHECK(type == FeatureEntry::FEATURE_VALUE ||
+         type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE);
+  DCHECK_LT(index, num_options);
+
+  if (type == FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE && index > 0 &&
+      index < num_options - 1) {
+    // We have no variations for FEATURE_VALUE type. Option at |index|
+    // corresponds to variation at |index| - 1 as the first option is "Default".
+    return &feature_variations[index - 1];
+  } else {

[Alexei Svitkine (slow), 2016/06/10 15:46:49]

Nit: No } else { if the above if has a return.

(But you can keep it in the function above since that case is simpler and
doesn't require extra {}'s.)

[jkrcal, 2016/06/14 10:04:21]

Done.

+    return nullptr;
+  }
+}
+
 namespace testing {
 
 // WARNING: '@' is also used in the html file. If you update this constant you