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

chrome/browser/about_flags_histogram_unittest.cc

Index: chrome/browser/about_flags_histogram_unittest.cc
diff --git a/chrome/browser/about_flags_histogram_unittest.cc b/chrome/browser/about_flags_histogram_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fc4a2e981b31d6ca51c8127567be651c288b1e7f
--- /dev/null
+++ b/chrome/browser/about_flags_histogram_unittest.cc
@@ -0,0 +1,292 @@
+// Copyright (c) 2011 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 <stdint.h>
+
+#include "base/files/file_path.h"
+#include "base/memory/scoped_vector.h"
+#include "base/path_service.h"
+/*#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/testing_pref_service.h"*/

[Ilya Sherman, 2014/08/08 03:49:45]

Please update this.

[Alexander Alekseev, 2014/08/09 01:30:01]

Done.

+#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/about_flags.h"
+//#include "chrome/browser/pref_service_flags_storage.h"
+//#include "chrome/common/chrome_switches.h"
+//#include "chrome/common/pref_names.h"
+//#include "grit/chromium_strings.h"

[Ilya Sherman, 2014/08/08 03:49:45]

Please update this.

[Alexander Alekseev, 2014/08/09 01:30:01]

Done.

+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/libxml/chromium/libxml_utils.h"
+
+namespace {
+
+typedef std::map<std::string, uint32_t> SwitchToIdMap;
+
+// This is a helper function to the next one.
+// Extracts single enum (with integer values) from histograms.xml.
+// Expects |reader| to point at given enum.
+// Returns map { value => label }.
+// Returns empty map on error.
+std::map<uint32_t, std::string> ParseEnumFromHistogramsXml(
+    const std::string& enum_name,
+    XmlReader* reader) {
+  int entries_index = -1;
+
+  std::map<uint32_t, std::string> result;
+  bool success = true;
+
+  while (true) {
+    const std::string node_name = reader->NodeName();
+    if (node_name == "enum" && reader->IsClosingElement())
+      break;
+
+    if (node_name == "int") {
+      ++entries_index;
+      std::string value_str;
+      std::string label;
+      const bool has_value = reader->NodeAttribute("value", &value_str);
+      const bool has_label = reader->NodeAttribute("label", &label);
+      if (!has_value) {
+        ADD_FAILURE() << "Bad " << enum_name << " enum entry (at index "
+                      << entries_index << ", label='" << label
+                      << "'): No 'value' attribute.";
+        success = false;
+      }
+      if (!has_label) {
+        ADD_FAILURE() << "Bad " << enum_name << " enum entry (at index "
+                      << entries_index << ", value_str='" << value_str
+                      << "'): No 'label' attribute.";
+        success = false;
+      }
+
+      uint32_t value;
+      if (has_value && !base::StringToUint(value_str, &value)) {
+        ADD_FAILURE() << "Bad " << enum_name << " enum entry (at index "
+                      << entries_index << ", label='" << label
+                      << "', value_str='" << value_str
+                      << "'): 'value' attribute is not integer.";
+        success = false;
+      }
+      if (result.count(value)) {
+        ADD_FAILURE() << "Bad " << enum_name << " enum entry (at index "
+                      << entries_index << ", label='" << label
+                      << "', value_str='" << value_str
+                      << "'): duplicate value '" << value_str
+                      << "' found in enum. The previous one has label='"
+                      << result[value] << "'.";
+        success = false;
+      }
+      if (success) {
+        result[value] = label;
+      }
+    }
+    // All enum entries are on the same level, so it is enough to iterate
+    // until possible.
+    reader->Next();
+  }
+  return (success ? result : std::map<uint32_t, std::string>());
+}
+
+// Find and read given enum (with integer values) from histograms.xml.
+// |enum_name| - enum name.
+// |histograms_xml| - must be loaded histograms.xml file.
+//
+// Returns map { value => label } so that:
+//   <int value="9" label="enable-pinch-virtual-viewport"/>
+// becomes:
+//   { 9 => "enable-pinch-virtual-viewport" }
+// Returns empty map on error.
+std::map<uint32_t, std::string> ReadEnumFromHistogramsXml(
+    const std::string& enum_name,
+    XmlReader* histograms_xml) {
+  std::map<uint32_t, std::string> login_custom_flags;
+
+  // Implement simple depth first search.
+  while (true) {
+    const std::string node_name = histograms_xml->NodeName();
+    if (node_name == "enum") {
+      std::string name;
+      if (histograms_xml->NodeAttribute("name", &name) && name == enum_name) {
+        if (!login_custom_flags.empty()) {
+          EXPECT_TRUE(login_custom_flags.empty())
+              << "Duplicate enum '" << enum_name << "' found in histograms.xml";
+          return std::map<uint32_t, std::string>();
+        }
+
+        const bool got_into_enum = histograms_xml->Read();
+        if (got_into_enum) {
+          login_custom_flags =
+              ParseEnumFromHistogramsXml(enum_name, histograms_xml);
+          EXPECT_FALSE(login_custom_flags.empty())
+              << "Bad enum '" << enum_name
+              << "' found in histograms.xml (format error).";
+        } else {
+          EXPECT_TRUE(got_into_enum)
+              << "Bad enum '" << enum_name
+              << "' (looks empty) found in histograms.xml.";
+        }
+        if (login_custom_flags.empty())
+          return std::map<uint32_t, std::string>();
+      }
+    }
+    // Go deeper if possible (stops at the closing tag of the deepest node).
+    if (histograms_xml->Read())
+      continue;
+
+    // Try next node on the same level (skips closing tag).
+    if (histograms_xml->Next())
+      continue;
+
+    // Go up until next node on the same level exists.
+    while (histograms_xml->Depth() && !histograms_xml->SkipToElement()) {
+    }
+
+    // Reached top. histograms.xml consists of the single top level node
+    // 'histogram-configuration', so this is the end.
+    if (!histograms_xml->Depth())
+      break;
+  }
+  EXPECT_FALSE(login_custom_flags.empty())
+      << "Enum '" << enum_name << "' is not found in histograms.xml.";
+  return login_custom_flags;
+}
+
+std::string FilePathStringTypeToString(const base::FilePath::StringType& path) {
+#if defined(OS_WIN)
+  return UTF16ToUTF8(path);
+#else
+  return path;
+#endif
+}
+
+std::set<std::string> GetAllSwitchesForTesting() {
+  std::set<std::string> result;
+
+  size_t num_experiments = 0;
+  const about_flags::Experiment* experiments =
+      about_flags::testing::GetExperiments(&num_experiments);
+
+  for (size_t i = 0; i < num_experiments; ++i) {
+    const about_flags::Experiment& experiment = experiments[i];
+    if (experiment.type == about_flags::Experiment::SINGLE_VALUE) {
+      result.insert(experiment.command_line_switch);
+    } else if (experiment.type == about_flags::Experiment::MULTI_VALUE) {
+      for (int j = 0; j < experiment.num_choices; ++j) {
+        result.insert(experiment.choices[j].command_line_switch);
+      }
+    } else {
+      DCHECK_EQ(experiment.type, about_flags::Experiment::ENABLE_DISABLE_VALUE);
+      result.insert(experiment.command_line_switch);
+      result.insert(experiment.disable_command_line_switch);
+    }
+  }
+  return result;
+}
+
+}  // anonymous namespace
+
+namespace about_flags {
+
+class AboutFlagsHistogramTest : public ::testing::Test {
+ protected:
+  // This is a helper function to check that all IDs in enum LoginCustomFlags in
+  // histograms.xml are unique.
+  void SetSwitchToHistogramIdMapping(const std::string& switch_name,
+                                     const uint32_t switch_histogram_id,
+                                     std::map<std::string, uint32_t>* out_map) {
+    const std::pair<std::map<std::string, uint32_t>::iterator, bool> status =
+        out_map->insert(std::make_pair(switch_name, switch_histogram_id));
+    if (!status.second) {
+      EXPECT_TRUE(status.first->second == switch_histogram_id)
+          << "Duplicate switch '" << switch_name
+          << "' found in enum 'LoginCustomFlags' in histograms.xml.";
+    }
+  }
+
+  // This method generates a hint for the user for what string should be added
+  // to the enum LoginCustomFlags to make in consistent.
+  std::string GetHistogramEnumEntryText(const std::string& switch_name,
+                                        uint32_t value) {
+    return base::StringPrintf(
+        "<int value=\"%u\" label=\"%s\"/>", value, switch_name.c_str());
+  }
+};
+
+TEST_F(AboutFlagsHistogramTest, CheckHistograms) {
+  base::FilePath histograms_xml_file_path;
+  ASSERT_TRUE(
+      PathService::Get(base::DIR_SOURCE_ROOT, &histograms_xml_file_path));
+  histograms_xml_file_path = histograms_xml_file_path.AppendASCII("tools")
+      .AppendASCII("metrics")
+      .AppendASCII("histograms")
+      .AppendASCII("histograms.xml");
+
+  XmlReader histograms_xml;
+  ASSERT_TRUE(histograms_xml.LoadFile(
+      FilePathStringTypeToString(histograms_xml_file_path.value())));
+  std::map<uint32_t, std::string> login_custom_flags =
+      ReadEnumFromHistogramsXml("LoginCustomFlags", &histograms_xml);
+  ASSERT_TRUE(login_custom_flags.size())
+      << "Error reading enum 'LoginCustomFlags' from histograms.xml.";
+
+  // Build reverse map {switch_name => id} from login_custom_flags.
+  SwitchToIdMap histograms_xml_switches_ids;
+
+  EXPECT_TRUE(login_custom_flags.count(kBadSwitchFormatHistogramId))
+      << "Entry for UMA ID of incorrect command-line flag is not found in "
+         "histograms.xml enum LoginCustomFlags. "
+         "Consider adding entry:\n"
+      << "  " << GetHistogramEnumEntryText("BAD_FLAG_FORMAT", 0);
+  // Check that all LoginCustomFlags entries have correct values.
+  for (std::map<uint32_t, std::string>::const_iterator it =
+           login_custom_flags.begin();
+       it != login_custom_flags.end();
+       ++it) {
+    if (it->first == kBadSwitchFormatHistogramId) {
+      // Add eror value with empty name.
+      SetSwitchToHistogramIdMapping(
+          "", it->first, &histograms_xml_switches_ids);
+      continue;
+    }
+    const uint32_t uma_id = GetSwitchUMAId(it->second);
+    EXPECT_EQ(uma_id, it->first)
+        << "histograms.xml enum LoginCustomFlags "
+           "entry '" << it->second << "' has incorrect value=" << it->first
+        << ", but " << uma_id << " is expected. Consider changing entry to:\n"
+        << "  " << GetHistogramEnumEntryText(it->second, uma_id);
+    SetSwitchToHistogramIdMapping(
+        it->second, it->first, &histograms_xml_switches_ids);
+  }
+
+  // Check that all flags in about_flags.cc have entries in login_custom_flags.
+  std::set<std::string> all_switches = GetAllSwitchesForTesting();
+  for (std::set<std::string>::const_iterator it = all_switches.begin();
+       it != all_switches.end();
+       ++it) {
+    // Skip empty placeholders.
+    if (it->empty())
+      continue;
+    const uint32_t uma_id = GetSwitchUMAId(*it);
+    EXPECT_NE(kBadSwitchFormatHistogramId, uma_id)
+        << "Command-line switch '" << *it
+        << "' from about_flags.cc has UMA ID equal to reserved value "
+           "kBadSwitchFormatHistogramId=" << kBadSwitchFormatHistogramId
+        << ". Please modify switch name.";
+    SwitchToIdMap::iterator enum_entry =
+        histograms_xml_switches_ids.lower_bound(*it);
+
+    // Ignore case here when switch ID is incorrect - it has already been
+    // reported in the previous loop.
+    EXPECT_TRUE(enum_entry != histograms_xml_switches_ids.end() &&
+                enum_entry->first == *it)
+        << "histograms.xml enum LoginCustomFlags doesn't contain switch '"
+        << *it << "' (value=" << uma_id
+        << " expected). Consider adding entry:\n"
+        << "  " << GetHistogramEnumEntryText(*it, uma_id);
+  }
+}
+
+}  // namespace about_flags