diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 7d76fb970..512376a59 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/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, diff --git a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt index 8d325d612..103d6a6d0 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeature.kt +++ b/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?, ) diff --git a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt b/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt index d14f8e5ce..150cb6fc4 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentsyncedtabs/view/RecentSyncedTab.kt +++ b/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( - url = tab.url, - key = tab.url.hashCode().toString(), - modifier = Modifier - .size(108.dp, 80.dp) - .clip(RoundedCornerShape(8.dp)) - ) + val imageModifier = Modifier + .size(108.dp, 80.dp) + .clip(RoundedCornerShape(8.dp)) + + if (tab.previewImageUrl != null) { + 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( diff --git a/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeatureTest.kt index 99174dd28..3cddf06b4 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/RecentSyncedTabFeatureTest.kt +++ b/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() + 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() + 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 + ) } diff --git a/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/controller/DefaultRecentSyncedTabControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/controller/DefaultRecentSyncedTabControllerTest.kt index 23a4c9196..4869b1adc 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recentsyncedtabs/controller/DefaultRecentSyncedTabControllerTest.kt +++ b/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)