From 86c0e8156de04a8b574a278e7e3c43249dafbcbb Mon Sep 17 00:00:00 2001 From: Matthew Tighe Date: Tue, 19 Dec 2023 16:13:10 -0800 Subject: [PATCH] Bug 1861459 - Update AppState.mode based on selected tab changes --- .../fenix/bindings/BrowserStoreBinding.kt | 37 +++++++++ .../fenix/components/appstate/AppAction.kt | 8 ++ .../fenix/components/appstate/AppState.kt | 2 + .../components/appstate/AppStoreReducer.kt | 5 ++ .../fenix/bindings/BrowserStoreBindingTest.kt | 79 +++++++++++++++++++ .../appstate/AppStoreReducerTest.kt | 47 +++++++++++ 6 files changed, 178 insertions(+) create mode 100644 app/src/main/java/org/mozilla/fenix/bindings/BrowserStoreBinding.kt create mode 100644 app/src/test/java/org/mozilla/fenix/bindings/BrowserStoreBindingTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/bindings/BrowserStoreBinding.kt b/app/src/main/java/org/mozilla/fenix/bindings/BrowserStoreBinding.kt new file mode 100644 index 000000000..85242e93c --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/bindings/BrowserStoreBinding.kt @@ -0,0 +1,37 @@ +/* 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.bindings + +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.flow.distinctUntilChangedBy +import mozilla.components.browser.state.selector.selectedTab +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.lib.state.helpers.AbstractBinding +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction + +/** + * Binding to update the [AppStore] based on changes to [BrowserState]. + */ +class BrowserStoreBinding( + browserStore: BrowserStore, + private val appStore: AppStore, +) : AbstractBinding(browserStore) { + override suspend fun onState(flow: Flow) { + // Update the AppStore with the latest selected tab + flow.distinctUntilChangedBy { it.selectedTabId } + .collectLatest { state -> + state.selectedTab?.let { tab -> + // Ignore re-observations due to lifecycle events, or other pieces of state like + // [mode] may get overwritten + if (appStore.state.selectedTabId != tab.id) { + appStore.dispatch(AppAction.SelectedTabChanged(tab)) + } + } + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index eae237c46..07eb907e7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.components.appstate +import mozilla.components.browser.state.state.TabSessionState import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.top.sites.TopSite import mozilla.components.lib.crash.Crash.NativeCodeCrash @@ -128,6 +129,13 @@ sealed class AppAction : Action { */ data class RemoveRecentSyncedTab(val syncedTab: RecentSyncedTab) : AppAction() + /** + * Action indicating that the selected tab has been changed. + * + * @property tab The tab that has been selected. + */ + data class SelectedTabChanged(val tab: TabSessionState) : AppAction() + /** * [Action]s related to interactions with the Messaging Framework. */ diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt index 77edd2d01..a056a00e7 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt @@ -38,6 +38,7 @@ import org.mozilla.fenix.wallpapers.WallpaperState * @property expandedCollections A set containing the ids of the [TabCollection] that are expanded * in the [HomeFragment]. * @property mode Whether the app is in private browsing mode. + * @property selectedTabId The currently selected tab ID. This should be bound to [BrowserStore]. * @property topSites The list of [TopSite] in the [HomeFragment]. * @property showCollectionPlaceholder If true, shows a placeholder when there are no collections. * @property recentTabs The list of recent [RecentTab] in the [HomeFragment]. @@ -64,6 +65,7 @@ data class AppState( val collections: List = emptyList(), val expandedCollections: Set = emptySet(), val mode: BrowsingMode = BrowsingMode.Normal, + val selectedTabId: String? = null, val topSites: List = emptyList(), val showCollectionPlaceholder: Boolean = false, val recentTabs: List = emptyList(), diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index e17530153..bf0ab3245 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -8,6 +8,7 @@ import androidx.annotation.VisibleForTesting import mozilla.components.service.pocket.PocketStory.PocketRecommendedStory import mozilla.components.service.pocket.PocketStory.PocketSponsoredStory import mozilla.components.service.pocket.ext.recordNewImpression +import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.shopping.ShoppingStateReducer import org.mozilla.fenix.ext.filterOutTab @@ -100,6 +101,10 @@ internal object AppStoreReducer { else -> state.recentSyncedTabState }, ) + is AppAction.SelectedTabChanged -> state.copy( + selectedTabId = action.tab.id, + mode = BrowsingMode.fromBoolean(action.tab.content.private), + ) is AppAction.DisbandSearchGroupAction -> state.copy( recentHistory = state.recentHistory.filterNot { it is RecentHistoryGroup && it.title.equals(action.searchTerm, true) diff --git a/app/src/test/java/org/mozilla/fenix/bindings/BrowserStoreBindingTest.kt b/app/src/test/java/org/mozilla/fenix/bindings/BrowserStoreBindingTest.kt new file mode 100644 index 000000000..0aaf57ca0 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/bindings/BrowserStoreBindingTest.kt @@ -0,0 +1,79 @@ +/* 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.bindings + +import mozilla.components.browser.state.action.TabListAction +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.createTab +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.components.support.test.rule.runTestOnMain +import org.junit.Rule +import org.junit.Test +import org.mockito.Mockito.never +import org.mockito.Mockito.spy +import org.mockito.Mockito.verify +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.components.appstate.AppState + +class BrowserStoreBindingTest { + + @get:Rule + val coroutineRule = MainCoroutineRule() + + lateinit var browserStore: BrowserStore + lateinit var appStore: AppStore + + private val tabId1 = "1" + private val tabId2 = "2" + private val tab1 = createTab(url = tabId1, id = tabId1) + private val tab2 = createTab(url = tabId2, id = tabId2) + + @Test + fun `WHEN selected tab changes THEN app action dispatched with update`() = runTestOnMain { + appStore = spy(AppStore()) + browserStore = BrowserStore( + BrowserState( + tabs = listOf(tab1, tab2), + selectedTabId = tabId1, + ), + ) + + val binding = BrowserStoreBinding(browserStore, appStore) + binding.start() + browserStore.dispatch(TabListAction.SelectTabAction(tabId2)).joinBlocking() + + // consume initial state + verify(appStore).dispatch(AppAction.SelectedTabChanged(tab1)) + // verify response to Browser Store dispatch + verify(appStore).dispatch(AppAction.SelectedTabChanged(tab2)) + } + + @Test + fun `GIVEN selected tab id is set WHEN update is observed with same id THEN update is ignored`() { + appStore = spy( + AppStore( + AppState( + selectedTabId = tabId2, + ), + ), + ) + browserStore = BrowserStore( + BrowserState( + tabs = listOf(tab1, tab2), + selectedTabId = tabId2, + ), + ) + + val binding = BrowserStoreBinding(browserStore, appStore) + binding.start() + browserStore.dispatch(TabListAction.SelectTabAction(tabId2)).joinBlocking() + + // the selected tab should only be dispatched on initialization + verify(appStore, never()).dispatch(AppAction.SelectedTabChanged(tab2)) + } +} diff --git a/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt b/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt index 7be038c02..2997e14c0 100644 --- a/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/appstate/AppStoreReducerTest.kt @@ -5,10 +5,12 @@ package org.mozilla.fenix.components.appstate import io.mockk.mockk +import mozilla.components.browser.state.state.createTab import mozilla.components.lib.crash.Crash.NativeCodeCrash import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test +import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.appstate.AppAction.AddNonFatalCrash import org.mozilla.fenix.components.appstate.AppAction.RemoveAllNonFatalCrashes import org.mozilla.fenix.components.appstate.AppAction.RemoveNonFatalCrash @@ -69,4 +71,49 @@ class AppStoreReducerTest { assertTrue(updatedState.nonFatalCrashes.isEmpty()) } + + @Test + fun `GIVEN mode is private WHEN selected tab changes to normal mode THEN state is updated to normal mode`() { + val initialState = AppState( + selectedTabId = null, + mode = BrowsingMode.Private, + ) + + val updatedState = AppStoreReducer.reduce( + initialState, + AppAction.SelectedTabChanged(createTab("", private = false)), + ) + + assertFalse(updatedState.mode.isPrivate) + } + + @Test + fun `GIVEN mode is normal WHEN selected tab changes to private mode THEN state is updated to private mode`() { + val initialState = AppState( + selectedTabId = null, + mode = BrowsingMode.Normal, + ) + + val updatedState = AppStoreReducer.reduce( + initialState, + AppAction.SelectedTabChanged(createTab("", private = true)), + ) + + assertTrue(updatedState.mode.isPrivate) + } + + @Test + fun `WHEN selected tab changes to a tab in the same mode THEN mode is unchanged`() { + val initialState = AppState( + selectedTabId = null, + mode = BrowsingMode.Normal, + ) + + val updatedState = AppStoreReducer.reduce( + initialState, + AppAction.SelectedTabChanged(createTab("", private = false)), + ) + + assertFalse(updatedState.mode.isPrivate) + } }