ishan
/
fenix
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

For #26399 - Use a list of tabs for recent synced tabs success state

This commit is contained in:
Alexandru2909 2022-08-11 16:22:39 +03:00 committed by mergify[bot]
parent 77b959ec97
commit e2f1c5fc9c
4 changed files with 104 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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 }
?.let {
val tab = it.tabs.firstOrNull()?.active() ?: return
.flatMap {
it.tabs.map { tab ->
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,
deviceType = it.device.deviceType,
title = tab.title,
url = tab.url,
deviceDisplayName = deviceTab.device.displayName,
deviceType = deviceTab.device.deviceType,
title = activeTabEntry.title,
url = activeTabEntry.url,
previewImageUrl = previewImageUrl
)
}
if (syncedTab == null) {
if (syncedTabs.isEmpty()) {
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
)

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

@ -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,

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

@ -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 success = RecentSyncedTabState.Success(recentSyncedTab)
val recentSyncedTabs = listOf(RecentSyncedTab("device name", DeviceType.DESKTOP, "title", "url", null))
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

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

@ -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 {
val account = mockk<Account>()
syncStore.setState(account = account)
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 { 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))
fun `GIVEN tabs from different remote devices WHEN dispatching recent synced tab THEN most recently accessed tabs are set in the Success state`() =
runTest {
val account = mockk<Account>()
syncStore.setState(account = account)
every { appStore.state } returns mockk {
every { recentSyncedTabState } returns RecentSyncedTabState.Loading
}
coEvery { historyStorage.getDetailedVisits(any(), any()) } returns listOf()
val firstDeviceTabs = listOf(
createActiveTab("first", "https://local.com", null),
createActiveTab("second", "https://github.com", null)
)
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
}