check_android_configuration no longer fails build

The check_android_configuration gn arg is only used by the
android-binary-size trybot. This cl changes the behavior of the arg to
instead of breaking the build to touch a stamp file that will be looked
for by the trybot. This allows the trybot to calculate the binary size
differences before failing.

Bug: 1011227
Change-Id: I5ac888c32e93264b38da707ad86a46b11c6cbb1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854499
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707868}

build/android/BUILD.gn

@@ -50,6 +50,8 @@ if (enable_java_templates) {
     "android_sdk_version=$android_sdk_version",
     "android_ndk_root=" + rebase_path(android_ndk_root, root_build_dir),
     "android_tool_prefix=" + rebase_path(android_tool_prefix, root_build_dir),
+    "android_configuration_failure_dir=" +
+        rebase_path(android_configuration_failure_dir, root_build_dir),
   ]
   if (defined(android_secondary_abi_cpu)) {
     _secondary_label_info =

build/android/gyp/compile_resources.py

@@ -69,11 +69,9 @@ def _ParseArgs(args):
   input_opts.add_argument(
       '--android-manifest-normalized', help='Normalized manifest.')
   input_opts.add_argument(
-      '--fail-if-unexpected-android-manifest',
-      action='store_true',
-      help=
-      'Fail if expected manifest contents do not match final manifest contents.'
-  )
+      '--android-manifest-expectations-failure-file',
+      help='Write to this file if expected manifest contents do not match '
+      'final manifest contents.')
   input_opts.add_argument(
       '--r-java-root-package-name',
       default='base',
@@ -476,18 +474,26 @@ def _FixManifest(options, temp_dir):
 
 
 def _VerifyManifest(actual_manifest, expected_manifest, normalized_manifest,
-                    fail_if_unexpected_manifest):
+                    unexpected_manifest_failure_file):
   with build_utils.AtomicOutput(normalized_manifest) as normalized_output:
     normalized_output.write(manifest_utils.NormalizeManifest(actual_manifest))
   msg = diff_utils.DiffFileContents(expected_manifest, normalized_manifest)
-  if msg:
-    sys.stderr.write("""\
+  if not msg:
+    return
+
+  msg_header = """\
 AndroidManifest.xml expectations file needs updating. For details see:
 https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README.md
-""")
-    sys.stderr.write(msg)
-    if fail_if_unexpected_manifest:
-      sys.exit(1)
+"""
+  sys.stderr.write(msg_header)
+  sys.stderr.write(msg)
+  if unexpected_manifest_failure_file:
+    build_utils.MakeDirectory(os.path.dirname(unexpected_manifest_failure_file))
+    with open(unexpected_manifest_failure_file, 'w') as f:
+      f.write(msg_header)
+      f.write(msg)
+    #TODO(mheikal): remove once trybot recipe handles failure files.
+    sys.exit(1)
 
 
 def _CreateKeepPredicate(resource_blacklist_regex,
@@ -741,7 +747,7 @@ def _PackageApk(options, build):
   if options.android_manifest_expected:
     _VerifyManifest(fixed_manifest, options.android_manifest_expected,
                     options.android_manifest_normalized,
-                    options.fail_if_unexpected_android_manifest)
+                    options.android_manifest_expectations_failure_file)
 
   link_command += [
       '--manifest', fixed_manifest, '--rename-manifest-package',

build/android/gyp/proguard.py

@@ -93,10 +93,9 @@ def _ParseOptions():
       '--expected-configs-file',
       help='Path to a file containing the expected merged ProGuard configs')
   parser.add_argument(
-      '--verify-expected-configs',
-      action='store_true',
-      help='Fail if the expected merged ProGuard configs differ from the '
-      'generated merged ProGuard configs.')
+      '--proguard-expectations-failure-file',
+      help='Path to file written to if the expected merged ProGuard configs '
+      'differ from the generated merged ProGuard configs.')
   parser.add_argument(
       '--classpath',
       action='append',
@@ -137,17 +136,23 @@ def _ParseOptions():
   return options
 
 
-def _VerifyExpectedConfigs(expected_path, actual_path, fail_on_exit):
+def _VerifyExpectedConfigs(expected_path, actual_path, failure_file_path):
   msg = diff_utils.DiffFileContents(expected_path, actual_path)
   if not msg:
     return
 
-  sys.stderr.write("""\
+  msg_header = """\
 ProGuard flag expectations file needs updating. For details see:
 https://chromium.googlesource.com/chromium/src/+/HEAD/chrome/android/java/README.md
-""")
+"""
+  sys.stderr.write(msg_header)
   sys.stderr.write(msg)
-  if fail_on_exit:
+  if failure_file_path:
+    build_utils.MakeDirectory(os.path.dirname(failure_file_path))
+    with open(failure_file_path, 'w') as f:
+      f.write(msg_header)
+      f.write(msg)
+    #TODO(mheikal): remove once trybot reciepe handles the failure files.
     sys.exit(1)
 
 
@@ -394,7 +399,7 @@ def main():
     if options.expected_configs_file:
       _VerifyExpectedConfigs(options.expected_configs_file,
                              options.output_config,
-                             options.verify_expected_configs)
+                             options.proguard_expectations_failure_file)
 
   if options.r8_path:
     _OptimizeWithR8(options, options.proguard_configs, libraries,

build/config/android/config.gni

@@ -206,6 +206,9 @@ if (is_android || is_chromeos) {
     # Checks that proguard flags have not changed (!is_java_debug only).
     check_android_configuration = false
 
+    # Where to write failed expectations if check_android_configuration is true.
+    android_configuration_failure_dir = "$root_build_dir/failed_expectations"
+
     # Enable the chrome build for devices without touchscreens.
     notouch_build = false
 

build/config/android/internal_rules.gni

@@ -1030,7 +1030,13 @@ if (enable_java_templates) {
             rebase_path(_expected_configs_file, root_build_dir),
           ]
           if (check_android_configuration) {
-            args += [ "--verify-expected-configs" ]
+            _failed_proguard_expectation_file =
+                "$android_configuration_failure_dir/" +
+                "${invoker.failed_proguard_expectation_file}"
+            args += [
+              "--proguard-expectations-failure-file",
+              rebase_path(_failed_proguard_expectation_file, root_build_dir),
+            ]
           }
         }
       }
@@ -1172,6 +1178,7 @@ if (enable_java_templates) {
                                  "build_config",
                                  "disable_r8_outlining",
                                  "deps",
+                                 "failed_proguard_expectation_file",
                                  "proguard_expectations_file",
                                  "proguard_jar_path",
                                  "proguard_mapping_path",
@@ -2400,7 +2407,14 @@ if (enable_java_templates) {
           rebase_path(_normalized_output, root_build_dir),
         ]
         if (check_android_configuration) {
-          args += [ "--fail-if-unexpected-android-manifest" ]
+          _manfiest_expectations_failure_filepath =
+              "$android_configuration_failure_dir/" +
+              "${invoker.failed_manifest_expectation_file}"
+          args += [
+            "--android-manifest-expectations-failure-file",
+            rebase_path(_manfiest_expectations_failure_filepath,
+                        root_build_dir),
+          ]
         }
       }
       if (defined(invoker.manifest_package)) {

build/config/android/rules.gni

@@ -2424,6 +2424,7 @@ if (enable_java_templates) {
                              [
                                "aapt_locale_whitelist",
                                "app_as_shared_lib",
+                               "failed_manifest_expectation_file",
                                "manifest_package",
                                "max_sdk_version",
                                "no_xml_namespaces",
@@ -3467,6 +3468,8 @@ if (enable_java_templates) {
                                "deps",
                                "enable_chromium_linker_tests",
                                "enable_multidex",
+                               "failed_manifest_expectation_file",
+                               "failed_proguard_expectation_file",
                                "generate_buildconfig_java",
                                "generate_final_jni",
                                "input_jars_paths",
@@ -4560,6 +4563,7 @@ if (enable_java_templates) {
           proguard_mapping_path = _proguard_mapping_path
           forward_variables_from(invoker,
                                  [
+                                   "failed_proguard_expectation_file",
                                    "proguard_jar_path",
                                    "min_sdk_version",
                                  ])

chrome/android/BUILD.gn

@@ -1730,6 +1730,8 @@ template("monochrome_public_apk_or_module_tmpl") {
                              "target_type",
                              "use_trichrome_library",
                              "verify_manifest",
+                             "failed_manifest_expectation_file",
+                             "failed_proguard_expectation_file",
                              "version_code",
                              "version_name",
                            ])
@@ -2329,6 +2331,8 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
     if (defined(invoker.verify_android_configuration) &&
         invoker.verify_android_configuration) {
       verify_manifest = true
+      failed_manifest_expectation_file =
+          "monochrome_public_bundle.android_manifest.failed"
     }
 
     if (_is_trichrome && trichrome_synchronized_proguard) {
@@ -2355,6 +2359,8 @@ template("monochrome_or_trichrome_public_bundle_tmpl") {
       if (defined(invoker.verify_android_configuration) &&
           invoker.verify_android_configuration) {
         verify_proguard_flags = true
+        failed_proguard_expectation_file =
+            "monochrome_public_bundle.proguard_flags.failed"
       }
     }
 

chrome/android/chrome_bundle_tmpl.gni

@@ -53,6 +53,7 @@ template("chrome_bundle") {
                              "base_module_target",
                              "bundle_name",
                              "compress_shared_libraries",
+                             "failed_proguard_expectation_file",
                              "keystore_name",
                              "keystore_password",
                              "keystore_path",

chrome/android/chrome_public_apk_tmpl.gni

@@ -326,6 +326,8 @@ template("monochrome_public_common_apk_or_module_tmpl") {
   chrome_public_common_apk_or_module_tmpl(target_name) {
     forward_variables_from(invoker,
                            [
+                             "failed_manifest_expectation_file",
+                             "failed_proguard_expectation_file",
                              "version_code",
                              "verify_manifest",
                            ])