2084316 - Crash tapping "Go to settings" on the experiment branches snackbar after leaving the screen

Status: RESOLVED FIXED

(Firefox for Android :: Experimentation and Telemetry, defect, P1)

Description

[Julie De Lorenzo [:jdelorenzo]]

Tapping "Go to settings" on the "Enable telemetry to send data." snackbar crashes with an IllegalStateException if NimbusBranchesFragment is already gone.

showSnackbarToEnableExperiments attaches the snackbar to view.rootView rather than the Fragment's own view, so it stays on screen after the Fragment is destroyed. Its action lambda captured the stale Fragment and calls findNavController(), which throws -

java.lang.IllegalStateException
  at androidx.fragment.app.Fragment.getParentFragmentManager(Fragment.java:14)
  at androidx.navigation.fragment.FragmentKt.findNavController(Fragment.kt:18)
  at org.mozilla.fenix.nimbus.NimbusBranchesFragment$$ExternalSyntheticLambda5.invoke(R8$$SyntheticClass:3)
  at org.mozilla.fenix.compose.snackbar.Snackbar$Companion$$ExternalSyntheticLambda0.invoke(R8$$SyntheticClass:7)
  at mozilla.components.compose.base.snackbar.SnackbarData.performAction(Snackbar.kt:3)

Samples: 28a46d2f-6d3b-474d-bdea-2c0160261016 (158.0), 260a62e5-35b7-4f42-ac3c-a93780261001 (156.0.1)

Split out of bug 2066925

Comment 1

[Hackbot [:hackbot]]

NimbusBranchesFragment.kt passes view.rootView to Snackbar.make, so findSuitableParent in Snackbar.kt falls back to the window DecorView and the snackbar outlives the fragment, leaving its action to call findNavController() on a detached fragment. The fragment's own view is the CoordinatorLayout root of mozac_service_nimbus_experiment_details.xml, which findSuitableParent picks first and which is removed in onDestroyView.

Fix: Pass view instead of view.rootView, and have the action use a NavController captured in onViewCreated with nav(R.id.nimbusBranchesFragment, ...) from NavController.kt, so a tap during the exit animation does nothing. No test covers the fragment today, so add a Robolectric one beside NimbusBranchesControllerTest.kt, following the detached-fragment test in TabManagementFragmentTest.kt.

Suggested severity: S3. It is a reproducible crash, but the Nimbus Experiments screen is only reachable after unlocking the secret debug menu, so few release users can hit it.

---

If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
If you want to categorize your feedback you can add one of the following tags: ai-triage-{wrong-file, wrong-cause, hallucination, out-of-scope, not-a-bug, wrong-fix, shallow-fix, wrong-severity}.

Needinfo [e-mail] to have a patch generated for this bug, include any questions or directions as needed in your comment.

Comment 2

[Hackbot [:hackbot]]

Attachment: Bug 2084316 - Don't access a detached NimbusBranchesFragment from the enable-experiments snackbar action

The "Enable telemetry to send data." snackbar was attached to view.rootView, so it could outlive NimbusBranchesFragment, and its "Go to settings" action called findNavController() on the detached fragment, which threw an IllegalStateException.

• Attach the snackbar to the fragment's own view (a CoordinatorLayout), so it is removed together with the fragment.
• Capture the NavController in onViewCreated and navigate with nav(R.id.nimbusBranchesFragment, ...), so a tap that arrives after the fragment has been left (e.g. during the exit transition) does nothing instead of crashing.
• Resolve the snackbar strings from the parent view's context instead of the fragment.

Comment 3

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 2084316 - Use SnackbarDelegate with a non-root view

Comment 4

[Jonathan Almeida [:jonalmeida]]

Attachment: Bug 2084316 - Do not execute onClick in Snackbar if not attached to a window

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/11124311eafb
https://hg.mozilla.org/integration/autoland/rev/414b4ca33522
Use SnackbarDelegate with a non-root view r=android-reviewers,mcarare

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2e185039d9be
https://hg.mozilla.org/integration/autoland/rev/02edb0c1927d
Revert "Bug 2084316 - Use SnackbarDelegate with a non-root view r=android-reviewers,mcarare" for causing android failures

Comment 7

[rperta]

Backed out for causing android failures
Backout link
Push with failures
Failure log(s)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/54143156779a
https://hg.mozilla.org/integration/autoland/rev/b9cc4077634a
Use SnackbarDelegate with a non-root view r=android-reviewers,mcarare
https://github.com/mozilla-firefox/firefox/commit/d944f5a3d38d
https://hg.mozilla.org/integration/autoland/rev/3119c03c2b3e
Do not execute onClick in Snackbar if not attached to a window r=android-reviewers,mcarare

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b9cc4077634a
https://hg.mozilla.org/mozilla-central/rev/3119c03c2b3e