Replace aapt(1) with an R.txt generator

We currently use aapt(1) to generate an R.txt file so that we can compile our
resources and java targets concurrently. This cl replaces aapt with a custom
res/ parser that extracts resource java constant names to output a dummy R.txt
file. This new implementation has a significant speedup over aapt (60% on
average).

Bug: 820460
Change-Id: I7c912390a7789385a84a4102aee6bdb70011b248
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015609Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736873}

build/android/gyp/prepare_resources.py

@@ -19,16 +19,9 @@ from util import build_utils
 from util import jar_info_utils
 from util import manifest_utils
 from util import md5_check
+from util import resources_parser
 from util import resource_utils
 
-_AAPT_IGNORE_PATTERN = ':'.join([
-    '*OWNERS',  # Allow OWNERS files within res/
-    '*.py',  # PRESUBMIT.py sometimes exist.
-    '*.pyc',
-    '*~',  # Some editors create these as temp files.
-    '.*',  # Never makes sense to include dot(files/dirs).
-    '*.d.stamp', # Ignore stamp files
-    ])
 
 def _ParseArgs(args):
   """Parses command line options.
@@ -38,9 +31,6 @@ def _ParseArgs(args):
   """
   parser, input_opts, output_opts = resource_utils.ResourceArgsParser()
 
-  input_opts.add_argument(
-      '--aapt-path', required=True, help='Path to the Android aapt tool')
-
   input_opts.add_argument(
       '--res-sources-path',
       required=True,
@@ -150,32 +140,9 @@ def _GenerateRTxt(options, dep_subdirs, gen_dir):
     gen_dir: Locates where the aapt-generated files will go. In particular
       the output file is always generated as |{gen_dir}/R.txt|.
   """
-  # NOTE: This uses aapt rather than aapt2 because 'aapt2 compile' does not
-  # support the --output-text-symbols option yet (https://crbug.com/820460).
-  package_command = [
-      options.aapt_path,
-      'package',
-      '-m',
-      '-M',
-      manifest_utils.EMPTY_ANDROID_MANIFEST_PATH,
-      '--no-crunch',
-      '--auto-add-overlay',
-      '--no-version-vectors',
-  ]
-  for j in options.include_resources:
-    package_command += ['-I', j]
-
   ignore_pattern = resource_utils.AAPT_IGNORE_PATTERN
   if options.strip_drawables:
     ignore_pattern += ':*drawable*'
-  package_command += [
-      '--output-text-symbols',
-      gen_dir,
-      '-J',
-      gen_dir,  # Required for R.txt generation.
-      '--ignore-assets',
-      ignore_pattern
-  ]
 
   # Adding all dependencies as sources is necessary for @type/foo references
   # to symbols within dependencies to resolve. However, it has the side-effect
@@ -183,15 +150,10 @@ def _GenerateRTxt(options, dep_subdirs, gen_dir):
   # E.g.: It enables an arguably incorrect usage of
   # "mypackage.R.id.lib_symbol" where "libpackage.R.id.lib_symbol" would be
   # more correct. This is just how Android works.
-  for d in dep_subdirs:
-    package_command += ['-S', d]
-
-  for d in options.resource_dirs:
-    package_command += ['-S', d]
+  resource_dirs = dep_subdirs + options.resource_dirs
 
-  # Only creates an R.txt
-  build_utils.CheckOutput(
-      package_command, print_stdout=False, print_stderr=False)
+  resources_parser.RTxtGenerator(resource_dirs, ignore_pattern).WriteRTxtFile(
+      os.path.join(gen_dir, 'R.txt'))
 
 
 def _OnStaleMd5(options):
@@ -209,10 +171,6 @@ def _OnStaleMd5(options):
       _GenerateRTxt(options, dep_subdirs, build.gen_dir)
       r_txt_path = build.r_txt_path
 
-      # 'aapt' doesn't generate any R.txt file if res/ was empty.
-      if not os.path.exists(r_txt_path):
-        build_utils.Touch(r_txt_path)
-
     if options.r_text_out:
       shutil.copyfile(r_txt_path, options.r_text_out)
 
@@ -273,7 +231,6 @@ def main(args):
   ]
 
   possible_input_paths = [
-    options.aapt_path,
     options.android_manifest,
   ]
   possible_input_paths += options.include_resources

build/android/gyp/prepare_resources.pydeps

@@ -29,3 +29,4 @@ util/jar_info_utils.py
 util/manifest_utils.py
 util/md5_check.py
 util/resource_utils.py
+util/resources_parser.py

build/android/gyp/util/resources_parser.py

+# Copyright 2020 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 collections
+import os
+import re
+from xml.etree import ElementTree
+
+from util import build_utils
+from util import resource_utils
+
+_TextSymbolEntry = collections.namedtuple(
+    'RTextEntry', ('java_type', 'resource_type', 'name', 'value'))
+
+_DUMMY_RTXT_ID = '0x7f010001'
+_DUMMY_RTXT_INDEX = '1'
+
+
+def _ResourceNameToJavaSymbol(resource_name):
+  return re.sub('[\.:]', '_', resource_name)
+
+
+class RTxtGenerator(object):
+  def __init__(self,
+               res_dirs,
+               ignore_pattern=resource_utils.AAPT_IGNORE_PATTERN):
+    self.res_dirs = res_dirs
+    self.ignore_pattern = ignore_pattern
+
+  def _ParseDeclareStyleable(self, node):
+    ret = set()
+    stylable_name = _ResourceNameToJavaSymbol(node.attrib['name'])
+    ret.add(
+        _TextSymbolEntry('int[]', 'styleable', stylable_name,
+                         '{{{}}}'.format(_DUMMY_RTXT_ID)))
+    for child in node:
+      if child.tag == 'eat-comment':
+        continue
+      if child.tag != 'attr':
+        # This parser expects everything inside <declare-stylable/> to be either
+        # an attr or an eat-comment. If new resource xml files are added that do
+        # not conform to this, this parser needs updating.
+        raise Exception('Unexpected tag {} inside <delcare-stylable/>'.format(
+            child.tag))
+      entry_name = '{}_{}'.format(
+          stylable_name, _ResourceNameToJavaSymbol(child.attrib['name']))
+      ret.add(
+          _TextSymbolEntry('int', 'styleable', entry_name, _DUMMY_RTXT_INDEX))
+      if not child.attrib['name'].startswith('android:'):
+        resource_name = _ResourceNameToJavaSymbol(child.attrib['name'])
+        ret.add(_TextSymbolEntry('int', 'attr', resource_name, _DUMMY_RTXT_ID))
+      for entry in child:
+        if entry.tag not in ('enum', 'flag'):
+          # This parser expects everything inside <attr/> to be either an
+          # <enum/> or an <flag/>. If new resource xml files are added that do
+          # not conform to this, this parser needs updating.
+          raise Exception('Unexpected tag {} inside <attr/>'.format(entry.tag))
+        resource_name = _ResourceNameToJavaSymbol(entry.attrib['name'])
+        ret.add(_TextSymbolEntry('int', 'id', resource_name, _DUMMY_RTXT_ID))
+    return ret
+
+  def _ExtractNewIdsFromNode(self, node):
+    ret = set()
+    # Sometimes there are @+id/ in random attributes (not just in android:id)
+    # and apparently that is valid. See:
+    # https://developer.android.com/reference/android/widget/RelativeLayout.LayoutParams.html
+    for value in node.attrib.values():
+      if value.startswith('@+id/'):
+        resource_name = value[5:]
+        ret.add(_TextSymbolEntry('int', 'id', resource_name, _DUMMY_RTXT_ID))
+    for child in node:
+      ret.update(self._ExtractNewIdsFromNode(child))
+    return ret
+
+  def _ExtractNewIdsFromXml(self, xml_path):
+    root = ElementTree.parse(xml_path).getroot()
+    return self._ExtractNewIdsFromNode(root)
+
+  def _ParseValuesXml(self, xml_path):
+    ret = set()
+    root = ElementTree.parse(xml_path).getroot()
+    assert root.tag == 'resources'
+    for child in root:
+      if child.tag == 'eat-comment':
+        # eat-comment is just a dummy documentation element.
+        continue
+      if child.tag == 'declare-styleable':
+        ret.update(self._ParseDeclareStyleable(child))
+      else:
+        if child.tag == 'item':
+          resource_type = child.attrib['type']
+        elif child.tag in ('array', 'integer-array', 'string-array'):
+          resource_type = 'array'
+        else:
+          resource_type = child.tag
+        name = _ResourceNameToJavaSymbol(child.attrib['name'])
+        ret.add(_TextSymbolEntry('int', resource_type, name, _DUMMY_RTXT_ID))
+    return ret
+
+  def _CollectResourcesListFromDirectory(self, res_dir):
+    ret = set()
+    globs = resource_utils._GenerateGlobs(self.ignore_pattern)
+    for root, _, files in os.walk(res_dir):
+      resource_type = os.path.basename(root)
+      if '-' in resource_type:
+        resource_type = resource_type[:resource_type.index('-')]
+      for f in files:
+        if build_utils.MatchesGlob(f, globs):
+          continue
+        if resource_type == 'values':
+          ret.update(self._ParseValuesXml(os.path.join(root, f)))
+        else:
+          if '.' in f:
+            resource_name = f[:f.index('.')]
+          else:
+            resource_name = f
+          ret.add(
+              _TextSymbolEntry('int', resource_type, resource_name,
+                               _DUMMY_RTXT_ID))
+          # Other types not just layouts can contain new ids (eg: Menus and
+          # Drawables). Just in case, look for new ids in all files.
+          if f.endswith('.xml'):
+            ret.update(self._ExtractNewIdsFromXml(os.path.join(root, f)))
+    return ret
+
+  def _CollectResourcesListFromDirectories(self):
+    ret = set()
+    for res_dir in self.res_dirs:
+      ret.update(self._CollectResourcesListFromDirectory(res_dir))
+    return ret
+
+  def WriteRTxtFile(self, rtxt_path):
+    resources = self._CollectResourcesListFromDirectories()
+    with open(rtxt_path, 'w') as f:
+      for resource in resources:
+        line = '{0.java_type} {0.resource_type} {0.name} {0.value}\n'.format(
+            resource)
+        f.write(line)

build/config/android/internal_rules.gni

@@ -829,7 +829,6 @@ template("test_runner_script") {
 
 if (enable_java_templates) {
   android_sdk_jar = "$android_sdk/android.jar"
-  android_default_aapt_path = "$android_sdk_build_tools/aapt"
 
   template("android_lint") {
     _min_sdk_version = 19
@@ -1842,11 +1841,8 @@ if (enable_java_templates) {
         invoker.r_text_out_path,
       ]
 
-      _android_aapt_path = android_default_aapt_path
-
       inputs = [
         invoker.build_config,
-        _android_aapt_path,
         invoker.res_sources_path,
       ]
 
@@ -1858,8 +1854,6 @@ if (enable_java_templates) {
         "--depfile",
         rebase_path(depfile, root_build_dir),
         "--include-resources=@FileArg($_rebased_build_config:android:sdk_jars)",
-        "--aapt-path",
-        rebase_path(_android_aapt_path, root_build_dir),
         "--dependencies-res-zips=@FileArg($_rebased_build_config:deps_info:dependency_zips)",
         "--extra-res-packages=@FileArg($_rebased_build_config:deps_info:extra_package_names)",
         "--extra-r-text-files=@FileArg($_rebased_build_config:deps_info:extra_r_text_files)",