commit a07e6bed44d2163b5fbd9c2fe21a45e0efc90dbc parent ea726ec371fb956762e04158225ae0a78cdebd1c Author: mcarare <48995920+mcarare@users.noreply.github.com> Date: Mon, 10 Nov 2025 12:00:41 +0000 Bug 1966853 - Centralize navigation logic in MainActivityNavigation r=android-reviewers,avirvara •Introduces an AppNavigation interface to make Navigator testable. •Removes direct Activity dependency from MainActivityNavigation, injecting dependencies and callbacks instead. •Moves UI-related logic (e.g., snackbars) into MainActivity.• Modernizes all fragment transactions to use KTX best practices (commit {}, replace<F>(), and bundleFor patterns). Differential Revision: https://phabricator.services.mozilla.com/D269610 Diffstat:
12 files changed, 664 insertions(+), 303 deletions(-)
diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/activity/MainActivity.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/activity/MainActivity.kt @@ -28,11 +28,13 @@ import mozilla.components.concept.engine.EngineView import mozilla.components.feature.search.widget.BaseVoiceSearchActivity import mozilla.components.lib.auth.canUseBiometricFeature import mozilla.components.lib.crash.Crash +import mozilla.components.lib.state.ext.flow import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.utils.SafeIntent import mozilla.components.support.utils.StatusBarUtils import mozilla.telemetry.glean.private.NoExtras import org.mozilla.experiments.nimbus.initializeTooling +import org.mozilla.experiments.nimbus.internal.FeatureHolder import org.mozilla.focus.GleanMetrics.AppOpened import org.mozilla.focus.GleanMetrics.Notifications import org.mozilla.focus.R @@ -44,9 +46,13 @@ import org.mozilla.focus.ext.settings import org.mozilla.focus.ext.updateSecureWindowFlags import org.mozilla.focus.fragment.BrowserFragment import org.mozilla.focus.fragment.UrlInputFragment +import org.mozilla.focus.fragment.onboarding.OnboardingStorage import org.mozilla.focus.navigation.MainActivityNavigation import org.mozilla.focus.navigation.Navigator +import org.mozilla.focus.nimbus.FocusNimbus +import org.mozilla.focus.nimbus.Onboarding import org.mozilla.focus.searchwidget.ExternalIntentNavigation +import org.mozilla.focus.searchwidget.SearchWidgetUtils import org.mozilla.focus.session.IntentProcessor import org.mozilla.focus.session.PrivateNotificationFeature import org.mozilla.focus.shortcut.HomeScreen @@ -55,6 +61,7 @@ import org.mozilla.focus.state.Screen import org.mozilla.focus.telemetry.startuptelemetry.StartupPathProvider import org.mozilla.focus.telemetry.startuptelemetry.StartupTypeTelemetry import org.mozilla.focus.utils.SupportUtils +import org.mozilla.focus.utils.ViewUtils private const val REQUEST_TIME_OUT = 2000L @@ -67,8 +74,21 @@ open class MainActivity : EdgeToEdgeActivity() { private val intentProcessor by lazy { IntentProcessor(this, components.tabsUseCases, components.customTabsUseCases) } + private val onboardingStorage by lazy { OnboardingStorage(this) } + private val navigator by lazy { + Navigator( + components.appStore.flow(), + MainActivityNavigation( + supportFragmentManager = supportFragmentManager, + onboardingStorage = onboardingStorage, + isInPictureInPictureMode = { isInPictureInPictureMode }, + shouldAnimateHome = ::shouldAnimateHome, + showStartBrowsingCfr = ::showStartBrowsingCfr, + onEraseAction = ::reactToEraseAction, + ), + ) + } - private val navigator by lazy { Navigator(components.appStore, MainActivityNavigation(this)) } private val tabCount: Int get() = components.store.state.privateTabs.size @@ -309,6 +329,92 @@ open class MainActivity : EdgeToEdgeActivity() { } } + /** + * Display the widget promo at first data clearing action and if it wasn't added after 5th Focus session + * or display branded snackbar when widget promo is not shown. + */ + private fun reactToEraseAction() { + val onboardingFeature = FocusNimbus.features.onboarding + + val clearBrowsingSessions = components.settings.getClearBrowsingSessions() + components.settings.addClearBrowsingSessions(INCREMENT_CLEAR_BROWSING_SESSIONS_BY) + + if (shouldShowWidgetPromo(onboardingFeature, clearBrowsingSessions)) { + showWidgetPromo(onboardingFeature) + } else { + showBrandedFeedbackSnackbar() + } + } + + /** + * Determines if the widget promo should be displayed based on feature flags, + * widget installation status, and the number of data clearing actions. + * + * @param onboardingFeature The feature holder for onboarding. + * @param clearCount The number of times data has been cleared. + * @return True if the widget promo should be shown, false otherwise. + */ + private fun shouldShowWidgetPromo( + onboardingFeature: FeatureHolder<Onboarding>, + clearCount: Int, + ): Boolean { + val isPromoEnabled = onboardingFeature.value().isPromoteSearchWidgetDialogEnabled + val isWidgetNotInstalled = !settings.searchWidgetInstalled + val isEligibleSessionCount = + clearCount == FIRST_DATA_CLEARING_ACTION_COUNT || + clearCount == FIFTH_FOCUS_SESSION_THRESHOLD_FOR_PROMO + return isPromoEnabled && isWidgetNotInstalled && isEligibleSessionCount + } + + private fun showWidgetPromo(onboardingFeature: FeatureHolder<Onboarding>) { + onboardingFeature.recordExposure() + SearchWidgetUtils.showPromoteSearchWidgetDialog(this) + } + + /** + * Shows a branded snackbar to provide feedback to the user after an erase action. + * This is typically shown when the widget promo is not displayed. + */ + private fun showBrandedFeedbackSnackbar() { + val rootView = findViewById<View>(android.R.id.content) + ViewUtils.showBrandedSnackbar( + rootView, + R.string.feedback_erase2, + resources.getInteger(R.integer.erase_snackbar_delay), + ) + } + + /** + * Shows the "Start Browsing" CFR if the conditions are met. + * + * This function checks if: + * - The CFR feature is enabled in Nimbus. + * - It's not the first run of the app. + * - The app settings indicate that the CFR should be shown. + * + * If all conditions are true, it sends an exposure event for the onboarding feature + * and dispatches an action to show the CFR. + */ + private fun showStartBrowsingCfr() { + val onboardingConfig = FocusNimbus.features.onboarding.value() + if (onboardingConfig.isCfrEnabled && + !settings.isFirstRun && + settings.shouldShowStartBrowsingCfr + ) { + FocusNimbus.features.onboarding.recordExposure() + components.appStore.dispatch( + AppAction.ShowStartBrowsingCfrChange(true), + ) + } + } + + private fun shouldAnimateHome(): Boolean { + val browserFragment = + supportFragmentManager.findFragmentByTag(BrowserFragment.FRAGMENT_TAG) as? BrowserFragment + ?: return false + return browserFragment.isResumed + } + override fun onCreateView(parent: View?, name: String, context: Context, attrs: AttributeSet): View? { return if (name == EngineView::class.java.name) { components.engine.createView(context, attrs).asView() @@ -383,6 +489,16 @@ open class MainActivity : EdgeToEdgeActivity() { } } + /** + * Gets the [ActionBar] for this activity. + * + * This function lazily inflates the toolbar from a `ViewStub` the first time it's called, + * sets it as the `supportActionBar`, and adjusts its padding and height to account for the + * status bar, ensuring content is not obscured. On subsequent calls, it returns the + * already inflated and configured `ActionBar`. + * + * @return The configured [ActionBar] for the activity. + */ fun getToolbar(): ActionBar { return if (isToolbarInflated) { supportActionBar!! @@ -414,6 +530,12 @@ open class MainActivity : EdgeToEdgeActivity() { components.notificationsDelegate.unBindActivity(this) } + /** + * Represents the different ways an application can be opened, used for telemetry purposes. + * This helps distinguish between a fresh start and resuming from the background. + * + * @property type The string representation of the open type, used for metrics. + */ enum class AppOpenType(val type: String) { LAUNCH("Launch"), RESUME("Resume"), @@ -424,5 +546,9 @@ open class MainActivity : EdgeToEdgeActivity() { const val ACTION_OPEN = "open" const val EXTRA_NOTIFICATION = "notification" + + private const val FIRST_DATA_CLEARING_ACTION_COUNT = 0 + private const val FIFTH_FOCUS_SESSION_THRESHOLD_FOR_PROMO = 4 + private const val INCREMENT_CLEAR_BROWSING_SESSIONS_BY = 1 } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/BrowserFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/BrowserFragment.kt @@ -1195,5 +1195,17 @@ class BrowserFragment : } return fragment } + + /** + * Creates a [Bundle] containing the arguments needed for this fragment. + * + * @param tabId The ID of the tab to create the fragment for. + * @return A [Bundle] with the tab ID set. + */ + fun bundleForTab(tabId: String): Bundle { + return Bundle().apply { + putString(ARGUMENT_SESSION_UUID, tabId) + } + } } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/CrashListFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/CrashListFragment.kt @@ -18,7 +18,7 @@ import org.mozilla.focus.ext.components /** * Fragment showing list of past crashes. */ -class CrashListFragment(private val paddingNeeded: Boolean = false) : AbstractCrashListFragment() { +class CrashListFragment : AbstractCrashListFragment() { override val reporter: CrashReporter by lazy { requireContext().components.crashReporter } override fun onCrashServiceSelected(url: String) { @@ -33,7 +33,9 @@ class CrashListFragment(private val paddingNeeded: Boolean = false) : AbstractCr override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - if (paddingNeeded) { + val showAll = arguments?.getBoolean(ARG_SHOW_ALL, false) ?: false + + if (showAll) { val originalTopPadding = view.paddingTop ViewCompat.setOnApplyWindowInsetsListener(view) { _, windowInsets -> @@ -44,4 +46,9 @@ class CrashListFragment(private val paddingNeeded: Boolean = false) : AbstractCr } } } + + companion object { + const val FRAGMENT_TAG = "crash-list-fragment" + const val ARG_SHOW_ALL = "show_all" + } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/UrlInputFragment.kt @@ -80,39 +80,19 @@ class UrlInputFragment : private const val ANIMATION_DURATION = 200 /** - * Creates a new [UrlInputFragment] that does not yet have an associated session. + * Creates a [Bundle] containing the provided [tabId]. + * This is used to create a new [UrlInputFragment] for an existing tab session. * - * @return A new [UrlInputFragment] instance. + * @param tabId The unique identifier of the tab. + * @return A [Bundle] with the tab ID and animation arguments. */ - @JvmStatic - fun createWithoutSession(): UrlInputFragment { - val arguments = Bundle() - - val fragment = UrlInputFragment() - fragment.arguments = arguments - - return fragment - } - - /** - * Creates a new [UrlInputFragment] that has a session associated with it. - * - * @param tabId The id of the tab that should be displayed. - * @return A new [UrlInputFragment] instance. - */ - @JvmStatic - fun createWithTab( + fun bundleForTab( tabId: String, - ): UrlInputFragment { - val arguments = Bundle() - - arguments.putString(ARGUMENT_SESSION_UUID, tabId) - arguments.putString(ARGUMENT_ANIMATION, ANIMATION_BROWSER_SCREEN) - - val fragment = UrlInputFragment() - fragment.arguments = arguments - - return fragment + ): Bundle { + return Bundle().apply { + putString(ARGUMENT_ANIMATION, ANIMATION_BROWSER_SCREEN) + putString(ARGUMENT_SESSION_UUID, tabId) + } } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/onboarding/OnboardingFirstFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/onboarding/OnboardingFirstFragment.kt @@ -80,4 +80,11 @@ class OnboardingFirstFragment : Fragment() { } } } + + /** + * Companion object for the [OnboardingFirstFragment]. + */ + companion object { + const val FRAGMENT_TAG = "onboarding-first-fragment" + } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/onboarding/OnboardingSecondFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/fragment/onboarding/OnboardingSecondFragment.kt @@ -86,4 +86,11 @@ class OnboardingSecondFragment : Fragment() { onboardingInteractor.onFinishOnBoarding() } } + + /** + * Companion object for [OnboardingSecondFragment]. + */ + companion object { + const val FRAGMENT_TAG = "onboarding-second-fragment" + } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/MainActivityNavigation.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/MainActivityNavigation.kt @@ -5,230 +5,146 @@ package org.mozilla.focus.navigation import android.os.Bundle +import androidx.fragment.app.Fragment +import androidx.fragment.app.FragmentManager +import androidx.fragment.app.FragmentTransaction import androidx.fragment.app.commit +import androidx.fragment.app.replace import org.mozilla.focus.R import org.mozilla.focus.activity.MainActivity -import org.mozilla.focus.autocomplete.AutocompleteAddFragment -import org.mozilla.focus.autocomplete.AutocompleteListFragment -import org.mozilla.focus.autocomplete.AutocompleteRemoveFragment -import org.mozilla.focus.autocomplete.AutocompleteSettingsFragment import org.mozilla.focus.biometrics.BiometricAuthenticationFragment -import org.mozilla.focus.cookiebanner.CookieBannerFragment -import org.mozilla.focus.exceptions.ExceptionsListFragment -import org.mozilla.focus.exceptions.ExceptionsRemoveFragment -import org.mozilla.focus.ext.components -import org.mozilla.focus.ext.settings import org.mozilla.focus.fragment.BrowserFragment import org.mozilla.focus.fragment.CrashListFragment import org.mozilla.focus.fragment.UrlInputFragment -import org.mozilla.focus.fragment.about.AboutFragment import org.mozilla.focus.fragment.onboarding.OnboardingFirstFragment import org.mozilla.focus.fragment.onboarding.OnboardingSecondFragment import org.mozilla.focus.fragment.onboarding.OnboardingStep import org.mozilla.focus.fragment.onboarding.OnboardingStorage -import org.mozilla.focus.locale.screen.LanguageFragment -import org.mozilla.focus.navigation.MainActivityNavigation.SessionWidgetPromoThresholds.FIFTH_CLEAR_SESSION_COUNT -import org.mozilla.focus.navigation.MainActivityNavigation.SessionWidgetPromoThresholds.FIRST_CLEAR_SESSION_COUNT import org.mozilla.focus.nimbus.FocusNimbus -import org.mozilla.focus.nimbus.Onboarding -import org.mozilla.focus.searchwidget.SearchWidgetUtils -import org.mozilla.focus.settings.AboutLibrariesFragment -import org.mozilla.focus.settings.GeneralSettingsFragment -import org.mozilla.focus.settings.InstalledSearchEnginesSettingsFragment -import org.mozilla.focus.settings.ManualAddSearchEngineSettingsFragment -import org.mozilla.focus.settings.MozillaSettingsFragment -import org.mozilla.focus.settings.RemoveSearchEnginesSettingsFragment -import org.mozilla.focus.settings.SearchSettingsFragment -import org.mozilla.focus.settings.SettingsFragment -import org.mozilla.focus.settings.advanced.AdvancedSettingsFragment -import org.mozilla.focus.settings.advanced.SecretSettingsFragment -import org.mozilla.focus.settings.permissions.SitePermissionsFragment import org.mozilla.focus.settings.permissions.permissionoptions.SitePermission import org.mozilla.focus.settings.permissions.permissionoptions.SitePermissionOptionsFragment -import org.mozilla.focus.settings.privacy.PrivacySecuritySettingsFragment -import org.mozilla.focus.state.AppAction import org.mozilla.focus.state.Screen -import org.mozilla.focus.utils.ViewUtils /** - * Class performing the actual navigation in [MainActivity] by performing fragment transactions if - * needed. + * A helper class that manages fragment-based navigation within [MainActivity]. + * + * This class centralizes the logic for creating and switching between different fragments + * (e.g., home screen, browser, settings, onboarding) using the [FragmentManager]. + * By encapsulating navigation, it simplifies management and testing. + * + * @param supportFragmentManager The [FragmentManager] used to perform fragment transactions. + * @param onboardingStorage Manages the state and progress of the user onboarding flow. + * @param isInPictureInPictureMode A lambda that returns whether the activity is currently in PiP mode. + * @param shouldAnimateHome A lambda that determines if the home screen transition should be animated. + * @param showStartBrowsingCfr A function to trigger the "start browsing" CFR if applicable. + * @param onEraseAction A function to potentially trigger the widget promotion. */ class MainActivityNavigation( - private val activity: MainActivity, -) { + private val supportFragmentManager: FragmentManager, + private val onboardingStorage: OnboardingStorage, + private val isInPictureInPictureMode: () -> Boolean, + private val shouldAnimateHome: () -> Boolean, + private val showStartBrowsingCfr: () -> Unit, + private val onEraseAction: () -> Any, +) : AppNavigation { + /** * Home screen. */ - fun home() { - val fragmentManager = activity.supportFragmentManager - val browserFragment = fragmentManager.findFragmentByTag(BrowserFragment.FRAGMENT_TAG) as BrowserFragment? - - val isShowingBrowser = browserFragment != null - val crashReporterIsVisible = browserFragment?.crashReporterIsVisible() == true - - if (isShowingBrowser && !crashReporterIsVisible) { - showPromoteSearchWidgetDialogOrBrandedSnackbar() + override fun navigateToHome() { + // The erase action should only be triggered when we are navigating away from an existing browser session. + if (supportFragmentManager.findFragmentByTag(BrowserFragment.FRAGMENT_TAG) != null) { + onEraseAction() } - // We add the url input fragment to the layout if it doesn't exist yet. - val transaction = fragmentManager - .beginTransaction() - // We only want to play the animation if a browser fragment is added and resumed. // If it is not resumed then the application is currently in the process of resuming // and the session was removed while the app was in the background (e.g. via the // notification). In this case we do not want to show the content and remove the // browser fragment immediately. - val shouldAnimate = isShowingBrowser && browserFragment.isResumed - - if (shouldAnimate) { - transaction.setCustomAnimations(0, R.anim.erase_animation) + commitFragmentTransaction(allowStateLoss = true) { + if (shouldAnimateHome()) { + setCustomAnimations(0, R.anim.erase_animation) + } + replace<UrlInputFragment>(R.id.container, UrlInputFragment.FRAGMENT_TAG) } - showStartBrowsingCfr() - // Currently this callback can get invoked while the app is in the background. Therefore we are using - // commitAllowingStateLoss() here because we can't do a fragment transaction while the app is in the - // background - like we already do in showBrowserScreenForCurrentSession(). - // Ideally we'd make it possible to pause observers while the app is in the background: - // https://github.com/mozilla-mobile/android-components/issues/876 - transaction - .replace( - R.id.container, - UrlInputFragment.createWithoutSession(), - UrlInputFragment.FRAGMENT_TAG, - ) - .commitAllowingStateLoss() - } - - private fun showStartBrowsingCfr() { - val onboardingConfig = FocusNimbus.features.onboarding.value() - if ( - onboardingConfig.isCfrEnabled && - !activity.settings.isFirstRun && - activity.settings.shouldShowStartBrowsingCfr - ) { - FocusNimbus.features.onboarding.recordExposure() - activity.components.appStore.dispatch(AppAction.ShowStartBrowsingCfrChange(true)) - } - } - - /** - * Display the widget promo at first data clearing action and if it wasn't added after 5th Focus session - * or display branded snackbar when widget promo is not shown. - */ - private fun showPromoteSearchWidgetDialogOrBrandedSnackbar() { - val onboardingFeature = FocusNimbus.features.onboarding - val onboardingConfig = onboardingFeature.value() - - val clearBrowsingSessions = activity.components.settings.getClearBrowsingSessions() - activity.components.settings.addClearBrowsingSessions(1) - - if (shouldShowPromoteSearchWidgetDialog(onboardingConfig) && - ( - clearBrowsingSessions == FIRST_CLEAR_SESSION_COUNT || clearBrowsingSessions == FIFTH_CLEAR_SESSION_COUNT - ) - ) { - onboardingFeature.recordExposure() - SearchWidgetUtils.showPromoteSearchWidgetDialog(activity) - } else { - ViewUtils.showBrandedSnackbar( - activity.findViewById(android.R.id.content), - R.string.feedback_erase2, - activity.resources.getInteger(R.integer.erase_snackbar_delay), - ) - } - } - - private fun shouldShowPromoteSearchWidgetDialog(onboadingConfig: Onboarding): Boolean { - return ( - onboadingConfig.isPromoteSearchWidgetDialogEnabled && - !activity.components.settings.searchWidgetInstalled - ) } /** * Show browser for tab with the given [tabId]. */ - fun browser(tabId: String) { - val fragmentManager = activity.supportFragmentManager + override fun navigateToBrowser(tabId: String) { + val currentFragment = supportFragmentManager.findFragmentById(R.id.container) - val urlInputFragment = fragmentManager.findFragmentByTag(UrlInputFragment.FRAGMENT_TAG) as UrlInputFragment? - if (urlInputFragment != null) { - fragmentManager - .beginTransaction() - .remove(urlInputFragment) - .commitAllowingStateLoss() + // Check if the correct BrowserFragment is already visible. + if (currentFragment is BrowserFragment && currentFragment.tab.id == tabId) { + return } - val browserFragment = fragmentManager.findFragmentByTag(BrowserFragment.FRAGMENT_TAG) as BrowserFragment? - if (browserFragment == null || browserFragment.tab.id != tabId) { - fragmentManager - .beginTransaction() - .replace(R.id.container, BrowserFragment.createForTab(tabId), BrowserFragment.FRAGMENT_TAG) - .commitAllowingStateLoss() + commitFragmentTransaction(allowStateLoss = true) { + val args = BrowserFragment.bundleForTab(tabId) + replace<BrowserFragment>(R.id.container, BrowserFragment.FRAGMENT_TAG, args) } } /** - * Edit URL of tab with the given [tabId]. + * Opens the URL input fragment to allow editing the URL of the tab + * associated with the given [tabId]. + * + * If a URL input fragment is already open for this tab, this function + * does nothing. + * + * @param tabId The ID of the tab whose URL is to be edited. */ - fun edit( + override fun navigateToEditUrl( tabId: String, ) { - val fragmentManager = activity.supportFragmentManager + val existingFragment = + supportFragmentManager.findFragmentByTag(UrlInputFragment.FRAGMENT_TAG) as? UrlInputFragment - val urlInputFragment = fragmentManager.findFragmentByTag(UrlInputFragment.FRAGMENT_TAG) as UrlInputFragment? - if (urlInputFragment != null && urlInputFragment.tab?.id == tabId) { + if (existingFragment?.tab?.id == tabId) { // There's already an UrlInputFragment for this tab. return } - val urlFragment = UrlInputFragment.createWithTab(tabId) - - fragmentManager - .beginTransaction() - .add(R.id.container, urlFragment, UrlInputFragment.FRAGMENT_TAG) - .commit() + val args = UrlInputFragment.bundleForTab(tabId) + commitFragmentTransaction { + add(R.id.container, UrlInputFragment::class.java, args, UrlInputFragment.FRAGMENT_TAG) + } } /** * Show onBoarding. */ - fun firstRun() { + override fun navigateToFirstRun() { FocusNimbus.features.onboarding.recordExposure() - val onBoardingStorage = OnboardingStorage(activity) - val onboardingFragment = when (onBoardingStorage.getCurrentOnboardingStep()) { - OnboardingStep.ON_BOARDING_FIRST_SCREEN -> { - OnboardingFirstFragment() - } - OnboardingStep.ON_BOARDING_SECOND_SCREEN -> { - OnboardingSecondFragment() - } - } + when (onboardingStorage.getCurrentOnboardingStep()) { + OnboardingStep.ON_BOARDING_FIRST_SCREEN -> + navigateTo<OnboardingFirstFragment>(tag = OnboardingFirstFragment.FRAGMENT_TAG) - activity.supportFragmentManager - .beginTransaction() - .replace(R.id.container, onboardingFragment, onboardingFragment::class.java.simpleName) - .commit() + OnboardingStep.ON_BOARDING_SECOND_SCREEN -> + navigateTo<OnboardingSecondFragment>(tag = OnboardingSecondFragment.FRAGMENT_TAG) + } } - fun showOnBoardingSecondScreen() { - activity.supportFragmentManager - .beginTransaction() - .replace(R.id.container, OnboardingSecondFragment(), OnboardingSecondFragment::class.java.simpleName) - .commit() + /** + * Shows the second screen of the onboarding process. + * This function replaces the current fragment with [OnboardingSecondFragment]. + */ + override fun navigateToOnboardingSecondScreen() { + navigateTo<OnboardingSecondFragment>(tag = OnboardingSecondFragment.FRAGMENT_TAG) } /** * Show content of about:crashes */ - fun showCrashList() { - activity.supportFragmentManager - .beginTransaction() - .replace(R.id.container, CrashListFragment(true), CrashListFragment()::class.java.simpleName) - .commit() + override fun showCrashList() { + navigateTo<CrashListFragment>( + tag = CrashListFragment.FRAGMENT_TAG, + args = Bundle().apply { putBoolean("show_all", true) }, + ) } /** @@ -237,82 +153,118 @@ class MainActivityNavigation( * @param bundle it is used for app navigation. If the user can unlock with success he should * be redirected to a certain screen. It comes from the external intent. */ - fun lock(bundle: Bundle? = null) { - val fragmentManager = activity.supportFragmentManager + override fun navigateToLockScreen(bundle: Bundle?) { + if (isInPictureInPictureMode()) { + return + } val biometricAuthenticationFragment = - fragmentManager.findFragmentByTag(BiometricAuthenticationFragment.FRAGMENT_TAG) + supportFragmentManager.findFragmentByTag(BiometricAuthenticationFragment.FRAGMENT_TAG) if (biometricAuthenticationFragment != null) { bundle?.let { biometricAuthenticationFragment.arguments = it } return } - if (activity.isInPictureInPictureMode) { - return - } - - fragmentManager.commit { - fragmentManager.fragments.forEach { remove(it) } - replace( + commitFragmentTransaction { + supportFragmentManager.fragments.forEach { remove(it) } + replace<BiometricAuthenticationFragment>( R.id.container, - BiometricAuthenticationFragment.createWithDestinationData(bundle), BiometricAuthenticationFragment.FRAGMENT_TAG, + bundle, ) } } - @Suppress("CyclomaticComplexMethod") - fun settings(page: Screen.Settings.Page) { - val fragment = when (page) { - Screen.Settings.Page.Start -> SettingsFragment() - Screen.Settings.Page.General -> GeneralSettingsFragment() - Screen.Settings.Page.Privacy -> PrivacySecuritySettingsFragment() - Screen.Settings.Page.Search -> SearchSettingsFragment() - Screen.Settings.Page.Advanced -> AdvancedSettingsFragment() - Screen.Settings.Page.Mozilla -> MozillaSettingsFragment() - Screen.Settings.Page.PrivacyExceptions -> ExceptionsListFragment() - Screen.Settings.Page.PrivacyExceptionsRemove -> ExceptionsRemoveFragment() - Screen.Settings.Page.SitePermissions -> SitePermissionsFragment() - Screen.Settings.Page.SecretSettings -> SecretSettingsFragment() - Screen.Settings.Page.SearchList -> InstalledSearchEnginesSettingsFragment() - Screen.Settings.Page.SearchRemove -> RemoveSearchEnginesSettingsFragment() - Screen.Settings.Page.SearchAdd -> ManualAddSearchEngineSettingsFragment() - Screen.Settings.Page.SearchAutocomplete -> AutocompleteSettingsFragment() - Screen.Settings.Page.SearchAutocompleteList -> AutocompleteListFragment() - Screen.Settings.Page.SearchAutocompleteAdd -> AutocompleteAddFragment() - Screen.Settings.Page.SearchAutocompleteRemove -> AutocompleteRemoveFragment() - Screen.Settings.Page.About -> AboutFragment() - Screen.Settings.Page.Licenses -> AboutLibrariesFragment() - Screen.Settings.Page.Locale -> LanguageFragment() - Screen.Settings.Page.CookieBanner -> CookieBannerFragment() - Screen.Settings.Page.CrashList -> CrashListFragment() - } + /** + * Navigates to the specified settings page. + * + * This function determines which settings fragment to display based on the [page] parameter. + * It then replaces the current fragment with the new settings fragment, ensuring that + * the same fragment is not added multiple times. + * + * @param page The specific settings page to navigate to, defined in [Screen.Settings.Page]. + */ + override fun navigateToSettings(page: Screen.Settings.Page) { + val tag = "settings_${page.fragmentClass.simpleName}" + + navigateTo(page.fragmentClass, tag = tag) + } - val tag = "settings_" + fragment::class.java.simpleName + /** + * Navigates to the site permission options screen. + * This function displays a fragment that allows users to configure specific permissions for a website. + * + * @param sitePermission The [SitePermission] object containing information about the site + * and its current permissions. + */ + override fun navigateToSitePermissionOptions(sitePermission: SitePermission) { + navigateTo<SitePermissionOptionsFragment>( + tag = SitePermissionOptionsFragment.FRAGMENT_TAG, + args = SitePermissionOptionsFragment.bundleForSitePermission(sitePermission), + ) + } - val fragmentManager = activity.supportFragmentManager - if (fragmentManager.findFragmentByTag(tag) != null) { + /** + * A generic navigator to replace the fragment in the main container. + * + * This function handles the transaction to replace the currently displayed fragment + * with a new one. It prevents redundant transactions by checking if a fragment with the + * same tag is already the active one in the container. + * + * @param fragmentClass The class of the fragment to instantiate and display. + * @param tag A unique tag used to identify the fragment in the FragmentManager. + * @param args An optional [Bundle] of arguments to pass to the new fragment. + * @param allowStateLoss If true, the transaction can be committed even after + * the activity's state has been saved. + * @param transactionSetup An optional lambda to apply custom configurations to the [FragmentTransaction], + * such as setting custom animations. + */ + private fun <F : Fragment> navigateTo( + fragmentClass: Class<F>, + tag: String, + args: Bundle? = null, + allowStateLoss: Boolean = false, + transactionSetup: (FragmentTransaction.() -> Unit)? = null, + ) { + // Don't navigate if the fragment is already the current one in the container. + if (supportFragmentManager.findFragmentById(R.id.container)?.tag == tag) { return } - fragmentManager.beginTransaction() - .replace(R.id.container, fragment, tag) - .commit() + commitFragmentTransaction(allowStateLoss) { + transactionSetup?.invoke(this) + replace(R.id.container, fragmentClass, args, tag) + } } - fun sitePermissionOptionsFragment(sitePermission: SitePermission) { - val fragmentManager = activity.supportFragmentManager - fragmentManager.beginTransaction() - .replace( - R.id.container, - SitePermissionOptionsFragment.addSitePermission(sitePermission = sitePermission), - SitePermissionOptionsFragment.FRAGMENT_TAG, - ) - .commit() - } + /** + * A reified version of the generic navigator for cleaner call sites. + */ + private inline fun <reified F : Fragment> navigateTo( + tag: String, + args: Bundle? = null, + allowStateLoss: Boolean = false, + noinline transactionSetup: (FragmentTransaction.() -> Unit)? = null, + ) = navigateTo(F::class.java, tag, args, allowStateLoss, transactionSetup) - private object SessionWidgetPromoThresholds { - const val FIRST_CLEAR_SESSION_COUNT = 0 - const val FIFTH_CLEAR_SESSION_COUNT = 4 + /** + * A wrapper for fragment transactions that sets common options. + * + * This function simplifies committing fragment transactions by providing a centralized + * place to configure default behaviors, such as enabling reordering. It uses the + * `commit` extension function from `androidx.fragment.app` for safer and more + * concise transaction management. + * + * @param allowStateLoss Whether the transaction can be committed after the activity's + * state has been saved. Setting this to `true` can prevent crashes but may result + * in the loss of the fragment state if the activity is restored. Defaults to `false`. + * @param block A lambda with a [FragmentTransaction] receiver, containing the + * specific operations for this transaction (e.g., `replace`, `add`, `remove`). + */ + private fun commitFragmentTransaction(allowStateLoss: Boolean = false, block: FragmentTransaction.() -> Unit) { + supportFragmentManager.commit(allowStateLoss) { + setReorderingAllowed(true) + block() + } } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/Navigator.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/navigation/Navigator.kt @@ -4,54 +4,131 @@ package org.mozilla.focus.navigation +import android.os.Bundle import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.cancel +import kotlinx.coroutines.Job +import kotlinx.coroutines.MainScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.flow.map -import mozilla.components.lib.state.ext.flowScoped +import kotlinx.coroutines.launch import mozilla.components.support.base.feature.LifecycleAwareFeature +import org.mozilla.focus.settings.permissions.permissionoptions.SitePermission import org.mozilla.focus.state.AppState -import org.mozilla.focus.state.AppStore import org.mozilla.focus.state.Screen /** - * The [Navigator] subscribes to the [AppStore] and initiates a navigation with the help of the - * provided [MainActivityNavigation] if the [Screen] in the [AppState] changes. + * The [Navigator] observes changes to the [AppState] and triggers navigation + * actions based on the current [Screen]. + * + * It subscribes to a flow of [AppState], and whenever the [Screen] changes, + * it calls the appropriate method on the provided [AppNavigation] implementation + * to navigate the user to the new screen. + * + * @param stateFlow A [Flow] that emits the current [AppState]. + * @param navigation An implementation of [AppNavigation] that handles the actual screen transitions. + * @param dispatcherScope The [CoroutineScope] in which the state observation will run. Defaults to [MainScope]. */ class Navigator( - val store: AppStore, - val navigation: MainActivityNavigation, + private val stateFlow: Flow<AppState>, + private val navigation: AppNavigation, + private val dispatcherScope: CoroutineScope = MainScope(), ) : LifecycleAwareFeature { - private var scope: CoroutineScope? = null + private var navigationJob: Job? = null override fun start() { - scope = store.flowScoped { flow -> subscribe(flow) } + if (navigationJob?.isActive == true) return + + navigationJob = dispatcherScope.launch { + subscribe() + } } override fun stop() { - scope?.cancel() + navigationJob?.cancel() + navigationJob = null } - private suspend fun subscribe(flow: Flow<AppState>) { - flow.map { state -> state.screen } + private suspend fun subscribe() { + stateFlow.map { state -> state.screen } .distinctUntilChangedBy { screen -> screen.id } .collect { screen -> navigateTo(screen) } } private fun navigateTo(screen: Screen) { when (screen) { - is Screen.Home -> navigation.home() - is Screen.Browser -> navigation.browser(screen.tabId) - is Screen.EditUrl -> navigation.edit( + is Screen.Home -> navigation.navigateToHome() + is Screen.Browser -> navigation.navigateToBrowser(screen.tabId) + is Screen.EditUrl -> navigation.navigateToEditUrl( screen.tabId, ) - is Screen.FirstRun -> navigation.firstRun() - is Screen.Locked -> navigation.lock(screen.bundle) - is Screen.Settings -> navigation.settings(screen.page) - is Screen.SitePermissionOptionsScreen -> navigation.sitePermissionOptionsFragment(screen.sitePermission) - is Screen.OnboardingSecondScreen -> navigation.showOnBoardingSecondScreen() + is Screen.FirstRun -> navigation.navigateToFirstRun() + is Screen.Locked -> navigation.navigateToLockScreen(screen.bundle) + is Screen.Settings -> navigation.navigateToSettings(screen.page) + is Screen.SitePermissionOptionsScreen -> navigation.navigateToSitePermissionOptions(screen.sitePermission) + is Screen.OnboardingSecondScreen -> navigation.navigateToOnboardingSecondScreen() is Screen.CrashListScreen -> navigation.showCrashList() } } } + +/** + * Defines the navigation actions available within the application. + * Each method corresponds to a specific screen or navigation flow. + */ +interface AppNavigation { + /** + * Navigates to the home screen. + */ + fun navigateToHome() + + /** + * Navigates to the browser screen with the specified tab ID. + * + * @param tabId The ID of the tab to navigate to. + */ + fun navigateToBrowser(tabId: String) + + /** + * Navigates to the edit URL screen for the given tab. + * + * @param tabId The ID of the tab to navigate to the edit URL screen for. + */ + fun navigateToEditUrl(tabId: String) + + /** + * Navigates to the first run screen. + */ + fun navigateToFirstRun() + + /** + * Navigates to the settings screen. + * + * @param page The specific settings page to navigate to. + */ + fun navigateToSettings(page: Screen.Settings.Page) + + /** + * Navigates to the site permission options screen. + * + * @param sitePermission The [SitePermission] for which the options are to be displayed. + */ + fun navigateToSitePermissionOptions(sitePermission: SitePermission) + + /** + * Navigates to the second screen of the onboarding flow. + */ + fun navigateToOnboardingSecondScreen() + + /** + * Shows lock screen. + * + * @param bundle Optional bundle of data to pass to the lock screen. + */ + fun navigateToLockScreen(bundle: Bundle? = null) + + /** + * Navigates to the crash list screen. + */ + fun showCrashList() +} diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/settings/permissions/permissionoptions/SitePermissionOptionsFragment.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/settings/permissions/permissionoptions/SitePermissionOptionsFragment.kt @@ -18,6 +18,19 @@ import org.mozilla.focus.settings.BaseComposeFragment import org.mozilla.focus.settings.permissions.SitePermissionOption import org.mozilla.focus.state.AppAction +/** + * A fragment that displays a list of options for a specific site permission (e.g., Camera, + * Location). It allows the user to select an option, such as "Ask every time", "Allowed", or + * "Blocked". + * + * This fragment receives a [SitePermission] object via its arguments, which defines the permission + * to be configured. It uses a `SitePermissionOptionsScreenStore` to manage its state and an + * interactor to handle user actions. + * + * For permissions that require a corresponding Android system permission (like Camera or + * Location), this fragment also manages the logic to check for the system-level permission and + * guides the user to the app's settings if it's denied. + */ class SitePermissionOptionsFragment : BaseComposeFragment() { private lateinit var sitePermissionOptionsScreenStore: SitePermissionOptionsScreenStore @@ -34,17 +47,16 @@ class SitePermissionOptionsFragment : BaseComposeFragment() { private const val SITE_PERMISSION = "sitePermission" /** - * Creates a new instance of [SitePermissionOptionsFragment] with the given [SitePermission]. + * Creates a [Bundle] containing the provided [SitePermission]. This is useful for passing + * the site permission data to fragments or other components. * - * @param sitePermission The [SitePermission] to be displayed in the fragment. - * @return A new instance of [SitePermissionOptionsFragment]. + * @param sitePermission The [SitePermission] to be added to the bundle. + * @return A new [Bundle] instance with the site permission parcelable. */ - fun addSitePermission(sitePermission: SitePermission): SitePermissionOptionsFragment { - val fragment = SitePermissionOptionsFragment() - fragment.arguments = Bundle().apply { + fun bundleForSitePermission(sitePermission: SitePermission): Bundle { + return Bundle().apply { putParcelable(SITE_PERMISSION, sitePermission) } - return fragment } } diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/state/AppState.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/state/AppState.kt @@ -5,9 +5,32 @@ package org.mozilla.focus.state import android.os.Bundle +import androidx.fragment.app.Fragment import mozilla.components.feature.top.sites.TopSite import mozilla.components.lib.state.State +import org.mozilla.focus.autocomplete.AutocompleteAddFragment +import org.mozilla.focus.autocomplete.AutocompleteListFragment +import org.mozilla.focus.autocomplete.AutocompleteRemoveFragment +import org.mozilla.focus.autocomplete.AutocompleteSettingsFragment +import org.mozilla.focus.cookiebanner.CookieBannerFragment +import org.mozilla.focus.exceptions.ExceptionsListFragment +import org.mozilla.focus.exceptions.ExceptionsRemoveFragment +import org.mozilla.focus.fragment.CrashListFragment +import org.mozilla.focus.fragment.about.AboutFragment +import org.mozilla.focus.locale.screen.LanguageFragment +import org.mozilla.focus.settings.AboutLibrariesFragment +import org.mozilla.focus.settings.GeneralSettingsFragment +import org.mozilla.focus.settings.InstalledSearchEnginesSettingsFragment +import org.mozilla.focus.settings.ManualAddSearchEngineSettingsFragment +import org.mozilla.focus.settings.MozillaSettingsFragment +import org.mozilla.focus.settings.RemoveSearchEnginesSettingsFragment +import org.mozilla.focus.settings.SearchSettingsFragment +import org.mozilla.focus.settings.SettingsFragment +import org.mozilla.focus.settings.advanced.AdvancedSettingsFragment +import org.mozilla.focus.settings.advanced.SecretSettingsFragment +import org.mozilla.focus.settings.permissions.SitePermissionsFragment import org.mozilla.focus.settings.permissions.permissionoptions.SitePermission +import org.mozilla.focus.settings.privacy.PrivacySecuritySettingsFragment import java.util.UUID /** @@ -100,32 +123,32 @@ sealed class Screen { data class Settings( val page: Page = Page.Start, ) : Screen() { - enum class Page { - Start, + enum class Page(val fragmentClass: Class<out Fragment>) { + Start(SettingsFragment::class.java), - General, - Privacy, - Search, - Advanced, - Mozilla, - About, - Licenses, - Locale, + General(GeneralSettingsFragment::class.java), + Privacy(PrivacySecuritySettingsFragment::class.java), + Search(SearchSettingsFragment::class.java), + Advanced(AdvancedSettingsFragment::class.java), + Mozilla(MozillaSettingsFragment::class.java), + About(AboutFragment::class.java), + Licenses(AboutLibrariesFragment::class.java), + Locale(LanguageFragment::class.java), - PrivacyExceptions, - PrivacyExceptionsRemove, - CookieBanner, - SitePermissions, - SecretSettings, + PrivacyExceptions(ExceptionsListFragment::class.java), + PrivacyExceptionsRemove(ExceptionsRemoveFragment::class.java), + CookieBanner(CookieBannerFragment::class.java), + SitePermissions(SitePermissionsFragment::class.java), + SecretSettings(SecretSettingsFragment::class.java), - SearchList, - SearchRemove, - SearchAdd, - SearchAutocomplete, - SearchAutocompleteList, - SearchAutocompleteAdd, - SearchAutocompleteRemove, - CrashList, + SearchList(InstalledSearchEnginesSettingsFragment::class.java), + SearchRemove(RemoveSearchEnginesSettingsFragment::class.java), + SearchAdd(ManualAddSearchEngineSettingsFragment::class.java), + SearchAutocomplete(AutocompleteSettingsFragment::class.java), + SearchAutocompleteList(AutocompleteListFragment::class.java), + SearchAutocompleteAdd(AutocompleteAddFragment::class.java), + SearchAutocompleteRemove(AutocompleteRemoveFragment::class.java), + CrashList(CrashListFragment::class.java), } } } diff --git a/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/navigation/NavigatorTest.kt b/mobile/android/focus-android/app/src/test/java/org/mozilla/focus/navigation/NavigatorTest.kt @@ -0,0 +1,164 @@ +package org.mozilla.focus.navigation + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runTest +import mozilla.components.support.test.any +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.never +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoMoreInteractions +import org.mockito.junit.MockitoJUnitRunner +import org.mozilla.focus.state.AppState +import org.mozilla.focus.state.Screen + +@RunWith(MockitoJUnitRunner::class) +class NavigatorTest { + + @Mock + private lateinit var navigation: AppNavigation + + private lateinit var navigator: Navigator + + private val testDispatcher = StandardTestDispatcher() + private lateinit var navigatorTestScope: CoroutineScope + + private lateinit var stateFlow: MutableStateFlow<AppState> + + private val homeScreen = Screen.Home + private val browserScreen = Screen.Browser(tabId = "tab1", false) + private val settingsScreen = Screen.Settings(Screen.Settings.Page.Start) + private val otherBrowserScreen = Screen.Browser(tabId = "tab2", false) + + @Before + fun setUp() { + navigatorTestScope = CoroutineScope(testDispatcher + Job()) + + stateFlow = MutableStateFlow(AppState(screen = homeScreen)) + + navigator = Navigator( + stateFlow = stateFlow, + navigation = navigation, + dispatcherScope = navigatorTestScope, + ) + } + + @After + fun tearDown() { + navigatorTestScope.coroutineContext[Job]?.cancel() + } + + @Test + fun `navigator starts and navigates to initial screen`() = runTest(testDispatcher.scheduler) { + navigator.start() + testDispatcher.scheduler.advanceUntilIdle() + + verify(navigation).navigateToHome() + + verifyNoMoreInteractions(navigation) + } + + @Test + fun `navigates to browser screen when app state changes`() = runTest(testDispatcher.scheduler) { + navigator.start() + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToHome() + + stateFlow.value = AppState(screen = browserScreen) + testDispatcher.scheduler.advanceUntilIdle() + + verify(navigation).navigateToBrowser(browserScreen.tabId) + verifyNoMoreInteractions(navigation) + } + + @Test + fun `does not navigate if screen ID remains the same`() = runTest(testDispatcher.scheduler) { + stateFlow.value = AppState(screen = browserScreen) // Set initial for this test + navigator.start() + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToBrowser(browserScreen.tabId) + + stateFlow.value = AppState(screen = Screen.Browser(tabId = browserScreen.tabId, false)) + testDispatcher.scheduler.advanceUntilIdle() + + verify(navigation, times(1)).navigateToBrowser(browserScreen.tabId) + verifyNoMoreInteractions(navigation) + } + + @Test + fun `navigates when screen ID changes even if type is same`() = runTest(testDispatcher.scheduler) { + stateFlow.value = AppState(screen = browserScreen) // browser_id_1 + navigator.start() + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToBrowser(browserScreen.tabId) + + stateFlow.value = AppState(screen = otherBrowserScreen) // browser_id_2 + testDispatcher.scheduler.advanceUntilIdle() + + verify(navigation).navigateToBrowser(otherBrowserScreen.tabId) + verifyNoMoreInteractions(navigation) + } + + @Test + fun `navigates through multiple different screens`() = runTest(testDispatcher.scheduler) { + navigator.start() // Collection starts + + stateFlow.value = AppState(screen = homeScreen) + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToHome() + + stateFlow.value = AppState(screen = browserScreen) + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToBrowser(browserScreen.tabId) + + stateFlow.value = AppState(screen = settingsScreen) + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToSettings(settingsScreen.page) + + verifyNoMoreInteractions(navigation) + } + + @Test + fun `stop cancels observation and no further navigations occur`() = runTest(testDispatcher.scheduler) { + navigator.start() + stateFlow.value = AppState(screen = homeScreen) + testDispatcher.scheduler.advanceUntilIdle() + verify(navigation).navigateToHome() + + navigator.stop() // This should cancel the job in navigatorTestScope + testDispatcher.scheduler.advanceUntilIdle() // Allow cancellation to process + + stateFlow.value = AppState(screen = browserScreen) + testDispatcher.scheduler.advanceUntilIdle() + + verify(navigation, never()).navigateToBrowser(any()) + verify(navigation, times(1)).navigateToHome() // From before stop + verifyNoMoreInteractions(navigation) + } + + @Test + fun `start does not restart if already active`() = runTest(testDispatcher.scheduler) { + navigator.start() + stateFlow.value = AppState(screen = homeScreen) + testDispatcher.scheduler.advanceUntilIdle() // Initial collection + verify(navigation, times(1)).navigateToHome() + + // Try to start again + navigator.start() + stateFlow.value = AppState(screen = browserScreen) // Emit a new state + testDispatcher.scheduler.advanceUntilIdle() + + // Should not have restarted the collection, so homeScreen should not be re-emitted to navigator + // And browserScreen should be the next navigation + verify(navigation, times(1)).navigateToHome() // Still only once + verify(navigation, times(1)).navigateToBrowser(browserScreen.tabId) + verifyNoMoreInteractions(navigation) + } +} diff --git a/mobile/android/focus-android/quality/detekt-baseline.xml b/mobile/android/focus-android/quality/detekt-baseline.xml @@ -59,7 +59,6 @@ <ID>UndocumentedPublicClass:IntentProcessor.kt$IntentProcessor.Result$None : Result</ID> <ID>UndocumentedPublicClass:IntentProcessor.kt$IntentProcessor.Result$Tab : Result</ID> <ID>UndocumentedPublicClass:LocaleDescriptor.kt$LocaleDescriptor : Comparable</ID> - <ID>UndocumentedPublicClass:MainActivity.kt$MainActivity$AppOpenType</ID> <ID>UndocumentedPublicClass:ManualAddSearchEnginePreference.kt$ManualAddSearchEnginePreference : Preference</ID> <ID>UndocumentedPublicClass:ManualAddSearchEngineSettingsFragment.kt$ManualAddSearchEngineSettingsFragment : BaseSettingsFragment</ID> <ID>UndocumentedPublicClass:MozillaPreference.kt$MozillaPreference : Preference</ID> @@ -105,7 +104,6 @@ <ID>UndocumentedPublicClass:SitePermissionOption.kt$SitePermissionOption$AskToAllow : SitePermissionOption</ID> <ID>UndocumentedPublicClass:SitePermissionOption.kt$SitePermissionOption$Blocked : SitePermissionOption</ID> <ID>UndocumentedPublicClass:SitePermissionOptionListItem.kt$SitePermissionOptionListItem</ID> - <ID>UndocumentedPublicClass:SitePermissionOptionsFragment.kt$SitePermissionOptionsFragment : BaseComposeFragment</ID> <ID>UndocumentedPublicClass:SitePermissionOptionsScreenStore.kt$SitePermissionOptionsScreenAction : Action</ID> <ID>UndocumentedPublicClass:SitePermissionOptionsScreenStore.kt$SitePermissionOptionsScreenAction$AndroidPermission : SitePermissionOptionsScreenAction</ID> <ID>UndocumentedPublicClass:SitePermissionOptionsScreenStore.kt$SitePermissionOptionsScreenAction$InitSitePermissionOptions : SitePermissionOptionsScreenAction</ID> @@ -160,10 +158,6 @@ <ID>UndocumentedPublicFunction:HomeMenu.kt$HomeMenu$fun getMenuBuilder(): BrowserMenuBuilder</ID> <ID>UndocumentedPublicFunction:LocaleDescriptor.kt$LocaleDescriptor$fun getNativeName(): String?</ID> <ID>UndocumentedPublicFunction:LocaleDescriptor.kt$LocaleDescriptor$fun getTag(): String</ID> - <ID>UndocumentedPublicFunction:MainActivity.kt$MainActivity$fun getToolbar(): ActionBar</ID> - <ID>UndocumentedPublicFunction:MainActivityNavigation.kt$MainActivityNavigation$@Suppress("CyclomaticComplexMethod") fun settings(page: Screen.Settings.Page)</ID> - <ID>UndocumentedPublicFunction:MainActivityNavigation.kt$MainActivityNavigation$fun showOnBoardingSecondScreen()</ID> - <ID>UndocumentedPublicFunction:MainActivityNavigation.kt$MainActivityNavigation$fun sitePermissionOptionsFragment(sitePermission: SitePermission)</ID> <ID>UndocumentedPublicFunction:ManualAddSearchEnginePreference.kt$ManualAddSearchEnginePreference$fun setProgressViewShown(isShown: Boolean)</ID> <ID>UndocumentedPublicFunction:ManualAddSearchEnginePreference.kt$ManualAddSearchEnginePreference$fun setSearchQueryErrorText(err: String)</ID> <ID>UndocumentedPublicFunction:ManualAddSearchEnginePreference.kt$ManualAddSearchEnginePreference$fun validateEngineNameAndShowError(engineName: String, existingEngines: List<SearchEngine>): Boolean</ID>