Fix presubmit_scheme_histograms.py and presubmit_bad_message_reasons.py

tools/metrics/histograms/update_histogram_enum.py

Index: tools/metrics/histograms/update_histogram_enum.py
diff --git a/tools/metrics/histograms/update_histogram_enum.py b/tools/metrics/histograms/update_histogram_enum.py
index f0a29e0fe91c7aaf1c0500e82c572c0d836c4059..c62b858423553ec365fe65c8480bfb1a4e2b0768 100644
--- a/tools/metrics/histograms/update_histogram_enum.py
+++ b/tools/metrics/histograms/update_histogram_enum.py
@@ -87,12 +87,12 @@ def ReadHistogramValues(filename, start_marker, end_marker, strip_k_prefix):
         label = m.group(1)
       else:
         continue
-    # If two enum labels have the same value
-    if enum_value in result:
-      return result, (result[enum_value], label)
     if strip_k_prefix:
       assert label.startswith('k'), "Enum " + label + " should start with 'k'."
       label = label[1:]

[Łukasz Anforowicz, 2017/04/26 22:20:54]

Stripping of 'k' prefix needs to happen *before* reporting duplicates (otherwise
the |label| doesn't have the prefix stripped, but |result[enum_value]| does have
the prefix stripped) and this results in an inconsistently looking error message
being returned from CheckPresubmitErrors below.

+    # If two enum labels have the same value
+    if enum_value in result:
+      return result, (result[enum_value], label)
     result[enum_value] = label
     enum_value += 1
   return result, None
@@ -180,30 +180,45 @@ def _GetOldAndUpdatedXml(histogram_enum_name, source_enum_values,
   return (xml, new_xml)
 
 
-def HistogramNeedsUpdate(histogram_enum_name, source_enum_path, start_marker,
+def CheckPresubmitErrors(histogram_enum_name, update_script_name,
+                         source_enum_path, start_marker,
                          end_marker, strip_k_prefix = False):
-  """Reads a C++ enum from a .h file and does a dry run of updating
-  histograms.xml to match. Returns true if the histograms.xml file would be
-  changed.
+  """Reads a C++ enum from a .h file and checks for presubmit violations:
+  1. Failure to update histograms.xml to match
+  2. Introduction of duplicate values.
 
   Args:
       histogram_enum_name: The name of the XML <enum> attribute to update.
+      update_script_name: The name of an update script to run to update the UMA
+          mappings for the enum.
       source_enum_path: A unix-style path, relative to src/, giving
           the C++ header file from which to read the enum.
       start_marker: A regular expression that matches the start of the C++ enum.
       end_marker: A regular expression that matches the end of the C++ enum.
       strip_k_prefix: Set to True if enum values are declared as kFoo and the
           'k' should be stripped.
+
+  Returns:
+      A string with presubmit failure description, or None (of no failures).

[Ilya Sherman, 2017/04/27 22:28:09]

nit: s/of/if

[Łukasz Anforowicz, 2017/04/27 22:35:41]

Done.

   """
   Log('Reading histogram enum definition from "{0}".'.format(source_enum_path))
   source_enum_values, duplicated_values = ReadHistogramValues(
       source_enum_path, start_marker, end_marker, strip_k_prefix)
+
   if duplicated_values:
-    return False, duplicated_values
+    return ('%s enum has been updated and there exists '

[Ilya Sherman, 2017/04/27 22:28:09]

nit: s/exists/exist

[Łukasz Anforowicz, 2017/04/27 22:35:41]

Done.

+            'duplicated values between (%s) and (%s)' % (histogram_enum_name,
+                                                         duplicated_values[0],
+                                                         duplicated_values[1]))
 
   (xml, new_xml) = _GetOldAndUpdatedXml(histogram_enum_name, source_enum_values,
                                         source_enum_path)
-  return xml != new_xml, None
+  if xml != new_xml:
+    return ('%s enum has been updated and the UMA mapping needs to be '
+            'regenerated. Please run %s in src/tools/metrics/histograms/ to '
+            'update the mapping.' % (histogram_enum_name, update_script_name))
+
+  return None
 
 
 def UpdateHistogramFromDict(histogram_enum_name, source_enum_values,