From 990bfa7e6dd5894d61473a41da6129e6947974e2 Mon Sep 17 00:00:00 2001 From: MarcLeclair Date: Tue, 13 Apr 2021 20:48:45 -0400 Subject: [PATCH] 16900 make navgraph inflation asynchronous (#18889) * For #16900: implement async navgraph inflation For #16900: removed nav graph from xml For #16900: inflate navGraph programatically For #16900: Made NavGraph inflation asynchronous For #16900: Changed to block with runBlocking For #16900: Refactored blocking call into a function For 16900: NavGraph inflation is now async We now attach the nav graph (or check if its attached) on every nav call ( an extension function for NavController). This is done by checking the value of the job stored in PerfNavController.map which keeps track of the job with the NavController as a Key. If the job hasn't been completed, it will block the main thread until the job is done. The job itself is responsible for attaching the navgraph to the navcontroller (and the inflation of the latter too) For 16900: rebased upstream master For 16900: Rebase on master For #16900: Fixed Async Navgraph navigation per review comments. 1)The Asynchronous method is now found in NavGraphProvider.kt. It creates a job on the IO dispatcher 2)The Job is tracked through a WeakHashMap from Controller --> NavGraph 3)The Coroutine scope doesn't use MainScope() anymore 4)The Coroutine is cancelled if the Activity is destroyed 5)The tests mockk the blockForNavGraphInflation method through the FenixReoboelectricTestApplication instead of calling the mock every setup() For #16900: inflateNavGraphAsync now takes navController For #16900: Pass lifecycleScope to NavGraphProvider For #16900: removed unused mock For #16900: Added linter rules for navigate calls We need linting rules to make sure no one calls the NavController.navigate() methods For #16900: Added TestRule to help abstract the mocks in the code For 16900: Fix linting problems For #16900: Cleaned duplicated code in tests For #16900: cleaned up NavGraphTestRule for finished test For #16900: had to revert an accidentally edited file For #16900: rebased master * For #16900: Review nits for async navgraph This is composed of squash commits, the original messages can be found below: -> DisableNavGraphProviderAssertionRule + kdoc. Use test rule in RobolectricApplication. Fix failing CrashReporterControllerTest Fix blame by -> navigate in tests. This commit was generated by the following commands only: ``` find app/src/test -type f -exec sed -i '' "/import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph/d" {} \; find app/src/test -type f -exec sed -i "" "s/navigateBlockingForAsyncNavGraph/navigate/g" {} \; git checkout app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker ``` Fix various blame This is expected to be squashed into the first commit so, if so, it'd fix the blame. Move test rule to helpers pkg. add missing license header Add import change I missed fix unused imports Replace robolectricTestrunner with test rule. Improve navGraphProvider docs Remove unnecessary rule as defined by robolectric. add clarifying comment to robolectric remove unnecessary space * For #16900: nit fixes for MozillaNavigateCheck and lint fixes 3 squash commits: *Changed violation message and fixed the lint rule for MozillaNavigateCheck *Added suppression to NavController.kt *Fixed detekt violations * For 16900: Fixed failing tests Co-authored-by: Michael Comella --- .../perf/StartupExcessiveResourceUseTest.kt | 2 +- .../mozilla/fenix/AppRequestInterceptor.kt | 3 +- .../java/org/mozilla/fenix/HomeActivity.kt | 15 +++-- .../fenix/addons/AddonsManagementView.kt | 3 +- .../addons/InstalledAddonDetailsFragment.kt | 7 ++- .../fenix/browser/BaseBrowserFragment.kt | 7 ++- .../mozilla/fenix/browser/BrowserFragment.kt | 3 +- .../toolbar/BrowserToolbarController.kt | 7 ++- .../toolbar/BrowserToolbarMenuController.kt | 9 +-- .../org/mozilla/fenix/ext/NavController.kt | 23 +++++++- .../org/mozilla/fenix/home/HomeFragment.kt | 6 +- .../intent/CrashReporterIntentProcessor.kt | 3 +- .../home/intent/DeepLinkIntentProcessor.kt | 3 +- .../OnboardingManualSignInViewHolder.kt | 3 +- .../library/bookmarks/BookmarkController.kt | 6 +- .../library/bookmarks/BookmarkFragment.kt | 8 +-- .../fenix/library/bookmarks/BookmarkView.kt | 3 +- .../library/history/HistoryController.kt | 5 +- .../fenix/library/history/HistoryFragment.kt | 4 +- .../RecentlyClosedController.kt | 5 +- .../fenix/nimbus/NimbusExperimentsView.kt | 3 +- .../mozilla/fenix/perf/NavGraphProvider.kt | 59 +++++++++++++++++++ .../fenix/settings/SettingsFragment.kt | 3 +- .../settings/TrackingProtectionFragment.kt | 3 +- .../fenix/settings/about/AboutFragment.kt | 3 +- .../settings/account/TurnOnSyncFragment.kt | 3 +- .../creditcards/CreditCardsSettingFragment.kt | 11 ++-- .../CreditCardsManagementController.kt | 3 +- .../DeleteBrowsingDataFragment.kt | 3 +- .../logins/controller/LoginsListController.kt | 3 +- .../SavedLoginsStorageController.kt | 3 +- .../logins/fragment/LoginDetailFragment.kt | 3 +- .../fragment/SavedLoginsAuthFragment.kt | 13 ++-- .../quicksettings/QuickSettingsController.kt | 3 +- .../search/RadioSearchEngineListPreference.kt | 3 +- .../settings/search/SearchEngineFragment.kt | 3 +- ...itePermissionsDetailsExceptionsFragment.kt | 3 +- .../SitePermissionsFragment.kt | 5 +- .../mozilla/fenix/share/ShareController.kt | 3 +- .../fenix/sync/SyncedTabsViewHolder.kt | 3 +- .../fenix/tabstray/NavigationInteractor.kt | 11 ++-- .../fenix/tabstray/TabsTrayController.kt | 4 +- .../fenix/tabstray/TabsTrayFragment.kt | 5 +- .../fenix/tabtray/TabTrayController.kt | 15 +++-- .../fenix/tabtray/TabTrayDialogFragment.kt | 9 +-- app/src/main/res/layout/activity_home.xml | 4 +- .../crashes/CrashReporterControllerTest.kt | 6 ++ .../mozilla/fenix/ext/NavControllerTest.kt | 5 ++ .../DisableNavGraphProviderAssertionRule.kt | 43 ++++++++++++++ .../FenixRobolectricTestApplication.kt | 23 +++++++- .../DefaultSessionControlControllerTest.kt | 4 ++ .../bookmarks/BookmarkControllerTest.kt | 5 ++ .../search/SearchDialogControllerTest.kt | 8 ++- .../account/AccountSettingsInteractorTest.kt | 5 ++ ...aultCreditCardsManagementControllerTest.kt | 5 ++ .../tabstray/NavigationInteractorTest.kt | 5 ++ .../tabtray/DefaultTabTrayControllerTest.kt | 5 ++ config/detekt.yml | 3 + .../detektrules/CustomRulesetProvider.kt | 4 +- .../detektrules/perf/MozillaNavigateCheck.kt | 50 ++++++++++++++++ 60 files changed, 386 insertions(+), 94 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/perf/NavGraphProvider.kt create mode 100644 app/src/test/java/org/mozilla/fenix/helpers/DisableNavGraphProviderAssertionRule.kt create mode 100644 mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaNavigateCheck.kt diff --git a/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt b/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt index 3575a7101..b40ad62b1 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt @@ -21,7 +21,7 @@ import org.mozilla.fenix.helpers.HomeActivityTestRule // BEFORE INCREASING THESE VALUES, PLEASE CONSULT WITH THE PERF TEAM. private const val EXPECTED_SUPPRESSION_COUNT = 11 -private const val EXPECTED_RUNBLOCKING_COUNT = 2 +private const val EXPECTED_RUNBLOCKING_COUNT = 3 private const val EXPECTED_COMPONENT_INIT_COUNT = 42 private const val EXPECTED_VIEW_HIERARCHY_DEPTH = 12 private const val EXPECTED_RECYCLER_VIEW_CONSTRAINT_LAYOUT_CHILDREN = 4 diff --git a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt index 1e4d8926a..f026bf3bc 100644 --- a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -15,6 +15,7 @@ import mozilla.components.concept.engine.request.RequestInterceptor import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.isOnline +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import java.lang.ref.WeakReference class AppRequestInterceptor( @@ -97,7 +98,7 @@ class AppRequestInterceptor( // Navigate and trigger add-on installation. matchResult.groupValues.getOrNull(1)?.let { addonId -> - navController?.get()?.navigate( + navController?.get()?.navigateBlockingForAsyncNavGraph( NavGraphDirections.actionGlobalAddonsManagementFragment(addonId) ) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 61f8446cb..64762fa68 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -33,12 +33,12 @@ import androidx.navigation.ui.AppBarConfiguration import androidx.navigation.ui.NavigationUI import kotlinx.android.synthetic.main.activity_home.* import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job -import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlinx.coroutines.Dispatchers.IO import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.search.SearchEngine @@ -79,6 +79,7 @@ import org.mozilla.fenix.exceptions.trackingprotection.TrackingProtectionExcepti import org.mozilla.fenix.ext.alreadyOnDestination import org.mozilla.fenix.ext.breadcrumb import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.measureNoInline import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav @@ -94,6 +95,7 @@ import org.mozilla.fenix.library.bookmarks.BookmarkFragmentDirections import org.mozilla.fenix.library.bookmarks.DesktopFolders import org.mozilla.fenix.library.history.HistoryFragmentDirections import org.mozilla.fenix.library.recentlyclosed.RecentlyClosedFragmentDirections +import org.mozilla.fenix.perf.NavGraphProvider import org.mozilla.fenix.perf.Performance import org.mozilla.fenix.perf.PerformanceInflater import org.mozilla.fenix.perf.ProfilerMarkers @@ -131,7 +133,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { // components requires context to access. protected val homeActivityInitTimeStampNanoSeconds = SystemClock.elapsedRealtimeNanos() - private var webExtScope: CoroutineScope? = null lateinit var themeManager: ThemeManager lateinit var browsingModeManager: BrowsingModeManager @@ -193,7 +194,11 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { components.publicSuffixList.prefetch() setupThemeAndBrowsingMode(getModeFromIntentOrLastKnown(intent)) - setContentView(R.layout.activity_home) + setContentView(R.layout.activity_home).run { + // Do not call anything between setContentView and inflateNavGraphAsync. + // It needs to start its job as early as possible. + NavGraphProvider.inflateNavGraphAsync(navHost.navController, lifecycleScope) + } // Must be after we set the content view if (isVisuallyComplete) { @@ -934,7 +939,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { webExtensionId = webExtensionState.id, webExtensionTitle = webExtensionState.name ) - navHost.navController.navigate(action) + navHost.navController.navigateBlockingForAsyncNavGraph(action) } /** diff --git a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementView.kt b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementView.kt index 4e9e24cab..d1994c78a 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementView.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementView.kt @@ -8,6 +8,7 @@ import androidx.navigation.NavController import mozilla.components.feature.addons.Addon import mozilla.components.feature.addons.ui.AddonsManagerAdapterDelegate import org.mozilla.fenix.R +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.navigateSafe /** @@ -55,6 +56,6 @@ class AddonsManagementView( AddonsManagementFragmentDirections.actionAddonsManagementFragmentToNotYetSupportedAddonFragment( unsupportedAddons.toTypedArray() ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt index 3248a61fc..cb550e2b5 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt @@ -25,6 +25,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.ext.runIfFragmentIsAttached @@ -215,7 +216,7 @@ class InstalledAddonDetailsFragment : Fragment() { InstalledAddonDetailsFragmentDirections .actionInstalledAddonFragmentToAddonInternalSettingsFragment(addon) } - Navigation.findNavController(this).navigate(directions) + Navigation.findNavController(this).navigateBlockingForAsyncNavGraph(directions) } } } @@ -226,7 +227,7 @@ class InstalledAddonDetailsFragment : Fragment() { InstalledAddonDetailsFragmentDirections.actionInstalledAddonFragmentToAddonDetailsFragment( addon ) - Navigation.findNavController(view).navigate(directions) + Navigation.findNavController(view).navigateBlockingForAsyncNavGraph(directions) } } @@ -236,7 +237,7 @@ class InstalledAddonDetailsFragment : Fragment() { InstalledAddonDetailsFragmentDirections.actionInstalledAddonFragmentToAddonPermissionsDetailsFragment( addon ) - Navigation.findNavController(view).navigate(directions) + Navigation.findNavController(view).navigateBlockingForAsyncNavGraph(directions) } } 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 7d80abbdd..0307b146a 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -128,6 +128,7 @@ import java.lang.ref.WeakReference import mozilla.components.feature.session.behavior.EngineViewBrowserToolbarBehavior import mozilla.components.feature.webauthn.WebAuthnFeature import mozilla.components.support.base.feature.ActivityResultHandler +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import mozilla.components.support.ktx.android.view.enterToImmersiveMode import org.mozilla.fenix.GleanMetrics.PerfStartup import org.mozilla.fenix.ext.measureNoInline @@ -557,7 +558,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit showPage = true, sessionId = getCurrentTab()?.id ) - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } }, onNeedToRequestPermissions = { permissions -> @@ -568,7 +569,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit browserAnimator.captureEngineViewAndDrawStatically { val directions = NavGraphDirections.actionGlobalSavedLoginsAuthFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } } ), @@ -979,7 +980,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit } override fun onBackLongPressed(): Boolean { - findNavController().navigate( + findNavController().navigateBlockingForAsyncNavGraph( NavGraphDirections.actionGlobalTabHistoryDialogFragment( activeSessionId = customTabSessionId ) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 58515a908..ac70ff92c 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -33,6 +33,7 @@ import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.requireComponents @@ -264,7 +265,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { ) .setText(view.context.getString(messageStringRes)) .setAction(requireContext().getString(R.string.create_collection_view)) { - findNavController().navigate( + findNavController().navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalHome(focusOnAddressBar = false) ) } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt index 942390abd..fa303fc23 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarController.kt @@ -24,6 +24,7 @@ import org.mozilla.fenix.browser.readermode.ReaderModeController import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.HomeScreenViewModel @@ -118,7 +119,7 @@ class DefaultBrowserToolbarController( // When closing the last tab we must show the undo snackbar in the home fragment if (store.state.getNormalOrPrivateTabs(it.content.private).count() == 1) { homeViewModel.sessionToDelete = it.id - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalHome() ) } else { @@ -132,7 +133,7 @@ class DefaultBrowserToolbarController( Event.TabCounterMenuItemTapped(Event.TabCounterMenuItemTapped.Item.NEW_TAB) ) activity.browsingModeManager.mode = BrowsingMode.Normal - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalHome(focusOnAddressBar = true) ) } @@ -143,7 +144,7 @@ class DefaultBrowserToolbarController( ) ) activity.browsingModeManager.mode = BrowsingMode.Private - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalHome(focusOnAddressBar = true) ) } diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt index da48bb067..a21989c7e 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt @@ -39,6 +39,7 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.openSetDefaultBrowserOption @@ -162,7 +163,7 @@ class DefaultBrowserToolbarMenuController( } is ToolbarMenu.Item.Back -> { if (item.viewHistory) { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment( activeSessionId = customTabSessionId ) @@ -175,7 +176,7 @@ class DefaultBrowserToolbarMenuController( } is ToolbarMenu.Item.Forward -> { if (item.viewHistory) { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment( activeSessionId = customTabSessionId ) @@ -212,7 +213,7 @@ class DefaultBrowserToolbarMenuController( ), showPage = true ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } is ToolbarMenu.Item.Settings -> browserAnimator.captureEngineViewAndDrawStatically { val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment() @@ -341,7 +342,7 @@ class DefaultBrowserToolbarMenuController( ) } is ToolbarMenu.Item.NewTab -> { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( BrowserFragmentDirections.actionGlobalHome(focusOnAddressBar = true) ) } 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 4034c2472..8ba9cebfd 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/NavController.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/NavController.kt @@ -2,6 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +// We suppress the calls to `navigate` since we invoke the Android `NavController.navigate` through +// this file. Detekt checks for the `navigate()` function calls, which should be ignored in this file. +@file:Suppress("MozillaNavigateCheck") package org.mozilla.fenix.ext import androidx.annotation.IdRes @@ -10,12 +13,15 @@ import androidx.navigation.NavDirections import androidx.navigation.NavOptions import io.sentry.Sentry import org.mozilla.fenix.components.isSentryEnabled +import org.mozilla.fenix.perf.NavGraphProvider /** * Navigate from the fragment with [id] using the given [directions]. * If the id doesn't match the current destination, an error is recorded. */ fun NavController.nav(@IdRes id: Int?, directions: NavDirections, navOptions: NavOptions? = null) { + NavGraphProvider.blockForNavGraphInflation(this) + if (id == null || this.currentDestination?.id == id) { this.navigate(directions, navOptions) } else { @@ -23,6 +29,21 @@ fun NavController.nav(@IdRes id: Int?, directions: NavDirections, navOptions: Na } } +fun NavController.navigateBlockingForAsyncNavGraph(resId: Int) { + NavGraphProvider.blockForNavGraphInflation(this) + this.navigate(resId) +} + +fun NavController.navigateBlockingForAsyncNavGraph(directions: NavDirections) { + NavGraphProvider.blockForNavGraphInflation(this) + this.navigate(directions) +} + +fun NavController.navigateBlockingForAsyncNavGraph(directions: NavDirections, navOptions: NavOptions?) { + NavGraphProvider.blockForNavGraphInflation(this) + this.navigate(directions, navOptions) +} + fun NavController.alreadyOnDestination(@IdRes destId: Int?): Boolean { return destId?.let { currentDestination?.id == it || popBackStack(it, false) } ?: false } @@ -38,6 +59,6 @@ fun NavController.navigateSafe( directions: NavDirections ) { if (currentDestination?.id == resId) { - this.navigate(directions) + this.navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index e9b839468..c4c9ab5e5 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -102,6 +102,7 @@ import org.mozilla.fenix.components.toolbar.FenixTabCounterMenu import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.hideToolbar +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.measureNoInline import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav @@ -533,7 +534,7 @@ class HomeFragment : Fragment() { requireContext().getString(R.string.snackbar_deleted_undo), { requireComponents.useCases.tabsUseCases.undo.invoke() - findNavController().navigate( + findNavController().navigateBlockingForAsyncNavGraph( HomeFragmentDirections.actionGlobalBrowser(null) ) }, @@ -624,7 +625,8 @@ class HomeFragment : Fragment() { } private fun navToSavedLogins() { - findNavController().navigate(HomeFragmentDirections.actionGlobalSavedLoginsAuthFragment()) + findNavController().navigateBlockingForAsyncNavGraph( + HomeFragmentDirections.actionGlobalSavedLoginsAuthFragment()) } private fun dispatchModeChanges(mode: Mode) { diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/CrashReporterIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/CrashReporterIntentProcessor.kt index dfc6feac4..7f3238e08 100644 --- a/app/src/main/java/org/mozilla/fenix/home/intent/CrashReporterIntentProcessor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/intent/CrashReporterIntentProcessor.kt @@ -8,6 +8,7 @@ import android.content.Intent import androidx.navigation.NavController import mozilla.components.lib.crash.Crash import org.mozilla.fenix.NavGraphDirections +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph /** * When the app crashes, the user has the option to report it. @@ -26,6 +27,6 @@ class CrashReporterIntentProcessor : HomeIntentProcessor { private fun openToCrashReporter(intent: Intent, navController: NavController) { val directions = NavGraphDirections.actionGlobalCrashReporter(intent) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/DeepLinkIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/DeepLinkIntentProcessor.kt index 71d0c69a7..ed2c38cf2 100644 --- a/app/src/main/java/org/mozilla/fenix/home/intent/DeepLinkIntentProcessor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/intent/DeepLinkIntentProcessor.kt @@ -21,6 +21,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.SearchWidgetCreator import org.mozilla.fenix.ext.alreadyOnDestination +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.home.intent.DeepLinkIntentProcessor.DeepLinkVerifier import org.mozilla.fenix.settings.SupportUtils @@ -76,7 +77,7 @@ class DeepLinkIntentProcessor( } if (!navController.alreadyOnDestination(globalDirections.destinationId)) { - navController.navigate(globalDirections.navDirections) + navController.navigateBlockingForAsyncNavGraph(globalDirections.navDirections) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingManualSignInViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingManualSignInViewHolder.kt index 00e5f0fcb..a40cc55de 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingManualSignInViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/OnboardingManualSignInViewHolder.kt @@ -11,6 +11,7 @@ import kotlinx.android.synthetic.main.onboarding_manual_signin.view.* import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.home.HomeFragmentDirections class OnboardingManualSignInViewHolder(view: View) : RecyclerView.ViewHolder(view) { @@ -22,7 +23,7 @@ class OnboardingManualSignInViewHolder(view: View) : RecyclerView.ViewHolder(vie it.context.components.analytics.metrics.track(Event.OnboardingManualSignIn) val directions = HomeFragmentDirections.actionGlobalTurnOnSync() - Navigation.findNavController(view).navigate(directions) + Navigation.findNavController(view).navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt index 0b80684e4..3dc18e26d 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt @@ -88,7 +88,7 @@ class DefaultBookmarkController( } override fun handleBookmarkEdit(node: BookmarkNode) { - navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(node.guid)) + navigateToGivenDirection(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(node.guid)) } override fun handleBookmarkSelected(node: BookmarkNode) { @@ -118,7 +118,7 @@ class DefaultBookmarkController( } override fun handleBookmarkSharing(item: BookmarkNode) { - navigate( + navigateToGivenDirection( BookmarkFragmentDirections.actionGlobalShareFragment( data = arrayOf(ShareData(url = item.url, title = item.title)) ) @@ -182,7 +182,7 @@ class DefaultBookmarkController( } } - private fun navigate(directions: NavDirections) { + private fun navigateToGivenDirection(directions: NavDirections) { invokePendingDeletion.invoke() navController.nav(R.id.bookmarkFragment, directions) } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 8769e35af..4b93a4f31 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -202,7 +202,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan true } R.id.add_bookmark_folder -> { - navigate( + navigateToBookmarkFragment( BookmarkFragmentDirections .actionBookmarkFragmentToBookmarkAddFolderFragment() ) @@ -226,7 +226,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan val shareTabs = bookmarkStore.state.mode.selectedItems.map { ShareData(url = it.url, title = it.title) } - navigate( + navigateToBookmarkFragment( BookmarkFragmentDirections.actionGlobalShareFragment( data = shareTabs.toTypedArray() ) @@ -243,10 +243,10 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan private fun showTabTray() { invokePendingDeletion() - navigate(BookmarkFragmentDirections.actionGlobalTabTrayDialogFragment()) + navigateToBookmarkFragment(BookmarkFragmentDirections.actionGlobalTabTrayDialogFragment()) } - private fun navigate(directions: NavDirections) { + private fun navigateToBookmarkFragment(directions: NavDirections) { invokePendingDeletion() findNavController().nav( R.id.bookmarkFragment, diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt index 6c581a8f8..d08bfd97e 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt @@ -15,6 +15,7 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.support.base.feature.UserInteractionHandler import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.library.LibraryPageView import org.mozilla.fenix.selection.SelectionInteractor @@ -119,7 +120,7 @@ class BookmarkView( adapter = bookmarkAdapter } view.bookmark_folders_sign_in.setOnClickListener { - navController.navigate(NavGraphDirections.actionGlobalTurnOnSync()) + navController.navigateBlockingForAsyncNavGraph(NavGraphDirections.actionGlobalTurnOnSync()) } view.swipe_refresh.setOnRefreshListener { interactor.onRequestSync() diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt index 96ef57698..8d9114a43 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt @@ -17,6 +17,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph @Suppress("TooManyFunctions") interface HistoryController { @@ -94,7 +95,7 @@ class DefaultHistoryController( } override fun handleShare(item: HistoryItem) { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( HistoryFragmentDirections.actionGlobalShareFragment( data = arrayOf(ShareData(url = item.url, title = item.title)) ) @@ -110,7 +111,7 @@ class DefaultHistoryController( } override fun handleEnterRecentlyClosed() { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( HistoryFragmentDirections.actionGlobalRecentlyClosed(), NavOptions.Builder().setPopUpTo(R.id.recentlyClosedFragment, true).build() ) diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index e3a826621..7d96b91b7 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -304,10 +304,10 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl val directions = HistoryFragmentDirections.actionGlobalShareFragment( data = data.toTypedArray() ) - navigate(directions) + navigateToHistoryFragment(directions) } - private fun navigate(directions: NavDirections) { + private fun navigateToHistoryFragment(directions: NavDirections) { invokePendingDeletion() findNavController().nav( R.id.historyFragment, diff --git a/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedController.kt b/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedController.kt index a4461a246..47f1b2935 100644 --- a/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedController.kt @@ -19,6 +19,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.FenixSnackbar +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph interface RecentlyClosedController { fun handleOpen(item: RecoverableTab, mode: BrowsingMode? = null) @@ -48,7 +49,7 @@ class DefaultRecentlyClosedController( } override fun handleNavigateToHistory() { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( RecentlyClosedFragmentDirections.actionGlobalHistoryFragment(), NavOptions.Builder().setPopUpTo(R.id.historyFragment, true).build() ) @@ -64,7 +65,7 @@ class DefaultRecentlyClosedController( } override fun handleShare(item: RecoverableTab) { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( RecentlyClosedFragmentDirections.actionGlobalShareFragment( data = arrayOf(ShareData(url = item.url, title = item.title)) ) diff --git a/app/src/main/java/org/mozilla/fenix/nimbus/NimbusExperimentsView.kt b/app/src/main/java/org/mozilla/fenix/nimbus/NimbusExperimentsView.kt index 3fd92a070..1bc6e030f 100644 --- a/app/src/main/java/org/mozilla/fenix/nimbus/NimbusExperimentsView.kt +++ b/app/src/main/java/org/mozilla/fenix/nimbus/NimbusExperimentsView.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.nimbus import androidx.navigation.NavController import mozilla.components.service.nimbus.ui.NimbusExperimentsAdapterDelegate import org.mozilla.experiments.nimbus.EnrolledExperiment +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph /** * View used for managing Nimbus experiments. @@ -21,6 +22,6 @@ class NimbusExperimentsView( experiment.userFacingName ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/perf/NavGraphProvider.kt b/app/src/main/java/org/mozilla/fenix/perf/NavGraphProvider.kt new file mode 100644 index 000000000..a6b60a5ca --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/perf/NavGraphProvider.kt @@ -0,0 +1,59 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.perf + +import androidx.lifecycle.LifecycleCoroutineScope +import androidx.navigation.NavController +import java.util.WeakHashMap +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import org.mozilla.fenix.R + +/** + * This class asynchronously loads the navigation graph XML. This is a performance optimization: + * large nav graphs take ~29ms to inflate on the Moto G5 (#16900). This also seemingly prevents the + * HomeFragment layout XML from being unnecessarily inflated when it isn't used, improving perf by + * ~148ms on the Moto G5 for VIEW start up (#18245) though it was unintentional and we may wish to + * implement more intentional for that. + * + * This class is defined as an Object, rather than as a class instance in our Components, because + * it needs to be called by the [NavController] extension function which can't easily access Components. + * + * To use this class properly, [inflateNavGraphAsync] must be called first before + * [blockForNavGraphInflation] using the same [NavController] instance. + */ +object NavGraphProvider { + + // We want to store member state on the NavController. However, there is no way to do this. + // Instead, we store state as part of a map: NavController instance -> State. In order to + // garbage collect our data when the NavController is no longer relevant, we use a WeakHashMap. + private val map = WeakHashMap() + + fun inflateNavGraphAsync(navController: NavController, lifecycleScope: LifecycleCoroutineScope) { + val inflationJob = lifecycleScope.launch(Dispatchers.IO) { + val inflater = navController.navInflater + navController.graph = inflater.inflate(R.navigation.nav_graph) + } + + map[navController] = inflationJob + } + + /** + * The job should block the main thread if it isn't completed so that the NavGraph can be loaded + * before any navigation is done. + * + * [inflateNavGraphAsync] must be called before this method. + * + * @throws IllegalStateException if [inflateNavGraphAsync] wasn't called first for this [NavController] + */ + fun blockForNavGraphInflation(navController: NavController) { + val inflationJob = map[navController] ?: throw IllegalStateException("Expected " + + "`NavGraphProvider.inflateNavGraphAsync` to be called before this method with the same " + + "`NavController` instance. If this occurred in a test, you probably need to add the " + + "DisableNavGraphProviderAssertionRule.") + runBlockingIncrement { inflationJob.join() } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt index c872dd61a..b987ffdbc 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt @@ -45,6 +45,7 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.application import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.navigateToNotificationsSettings import org.mozilla.fenix.ext.requireComponents @@ -480,7 +481,7 @@ class SettingsFragment : PreferenceFragmentCompat() { private fun navigateFromSettings(directions: NavDirections) { view?.findNavController()?.let { navController -> if (navController.currentDestination?.id == R.id.settingsFragment) { - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/TrackingProtectionFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/TrackingProtectionFragment.kt index 66bb347cc..92be8a332 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/TrackingProtectionFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/TrackingProtectionFragment.kt @@ -16,6 +16,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings @@ -32,7 +33,7 @@ class TrackingProtectionFragment : PreferenceFragmentCompat() { private val exceptionsClickListener = Preference.OnPreferenceClickListener { val directions = TrackingProtectionFragmentDirections.actionTrackingProtectionFragmentToExceptionsFragment() - requireView().findNavController().navigate(directions) + requireView().findNavController().navigateBlockingForAsyncNavGraph(directions) true } private lateinit var customCookies: CheckBoxPreference diff --git a/app/src/main/java/org/mozilla/fenix/settings/about/AboutFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/about/AboutFragment.kt index 9f80cf5e6..ba1d7a886 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/about/AboutFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/about/AboutFragment.kt @@ -22,6 +22,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.crashes.CrashListActivity +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.settings.SupportUtils @@ -178,7 +179,7 @@ class AboutFragment : Fragment(), AboutPageListener { private fun openLibrariesPage() { val navController = findNavController() - navController.navigate(R.id.action_aboutFragment_to_aboutLibrariesFragment) + navController.navigateBlockingForAsyncNavGraph(R.id.action_aboutFragment_to_aboutLibrariesFragment) } override fun onAboutItemClicked(item: AboutItem) { 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 19973a28e..2433bb5b2 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 @@ -26,6 +26,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar @@ -59,7 +60,7 @@ class TurnOnSyncFragment : Fragment(), AccountObserver { private fun navigateToPairFragment() { val directions = TurnOnSyncFragmentDirections.actionTurnOnSyncFragmentToPairFragment() - requireView().findNavController().navigate(directions) + requireView().findNavController().navigateBlockingForAsyncNavGraph(directions) requireComponents.analytics.metrics.track(Event.SyncAuthScanPairing) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt index aa19cff8b..bcb197137 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt @@ -11,6 +11,7 @@ import androidx.preference.PreferenceFragmentCompat import mozilla.components.service.fxa.SyncEngine import org.mozilla.fenix.R import org.mozilla.fenix.ext.getPreferenceKey +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.SyncPreferenceView @@ -39,17 +40,17 @@ class CreditCardsSettingFragment : PreferenceFragmentCompat() { onSignInToSyncClicked = { val directions = CreditCardsSettingFragmentDirections.actionCreditCardsSettingFragmentToTurnOnSyncFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) }, onSyncStatusClicked = { val directions = CreditCardsSettingFragmentDirections.actionGlobalAccountSettingsFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) }, onReconnectClicked = { val directions = CreditCardsSettingFragmentDirections.actionGlobalAccountProblemFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } ) } @@ -61,13 +62,13 @@ class CreditCardsSettingFragment : PreferenceFragmentCompat() { val directions = CreditCardsSettingFragmentDirections .actionCreditCardsSettingFragmentToCreditCardEditorFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } getPreferenceKey(R.string.pref_key_credit_cards_manage_saved_cards) -> { val directions = CreditCardsSettingFragmentDirections .actionCreditCardsSettingFragmentToCreditCardsManagementFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/creditcards/controller/CreditCardsManagementController.kt b/app/src/main/java/org/mozilla/fenix/settings/creditcards/controller/CreditCardsManagementController.kt index d40478348..77827c553 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/controller/CreditCardsManagementController.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/controller/CreditCardsManagementController.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.settings.creditcards.controller import androidx.navigation.NavController +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.settings.creditcards.CreditCardsManagementFragmentDirections /** @@ -27,7 +28,7 @@ class DefaultCreditCardsManagementController( ) : CreditCardsManagementController { override fun handleCreditCardClicked() { - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( CreditCardsManagementFragmentDirections .actionCreditCardsManagementFragmentToCreditCardEditorFragment() ) diff --git a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt index 4bb7dcd1c..a1d0fd7c0 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/deletebrowsingdata/DeleteBrowsingDataFragment.kt @@ -28,6 +28,7 @@ import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar @@ -199,7 +200,7 @@ class DeleteBrowsingDataFragment : Fragment(R.layout.fragment_delete_browsing_da // If the user deletes all open tabs we need to make sure we remove // the BrowserFragment from the backstack. popBackStack(R.id.homeFragment, false) - navigate(DeleteBrowsingDataFragmentDirections.actionGlobalSettingsFragment()) + navigateBlockingForAsyncNavGraph(DeleteBrowsingDataFragmentDirections.actionGlobalSettingsFragment()) } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt index 8d4c5629f..2f662ecca 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/LoginsListController.kt @@ -8,6 +8,7 @@ import androidx.navigation.NavController import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.settings.logins.LoginsAction import org.mozilla.fenix.settings.logins.LoginsFragmentStore @@ -40,7 +41,7 @@ class LoginsListController( fun handleItemClicked(item: SavedLogin) { loginsFragmentStore.dispatch(LoginsAction.LoginSelected(item)) metrics.track(Event.OpenOneLogin) - navController.navigate( + navController.navigateBlockingForAsyncNavGraph( SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(item.guid) ) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt index 5257c73f8..885a2ce90 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/controller/SavedLoginsStorageController.kt @@ -20,6 +20,7 @@ import mozilla.components.service.sync.logins.LoginsStorageException import mozilla.components.service.sync.logins.NoSuchRecordException import mozilla.components.service.sync.logins.SyncableLoginsStorage import org.mozilla.fenix.R +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.settings.logins.LoginsAction import org.mozilla.fenix.settings.logins.LoginsFragmentStore import org.mozilla.fenix.settings.logins.fragment.EditLoginFragmentDirections @@ -83,7 +84,7 @@ open class SavedLoginsStorageController( EditLoginFragmentDirections.actionEditLoginFragmentToLoginDetailFragment( loginId ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } } saveLoginJob?.invokeOnCompletion { diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt index 10b031877..3826179c0 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/LoginDetailFragment.kt @@ -33,6 +33,7 @@ import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.increaseTapArea +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.redirectToReAuth import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings @@ -199,7 +200,7 @@ class LoginDetailFragment : Fragment(R.layout.fragment_login_detail) { LoginDetailFragmentDirections.actionLoginDetailFragmentToEditLoginFragment( login!! ) - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } private fun displayDeleteLoginDialog() { 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 021ef626d..f92d070d4 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 @@ -27,6 +27,7 @@ import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached import org.mozilla.fenix.ext.secure @@ -130,17 +131,17 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat() { onSignInToSyncClicked = { val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) }, onSyncStatusClicked = { val directions = SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) }, onReconnectClicked = { val directions = SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } ) @@ -213,19 +214,19 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat() { context?.components?.analytics?.metrics?.track(Event.OpenLogins) val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToLoginsListFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } private fun navigateToSaveLoginSettingFragment() { val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToSavedLoginsSettingFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } private fun navigateToLoginExceptionFragment() { val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToLoginExceptionsFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt b/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt index f93b9c69f..c8dbe8ac4 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsController.kt @@ -18,6 +18,7 @@ import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.tabs.TabsUseCases.AddNewTabUseCase import mozilla.components.support.base.feature.OnNeedToRequestPermissions import org.mozilla.fenix.components.PermissionStorage +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.settings.PhoneFeature import org.mozilla.fenix.settings.quicksettings.ext.shouldBeEnabled import org.mozilla.fenix.settings.toggle @@ -199,6 +200,6 @@ class DefaultQuickSettingsController( private fun navigateToManagePhoneFeature(phoneFeature: PhoneFeature) { val directions = QuickSettingsSheetDialogFragmentDirections .actionGlobalSitePermissionsManagePhoneFeature(phoneFeature) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt index caea80564..fb79c79cf 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt @@ -38,6 +38,7 @@ import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import org.mozilla.fenix.R import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.utils.allowUndo class RadioSearchEngineListPreference @JvmOverloads constructor( @@ -146,7 +147,7 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( val directions = SearchEngineFragmentDirections .actionSearchEngineFragmentToEditCustomSearchEngineFragment(engine.id) - Navigation.findNavController(view).navigate(directions) + Navigation.findNavController(view).navigateBlockingForAsyncNavGraph(directions) } private fun deleteSearchEngine( 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 c5971e7f9..41d648b57 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 @@ -13,6 +13,7 @@ import androidx.preference.SwitchPreference import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.R import org.mozilla.fenix.ext.getPreferenceKey +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.SharedPreferenceUpdater @@ -99,7 +100,7 @@ class SearchEngineFragment : PreferenceFragmentCompat() { getPreferenceKey(R.string.pref_key_add_search_engine) -> { val directions = SearchEngineFragmentDirections .actionSearchEngineFragmentToAddSearchEngineFragment() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsDetailsExceptionsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsDetailsExceptionsFragment.kt index 6eefefffc..dcb2bcc9c 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsDetailsExceptionsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsDetailsExceptionsFragment.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.withContext import mozilla.components.feature.sitepermissions.SitePermissions import org.mozilla.fenix.R import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar @@ -161,6 +162,6 @@ class SitePermissionsDetailsExceptionsFragment : PreferenceFragmentCompat() { phoneFeature = phoneFeature, sitePermissions = sitePermissions ) - requireView().findNavController().navigate(directions) + requireView().findNavController().navigateBlockingForAsyncNavGraph(directions) } } 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 ce03bf978..688766782 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 @@ -12,6 +12,7 @@ import androidx.preference.PreferenceFragmentCompat import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.getPreferenceKey +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar @@ -42,7 +43,7 @@ class SitePermissionsFragment : PreferenceFragmentCompat() { exceptionsCategory.onPreferenceClickListener = OnPreferenceClickListener { val directions = SitePermissionsFragmentDirections.actionSitePermissionsToExceptions() - Navigation.findNavController(requireView()).navigate(directions) + Navigation.findNavController(requireView()).navigateBlockingForAsyncNavGraph(directions) true } } @@ -76,6 +77,6 @@ class SitePermissionsFragment : PreferenceFragmentCompat() { requireComponents.analytics.metrics.track(Event.AutoPlaySettingVisited) } - Navigation.findNavController(requireView()).navigate(directions) + Navigation.findNavController(requireView()).navigateBlockingForAsyncNavGraph(directions) } } diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt index bb7c72643..5607edf4d 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt @@ -31,6 +31,7 @@ import mozilla.components.support.ktx.kotlin.isExtensionUrl import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.share.listadapters.AppShareOption @@ -121,7 +122,7 @@ class DefaultShareController( override fun handleAddNewDevice() { val directions = ShareFragmentDirections.actionShareFragmentToAddNewDeviceFragment() - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } override fun handleShareToDevice(device: Device) { diff --git a/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsViewHolder.kt b/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsViewHolder.kt index a48d6d264..375cec1e6 100644 --- a/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/sync/SyncedTabsViewHolder.kt @@ -19,6 +19,7 @@ import mozilla.components.concept.sync.DeviceType import mozilla.components.feature.syncedtabs.view.SyncedTabsView import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.sync.SyncedTabsAdapter.AdapterItem /** @@ -63,7 +64,7 @@ sealed class SyncedTabsViewHolder(itemView: View) : RecyclerView.ViewHolder(item errorItem.navController?.let { navController -> itemView.sync_tabs_error_cta_button.visibility = VISIBLE itemView.sync_tabs_error_cta_button.setOnClickListener { - navController.navigate(NavGraphDirections.actionGlobalTurnOnSync()) + navController.navigateBlockingForAsyncNavGraph(NavGraphDirections.actionGlobalTurnOnSync()) } } } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt index 0d031e77d..3f083778f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/NavigationInteractor.kt @@ -21,6 +21,7 @@ import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.bookmarks.BookmarksUseCase import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.home.HomeFragment import org.mozilla.fenix.tabstray.ext.getTabSessionState @@ -91,11 +92,13 @@ class DefaultNavigationInteractor( } override fun onTabSettingsClicked() { - navController.navigate(TabsTrayFragmentDirections.actionGlobalTabSettingsFragment()) + navController.navigateBlockingForAsyncNavGraph( + TabsTrayFragmentDirections.actionGlobalTabSettingsFragment()) } override fun onOpenRecentlyClosedClicked() { - navController.navigate(TabsTrayFragmentDirections.actionGlobalRecentlyClosed()) + navController.navigateBlockingForAsyncNavGraph( + TabsTrayFragmentDirections.actionGlobalRecentlyClosed()) metrics.track(Event.RecentlyClosedTabsOpened) } @@ -106,7 +109,7 @@ class DefaultNavigationInteractor( val directions = TabsTrayFragmentDirections.actionGlobalShareFragment( data = data.toTypedArray() ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } override fun onShareTabsOfTypeClicked(private: Boolean) { @@ -117,7 +120,7 @@ class DefaultNavigationInteractor( val directions = TabsTrayFragmentDirections.actionGlobalShareFragment( data = data.toTypedArray() ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } @OptIn(ExperimentalCoroutinesApi::class) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt index 507cfe1a7..fd1593088 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -14,6 +14,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.tabtray.TabTrayDialogFragmentDirections interface TabsTrayController { @@ -43,7 +44,8 @@ class DefaultTabsTrayController( override fun onNewTabTapped(isPrivate: Boolean) { val startTime = profiler?.getProfilerTime() browsingModeManager.mode = BrowsingMode.fromBoolean(isPrivate) - navController.navigate(TabTrayDialogFragmentDirections.actionGlobalHome(focusOnAddressBar = true)) + navController.navigateBlockingForAsyncNavGraph( + TabTrayDialogFragmentDirections.actionGlobalHome(focusOnAddressBar = true)) navigationInteractor.onTabTrayDismissed() profiler?.addMarker( "DefaultTabTrayController.onNewTabTapped", diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt index e73c516c0..2bc91dc0f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayFragment.kt @@ -33,6 +33,7 @@ import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.home.HomeScreenViewModel import org.mozilla.fenix.tabstray.browser.BrowserTrayInteractor @@ -241,7 +242,7 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { } if (!navController.popBackStack(R.id.browserFragment, false)) { - navController.navigate(R.id.browserFragment) + navController.navigateBlockingForAsyncNavGraph(R.id.browserFragment) } } @@ -301,7 +302,7 @@ class TabsTrayFragment : AppCompatDialogFragment(), TabsTrayInteractor { private fun dismissTabTrayAndNavigateHome(sessionId: String) { homeViewModel.sessionToDelete = sessionId val directions = NavGraphDirections.actionGlobalHome() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) dismissAllowingStateLoss() } } diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt index 4ce4c635c..e6f2aa698 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayController.kt @@ -26,6 +26,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.home.HomeFragment /** @@ -104,7 +105,8 @@ class DefaultTabTrayController( override fun handleNewTabTapped(private: Boolean) { val startTime = profiler?.getProfilerTime() browsingModeManager.mode = BrowsingMode.fromBoolean(private) - navController.navigate(TabTrayDialogFragmentDirections.actionGlobalHome(focusOnAddressBar = true)) + navController.navigateBlockingForAsyncNavGraph( + TabTrayDialogFragmentDirections.actionGlobalHome(focusOnAddressBar = true)) dismissTabTray() profiler?.addMarker( "DefaultTabTrayController.onNewTabTapped", @@ -113,7 +115,8 @@ class DefaultTabTrayController( } override fun handleTabSettingsClicked() { - navController.navigate(TabTrayDialogFragmentDirections.actionGlobalTabSettingsFragment()) + navController.navigateBlockingForAsyncNavGraph( + TabTrayDialogFragmentDirections.actionGlobalTabSettingsFragment()) } override fun handleTabTrayDismissed() { @@ -148,7 +151,7 @@ class DefaultTabTrayController( val directions = TabTrayDialogFragmentDirections.actionGlobalShareFragment( data = data.toTypedArray() ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } override fun handleShareSelectedTabsClicked(selectedTabs: Set) { @@ -158,7 +161,7 @@ class DefaultTabTrayController( val directions = TabTrayDialogFragmentDirections.actionGlobalShareFragment( data = data.toTypedArray() ) - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) } override fun handleBookmarkSelectedTabs(selectedTabs: Set) { @@ -236,13 +239,13 @@ class DefaultTabTrayController( override fun handleRecentlyClosedClicked() { val directions = TabTrayDialogFragmentDirections.actionGlobalRecentlyClosed() - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) metrics.track(Event.RecentlyClosedTabsOpened) } override fun handleGoToTabsSettingClicked() { val directions = TabTrayDialogFragmentDirections.actionGlobalTabSettingsFragment() - navController.navigate(directions) + navController.navigateBlockingForAsyncNavGraph(directions) metrics.track(Event.TabsTrayCfrTapped) } } diff --git a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt index d47cbd0ff..213a7418f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/tabtray/TabTrayDialogFragment.kt @@ -57,6 +57,7 @@ import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getDefaultCollectionNumber +import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings @@ -331,7 +332,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler private fun dismissTabTrayAndNavigateHome(sessionId: String) { homeViewModel.sessionToDelete = sessionId val directions = NavGraphDirections.actionGlobalHome() - findNavController().navigate(directions) + findNavController().navigateBlockingForAsyncNavGraph(directions) dismissAllowingStateLoss() } @@ -344,7 +345,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler dismissAllowingStateLoss() if (findNavController().currentDestination?.id == R.id.browserFragment) return if (!findNavController().popBackStack(R.id.browserFragment, false)) { - findNavController().navigate(R.id.browserFragment) + findNavController().navigateBlockingForAsyncNavGraph(R.id.browserFragment) } } @@ -379,7 +380,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler .setText(requireContext().getString(messageStringRes)) .setAction(requireContext().getString(R.string.create_collection_view)) { dismissAllowingStateLoss() - findNavController().navigate( + findNavController().navigateBlockingForAsyncNavGraph( TabTrayDialogFragmentDirections.actionGlobalHome( focusOnAddressBar = false, focusOnCollection = collectionToSelect ?: -1L @@ -403,7 +404,7 @@ class TabTrayDialogFragment : AppCompatDialogFragment(), UserInteractionHandler .setText(requireContext().getString(R.string.snackbar_message_bookmarks_saved)) .setAction(requireContext().getString(R.string.snackbar_message_bookmarks_view)) { dismissAllowingStateLoss() - findNavController().navigate( + findNavController().navigateBlockingForAsyncNavGraph( TabTrayDialogFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id) ) } diff --git a/app/src/main/res/layout/activity_home.xml b/app/src/main/res/layout/activity_home.xml index 5dc4bdefa..c016bdfd3 100644 --- a/app/src/main/res/layout/activity_home.xml +++ b/app/src/main/res/layout/activity_home.xml @@ -17,11 +17,11 @@ android:layout_width="match_parent" android:layout_height="56dp" /> + + app:defaultNavHost="true"/> diff --git a/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt b/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt index 27fef2732..c8fda762d 100644 --- a/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/crashes/CrashReporterControllerTest.kt @@ -12,14 +12,20 @@ import io.mockk.verify import mozilla.components.lib.crash.Crash import mozilla.components.support.test.ext.joinBlocking import org.junit.Before +import org.junit.Rule import org.junit.Test +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.R import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.metrics.Event + import org.mozilla.fenix.utils.Settings class CrashReporterControllerTest { + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + private lateinit var components: Components private lateinit var crash: Crash private lateinit var sessionId: String diff --git a/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt b/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt index d459eec29..b3a342073 100644 --- a/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/ext/NavControllerTest.kt @@ -18,11 +18,16 @@ import io.mockk.mockkStatic import io.mockk.verify import io.sentry.Sentry import org.junit.Before +import org.junit.Rule import org.junit.Test import org.mozilla.fenix.components.isSentryEnabled +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule class NavControllerTest { + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + private val currentDestId = 4 @MockK(relaxUnitFun = true) private lateinit var navController: NavController @MockK private lateinit var navDirections: NavDirections diff --git a/app/src/test/java/org/mozilla/fenix/helpers/DisableNavGraphProviderAssertionRule.kt b/app/src/test/java/org/mozilla/fenix/helpers/DisableNavGraphProviderAssertionRule.kt new file mode 100644 index 000000000..7b238a6ca --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/helpers/DisableNavGraphProviderAssertionRule.kt @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.helpers + +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.unmockkObject +import org.junit.rules.TestWatcher +import org.junit.runner.Description +import org.mozilla.fenix.perf.NavGraphProvider + +/** + * Disables the call order assertions defined by the [NavGraphProvider] for use in testing. + * This is necessary because unit tests generally don't follow the application lifecycle and thus + * call the methods out of order, causing an assertion to be thrown unexpectedly. You may need to + * apply this rule if you see the following exception in your test: + * + * Unfortunately, JUnit 4 discourages setting test state globally so we apply this to each test that + * has the failure rather than disabling it globally. + */ +class DisableNavGraphProviderAssertionRule : TestWatcher() { + + // public for code reuse. + fun setUp() { + mockkObject(NavGraphProvider) + every { NavGraphProvider.blockForNavGraphInflation(any()) } returns Unit + } + + // public for code reuse. + fun tearDown() { // + unmockkObject(NavGraphProvider) + } + + override fun starting(description: Description?) { + setUp() + } + + override fun finished(description: Description?) { + tearDown() + } +} diff --git a/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt b/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt index e1a108743..af9614bac 100644 --- a/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt +++ b/app/src/test/java/org/mozilla/fenix/helpers/FenixRobolectricTestApplication.kt @@ -7,13 +7,19 @@ package org.mozilla.fenix.helpers import org.mozilla.fenix.FenixApplication import org.mozilla.fenix.R import org.mozilla.fenix.components.TestComponents +import org.robolectric.TestLifecycleApplication +import java.lang.reflect.Method /** * An override of our application for use in Robolectric-based unit tests. We're forced to override * because our standard application fails to initialize in Robolectric with exceptions like: * "Crash handler service must run in a separate process". */ -class FenixRobolectricTestApplication : FenixApplication() { +class FenixRobolectricTestApplication : FenixApplication(), TestLifecycleApplication { + + // Though JUnit 4 discourages global rules, we can apply global rules in robolectric so we do + // to prevent confusion from devs. + private val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() override fun onCreate() { super.onCreate() @@ -37,4 +43,19 @@ class FenixRobolectricTestApplication : FenixApplication() { // https://github.com/mozilla-mobile/fenix/pull/15646#issuecomment-709411141 setTheme(R.style.NormalTheme) } + + // Before test runs before the test class is initialized + override fun beforeTest(method: Method?) {} + + // Prepare test runs once the test class and all its member variables (mock and + // non mocks) are initialized. This method runs after application.onCreate + override fun prepareTest(test: Any?) { + // We call this in prepareTest rather than beforeTest because member vars + // are initialized so it feels more correct to call it here. + disableNavGraphProviderAssertionRule.setUp() + } + + override fun afterTest(method: Method?) { + disableNavGraphProviderAssertionRule.tearDown() + } } diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index af94c24df..32fc8d255 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -39,6 +39,7 @@ import org.junit.Rule import org.junit.Test import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.R import org.mozilla.fenix.components.Analytics import org.mozilla.fenix.components.TabCollectionStorage @@ -113,6 +114,9 @@ class DefaultSessionControlControllerTest { private lateinit var store: BrowserStore private lateinit var controller: DefaultSessionControlController + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Before fun setup() { store = BrowserStore( diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt index d55eb3818..e2d6c8960 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt @@ -30,9 +30,11 @@ import mozilla.components.concept.storage.BookmarkNodeType import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before +import org.junit.Rule import org.junit.Test import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.Services @@ -87,6 +89,9 @@ class BookmarkControllerTest { BookmarkNodeType.FOLDER, BookmarkRoot.Root.id, null, 0, BookmarkRoot.Root.name, null, null ) + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Before fun setup() { every { homeActivity.components.services } returns services diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt index 96218a32d..5d14550ce 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt @@ -26,13 +26,16 @@ import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.tabs.TabsUseCases import org.junit.After import org.junit.Before +import org.junit.Rule import org.junit.Test import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricsUtils + import org.mozilla.fenix.search.SearchDialogFragmentDirections.Companion.actionGlobalAddonsManagementFragment import org.mozilla.fenix.search.SearchDialogFragmentDirections.Companion.actionGlobalSearchEngineFragment import org.mozilla.fenix.settings.SupportUtils @@ -55,13 +58,14 @@ class SearchDialogControllerTest { private lateinit var controller: SearchDialogController + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Before fun setUp() { MockKAnnotations.init(this) mockkObject(MetricsUtils) - val browserStore = BrowserStore() - every { store.state.tabId } returns "test-tab-id" every { store.state.searchEngineSource.searchEngine } returns searchEngine every { sessionManager.select(any()) } just Runs diff --git a/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt index 78e4b6063..b3101d930 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/account/AccountSettingsInteractorTest.kt @@ -13,11 +13,16 @@ import io.mockk.verify import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.Rule import org.junit.Test +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.R class AccountSettingsInteractorTest { + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Test fun onSyncNow() { var ranSyncNow = false diff --git a/app/src/test/java/org/mozilla/fenix/settings/creditcards/DefaultCreditCardsManagementControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/creditcards/DefaultCreditCardsManagementControllerTest.kt index c70a37a73..40097f675 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/creditcards/DefaultCreditCardsManagementControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/creditcards/DefaultCreditCardsManagementControllerTest.kt @@ -9,8 +9,10 @@ import io.mockk.mockk import io.mockk.spyk import io.mockk.verify import org.junit.Before +import org.junit.Rule import org.junit.Test import org.mozilla.fenix.settings.creditcards.controller.DefaultCreditCardsManagementController +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule class DefaultCreditCardsManagementControllerTest { @@ -18,6 +20,9 @@ class DefaultCreditCardsManagementControllerTest { private lateinit var controller: DefaultCreditCardsManagementController + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Before fun setup() { controller = spyk( diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt index 5ef275512..a874e7b3b 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/NavigationInteractorTest.kt @@ -20,10 +20,12 @@ import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.tabstray.Tab import org.junit.Assert.assertTrue import org.junit.Before +import org.junit.Rule import org.junit.Test import org.mozilla.fenix.collections.CollectionsDialog import org.mozilla.fenix.collections.show import org.mozilla.fenix.components.TabCollectionStorage +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.components.bookmarks.BookmarksUseCase import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController @@ -42,6 +44,9 @@ class NavigationInteractorTest { private val context: Context = mockk(relaxed = true) private val collectionStorage: TabCollectionStorage = mockk(relaxed = true) + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Before fun setup() { store = BrowserStore(initialState = BrowserState(tabs = listOf(testTab))) diff --git a/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt index 3c974a981..fafea08ef 100644 --- a/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabtray/DefaultTabTrayControllerTest.kt @@ -31,8 +31,10 @@ import mozilla.components.feature.tabs.TabsUseCases import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Before +import org.junit.Rule import org.junit.Test import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.helpers.DisableNavGraphProviderAssertionRule import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager @@ -68,6 +70,9 @@ class DefaultTabTrayControllerTest { private val tab1 = createTab(url = "http://firefox.com", id = "5678") private val tab2 = createTab(url = "http://mozilla.org", id = "1234") + @get:Rule + val disableNavGraphProviderAssertionRule = DisableNavGraphProviderAssertionRule() + @Before fun setUp() { mockkStatic("org.mozilla.fenix.ext.SessionManagerKt") diff --git a/config/detekt.yml b/config/detekt.yml index 8a28b5c78..18badb710 100644 --- a/config/detekt.yml +++ b/config/detekt.yml @@ -122,6 +122,9 @@ mozilla-detekt-rules: excludes: "**/*Test.kt, **/*Spec.kt, **/test/**, **/androidTest/**" MozillaUseLazyMonitored: active: true + MozillaNavigateCheck: + active: true + excludes: "**/*Test.kt, **/*Spec.kt, **/test/**, **/androidTest/**" empty-blocks: active: true diff --git a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt index 3e829d0f9..aa65b96cf 100644 --- a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt +++ b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt @@ -8,6 +8,7 @@ import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.RuleSet import io.gitlab.arturbosch.detekt.api.RuleSetProvider import org.mozilla.fenix.detektrules.perf.MozillaBannedPropertyAccess +import org.mozilla.fenix.detektrules.perf.MozillaNavigateCheck import org.mozilla.fenix.detektrules.perf.MozillaStrictModeSuppression import org.mozilla.fenix.detektrules.perf.MozillaRunBlockingCheck import org.mozilla.fenix.detektrules.perf.MozillaUseLazyMonitored @@ -22,7 +23,8 @@ class CustomRulesetProvider : RuleSetProvider { MozillaStrictModeSuppression(config), MozillaCorrectUnitTestRunner(config), MozillaRunBlockingCheck(config), - MozillaUseLazyMonitored(config) + MozillaUseLazyMonitored(config), + MozillaNavigateCheck(config) ) ) } diff --git a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaNavigateCheck.kt b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaNavigateCheck.kt new file mode 100644 index 000000000..1f6dbcfd9 --- /dev/null +++ b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaNavigateCheck.kt @@ -0,0 +1,50 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.detektrules.perf + +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +import io.gitlab.arturbosch.detekt.api.Issue +import io.gitlab.arturbosch.detekt.api.Rule +import io.gitlab.arturbosch.detekt.api.Severity +import org.jetbrains.kotlin.psi.KtCallExpression + +private const val VIOLATION_MSG = "If you named a method `navigate`, please rename it to something a bit" + + "more specific such as `navigateTo...`. However, if you are trying to invoke the Android NavController.navigate" + + "please use `org.mozilla.fenix.ext.NavController.navigateBlockingForAsyncNavGraph`" + + "instead. Because using `navigate` directly in the code can lead to the NavGraph not being loaded" + + "since it relies on a blocking call done in `navigateBlockingForAsyncNavGraph`." + +/** + * A check to prevent the use of `navController.navigate`. Since the NavGraph is loaded dynamically + * and asynchronously, there is a check in place to make sure the graph is loaded by wrapping the + * `navigate` function. However, using it directly might lead to code that needs the NavGraph being + * called before it being inflated. + */ +class MozillaNavigateCheck(config: Config = Config.empty) : Rule(config) { + + override val issue = Issue( + "MozillaNavigateCheck", + Severity.Performance, + "Prevents us from calling `navController.navigate` instead of the functions that" + + "wrap it", + Debt.TEN_MINS + ) + + + override fun visitCallExpression(expression: KtCallExpression) { + super.visitCallExpression(expression) + + val calledMethod = expression.calleeExpression?.firstChild?.node?.chars + + //We check for the navigate method and we have to ignore our extension function file, since + //we call navigate there + if (calledMethod == "navigate" ) { + report(CodeSmell(issue, Entity.from(expression), VIOLATION_MSG)) + } + } +}