mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.47k stars 1.27k forks source link

Crash in QuickSettingsSheetDialogFragment$onCreateView$1.invoke(QuickSettingsSheetDialogFragment.kt:1) #23256

Closed Amejia481 closed 2 years ago

Amejia481 commented 2 years ago

Socorro https://crash-stats.mozilla.org/signature/?product=Fenix&version=98.0a1&signature=java.lang.IllegalStateException%3A%20at%20androidx.fragment.app.Fragment.getParentFragmentManager%28Fragment.java%29&date=%3E%3D2022-01-10T18%3A50%3A00.000Z&date=%3C2022-01-17T18%3A50%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1

java.lang.IllegalStateException
    at androidx.fragment.app.Fragment.getParentFragmentManager(Fragment.java:2)
    at androidx.navigation.fragment.NavHostFragment.findNavController(NavHostFragment.java:3)
    at kotlin.jvm.internal.ArrayIteratorKt.findNavController(ArrayIterator.kt:1)
    at org.mozilla.fenix.settings.quicksettings.QuickSettingsSheetDialogFragment$onCreateView$1.invoke(QuickSettingsSheetDialogFragment.kt:1)
    at org.mozilla.fenix.settings.quicksettings.DefaultQuickSettingsController.handleClearSiteDataClicked(QuickSettingsController.kt:5)
    at org.mozilla.fenix.settings.quicksettings.QuickSettingsInteractor.onClearSiteDataClicked(QuickSettingsInteractor.kt:1)
    at org.mozilla.fenix.settings.quicksettings.ClearSiteDataView$$ExternalSyntheticLambda0.onClick(Unknown Source:4)
    at androidx.appcompat.app.AlertController$ButtonHandler.handleMessage(AlertController.java:3)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loop(Looper.java:280)
    at android.app.ActivityThread.main(ActivityThread.java:6710)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

┆Issue is synchronized with this Jira Task

mcarare commented 2 years ago

@Amejia481 Is the quick settings sheet supposed to be dismissed after the cookies and site data have been cleared? I do not see any UX directions on that. If we do not dismiss it we might avoid some concurrency issue with the clear site data dialog being dismissed and find findNavController being called.

If it is supposed to be, we can try to pass an actual navController for the DefaultQuickSettingsController instead of passing the lambda to get one.

Amejia481 commented 2 years ago

@Amejia481 Is the quick settings sheet supposed to be dismissed after the cookies and site data have been cleared? I do not see any UX directions on that. If we do not dismiss it we might avoid some concurrency issue with the clear site data dialog being dismissed and find findNavController being called.

Yes, I think we should dismiss it. We can even dismiss it before opening the confirmation dialog.

mcarare commented 2 years ago

For QA: Not sure if QA could reproduce this (STR: just use the clear site feature), but the testing should include re-testing the clear site feature.

Also, the top crashing devices for this were:

Ideally, QA should test on these devices.

SoftVision-LorandJanos commented 2 years ago

I couldn’t reproduce the crash, using a Samsung Galaxy Tab S3 ( Android 9) and an Oppo Reno 6 with Android 11, as the closest to the devices mentioned.

mcarare commented 2 years ago

Closing this as per the comment above.