Collect UMA statistics on which chrome://flags lead to chrome restart on ChromeOS.

chrome/browser/about_flags.cc

 // Copyright (c) 2012 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 <iterator>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 1915 matching lines @@
   {
     "enable-data-reduction-proxy-dev",
     IDS_FLAGS_ENABLE_DATA_REDUCTION_PROXY_DEV_NAME,
     IDS_FLAGS_ENABLE_DATA_REDUCTION_PROXY_DEV_DESCRIPTION,
     kOsAndroid,
     ENABLE_DISABLE_VALUE_TYPE(
         data_reduction_proxy::switches::kEnableDataReductionProxyDev,
         data_reduction_proxy::switches::kDisableDataReductionProxyDev)
   },
 #endif
+
+  // Add new choice before this line.
+  //
+  // If it can be used on ChromeOS, please also add it to
+  // chrome/browser/chromeos/login/report_custom_flags.cc

[sky, 2014/06/19 20:23:28]

This is not maintainable. You need to come up with a better way to make this
maintainable. Maybe by exposing some way to get the set of switches? Hopefully
in some way that doesn't require you to have a stable index.

[Alexander Alekseev, 2014/06/20 18:42:45]

We need histogram here to know which flags lead to bad user experience most. So
we need a stable index here. And even more, it must be in histograms.xml .

Right now it is designed in a way, that new flags which were not added to
report_custom_flags.cc, will be reported as "unknown flag".
And if we see large value in "unknown flag" bucket, we can add these flags
manually.

On the other hand, I can create script like extract_actions.py to
rebuild that table and histograms.xml. And another script to presubmit to check
for consistency. But it will not handle merge automatically. It is also
problematic to automatically analyze all #ifdefs (like #ifndef OS_ANDROID).


So, what design would you prefer?

 };
 
 const Experiment* experiments = kExperiments;
 size_t num_experiments = arraysize(kExperiments);
 
 // Stores and encapsulates the little state that about:flags has.
 class FlagsState {
  public:
   FlagsState() : needs_restart_(false) {}
   void ConvertFlagsToSwitches(FlagsStorage* flags_storage,
@@ skipping 191 matching lines @@
 
 void ConvertFlagsToSwitches(FlagsStorage* flags_storage,
                             CommandLine* command_line,
                             SentinelsMode sentinels) {
   FlagsState::GetInstance()->ConvertFlagsToSwitches(flags_storage,
                                                     command_line,
                                                     sentinels);
 }
 
 bool AreSwitchesIdenticalToCurrentCommandLine(
-    const CommandLine& new_cmdline, const CommandLine& active_cmdline) {
+    const CommandLine& new_cmdline,
+    const CommandLine& active_cmdline,
+    std::set<CommandLine::StringType>* out_difference) {
   std::set<CommandLine::StringType> new_flags =
       ExtractFlagsFromCommandLine(new_cmdline);
   std::set<CommandLine::StringType> active_flags =
       ExtractFlagsFromCommandLine(active_cmdline);
 
+  bool result = true;
   // Needed because std::equal doesn't check if the 2nd set is empty.
-  if (new_flags.size() != active_flags.size())
-    return false;
+  if (new_flags.size() != active_flags.size()) {
+    result = false;
+  } else {
+    result =
+        std::equal(new_flags.begin(), new_flags.end(), active_flags.begin());
+  }
 
-  return std::equal(new_flags.begin(), new_flags.end(), active_flags.begin());
+  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;
 }
 
 void GetFlagsExperimentsData(FlagsStorage* flags_storage,
                              FlagAccess access,
                              base::ListValue* supported_experiments,
                              base::ListValue* unsupported_experiments) {
   std::set<std::string> enabled_experiments;
   GetSanitizedEnabledFlags(flags_storage, &enabled_experiments);
 
   int current_platform = GetCurrentPlatform();
@@ skipping 286 matching lines @@
 }
 
 const Experiment* GetExperiments(size_t* count) {
   *count = num_experiments;
   return experiments;
 }
 
 }  // namespace testing
 
 }  // namespace about_flags