Clean up histogram'd media enum max values.

media/PRESUBMIT.py

Index: media/PRESUBMIT.py
diff --git a/media/PRESUBMIT.py b/media/PRESUBMIT.py
index 255769720f4f86dd98f64aa352c14a4ba27d5420..d702df0b37c3f6396493864b2a679bee9016cd42 100644
--- a/media/PRESUBMIT.py
+++ b/media/PRESUBMIT.py
@@ -86,11 +86,81 @@ def _CheckForMessageLoopProxy(input_api, output_api):
 
   return []
 
+def _CheckForHistogramOffByOne(input_api, output_api):
+  """Make sure histogram enum maxes are used properly"""
+
+  # A general-purpose chunk of regex to match whitespace and/or comments
+  # that may be interspersed with the code we're interested in:
+  comment = r'/\*.*?\*/|//[^\n]*'
+  whitespace = r'(?:[\n\t ]|(?:' + comment + '))*'
+
+  # The name is assumed to be a literal string.
+  histogram_name = r'"[^"]*"'
+
+  # This can be an arbitrary expression, so just ensure it isn't a ; to prevent
+  # matching past the end of this statement.
+  histogram_value = r'[^;]*'
+
+  # In parens so we can retrieve it for further checks.
+  histogram_max = r'([^;,]*)'
+
+  # This should match a uma histogram enumeration macro expression.
+  uma_macro_re = input_api.re.compile(
+      r'\bUMA_HISTOGRAM_ENUMERATION\(' + whitespace + histogram_name + r',' +

[Ami GONE FROM CHROMIUM, 2014/02/04 02:56:46]

Sadly this looks right ;)

(please make sure that every file under media/ obeys this expectation)

+      whitespace + histogram_value + r',' + whitespace + histogram_max +
+      whitespace + r'\)' + whitespace + ';')
+
+  uma_max_re = input_api.re.compile(r'.*(?:Max|MAX).* \+ 1')
+
+  problems = []
+
+  for f in input_api.AffectedSourceFiles(_FilterFile):
+    contents = input_api.ReadFile(f)
+
+    # We want to match across lines, but still report a line number, so we keep
+    # track of the line we're on as we search through the file.
+    line_number = 1
+
+    # We search the entire file, then check if any violations are in the changed
+    # areas, this is inefficient, but simple. A UMA_HISTOGRAM_ENUMERATION call
+    # will often span multiple lines, so finding a match looking just at the
+    # deltas line-by-line won't catch problems.
+    match = uma_macro_re.search(contents)
+    while match:
+      line_number += contents.count('\n', 0, match.start())
+      max_arg = match.group(1) # The third argument.
+
+      if not uma_max_re.match(max_arg):
+        uma_range = range(match.start(), match.end() + 1)
+        # Check if any part of the match is in the changed lines:
+        for num, line in f.ChangedContents():
+          if line_number <= num <= line_number + match.group().count('\n'):
+            problems.append('%s:%d' % (f, line_number))
+            break
+
+      # Strip off the file contents up to the end of the match and update the
+      # line number.
+      contents = contents[match.end():]
+      line_number += match.group().count('\n')
+      match = uma_macro_re.search(contents)
+
+  if problems:
+    return [output_api.PresubmitError(
+      'UMA_HISTOGRAM_ENUMERATION reports in src/media/ are expected to adhere\n'
+      'to the following guidelines:\n'
+      ' - The max value (3rd argument) should be an enum value equal to the\n'
+      '   last valid value, e.g. FOO_MAX = LAST_VALID_FOO.\n'
+      ' - 1 must be added to that max value.\n'
+      'Contact rileya@chromium.org if you have questions.' , problems)]
+
+  return []
+
 
 def _CheckChange(input_api, output_api):
   results = []
   results.extend(_CheckForUseOfWrongClock(input_api, output_api))
   results.extend(_CheckForMessageLoopProxy(input_api, output_api))
+  results.extend(_CheckForHistogramOffByOne(input_api, output_api))
   return results