Add presubmit check and automatic update script for ExtensionPermission enum.

tools/metrics/histograms/update_histogram_enum.py

-# Copyright 2013 The Chromium Authors. All rights reserved.
+# Copyright 2014 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.
 
-"""Updates ExtensionFunctions enum in histograms.xml file with values read from
-extension_function_histogram_value.h.
-
-If the file was pretty-printed, the updated version is pretty-printed too.
-"""

[Ilya Sherman, 2014/03/17 22:21:10]

Docs for this file would still be useful.

-
 import logging
-import os
+import print_style
 import re
 import sys
 
 from xml.dom import minidom
-import print_style
-
-# Import the metrics/common module.
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
-from diff_util import PromptUserToAcceptDiff
-
-HISTOGRAMS_PATH = 'histograms.xml'
-ENUM_NAME = 'ExtensionFunctions'
-
-EXTENSION_FUNCTIONS_HISTOGRAM_VALUE_PATH = \
-  '../../../extensions/browser/extension_function_histogram_value.h'
-ENUM_START_MARKER = "^enum HistogramValue {"
-ENUM_END_MARKER = "^ENUM_BOUNDARY"
-
+from diffutil import PromptUserToAcceptDiff
 
 class UserError(Exception):
   def __init__(self, message):
     Exception.__init__(self, message)
 
   @property
   def message(self):
     return self.args[0]
 
-def ExtractRegexGroup(line, regex):
-    m = re.match(regex, line)
-    if m:
-      return m.group(1)
-    else:
-      return None
+
+def Log(message):
+  logging.info(message)
 
 
-def ReadHistogramValues(filename):
-  """Returns a list of pairs (label, value) corresponding to HistogramValue.
+def ExtractRegexGroup(line, regex):
+  m = re.match(regex, line)
+  if m:
+    return m.group(1)
+  else:
+    return None
 
-  Reads the extension_function_histogram_value.h file, locates the
-  HistogramValue enum definition and returns a pair for each entry.
+
+def ReadHistogramValues(filename, start_marker, end_marker):
+  """Reads in values from |filename|, returning a list of (label, value) pairs
+  corresponding to the enum framed by |start_marker| and |end_marker|.
   """
-
   # Read the file as a list of lines
   with open(filename) as f:
     content = f.readlines()
 
   # Locate the enum definition and collect all entries in it
   inside_enum = False # We haven't found the enum definition yet
   result = []
   for line in content:
     line = line.strip()
     if inside_enum:
       # Exit condition: we reached last enum value
-      if re.match(ENUM_END_MARKER, line):
+      if re.match(end_marker, line):
         inside_enum = False
       else:
         # Inside enum: generate new xml entry
         label = ExtractRegexGroup(line.strip(), "^([\w]+)")
         if label:
           result.append((label, enum_value))
           enum_value += 1
     else:
-      if re.match(ENUM_START_MARKER, line):
+      if re.match(start_marker, line):
         inside_enum = True
         enum_value = 0 # Start at 'UNKNOWN'
   return result
 
 
-def UpdateHistogramDefinitions(histogram_values, document):
-  """Sets the children of <enum name="ExtensionFunctions" ...> node in
-  |document| to values generated from policy ids contained in
-  |policy_templates|.
+def UpdateHistogramDefinitions(histogram_enum_name, source_enum_values,
+                               source_enum_path, document,
+                               labelTransformer):
+  """Sets the children of <enum name=|histogram_enum_name| ...> node in
+  |document| to values generated from (label, value) pairs contained in
+  |source_enum_values|.
 
-  Args:
-    histogram_values: A list of pairs (label, value) defining each extension
-                      function
-    document: A minidom.Document object representing parsed histogram
-              definitions XML file.
-
+  |labelTransformer| is a function used to transform labels from the
+  (label, value) pairs into a different format or style. Since label is
+  a string, defaulting labelTransformer to str() effectively does nothing.

[Ilya Sherman, 2014/03/17 22:21:10]

Is the labelTransformer parameter still needed?

   """
   # Find ExtensionFunctions enum.
   for enum_node in document.getElementsByTagName('enum'):
-    if enum_node.attributes['name'].value == ENUM_NAME:
-        extension_functions_enum_node = enum_node
-        break
+    if enum_node.attributes['name'].value == histogram_enum_name:
+      histogram_enum_node = enum_node
+      break
   else:
-    raise UserError('No policy enum node found')
+    raise UserError('No {0} enum node found'.format(histogram_enum_name))
 
   # Remove existing values.
-  while extension_functions_enum_node.hasChildNodes():
-    extension_functions_enum_node.removeChild(
-      extension_functions_enum_node.lastChild)
+  while histogram_enum_node.hasChildNodes():
+    histogram_enum_node.removeChild(histogram_enum_node.lastChild)
 
   # Add a "Generated from (...)" comment
-  comment = ' Generated from {0} '.format(
-    EXTENSION_FUNCTIONS_HISTOGRAM_VALUE_PATH)
-  extension_functions_enum_node.appendChild(document.createComment(comment))
+  comment = ' Generated from {0} '.format(source_enum_path)
+  histogram_enum_node.appendChild(document.createComment(comment))
 
   # Add values generated from policy templates.
-  for (label, value) in histogram_values:
+  for (label, value) in source_enum_values:
     node = document.createElement('int')
     node.attributes['value'] = str(value)
-    node.attributes['label'] = label
-    extension_functions_enum_node.appendChild(node)
+    node.attributes['label'] = labelTransformer(label)
+    histogram_enum_node.appendChild(node)
 
-def Log(message):
-  logging.info(message)
 
-def main():
+def UpdateHistogramEnum(histogram_enum_name, source_enum_path,
+                        start_marker, end_marker, labelTransformer=str):
+  """Updates |histogram_enum_name| enum in histograms.xml file with values
+  read from |source_enum_path|, where |start_marker| and |end_marker| indicate
+  the beginning and end of the source enum definition, respectively.
+
+  |labelTransformer| is a function whose usage is described in
+  UpdateHistogramDefinitions.

[Ilya Sherman, 2014/03/17 22:21:10]

Ditto.

+  """
+  HISTOGRAMS_PATH = 'histograms.xml'

[Ilya Sherman, 2014/03/17 22:21:10]

Would be nice to share this logic:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...

If you prefer to defer that to a follow-up CL, please add a TODO here.

+
   if len(sys.argv) > 1:
     print >>sys.stderr, 'No arguments expected!'
     sys.stderr.write(__doc__)
     sys.exit(1)
 
-  Log('Reading histogram enum definition from "%s".'
-      % (EXTENSION_FUNCTIONS_HISTOGRAM_VALUE_PATH))
-  histogram_values = ReadHistogramValues(
-    EXTENSION_FUNCTIONS_HISTOGRAM_VALUE_PATH)
+  Log('Reading histogram enum definition from "{0}".'.format(source_enum_path))
+  source_enum_values = ReadHistogramValues(source_enum_path, start_marker,
+                                           end_marker)
 
-  Log('Reading existing histograms from "%s".' % (HISTOGRAMS_PATH))
+  Log('Reading existing histograms from "{0}".'.format(HISTOGRAMS_PATH))
   with open(HISTOGRAMS_PATH, 'rb') as f:
     histograms_doc = minidom.parse(f)
     f.seek(0)
     xml = f.read()
 
   Log('Comparing histograms enum with new enum definition.')
-  UpdateHistogramDefinitions(histogram_values, histograms_doc)
+  UpdateHistogramDefinitions(histogram_enum_name, source_enum_values,
+      source_enum_path, histograms_doc, labelTransformer)

[Ilya Sherman, 2014/03/17 22:21:10]

nit: Please align the arguments, so either indent this line to the paren, or
wrap all the args to be indented by four spaces.

 
   Log('Writing out new histograms file.')
   new_xml = print_style.GetPrintStyle().PrettyPrintNode(histograms_doc)
-
   if PromptUserToAcceptDiff(xml, new_xml, 'Is the updated version acceptable?'):
     with open(HISTOGRAMS_PATH, 'wb') as f:
       f.write(new_xml)
 
   Log('Done.')
-
-
-if __name__ == '__main__':
-  main()