Android: Add golden AndroidManifest files.

Purpose is to have manifest changes reviewed so that unexpected changes
can't sneak into the merged manifest (ex. adding an aar dep that needs
extra permissions).

Any deviations from the golden files are printed for release builds
and will fail the build if the GN arg
check_android_configuration = true.

Bug: 882528
Change-Id: I3fee0546004df18ada0fc33077c978edaa64f127
Reviewed-on: https://chromium-review.googlesource.com/c/1409406
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Auto-Submit: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625409}

build/android/gyp/merge_manifest.py

@@ -14,6 +14,7 @@ import tempfile
 import xml.dom.minidom as minidom
 
 from util import build_utils
+from util import diff_utils
 
 # Tools library directory - relative to Android SDK root
 SDK_TOOLS_LIB_DIR = os.path.join('tools', 'lib')
@@ -72,6 +73,12 @@ def main(argv):
   parser.add_argument('--root-manifest',
                       help='Root manifest which to merge into',
                       required=True)
+  parser.add_argument(
+      '--expected-manifest', help='Expected contents for the merged manifest.')
+  parser.add_argument(
+      '--verify-expected-manifest',
+      action='store_true',
+      help='Fail if expected contents do not match merged manifest contents.')
   parser.add_argument('--output', help='Output manifest path', required=True)
   parser.add_argument('--extras',
                       help='GN list of additional manifest to merge')
@@ -101,6 +108,24 @@ def main(argv):
         # The merger doesn't set a nonzero exit code for failures.
         fail_func=lambda returncode, stderr: returncode != 0 or
           build_utils.IsTimeStale(f.name, [root_manifest] + extras))
+
+  if args.expected_manifest:
+    diff = diff_utils.DiffFileContents(args.expected_manifest, args.output)
+    if diff:
+      print """
+{}
+
+Detected AndroidManifest change. Please update by running:
+
+cp {} {}
+
+See https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README.md
+for more info.
+""".format(diff, os.path.abspath(args.output),
+           os.path.abspath(args.expected_manifest))
+      if args.verify_expected_manifest:
+        sys.exit(1)
+
   if args.depfile:
     inputs = extras + classpath.split(':')
     build_utils.WriteDepfile(args.depfile, args.output, inputs=inputs,

build/android/gyp/merge_manifest.pydeps

@@ -4,4 +4,5 @@
 merge_manifest.py
 util/__init__.py
 util/build_utils.py
+util/diff_utils.py
 util/md5_check.py

build/android/gyp/proguard.py

@@ -112,7 +112,7 @@ Detected Proguard flags change. Please update by running:
 
 cp {} {}
 
-See https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README.md#fixing-build-failures
+See https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README.md
 for more info.
 """.format(diff, os.path.abspath(actual_path), os.path.abspath(expected_path))
   if fail_on_exit:

build/config/android/internal_rules.gni

@@ -1748,6 +1748,17 @@ if (enable_java_templates) {
         "--extras",
         "@FileArg($_rebased_build_config:extra_android_manifests)",
       ]
+
+      if (defined(invoker.expected_manifest)) {
+        inputs += [ invoker.expected_manifest ]
+        args += [
+          "--expected-manifest",
+          rebase_path(invoker.expected_manifest, root_build_dir),
+        ]
+        if (check_android_configuration) {
+          args += [ "--verify-expected-manifest" ]
+        }
+      }
     }
   }
 

build/config/android/rules.gni

@@ -1951,8 +1951,8 @@ if (enable_java_templates) {
   #     resources.arsc file in the apk or module.
   #   resources_config_path: Path to the aapt2 optimize config file that tags
   #     resources with acceptable/non-acceptable optimizations.
-  #   proguard_expectations_file: Path to generated file containing the merged
-  #     proguard flags from all input config files.
+  #   verify_android_configuration: Enables verification of expected merged
+  #     manifest and proguard flags based on a golden file.
   template("android_apk_or_module") {
     forward_variables_from(invoker, [ "testonly" ])
 
@@ -2165,6 +2165,13 @@ if (enable_java_templates) {
       _incremental_install_json_path = "$root_out_dir/gen.runtime/$_target_dir_name/$target_name.incremental.json"
     }
 
+    _verify_android_configuration =
+        defined(invoker.verify_android_configuration) &&
+        invoker.verify_android_configuration && !is_java_debug
+    if (_verify_android_configuration) {
+      _target_src_dir = get_label_info(":$target_name", "dir")
+    }
+
     _android_manifest =
         "$target_gen_dir/${_template_name}_manifest/AndroidManifest.xml"
     _merge_manifest_target = "${_template_name}__merge_manifests"
@@ -2172,6 +2179,10 @@ if (enable_java_templates) {
       input_manifest = _android_root_manifest
       output_manifest = _android_manifest
       build_config = _build_config
+      if (_verify_android_configuration) {
+        expected_manifest =
+            "$_target_src_dir/java/$_template_name.AndroidManifest.expected"
+      }
       deps = _android_root_manifest_deps + [ ":$_build_config_target" ]
     }
 
@@ -2568,7 +2579,6 @@ if (enable_java_templates) {
                                [
                                  "min_sdk_version",
                                  "dexlayout_profile",
-                                 "proguard_expectations_file",
                                ])
         proguard_enabled = _proguard_enabled
         build_config = _build_config
@@ -2581,6 +2591,11 @@ if (enable_java_templates) {
           deps += _deps + [ ":$_compile_resources_target" ]
           proguard_configs = [ _jar_path ]
           proguard_mapping_path = _proguard_mapping_path
+          if (!defined(invoker.proguard_jar_path) &&
+              _verify_android_configuration) {
+            proguard_expectations_file =
+                "$_target_src_dir/java/$_template_name.proguard_flags.expected"
+          }
         } else {
           input_jars = [ _lib_dex_path ]
           input_dex_classpath =
@@ -2967,7 +2982,7 @@ if (enable_java_templates) {
                                "product_version_resources_dep",
                                "proguard_configs",
                                "proguard_enabled",
-                               "proguard_expectations_file",
+                               "verify_android_configuration",
                                "proguard_jar_path",
                                "resource_blacklist_regex",
                                "resource_blacklist_exceptions",

chrome/android/BUILD.gn

@@ -1545,7 +1545,7 @@ template("monochrome_public_apk_or_module_tmpl") {
                              "apk_name",
                              "is_base_module",
                              "module_name",
-                             "proguard_expectations_file",
+                             "verify_android_configuration",
                              "proguard_jar_path",
                              "target_type",
                              "use_trichrome_library",
@@ -1587,11 +1587,10 @@ monochrome_public_apk_or_module_tmpl("monochrome_public_apk") {
   apk_name = "MonochromePublic"
   target_type = "android_apk"
 
-  # Having //clank present causes different flags because of how play services
-  # is wired up.
-  if (!is_java_debug && !enable_chrome_android_internal) {
-    proguard_expectations_file =
-        "//chrome/android/java/monochrome_public_apk.proguard_flags.expected"
+  # Having //clank present causes different flags because of how play
+  # services is wired up.
+  if (!enable_chrome_android_internal) {
+    verify_android_configuration = true
   }
 }
 

chrome/android/OWNERS

@@ -12,6 +12,7 @@ per-file java_sources.gni=*
 per-file BUILD.gn=bsheedy@chromium.org
 
 per-file *.proguard_flags.expected=*
+per-file *.AndroidManifest.expected=*
 
 # Translation artifacts:
 per-file *.xtb=file://tools/translation/TRANSLATION_OWNERS

chrome/android/java/README.md

-## //chrome/android/java/*proguard_flags.expected files
+# //chrome/android/java/*.expected files
 
-### Proguard flags
+## Proguard flags
 
 [Proguard](https://www.guardsquare.com/en/products/proguard) is used in the
 build to obfuscate and minify Java code.
@@ -23,7 +23,7 @@ For example, the following rule specifies that all public classes with a
 contains all proguard configs used when building MonochromePublic.apk, and is
 generated by the `proguard()` build step.
 
-### Why do we care about proguard flag discrepancies?
+### Why do we care about Proguard flag discrepancies?
 
 Some configs are explicitly added ([ex](proguard.flags)) while others are pulled
 in implicitly by GN deps (ex. `aar_prebuilt()` deps). In the later case, these
@@ -36,10 +36,30 @@ regressions. Example of where this has happened before:
 Having checked-in versions of the Proguard configs used allows us to identify
 and address these issues earlier.
 
+## AndroidManifest.xml
+
+Each Android application has a manifest that contains information about the app
+(ex. permissions required, services exposed, etc).
+
+### What are `*.AndroidManifest.expected` files?
+
+[monochrome_public_apk.AndroidManifest.expected](monochrome_public_apk.AndroidManifest.expected)
+contains the contents of the final merged manifest used when building
+MonochromePublic.apk.
+
+### Why do we care about AndroidManifest discrepancies?
+
+While most manifest changes are reviewed when the manifest template file
+changes, manifest entries that are pulled in via. deps (through manifest
+merging) can cause real bugs (permissions issues, security vulnerabilities).
+
+## Build failures caused by `*.expected` files
+
 ### What is the build error telling me?
 
-The build error is indicating that your CL has caused proguard rules to be
-added/removed/changed and that the expected file needs to be updated.
+The build error is indicating that your CL has caused a mismatch between the
+expected file and the generated file and that either the issue requires
+attention or the expected file needs updating.
 
 ### Fixing build failures
 
@@ -56,7 +76,7 @@ autoninja -C $CHROMIUM_OUTPUT_DIR monochrome_public_apk
 ```
 
 3. Run the command suggested in the error message to copy the contents of the
-   generated proguard config file to the expected config file.
+   generated file to the expected file path
 
 4. Add the updated `.expected` file to your CL
 
@@ -67,7 +87,7 @@ autoninja -C $CHROMIUM_OUTPUT_DIR monochrome_public_apk
 
 On the [android-binary-size](https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-binary-size)
 trybot we set `check_android_configuration=true` which causes any differences
-between the expected and generated Proguard configs to fail the build.
+between the expected and generated files to fail the build.
 
 Without this argument the error message is shown but doesn't fail the build.