about_flags::ReportCustomFlags() should use signed 32-bit IDs.

chrome/browser/about_flags_unittest.cc

 // 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"
@@ skipping 23 matching lines @@
 const char kSwitch3[] = "switch3";
 const char kValueForSwitch2[] = "value_for_switch2";
 
 const char kMultiSwitch1[] = "multi_switch1";
 const char kMultiSwitch2[] = "multi_switch2";
 const char kValueForMultiSwitch2[] = "value_for_multi_switch2";
 
 const char kEnableDisableValue1[] = "value1";
 const char kEnableDisableValue2[] = "value2";
 
-typedef std::map<std::string, uint32_t> SwitchToIdMap;
+typedef base::HistogramBase::Sample Sample;
+typedef std::map<std::string, Sample> SwitchToIdMap;
 
 // This is a helper function to the ReadEnumFromHistogramsXml().
 // 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(
+std::map<Sample, std::string> ParseEnumFromHistogramsXml(
     const std::string& enum_name,
     XmlReader* reader) {
   int entries_index = -1;
 
-  std::map<uint32_t, std::string> result;
+  std::map<Sample, 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)) {
+      Sample value;
+      if (has_value && !base::StringToInt(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>());
+  return (success ? result : std::map<Sample, 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(
+std::map<Sample, std::string> ReadEnumFromHistogramsXml(
     const std::string& enum_name,
     XmlReader* histograms_xml) {
-  std::map<uint32_t, std::string> login_custom_flags;
+  std::map<Sample, 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>();
+          return std::map<Sample, 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>();
+          return std::map<Sample, 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;
 
@@ skipping 508 matching lines @@
     std::string name = experiments->internal_name;
     EXPECT_EQ(std::string::npos, name.find(testing::kMultiSeparator)) << i;
   }
 }
 
 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 =
+                                     const Sample switch_histogram_id,
+                                     std::map<std::string, Sample>* out_map) {
+    const std::pair<std::map<std::string, Sample>::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) {
+                                        Sample value) {
     return base::StringPrintf(
-        "<int value=\"%u\" label=\"%s\"/>", value, switch_name.c_str());
+        "<int value=\"%d\" 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 =
+  std::map<Sample, 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 =
+  for (std::map<Sample, 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);
+    const Sample 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);
+    const Sample 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