From c9b8f57f96e9188746391885a065428df62f3ff9 Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Wed, 17 Feb 2021 10:20:06 -0500 Subject: [PATCH] Refactor BrowserToolbarMenuController to use browser store --- .../fenix/browser/BaseBrowserFragment.kt | 4 +- .../toolbar/BrowserToolbarMenuController.kt | 95 ++++++----- ...DefaultBrowserToolbarMenuControllerTest.kt | 156 +++++++++--------- 3 files changed, 137 insertions(+), 118 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index d3525c2f2..e8ed31cee 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -318,17 +318,17 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit } ) val browserToolbarMenuController = DefaultBrowserToolbarMenuController( + store = store, activity = activity, navController = findNavController(), metrics = requireComponents.analytics.metrics, settings = context.settings(), readerModeController = readerMenuController, - sessionManager = requireComponents.core.sessionManager, sessionFeature = sessionFeature, findInPageLauncher = { findInPageIntegration.withFeature { it.launch() } }, swipeRefresh = swipeRefresh, browserAnimator = browserAnimator, - customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) }, + customTabSessionId = customTabSessionId, openInFenixIntent = openInFenixIntent, bookmarkTapped = { url: String, title: String -> viewLifecycleOwner.lifecycleScope.launch { diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt index 46dcdb1e6..d72f9b133 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt @@ -15,9 +15,10 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.MainScope import kotlinx.coroutines.launch import mozilla.appservices.places.BookmarkRoot -import mozilla.components.browser.session.Session -import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.selector.findCustomTabOrSelectedTab import mozilla.components.browser.state.selector.findTab +import mozilla.components.browser.state.selector.selectedTab +import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.EngineSession.LoadUrlFlags import mozilla.components.concept.engine.prompt.ShareData @@ -53,17 +54,17 @@ interface BrowserToolbarMenuController { @Suppress("LargeClass", "ForbiddenComment") class DefaultBrowserToolbarMenuController( + private val store: BrowserStore, private val activity: HomeActivity, private val navController: NavController, private val metrics: MetricController, private val settings: Settings, private val readerModeController: ReaderModeController, private val sessionFeature: ViewBoundFeatureWrapper, - private val sessionManager: SessionManager, private val findInPageLauncher: () -> Unit, private val browserAnimator: BrowserAnimator, private val swipeRefresh: SwipeRefreshLayout, - private val customTabSession: Session?, + private val customTabSessionId: String?, private val openInFenixIntent: Intent, private val bookmarkTapped: (String, String) -> Unit, private val scope: CoroutineScope, @@ -73,7 +74,7 @@ class DefaultBrowserToolbarMenuController( ) : BrowserToolbarMenuController { private val currentSession - get() = customTabSession ?: sessionManager.selectedSession + get() = store.state.findCustomTabOrSelectedTab(customTabSessionId) // We hold onto a reference of the inner scope so that we can override this with the // TestCoroutineScope to ensure sequential execution. If we didn't have this, our tests @@ -84,6 +85,7 @@ class DefaultBrowserToolbarMenuController( @Suppress("ComplexMethod", "LongMethod") override fun handleToolbarItemInteraction(item: ToolbarMenu.Item) { val sessionUseCases = activity.components.useCases.sessionUseCases + val customTabUseCases = activity.components.useCases.customTabsUseCases trackToolbarItemInteraction(item) Do exhaustive when (item) { @@ -104,26 +106,27 @@ class DefaultBrowserToolbarMenuController( } } is ToolbarMenu.Item.OpenInFenix -> { - // Stop the SessionFeature from updating the EngineView and let it release the session - // from the EngineView so that it can immediately be rendered by a different view once - // we switch to the actual browser. - sessionFeature.get()?.release() + customTabSessionId?.let { + // Stop the SessionFeature from updating the EngineView and let it release the session + // from the EngineView so that it can immediately be rendered by a different view once + // we switch to the actual browser. + sessionFeature.get()?.release() - // Strip the CustomTabConfig to turn this Session into a regular tab and then select it - customTabSession!!.customTabConfig = null - sessionManager.select(customTabSession) + // Turn this Session into a regular tab and then select it + customTabUseCases.migrate(customTabSessionId, select = true) - // Switch to the actual browser which should now display our new selected session - activity.startActivity(openInFenixIntent.apply { - // We never want to launch the browser in the same task as the external app - // activity. So we force a new task here. IntentReceiverActivity will do the - // right thing and take care of routing to an already existing browser and avoid - // cloning a new one. - flags = flags or Intent.FLAG_ACTIVITY_NEW_TASK - }) + // Switch to the actual browser which should now display our new selected session + activity.startActivity(openInFenixIntent.apply { + // We never want to launch the browser in the same task as the external app + // activity. So we force a new task here. IntentReceiverActivity will do the + // right thing and take care of routing to an already existing browser and avoid + // cloning a new one. + flags = flags or Intent.FLAG_ACTIVITY_NEW_TASK + }) - // Close this activity (and the task) since it is no longer displaying any session - activity.finishAndRemoveTask() + // Close this activity (and the task) since it is no longer displaying any session + activity.finishAndRemoveTask() + } } is ToolbarMenu.Item.Quit -> { // We need to show the snackbar while the browsing data is deleting (if "Delete @@ -150,7 +153,7 @@ class DefaultBrowserToolbarMenuController( val appLinksUseCases = activity.components.useCases.appLinksUseCases val getRedirect = appLinksUseCases.appLinkRedirect currentSession?.let { - val redirect = getRedirect.invoke(it.url) + val redirect = getRedirect.invoke(it.content.url) redirect.appIntent?.flags = Intent.FLAG_ACTIVITY_NEW_TASK appLinksUseCases.openAppLink.invoke(redirect.appIntent) } @@ -161,22 +164,26 @@ class DefaultBrowserToolbarMenuController( if (item.viewHistory) { navController.navigate( BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment( - activeSessionId = customTabSession?.id + activeSessionId = customTabSessionId ) ) } else { - sessionUseCases.goBack.invoke(currentSession) + currentSession?.let { + sessionUseCases.goBack.invoke(it.id) + } } } is ToolbarMenu.Item.Forward -> { if (item.viewHistory) { navController.navigate( BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment( - activeSessionId = customTabSession?.id + activeSessionId = customTabSessionId ) ) } else { - sessionUseCases.goForward.invoke(currentSession) + currentSession?.let { + sessionUseCases.goForward.invoke(it.id) + } } } is ToolbarMenu.Item.Reload -> { @@ -186,15 +193,21 @@ class DefaultBrowserToolbarMenuController( LoadUrlFlags.none() } - sessionUseCases.reload.invoke(currentSession, flags = flags) + currentSession?.let { + sessionUseCases.reload.invoke(it.id, flags = flags) + } + } + is ToolbarMenu.Item.Stop -> { + currentSession?.let { + sessionUseCases.stopLoading.invoke(it.id) + } } - is ToolbarMenu.Item.Stop -> sessionUseCases.stopLoading.invoke(currentSession) is ToolbarMenu.Item.Share -> { val directions = NavGraphDirections.actionGlobalShareFragment( data = arrayOf( ShareData( url = getProperUrl(currentSession), - title = currentSession?.title + title = currentSession?.content?.title ) ), showPage = true @@ -211,10 +224,14 @@ class DefaultBrowserToolbarMenuController( BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment() ) } - is ToolbarMenu.Item.RequestDesktop -> sessionUseCases.requestDesktopSite.invoke( - item.isChecked, - currentSession - ) + is ToolbarMenu.Item.RequestDesktop -> { + currentSession?.let { + sessionUseCases.requestDesktopSite.invoke( + item.isChecked, + it.id + ) + } + } is ToolbarMenu.Item.AddToTopSites -> { scope.launch { val context = swipeRefresh.context @@ -234,7 +251,7 @@ class DefaultBrowserToolbarMenuController( ioScope.launch { currentSession?.let { with(activity.components.useCases.topSitesUseCase) { - addPinnedSites(it.title, it.url) + addPinnedSites(it.content.title, it.content.url) } } }.join() @@ -294,8 +311,8 @@ class DefaultBrowserToolbarMenuController( } } is ToolbarMenu.Item.Bookmark -> { - sessionManager.selectedSession?.let { - getProperUrl(it)?.let { url -> bookmarkTapped(url, it.title) } + store.state.selectedTab?.let { + getProperUrl(it)?.let { url -> bookmarkTapped(url, it.content.title) } } } is ToolbarMenu.Item.Bookmarks -> browserAnimator.captureEngineViewAndDrawStatically { @@ -325,13 +342,13 @@ class DefaultBrowserToolbarMenuController( } } - private fun getProperUrl(currentSession: Session?): String? { + private fun getProperUrl(currentSession: SessionState?): String? { return currentSession?.id?.let { val currentTab = browserStore.state.findTab(it) if (currentTab?.readerState?.active == true) { currentTab.readerState.activeUrl } else { - currentSession.url + currentSession.content.url } } } diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt index 211cf0fc9..da473a25d 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt @@ -24,10 +24,11 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest import mozilla.appservices.places.BookmarkRoot -import mozilla.components.browser.session.Session -import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.action.CustomTabListAction import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ReaderState +import mozilla.components.browser.state.state.TabSessionState +import mozilla.components.browser.state.state.createCustomTab import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.EngineSession @@ -36,9 +37,11 @@ import mozilla.components.feature.search.SearchUseCases import mozilla.components.feature.session.SessionFeature import mozilla.components.feature.session.SessionUseCases import mozilla.components.feature.tab.collections.TabCollection +import mozilla.components.feature.tabs.CustomTabsUseCases import mozilla.components.feature.top.sites.DefaultTopSitesStorage import mozilla.components.feature.top.sites.TopSitesUseCases import mozilla.components.support.base.feature.ViewBoundFeatureWrapper +import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.After import org.junit.Before @@ -76,13 +79,12 @@ class DefaultBrowserToolbarMenuControllerTest { @RelaxedMockK private lateinit var navController: NavController @RelaxedMockK private lateinit var findInPageLauncher: () -> Unit @RelaxedMockK private lateinit var bookmarkTapped: (String, String) -> Unit - @RelaxedMockK private lateinit var sessionManager: SessionManager - @RelaxedMockK private lateinit var currentSession: Session @RelaxedMockK private lateinit var openInFenixIntent: Intent @RelaxedMockK private lateinit var metrics: MetricController @RelaxedMockK private lateinit var settings: Settings @RelaxedMockK private lateinit var searchUseCases: SearchUseCases @RelaxedMockK private lateinit var sessionUseCases: SessionUseCases + @RelaxedMockK private lateinit var customTabUseCases: CustomTabsUseCases @RelaxedMockK private lateinit var browserAnimator: BrowserAnimator @RelaxedMockK private lateinit var snackbar: FenixSnackbar @RelaxedMockK private lateinit var tabCollectionStorage: TabCollectionStorage @@ -91,7 +93,9 @@ class DefaultBrowserToolbarMenuControllerTest { @MockK private lateinit var sessionFeatureWrapper: ViewBoundFeatureWrapper @RelaxedMockK private lateinit var sessionFeature: SessionFeature @RelaxedMockK private lateinit var topSitesStorage: DefaultTopSitesStorage - @RelaxedMockK private lateinit var browserStore: BrowserStore + + private lateinit var browserStore: BrowserStore + private lateinit var selectedTab: TabSessionState @Before fun setUp() { @@ -106,18 +110,25 @@ class DefaultBrowserToolbarMenuControllerTest { every { FenixSnackbar.make(any(), any(), any(), any()) } returns snackbar every { activity.components.useCases.sessionUseCases } returns sessionUseCases + every { activity.components.useCases.customTabsUseCases } returns customTabUseCases every { activity.components.useCases.searchUseCases } returns searchUseCases every { activity.components.useCases.topSitesUseCase } returns topSitesUseCase - every { sessionManager.selectedSession } returns currentSession every { sessionFeatureWrapper.get() } returns sessionFeature every { navController.currentDestination } returns mockk { every { id } returns R.id.browserFragment } - every { currentSession.id } returns "1" every { settings.topSitesMaxLimit } returns 16 val onComplete = slot<() -> Unit>() every { browserAnimator.captureEngineViewAndDrawStatically(capture(onComplete)) } answers { onComplete.captured.invoke() } + + selectedTab = createTab("https://www.mozilla.org", id = "1") + browserStore = BrowserStore( + initialState = BrowserState( + tabs = listOf(selectedTab), + selectedTabId = selectedTab.id + ) + ) } @After @@ -134,23 +145,20 @@ class DefaultBrowserToolbarMenuControllerTest { val item = ToolbarMenu.Item.Bookmark val title = "Mozilla" - val readerUrl = "moz-extension://1234" - val readerTab = createTab( - url = readerUrl, + val url = "https://mozilla.org" + val regularTab = createTab( + url = url, readerState = ReaderState(active = false, activeUrl = "https://1234.org"), title = title ) - browserStore = - BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id)) - every { currentSession.id } returns readerTab.id - every { currentSession.title } returns title - every { currentSession.url } returns "https://mozilla.org" + val store = + BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id)) - val controller = createController(scope = this) + val controller = createController(scope = this, store = store) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) } - verify { bookmarkTapped("https://mozilla.org", title) } + verify { bookmarkTapped(url, title) } } } @@ -167,11 +175,8 @@ class DefaultBrowserToolbarMenuControllerTest { ) browserStore = BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id)) - every { currentSession.id } returns readerTab.id - every { currentSession.title } returns title - every { currentSession.url } returns readerUrl - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) } @@ -182,18 +187,22 @@ class DefaultBrowserToolbarMenuControllerTest { @Test fun `WHEN open in Fenix menu item is pressed THEN menu item is handled correctly`() = runBlockingTest { if (!FeatureFlags.toolbarMenuFeature) { - val controller = createController(scope = this, customTabSession = currentSession) + + val customTab = createCustomTab("https://mozilla.org") + browserStore.dispatch(CustomTabListAction.AddCustomTabAction(customTab)).joinBlocking() + val controller = createController( + scope = this, + store = browserStore, + customTabSessionId = customTab.id + ) val item = ToolbarMenu.Item.OpenInFenix - every { currentSession.customTabConfig } returns mockk() every { activity.startActivity(any()) } just Runs - controller.handleToolbarItemInteraction(item) verify { sessionFeature.release() } - verify { currentSession.customTabConfig = null } - verify { sessionManager.select(currentSession) } + verify { customTabUseCases.migrate(customTab.id, true) } verify { activity.startActivity(openInFenixIntent) } verify { activity.finishAndRemoveTask() } } @@ -205,7 +214,7 @@ class DefaultBrowserToolbarMenuControllerTest { val item = ToolbarMenu.Item.Quit val testScope = this - val controller = createController(scope = testScope) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) @@ -218,7 +227,7 @@ class DefaultBrowserToolbarMenuControllerTest { if (!FeatureFlags.toolbarMenuFeature) { val item = ToolbarMenu.Item.OpenInApp - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) @@ -230,7 +239,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN reader mode menu item is pressed THEN handle appearance change`() = runBlockingTest { val item = ToolbarMenu.Item.ReaderModeAppearance - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) @@ -243,18 +252,18 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN backwards nav menu item is pressed THEN the session navigates back with active session`() = runBlockingTest { val item = ToolbarMenu.Item.Back(false) - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BACK)) } - verify { sessionUseCases.goBack(currentSession) } + verify { sessionUseCases.goBack(browserStore.state.selectedTabId!!) } } @Test fun `WHEN backwards nav menu item is long pressed THEN the session navigates back with no active session`() = runBlockingTest { val item = ToolbarMenu.Item.Back(true) - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) val directions = BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment(null) @@ -267,18 +276,18 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN forward nav menu item is pressed THEN the session navigates forward to active session`() = runBlockingTest { val item = ToolbarMenu.Item.Forward(false) - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.FORWARD)) } - verify { sessionUseCases.goForward(currentSession) } + verify { sessionUseCases.goForward(selectedTab.id) } } @Test fun `WHEN forward nav menu item is long pressed THEN the browser navigates forward with no active session`() = runBlockingTest { val item = ToolbarMenu.Item.Forward(true) - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) val directions = BrowserFragmentDirections.actionGlobalTabHistoryDialogFragment(null) @@ -291,24 +300,24 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN reload nav menu item is pressed THEN the session reloads from cache`() = runBlockingTest { val item = ToolbarMenu.Item.Reload(false) - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.RELOAD)) } - verify { sessionUseCases.reload(currentSession) } + verify { sessionUseCases.reload(selectedTab.id) } } @Test fun `WHEN reload nav menu item is long pressed THEN the session reloads with no cache`() = runBlockingTest { val item = ToolbarMenu.Item.Reload(true) - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.RELOAD)) } verify { sessionUseCases.reload( - currentSession, + selectedTab.id, EngineSession.LoadUrlFlags.select(EngineSession.LoadUrlFlags.BYPASS_CACHE) ) } @@ -318,18 +327,18 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN stop nav menu item is pressed THEN the session stops loading`() = runBlockingTest { val item = ToolbarMenu.Item.Stop - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.STOP)) } - verify { sessionUseCases.stopLoading(currentSession) } + verify { sessionUseCases.stopLoading(selectedTab.id) } } @Test fun `WHEN settings menu item is pressed THEN menu item is handled`() = runBlockingTest { val item = ToolbarMenu.Item.Settings - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment() @@ -342,7 +351,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN bookmark menu item is pressed THEN navigate to bookmarks page`() = runBlockingTest { val item = ToolbarMenu.Item.Bookmarks - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) val directions = BrowserFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id) @@ -355,7 +364,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN history menu item is pressed THEN navigate to history page`() = runBlockingTest { val item = ToolbarMenu.Item.History - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) val directions = BrowserFragmentDirections.actionGlobalHistoryFragment() @@ -372,14 +381,14 @@ class DefaultBrowserToolbarMenuControllerTest { every { sessionUseCases.requestDesktopSite } returns requestDesktopSiteUseCase - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.DESKTOP_VIEW_ON)) } verify { requestDesktopSiteUseCase.invoke( true, - currentSession + selectedTab.id ) } } @@ -392,14 +401,14 @@ class DefaultBrowserToolbarMenuControllerTest { every { sessionUseCases.requestDesktopSite } returns requestDesktopSiteUseCase - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.DESKTOP_VIEW_OFF)) } verify { requestDesktopSiteUseCase.invoke( false, - currentSession + selectedTab.id ) } } @@ -414,10 +423,10 @@ class DefaultBrowserToolbarMenuControllerTest { swipeRefreshLayout.context.getString(R.string.snackbar_added_to_top_sites) } returns "Added to top sites!" - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) - verify { addPinnedSiteUseCase.invoke(currentSession.title, currentSession.url) } + verify { addPinnedSiteUseCase.invoke(selectedTab.content.title, selectedTab.content.url) } verify { snackbar.setText("Added to top sites!") } verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.ADD_TO_TOP_SITES)) } } @@ -426,7 +435,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN addon extensions menu item is pressed THEN navigate to addons manager`() = runBlockingTest { val item = ToolbarMenu.Item.AddonsManager - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.ADDONS_MANAGER)) } @@ -436,7 +445,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN Add To Home Screen menu item is pressed THEN add site`() = runBlockingTest { val item = ToolbarMenu.Item.AddToHomeScreen - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.ADD_TO_HOMESCREEN)) } @@ -446,18 +455,14 @@ class DefaultBrowserToolbarMenuControllerTest { fun `IF reader mode is inactive WHEN share menu item is pressed THEN navigate to share screen`() = runBlockingTest { val item = ToolbarMenu.Item.Share val title = "Mozilla" - val readerUrl = "moz-extension://1234" - val readerTab = createTab( - url = readerUrl, + val url = "https://mozilla.org" + val regularTab = createTab( + url = url, readerState = ReaderState(active = false, activeUrl = "https://1234.org"), title = title ) - browserStore = BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id)) - every { currentSession.id } returns readerTab.id - every { currentSession.title } returns title - every { currentSession.url } returns "https://mozilla.org" - - val controller = createController(scope = this) + browserStore = BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id)) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SHARE)) } @@ -484,11 +489,7 @@ class DefaultBrowserToolbarMenuControllerTest { title = title ) browserStore = BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id)) - every { currentSession.id } returns readerTab.id - every { currentSession.title } returns title - every { currentSession.url } returns readerUrl - - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SHARE)) } @@ -508,7 +509,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN Find In Page menu item is pressed THEN launch finder`() = runBlockingTest { val item = ToolbarMenu.Item.FindInPage - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { findInPageLauncher() } @@ -521,7 +522,7 @@ class DefaultBrowserToolbarMenuControllerTest { val cachedTabCollections: List = mockk(relaxed = true) every { tabCollectionStorage.cachedTabCollections } returns cachedTabCollections - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { @@ -537,8 +538,8 @@ class DefaultBrowserToolbarMenuControllerTest { val directions = BrowserFragmentDirections.actionGlobalCollectionCreationFragment( saveCollectionStep = SaveCollectionStep.SelectCollection, - tabIds = arrayOf(currentSession.id), - selectedTabIds = arrayOf(currentSession.id) + tabIds = arrayOf(selectedTab.id), + selectedTabIds = arrayOf(selectedTab.id) ) verify { navController.navigate(directionsEq(directions), null) } } @@ -549,7 +550,7 @@ class DefaultBrowserToolbarMenuControllerTest { val cachedTabCollectionsEmpty: List = emptyList() every { tabCollectionStorage.cachedTabCollections } returns cachedTabCollectionsEmpty - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SAVE_TO_COLLECTION)) } @@ -562,8 +563,8 @@ class DefaultBrowserToolbarMenuControllerTest { } val directions = BrowserFragmentDirections.actionGlobalCollectionCreationFragment( saveCollectionStep = SaveCollectionStep.NameCollection, - tabIds = arrayOf(currentSession.id), - selectedTabIds = arrayOf(currentSession.id) + tabIds = arrayOf(selectedTab.id), + selectedTabIds = arrayOf(selectedTab.id) ) verify { navController.navigate(directionsEq(directions), null) } } @@ -572,7 +573,7 @@ class DefaultBrowserToolbarMenuControllerTest { fun `WHEN New Tab menu item is pressed THEN navigate to a new tab home`() = runBlockingTest { val item = ToolbarMenu.Item.NewTab - val controller = createController(scope = this) + val controller = createController(scope = this, store = browserStore) controller.handleToolbarItemInteraction(item) @@ -589,23 +590,24 @@ class DefaultBrowserToolbarMenuControllerTest { private fun createController( scope: CoroutineScope, + store: BrowserStore, activity: HomeActivity = this.activity, - customTabSession: Session? = null + customTabSessionId: String? = null ) = DefaultBrowserToolbarMenuController( + store = store, activity = activity, navController = navController, metrics = metrics, settings = settings, findInPageLauncher = findInPageLauncher, browserAnimator = browserAnimator, - customTabSession = customTabSession, + customTabSessionId = customTabSessionId, openInFenixIntent = openInFenixIntent, scope = scope, swipeRefresh = swipeRefreshLayout, tabCollectionStorage = tabCollectionStorage, bookmarkTapped = bookmarkTapped, readerModeController = readerModeController, - sessionManager = sessionManager, sessionFeature = sessionFeatureWrapper, topSitesStorage = topSitesStorage, browserStore = browserStore