diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index fcd69da513..7cb154fa84 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -81,7 +81,7 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.toolbar.ToolbarPosition 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 /** @@ -628,7 +628,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { tabViewSetting.set(settings.getTabViewPingString()) 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) { packageManager.getInstallSourceInfo(packageName).installingPackageName diff --git a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt index 69bacb9026..9843bdb49b 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/BrowserState.kt @@ -11,10 +11,22 @@ import mozilla.components.browser.state.state.TabSessionState import mozilla.components.feature.tabs.ext.hasMediaPlayed import org.mozilla.fenix.home.recenttabs.RecentTab 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.isNormalTabInactive +import org.mozilla.fenix.utils.Settings +import java.util.concurrent.TimeUnit 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. * @@ -109,3 +121,23 @@ fun List.toSearchGroup( 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 + 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 { + return if (settings.inactiveTabsAreEnabled) { + potentialInactiveTabs + } else { + emptyList() + } +} diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt index dc65ddeace..bce694ae82 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayController.kt @@ -21,8 +21,8 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.home.HomeFragment -import org.mozilla.fenix.tabstray.browser.DEFAULT_ACTIVE_DAYS -import org.mozilla.fenix.tabstray.ext.inactiveTabs +import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS +import org.mozilla.fenix.ext.potentialInactiveTabs import java.util.concurrent.TimeUnit interface TabsTrayController { @@ -218,7 +218,7 @@ class DefaultTabsTrayController( override fun handleDeleteAllInactiveTabs() { metrics.track(Event.TabsTrayCloseAllInactiveTabs) - browserStore.state.inactiveTabs.map { it.id }.let { + browserStore.state.potentialInactiveTabs.map { it.id }.let { tabsUseCases.removeTabs(it) } showUndoSnackbarForTab(false) diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInactiveTabsOnboardingBinding.kt b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInactiveTabsOnboardingBinding.kt index a9414a7341..a4a382c27f 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInactiveTabsOnboardingBinding.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayInactiveTabsOnboardingBinding.kt @@ -29,7 +29,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.browser.infobanner.InfoBanner import org.mozilla.fenix.databinding.ComponentTabstray2Binding 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 @OptIn(ExperimentalCoroutinesApi::class) @@ -49,7 +49,7 @@ class TabsTrayInactiveTabsOnboardingBinding( .ifChanged() .collect { val inactiveTabsList = - if (settings.inactiveTabsAreEnabled) { store.state.inactiveTabs } else emptyList() + if (settings.inactiveTabsAreEnabled) { store.state.potentialInactiveTabs } else emptyList() if (inactiveTabsList.isNotEmpty() && shouldShowOnboardingForInactiveTabs()) { createInactiveCFR() } diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt index 79c9d94d62..18fb960848 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/NormalBrowserTrayList.kt @@ -11,21 +11,11 @@ import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.tabstray.TabViewHolder import mozilla.components.feature.tabs.tabstray.TabsFeature import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.maxActiveTime import org.mozilla.fenix.ext.settings import org.mozilla.fenix.tabstray.ext.browserAdapter import org.mozilla.fenix.tabstray.ext.inactiveTabsAdapter 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( context: Context, diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt index 5ffc90e7b8..875a62e788 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/browser/TabSorter.kt @@ -10,6 +10,7 @@ import mozilla.components.browser.tabstray.TabsTray import mozilla.components.feature.tabs.tabstray.TabsFeature import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.maxActiveTime import org.mozilla.fenix.ext.toSearchGroup import org.mozilla.fenix.tabstray.ext.browserAdapter import org.mozilla.fenix.tabstray.ext.hasSearchTerm diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt index 8a93a78004..86eba261ea 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/ext/TabSelectors.kt @@ -9,7 +9,7 @@ import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.TabSessionState 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. @@ -32,12 +32,6 @@ fun BrowserState.findPrivateTab(tabId: String): TabSessionState? { return privateTabs.firstOrNull { it.id == tabId } } -/** - * The list of inactive tabs in the tabs tray filtered based on [maxActiveTime]. - */ -val BrowserState.inactiveTabs: List - get() = normalTabs.filter { it.isNormalTabInactive(maxActiveTime) } - /** * The list of normal tabs in the tabs tray filtered appropriately based on feature flags. */ diff --git a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt index dfe8318028..e5b0f3d340 100644 --- a/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/tabstray/viewholders/NormalBrowserPageViewHolder.kt @@ -21,11 +21,11 @@ import org.mozilla.fenix.tabstray.TabsTrayAction import org.mozilla.fenix.tabstray.TabsTrayInteractor import org.mozilla.fenix.tabstray.TabsTrayStore 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.defaultBrowserLayoutColumns 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.inactiveTabsAdapter import org.mozilla.fenix.tabstray.ext.isNormalTabActiveWithSearchTerm @@ -92,7 +92,7 @@ class NormalBrowserPageViewHolder( // Update tabs into the inactive adapter. 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. appStore.dispatch(AppAction.UpdateInactiveExpanded(true)) diff --git a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt index f666258589..cab85ee42c 100644 --- a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt +++ b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt @@ -33,7 +33,7 @@ import org.mozilla.fenix.GleanMetrics.SearchDefaultEngine import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.components.toolbar.ToolbarPosition 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.Settings import org.robolectric.annotation.Config @@ -144,7 +144,7 @@ class FenixApplicationTest { every { application.reportHomeScreenMetrics(settings) } just Runs every { settings.inactiveTabsAreEnabled } returns true 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) diff --git a/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt b/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt index 3b25265a0b..90dfad04a1 100644 --- a/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt +++ b/app/src/test/java/org/mozilla/fenix/ext/BrowserStateTest.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.ext +import io.mockk.every import io.mockk.mockk import mozilla.components.browser.state.state.BrowserState 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 org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test import org.mozilla.fenix.home.recenttabs.RecentTab +import org.mozilla.fenix.utils.Settings class BrowserStateTest { @@ -318,4 +321,96 @@ class BrowserStateTest { ) 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))) + } } diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt index 40b46dffc6..734b9a6252 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/DefaultTabsTrayControllerTest.kt @@ -33,8 +33,8 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.home.HomeFragment -import org.mozilla.fenix.tabstray.browser.maxActiveTime -import org.mozilla.fenix.tabstray.ext.inactiveTabs +import org.mozilla.fenix.ext.maxActiveTime +import org.mozilla.fenix.ext.potentialInactiveTabs class DefaultTabsTrayControllerTest { @MockK(relaxed = true) @@ -409,7 +409,7 @@ class DefaultTabsTrayControllerTest { every { browserStore.state } returns mockk() try { mockkStatic("mozilla.components.browser.state.selector.SelectorsKt") - every { browserStore.state.inactiveTabs } returns listOf(inactiveTab) + every { browserStore.state.potentialInactiveTabs } returns listOf(inactiveTab) controller.handleDeleteAllInactiveTabs() @@ -433,7 +433,7 @@ class DefaultTabsTrayControllerTest { every { browserStore.state } returns mockk() try { mockkStatic("mozilla.components.browser.state.selector.SelectorsKt") - every { browserStore.state.inactiveTabs } returns listOf(inactiveTab) + every { browserStore.state.potentialInactiveTabs } returns listOf(inactiveTab) createController().handleDeleteAllInactiveTabs() diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/ext/TabSessionStateKtTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/ext/TabSessionStateKtTest.kt index 32590fa74f..7959b0071b 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/ext/TabSessionStateKtTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/ext/TabSessionStateKtTest.kt @@ -10,7 +10,7 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before 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 class TabSessionStateKtTest {