2042018 - Crash in [@ java.lang.IllegalStateException: at androidx.fragment.app.Fragment.requireContext(Fragment.java)]

Status: VERIFIED FIXED

(Firefox for Android :: Downloads, defect)

Description

[Donal Meehan [:dmeehan]]

Crash report: https://crash-stats.mozilla.org/report/index/ab33639b-020b-4009-8de1-5aa320260524

Top 10 frames:

0  androidx.fragment.app.Fragment  requireContext  Fragment.java:18
1  org.mozilla.fenix.browser.BaseBrowserFragment$$ExternalSyntheticLambda86  onDismiss  R8$$SyntheticClass:6
2  android.app.Dialog$ListenersHandler  handleMessage  Dialog.java:1477
3  android.os.Handler  dispatchMessage  Handler.java:106
4  android.os.Looper  loopOnce  Looper.java:201
5  android.os.Looper  loop  Looper.java:288
6  android.app.ActivityThread  main  ActivityThread.java:8061
7  java.lang.reflect.Method  invoke  Method.java:-2
8  com.android.internal.os.RuntimeInit$MethodAndArgsCaller  run  RuntimeInit.java:703
9  com.android.internal.os.ZygoteInit  main  ZygoteInit.java:911

Comment 1

[Donal Meehan [:dmeehan]]

This crash existed before 151, but there is an increase in volume compared to 150.
Claude thinks the regressor is Bug 2002273 for the following reason:

Trivial diff in behavior:

  - Before: the dismiss handler used a context variable that was saved at the top of a much larger function. That saved
  reference stays alive even after the fragment detaches, so the handler runs fine.
  - After: the dialog code got moved into its own little helper method, showFirstPartyDownloadDialog. In moving it, the
  dismiss handler stopped using the saved context and started calling requireContext() directly — which throws if the fragment
   isn't attached.

  That's the entire regression. It's a one-word change (context → requireContext()) introduced incidentally by a refactor, not
   on purpose.

  The fix

  Re-save the context at the top of the helper method, then use the saved one in the dismiss handler — same shape as the
  working sibling dialog on the previous line.

  In mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt, the showFirstPartyDownloadDialog
   method (around line 2520):

  private fun showFirstPartyDownloadDialog(
      currentDownloadState: CurrentDownloadState,
      positiveAction: PositiveActionCallback,
      negativeAction: NegativeActionCallback,
  ) {
      val context = context ?: return                    // ← add
      …
      downloadDialog = MaterialAlertDialogBuilder(context)  // ← was requireContext()
          …
          .setOnDismissListener {
              downloadDialog = null
              context.components.analytics.crashReporter.recordCrashBreadcrumb(  // ← was requireContext()
                  Breadcrumb("FirstPartyDownloadDialog onDismiss"),
              )
          }.show()
  }
  
  Two changes:

  1. val context = context ?: return at the top — grabs the current context once, bails out cleanly if the fragment isn't
  attached.
  2. Replace both requireContext() calls inside the method with that saved context.
  
  The dismiss handler now uses the captured reference, which stays valid even if the fragment detaches before the dialog
  finishes dismissing. The crash goes away.

  This is the same pattern the other (unchanged) dialog on line 776 of the same file uses correctly — Bug 2002273 just forgot
  to carry the pattern over when it extracted the new method.

Could you take a look and confirm if Claude is correct?

Comment 2

[Iorga Gabriel]

Attachment: Bug 2042018 - Fix Crash in [@ java.lang.IllegalStateException: at androidx.fragment.app.Fragment.requireContext(Fragment.java)].

Comment 3

[Donal Meehan [:dmeehan]]

Clearning NI since there is a patch. We could include this in the dot release next week.

Comment 4

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

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

• Top 10 AArch64 and ARM crashes on nightly
• Top 10 AArch64 and ARM crashes on beta

For more information, please visit BugBot documentation.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cd8a8833fc7d
https://hg.mozilla.org/integration/autoland/rev/3e3e0214a37b
Fix Crash in [@ java.lang.IllegalStateException: at androidx.fragment.app.Fragment.requireContext(Fragment.java)]. r=android-reviewers,tthibaud

Comment 6

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/3e3e0214a37b

Comment 7

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

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

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox152 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Donal Meehan [:dmeehan]]

To add to comment 7, please add an uplift request for release

Comment 9

[Iorga Gabriel]

Attachment: Bug 2042018 - Fix Crash in [@ java.lang.IllegalStateException: at androidx.fragment.app.Fragment.requireContext(Fragment.java)].

Original Revision: https://phabricator.services.mozilla.com/D302274

Comment 10

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: The app will crash
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: yes
• Steps to reproduce for manual QE testing: I don't have any.
• Risk associated with taking this patch: low
• Explanation of risk level: It was a if check.
• String changes made/needed?: no
• Is Android affected?: yes

Comment 11

[Iorga Gabriel]

Attachment: Bug 2042018 - Fix Crash in [@ java.lang.IllegalStateException: at androidx.fragment.app.Fragment.requireContext(Fragment.java)].

Original Revision: https://phabricator.services.mozilla.com/D302274

Comment 12

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: The app will crash
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: yes
• Steps to reproduce for manual QE testing: I don't have any.
• Risk associated with taking this patch: low
• Explanation of risk level: It was a if check.
• String changes made/needed?: no
• Is Android affected?: yes

Comment 13

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/fa0c24fe9fc6
https://hg.mozilla.org/releases/mozilla-beta/rev/50b7edf5512c

Comment 14

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/2de797f3f876
https://hg.mozilla.org/releases/mozilla-release/rev/af953e0efab4

Comment 15

[Mira Lobontiu (Android QA)]

Verified there is no crash encountered while renaming the downloaded files.
Tested on Firefox for Android Beta 152.0b6 with Pixel Tablet 9Android 16), Xiaomi 12T (Android 12), and Asus Zenfone (Android 13).
Tested on Firefox for Android Nightly 153.0a1 from 6/1 on Pixel 6 (Android 17), and Huawei Pura 70 (Android 12).