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

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 7577556a0b3e3a97ca013d70cf3a88c938e8c25c..1c923fb98ffa8d0a302694f8221e950cae9c9e91 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -11,6 +11,8 @@
 
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
+#include "base/metrics/sparse_histogram.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -22,6 +24,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "components/autofill/core/common/autofill_switches.h"
 #include "components/cloud_devices/common/cloud_devices_switches.h"
+#include "components/metrics/metrics_hashes.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "content/public/browser/user_metrics.h"
 #include "extensions/common/switches.h"
@@ -61,6 +64,8 @@ using base::UserMetricsAction;
 
 namespace about_flags {
 
+const uint32_t kBadSwitchFormatHistogramId = 0;
+
 // Macros to simplify specifying the type.
 #define SINGLE_VALUE_TYPE_AND_VALUE(command_line_switch, switch_value) \
     Experiment::SINGLE_VALUE, \
@@ -2144,17 +2149,31 @@ void ConvertFlagsToSwitches(FlagsStorage* flags_storage,
 }
 
 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 = false;
   // 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 =
+        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 std::equal(new_flags.begin(), new_flags.end(), active_flags.begin());
+  return result;
 }
 
 void GetFlagsExperimentsData(FlagsStorage* flags_storage,
@@ -2262,6 +2281,40 @@ void RecordUMAStatistics(FlagsStorage* flags_storage) {
   content::RecordAction(UserMetricsAction("StartupTick"));
 }
 
+uint32_t GetSwitchUMAId(const std::string& switch_name) {
+  return static_cast<uint32_t>(metrics::HashMetricName(switch_name));
+}
+
+void ReportCustomFlags(const std::string& uma_histogram_hame,
+                       const std::set<std::string>& command_line_difference) {
+  for (std::set<std::string>::const_iterator it =
+           command_line_difference.begin();
+       it != command_line_difference.end();
+       ++it) {
+    int uma_id = about_flags::kBadSwitchFormatHistogramId;
+    if (StartsWithASCII(*it, "--", true /* case_sensitive */)) {
+      // Skip '--' before switch name.
+      std::string switch_name(it->substr(2));
+
+      // Kill value, if any.
+      const size_t value_pos = switch_name.find('=');
+      if (value_pos != std::string::npos)
+        switch_name.resize(value_pos);
+
+      uma_id = GetSwitchUMAId(switch_name);
+    } else {
+      NOTREACHED() << "ReportCustomFlags(): flag '" << *it
+                   << "' has incorrect format.";
+    }
+    VLOG(1) << "ReportCustomFlags(): histogram='" << uma_histogram_hame << "' '"

[sky, 2014/08/11 23:45:44]

Is there a reason you want VLOG here and not DVLOG?

[Alexander Alekseev, 2014/08/12 18:02:25]

The idea is see the "reason to restart" in verbose log.

[Alexander Alekseev, 2014/08/12 18:03:15]

"The idea is to see" ...

[sky, 2014/08/12 19:57:13]

Certainly, but why release and not debug? Do you really need this for release
builds?

[Alexander Alekseev, 2014/08/13 19:59:34]

Running debug build on device is very slow. So it is usually
"last resort" option.

I moved VLOG() to chromeos-specific code, and replaced if here with DVLOG().

+            << *it << "', uma_id=" << uma_id;
+
+    // Sparse histogram macro does not cache the histogram, so it's safe
+    // to use macro with non-static histogram name here.
+    UMA_HISTOGRAM_SPARSE_SLOWLY(uma_histogram_hame, uma_id);
+  }
+}
+
 //////////////////////////////////////////////////////////////////////////////
 // FlagsState implementation.