Allow overriding variation parameter via chrome://flags.

chrome/browser/about_flags_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/about_flags.h"
 
 #include <stddef.h>
 
 #include <map>
 #include <set>
@@ skipping 166 matching lines @@
       testing::GetFeatureEntries(&num_entries);
 
   for (size_t i = 0; i < num_entries; ++i) {
     const flags_ui::FeatureEntry& entry = entries[i];
     switch (entry.type) {
       case flags_ui::FeatureEntry::SINGLE_VALUE:
       case flags_ui::FeatureEntry::SINGLE_DISABLE_VALUE:
         result.insert(entry.command_line_switch);
         break;
       case flags_ui::FeatureEntry::MULTI_VALUE:
-        for (int j = 0; j < entry.num_choices; ++j) {
+        for (int j = 0; j < entry.num_options; ++j) {
           result.insert(entry.choices[j].command_line_switch);

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

Iterating num_options times but uses the choices array gives the impression
there's a bug. Is there a reason you didn't rename choices to options?

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

I did not rename choices to options because I wanted to distinguish the more
general notion of options (applies to MULTI_VALUE, ENABLE_DISABLE_VALUE,
FEATURE_VALUE, and FEATURE_WITH_VARIATIONS_VALUE) from the notion of choice
(applies only to MULTI_VALUE).

I agree it looks like a bug. To address this, I added a getter function
ChoiceForOption(int option) and replaced all instances of "choice[i]" by
"ChoiceForOption(i)".

(The name is slightly cumbersome, yet it aligns with all other ...ForOption
functions of FeatureEntry. Another step would be to replace "Choice" by
something more specific such as "Switch". I am not sure if it is worth renaming
it all around.)

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

Adding the function sounds good to me.

         }
         break;
       case flags_ui::FeatureEntry::ENABLE_DISABLE_VALUE:
         result.insert(entry.command_line_switch);
         result.insert(entry.disable_command_line_switch);
         break;
       case flags_ui::FeatureEntry::FEATURE_VALUE:
         break;
     }
   }
@@ skipping 102 matching lines @@
     EXPECT_TRUE(enum_entry != histograms_xml_switches_ids.end() &&
                 enum_entry->first == flag)
         << "histograms.xml enum LoginCustomFlags doesn't contain switch '"
         << flag << "' (value=" << uma_id
         << " expected). Consider adding entry:\n"
         << "  " << GetHistogramEnumEntryText(flag, uma_id);
   }
 }
 
 }  // namespace about_flags