Autogenerate resource package IDs of app bundle modules

This simplifies instantiating new feature modules.

Bug: 950056
Change-Id: Ic06aef2f99d9c08fe23c6df202948f72bba87593
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804275
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697010}

build/android/gyp/compile_resources.py

@@ -50,13 +50,6 @@ _PNG_WEBP_BLACKLIST_PATTERN = re.compile('|'.join([
     r'.*daydream_icon_.*\.png']))
 
 
-def _ListToDictionary(lst, separator):
-  """Splits each element of the passed-in |lst| using |separator| and creates
-  dictionary treating first element of the split as the key and second as the
-  value."""
-  return dict(item.split(separator, 1) for item in lst)
-
-
 def _ParseArgs(args):
   """Parses command line options.
 
@@ -100,17 +93,13 @@ def _ParseArgs(args):
 
   input_opts.add_argument(
       '--package-id',
-      help='Custom package ID for resources (instead of 0x7f). Cannot be used '
-      'with --shared-resources.')
-
-  input_opts.add_argument(
-      '--package-name-to-id-mapping',
-      help='List containing mapping from package name to package IDs that will '
-      'be assigned.')
+      type=int,
+      help='Decimal integer representing custom package ID for resources '
+      '(instead of 127==0x7f). Cannot be used with --shared-resources.')
 
   input_opts.add_argument(
       '--package-name',
-      help='Package name that will be used to determine package ID.')
+      help='Package name that will be used to create R class.')
 
   input_opts.add_argument(
       '--rename-manifest-package', help='Package name to force AAPT to use.')
@@ -266,11 +255,8 @@ def _ParseArgs(args):
     parser.error(
         '--resources-path-map-out-path requires --short-resource-paths')
 
-  if options.package_name_to_id_mapping:
-    package_names_list = build_utils.ParseGnList(
-        options.package_name_to_id_mapping)
-    options.package_name_to_id_mapping = _ListToDictionary(
-        package_names_list, '=')
+  if options.package_id and options.shared_resources:
+    parser.error('--package-id and --shared-resources are mutually exclusive')
 
   return options
 
@@ -423,19 +409,6 @@ def _MoveImagesToNonMdpiFolders(res_root):
   return renamed_paths
 
 
-def _PackageIdFromOptions(options):
-  package_id = None
-  if options.package_id:
-    package_id = options.package_id
-  if options.package_name:
-    package_id = options.package_name_to_id_mapping.get(options.package_name)
-    if package_id is None:
-      raise Exception(
-          'Package name %s is not present in package_name_to_id_mapping.' %
-          options.package_name)
-  return package_id
-
-
 def _FixManifest(options, temp_dir):
   """Fix the APK's AndroidManifest.xml.
 
@@ -751,9 +724,12 @@ def _PackageApk(options, build):
   if options.no_xml_namespaces:
     link_command.append('--no-xml-namespaces')
 
-  package_id = _PackageIdFromOptions(options)
-  if package_id is not None:
-    link_command += ['--package-id', package_id, '--allow-reserved-package-id']
+  if options.package_id:
+    link_command += [
+        '--package-id',
+        hex(options.package_id),
+        '--allow-reserved-package-id',
+    ]
 
   fixed_manifest, desired_manifest_package_name = _FixManifest(
       options, build.temp_dir)
@@ -962,10 +938,12 @@ def main(args):
     build_utils.ZipDir(build.srcjar_path, build.srcjar_dir)
 
     # Sanity check that the created resources have the expected package ID.
-    expected_id = _PackageIdFromOptions(options)
-    if expected_id is None:
-      expected_id = '0x00' if options.shared_resources else '0x7f'
-    expected_id = int(expected_id, 16)
+    if options.package_id:
+      expected_id = options.package_id
+    elif options.shared_resources:
+      expected_id = 0
+    else:
+      expected_id = 127  # == '0x7f'.
     _, package_id = resource_utils.ExtractArscPackage(
         options.aapt2_path,
         build.arsc_path if options.arsc_path else build.proto_path)

build/config/android/internal_rules.gni

@@ -2025,16 +2025,10 @@ if (enable_java_templates) {
   #   post_process_script: (optional)
   #
   #   package_name: (optional)
-  #     Name of the package for the purpose of assigning package ID.
-  #
-  #   package_name_to_id_mapping: (optional)
-  #     List containing mapping from package names to package IDs. It will be
-  #     used to determine which package ID to assign if package_name variable
-  #     was passed in.
+  #     Name of the package for the purpose of creating R class.
   #
   #   package_id: (optional)
-  #     Use a custom package ID in resource IDs (same purpose as
-  #    package_name_to_id_mapping)
+  #     Use a custom package ID in resource IDs.
   #
   #   arsc_package_name: (optional)
   #     Use this package name in the arsc file rather than the package name
@@ -2270,8 +2264,8 @@ if (enable_java_templates) {
       }
       if (defined(invoker.package_name)) {
         args += [
-          "--package-name=${invoker.package_name}",
-          "--package-name-to-id-mapping=${invoker.package_name_to_id_mapping}",
+          "--package-name",
+          invoker.package_name,
         ]
       }
       if (defined(invoker.arsc_package_name)) {

build/config/android/rules.gni

@@ -2457,8 +2457,8 @@ if (enable_java_templates) {
                                "manifest_package",
                                "max_sdk_version",
                                "no_xml_namespaces",
+                               "package_id",
                                "package_name",
-                               "package_name_to_id_mapping",
                                "png_to_webp",
                                "r_java_root_package_name",
                                "resource_blacklist_exceptions",
@@ -3518,8 +3518,8 @@ if (enable_java_templates) {
                                "native_lib_version_rule",
                                "negative_main_dex_globs",
                                "no_xml_namespaces",
+                               "package_id",
                                "package_name",
-                               "package_name_to_id_mapping",
                                "png_to_webp",
                                "product_version_resources_dep",
                                "proguard_configs",

chrome/android/chrome_bundle_tmpl.gni

@@ -16,8 +16,10 @@ import("//chrome/android/modules/chrome_feature_module_tmpl.gni")
 #     a module descriptor.
 template("chrome_bundle") {
   _bundle_target_name = target_name
+  _package_id = 126  # == 0x7e.
   _extra_modules = []
   foreach(_module_desc, invoker.module_descs) {
+    assert(_package_id > 2, "Too many modules, ran out of package IDs!")
     chrome_feature_module(
         "${_bundle_target_name}__${_module_desc.name}_bundle_module") {
       forward_variables_from(invoker,
@@ -34,10 +36,15 @@ template("chrome_bundle") {
       version_name = chrome_version_name
       uncompress_shared_libraries =
           invoker.is_monochrome_or_trichrome || chromium_linker_supported
+
+      # Each module needs a unique resource package ID so that we don't have ID
+      # collisions between feature modules.
+      package_id = _package_id
     }
     _module_desc.module_target =
         ":${_bundle_target_name}__${_module_desc.name}_bundle_module"
     _extra_modules += [ _module_desc ]
+    _package_id -= 1
   }
 
   android_app_bundle(target_name) {

chrome/android/modules/chrome_feature_module_tmpl.gni

@@ -33,6 +33,7 @@ template("chrome_feature_module") {
                              "base_module_target",
                              "manifest_package",
                              "min_sdk_version",
+                             "package_id",
                              "uncompress_shared_libraries",
                              "version_code",
                              "version_name",
@@ -54,7 +55,6 @@ template("chrome_feature_module") {
     proguard_enabled = !is_java_debug
 
     package_name = _module_desc.name
-    package_name_to_id_mapping = resource_packages_id_mapping
 
     _loadable_modules_32_bit = []
     _loadable_modules_64_bit = []

chrome/android/modules/chrome_feature_modules.gni

@@ -18,18 +18,6 @@ if (enable_arcore) {
   import("//chrome/android/features/ar/ar_module.gni")
 }
 
-# Mapping that controls package IDs assigned to modules. The package IDs have to
-# be unique and lower than 0x7f.
-# TODO(crbug.com/984158): Autogenerate instead of explicit list.
-resource_packages_id_mapping = [
-  "ar=0x7e",
-  "vr=0x7d",
-  "tab_ui=0x7c",
-  "autofill_assistant=0x7b",
-  "dev_ui=0x7a",
-  "test_dummy=0x79",
-]
-
 # List of feature module descs for each Chrome flavor. These lists are used to
 # autogenerate the relevant module targets and bundles. A feature module desc
 # is a GN scope with the following fields:

docs/android_dynamic_feature_modules.md

@@ -61,17 +61,6 @@ First, create the file
 </manifest>
 ```
 
-Then, add a package ID for Foo so that Foo's resources have unique identifiers.
-For this, add a new ID to
-`//chrome/android/modules/chrome_feature_modules.gni`:
-
-```gn
-resource_packages_id_mapping = [
-  ...,
-  "foo=0x{XX}", # Set {XX} to next lower hex number.
-]
-```
-
 Next, create a descriptor configuring the Foo module. To do this, create
 `//chrome/android/features/foo/foo_module.gni` and add the following: