[fenix] For https://github.com/mozilla-mobile/fenix/issues/26399 - Use a list of tabs for recent synced tabs success state

app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt

@@ -9,6 +9,8 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import mozilla.components.concept.storage.HistoryStorage
+import mozilla.components.browser.storage.sync.Tab
+import mozilla.components.concept.sync.Device
 import mozilla.components.concept.sync.DeviceType
 import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage
 import mozilla.components.lib.state.ext.flow
@@ -22,10 +24,10 @@ import mozilla.components.service.fxa.sync.SyncReason
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl
 import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
-import org.mozilla.fenix.components.AppStore
-import org.mozilla.fenix.components.appstate.AppAction
 import mozilla.telemetry.glean.GleanTimerId
 import org.mozilla.fenix.GleanMetrics.RecentSyncedTabs
+import org.mozilla.fenix.components.AppStore
+import org.mozilla.fenix.components.appstate.AppAction
 import java.util.concurrent.TimeUnit
 
 /**
@@ -50,7 +52,7 @@ class RecentSyncedTabFeature(
 ) : LifecycleAwareFeature {
 
     private var syncStartId: GleanTimerId? = null
-    private var lastSyncedTab: RecentSyncedTab? = null
+    private var lastSyncedTabs: List<RecentSyncedTab>? = null
 
     override fun start() {
         collectAccountUpdates()
@@ -110,11 +112,20 @@ class RecentSyncedTabFeature(
             return
         }
 
-        val syncedTab = storage.getSyncedDeviceTabs()
+        val syncedTabs = storage.getSyncedDeviceTabs()
             .filterNot { it.device.isCurrentDevice || it.tabs.isEmpty() }
-            .maxByOrNull { it.device.lastAccessTime ?: 0 }
+            .flatMap {
-            ?.let {
+                it.tabs.map { tab ->
-                val tab = it.tabs.firstOrNull()?.active() ?: return
+                    SyncedDeviceTab(it.device, tab)
+                }
+            }
+            .ifEmpty { return }
+            // We want to get the last device used based on the most recent accessed tab,
+            // as described here: https://github.com/mozilla-mobile/fenix/issues/26398
+            .sortedByDescending { deviceTab -> deviceTab.tab.lastUsed }
+            .take(MAX_RECENT_SYNCED_TABS)
+            .map { deviceTab ->
+                val activeTabEntry = deviceTab.tab.active()
 
                 val currentTime = System.currentTimeMillis()
                 val maxAgeInMs = TimeUnit.DAYS.toMillis(DAYS_HISTORY_FOR_PREVIEW_IMAGE)
@@ -126,28 +137,27 @@ class RecentSyncedTabFeature(
                 // Searching history entries for any that share a top level domain and have a
                 // preview image URL available casts a wider net for finding a suitable image.
                 val previewImageUrl = history.find { entry ->
-                    entry.url.contains(tab.url.tryGetHostFromUrl()) && entry.previewImageUrl != null
+                    entry.url.contains(activeTabEntry.url.tryGetHostFromUrl()) && entry.previewImageUrl != null
                 }?.previewImageUrl
 
                 RecentSyncedTab(
-                    deviceDisplayName = it.device.displayName,
+                    deviceDisplayName = deviceTab.device.displayName,
-                    deviceType = it.device.deviceType,
+                    deviceType = deviceTab.device.deviceType,
-                    title = tab.title,
+                    title = activeTabEntry.title,
-                    url = tab.url,
+                    url = activeTabEntry.url,
                     previewImageUrl = previewImageUrl
                 )
             }
-
+        if (syncedTabs.isEmpty()) {
-        if (syncedTab == null) {
             appStore.dispatch(
                 AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)
             )
         } else {
-            recordMetrics(syncedTab, lastSyncedTab, syncStartId)
+            recordMetrics(syncedTabs.first(), lastSyncedTabs?.first(), syncStartId)
             appStore.dispatch(
-                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(syncedTab))
+                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(syncedTabs))
             )
-            lastSyncedTab = syncedTab
+            lastSyncedTabs = syncedTabs
         }
     }
 
@@ -180,6 +190,11 @@ class RecentSyncedTabFeature(
          */
 
         const val DAYS_HISTORY_FOR_PREVIEW_IMAGE = 3L
+
+        /**
+         * Number of recent synced tabs we want to keep in the success state.
+         */
+        const val MAX_RECENT_SYNCED_TABS = 8
     }
 }
 
@@ -198,9 +213,9 @@ sealed class RecentSyncedTabState {
     object Loading : RecentSyncedTabState()
 
     /**
-     * A user is authenticated and the most recent synced tab has been found.
+     * A user is authenticated and most recent synced tabs have been found.
      */
-    data class Success(val tab: RecentSyncedTab) : RecentSyncedTabState()
+    data class Success(val tabs: List<RecentSyncedTab>) : RecentSyncedTabState()
 }
 
 /**
@@ -218,3 +233,14 @@ data class RecentSyncedTab(
     val url: String,
     val previewImageUrl: String?,
 )
+
+/**
+ * Class representing a tab from a synced device.
+ *
+ * @param device The synced [Device].
+ * @param tab The tab from the synced device.
+ */
+private data class SyncedDeviceTab(
+    val device: Device,
+    val tab: Tab
+)

app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTabViewHolder.kt

@@ -48,7 +48,7 @@ class RecentSyncedTabViewHolder(
             val syncedTab = when (it) {
                 RecentSyncedTabState.None,
                 RecentSyncedTabState.Loading -> null
-                is RecentSyncedTabState.Success -> it.tab
+                is RecentSyncedTabState.Success -> it.tabs.firstOrNull()
             }
             RecentSyncedTab(
                 tab = syncedTab,

app/src/test/java/org/mozilla/fenix/components/AppStoreTest.kt

@@ -165,11 +165,11 @@ class AppStoreTest {
         appStore.dispatch(AppAction.RecentSyncedTabStateChange(loading)).join()
         assertEquals(loading, appStore.state.recentSyncedTabState)
 
-        val recentSyncedTab = RecentSyncedTab("device name", DeviceType.DESKTOP, "title", "url", null)
+        val recentSyncedTabs = listOf(RecentSyncedTab("device name", DeviceType.DESKTOP, "title", "url", null))
-        val success = RecentSyncedTabState.Success(recentSyncedTab)
+        val success = RecentSyncedTabState.Success(recentSyncedTabs)
         appStore.dispatch(AppAction.RecentSyncedTabStateChange(success)).join()
         assertEquals(success, appStore.state.recentSyncedTabState)
-        assertEquals(recentSyncedTab, (appStore.state.recentSyncedTabState as RecentSyncedTabState.Success).tab)
+        assertEquals(recentSyncedTabs, (appStore.state.recentSyncedTabState as RecentSyncedTabState.Success).tabs)
     }
 
     @Test

app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeatureTest.kt

@@ -178,7 +178,7 @@ class RecentSyncedTabFeatureTest {
         syncStore.setState(status = SyncStatus.Idle)
         runCurrent()
 
-        val expected = activeTab.toRecentSyncedTab(deviceAccessed1)
+        val expected = listOf(activeTab.toRecentSyncedTab(deviceAccessed1))
         verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expected))) }
     }
 
@@ -202,10 +202,10 @@ class RecentSyncedTabFeatureTest {
         syncStore.setState(status = SyncStatus.Idle)
         runCurrent()
 
-        val expectedTab = remoteTab.toRecentSyncedTab(deviceAccessed1)
+        val expectedTabs = listOf(remoteTab.toRecentSyncedTab(deviceAccessed1))
         verify {
             appStore.dispatch(
-                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
+                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTabs))
             )
         }
     }
@@ -229,41 +229,63 @@ class RecentSyncedTabFeatureTest {
         syncStore.setState(status = SyncStatus.Idle)
         runCurrent()
 
-        val expectedTab = remoteTab.toRecentSyncedTab(deviceAccessed1)
+        val expectedTabs = listOf(remoteTab.toRecentSyncedTab(deviceAccessed1))
         verify {
             appStore.dispatch(
-                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
+                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTabs))
             )
         }
     }
 
     @Test
-    fun `GIVEN tabs from different remote devices WHEN dispatching recent synced tab THEN most recently accessed device is used`() = runTest {
+    fun `GIVEN tabs from different remote devices WHEN dispatching recent synced tab THEN most recently accessed tabs are set in the Success state`() =
-        val account = mockk<Account>()
+        runTest {
-        syncStore.setState(account = account)
+            val account = mockk<Account>()
-        every { appStore.state } returns mockk {
+            syncStore.setState(account = account)
-            every { recentSyncedTabState } returns RecentSyncedTabState.Loading
+            every { appStore.state } returns mockk {
-        }
+                every { recentSyncedTabState } returns RecentSyncedTabState.Loading
-        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
+            }
-        val firstTab = createActiveTab("first", "https://local.com", null)
+            coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
-        val secondTab = createActiveTab("remote", "https://mozilla.org", null)
+            val firstDeviceTabs = listOf(
-        val syncedTabs = listOf(
+                createActiveTab("first", "https://local.com", null),
-            SyncedDeviceTabs(deviceAccessed1, listOf(firstTab)),
+                createActiveTab("second", "https://github.com", null)
-            SyncedDeviceTabs(deviceAccessed2, listOf(secondTab))
-        )
-        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns syncedTabs
-
-        feature.start()
-        syncStore.setState(status = SyncStatus.Idle)
-        runCurrent()
-
-        val expectedTab = secondTab.toRecentSyncedTab(deviceAccessed2)
-        verify {
-            appStore.dispatch(
-                AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTab))
             )
+            val secondDeviceTabs = listOf(
+                createActiveTab("first", "https://mozilla.org", null),
+                createActiveTab("second", "https://www.mozilla.org/en-US/firefox", null)
+            )
+            val currentTime = System.currentTimeMillis()
+            // Delay used to change last used times of tabs
+            val usedDelay = 5 * 60 * 1000
+            every { firstDeviceTabs[0].lastUsed } returns currentTime
+            every { firstDeviceTabs[1].lastUsed } returns currentTime - 2 * usedDelay
+            every { secondDeviceTabs[0].lastUsed } returns currentTime - usedDelay
+            every { secondDeviceTabs[1].lastUsed } returns currentTime - 3 * usedDelay
+            val syncedTabs = listOf(
+                SyncedDeviceTabs(deviceAccessed1, firstDeviceTabs),
+                SyncedDeviceTabs(deviceAccessed2, secondDeviceTabs)
+            )
+            coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns syncedTabs
+
+            feature.start()
+            syncStore.setState(status = SyncStatus.Idle)
+            runCurrent()
+
+            // The order of the tabs should be given by the `lastUsed` property
+            val expectedTabs =
+                (firstDeviceTabs + secondDeviceTabs).sortedByDescending { it.lastUsed }.map {
+                    if (it in firstDeviceTabs) {
+                        it.toRecentSyncedTab(deviceAccessed1)
+                    } else {
+                        it.toRecentSyncedTab(deviceAccessed2)
+                    }
+                }
+            verify {
+                appStore.dispatch(
+                    AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expectedTabs))
+                )
+            }
         }
-    }
 
     @Test
     fun `GIVEN sync tabs are disabled WHEN dispatching recent synced tab THEN dispatch none`() = runTest {
@@ -421,7 +443,7 @@ class RecentSyncedTabFeatureTest {
         syncStore.setState(status = SyncStatus.LoggedOut)
         runCurrent()
 
-        val expected = tab.toRecentSyncedTab(deviceAccessed1)
+        val expected = listOf(tab.toRecentSyncedTab(deviceAccessed1))
         verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expected))) }
         verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
     }
@@ -450,7 +472,7 @@ class RecentSyncedTabFeatureTest {
         syncStore.setState(status = SyncStatus.Idle)
         runCurrent()
 
-        val expected = activeTab.toRecentSyncedTab(deviceAccessed1, previewUrl)
+        val expected = listOf(activeTab.toRecentSyncedTab(deviceAccessed1, previewUrl))
         verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expected))) }
     }
 
@@ -477,7 +499,7 @@ class RecentSyncedTabFeatureTest {
         syncStore.setState(status = SyncStatus.Idle)
         runCurrent()
 
-        val expected = activeTab.toRecentSyncedTab(deviceAccessed1, null)
+        val expected = listOf(activeTab.toRecentSyncedTab(deviceAccessed1, null))
         verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expected))) }
     }
 
@@ -489,6 +511,7 @@ class RecentSyncedTabFeatureTest {
         val tab = mockk<Tab>()
         val tabEntry = TabEntry(title, url, iconUrl)
         every { tab.active() } returns tabEntry
+        every { tab.lastUsed } returns System.currentTimeMillis()
         return tab
     }