1792796 - Android 5.1 RTL Compose crash in [@ java.lang.ArrayIndexOutOfBoundsException: at androidx.compose.foundation.layout.Arrangement.placeRightOrBottom/placeLeftOrTop

Status: RESOLVED WORKSFORME

(Firefox for Android :: General, defect, P5)

Description

[Chris Peterson [:cpeterson]]

This Android Compose crash looks a regression in 106.0a1 build ID 20220908092810.

java.lang.ArrayIndexOutOfBoundsException: length=2; index=2
	at androidx.compose.foundation.layout.Arrangement.placeRightOrBottom$foundation_layout_release(Arrangement.kt:5)
	at androidx.compose.foundation.layout.Arrangement$Start$1.arrange(Arrangement.kt:3)
	at androidx.compose.foundation.layout.RowKt$rowMeasurePolicy$1$1.invoke(Row.kt:4)
	at androidx.compose.foundation.layout.RowColumnImplKt$rowColumnMeasurePolicy$1$measure$4.invoke(RowColumnImpl.kt:13)
	at androidx.compose.ui.layout.MeasureScope$layout$1.placeChildren(MeasureScope.kt:9)
	at androidx.compose.ui.node.LayoutNode$layoutChildren$1.invoke(LayoutNode.kt:17)
	at androidx.compose.ui.node.OwnerSnapshotObserver.observeReads$ui_release(OwnerSnapshotObserver.kt:34)
	at androidx.compose.ui.node.LayoutNode.layoutChildren$ui_release(LayoutNode.kt:16)

Comment 1

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

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 AArch64 and ARM crashes on beta

For more information, please visit auto_nag documentation.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

https://github.com/mozilla-mobile/android-components/commit/6bc10e429c5c04611ee6254269a76f11bec313d0 seems like the most likely candidate for causing this crash spike

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Fenix picked up the compose update a couple days before that, so that's also possible given the relatively low volume of crashes on Nightly.
https://github.com/mozilla-mobile/fenix/commit/bcfd0eb125e654a863fb55b63db072a207b383e1

Comment 4

[Mihai Adrian Carare [:mcarare]]

I looked into the change log for the updates, and it does not seem to be anything changed in the files from the stack traces.
Also, interestingly enough, the crash only occurs on SDK 22 ( Android 5.1 )

Comment 5

[Mihai Adrian Carare [:mcarare]]

No clear STR for reproducing this. I have not been able to reproduce this on an emulator with SDK 22 and I only see mainly Huawei and Lenovo devices in the device list. (No such physical device at SV is available for testing ).

The stack trace indicates that this is a crash that occurs when the home fragment is displayed and onMeasure is called, so no real way to avoid that. Also, no traces as to what Compose component on the main screen is influencing the end result (it might be just the parent layout placing children inside, so again, no way to avoid that ).

As the crashes started happening on Compose version upgrade and we do not have the option to downgrade it (it is a requirement for adopting Android 13) I propose we upgrade to 1.3.0. as soon as it is available (it's already at RC since 5th of October) and follow this signature to see if it still shows up.

As a note, I looked into all the release note changes, and none seem to be related to this. Also, I looked into reports of this on the Google issue tracker and there is none. I would have filled one, but with no reproducible sample, it would be ignored. Being isolated to SDK 22 and a few devices would not help prioritize it either.

Comment 6

[Chris Peterson [:cpeterson]]

Looks like Compose 1.3.0 was released on October 4, so we can try updating now. That is Fenix bug https://github.com/mozilla-mobile/fenix/issues/27536

https://developer.android.com/jetpack/androidx/releases/compose

Comment 7

[Mihai Adrian Carare [:mcarare]]

The upgrade of Compose is currently blocked by compiling with SDK 33.

Comment 8

[Dianna Smith [:diannaS]]

:cpeterson is the plan to fix this in 109 now that bug 1798131 bug 1771343 is in fx108?

Comment 9

[Chris Peterson [:cpeterson]]

(In reply to Dianna Smith [:diannaS] from comment #8)

:cpeterson is the plan to fix this in 109 now that bug 1798131 is in fx108?

Do you mean Android 13 bug 1771343? Bug 1798131 is a Linux desktop bug.

Bug 1771343 was about updating GeckoView. We still need to update A-C (bug 1796353) and Fenix (https://github.com/mozilla-mobile/fenix/issues/25808 and https://github.com/mozilla-mobile/fenix/issues/27407), which we're trying to do in Fenix 109.

We won't be able to uplift all those changes to Beta 108, so I'm setting the status-firefox108 tracking flag to wontfix.

Comment 10

[Dianna Smith [:diannaS]]

ty yes, copy paste mistake on my part

Comment 11

[Chris Peterson [:cpeterson]]

P1 because this bug is our top crash (behind EmptyMinidump). We're hoping that upgrading Jetpack Compose in https://github.com/mozilla-mobile/fenix/issues/27536 will fix these crashes.

Comment 12

[Chris Peterson [:cpeterson]]

I will monitor Beta 109 and Nightly 110 for crash reports.

Comment 13

[Chris Peterson [:cpeterson]]

https://github.com/mozilla-mobile/fenix/issues/27536 updated Fenix 109.0a1 from Compose 1.2.1 to 1.3.1, but these ArrayIndexOutOfBoundsExceptions are still our top crash in Fenix 109.0b and 110.0a1. As Mihai noted above, 100% of these crashes are from SDK 22 (Android 5.1).

Comment 14

[Mihai Adrian Carare [:mcarare]]

Looking at the first reports of this crash, it seems the crash did not occur in the first builds after https://github.com/mozilla-mobile/fenix/commit/bcfd0eb125e654a863fb55b63db072a207b383e1 landed, but after the new onboarding was enabled. (20220908092810).
This info aligns with the breadcrumbs we can find in most of the sentry reports for showing the onboarding dialog.

To further build upon the conclusion that is not the update that caused the increase in the crashes with this signature, we can look into Focus.
The 1.2.1 update landed in 106 (https://github.com/mozilla-mobile/focus-android/commit/fc9f713545fcef569eb0ce1284fb881a90c6b045 ) but the first crashes occurred only from 107: https://crash-stats.mozilla.org/signature/?signature=java.lang.ArrayIndexOutOfBoundsException%3A%20at%20androidx.compose.foundation.layout.Arrangement.placeRightOrBottom%24foundation_layout_release%28Arrangement.kt%3A46%29&product=Focus&date=%3E%3D2022-06-19T14%3A46%3A00.000Z&date=%3C2022-12-19T14%3A46%3A00.000Z&_sort=-date. 107 is the version the new onboarding was enabled by default in Focus. (it uses a similar dialog structure)

This does not identify the actual culprit, but it helps by removing compose updates from the investigation.

P.S.: If I was mistaken on the timeline, please correct me, as it helps pinpoint the change that made the crash rate spike.

Comment 15

[Mihai Adrian Carare [:mcarare]]

See also :https://issuetracker.google.com/issues/259993814

Comment 16

[Chris Peterson [:cpeterson]]

99% of these crashes are from users with Persian (fa_IR) or Arabic (ar_EG) locales, so this is probably an RTL bug. The Google bug report https://issuetracker.google.com/issues/259993814 also mentions Android 5.1 and RTL locales.

https://crash-stats.mozilla.org/search/?signature=~java.lang.ArrayIndexOutOfBoundsException%3A%20at%20androidx.compose.foundation.layout.Arrangement.place&product=Fenix&product=Focus&date=%3E%3D2022-06-19T19%3A48%3A00.000Z&date=%3C2022-12-19T19%3A48%3A00.000Z&_facets=signature&_facets=version&_facets=major_version&_facets=cpu_arch&_facets=android_version&_facets=useragent_locale&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-useragent_locale

Comment 17

[Chris Peterson [:cpeterson]]

Dropping priority to P5. Unfortunately, this bug is not currently actionable. Based on crash stats, I estimate that fewer than 1000 users are affected.

Comment 18

[Chris Peterson [:cpeterson]]

Can we work around this crash by disabling RTL text layout on Firefox's homepage for users on Android 5.1? Is RTL something we can control in our own layouts or is it controlled by the OS? Showing LTR text would be better than crashing.

Comment 19

[Chris Peterson [:cpeterson]]

Harrison will try disabling RTL by adding supportRtl="false" to the Home activity's manifest.

Comment 20

[Chris Peterson [:cpeterson]]

Syncing this bug to Jira because it's more of an issue for the Experience team than the Foundation team.

(In reply to Chris Peterson [:cpeterson] from comment #19)

Harrison will try disabling RTL by adding supportRtl="false" to the Home activity's manifest.

Petru says:

If the issue affects Compose code from the homescreen then we can disable RTL in only the Compose code by wrapping composables in a CompositionLocalProvider which will disable RTL only for the wrapped composables and only on the affected Android versions with something like:

@Composable
fun layoutDirection() = if (Build.VERSION.SDK_INT > Build.VERSION_CODES.LOLLIPOP) {
    LocalLayoutDirection.current
} else {
    LayoutDirection.Ltr
}

@Composable
fun CurrentComposable() {
    CompositionLocalProvider(LocalLayoutDirection provides layoutDirection()) {
        // .. the current compose code
    }
}

Comment 21

[Chris Peterson [:cpeterson]]

Removing the [geckoview:m110] whiteboard tag because this looks like an Experience team bug, not a Foundation team bug.

Comment 22

[Chris Peterson [:cpeterson]]

I filed a bug in Google's Compose bug tracker: https://issuetracker.google.com/issues/266059178

Comment 23

[BMO Automation]

Attachment: [mozilla-mobile/fenix] For 1792796 - Force Left to Right layout direction for Onboarding whe… (#28727)

Comment 24

[Donal Meehan [:dmeehan]]

:harrisono any updates on a fix for this? The attached PR is closed with a comment that there will be a follow-up.
Wondering if we can expect an uplift for 111, or if this should ride the train when it lands and 111 should be wontfix?

Comment 25

[BMO Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1792796 - Force Left to Right layout direction for Onboarding whe… (#850)

Comment 26

[[:harrisono]]

:dmeehan I have recreated the branch on the new monorepo and created a new PR for it.
https://github.com/mozilla-mobile/firefox-android/pull/850
I think it's best to let this ride the train through nightly and beta before we land to a full release.

Comment 27

[Donal Meehan [:dmeehan]]

(In reply to [:harrisono] from comment #26)

:dmeehan I have recreated the branch on the new monorepo and created a new PR for it.
https://github.com/mozilla-mobile/firefox-android/pull/850
I think it's best to let this ride the train through nightly and beta before we land to a full release.

Thanks for the info - we still have another 2 weeks of beta before 111 goes to release
https://whattrainisitnow.com/release/?version=111
I'll set 111 as fix-optional, if the patch is low risk and after some nightly testing we may still have time for 111 - depending on when it lands

Comment 28

[[:harrisono]]

(In reply to Donal Meehan [:dmeehan] from comment #27)

(In reply to [:harrisono] from comment #26)

:dmeehan I have recreated the branch on the new monorepo and created a new PR for it.
https://github.com/mozilla-mobile/firefox-android/pull/850
I think it's best to let this ride the train through nightly and beta before we land to a full release.

Thanks for the info - we still have another 2 weeks of beta before 111 goes to release
https://whattrainisitnow.com/release/?version=111
I'll set 111 as fix-optional, if the patch is low risk and after some nightly testing we may still have time for 111 - depending on when it lands

Sounds good, thank you for following up on this. I had forgotten to recreate the PR on the new monorepo.

Comment 29

[Chris Peterson [:cpeterson]]

The PR was merged on 2023-02-28:
https://github.com/mozilla-mobile/firefox-android/commit/9c1b58d4a8189dcbb647e05884b552552628dc0c

But I see we still have some crash reports from build IDs >= 2023-03-01, so I don't think this speculative fix fixed the crash:

https://crash-stats.mozilla.org/search/?signature=~java.lang.ArrayIndexOutOfBoundsException%3A%20at%20androidx.compose.foundation.layout.Arrangement.place&release_channel=nightly&build_id=%3E%3D20230301000000&product=Fenix&date=%3E%3D2023-02-21T17%3A19%3A00.000Z&date=%3C2023-03-07T17%3A19%3A00.000Z&_facets=signature&_facets=version&_facets=cpu_arch&_facets=major_version&_facets=build_id&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-build_id

I'll keep this bug open for now and continue to monitor the crash volume when 112 hits Beta next week.

Comment 30

[Donal Meehan [:dmeehan]]

:cpeterson/:harrisono , re: comment 29 - There is still crash volume on beta for 112.

Comment 31

[Chris Peterson [:cpeterson]]

(In reply to Donal Meehan [:dmeehan] from comment #30)

:cpeterson/:harrisono , re: comment 29 - There is still crash volume on beta for 112.

Harrison's speculative fix didn't fix the crashes, so he plans to file a new bug to revert his speculative fix. We don't have any other leads for this bug. A Google engineer reviewed the code and said they didn't see any obvious bugs in Fenix's code and that Google would be unlikely to make a fix to Compose for a bug that only affects Android 5.1: https://issuetracker.google.com/issues/266059178#comment5

Donal, do you recommend that we resolve this bug as WONTFIX or drop the priority to P5?

Comment 32

[Donal Meehan [:dmeehan]]

You could drop the priority/severity if you triaged it as a lower priority.
I've set 112/113 to wontfix if you don't think this can be addressed

Comment 33

[Joe M [:jmahon]]

I'm going to close the associated Kanban ticket (https://mozilla-hub.atlassian.net/browse/FXDROID-35) and leave this in our official Bugzilla backlog instead of tracking it in Jira.

If someone picks this ticket up in the future, feel free to either re-open that Jira ticket, or create a new one from scratch.

Comment 34

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

Based on the topcrash criteria, the crash signatures linked to this bug are not in the topcrash signatures anymore.

For more information, please visit BugBot documentation.

Comment 35

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

Closing because no crashes reported for 12 weeks.