From github: https://github.com/mozilla-mobile/fenix/issues/17703.

zipalign is an archive alignment tool that provides important optimization to Android application (APK) files. The purpose is to ensure that all uncompressed data starts with a particular alignment relative to the start of the file. Specifically, it causes all uncompressed data within the APK, such as images or raw files, to be aligned on 4-byte boundaries. This allows all portions to be accessed directly with mmap() even if they contain binary data with alignment restrictions. The benefit is a reduction in the amount of RAM consumed when running the application.

In https://github.com/mozilla-mobile/fenix/issues/12527#issuecomment-769981299, I determined that autograph (our signing server) is not zipaligning fenix APKs and discussion in #releaseduty-mobile indicates we may not be zipaligning before sending the APK to autograph for signing. If APKs are zipaligned at this point, it's probably coincidence (or Google is automatically doing it but I can't find evidence that that is the case).

We should make sure we're zipaligning fenix builds for optimal performance. My recommendation would be to do these steps:

1. zipalign
2. sign (via autograph)
3. verify zipalign (i.e. zipalign -c 4 <apk>), failing build if not zipaligned

I think we'll need releng expertise to implement this.

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

This ticket came up in today's Android monorepo call. NI'ing myself to look into the details and reply here.

As far as I can tell we call zipalign on the apk after signing here: https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L206

zip_align_apk is https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L644-L671

One thing that we might want to change is we don't currently seem to pass -p to zipalign, which https://developer.android.com/studio/command-line/zipalign.html recommends to page-align .so files.

I agree with :jcristau. We have zipaligned APKs since bug 1335370 which was 6 years ago.

I determined that autograph (our signing server) is not zipaligning fenix APKs and
I'm not sure how this was determined but that doesn't sound correct to me. Maybe there was a regression in 2021, but as :jcristau said, we're calling zipalign these days. Moreover, like said in bug 1335370 comment 0, Google Play refuses APKs that aren't signed. Therefore, this potential regression in 2021 would have to match another regression on Google Play's end.

If people remember how things were in 2021, I'm happy to discuss details. In the meantime and based on the facts above, I assume there was an error in the original bug report.

discussion in #releaseduty-mobile indicates we may not be zipaligning before sending the APK to autograph for signing

This is actually on purpose. Signing APKs with the v1 scheme[1] actually injects some files in the APK. An APK file is a actually a zip archive. Adding files to a zip changes the archive. Zipaligning is actually a step that modifies the archive for performance reasons[2]. Therefore, there is no point of optimizing the archive before adding the last missing files. That's why we don't zipalign files before sending them for signing.

One thing that we might want to change is we don't currently seem to pass -p to zipalign, which https://developer.android.com/studio/command-line/zipalign.html recommends to page-align .so files.

Great call! While the rest of the bug is invalid I think we can still improve this part. This is a good-first-bug. I'm happy to mentor it. It should just be a matter of changing this line[3] to pass the -p flag.

[1] https://source.android.com/docs/security/features/apksigning#v1
[2] https://developer.android.com/studio/command-line/zipalign.html
[3] https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L667

(In reply to Johan Lorenzo [:jlorenzo] from comment #4)

Great call! While the rest of the bug is invalid I think we can still improve this part. This is a good-first-bug. I'm happy to mentor it. It should just be a matter of changing this line[3] to pass the -p flag.

[...]
[3] https://github.com/mozilla-releng/scriptworker-scripts/blob/df61f036ab5fddf56c7a34e4516db8cd2a7dde9d/signingscript/src/signingscript/sign.py#L667

I'm sorry. I took a closer look at this code and I just realized I was wrong. zip_align_apk() is now dead code. We've let autograph handle zipalignment since bug 1618531[1]. Autograph does so by shelling out to Google's apksigner[2]. It's not crystal clear per the Android documentation but apksigner does take care of zipaligning APKs[3]:

Caution: If you sign your APK using apksigner and make further changes to the APK, the APK's signature is invalidated. If you use zipalign to align your APK, use it before signing the APK.

I checked locally with the latest Fenix Nightly and even .so files are already zipaligned:

$ zipalign -c -p -v 4 ~/Downloads/target.apk
Verifying alignment of /Users/johanlorenzo/Downloads/target(1).apk (4)...
      87 META-INF/com/android/build/gradle/app-metadata.properties (OK - compressed)
     196 assets/dexopt/baseline.prof (OK)
    4420 assets/dexopt/baseline.profm (OK)
    4776 classes.dex (OK - compressed)
 4256229 classes2.dex (OK - compressed)
 6516802 lib/arm64-v8a/libfreebl3.so (OK - compressed)
 6766328 lib/arm64-v8a/libipcclientcerts.so (OK - compressed)
 6940951 lib/arm64-v8a/libjnidispatch.so (OK - compressed)
 6983977 lib/arm64-v8a/liblgpllibs.so (OK - compressed)
 6996122 lib/arm64-v8a/libmegazord.so (OK - compressed)
10782708 lib/arm64-v8a/libmozavcodec.so (OK - compressed)
10873254 lib/arm64-v8a/libmozavutil.so (OK - compressed)
10955651 lib/arm64-v8a/libmozglue.so (OK - compressed)
11452530 lib/arm64-v8a/libnss3.so (OK - compressed)
12447413 lib/arm64-v8a/libnssckbi.so (OK - compressed)
12736306 lib/arm64-v8a/libplugin-container.so (OK - compressed)
12802000 lib/arm64-v8a/libsentry-android.so (OK - compressed)
12807202 lib/arm64-v8a/libsentry.so (OK - compressed)
13184537 lib/arm64-v8a/libsoftokn3.so (OK - compressed)
13292843 lib/arm64-v8a/libxul.so (OK - compressed)
[...]

Therefore, there's no action needed to zipalign .so files. I'm sorry for leading you to write a patch, :royang! I had forgotten about this until today.

That said, there's one more thing to do on the RelEng side: we should get rid of this dead code. I filed bug 1828876 for it.

[1] https://github.com/mozilla-releng/scriptworker-scripts/commit/8155d6299a3917be35a02eda81ab239705f9a497
[2] https://github.com/mozilla-services/autograph/blob/0b2dcedd1738720c8642a3fd65b2793991c3709a/signer/apk2/apk2.go#L148
[3] https://developer.android.com/tools/apksigner