Show all experiments, even those that are unavailable on the current platform

chrome/browser/about_flags.cc

Index: chrome/browser/about_flags.cc
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 543a83007afbb6ab9edb377abe51fd274787f4ca..2750089f7271586b7c3ada53a16aa1529667e699 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -42,6 +42,18 @@ namespace {
 
 const unsigned kOsAll = kOsMac | kOsWin | kOsLinux | kOsCrOS;
 
+std::map<int, std::string> nameMap;

[Nico, 2011/12/16 20:25:07]

Global non-pods need both static initialization and exit time destruction, both
things that we're trying to get rid of (and both things that will turn bots red
nowadays)

+
+std::map<int, std::string> OSNames() {
+  if (nameMap.empty()) {
+    nameMap[kOsMac] = "Mac";
+    nameMap[kOsWin] = "Windows";
+    nameMap[kOsLinux] = "Linux";
+    nameMap[kOsCrOS] = "Chrome OS";

[Nico, 2011/12/16 20:25:07]

std::map<int, ...> with less than many keys is almost always better done as a
switch statement

+  }
+  return nameMap;
+}
+
 // Names for former Chrome OS Labs experiments, shared with prefs migration
 // code.
 const char kMediaPlayerExperimentName[] = "media-player";
@@ -631,8 +643,6 @@ ListValue* GetFlagsExperimentsData(PrefService* prefs) {
   ListValue* experiments_data = new ListValue();
   for (size_t i = 0; i < num_experiments; ++i) {
     const Experiment& experiment = experiments[i];
-    if (!(experiment.supported_platforms & current_platform))
-      continue;
 
     DictionaryValue* data = new DictionaryValue();
     data->SetString("internal_name", experiment.internal_name);
@@ -641,6 +651,18 @@ ListValue* GetFlagsExperimentsData(PrefService* prefs) {
     data->SetString("description",
                     l10n_util::GetStringUTF16(
                         experiment.visible_description_id));
+    bool supported = experiment.supported_platforms & current_platform;
+    data->SetBoolean("supported", supported);
+
+    ListValue* supported_platforms = new ListValue();
+    std::map<int, std::string> os_names = OSNames();
+    for (std::map<int, std::string>::const_iterator iter = os_names.begin();
+         iter != os_names.end(); ++iter) {
+      if (experiment.supported_platforms & iter->first) {
+        supported_platforms->Append(new StringValue(iter->second));
+      }
+    }

[Nico, 2011/12/16 20:25:07]

supported_platforms->Append(new StringValue("Mac"));
supported_platforms->Append(new StringValue("Windows"));
supported_platforms->Append(new StringValue("Linux"));
supported_platforms->Append(new StringValue("ChromeOS"));

does the same thing, is easier to understand, and less code

[Tyler Breisacher (Chromium), 2011/12/16 21:18:11]

Wouldn't that append all four platforms to every single experiment? We want to
only show the platform(s) where this particular experiment is available. I'm not
sure how to do this more clearly.

[Nico, 2011/12/16 21:38:04]

Ah, right. Either write the function you have above with a switch and do

for (int platforms = experiment.supported_platforms; platforms; platforms &=
platforms - 1)
  supported_platforms->Append(new StringValue(NameForPlatform(platform &
-platform)));

(which I suppose is a bit hard to read) or do

void AddOsStrings(bitmask, list) {
  struct { int bit, const char str[] } kBitsToOs = {
    ...
  }
  for (int i = 0; i < ARRAYSIZE_UNSAFE(kBitsToOs); ++i)
    if (bitmask & kBitsToOs[i].bit)
      list->Append(new StringValue(kBitsToOs[i].str)
}

and call that from here

I like the second approach, since it also reduces the number of nested loops in
GetFlagsExperimentData

+    data->Set("supported_platforms", supported_platforms);
 
     switch (experiment.type) {
       case Experiment::SINGLE_VALUE: