For #22155 - New BrowserState.actualInactiveTabs public method

This allows querying from all throughout the app which of the current tabs are
inactive while taking into consideration whether the feature is enabled or not
such that when the feature is disabled it will always return an empty result.
upstream-sync
Mugurell 3 years ago committed by mergify[bot]
parent 2089c29fea
commit f9b67091c6

@ -81,7 +81,7 @@ import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.metrics.MozillaProductDetector
import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks
import org.mozilla.fenix.tabstray.ext.inactiveTabs import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings
/** /**
@ -628,7 +628,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
tabViewSetting.set(settings.getTabViewPingString()) tabViewSetting.set(settings.getTabViewPingString())
closeTabSetting.set(settings.getTabTimeoutPingString()) closeTabSetting.set(settings.getTabTimeoutPingString())
inactiveTabsCount.set(browserStore.state.inactiveTabs.size.toLong()) inactiveTabsCount.set(browserStore.state.potentialInactiveTabs.size.toLong())
val installSourcePackage = if (SDK_INT >= Build.VERSION_CODES.R) { val installSourcePackage = if (SDK_INT >= Build.VERSION_CODES.R) {
packageManager.getInstallSourceInfo(packageName).installingPackageName packageManager.getInstallSourceInfo(packageName).installingPackageName

@ -11,10 +11,22 @@ import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.feature.tabs.ext.hasMediaPlayed import mozilla.components.feature.tabs.ext.hasMediaPlayed
import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recenttabs.RecentTab
import org.mozilla.fenix.tabstray.browser.TabGroup import org.mozilla.fenix.tabstray.browser.TabGroup
import org.mozilla.fenix.tabstray.browser.maxActiveTime
import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithSearchTerm import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithSearchTerm
import org.mozilla.fenix.tabstray.ext.isNormalTabInactive
import org.mozilla.fenix.utils.Settings
import java.util.concurrent.TimeUnit
import kotlin.math.max import kotlin.math.max
/**
* The time until which a tab is considered in-active (in days).
*/
const val DEFAULT_ACTIVE_DAYS = 14L
/**
* The maximum time from when a tab was created or accessed until it is considered "inactive".
*/
val maxActiveTime = TimeUnit.DAYS.toMillis(DEFAULT_ACTIVE_DAYS)
/** /**
* Get the last opened normal tab, last tab with in progress media and last search term group, if available. * Get the last opened normal tab, last tab with in progress media and last search term group, if available.
* *
@ -109,3 +121,23 @@ fun List<TabSessionState>.toSearchGroup(
return groups to remainderTabs return groups to remainderTabs
} }
/**
* List of all inactive tabs based on [maxActiveTime].
* The user may have disabled the feature so for user interactions consider using the [actualInactiveTabs] method
* or an in place check of the feature status.
*/
val BrowserState.potentialInactiveTabs: List<TabSessionState>
get() = normalTabs.filter { it.isNormalTabInactive(maxActiveTime) }
/**
* List of all inactive tabs based on [maxActiveTime].
* The result will be always be empty if the user disabled the feature.
*/
fun BrowserState.actualInactiveTabs(settings: Settings): List<TabSessionState> {
return if (settings.inactiveTabsAreEnabled) {
potentialInactiveTabs
} else {
emptyList()
}
}

@ -21,8 +21,8 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.home.HomeFragment import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.tabstray.browser.DEFAULT_ACTIVE_DAYS import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS
import org.mozilla.fenix.tabstray.ext.inactiveTabs import org.mozilla.fenix.ext.potentialInactiveTabs
import java.util.concurrent.TimeUnit import java.util.concurrent.TimeUnit
interface TabsTrayController { interface TabsTrayController {
@ -218,7 +218,7 @@ class DefaultTabsTrayController(
override fun handleDeleteAllInactiveTabs() { override fun handleDeleteAllInactiveTabs() {
metrics.track(Event.TabsTrayCloseAllInactiveTabs) metrics.track(Event.TabsTrayCloseAllInactiveTabs)
browserStore.state.inactiveTabs.map { it.id }.let { browserStore.state.potentialInactiveTabs.map { it.id }.let {
tabsUseCases.removeTabs(it) tabsUseCases.removeTabs(it)
} }
showUndoSnackbarForTab(false) showUndoSnackbarForTab(false)

@ -29,7 +29,7 @@ import org.mozilla.fenix.R
import org.mozilla.fenix.browser.infobanner.InfoBanner import org.mozilla.fenix.browser.infobanner.InfoBanner
import org.mozilla.fenix.databinding.ComponentTabstray2Binding import org.mozilla.fenix.databinding.ComponentTabstray2Binding
import org.mozilla.fenix.databinding.OnboardingInactiveTabsCfrBinding import org.mozilla.fenix.databinding.OnboardingInactiveTabsCfrBinding
import org.mozilla.fenix.tabstray.ext.inactiveTabs import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
@ -49,7 +49,7 @@ class TabsTrayInactiveTabsOnboardingBinding(
.ifChanged() .ifChanged()
.collect { .collect {
val inactiveTabsList = val inactiveTabsList =
if (settings.inactiveTabsAreEnabled) { store.state.inactiveTabs } else emptyList() if (settings.inactiveTabsAreEnabled) { store.state.potentialInactiveTabs } else emptyList()
if (inactiveTabsList.isNotEmpty() && shouldShowOnboardingForInactiveTabs()) { if (inactiveTabsList.isNotEmpty() && shouldShowOnboardingForInactiveTabs()) {
createInactiveCFR() createInactiveCFR()
} }

@ -11,21 +11,11 @@ import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.tabstray.TabViewHolder import mozilla.components.browser.tabstray.TabViewHolder
import mozilla.components.feature.tabs.tabstray.TabsFeature import mozilla.components.feature.tabs.tabstray.TabsFeature
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.maxActiveTime
import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.tabstray.ext.browserAdapter import org.mozilla.fenix.tabstray.ext.browserAdapter
import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter
import org.mozilla.fenix.tabstray.ext.isNormalTabInactive import org.mozilla.fenix.tabstray.ext.isNormalTabInactive
import java.util.concurrent.TimeUnit
/**
* The time until which a tab is considered in-active (in days).
*/
const val DEFAULT_ACTIVE_DAYS = 14L
/**
* The maximum time from when a tab was created or accessed until it is considered "inactive".
*/
val maxActiveTime = TimeUnit.DAYS.toMillis(DEFAULT_ACTIVE_DAYS)
class NormalBrowserTrayList @JvmOverloads constructor( class NormalBrowserTrayList @JvmOverloads constructor(
context: Context, context: Context,

@ -10,6 +10,7 @@ import mozilla.components.browser.tabstray.TabsTray
import mozilla.components.feature.tabs.tabstray.TabsFeature import mozilla.components.feature.tabs.tabstray.TabsFeature
import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.maxActiveTime
import org.mozilla.fenix.ext.toSearchGroup import org.mozilla.fenix.ext.toSearchGroup
import org.mozilla.fenix.tabstray.ext.browserAdapter import org.mozilla.fenix.tabstray.ext.browserAdapter
import org.mozilla.fenix.tabstray.ext.hasSearchTerm import org.mozilla.fenix.tabstray.ext.hasSearchTerm

@ -9,7 +9,7 @@ import mozilla.components.browser.state.selector.privateTabs
import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.TabSessionState
import org.mozilla.fenix.ext.toSearchGroup import org.mozilla.fenix.ext.toSearchGroup
import org.mozilla.fenix.tabstray.browser.maxActiveTime import org.mozilla.fenix.ext.maxActiveTime
/** /**
* The currently selected tab if there's one that is private. * The currently selected tab if there's one that is private.
@ -32,12 +32,6 @@ fun BrowserState.findPrivateTab(tabId: String): TabSessionState? {
return privateTabs.firstOrNull { it.id == tabId } return privateTabs.firstOrNull { it.id == tabId }
} }
/**
* The list of inactive tabs in the tabs tray filtered based on [maxActiveTime].
*/
val BrowserState.inactiveTabs: List<TabSessionState>
get() = normalTabs.filter { it.isNormalTabInactive(maxActiveTime) }
/** /**
* The list of normal tabs in the tabs tray filtered appropriately based on feature flags. * The list of normal tabs in the tabs tray filtered appropriately based on feature flags.
*/ */

@ -21,11 +21,11 @@ import org.mozilla.fenix.tabstray.TabsTrayAction
import org.mozilla.fenix.tabstray.TabsTrayInteractor import org.mozilla.fenix.tabstray.TabsTrayInteractor
import org.mozilla.fenix.tabstray.TabsTrayStore import org.mozilla.fenix.tabstray.TabsTrayStore
import org.mozilla.fenix.tabstray.browser.containsTabId import org.mozilla.fenix.tabstray.browser.containsTabId
import org.mozilla.fenix.tabstray.browser.maxActiveTime import org.mozilla.fenix.ext.maxActiveTime
import org.mozilla.fenix.tabstray.ext.browserAdapter import org.mozilla.fenix.tabstray.ext.browserAdapter
import org.mozilla.fenix.tabstray.ext.defaultBrowserLayoutColumns import org.mozilla.fenix.tabstray.ext.defaultBrowserLayoutColumns
import org.mozilla.fenix.tabstray.ext.getNormalTrayTabs import org.mozilla.fenix.tabstray.ext.getNormalTrayTabs
import org.mozilla.fenix.tabstray.ext.inactiveTabs import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.tabstray.ext.titleHeaderAdapter import org.mozilla.fenix.tabstray.ext.titleHeaderAdapter
import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter
import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithSearchTerm import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithSearchTerm
@ -92,7 +92,7 @@ class NormalBrowserPageViewHolder(
// Update tabs into the inactive adapter. // Update tabs into the inactive adapter.
if (inactiveTabsAreEnabled && selectedTab.isNormalTabInactive(maxActiveTime)) { if (inactiveTabsAreEnabled && selectedTab.isNormalTabInactive(maxActiveTime)) {
val inactiveTabsList = browserStore.state.inactiveTabs val inactiveTabsList = browserStore.state.potentialInactiveTabs
// We want to expand the inactive section first before we want to fire our scroll observer. // We want to expand the inactive section first before we want to fire our scroll observer.
appStore.dispatch(AppAction.UpdateInactiveExpanded(true)) appStore.dispatch(AppAction.UpdateInactiveExpanded(true))

@ -33,7 +33,7 @@ import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine
import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.metrics.MozillaProductDetector
import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.components.toolbar.ToolbarPosition
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.tabstray.ext.inactiveTabs import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.utils.BrowsersCache import org.mozilla.fenix.utils.BrowsersCache
import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings
import org.robolectric.annotation.Config import org.robolectric.annotation.Config
@ -144,7 +144,7 @@ class FenixApplicationTest {
every { application.reportHomeScreenMetrics(settings) } just Runs every { application.reportHomeScreenMetrics(settings) } just Runs
every { settings.inactiveTabsAreEnabled } returns true every { settings.inactiveTabsAreEnabled } returns true
mockkStatic("org.mozilla.fenix.tabstray.ext.TabSelectorsKt") { mockkStatic("org.mozilla.fenix.tabstray.ext.TabSelectorsKt") {
every { browserStore.state.inactiveTabs } returns listOf(mockk(), mockk()) every { browserStore.state.potentialInactiveTabs } returns listOf(mockk(), mockk())
application.setStartupMetrics(browserStore, settings, browsersCache, mozillaProductDetector) application.setStartupMetrics(browserStore, settings, browsersCache, mozillaProductDetector)

@ -4,6 +4,7 @@
package org.mozilla.fenix.ext package org.mozilla.fenix.ext
import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.LastMediaAccessState import mozilla.components.browser.state.state.LastMediaAccessState
@ -11,8 +12,10 @@ import mozilla.components.browser.state.state.createTab
import mozilla.components.concept.storage.HistoryMetadataKey import mozilla.components.concept.storage.HistoryMetadataKey
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test import org.junit.Test
import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recenttabs.RecentTab
import org.mozilla.fenix.utils.Settings
class BrowserStateTest { class BrowserStateTest {
@ -318,4 +321,96 @@ class BrowserStateTest {
) )
assertEquals(normalTab3.id, browserState.secondToLastOpenedNormalTab!!.id) assertEquals(normalTab3.id, browserState.secondToLastOpenedNormalTab!!.id)
} }
@Test
fun `GIVEN a list of tabs WHEN potentialInactiveTabs is called THEN return the normal tabs which haven't been active lately`() {
// An inactive tab is one created or accessed more than [maxActiveTime] ago
// checked against [System.currentTimeMillis]
//
// createTab() sets the [createdTime] property to [System.currentTimeMillis] this meaning
// that the default tab is considered active.
val normalTab1 = createTab(url = "url1", id = "1", createdAt = 1)
val normalTab2 = createTab(url = "url2", id = "2")
val normalTab3 = createTab(url = "url3", id = "3", createdAt = 1)
val normalTab4 = createTab(url = "url4", id = "4")
val privateTab1 = createTab(url = "url1", id = "6", private = true)
val privateTab2 = createTab(url = "url2", id = "7", private = true, createdAt = 1)
val privateTab3 = createTab(url = "url3", id = "8", private = true)
val privateTab4 = createTab(url = "url4", id = "9", private = true, createdAt = 1)
val browserState = BrowserState(
tabs = listOf(
normalTab1, normalTab2, normalTab3, normalTab4,
privateTab1, privateTab2, privateTab3, privateTab4
)
)
val result = browserState.potentialInactiveTabs
assertEquals(2, result.size)
assertTrue(result.containsAll(listOf(normalTab1, normalTab3)))
}
@Test
fun `GIVEN inactiveTabs feature is disabled WHEN actualInactiveTabs is called THEN return an empty result`() {
// An inactive tab is one created or accessed more than [maxActiveTime] ago
// checked against [System.currentTimeMillis]
//
// createTab() sets the [createdTime] property to [System.currentTimeMillis] this meaning
// that the default tab is considered active.
val normalTab1 = createTab(url = "url1", id = "1", createdAt = 1)
val normalTab2 = createTab(url = "url2", id = "2")
val normalTab3 = createTab(url = "url3", id = "3", createdAt = 1)
val normalTab4 = createTab(url = "url4", id = "4")
val privateTab1 = createTab(url = "url1", id = "6", private = true)
val privateTab2 = createTab(url = "url2", id = "7", private = true, createdAt = 1)
val privateTab3 = createTab(url = "url3", id = "8", private = true)
val privateTab4 = createTab(url = "url4", id = "9", private = true, createdAt = 1)
val browserState = BrowserState(
tabs = listOf(
normalTab1, normalTab2, normalTab3, normalTab4,
privateTab1, privateTab2, privateTab3, privateTab4
)
)
val settings: Settings = mockk() {
every { inactiveTabsAreEnabled } returns false
}
val result = browserState.actualInactiveTabs(settings)
assertTrue(result.isEmpty())
}
@Test
fun `GIVEN inactiveTabs feature is enabled WHEN actualInactiveTabs is called THEN return the normal tabs which haven't been active lately`() {
// An inactive tab is one created or accessed more than [maxActiveTime] ago
// checked against [System.currentTimeMillis]
//
// createTab() sets the [createdTime] property to [System.currentTimeMillis] this meaning
// that the default tab is considered active.
val normalTab1 = createTab(url = "url1", id = "1", createdAt = 1)
val normalTab2 = createTab(url = "url2", id = "2")
val normalTab3 = createTab(url = "url3", id = "3", createdAt = 1)
val normalTab4 = createTab(url = "url4", id = "4")
val privateTab1 = createTab(url = "url1", id = "6", private = true)
val privateTab2 = createTab(url = "url2", id = "7", private = true, createdAt = 1)
val privateTab3 = createTab(url = "url3", id = "8", private = true)
val privateTab4 = createTab(url = "url4", id = "9", private = true, createdAt = 1)
val browserState = BrowserState(
tabs = listOf(
normalTab1, normalTab2, normalTab3, normalTab4,
privateTab1, privateTab2, privateTab3, privateTab4
)
)
val settings: Settings = mockk() {
every { inactiveTabsAreEnabled } returns true
}
val result = browserState.actualInactiveTabs(settings)
assertEquals(2, result.size)
assertTrue(result.containsAll(listOf(normalTab1, normalTab3)))
}
} }

@ -33,8 +33,8 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.home.HomeFragment import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.tabstray.browser.maxActiveTime import org.mozilla.fenix.ext.maxActiveTime
import org.mozilla.fenix.tabstray.ext.inactiveTabs import org.mozilla.fenix.ext.potentialInactiveTabs
class DefaultTabsTrayControllerTest { class DefaultTabsTrayControllerTest {
@MockK(relaxed = true) @MockK(relaxed = true)
@ -409,7 +409,7 @@ class DefaultTabsTrayControllerTest {
every { browserStore.state } returns mockk() every { browserStore.state } returns mockk()
try { try {
mockkStatic("mozilla.components.browser.state.selector.SelectorsKt") mockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
every { browserStore.state.inactiveTabs } returns listOf(inactiveTab) every { browserStore.state.potentialInactiveTabs } returns listOf(inactiveTab)
controller.handleDeleteAllInactiveTabs() controller.handleDeleteAllInactiveTabs()
@ -433,7 +433,7 @@ class DefaultTabsTrayControllerTest {
every { browserStore.state } returns mockk() every { browserStore.state } returns mockk()
try { try {
mockkStatic("mozilla.components.browser.state.selector.SelectorsKt") mockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
every { browserStore.state.inactiveTabs } returns listOf(inactiveTab) every { browserStore.state.potentialInactiveTabs } returns listOf(inactiveTab)
createController().handleDeleteAllInactiveTabs() createController().handleDeleteAllInactiveTabs()

@ -10,7 +10,7 @@ import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue import org.junit.Assert.assertTrue
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import org.mozilla.fenix.tabstray.browser.DEFAULT_ACTIVE_DAYS import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS
import java.util.concurrent.TimeUnit import java.util.concurrent.TimeUnit
class TabSessionStateKtTest { class TabSessionStateKtTest {

Loading…
Cancel
Save