commit d8715cf302fb2933b6264b68de900dca740479b6
parent b4f44dfecb7947da0308ee1d8b3de64a674d9870
Author: Julie De Lorenzo <jdelorenzo@mozilla.com>
Date: Sat, 20 Dec 2025 01:02:36 +0000
Bug 2007113: Fix crash with synced tabs update when devices added r=android-reviewers,007
Differential Revision: https://phabricator.services.mozilla.com/D277230
Diffstat:
3 files changed, 98 insertions(+), 6 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayStore.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayStore.kt
@@ -15,7 +15,10 @@ import org.mozilla.fenix.tabstray.redux.reducer.TabSearchActionReducer
import org.mozilla.fenix.tabstray.redux.state.TabSearchState
import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsListItem
-private const val DEFAULT_SYNCED_TABS_EXPANDED_STATE = true
+/**
+ * The default state of the synced tabs expanded state, which is true.
+ */
+internal const val DEFAULT_SYNCED_TABS_EXPANDED_STATE = true
/**
* Value type that represents the state of the tabs tray.
@@ -405,11 +408,11 @@ internal object TabsTrayReducer {
* @param action the action containing updated tabs.
*/
private fun handleSyncedTabUpdate(state: TabsTrayState, action: TabsTrayAction.UpdateSyncedTabs): TabsTrayState {
- return if (state.syncedTabs.isNotEmpty() && action.tabs.isNotEmpty()) {
+ return if (syncStateExists(state, action) && syncedDevicesUnchanged(state, action)) {
state.copy(
syncedTabs = action.tabs,
expandedSyncedTabs = action.tabs.mapIndexed { index, item ->
- if (state.syncedTabs[index] == item) {
+ if (state.syncedTabs[index] == item && index < state.expandedSyncedTabs.size) {
state.expandedSyncedTabs[index]
} else {
DEFAULT_SYNCED_TABS_EXPANDED_STATE
@@ -427,6 +430,16 @@ private fun handleSyncedTabUpdate(state: TabsTrayState, action: TabsTrayAction.U
}
}
+// Does previous state exist for the SyncedTabs we might want to preserve?
+private fun syncStateExists(state: TabsTrayState, action: TabsTrayAction.UpdateSyncedTabs): Boolean {
+ return state.syncedTabs.isNotEmpty() && action.tabs.isNotEmpty()
+}
+
+// Has the list of devices synced in SyncedTabs list changed?
+private fun syncedDevicesUnchanged(state: TabsTrayState, action: TabsTrayAction.UpdateSyncedTabs): Boolean {
+ return state.syncedTabs.size == action.tabs.size
+}
+
/**
* When a synced tab header's expansion is toggled, that item should be expanded or collapsed.
* The rest of the list should be unchanged.
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsList.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/syncedtabs/SyncedTabsList.kt
@@ -338,7 +338,8 @@ internal fun getFakeSyncedTabList(): List<SyncedTabsListItem> = listOf(
/**
* Helper function to create a [SyncedTabsListItem.Tab] for previewing.
*/
-private fun generateFakeTab(
+@VisibleForTesting
+internal fun generateFakeTab(
tabName: String,
tabUrl: String,
action: SyncedTabsListItem.Tab.Action = SyncedTabsListItem.Tab.Action.None,
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayStoreReducerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/tabstray/TabsTrayStoreReducerTest.kt
@@ -12,6 +12,8 @@ import org.junit.Test
import org.mozilla.fenix.tabstray.navigation.TabManagerNavDestination
import org.mozilla.fenix.tabstray.redux.reducer.TabSearchActionReducer
import org.mozilla.fenix.tabstray.redux.state.TabSearchState
+import org.mozilla.fenix.tabstray.syncedtabs.SyncedTabsListItem
+import org.mozilla.fenix.tabstray.syncedtabs.generateFakeTab
import org.mozilla.fenix.tabstray.syncedtabs.getFakeSyncedTabList
class TabsTrayStoreReducerTest {
@@ -104,7 +106,7 @@ class TabsTrayStoreReducerTest {
TabsTrayAction.UpdateSyncedTabs(syncedTabs),
)
- assertTrue(resultState.expandedSyncedTabs.all { true })
+ assertTrue(resultState.expandedSyncedTabs.all { DEFAULT_SYNCED_TABS_EXPANDED_STATE })
}
@Test
@@ -145,7 +147,83 @@ class TabsTrayStoreReducerTest {
TabsTrayAction.UpdateSyncedTabs(newSyncedTabs),
)
- assertTrue(resultState.expandedSyncedTabs.all { true })
+ assertTrue(resultState.expandedSyncedTabs.all { DEFAULT_SYNCED_TABS_EXPANDED_STATE })
+ }
+
+ @Test
+ fun `GIVEN synced tabs WHEN UpdateSyncedTabs is called with smaller device list THEN the expanded states are reset`() {
+ val expectedExpansionList = listOf(true, true, false, false)
+ val syncedTabs = getFakeSyncedTabList()
+ val newSyncedTabs = listOf(
+ SyncedTabsListItem.DeviceSection(
+ displayName = "Device 1",
+ tabs = listOf(
+ generateFakeTab("Mozilla", "www.mozilla.org"),
+ generateFakeTab("Google", "www.google.com"),
+ generateFakeTab("", "www.google.com"),
+ ),
+ ),
+ )
+ val initialState = TabsTrayState(syncedTabs = syncedTabs, expandedSyncedTabs = expectedExpansionList)
+
+ val resultState = TabsTrayReducer.reduce(
+ initialState,
+ TabsTrayAction.UpdateSyncedTabs(newSyncedTabs),
+ )
+
+ assertTrue(resultState.expandedSyncedTabs.all { DEFAULT_SYNCED_TABS_EXPANDED_STATE })
+ }
+
+ @Test
+ fun `GIVEN synced tabs WHEN UpdateSyncedTabs is called with a larger device list THEN the expanded states are reset`() {
+ val expectedExpansionList = listOf(true, true, false, false)
+ val syncedTabs = listOf(
+ SyncedTabsListItem.DeviceSection(
+ displayName = "Device 1",
+ tabs = listOf(
+ generateFakeTab("Mozilla", "www.mozilla.org"),
+ generateFakeTab("Google", "www.google.com"),
+ generateFakeTab("", "www.google.com"),
+ ),
+ ),
+ )
+ val newSyncedTabs = getFakeSyncedTabList()
+ val initialState = TabsTrayState(syncedTabs = syncedTabs, expandedSyncedTabs = expectedExpansionList)
+
+ val resultState = TabsTrayReducer.reduce(
+ initialState,
+ TabsTrayAction.UpdateSyncedTabs(newSyncedTabs),
+ )
+
+ assertTrue(resultState.expandedSyncedTabs.all { DEFAULT_SYNCED_TABS_EXPANDED_STATE })
+ }
+
+ @Test
+ fun `GIVEN synced tabs state larger than expanded synced tabs WHEN UpdateSyncedTabs is called THEN it is handled gracefully`() {
+ val syncedTabs = getFakeSyncedTabList()
+ val newSyncedTabs = getFakeSyncedTabList().reversed()
+ val initialState = TabsTrayState(syncedTabs = syncedTabs, expandedSyncedTabs = emptyList())
+
+ val resultState = TabsTrayReducer.reduce(
+ initialState,
+ TabsTrayAction.UpdateSyncedTabs(newSyncedTabs),
+ )
+
+ assertTrue(resultState.expandedSyncedTabs.all { DEFAULT_SYNCED_TABS_EXPANDED_STATE })
+ }
+
+ @Test
+ fun `GIVEN synced tabs state smaller than expanded synced tabs WHEN UpdateSyncedTabs is called THEN it is handled gracefully`() {
+ val syncedTabs = getFakeSyncedTabList()
+ val newSyncedTabs = getFakeSyncedTabList().reversed()
+ val initialState = TabsTrayState(syncedTabs = syncedTabs, expandedSyncedTabs = listOf(true, true, false, false, false, false, false, false, false, false))
+
+ val resultState = TabsTrayReducer.reduce(
+ initialState,
+ TabsTrayAction.UpdateSyncedTabs(newSyncedTabs),
+ )
+
+ assertTrue(resultState.expandedSyncedTabs.all { DEFAULT_SYNCED_TABS_EXPANDED_STATE })
}
@Test
