Change actions.txt to actions.xml

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 1e6526f26ea9c9dca0f46179f948e5a14216afed..c771f522321a148fd2e1d1add8d3cbea7d2a1291 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -11,6 +11,7 @@ for more details about the presubmit API built into gcl.
 
 import re
 import sys
+from xml.dom import minidom
 
 
 _EXCLUDED_PATHS = (
@@ -1014,28 +1015,34 @@ def _CheckCygwinShell(input_api, output_api):
 
 def _CheckUserActionUpdate(input_api, output_api):
   """Checks if any new user action has been added."""
-  if any('chromeactions.txt' == input_api.os_path.basename(f) for f in
+  if any('actions.xml' == input_api.os_path.basename(f) for f in
          input_api.LocalPaths()):
-    # If chromeactions.txt is already included in the changelist, the PRESUBMIT
-    # for chromeactions.txt will do a more complete presubmit check.
+    # If actions.xml is already included in the changelist, the PRESUBMIT
+    # for actions.xml will do a more complete presubmit check.
     return []
 
-  with open('tools/metrics/actions/chromeactions.txt') as f:
-    current_actions = f.read()
-
+  current_actions = []
   file_filter = lambda f: f.LocalPath().endswith(('.cc', '.mm'))
   action_re = r'[^a-zA-Z]UserMetricsAction\("([^"]*)'
   for f in input_api.AffectedFiles(file_filter=file_filter):
     for line_num, line in f.ChangedContents():
       match = input_api.re.search(action_re, line)
       if match:
+        # Get all current user actions.
+        if not current_actions:
+          try:
+            current_actions = [a.getAttribute('name') for a in
+                minidom.parse('tools/metrics/actions/actions.xml').

[Jói, 2014/03/05 09:09:35]

I am concerned about the performance of parsing an XML file every time presubmit
checks are run on a change with files that contain actions.

An alternative would be to search for name="[action name]" in the file, which
would only require a line-by-line scan or just a slurp into memory and a
string.contains().

Could you do a quick measurement where you hack PRESUBMIT.py to run
_CheckUserActionUpdate 100 times instead of once, and logs the total time it
took to run, on a change that includes some new actions?

[yiyaoliu, 2014/03/05 15:07:07]

With 1 or 5 user actions code added, the presubmit takes around 14s for both
when _CheckUserActionUpdate runs for 100 times

+                    getElementsByTagName('action')]
+          except:
+            return [output_api.PresubmitPromptWarning(
+                'Error parsing tools/metrics/actions/actions.xml.')]
         for action_name in match.groups():
-          name_pattern = r'\t%s\n' % action_name
-          if name_pattern not in current_actions:
+          if action_name not in current_actions:
             return [output_api.PresubmitPromptWarning(
               'File %s line %d: %s is missing in '
-              'tools/metrics/actions/chromeactions.txt. Please run '
-              'tools/metrics/actions/extract_actions.py --hash to update.'
+              'tools/metrics/actions/actions.xml. Please run '
+              'tools/metrics/actions/extract_actions.py to update.'
               % (f.LocalPath(), line_num, action_name))]
   return []