commit 190c684e0ea148d9dba216b4d016186e2135404a parent 9dcbdfbe3b6a1100ab636ac24b24fc417ab48547 Author: Mugurell <Mugurell@users.noreply.github.com> Date: Thu, 20 Nov 2025 09:12:31 +0000 Bug 1996643 - part 2 - Migrate from the environment idiom for the composable toolbar r=android-reviewers,skhan,nalexander Differential Revision: https://phabricator.services.mozilla.com/D272155 Diffstat:
29 files changed, 1040 insertions(+), 1714 deletions(-)
diff --git a/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/store/BrowserToolbarStore.kt b/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/store/BrowserToolbarStore.kt @@ -19,7 +19,7 @@ import mozilla.components.lib.state.Store /** * [Store] for maintaining the state of the browser toolbar. */ -open class BrowserToolbarStore( +class BrowserToolbarStore( initialState: BrowserToolbarState = BrowserToolbarState(), middleware: List<Middleware<BrowserToolbarState, BrowserToolbarAction>> = emptyList(), ) : Store<BrowserToolbarState, BrowserToolbarAction>( @@ -140,8 +140,6 @@ private fun reduce(state: BrowserToolbarState, action: BrowserToolbarAction): Br ), ) - is EnvironmentRehydrated, - is EnvironmentCleared, is BrowserToolbarEvent, -> { // no-op diff --git a/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/store/Environment.kt b/mobile/android/android-components/components/compose/browser-toolbar/src/main/java/mozilla/components/compose/browser/toolbar/store/Environment.kt @@ -1,23 +0,0 @@ -/* 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 mozilla.components.compose.browser.toolbar.store - -/** - * The current environment in which the browser toolbar is used allowing access to various - * other application features that the toolbar integrates with. - */ -interface Environment - -/** - * Signals a new valid [Environment] has been set. - * - * @property environment The new [Environment]. - */ -data class EnvironmentRehydrated(val environment: Environment) : BrowserToolbarAction - -/** - * Signals the current [Environment] is not valid anymore. - */ -data object EnvironmentCleared : BrowserToolbarAction diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarkFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarkFragment.kt @@ -15,8 +15,7 @@ import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.ViewCompositionStrategy import androidx.core.content.getSystemService import androidx.fragment.app.Fragment -import androidx.lifecycle.DefaultLifecycleObserver -import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.coroutineScope import androidx.lifecycle.lifecycleScope import androidx.navigation.NavDirections import androidx.navigation.NavHostController @@ -24,9 +23,8 @@ import androidx.navigation.fragment.findNavController import mozilla.components.browser.state.state.searchEngines import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.Mode +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.NavGraphDirections @@ -38,7 +36,6 @@ import org.mozilla.fenix.components.accounts.FenixFxAEntryPoint import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.components.search.BOOKMARKS_SEARCH_ENGINE_ID -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.ext.bookmarkStorage import org.mozilla.fenix.ext.hideToolbar import org.mozilla.fenix.ext.nav @@ -51,7 +48,6 @@ import org.mozilla.fenix.search.BrowserToolbarSearchMiddleware import org.mozilla.fenix.search.BrowserToolbarSearchStatusSyncMiddleware import org.mozilla.fenix.search.BrowserToolbarToFenixSearchMapperMiddleware import org.mozilla.fenix.search.FenixSearchMiddleware -import org.mozilla.fenix.search.SearchFragmentAction import org.mozilla.fenix.search.SearchFragmentState import org.mozilla.fenix.search.SearchFragmentStore import org.mozilla.fenix.search.createInitialSearchFragmentState @@ -224,39 +220,30 @@ class BookmarkFragment : Fragment() { // Default empty store. This is not used without the composable toolbar. BrowserToolbarStore(BrowserToolbarState(mode = Mode.EDIT)) } - else -> StoreProvider.get(this) { + else -> fragmentStore(BrowserToolbarState(mode = Mode.EDIT)) { + val lifecycleScope = viewLifecycleOwner.lifecycle.coroutineScope + BrowserToolbarStore( - initialState = BrowserToolbarState(mode = Mode.EDIT), + initialState = it, middleware = listOf( - BrowserToolbarSearchStatusSyncMiddleware(requireComponents.appStore), + BrowserToolbarSearchStatusSyncMiddleware( + appStore = requireComponents.appStore, + browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, + scope = lifecycleScope, + ), BrowserToolbarSearchMiddleware( + uiContext = requireActivity(), appStore = requireComponents.appStore, browserStore = requireComponents.core.store, components = requireComponents, - settings = requireComponents.settings, - ), - ), - ) - }.also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - context = requireContext(), - fragment = this, navController = findNavController(), browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, + settings = requireComponents.settings, + scope = lifecycleScope, ), ), ) - - viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(EnvironmentCleared) - } - }, - ) - } + }.value } private fun buildSearchStore( @@ -266,19 +253,31 @@ class BookmarkFragment : Fragment() { // Default empty store. This is not used without the composable toolbar. SearchFragmentStore(SearchFragmentState.EMPTY) } - else -> StoreProvider.get(this) { + else -> fragmentStore( + createInitialSearchFragmentState( + activity = requireActivity() as HomeActivity, + components = requireComponents, + tabId = null, + pastedText = null, + searchAccessPoint = MetricsUtils.Source.NONE, + ), + ) { + val lifecycleScope = viewLifecycleOwner.lifecycle.coroutineScope + SearchFragmentStore( - initialState = createInitialSearchFragmentState( - activity = requireActivity() as HomeActivity, - components = requireComponents, - tabId = null, - pastedText = null, - searchAccessPoint = MetricsUtils.Source.NONE, - ), + initialState = it, middleware = listOf( - BrowserToolbarToFenixSearchMapperMiddleware(toolbarStore), - BrowserStoreToFenixSearchMapperMiddleware(requireComponents.core.store), + BrowserToolbarToFenixSearchMapperMiddleware( + toolbarStore = toolbarStore, + browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, + scope = lifecycleScope, + ), + BrowserStoreToFenixSearchMapperMiddleware( + browserStore = requireComponents.core.store, + scope = lifecycleScope, + ), FenixSearchMiddleware( + fragment = this@BookmarkFragment, engine = requireComponents.core.engine, useCases = requireComponents.useCases, nimbusComponents = requireComponents.nimbus, @@ -286,29 +285,12 @@ class BookmarkFragment : Fragment() { appStore = requireComponents.appStore, browserStore = requireComponents.core.store, toolbarStore = toolbarStore, - ), - ), - ) - }.also { - it.dispatch( - SearchFragmentAction.EnvironmentRehydrated( - SearchFragmentStore.Environment( - context = requireContext(), - viewLifecycleOwner = viewLifecycleOwner, + navController = this@BookmarkFragment.findNavController(), browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, - navController = findNavController(), ), ), ) - - viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(SearchFragmentAction.EnvironmentCleared) - } - }, - ) - } + }.value } override fun onResume() { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -1389,7 +1389,7 @@ abstract class BaseBrowserFragment : store: BrowserStore, readerModeController: DefaultReaderModeController, ): BrowserToolbarComposable { - val toolbarStore = buildToolbarStore(activity, readerModeController) + val toolbarStore by buildToolbarStore(activity, readerModeController) browserNavigationBar = BrowserNavigationBar( @@ -1477,13 +1477,13 @@ abstract class BaseBrowserFragment : modifier: Modifier, ) = AwesomeBarComposable( activity = activity, + fragment = this, modifier = modifier, components = requireComponents, appStore = requireComponents.appStore, browserStore = requireComponents.core.store, toolbarStore = toolbarStore, navController = findNavController(), - lifecycleOwner = this, showScrimWhenNoSuggestions = true, ).also { awesomeBarComposable = it diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserToolbarStoreBuilder.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserToolbarStoreBuilder.kt @@ -6,8 +6,8 @@ package org.mozilla.fenix.browser import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment -import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.coroutineScope import androidx.navigation.NavController import mozilla.components.browser.state.state.CustomTabSessionState import mozilla.components.browser.state.store.BrowserStore @@ -17,21 +17,19 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarInteractio import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore import mozilla.components.compose.browser.toolbar.store.DisplayState -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.browser.readermode.ReaderModeController import org.mozilla.fenix.browser.store.BrowserScreenStore import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.Components -import org.mozilla.fenix.components.StoreProvider -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.components.toolbar.BrowserToolbarMiddleware import org.mozilla.fenix.components.toolbar.BrowserToolbarTelemetryMiddleware import org.mozilla.fenix.components.toolbar.CustomTabBrowserToolbarMiddleware -import org.mozilla.fenix.components.toolbar.CustomTabToolbarEnvironment import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.isTallWindow +import org.mozilla.fenix.ext.isWideWindow import org.mozilla.fenix.search.BrowserToolbarSearchMiddleware import org.mozilla.fenix.search.BrowserToolbarSearchStatusSyncMiddleware import org.mozilla.fenix.utils.Settings @@ -73,46 +71,68 @@ object BrowserToolbarStoreBuilder { readerModeController: ReaderModeController, settings: Settings, customTabSession: CustomTabSessionState? = null, - ) = StoreProvider.get(fragment) { - BrowserToolbarStore( - initialState = BrowserToolbarState( - displayState = DisplayState( - pageOrigin = PageOrigin( - hint = R.string.search_hint, - title = null, - url = null, - onClick = object : BrowserToolbarEvent {}, - ), + ) = fragment.fragmentStore( + BrowserToolbarState( + displayState = DisplayState( + pageOrigin = PageOrigin( + hint = R.string.search_hint, + title = null, + url = null, + onClick = object : BrowserToolbarEvent {}, ), ), + ), + ) { + val lifecycleScope = fragment.viewLifecycleOwner.lifecycle.coroutineScope + + BrowserToolbarStore( + initialState = it, middleware = when (customTabSession) { null -> listOf( BrowserToolbarMiddleware( + uiContext = activity, appStore = appStore, browserScreenStore = browserScreenStore, browserStore = browserStore, permissionsStorage = components.core.geckoSitePermissionsStorage, cookieBannersStorage = components.core.cookieBannersStorage, + bookmarksStorage = activity.components.core.bookmarksStorage, trackingProtectionUseCases = components.useCases.trackingProtectionUseCases, useCases = components.useCases, nimbusComponents = components.nimbus, clipboard = activity.components.clipboardHandler, publicSuffixList = components.publicSuffixList, settings = settings, - bookmarksStorage = activity.components.core.bookmarksStorage, + navController = navController, + browsingModeManager = browsingModeManager, + readerModeController = readerModeController, + browserAnimator = browserAnimator, + thumbnailsFeature = thumbnailsFeature, + isWideScreen = { fragment.isWideWindow() }, + isTallScreen = { fragment.isTallWindow() }, + scope = lifecycleScope, + ), + BrowserToolbarSearchStatusSyncMiddleware( + appStore = appStore, + browsingModeManager = browsingModeManager, + scope = lifecycleScope, ), - BrowserToolbarSearchStatusSyncMiddleware(appStore), BrowserToolbarSearchMiddleware( + uiContext = activity, appStore = appStore, browserStore = browserStore, components = components, + navController = navController, + browsingModeManager = browsingModeManager, settings = settings, + scope = lifecycleScope, ), BrowserToolbarTelemetryMiddleware(), ) else -> listOf( CustomTabBrowserToolbarMiddleware( + uiContext = activity, requireNotNull(customTabSession).id, browserStore = browserStore, appStore = appStore, @@ -122,40 +142,13 @@ object BrowserToolbarStoreBuilder { trackingProtectionUseCases = components.useCases.trackingProtectionUseCases, publicSuffixList = components.publicSuffixList, clipboard = activity.components.clipboardHandler, + navController = navController, + closeTabDelegate = { activity.finishAndRemoveTask() }, settings = settings, + scope = lifecycleScope, ), ) }, ) - }.also { - it.dispatch( - EnvironmentRehydrated( - when (customTabSession) { - null -> BrowserToolbarEnvironment( - context = activity, - fragment = fragment, - navController = navController, - browsingModeManager = browsingModeManager, - browserAnimator = browserAnimator, - thumbnailsFeature = thumbnailsFeature, - readerModeController = readerModeController, - ) - else -> CustomTabToolbarEnvironment( - context = activity, - viewLifecycleOwner = fragment.viewLifecycleOwner, - navController = navController, - closeTabDelegate = { activity.finishAndRemoveTask() }, - ) - }, - ), - ) - - fragment.viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(EnvironmentCleared) - } - }, - ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarEnvironment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarEnvironment.kt @@ -1,38 +0,0 @@ -/* 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.components.toolbar - -import android.content.Context -import androidx.fragment.app.Fragment -import androidx.navigation.NavController -import mozilla.components.browser.thumbnails.BrowserThumbnails -import mozilla.components.compose.browser.toolbar.store.Environment -import org.mozilla.fenix.browser.BrowserAnimator -import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.browser.readermode.ReaderModeController - -/** - * The current environment in which the browser toolbar is used allowing access to various - * other application features that the toolbar integrates with. - * - * This is Activity/Fragment lifecycle dependent and should be handled carefully to avoid memory leaks. - * - * @property context [Context] used for various system interactions. - * @property fragment Hosting [Fragment] used as the lifecycle owner and context for UI operations. - * @property navController [NavController] to use for navigating to other in-app destinations. - * @property browsingModeManager [BrowsingModeManager] for querying the current browsing mode. - * @property browserAnimator Helper for animating the browser content when navigating to other screens. - * @property thumbnailsFeature [BrowserThumbnails] for requesting screenshots of the current tab. - * @property readerModeController [ReaderModeController] for showing or hiding the reader view UX. - */ -data class BrowserToolbarEnvironment( - val context: Context, - val fragment: Fragment, - val navController: NavController, - val browsingModeManager: BrowsingModeManager, - val browserAnimator: BrowserAnimator? = null, - val thumbnailsFeature: () -> BrowserThumbnails? = { null }, - val readerModeController: ReaderModeController? = null, -) : Environment diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt @@ -4,12 +4,12 @@ package org.mozilla.fenix.components.toolbar +import android.content.Context import android.os.Build import androidx.annotation.VisibleForTesting -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import androidx.navigation.NavController import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow @@ -26,6 +26,7 @@ import mozilla.components.browser.state.selector.selectedTab import mozilla.components.browser.state.state.content.ShareResourceState import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.browser.thumbnails.BrowserThumbnails import mozilla.components.compose.browser.toolbar.concept.Action import mozilla.components.compose.browser.toolbar.concept.Action.ActionButton import mozilla.components.compose.browser.toolbar.concept.Action.ActionButtonRes @@ -53,8 +54,6 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.B import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuButton.Text.StringResText import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuDivider import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.ProgressBarConfig import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.concept.engine.EngineSession.LoadUrlFlags @@ -84,10 +83,13 @@ import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.ReaderMode import org.mozilla.fenix.GleanMetrics.Translations import org.mozilla.fenix.R +import org.mozilla.fenix.browser.BrowserAnimator import org.mozilla.fenix.browser.BrowserFragmentDirections import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Normal import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Private +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager +import org.mozilla.fenix.browser.readermode.ReaderModeController import org.mozilla.fenix.browser.store.BrowserScreenAction import org.mozilla.fenix.browser.store.BrowserScreenStore import org.mozilla.fenix.components.AppStore @@ -121,8 +123,6 @@ import org.mozilla.fenix.components.toolbar.TabCounterInteractions.AddNewTab import org.mozilla.fenix.components.toolbar.TabCounterInteractions.CloseCurrentTab import org.mozilla.fenix.components.toolbar.TabCounterInteractions.TabCounterClicked import org.mozilla.fenix.components.toolbar.TabCounterInteractions.TabCounterLongClicked -import org.mozilla.fenix.ext.isTallWindow -import org.mozilla.fenix.ext.isWideWindow import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.nimbus.FxNimbus @@ -182,42 +182,57 @@ internal sealed class PageEndActionsInteractions : BrowserToolbarEvent { /** * [Middleware] responsible for configuring and handling interactions with the composable toolbar. * + * @param uiContext [Context] used for various system interactions. * @param appStore [AppStore] allowing to integrate with other features of the applications. * @param browserScreenStore [BrowserScreenStore] used for integration with other browser screen functionalities. * @param browserStore [BrowserStore] to sync from. * @param permissionsStorage [SitePermissionsStorage] to find currently selected tab site permissions. * @param cookieBannersStorage [CookieBannersStorage] to get the current status of cookie banner ui mode. - * @param trackingProtectionUseCases [TrackingProtectionUseCases] allowing to query - * tracking protection data of the current tab. + * @param bookmarksStorage [BookmarksStorage] to read and write bookmark data related to the current site. + * @param trackingProtectionUseCases [TrackingProtectionUseCases] allowing to query tracking protection data + * of the current tab. * @param useCases [UseCases] helping this integrate with other features of the applications. + * @param sessionUseCases [SessionUseCases] for interacting with the current session. * @param nimbusComponents [NimbusComponents] used for accessing Nimbus events to use in telemetry. * @param clipboard [ClipboardHandler] to use for reading from device's clipboard. * @param publicSuffixList [PublicSuffixList] used to obtain the base domain of the current site. * @param settings [Settings] for accessing user preferences. - * @param sessionUseCases [SessionUseCases] for interacting with the current session. - * @param bookmarksStorage [BookmarksStorage] to read and write bookmark data related to the current site. + * @param navController [NavController] to use for navigating to other in-app destinations. + * @param browsingModeManager [BrowsingModeManager] for querying the current browsing mode. + * @param readerModeController [ReaderModeController] for showing or hiding the reader view UX. + * @param browserAnimator Helper for animating the browser content when navigating to other screens. + * @param thumbnailsFeature [BrowserThumbnails] for requesting screenshots of the current tab. + * @param isWideScreen Callback for checking if the screen is wide. + * @param isTallScreen Callback for checking if the screen is tall. + * @param scope [CoroutineScope] used for running long running operations in background. * @param ioDispatcher [CoroutineDispatcher] to use for IO operations. */ @Suppress("LargeClass", "LongParameterList", "TooManyFunctions") class BrowserToolbarMiddleware( + private val uiContext: Context, private val appStore: AppStore, private val browserScreenStore: BrowserScreenStore, private val browserStore: BrowserStore, private val permissionsStorage: SitePermissionsStorage, private val cookieBannersStorage: CookieBannersStorage, + private val bookmarksStorage: BookmarksStorage, private val trackingProtectionUseCases: TrackingProtectionUseCases, private val useCases: UseCases, + private val sessionUseCases: SessionUseCases = SessionUseCases(browserStore), private val nimbusComponents: NimbusComponents, private val clipboard: ClipboardHandler, private val publicSuffixList: PublicSuffixList, private val settings: Settings, - private val sessionUseCases: SessionUseCases = SessionUseCases(browserStore), - private val bookmarksStorage: BookmarksStorage, + private val navController: NavController, + private val browsingModeManager: BrowsingModeManager, + private val readerModeController: ReaderModeController, + private val browserAnimator: BrowserAnimator, + private val thumbnailsFeature: () -> BrowserThumbnails?, + private val isWideScreen: () -> Boolean, + private val isTallScreen: () -> Boolean, + private val scope: CoroutineScope, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, ) : Middleware<BrowserToolbarState, BrowserToolbarAction> { - @VisibleForTesting - internal var environment: BrowserToolbarEnvironment? = null - @Suppress("LongMethod", "CyclomaticComplexMethod", "NestedBlockDepth", "ReturnCount", "CognitiveComplexMethod") override fun invoke( context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>, @@ -230,22 +245,13 @@ class BrowserToolbarMiddleware( appStore.dispatch(SearchEnded) - updateStartPageActions(context) - } - - is EnvironmentRehydrated -> { - next(action) - - environment = action.environment as? BrowserToolbarEnvironment - updateStartBrowserActions(context) updateStartPageActions(context) updateCurrentPageOrigin(context) - environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch { - updateEndBrowserActions(context) - } updateEndPageActions(context) - environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch { + + scope.launch { + updateEndBrowserActions(context) updateNavigationActions(context) } @@ -265,18 +271,12 @@ class BrowserToolbarMiddleware( observePermissionHighlightsUpdates(context) } - is EnvironmentCleared -> { - next(action) - - environment = null - } - is StartPageActions.SiteInfoClicked -> { onSiteInfoClicked() } is MenuClicked -> { - environment?.navController?.nav( + navController.nav( R.id.browserFragment, BrowserFragmentDirections.actionGlobalMenuDialogFragment( accesspoint = MenuAccessPoint.Browser, @@ -287,30 +287,28 @@ class BrowserToolbarMiddleware( } is TabCounterClicked -> { - runWithinEnvironment { - thumbnailsFeature()?.requestScreenshot() + thumbnailsFeature()?.requestScreenshot() - if (settings.tabManagerEnhancementsEnabled) { - navController.nav( - R.id.browserFragment, - BrowserFragmentDirections.actionGlobalTabManagementFragment( - page = when (browsingModeManager.mode) { - Normal -> Page.NormalTabs - Private -> Page.PrivateTabs - }, - ), - ) - } else { - navController.nav( - R.id.browserFragment, - BrowserFragmentDirections.actionGlobalTabsTrayFragment( - page = when (browsingModeManager.mode) { - Normal -> Page.NormalTabs - Private -> Page.PrivateTabs - }, - ), - ) - } + if (settings.tabManagerEnhancementsEnabled) { + navController.nav( + R.id.browserFragment, + BrowserFragmentDirections.actionGlobalTabManagementFragment( + page = when (browsingModeManager.mode) { + Normal -> Page.NormalTabs + Private -> Page.PrivateTabs + }, + ), + ) + } else { + navController.nav( + R.id.browserFragment, + BrowserFragmentDirections.actionGlobalTabsTrayFragment( + page = when (browsingModeManager.mode) { + Normal -> Page.NormalTabs + Private -> Page.PrivateTabs + }, + ), + ) } next(action) @@ -334,14 +332,11 @@ class BrowserToolbarMiddleware( } if (!selectedTab.content.private) { - runWithinEnvironment { - navController.navigate( - BrowserFragmentDirections.actionGlobalHome( - sessionToDelete = selectedTab.id, - ), - ) - return@let - } + navController.navigate( + BrowserFragmentDirections.actionGlobalHome( + sessionToDelete = selectedTab.id, + ), + ) } val privateDownloads = browserStore.state.downloads.filter { @@ -355,13 +350,11 @@ class BrowserToolbarMiddleware( ), ) } else { - runWithinEnvironment { - navController.navigate( - BrowserFragmentDirections.actionGlobalHome( - sessionToDelete = selectedTab.id, - ), - ) - } + navController.navigate( + BrowserFragmentDirections.actionGlobalHome( + sessionToDelete = selectedTab.id, + ), + ) } } } @@ -372,14 +365,12 @@ class BrowserToolbarMiddleware( val selectedTab = browserStore.state.selectedTab ?: return val searchTerms = selectedTab.content.searchTerms if (searchTerms.isBlank()) { - runWithinEnvironment { - navController.navigate( - BrowserFragmentDirections.actionGlobalHome( - focusOnAddressBar = true, - sessionToStartSearchFor = selectedTab.id, - ), - ) - } + navController.navigate( + BrowserFragmentDirections.actionGlobalHome( + focusOnAddressBar = true, + sessionToStartSearchFor = selectedTab.id, + ), + ) } else { context.dispatch(SearchQueryUpdated(BrowserToolbarQuery(searchTerms))) appStore.dispatch(SearchStarted(selectedTab.id)) @@ -400,7 +391,7 @@ class BrowserToolbarMiddleware( appStore.dispatch(URLCopiedToClipboard) } } - is PasteFromClipboardClicked -> runWithinEnvironment { + is PasteFromClipboardClicked -> { context.dispatch(SearchQueryUpdated(BrowserToolbarQuery(clipboard.text.orEmpty()))) appStore.dispatch(SearchStarted(browserStore.state.selectedTabId)) } @@ -436,7 +427,7 @@ class BrowserToolbarMiddleware( searchTermOrURL = it, newTab = false, searchEngine = searchEngine, - private = environment?.browsingModeManager?.mode == Private, + private = browsingModeManager?.mode == Private, ) } ?: run { Logger("BrowserOriginContextMenu").error("Clipboard contains URL but unable to read text") @@ -463,15 +454,15 @@ class BrowserToolbarMiddleware( next(action) } - is ReaderModeClicked -> runWithinEnvironment { + is ReaderModeClicked -> { when (action.isActive) { true -> { ReaderMode.closed.record(NoExtras()) - readerModeController?.hideReaderView() + readerModeController.hideReaderView() } false -> { ReaderMode.opened.record(NoExtras()) - readerModeController?.showReaderView() + readerModeController.showReaderView() } } @@ -482,12 +473,10 @@ class BrowserToolbarMiddleware( Translations.action.record(Translations.ActionExtra("main_flow_toolbar")) appStore.dispatch(SnackbarDismissed) - runWithinEnvironment { - navController.navigateSafe( - resId = R.id.browserFragment, - directions = BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(), - ) - } + navController.navigateSafe( + resId = R.id.browserFragment, + directions = BrowserFragmentDirections.actionBrowserFragmentToTranslationsDialogFragment(), + ) } is RefreshClicked -> { @@ -514,7 +503,7 @@ class BrowserToolbarMiddleware( val selectedTab = browserStore.state.selectedTab selectedTab?.let { - environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch(ioDispatcher) { + scope.launch(ioDispatcher) { val parentGuid = settings.lastSavedFolderCache.getGuid() ?: BookmarkRoot.Mobile.id val parentNode = bookmarksStorage.getBookmark(parentGuid).getOrNull() val guidToEdit = useCases.bookmarksUseCases.addBookmark( @@ -536,10 +525,10 @@ class BrowserToolbarMiddleware( next(action) } - is EditBookmarkClicked -> runWithinEnvironment { + is EditBookmarkClicked -> { val selectedTab = browserStore.state.selectedTab ?: return - environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch(Dispatchers.Main) { + scope.launch(Dispatchers.Main) { val guidToEdit: String? = withContext(ioDispatcher) { bookmarksStorage .getBookmarksWithUrl(selectedTab.content.url) @@ -562,7 +551,7 @@ class BrowserToolbarMiddleware( next(action) } - is ShareClicked -> runWithinEnvironment { + is ShareClicked -> { val selectedTab = browserStore.state.selectedTab ?: return if (selectedTab.content.url.isContentUrl()) { browserStore.dispatch( @@ -590,7 +579,7 @@ class BrowserToolbarMiddleware( next(action) } - is HomepageClicked -> runWithinEnvironment { + is HomepageClicked -> { if (settings.enableHomepageAsNewTab) { useCases.fenixBrowserUseCases.navigateToHomepage() } else { @@ -606,18 +595,15 @@ class BrowserToolbarMiddleware( } } - private fun showTabHistory() = runWithinEnvironment { - navController.nav( - R.id.browserFragment, - BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment( - activeSessionId = null, - ), - ) - } + private fun showTabHistory() = navController.nav( + R.id.browserFragment, + BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment( + activeSessionId = null, + ), + ) private fun onSiteInfoClicked() { val tab = browserStore.state.selectedTab ?: return - val scope = environment?.fragment?.viewLifecycleOwner?.lifecycleScope ?: return scope.launch(ioDispatcher) { val sitePermissions: SitePermissions? = tab.content.url.getOrigin()?.let { origin -> permissionsStorage.findSitePermissionsBy(origin, private = tab.content.private) @@ -662,7 +648,7 @@ class BrowserToolbarMiddleware( cookieBannerUIMode = cookieBannerUIMode, ) } - environment?.navController?.nav( + navController.nav( R.id.browserFragment, directions, ) @@ -718,7 +704,7 @@ class BrowserToolbarMiddleware( * - The navigation buttons (forward, back, and refresh) are always shown on the left side of the address bar. */ private fun buildStartBrowserActions(): List<Action> { - val isWideScreen = environment?.fragment?.isWideWindow() == true + val isWideScreen = isWideScreen() return listOf( ToolbarActionConfig(ToolbarAction.Back) { isWideScreen }, @@ -736,7 +722,7 @@ class BrowserToolbarMiddleware( * - The page action buttons (Share and Translate), which were removed from smaller devices, are shown again. */ private fun buildEndPageActions(): List<Action> { - val isWideScreen = environment?.fragment?.isWideWindow() == true + val isWideScreen = isWideScreen() val tabStripEnabled = settings.isTabStripEnabled val simpleShortcut = ShortcutType.fromValue(settings.toolbarSimpleShortcutKey) val translateShortcutEnabled = simpleShortcut == ShortcutType.TRANSLATE @@ -762,8 +748,8 @@ class BrowserToolbarMiddleware( } private suspend fun buildEndBrowserActions(): List<Action> { - val isWideWindow = environment?.fragment?.isWideWindow() == true - val isTallWindow = environment?.fragment?.isTallWindow() == true + val isWideWindow = isWideScreen() + val isTallWindow = isTallScreen() val tabStripEnabled = settings.isTabStripEnabled val shouldUseExpandedToolbar = settings.shouldUseExpandedToolbar val useCustomPrimary = settings.shouldShowToolbarCustomization @@ -804,9 +790,8 @@ class BrowserToolbarMiddleware( * - The toolbar redesign customization option is also hidden. */ private suspend fun buildNavigationActions(): List<Action> { - val environment = environment ?: return emptyList() - val isWideWindow = environment.fragment.isWideWindow() - val isTallWindow = environment.fragment.isTallWindow() + val isWideWindow = isWideScreen() + val isTallWindow = isTallScreen() val shouldUseExpandedToolbar = settings.shouldUseExpandedToolbar val useCustomPrimary = settings.shouldShowToolbarCustomization val primarySlotAction = ShortcutType.fromValue(settings.toolbarExpandedShortcutKey) @@ -874,7 +859,7 @@ class BrowserToolbarMiddleware( private fun openNewTab( browsingMode: BrowsingMode, - ) = runWithinEnvironment { + ) { if (settings.enableHomepageAsNewTab) { useCases.fenixBrowserUseCases.addNewHomepageTab( private = browsingMode.isPrivate, @@ -946,7 +931,7 @@ class BrowserToolbarMiddleware( private fun updateCurrentPageOrigin( context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>, - ) = environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch { + ) = scope.launch { val url = browserStore.state.selectedTab?.content?.url?.let { it.applyRegistrableDomainSpan(publicSuffixList) } @@ -1089,17 +1074,7 @@ class BrowserToolbarMiddleware( private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.fragment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } - - private inline fun runWithinEnvironment( - block: BrowserToolbarEnvironment.() -> Unit, - ) = environment?.let { block(it) } + ): Job = scope.launch { flow().observe() } @VisibleForTesting internal enum class ToolbarAction { @@ -1135,12 +1110,12 @@ class BrowserToolbarMiddleware( ): Action = when (toolbarAction) { ToolbarAction.NewTab -> ActionButtonRes( drawableResId = iconsR.drawable.mozac_ic_plus_24, - contentDescription = if (environment?.browsingModeManager?.mode == Private) { + contentDescription = if (browsingModeManager?.mode == Private) { R.string.home_screen_shortcut_open_new_private_tab_2 } else { R.string.home_screen_shortcut_open_new_tab_2 }, - onClick = if (environment?.browsingModeManager?.mode == Private) { + onClick = if (browsingModeManager?.mode == Private) { AddNewPrivateTab(source) } else { AddNewTab(source) @@ -1222,14 +1197,13 @@ class BrowserToolbarMiddleware( ) ToolbarAction.TabCounter -> { - val environment = requireNotNull(environment) - val isInPrivateMode = environment.browsingModeManager.mode.isPrivate + val isInPrivateMode = browsingModeManager.mode.isPrivate val tabsCount = browserStore.state.getNormalOrPrivateTabs(isInPrivateMode).size val tabCounterDescription = if (isInPrivateMode) { - environment.context.getString(tabcounterR.string.mozac_tab_counter_private, tabsCount.toString()) + uiContext.getString(tabcounterR.string.mozac_tab_counter_private, tabsCount.toString()) } else { - environment.context.getString(tabcounterR.string.mozac_tab_counter_open_tab_tray, tabsCount.toString()) + uiContext.getString(tabcounterR.string.mozac_tab_counter_open_tab_tray, tabsCount.toString()) } TabCounterAction( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/CustomTabBrowserToolbarMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/CustomTabBrowserToolbarMiddleware.kt @@ -4,16 +4,16 @@ package org.mozilla.fenix.components.toolbar +import android.content.Context import android.content.Intent import android.os.Build import androidx.annotation.VisibleForTesting import androidx.appcompat.content.res.AppCompatResources import androidx.core.graphics.drawable.toDrawable import androidx.core.net.toUri -import androidx.lifecycle.Lifecycle.State.RESUMED import androidx.lifecycle.ViewModel -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import androidx.navigation.NavController +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow @@ -38,8 +38,6 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.Init import mozilla.components.compose.browser.toolbar.store.BrowserToolbarInteraction.BrowserToolbarEvent import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.ProgressBarConfig import mozilla.components.concept.engine.cookiehandling.CookieBannersStorage import mozilla.components.concept.engine.permission.SitePermissions @@ -97,6 +95,7 @@ private const val CUSTOM_BUTTON_CLICK_RETURN_CODE = 0 * * This is also a [ViewModel] allowing to be easily persisted between activity restarts. * + * @param uiContext [Context] used for various system interactions. * @param customTabId [String] of the custom tab in which the toolbar is shown. * @param browserStore [BrowserStore] to sync from. * @param appStore [AppStore] allowing to integrate with other features of the applications. @@ -107,10 +106,14 @@ private const val CUSTOM_BUTTON_CLICK_RETURN_CODE = 0 * tracking protection data of the current tab. * @param publicSuffixList [PublicSuffixList] used to obtain the base domain of the current site. * @param clipboard [ClipboardHandler] to use for reading from device's clipboard. + * @param navController [NavController] to use for navigating to other in-app destinations. + * @param closeTabDelegate Callback for when the current custom tab needs to be closed. * @param settings [Settings] for accessing user preferences. + * @param scope [CoroutineScope] used for running long running operations in background. */ @Suppress("LongParameterList") class CustomTabBrowserToolbarMiddleware( + private val uiContext: Context, private val customTabId: String, private val browserStore: BrowserStore, private val appStore: AppStore, @@ -120,10 +123,11 @@ class CustomTabBrowserToolbarMiddleware( private val trackingProtectionUseCases: TrackingProtectionUseCases, private val publicSuffixList: PublicSuffixList, private val clipboard: ClipboardHandler, + private val navController: NavController, + private val closeTabDelegate: () -> Unit, private val settings: Settings, -) : Middleware<BrowserToolbarState, BrowserToolbarAction>, ViewModel() { - @VisibleForTesting - internal var environment: CustomTabToolbarEnvironment? = null + private val scope: CoroutineScope, +) : Middleware<BrowserToolbarState, BrowserToolbarAction> { private val customTab get() = browserStore.state.findCustomTab(customTabId) private var wasTitleShown = false @@ -140,17 +144,10 @@ class CustomTabBrowserToolbarMiddleware( val customTab = customTab updateStartPageActions(context, customTab) - updateEndBrowserActions(context, customTab) - } - - is EnvironmentRehydrated -> { - next(action) - - environment = action.environment as? CustomTabToolbarEnvironment - updateStartBrowserActions(context, customTab) updateCurrentPageOrigin(context, customTab) updateEndPageActions(context, customTab) + updateEndBrowserActions(context, customTab) observePageLoadUpdates(context) observePageOriginUpdates(context) @@ -158,19 +155,13 @@ class CustomTabBrowserToolbarMiddleware( observePageTrackingProtectionUpdates(context) } - is EnvironmentCleared -> { - next(action) - - environment = null - } - is CloseClicked -> { Toolbar.buttonTapped.record( Toolbar.ButtonTappedExtra(source = SOURCE_CUSTOM_BAR, item = ACTION_CLOSE_CLICKED), ) useCases.remove(customTabId) - environment?.closeTabDelegate() + closeTabDelegate() } is SiteInfoClicked -> { @@ -178,16 +169,15 @@ class CustomTabBrowserToolbarMiddleware( Toolbar.ButtonTappedExtra(source = SOURCE_CUSTOM_BAR, item = ACTION_SECURITY_INDICATOR_CLICKED), ) - val environment = environment ?: return val customTab = requireNotNull(customTab) - environment.viewLifecycleOwner.lifecycleScope.launch(Dispatchers.IO) { + scope.launch(Dispatchers.IO) { val sitePermissions: SitePermissions? = customTab.content.url.getOrigin()?.let { origin -> permissionsStorage.findSitePermissionsBy(origin, private = customTab.content.private) } - environment.viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { + scope.launch(Dispatchers.Main) { trackingProtectionUseCases.containsException(customTabId) { isExcepted -> - environment.viewLifecycleOwner.lifecycleScope.launch { + scope.launch { val cookieBannerUIMode = cookieBannersStorage.getCookieBannerUIMode( tab = customTab, isFeatureEnabledInPrivateMode = settings.shouldUseCookieBannerPrivateMode, @@ -225,7 +215,7 @@ class CustomTabBrowserToolbarMiddleware( cookieBannerUIMode = cookieBannerUIMode, ) } - environment.navController.nav( + navController.nav( R.id.externalAppBrowserFragment, directions, ) @@ -239,10 +229,9 @@ class CustomTabBrowserToolbarMiddleware( Toolbar.buttonTapped.record( Toolbar.ButtonTappedExtra(source = SOURCE_CUSTOM_BAR, item = ACTION_SITE_CUSTOM_CLICKED), ) - val environment = environment ?: return val customTab = customTab customTab?.config?.actionButtonConfig?.pendingIntent?.send( - environment.context, + uiContext, CUSTOM_BUTTON_CLICK_RETURN_CODE, Intent(null, customTab.content.url.toUri()), ) @@ -253,7 +242,7 @@ class CustomTabBrowserToolbarMiddleware( Toolbar.ButtonTappedExtra(source = SOURCE_CUSTOM_BAR, item = ACTION_SHARE_CLICKED), ) val customTab = customTab - environment?.navController?.navigate( + navController.navigate( NavGraphDirections.actionGlobalShareFragment( sessionId = customTabId, data = arrayOf( @@ -271,15 +260,13 @@ class CustomTabBrowserToolbarMiddleware( Toolbar.buttonTapped.record( Toolbar.ButtonTappedExtra(source = SOURCE_CUSTOM_BAR, item = ACTION_MENU_CLICKED), ) - runWithinEnvironment { - navController.nav( - R.id.externalAppBrowserFragment, - BrowserFragmentDirections.actionGlobalMenuDialogFragment( - accesspoint = MenuAccessPoint.External, - customTabSessionId = customTabId, - ), - ) - } + navController.nav( + R.id.externalAppBrowserFragment, + BrowserFragmentDirections.actionGlobalMenuDialogFragment( + accesspoint = MenuAccessPoint.External, + customTabSessionId = customTabId, + ), + ) } is CopyToClipboardClicked -> { @@ -366,7 +353,7 @@ class CustomTabBrowserToolbarMiddleware( context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>, customTab: CustomTabSessionState?, ) { - environment?.viewLifecycleOwner?.lifecycleScope?.launch { + scope.launch { context.dispatch( BrowserDisplayToolbarAction.PageOriginUpdated( PageOrigin( @@ -400,7 +387,6 @@ class CustomTabBrowserToolbarMiddleware( ) private fun buildStartBrowserActions(customTab: CustomTabSessionState?): List<Action> { - val environment = environment ?: return emptyList() val customTabConfig = customTab?.config val customIconBitmap = customTabConfig?.closeButtonIcon @@ -409,12 +395,12 @@ class CustomTabBrowserToolbarMiddleware( ActionButton( drawable = when (customIconBitmap) { null -> AppCompatResources.getDrawable( - environment.context, iconsR.drawable.mozac_ic_cross_24, + uiContext, iconsR.drawable.mozac_ic_cross_24, ) - else -> customIconBitmap.toDrawable(environment.context.resources) + else -> customIconBitmap.toDrawable(uiContext.resources) }, - contentDescription = environment.context.getString( + contentDescription = uiContext.getString( customtabsR.string.mozac_feature_customtabs_exit_button, ), onClick = CloseClicked, @@ -458,7 +444,6 @@ class CustomTabBrowserToolbarMiddleware( } private fun buildEndPageActions(customTab: CustomTabSessionState?): List<ActionButton> { - val environment = environment ?: return emptyList() val customButtonConfig = customTab?.config?.actionButtonConfig val customButtonIcon = customButtonConfig?.icon @@ -466,7 +451,7 @@ class CustomTabBrowserToolbarMiddleware( null -> emptyList() else -> listOf( ActionButton( - drawable = customButtonIcon.toDrawable(environment.context.resources), + drawable = customButtonIcon.toDrawable(uiContext.resources), shouldTint = customTab.content.private || customButtonConfig.tint, contentDescription = customButtonConfig.description, onClick = CustomButtonClicked, @@ -539,17 +524,7 @@ class CustomTabBrowserToolbarMiddleware( private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } - - private inline fun runWithinEnvironment( - block: CustomTabToolbarEnvironment.() -> Unit, - ) = environment?.let { block(it) } + ): Job = scope.launch { flow().observe() } /** * Static functionalities of the [BrowserToolbarMiddleware]. diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/CustomTabToolbarEnvironment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/CustomTabToolbarEnvironment.kt @@ -1,28 +0,0 @@ -/* 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.components.toolbar - -import android.content.Context -import androidx.lifecycle.LifecycleOwner -import androidx.navigation.NavController -import mozilla.components.compose.browser.toolbar.store.Environment - -/** - * The current environment in which the browser toolbar is used allowing access to various - * other application features that the toolbar integrates with. - * - * This is Activity/Fragment lifecycle dependent and should be handled carefully to avoid memory leaks. - * - * @property context [Context] to access application resources and interact with other system functionalities. - * @property viewLifecycleOwner [LifecycleOwner] depending on which lifecycle related operations will be scheduled. - * @property navController [NavController] to use for navigating to other in-app destinations. - * @property closeTabDelegate Callback for when the current custom tab needs to be closed. - */ -data class CustomTabToolbarEnvironment( - val context: Context, - val viewLifecycleOwner: LifecycleOwner, - val navController: NavController, - val closeTabDelegate: () -> Unit, -) : Environment diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/listscreen/store/DownloadUIStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/downloads/listscreen/store/DownloadUIStore.kt @@ -29,7 +29,7 @@ class DownloadUIStore( * The DownloadState Reducer. */ @Suppress("LongMethod") -fun downloadStateReducer( +private fun downloadStateReducer( state: DownloadUIState, action: DownloadUIAction, ): DownloadUIState { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -599,7 +599,7 @@ class HomeFragment : Fragment() { private fun buildToolbar(activity: HomeActivity): FenixHomeToolbar = when (activity.settings().shouldUseComposableToolbar) { true -> { - val toolbarStore = buildToolbarStore(activity) + val toolbarStore by buildToolbarStore(activity) homeNavigationBar = HomeNavigationBar( context = activity, @@ -1385,13 +1385,13 @@ class HomeFragment : Fragment() { ) = context?.let { AwesomeBarComposable( activity = requireActivity() as HomeActivity, + fragment = this, modifier = modifier, components = requireComponents, appStore = requireComponents.appStore, browserStore = requireComponents.core.store, toolbarStore = toolbarStore, navController = findNavController(), - lifecycleOwner = this, tabId = args.sessionToStartSearchFor, searchAccessPoint = args.searchAccessPoint, ).also { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/store/HomeToolbarStoreBuilder.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/store/HomeToolbarStoreBuilder.kt @@ -6,19 +6,19 @@ package org.mozilla.fenix.home.store import android.content.Context import androidx.fragment.app.Fragment -import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.coroutineScope import androidx.navigation.NavController import mozilla.components.browser.state.store.BrowserStore import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.AppStore -import org.mozilla.fenix.components.StoreProvider -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.isTallWindow +import org.mozilla.fenix.ext.isWideWindow +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.toolbar.BrowserToolbarMiddleware import org.mozilla.fenix.home.toolbar.BrowserToolbarTelemetryMiddleware import org.mozilla.fenix.search.BrowserToolbarSearchMiddleware @@ -45,44 +45,42 @@ object HomeToolbarStoreBuilder { appStore: AppStore, browserStore: BrowserStore, browsingModeManager: BrowsingModeManager, - ) = StoreProvider.get(fragment) { + ) = fragment.fragmentStore(BrowserToolbarState()) { + val lifecycleScope = fragment.viewLifecycleOwner.lifecycle.coroutineScope + BrowserToolbarStore( - initialState = BrowserToolbarState(), + initialState = it, middleware = listOf( - BrowserToolbarSearchStatusSyncMiddleware(appStore), + BrowserToolbarSearchStatusSyncMiddleware( + appStore = appStore, + browsingModeManager = browsingModeManager, + scope = lifecycleScope, + ), BrowserToolbarMiddleware( + uiContext = context, appStore = appStore, browserStore = browserStore, clipboard = context.components.clipboardHandler, useCases = context.components.useCases, + navController = navController, + browsingModeManager = browsingModeManager, + settings = context.settings(), + isWideScreen = { fragment.isWideWindow() }, + isTallScreen = { fragment.isTallWindow() }, + scope = lifecycleScope, ), BrowserToolbarSearchMiddleware( + uiContext = context, appStore = appStore, browserStore = browserStore, components = context.components, - settings = context.components.settings, - ), - BrowserToolbarTelemetryMiddleware(), - ), - ) - }.also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - context = context, - fragment = fragment, navController = navController, browsingModeManager = browsingModeManager, + settings = context.components.settings, + scope = lifecycleScope, ), + BrowserToolbarTelemetryMiddleware(), ), ) - - fragment.viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(EnvironmentCleared) - } - }, - ) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddleware.kt @@ -4,10 +4,10 @@ package org.mozilla.fenix.home.toolbar +import android.content.Context import androidx.annotation.VisibleForTesting -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import androidx.navigation.NavController +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChangedBy @@ -42,8 +42,6 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.B import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuButton.Icon.DrawableResIcon import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuButton.Text.StringResText import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.Mode import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.lib.state.Middleware @@ -59,16 +57,13 @@ import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Normal import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Private +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.UseCases import org.mozilla.fenix.components.appstate.AppAction.SearchAction.SearchStarted import org.mozilla.fenix.components.appstate.SupportedMenuNotifications import org.mozilla.fenix.components.menu.MenuAccessPoint -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment -import org.mozilla.fenix.ext.isTallWindow -import org.mozilla.fenix.ext.isWideWindow import org.mozilla.fenix.ext.nav -import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.HomeFragmentDirections import org.mozilla.fenix.home.toolbar.DisplayActions.FakeClicked import org.mozilla.fenix.home.toolbar.DisplayActions.MenuClicked @@ -81,6 +76,7 @@ import org.mozilla.fenix.search.BrowserToolbarSearchMiddleware import org.mozilla.fenix.search.ext.searchEngineShortcuts import org.mozilla.fenix.settings.ShortcutType import org.mozilla.fenix.tabstray.Page +import org.mozilla.fenix.utils.Settings import mozilla.components.lib.state.Action as MVIAction import mozilla.components.ui.icons.R as iconsR import mozilla.components.ui.tabcounter.R as tabcounterR @@ -105,19 +101,32 @@ internal sealed class PageOriginInteractions : BrowserToolbarEvent { /** * [Middleware] responsible for configuring and handling interactions with the composable toolbar. * + * @param uiContext [Context] used for various system interactions. * @param appStore [AppStore] to sync from. * @param browserStore [BrowserStore] to sync from. * @param clipboard [ClipboardHandler] to use for reading from device's clipboard. * @param useCases [UseCases] helping this integrate with other features of the applications. + * @param navController [NavController] to use for navigating to other in-app destinations. + * @param browsingModeManager [BrowsingModeManager] for querying the current browsing mode. + * @param settings [Settings] for accessing application settings. + * @param isWideScreen Callback for checking if the screen is wide. + * @param isTallScreen Callback for checking if the screen is tall. + * @param scope [CoroutineScope] used for running long running operations in background. */ +@Suppress("LongParameterList") class BrowserToolbarMiddleware( + private val uiContext: Context, private val appStore: AppStore, private val browserStore: BrowserStore, private val clipboard: ClipboardHandler, private val useCases: UseCases, + private val navController: NavController, + private val browsingModeManager: BrowsingModeManager, + private val settings: Settings, + private val isWideScreen: () -> Boolean, + private val isTallScreen: () -> Boolean, + private val scope: CoroutineScope, ) : Middleware<BrowserToolbarState, BrowserToolbarAction> { - @VisibleForTesting - internal var environment: BrowserToolbarEnvironment? = null private var syncCurrentSearchEngineJob: Job? = null private var observeBrowserSearchStateJob: Job? = null @@ -131,17 +140,11 @@ class BrowserToolbarMiddleware( is Init -> { next(action) - updatePageOrigin(context) - } - - is EnvironmentRehydrated -> { - next(action) - - environment = action.environment as? BrowserToolbarEnvironment - if (context.state.mode == Mode.DISPLAY) { observeSearchStateUpdates(context) } + + updatePageOrigin(context) updateEndBrowserActions(context) updateNavigationActions(context) updateToolbarActionsBasedOnOrientation(context) @@ -149,12 +152,6 @@ class BrowserToolbarMiddleware( updateMenuHighlight(context) } - is EnvironmentCleared -> { - next(action) - - environment = null - } - is EnterEditMode -> { next(action) @@ -168,40 +165,36 @@ class BrowserToolbarMiddleware( } is MenuClicked -> { - runWithinEnvironment { - navController.nav( - R.id.homeFragment, - HomeFragmentDirections.actionGlobalMenuDialogFragment( - accesspoint = MenuAccessPoint.Home, - ), - ) - } + navController.nav( + R.id.homeFragment, + HomeFragmentDirections.actionGlobalMenuDialogFragment( + accesspoint = MenuAccessPoint.Home, + ), + ) next(action) } is TabCounterClicked -> { - runWithinEnvironment { - if (this.context.settings().tabManagerEnhancementsEnabled) { - navController.nav( - R.id.homeFragment, - NavGraphDirections.actionGlobalTabManagementFragment( - page = when (browsingModeManager.mode) { - Normal -> Page.NormalTabs - Private -> Page.PrivateTabs - }, - ), - ) - } else { - navController.nav( - R.id.homeFragment, - NavGraphDirections.actionGlobalTabsTrayFragment( - page = when (browsingModeManager.mode) { - Normal -> Page.NormalTabs - Private -> Page.PrivateTabs - }, - ), - ) - } + if (settings.tabManagerEnhancementsEnabled) { + navController.nav( + R.id.homeFragment, + NavGraphDirections.actionGlobalTabManagementFragment( + page = when (browsingModeManager.mode) { + Normal -> Page.NormalTabs + Private -> Page.PrivateTabs + }, + ), + ) + } else { + navController.nav( + R.id.homeFragment, + NavGraphDirections.actionGlobalTabsTrayFragment( + page = when (browsingModeManager.mode) { + Normal -> Page.NormalTabs + Private -> Page.PrivateTabs + }, + ), + ) } next(action) } @@ -222,18 +215,16 @@ class BrowserToolbarMiddleware( openNewTab(context, searchTerms = clipboard.text) } is LoadFromClipboardClicked -> { - runWithinEnvironment { - clipboard.extractURL()?.let { - useCases.fenixBrowserUseCases.loadUrlOrSearch( - searchTermOrURL = it, - newTab = true, - private = browsingModeManager.mode == Private, - searchEngine = reconcileSelectedEngine(), - ) - navController.navigate(R.id.browserFragment) - } ?: run { - Logger("HomeOriginContextMenu").error("Clipboard contains URL but unable to read text") - } + clipboard.extractURL()?.let { + useCases.fenixBrowserUseCases.loadUrlOrSearch( + searchTermOrURL = it, + newTab = true, + private = browsingModeManager.mode == Private, + searchEngine = reconcileSelectedEngine(), + ) + navController.navigate(R.id.browserFragment) + } ?: run { + Logger("HomeOriginContextMenu").error("Clipboard contains URL but unable to read text") } } @@ -246,11 +237,9 @@ class BrowserToolbarMiddleware( browsingMode: BrowsingMode? = null, searchTerms: String? = null, ) { - runWithinEnvironment { - browsingMode?.let { browsingModeManager.mode = it } - context.dispatch(SearchQueryUpdated(BrowserToolbarQuery(searchTerms ?: ""))) - appStore.dispatch(SearchStarted()) - } + browsingMode?.let { browsingModeManager.mode = it } + context.dispatch(SearchQueryUpdated(BrowserToolbarQuery(searchTerms ?: ""))) + appStore.dispatch(SearchStarted()) } private fun observeSearchStateUpdates(context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>) { @@ -312,21 +301,19 @@ class BrowserToolbarMiddleware( } private fun buildStartPageActions(selectedSearchEngine: SearchEngine?): List<Action> { - val environment = environment ?: return emptyList() - return listOfNotNull( BrowserToolbarSearchMiddleware.buildSearchSelector( selectedSearchEngine = selectedSearchEngine, searchEngineShortcuts = browserStore.state.search.searchEngineShortcuts, - resources = environment.context.resources, + resources = uiContext.resources, ), ) } private fun buildEndBrowserActions(): List<Action> { - val isWideWindow = environment?.fragment?.isWideWindow() == true - val isTallWindow = environment?.fragment?.isTallWindow() == true - val shouldUseExpandedToolbar = environment?.context?.settings()?.shouldUseExpandedToolbar == true + val isWideWindow = isWideScreen() + val isTallWindow = isTallScreen() + val shouldUseExpandedToolbar = settings.shouldUseExpandedToolbar return listOf( HomeToolbarActionConfig(HomeToolbarAction.TabCounter) { @@ -363,10 +350,8 @@ class BrowserToolbarMiddleware( * - The toolbar redesign customization option is also hidden. */ private fun buildNavigationActions(): List<Action> { - val environment = environment ?: return emptyList() - val settings = environment.context.settings() - val isWideWindow = environment.fragment.isWideWindow() - val isTallWindow = environment.fragment.isTallWindow() + val isWideWindow = isWideScreen() + val isTallWindow = isTallScreen() val shouldUseExpandedToolbar = settings.shouldUseExpandedToolbar val useCustomPrimary = settings.shouldShowToolbarCustomization && shouldUseExpandedToolbar val primarySlotAction = ShortcutType.fromValue(settings.toolbarExpandedShortcutKey) @@ -396,8 +381,10 @@ class BrowserToolbarMiddleware( } private fun buildTabCounterMenu(source: Source): CombinedEventAndMenu? { + val currentBrowsingMode = browsingModeManager.mode + return CombinedEventAndMenu(TabCounterLongClicked(source)) { - when (environment?.browsingModeManager?.mode) { + when (currentBrowsingMode) { Private -> listOf( BrowserToolbarMenuButton( icon = DrawableResIcon(iconsR.drawable.mozac_ic_plus_24), @@ -457,17 +444,7 @@ class BrowserToolbarMiddleware( private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.fragment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } - - private inline fun runWithinEnvironment( - block: BrowserToolbarEnvironment.() -> Unit, - ) = environment?.let { block(it) } + ): Job = scope.launch { flow().observe() } private fun reconcileSelectedEngine(): SearchEngine? = appStore.state.searchState.selectedSearchEngine?.searchEngine @@ -496,14 +473,13 @@ class BrowserToolbarMiddleware( source: Source = Source.AddressBar, ): Action = when (action) { HomeToolbarAction.TabCounter -> { - val environment = requireNotNull(environment) - val isInPrivateMode = environment.browsingModeManager.mode.isPrivate + val isInPrivateMode = browsingModeManager.mode.isPrivate val tabsCount = browserStore.state.getNormalOrPrivateTabs(isInPrivateMode).size val tabCounterDescription = if (isInPrivateMode) { - environment.context.getString(tabcounterR.string.mozac_tab_counter_private, tabsCount.toString()) + uiContext.getString(tabcounterR.string.mozac_tab_counter_private, tabsCount.toString()) } else { - environment.context.getString(tabcounterR.string.mozac_tab_counter_open_tab_tray, tabsCount.toString()) + uiContext.getString(tabcounterR.string.mozac_tab_counter_open_tab_tray, tabsCount.toString()) } TabCounterAction( @@ -542,12 +518,12 @@ class BrowserToolbarMiddleware( HomeToolbarAction.NewTab -> ActionButtonRes( drawableResId = iconsR.drawable.mozac_ic_plus_24, - contentDescription = if (environment?.browsingModeManager?.mode == Private) { + contentDescription = if (browsingModeManager.mode == Private) { R.string.home_screen_shortcut_open_new_private_tab_2 } else { R.string.home_screen_shortcut_open_new_tab_2 }, - onClick = if (environment?.browsingModeManager?.mode == Private) { + onClick = if (browsingModeManager.mode == Private) { AddNewPrivateTab(source) } else { AddNewTab(source) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -47,9 +47,8 @@ import androidx.core.view.MenuProvider import androidx.core.view.isVisible import androidx.core.view.updateLayoutParams import androidx.fragment.app.DialogFragment -import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.coroutineScope import androidx.lifecycle.lifecycleScope import androidx.navigation.NavDirections import androidx.navigation.NavOptions @@ -81,14 +80,13 @@ import mozilla.components.compose.browser.toolbar.BrowserToolbar import mozilla.components.compose.browser.toolbar.store.BrowserEditToolbarAction import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.Mode import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.lib.state.ext.flowScoped import mozilla.components.lib.state.ext.observeAsComposableState +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import mozilla.components.support.ktx.android.view.hideKeyboard @@ -108,7 +106,6 @@ import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.history.DefaultPagedHistoryProvider import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.components.search.HISTORY_SEARCH_ENGINE_ID -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.databinding.FragmentHistoryBinding import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView @@ -131,7 +128,6 @@ import org.mozilla.fenix.search.BrowserToolbarSearchMiddleware import org.mozilla.fenix.search.BrowserToolbarSearchStatusSyncMiddleware import org.mozilla.fenix.search.BrowserToolbarToFenixSearchMapperMiddleware import org.mozilla.fenix.search.FenixSearchMiddleware -import org.mozilla.fenix.search.SearchFragmentAction import org.mozilla.fenix.search.SearchFragmentAction.SuggestionClicked import org.mozilla.fenix.search.SearchFragmentAction.SuggestionSelected import org.mozilla.fenix.search.SearchFragmentStore @@ -146,8 +142,8 @@ private const val MATERIAL_DESIGN_SCRIM = "#52000000" @SuppressWarnings("TooManyFunctions", "LargeClass") class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler, MenuProvider { private lateinit var historyStore: HistoryFragmentStore - private lateinit var toolbarStore: BrowserToolbarStore - private val searchStore by lazy { buildSearchStore(toolbarStore) } + private lateinit var searchStore: SearchFragmentStore + private val toolbarStore by buildToolbarStore() private lateinit var historyProvider: DefaultPagedHistoryProvider @@ -210,6 +206,8 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler, ), ) } + searchStore = buildSearchStore(toolbarStore).value + _historyView = HistoryView( container = binding.historyLayout, onZeroItemsLoaded = { @@ -295,7 +293,6 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler, requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED) if (requireContext().settings().shouldUseComposableToolbar) { - toolbarStore = buildToolbarStore() qrScanFenixFeature = QrScanFenixFeature.register(this, qrScanLauncher) voiceSearchFeature = VoiceSearchFeature.register(this, voiceSearchLauncher) } @@ -790,56 +787,61 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler, }.create().withCenterAlignedButtons() } - private fun buildToolbarStore() = StoreProvider.get(this) { + private fun buildToolbarStore() = fragmentStore( + BrowserToolbarState(mode = Mode.EDIT), + ) { + val lifecycleScope = viewLifecycleOwner.lifecycle.coroutineScope + BrowserToolbarStore( - initialState = BrowserToolbarState(mode = Mode.EDIT), + initialState = it, middleware = listOf( BrowserToolbarSyncToHistoryMiddleware(historyStore), - BrowserToolbarSearchStatusSyncMiddleware(requireComponents.appStore), + BrowserToolbarSearchStatusSyncMiddleware( + appStore = requireComponents.appStore, + browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, + scope = lifecycleScope, + ), BrowserToolbarSearchMiddleware( + uiContext = requireActivity(), appStore = requireComponents.appStore, browserStore = requireComponents.core.store, components = requireComponents, - settings = requireComponents.settings, - ), - ), - ) - }.also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - context = requireContext(), - fragment = this, navController = findNavController(), browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, + settings = requireComponents.settings, + scope = lifecycleScope, ), ), ) - - viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(EnvironmentCleared) - } - }, - ) } private fun buildSearchStore( toolbarStore: BrowserToolbarStore, - ) = StoreProvider.get(this) { + ) = fragmentStore( + createInitialSearchFragmentState( + activity = requireActivity() as HomeActivity, + components = requireComponents, + tabId = null, + pastedText = null, + searchAccessPoint = MetricsUtils.Source.NONE, + ), + ) { + val lifecycleScope = viewLifecycleOwner.lifecycle.coroutineScope + SearchFragmentStore( - initialState = createInitialSearchFragmentState( - activity = requireActivity() as HomeActivity, - components = requireComponents, - tabId = null, - pastedText = null, - searchAccessPoint = MetricsUtils.Source.NONE, - ), + initialState = it, middleware = listOf( - BrowserToolbarToFenixSearchMapperMiddleware(toolbarStore), - BrowserStoreToFenixSearchMapperMiddleware(requireComponents.core.store), + BrowserToolbarToFenixSearchMapperMiddleware( + toolbarStore = toolbarStore, + browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, + scope = lifecycleScope, + ), + BrowserStoreToFenixSearchMapperMiddleware( + browserStore = requireComponents.core.store, + scope = lifecycleScope, + ), FenixSearchMiddleware( + fragment = this@HistoryFragment, engine = requireComponents.core.engine, useCases = requireComponents.useCases, nimbusComponents = requireComponents.nimbus, @@ -847,28 +849,11 @@ class HistoryFragment : LibraryPageFragment<History>(), UserInteractionHandler, appStore = requireComponents.appStore, browserStore = requireComponents.core.store, toolbarStore = toolbarStore, - ), - ), - ) - }.also { - it.dispatch( - SearchFragmentAction.EnvironmentRehydrated( - SearchFragmentStore.Environment( - context = requireContext(), - viewLifecycleOwner = viewLifecycleOwner, - browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, navController = findNavController(), + browsingModeManager = (requireActivity() as HomeActivity).browsingModeManager, ), ), ) - - viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(SearchFragmentAction.EnvironmentCleared) - } - }, - ) } companion object { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserStoreToFenixSearchMapperMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserStoreToFenixSearchMapperMiddleware.kt @@ -5,9 +5,7 @@ package org.mozilla.fenix.search import androidx.annotation.VisibleForTesting -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged @@ -19,8 +17,7 @@ import mozilla.components.lib.state.MiddlewareContext import mozilla.components.lib.state.State import mozilla.components.lib.state.Store import mozilla.components.lib.state.ext.flow -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentCleared -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentRehydrated +import org.mozilla.fenix.search.SearchFragmentAction.Init import org.mozilla.fenix.search.SearchFragmentAction.UpdateSearchState import org.mozilla.fenix.search.SearchFragmentStore.Environment import mozilla.components.lib.state.Action as MVIAction @@ -29,9 +26,11 @@ import mozilla.components.lib.state.Action as MVIAction * [SearchFragmentStore] [Middleware] to synchronize search related details from [BrowserStore]. * * @param browserStore The [BrowserStore] to sync from. + * @param scope [CoroutineScope] used for running long running operations in background. */ class BrowserStoreToFenixSearchMapperMiddleware( private val browserStore: BrowserStore, + private val scope: CoroutineScope, ) : Middleware<SearchFragmentState, SearchFragmentAction> { @VisibleForTesting internal var environment: Environment? = null @@ -44,13 +43,8 @@ class BrowserStoreToFenixSearchMapperMiddleware( ) { next(action) - if (action is EnvironmentRehydrated) { - environment = action.environment - + if (action is Init) { observeBrowserSearchState(context) - } else if (action is EnvironmentCleared) { - observeBrowserSearchStateJob?.cancel() - environment = null } } @@ -68,11 +62,5 @@ class BrowserStoreToFenixSearchMapperMiddleware( private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } + ): Job = scope.launch { flow().observe() } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserToolbarSearchMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserToolbarSearchMiddleware.kt @@ -4,16 +4,15 @@ package org.mozilla.fenix.search +import android.content.Context import android.content.Intent import android.content.res.Resources import android.speech.RecognizerIntent import androidx.annotation.VisibleForTesting import androidx.core.graphics.drawable.toDrawable -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle import androidx.navigation.NavController import kotlinx.coroutines.CoroutineExceptionHandler +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher @@ -44,6 +43,7 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.CommitUrl import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.EnterEditMode import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.ExitEditMode +import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.Init import mozilla.components.compose.browser.toolbar.store.BrowserToolbarInteraction.BrowserToolbarEvent import mozilla.components.compose.browser.toolbar.store.BrowserToolbarInteraction.BrowserToolbarMenu import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem @@ -51,8 +51,6 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.B import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuDivider import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.toolbar.AutocompleteProvider @@ -70,7 +68,7 @@ import org.mozilla.fenix.GleanMetrics.Toolbar import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R import org.mozilla.fenix.browser.BrowserFragmentDirections -import org.mozilla.fenix.browser.browsingmode.BrowsingMode.Private +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.appstate.AppAction @@ -84,7 +82,6 @@ import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.components.search.BOOKMARKS_SEARCH_ENGINE_ID import org.mozilla.fenix.components.search.HISTORY_SEARCH_ENGINE_ID import org.mozilla.fenix.components.search.TABS_SEARCH_ENGINE_ID -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.ext.toolbarHintRes import org.mozilla.fenix.search.EditPageEndActionsInteractions.ClearSearchClicked import org.mozilla.fenix.search.EditPageEndActionsInteractions.QrScannerClicked @@ -138,21 +135,28 @@ internal sealed class EditPageEndActionsInteractions : BrowserToolbarEvent { * [BrowserToolbarStore] middleware handling the configuration of the composable toolbar * while in edit mode. * + * @param uiContext [Context] used for various system interactions. * @param appStore [AppStore] used for querying and updating application state. * @param browserStore [BrowserStore] used for querying and updating browser state. * @param components [Components] for accessing other functionalities of the application. + * @param navController [NavController] to use for navigating to other in-app destinations. + * @param browsingModeManager [BrowsingModeManager] for querying the current browsing mode. * @param settings [Settings] for accessing application settings. + * @param scope [CoroutineScope] used for running long running operations in background. * @param autocompleteDispatcher [CoroutineContext] used for querying autocomplete suggestions. */ +@Suppress("LongParameterList") class BrowserToolbarSearchMiddleware( + private val uiContext: Context, private val appStore: AppStore, private val browserStore: BrowserStore, private val components: Components, + private val navController: NavController, + private val browsingModeManager: BrowsingModeManager, private val settings: Settings, + private val scope: CoroutineScope, private val autocompleteDispatcher: CoroutineContext = defaultAutocompleteDispatcher, ) : Middleware<BrowserToolbarState, BrowserToolbarAction> { - @VisibleForTesting - internal var environment: BrowserToolbarEnvironment? = null private var syncCurrentSearchEngineJob: Job? = null private var syncAvailableSearchEnginesJob: Job? = null private var observeQRScannerInputJob: Job? = null @@ -170,18 +174,12 @@ class BrowserToolbarSearchMiddleware( } when (action) { - is EnvironmentRehydrated -> { - environment = action.environment as? BrowserToolbarEnvironment - + is Init -> { if (context.state.isEditMode()) { syncCurrentSearchEngine(context) } } - is EnvironmentCleared -> { - environment = null - } - is EnterEditMode -> { refreshConfigurationAfterSearchEngineChange( context = context, @@ -217,7 +215,7 @@ class BrowserToolbarSearchMiddleware( context.dispatch(SearchQueryUpdated(BrowserToolbarQuery(""))) appStore.dispatch(SearchEnded) browserStore.dispatch(EngagementFinished(abandoned = true)) - environment?.navController?.navigate( + navController.navigate( BrowserFragmentDirections.actionGlobalSearchEngineFragment(), ) } @@ -235,8 +233,6 @@ class BrowserToolbarSearchMiddleware( return } - val navController = environment?.navController ?: return - when (action.text) { "about:crashes" -> { // The list of past crashes can be accessed via "settings > about", but desktop and @@ -322,7 +318,7 @@ class BrowserToolbarSearchMiddleware( searchTermOrURL = text, newTab = newTab, forceSearch = !isDefaultEngine, - private = environment?.browsingModeManager?.mode == Private, + private = browsingModeManager.mode.isPrivate, searchEngine = searchEngine, ) @@ -371,10 +367,8 @@ class BrowserToolbarSearchMiddleware( selectedSearchEngine: SearchEngine?, searchEngineShortcuts: List<SearchEngine>, ) { - val environment = environment ?: return - val searchSelector = buildSearchSelector( - selectedSearchEngine, searchEngineShortcuts, environment.context.resources, + selectedSearchEngine, searchEngineShortcuts, uiContext.resources, ) context.dispatch( SearchActionsStartUpdated( @@ -426,7 +420,7 @@ class BrowserToolbarSearchMiddleware( val isBackspacing = query.previous?.startsWith(query.current) == true && query.previous?.length == query.current.length + 1 if (shouldCheckForSuggestions && !isBackspacing) { - updateAutocompleteJob = environment?.fragment?.viewLifecycleOwner?.lifecycleScope?.launch { + updateAutocompleteJob = scope.launch { context.dispatch( BrowserEditToolbarAction.AutocompleteSuggestionUpdated( withContext(autocompleteDispatcher) { @@ -551,9 +545,9 @@ class BrowserToolbarSearchMiddleware( searchTermOrURL = it.qrScannerState.lastScanData, newTab = appStore.state.searchState.sourceTabId == null, flags = EngineSession.LoadUrlFlags.external(), - private = environment?.browsingModeManager?.mode == Private, + private = browsingModeManager.mode.isPrivate, ) - environment?.navController?.navigate(R.id.action_global_browser) + navController.navigate(R.id.action_global_browser) } } } @@ -581,20 +575,13 @@ class BrowserToolbarSearchMiddleware( } @VisibleForTesting - internal fun isSpeechRecognitionAvailable() = environment?.context?.let { + internal fun isSpeechRecognitionAvailable() = Intent(RecognizerIntent.ACTION_RECOGNIZE_SPEECH) - .resolveActivity(it.packageManager) != null - } ?: false + .resolveActivity(uiContext.packageManager) != null private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.fragment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } + ): Job = scope.launch { flow().observe() } /** * Static functionalities of the [BrowserToolbarSearchMiddleware]. diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserToolbarSearchStatusSyncMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserToolbarSearchStatusSyncMiddleware.kt @@ -4,42 +4,36 @@ package org.mozilla.fenix.search -import androidx.annotation.VisibleForTesting -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.launch import mozilla.components.compose.browser.toolbar.store.BrowserEditToolbarAction.PrivateModeUpdated import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.EnterEditMode import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.ExitEditMode +import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.Init import mozilla.components.compose.browser.toolbar.store.BrowserToolbarState import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.MiddlewareContext -import mozilla.components.lib.state.State -import mozilla.components.lib.state.Store import mozilla.components.lib.state.ext.flow +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction.SearchAction.SearchEnded -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment -import mozilla.components.lib.state.Action as MVIAction /** * [Middleware] for synchronizing whether a search is active between [BrowserToolbarStore] and [AppStore]. * * @param appStore [AppStore] through which the toolbar updates can be integrated with other application features. + * @param browsingModeManager [BrowsingModeManager] for querying the current browsing mode. + * @param scope [CoroutineScope] used for running long running operations in background. */ class BrowserToolbarSearchStatusSyncMiddleware( private val appStore: AppStore, + private val browsingModeManager: BrowsingModeManager, + private val scope: CoroutineScope, ) : Middleware<BrowserToolbarState, BrowserToolbarAction> { - @VisibleForTesting - internal var environment: BrowserToolbarEnvironment? = null private var syncSearchActiveJob: Job? = null override fun invoke( @@ -49,14 +43,9 @@ class BrowserToolbarSearchStatusSyncMiddleware( ) { next(action) - if (action is EnvironmentRehydrated) { - environment = action.environment as? BrowserToolbarEnvironment + if (action is Init) { syncSearchActive(context) } - if (action is EnvironmentCleared) { - syncSearchActiveJob?.cancel() - environment = null - } if (action is ExitEditMode) { // Only support the toolbar triggering exiting search mode in the application. @@ -67,12 +56,13 @@ class BrowserToolbarSearchStatusSyncMiddleware( } private fun syncSearchActive(context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>) { - syncSearchActiveJob = appStore.observeWhileActive { - distinctUntilChangedBy { it.searchState.isSearchActive } + syncSearchActiveJob = scope.launch { + appStore.flow() + .distinctUntilChangedBy { it.searchState.isSearchActive } .collect { if (it.searchState.isSearchActive) { context.dispatch( - PrivateModeUpdated(environment?.browsingModeManager?.mode?.isPrivate == true), + PrivateModeUpdated(browsingModeManager.mode.isPrivate), ) context.dispatch(EnterEditMode) } else { @@ -81,14 +71,4 @@ class BrowserToolbarSearchStatusSyncMiddleware( } } } - - private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( - crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.fragment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserToolbarToFenixSearchMapperMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/BrowserToolbarToFenixSearchMapperMiddleware.kt @@ -4,12 +4,8 @@ package org.mozilla.fenix.search -import androidx.annotation.VisibleForTesting -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.flow.map @@ -20,27 +16,25 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore import mozilla.components.compose.browser.toolbar.store.Mode import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.MiddlewareContext -import mozilla.components.lib.state.State -import mozilla.components.lib.state.Store import mozilla.components.lib.state.ext.flow -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentCleared -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentRehydrated +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager +import org.mozilla.fenix.search.SearchFragmentAction.Init import org.mozilla.fenix.search.SearchFragmentAction.SearchStarted -import org.mozilla.fenix.search.SearchFragmentStore.Environment -import mozilla.components.lib.state.Action as MVIAction /** * [SearchFragmentStore] [Middleware] to synchronize search related details from [BrowserToolbarStore]. * * @param toolbarStore The [BrowserToolbarStore] to sync from. + * @param browsingModeManager [BrowsingModeManager] for querying the current browsing mode. + * @param scope [CoroutineScope] used for running long running operations in background. * @param browserStore The [BrowserStore] to sync from. */ class BrowserToolbarToFenixSearchMapperMiddleware( private val toolbarStore: BrowserToolbarStore, + private val browsingModeManager: BrowsingModeManager, + private val scope: CoroutineScope, private val browserStore: BrowserStore? = null, ) : Middleware<SearchFragmentState, SearchFragmentAction> { - @VisibleForTesting - internal var environment: Environment? = null private var syncSearchStartedJob: Job? = null private var syncSearchQueryJob: Job? = null @@ -49,16 +43,12 @@ class BrowserToolbarToFenixSearchMapperMiddleware( next: (SearchFragmentAction) -> Unit, action: SearchFragmentAction, ) { - if (action is EnvironmentRehydrated) { - environment = action.environment - + if (action is Init) { syncSearchStatus(context) if (toolbarStore.state.isEditMode()) { syncUserQuery(context) } - } else if (action is EnvironmentCleared) { - environment = null } next(action) @@ -66,8 +56,9 @@ class BrowserToolbarToFenixSearchMapperMiddleware( private fun syncSearchStatus(context: MiddlewareContext<SearchFragmentState, SearchFragmentAction>) { syncSearchStartedJob?.cancel() - syncSearchStartedJob = toolbarStore.observeWhileActive { - distinctUntilChangedBy { it.mode } + syncSearchStartedJob = scope.launch { + toolbarStore.flow() + .distinctUntilChangedBy { it.mode } .collect { if (it.mode == Mode.EDIT) { val editState = toolbarStore.state.editState @@ -75,7 +66,7 @@ class BrowserToolbarToFenixSearchMapperMiddleware( SearchStarted( selectedSearchEngine = null, isUserSelected = true, - inPrivateMode = environment?.browsingModeManager?.mode?.isPrivate == true, + inPrivateMode = browsingModeManager.mode.isPrivate, searchStartedForCurrentUrl = editState.isQueryPrefilled && browserStore?.state?.selectedTab?.content?.url == editState.query.current, ), @@ -91,8 +82,9 @@ class BrowserToolbarToFenixSearchMapperMiddleware( private fun syncUserQuery(context: MiddlewareContext<SearchFragmentState, SearchFragmentAction>) { syncSearchQueryJob?.cancel() - syncSearchQueryJob = toolbarStore.observeWhileActive { - map { it.editState.query } + syncSearchQueryJob = scope.launch { + toolbarStore.flow() + .map { it.editState.query } .distinctUntilChanged() .collect { query -> val isSearchStartedForCurrentUrl = context.state.searchStartedForCurrentUrl @@ -112,14 +104,4 @@ class BrowserToolbarToFenixSearchMapperMiddleware( private fun stopSyncingUserQuery() { syncSearchQueryJob?.cancel() } - - private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( - crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } - } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/FenixSearchMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/FenixSearchMiddleware.kt @@ -5,9 +5,11 @@ package org.mozilla.fenix.search import androidx.annotation.VisibleForTesting -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.lifecycleScope -import androidx.lifecycle.repeatOnLifecycle +import androidx.fragment.app.Fragment +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.coroutineScope +import androidx.navigation.NavController import kotlinx.coroutines.Job import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChangedBy @@ -37,6 +39,7 @@ import org.mozilla.fenix.GleanMetrics.History import org.mozilla.fenix.GleanMetrics.Toolbar import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.NimbusComponents import org.mozilla.fenix.components.UseCases @@ -49,8 +52,6 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.telemetryName import org.mozilla.fenix.nimbus.FxNimbus -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentCleared -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentRehydrated import org.mozilla.fenix.search.SearchFragmentAction.Init import org.mozilla.fenix.search.SearchFragmentAction.PrivateSuggestionsCardAccepted import org.mozilla.fenix.search.SearchFragmentAction.SearchEnginesSelectedActions @@ -60,7 +61,6 @@ import org.mozilla.fenix.search.SearchFragmentAction.SearchSuggestionsVisibility import org.mozilla.fenix.search.SearchFragmentAction.SuggestionClicked import org.mozilla.fenix.search.SearchFragmentAction.SuggestionSelected import org.mozilla.fenix.search.SearchFragmentAction.UpdateQuery -import org.mozilla.fenix.search.SearchFragmentStore.Environment import org.mozilla.fenix.search.awesomebar.DefaultSuggestionIconProvider import org.mozilla.fenix.search.awesomebar.DefaultSuggestionsStringsProvider import org.mozilla.fenix.search.awesomebar.SearchSuggestionsProvidersBuilder @@ -73,6 +73,7 @@ import mozilla.components.lib.state.Action as MVIAction /** * [SearchFragmentStore] [Middleware] that will handle the setup of the search UX and related user interactions. * + * @param fragment [Fragment] in which this middleware is used. Will observe it's lifecycle for cleanup. * @param engine [Engine] used for speculative connections to search suggestions URLs. * @param useCases [UseCases] helping this integrate with other features of the applications. * @param nimbusComponents [NimbusComponents] used for accessing Nimbus events to use in telemetry. @@ -80,9 +81,12 @@ import mozilla.components.lib.state.Action as MVIAction * @param appStore [AppStore] to sync search related data with. * @param browserStore [BrowserStore] to sync search related data with. * @param toolbarStore [BrowserToolbarStore] used for querying and updating the toolbar state. + * @param navController [NavController] to use for navigating to other in-app destinations. + * @param browsingModeManager [BrowsingModeManager] used for querying and updating the browsing mode. */ @Suppress("LongParameterList") class FenixSearchMiddleware( + private val fragment: Fragment, private val engine: Engine, private val useCases: UseCases, private val nimbusComponents: NimbusComponents, @@ -90,9 +94,9 @@ class FenixSearchMiddleware( private val appStore: AppStore, private val browserStore: BrowserStore, private val toolbarStore: BrowserToolbarStore, + private val navController: NavController, + private val browsingModeManager: BrowsingModeManager, ) : Middleware<SearchFragmentState, SearchFragmentAction> { - @VisibleForTesting - internal var environment: Environment? = null private var observeSearchEnginesChangeJob: Job? = null @VisibleForTesting @@ -103,12 +107,15 @@ class FenixSearchMiddleware( next: (SearchFragmentAction) -> Unit, action: SearchFragmentAction, ) { - if (handleEnvironmentUpdates(context, next, action)) return - when (action) { is Init -> { next(action) + suggestionsProvidersBuilder = buildSearchSuggestionsProvider(context) + updateSearchProviders(context) + + setupSuggestionsProvidersCleanup(context) + context.dispatch( SearchFragmentAction.UpdateSearchState( browserStore.state.search, @@ -176,38 +183,6 @@ class FenixSearchMiddleware( } } - private fun handleEnvironmentUpdates( - context: MiddlewareContext<SearchFragmentState, SearchFragmentAction>, - next: (SearchFragmentAction) -> Unit, - action: SearchFragmentAction, - ) = when (action) { - is EnvironmentRehydrated -> { - next(action) - - environment = action.environment - - suggestionsProvidersBuilder = buildSearchSuggestionsProvider(context) - updateSearchProviders(context) - - true - } - - is EnvironmentCleared -> { - next(action) - - environment = null - - // Search providers may keep hard references to lifecycle dependent objects - // so we need to reset them when the environment is cleared. - suggestionsProvidersBuilder = null - context.dispatch(SearchProvidersUpdated(emptyList())) - - true - } - - else -> false - } - /** * Observe when the user changes the search engine to use for the current in-progress search * and update the suggestions providers used and shown suggestions accordingly. @@ -268,7 +243,7 @@ class FenixSearchMiddleware( val showPrivatePrompt = with(context.state) { !settings.showSearchSuggestionsInPrivateOnboardingFinished && - environment?.browsingModeManager?.mode?.isPrivate == true && + browsingModeManager.mode.isPrivate && !isSearchSuggestionsFeatureEnabled() && !showSearchShortcuts && url != query } @@ -300,20 +275,20 @@ class FenixSearchMiddleware( internal fun buildSearchSuggestionsProvider( context: MiddlewareContext<SearchFragmentState, SearchFragmentAction>, ): SearchSuggestionsProvidersBuilder? { - val environment = environment ?: return null + val uiContext = fragment.context ?: return null return SearchSuggestionsProvidersBuilder( - components = environment.context.components, - browsingModeManager = environment.browsingModeManager, + components = uiContext.components, + browsingModeManager = browsingModeManager, includeSelectedTab = context.state.tabId == null, loadUrlUseCase = loadUrlUseCase(context), searchUseCase = searchUseCase(context), selectTabUseCase = selectTabUseCase(), suggestionsStringsProvider = DefaultSuggestionsStringsProvider( - environment.context, - DefaultSearchEngineProvider(environment.context.components.core.store), + uiContext, + DefaultSearchEngineProvider(uiContext.components.core.store), ), - suggestionIconProvider = DefaultSuggestionIconProvider(environment.context), + suggestionIconProvider = DefaultSuggestionIconProvider(uiContext), onSearchEngineShortcutSelected = ::handleSearchEngineSuggestionClicked, onSearchEngineSuggestionSelected = ::handleSearchEngineSuggestionClicked, onSearchEngineSettingsClicked = { handleClickSearchEngineSettings() }, @@ -337,7 +312,7 @@ class FenixSearchMiddleware( } else { context.state.tabId == null }, - usePrivateMode = environment?.browsingModeManager?.mode?.isPrivate == true, + usePrivateMode = browsingModeManager.mode.isPrivate, flags = flags, ) @@ -365,7 +340,7 @@ class FenixSearchMiddleware( } else { context.state.tabId == null }, - usePrivateMode = environment?.browsingModeManager?.mode?.isPrivate == true, + usePrivateMode = browsingModeManager.mode.isPrivate, forceSearch = true, searchEngine = searchEngine, ) @@ -393,7 +368,7 @@ class FenixSearchMiddleware( override fun invoke(tabId: String) { useCases.tabsUseCases.selectTab(tabId) - environment?.navController?.navigate(R.id.browserFragment) + navController.navigate(R.id.browserFragment) browserStore.dispatch(AwesomeBarAction.EngagementFinished(abandoned = false)) } @@ -407,7 +382,7 @@ class FenixSearchMiddleware( searchEngine: SearchEngine? = null, flags: LoadUrlFlags = LoadUrlFlags.none(), ) { - environment?.navController?.navigate(R.id.browserFragment) + navController.navigate(R.id.browserFragment) useCases.fenixBrowserUseCases.loadUrlOrSearch( searchTermOrURL = url, newTab = createNewTab, @@ -454,8 +429,6 @@ class FenixSearchMiddleware( context: MiddlewareContext<SearchFragmentState, SearchFragmentAction>, searchEngine: SearchEngine, ) { - val environment = environment ?: return - when { searchEngine.type == SearchEngine.Type.APPLICATION && searchEngine.id == HISTORY_SEARCH_ENGINE_ID -> { context.dispatch(SearchFragmentAction.SearchHistoryEngineSelected(searchEngine)) @@ -470,7 +443,7 @@ class FenixSearchMiddleware( context.dispatch( SearchFragmentAction.SearchDefaultEngineSelected( engine = searchEngine, - browsingMode = environment.browsingModeManager.mode, + browsingMode = browsingModeManager.mode, settings = settings, ), ) @@ -479,7 +452,7 @@ class FenixSearchMiddleware( context.dispatch( SearchFragmentAction.SearchShortcutEngineSelected( engine = searchEngine, - browsingMode = environment.browsingModeManager.mode, + browsingMode = browsingModeManager.mode, settings = settings, ), ) @@ -494,18 +467,26 @@ class FenixSearchMiddleware( @VisibleForTesting internal fun handleClickSearchEngineSettings() { val directions = SearchDialogFragmentDirections.actionGlobalSearchEngineFragment() - environment?.navController?.navigateSafe(R.id.searchDialogFragment, directions) + navController.navigateSafe(R.id.searchDialogFragment, directions) browserStore.dispatch(AwesomeBarAction.EngagementFinished(abandoned = true)) } private inline fun <S : State, A : MVIAction> Store<S, A>.observeWhileActive( crossinline observe: suspend (Flow<S>.() -> Unit), - ): Job? = environment?.viewLifecycleOwner?.run { - lifecycleScope.launch { - repeatOnLifecycle(RESUMED) { - flow().observe() - } - } + ): Job = fragment.viewLifecycleOwner.lifecycle.coroutineScope.launch { flow().observe() } + + private fun setupSuggestionsProvidersCleanup( + context: MiddlewareContext<SearchFragmentState, SearchFragmentAction>, + ) { + fragment.viewLifecycleOwner.lifecycle.addObserver( + object : DefaultLifecycleObserver { + override fun onDestroy(owner: LifecycleOwner) { + // Search providers may keep hard references to lifecycle dependent objects + // so we need to reset them when the environment is cleared. + context.dispatch(SearchProvidersUpdated(emptyList())) + } + }, + ) } /** @@ -515,9 +496,7 @@ class FenixSearchMiddleware( */ @VisibleForTesting internal fun isSearchSuggestionsFeatureEnabled(): Boolean { - val environment = environment ?: return false - - return when (environment.browsingModeManager.mode) { + return when (browsingModeManager.mode) { BrowsingMode.Normal -> settings.shouldShowSearchSuggestions BrowsingMode.Private -> settings.shouldShowSearchSuggestions && settings.shouldShowSearchSuggestionsInPrivate diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt @@ -26,7 +26,6 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.search.SearchFragmentAction.Init -import org.mozilla.fenix.search.SearchFragmentStore.Environment import org.mozilla.fenix.utils.Settings /** @@ -425,16 +424,6 @@ sealed class SearchFragmentAction : Action { ) : SearchFragmentAction() /** - * Signals a new valid [Environment] has been set. - */ - data class EnvironmentRehydrated(val environment: Environment) : SearchFragmentAction() - - /** - * Signals the current [Environment] is not valid anymore. - */ - data object EnvironmentCleared : SearchFragmentAction() - - /** * Action indicating the user allowed to show suggestions in private mode. */ data object PrivateSuggestionsCardAccepted : SearchFragmentAction() @@ -630,8 +619,6 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen state.copy(searchStartedForCurrentUrl = action.searchStartedForCurrentUrl) } - is SearchFragmentAction.EnvironmentRehydrated, - is SearchFragmentAction.EnvironmentCleared, is SearchFragmentAction.SuggestionClicked, is SearchFragmentAction.PrivateSuggestionsCardAccepted, is SearchFragmentAction.SuggestionSelected, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarComposable.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarComposable.kt @@ -26,8 +26,7 @@ import androidx.compose.ui.platform.LocalSoftwareKeyboardController import androidx.compose.ui.platform.LocalView import androidx.core.graphics.toColorInt import androidx.fragment.app.Fragment -import androidx.lifecycle.DefaultLifecycleObserver -import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.coroutineScope import androidx.navigation.NavController import mozilla.components.browser.state.action.AwesomeBarAction import mozilla.components.browser.state.store.BrowserStore @@ -37,12 +36,12 @@ import mozilla.components.compose.browser.toolbar.store.BrowserEditToolbarAction import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.lib.state.ext.observeAsComposableState +import mozilla.components.lib.state.helpers.StoreProvider.Companion.fragmentStore import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.Components -import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.appstate.AppAction.SearchAction.SearchEnded import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.ext.settings @@ -62,13 +61,13 @@ private const val MATERIAL_DESIGN_SCRIM = "#52000000" * Wrapper over a [Composable] to show search suggestions, responsible for its setup. * * @param activity [HomeActivity] providing the ability to open URLs and querying the current browsing mode. + * @param fragment [Fragment] to the lifecycle of which long running operations and objects will be tied to. * @param modifier [Modifier] to be applied to the [Composable]. * @param components [Components] for accessing other functionalities of the application. * @param appStore [AppStore] for accessing the current application state. * @param browserStore [BrowserStore] for accessing the current browser state. * @param toolbarStore [BrowserToolbarStore] for accessing the current toolbar state. * @param navController [NavController] for navigating to other destinations in the application. - * @param lifecycleOwner [Fragment] for controlling the lifetime of long running operations. * @param tabId [String] Id of the current tab for which a new search was started. * @param showScrimWhenNoSuggestions Whether to show a scrim when no suggestions are available. * @param searchAccessPoint Where search was started from. @@ -76,18 +75,18 @@ private const val MATERIAL_DESIGN_SCRIM = "#52000000" @Suppress("LongParameterList") class AwesomeBarComposable( private val activity: HomeActivity, + private val fragment: Fragment, private val modifier: Modifier, private val components: Components, private val appStore: AppStore, private val browserStore: BrowserStore, private val toolbarStore: BrowserToolbarStore, private val navController: NavController, - private val lifecycleOwner: Fragment, private val tabId: String? = null, private val showScrimWhenNoSuggestions: Boolean = false, private val searchAccessPoint: MetricsUtils.Source = MetricsUtils.Source.NONE, ) { - private val searchStore = initializeSearchStore() + private val searchStore by initializeSearchStore() /** * [Composable] fully integrated with [BrowserStore] and [BrowserToolbarStore] @@ -245,19 +244,32 @@ class AwesomeBarComposable( } } - private fun initializeSearchStore() = StoreProvider.get(lifecycleOwner) { + private fun initializeSearchStore() = fragment.fragmentStore( + createInitialSearchFragmentState( + activity = activity, + components = components, + tabId = tabId, + pastedText = null, + searchAccessPoint = searchAccessPoint, + ), + ) { + val lifecycleScope = fragment.viewLifecycleOwner.lifecycle.coroutineScope + SearchFragmentStore( - initialState = createInitialSearchFragmentState( - activity = activity, - components = components, - tabId = tabId, - pastedText = null, - searchAccessPoint = searchAccessPoint, - ), + initialState = it, middleware = listOf( - BrowserToolbarToFenixSearchMapperMiddleware(toolbarStore, browserStore), - BrowserStoreToFenixSearchMapperMiddleware(browserStore), + BrowserToolbarToFenixSearchMapperMiddleware( + toolbarStore = toolbarStore, + browsingModeManager = activity.browsingModeManager, + scope = lifecycleScope, + browserStore = browserStore, + ), + BrowserStoreToFenixSearchMapperMiddleware( + browserStore = browserStore, + scope = lifecycleScope, + ), FenixSearchMiddleware( + fragment = fragment, engine = components.core.engine, useCases = components.useCases, nimbusComponents = components.nimbus, @@ -265,27 +277,10 @@ class AwesomeBarComposable( appStore = appStore, browserStore = browserStore, toolbarStore = toolbarStore, - ), - ), - ) - }.also { - it.dispatch( - SearchFragmentAction.EnvironmentRehydrated( - SearchFragmentStore.Environment( - context = activity, - viewLifecycleOwner = lifecycleOwner.viewLifecycleOwner, - browsingModeManager = activity.browsingModeManager, navController = navController, + browsingModeManager = activity.browsingModeManager, ), ), ) - - lifecycleOwner.viewLifecycleOwner.lifecycle.addObserver( - object : DefaultLifecycleObserver { - override fun onDestroy(owner: LifecycleOwner) { - it.dispatch(SearchFragmentAction.EnvironmentCleared) - } - }, - ) } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt @@ -4,10 +4,6 @@ package org.mozilla.fenix.components.toolbar -import android.content.Context -import android.content.res.Configuration -import androidx.fragment.app.Fragment -import androidx.fragment.app.FragmentManager import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry @@ -23,7 +19,9 @@ import io.mockk.mockk import io.mockk.slot import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.MainScope import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.BrowserAction import mozilla.components.browser.state.action.ContentAction @@ -72,8 +70,6 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.B import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuButton.Text.StringResText import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuDivider import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.ProgressBarConfig import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.concept.engine.Engine @@ -96,6 +92,7 @@ import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainLooperTestRule import mozilla.components.support.utils.ClipboardHandler import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull @@ -125,7 +122,6 @@ import org.mozilla.fenix.browser.store.BrowserScreenAction.PageTranslationStatus import org.mozilla.fenix.browser.store.BrowserScreenAction.ReaderModeStatusUpdated import org.mozilla.fenix.browser.store.BrowserScreenState import org.mozilla.fenix.browser.store.BrowserScreenStore -import org.mozilla.fenix.browser.store.BrowserScreenStore.Environment import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.NimbusComponents import org.mozilla.fenix.components.UseCases @@ -164,7 +160,6 @@ import org.mozilla.fenix.components.toolbar.TabCounterInteractions.TabCounterCli import org.mozilla.fenix.components.toolbar.TabCounterInteractions.TabCounterLongClicked import org.mozilla.fenix.components.usecases.FenixBrowserUseCases import org.mozilla.fenix.ext.directionsEq -import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixGleanTestRule import org.mozilla.fenix.settings.ShortcutType import org.mozilla.fenix.tabstray.Page @@ -190,7 +185,6 @@ class BrowserToolbarMiddlewareTest { } private val browserStore = BrowserStore() private val clipboard: ClipboardHandler = mockk(relaxed = true) - private val lifecycleOwner = FakeLifecycleOwner(Lifecycle.State.RESUMED) private val navController: NavController = mockk(relaxed = true) private val browsingModeManager = SimpleBrowsingModeManager(Normal) private val browserAnimator: BrowserAnimator = mockk(relaxed = true) @@ -218,26 +212,11 @@ class BrowserToolbarMiddlewareTest { private val publicSuffixList = PublicSuffixList(testContext) private val bookmarksStorage: BookmarksStorage = mockk() private lateinit var appStore: AppStore - private lateinit var configuration: Configuration - private lateinit var fragment: Fragment - private lateinit var mockContext: Context @Before fun setup() { appStore = spyk(AppStore()) coEvery { bookmarksStorage.getBookmarksWithUrl(any()) } returns Result.success(listOf(mockk())) - mockContext = mockk(relaxed = true) { - every { settings() } returns settings - } - fragment = spyk(Fragment()).apply { - every { context } returns mockContext - } - every { fragment.viewLifecycleOwner } returns lifecycleOwner - configuration = Configuration().apply { - screenHeightDp = 700 - screenWidthDp = 400 - } - every { mockContext.resources.configuration } returns configuration every { settings.toolbarPosition } returns ToolbarPosition.TOP } @@ -284,12 +263,9 @@ class BrowserToolbarMiddlewareTest { tabs = listOf(createTab("test.com", private = false)), ), ) - val middleware = buildMiddleware(browserStore = browserStore) + val middleware = buildMiddleware(browserStore = browserStore, browsingModeManager = browsingModeManager) - val toolbarStore = buildStore( - middleware = middleware, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) val toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd val tabCounterButton = toolbarBrowserActions[1] as TabCounterAction @@ -307,12 +283,9 @@ class BrowserToolbarMiddlewareTest { ), ), ) - val middleware = buildMiddleware(browserStore = browserStore) + val middleware = buildMiddleware(browserStore = browserStore, browsingModeManager = browsingModeManager) - val toolbarStore = buildStore( - middleware = middleware, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) val toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd val tabCounterButton = toolbarBrowserActions[1] as TabCounterAction @@ -344,18 +317,6 @@ class BrowserToolbarMiddlewareTest { } @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val middleware = buildMiddleware() - val store = buildStore(middleware) - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } - - @Test fun `GIVEN ABOUT_HOME URL WHEN the page origin is modified THEN update the page origin`() = runTest { val tab = createTab("https://mozilla.com/") val browserStore = BrowserStore( @@ -394,16 +355,19 @@ class BrowserToolbarMiddlewareTest { orientation = Portrait, ), ) - val middleware = buildMiddleware(appStore = appStore) + var isWideScreen = false + var isTallScreen = false + val middleware = buildMiddleware( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) val toolbarStore = buildStore(middleware) var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(3, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } + isWideScreen = true appStore.dispatch(AppAction.OrientationChange(Landscape)) mainLooperRule.idle() @@ -425,7 +389,13 @@ class BrowserToolbarMiddlewareTest { orientation = Landscape, ), ) - val middleware = buildMiddleware(appStore = appStore) + var isWideScreen = false + var isTallScreen = false + val middleware = buildMiddleware( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) val toolbarStore = buildStore(middleware) @@ -438,10 +408,7 @@ class BrowserToolbarMiddlewareTest { assertEqualsTabCounterButton(expectedTabCounterButton(), tabCounterButton) assertEquals(expectedMenuButton(), menuButton) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } + isWideScreen = true appStore.dispatch(AppAction.OrientationChange(Portrait)) mainLooperRule.idle() @@ -454,12 +421,9 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN in normal browsing WHEN the number of normal opened tabs is modified THEN update the tab counter`() = runTest { val browsingModeManager = SimpleBrowsingModeManager(Normal) val browserStore = BrowserStore() - val middleware = buildMiddleware(browserStore = browserStore) + val middleware = buildMiddleware(browserStore = browserStore, browsingModeManager = browsingModeManager) - val toolbarStore = buildStore( - middleware = middleware, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(3, toolbarBrowserActions.size) @@ -488,11 +452,9 @@ class BrowserToolbarMiddlewareTest { tabs = listOf(initialNormalTab, initialPrivateTab), ), ) - val middleware = buildMiddleware(browserStore = browserStore) - val toolbarStore = buildStore( - middleware = middleware, - browsingModeManager = browsingModeManager, - ) + val middleware = buildMiddleware(browserStore = browserStore, browsingModeManager = browsingModeManager) + + val toolbarStore = buildStore(middleware) var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(3, toolbarBrowserActions.size) @@ -518,10 +480,7 @@ class BrowserToolbarMiddlewareTest { ) } answers { browserAnimatorActionCaptor.captured.invoke(true) } val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) val newTabButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes toolbarStore.dispatch(newTabButton.onClick as BrowserToolbarEvent) @@ -539,10 +498,7 @@ class BrowserToolbarMiddlewareTest { } answers { browserAnimatorActionCaptor.captured.invoke(true) } every { settings.enableHomepageAsNewTab } returns true val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) val newTabButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes toolbarStore.dispatch(newTabButton.onClick as BrowserToolbarEvent) @@ -560,10 +516,7 @@ class BrowserToolbarMiddlewareTest { } answers { browserAnimatorActionCaptor.captured.invoke(true) } every { settings.enableHomepageSearchBar } returns true val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) val newTabButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes toolbarStore.dispatch(newTabButton.onClick as BrowserToolbarEvent) @@ -575,10 +528,7 @@ class BrowserToolbarMiddlewareTest { every { navController.currentDestination?.id } returns R.id.browserFragment val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) val menuButton = toolbarStore.state.displayState.browserActionsEnd[2] as ActionButtonRes toolbarStore.dispatch(menuButton.onClick as BrowserToolbarEvent) @@ -596,16 +546,9 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN browsing in normal mode WHEN clicking the tab counter button THEN open the tabs tray in normal mode`() { every { navController.currentDestination?.id } returns R.id.browserFragment - val browsingModeManager = SimpleBrowsingModeManager(Normal) - val thumbnailsFeature: BrowserThumbnails = mockk(relaxed = true) - val middleware = buildMiddleware(browserStore = browserStore) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - thumbnailsFeature = thumbnailsFeature, - ) + val middleware = buildMiddleware(browserStore = browserStore, browsingModeManager = browsingModeManager) + val toolbarStore = buildStore(middleware) val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction toolbarStore.dispatch(tabCounterButton.onClick) @@ -628,13 +571,12 @@ class BrowserToolbarMiddlewareTest { } val browsingModeManager = SimpleBrowsingModeManager(Private) val thumbnailsFeature: BrowserThumbnails = mockk(relaxed = true) - val middleware = buildMiddleware(browserStore = browserStore) - val toolbarStore = buildStore( - middleware = middleware, + val middleware = buildMiddleware( navController = navController, browsingModeManager = browsingModeManager, - thumbnailsFeature = thumbnailsFeature, + thumbnailsFeature = { thumbnailsFeature }, ) + val toolbarStore = buildStore(middleware) val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction toolbarStore.dispatch(tabCounterButton.onClick) @@ -658,12 +600,8 @@ class BrowserToolbarMiddlewareTest { fun `WHEN clicking on the first option in the toolbar long click menu THEN open a new normal tab`() { val navController: NavController = mockk(relaxed = true) val browsingModeManager = SimpleBrowsingModeManager(Normal) - val middleware = buildMiddleware(browserStore = browserStore) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val middleware = buildMiddleware(navController = navController, browsingModeManager = browsingModeManager) + val toolbarStore = buildStore(middleware) val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction assertEqualsTabCounterButton(expectedTabCounterButton(0, false), tabCounterButton) val tabCounterMenuItems = (tabCounterButton.onLongClick as CombinedEventAndMenu).menu.items() @@ -680,8 +618,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN no search terms for the current tab WHEN the page origin is clicked THEN start search in the home screen`() { - val navController: NavController = mockk(relaxed = true) - val browsingModeManager = SimpleBrowsingModeManager(Normal) val currentTab = createTab("test.com") val browserStore = BrowserStore( BrowserState( @@ -690,11 +626,7 @@ class BrowserToolbarMiddlewareTest { ), ) val middleware = buildMiddleware(browserStore = browserStore) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(toolbarStore.state.displayState.pageOrigin.onClick as BrowserToolbarAction) @@ -710,8 +642,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN the current tab has search terms WHEN the page origin is clicked THEN start search in the browser screen`() { - val navController: NavController = mockk(relaxed = true) - val browsingModeManager = SimpleBrowsingModeManager(Normal) val currentTab = createTab("test.com", searchTerms = "test") val browserStore = BrowserStore( BrowserState( @@ -720,11 +650,7 @@ class BrowserToolbarMiddlewareTest { ), ) val middleware = buildMiddleware(browserStore = browserStore) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(toolbarStore.state.displayState.pageOrigin.onClick as BrowserToolbarAction) @@ -736,7 +662,7 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN clicking on the URL THEN record telemetry`() { val middleware = buildMiddleware() - val toolbarStore = buildStore(middleware, navController = navController) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(toolbarStore.state.displayState.pageOrigin.onClick as BrowserToolbarAction) @@ -746,8 +672,6 @@ class BrowserToolbarMiddlewareTest { @Test @Config(sdk = [30]) fun `GIVEN on Android 11 WHEN choosing to copy the current URL to clipboard THEN copy to clipboard and show a snackbar`() { - val navController: NavController = mockk(relaxed = true) - val browsingModeManager = SimpleBrowsingModeManager(Normal) val clipboard = ClipboardHandler(testContext) val currentTab = createTab("test.com") val browserStore = BrowserStore( @@ -757,15 +681,10 @@ class BrowserToolbarMiddlewareTest { ), ) val middleware = buildMiddleware( - appStore = appStore, browserStore = browserStore, clipboard = clipboard, ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(CopyToClipboardClicked) @@ -786,15 +705,10 @@ class BrowserToolbarMiddlewareTest { ), ) val middleware = buildMiddleware( - appStore = appStore, browserStore = browserStore, clipboard = clipboard, ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(CopyToClipboardClicked) @@ -815,15 +729,10 @@ class BrowserToolbarMiddlewareTest { ), ) val middleware = buildMiddleware( - appStore = appStore, browserStore = browserStore, clipboard = clipboard, ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(CopyToClipboardClicked) @@ -834,8 +743,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN choosing to paste from clipboard THEN start a new search with the current clipboard text`() { - val navController: NavController = mockk(relaxed = true) - val browsingModeManager = SimpleBrowsingModeManager(Normal) val queryText = "test" val clipboard = ClipboardHandler(testContext).also { it.text = queryText @@ -851,11 +758,7 @@ class BrowserToolbarMiddlewareTest { browserStore = browserStore, clipboard = clipboard, ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(PasteFromClipboardClicked) @@ -867,8 +770,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN choosing to load URL from clipboard THEN start load the URL from clipboard in a new tab`() { - val navController: NavController = mockk(relaxed = true) - val browsingModeManager = SimpleBrowsingModeManager(Normal) val clipboardUrl = "https://www.mozilla.com" val clipboard = ClipboardHandler(testContext).also { it.text = clipboardUrl @@ -889,11 +790,7 @@ class BrowserToolbarMiddlewareTest { useCases = useCases, clipboard = clipboard, ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val toolbarStore = buildStore(middleware) every { appStore.state.searchState.selectedSearchEngine?.searchEngine } returns searchEngine @@ -914,14 +811,9 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN clicking on the second option in the toolbar long click menu THEN open a new private tab`() { - val navController: NavController = mockk(relaxed = true) val browsingModeManager = SimpleBrowsingModeManager(Normal) - val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, - ) + val middleware = buildMiddleware(browsingModeManager = browsingModeManager) + val toolbarStore = buildStore(middleware) val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction assertEqualsTabCounterButton(expectedTabCounterButton(0, false), tabCounterButton) val tabCounterMenuItems = (tabCounterButton.onLongClick as CombinedEventAndMenu).menu.items() @@ -938,7 +830,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN multiple tabs opened WHEN clicking on the close tab item in the tab counter long click menu THEN close the current tab`() { - val navController: NavController = mockk(relaxed = true) val browsingModeManager = SimpleBrowsingModeManager(Private) val currentTab = createTab("test.com", private = true) val browserStore = BrowserStore( @@ -950,15 +841,10 @@ class BrowserToolbarMiddlewareTest { val tabsUseCases: TabsUseCases = mockk(relaxed = true) every { useCases.tabsUseCases } returns tabsUseCases val middleware = buildMiddleware( - appStore = appStore, browserStore = browserStore, - useCases = useCases, - ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, browsingModeManager = browsingModeManager, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction @@ -987,16 +873,11 @@ class BrowserToolbarMiddlewareTest { ), ) val tabsUseCases: TabsUseCases = mockk(relaxed = true) + every { useCases.tabsUseCases } returns tabsUseCases val middleware = buildMiddleware( - appStore = appStore, browserStore = browserStore, - useCases = useCases, - ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - browsingModeManager = browsingModeManager, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction @@ -1031,16 +912,12 @@ class BrowserToolbarMiddlewareTest { ), ) val tabsUseCases: TabsUseCases = mockk(relaxed = true) + every { useCases.tabsUseCases } returns tabsUseCases val middleware = buildMiddleware( - appStore = appStore, browserStore = browserStore, - useCases = useCases, - ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, browsingModeManager = browsingModeManager, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction @@ -1079,16 +956,14 @@ class BrowserToolbarMiddlewareTest { ), ) val tabsUseCases: TabsUseCases = mockk(relaxed = true) + every { useCases.tabsUseCases } returns tabsUseCases val middleware = buildMiddleware( appStore = appStore, browserStore = browserStore, useCases = useCases, - ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, browsingModeManager = browsingModeManager, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction @@ -1133,16 +1008,14 @@ class BrowserToolbarMiddlewareTest { ), ) val tabsUseCases: TabsUseCases = mockk(relaxed = true) + every { useCases.tabsUseCases } returns tabsUseCases val middleware = buildMiddleware( appStore = appStore, browserStore = browserStore, useCases = useCases, - ) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, browsingModeManager = browsingModeManager, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val tabCounterButton = toolbarStore.state.displayState.browserActionsEnd[1] as TabCounterAction @@ -1179,7 +1052,6 @@ class BrowserToolbarMiddlewareTest { ) val middleware = buildMiddleware( browserStore = browserStore, - useCases = useCases, ) val toolbarStore = buildStore(middleware).also { it.dispatch(BrowserToolbarAction.Init()) @@ -1209,7 +1081,6 @@ class BrowserToolbarMiddlewareTest { ) val middleware = buildMiddleware( browserStore = browserStore, - useCases = useCases, ) val toolbarStore = buildStore(middleware).also { it.dispatch(BrowserToolbarAction.Init()) @@ -1237,15 +1108,11 @@ class BrowserToolbarMiddlewareTest { ), ) val browserScreenStore = buildBrowserScreenStore() - val readerModeController: ReaderModeController = mockk(relaxed = true) val middleware = buildMiddleware( browserScreenStore = browserScreenStore, browserStore = browserStore, ) - val toolbarStore = buildStore( - middleware = middleware, - readerModeController = readerModeController, - ) + val toolbarStore = buildStore(middleware) browserScreenStore.dispatch( ReaderModeStatusUpdated( @@ -1255,6 +1122,7 @@ class BrowserToolbarMiddlewareTest { ), ), ) + mainLooperRule.idle() val readerModeButton = toolbarStore.state.displayState.pageActionsEnd[0] as ActionButtonRes assertEquals(expectedReaderModeButton(false), readerModeButton) @@ -1274,15 +1142,11 @@ class BrowserToolbarMiddlewareTest { ), ) val browserScreenStore = buildBrowserScreenStore() - val readerModeController: ReaderModeController = mockk(relaxed = true) val middleware = buildMiddleware( browserScreenStore = browserScreenStore, browserStore = browserStore, ) - val toolbarStore = buildStore( - middleware = middleware, - readerModeController = readerModeController, - ) + val toolbarStore = buildStore(middleware) browserScreenStore.dispatch( ReaderModeStatusUpdated( @@ -1292,6 +1156,7 @@ class BrowserToolbarMiddlewareTest { ), ), ) + mainLooperRule.idle() val readerModeButton = toolbarStore.state.displayState.pageActionsEnd[0] as ActionButtonRes assertEquals(expectedReaderModeButton(true), readerModeButton) @@ -1303,15 +1168,14 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN on a wide window WHEN translation is possible THEN show a translate button`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseExpandedToolbar } returns false val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore(middleware, browsingModeManager = browsingModeManager, navController = navController) + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + isTallScreen = { false }, + ) + val toolbarStore = buildStore(middleware) browserScreenStore.dispatch( PageTranslationStatusUpdated( @@ -1322,6 +1186,7 @@ class BrowserToolbarMiddlewareTest { ), ), ) + mainLooperRule.idle() val translateButton = toolbarStore.state.displayState.pageActionsEnd[0] assertEquals(expectedTranslateButton(), translateButton) @@ -1329,19 +1194,14 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN the current page is translated AND a wide window WHEN knowing of this state THEN update the translate button to show this`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseExpandedToolbar } returns true val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + isTallScreen = { false }, ) + val toolbarStore = buildStore(middleware) browserScreenStore.dispatch( PageTranslationStatusUpdated( @@ -1352,6 +1212,7 @@ class BrowserToolbarMiddlewareTest { ), ), ) + mainLooperRule.idle() var translateButton = toolbarStore.state.displayState.pageActionsEnd[0] assertEquals(expectedTranslateButton(), translateButton) @@ -1364,6 +1225,7 @@ class BrowserToolbarMiddlewareTest { ), ), ) + mainLooperRule.idle() translateButton = toolbarStore.state.displayState.pageActionsEnd[0] assertEquals( expectedTranslateButton(isActive = true), @@ -1373,11 +1235,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN translation is possible WHEN tapping on the translate button THEN allow user to choose how to translate`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseExpandedToolbar } returns true val currentNavDestination: NavDestination = mockk { every { id } returns R.id.browserFragment @@ -1387,12 +1244,13 @@ class BrowserToolbarMiddlewareTest { } val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, navController = navController, + isWideScreen = { true }, + isTallScreen = { false }, ) + val toolbarStore = buildStore(middleware) browserScreenStore.dispatch( PageTranslationStatusUpdated( PageTranslationStatus( @@ -1402,6 +1260,7 @@ class BrowserToolbarMiddlewareTest { ), ), ) + mainLooperRule.idle() val translateButton = toolbarStore.state.displayState.pageActionsEnd[0] as ActionButtonRes @@ -1421,27 +1280,21 @@ class BrowserToolbarMiddlewareTest { every { settings.shouldUseExpandedToolbar } returns false val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, - ) + val middleware = buildMiddleware(appStore, browserScreenStore) + val toolbarStore = buildStore(middleware) assertTrue(toolbarStore.state.displayState.pageActionsEnd.isEmpty()) } @Test fun `GIVEN on a wide screen with tabstrip is disabled THEN show a share button as page end action`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.isTabStripEnabled } returns false val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore(middleware, browsingModeManager = browsingModeManager, navController = navController) + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + ) + val toolbarStore = buildStore(middleware) val shareButton = toolbarStore.state.displayState.pageActionsEnd[0] assertEquals(expectedShareButton(), shareButton) @@ -1451,23 +1304,14 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN on a large screen with tabstrip is enabled THEN don't show a share button as page end action`() { every { settings.isTabStripEnabled } returns true val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, - ) + val middleware = buildMiddleware(appStore, browserScreenStore) + val toolbarStore = buildStore(middleware) assertTrue(toolbarStore.state.displayState.pageActionsEnd.isEmpty()) } @Test fun `GIVEN the current tab shows a content page WHEN the share button is clicked THEN record telemetry and start sharing the local resource`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.isTabStripEnabled } returns true every { settings.shouldUseExpandedToolbar } returns false val browserScreenStore = buildBrowserScreenStore() @@ -1480,12 +1324,12 @@ class BrowserToolbarMiddlewareTest { ), middleware = listOf(captureMiddleware), ) - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + browserStore = browserStore, + isWideScreen = { true }, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val shareButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes @@ -1501,11 +1345,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN the current tab shows a normal webpage WHEN the share button is clicked THEN record telemetry and open the share dialog`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.isTabStripEnabled } returns true every { settings.shouldUseExpandedToolbar } returns false every { navController.currentDestination?.id } returns R.id.browserFragment @@ -1518,12 +1357,12 @@ class BrowserToolbarMiddlewareTest { selectedTabId = currentTab.id, ), ) - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + browserStore = browserStore, + isWideScreen = { true }, ) + val toolbarStore = buildStore(middleware) mainLooperRule.idle() val shareButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes @@ -1553,18 +1392,13 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN on a small width with tabstrip is enabled and not using the extended layout THEN don't show a share button as browser end action`() { every { settings.shouldUseExpandedToolbar } returns false every { settings.isTabStripEnabled } returns true - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 500 - } - every { mockContext.resources.configuration } returns configuration val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { false }, + isTallScreen = { false }, ) + val toolbarStore = buildStore(middleware) assertEquals(3, toolbarStore.state.displayState.browserActionsEnd.size) val newTabButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes @@ -1579,29 +1413,22 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN expanded toolbar with tabstrip and tall window WHEN changing to short window THEN show new tab, tab counter and menu`() = runTest { every { settings.isTabStripEnabled } returns true every { settings.shouldUseExpandedToolbar } returns true - configuration = Configuration().apply { - screenHeightDp = 500 - screenWidthDp = 300 - } - every { mockContext.resources.configuration } returns configuration val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, + var isWideScreen = false + var isTallScreen = true + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, ) + val toolbarStore = buildStore(middleware) var navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(5, navigationActions.size) var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(0, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 300 - screenWidthDp = 500 - } - every { mockContext.resources.configuration } returns configuration + isTallScreen = false appStore.dispatch(AppAction.OrientationChange(Portrait)) mainLooperRule.idle() @@ -1619,21 +1446,15 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN on a wide screen with tabstrip is enabled and not using the extended layout THEN show a share button as browser end action`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration - every { settings.isTabStripEnabled } returns true every { settings.shouldUseExpandedToolbar } returns false val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + isTallScreen = { false }, ) + val toolbarStore = buildStore(middleware) val shareButton = toolbarStore.state.displayState.browserActionsEnd[0] assertEquals(expectedShareButton(), shareButton) @@ -1641,16 +1462,14 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN short window with tabstrip is enabled and not using the extended layout THEN show a share button as browser end action`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.isTabStripEnabled } returns true every { settings.shouldUseExpandedToolbar } returns false val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore(middleware, browsingModeManager = browsingModeManager, navController = navController) + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + ) + val toolbarStore = buildStore(middleware) val shareButton = toolbarStore.state.displayState.browserActionsEnd[0] assertEquals(expectedShareButton(), shareButton) @@ -1662,12 +1481,8 @@ class BrowserToolbarMiddlewareTest { every { settings.shouldUseExpandedToolbar } returns true val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, - navController = navController, - ) + val middleware = buildMiddleware(appStore, browserScreenStore) + val toolbarStore = buildStore(middleware) assertTrue(toolbarStore.state.displayState.pageActionsEnd.isEmpty()) } @@ -1686,7 +1501,7 @@ class BrowserToolbarMiddlewareTest { } every { browserScreenState.pageTranslationStatus } returns pageTranslationStatus - var middleware = buildMiddleware(appStore, browserScreenStore, browserStore) + var middleware = buildMiddleware(appStore) var toolbarStore = buildStore(middleware) assertEquals( @@ -1694,13 +1509,11 @@ class BrowserToolbarMiddlewareTest { toolbarStore.state.displayState.pageActionsEnd, ) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration - - middleware = buildMiddleware(appStore, browserScreenStore, browserStore) + middleware = buildMiddleware( + appStore = appStore, + isWideScreen = { true }, + isTallScreen = { false }, + ) toolbarStore = buildStore(middleware) assertEquals( @@ -1712,12 +1525,11 @@ class BrowserToolbarMiddlewareTest { toolbarStore.state.displayState.pageActionsEnd, ) - configuration = Configuration().apply { - screenHeightDp = 700 - screenWidthDp = 400 - } - every { mockContext.resources.configuration } returns configuration - middleware = buildMiddleware(appStore, browserScreenStore, browserStore) + middleware = buildMiddleware( + appStore = appStore, + isWideScreen = { false }, + isTallScreen = { true }, + ) toolbarStore = buildStore(middleware) assertEquals( @@ -1728,13 +1540,10 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN device has wide window WHEN a website is loaded THEN show navigation buttons`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseBottomToolbar } returns false - val middleware = buildMiddleware() + val middleware = buildMiddleware( + isWideScreen = { true }, + ) val toolbarStore = buildStore(middleware) val displayGoBackButton = toolbarStore.state.displayState.browserActionsStart[0] @@ -1746,12 +1555,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN the back button is shown WHEN interacted with THEN go back or show history`() = runTest { every { navController.currentDestination?.id } returns R.id.browserFragment - - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseBottomToolbar } returns false val currentTab = createTab("test.com", private = false) val captureMiddleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() @@ -1765,7 +1568,10 @@ class BrowserToolbarMiddlewareTest { ), middleware = listOf(captureMiddleware) + EngineMiddleware.create(engine), ) - val middleware = buildMiddleware(appStore, browserStore = browserStore) + val middleware = buildMiddleware( + browserStore = browserStore, + isWideScreen = { true }, + ) val toolbarStore = buildStore(middleware) val backButton = toolbarStore.state.displayState.browserActionsStart[0] as ActionButtonRes @@ -1786,11 +1592,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN the forward button is shown WHEN interacted with THEN go forward or show history`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseBottomToolbar } returns false val currentTab = createTab("test.com", private = false) val captureMiddleware = CaptureActionsMiddleware<BrowserState, BrowserAction>() @@ -1801,7 +1602,10 @@ class BrowserToolbarMiddlewareTest { ), middleware = listOf(captureMiddleware) + EngineMiddleware.create(mockk()), ) - val middleware = buildMiddleware(appStore, browserStore = browserStore) + val middleware = buildMiddleware( + browserStore = browserStore, + isWideScreen = { true }, + ) val toolbarStore = buildStore(middleware) val forwardButton = toolbarStore.state.displayState.browserActionsStart[1] as ActionButtonRes @@ -1816,11 +1620,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN device has wide window WHEN a website is loaded THEN show refresh button`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration val browsingModeManager = SimpleBrowsingModeManager(Private) val currentNavDestination: NavDestination = mockk { every { id } returns R.id.browserFragment @@ -1849,11 +1648,15 @@ class BrowserToolbarMiddlewareTest { } val middleware = buildMiddleware( - appStore, browserScreenStore, browserStore, useCases, sessionUseCases = sessionUseCases, - ) - val toolbarStore = buildStore( - middleware, browsingModeManager = browsingModeManager, navController = navController, + browserScreenStore = browserScreenStore, + browserStore = browserStore, + useCases = useCases, + sessionUseCases = sessionUseCases, + navController = navController, + browsingModeManager = browsingModeManager, + isWideScreen = { true }, ) + val toolbarStore = buildStore(middleware) val loadUrlFlagsUsed = mutableListOf<LoadUrlFlags>() @@ -1871,11 +1674,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN device have a wide window WHEN a website is loaded THEN show refresh button`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration val browsingModeManager = SimpleBrowsingModeManager(Private) val currentNavDestination: NavDestination = mockk { every { id } returns R.id.browserFragment @@ -1903,17 +1701,15 @@ class BrowserToolbarMiddlewareTest { every { fenixBrowserUseCases } returns browserUseCases } val middleware = buildMiddleware( - appStore, - browserScreenStore, - browserStore, - useCases, + browserScreenStore = browserScreenStore, + browserStore = browserStore, + useCases = useCases, sessionUseCases = sessionUseCases, - ) - val toolbarStore = buildStore( - middleware, - browsingModeManager = browsingModeManager, navController = navController, + browsingModeManager = browsingModeManager, + isWideScreen = { true }, ) + val toolbarStore = buildStore(middleware) val loadUrlFlagsUsed = mutableListOf<LoadUrlFlags>() @@ -1932,11 +1728,6 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN a loaded tab WHEN the refresh button is pressed THEN show stop refresh button`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration val browsingModeManager = SimpleBrowsingModeManager(Private) val currentNavDestination: NavDestination = mockk { every { id } returns R.id.browserFragment @@ -1964,11 +1755,15 @@ class BrowserToolbarMiddlewareTest { every { fenixBrowserUseCases } returns browserUseCases } val middleware = buildMiddleware( - appStore, browserScreenStore, browserStore, useCases, sessionUseCases = sessionUseCases, - ) - val toolbarStore = buildStore( - middleware, browsingModeManager = browsingModeManager, navController = navController, + browserScreenStore = browserScreenStore, + browserStore = browserStore, + useCases = useCases, + sessionUseCases = sessionUseCases, + navController = navController, + browsingModeManager = browsingModeManager, + isWideScreen = { true }, ) + val toolbarStore = buildStore(middleware) val loadUrlFlagsUsed = mutableListOf<LoadUrlFlags>() @@ -2578,10 +2373,7 @@ class BrowserToolbarMiddlewareTest { testContext.getString(tabcounterR.string.mozac_tab_counter_open_tab_tray, 3), action.contentDescription, ) - assertEquals( - middleware.environment?.browsingModeManager?.mode == Private, - action.showPrivacyMask, - ) + assertFalse(action.showPrivacyMask) assertEquals(TabCounterClicked(Source.AddressBar), action.onClick) assertNotNull(action.onLongClick) } @@ -2653,14 +2445,11 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN initializing the navigation bar AND should not use simple toolbar AND in short window THEN add no navigation bar actions`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldUseExpandedToolbar } returns true - val middleware = buildMiddleware(appStore = appStore) + val middleware = buildMiddleware( + isWideScreen = { true }, + ) val toolbarStore = buildStore(middleware) val navigationActions = toolbarStore.state.displayState.navigationActions @@ -2675,7 +2464,13 @@ class BrowserToolbarMiddlewareTest { ), ) every { settings.shouldUseExpandedToolbar } returns true - val middleware = buildMiddleware(appStore = appStore) + var isWideScreen = false + var isTallScreen = true + val middleware = buildMiddleware( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) val toolbarStore = buildStore(middleware) var navigationActions = toolbarStore.state.displayState.navigationActions @@ -2684,11 +2479,8 @@ class BrowserToolbarMiddlewareTest { var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(0, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration + isWideScreen = true + isTallScreen = false appStore.dispatch(AppAction.OrientationChange(Landscape)) mainLooperRule.idle() @@ -2908,16 +2700,14 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN share shortcut is selected THEN update end page actions without share action`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.isTabStripEnabled } returns false every { settings.toolbarSimpleShortcutKey } returns ShortcutType.SHARE.value val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore(middleware, browsingModeManager = browsingModeManager, navController = navController) + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + ) + val toolbarStore = buildStore(middleware) val endPageActions = toolbarStore.state.displayState.pageActionsEnd assertEquals(emptyList<Action>(), endPageActions) @@ -2925,15 +2715,13 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN translate shortcut is selected THEN update end page actions without translate action`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.toolbarSimpleShortcutKey } returns ShortcutType.TRANSLATE.value val browserScreenStore = buildBrowserScreenStore() - val middleware = buildMiddleware(appStore, browserScreenStore, browserStore) - val toolbarStore = buildStore(middleware, browsingModeManager = browsingModeManager, navController = navController) + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isWideScreen = { true }, + ) + val toolbarStore = buildStore(middleware) browserScreenStore.dispatch( PageTranslationStatusUpdated( @@ -2963,10 +2751,7 @@ class BrowserToolbarMiddlewareTest { every { settings.toolbarSimpleShortcutKey } returns ShortcutType.HOMEPAGE.value val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) val homepageButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes toolbarStore.dispatch(homepageButton.onClick as BrowserToolbarEvent) @@ -2980,10 +2765,7 @@ class BrowserToolbarMiddlewareTest { every { settings.toolbarSimpleShortcutKey } returns ShortcutType.HOMEPAGE.value val middleware = buildMiddleware() - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) val newTabButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes toolbarStore.dispatch(newTabButton.onClick as BrowserToolbarEvent) @@ -2992,16 +2774,16 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN expanded toolbar is used and navbar is hidden WHEN building end browser actions THEN use simple toolbar shortcuts`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { settings.shouldShowToolbarCustomization } returns true every { settings.shouldUseExpandedToolbar } returns true every { settings.toolbarSimpleShortcutKey } returns ShortcutType.HOMEPAGE.value - val toolbarStore = buildStore() + val middleware = buildMiddleware( + browserScreenStore = browserScreenStore, + isTallScreen = { false }, + isWideScreen = { true }, + ) + val toolbarStore = buildStore(middleware) val homepageButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes assertEquals(expectedHomepageButton(), homepageButton) @@ -3132,15 +2914,16 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN simple toolbar use share shortcut AND wide window with tabstrip enabled WHEN initializing toolbar THEN only show one Share in end browser actions`() { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } every { settings.shouldShowToolbarCustomization } returns true every { settings.isTabStripEnabled } returns true every { settings.toolbarSimpleShortcutKey } returns ShortcutType.SHARE.value - val toolbarStore = buildStore() + val middleware = buildMiddleware( + appStore = appStore, + isWideScreen = { true }, + isTallScreen = { false }, + ) + val toolbarStore = buildStore(middleware) assertEquals(3, toolbarStore.state.displayState.browserActionsEnd.size) val shareButton = toolbarStore.state.displayState.browserActionsEnd[0] as ActionButtonRes @@ -3464,79 +3247,65 @@ class BrowserToolbarMiddlewareTest { appStore: AppStore = this.appStore, browserScreenStore: BrowserScreenStore = this.browserScreenStore, browserStore: BrowserStore = this.browserStore, + permissionsStorage: SitePermissionsStorage = this.permissionsStorage, + cookieBannersStorage: CookieBannersStorage = this.cookieBannersStorage, + trackingProtectionUseCases: TrackingProtectionUseCases = this.trackingProtectionUseCases, + bookmarksStorage: BookmarksStorage = this.bookmarksStorage, useCases: UseCases = this.useCases, + sessionUseCases: SessionUseCases = SessionUseCases(browserStore), nimbusComponents: NimbusComponents = this.nimbusComponents, clipboard: ClipboardHandler = this.clipboard, publicSuffixList: PublicSuffixList = this.publicSuffixList, settings: Settings = this.settings, - permissionsStorage: SitePermissionsStorage = this.permissionsStorage, - cookieBannersStorage: CookieBannersStorage = this.cookieBannersStorage, - trackingProtectionUseCases: TrackingProtectionUseCases = this.trackingProtectionUseCases, - sessionUseCases: SessionUseCases = SessionUseCases(browserStore), - bookmarksStorage: BookmarksStorage = this.bookmarksStorage, + navController: NavController = this.navController, + browsingModeManager: BrowsingModeManager = this.browsingModeManager, + readerModeController: ReaderModeController = this.readerModeController, + browserAnimator: BrowserAnimator = this.browserAnimator, + thumbnailsFeature: () -> BrowserThumbnails = { this.thumbnailsFeature }, + isWideScreen: () -> Boolean = { false }, + isTallScreen: () -> Boolean = { true }, + scope: CoroutineScope = MainScope(), ) = BrowserToolbarMiddleware( + uiContext = testContext, appStore = appStore, browserScreenStore = browserScreenStore, browserStore = browserStore, + permissionsStorage = permissionsStorage, + cookieBannersStorage = cookieBannersStorage, + bookmarksStorage = bookmarksStorage, + trackingProtectionUseCases = trackingProtectionUseCases, useCases = useCases, nimbusComponents = nimbusComponents, clipboard = clipboard, publicSuffixList = publicSuffixList, settings = settings, - permissionsStorage = permissionsStorage, - cookieBannersStorage = cookieBannersStorage, - trackingProtectionUseCases = trackingProtectionUseCases, + navController = navController, + browsingModeManager = browsingModeManager, + readerModeController = readerModeController, + browserAnimator = browserAnimator, + thumbnailsFeature = thumbnailsFeature, + isWideScreen = isWideScreen, + isTallScreen = isTallScreen, sessionUseCases = sessionUseCases, - bookmarksStorage = bookmarksStorage, + scope = scope, ioDispatcher = Dispatchers.Main, ) private fun buildStore( middleware: BrowserToolbarMiddleware = buildMiddleware(), - context: Context = testContext, - navController: NavController = this@BrowserToolbarMiddlewareTest.navController, - browsingModeManager: BrowsingModeManager = this@BrowserToolbarMiddlewareTest.browsingModeManager, - browserAnimator: BrowserAnimator = this@BrowserToolbarMiddlewareTest.browserAnimator, - thumbnailsFeature: BrowserThumbnails? = this@BrowserToolbarMiddlewareTest.thumbnailsFeature, - readerModeController: ReaderModeController = this@BrowserToolbarMiddlewareTest.readerModeController, ) = BrowserToolbarStore( middleware = listOf(middleware), ).also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - context = context, - fragment = fragment, - navController = navController, - browsingModeManager = browsingModeManager, - browserAnimator = browserAnimator, - thumbnailsFeature = { thumbnailsFeature }, - readerModeController = readerModeController, - ), - ), - ) + mainLooperRule.idle() // to complete the initial setup happening in coroutines } private fun buildBrowserScreenStore( initialState: BrowserScreenState = BrowserScreenState(), middlewares: List<Middleware<BrowserScreenState, BrowserScreenAction>> = emptyList(), - context: Context = testContext, - viewLifecycleOwner: LifecycleOwner = lifecycleOwner, - fragmentManager: FragmentManager = mockk(), ) = BrowserScreenStore( initialState = initialState, middleware = middlewares, - ).also { - it.dispatch( - BrowserScreenAction.EnvironmentRehydrated(Environment(context, viewLifecycleOwner, fragmentManager)), - ) - } - - private class FakeLifecycleOwner(initialState: Lifecycle.State) : LifecycleOwner { - override val lifecycle: Lifecycle = LifecycleRegistry(this).apply { - currentState = initialState - } - } + ) private fun fakeSearchState() = SearchState( region = RegionState("US", "US"), diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/CustomTabBrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/CustomTabBrowserToolbarMiddlewareTest.kt @@ -18,6 +18,11 @@ import io.mockk.every import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.ContentAction.UpdateProgressAction import mozilla.components.browser.state.action.ContentAction.UpdateSecurityInfoAction @@ -37,8 +42,6 @@ import mozilla.components.compose.browser.toolbar.concept.PageOrigin import mozilla.components.compose.browser.toolbar.concept.PageOrigin.Companion.ContextualMenuOption import mozilla.components.compose.browser.toolbar.concept.PageOrigin.Companion.PageOriginContextualMenuInteractions.CopyToClipboardClicked import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.store.ProgressBarConfig import mozilla.components.concept.engine.cookiehandling.CookieBannersStorage import mozilla.components.concept.engine.permission.SitePermissionsStorage @@ -295,18 +298,6 @@ class CustomTabBrowserToolbarMiddlewareTest { } @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val middleware = buildMiddleware() - val store = buildStore(middleware) - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } - - @Test fun `GIVEN the website is insecure WHEN the conection becomes secure THEN update appropriate security indicator`() = runTest { val customTab = createCustomTab( url = "URL", @@ -459,11 +450,8 @@ class CustomTabBrowserToolbarMiddlewareTest { val navController: NavController = mockk(relaxed = true) every { customTab.content.url } returns "https://mozilla.test" val clipboard = ClipboardHandler(testContext) - val middleware = buildMiddleware(appStore = appStore, clipboard = clipboard) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val middleware = buildMiddleware(appStore = appStore, clipboard = clipboard, navController = navController) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(CopyToClipboardClicked) @@ -476,14 +464,10 @@ class CustomTabBrowserToolbarMiddlewareTest { @Config(sdk = [33]) fun `GIVEN on Android 13 WHEN choosing to copy the current URL to clipboard THEN copy to clipboard and don't show a snackbar`() { val appStore: AppStore = mockk(relaxed = true) - val navController: NavController = mockk(relaxed = true) every { customTab.content.url } returns "https://mozilla.test" val clipboard = ClipboardHandler(testContext) val middleware = buildMiddleware(appStore = appStore, clipboard = clipboard) - val toolbarStore = buildStore( - middleware = middleware, - navController = navController, - ) + val toolbarStore = buildStore(middleware) toolbarStore.dispatch(CopyToClipboardClicked) @@ -743,8 +727,12 @@ class CustomTabBrowserToolbarMiddlewareTest { trackingProtectionUseCases: TrackingProtectionUseCases = this.trackingProtectionUseCases, publicSuffixList: PublicSuffixList = this.publicSuffixList, clipboard: ClipboardHandler = this.clipboard, + navController: NavController = this.navController, + closeTabDelegate: () -> Unit = this.closeTabDelegate, settings: Settings = this.settings, + scope: CoroutineScope = MainScope(), ) = CustomTabBrowserToolbarMiddleware( + uiContext = testContext, customTabId = this.customTabId, browserStore = browserStore, appStore = appStore, @@ -754,29 +742,17 @@ class CustomTabBrowserToolbarMiddlewareTest { trackingProtectionUseCases = trackingProtectionUseCases, publicSuffixList = publicSuffixList, clipboard = clipboard, + navController = navController, + closeTabDelegate = closeTabDelegate, settings = settings, + scope = scope, ) private fun buildStore( middleware: CustomTabBrowserToolbarMiddleware = buildMiddleware(), - context: Context = testContext, - lifecycleOwner: LifecycleOwner = this@CustomTabBrowserToolbarMiddlewareTest.lifecycleOwner, - navController: NavController = this@CustomTabBrowserToolbarMiddlewareTest.navController, - closeTabDelegate: () -> Unit = this@CustomTabBrowserToolbarMiddlewareTest.closeTabDelegate, ) = BrowserToolbarStore( middleware = listOf(middleware), - ).also { - it.dispatch( - EnvironmentRehydrated( - CustomTabToolbarEnvironment( - context = context, - viewLifecycleOwner = lifecycleOwner, - navController = navController, - closeTabDelegate = closeTabDelegate, - ), - ), - ) - } + ) private fun assertPageOriginEquals(expected: PageOrigin, actual: PageOrigin) { assertEquals(expected.hint, actual.hint) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/home/toolbar/BrowserToolbarMiddlewareTest.kt @@ -5,18 +5,15 @@ package org.mozilla.fenix.home.toolbar import android.content.Context -import android.content.res.Configuration import android.os.Looper -import androidx.fragment.app.Fragment -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleOwner -import androidx.lifecycle.LifecycleRegistry import androidx.navigation.NavController import androidx.test.ext.junit.runners.AndroidJUnit4 import io.mockk.every import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.MainScope import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.SearchAction.ApplicationSearchEnginesLoaded import mozilla.components.browser.state.action.TabListAction.AddTabAction @@ -46,13 +43,12 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.B import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuButton.Icon.DrawableResIcon import mozilla.components.compose.browser.toolbar.store.BrowserToolbarMenuItem.BrowserToolbarMenuButton.Text.StringResText import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainLooperTestRule import mozilla.components.support.utils.ClipboardHandler import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull @@ -80,7 +76,6 @@ import org.mozilla.fenix.components.appstate.SupportedMenuNotifications import org.mozilla.fenix.components.appstate.search.SearchState import org.mozilla.fenix.components.appstate.search.SelectedSearchEngine import org.mozilla.fenix.components.menu.MenuAccessPoint -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.components.usecases.FenixBrowserUseCases import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.settings @@ -98,6 +93,7 @@ import org.mozilla.fenix.search.fixtures.assertSearchSelectorEquals import org.mozilla.fenix.search.fixtures.buildExpectedSearchSelector import org.mozilla.fenix.settings.ShortcutType import org.mozilla.fenix.tabstray.Page +import org.mozilla.fenix.utils.Settings import org.robolectric.Shadows.shadowOf import mozilla.components.ui.icons.R as iconsR import mozilla.components.ui.tabcounter.R as tabcounterR @@ -111,12 +107,8 @@ class BrowserToolbarMiddlewareTest { val gleanRule = FenixGleanTestRule(testContext) private val browserStore = BrowserStore() - private val lifecycleOwner = FakeLifecycleOwner(Lifecycle.State.RESUMED) private val browsingModeManager = SimpleBrowsingModeManager(Normal) - private val mockContext: Context = mockk(relaxed = true) private lateinit var appStore: AppStore - private lateinit var fragment: Fragment - private lateinit var configuration: Configuration @Before fun setup() = runTest { @@ -126,28 +118,11 @@ class BrowserToolbarMiddlewareTest { every { testContext.settings().tabManagerEnhancementsEnabled } returns false every { testContext.settings().shouldShowToolbarCustomization } returns false every { testContext.settings().toolbarExpandedShortcutKey } returns ShortcutType.BOOKMARK.value - - fragment = spyk(Fragment()).apply { - every { context } returns mockContext - } - every { fragment.viewLifecycleOwner } returns lifecycleOwner - configuration = Configuration().apply { - screenHeightDp = 700 - screenWidthDp = 400 - } - every { mockContext.resources.configuration } returns configuration } @Test fun `WHEN initializing the toolbar THEN add browser end actions`() = runTest { - val middleware = BrowserToolbarMiddleware( - appStore, - browserStore, - mockk(), - mockk(), - ) - - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(2, toolbarBrowserActions.size) @@ -160,9 +135,8 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN initializing the toolbar AND should use expanded toolbar THEN don't add browser end actions`() = runTest { every { testContext.settings().shouldUseExpandedToolbar } returns true - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(0, toolbarBrowserActions.size) @@ -171,9 +145,8 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN initializing the navigation bar AND should use expanded toolbar THEN add navigation bar actions`() = runTest { every { testContext.settings().shouldUseExpandedToolbar } returns true - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(5, navigationActions.size) @@ -194,15 +167,11 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN initializing the navigation bar AND should use expanded toolbar AND window is short THEN add no navigation bar actions`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration every { testContext.settings().shouldUseExpandedToolbar } returns true - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + isWideScreen = { true }, + ) val navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(0, navigationActions.size) @@ -219,9 +188,14 @@ class BrowserToolbarMiddlewareTest { ), ) every { testContext.settings().shouldUseExpandedToolbar } returns true - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + var isWideScreen = false + var isTallScreen = true + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) var navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(5, navigationActions.size) @@ -229,11 +203,8 @@ class BrowserToolbarMiddlewareTest { var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(0, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration + isWideScreen = true + isTallScreen = false appStore.dispatch(AppAction.OrientationChange(Landscape)) mainLooperRule.idle() @@ -252,10 +223,9 @@ class BrowserToolbarMiddlewareTest { tabs = listOf(createTab("test.com", private = false)), ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + browserStore = browserStore, browsingModeManager = browsingModeManager, ) @@ -275,10 +245,8 @@ class BrowserToolbarMiddlewareTest { ), ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + browserStore = browserStore, browsingModeManager = browsingModeManager, ) @@ -296,9 +264,7 @@ class BrowserToolbarMiddlewareTest { contextualMenuOptions = listOf(PasteFromClipboard, LoadFromClipboard), onClick = OriginClicked, ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val originConfiguration = toolbarStore.state.displayState.pageOrigin assertEquals(expectedConfiguration, originConfiguration) @@ -306,41 +272,30 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN clicking on the URL THEN record telemetry`() { - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() toolbarStore.dispatch(toolbarStore.state.displayState.pageOrigin.onClick as BrowserToolbarAction) assertEquals("HOME", Events.searchBarTapped.testGetValue()?.last()?.extra?.get("source")) } - @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val store = buildStore(middleware) - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } - // Testing updated configuration @Test fun `GIVEN tall window WHEN changing to short window THEN show browser end actions`() = runTest { - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + var isWideScreen = false + var isTallScreen = true + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) mainLooperRule.idle() var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(2, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration + isWideScreen = true + isTallScreen = false appStore.dispatch(AppAction.OrientationChange(Landscape)) mainLooperRule.idle() @@ -354,22 +309,19 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN short window WHEN changing to tall window THEN show all browser end actions`() = runTest { - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + var isWideScreen = true + var isTallScreen = false + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) mainLooperRule.idle() var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(2, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 700 - screenWidthDp = 400 - } - every { mockContext.resources.configuration } returns configuration + isWideScreen = false + isTallScreen = true appStore.dispatch(AppAction.OrientationChange(Portrait)) mainLooperRule.idle() @@ -385,24 +337,21 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN expanded toolbar with tabstrip and tall window WHEN changing to short window THEN show tab counter and menu`() = runTest { every { testContext.settings().shouldUseExpandedToolbar } returns true every { testContext.settings().isTabStripEnabled } returns true - configuration = Configuration().apply { - screenHeightDp = 700 - screenWidthDp = 400 - } - every { mockContext.resources.configuration } returns configuration - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + var isWideScreen = false + var isTallScreen = true + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + isWideScreen = { isWideScreen }, + isTallScreen = { isTallScreen }, + ) mainLooperRule.idle() var navigationActions = toolbarStore.state.displayState.navigationActions assertEquals(5, navigationActions.size) var toolbarBrowserActions = toolbarStore.state.displayState.browserActionsEnd assertEquals(0, toolbarBrowserActions.size) - configuration = Configuration().apply { - screenHeightDp = 400 - screenWidthDp = 700 - } - every { mockContext.resources.configuration } returns configuration + isWideScreen = true + isTallScreen = false appStore.dispatch(AppAction.OrientationChange(Portrait)) mainLooperRule.idle() @@ -420,9 +369,8 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN in normal browsing WHEN the number of normal opened tabs is modified THEN update the tab counter`() = runTest { val browsingModeManager = SimpleBrowsingModeManager(Normal) val browserStore = BrowserStore() - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + browserStore = browserStore, browsingModeManager = browsingModeManager, ) mainLooperRule.idle() @@ -453,9 +401,8 @@ class BrowserToolbarMiddlewareTest { tabs = listOf(initialNormalTab, initialPrivateTab), ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + browserStore = browserStore, browsingModeManager = browsingModeManager, ) mainLooperRule.idle() @@ -478,9 +425,7 @@ class BrowserToolbarMiddlewareTest { @Test fun `WHEN clicking the menu button THEN open the menu`() { val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, ) val menuButton = toolbarStore.state.displayState.browserActionsEnd[1] as ActionButtonRes @@ -501,9 +446,7 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN browsing in normal mode WHEN clicking the tab counter button THEN open the tabs tray in normal mode`() { val browsingModeManager = SimpleBrowsingModeManager(Normal) val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, browsingModeManager = browsingModeManager, ) @@ -523,9 +466,7 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN browsing in private mode WHEN clicking the tab counter button THEN open the tabs tray in private mode`() { val browsingModeManager = SimpleBrowsingModeManager(Private) val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, browsingModeManager = browsingModeManager, ) @@ -545,9 +486,7 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN browsing in normal mode WHEN clicking on the long click menu option THEN open a new private tab`() { val browsingModeManager = SimpleBrowsingModeManager(Normal) val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, browsingModeManager = browsingModeManager, ) @@ -566,9 +505,7 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN browsing in private mode WHEN clicking on the long click menu option THEN open a new normal tab`() { val browsingModeManager = SimpleBrowsingModeManager(Private) val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, browsingModeManager = browsingModeManager, ) @@ -587,9 +524,7 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN in normal browsing mode WHEN the page origin is clicked THEN start the search UX for normal browsing`() { val browsingModeManager = SimpleBrowsingModeManager(Normal) val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, browsingModeManager = browsingModeManager, ) @@ -603,9 +538,7 @@ class BrowserToolbarMiddlewareTest { fun `GIVEN in private browsing mode WHEN the page origin is clicked THEN start the search UX for private browsing`() { val browsingModeManager = SimpleBrowsingModeManager(Private) val navController: NavController = mockk(relaxed = true) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( navController = navController, browsingModeManager = browsingModeManager, ) @@ -621,10 +554,8 @@ class BrowserToolbarMiddlewareTest { val clipboard = ClipboardHandler(testContext).also { it.text = "test" } - val middleware = BrowserToolbarMiddleware(appStore, browserStore, clipboard, mockk()) - val toolbarStore = buildStore( - middleware = middleware, - navController = mockk(), + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + clipboard = clipboard, browsingModeManager = browsingModeManager, ) @@ -647,9 +578,9 @@ class BrowserToolbarMiddlewareTest { every { fenixBrowserUseCases } returns browserUseCases } val selectedSearchEngine = appStore.state.searchState.selectedSearchEngine?.searchEngine - val middleware = BrowserToolbarMiddleware(appStore, browserStore, clipboard, useCases) - val toolbarStore = buildStore( - middleware = middleware, + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + clipboard = clipboard, + useCases = useCases, navController = navController, browsingModeManager = browsingModeManager, ) @@ -671,8 +602,9 @@ class BrowserToolbarMiddlewareTest { fun `WHEN the selected search engine changes THEN update the search selector`() { val appStore = AppStore() - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + ) val newSearchEngine = SearchEngine("test", "Test", mock(), type = APPLICATION) appStore.dispatch(SearchEngineSelected(newSearchEngine, true)) @@ -695,10 +627,12 @@ class BrowserToolbarMiddlewareTest { ), ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + ) browserStore.dispatch(ApplicationSearchEnginesLoaded(listOf(otherSearchEngine))) + mainLooperRule.idle() assertNotEquals( appStore.state.searchState.selectedSearchEngine?.searchEngine, @@ -722,8 +656,9 @@ class BrowserToolbarMiddlewareTest { ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - buildStore(middleware) + val (middleware, _) = buildMiddlewareAndAddToStore( + browserStore = browserStore, + ) val action = middleware.buildHomeAction( action = HomeToolbarAction.TabCounter, @@ -734,18 +669,14 @@ class BrowserToolbarMiddlewareTest { testContext.getString(tabcounterR.string.mozac_tab_counter_open_tab_tray, 3), action.contentDescription, ) - assertEquals( - middleware.environment?.browsingModeManager?.mode == Private, - action.showPrivacyMask, - ) + assertFalse(action.showPrivacyMask) assertEquals(TabCounterClicked(Source.AddressBar), action.onClick) assertNotNull(action.onLongClick) } @Test fun `WHEN building Menu action THEN returns Menu ActionButton`() { - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - buildStore(middleware) + val (middleware, _) = buildMiddlewareAndAddToStore() val action = middleware.buildHomeAction( action = HomeToolbarAction.Menu, @@ -761,8 +692,9 @@ class BrowserToolbarMiddlewareTest { @Test fun `GIVEN the menu button is not highlighted WHEN a menu item is highlighted THEN highlight menu button`() = runTest { val appStore = AppStore() - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + ) mainLooperRule.idle() val initialMenuButton = toolbarStore.state.displayState.browserActionsEnd[1] as ActionButtonRes @@ -785,8 +717,9 @@ class BrowserToolbarMiddlewareTest { supportedMenuNotifications = setOf(SupportedMenuNotifications.Downloads), ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + ) mainLooperRule.idle() val initialMenuButton = toolbarStore.state.displayState.browserActionsEnd[1] as ActionButtonRes @@ -809,8 +742,9 @@ class BrowserToolbarMiddlewareTest { supportedMenuNotifications = setOf(SupportedMenuNotifications.OpenInApp), ), ) - val middleware = BrowserToolbarMiddleware(appStore, browserStore, mockk(), mockk()) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore( + appStore = appStore, + ) mainLooperRule.idle() val menuButton = toolbarStore.state.displayState.browserActionsEnd[1] as ActionButtonRes @@ -823,13 +757,7 @@ class BrowserToolbarMiddlewareTest { every { testContext.settings().shouldUseExpandedToolbar } returns true every { testContext.settings().toolbarExpandedShortcutKey } returns ShortcutType.TRANSLATE.value - val middleware = BrowserToolbarMiddleware( - appStore, - browserStore, - mockk(), - mockk(), - ) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val translateButton = toolbarStore.state.displayState.navigationActions.first() as ActionButtonRes assertEquals(expectedTranslateButton, translateButton) @@ -841,13 +769,7 @@ class BrowserToolbarMiddlewareTest { every { testContext.settings().shouldUseExpandedToolbar } returns true every { testContext.settings().toolbarExpandedShortcutKey } returns ShortcutType.HOMEPAGE.value - val middleware = BrowserToolbarMiddleware( - appStore, - browserStore, - mockk(), - mockk(), - ) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val homepageButton = toolbarStore.state.displayState.navigationActions.first() as ActionButtonRes assertEquals(expectedHomepageButton, homepageButton) @@ -859,13 +781,7 @@ class BrowserToolbarMiddlewareTest { every { testContext.settings().shouldUseExpandedToolbar } returns true every { testContext.settings().toolbarExpandedShortcutKey } returns ShortcutType.BACK.value - val middleware = BrowserToolbarMiddleware( - appStore, - browserStore, - mockk(), - mockk(), - ) - val toolbarStore = buildStore(middleware) + val (_, toolbarStore) = buildMiddlewareAndAddToStore() val backButton = toolbarStore.state.displayState.navigationActions.first() as ActionButtonRes assertEquals(expectedBackButton, backButton) @@ -899,24 +815,71 @@ class BrowserToolbarMiddlewareTest { ) } - private fun buildStore( - middleware: BrowserToolbarMiddleware, - context: Context = testContext, + private fun buildMiddlewareAndAddToStore( + uiContext: Context = testContext, + appStore: AppStore = this.appStore, + browserStore: BrowserStore = this.browserStore, + clipboard: ClipboardHandler = mockk(), + useCases: UseCases = mockk(), + navController: NavController = mockk(), + browsingModeManager: BrowsingModeManager = this.browsingModeManager, + settings: Settings = testContext.settings(), + isWideScreen: () -> Boolean = { false }, + isTallScreen: () -> Boolean = { true }, + scope: CoroutineScope = MainScope(), + ): Pair<BrowserToolbarMiddleware, BrowserToolbarStore> { + val middleware = buildMiddleware( + uiContext = uiContext, + appStore = appStore, + browserStore = browserStore, + clipboard = clipboard, + useCases = useCases, + navController = navController, + browsingModeManager = browsingModeManager, + settings = settings, + isWideScreen = isWideScreen, + isTallScreen = isTallScreen, + scope = scope, + ) + val store = buildStore( + middleware = middleware, + ) + + return middleware to store + } + + private fun buildMiddleware( + uiContext: Context = testContext, + appStore: AppStore = this.appStore, + browserStore: BrowserStore = this.browserStore, + clipboard: ClipboardHandler = mockk(), + useCases: UseCases = mockk(), navController: NavController = mockk(), browsingModeManager: BrowsingModeManager = this.browsingModeManager, + settings: Settings = testContext.settings(), + isWideScreen: () -> Boolean = { false }, + isTallScreen: () -> Boolean = { true }, + scope: CoroutineScope = MainScope(), + ) = BrowserToolbarMiddleware( + uiContext = uiContext, + appStore = appStore, + browserStore = browserStore, + clipboard = clipboard, + useCases = useCases, + navController = navController, + browsingModeManager = browsingModeManager, + settings = settings, + isWideScreen = isWideScreen, + isTallScreen = isTallScreen, + scope = scope, + ) + + private fun buildStore( + middleware: BrowserToolbarMiddleware, ) = BrowserToolbarStore( middleware = listOf(middleware), ).also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - context = context, - fragment = fragment, - navController = navController, - browsingModeManager = browsingModeManager, - ), - ), - ) + mainLooperRule.idle() // to complete the initial setup happening in coroutines } private fun expectedSearchSelector( @@ -1045,10 +1008,4 @@ class BrowserToolbarMiddlewareTest { state = ActionButton.State.DISABLED, onClick = FakeClicked, ) - - private class FakeLifecycleOwner(initialState: Lifecycle.State) : LifecycleOwner { - override val lifecycle: Lifecycle = LifecycleRegistry(this).apply { - currentState = initialState - } - } } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserStoreToFenixSearchMapperMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserStoreToFenixSearchMapperMiddlewareTest.kt @@ -4,29 +4,26 @@ package org.mozilla.fenix.search -import androidx.lifecycle.Lifecycle.State.RESUMED import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.SearchAction.ApplicationSearchEnginesLoaded import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.SearchState import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull import org.junit.Test import org.junit.runner.RunWith -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentCleared -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentRehydrated import org.mozilla.fenix.search.fixtures.EMPTY_SEARCH_FRAGMENT_STATE import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class BrowserStoreToFenixSearchMapperMiddlewareTest { + @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `WHEN the browser search state changes THEN update the application search state`() { + fun `WHEN the browser search state changes THEN update the application search state`() = runTest(UnconfinedTestDispatcher()) { val defaultSearchEngine: SearchEngine = mockk() val newSearchEngines: List<SearchEngine> = listOf(defaultSearchEngine, mockk()) val browserStore = BrowserStore( @@ -36,7 +33,7 @@ class BrowserStoreToFenixSearchMapperMiddlewareTest { ), ), ) - val middleware = BrowserStoreToFenixSearchMapperMiddleware(browserStore) + val middleware = BrowserStoreToFenixSearchMapperMiddleware(browserStore, backgroundScope) val searchStore = buildStore(middleware) browserStore.dispatch(ApplicationSearchEnginesLoaded(newSearchEngines)) @@ -44,31 +41,8 @@ class BrowserStoreToFenixSearchMapperMiddlewareTest { assertEquals(defaultSearchEngine, searchStore.state.defaultEngine) } - @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val middleware = BrowserStoreToFenixSearchMapperMiddleware(mockk(relaxed = true)) - val store = buildStore(middleware) - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } - private fun buildStore(middleware: BrowserStoreToFenixSearchMapperMiddleware) = SearchFragmentStore( initialState = EMPTY_SEARCH_FRAGMENT_STATE, middleware = listOf(middleware), - ).also { - it.dispatch( - EnvironmentRehydrated( - SearchFragmentStore.Environment( - context = testContext, - viewLifecycleOwner = TestLifecycleOwner(RESUMED), - browsingModeManager = mockk(), - navController = mockk(), - ), - ), - ) - } + ) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserToolbarSearchMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserToolbarSearchMiddlewareTest.kt @@ -4,10 +4,8 @@ package org.mozilla.fenix.search +import android.content.Context import android.os.Looper -import androidx.fragment.app.Fragment -import androidx.lifecycle.Lifecycle.State.RESUMED -import androidx.lifecycle.LifecycleOwner import androidx.navigation.NavController import androidx.navigation.NavDirections import io.mockk.Runs @@ -20,7 +18,9 @@ import io.mockk.slot import io.mockk.spyk import io.mockk.verify import io.mockk.verifyOrder +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.MainScope import mozilla.components.browser.domains.autocomplete.BaseDomainAutocompleteProvider import mozilla.components.browser.state.action.AwesomeBarAction.EngagementFinished import mozilla.components.browser.state.action.SearchAction.ApplicationSearchEnginesLoaded @@ -40,8 +40,6 @@ import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.Ent import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.ExitEditMode import mozilla.components.compose.browser.toolbar.store.BrowserToolbarInteraction.BrowserToolbarEvent import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.EngineSession @@ -49,17 +47,16 @@ import mozilla.components.concept.toolbar.AutocompleteProvider import mozilla.components.concept.toolbar.AutocompleteResult import mozilla.components.feature.awesomebar.provider.SessionAutocompleteProvider import mozilla.components.feature.syncedtabs.SyncedTabsAutocompleteProvider -import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.middleware.CaptureActionsMiddleware import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainLooperTestRule import mozilla.telemetry.glean.testing.GleanTestRule import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue -import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -85,9 +82,7 @@ import org.mozilla.fenix.components.appstate.search.SelectedSearchEngine import org.mozilla.fenix.components.search.BOOKMARKS_SEARCH_ENGINE_ID import org.mozilla.fenix.components.search.HISTORY_SEARCH_ENGINE_ID import org.mozilla.fenix.components.search.TABS_SEARCH_ENGINE_ID -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment import org.mozilla.fenix.components.usecases.FenixBrowserUseCases -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner import org.mozilla.fenix.search.EditPageEndActionsInteractions.ClearSearchClicked import org.mozilla.fenix.search.EditPageEndActionsInteractions.QrScannerClicked import org.mozilla.fenix.search.EditPageEndActionsInteractions.VoiceSearchButtonClicked @@ -116,38 +111,20 @@ class BrowserToolbarSearchMiddlewareTest { @get:Rule val gleanTestRule = GleanTestRule(testContext) + @get:Rule + val mainLooperRule = MainLooperTestRule() + val appStore = AppStore() val browserStore: BrowserStore = mockk(relaxed = true) { every { state.search } returns fakeSearchState() } val components: Components = mockk() val settings: Settings = mockk(relaxed = true) - val lifecycleOwner: LifecycleOwner = TestLifecycleOwner(RESUMED) val navController: NavController = mockk { every { navigate(any<NavDirections>()) } just Runs every { navigate(any<Int>()) } just Runs } val browsingModeManager: BrowsingModeManager = mockk() - private lateinit var fragment: Fragment - - @Before - fun setup() { - fragment = spyk(Fragment()).apply { - every { context } returns testContext - } - every { fragment.getViewLifecycleOwner() } returns lifecycleOwner - } - - @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val (middleware, store) = buildMiddlewareAndAddToStore() - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } @Test fun `WHEN the toolbar enters in edit mode THEN a new search selector button is added`() { @@ -332,7 +309,7 @@ class BrowserToolbarSearchMiddlewareTest { } every { components.core.engine } returns engine configureAutocompleteProvidersInComponents() - val middleware = spyk(buildMiddleware(appStore, browserStore, components, settings)) + val middleware = spyk(buildMiddleware()) every { middleware.isSpeechRecognitionAvailable() } returns true val store = buildStore(middleware) val autocompleteProvidersSlot = slot<List<AutocompleteProvider>>() @@ -383,7 +360,7 @@ class BrowserToolbarSearchMiddlewareTest { } every { components.core.engine } returns engine configureAutocompleteProvidersInComponents() - val middleware = spyk(buildMiddleware(appStore, browserStore, components, settings)) + val middleware = spyk(buildMiddleware()) val store = buildStore(middleware) val autocompleteProvidersSlot = slot<List<AutocompleteProvider>>() @@ -426,7 +403,7 @@ class BrowserToolbarSearchMiddlewareTest { } every { components.core.engine } returns engine configureAutocompleteProvidersInComponents() - val middleware = spyk(buildMiddleware(appStore, browserStore, components, settings)) + val middleware = spyk(buildMiddleware()) val store = buildStore(middleware) val autocompleteProvidersSlot = slot<List<AutocompleteProvider>>() @@ -470,7 +447,7 @@ class BrowserToolbarSearchMiddlewareTest { } every { components.core.engine } returns engine configureAutocompleteProvidersInComponents() - val middleware = spyk(buildMiddleware(appStore, browserStore, components, settings)) + val middleware = spyk(buildMiddleware()) val store = buildStore(middleware) val autocompleteProvidersSlot = slot<List<AutocompleteProvider>>() @@ -715,12 +692,12 @@ class BrowserToolbarSearchMiddlewareTest { ), ) val browserStore = BrowserStore() - val (_, store) = buildMiddlewareAndAddToStore(appStore, browserStore) + val (_, store) = buildMiddlewareAndAddToStore(testContext, appStore, browserStore) store.dispatch(EnterEditMode) val newSearchEngines = fakeSearchState().applicationSearchEngines browserStore.dispatch(ApplicationSearchEnginesLoaded(newSearchEngines)) - shadowOf(Looper.getMainLooper()).idle() // wait for observing and processing the search engines update + mainLooperRule.idle() assertSearchSelectorEquals( expectedSearchSelector(selectedSearchEngine, newSearchEngines), @@ -985,7 +962,7 @@ class BrowserToolbarSearchMiddlewareTest { store.dispatch(qrScannerButton.onClick as BrowserToolbarEvent) appStore.dispatch(QrScannerInputAvailable("mozilla.test")) - shadowOf(Looper.getMainLooper()).idle() // wait for observing and processing qr scan result + mainLooperRule.idle() assertEquals("mozilla.test", store.state.editState.query.current) appStoreActionsCaptor.assertLastAction(QrScannerInputConsumed::class) @@ -1019,7 +996,7 @@ class BrowserToolbarSearchMiddlewareTest { store.dispatch(qrScannerButton.onClick as BrowserToolbarEvent) appStore.dispatch(QrScannerInputAvailable("test.mozilla")) - shadowOf(Looper.getMainLooper()).idle() // wait for observing and processing qr scan result + mainLooperRule.idle() assertEquals("test.mozilla", store.state.editState.query.current) appStoreActionsCaptor.assertLastAction(QrScannerInputConsumed::class) @@ -1058,7 +1035,7 @@ class BrowserToolbarSearchMiddlewareTest { store.dispatch(qrScannerButton.onClick as BrowserToolbarEvent) appStore.dispatch(QrScannerInputAvailable("test.com")) - shadowOf(Looper.getMainLooper()).idle() // wait for observing and processing qr scan result + mainLooperRule.idle() assertEquals("test.com", store.state.editState.query.current) appStoreActionsCaptor.assertLastAction(QrScannerInputConsumed::class) @@ -1120,41 +1097,56 @@ class BrowserToolbarSearchMiddlewareTest { ) private fun buildMiddlewareAndAddToStore( + uiContext: Context = testContext, appStore: AppStore = this.appStore, browserStore: BrowserStore = this.browserStore, components: Components = this.components, - settings: Settings = this.settings, navController: NavController = this.navController, browsingModeManager: BrowsingModeManager = this.browsingModeManager, + settings: Settings = this.settings, + scope: CoroutineScope = MainScope(), ): Pair<BrowserToolbarSearchMiddleware, BrowserToolbarStore> { - val middleware = buildMiddleware(appStore, browserStore, components, settings) - val store = buildStore(middleware, navController, browsingModeManager) + val middleware = buildMiddleware( + uiContext = uiContext, + appStore = appStore, + browserStore = browserStore, + components = components, + navController = navController, + browsingModeManager = browsingModeManager, + settings = settings, + scope = scope, + ) + val store = buildStore(middleware) return middleware to store } private fun buildStore( middleware: BrowserToolbarSearchMiddleware = buildMiddleware(), - navController: NavController = this.navController, - browsingModeManager: BrowsingModeManager = this.browsingModeManager, ) = BrowserToolbarStore( - middleware = listOf(middleware), - ).also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - testContext, fragment, navController, browsingModeManager, - ), - ), - ) - } + middleware = listOf(middleware), + ) private fun buildMiddleware( + uiContext: Context = testContext, appStore: AppStore = this.appStore, browserStore: BrowserStore = this.browserStore, components: Components = this.components, + navController: NavController = this.navController, + browsingModeManager: BrowsingModeManager = this.browsingModeManager, settings: Settings = this.settings, - ) = BrowserToolbarSearchMiddleware(appStore, browserStore, components, settings, Dispatchers.Main) + scope: CoroutineScope = MainScope(), + ) = BrowserToolbarSearchMiddleware( + uiContext = uiContext, + appStore = appStore, + browserStore = browserStore, + components = components, + navController = navController, + browsingModeManager = browsingModeManager, + settings = settings, + scope = scope, + autocompleteDispatcher = Dispatchers.Main, + ) private fun configureAutocompleteProvidersInComponents() { val autocompleteSuggestion = AutocompleteResult( diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserToolbarSearchStatusSyncMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserToolbarSearchStatusSyncMiddlewareTest.kt @@ -4,25 +4,17 @@ package org.mozilla.fenix.search -import androidx.fragment.app.Fragment -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleOwner import io.mockk.every import io.mockk.mockk -import io.mockk.spyk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.MainScope import kotlinx.coroutines.test.runTest import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.EnterEditMode import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.ExitEditMode import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore -import mozilla.components.compose.browser.toolbar.store.EnvironmentCleared -import mozilla.components.compose.browser.toolbar.store.EnvironmentRehydrated -import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainLooperTestRule import org.junit.Assert.assertFalse -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull import org.junit.Assert.assertTrue -import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -31,8 +23,6 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction.SearchAction.SearchEnded import org.mozilla.fenix.components.appstate.AppAction.SearchAction.SearchStarted -import org.mozilla.fenix.components.toolbar.BrowserToolbarEnvironment -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) @@ -42,26 +32,7 @@ class BrowserToolbarSearchStatusSyncMiddlewareTest { val mainLooperRule = MainLooperTestRule() private val appStore = AppStore() - private val lifecycleOwner: LifecycleOwner = TestLifecycleOwner(Lifecycle.State.RESUMED) private val browsingModeManager: BrowsingModeManager = mockk(relaxed = true) - private lateinit var fragment: Fragment - - @Before - fun setup() { - fragment = spyk(Fragment()) - every { fragment.getViewLifecycleOwner() } returns lifecycleOwner - } - - @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val (middleware, toolbarStore) = buildMiddlewareAndAddToSearchStore() - - assertNotNull(middleware.environment) - - toolbarStore.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - } @Test fun `WHEN the toolbar exits search mode THEN synchronize search being ended for the application`() = runTest { @@ -134,26 +105,19 @@ class BrowserToolbarSearchStatusSyncMiddlewareTest { private fun buildMiddlewareAndAddToSearchStore( appStore: AppStore = this.appStore, + browsingModeManager: BrowsingModeManager = this.browsingModeManager, + scope: CoroutineScope = MainScope(), ): Pair<BrowserToolbarSearchStatusSyncMiddleware, BrowserToolbarStore> { - val middleware = buildMiddleware(appStore) + val middleware = buildMiddleware(appStore, browsingModeManager, scope) val toolbarStore = BrowserToolbarStore( middleware = listOf(middleware), - ).also { - it.dispatch( - EnvironmentRehydrated( - BrowserToolbarEnvironment( - context = testContext, - navController = mockk(), - fragment = fragment, - browsingModeManager = browsingModeManager, - ), - ), - ) - } + ) return middleware to toolbarStore } private fun buildMiddleware( appStore: AppStore = this.appStore, - ) = BrowserToolbarSearchStatusSyncMiddleware(appStore) + browsingModeManager: BrowsingModeManager = this.browsingModeManager, + scope: CoroutineScope = MainScope(), + ) = BrowserToolbarSearchStatusSyncMiddleware(appStore, browsingModeManager, scope) } diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserToolbarToFenixSearchMapperMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/BrowserToolbarToFenixSearchMapperMiddlewareTest.kt @@ -4,33 +4,41 @@ package org.mozilla.fenix.search -import androidx.lifecycle.Lifecycle import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import mozilla.components.browser.state.selector.selectedTab +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.createTab +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.compose.browser.toolbar.store.BrowserEditToolbarAction.SearchQueryUpdated import mozilla.components.compose.browser.toolbar.store.BrowserToolbarAction.EnterEditMode import mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore import mozilla.components.compose.browser.toolbar.ui.BrowserToolbarQuery import mozilla.components.lib.state.Middleware import mozilla.components.support.test.middleware.CaptureActionsMiddleware -import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainLooperTestRule import org.junit.Assert.assertEquals -import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentCleared -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentRehydrated import org.mozilla.fenix.search.SearchFragmentAction.SearchStarted import org.mozilla.fenix.search.fixtures.EMPTY_SEARCH_FRAGMENT_STATE import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class BrowserToolbarToFenixSearchMapperMiddlewareTest { + @get:Rule + val mainLooperRule = MainLooperTestRule() + val toolbarStore = BrowserToolbarStore() private val browsingModeManager: BrowsingModeManager = mockk { every { mode } returns BrowsingMode.Private @@ -43,6 +51,7 @@ class BrowserToolbarToFenixSearchMapperMiddlewareTest { val searchStore = buildSearchStore(listOf(searchStatusMapperMiddleware, captorMiddleware)) toolbarStore.dispatch(EnterEditMode) + mainLooperRule.idle() captorMiddleware.assertLastAction(SearchStarted::class) { assertNull(it.selectedSearchEngine) @@ -51,52 +60,55 @@ class BrowserToolbarToFenixSearchMapperMiddlewareTest { } @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null`() { - val searchStatusMapperMiddleware = buildMiddleware() - val searchStore = buildSearchStore(listOf(searchStatusMapperMiddleware)) - - assertNotNull(searchStatusMapperMiddleware.environment) - - searchStore.dispatch(EnvironmentCleared) - - assertNull(searchStatusMapperMiddleware.environment) - } - - @Test fun `GIVEN search was started WHEN there's a new query in the toolbar THEN update the search state`() { val searchStore = buildSearchStore(listOf(buildMiddleware())) toolbarStore.dispatch(EnterEditMode) searchStore.dispatch(SearchStarted(mockk(), false, false, searchStartedForCurrentUrl = false)) + mainLooperRule.idle() toolbarStore.dispatch(SearchQueryUpdated(BrowserToolbarQuery("t"))) + mainLooperRule.idle() assertEquals("t", searchStore.state.query) toolbarStore.dispatch(SearchQueryUpdated(BrowserToolbarQuery("te"))) + mainLooperRule.idle() assertEquals("te", searchStore.state.query) toolbarStore.dispatch(SearchQueryUpdated(BrowserToolbarQuery("tes"))) + mainLooperRule.idle() assertEquals("tes", searchStore.state.query) toolbarStore.dispatch(SearchQueryUpdated(BrowserToolbarQuery("test"))) + mainLooperRule.idle() assertEquals("test", searchStore.state.query) } @Test fun `GIVEN search was started for the current URL WHEN there's a new query in the toolbar THEN don't update the search state`() { - val searchStore = buildSearchStore(listOf(buildMiddleware())) + val currentTab = createTab("https://mozilla.org") + val browserStore = BrowserStore( + BrowserState( + tabs = listOf(currentTab), + selectedTabId = currentTab.id, + ), + ) + val searchStore = buildSearchStore(listOf(buildMiddleware(browserStore = browserStore))) toolbarStore.dispatch(EnterEditMode) - searchStore.dispatch(SearchStarted(mockk(), false, false, searchStartedForCurrentUrl = true)) toolbarStore.dispatch( SearchQueryUpdated(BrowserToolbarQuery("https://mozilla.org"), isQueryPrefilled = true), ) + searchStore.dispatch(SearchStarted(mockk(), false, false, searchStartedForCurrentUrl = true)) + mainLooperRule.idle() assertEquals("", searchStore.state.query) toolbarStore.dispatch(SearchQueryUpdated(BrowserToolbarQuery("t"))) + mainLooperRule.idle() assertEquals("t", searchStore.state.query) toolbarStore.dispatch(SearchQueryUpdated(BrowserToolbarQuery("https://mozilla.org"))) + mainLooperRule.idle() assertEquals("https://mozilla.org", searchStore.state.query) } @@ -105,22 +117,14 @@ class BrowserToolbarToFenixSearchMapperMiddlewareTest { ) = SearchFragmentStore( initialState = emptySearchState, middleware = middlewares, - ).also { - it.dispatch( - EnvironmentRehydrated( - SearchFragmentStore.Environment( - context = testContext, - viewLifecycleOwner = TestLifecycleOwner(Lifecycle.State.RESUMED), - browsingModeManager = browsingModeManager, - navController = mockk(), - ), - ), - ) - } + ) private fun buildMiddleware( toolbarStore: BrowserToolbarStore = this.toolbarStore, - ) = BrowserToolbarToFenixSearchMapperMiddleware(toolbarStore) + browsingModeManager: BrowsingModeManager = this.browsingModeManager, + scope: CoroutineScope = MainScope(), + browserStore: BrowserStore? = null, + ) = BrowserToolbarToFenixSearchMapperMiddleware(toolbarStore, browsingModeManager, scope, browserStore) private val emptySearchState = EMPTY_SEARCH_FRAGMENT_STATE.copy( searchEngineSource = mockk(), diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/FenixSearchMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/search/FenixSearchMiddlewareTest.kt @@ -4,8 +4,10 @@ package org.mozilla.fenix.search -import android.os.Looper -import androidx.lifecycle.Lifecycle.State.RESUMED +import androidx.fragment.app.Fragment +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry import androidx.navigation.NavController import androidx.navigation.NavDirections import io.mockk.Runs @@ -14,6 +16,7 @@ import io.mockk.just import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.test.runTest import mozilla.components.browser.state.action.AwesomeBarAction import mozilla.components.browser.state.action.AwesomeBarAction.EngagementFinished import mozilla.components.browser.state.action.BrowserAction @@ -35,6 +38,7 @@ import mozilla.components.lib.state.MiddlewareContext import mozilla.components.lib.state.Store import mozilla.components.support.test.middleware.CaptureActionsMiddleware import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainLooperTestRule import mozilla.telemetry.glean.testing.GleanTestRule import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -62,25 +66,20 @@ import org.mozilla.fenix.components.appstate.search.SelectedSearchEngine import org.mozilla.fenix.components.search.BOOKMARKS_SEARCH_ENGINE_ID import org.mozilla.fenix.components.usecases.FenixBrowserUseCases import org.mozilla.fenix.ext.telemetryName -import org.mozilla.fenix.helpers.lifecycle.TestLifecycleOwner import org.mozilla.fenix.search.SearchEngineSource.Bookmarks import org.mozilla.fenix.search.SearchEngineSource.Shortcut -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentCleared -import org.mozilla.fenix.search.SearchFragmentAction.EnvironmentRehydrated import org.mozilla.fenix.search.SearchFragmentAction.SearchProvidersUpdated import org.mozilla.fenix.search.SearchFragmentAction.SearchShortcutEngineSelected import org.mozilla.fenix.search.SearchFragmentAction.SearchStarted import org.mozilla.fenix.search.SearchFragmentAction.SearchSuggestionsVisibilityUpdated import org.mozilla.fenix.search.SearchFragmentAction.SuggestionClicked import org.mozilla.fenix.search.SearchFragmentAction.SuggestionSelected -import org.mozilla.fenix.search.SearchFragmentStore.Environment import org.mozilla.fenix.search.awesomebar.SearchSuggestionsProvidersBuilder import org.mozilla.fenix.search.fixtures.EMPTY_SEARCH_FRAGMENT_STATE import org.mozilla.fenix.telemetry.ACTION_SEARCH_ENGINE_SELECTED import org.mozilla.fenix.telemetry.SOURCE_ADDRESS_BAR import org.mozilla.fenix.utils.Settings import org.robolectric.RobolectricTestRunner -import org.robolectric.Shadows.shadowOf import org.mozilla.fenix.components.appstate.search.SearchState as AppSearchState @RunWith(RobolectricTestRunner::class) @@ -88,6 +87,9 @@ class FenixSearchMiddlewareTest { @get:Rule val gleanTestRule = GleanTestRule(testContext) + @get:Rule + val mainLooperRule = MainLooperTestRule() + private val engine: Engine = mockk { every { speculativeCreateSession(any(), any()) } just Runs } @@ -255,6 +257,7 @@ class FenixSearchMiddlewareTest { val defaultSearchEngine = fakeSearchEnginesState().selectedOrDefaultSearchEngine store.dispatch(SearchStarted(defaultSearchEngine, false, false, true)) + mainLooperRule.idle() searchActionsCaptor.assertLastAction(SearchSuggestionsVisibilityUpdated::class) { assertTrue(it.visible) @@ -309,6 +312,7 @@ class FenixSearchMiddlewareTest { every { middleware.buildSearchSuggestionsProvider(any()) } returns expectedSearchSuggestionsProvider store.dispatch(SearchStarted(null, false, false, false)) // this triggers observing the search engine updates + mainLooperRule.idle() searchActionsCaptor.assertLastAction(SearchShortcutEngineSelected::class) { assertEquals(newSearchEngineSelection, it.engine) @@ -319,7 +323,7 @@ class FenixSearchMiddlewareTest { } @Test - fun `When needing to load an URL THEN open it in browser, record search ended and record telemetry`() { + fun `WHEN needing to load an URL THEN open it in browser, record search ended and record telemetry`() { val url = "https://mozilla.com" val flags = LoadUrlFlags.all() every { settings.enableHomepageAsNewTab } returns true @@ -347,14 +351,14 @@ class FenixSearchMiddlewareTest { } @Test - fun `WHEN needing to search for specific terms THEN open them in browser, record search ended and record telemetry`() { + fun `WHEN needing to search for specific terms THEN open them in browser, record search ended and record telemetry`() = runTest { val searchTerm = "test" every { settings.enableHomepageAsNewTab } returns true val nimbusEventsStore: NimbusEventStore = mockk { every { recordEvent(any()) } just Runs } every { nimbusComponents.events } returns nimbusEventsStore - val middleware = buildMiddleware(nimbusComponents = nimbusComponents) + val middleware = buildMiddleware() val store = buildStore(middleware) val context = buildContext(store) @@ -412,7 +416,7 @@ class FenixSearchMiddlewareTest { appStore.dispatch(AppAction.SearchAction.SearchStarted()) store.dispatch(SearchStarted(defaultSearchEngine, false, false, false)) appStore.dispatch(SearchEngineSelected(searchEngineClicked, true)) - shadowOf(Looper.getMainLooper()).idle() + mainLooperRule.idle() assertEquals(Bookmarks(searchEngineClicked), store.state.searchEngineSource) assertNotNull(store.state.defaultEngine) @@ -487,18 +491,6 @@ class FenixSearchMiddlewareTest { verify { toolbarStore.dispatch(BrowserEditToolbarAction.SearchQueryUpdated(BrowserToolbarQuery("test"))) } } - @Test - fun `GIVEN an environment was already set WHEN it is cleared THEN reset it to null and clear search suggestions providers`() { - val (middleware, store) = buildMiddlewareAndAddToSearchStore() - - assertNotNull(middleware.environment) - - store.dispatch(EnvironmentCleared) - - assertNull(middleware.environment) - assertEquals(emptyList<SuggestionProvider>(), store.state.searchSuggestionsProviders) - } - private fun buildMiddlewareAndAddToSearchStore( engine: Engine = this.engine, useCases: UseCases = this.useCases, @@ -508,7 +500,13 @@ class FenixSearchMiddlewareTest { toolbarStore: BrowserToolbarStore = this.toolbarStore, ): Pair<FenixSearchMiddleware, SearchFragmentStore> { val middleware = buildMiddleware( - engine, useCases, nimbusComponents, settings, appStore, browserStore, toolbarStore, + engine = engine, + useCases = useCases, + nimbusComponents = nimbusComponents, + settings = settings, + appStore = appStore, + browserStore = browserStore, + toolbarStore = toolbarStore, ) every { middleware.buildSearchSuggestionsProvider(any()) } returns mockk(relaxed = true) @@ -525,9 +523,14 @@ class FenixSearchMiddlewareTest { appStore: AppStore = this.appStore, browserStore: BrowserStore = this.browserStore, toolbarStore: BrowserToolbarStore = this.toolbarStore, + navController: NavController = this.navController, + browsingModeManager: BrowsingModeManager = this.browsingModeManager, ): FenixSearchMiddleware { val middleware = spyk( FenixSearchMiddleware( + fragment = spyk(Fragment()) { + every { viewLifecycleOwner } returns FakeLifecycleOwner(Lifecycle.State.RESUMED) + }, engine = engine, useCases = useCases, nimbusComponents = nimbusComponents, @@ -535,6 +538,8 @@ class FenixSearchMiddlewareTest { appStore = appStore, browserStore = browserStore, toolbarStore = toolbarStore, + navController = navController, + browsingModeManager = browsingModeManager, ), ) every { middleware.buildSearchSuggestionsProvider(any()) } returns mockk(relaxed = true) @@ -547,18 +552,7 @@ class FenixSearchMiddlewareTest { ) = SearchFragmentStore( initialState = buildEmptySearchState(), middleware = listOf(middleware, searchActionsCaptor), - ).also { - it.dispatch( - EnvironmentRehydrated( - Environment( - context = testContext, - viewLifecycleOwner = TestLifecycleOwner(RESUMED), - browsingModeManager = browsingModeManager, - navController = navController, - ), - ), - ) - } + ) private fun buildContext( store: SearchFragmentStore, @@ -630,3 +624,9 @@ class FenixSearchMiddlewareTest { assertEquals(extra, last.extra?.get("extra")) } } + +private class FakeLifecycleOwner(initialState: Lifecycle.State) : LifecycleOwner { + override val lifecycle: Lifecycle = LifecycleRegistry(this).apply { + currentState = initialState + } +}