1839239 - Crash in [@ android.util.AndroidRuntimeException: at com.android.internal.policy.impl.PhoneWindow.requestFeature(PhoneWindow.java)] "requestFeature() must be called before adding content"

Status: VERIFIED FIXED

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

Description

[Chris Peterson [:cpeterson]]

Crash report: https://crash-stats.mozilla.org/report/index/527e74fb-04ec-4175-9eeb-cd0b90230617

Java stack trace:

android.util.AndroidRuntimeException: requestFeature() must be called before adding content
	at com.android.internal.policy.impl.PhoneWindow.requestFeature(PhoneWindow.java:359)
	at com.android.internal.app.AlertController.installContent(AlertController.java:233)
	at android.app.AlertDialog.onCreate(AlertDialog.java:356)
	at android.app.Dialog.dispatchOnCreate(Dialog.java:463)
	at android.app.Dialog.show(Dialog.java:288)
	at androidx.fragment.app.DialogFragment.onStart(DialogFragment.java:11)
	at androidx.fragment.app.FragmentStateManager.start(FragmentStateManager.java:30)
	at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:57)
	at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1139)
	at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:92)
	at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:74)
	at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:4)
	at android.os.Handler.handleCallback(Handler.java:739)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:145)
	at android.app.ActivityThread.main(ActivityThread.java:5951)
	at java.lang.reflect.Method.invoke(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:372)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1388)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1183)

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

:jonalmeida, could you consider increasing the severity of this top-crash bug?

For more information, please visit BugBot documentation.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Bryan, can you please help find an assignee for this Fenix topcrash? Thanks!

Comment 3

[Chris Peterson [:cpeterson]]

This crash looks like a regression in 115: we have crash reports from 115-117 (Nightly, Beta, and Release), but none before 115.

100% of the crash reports are from devices running API 21-23 (Android 5.0, 5.1, and 6.0). 70% are from ARMv7 devices.

Comment 4

[Chris Peterson [:cpeterson]]

While nearly all the crash reports are from Fenix, there are a few from Focus 115.0, so we know this crash is in code common to Fenix and Focus.

The first crash reports were from Nightly 115.0a1 build ID 20230510213701. If this crash was caused by a code change, it might have landed before this build, but here is the mozilla-central change log for build ID 20230510213701:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ab25e7dfc0dc5bb8217fb436aac0957faf03a793&tochange=ffee8e3a7870875ab78af162638548f72cdabddf

The only suspicious Android change in that change log is compositor bug 1824083. Seems more likely that a change in Fenix/Focus or A-C code would cause this crash related to fragments and alerts.

Comment 5

[Arturo Mejia [:amejia]]

Sentry crash report https://mozilla.sentry.io/issues/4279189452/?project=6375561&query=is%3Aunresolved+level%3Afatal+requestFeature&referrer=issue-stream&sort=freq&statsPeriod=14d&stream_index=0

Sentry Focus :
https://mozilla.sentry.io/issues/4279093335/?project=6176960&query=is%3Aunresolved+requestFeature&referrer=issue-stream&statsPeriod=14d&stream_index=0

Comment 6

[Roger Yang [:royang]]

Hi Jamie, is this something that could possibly be related to bug 1824083?

Comment 7

[Chris Peterson [:cpeterson]]

Do we know what the stack trace is trying to do calling AlertDialog.onCreate() and PhoneWindow.requestFeature()? Is this code trying to ask the user for a new app permission?

android.util.AndroidRuntimeException: requestFeature() must be called before adding content
	at com.android.internal.policy.impl.PhoneWindow.requestFeature(PhoneWindow.java:359)
	at com.android.internal.app.AlertController.installContent(AlertController.java:233)
	at android.app.AlertDialog.onCreate(AlertDialog.java:356)
	at android.app.Dialog.dispatchOnCreate(Dialog.java:463)
	at android.app.Dialog.show(Dialog.java:288)
	at androidx.fragment.app.DialogFragment.onStart(DialogFragment.java:11)
	at androidx.fragment.app.FragmentStateManager.start(FragmentStateManager.java:30)
	at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:57)
	at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1139)
	at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:92)
	at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:74)
	at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:4)
	at android.os.Handler.handleCallback(Handler.java:739)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:145)
	at android.app.ActivityThread.main(ActivityThread.java:5951)
	at java.lang.reflect.Method.invoke(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:372)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1388)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1183)

Comment 8

[Jamie Nicol [:jnicol]]

(In reply to Roger Yang [:royang] from comment #6)

Hi Jamie, is this something that could possibly be related to bug 1824083?

It seems pretty unlikely given the stack trace, but to encounter that bug in the first place there had to be something strange going on, so I won't rule it out completely.

The nightly crash numbers are pretty low, with large gaps between build IDs that have >= 1 crash. So I don't think we can safely look at the changelog for a single nightly version. Also given the stack trace has nothing gecko-related in it, perhaps we ought to look at the android-components changelog too.

Comment 9

[Arturo Mejia [:amejia]]

I think it's really unlikely this is gecko/GeckoView related, I think this is something that could be on the Android layer.

Comment 10

[Arturo Mejia [:amejia]]

Attachment: Android_versions.png

Checking the Sentry Android versions, it looks like is happening on older Android versions, where newest one is Android 6.0.1.

Comment 11

[Arturo Mejia [:amejia]]

This patch seen a bit suspicious, I'm not completely sure if it could be the source.

Comment 12

[Chris Peterson [:cpeterson]]

(In reply to Arturo Mejia [:amejia] from comment #10)

Checking the Sentry Android versions, it looks like is happening on older Android versions, where newest one is Android 6.0.1.

Socorro shows the same pattern: 100% of the crash reports are from API 21, 22, or 23 (Android 5.0 - 6.0):

https://crash-stats.mozilla.org/search/?signature=~requestFeature%28PhoneWindow&product=Fenix&product=Focus&date=%3E%3D2023-01-10T21%3A48%3A00.000Z&date=%3C2023-07-10T21%3A48%3A00.000Z&_facets=signature&_facets=version&_facets=major_version&_facets=cpu_arch&_facets=android_version&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-android_version

Comment 13

[Arturo Mejia [:amejia]]

As the stack-trace is very generic, it's difficult to know for sure the source, I think adding some extra information to the crash could help, I'll add some Sentry breadcrumbs on each part where we create a dialog, that will give us some hints about which is the dialog the produce crash.

Comment 14

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (#2804)

Comment 15

[Chris Peterson [:cpeterson]]

In the meantime, should we temporarily back out Jonathan's tabs tray fix from bug 1822448 to see if that fixes the issue?

Comment 16

[Arturo Mejia [:amejia]]

We could but I'm not 100% sure that it is the root cause. We could back it out and see if the crashes reduce and it doesn't we can put it back, what do you think?

Comment 17

[BMO Github Automation]

Authored by https://github.com/Amejia481
https://github.com/mozilla-mobile/firefox-android/commit/19a1105e8b1a6699aa83467dbaeb5e84cd938c79
[main] Bug 1839239 - Add crash bread crumbs for dialogs.

Comment 18

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2811)

Comment 19

[Arturo Mejia [:amejia]]

Comment on attachment 9343489 [details] [review]
[mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2811)

Beta/Release Uplift Approval Request

• User impact if declined: The rate on this bug will continue increasing without a solution.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are only collecting more information about the crash, nothing visible to the user.
• String changes made/needed:
• Is Android affected?: Yes

Comment 20

[Donal Meehan [:dmeehan]]

:amejia could you add a beta uplift request also on a backport for 116?
We should get this into beta first and then we can pick it for next week's 115 dot release.

Comment 21

[Arturo Mejia [:amejia]]

Sure!

Comment 22

[Arturo Mejia [:amejia]]

The crash rate in beta is similar is similar to nightly, both are really low, it looks like we don't have enough of those devices there, it's very likely that we will need to wait until the next week's 115 dot release to start getting data.

Comment 23

[BMO Github Automation]

Attachment: [mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2813)

Comment 24

[Arturo Mejia [:amejia]]

Comment on attachment 9343508 [details] [review]
[mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2813)

Beta/Release Uplift Approval Request

• User impact if declined: The rate on this bug could continue increasing without a solution.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are only collecting more information about the crash, nothing visible to the user.
• String changes made/needed:
• Is Android affected?: Yes

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9343508 [details] [review]
[mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2813)

Approved for 116.0b5.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9343508 [details] [review]
[mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2813)

Sorry, reverting this back as it looks like the backport PR needs more work before it's ready to land.

Comment 27

[Arturo Mejia [:amejia]]

Thanks Ryan!
The patch is now updated!

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9343508 [details] [review]
[mozilla-mobile/firefox-android] Bug 1839239 - Add crash bread crumbs for dialogs. (backport #2804) (#2813)

Thanks, approved again for 116.0b5!

Comment 29

[BMO Github Automation]

Authored by https://github.com/Amejia481
https://github.com/mozilla-mobile/firefox-android/commit/2b4e399890984b57ce71124f9961e023b02fda57
[releases_v116] Bug 1839239 - Add crash bread crumbs for dialogs.

Comment 30

[Chris Peterson [:cpeterson]]

(In reply to Arturo Mejia [:amejia] from comment #16)

We could but I'm not 100% sure that it is the root cause. We could back it out and see if the crashes reduce and it doesn't we can put it back, what do you think?

Since this bug's crash rate continues to climb in 115, I think backing out Jonathan's change is worth testing. His fix was pretty small and should be easy to back out. Plus, the bug we would be temporarily reintroducing (an animation issue on older Android versions) isn't as bad as this crash.

Since the crash rate is pretty low in Nightly and Beta, we will probably need to uplift to next week's 115 dot release to see if there is any change.

The uplift deadline for next week's 115 dot release is this Friday (July 14).

Comment 31

[Arturo Mejia [:amejia]]

Luckily we started to get some data in nightly:

We got 4 reports

• 3 pointing TimePickerDialogFragment (1, 2, and 3) Probably the same user.
• 1 pointing SyncManager sync ended.

As this stack is pretty generic is very likely that is no one issue, but multiple issues aggregated in a generic stack.
I started taking a look the new data to see if something that we can address.

Comment 32

[Arturo Mejia [:amejia]]

The last changes to TimePickerDialogFragment was this commit Bug 1815637 - Add rtl support which applies withCenterAlignedButtons to practically every dialog.

Comment 33

[Arturo Mejia [:amejia]]

I was able to reproduce the crash locally, with an emulator Pixel 6 API 23 (Android 6 Google APIs | arm64) .

STR:

1. Go to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/time
2. Tap on the time input with the label "Choose a time for your meeting"

After removing the calls to withCenterAlignedButtons I don't get the crash anymore.

As the crash doesn't happen on newer version of Android, we could disable withCenterAlignedButtons on API levels lower or equal to 23, so we don't lose the improvements for newer Android versions.

Comment 34

[Arturo Mejia [:amejia]]

Unfortunately, checking closer at the reports we also have small number of Android versions like 13, and 12. I'll back out Bug 1815637 - Add rtl support.

Adding Mike for future research about bug #1815637

Comment 35

[Arturo Mejia [:amejia]]

This is the patch for back out Bug 1815637 - Add rtl support.

Comment 36

[Arturo Mejia [:amejia]]

Attachment: GitHub Pull Request

Comment 37

[Arturo Mejia [:amejia]]

Attachment: Reverts "Bug 1815637 - Add rtl support."

Comment 38

[Chris Peterson [:cpeterson]]

Mike, since Arturo is backing out your fix for RTL bug 1815637, should we reopen that bug, file a new bug, or accept that the RTL issue won't be fixed?

Comment 39

[Arturo Mejia [:amejia]]

Sorry, I just noticed the message, and I already opened the bug.

Comment 40

[Arturo Mejia [:amejia]]

Hi QA team,
Could you help us to verify that the crash doesn't happen on nightly steps can be found on comment #33.

Thanks in advance!

Comment 41

[Joe M [:jmahon]]

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

Mike, since Arturo is backing out your fix for RTL bug 1815637, should we reopen that bug, file a new bug, or accept that the RTL issue won't be fixed?

PSA: Mike's out on PTO this week - with bug 1815637 reopened, he can make a call on next steps for it when he's back online next week.

Comment 42

[Delia Pop (Android QA)]

(In reply to Arturo Mejia [:amejia] from comment #40)

Hi QA team,
Could you help us to verify that the crash doesn't happen on nightly steps can be found on [comment #33]

Hi, we followed the STR using the latest Nightly 117.0a1 from 07/14 and the application remained stable.
Devices used for testing:

• Huawei MediaPad M2 (Android 5.1.1)
• Sony Xperia (Android 6.0.1)
• OPPO A15s (Android 10)
• Xiaomi 12 Pro (Android 13)

Comment 43

[Arturo Mejia [:amejia]]

Thanks Delia and Joe!

Comment 44

[Arturo Mejia [:amejia]]

Attachment: Reverts "Bug 1815637 - Add rtl support." (backport #2835) in Beta

Comment 45

[Arturo Mejia [:amejia]]

Comment on attachment 9343891 [details] [review]
Reverts "Bug 1815637 - Add rtl support." (backport #2835) in Beta

Beta/Release Uplift Approval Request

• User impact if declined: The rate on this bug could continue increasing.
• 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:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are just reverting a patch.
• String changes made/needed:
• Is Android affected?: Yes

Comment 46

[Arturo Mejia [:amejia]]

Attachment: Reverts "Bug 1815637 - Add rtl support." (backport #2835) in Release

Comment 47

[Arturo Mejia [:amejia]]

Comment on attachment 9343908 [details] [review]
Reverts "Bug 1815637 - Add rtl support." (backport #2835) in Release

Beta/Release Uplift Approval Request

• User impact if declined: The rate on this bug could continue increasing.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We are just reverting a patch, and verify the crash was fixed with the revert
• String changes made/needed:
• Is Android affected?: Yes

Comment 48

[Dianna Smith [:diannaS]]

Comment on attachment 9343891 [details] [review]
Reverts "Bug 1815637 - Add rtl support." (backport #2835) in Beta

Approved for 116.0b6

Comment 49

[Lorand Janos]

There is no crash on the latest Beta 116.0b6 build with steps from https://bugzilla.mozilla.org/show_bug.cgi?id=1839239#c33.

Devices used:

• Lenovo Tab P11 Plus (Android 12)
• Oppo Find X5 (Android 13)
• Google Pixel 7 Pro (Android 14)
• Samsung S22 Ultra (Android 13)

Also checked on the latest Nightly 117.0a1 from 2023-07-17 with the same result.

Marking the ticket as verified for 116 and 117.

Comment 50

[Dianna Smith [:diannaS]]

Comment on attachment 9343908 [details] [review]
Reverts "Bug 1815637 - Add rtl support." (backport #2835) in Release

Approved for Mobile 115.2.1

Comment 51

[BMO Github Automation]

Authored by https://github.com/mergify[bot]
https://github.com/mozilla-mobile/firefox-android/commit/046c284cb99ec97ef18f187ca91f56bcd8ddfd4b
[releases_v115] Bug 1839239 - Add crash bread crumbs for dialogs. (#2811)

Comment 52

[Andrei Mocirean]

Verified as fixed on Fenix RC 115.2.1, using the STR from Comment 33.

Devices used for testing:

• Samsung Galaxy Tab S3 (Android 9)
• Oppo Find X5 (Android 12)
• Google Pixel 7 (Android 14)
• Samsung Galaxy Tab A6 (Android 5.1.1)
• Google Pixel 7 Pro (Android 14)
• Lenovo Tab P11 Plus (Android 12)
• LG Nexus 5 (Android 6.0.1)