Fix logging of Launch.FlagsAtStartup histogram.

components/flags_ui/flags_state.cc

 // Copyright 2015 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 "components/flags_ui/flags_state.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/callback.h"
@@ skipping 210 matching lines @@
       feature_trial_name, internal::kTrialGroupAboutFlags);
   if (!trial) {
     DLOG(WARNING) << "Could not create the trial " << feature_trial_name
                   << " with group " << internal::kTrialGroupAboutFlags;
   }
   return trial;
 }
 
 }  // 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),
       num_feature_entries_(num_feature_entries),
       needs_restart_(false) {}
 
 FlagsState::~FlagsState() {}
 
 void FlagsState::ConvertFlagsToSwitches(
     FlagsStorage* flags_storage,
     base::CommandLine* command_line,
     SentinelsMode sentinels,
     const char* enable_features_flag_name,
     const char* disable_features_flag_name) {
   std::set<std::string> enabled_entries;
+  std::map<std::string, SwitchEntry> name_to_switch_map;
 

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

nit: I'd omit this newline.

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

Done.

-  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;
-    }
-  }
-
+  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_;
 }
 
 void FlagsState::SetFeatureEntryEnabled(FlagsStorage* flags_storage,
                                         const std::string& internal_name,
                                         bool enable) {
   size_t at_index = internal_name.find(testing::kMultiSeparator);
   if (at_index != std::string::npos) {
     DCHECK(enable);
@@ skipping 257 matching lines @@
   if (out_difference && !result) {
     std::set_symmetric_difference(
         new_flags.begin(), new_flags.end(), active_flags.begin(),
         active_flags.end(),
         std::inserter(*out_difference, out_difference->begin()));
   }
 
   return result;
 }
 
+FlagsState::SwitchEntry::SwitchEntry() : feature_state(false) {}
+FlagsState::SwitchEntry::~SwitchEntry() {}
+
 void FlagsState::AddSwitchMapping(
     const std::string& key,
     const std::string& switch_name,
     const std::string& switch_value,
     std::map<std::string, SwitchEntry>* name_to_switch_map) {
   DCHECK(!base::ContainsKey(*name_to_switch_map, key));
 
   SwitchEntry* entry = &(*name_to_switch_map)[key];
   entry->switch_name = switch_name;
   entry->switch_value = switch_value;
@@ skipping 125 matching lines @@
 #endif
   }
 
   std::set<std::string> new_enabled_entries =
       base::STLSetIntersection<std::set<std::string>>(platform_entries,
                                                       *result);
 
   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