Add support for uncompress library in APK to the build system

build/android/gyp/finalize_apk.py

Index: build/android/gyp/finalize_apk.py
diff --git a/build/android/gyp/finalize_apk.py b/build/android/gyp/finalize_apk.py
index d64e10d0f57cba2a7b9185c2d58e53cd46751014..cca539f76d2d15b1e5b03c4fdae7b7dda88c8470 100755
--- a/build/android/gyp/finalize_apk.py
+++ b/build/android/gyp/finalize_apk.py
@@ -14,6 +14,16 @@ import tempfile
 
 from util import build_utils
 
+def RenameLibInApk(rezip_path, in_zip_file, out_zip_file):
+  rename_cmd = [
+      rezip_path,
+      'rename',
+      in_zip_file,
+      out_zip_file,
+    ]
+  build_utils.CheckOutput(rename_cmd)
+
+
 def SignApk(key_path, key_name, key_passwd, unsigned_path, signed_path):
   shutil.copy(unsigned_path, signed_path)
   sign_cmd = [
@@ -38,6 +48,34 @@ def AlignApk(zipalign_path, unaligned_path, final_path):
   build_utils.CheckOutput(align_cmd)
 
 
+def UncompressLibAndPageAlignInApk(rezip_path, in_zip_file, out_zip_file):
+  rename_cmd = [
+      rezip_path,
+      'inflatealign',
+      in_zip_file,
+      out_zip_file,
+    ]
+  build_utils.CheckOutput(rename_cmd)
+
+
+def DropDataDescriptorsInApk(rezip_path, in_zip_file, out_zip_file):
+  rename_cmd = [
+      rezip_path,
+      'dropdescriptors',
+      in_zip_file,
+      out_zip_file,
+    ]
+  build_utils.CheckOutput(rename_cmd)
+
+
+def RemoveTmpApk(tmp_apk):
+  rm_cmd = [
+    'rm',
+    tmp_apk,
+    ]
+  build_utils.CheckOutput(rm_cmd)
+
+
 def main():
   parser = optparse.OptionParser()
 
@@ -49,14 +87,48 @@ def main():
   parser.add_option('--key-passwd', help='Keystore password')
   parser.add_option('--key-name', help='Keystore name')
   parser.add_option('--stamp', help='Path to touch on success.')
+  parser.add_option('--uncompress-lib', type='int',

[rmcilroy, 2014/06/19 09:46:12]

As discussed below, I think you should also add a '--rename-lib' option, both
for clarity on what the options do, and to enable independently testing of apks
which contain an uncompressed library which is not loaded in-place by the crazy
linker (i.e., is still copied and loaded by system linker).

[Anton, 2014/06/19 13:39:45]

It is over engineering to implement functionality we don't want or need and it
will make this tool more complicated and harder to understand.

[rmcilroy, 2014/06/20 14:42:59]

I disagree, for testing and debugging as stated above.

+      help='If non-zero, build an APK with the library uncompressed')

[rmcilroy, 2014/06/19 09:46:12]

Please make this an "action='store_true'" type of option rather than an int
type.  Also add a full stop in the help text here and below to be consistent
with the other options.

[Anton, 2014/06/19 13:39:45]

This just makes things more complicated with no upside.

+  parser.add_option('--rezip-path', help='Path to the rezip executable')

[rmcilroy, 2014/06/19 09:46:12]

nit  - move this up to be below --zipalign-path 

[Anton, 2014/06/19 13:39:45]

Done.

 
   options, _ = parser.parse_args()
 
   with tempfile.NamedTemporaryFile() as intermediate_file:
     signed_apk_path = intermediate_file.name
-    SignApk(options.key_path, options.key_name, options.key_passwd,
-            options.unsigned_apk_path, signed_apk_path)
-    AlignApk(options.zipalign_path, signed_apk_path, options.final_apk_path)
+    if options.uncompress_lib:
+      # In this branch we are building an APK with the shared library
+      # uncompressed and page aligned, so that it can be loaded directly
+      # by the android chrome linker.
+      #
+      # We alter the name of the library so that the Android Package Manager
+      # does not extract it into a separate file. This must be done before
+      # signing as the filename is part of the signed manifest.
+      apk_to_sign = intermediate_file.name + ".librenamed"

[rmcilroy, 2014/06/19 09:46:12]

Use the same "with tempfile.NamedTemporaryFile()" approach to create the
temporary intermediate filesnames as is used for signed_apk_path to create the
intermediates you need. 

[Anton, 2014/06/19 13:39:45]

Done.

+      RenameLibInApk(options.rezip_path, options.unsigned_apk_path, apk_to_sign)
+      SignApk(options.key_path, options.key_name, options.key_passwd,
+              apk_to_sign, signed_apk_path)
+      RemoveTmpApk(apk_to_sign)
+
+      # Signing adds data descriptors to the APK. These are redundant
+      # information. We remove them as otherwise they can cause a
+      # miscalculation in the page alignment.
+      apk_without_descriptors = intermediate_file.name + ".without_descriptors"
+      DropDataDescriptorsInApk(
+          options.rezip_path, signed_apk_path, apk_without_descriptors)
+
+      # Align uncompress items to 4 bytes (as normal for all APKs).
+      aligned_apk = intermediate_file.name + ".aligned"
+      AlignApk(options.zipalign_path, apk_without_descriptors, aligned_apk)
+      RemoveTmpApk(apk_without_descriptors)
+
+      # Uncompress the library and make sure that it is page aligned.
+      UncompressLibAndPageAlignInApk(
+          options.rezip_path, aligned_apk, options.final_apk_path)
+      RemoveTmpApk(aligned_apk)
+    else:

[rmcilroy, 2014/06/19 09:46:12]

I think this is pretty hacky to have two complete branches for the different
flows (which leads to maintenance issues if someone changes one branch without
remembering to change the other).  With the --rename_library flag added, how
about merging it into one branch, e.g.:

  if (options.rename_library):
    RenameLibInApk(...)
  else:
    apk_to_sign = unsigned_apk_path
  SignApk(...)
  DropDataDescriptorsInApk(...)  # presumably this is safe to do even if we
aren't uncompressing the library ? 
  if (options.uncompress_lib):
    UncompressLibAndPageAlignInApk(...)
  else:
    apk_to_align = apk_without_descriptors
  AlignApk(...)

WDYT?

[Anton, 2014/06/19 13:39:45]

Actually I separated it into two branches which made it look much cleaner and
more readable.

[rmcilroy, 2014/06/20 14:42:59]

I disagree entirely - it makes it more difficult to follow the differences and
will end up leading to steps being added to one branch but not the other. 

+      SignApk(options.key_path, options.key_name, options.key_passwd,
+              options.unsigned_apk_path, signed_apk_path)
+      AlignApk(options.zipalign_path, signed_apk_path,  options.final_apk_path)
 
   if options.stamp:
     build_utils.Touch(options.stamp)
@@ -64,5 +136,3 @@ def main():
 
 if __name__ == '__main__':
   sys.exit(main())
-
-