[fenix] Fixes https://github.com/mozilla-mobile/fenix/issues/26124: recent synced tab will use preview image URLs from history

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -284,6 +284,7 @@ class HomeFragment : Fragment() {
                         syncStore = requireComponents.backgroundServices.syncStore,
                         storage = requireComponents.backgroundServices.syncedTabsStorage,
                         accountManager = requireComponents.backgroundServices.accountManager,
+                        historyStorage = requireComponents.core.historyStorage,
                         coroutineScope = viewLifecycleOwner.lifecycleScope,
                     ),
                     owner = viewLifecycleOwner,

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

@@ -8,6 +8,7 @@ import android.content.Context
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
+import mozilla.components.concept.storage.HistoryStorage
 import mozilla.components.concept.sync.DeviceType
 import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage
 import mozilla.components.lib.state.ext.flow
@@ -19,11 +20,13 @@ import mozilla.components.service.fxa.store.SyncStatus
 import mozilla.components.service.fxa.store.SyncStore
 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 java.util.concurrent.TimeUnit
 
 /**
  * Delegate to handle layout updates and dispatch actions related to the recent synced tab.
@@ -32,14 +35,17 @@ import org.mozilla.fenix.GleanMetrics.RecentSyncedTabs
  * @property syncStore Store to observe for changes to Sync and account status.
  * @property storage Storage layer for synced tabs.
  * @property accountManager Account manager to initiate Syncs and refresh devices.
+ * @property historyStorage Storage for searching history for preview image URLs matching synced tab.
  * @property coroutineScope The scope to collect Sync state Flow updates in.
  */
+@Suppress("LongParameterList")
 class RecentSyncedTabFeature(
     private val context: Context,
     private val appStore: AppStore,
     private val syncStore: SyncStore,
     private val storage: SyncedTabsStorage,
     private val accountManager: FxaAccountManager,
+    private val historyStorage: HistoryStorage,
     private val coroutineScope: CoroutineScope,
 ) : LifecycleAwareFeature {
 
@@ -109,12 +115,26 @@ class RecentSyncedTabFeature(
             .maxByOrNull { it.device.lastAccessTime ?: 0 }
             ?.let {
                 val tab = it.tabs.firstOrNull()?.active() ?: return
+
+                val currentTime = System.currentTimeMillis()
+                val maxAgeInMs = TimeUnit.DAYS.toMillis(DAYS_HISTORY_FOR_PREVIEW_IMAGE)
+                val history = historyStorage.getDetailedVisits(
+                    start = currentTime - maxAgeInMs,
+                    end = currentTime
+                )
+
+                // 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
+                }?.previewImageUrl
+
                 RecentSyncedTab(
                     deviceDisplayName = it.device.displayName,
                     deviceType = it.device.deviceType,
                     title = tab.title,
                     url = tab.url,
-                    iconUrl = tab.iconUrl
+                    previewImageUrl = previewImageUrl
                 )
             }
 
@@ -152,6 +172,15 @@ class RecentSyncedTabFeature(
     private fun isSyncedTabsEngineEnabled(): Boolean {
         return SyncEnginesStorage(context).getStatus()[SyncEngine.Tabs] ?: true
     }
+
+    companion object {
+        /**
+         * The number of days to search history for a preview image URL to display for a synced
+         * tab.
+         */
+
+        const val DAYS_HISTORY_FOR_PREVIEW_IMAGE = 3L
+    }
 }
 
 /**
@@ -180,12 +209,12 @@ sealed class RecentSyncedTabState {
  * @param deviceDisplayName The device the tab was viewed on.
  * @param title The title of the tab.
  * @param url The url of the tab.
- * @param iconUrl The url used to retrieve the icon of the tab.
+ * @param previewImageUrl The url used to retrieve the preview image of the tab.
  */
 data class RecentSyncedTab(
     val deviceDisplayName: String,
     val deviceType: DeviceType,
     val title: String,
     val url: String,
-    val iconUrl: String?,
+    val previewImageUrl: String?,
 )

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

@@ -26,6 +26,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
+import androidx.compose.ui.layout.ContentScale
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.style.TextOverflow
@@ -34,6 +35,7 @@ import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
 import mozilla.components.concept.sync.DeviceType
 import org.mozilla.fenix.R
+import org.mozilla.fenix.compose.Image
 import org.mozilla.fenix.compose.ThumbnailCard
 import org.mozilla.fenix.compose.button.Button
 import org.mozilla.fenix.home.recentsyncedtabs.RecentSyncedTab
@@ -68,13 +70,23 @@ fun RecentSyncedTab(
                 if (tab == null) {
                     RecentTabImagePlaceholder()
                 } else {
-                    ThumbnailCard(
+                    val imageModifier = Modifier
-                        url = tab.url,
+                        .size(108.dp, 80.dp)
-                        key = tab.url.hashCode().toString(),
+                        .clip(RoundedCornerShape(8.dp))
-                        modifier = Modifier
+
-                            .size(108.dp, 80.dp)
+                    if (tab.previewImageUrl != null) {
-                            .clip(RoundedCornerShape(8.dp))
+                        Image(
-                    )
+                            url = tab.previewImageUrl,
+                            contentScale = ContentScale.Crop,
+                            modifier = imageModifier
+                        )
+                    } else {
+                        ThumbnailCard(
+                            url = tab.url,
+                            key = tab.url.hashCode().toString(),
+                            modifier = imageModifier
+                        )
+                    }
                 }
 
                 Spacer(modifier = Modifier.width(16.dp))
@@ -198,7 +210,7 @@ private fun LoadedRecentSyncedTab() {
         deviceType = DeviceType.DESKTOP,
         title = "This is a long site title",
         url = "https://mozilla.org",
-        iconUrl = "https://mozilla.org",
+        previewImageUrl = "https://mozilla.org",
     )
     FirefoxTheme(theme = Theme.getTheme()) {
         RecentSyncedTab(

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

@@ -20,6 +20,9 @@ import kotlinx.coroutines.test.setMain
 import mozilla.components.browser.storage.sync.SyncedDeviceTabs
 import mozilla.components.browser.storage.sync.Tab
 import mozilla.components.browser.storage.sync.TabEntry
+import mozilla.components.concept.storage.HistoryStorage
+import mozilla.components.concept.storage.VisitInfo
+import mozilla.components.concept.storage.VisitType
 import mozilla.components.concept.sync.Device
 import mozilla.components.concept.sync.DeviceType
 import mozilla.components.feature.syncedtabs.storage.SyncedTabsStorage
@@ -88,7 +91,8 @@ class RecentSyncedTabFeatureTest {
 
     private val appStore: AppStore = mockk()
     private val accountManager: FxaAccountManager = mockk(relaxed = true)
-    private val storage: SyncedTabsStorage = mockk()
+    private val syncedTabsStorage: SyncedTabsStorage = mockk()
+    private val historyStorage: HistoryStorage = mockk()
 
     private val syncStore = SyncStore()
 
@@ -109,7 +113,8 @@ class RecentSyncedTabFeatureTest {
             appStore = appStore,
             syncStore = syncStore,
             accountManager = accountManager,
-            storage = storage,
+            storage = syncedTabsStorage,
+            historyStorage = historyStorage,
             coroutineScope = TestScope(),
         )
     }
@@ -160,8 +165,9 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.Loading
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val activeTab = createActiveTab()
-        coEvery { storage.getSyncedDeviceTabs() } returns listOf(
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(
             SyncedDeviceTabs(
                 device = deviceAccessed1,
                 tabs = listOf(activeTab)
@@ -183,13 +189,14 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.Loading
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val localTab = createActiveTab("local", "https://local.com", null)
         val remoteTab = createActiveTab("remote", "https://mozilla.org", null)
         val syncedTabs = listOf(
             SyncedDeviceTabs(currentDevice, listOf(localTab)),
             SyncedDeviceTabs(deviceAccessed1, listOf(remoteTab))
         )
-        coEvery { storage.getSyncedDeviceTabs() } returns syncedTabs
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns syncedTabs
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -210,12 +217,13 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.Loading
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val remoteTab = createActiveTab("remote", "https://mozilla.org", null)
         val syncedTabs = listOf(
             SyncedDeviceTabs(deviceAccessed2, listOf()),
             SyncedDeviceTabs(deviceAccessed1, listOf(remoteTab))
         )
-        coEvery { storage.getSyncedDeviceTabs() } returns syncedTabs
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns syncedTabs
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -236,13 +244,14 @@ class RecentSyncedTabFeatureTest {
         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)
         val secondTab = createActiveTab("remote", "https://mozilla.org", null)
         val syncedTabs = listOf(
             SyncedDeviceTabs(deviceAccessed1, listOf(firstTab)),
             SyncedDeviceTabs(deviceAccessed2, listOf(secondTab))
         )
-        coEvery { storage.getSyncedDeviceTabs() } returns syncedTabs
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns syncedTabs
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -271,7 +280,7 @@ class RecentSyncedTabFeatureTest {
         val syncedTabs = listOf(
             SyncedDeviceTabs(deviceAccessed1, listOf(firstTab)),
         )
-        coEvery { storage.getSyncedDeviceTabs() } returns syncedTabs
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns syncedTabs
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -291,8 +300,9 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.Loading
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val tab = SyncedDeviceTabs(deviceAccessed1, listOf(createActiveTab()))
-        coEvery { storage.getSyncedDeviceTabs() } returns listOf(tab)
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(tab)
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -308,8 +318,9 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.None
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val tab = SyncedDeviceTabs(deviceAccessed1, listOf(createActiveTab()))
-        coEvery { storage.getSyncedDeviceTabs() } returns listOf(tab)
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(tab)
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -325,8 +336,9 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.None
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val tab = SyncedDeviceTabs(deviceAccessed1, listOf(createActiveTab()))
-        coEvery { storage.getSyncedDeviceTabs() } returns listOf(tab)
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(tab)
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -348,7 +360,7 @@ class RecentSyncedTabFeatureTest {
         }
         val tabs1 = listOf(SyncedDeviceTabs(deviceAccessed1, listOf(createActiveTab())))
         val tabs2 = listOf(SyncedDeviceTabs(deviceAccessed2, listOf(createActiveTab())))
-        coEvery { storage.getSyncedDeviceTabs() } returnsMany listOf(tabs1, tabs2)
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returnsMany listOf(tabs1, tabs2)
 
         feature.start()
         syncStore.setState(status = SyncStatus.Idle)
@@ -396,8 +408,9 @@ class RecentSyncedTabFeatureTest {
         every { appStore.state } returns mockk {
             every { recentSyncedTabState } returns RecentSyncedTabState.None
         }
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
         val tab = createActiveTab()
-        coEvery { storage.getSyncedDeviceTabs() } returns listOf(
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(
             SyncedDeviceTabs(deviceAccessed1, listOf(tab))
         )
 
@@ -413,6 +426,61 @@ class RecentSyncedTabFeatureTest {
         verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.None)) }
     }
 
+    @Test
+    fun `GIVEN history entry contains synced tab host and has a preview image URL WHEN dispatched THEN preview url is included`() = runTest {
+        val account = mockk<Account>()
+        syncStore.setState(account = account)
+        every { appStore.state } returns mockk {
+            every { recentSyncedTabState } returns RecentSyncedTabState.Loading
+        }
+        val activeTab = createActiveTab()
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(
+            SyncedDeviceTabs(
+                device = deviceAccessed1,
+                tabs = listOf(activeTab)
+            )
+        )
+        val longerThanSyncedTabUrl = activeTab.active().url + "/some/more/paths"
+        val previewUrl = "preview"
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf(
+            activeTab.toVisitInfo(longerThanSyncedTabUrl, previewUrl)
+        )
+
+        feature.start()
+        syncStore.setState(status = SyncStatus.Idle)
+        runCurrent()
+
+        val expected = activeTab.toRecentSyncedTab(deviceAccessed1, previewUrl)
+        verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expected))) }
+    }
+
+    @Test
+    fun `GIVEN history entry contains synced tab host but has no preview image URL WHEN dispatched THEN preview url is not included`() = runTest {
+        val account = mockk<Account>()
+        syncStore.setState(account = account)
+        every { appStore.state } returns mockk {
+            every { recentSyncedTabState } returns RecentSyncedTabState.Loading
+        }
+        val activeTab = createActiveTab()
+        coEvery { syncedTabsStorage.getSyncedDeviceTabs() } returns listOf(
+            SyncedDeviceTabs(
+                device = deviceAccessed1,
+                tabs = listOf(activeTab)
+            )
+        )
+        val longerThanSyncedTabUrl = activeTab.active().url + "/some/more/paths"
+        coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf(
+            activeTab.toVisitInfo(longerThanSyncedTabUrl, null)
+        )
+
+        feature.start()
+        syncStore.setState(status = SyncStatus.Idle)
+        runCurrent()
+
+        val expected = activeTab.toRecentSyncedTab(deviceAccessed1, null)
+        verify { appStore.dispatch(AppAction.RecentSyncedTabStateChange(RecentSyncedTabState.Success(expected))) }
+    }
+
     private fun createActiveTab(
         title: String = "title",
         url: String = "url",
@@ -424,12 +492,15 @@ class RecentSyncedTabFeatureTest {
         return tab
     }
 
-    private fun Tab.toRecentSyncedTab(device: Device) = RecentSyncedTab(
+    private fun Tab.toRecentSyncedTab(
+        device: Device,
+        previewImageUrl: String? = null
+    ) = RecentSyncedTab(
         deviceDisplayName = device.displayName,
         deviceType = device.deviceType,
         title = this.active().title,
         url = this.active().url,
-        iconUrl = this.active().iconUrl
+        previewImageUrl = previewImageUrl
     )
 
     private fun SyncStore.setState(
@@ -444,4 +515,13 @@ class RecentSyncedTabFeatureTest {
         }
         this.waitUntilIdle()
     }
+
+    private fun Tab.toVisitInfo(url: String, previewUrl: String?) = VisitInfo(
+        title = this.active().title,
+        url = url,
+        visitTime = 0L,
+        visitType = VisitType.TYPED,
+        previewImageUrl = previewUrl,
+        isRemote = false
+    )
 }

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

@@ -64,7 +64,7 @@ class DefaultRecentSyncedTabControllerTest {
             deviceType = DeviceType.DESKTOP,
             title = "title",
             url = url,
-            iconUrl = null
+            previewImageUrl = null
         )
         val store = BrowserStore(
             initialState = BrowserState(
@@ -100,7 +100,7 @@ class DefaultRecentSyncedTabControllerTest {
             deviceType = DeviceType.DESKTOP,
             title = "title",
             url = url,
-            iconUrl = null
+            previewImageUrl = null
         )
         val store = BrowserStore(
             initialState = BrowserState(
@@ -155,7 +155,7 @@ class DefaultRecentSyncedTabControllerTest {
             deviceType = deviceType,
             title = "title",
             url = url,
-            iconUrl = null
+            previewImageUrl = null
         )
 
         every { tabsUseCases.selectOrAddTab } returns mockk(relaxed = true)