Allow building app with multiple GeckoView architectures included

RESOLVED FIXED in Firefox 65

Status

enhancement
P2
normal
RESOLVED FIXED
9 months ago
2 months ago

People

(Reporter: sebastian, Assigned: snorp)

Tracking

(Blocks 2 bugs)

unspecified
mozilla65
All
Android
Dependency tree / graph

Firefox Tracking Flags

(geckoview64 wontfix, firefox64 wontfix, firefox65 fixed)

Details

Attachments

(1 attachment)

Currently we build multiple GeckoView AARs based on CPU architecture. Inside the apps we have configured gradle to build custom build variants where every variant only uses one GeckoView artifact.

This has multiple drawbacks for app developers:

* The build variant setup is complicated and error-prone

* The build variant setup is slowing down builds because a lot of variants have to be build (some of our apps have multiple flavor dimensions)

* The Android Gradle plugin already supports APK splits out of the box and can split by things like ABI (but right now we can only include one GeckoView):
https://developer.android.com/studio/build/configure-apk-splits#configure-abi-split

* With the new publishing format "Android App Bundles" we do not need to build multiple APKs anymore. The Play Store will dynamically compose a minimal APK for the device that wants to install the app. One of the used dimensions for that is the CPU architecture. However we can only include one GeckoView in our build..
https://developer.android.com/platform/technology/app-bundle/

Those reasons make it desirable to include all GeckoView architectures into the build and let Gradle (ABI split) or Google Play (Android App Bundle) decide what to include for a specific setup.

Is this something we could support? What would be needed for that?
I assume there are two (theoretically) options:

A) Make it possible to add multiple GeckoView builds into an application
B) Build an "universal" GeckoView artifact that includes everything for all CPU architectures (assuming this would work with APK split / App bundles)

Using (A) currently causes the build to fail because of duplicated Java classes (the first one it complains about is Exoplayer). Would it be an option to move those classes to a "geckoview-base" artifact that the other artifacts depend on? This would allow gradle to not include those multiple times.
> (the first one it complains about is Exoplayer)

Technically at least Exoplayer is already a separate AAR:
https://github.com/google/ExoPlayer

However we do not list it as dependency in the POM file and instead have it in the tree[1] and package it into our own AAR and then it will be duplicated once we add two GeckoView AARs to an app. I assume the same error will show up for our own Java/Kotlin code once the exoplayer issue is resolved.

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2
An additional reason why I would like to add multiple CPU architectures at once: We do the same for our Rust-based components. We build just one component that contains the code for all CPU architectures. Apps can then use the APK split feature or Android App Bundles to pick what they need. Together with GeckoView and Rust-based components in the same app things get very complicated.
NI: :snorp and :nalexander (doesn't accept NI requests) may know if this is possible and what would be required to get there.
Flags: needinfo?(snorp)
Building a fat AAR should be possible. I think the easiest way may be to simply add a new task that merges the native parts of the individual AARs. Right now the Java build would be unnecessarily duplicated, but that might be solvable if care enough. There will be some other fallout due to things like AppConstants.MOZ_APP_ABI being unusable.

The big wrench in the gears may be the fact that libxul.so and friends are in assets/ instead of lib/ because of the custom xz compression. Downstream tools won't understand that, so we'd have to stop that. This would be a significant APK size increase; I'm guessing maybe 20%.
Flags: needinfo?(snorp)
> The big wrench in the gears may be the fact that libxul.so and friends are in assets/ instead of lib/ because of the custom xz compression. Downstream tools won't understand that, so we'd have to stop that. This would be a significant APK size increase; I'm guessing maybe 20%.

It would be interesting to measure that and see how much Google Play can save with app bundles. Also as I understand it not doing our own compression would allow Google Play to create smaller app update patches resulting in smaller update sizes [1]?

[1] https://github.com/googlesamples/apk-patch-size-estimator
Sebastian, can you help us assign a priority here? Do we need this soon?
Flags: needinfo?(s.kaspari)
> Sebastian, can you help us assign a priority here? Do we need this soon?

"Soon" is vague here. It's not a Focus blocker.

I think there are two driving factors here:

* Eventually we want to use Android App Bundles because it is the most promising way to ship small APKs to our users.

* We want to stop maintaining this custom split APK solution that will get even more complex once we have more libraries with native (Rust) code that ship as a "multiarch" AAR.

So, yeah it's not a P1 but I would be good if this could find a place on the roadmap.
Flags: needinfo?(s.kaspari)
Thanks, it makes sense for us to choose the right path forward as early as we can. Going with P2.
Priority: -- → P2
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #5)
> Building a fat AAR should be possible. I think the easiest way may be to
> simply add a new task that merges the native parts of the individual AARs.
> Right now the Java build would be unnecessarily duplicated, but that might
> be solvable if care enough. There will be some other fallout due to things
> like AppConstants.MOZ_APP_ABI being unusable.

Good point.  Probably a lot of the "don't load on bad ABIs" would need to become more dynamic.

The way that I would try to achieve this is for the GeckoView universal AAR to depend on N arch-specific AARs and have the arch-specific AARs only include libraries.

> The big wrench in the gears may be the fact that libxul.so and friends are
> in assets/ instead of lib/ because of the custom xz compression. Downstream
> tools won't understand that, so we'd have to stop that. This would be a
> significant APK size increase; I'm guessing maybe 20%.

We can prefix the assets/ path -- so assets/{armeabi-v7a,arm-v8a,x86}.  This is just good hygiene across the board.  Of course we'd need to make the choice of assets/ path depend on the device more, so there's another point of failure... but it feels do-able.
This allows us to use the same Java code for any native platform,
enabling a "fat" AAR.
Assignee: nobody → snorp

Comment 12

6 months ago
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7174015fbbd5
Make Java parts of GeckoView independent from build ABI r=jchen
Flags: needinfo?(snorp)

Comment 14

6 months ago
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bae43dfd3700
Make Java parts of GeckoView independent from build ABI r=jchen

Comment 15

6 months ago
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3bd6b94b23e5
Backed out changeset bae43dfd3700 for toolchains gradle bustages. CLOSED TREE

Comment 16

6 months ago
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0bb898d00ed7
Make Java parts of GeckoView independent from build ABI r=jchen

Comment 17

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0bb898d00ed7
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
64=wontfix because we don't need to uplift to Beta.
status-geckoview64=wontfix

Updated

5 months ago
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.