Fix build issues with creating R.java in java_library targets (Reland)

A collection of bugfixes for creating R.java in java_library_impl
targets:
- test libraries support.
- apk under test R.java shadowing bugfix.
- always generate onResourcesLoaded like package resources did.
- Fix issue where R.java is not generated for the apk package name.
- Change webAPK manifest package name (remove .new because it is a java
keyword).

Reason for reland: Fixed build issue with webAPKs.
Original CL: https://chromium-review.googlesource.com/c/chromium/src/+/2302919
This reverts commit 1895ffb4

Bug: 1073476
Change-Id: Ic095091eb64db32d1653e219f507463de59cacf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307809
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790428}

build/android/gyp/compile_resources.py

@@ -1100,6 +1100,12 @@ def _OnStaleMd5(options):
     custom_root_package_name = options.r_java_root_package_name
     grandparent_custom_package_name = None
 
+    # Always generate an R.java file for the package listed in
+    # AndroidManifest.xml because this is where Android framework looks to find
+    # onResourcesLoaded() for shared library apks. While not actually necessary
+    # for application apks, it also doesn't hurt.
+    apk_package_name = manifest_package_name
+
     if options.package_name and not options.arsc_package_name:
       # Feature modules have their own custom root package name and should
       # inherit from the appropriate base module package. This behaviour should
@@ -1108,15 +1114,14 @@ def _OnStaleMd5(options):
       # apk under test.
       custom_root_package_name = options.package_name
       grandparent_custom_package_name = options.r_java_root_package_name
-
-    if options.shared_resources or options.app_as_shared_lib:
-      package_for_library = manifest_package_name
-    else:
-      package_for_library = None
+      # Feature modules have the same manifest package as the base module but
+      # they should not create an R.java for said manifest package because it
+      # will be created in the base module.
+      apk_package_name = None
 
     logging.debug('Creating R.srcjar')
     resource_utils.CreateRJavaFiles(
-        build.srcjar_dir, package_for_library, build.r_txt_path,
+        build.srcjar_dir, apk_package_name, build.r_txt_path,
         options.extra_res_packages, rjava_build_options, options.srcjar_out,
         custom_root_package_name, grandparent_custom_package_name,
         options.extra_main_r_text_files)

build/android/gyp/create_r_java.py

@@ -22,6 +22,7 @@ def _CreateRJava(resource_zips, package_name, srcjar_out, rtxt_out):
     rjava_build_options = resource_utils.RJavaBuildOptions()
     rjava_build_options.ExportAllResources()
     rjava_build_options.ExportAllStyleables()
+    rjava_build_options.GenerateOnResourcesLoaded(fake=True)
     resource_utils.CreateRJavaFiles(build.srcjar_dir,
                                     package_name,
                                     rtxt_out,

build/android/gyp/prepare_resources.py

@@ -191,7 +191,7 @@ def _OnStaleMd5(options):
         rjava_build_options.ExportAllResources()
         rjava_build_options.ExportAllStyleables()
         if options.shared_resources:
-          rjava_build_options.GenerateOnResourcesLoaded()
+          rjava_build_options.GenerateOnResourcesLoaded(fake=True)
 
         # Not passing in custom_root_package_name or parent to keep
         # file names unique.

build/android/gyp/util/resource_utils.py

@@ -402,6 +402,7 @@ class RJavaBuildOptions:
     self.has_on_resources_loaded = False
     self.export_const_styleable = False
     self.final_package_id = None
+    self.fake_on_resources_loaded = False
 
   def ExportNoResources(self):
     """Make all resource IDs final, and don't generate a method."""
@@ -435,15 +436,20 @@ class RJavaBuildOptions:
     """
     self.export_const_styleable = True
 
-  def GenerateOnResourcesLoaded(self):
+  def GenerateOnResourcesLoaded(self, fake=False):
     """Generate an onResourcesLoaded() method.
 
     This Java method will be called at runtime by the framework when
     the corresponding library (which includes the R.java source file)
     will be loaded at runtime. This corresponds to the --shared-resources
     or --app-as-shared-lib flags of 'aapt package'.
+
+    if |fake|, then the method will be empty bodied to compile faster. This
+    useful for dummy R.java files that will eventually be replaced by real
+    ones.
     """
     self.has_on_resources_loaded = True
+    self.fake_on_resources_loaded = fake
 
   def SetFinalPackageId(self, package_id):
     """Sets a package ID to be used for resources marked final."""
@@ -669,8 +675,7 @@ def _RenderRootRJavaSource(package, all_resources_by_type, rjava_build_options,
     extends_string = 'extends {{ parent_path }}.R.{{ resource_type }} '
     dep_path = GetCustomPackagePath(grandparent_custom_package_name)
 
-  template = Template(
-      """/* AUTO-GENERATED FILE.  DO NOT MODIFY. */
+  template = Template("""/* AUTO-GENERATED FILE.  DO NOT MODIFY. */
 
 package {{ package }};
 
@@ -690,6 +695,10 @@ public final class R {
     }
     {% endfor %}
     {% if has_on_resources_loaded %}
+      {% if fake_on_resources_loaded %}
+    public static void onResourcesLoaded(int packageId) {
+    }
+      {% else %}
     private static boolean sResourcesDidLoad;
     public static void onResourcesLoaded(int packageId) {
         if (sResourcesDidLoad) {
@@ -718,15 +727,17 @@ public final class R {
         {% endfor %}
     }
     {% endfor %}
+      {% endif %}
     {% endif %}
 }
 """,
-      trim_blocks=True,
-      lstrip_blocks=True)
+                      trim_blocks=True,
+                      lstrip_blocks=True)
   return template.render(
       package=package,
       resource_types=sorted(_ALL_RESOURCE_TYPES),
       has_on_resources_loaded=rjava_build_options.has_on_resources_loaded,
+      fake_on_resources_loaded=rjava_build_options.fake_on_resources_loaded,
       final_resources=final_resources_by_type,
       non_final_resources=non_final_resources_by_type,
       startIndex=_GetNonSystemIndex,

build/android/gyp/write_build_config.py

@@ -1211,10 +1211,7 @@ def main(argv):
   if options.type == 'android_apk' and options.tested_apk_config:
     tested_apk_deps = Deps([options.tested_apk_config])
     tested_apk_config = tested_apk_deps.Direct()[0]
-    tested_apk_resources_deps = tested_apk_deps.All('android_resources')
     gradle['apk_under_test'] = tested_apk_config['name']
-    all_resources_deps = [
-        d for d in all_resources_deps if not d in tested_apk_resources_deps]
 
   if options.type == 'android_app_bundle_module':
     deps_info['is_base_module'] = bool(options.is_base_module)
@@ -1402,7 +1399,6 @@ def main(argv):
             c['package_name'] for c in all_library_deps if 'package_name' in c
         ])
 
-
     # For feature modules, remove any resources that already exist in the base
     # module.
     if base_module_build_config:
@@ -1415,14 +1411,21 @@ def main(argv):
           base_module_build_config['deps_info']['extra_package_names']
       ]
 
-    config['deps_info']['dependency_zips'] = dependency_zips
-    config['deps_info']['extra_package_names'] = extra_package_names
     if options.type == 'android_apk' and options.tested_apk_config:
       config['deps_info']['arsc_package_name'] = (
           tested_apk_config['package_name'])
+      # We should not shadow the actual R.java files of the apk_under_test by
+      # creating new R.java files with the same package names in the tested apk.
+      extra_package_names = [
+          package for package in extra_package_names
+          if package not in tested_apk_config['extra_package_names']
+      ]
     if options.res_size_info:
       config['deps_info']['res_size_info'] = options.res_size_info
 
+    config['deps_info']['dependency_zips'] = dependency_zips
+    config['deps_info']['extra_package_names'] = extra_package_names
+
   if options.type == 'group':
     if options.extra_classpath_jars:
       # These are .jars to add to javac classpath but not to runtime classpath.

build/config/android/internal_rules.gni

@@ -597,7 +597,11 @@ template("generate_android_wrapper") {
 
 template("generate_r_java") {
   action_with_pydeps(target_name) {
-    forward_variables_from(invoker, [ "deps" ])
+    forward_variables_from(invoker,
+                           [
+                             "deps",
+                             "testonly",
+                           ])
     if (!defined(deps)) {
       deps = []
     }

build/config/android/rules.gni

@@ -978,9 +978,6 @@ if (enable_java_templates) {
       # No package means resources override their deps.
       if (defined(custom_package) || defined(android_manifest)) {
         r_text = _r_text_out_path
-      } else {
-        assert(defined(invoker.deps),
-               "Must specify deps when custom_package is omitted.")
       }
       if (defined(_srcjar_path)) {
         srcjar = _srcjar_path

chrome/android/webapk/shell_apk/BUILD.gn

@@ -307,7 +307,7 @@ webapk_tmpl("new_splash_webapk") {
   config_file = "manifest/bound_manifest_config.json"
   manifest_to_upload_dep = ":generate_new_style_manifest_for_upload"
   apk_name = "NewSplashWebApk"
-  apk_package_name = "org.chromium.webapk.new.splash"
+  apk_package_name = "org.chromium.webapk.new_splash"
 }
 
 # Used by javatests