android: build: Simplify chrome/monochrome GN templates.

This CL simplifies and refactors a bit the GN templates
used to build the [mono]chrome[_modern]_public_XXX templates
(including APKs and bundles).

- Add build/android/docs/android_app_bundles.md to better
  explain what is going on here.

- Unsynchronized proguarding of application bundles was
  broken by the CL that introduced synchronized proguarding.
  This CL fixes it by simply making synchronized proguarding
  the default for all app bundles (when proguard_enabled is
  true). It doesn't really make sense to support regular
  proguarding, since the chances that something wrong will
  happen are too high.

  This also reduces the number of build state variants that
  need to be supported for an individual bundle module.

- The chrome_public_apk_tmpl_shared() GN template defined in
  chrome/android/BUILD.gn was not very useful and only serves
  to add some dependencies. Remove it and adjust all callers
  to add the equivalent dependency list to their |deps|
  variable instead, to simplify things.

- Remove chrome_modern_public_apk_or_module_tmpl() entirely.
  Instead, add an |is_modern| variable to
  chrome_public_apk_or_module_tmpl() that takes care of adding
  the necessary declarations corresponding to modern builds
  to the template.

- Hard-code version_name to chrome_version in
  chrome_public_apk_or_module_tmpl() since all callers define
  it to |chrome_version| anyway.

- rules.gni: Add missing variable forward, required for
  the clank/ targets only.

- Rename chrome_public_apk_tmpl() to
  chrome_public_common_apk_or_module_tmpl() to match its
  intended usage. Note that chrome_public_apk_tmpl()
  wrappers are still available to ensure the clank/ build
  doesn't break.

  Similarly, define monochrome_public_common_apk_or_module_tmpl()
  (yes, a bit long, but really only internal).

- Add documentation to various intermediate GN templates to
  make it easier to understand what's going on.

BUG=865108
R=agrieve@chromium.org, tiborg@chromium.org, yfriedman@chromium.org

Change-Id: Ia4f79429a8e32c97630c90ce126362eb7534252c
Reviewed-on: https://chromium-review.googlesource.com/1154979
Commit-Queue: David Turner <digit@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580522}

build/android/docs/android_app_bundles.md

+# Introduction
+
+This document describes how the Chromium build system supports Android app
+bundles.
+
+[TOC]
+
+# Overview of app bundles
+
+An Android app bundle is an alternative application distribution format for
+Android applications on the Google Play Store, that allows reducing the size
+of binaries sent for installation to individual devices that run on Android L
+and beyond. For more information about them, see the official Android
+documentation at:
+
+  https://developer.android.com/guide/app-bundle/
+
+For the context of this document, the most important points are:
+
+  - Unlike a regular APK (e.g. `foo.apk`), the bundle (e.g. `foo.aab`) cannot
+    be installed directly on a device.
+
+  - Instead, it must be processed into a set of installable split APKs, which
+    are stored inside a special zip archive (e.g. `foo.apks`).
+
+  - The splitting can be based on various criteria: e.g. language or screen
+    density for resources, or cpu ABI for native code.
+
+  - The bundle also uses the notion of modules to separate several application
+    features. Each module has its own code, assets and resources, and can be
+    installed separately from the rest of the application if needed.
+
+  - The main application itself is stored in the '`base`' module (this name
+    cannot be changed).
+
+
+# Declaring app bundles with GN templates
+
+Here's an example that shows how to declare a simple bundle that contains
+a single base module, which enables language-based splits:
+
+```gn
+
+  # First declare the first bundle module. The base module is the one
+  # that contains the main application's code, resources and assets.
+  android_app_bundle_module("foo_base_module") {
+    # Declaration are similar to android_apk here.
+    ...
+  }
+
+  # Second, declare the bundle itself.
+  android_app_bundle("foo_bundle") {
+    # Indicate the base module to use for this bundle
+    base_module_target = ":foo_base_module"
+
+    # The name of our bundle file (without any suffix). Default would
+    # be 'foo_bundle' otherwise.
+    bundle_name = "FooBundle"
+
+    # Signing your bundle is required to upload it to the Play Store
+    # but since signing is very slow, avoid doing it for non official
+    # builds. Signing the bundle is not required for local testing.
+    sign_bundle = is_official_build
+
+    # Enable language-based splits for this bundle. Which means that
+    # resources and assets specific to a given language will be placed
+    # into their own split APK in the final .apks archive.
+    enable_language_splits = true
+
+    # Proguard settings must be passed at the bundle, not module, target.
+    proguard_enabled = !is_java_debug
+  }
+```
+
+When generating the `foo_bundle` target with Ninja, you will end up with
+the following:
+
+  - The bundle file under `out/Release/apks/FooBundle.aab`
+
+  - A helper script called `out/Release/bin/foo_bundle`, which can be used
+    to install / launch / uninstall the bundle on local devices.
+
+    This works like an APK wrapper script (e.g. `foo_apk`). Use `--help`
+    to see all possible commands supported by the script.
+
+If you need more modules besides the base one, you will need to list all the
+extra ones using the extra_modules variable which takes a list of GN scopes,
+as in:
+
+```gn
+
+  android_app_bundle_module("foo_base_module") {
+    ...
+  }
+
+  android_app_bundle_module("foo_extra_module") {
+    ...
+  }
+
+  android_app_bundle("foo_bundle") {
+    base_module_target = ":foo_base_module"
+
+    extra_modules = [
+      { # NOTE: Scopes require one field per line, and no comma separators.
+        name = "my_module"
+        module_target = ":foo_extra_module"
+      }
+    ]
+
+    ...
+  }
+```
+
+Note that each extra module is identified by a unique name, which cannot
+be '`base`'.
+
+
+# Bundle signature issues
+
+Signing an app bundle is not necessary, unless you want to upload it to the
+Play Store. Since this process is very slow (it uses `jarsigner` instead of
+the much faster `apkbuilder`), you can control it with the `sign_bundle`
+variable, as described in the example above.
+
+The `.apks` archive however always contains signed split APKs. The keystore
+path/password/alias being used are the default ones, unless you use custom
+values when declaring the bundle itself, as in:
+
+```gn
+  android_app_bundle("foo_bundle") {
+    ...
+    keystore_path = "//path/to/keystore"
+    keystore_password = "K3y$t0Re-Pa$$w0rd"
+    keystore_name = "my-signing-key-name"
+  }
+```
+
+These values are not stored in the bundle itself, but in the wrapper script,
+which will use them to generate the `.apks` archive for you. This allows you
+to properly install updates on top of existing applications on any device.
+
+
+# Proguard and bundles
+
+When using an app bundle that is made of several modules, it is crucial to
+ensure that proguard, if enabled:
+
+- Keeps the obfuscated class names used by each module consistent.
+- Does not remove classes that are not used in one module, but referenced
+  by others.
+
+To achieve this, a special scheme called *synchronized proguarding* is
+performed, which consists of the following steps:
+
+- The list of unoptimized .jar files from all modules are sent to a single
+  proguard command. This generates a new temporary optimized *group* .jar file.
+
+- Each module extracts the optimized class files from the optimized *group*
+  .jar file, to generate its own, module-specific, optimized .jar.
+
+- Each module-specific optimized .jar is then sent to dex generation.
+
+This synchronized proguarding step is added by the `android_app_bundle()` GN
+template. In practice this means the following:
+
+  - If `proguard_enabled` and `proguard_jar_path` must be passed to
+    `android_app_bundle` targets, but not to `android_app_bundle_module` ones.
+
+  - `proguard_configs` can be still passed to individual modules, just
+    like regular APKs. All proguard configs will be merged during the
+    synchronized proguard step.
+
+
+# Manual generation and installation of .apks archives
+
+Note that the `foo_bundle` script knows how to generate the .apks archive
+from the bundle file, and install it to local devices for you. For example,
+to install and launch a bundle, use:
+
+```sh
+  out/Release/bin/foo_bundle run
+```
+
+If you want to manually look or use the `.apks` archive, use the following
+command to generate it:
+
+```sh
+  out/Release/bin/foo_bundle build-bundle-apks \
+      --output-apks=/tmp/BundleFoo.apks
+```
+
+All split APKs within the archive will be properly signed. And you will be
+able to look at its content (with `unzip -l`), or install it manually with:
+
+```sh
+  build/android/gyp/bundletool.py install-apks \
+      --apks=/tmp/BundleFoo.apks \
+      --adb=$(which adb)
+```

build/android/gyp/write_build_config.py

@@ -499,12 +499,6 @@ invoking `javac`.
 
 This type corresponds to an Android app bundle (`.aab` file).
 
-* `deps_info['synchronized_proguard_enabled"]`:
-True to indicate that the app modules of this bundle should be proguarded in a
-single synchronized proguarding step. Synchronized proguarding means that all
-un-optimized jars for all modules are sent to a single proguard command. The
-resulting optimized jar is then split into optimized app module jars after.
-
 --------------- END_MARKDOWN ---------------------------------------------------
 """
 
@@ -862,8 +856,6 @@ def main(argv):
       'test apk).')
   parser.add_option('--proguard-enabled', action='store_true',
       help='Whether proguard is enabled for this apk or bundle module.')
-  parser.add_option('--synchronized-proguard-enabled', action='store_true',
-      help='Whether synchronized proguarding is enabled for this bundle.')
   parser.add_option('--proguard-configs',
       help='GN-list of proguard flag files to use in final apk.')
   parser.add_option('--proguard-output-jar-path',
@@ -1253,16 +1245,6 @@ def main(argv):
         raise Exception('Deps %s have proguard enabled while deps %s have '
                         'proguard disabled' % (deps_proguard_enabled,
                                                deps_proguard_disabled))
-      if (bool(deps_proguard_enabled) !=
-              bool(options.synchronized_proguard_enabled) or
-          bool(deps_proguard_disabled) ==
-              bool(options.synchronized_proguard_enabled)):
-        raise Exception('Modules %s of bundle %s have opposite proguard '
-                        'enabling flags than bundle' % (deps_proguard_enabled +
-                            deps_proguard_disabled, config['deps_info']['name'])
-                        )
-      deps_info['synchronized_proguard_enabled'] = bool(
-          options.synchronized_proguard_enabled)
     else:
       deps_info['proguard_enabled'] = bool(options.proguard_enabled)
       if options.proguard_output_jar_path:

build/config/android/internal_rules.gni

@@ -376,10 +376,6 @@ template("write_build_config") {
     if (defined(invoker.proguard_enabled) && invoker.proguard_enabled) {
       args += [ "--proguard-enabled" ]
     }
-    if (defined(invoker.synchronized_proguard_enabled) &&
-        invoker.synchronized_proguard_enabled) {
-      args += [ "--synchronized-proguard-enabled" ]
-    }
     if (defined(invoker.proguard_output_jar_path)) {
       _rebased_proguard_output_jar_path =
           rebase_path(invoker.proguard_output_jar_path, root_build_dir)

build/config/android/rules.gni

@@ -2387,20 +2387,10 @@ if (enable_java_templates) {
       }
     }
 
-    # When using an app bundle that is made of several modules, it is crucial to
-    # ensure that the obfuscated class names used by each module are consistent.
-    #
-    # To achieve this, all modules belonging to the same app bundle must
-    # be grouped into a single .jar, that will be proguarded all at once. This
-    # is called "synchronized proguarding", and ensures that the obfuscated
-    # names of all classes remain consistent across modules.
-    #
-    # Later, this single optimized .jar is processed to generate module-specific
-    # .jar files that only contain the obfuscated classes needed by the module
-    # itself.
-    #
-    # This synchronized proguarding step is added by the bundle target.
-    if (!_is_bundle_module || !_proguard_enabled) {
+    # Dex generation for app bundle modules with proguarding enabled takes
+    # place later due to synchronized proguarding. For more details,
+    # read build/android/docs/android_app_bundles.md
+    if (!(_is_bundle_module && _proguard_enabled)) {
       if (_proguard_enabled) {
         _proguard_target = "${_template_name}__proguard"
         proguard(_proguard_target) {
@@ -2486,6 +2476,13 @@ if (enable_java_templates) {
         use_pool = true
       }
     } else {
+      # A small sanity check to help developers with a subtle point!
+      assert(
+          !defined(invoker.proguard_jar_path),
+          "proguard_jar_path should not be used for app bundle modules " +
+              "when proguard is enabled. Pass it to the android_app_bundle() " +
+              "target instead!")
+
       _final_deps += [ ":$_java_target" ]
     }
 
@@ -3655,13 +3652,12 @@ if (enable_java_templates) {
   #      APKs use 'chrome-command-line', the WebView one uses
   #      'webview-command-line').
   #
-  #    synchronized_proguard_enabled: True if synchronized proguarding is
-  #      enabled for this bundle. WARNING: All modules that contain Java must
-  #      have proguarding enabled if this is true and disabled if this is false.
+  #    proguard_enabled: Optional. True if proguarding is enabled for this
+  #      bundle. Default is to enable this only for release builds. Note that
+  #      this will always perform synchronized proguarding.
   #
-  #    proguard_jar_path: Path to custom proguard jar used for synchronized
-  #      proguarding. WARNING: Can only be set if
-  #      |synchronized_proguard_enabled| is set to true.
+  #    proguard_jar_path: Optional. Path to custom proguard jar used for
+  #      proguarding.
   #
   # Example:
   #   android_app_bundle("chrome_public_bundle") {
@@ -3697,9 +3693,8 @@ if (enable_java_templates) {
       _all_modules += invoker.extra_modules
     }
 
-    _synchronized_proguard_enabled =
-        defined(invoker.synchronized_proguard_enabled) &&
-        invoker.synchronized_proguard_enabled
+    _proguard_enabled =
+        defined(invoker.proguard_enabled) && invoker.proguard_enabled
 
     # Make build config, which is required for synchronized proguarding.
     _module_targets = []
@@ -3713,16 +3708,16 @@ if (enable_java_templates) {
       type = "android_app_bundle"
       possible_config_deps = _module_targets
       build_config = _build_config
-      synchronized_proguard_enabled = _synchronized_proguard_enabled
     }
 
-    if (_synchronized_proguard_enabled) {
+    if (_proguard_enabled) {
       # Proguard all modules together to keep binary size small while still
       # maintaining compatibility between modules.
       _proguard_output_jar_path =
           "${target_gen_dir}/${target_name}/${target_name}.proguard.jar"
       _sync_proguard_target = "${target_name}__sync_proguard"
       proguard(_sync_proguard_target) {
+        forward_variables_from(invoker, [ "proguard_jar_path" ])
         build_config = _build_config
         deps = _module_targets + [ ":$_build_config_target" ]
 
@@ -3731,10 +3726,6 @@ if (enable_java_templates) {
           "--proguard-configs=@FileArg($_rebased_build_config:deps_info:proguard_all_configs)",
           "--input-paths=@FileArg($_rebased_build_config:deps_info:java_runtime_classpath)",
         ]
-
-        if (defined(invoker.proguard_jar_path)) {
-          proguard_jar_path = invoker.proguard_jar_path
-        }
       }
     }
 
@@ -3749,7 +3740,7 @@ if (enable_java_templates) {
       _module_build_config =
           "$_module_target_gen_dir/${_module_target_name}.build_config"
 
-      if (_synchronized_proguard_enabled) {
+      if (_proguard_enabled) {
         # Extract optimized classes for each module and dex them.
 
         _module_final_dex_target = "${target_name}__${_module.name}__dex"

chrome/android/chrome_public_apk_tmpl.gni

@@ -25,7 +25,43 @@ default_chrome_public_jinja_variables = [
   "enable_vr=$enable_vr",
 ]
 
-template("chrome_public_apk_tmpl") {
+# A template used to declare any target that will implement a full Chromium
+# or Chrome application, either as an APK, or an app bundle module.
+#
+# Variables:
+#   target_type: Either 'android_apk' or 'android_app_bundle_module'.
+#   apk_name: For APK target types, the final APK name without a suffix.
+#   module_name: For bundle module target types, the module's name without a
+#     suffix.
+#   is_base_module: For bundle module target types, true iff this is a base
+#     application module, instead of a feature module.
+#   android_manifest: Application manifest path.
+#   android_manifest_dep: Name of target generating the android_manifest.
+#   shared_libraries: List of native shared libraries targets to include in
+#     the final target (e.g. [ ":libchrome" ]).
+#   add_unwind_tables_in_apk: Optional. If true, add the unwind tables to the
+#     final APK or bundle.
+#   is_modern: If true, indicates this corresponds to a chrome_modern_XXX
+#     target that can only run on Android L-M.
+#   png_to_webp: Optional. If true, convert image resources to webp format.
+#     requires Android K+, since these were not supported by Android properly
+#     before 4.3.0.
+#   load_library_from_apk: Optional. If true, native libraries will be loaded
+#     directly from the APK (and stored zipaligned and uncompressed). This
+#     requires either the Chromium linker, or Android M+.
+#   version_name: Application version name (e.g. "Developer Build").
+#   is_modern: Optional. true iff this is a chrome_modern derived build.
+#
+#   Plus all other variables accepted by android_apk() or
+#   android_app_bundle_module(), depending on the target type.
+#
+template("chrome_public_common_apk_or_module_tmpl") {
+  assert(defined(invoker.target_type), "target_type is required!")
+  assert(
+      invoker.target_type == "android_apk" ||
+          invoker.target_type == "android_app_bundle_module",
+      "Invalid target_type definition, should be 'android_apk' or 'android_app_bundle_module'")
+
   # Adds unwind table asset to the chrome apk for the given library target. This
   # is not part of generic apk assets target since it depends on the main shared
   # library of the apk, to extract unwind tables.
@@ -47,6 +83,10 @@ template("chrome_public_apk_tmpl") {
   } else {
     _target_type = invoker.target_type
   }
+
+  _is_modern = defined(invoker.is_modern) && invoker.is_modern
+  assert(_is_modern || !_is_modern)  # Mark as used.
+
   target(_target_type, target_name) {
     forward_variables_from(invoker, "*")
     exclude_xxxhdpi = true
@@ -133,8 +173,10 @@ template("chrome_public_apk_tmpl") {
   }
 }
 
-template("monochrome_public_apk_tmpl") {
-  chrome_public_apk_tmpl(target_name) {
+# The equivalent of chrome_common_apk_or_module_tmpl for all builds of
+# monochrome.
+template("monochrome_public_common_apk_or_module_tmpl") {
+  chrome_public_common_apk_or_module_tmpl(target_name) {
     # Always build 64-bit //android_webview:monochrome because Chrome runs
     # in 32-bit mode.
     if (android_64bit_target_cpu) {
@@ -225,3 +267,21 @@ template("monochrome_public_apk_tmpl") {
     }
   }
 }
+
+# These empty templates are still being called from the clank/ BUILD.gn
+# scripts. Remove them when they have been fixed to call the
+# xxx_common_apk_or_module_tmpl templates above.
+
+template("chrome_public_apk_tmpl") {
+  chrome_public_common_apk_or_module_tmpl(target_name) {
+    forward_variables_from(invoker, "*")
+    target_type = "android_apk"
+  }
+}
+
+template("monochrome_public_apk_tmpl") {
+  monochrome_public_common_apk_or_module_tmpl(target_name) {
+    forward_variables_from(invoker, "*")
+    target_type = "android_apk"
+  }
+}

docs/README.md

@@ -258,6 +258,8 @@ used when committed.
     coverage data with the EMMA tool.
 *   [Android BuildConfig files](../build/android/docs/build_config.md) -
     What are .build_config files and how they are used.
+*   [Android App Bundles](../build/android/docs/android_app_bundles.md) -
+    How to build Android app bundles for Chrome.
 
 ### Misc iOS-Specific Docs
 *   [Continuous Build and Test Infrastructure for Chromium for iOS](ios/infra.md)