From bc4046035ff3c292b79e227c9bf27b4660cd3486 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Tue, 15 Aug 2023 14:35:47 -0400 Subject: [PATCH] Bug 1842039 - Add bread crumbs for IllegalArgumentException when navigating. Co-authored-by: Jonathan Almeida Co-authored-by: Arturo Mejia (cherry picked from commit 1ea1b4ebdfe14df1b4337cd97123ffbd23e88400) --- .../fenix/browser/BaseBrowserFragment.kt | 9 ++++--- .../org/mozilla/fenix/ext/NavController.kt | 26 +++++++++++++++++++ .../controller/NimbusBranchesController.kt | 9 +++++-- .../fenix/onboarding/OnboardingFragment.kt | 1 + .../controller/OnboardingController.kt | 10 +++++-- .../fenix/settings/HomeSettingsFragment.kt | 8 ++++-- .../settings/account/TurnOnSyncFragment.kt | 11 +++++++- .../fragment/SavedLoginsAuthFragment.kt | 8 +++++- .../settings/search/SearchEngineFragment.kt | 19 ++++++++++++-- .../SitePermissionsFragment.kt | 12 +++++++-- .../DefaultOnboardingControllerTest.kt | 1 + 11 files changed, 99 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 58dd8855f..f5a710894 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -142,6 +142,7 @@ import org.mozilla.fenix.ext.getFenixAddons import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.hideToolbar import org.mozilla.fenix.ext.nav +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.registerForActivityResult import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached @@ -1439,12 +1440,14 @@ abstract class BaseBrowserFragment : MetricsUtils.BookmarkAction.EDIT, TOAST_METRIC_SOURCE, ) - nav( - R.id.browserFragment, - BrowserFragmentDirections.actionGlobalBookmarkEditFragment( + findNavController().navigateWithBreadcrumb( + directions = BrowserFragmentDirections.actionGlobalBookmarkEditFragment( guid, true, ), + navigateFrom = "BrowserFragment", + navigateTo = "ActionGlobalBookmarkEditFragment", + crashReporter = it.context.components.analytics.crashReporter, ) } .show() diff --git a/app/src/main/java/org/mozilla/fenix/ext/NavController.kt b/app/src/main/java/org/mozilla/fenix/ext/NavController.kt index ea9fbf18d..8026c2119 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/NavController.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/NavController.kt @@ -9,7 +9,10 @@ import androidx.annotation.IdRes import androidx.navigation.NavController import androidx.navigation.NavDirections import androidx.navigation.NavOptions +import mozilla.components.concept.base.crash.Breadcrumb +import mozilla.components.lib.crash.CrashReporter import mozilla.components.support.base.log.logger.Logger +import java.lang.IllegalArgumentException /** * Navigate from the fragment with [id] using the given [directions]. @@ -36,6 +39,29 @@ fun NavController.navigateSafe( } } +/** + * Navigates using the given [directions], and submit a Breadcrumb + * when an [IllegalArgumentException] happens. + */ +fun NavController.navigateWithBreadcrumb( + directions: NavDirections, + navigateFrom: String, + navigateTo: String, + crashReporter: CrashReporter, +) { + try { + this.navigate(directions) + } catch (e: IllegalArgumentException) { + crashReporter.recordCrashBreadcrumb( + Breadcrumb( + "Navigation - " + + "where we are: $currentDestination," + "where we are going: $navigateTo, " + + "where we thought we were: $navigateFrom", + ), + ) + } +} + /** * Checks if the Fragment with a [fragmentClassName] is on top of the back queue. */ diff --git a/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt b/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt index 0b3c5327c..3e71788e1 100644 --- a/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt +++ b/app/src/main/java/org/mozilla/fenix/nimbus/controller/NimbusBranchesController.kt @@ -11,7 +11,9 @@ import mozilla.components.service.nimbus.ui.NimbusBranchesAdapterDelegate import org.mozilla.experiments.nimbus.Branch import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar +import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.settings import org.mozilla.fenix.nimbus.NimbusBranchesAction import org.mozilla.fenix.nimbus.NimbusBranchesFragmentDirections @@ -51,9 +53,12 @@ class NimbusBranchesController( ) .setText(snackbarText) .setAction(buttonText) { - navController.navigate( - NimbusBranchesFragmentDirections + navController.navigateWithBreadcrumb( + directions = NimbusBranchesFragmentDirections .actionNimbusBranchesFragmentToDataChoicesFragment(), + navigateFrom = "NimbusBranchesController", + navigateTo = "ActionNimbusBranchesFragmentToDataChoicesFragment", + crashReporter = context.components.analytics.crashReporter, ) } .show() diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt index 4e58472b5..fe784f10f 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt @@ -100,6 +100,7 @@ class OnboardingFragment : Fragment() { activity = activity, navController = findNavController(), onboarding = requireComponents.fenixOnboarding, + crashReporter = requireComponents.analytics.crashReporter, ), privateBrowsingController = DefaultPrivateBrowsingController( activity = activity, diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/controller/OnboardingController.kt b/app/src/main/java/org/mozilla/fenix/onboarding/controller/OnboardingController.kt index eb2e93e40..e012e661e 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/controller/OnboardingController.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/controller/OnboardingController.kt @@ -5,8 +5,10 @@ package org.mozilla.fenix.onboarding.controller import androidx.navigation.NavController +import mozilla.components.lib.crash.CrashReporter import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.onboarding.FenixOnboarding import org.mozilla.fenix.onboarding.OnboardingFragmentDirections import org.mozilla.fenix.onboarding.interactor.OnboardingInteractor @@ -34,13 +36,17 @@ class DefaultOnboardingController( private val activity: HomeActivity, private val navController: NavController, private val onboarding: FenixOnboarding, + private val crashReporter: CrashReporter, ) : OnboardingController { override fun handleFinishOnboarding(focusOnAddressBar: Boolean) { onboarding.finish() - navController.navigate( - OnboardingFragmentDirections.actionHome(focusOnAddressBar = focusOnAddressBar), + navController.navigateWithBreadcrumb( + directions = OnboardingFragmentDirections.actionHome(focusOnAddressBar = focusOnAddressBar), + navigateFrom = "OnboardingFragment", + navigateTo = "ActionHome", + crashReporter = crashReporter, ) if (focusOnAddressBar) { diff --git a/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt index 5c5f0ecbf..9bba8fca3 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/HomeSettingsFragment.kt @@ -15,6 +15,7 @@ import org.mozilla.fenix.GleanMetrics.CustomizeHome import org.mozilla.fenix.R import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.utils.view.addToRadioGroup @@ -167,8 +168,11 @@ class HomeSettingsFragment : PreferenceFragmentCompat() { requirePreference(R.string.pref_key_wallpapers).apply { setOnPreferenceClickListener { - view?.findNavController()?.navigate( - HomeSettingsFragmentDirections.actionHomeSettingsFragmentToWallpaperSettingsFragment(), + view?.findNavController()?.navigateWithBreadcrumb( + directions = HomeSettingsFragmentDirections.actionHomeSettingsFragmentToWallpaperSettingsFragment(), + navigateFrom = "HomeSettingsFragment", + navigateTo = "ActionHomeSettingsFragmentToWallpaperSettingsFragment", + crashReporter = context.components.analytics.crashReporter, ) true } diff --git a/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt index 07caa17bb..ab03b079b 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/account/TurnOnSyncFragment.kt @@ -27,7 +27,9 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.databinding.FragmentTurnOnSyncBinding +import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.increaseTapArea +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar @@ -66,7 +68,14 @@ class TurnOnSyncFragment : Fragment(), AccountObserver { val directions = TurnOnSyncFragmentDirections.actionTurnOnSyncFragmentToPairFragment( entrypoint = args.entrypoint, ) - requireView().findNavController().navigate(directions) + context?.let { + requireView().findNavController().navigateWithBreadcrumb( + directions = directions, + navigateFrom = "TurnOnSyncFragment", + navigateTo = "ActionTurnOnSyncFragmentToPairFragment", + crashReporter = it.components.analytics.crashReporter, + ) + } SyncAuth.scanPairing.record(NoExtras()) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt index 1dc5496a6..ddb2b97fc 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt @@ -31,6 +31,7 @@ import org.mozilla.fenix.GleanMetrics.Logins import org.mozilla.fenix.R import org.mozilla.fenix.components.accounts.FenixFxAEntryPoint import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.registerForActivityResult import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached @@ -246,7 +247,12 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat() { private fun navigateToSaveLoginSettingFragment() { val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToSavedLoginsSettingFragment() - findNavController().navigate(directions) + findNavController().navigateWithBreadcrumb( + directions = directions, + navigateFrom = "SavedLoginsAuthFragment", + navigateTo = "ActionSavedLoginsAuthFragmentToSavedLoginsSettingFragment", + crashReporter = requireComponents.analytics.crashReporter, + ) } private fun navigateToLoginExceptionFragment() { diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt index 3b2fe6694..683f77d3e 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt @@ -16,6 +16,7 @@ import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.R import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.SharedPreferenceUpdater @@ -131,7 +132,14 @@ class SearchEngineFragment : PreferenceFragmentCompat() { getPreferenceKey(R.string.pref_key_add_search_engine) -> { val directions = SearchEngineFragmentDirections .actionSearchEngineFragmentToAddSearchEngineFragment() - findNavController().navigate(directions) + context?.let { + findNavController().navigateWithBreadcrumb( + directions = directions, + navigateFrom = "SearchEngineFragment", + navigateTo = "ActionSearchEngineFragmentToAddSearchEngineFragment", + it.components.analytics.crashReporter, + ) + } } getPreferenceKey(R.string.pref_key_default_search_engine) -> { val directions = SearchEngineFragmentDirections @@ -141,7 +149,14 @@ class SearchEngineFragment : PreferenceFragmentCompat() { getPreferenceKey(R.string.pref_key_manage_search_shortcuts) -> { val directions = SearchEngineFragmentDirections .actionSearchEngineFragmentToSearchShortcutsFragment() - findNavController().navigate(directions) + context?.let { + findNavController().navigateWithBreadcrumb( + directions = directions, + navigateFrom = "SearchEngineFragment", + navigateTo = "ActionSearchEngineFragmentToSearchShortcutsFragment", + it.components.analytics.crashReporter, + ) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt index cda6359c7..961509a69 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt @@ -13,7 +13,9 @@ import mozilla.components.service.glean.private.NoExtras import org.mozilla.fenix.Config import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.R +import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey +import org.mozilla.fenix.ext.navigateWithBreadcrumb import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.PhoneFeature @@ -79,7 +81,13 @@ class SitePermissionsFragment : PreferenceFragmentCompat() { if (phoneFeature == PhoneFeature.AUTOPLAY_AUDIBLE) { Autoplay.visitedSetting.record(NoExtras()) } - - Navigation.findNavController(requireView()).navigate(directions) + context?.let { + Navigation.findNavController(requireView()).navigateWithBreadcrumb( + directions = directions, + navigateFrom = "SitePermissionsFragment", + navigateTo = "ActionSitePermissionsToManagePhoneFeatures", + crashReporter = it.components.analytics.crashReporter, + ) + } } } diff --git a/app/src/test/java/org/mozilla/fenix/onboarding/DefaultOnboardingControllerTest.kt b/app/src/test/java/org/mozilla/fenix/onboarding/DefaultOnboardingControllerTest.kt index 4ff0d7d4e..919076612 100644 --- a/app/src/test/java/org/mozilla/fenix/onboarding/DefaultOnboardingControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/onboarding/DefaultOnboardingControllerTest.kt @@ -54,6 +54,7 @@ class DefaultOnboardingControllerTest { activity = activity, navController = navController, onboarding = onboarding, + crashReporter = mockk(relaxed = true), ) }