Presubmit checks for user actions intorduced in HTML files.

chrome/browser/resources/PRESUBMIT.py

Index: chrome/browser/resources/PRESUBMIT.py
diff --git a/chrome/browser/resources/PRESUBMIT.py b/chrome/browser/resources/PRESUBMIT.py
new file mode 100644
index 0000000000000000000000000000000000000000..e7fdaa19d0bb5f5302750c4eeeccb3be39e57954
--- /dev/null
+++ b/chrome/browser/resources/PRESUBMIT.py
@@ -0,0 +1,53 @@
+# Copyright (c) 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.
+"""Presubmit script for HTML files in .

[yao, 2014/11/12 22:23:21]

Finish the sentence plz :) 

[gayane -on leave until 09-2017, 2014/11/15 00:09:09]

Done.

+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details about the presubmit API built into gcl.
+"""
+action_xml_path = '../../../tools/metrics/actions/actions.xml'
+
+def CheckUserActionUpdate(input_api, output_api):
+  """Checks if any new user action has been added."""
+  if any('actions.xml' == input_api.os_path.basename(f.LocalPath()) for f in
+         input_api.AffectedFiles()):
+    # If actions.xml is already included in the changelist, the PRESUBMIT
+    # for actions.xml will do a more complete presubmit check.
+    return []
+
+  file_filter = lambda f: f.LocalPath().endswith(('.html'))
+  action_re = r'(^|\s+)metric\s*=\s*"([^ ]*)"'
+  current_actions = None
+  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:
+        # Loads contents in tools/metrics/actions/actions.xml to memory. It's
+        # loaded only once.
+        if not current_actions:
+          with open(action_xml_path) as actions_f:
+            current_actions = actions_f.read()
+
+        # Search for the matched user action name in |current_actions|.
+        action_name = match.group(2)
+        action = 'name="{0}"'.format(action_name)
+        action_disabled = 'name="{0}_disabled"'.format(action_name)
+        action_enabled = 'name="{0}_enabled"'.format(action_name)

[Alexei Svitkine (slow), 2014/11/12 22:27:00]

From extract_actions.py, it looks like the actual suffixes we want here are
'_Enable' and '_Disable'.

[gayane -on leave until 09-2017, 2014/11/15 00:09:09]

Done.

+
+        if action not in current_actions and\
+            (action_enabled not in current_actions or\
+            action_disabled not in current_actions):

[yao, 2014/11/12 22:23:21]

I wonder how fast this is. Could you test? 

I remember they don't like it if we make presubmit a lot slower. 

[Alexei Svitkine (slow), 2014/11/12 22:27:00]

I think it would be cleaner to make a helper function for this, e.g.

if not IsActionPresent(current_actions, action_name):
  return [output_api....]

Then, in IsActionPresent, you can do the name formatting and this logic.

[gayane -on leave until 09-2017, 2014/11/15 00:09:09]

Done.

[gayane -on leave until 09-2017, 2014/11/15 00:09:09]

I tried to make it a little bit better. Now it will check whether the action
should be boolean or not (which is necessary for correct check anyway). If it is
not boolean then its just one pass over the
actions.xml. If it is boolean then it first will search for
{action_name}_Disabled and if found will search for {action_name}_Enabled
continueing from the found place. Overall there will be one pass over
actions.xml +
few regex on the new_content to find out if it is boolean or not.

I assume new content which has action defined wouldn't be usually that long.

+          return [output_api.PresubmitPromptWarning(
+            'File %s line %d: %s is missing in '
+            'tools/metrics/actions/actions.xml. Please run '
+            'tools/metrics/actions/extract_actions.py to update.'
+            % (f.LocalPath(), line_num, action_name), [])]
+  return []
+

[Alexei Svitkine (slow), 2014/11/12 22:27:00]

Nit: Add an extra line.

[gayane -on leave until 09-2017, 2014/11/15 00:09:09]

Done.

+def CheckChangeOnUpload(input_api, output_api):
+  return CheckUserActionUpdate(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return CheckUserActionUpdate(input_api, output_api)