Revert "Reland "Add -noverify to build commands to improve runtime (local builds only)""

This reverts commit a1325695b587ce991a6c4779c9bb4572679bb6a6.

Reason for revert: Android ASAN bot compilation failure. crbug.com/1120202

Original change's description:
> Reland "Add -noverify to build commands to improve runtime (local builds only)"
>
> This reverts commit 17e2a91739763a890f8e80db5689f3bb0b49723a.
>
> Reason for revert: Ensure buildtools.py is a no-op on bots.
>
> Original change's description:
> > Revert "Add -noverify to build commands to improve runtime (local builds only)"
> >
> > This reverts commit 132e1debe3ff6fe2732b8b612534d7c207651777.
> >
> > Reason for revert: Android ASAN bot compilation failures: https://ci.chromium.org/p/chromium/builders/ci/Android%20ASAN%20%28dbg%29/26489
> >
> > Original change's description:
> > > Add -noverify to build commands to improve runtime (local builds only)
> > >
> > > The flag is guarded by treat_warnings_as_errors=false so that it
> > > will apply only to local developer builds.
> > >
> > > Results from //tools/android/build_speed/benchmark.py base_java_sig:
> > > With change:
> > > base_java_sig: 68.0s avg (68.8s, 67.2s)
> > > base_java_sig: 68.4s avg (68.6s, 68.2s)
> > >
> > > Without change:
> > > base_java_sig: 69.4s avg (69.3s, 69.5s)
> > > base_java_sig: 69.7s avg (69.7s, 69.7s)
> > >
> > > time out/Debug/bin/run_base_junit_tests:
> > > With Change:
> > > real 0m22.741s
> > > real 0m23.543s
> > >
> > > Without change:
> > > real 0m23.176s
> > > real 0m24.328s
> > >
> > > Bug: 1117222
> > > Change-Id: I4bfd742165468bf9538e6fa0dc8f35daa9e0f23f
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2313299
> > > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > > Auto-Submit: Andrew Grieve <agrieve@chromium.org>
> > > Reviewed-by: Peter Wen <wnwen@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#799285}
> >
> > TBR=wnwen@chromium.org,agrieve@chromium.org
> >
> > Change-Id: I82f411e5dab89c361a30c33e9103bd8561f19765
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 1117222
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362493
> > Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
> > Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#799471}
>
> Bug: 1117222
> Change-Id: Ib43bfe72869def7da43b237360df0e155149a963
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363975
> Reviewed-by: Peter Wen <wnwen@chromium.org>
> Commit-Queue: Peter Wen <wnwen@chromium.org>
> Auto-Submit: Andrew Grieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#800233}

TBR=wnwen@chromium.org,agrieve@chromium.org

Change-Id: I0d617bcfc7a311bfae743445fccaaab26364511e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1117222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368541
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800435}
GitOrigin-RevId: f05ab16d76cbf8d7372a3a4a1a94c80c3f1c97bc

android/gyp/apkbuilder.py

@@ -119,9 +119,6 @@ def _ParseArgs(args):
       '--library-renames',
       action='append',
       help='The list of library files that we prepend crazy. to their names.')
-  parser.add_argument('--warnings-as-errors',
-                      action='store_true',
-                      help='Treat all warnings as errors.')
   diff_utils.AddCommandLineFlags(parser)
   options = parser.parse_args(args)
   options.assets = build_utils.ParseGnList(options.assets)
@@ -539,15 +536,9 @@ def main(args):
 
     if options.format == 'apk':
       zipalign_path = None if fast_align else options.zipalign_path
-      finalize_apk.FinalizeApk(options.apksigner_jar,
-                               zipalign_path,
-                               f.name,
-                               f.name,
-                               options.key_path,
-                               options.key_passwd,
-                               options.key_name,
-                               int(options.min_sdk_version),
-                               warnings_as_errors=options.warnings_as_errors)
+      finalize_apk.FinalizeApk(options.apksigner_jar, zipalign_path, f.name,
+                               f.name, options.key_path, options.key_passwd,
+                               options.key_name, int(options.min_sdk_version))
     logging.debug('Moving file into place')
 
     if options.depfile:

android/gyp/bundletool.py

@@ -24,16 +24,11 @@ BUNDLETOOL_VERSION = '0.13.3'
 BUNDLETOOL_JAR_PATH = os.path.join(
     BUNDLETOOL_DIR, 'bundletool-all-%s.jar' % BUNDLETOOL_VERSION)
 
-
-def RunBundleTool(args, warnings_as_errors=()):
-  # Use () instead of None because command-line flags are None by default.
-  verify = warnings_as_errors == () or warnings_as_errors
-  cmd = build_utils.JavaCmd(verify)
-  cmd += ['-jar', BUNDLETOOL_JAR_PATH]
-  cmd += args
-  logging.debug(' '.join(cmd))
+def RunBundleTool(args):
+  args = [build_utils.JAVA_PATH, '-jar', BUNDLETOOL_JAR_PATH] + args
+  logging.debug(' '.join(args))
   return build_utils.CheckOutput(
-      cmd,
+      args,
       print_stderr=True,
       fail_on_output=False,
       stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings)

android/gyp/compile_java.py

@@ -395,7 +395,8 @@ def _OnStaleMd5(options, javac_cmd, javac_args, java_files):
       raise Exception('--enable-kythe-annotations requires '
                       'KYTHE_ROOT_DIRECTORY and KYTHE_OUTPUT_DIRECTORY '
                       'environment variables to be set.')
-    javac_extractor_cmd = build_utils.JavaCmd() + [
+    javac_extractor_cmd = [
+        build_utils.JAVA_PATH,
         '-jar',
         _JAVAC_EXTRACTOR,
     ]

android/gyp/create_app_bundle.py

@@ -98,9 +98,6 @@ def _ParseArgs(args):
       'listed there _and_ in --base-module-rtxt-path will '
       'be kept in the base bundle module, even if language'
       ' splitting is enabled.')
-  parser.add_argument('--warnings-as-errors',
-                      action='store_true',
-                      help='Treat all warnings as errors.')
 
   parser.add_argument('--keystore-path', help='Keystore path')
   parser.add_argument('--keystore-password', help='Keystore password')
@@ -421,21 +418,17 @@ def main(args):
     with open(tmp_bundle_config, 'w') as f:
       f.write(bundle_config)
 
-    cmd_args = build_utils.JavaCmd(options.warnings_as_errors) + [
-        '-jar',
-        bundletool.BUNDLETOOL_JAR_PATH,
-        'build-bundle',
-        '--modules=' + ','.join(module_zips),
-        '--output=' + tmp_unsigned_bundle,
-        '--config=' + tmp_bundle_config,
+    cmd_args = [
+        build_utils.JAVA_PATH, '-jar', bundletool.BUNDLETOOL_JAR_PATH,
+        'build-bundle', '--modules=' + ','.join(module_zips),
+        '--output=' + tmp_unsigned_bundle, '--config=' + tmp_bundle_config
     ]
 
     build_utils.CheckOutput(
         cmd_args,
         print_stdout=True,
         print_stderr=True,
-        stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings,
-        fail_on_output=options.warnings_as_errors)
+        stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings)
 
     if options.keystore_path:
       # NOTE: As stated by the public documentation, apksigner cannot be used
@@ -450,9 +443,7 @@ def main(args):
           tmp_unsigned_bundle,
           options.key_name,
       ]
-      build_utils.CheckOutput(signing_cmd_args,
-                              print_stderr=True,
-                              fail_on_output=options.warnings_as_errors)
+      build_utils.CheckOutput(signing_cmd_args, print_stderr=True)
 
     shutil.move(tmp_bundle, options.out_bundle)
 

android/gyp/create_java_binary_script.py

@@ -91,8 +91,7 @@ def main(argv):
 
   run_dir = os.path.dirname(options.output)
   classpath = [os.path.relpath(p, run_dir) for p in classpath]
-  java_path = os.path.relpath(
-      os.path.join(build_utils.JAVA_HOME, 'bin', 'java'), run_dir)
+  java_path = os.path.relpath(build_utils.JAVA_PATH, run_dir)
 
   with build_utils.AtomicOutput(options.output) as script:
     script.write(

android/gyp/desugar.py

@@ -25,15 +25,13 @@ def main():
                       help='Classpath.')
   parser.add_argument('--bootclasspath', required=True,
                       help='Path to javac bootclasspath interface jar.')
-  parser.add_argument('--warnings-as-errors',
-                      action='store_true',
-                      help='Treat all warnings as errors.')
   options = parser.parse_args(args)
 
   options.bootclasspath = build_utils.ParseGnList(options.bootclasspath)
   options.classpath = build_utils.ParseGnList(options.classpath)
 
-  cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
+  cmd = [
+      build_utils.JAVA_PATH,
       '-jar',
       options.desugar_jar,
       '--input',
@@ -52,8 +50,7 @@ def main():
   build_utils.CheckOutput(
       cmd,
       print_stdout=False,
-      stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings,
-      fail_on_output=options.warnings_as_errors)
+      stderr_filter=build_utils.FilterReflectiveAccessJavaWarnings)
 
   if options.depfile:
     build_utils.WriteDepfile(options.depfile,

android/gyp/dex.py

@@ -472,7 +472,9 @@ def _OnStaleMd5(changes, options, final_dex_inputs, dex_cmd):
 
 
 def MergeDexForIncrementalInstall(r8_jar_path, src_paths, dest_dex_jar):
-  dex_cmd = build_utils.JavaCmd(verify=False) + [
+  dex_cmd = [
+      build_utils.JAVA_PATH,
+      '-Xmx1G',
       '-cp',
       r8_jar_path,
       'com.android.tools.r8.D8',
@@ -507,7 +509,9 @@ def main(args):
     track_subpaths_allowlist = None
   final_dex_inputs += options.dex_inputs
 
-  dex_cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
+  dex_cmd = [
+      build_utils.JAVA_PATH,
+      '-Xmx1G',
       '-cp',
       options.r8_jar_path,
       'com.android.tools.r8.D8',

android/gyp/dex_jdk_libs.py

@@ -39,7 +39,9 @@ def DexJdkLibJar(r8_path, min_api, desugar_jdk_libs_json, desugar_jdk_libs_jar,
                  warnings_as_errors):
   # TODO(agrieve): Spews a lot of stderr about missing classes.
   with build_utils.TempDir() as tmp_dir:
-    cmd = build_utils.JavaCmd(warnings_as_errors) + [
+    cmd = [
+        build_utils.JAVA_PATH,
+        '-Xmx1G',
         '-cp',
         r8_path,
         'com.android.tools.r8.L8',

android/gyp/dexsplitter.py

@@ -48,7 +48,8 @@ def _ParseOptions(args):
 
 
 def _RunDexsplitter(options, output_dir):
-  cmd = build_utils.JavaCmd() + [
+  cmd = [
+      build_utils.JAVA_PATH,
       '-cp',
       options.r8_path,
       'com.android.tools.r8.dexsplitter.DexSplitter',

android/gyp/finalize_apk.py

@@ -12,32 +12,24 @@ import tempfile
 from util import build_utils
 
 
-def FinalizeApk(apksigner_path,
-                zipalign_path,
-                unsigned_apk_path,
-                final_apk_path,
-                key_path,
-                key_passwd,
-                key_name,
-                min_sdk_version,
-                warnings_as_errors=False):
+def FinalizeApk(apksigner_path, zipalign_path, unsigned_apk_path,
+                final_apk_path, key_path, key_passwd, key_name,
+                min_sdk_version):
   # Use a tempfile so that Ctrl-C does not leave the file with a fresh mtime
   # and a corrupted state.
   with tempfile.NamedTemporaryFile() as staging_file:
     if zipalign_path:
       # v2 signing requires that zipalign happen first.
       logging.debug('Running zipalign')
-      zipalign_cmd = [
+      subprocess.check_output([
           zipalign_path, '-p', '-f', '4', unsigned_apk_path, staging_file.name
-      ]
-      build_utils.CheckOutput(zipalign_cmd,
-                              print_stdout=True,
-                              fail_on_output=warnings_as_errors)
+      ])
       signer_input_path = staging_file.name
     else:
       signer_input_path = unsigned_apk_path
 
-    sign_cmd = build_utils.JavaCmd(warnings_as_errors) + [
+    sign_cmd = [
+        build_utils.JAVA_PATH,
         '-jar',
         apksigner_path,
         'sign',
@@ -66,8 +58,6 @@ def FinalizeApk(apksigner_path,
       sign_cmd += ['--min-sdk-version', '1']
 
     logging.debug('Signing apk')
-    build_utils.CheckOutput(sign_cmd,
-                            print_stdout=True,
-                            fail_on_output=warnings_as_errors)
+    subprocess.check_call(sign_cmd)
     shutil.move(staging_file.name, final_apk_path)
     staging_file.delete = False

android/gyp/jacoco_instr.py

@@ -199,8 +199,8 @@ def _RunInstrumentCommand(parser):
     source_files.extend(build_utils.ReadSourcesList(args.java_sources_file))
 
   with build_utils.TempDir() as temp_dir:
-    instrument_cmd = build_utils.JavaCmd() + [
-        '-jar', args.jacococli_jar, 'instrument'
+    instrument_cmd = [
+        build_utils.JAVA_PATH, '-jar', args.jacococli_jar, 'instrument'
     ]
 
     if not args.files_to_instrument:

android/gyp/main_dex_list.py

@@ -39,9 +39,6 @@ def _ParseArgs():
   parser.add_argument('--negative-main-dex-globs',
       help='GN-list of globs of .class names (e.g. org/chromium/foo/Bar.class) '
            'that will fail the build if they match files in the main dex.')
-  parser.add_argument('--warnings-as-errors',
-                      action='store_true',
-                      help='Treat all warnings as errors.')
 
   args = parser.parse_args(build_utils.ExpandFileArgs(sys.argv[1:]))
 
@@ -57,7 +54,9 @@ def _ParseArgs():
 
 def main():
   args = _ParseArgs()
-  proguard_cmd = build_utils.JavaCmd(args.warnings_as_errors) + [
+  proguard_cmd = [
+      build_utils.JAVA_PATH,
+      '-Xmx1G',
       '-cp',
       args.r8_path,
       'com.android.tools.r8.R8',
@@ -97,9 +96,7 @@ def main():
         proguard_cmd += ['--pg-conf', proguard_flags_file.name]
         for injar in args.class_inputs:
           proguard_cmd.append(injar)
-        build_utils.CheckOutput(proguard_cmd,
-                                print_stderr=False,
-                                fail_on_output=args.warnings_as_errors)
+        build_utils.CheckOutput(proguard_cmd, print_stderr=False)
 
       # Record the classes kept by ProGuard. Not used by the build, but useful
       # for debugging what classes are kept by ProGuard vs. MainDexListBuilder.
@@ -110,7 +107,8 @@ def main():
 
       # Step 2: Expand inclusion list to all classes referenced by the .class
       # files of kept classes (non-recursive).
-      main_dex_list_cmd = build_utils.JavaCmd() + [
+      main_dex_list_cmd = [
+          build_utils.JAVA_PATH,
           '-cp',
           args.dx_path,
           'com.android.multidex.MainDexListBuilder',
@@ -121,8 +119,7 @@ def main():
           temp_jar.name,
           ':'.join(args.class_inputs)
       ]
-      main_dex_list = build_utils.CheckOutput(
-          main_dex_list_cmd, fail_on_output=args.warnings_as_errors)
+      main_dex_list = build_utils.CheckOutput(main_dex_list_cmd)
 
   except build_utils.CalledProcessError as e:
     if 'output jar is empty' in e.output:

android/gyp/merge_manifest.py

@@ -85,15 +85,13 @@ def main(argv):
   parser.add_argument(
       '--manifest-package',
       help='Package name of the merged AndroidManifest.xml.')
-  parser.add_argument('--warnings-as-errors',
-                      action='store_true',
-                      help='Treat all warnings as errors.')
   args = parser.parse_args(argv)
 
   classpath = _BuildManifestMergerClasspath(args.android_sdk_cmdline_tools)
 
   with build_utils.AtomicOutput(args.output) as output:
-    cmd = build_utils.JavaCmd(args.warnings_as_errors) + [
+    cmd = [
+        build_utils.JAVA_PATH,
         '-cp',
         classpath,
         _MANIFEST_MERGER_MAIN_CLASS,
@@ -125,13 +123,11 @@ def main(argv):
           '--property',
           'PACKAGE=' + package,
       ]
-      build_utils.CheckOutput(
-          cmd,
+      build_utils.CheckOutput(cmd,
         # https://issuetracker.google.com/issues/63514300:
         # The merger doesn't set a nonzero exit code for failures.
-          fail_func=lambda returncode, stderr: returncode != 0 or build_utils.
-          IsTimeStale(output.name, [root_manifest] + extras),
-          fail_on_output=args.warnings_as_errors)
+        fail_func=lambda returncode, stderr: returncode != 0 or
+          build_utils.IsTimeStale(output.name, [root_manifest] + extras))
 
     # Check for correct output.
     _, manifest, _ = manifest_utils.ParseManifest(output.name)

android/gyp/proguard.py

@@ -251,12 +251,14 @@ def _OptimizeWithR8(options,
       base_dex_context = _DexPathContext('base', options.output_path,
                                          options.input_paths, tmp_output)
 
-    cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
+    cmd = [
+        build_utils.JAVA_PATH,
         '-Dcom.android.tools.r8.allowTestProguardOptions=1',
     ]
     if options.disable_outlining:
       cmd += [' -Dcom.android.tools.r8.disableOutlining=1']
     cmd += [
+        '-Xmx1G',
         '-cp',
         options.r8_path,
         'com.android.tools.r8.R8',

android/gyp/turbine.py

@@ -8,6 +8,7 @@ import argparse
 import logging
 import os
 import shutil
+import subprocess
 import sys
 import time
 
@@ -46,9 +47,7 @@ def _OnStaleMd5(options, cmd, javac_cmd, files, classpath):
     cmd += ['--output', output_jar.name, '--gensrc_output', generated_jar.name]
     logging.debug('Command: %s', cmd)
     start = time.time()
-    build_utils.CheckOutput(cmd,
-                            print_stdout=True,
-                            fail_on_output=options.warnings_as_errors)
+    subprocess.check_call(cmd)
     end = time.time() - start
     logging.info('Header compilation took %ss', end)
 
@@ -95,9 +94,6 @@ def main(argv):
       '--generated-jar-path',
       required=True,
       help='Output path for generated source files.')
-  parser.add_argument('--warnings-as-errors',
-                      action='store_true',
-                      help='Treat all warnings as errors.')
   options, unknown_args = parser.parse_known_args(argv)
 
   options.bootclasspath = build_utils.ParseGnList(options.bootclasspath)
@@ -112,8 +108,9 @@ def main(argv):
     if arg.startswith('@'):
       files.extend(build_utils.ReadSourcesList(arg[1:]))
 
-  cmd = build_utils.JavaCmd(options.warnings_as_errors) + [
-      '-classpath', options.turbine_jar_path, 'com.google.turbine.main.Main'
+  cmd = [
+      build_utils.JAVA_PATH, '-classpath', options.turbine_jar_path,
+      'com.google.turbine.main.Main'
   ]
   javac_cmd = []
 

android/gyp/util/build_utils.py

@@ -33,32 +33,18 @@ DIR_SOURCE_ROOT = os.path.relpath(
             os.path.dirname(__file__), os.pardir, os.pardir, os.pardir,
             os.pardir)))
 JAVA_HOME = os.path.join(DIR_SOURCE_ROOT, 'third_party', 'jdk', 'current')
+JAVA_PATH = os.path.join(JAVA_HOME, 'bin', 'java')
 JAVAC_PATH = os.path.join(JAVA_HOME, 'bin', 'javac')
 JAVAP_PATH = os.path.join(JAVA_HOME, 'bin', 'javap')
 RT_JAR_PATH = os.path.join(DIR_SOURCE_ROOT, 'third_party', 'jdk', 'extras',
                            'java_8', 'jre', 'lib', 'rt.jar')
 
-# TODO(agrieve): Remove once safe to do so.
-JAVA_PATH = os.path.join(JAVA_HOME, 'bin', 'java')
-
 try:
   string_types = basestring
 except NameError:
   string_types = (str, bytes)
 
 
-def JavaCmd(verify=True):
-  ret = [os.path.join(JAVA_HOME, 'bin', 'java')]
-  # Limit heap to 1GB of RAM.
-  ret += ['-Xmx1G']
-
-  # Disable bytecode verification for local builds gives a ~2% speed-up.
-  if not verify:
-    ret += ['-noverify']
-
-  return ret
-
-
 @contextlib.contextmanager
 def TempDir(**kwargs):
   dirname = tempfile.mkdtemp(**kwargs)

config/android/internal_rules.gni

@@ -1367,11 +1367,9 @@ if (enable_java_templates) {
           "--classpath",
           rebase_path("//third_party/jacoco/lib/jacocoagent.jar",
                       root_build_dir),
+          "--noverify",
         ]
       }
-      if (use_jacoco_coverage || !treat_warnings_as_errors) {
-        args += [ "--noverify" ]
-      }
       if (defined(invoker.wrapper_script_args)) {
         args += [ "--" ] + invoker.wrapper_script_args
       }
@@ -1610,9 +1608,6 @@ if (enable_java_templates) {
           if (_input_class_jars != []) {
             args += [ "--class-inputs=${_rebased_input_class_jars}" ]
           }
-          if (treat_warnings_as_errors) {
-            args += [ "--warnings-as-errors" ]
-          }
         }
       }
 
@@ -2001,9 +1996,6 @@ if (enable_java_templates) {
             "--depfile",
             rebase_path(depfile, root_build_dir),
           ]
-          if (treat_warnings_as_errors) {
-            args += [ "--warnings-as-errors" ]
-          }
         }
 
         _deps = []
@@ -2145,10 +2137,6 @@ if (enable_java_templates) {
       if (defined(invoker.max_sdk_version)) {
         args += [ "--max-sdk-version=${invoker.max_sdk_version}" ]
       }
-
-      if (treat_warnings_as_errors) {
-        args += [ "--warnings-as-errors" ]
-      }
     }
   }
 
@@ -2967,9 +2955,6 @@ if (enable_java_templates) {
     if (_secondary_native_lib_placeholders != []) {
       _args += [ "--secondary-native-lib-placeholders=$_secondary_native_lib_placeholders" ]
     }
-    if (treat_warnings_as_errors) {
-      _args += [ "--warnings-as-errors" ]
-    }
 
     if (defined(invoker.expected_libs_and_assets)) {
       _expectations_target =
@@ -4198,10 +4183,6 @@ template("create_android_app_bundle_module") {
     ]
   }
 
-  if (treat_warnings_as_errors) {
-    _args += [ "--warnings-as-errors" ]
-  }
-
   if (defined(invoker.expected_libs_and_assets)) {
     _expectations_target = "${invoker.top_target_name}_validate_libs_and_assets"
     action_with_pydeps(_expectations_target) {

config/android/rules.gni

@@ -4880,9 +4880,6 @@ if (enable_java_templates) {
           invoker.compress_shared_libraries) {
         args += [ "--compress-shared-libraries" ]
       }
-      if (treat_warnings_as_errors) {
-        args += [ "--warnings-as-errors" ]
-      }
 
       if (_enable_language_splits) {
         args += [