Android: Add host_jar_path vs device_jar_path to build logic

Fixes desugared .jar files incorrectly being used in java_binary() /
junit_binary().

Bug: 1078484
Change-Id: Ia49a10d3e907844ed75ee658604e8eeaa2993fb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2241865
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#778433}

BUILD.gn

@@ -356,7 +356,6 @@ group("gn_all") {
       "//tools/android:push_apps_to_background",
       "//tools/android/audio_focus_grabber:audio_focus_grabber_apk",
       "//tools/android/customtabs_benchmark:customtabs_benchmark_apk",
-      "//tools/android/errorprone_plugin:errorprone_plugin_java",
       "//tools/android/kerberos/SpnegoAuthenticator:spnego_authenticator_apk",
       "//ui/android:ui_junit_tests",
       "//weblayer/public/java:client_aar",

build/android/bytecode/BUILD.gn

@@ -17,5 +17,4 @@ java_binary("bytecode_processor") {
   ]
   wrapper_script_name = "helper/bytecode_processor"
   enable_bytecode_checks = false
-  enable_desugar = false
 }

build/android/docs/java_toolchain.md

@@ -20,6 +20,12 @@ also have a default `jar_excluded_patterns` set (more on that later):
 All target names must end with "_java" so that the build system can distinguish
 them from non-java targets (or [other variations](https://cs.chromium.org/chromium/src/build/config/android/internal_rules.gni?rcl=ec2c17d7b4e424e060c3c7972842af87343526a1&l=20)).
 
+Most targets produce two separate `.jar` files:
+* Device `.jar`: Used to produce `.dex.jar`, which is used on-device.
+* Host `.jar`: For use on the host machine (`junit_binary` / `java_binary`).
+  * Host `.jar` files live in `lib.java/` so that they are archived in
+    builder/tester bots (which do not archive `obj/`).
+
 ## From Source to Final Dex
 
 ### Step 1: Create interface .jar with turbine or ijar
@@ -72,15 +78,24 @@ This step can be disabled via GN arg: `use_errorprone_java_compiler = false`
 [ErrorProne]: https://errorprone.info/
 [ep_plugins]: /tools/android/errorprone_plugin/
 
-### Step 3: Desugaring
+### Step 3: Desugaring (Device .jar Only)
 
-This step happens only when targets have `supports_android = true`.
+This step happens only when targets have `supports_android = true`. It is not
+applied to `.jar` files used by `junit_binary`.
 
 * `//third_party/bazel/desugar` converts certain Java 8 constructs, such as
   lambdas and default interface methods, into constructs that are compatible
   with Java 7.
 
-### Step 4: Filtering
+### Step 4: Instrumenting (Device .jar Only)
+
+This step happens only when this GN arg is set: `use_jacoco_coverage = true`
+
+* [Jacoco] adds instrumentation hooks to methods.
+
+[Jacoco]: https://www.eclemma.org/jacoco/
+
+### Step 5: Filtering
 
 This step happens only when targets that have `jar_excluded_patterns` or
 `jar_included_patterns` set (e.g. all `android_` targets).
@@ -97,27 +112,12 @@ This step happens only when targets that have `jar_excluded_patterns` or
 [Android Resources]: life_of_a_resource.md
 [apphooks]: /chrome/android/java/src/org/chromium/chrome/browser/AppHooksImpl.java
 
-### Step 5: Instrumentation
-
-This step happens only when this GN arg is set: `use_jacoco_coverage = true`
-
-* [Jacoco] adds instrumentation hooks to methods.
-
-[Jacoco]: https://www.eclemma.org/jacoco/
-
-### Step 6: Copy to lib.java
-
-* The `.jar` is copied into `$root_build_dir/lib.java` (under target-specific
-  subdirectories) so that it will be included by bot archive steps.
-  * These `.jar` files are the ones used when running `java_binary` and
-    `junit_binary` targets.
-
-### Step 7: Per-Library Dexing
+### Step 6: Per-Library Dexing
 
 This step happens only when targets have `supports_android = true`.
 
 * [d8] converts `.jar` files containing `.class` files into `.dex.jar` files
-  containing `.dex` files.
+  containing `classes.dex` files.
 * Dexing is incremental - it will reuse dex'ed classes from a previous build if
   the corresponding `.class` file is unchanged.
 * These per-library `.dex.jar` files are used directly by [incremental install],
@@ -128,7 +128,7 @@ This step happens only when targets have `supports_android = true`.
 [d8]: https://developer.android.com/studio/command-line/d8
 [incremental install]: /build/android/incremental_install/README.md
 
-### Step 8: Apk / Bundle Module Compile
+### Step 7: Apk / Bundle Module Compile
 
 * Each `android_apk` and `android_bundle_module` template has a nested
   `java_library` target. The nested library includes final copies of files
@@ -139,7 +139,7 @@ This step happens only when targets have `supports_android = true`.
 
 [JNI glue]: /base/android/jni_generator/README.md
 
-### Step 9: Final Dexing
+### Step 8: Final Dexing
 
 This step is skipped when building using [Incremental Install].
 
@@ -149,19 +149,11 @@ When `is_java_debug = true`:
 When `is_java_debug = false`:
 * [R8] performs whole-program optimization on all library `lib.java` `.jar`
   files and outputs a final `.r8dex.jar`.
-  * For App Bundles, R8 creates a single `.r8dex.jar` with the code from all
-    modules.
+  * For App Bundles, R8 creates a `.r8dex.jar` for each module.
 
 [Incremental Install]: /build/android/incremental_install/README.md
 [R8]: https://r8.googlesource.com/r8
 
-### Step 10: Bundle Module Dex Splitting
-
-This step happens only when `is_java_debug = false`.
-
-* [dexsplitter.py] splits the single `*dex.jar` into per-module `*dex.jar`
-  files.
-
 ## Test APKs with apk_under_test
 
 Test APKs are normal APKs that contain an `<instrumentation>` tag within their

build/android/gyp/dist_aar.py

@@ -14,7 +14,7 @@ import sys
 import tempfile
 import zipfile
 
-from filter_zip import CreatePathTransform
+import filter_zip
 from util import build_utils
 
 
@@ -117,8 +117,8 @@ def main(args):
         build_utils.AddToZipHermetic(
             z, 'AndroidManifest.xml', src_path=options.android_manifest)
 
-        path_transform = CreatePathTransform(options.jar_excluded_globs,
-                                             options.jar_included_globs, [])
+        path_transform = filter_zip.CreatePathTransform(
+            options.jar_excluded_globs, options.jar_included_globs, [])
         with tempfile.NamedTemporaryFile() as jar_file:
           build_utils.MergeZips(
               jar_file.name, options.jars, path_transform=path_transform)

build/android/gyp/filter_zip.py

@@ -5,6 +5,7 @@
 # found in the LICENSE file.
 
 import argparse
+import shutil
 import sys
 
 from util import build_utils
@@ -20,6 +21,21 @@ _RESOURCE_CLASSES = [
 
 def CreatePathTransform(exclude_globs, include_globs,
                         strip_resource_classes_for):
+  """Returns a function to strip paths for the given patterns.
+
+  Args:
+    exclude_globs: List of globs that if matched should be excluded.
+    include_globs: List of globs that if not matched should be excluded.
+    strip_resource_classes_for: List of Java packages for which to strip
+       R.java classes from.
+
+  Returns:
+    * None if no filters are needed.
+    * A function "(path) -> path" that returns None when |path| should be
+          stripped, or |path| otherwise.
+  """
+  if not (exclude_globs or include_globs or strip_resource_classes_for):
+    return None
   exclude_globs = list(exclude_globs or [])
   if strip_resource_classes_for:
     exclude_globs.extend(p.replace('.', '/') + '/' + f
@@ -52,19 +68,18 @@ def main():
   argv = build_utils.ExpandFileArgs(sys.argv[1:])
   args = parser.parse_args(argv)
 
-  if args.exclude_globs:
-    args.exclude_globs = build_utils.ParseGnList(args.exclude_globs)
-  if args.include_globs:
-    args.include_globs= build_utils.ParseGnList(args.include_globs)
-  if args.strip_resource_classes_for:
-    args.strip_resource_classes_for = build_utils.ParseGnList(
-        args.strip_resource_classes_for)
+  args.exclude_globs = build_utils.ParseGnList(args.exclude_globs)
+  args.include_globs = build_utils.ParseGnList(args.include_globs)
+  args.strip_resource_classes_for = build_utils.ParseGnList(
+      args.strip_resource_classes_for)
 
   path_transform = CreatePathTransform(args.exclude_globs, args.include_globs,
                                        args.strip_resource_classes_for)
   with build_utils.AtomicOutput(args.output) as f:
-    build_utils.MergeZips(
-        f.name, [args.input], path_transform=path_transform)
+    if path_transform:
+      build_utils.MergeZips(f.name, [args.input], path_transform=path_transform)
+    else:
+      shutil.copy(args.input, f.name)
 
 
 if __name__ == '__main__':

build/config/android/copy_ex.gni

@@ -28,6 +28,7 @@ template("copy_ex") {
                            [
                              "data",
                              "deps",
+                             "public_deps",
                              "testonly",
                              "visibility",
                            ])

build/config/android/rules.gni

@@ -1624,7 +1624,7 @@ if (enable_java_templates) {
         } else if (_use_unprocessed_jars) {
           args += [ "--input-zips=@FileArg($_rebased_build_config:deps_info:javac_full_classpath)" ]
         } else {
-          args += [ "--input-zips=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)" ]
+          args += [ "--input-zips=@FileArg($_rebased_build_config:deps_info:device_classpath)" ]
         }
       }
       _excludes = []
@@ -2139,8 +2139,7 @@ if (enable_java_templates) {
   template("android_apk_or_module") {
     forward_variables_from(invoker, [ "testonly" ])
     assert(defined(invoker.android_manifest))
-    _out_dir = "$target_out_dir/$target_name"
-    _base_path = "$_out_dir/$target_name"
+    _base_path = "$target_out_dir/$target_name/$target_name"
     _build_config = "$target_gen_dir/$target_name.build_config"
     _build_config_target = "$target_name$build_config_target_suffix"
 
@@ -2153,9 +2152,6 @@ if (enable_java_templates) {
       _target_sdk_version = invoker.target_sdk_version
     }
 
-    # JUnit tests use resource zip files. These must not be put in gen/
-    # directory or they will not be available to tester bots.
-    _jar_path = "$_base_path.jar"
     _template_name = target_name
 
     _is_bundle_module =
@@ -2352,8 +2348,7 @@ if (enable_java_templates) {
     _is_static_library_provider =
         defined(invoker.static_library_dependent_targets) && _proguard_enabled
     if (_is_static_library_provider) {
-      _static_library_sync_dex_path =
-          "$_out_dir/static_library_synchronized_proguard.r8dex.jar"
+      _static_library_sync_dex_path = "$_base_path.synchronized.r8dex.jar"
       _resource_ids_provider_deps = []
       foreach(_target, invoker.static_library_dependent_targets) {
         if (_target.is_resource_ids_provider) {
@@ -2800,7 +2795,6 @@ if (enable_java_templates) {
       supports_android = true
       requires_android = true
       srcjar_deps = _srcjar_deps
-      final_jar_path = _jar_path
       if (defined(_final_dex_path)) {
         final_dex_path = _final_dex_path
       }
@@ -2932,7 +2926,8 @@ if (enable_java_templates) {
             main_dex_list_input_classes_filearg = "@FileArg(${_rebased_build_config}:deps_info:java_runtime_classpath_extended)"
           }
         } else {
-          input_classes_filearg = "@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)"
+          input_classes_filearg =
+              "@FileArg($_rebased_build_config:deps_info:device_classpath)"
         }
 
         if (_is_static_library_provider) {

tools/android/errorprone_plugin/BUILD.gn

@@ -4,7 +4,11 @@
 
 import("//build/config/android/rules.gni")
 
-java_library("errorprone_plugin_java") {
+java_binary("errorprone_plugin") {
+  # main_class and wrapper script are not actually used.
+  # This target is referenced directly from java_library_impl().
+  main_class = "<ignore>"
+  wrapper_script_name = "bin/helper/errorprone_plugin"
   sources = [
     # Turned off because of existing code which fails the check
     # "src/org/chromium/tools/errorprone/plugin/NoContextGetApplicationContext.java",
@@ -21,9 +25,6 @@ java_library("errorprone_plugin_java") {
   enable_errorprone = false
   enable_bytecode_checks = false
 
-  # So that we don't need to inject jacoco runtime into the compiler's classpath.
-  jacoco_never_instrument = true
-
   annotation_processor_deps =
       [ "//third_party/android_deps:auto_service_processor" ]
   deps = [