Fix logging of Launch.FlagsAtStartup histogram.

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 00c0c2a258483c851e30c0989758d1d9b0b13517..55b949a08407bc08ad97bfe7c2b3166a4d8bf640 100644
--- a/components/flags_ui/flags_state.cc
+++ b/components/flags_ui/flags_state.cc
@@ -228,24 +228,6 @@ base::FieldTrial* RegisterFeatureVariationParameters(
 
 }  // namespace
 
-// Keeps track of affected switches for each FeatureEntry, based on which
-// choice is selected for it.
-struct SwitchEntry {
-  // Corresponding base::Feature to toggle.
-  std::string feature_name;
-
-  // If |feature_name| is not empty, the state (enable/disabled) to set.
-  bool feature_state;
-
-  // The name of the switch to add.
-  std::string switch_name;
-
-  // If |switch_name| is not empty, the value of the switch to set.
-  std::string switch_value;
-
-  SwitchEntry() : feature_state(false) {}
-};
-
 FlagsState::FlagsState(const FeatureEntry* feature_entries,
                        size_t num_feature_entries)
     : feature_entries_(feature_entries),
@@ -261,55 +243,35 @@ void FlagsState::ConvertFlagsToSwitches(
     const char* enable_features_flag_name,
     const char* disable_features_flag_name) {
   std::set<std::string> enabled_entries;
-
-  GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, &enabled_entries);
-
   std::map<std::string, SwitchEntry> name_to_switch_map;
-  for (size_t i = 0; i < num_feature_entries_; ++i) {
-    const FeatureEntry& e = feature_entries_[i];
-    switch (e.type) {
-      case FeatureEntry::SINGLE_VALUE:
-      case FeatureEntry::SINGLE_DISABLE_VALUE:
-        AddSwitchMapping(e.internal_name, e.command_line_switch,
-                         e.command_line_value, &name_to_switch_map);
-        break;
-      case FeatureEntry::MULTI_VALUE:
-        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.NameForOption(0), std::string(), std::string(),
-                         &name_to_switch_map);
-        AddSwitchMapping(e.NameForOption(1), e.command_line_switch,
-                         e.command_line_value, &name_to_switch_map);
-        AddSwitchMapping(e.NameForOption(2), e.disable_command_line_switch,
-                         e.disable_command_line_value, &name_to_switch_map);
-        break;
-      case FeatureEntry::FEATURE_VALUE:
-      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;
-    }
-  }
 

[Ilya Sherman, 2016/08/31 19:06:31]

nit: I'd omit this newline.

[Alexei Svitkine (slow), 2016/08/31 19:46:22]

Done.

+  GenerateFlagsToSwitchesMapping(flags_storage, &enabled_entries,
+                                 &name_to_switch_map);
   AddSwitchesToCommandLine(enabled_entries, name_to_switch_map, sentinels,
                            command_line, enable_features_flag_name,
                            disable_features_flag_name);
 }
 
+void FlagsState::GetSwitchesFromFlags(FlagsStorage* flags_storage,
+                                      std::set<std::string>* switches) {
+  std::set<std::string> enabled_entries;
+  std::map<std::string, SwitchEntry> name_to_switch_map;
+  GenerateFlagsToSwitchesMapping(flags_storage, &enabled_entries,
+                                 &name_to_switch_map);
+
+  for (const std::string& entry_name : enabled_entries) {
+    const auto& entry_it = name_to_switch_map.find(entry_name);
+    if (entry_it == name_to_switch_map.end()) {
+      NOTREACHED();
+      continue;
+    }

[Ilya Sherman, 2016/08/31 19:06:31]

nit: Please replace these four lines with a single DCHECK.

[Alexei Svitkine (slow), 2016/08/31 19:46:22]

Done. DCHECK_NE didn't compile, so I just used a DCHECK.

+
+    const SwitchEntry& entry = entry_it->second;
+    if (!entry.switch_name.empty())
+      switches->insert("--" + entry.switch_name);
+  }
+}
+
 bool FlagsState::IsRestartNeededToCommitChanges() {
   return needs_restart_;
 }
@@ -587,6 +549,9 @@ bool FlagsState::AreSwitchesIdenticalToCurrentCommandLine(
   return result;
 }
 
+FlagsState::SwitchEntry::SwitchEntry() : feature_state(false) {}
+FlagsState::SwitchEntry::~SwitchEntry() {}
+
 void FlagsState::AddSwitchMapping(
     const std::string& key,
     const std::string& switch_name,
@@ -732,4 +697,51 @@ void FlagsState::GetSanitizedEnabledFlagsForCurrentPlatform(
   result->swap(new_enabled_entries);
 }
 
+void FlagsState::GenerateFlagsToSwitchesMapping(
+    FlagsStorage* flags_storage,
+    std::set<std::string>* enabled_entries,
+    std::map<std::string, SwitchEntry>* name_to_switch_map) {
+  GetSanitizedEnabledFlagsForCurrentPlatform(flags_storage, enabled_entries);
+
+  for (size_t i = 0; i < num_feature_entries_; ++i) {
+    const FeatureEntry& e = feature_entries_[i];
+    switch (e.type) {
+      case FeatureEntry::SINGLE_VALUE:
+      case FeatureEntry::SINGLE_DISABLE_VALUE:
+        AddSwitchMapping(e.internal_name, e.command_line_switch,
+                         e.command_line_value, name_to_switch_map);
+        break;
+      case FeatureEntry::MULTI_VALUE:
+        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.NameForOption(0), std::string(), std::string(),
+                         name_to_switch_map);
+        AddSwitchMapping(e.NameForOption(1), e.command_line_switch,
+                         e.command_line_value, name_to_switch_map);
+        AddSwitchMapping(e.NameForOption(2), e.disable_command_line_switch,
+                         e.disable_command_line_value, name_to_switch_map);
+        break;
+      case FeatureEntry::FEATURE_VALUE:
+      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;
+    }
+  }
+}
+
 }  // namespace flags_ui