[Snowflake] Move presubmit checks to check XMLs on multiple directories

Extend presubmit checks for Android XMLs to ui/, content/ and
components/.

Bug: 775198
Change-Id: I2828d02687364c7aadaecf8f6fdd3745c2932c9e
Reviewed-on: https://chromium-review.googlesource.com/c/1364227
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615203}

PRESUBMIT.py

@@ -2490,6 +2490,24 @@ def _CheckAndroidWebkitImports(input_api, output_api):
   return results
 
 
+def _CheckAndroidXmlStyle(input_api, output_api, is_check_on_upload):
+  """Checks Android XML styles """
+  import sys
+  original_sys_path = sys.path
+  try:
+    sys.path = sys.path + [input_api.os_path.join(
+        input_api.PresubmitLocalPath(), 'tools', 'android', 'checkxmlstyle')]
+    import checkxmlstyle
+  finally:
+    # Restore sys.path to what it was before.
+    sys.path = original_sys_path
+
+  if is_check_on_upload:
+    return checkxmlstyle.CheckStyleOnUpload(input_api, output_api)
+  else:
+    return checkxmlstyle.CheckStyleOnCommit(input_api, output_api)
+
+
 class PydepsChecker(object):
   def __init__(self, input_api, pydeps_files):
     self._file_cache = {}
@@ -2985,7 +3003,7 @@ def _CheckCorrectProductNameInMessages(input_api, output_api):
 
 
 def _AndroidSpecificOnUploadChecks(input_api, output_api):
-  """Groups checks that target android code."""
+  """Groups upload checks that target android code."""
   results = []
   results.extend(_CheckAndroidCrLogUsage(input_api, output_api))
   results.extend(_CheckAndroidNewMdpiAssetLocation(input_api, output_api))
@@ -2994,6 +3012,13 @@ def _AndroidSpecificOnUploadChecks(input_api, output_api):
   results.extend(_CheckAndroidTestJUnitFrameworkImport(input_api, output_api))
   results.extend(_CheckAndroidTestAnnotationUsage(input_api, output_api))
   results.extend(_CheckAndroidWebkitImports(input_api, output_api))
+  results.extend(_CheckAndroidXmlStyle(input_api, output_api, True))
+  return results
+
+def _AndroidSpecificOnCommitChecks(input_api, output_api):
+  """Groups commit checks that target android code."""
+  results = []
+  results.extend(_CheckAndroidXmlStyle(input_api, output_api, False))
   return results
 
 
@@ -3472,6 +3497,7 @@ def GetTryServerMasterForBot(bot):
 def CheckChangeOnCommit(input_api, output_api):
   results = []
   results.extend(_CommonChecks(input_api, output_api))
+  results.extend(_AndroidSpecificOnCommitChecks(input_api, output_api))
   # Make sure the tree is 'open'.
   results.extend(input_api.canned_checks.CheckTreeIsOpen(
       input_api,

chrome/android/java/res_autofill_assistant/layout/autofill_assistant_sheet.xml

@@ -125,8 +125,7 @@
                         android:layout_marginBottom="2dp"
                         android:layout_marginTop="7dp"
                         android:gravity="bottom"
-                        android:textAppearance="@style/BlackCaptionDefault"
-                        android:textStyle="bold"
+                        android:textAppearance="@style/TextAppearance.DetailsTitle"
                         android:maxLines="1"
                         android:ellipsize="end"/>
                     <TextView

chrome/android/java/res_autofill_assistant/values-v17/styles.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2018 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. -->
+
+<resources>
+    <!-- TODO(crbuc.com/806868): Use a Chrome approved text appearance and remove this. -->
+    <style name="TextAppearance.DetailsTitle" parent="BlackCaptionDefault">
+        <item name="android:textStyle">bold</item>
+    </style>
+</resources>

tools/android/checkxmlstyle/PRESUBMIT.py

@@ -0,0 +1,24 @@
+# Copyright (c) 2018 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.
+
+"""Top-level presubmit script for checkxmlstyle.
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details on the presubmit API built into depot_tools.
+"""
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return _CommonChecks(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return _CommonChecks(input_api, output_api)
+
+
+def _CommonChecks(input_api, output_api):
+  result = []
+  result.extend(input_api.canned_checks.RunUnitTests(
+      input_api, output_api, ['./checkxmlstyle_test.py']))
+  return result

chrome/android/java/res/PRESUBMIT.py → tools/android/checkxmlstyle/checkxmlstyle.py

@@ -1,21 +1,20 @@
-# Copyright (c) 2017 The Chromium Authors. All rights reserved.
+# Copyright 2018 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 Android xml code.
+"""Script that is used by PRESUBMIT.py to check Android XML files.
 
-See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
-for more details about the presubmit API built into depot_tools.
-
-This presubmit checks for the following:
+This file checks for the following:
   - Colors are defined as RRGGBB or AARRGGBB
   - No (A)RGB values are referenced outside colors.xml
   - No duplicate (A)RGB values are referenced in colors.xml
   - XML namspace "app" is used for "http://schemas.android.com/apk/res-auto"
   - Android text attributes are only defined in text appearance styles
+  - Warning on adding new text appearance styles
 """
 
 from collections import defaultdict
+import os
 import re
 import xml.etree.ElementTree as ET
 
@@ -25,18 +24,29 @@
 XML_APP_NAMESPACE_PATTERN = re.compile(
     r'xmlns:(\w+)="http://schemas.android.com/apk/res-auto"')
 TEXT_APPEARANCE_STYLE_PATTERN = re.compile(r'^TextAppearance\.')
+INCLUDED_PATHS = [
+    r'^(chrome|ui|components|content)[\\/](.*[\\/])?java[\\/]res.+\.xml$'
+]
 
 
-def CheckChangeOnUpload(input_api, output_api):
+def CheckStyleOnUpload(input_api, output_api):
+  """Returns result for all the presubmit upload checks for XML files."""
   result = _CommonChecks(input_api, output_api)
   result.extend(_CheckNewTextAppearance(input_api, output_api))
   return result
 
 
-def CheckChangeOnCommit(input_api, output_api):
+def CheckStyleOnCommit(input_api, output_api):
+  """Returns result for all the presubmit commit checks for XML files."""
   return _CommonChecks(input_api, output_api)
 
 
+def IncludedFiles(input_api):
+  # Filter out XML files outside included paths and files that were deleted.
+  files = lambda f: input_api.FilterSourceFile(f, white_list=INCLUDED_PATHS)
+  return input_api.AffectedFiles(include_deletes=False, file_filter=files)
+
+
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   result = []
@@ -52,9 +62,7 @@ def _CommonChecks(input_api, output_api):
 def _CheckColorFormat(input_api, output_api):
   """Checks color (A)RGB values are of format either RRGGBB or AARRGGBB."""
   errors = []
-  for f in input_api.AffectedFiles(include_deletes=False):
-    if not f.LocalPath().endswith('.xml'):
-      continue
+  for f in IncludedFiles(input_api):
     # Ingnore vector drawable xmls
     contents = input_api.ReadFile(f)
     if '<vector' in contents:
@@ -84,9 +92,8 @@ def _CheckColorFormat(input_api, output_api):
 def _CheckColorReferences(input_api, output_api):
   """Checks no (A)RGB values are defined outside colors.xml."""
   errors = []
-  for f in input_api.AffectedFiles(include_deletes=False):
-    if (not f.LocalPath().endswith('.xml') or
-        f.LocalPath().endswith('/colors.xml')):
+  for f in IncludedFiles(input_api):
+    if f.LocalPath().endswith('/colors.xml'):
       continue
     # Ingnore vector drawable xmls
     contents = input_api.ReadFile(f)
@@ -115,7 +122,7 @@ def _CheckColorReferences(input_api, output_api):
 def _CheckDuplicateColors(input_api, output_api):
   """Checks colors defined by (A)RGB values in colors.xml are unique."""
   errors = []
-  for f in input_api.AffectedFiles(include_deletes=False):
+  for f in IncludedFiles(input_api):
     if not f.LocalPath().endswith('/colors.xml'):
       continue
     colors = defaultdict(int)
@@ -152,9 +159,7 @@ def _CheckDuplicateColors(input_api, output_api):
 def _CheckXmlNamespacePrefixes(input_api, output_api):
   """Checks consistency of prefixes used for XML namespace names."""
   errors = []
-  for f in input_api.AffectedFiles(include_deletes=False):
-    if not f.LocalPath().endswith('.xml'):
-      continue
+  for f in IncludedFiles(input_api):
     for line_number, line in f.ChangedContents():
       xml_app_namespace = XML_APP_NAMESPACE_PATTERN.search(line)
       if xml_app_namespace and not xml_app_namespace.group(1) == 'app':
@@ -183,9 +188,7 @@ def _CheckTextAppearance(input_api, output_api):
       'android:fontFamily', 'android:textAllCaps']
   namespace = {'android': 'http://schemas.android.com/apk/res/android'}
   errors = []
-  for f in input_api.AffectedFiles(include_deletes=False):
-    if not f.LocalPath().endswith('.xml'):
-      continue
+  for f in IncludedFiles(input_api):
     root = ET.fromstring(input_api.ReadFile(f))
     # Check if there are text attributes defined outside text appearances.
     for attribute in text_attributes:
@@ -256,9 +259,7 @@ def _CheckTextAppearance(input_api, output_api):
 def _CheckNewTextAppearance(input_api, output_api):
   """Checks whether a new text appearance style is defined."""
   errors = []
-  for f in input_api.AffectedFiles(include_deletes=False):
-    if not f.LocalPath().endswith('.xml'):
-      continue
+  for f in IncludedFiles(input_api):
     for line_number, line in f.ChangedContents():
       if '<style name="TextAppearance.' in line:
         errors.append(
@@ -276,4 +277,4 @@ def _CheckNewTextAppearance(input_api, output_api):
     See https://crbug.com/775198 for more information.
   ''',
         errors)]
-  return []
+  return []