Make android_aar_prebuilt() aware of remaining features

build/config/android/rules.gni

Index: build/config/android/rules.gni
diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni
index 9dc5d2ca3325901d8f2abcbad2c99740572d24ba..04409ae55d14133b9428fa0726cbf6fc86bf0b62 100644
--- a/build/config/android/rules.gni
+++ b/build/config/android/rules.gni
@@ -2606,6 +2606,10 @@ if (enable_java_templates) {
   #   aar_path: Path to the AAR.
   #   proguard_configs: List of proguard configs to use in final apk step for
   #     any apk that depends on this library.
+  #   ignore_assets: Whether to ignore assets found in the .aar.
+  #   ignore_manifest: Whether to ignore merging of AndroidManifest.xml.
+  #   ignore_resources: Whether to ignore resources found in the .aar.
+  #   ignore_native_libraries: Whether to ignore .so files found in the .aar.
   #   TODO(jbudorick@): remove this arguments after crbug.com/522043 is fixed.
   #   requires_android: Whether this target can only be used for compiling Android related targets.
   #
@@ -2614,9 +2618,15 @@ if (enable_java_templates) {
   #     aar_path = "foo.aar"
   #   }
   template("android_aar_prebuilt") {
-    assert(defined(invoker.aar_path))
     _output_path = "${target_gen_dir}/${target_name}"
     _unpack_target_name = "${target_name}__unpack_aar"
+    _ignore_assets = defined(invoker.ignore_assets) && invoker.ignore_assets
+    _ignore_manifest =
+        defined(invoker.ignore_manifest) && invoker.ignore_manifest
+    _ignore_resources =
+        defined(invoker.ignore_resources) && invoker.ignore_resources
+    _ignore_native_libraries = defined(invoker.ignore_native_libraries) &&
+                               invoker.ignore_native_libraries
 
     # Scan the AAR file and determine the resources and jar files.
     # Some libraries might not have resources; others might have two jars.
@@ -2629,6 +2639,22 @@ if (enable_java_templates) {
                     ],
                     "scope")
 
+    assert(_ignore_assets || _scanned_files.assets == [],
+           "android_aar_prebuilt() assets not yet supported." +
+               " Implement or use ignore_assets = true." +
+               " http://crbug.com/643966")
+    assert(_ignore_native_libraries || !_scanned_files.has_native_libraries,
+           "android_aar_prebuilt() with .so files is not supported." +
+               " Use ignore_native_libraries = true to silence this error.")
+    assert(_ignore_manifest || _scanned_files.is_manifest_empty,
+           "android_aar_prebuilt() manifest merging not yet supported and" +
+               " non-trivial AndroidManifest.xml detected." +
+               " Implement or use ignore_manifest = true." +
+               " http://crbug.com/643967")
+
+    # Create android_java_prebuilt targets for jar files witin jars/.

[Ian Wen, 2016/09/06 19:09:59]

s/jars\//libs\/

Also we should move it down to #2711?

[agrieve, 2016/09/06 21:00:51]

Done.

+    assert(_scanned_files.has_classes_jar || _scanned_files.subjars == [])
+
     action(_unpack_target_name) {
       script = "//build/android/gyp/aar.py"  # Unzips the AAR
       args = [
@@ -2651,19 +2677,20 @@ if (enable_java_templates) {
                 rebase_path(_scanned_files.resources, "", _output_path),
                 "abspath")
       }
-      if (defined(_scanned_files.jars)) {
-        outputs +=
-            get_path_info(rebase_path(_scanned_files.jars, "", _output_path),
-                          "abspath")
+      if (_scanned_files.has_classes_jar) {
+        outputs += [ "${_output_path}/classes.jar" ]
+      }
+      outputs +=
+          get_path_info(rebase_path(_scanned_files.subjars, "", _output_path),
+                        "abspath")
+      if (_scanned_files.has_proguard_flags) {
+        outputs += [ "${_output_path}/proguard.txt" ]
       }
     }
 
-    _resource_targets = []
-
     # Create the android_resources target for resources.
-    if (_scanned_files.resources != []) {
+    if (!_ignore_resources && _scanned_files.resources != []) {

[Ian Wen, 2016/09/06 19:09:59]

Q: which target do you expect will need to ignore resources? It used to be quite
often that people forgot about adding resources from support library.

[agrieve, 2016/09/06 21:00:51]

Just added for completeness. I don't have any targets that will think will use
this.

       _res_target_name = "${target_name}__res"
-      _resource_targets += [ ":$_res_target_name" ]
       android_resources(_res_target_name) {
         forward_variables_from(invoker, [ "deps" ])
         if (!defined(deps)) {
@@ -2680,16 +2707,38 @@ if (enable_java_templates) {
       }
     }
 
-    # Create android_java_prebuilt targets for jar files.

[Ian Wen, 2016/09/06 19:09:59]

Nit: either keep this line or remove #2663 as well?

[agrieve, 2016/09/06 21:00:51]

Done.

-    _jar_targets = []
-    _counter = 0
-    foreach(jar, _scanned_files.jars) {
-      _counter += 1
-      _current_target = "${target_name}__jar_$_counter"
-      _jar_targets += [ ":$_current_target" ]
+    _subjar_targets = []
+    foreach(_jar_subpath, _scanned_files.subjars) {
+      if (!defined(_counter)) {

[Ian Wen, 2016/09/06 19:09:59]

Q: Is this considered as a better practice to guard the local variable, rather
than declaring it before hand and use it like a C-style variable?

I don't see _counter declared in other places in this file...

[agrieve, 2016/09/06 21:00:51]

No. The reason for this change is a bit confusing.

Before, classes.jar was included in _scanned_files.jars, so this loop was always
entered. That made the variable always used.

Now, the loop is not always entered, so it was complaining that the variable was
unused.

I could have also used the assert() # mark as used trick, but opted for this
instead.

+        _counter = 0
+      } else {
+        _counter += 1
+      }
+      _labels = _scanned_files.subjar_labels
+      _current_target = "${target_name}__subjar_${_labels[_counter]}"
+      _subjar_targets += [ ":$_current_target" ]
       java_prebuilt(_current_target) {
         forward_variables_from(invoker,
                                [
+                                 "jar_excluded_patterns",
+                                 "requires_android",
+                               ])
+        deps = [
+          ":$_unpack_target_name",
+        ]
+        if (!defined(requires_android)) {
+          requires_android = true
+        }
+        supports_android = true
+        jar_path = "$_output_path/$_jar_subpath"
+      }
+    }
+
+    if (_scanned_files.has_classes_jar) {
+      _jar_target_name = "${target_name}__classes"
+      java_prebuilt(_jar_target_name) {
+        forward_variables_from(invoker,
+                               [
                                  "deps",
                                  "input_jars_paths",
                                  "jar_excluded_patterns",
@@ -2699,17 +2748,38 @@ if (enable_java_templates) {
         if (!defined(deps)) {
           deps = []
         }
-        deps += _resource_targets + [ ":$_unpack_target_name" ]
+        deps += _subjar_targets + [ ":$_unpack_target_name" ]
+        if (defined(_res_target_name)) {
+          deps += [ ":$_res_target_name" ]
+        }
         if (!defined(requires_android)) {
           requires_android = true
         }
         supports_android = true
-        jar_path = "${_output_path}/$jar"
+        jar_path = "$_output_path/classes.jar"
+
+        if (_scanned_files.has_proguard_flags) {
+          if (!defined(proguard_configs)) {
+            proguard_configs = []
+          }
+          proguard_configs += [ "$_output_path/proguard.txt" ]
+        }
       }
     }
 
     java_group(target_name) {
-      deps = _resource_targets + _jar_targets
+      deps = []
+      if (defined(_jar_target_name)) {
+        deps += [ ":$_jar_target_name" ]
+
+        # Although subjars are meant to be private, we add them as deps here
+        # because in practice they seem to contain classes required to be in the
+        # classpath.
+        deps += _subjar_targets
+      }
+      if (defined(_res_target_name)) {
+        deps += [ ":$_res_target_name" ]
+      }
     }
   }
 }