Auto-load PAKs of a module

We write the list of PAK files in a module to the module's Java
descriptor and load those PAK files when first accessing the module
via the autogenerated module class. This gets rid of the whitelist in
Module.java. Going forward DFM authors simply have to specify pak_deps
and paks in the module_desc.

APKs do not have proper Java module descriptors. Therefore, we add all
resources of modules that are packaged into the chrome APKs to the APK's
main resources.pak.

Change-Id: Iaa555de0682894397ee40e6a12c3681dfd0542eb
Bug: 986960
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1877139
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Auto-Submit: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711732}

chrome/android/BUILD.gn

@@ -1265,11 +1265,15 @@ if (current_toolchain == default_toolchain) {
   #
   # Variables:
   #    is_monochrome: If true, generate Monochrome targets rather than Chrome.
-  #    is_trichrome: Optionally generate Trichrome targets that use monochrome
+  #    is_trichrome: (Optional) Generate Trichrome targets that use monochrome
   #      library targets but don't include webview resources.
+  #    is_bundle_module: (Optional) If true, generate targets for base bundle
+  #      module.
   template("resource_packaging") {
     _is_monochrome = invoker.is_monochrome
     _is_trichrome = defined(invoker.is_trichrome) && invoker.is_trichrome
+    _is_bundle_module =
+        defined(invoker.is_bundle_module) && invoker.is_bundle_module
 
     if (_is_trichrome) {
       _variant = "trichrome_chrome"
@@ -1278,7 +1282,11 @@ if (current_toolchain == default_toolchain) {
     } else {
       _variant = "chrome"
     }
-    _variant += "_apk"
+    if (_is_bundle_module) {
+      _variant += "_bundle_module"
+    } else {
+      _variant += "_apk"
+    }
 
     if (enable_resource_whitelist_generation) {
       if (_is_trichrome || _is_monochrome) {
@@ -1397,9 +1405,8 @@ if (current_toolchain == default_toolchain) {
           "//weblayer:resources",
         ]
       }
-      if (!dfmify_dev_ui) {
-        additional_extra_paks +=
-            [ "$root_gen_dir/chrome/dev_ui_page_resources.pak" ]
+      if (!dfmify_dev_ui || !_is_bundle_module) {
+        additional_extra_paks += [ "$root_gen_dir/chrome/dev_ui_resources.pak" ]
         deps += [ "//chrome/browser/resources:dev_ui_paks" ]
       }
 
@@ -1454,13 +1461,26 @@ if (current_toolchain == default_toolchain) {
   resource_packaging("chrome_apk_pak_assets") {
     is_monochrome = false
   }
+  resource_packaging("chrome_bundle_module_pak_assets") {
+    is_monochrome = false
+    is_bundle_module = true
+  }
   resource_packaging("monochrome_apk_pak_assets") {
     is_monochrome = true
   }
+  resource_packaging("monochrome_bundle_module_pak_assets") {
+    is_monochrome = true
+    is_bundle_module = true
+  }
   resource_packaging("trichrome_chrome_apk_pak_assets") {
     is_monochrome = false
     is_trichrome = true
   }
+  resource_packaging("trichrome_chrome_bundle_module_pak_assets") {
+    is_monochrome = false
+    is_trichrome = true
+    is_bundle_module = true
+  }
 }  # current_toolchain == host_toolchain
 
 # Monochrome equivalent of Chrome's APK or bundle library template.
@@ -1621,6 +1641,7 @@ static_library("browser_test_support") {
 #    target, intended for Android L and M only.
 template("chrome_public_apk_or_module_tmpl") {
   _is_modern = defined(invoker.is_modern) && invoker.is_modern
+  _is_bundle_module = invoker.target_type == "android_app_bundle_module"
   chrome_public_common_apk_or_module_tmpl(target_name) {
     forward_variables_from(invoker,
                            [
@@ -1632,8 +1653,12 @@ template("chrome_public_apk_or_module_tmpl") {
                              "target_type",
                              "enable_multidex",
                            ])
-
-    deps = _chrome_public_shared_deps + [ ":chrome_apk_pak_assets" ]
+    deps = _chrome_public_shared_deps
+    if (_is_bundle_module) {
+      deps += [ ":chrome_bundle_module_pak_assets" ]
+    } else {
+      deps += [ ":chrome_apk_pak_assets" ]
+    }
 
     if (_is_modern) {
       android_manifest = chrome_modern_public_android_manifest

chrome/android/chrome_public_apk_tmpl.gni

@@ -234,16 +234,9 @@ template("chrome_public_common_apk_or_module_tmpl") {
 
     if (dfmify_dev_ui && (_target_type == "android_apk" ||
                           _target_type == "instrumentation_test_apk")) {
-      # Native resource split moves resources out of resources.pak, but this
-      # is a DFM feature, which APKs don't use. To make the resources split
-      # available for APKs, add dependencies to (1) Java classes, (2)
-      # resources of the resource split's DFM (e.g., DevUI DFM). The Java
-      # classes tells Chrome that the DFM is installed, which then causes the
-      # included resources to be loaded when needed.
-      deps += [
-        "//chrome/android/features/dev_ui:java",  # (1) from above.
-        "//chrome/android/features/dev_ui:pak_assets",  # (2) from above.
-      ]
+      # Dev UI is a feature in a DFM, and APKs don't use DFMs. To make the code
+      # available for APKs add a dependency on it.
+      deps += [ "//chrome/android/features/dev_ui:java" ]
     }
 
     if (!is_java_debug) {
@@ -318,6 +311,8 @@ template("chrome_public_common_apk_or_module_tmpl") {
 #     static library target to provide certain shared library deps, and that
 #     this target should not package webview deps.
 template("monochrome_public_common_apk_or_module_tmpl") {
+  _is_bundle_module = defined(invoker.target_type) &&
+                      invoker.target_type == "android_app_bundle_module"
   if (defined(invoker.enable_multidex)) {
     _enable_multidex = invoker.enable_multidex
   } else {
@@ -440,7 +435,12 @@ template("monochrome_public_common_apk_or_module_tmpl") {
     # Android N+ better supports multiple locales (https://crbug.com/780847).
     support_zh_hk = false
 
-    _deps += [ "//chrome/android:${_pak_prefix}_apk_pak_assets" ]
+    if (_is_bundle_module) {
+      _pak_prefix += "_bundle_module"
+    } else {
+      _pak_prefix += "_apk"
+    }
+    _deps += [ "//chrome/android:${_pak_prefix}_pak_assets" ]
 
     if (_enable_multidex && invoker.target_type == "android_apk" &&
         !defined(invoker.negative_main_dex_globs)) {

chrome/android/features/dev_ui/BUILD.gn

@@ -20,12 +20,3 @@ android_library("java") {
   ]
   java_files = [ "java/src/org/chromium/chrome/features/dev_ui/DevUiImpl.java" ]
 }
-
-android_assets("pak_assets") {
-  renaming_sources = [ "$root_gen_dir/chrome/dev_ui_page_resources.pak" ]
-  renaming_destinations = [ "dev_ui_resources.pak" ]
-  deps = [
-    "//chrome/browser/resources:dev_ui_paks",
-  ]
-  disable_compression = true
-}

chrome/android/features/dev_ui/dev_ui_module.gni

@@ -10,12 +10,8 @@ declare_args() {
 
 dev_ui_module_desc = {
   name = "dev_ui"
-
-  # These deps are also used to support non-bundle flows, e.g., APK builds and
-  # unit tests.
-  java_deps = [
-    "//chrome/android/features/dev_ui:java",
-    "//chrome/android/features/dev_ui:pak_assets",
-  ]
+  java_deps = [ "//chrome/android/features/dev_ui:java" ]
   android_manifest = "//chrome/android/features/dev_ui/java/AndroidManifest.xml"
+  paks = [ "$root_gen_dir/chrome/dev_ui_resources.pak" ]
+  pak_deps = [ "//chrome/browser/resources:dev_ui_paks" ]
 }

chrome/android/modules/chrome_feature_module_tmpl.gni

@@ -57,9 +57,20 @@ template("chrome_feature_module") {
     not_needed([ "_is_monochrome_or_trichrome" ])
   }
 
+  if (defined(_module_desc.pak_deps)) {
+    android_assets("${target_name}__pak_assets") {
+      sources = _module_desc.paks
+      deps = _module_desc.pak_deps
+      disable_compression = true
+    }
+  }
+
   module_desc_java("${target_name}__module_desc_java") {
     module_name = _module_desc.name
     shared_libraries = _shared_libraries
+    if (defined(_module_desc.pak_deps)) {
+      paks = _module_desc.paks
+    }
   }
 
   android_app_bundle_module(target_name) {
@@ -78,6 +89,9 @@ template("chrome_feature_module") {
     deps = [
       ":${target_name}__module_desc_java",
     ]
+    if (defined(_module_desc.pak_deps)) {
+      deps += [ ":${target_name}__pak_assets" ]
+    }
     if (defined(_module_desc.java_deps)) {
       deps += _module_desc.java_deps
     }

chrome/android/modules/chrome_feature_modules.gni

@@ -33,6 +33,8 @@ if (enable_arcore) {
 #     library files going into module if the module is executed in 32 bit.
 #   loadable_modules_64_bit: (Optional) List of additional 64-bit shared
 #     library files going into module if the module is executed in 64 bit.
+#   pak_deps: (Optional) Grit or repack targets of PAKs going into module.
+#   paks: (Optional) PAKs going into module.
 # Each new module needs to add a desc to one of the lists below.
 
 # Modules shipped in Chrome Modern (Android L+).

chrome/android/modules/test_dummy/internal/BUILD.gn

@@ -56,13 +56,3 @@ generate_jni_registration("jni_registration") {
   header_output = "$target_gen_dir/jni_registration.h"
   namespace = "test_dummy"
 }
-
-android_assets("pak_assets") {
-  sources = [
-    "$root_gen_dir/chrome/test_dummy_resources.pak",
-  ]
-  deps = [
-    "//chrome/android/features/test_dummy/internal:resources_native",
-  ]
-  disable_compression = true
-}

chrome/android/modules/test_dummy/test_dummy_module.gni

@@ -11,11 +11,12 @@ test_dummy_module_desc = {
   java_deps = [
     "//chrome/android/features/test_dummy/internal:java",
     "//chrome/android/modules/test_dummy/internal:java",
-    "//chrome/android/modules/test_dummy/internal:pak_assets",
   ]
-
   native_deps = [
     "//chrome/android/features/test_dummy/internal:native",
     "//chrome/android/modules/test_dummy/internal:native",
   ]
+  paks = [ "$root_gen_dir/chrome/test_dummy_resources.pak" ]
+  pak_deps =
+      [ "//chrome/android/features/test_dummy/internal:resources_native" ]
 }

chrome/browser/resources/BUILD.gn

@@ -363,7 +363,7 @@ if (enable_webui_tab_strip) {
 }
 
 repack("dev_ui_paks") {
-  output = "$root_gen_dir/chrome/dev_ui_page_resources.pak"
+  output = "$root_gen_dir/chrome/dev_ui_resources.pak"
 
   sources = [
     "$root_gen_dir/chrome/bluetooth_internals_resources.pak",

chrome/chrome_paks.gni

@@ -134,7 +134,7 @@ template("chrome_extra_paks") {
       # New paks should be added here by default.
       sources += [
         "$root_gen_dir/chrome/component_extension_resources.pak",
-        "$root_gen_dir/chrome/dev_ui_page_resources.pak",
+        "$root_gen_dir/chrome/dev_ui_resources.pak",
         "$root_gen_dir/chrome/downloads_resources.pak",
         "$root_gen_dir/chrome/local_ntp_resources.pak",
         "$root_gen_dir/chrome/settings_resources.pak",

chrome/test/BUILD.gn

@@ -3668,7 +3668,6 @@ test("unit_tests") {
       # that DevUI page tests would work.
       deps += [
         "//chrome/android/features/dev_ui:java",
-        "//chrome/android/features/dev_ui:pak_assets",
         "//chrome/android/modules/dev_ui/provider:java",
       ]
     }

components/module_installer/android/java/src/org/chromium/components/module_installer/builder/Module.java

@@ -132,18 +132,11 @@ public class Module<T> {
     private static void loadNative(String name) {
         // Can only initialize native once per lifetime of Chrome.
         if (sInitializedModules.contains(name)) return;
-        String[] libraries = loadModuleDescriptor(name).getLibraries();
-        // TODO(crbug.com/986960): Automatically determine if module has native
-        // resources instead of whitelisting.
-        boolean loadResources = false;
-        if ("test_dummy".equals(name)) {
-            loadResources = true;
-        }
-        if ("dev_ui".equals(name)) {
-            loadResources = true;
-        }
-        if (libraries.length > 0 || loadResources) {
-            ModuleJni.get().loadNative(name, libraries, loadResources);
+        ModuleDescriptor moduleDescriptor = loadModuleDescriptor(name);
+        String[] libraries = moduleDescriptor.getLibraries();
+        String[] paks = moduleDescriptor.getPaks();
+        if (libraries.length > 0 || paks.length > 0) {
+            ModuleJni.get().loadNative(name, libraries, paks);
         }
         sInitializedModules.add(name);
     }
@@ -165,6 +158,11 @@ public class Module<T> {
                 public String[] getLibraries() {
                     return new String[0];
                 }
+
+                @Override
+                public String[] getPaks() {
+                    return new String[0];
+                }
             };
         }
 
@@ -191,6 +189,6 @@ public class Module<T> {
 
     @NativeMethods
     interface Natives {
-        void loadNative(String name, String[] libraries, boolean loadResources);
+        void loadNative(String name, String[] libraries, String[] paks);
     }
 }

components/module_installer/android/java/src/org/chromium/components/module_installer/builder/ModuleDescriptor.java

@@ -12,4 +12,8 @@ public interface ModuleDescriptor {
      * Returns the list of native library names this module requires at runtime.
      */
     String[] getLibraries();
+    /**
+     * Returns the list of PAK resources files this module contains.
+     */
+    String[] getPaks();
 }

components/module_installer/android/module.cc

@@ -74,9 +74,9 @@ void RegisterJni(JNIEnv* env, void* library_handle, const std::string& name) {
   CHECK(registration_function(env)) << "JNI registration failed: " << name;
 }
 
-void LoadResources(const std::string& name) {
+void LoadResources(const std::string& pak) {
   module_installer::ScopedAllowModulePakLoad scoped_allow_module_pak_load;
-  ui::LoadPackFileFromApk("assets/" + name + "_resources.pak");
+  ui::LoadPackFileFromApk("assets/" + pak);
 }
 
 }  // namespace
@@ -85,7 +85,7 @@ static void JNI_Module_LoadNative(
     JNIEnv* env,
     const base::android::JavaParamRef<jstring>& jname,
     const base::android::JavaParamRef<jobjectArray>& jlibraries,
-    jboolean load_resources) {
+    const base::android::JavaParamRef<jobjectArray>& jpaks) {
   std::string name;
   base::android::ConvertJavaStringToUTF8(env, jname, &name);
   std::vector<std::string> libraries;
@@ -101,8 +101,10 @@ static void JNI_Module_LoadNative(
     // function.
     RegisterJni(env, library_handle, name);
   }
-  if (load_resources) {
-    LoadResources(name);
+  std::vector<std::string> paks;
+  base::android::AppendJavaStringArrayToStringVector(env, jpaks, &paks);
+  for (const auto& pak : paks) {
+    LoadResources(pak);
   }
 }
 

components/module_installer/android/module_desc_java.gni

@@ -17,6 +17,7 @@ import("//build/config/python.gni")
 #   shared_libraries: (Optional) List of shared_library targets the module
 #     requires at runtime. Will consider all transitively depended on
 #     shared_libraries.
+#   paks: (Optional) PAK files going into the module.
 template("module_desc_java") {
   _target_name = target_name
 
@@ -55,6 +56,13 @@ template("module_desc_java") {
       "--output",
       rebase_path(_srcjar, root_out_dir),
     ]
+    if (defined(invoker.paks)) {
+      _rebased_paks = []
+      foreach(_pak, invoker.paks) {
+        _rebased_paks += [ rebase_path(_pak, root_out_dir) ]
+      }
+      args += [ "--paks=$_rebased_paks" ]
+    }
   }
 
   android_library(_target_name) {

components/module_installer/android/module_desc_java.py

@@ -27,11 +27,17 @@ import org.chromium.base.annotations.UsedByReflection;
 @UsedByReflection("Module.java")
 public class ModuleDescriptor_{MODULE} implements ModuleDescriptor {{
     private static final String[] LIBRARIES = {{{LIBRARIES}}};
+    private static final String[] PAKS = {{{PAKS}}};
 
     @Override
     public String[] getLibraries() {{
         return LIBRARIES;
     }}
+
+    @Override
+    public String[] getPaks() {{
+        return PAKS;
+    }}
 }}
 """
 
@@ -41,10 +47,12 @@ def main():
   parser.add_argument('--module', required=True, help='The module name.')
   parser.add_argument(
       '--libraries', required=True, help='GN list of native library paths.')
+  parser.add_argument('--paks', help='GN list of PAK file paths')
   parser.add_argument(
       '--output', required=True, help='Path to the generated srcjar file.')
   options = parser.parse_args(build_utils.ExpandFileArgs(sys.argv[1:]))
   options.libraries = build_utils.ParseGnList(options.libraries)
+  options.paks = build_utils.ParseGnList(options.paks)
 
   libraries = []
   for path in options.libraries:
@@ -54,10 +62,12 @@ def main():
     assert filename.endswith('.so')
     # Remove lib prefix and .so suffix.
     libraries += [filename[3:-3]]
+  paks = options.paks if options.paks else []
 
   format_dict = {
       'MODULE': options.module,
       'LIBRARIES': ','.join(['"%s"' % l for l in libraries]),
+      'PAKS': ','.join(['"%s"' % os.path.basename(p) for p in paks]),
   }
   with build_utils.AtomicOutput(options.output) as f:
     with zipfile.ZipFile(f.name, 'w') as srcjar_file: