[Snowflake] Prevent from adding hex color values outside color_palette.xml

Moved some semantic names in color_palette.xml to semantic_colors.xml.
And added some presubmit checks to prevent from adding more hex argb
values in color resource files except color_palette.xml.

Bug: 775198
Change-Id: Ie27dd8dc0e4fa6506ed192c6f5f9c4b5b5f88566
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896241
Commit-Queue: Lijin Shen <lazzzis@google.com>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712462}

tools/android/checkxmlstyle/checkxmlstyle.py

@@ -6,8 +6,9 @@
 
 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
+  - No (A)RGB values are referenced outside color_palette.xml
+  - No duplicate (A)RGB values are referenced in color_palette.xml
+  - Colors in semantic_colors are only referecing colors in color_palette.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
@@ -18,16 +19,7 @@ import os
 import re
 import xml.etree.ElementTree as ET
 
-COLOR_PATTERN = re.compile(r'(>|")(#[0-9A-Fa-f]+)(<|")')
-VALID_COLOR_PATTERN = re.compile(
-    r'^#([0-9A-F][0-9A-E]|[0-9A-E][0-9A-F])?[0-9A-F]{6}$')
-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$'
-]
-
+import helpers
 
 def CheckStyleOnUpload(input_api, output_api):
   """Returns result for all the presubmit upload checks for XML files."""
@@ -43,7 +35,8 @@ def CheckStyleOnCommit(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)
+  files = lambda f: input_api.FilterSourceFile(
+      f, white_list=helpers.INCLUDED_PATHS)
   return input_api.AffectedFiles(include_deletes=False, file_filter=files)
 
 
@@ -53,12 +46,13 @@ def _CommonChecks(input_api, output_api):
   result.extend(_CheckColorFormat(input_api, output_api))
   result.extend(_CheckColorReferences(input_api, output_api))
   result.extend(_CheckDuplicateColors(input_api, output_api))
+  result.extend(_CheckSemanticColorsReferences(input_api, output_api))
   result.extend(_CheckXmlNamespacePrefixes(input_api, output_api))
   result.extend(_CheckTextAppearance(input_api, output_api))
   # Add more checks here
   return result
 
-
+### color resources below ###
 def _CheckColorFormat(input_api, output_api):
   """Checks color (A)RGB values are of format either RRGGBB or AARRGGBB."""
   errors = []
@@ -68,8 +62,8 @@ def _CheckColorFormat(input_api, output_api):
     if '<vector' in contents:
       continue
     for line_number, line in f.ChangedContents():
-      color = COLOR_PATTERN.search(line)
-      if color and not VALID_COLOR_PATTERN.match(color.group(2)):
+      color = helpers.COLOR_PATTERN.search(line)
+      if color and not helpers.VALID_COLOR_PATTERN.match(color.group(2)):
         errors.append(
             '  %s:%d\n    \t%s' % (f.LocalPath(), line_number, line.strip()))
   if errors:
@@ -90,18 +84,17 @@ def _CheckColorFormat(input_api, output_api):
 
 
 def _CheckColorReferences(input_api, output_api):
-  """Checks no (A)RGB values are defined outside colors.xml."""
+  """Checks no (A)RGB values are defined outside color_palette.xml."""
   errors = []
   warnings = []
   for f in IncludedFiles(input_api):
-    if (f.LocalPath().endswith('/colors.xml') or
-        f.LocalPath().endswith('/color_palette.xml')):
+    if f.LocalPath() == helpers.COLOR_PALETTE_RELATIVE_PATH:
       continue
     # Ignore new references in vector/shape drawable xmls
     contents = input_api.ReadFile(f)
     is_vector_drawable = '<vector' in contents or '<shape' in contents
     for line_number, line in f.ChangedContents():
-      if COLOR_PATTERN.search(line):
+      if helpers.COLOR_PATTERN.search(line):
         issue = '  %s:%d\n    \t%s' % (f.LocalPath(), line_number, line.strip())
         if is_vector_drawable:
           warnings.append(issue)
@@ -113,7 +106,7 @@ def _CheckColorReferences(input_api, output_api):
   '''
   Android Color Reference Check failed:
     Your new code added new color references that are not color resources from
-    chrome/android/java/res/values/colors.xml, listed below.
+    ui/android/java/res/values/color_palette.xml, listed below.
 
     This is banned, please use the existing color resources or create a new
     color resource in colors.xml, and reference the color by @color/....
@@ -126,7 +119,7 @@ def _CheckColorReferences(input_api, output_api):
   '''
   Android Color Reference Check warning:
     Your new code added new color references that are not color resources from
-    chrome/android/java/res/values/colors.xml, listed below.
+    ui/android/java/res/values/color_palette.xml, listed below.
 
     This is typically not needed even in vector/shape drawables. Please consider
     using an existing color resources if possible.
@@ -142,23 +135,22 @@ def _CheckColorReferences(input_api, output_api):
 
 
 def _CheckDuplicateColors(input_api, output_api):
-  """Checks colors defined by (A)RGB values in colors.xml are unique."""
+  """Checks colors defined by (A)RGB values in color_palette.xml are unique."""
   errors = []
   for f in IncludedFiles(input_api):
-    if not (f.LocalPath().endswith('/colors.xml')
-            or f.LocalPath().endswith('/color_palette.xml')):
+    if f.LocalPath() != helpers.COLOR_PALETTE_RELATIVE_PATH:
       continue
     colors = defaultdict(int)
     contents = input_api.ReadFile(f)
     # Get count for each color defined.
     for line in contents.splitlines(False):
-      color = COLOR_PATTERN.search(line)
+      color = helpers.COLOR_PATTERN.search(line)
       if color:
         colors[color.group(2)] += 1
 
     # Check duplicates in changed contents.
     for line_number, line in f.ChangedContents():
-      color = COLOR_PATTERN.search(line)
+      color = helpers.COLOR_PATTERN.search(line)
       if color and colors[color.group(2)] > 1:
         errors.append(
             '  %s:%d\n    \t%s' % (f.LocalPath(), line_number, line.strip()))
@@ -167,7 +159,7 @@ def _CheckDuplicateColors(input_api, output_api):
   '''
   Android Duplicate Color Declaration Check failed:
     Your new code added new colors by (A)RGB values that are already defined in
-    chrome/android/java/res/values/colors.xml, listed below.
+    ui/android/java/res/values/color_palette.xml, listed below.
 
     This is banned, please reference the existing color resource from colors.xml
     using @color/... and if needed, give the existing color resource a more
@@ -179,12 +171,50 @@ def _CheckDuplicateColors(input_api, output_api):
   return []
 
 
+def _CheckSemanticColorsReferences(input_api, output_api):
+  """
+  Checks colors defined in semantic_colors.xml only referencing
+  resources in color_palette.xml
+  """
+  color_palette = _colorXml2Dict(
+      input_api.ReadFile(helpers.COLOR_PALETTE_PATH))
+  errors = []
+
+  for f in IncludedFiles(input_api):
+    if not f.LocalPath().endswith('/semantic_colors.xml'):
+      continue
+
+    for line_number, line in f.ChangedContents():
+      r = helpers.COLOR_REFERENCE_PATTERN.search(line)
+      if not r:
+        continue
+      color = r.group()
+      if _removePrefix(color) not in color_palette:
+        errors.append(
+            '  %s:%d\n    \t%s' % (f.LocalPath(), line_number, line.strip()))
+
+  if errors:
+    return [output_api.PresubmitError(
+  '''
+  Android Semantic Color Reference Check failed:
+    Your new color values added in semantic_colors are not defined in
+    ui/android/java/res/values/color_palette.xml, listed below.
+
+    This is banned. Colors in semantic colors can only reference
+    the existing color resource from color_palette.xml.
+
+    See https://crbug.com/775198 for more information.
+  ''',
+        errors)]
+  return []
+
+
 def _CheckXmlNamespacePrefixes(input_api, output_api):
   """Checks consistency of prefixes used for XML namespace names."""
   errors = []
   for f in IncludedFiles(input_api):
     for line_number, line in f.ChangedContents():
-      xml_app_namespace = XML_APP_NAMESPACE_PATTERN.search(line)
+      xml_app_namespace = helpers.XML_APP_NAMESPACE_PATTERN.search(line)
       if xml_app_namespace and not xml_app_namespace.group(1) == 'app':
         errors.append(
             '  %s:%d\n    \t%s' % (f.LocalPath(), line_number, line.strip()))
@@ -204,6 +234,7 @@ def _CheckXmlNamespacePrefixes(input_api, output_api):
   return []
 
 
+### text appearance below ###
 def _CheckTextAppearance(input_api, output_api):
   """Checks text attributes are only used for text appearance styles in XMLs."""
   text_attributes = [
@@ -219,7 +250,7 @@ def _CheckTextAppearance(input_api, output_api):
       invalid_styles = []
       for style in root.findall('style') + root.findall('.//style'):
         name = style.get('name')
-        is_text_appearance = TEXT_APPEARANCE_STYLE_PATTERN.search(name)
+        is_text_appearance = helpers.TEXT_APPEARANCE_STYLE_PATTERN.search(name)
         item = style.find(".//item[@name='"+attribute+"']")
         if is_text_appearance is None and item is not None:
           invalid_styles.append(name)
@@ -301,3 +332,17 @@ def _CheckNewTextAppearance(input_api, output_api):
   ''',
         errors)]
   return []
+
+### helpers ###
+def _colorXml2Dict(content):
+  dct = dict()
+  tree = ET.fromstring(content)
+  for child in tree:
+    dct[child.attrib['name']] = child.text
+  return dct
+
+
+def _removePrefix(color, prefix = '@color/'):
+    if color.startswith(prefix):
+        return color[len(prefix):]
+    return color
\ No newline at end of file

tools/android/checkxmlstyle/checkxmlstyle_test.py

@@ -8,6 +8,7 @@ import sys
 import unittest
 
 import checkxmlstyle
+import helpers
 
 sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(
     os.path.dirname(os.path.abspath(__file__))))))
@@ -127,11 +128,25 @@ class ColorReferencesTest(unittest.TestCase):
   def testValidReferenceInColorResources(self):
     lines = ['<color name="color1">#61000000</color>']
     mock_input_api = MockInputApi()
-    mock_input_api.files = [MockFile('chrome/java/res_test/colors.xml', lines)]
+    mock_input_api.files = [
+        MockFile(helpers.COLOR_PALETTE_RELATIVE_PATH, lines)]
     errors = checkxmlstyle._CheckColorReferences(
         mock_input_api, MockOutputApi())
     self.assertEqual(0, len(errors))
 
+  def testReferenceInSemanticColors(self):
+    mock_input_api = MockInputApi()
+    mock_input_api.files = [
+      MockFile(helpers.COLOR_PALETTE_PATH,
+        ['<resources><color name="a">#f0f0f0</color></resources>']),
+      MockFile('ui/android/java/res/values/semantic_colors.xml',
+        ['<color name="b">@color/hello<color>',
+         '<color name="c">@color/a<color>'])
+    ]
+    errors = checkxmlstyle._CheckSemanticColorsReferences(
+      mock_input_api, MockOutputApi())
+    self.assertEqual(1, len(errors))
+
 
 class DuplicateColorsTest(unittest.TestCase):
 
@@ -139,14 +154,15 @@ class DuplicateColorsTest(unittest.TestCase):
     lines = ['<color name="color1">#61000000</color>',
              '<color name="color2">#61000000</color>']
     mock_input_api = MockInputApi()
-    mock_input_api.files = [MockFile('chrome/java/res_test/colors.xml', lines)]
+    mock_input_api.files = [
+        MockFile(helpers.COLOR_PALETTE_RELATIVE_PATH, lines)]
     errors = checkxmlstyle._CheckDuplicateColors(
         mock_input_api, MockOutputApi())
     self.assertEqual(1, len(errors))
     self.assertEqual(2, len(errors[0].items))
-    self.assertEqual('  chrome/java/res_test/colors.xml:1',
+    self.assertEqual('  %s:1' % helpers.COLOR_PALETTE_RELATIVE_PATH,
                      errors[0].items[0].splitlines()[0])
-    self.assertEqual('  chrome/java/res_test/colors.xml:2',
+    self.assertEqual('  %s:2' % helpers.COLOR_PALETTE_RELATIVE_PATH,
                      errors[0].items[1].splitlines()[0])
 
   def testSucess(self):

tools/android/checkxmlstyle/helpers.py

+# Copyright (c) 2019 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.
+
+import os
+import re
+
+_SRC_ROOT = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), '..', '..', '..'))
+COLOR_PALETTE_RELATIVE_PATH = 'ui/android/java/res/values/color_palette.xml'
+COLOR_PALETTE_PATH = os.path.join(_SRC_ROOT, COLOR_PALETTE_RELATIVE_PATH)
+SEMANTIC_COLORS_PATH = os.path.join(_SRC_ROOT,
+    'ui/android/java/res/values/semantic_colors.xml')
+
+COLOR_PATTERN = re.compile(r'(>|")(#[0-9A-Fa-f]+)(<|")')
+VALID_COLOR_PATTERN = re.compile(
+    r'^#([0-9A-F][0-9A-E]|[0-9A-E][0-9A-F])?[0-9A-F]{6}$')
+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$'
+]
+# TODO(lazzzis): check color references in java source files
+COLOR_REFERENCE_PATTERN = re.compile('''
+    @color/   # starts with '@color'
+    [\w|_]+   # color name is only composed of numbers, letters and underscore
+''', re.VERBOSE)

ui/android/java/res/values/color_palette.xml

 <?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 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 xmlns:tools="http://schemas.android.com/tools">
     <!-- Modern color palette -->
+    <color name="modern_white">@android:color/white</color>
     <color name="modern_blue_300">#8AB4F8</color>
+    <color name="modern_blue_300_alpha_55">#5A7198</color>
     <color name="modern_blue_600">#1A73E8</color>
+    <color name="modern_blue_600_alpha_65">#6AA4F0</color>
     <color name="modern_blue_700">#3367D6</color>
     <color name="modern_blue_800" tools:ignore="UnusedResources">#185ABC</color>
     <color name="modern_grey_50" tools:ignore="UnusedResources">#F8F9FA</color>
@@ -14,7 +22,7 @@
     <color name="modern_grey_300_alpha_38">#61DADCE0</color>
     <color name="modern_grey_400" tools:ignore="UnusedResources">#BDC1C6</color>
     <color name="modern_grey_500" tools:ignore="UnusedResources">#9AA0A6</color>
-    <color name="modern_grey_600" tools:ignore="UnusedResources">#80868B</color>
+    <color name="modern_grey_600">#80868B</color>
     <color name="modern_grey_700">#5F6368</color>
     <color name="modern_grey_800" tools:ignore="UnusedResources">#3C4043</color>
     <color name="modern_grey_800_alpha_38" tools:ignore="UnusedResources">#613C4043</color>
@@ -32,63 +40,12 @@
     <color name="white_alpha_12" tools:ignore="UnusedResources">#1FFFFFFF</color>
     <color name="white_alpha_20" tools:ignore="UnusedResources">#33FFFFFF</color>
     <color name="white_alpha_38" tools:ignore="UnusedResources">#61FFFFFF</color>
-    <color name="white_alpha_50" tools:ignore="UnusedResources">#80FFFFFF</color>
+    <color name="white_alpha_50">#80FFFFFF</color>
     <color name="white_alpha_70">#B3FFFFFF</color>
 
-    <color name="google_red_300" tools:ignore="UnusedResources">#F28B82</color>
-    <color name="google_red_600" tools:ignore="UnusedResources">#D93025</color>
-
-    <color name="google_green_300" tools:ignore="UnusedResources">#81C995</color>
-    <color name="google_green_600" tools:ignore="UnusedResources">#1E8E3E</color>
-
-    <!-- Semantic color references that don't change on night mode. -->
-    <!-- Common icon colors for drawables. -->
-    <color name="default_icon_color_dark">@color/modern_grey_800</color>
-    <color name="default_icon_color_dark_disabled">@color/modern_grey_800_alpha_38</color>
-    <color name="default_icon_color_white" tools:ignore="UnusedResources">
-        @android:color/white
-    </color>
-    <color name="default_icon_color_white_disabled" tools:ignore="UnusedResources">
-        @color/white_alpha_50
-    </color>
-    <color name="default_icon_color_white_pressed" tools:ignore="UnusedResources">
-        @color/white_alpha_50
-    </color>
-
-    <color name="default_icon_color_secondary_dark">@color/modern_grey_600</color>
-    <color name="default_icon_color_secondary_light">@color/white_alpha_70</color>
-
-    <!-- Common text colors -->
-    <color name="default_text_color_dark">@color/modern_grey_900</color>
-    <color name="default_text_color_light">@android:color/white</color>
-    <color name="default_text_color_dark_secondary" tools:ignore="UnusedResources">@color/modern_grey_700</color>
-
-    <color name="highlight_color_on_light_text" tools:ignore="UnusedResources">#5A7198</color>
-    <color name="highlight_color_on_dark_text" tools:ignore="UnusedResources">#6AA4F0</color>
-
-    <color name="default_red_light" tools:ignore="UnusedResources">@color/google_red_300</color>
-    <color name="default_red_dark" tools:ignore="UnusedResources">@color/google_red_600</color>
-
-    <color name="default_green_light" tools:ignore="UnusedResources">@color/google_green_300</color>
-    <color name="default_green_dark" tools:ignore="UnusedResources">@color/google_green_600</color>
+    <color name="google_red_300">#F28B82</color>
+    <color name="google_red_600">#D93025</color>
 
-    <!-- Common background colors. -->
-    <color name="default_bg_color_light">@android:color/white</color>
-    <color name="default_bg_color_dark" tools:ignore="UnusedResources">
-        @color/modern_grey_900
-    </color>
-    <color name="default_bg_color_dark_elev_1" tools:ignore="UnusedResources">
-        @color/modern_grey_900_with_grey_200_alpha_5
-    </color>
-    <color name="default_bg_color_dark_elev_2" tools:ignore="UnusedResources">
-        @color/modern_grey_900_with_grey_200_alpha_8
-    </color>
-    <color name="default_bg_color_dark_elev_3" tools:ignore="UnusedResources">
-        @color/modern_grey_900_with_grey_200_alpha_11
-    </color>
-    <color name="default_bg_color_dark_elev_4" tools:ignore="UnusedResources">
-        @color/modern_grey_900_with_grey_200_alpha_12
-    </color>
-    <color name="divider_bg_color_dark">@color/modern_grey_300</color>
-    <color name="divider_bg_color_light">@color/white_alpha_12</color>
+    <color name="google_green_300">#81C995</color>
+    <color name="google_green_600">#1E8E3E</color>
 </resources>

ui/android/java/res/values/semantic_colors.xml

+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2019 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 xmlns:tools="http://schemas.android.com/tools">
+    <!-- Semantic color references that don't change on night mode. -->
+    <!-- Common icon colors for drawables. -->
+    <color name="default_icon_color_dark">@color/modern_grey_800</color>
+    <color name="default_icon_color_dark_disabled">@color/modern_grey_800_alpha_38</color>
+    <color name="default_icon_color_white" tools:ignore="UnusedResources">
+        @color/modern_white
+    </color>
+    <color name="default_icon_color_white_disabled" tools:ignore="UnusedResources">
+        @color/white_alpha_50
+    </color>
+    <color name="default_icon_color_white_pressed" tools:ignore="UnusedResources">
+        @color/white_alpha_50
+    </color>
+    <color name="default_icon_color_secondary_dark">@color/modern_grey_600</color>
+    <color name="default_icon_color_secondary_light">@color/white_alpha_70</color>
+
+    <!-- Common text colors -->
+    <color name="default_text_color_dark">@color/modern_grey_900</color>
+    <color name="default_text_color_light">@color/modern_white</color>
+    <color name="default_text_color_dark_secondary" tools:ignore="UnusedResources">@color/modern_grey_700</color>
+
+    <color name="highlight_color_on_light_text" tools:ignore="UnusedResources">@color/modern_blue_300_alpha_55</color>
+    <color name="highlight_color_on_dark_text" tools:ignore="UnusedResources">@color/modern_blue_600_alpha_65</color>
+
+    <color name="default_red_light" tools:ignore="UnusedResources">@color/google_red_300</color>
+    <color name="default_red_dark" tools:ignore="UnusedResources">@color/google_red_600</color>
+
+    <color name="default_green_light" tools:ignore="UnusedResources">@color/google_green_300</color>
+    <color name="default_green_dark" tools:ignore="UnusedResources">@color/google_green_600</color>
+
+    <!-- Common background colors. -->
+    <color name="default_bg_color_light">@color/modern_white</color>
+    <color name="default_bg_color_dark">@color/modern_grey_900</color>
+    <color name="default_bg_color_dark_elev_1" tools:ignore="UnusedResources">
+        @color/modern_grey_900_with_grey_200_alpha_5
+    </color>
+    <color name="default_bg_color_dark_elev_2" tools:ignore="UnusedResources">
+        @color/modern_grey_900_with_grey_200_alpha_8
+    </color>
+    <color name="default_bg_color_dark_elev_3" tools:ignore="UnusedResources">
+        @color/modern_grey_900_with_grey_200_alpha_11
+    </color>
+    <color name="default_bg_color_dark_elev_4" tools:ignore="UnusedResources">
+        @color/modern_grey_900_with_grey_200_alpha_12
+    </color>
+    <color name="divider_bg_color_dark">@color/modern_grey_300</color>
+    <color name="divider_bg_color_light">@color/white_alpha_12</color>
+</resources>
\ No newline at end of file