1865634 - Some Android Nightly builds have empty `breakpadId` and `codeId` fields for shared libraries

Status: VERIFIED FIXED

(Core :: Gecko Profiler, defect, P2)

Description

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

I can't seem to reproduce it consistently but :padenot and another person in the Firefox Profiler Matrix channel have encountered this issue.

For example here's a profile from :padenot: https://share.firefox.dev/46sPzdn
If you look at the profile.libs array, the breakpadId and codeId of libxul.so are empty strings. codeId is not needed for the symbolication itself, it's needed for the assembly view, but it's still weird that it's empty. breakpadId is definitely necessary for the symbolication though.

Comment 1

[Julien Wajsberg [:julienw]]

Nazım found that the path for libxul.so for apks coming from the play store is different:

from play store:
/data/app/~~L9gwbE71f8ldQ5uKOC1TEg==/org.mozilla.fenix-WjJktxau7NFf6NYL9JoKTA==/split_config.arm64_v8a.apk!/lib/arm64-v8a/libxul.so
from apk:
/data/app/~~8ymB5WxZzJDqTX4U-j0BBw==/org.mozilla.fenix-X6nP2YTIjMRy-h2F_sSNeA==/lib/arm64/libxul.so

Maybe mmap is failing in https://searchfox.org/mozilla-central/rev/5134f3f1c7794fcff1bde59dc7b4ae65cc460919/mozglue/baseprofiler/core/shared-libraries-linux.cc#272

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:gbrown, since you are the author of the regressor, bug 1567912, could you take a look?

For more information, please visit BugBot documentation.

Comment 3

[Johan Lorenzo [:jlorenzo]]

Marking 121 as affected too because we shipped AAB on this version. :zmckenney, :gbrown, would you all have any ideas on how to proceed?

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

(In reply to Julien Wajsberg [:julienw] from comment #1)

Maybe mmap is failing in https://searchfox.org/mozilla-central/rev/5134f3f1c7794fcff1bde59dc7b4ae65cc460919/mozglue/baseprofiler/core/shared-libraries-linux.cc#272

Btw, the correct place is here: https://searchfox.org/mozilla-central/rev/5134f3f1c7794fcff1bde59dc7b4ae65cc460919/toolkit/crashreporter/google-breakpad/src/common/linux/file_id.cc#157
That's being called from: https://searchfox.org/mozilla-central/rev/5134f3f1c7794fcff1bde59dc7b4ae65cc460919/tools/profiler/core/shared-libraries-linux.cc#92
The baseprofiler one is a duplicate code that's not used at the moment.

Comment 5

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Until we fix the bug 1773313, I think I found an alternative way to fix this by adding android.bundle.enableUncompressedNativeLibs = false to gradle.properties file. But it looks like this feature is deprecated and it tells me this:

You can add the following to your build.gradle instead:
android {
    packagingOptions {
        jniLibs {
            useLegacyPackaging = true
        }
    }
}

But after discussing this with Zac McKenney on Matrix, he recommended to use useLegacyPackagingFromBundle instead. I haven't tried useLegacyPackaging or useLegacyPackagingFromBundle yet, but Zac's suggestion definitely sounds like the correct one.

On the other hand, I'm not so sure how it affects the apk size or the installed app size.

Comment 6

[Zac McKenney [:zmckenney]]

I've built with changes in the gradle file to continue compressing the libs as we have been doing for APK's and when extracting the APK's from the AAB it does show that with these changes it decreases the size of the APK file by almost 100MB. I don't have a lot of knowledge on native compression in APK's but from the docs it seems like the resulting on-device size will be increased. Also, I can't seem to find the profile.libs array anymore to determine if the change helps with the breakpadId or codeId. :canova would you be free tomorrow to help me find that?

Comment 7

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Thanks for testing! That's interesting that it actually decrease the APK size. But this means that we need to unpack and copy the .so files out of the APK files, which adds another copy in your device after it's installed. Is 100MB decrease a typo btw, it looks quite significant? I will test it myself as well but let me add the STR here so anyone who's interested can test it:

STR with remote profiling (there are other ways as well, but this is the easier IMO):

• Connect the phone to your machine and enable the remote debugging on Fenix settings.
• Go to about:debugging and enable the usb debugging there if you haven't already.
• Connect to your Fenix version on the left sidebar.
• After connecting click on your your machine/fenix that you want to profile.
• Click on the "Profile performance" button. Firefox Profiler recording panel will pop up.
• Press "start recording" button. Wait until "capture recording" button is clickable and click on it as well.
• After waiting some time, Firefox Profiler analysis view will show up.
• Open the DevTools console.
• Type "profile.libs" and check if all the objects in that array has non-empty breakpadId.
  (You can also validate it directly in the analysis UI without the console, but this is more reliable)

Hope this helps.

Comment 8

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Hmm actually adding useLegacyPackagingFromBundle = true doesn't fix the issue for me locally... Interestingly I added both useLegacyPackaging and useLegacyPackagingFromBundle to my gradle.properties and it doesn't extract the libs. When I add enableUncompressedNativeLibs again, things seem to be fixed but it gives me the same deprecation warning. We can get the breakpad and codeIds and libs are extracted outside.

Implementation note, for some reason adding the code (with changing it to useLegacyPackagingFromBundle) to build.gradle I mentioned in comment 5 didn't work. I had to add this to my gradle.properties file instead:

android.packagingOptions.jniLibs.useLegacyPackagingFromBundle=true

I also tried adding android.packagingOptions.jniLibs.useLegacyPackaging=true just to try to see and that doesn't make a difference either. I'm not sure why this happens.

Comment 9

[Zac McKenney [:zmckenney]]

Thank you, I was forgetting that you have to look in the developer console. I tested adding this legacy packaging to the bundle option in the gradle file and am seeing the codeId and breakpadId

bundle {
        packagingOptions {
            jniLibs {
                it.useLegacyPackaging = true
            }
        }
...

Here is a profile I ran with this included on a signed AAB that I created an APK from based on my Pixel 7 configuration.

Also, this will decrease the size of the resulting APK from within the AAB and that was not actually a typo. It is in fact a 97.3MB difference in resulting APK size. Although, the size on device for a user after decompressing will match what we have already been using for APK's so this shouldn't be an issue.

Comment 10

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Thanks for the code snippet! It looks like I was adding this to the fenix/build.gradle file which was wrong. After adding your recommendation to fenix/app/build.gradle file I can confirm that it fixes the issue for me too. Awesome!

Comment 11

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1865634 - Use legacy packaging for native code on AAB (#4611)

Comment 12

[Zac McKenney [:zmckenney]]

I've added the PR for the change above which should unblock the progress of moving to AAB on Fenix. I tested locally and this seems to work without issue but I will be requesting QA to at least smoke test the app in Nightly once it lands (still determining scope of that). :canova I want to leave this ticket open since the proposed PR is only a temporary fix but the changes needed to support AAB without legacy packaging should probably be tracked here or in another ticket.

Important note on changes needed to support AAB!

AAB's are only used in the Google Play Store and are required for Fenix as soon as possible (due to size limitations on the Play Store with APK's). We also need to support APK's because of stores like the Samsung Store and our archives. Just calling out that we will need to support both APK and AAB!

Comment 13

[BMO Github Automation]

Authored by Zac McKenney
https://github.com/mozilla-mobile/firefox-android/commit/e29acdb26ca6263416fe379cabdb67eb5b2794e6
[main] Bug 1865634 - Use legacy packaging for native code on AAB

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:zmckenney, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox121 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

[miralobontiu]

Verified as fixed on the latest Nightly 122.0a1 from 11/29 with Google Pixel 6 (Android 14).
Following these steps, I verified that the breakpadId had non-empty values.
Also ran a smoke & sanity check testing on the same build, and nothing new came out.

Comment 16

[Zac McKenney [:zmckenney]]

Thank you :miralobontiu! I can also confirm this on my Pixel 7. I will request uplift so we can have it in the Friday or Monday Beta build.

Comment 17

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1865634 - Use legacy packaging for native code on AAB (backport #4611) (#4642)

Comment 18

[Zac McKenney [:zmckenney]]

Comment on attachment 9366152 [details] [review]
[mozilla-mobile/firefox-android] Bug 1865634 - Use legacy packaging for native code on AAB (backport #4611) (#4642)

Beta/Release Uplift Approval Request

• User impact if declined: Profiler will not work properly and we cannot use it to test builds.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: (Already tested by QE) Capture a profile in about:debugging. When viewing the profile, open the web developer tools and run profile.libs. Look at the libxul.so breakpadId and codeId to confirm they are not missing.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is reverting the native libraries packaging back to what it was on APK's which would generally be considered low. Since this effects all native code the breadth is large which is why it is considered Medium. There are no alternatives other than reverting the AAB transition but this is a higher risk since we are already running into limits with APK size.
• String changes made/needed:
• Is Android affected?: Yes

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9366152 [details] [review]
[mozilla-mobile/firefox-android] Bug 1865634 - Use legacy packaging for native code on AAB (backport #4611) (#4642)

Approved for 121.0b6.

Comment 20

[BMO Github Automation]

Authored by Zac McKenney
https://github.com/mozilla-mobile/firefox-android/commit/c06a223086c12280a075103add7454342b5dc48c
[releases_v121] Bug 1865634 - Use legacy packaging for native code on AAB

Comment 21

[Margareta Eliza Ciure]

Verified as fixed on Firefox Beta 121.0b6 with Samsung Galaxy S23 Ultra (Android 13) and Tab: Sony Xperia (Android 6.0.1).