Presubmit checks for user actions intorduced in HTML files.

chrome/browser/resources/PRESUBMIT.py

+# Copyright (c) 2014 The Chromium Authors. All rights reserved.

[Alexei Svitkine (slow), 2014/11/15 00:15:47]

Nit: In new files, we don't include the (c) - please remove.

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

Done.

+# 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 chrome/browser/resources.
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details about the presubmit API built into gcl.
+"""
+
+import re
+
+ACTION_XML_PATH = '../../../tools/metrics/actions/actions.xml'
+
+def CheckUserActionUpdate(input_api, output_api, action_xml_path):
+  """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()
+        action_name = match.group(2)
+
+        # Search for the matched user action name in |current_actions|.
+        if not IsActionPresent(current_actions, action_name,
+            IsBoolean(f.NewContents(), action_name)):

[Alexei Svitkine (slow), 2014/11/15 00:15:47]

Nit: If you make is_boolean a local var above, does this if fit into one line?
If so, I'd prefer that.

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

Done.

+          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 []
+
+
+def IsActionPresent(current_actions, action_name, is_boolean):

[Alexei Svitkine (slow), 2014/11/15 00:15:47]

Please add some short comments above these methods that explain the params.

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

Done.

+  if not is_boolean:
+    action = 'name="{0}"'.format(action_name)
+    return action in current_actions
+  else:

[Alexei Svitkine (slow), 2014/11/15 00:15:47]

Nit: if there's an early return, remove the else and de-indent the remainder.

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

Done.

+    action_disabled = 'name="{0}_Disabled"'.format(action_name)
+    action_enabled = 'name="{0}_Enabled"'.format(action_name)
+
+    pos = current_actions.find(action_disabled)
+    return pos >= 0 and current_actions.find(action_enabled, pos) >= 0
+
+
+def IsBoolean(new_content_lines, action_name):
+  new_content = '\n'.join(new_content_lines)
+
+  html_element_re = r'<(.*?)(^|\s+)metric\s*=\s*"%s"(.*?)>' % (action_name)
+  type_re = r'datatype="boolean"|type="checkbox"|type="radio".*?'\
+             'value=("true"|"false")'
+
+  match = re.search(html_element_re, new_content)
+  if match:
+    left_content = match.group(1)
+    right_content = match.group(3)
+    if re.search(type_re, left_content) or re.search(type_re, right_content):
+      return True
+  return False
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return CheckUserActionUpdate(input_api, output_api, ACTION_XML_PATH)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return CheckUserActionUpdate(input_api, output_api, ACTION_XML_PATH)