Android: Use zipalign.py rather than zipalign for non-official apks

Saves 300ms on my machine

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

build/android/gyp/apkbuilder.py

@@ -181,7 +181,7 @@ def _ExpandPaths(paths):
   return ret
 
 
-def _AddAssets(apk, path_tuples, disable_compression=False):
+def _AddAssets(apk, path_tuples, fast_align, disable_compression=False):
   """Adds the given paths to the apk.
 
   Args:
@@ -193,10 +193,15 @@ def _AddAssets(apk, path_tuples, disable_compression=False):
   # locality of mmap'ed files.
   for target_compress in (False, True):
     for src_path, dest_path in path_tuples:
-
       compress = not disable_compression and (
           os.path.splitext(src_path)[1] not in _NO_COMPRESS_EXTENSIONS)
+
       if target_compress == compress:
+        # AddToZipHermetic() uses this logic to avoid growing small files.
+        # We need it here in order to set alignment correctly.
+        if compress and os.path.getsize(src_path) < 16:
+          compress = False
+
         apk_path = 'assets/' + dest_path
         try:
           apk.getinfo(apk_path)
@@ -204,11 +209,16 @@ def _AddAssets(apk, path_tuples, disable_compression=False):
           raise Exception('Multiple targets specified the asset path: %s' %
                           apk_path)
         except KeyError:
-          build_utils.AddToZipHermetic(apk, apk_path, src_path=src_path,
-                                       compress=compress)
+          zipalign.AddToZipHermetic(
+              apk,
+              apk_path,
+              src_path=src_path,
+              compress=compress,
+              alignment=0 if compress and not fast_align else 4)
 
 
-def _AddNativeLibraries(out_apk, native_libs, android_abi, uncompress):
+def _AddNativeLibraries(out_apk, native_libs, android_abi, uncompress,
+                        fast_align):
   """Add native libraries to APK."""
   has_crazy_linker = any(
       'android_linker' in os.path.basename(p) for p in native_libs)
@@ -229,10 +239,12 @@ def _AddNativeLibraries(out_apk, native_libs, android_abi, uncompress):
           basename = 'crazy.' + basename
 
     apk_path = 'lib/%s/%s' % (android_abi, basename)
-    build_utils.AddToZipHermetic(out_apk,
+    zipalign.AddToZipHermetic(
+        out_apk,
         apk_path,
         src_path=path,
-                                 compress=compress)
+        compress=compress,
+        alignment=0 if compress and not fast_align else 0x1000)
 
 
 def main(args):
@@ -249,6 +261,11 @@ def main(args):
     # Compresses about twice as fast as the default.
     zlib.Z_DEFAULT_COMPRESSION = 1
 
+  # Manually align only when alignment is necessary.
+  # Python's zip implementation duplicates file comments in the central
+  # directory, whereas zipalign does not, so use zipalign for official builds.
+  fast_align = options.format == 'apk' and not options.best_compression
+
   native_libs = sorted(options.native_libs)
 
   # Include native libs in the depfile_deps since GN doesn't know about the
@@ -300,15 +317,21 @@ def main(args):
   # Targets generally do not depend on apks, so no need for only_if_changed.
   with build_utils.AtomicOutput(options.output_apk, only_if_changed=False) as f:
     with zipfile.ZipFile(options.resource_apk) as resource_apk, \
-         zipfile.ZipFile(f, 'w', zipfile.ZIP_DEFLATED) as out_apk:
+         zipfile.ZipFile(f, 'w') as out_apk:
 
-      def copy_resource(zipinfo, out_dir=''):
-        compress = zipinfo.compress_type != zipfile.ZIP_STORED
-        build_utils.AddToZipHermetic(
+      def add_to_zip(zip_path, data, compress=True):
+        zipalign.AddToZipHermetic(
             out_apk,
+            zip_path,
+            data=data,
+            compress=compress,
+            alignment=0 if compress and not fast_align else 4)
+
+      def copy_resource(zipinfo, out_dir=''):
+        add_to_zip(
             out_dir + zipinfo.filename,
-            data=resource_apk.read(zipinfo.filename),
-            compress=compress)
+            resource_apk.read(zipinfo.filename),
+            compress=zipinfo.compress_type != zipfile.ZIP_STORED)
 
       # Make assets come before resources in order to maintain the same file
       # ordering as GYP / aapt. http://crbug.com/561862
@@ -321,35 +344,29 @@ def main(args):
 
       # 2. Assets
       logging.debug('Adding assets/')
-      _AddAssets(out_apk, assets, disable_compression=False)
-      _AddAssets(out_apk, uncompressed_assets, disable_compression=True)
+      _AddAssets(out_apk, assets, fast_align, disable_compression=False)
+      _AddAssets(
+          out_apk, uncompressed_assets, fast_align, disable_compression=True)
 
       # 3. Dex files
       logging.debug('Adding classes.dex')
-      if options.dex_file and options.dex_file.endswith('.zip'):
+      if options.dex_file:
         with zipfile.ZipFile(options.dex_file, 'r') as dex_zip:
           for dex in (d for d in dex_zip.namelist() if d.endswith('.dex')):
-            build_utils.AddToZipHermetic(
-                out_apk,
+            add_to_zip(
                 apk_dex_dir + dex,
-                data=dex_zip.read(dex),
-                compress=not options.uncompress_dex)
-      elif options.dex_file:
-        build_utils.AddToZipHermetic(
-            out_apk,
-            apk_dex_dir + 'classes.dex',
-            src_path=options.dex_file,
+                dex_zip.read(dex),
                 compress=not options.uncompress_dex)
 
       # 4. Native libraries.
       logging.debug('Adding lib/')
       _AddNativeLibraries(out_apk, native_libs, options.android_abi,
-                          options.uncompress_shared_libraries)
+                          options.uncompress_shared_libraries, fast_align)
 
       if options.secondary_android_abi:
         _AddNativeLibraries(out_apk, secondary_native_libs,
                             options.secondary_android_abi,
-                            options.uncompress_shared_libraries)
+                            options.uncompress_shared_libraries, fast_align)
 
       # Add a placeholder lib if the APK should be multi ABI but is missing libs
       # for one of the ABIs.
@@ -371,14 +388,14 @@ def main(args):
         # with stale builds when the only change is adding/removing
         # placeholders).
         apk_path = 'lib/%s/%s' % (options.android_abi, name)
-        build_utils.AddToZipHermetic(out_apk, apk_path, data='')
+        add_to_zip(apk_path, '')
 
       for name in sorted(secondary_native_lib_placeholders):
         # Note: Empty libs files are ignored by md5check (can cause issues
         # with stale builds when the only change is adding/removing
         # placeholders).
         apk_path = 'lib/%s/%s' % (options.secondary_android_abi, name)
-        build_utils.AddToZipHermetic(out_apk, apk_path, data='')
+        add_to_zip(apk_path, '')
 
       # 5. Resources
       logging.debug('Adding res/')
@@ -402,16 +419,14 @@ def main(args):
             if apk_path_lower.endswith('.class'):
               continue
 
-            build_utils.AddToZipHermetic(
-                out_apk,
-                apk_root_dir + apk_path,
-                data=java_resource_jar.read(apk_path))
+            add_to_zip(apk_root_dir + apk_path,
+                       java_resource_jar.read(apk_path))
 
     if options.format == 'apk':
-      finalize_apk.FinalizeApk(options.apksigner_jar, options.zipalign_path,
-                               f.name, f.name, options.key_path,
-                               options.key_passwd, options.key_name,
-                               int(options.min_sdk_version))
+      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))
     logging.debug('Moving file into place')
 
   if options.depfile:

build/android/gyp/finalize_apk.py

@@ -18,18 +18,23 @@ def FinalizeApk(apksigner_path, zipalign_path, unsigned_apk_path,
   # 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')
       subprocess.check_output([
-        zipalign_path, '-p', '-f', '4',
-        unsigned_apk_path, staging_file.name])
+          zipalign_path, '-p', '-f', '4', unsigned_apk_path, staging_file.name
+      ])
+      signer_input_path = staging_file.name
+    else:
+      signer_input_path = unsigned_apk_path
+
     sign_cmd = [
         build_utils.JAVA_PATH,
         '-jar',
         apksigner_path,
         'sign',
         '--in',
-        staging_file.name,
+        signer_input_path,
         '--out',
         staging_file.name,
         '--ks',