Use R.txt from AAR to generate R.java for it when building APK.

R.java and R.txt files for an AAR were generated by extracting all
resources from the AAR and its dependencies and running aapt on these
resources. This approach turned out to be problematic for AARs of
GMS (Play Services).

GMS is shipped as a bunch of AARs with dependencies between them.
However the way it deals with resources is rather unusual. GMS code
references all resources via com.google.android.gms.R class that
corresponds to the "basement" AAR. This AAR has R.txt with entries for
all resources of the GMS but contains only a small subset of the
resources. All other resources are provided by other AARs: they have
different packages and no R.txt files though.

The consumer of the GMS must generate com.google.android.gms.R class
with entries for all resources provided by used GMS AARs. Entries for
resources that belong to the unused AARs may be ommited. This is what
Gradle plugin for Android does.

However AAR support in Chromium discarded R.txt from "basement" then
generated very small com.google.android.gms.R without essential entries
and this caused crashes in runtime.

This CL changes processing of the AARs resources. If R.txt is present in
AAR then aapt-generated R.txt is discared. The one from AAR is used to
generate R.java for java dependencies of the AAR target and to generate
final R.java file during APK build which is compiled and included into
the final APK.

During obfuscation Proguard checks that all entries that are referenced
in code were in fact generated. This check cannot be done during resource
processing because "basement"'s R.txt lists resources that aren't
included in Chromium and this is because code that uses these resources
isn't included either.

BUG=670368
R=agrieve@chromium.org,dgn@chromium.org

Review-Url: https://codereview.chromium.org/2570313002
Cr-Commit-Position: refs/heads/master@{#439485}

build/android/gyp/aar.py

@@ -77,6 +77,7 @@ def main():
     data['has_classes_jar'] = False
     data['has_proguard_flags'] = False
     data['has_native_libraries'] = False
+    data['has_r_text_file'] = False
     with zipfile.ZipFile(aar_file) as z:
       data['is_manifest_empty'] = (
           _IsManifestEmpty(z.read('AndroidManifest.xml')))
@@ -101,6 +102,10 @@ def main():
           data['has_classes_jar'] = True
         elif name == 'proguard.txt':
           data['has_proguard_flags'] = True
+        elif name == 'R.txt':
+          # Some AARs, e.g. gvr_controller_java, have empty R.txt. Such AARs
+          # have no resources as well. We treat empty R.txt as having no R.txt.
+          data['has_r_text_file'] = (z.read('R.txt').strip() != '')
 
     print gn_helpers.ToGNString(data)
 

build/android/gyp/process_resources.py

@@ -74,7 +74,11 @@ def _ParseArgs(args):
   parser.add_option('--srcjar-out',
                     help='Path to srcjar to contain generated R.java.')
   parser.add_option('--r-text-out',
-                    help='Path to store the R.txt file generated by appt.')
+                    help='Path to store the generated R.txt file.')
+  parser.add_option('--r-text-in',
+                    help='Path to pre-existing R.txt for these resources. '
+                    'Resource names from it will be used to generate R.java '
+                    'instead of aapt-generated R.txt.')
 
   parser.add_option('--proguard-file',
                     help='Path to proguard.txt generated file')
@@ -169,8 +173,24 @@ def CreateRJavaFiles(srcjar_dir, main_r_txt_file, packages, r_txt_files,
     # figure out which entries belong to them, but use the values from the
     # main R.txt file.
     for entry in _ParseTextSymbolsFile(r_txt_file):
-      entry = all_resources[(entry.resource_type, entry.name)]
-      resources_by_type[entry.resource_type].append(entry)
+      entry = all_resources.get((entry.resource_type, entry.name))
+      # For most cases missing entry here is an error. It means that some
+      # library claims to have or depend on a resource that isn't included into
+      # the APK. There is one notable exception: Google Play Services (GMS).
+      # GMS is shipped as a bunch of AARs. One of them - basement - contains
+      # R.txt with ids of all resources, but most of the resources are in the
+      # other AARs. However, all other AARs reference their resources via
+      # basement's R.java so the latter must contain all ids that are in its
+      # R.txt. Most targets depend on only a subset of GMS AARs so some
+      # resources are missing, which is okay because the code that references
+      # them is missing too. We can't get an id for a resource that isn't here
+      # so the only solution is to skip the resource entry entirely.
+      #
+      # We can verify that all entries referenced in the code were generated
+      # correctly by running Proguard on the APK: it will report missing
+      # fields.
+      if entry:
+        resources_by_type[entry.resource_type].append(entry)
 
   for package, resources_by_type in resources_by_package.iteritems():
     package_r_java_dir = os.path.join(srcjar_dir, *package.split('.'))
@@ -434,6 +454,12 @@ def _OnStaleMd5(options):
       build_utils.Touch(r_txt_path)
 
     if not options.include_all_resources:
+      # --include-all-resources can only be specified for generating final R
+      # classes for APK. It makes no sense for APK to have pre-generated R.txt
+      # though, because aapt-generated already lists all available resources.
+      if options.r_text_in:
+        r_txt_path = options.r_text_in
+
       packages = list(options.extra_res_packages)
       r_txt_files = list(options.extra_r_text_files)
 

build/config/android/internal_rules.gni

@@ -2538,7 +2538,7 @@ if (enable_java_templates) {
 
     zip_path = invoker.zip_path
     srcjar_path = invoker.srcjar_path
-    r_text_path = invoker.r_text_path
+    r_text_out_path = invoker.r_text_out_path
     build_config = invoker.build_config
     android_manifest = invoker.android_manifest
 
@@ -2560,7 +2560,7 @@ if (enable_java_templates) {
       outputs = [
         zip_path,
         srcjar_path,
-        r_text_path,
+        r_text_out_path,
       ]
 
       _all_resource_dirs = []
@@ -2623,12 +2623,21 @@ if (enable_java_templates) {
         "--resource-zip-out",
         rebase_path(zip_path, root_build_dir),
         "--r-text-out",
-        rebase_path(r_text_path, root_build_dir),
+        rebase_path(r_text_out_path, root_build_dir),
         "--dependencies-res-zips=@FileArg($rebase_build_config:resources:dependency_zips)",
         "--extra-res-packages=@FileArg($rebase_build_config:resources:extra_package_names)",
         "--extra-r-text-files=@FileArg($rebase_build_config:resources:extra_r_text_files)",
       ]
 
+      if (defined(invoker.r_text_in_path)) {
+        _r_text_in_path = invoker.r_text_in_path
+        inputs += [ _r_text_in_path ]
+        args += [
+          "--r-text-in",
+          rebase_path(_r_text_in_path, root_build_dir),
+        ]
+      }
+
       if (non_constant_id) {
         args += [ "--non-constant-id" ]
       }

build/config/android/rules.gni

@@ -646,6 +646,8 @@ if (enable_java_templates) {
   #     different application at runtime to access the package's resources.
   #   app_as_shared_lib: If true make a resource package that can be loaded as
   #     both shared_resources and normal application.
+  #   r_text_file: (optional) path to pre-generated R.txt to be used when
+  #     generating R.java instead of resource-based aapt-generated one.
 
   # Example:
   #   android_resources("foo_resources") {
@@ -667,7 +669,7 @@ if (enable_java_templates) {
     base_path = "$target_gen_dir/$target_name"
     zip_path = base_path + ".resources.zip"
     srcjar_path = base_path + ".srcjar"
-    r_text_path = base_path + "_R.txt"
+    r_text_out_path = base_path + "_R.txt"
     build_config = base_path + ".build_config"
 
     build_config_target_name = "${target_name}__build_config"
@@ -700,7 +702,7 @@ if (enable_java_templates) {
 
       # No package means resources override their deps.
       if (defined(custom_package) || defined(android_manifest)) {
-        r_text = r_text_path
+        r_text = r_text_out_path
       } else {
         assert(defined(invoker.deps),
                "Must specify deps when custom_package is omitted.")
@@ -731,6 +733,10 @@ if (enable_java_templates) {
         deps += [ invoker.android_manifest_dep ]
       }
 
+      if (defined(invoker.r_text_file)) {
+        r_text_in_path = invoker.r_text_file
+      }
+
       # Always generate R.onResourcesLoaded() method, it is required for
       # compiling ResourceRewriter, there is no side effect because the
       # generated R.class isn't used in final apk.
@@ -1675,7 +1681,7 @@ if (enable_java_templates) {
                                "shared_resources",
                              ])
       srcjar_path = "${target_gen_dir}/${target_name}.srcjar"
-      r_text_path = "${target_gen_dir}/${target_name}_R.txt"
+      r_text_out_path = "${target_gen_dir}/${target_name}_R.txt"
       android_manifest = _android_manifest
       resource_dirs = [ "//build/android/ant/empty/res" ]
       zip_path = resources_zip_path
@@ -2710,8 +2716,14 @@ if (enable_java_templates) {
         "${_output_path}/AndroidManifest.xml",
       ]
 
-      if (_scanned_files.resources != []) {
+      if (_scanned_files.has_r_text_file) {
+        # Certain packages, in particular Play Services have no R.txt even
+        # though its presence is mandated by AAR spec. Such packages cause
+        # spurious rebuilds if this output is specified unconditionally.
         outputs += [ "${_output_path}/R.txt" ]
+      }
+
+      if (_scanned_files.resources != []) {
         outputs += get_path_info(
                 rebase_path(_scanned_files.resources, "", _output_path),
                 "abspath")
@@ -2728,7 +2740,7 @@ if (enable_java_templates) {
     }
 
     # Create the android_resources target for resources.
-    if (_scanned_files.resources != []) {
+    if (_scanned_files.resources != [] || _scanned_files.has_r_text_file) {
       _res_target_name = "${target_name}__res"
       android_resources(_res_target_name) {
         forward_variables_from(invoker, [ "deps" ])
@@ -2742,6 +2754,9 @@ if (enable_java_templates) {
             rebase_path(_scanned_files.resources, "", _output_path)
         android_manifest_dep = ":$_unpack_target_name"
         android_manifest = "${_output_path}/AndroidManifest.xml"
+        if (_scanned_files.has_r_text_file) {
+          r_text_file = "${_output_path}/R.txt"
+        }
         v14_skip = true
       }
     }

chrome/android/java/proguard.flags

@@ -27,9 +27,6 @@
   public <init>(...);
 }
 
-# Google Play Services warnings are about its resources.
--dontwarn com.google.android.gms.R**
-
 # The Google Play services library depends on the legacy Apache HTTP library,
 # and just adding it as proguard time dependency causes the following warnings:
 # `library class org.apache.http.params.HttpConnectionParams depends on program