Convert recently closed tabs code to use light-weight TabState

To lighten-up our memory usage and startup performance, all of the RecentlyClosed
machinery was converted to use a light-weight TabState - specifically, it's missing
EngineSessionState, which is expensive to obtain during startup, and potentially
very costly to keep in-memory.

When we actually need EngineSessionState (at the point of restoration of a tab), we
read and rehydrate it using provided storage implementation.
upstream-sync
Grigory Kruglov 2 years ago committed by mergify[bot]
parent 48141b25a9
commit a42018f124

@ -38,6 +38,7 @@ import mozilla.components.feature.pwa.ManifestStorage
import mozilla.components.feature.pwa.WebAppShortcutManager import mozilla.components.feature.pwa.WebAppShortcutManager
import mozilla.components.feature.readerview.ReaderViewMiddleware import mozilla.components.feature.readerview.ReaderViewMiddleware
import mozilla.components.feature.recentlyclosed.RecentlyClosedMiddleware import mozilla.components.feature.recentlyclosed.RecentlyClosedMiddleware
import mozilla.components.feature.recentlyclosed.RecentlyClosedTabsStorage
import mozilla.components.feature.search.middleware.AdsTelemetryMiddleware import mozilla.components.feature.search.middleware.AdsTelemetryMiddleware
import mozilla.components.feature.search.middleware.SearchMiddleware import mozilla.components.feature.search.middleware.SearchMiddleware
import mozilla.components.feature.search.region.RegionMiddleware import mozilla.components.feature.search.region.RegionMiddleware
@ -193,7 +194,7 @@ class Core(
val middlewareList = val middlewareList =
mutableListOf( mutableListOf(
LastAccessMiddleware(), LastAccessMiddleware(),
RecentlyClosedMiddleware(context, RECENTLY_CLOSED_MAX, engine), RecentlyClosedMiddleware(recentlyClosedTabsStorage, RECENTLY_CLOSED_MAX),
DownloadMiddleware(context, DownloadService::class.java), DownloadMiddleware(context, DownloadService::class.java),
ReaderViewMiddleware(), ReaderViewMiddleware(),
TelemetryMiddleware( TelemetryMiddleware(
@ -311,6 +312,8 @@ class Core(
*/ */
val lazyRemoteTabsStorage = lazyMonitored { RemoteTabsStorage() } val lazyRemoteTabsStorage = lazyMonitored { RemoteTabsStorage() }
val recentlyClosedTabsStorage = lazyMonitored { RecentlyClosedTabsStorage(context, engine, crashReporter) }
// For most other application code (non-startup), these wrappers are perfectly fine and more ergonomic. // For most other application code (non-startup), these wrappers are perfectly fine and more ergonomic.
val historyStorage: PlacesHistoryStorage get() = lazyHistoryStorage.value val historyStorage: PlacesHistoryStorage get() = lazyHistoryStorage.value
val bookmarksStorage: PlacesBookmarksStorage get() = lazyBookmarksStorage.value val bookmarksStorage: PlacesBookmarksStorage get() = lazyBookmarksStorage.value

@ -8,15 +8,15 @@ import android.view.LayoutInflater
import android.view.ViewGroup import android.view.ViewGroup
import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.ListAdapter import androidx.recyclerview.widget.ListAdapter
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.selection.SelectionHolder
class RecentlyClosedAdapter( class RecentlyClosedAdapter(
private val interactor: RecentlyClosedFragmentInteractor private val interactor: RecentlyClosedFragmentInteractor
) : ListAdapter<RecoverableTab, RecentlyClosedItemViewHolder>(DiffCallback), ) : ListAdapter<TabState, RecentlyClosedItemViewHolder>(DiffCallback),
SelectionHolder<RecoverableTab> { SelectionHolder<TabState> {
private var selectedTabs: Set<RecoverableTab> = emptySet() private var selectedTabs: Set<TabState> = emptySet()
override fun onCreateViewHolder( override fun onCreateViewHolder(
parent: ViewGroup, parent: ViewGroup,
@ -31,20 +31,20 @@ class RecentlyClosedAdapter(
holder.bind(getItem(position)) holder.bind(getItem(position))
} }
override val selectedItems: Set<RecoverableTab> override val selectedItems: Set<TabState>
get() = selectedTabs get() = selectedTabs
fun updateData(tabs: List<RecoverableTab>, selectedTabs: Set<RecoverableTab>) { fun updateData(tabs: List<TabState>, selectedTabs: Set<TabState>) {
this.selectedTabs = selectedTabs this.selectedTabs = selectedTabs
notifyItemRangeChanged(0, tabs.size) notifyItemRangeChanged(0, tabs.size)
submitList(tabs) submitList(tabs)
} }
private object DiffCallback : DiffUtil.ItemCallback<RecoverableTab>() { private object DiffCallback : DiffUtil.ItemCallback<TabState>() {
override fun areItemsTheSame(oldItem: RecoverableTab, newItem: RecoverableTab) = override fun areItemsTheSame(oldItem: TabState, newItem: TabState) =
oldItem.id == newItem.id oldItem.id == newItem.id
override fun areContentsTheSame(oldItem: RecoverableTab, newItem: RecoverableTab) = override fun areContentsTheSame(oldItem: TabState, newItem: TabState) =
oldItem == newItem oldItem == newItem
} }
} }

@ -6,10 +6,13 @@ package org.mozilla.fenix.library.recentlyclosed
import androidx.navigation.NavController import androidx.navigation.NavController
import androidx.navigation.NavOptions import androidx.navigation.NavOptions
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import mozilla.components.browser.state.action.RecentlyClosedAction import mozilla.components.browser.state.action.RecentlyClosedAction
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.feature.recentlyclosed.RecentlyClosedTabsStorage
import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.feature.tabs.TabsUseCases
import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.HomeActivity
@ -20,15 +23,15 @@ import org.mozilla.fenix.components.metrics.MetricController
@Suppress("TooManyFunctions") @Suppress("TooManyFunctions")
interface RecentlyClosedController { interface RecentlyClosedController {
fun handleOpen(tab: RecoverableTab, mode: BrowsingMode? = null) fun handleOpen(tab: TabState, mode: BrowsingMode? = null)
fun handleOpen(tabs: Set<RecoverableTab>, mode: BrowsingMode? = null) fun handleOpen(tabs: Set<TabState>, mode: BrowsingMode? = null)
fun handleDelete(tab: RecoverableTab) fun handleDelete(tab: TabState)
fun handleDelete(tabs: Set<RecoverableTab>) fun handleDelete(tabs: Set<TabState>)
fun handleShare(tabs: Set<RecoverableTab>) fun handleShare(tabs: Set<TabState>)
fun handleNavigateToHistory() fun handleNavigateToHistory()
fun handleRestore(item: RecoverableTab) fun handleRestore(item: TabState)
fun handleSelect(tab: RecoverableTab) fun handleSelect(tab: TabState)
fun handleDeselect(tab: RecoverableTab) fun handleDeselect(tab: TabState)
fun handleBackPressed(): Boolean fun handleBackPressed(): Boolean
} }
@ -37,16 +40,18 @@ class DefaultRecentlyClosedController(
private val navController: NavController, private val navController: NavController,
private val browserStore: BrowserStore, private val browserStore: BrowserStore,
private val recentlyClosedStore: RecentlyClosedFragmentStore, private val recentlyClosedStore: RecentlyClosedFragmentStore,
private val recentlyClosedTabsStorage: RecentlyClosedTabsStorage,
private val tabsUseCases: TabsUseCases, private val tabsUseCases: TabsUseCases,
private val activity: HomeActivity, private val activity: HomeActivity,
private val metrics: MetricController, private val metrics: MetricController,
private val openToBrowser: (item: RecoverableTab, mode: BrowsingMode?) -> Unit private val lifecycleScope: CoroutineScope,
private val openToBrowser: (url: String, mode: BrowsingMode?) -> Unit
) : RecentlyClosedController { ) : RecentlyClosedController {
override fun handleOpen(tab: RecoverableTab, mode: BrowsingMode?) { override fun handleOpen(tab: TabState, mode: BrowsingMode?) {
openToBrowser(tab, mode) openToBrowser(tab.url, mode)
} }
override fun handleOpen(tabs: Set<RecoverableTab>, mode: BrowsingMode?) { override fun handleOpen(tabs: Set<TabState>, mode: BrowsingMode?) {
if (mode == BrowsingMode.Normal) { if (mode == BrowsingMode.Normal) {
metrics.track(Event.RecentlyClosedTabsMenuOpenInNormalTab) metrics.track(Event.RecentlyClosedTabsMenuOpenInNormalTab)
} else if (mode == BrowsingMode.Private) { } else if (mode == BrowsingMode.Private) {
@ -56,26 +61,26 @@ class DefaultRecentlyClosedController(
tabs.forEach { tab -> handleOpen(tab, mode) } tabs.forEach { tab -> handleOpen(tab, mode) }
} }
override fun handleSelect(tab: RecoverableTab) { override fun handleSelect(tab: TabState) {
if (recentlyClosedStore.state.selectedTabs.isEmpty()) { if (recentlyClosedStore.state.selectedTabs.isEmpty()) {
metrics.track(Event.RecentlyClosedTabsEnterMultiselect) metrics.track(Event.RecentlyClosedTabsEnterMultiselect)
} }
recentlyClosedStore.dispatch(RecentlyClosedFragmentAction.Select(tab)) recentlyClosedStore.dispatch(RecentlyClosedFragmentAction.Select(tab))
} }
override fun handleDeselect(tab: RecoverableTab) { override fun handleDeselect(tab: TabState) {
if (recentlyClosedStore.state.selectedTabs.size == 1) { if (recentlyClosedStore.state.selectedTabs.size == 1) {
metrics.track(Event.RecentlyClosedTabsExitMultiselect) metrics.track(Event.RecentlyClosedTabsExitMultiselect)
} }
recentlyClosedStore.dispatch(RecentlyClosedFragmentAction.Deselect(tab)) recentlyClosedStore.dispatch(RecentlyClosedFragmentAction.Deselect(tab))
} }
override fun handleDelete(tab: RecoverableTab) { override fun handleDelete(tab: TabState) {
metrics.track(Event.RecentlyClosedTabsDeleteTab) metrics.track(Event.RecentlyClosedTabsDeleteTab)
browserStore.dispatch(RecentlyClosedAction.RemoveClosedTabAction(tab)) browserStore.dispatch(RecentlyClosedAction.RemoveClosedTabAction(tab))
} }
override fun handleDelete(tabs: Set<RecoverableTab>) { override fun handleDelete(tabs: Set<TabState>) {
metrics.track(Event.RecentlyClosedTabsMenuDelete) metrics.track(Event.RecentlyClosedTabsMenuDelete)
recentlyClosedStore.dispatch(RecentlyClosedFragmentAction.DeselectAll) recentlyClosedStore.dispatch(RecentlyClosedFragmentAction.DeselectAll)
tabs.forEach { tab -> tabs.forEach { tab ->
@ -91,7 +96,7 @@ class DefaultRecentlyClosedController(
) )
} }
override fun handleShare(tabs: Set<RecoverableTab>) { override fun handleShare(tabs: Set<TabState>) {
metrics.track(Event.RecentlyClosedTabsMenuShare) metrics.track(Event.RecentlyClosedTabsMenuShare)
val shareData = tabs.map { ShareData(url = it.url, title = it.title) } val shareData = tabs.map { ShareData(url = it.url, title = it.title) }
navController.navigate( navController.navigate(
@ -101,18 +106,19 @@ class DefaultRecentlyClosedController(
) )
} }
override fun handleRestore(item: RecoverableTab) { override fun handleRestore(item: TabState) {
metrics.track(Event.RecentlyClosedTabsOpenTab) lifecycleScope.launch {
metrics.track(Event.RecentlyClosedTabsOpenTab)
tabsUseCases.restore(item, recentlyClosedTabsStorage.engineStateStorage())
tabsUseCases.restore(item) browserStore.dispatch(
RecentlyClosedAction.RemoveClosedTabAction(item)
browserStore.dispatch( )
RecentlyClosedAction.RemoveClosedTabAction(item)
)
activity.openToBrowser( activity.openToBrowser(
from = BrowserDirection.FromRecentlyClosed from = BrowserDirection.FromRecentlyClosed
) )
}
} }
override fun handleBackPressed(): Boolean { override fun handleBackPressed(): Boolean {

@ -12,6 +12,7 @@ import android.view.MenuInflater
import android.view.MenuItem import android.view.MenuItem
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.findNavController
import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
@ -120,6 +121,8 @@ class RecentlyClosedFragment : LibraryPageFragment<RecoverableTab>(), UserIntera
activity = activity as HomeActivity, activity = activity as HomeActivity,
tabsUseCases = requireComponents.useCases.tabsUseCases, tabsUseCases = requireComponents.useCases.tabsUseCases,
metrics = metrics, metrics = metrics,
recentlyClosedTabsStorage = requireComponents.core.recentlyClosedTabsStorage.value,
lifecycleScope = lifecycleScope,
openToBrowser = ::openItem openToBrowser = ::openItem
) )
recentlyClosedInteractor = RecentlyClosedFragmentInteractor(recentlyClosedController) recentlyClosedInteractor = RecentlyClosedFragmentInteractor(recentlyClosedController)
@ -135,11 +138,11 @@ class RecentlyClosedFragment : LibraryPageFragment<RecoverableTab>(), UserIntera
_recentlyClosedFragmentView = null _recentlyClosedFragmentView = null
} }
private fun openItem(tab: RecoverableTab, mode: BrowsingMode? = null) { private fun openItem(url: String, mode: BrowsingMode? = null) {
mode?.let { (activity as HomeActivity).browsingModeManager.mode = it } mode?.let { (activity as HomeActivity).browsingModeManager.mode = it }
(activity as HomeActivity).openToBrowserAndLoad( (activity as HomeActivity).openToBrowserAndLoad(
searchTermOrURL = tab.url, searchTermOrURL = url,
newTab = true, newTab = true,
from = BrowserDirection.FromRecentlyClosed from = BrowserDirection.FromRecentlyClosed
) )

@ -4,7 +4,7 @@
package org.mozilla.fenix.library.recentlyclosed package org.mozilla.fenix.library.recentlyclosed
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
/** /**
* Interactor for the recently closed screen * Interactor for the recently closed screen
@ -14,7 +14,7 @@ class RecentlyClosedFragmentInteractor(
private val recentlyClosedController: RecentlyClosedController private val recentlyClosedController: RecentlyClosedController
) : RecentlyClosedInteractor { ) : RecentlyClosedInteractor {
override fun onDelete(tab: RecoverableTab) { override fun onDelete(tab: TabState) {
recentlyClosedController.handleDelete(tab) recentlyClosedController.handleDelete(tab)
} }
@ -22,15 +22,15 @@ class RecentlyClosedFragmentInteractor(
recentlyClosedController.handleNavigateToHistory() recentlyClosedController.handleNavigateToHistory()
} }
override fun open(item: RecoverableTab) { override fun open(item: TabState) {
recentlyClosedController.handleRestore(item) recentlyClosedController.handleRestore(item)
} }
override fun select(item: RecoverableTab) { override fun select(item: TabState) {
recentlyClosedController.handleSelect(item) recentlyClosedController.handleSelect(item)
} }
override fun deselect(item: RecoverableTab) { override fun deselect(item: TabState) {
recentlyClosedController.handleDeselect(item) recentlyClosedController.handleDeselect(item)
} }
} }

@ -4,7 +4,7 @@
package org.mozilla.fenix.library.recentlyclosed package org.mozilla.fenix.library.recentlyclosed
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import mozilla.components.lib.state.Action import mozilla.components.lib.state.Action
import mozilla.components.lib.state.State import mozilla.components.lib.state.State
import mozilla.components.lib.state.Store import mozilla.components.lib.state.Store
@ -23,9 +23,9 @@ class RecentlyClosedFragmentStore(initialState: RecentlyClosedFragmentState) :
* `RecentlyClosedFragmentState` through the reducer. * `RecentlyClosedFragmentState` through the reducer.
*/ */
sealed class RecentlyClosedFragmentAction : Action { sealed class RecentlyClosedFragmentAction : Action {
data class Change(val list: List<RecoverableTab>) : RecentlyClosedFragmentAction() data class Change(val list: List<TabState>) : RecentlyClosedFragmentAction()
data class Select(val tab: RecoverableTab) : RecentlyClosedFragmentAction() data class Select(val tab: TabState) : RecentlyClosedFragmentAction()
data class Deselect(val tab: RecoverableTab) : RecentlyClosedFragmentAction() data class Deselect(val tab: TabState) : RecentlyClosedFragmentAction()
object DeselectAll : RecentlyClosedFragmentAction() object DeselectAll : RecentlyClosedFragmentAction()
} }
@ -34,8 +34,8 @@ sealed class RecentlyClosedFragmentAction : Action {
* @property items List of recently closed tabs to display * @property items List of recently closed tabs to display
*/ */
data class RecentlyClosedFragmentState( data class RecentlyClosedFragmentState(
val items: List<RecoverableTab> = emptyList(), val items: List<TabState> = emptyList(),
val selectedTabs: Set<RecoverableTab> val selectedTabs: Set<TabState>
) : State ) : State
/** /**

@ -10,13 +10,13 @@ import androidx.appcompat.content.res.AppCompatResources
import androidx.core.view.isVisible import androidx.core.view.isVisible
import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.SimpleItemAnimator import androidx.recyclerview.widget.SimpleItemAnimator
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.databinding.ComponentRecentlyClosedBinding import org.mozilla.fenix.databinding.ComponentRecentlyClosedBinding
import org.mozilla.fenix.library.LibraryPageView import org.mozilla.fenix.library.LibraryPageView
import org.mozilla.fenix.selection.SelectionInteractor import org.mozilla.fenix.selection.SelectionInteractor
interface RecentlyClosedInteractor : SelectionInteractor<RecoverableTab> { interface RecentlyClosedInteractor : SelectionInteractor<TabState> {
/** /**
* Called when the view more history option is tapped. * Called when the view more history option is tapped.
*/ */
@ -27,7 +27,7 @@ interface RecentlyClosedInteractor : SelectionInteractor<RecoverableTab> {
* *
* @param tab the recently closed tab to delete. * @param tab the recently closed tab to delete.
*/ */
fun onDelete(tab: RecoverableTab) fun onDelete(tab: TabState)
} }
/** /**

@ -6,7 +6,7 @@ package org.mozilla.fenix.library.recentlyclosed
import android.view.View import android.view.View
import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.databinding.HistoryListItemBinding import org.mozilla.fenix.databinding.HistoryListItemBinding
import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.hideAndDisable
@ -16,12 +16,12 @@ import org.mozilla.fenix.selection.SelectionHolder
class RecentlyClosedItemViewHolder( class RecentlyClosedItemViewHolder(
view: View, view: View,
private val recentlyClosedFragmentInteractor: RecentlyClosedFragmentInteractor, private val recentlyClosedFragmentInteractor: RecentlyClosedFragmentInteractor,
private val selectionHolder: SelectionHolder<RecoverableTab>, private val selectionHolder: SelectionHolder<TabState>,
) : RecyclerView.ViewHolder(view) { ) : RecyclerView.ViewHolder(view) {
private val binding = HistoryListItemBinding.bind(view) private val binding = HistoryListItemBinding.bind(view)
private var item: RecoverableTab? = null private var item: TabState? = null
init { init {
binding.historyLayout.overflowView.apply { binding.historyLayout.overflowView.apply {
@ -34,7 +34,7 @@ class RecentlyClosedItemViewHolder(
} }
} }
fun bind(item: RecoverableTab) { fun bind(item: TabState) {
binding.historyLayout.titleView.text = binding.historyLayout.titleView.text =
item.title.ifEmpty { item.url } item.title.ifEmpty { item.url }
binding.historyLayout.urlView.text = item.url binding.historyLayout.urlView.text = item.url

@ -45,9 +45,9 @@ class SearchTermTabGroupMiddleware : Middleware<BrowserState, BrowserAction> {
} }
is TabListAction.RestoreAction -> { is TabListAction.RestoreAction -> {
action.tabs.forEach { tab -> action.tabs.forEach { tab ->
tab.historyMetadata?.searchTerm?.let { searchTerm -> tab.state.historyMetadata?.searchTerm?.let { searchTerm ->
context.dispatch( context.dispatch(
TabGroupAction.AddTabAction(SEARCH_TERM_TAB_GROUPS, searchTerm, tab.id) TabGroupAction.AddTabAction(SEARCH_TERM_TAB_GROUPS, searchTerm, tab.state.id)
) )
} }
} }

@ -68,11 +68,11 @@ class TelemetryLifecycleObserver(
crashedTabs = crashedTabs crashedTabs = crashedTabs
) )
} }
}
private data class TabState( private data class TabState(
val timestamp: Long = Clock.elapsedRealtime(), val timestamp: Long = Clock.elapsedRealtime(),
val totalTabs: Int, val totalTabs: Int,
val crashedTabs: Int, val crashedTabs: Int,
val activeEngineTabs: Int val activeEngineTabs: Int
) )
}

@ -25,6 +25,7 @@ import mozilla.components.browser.state.state.ReaderState
import mozilla.components.browser.state.state.SearchState import mozilla.components.browser.state.state.SearchState
import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.RecoverableTab
import mozilla.components.browser.state.state.recover.TabState
import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.Engine
@ -212,22 +213,24 @@ class DefaultSessionControlControllerTest {
@Test @Test
fun `handleCollectionOpenTabClicked with existing selected tab`() { fun `handleCollectionOpenTabClicked with existing selected tab`() {
val recoverableTab = RecoverableTab( val recoverableTab = RecoverableTab(
id = "test", engineSessionState = null,
parentId = null, state = TabState(
url = "https://www.mozilla.org", id = "test",
title = "Mozilla", parentId = null,
state = null, url = "https://www.mozilla.org",
contextId = null, title = "Mozilla",
readerState = ReaderState(), contextId = null,
lastAccess = 0, readerState = ReaderState(),
private = false lastAccess = 0,
private = false
)
) )
val tab = mockk<ComponentTab> { val tab = mockk<ComponentTab> {
every { restore(activity, engine, restoreSessionId = false) } returns recoverableTab every { restore(activity, engine, restoreSessionId = false) } returns recoverableTab
} }
val restoredTab = createTab(id = recoverableTab.id, url = recoverableTab.url) val restoredTab = createTab(id = recoverableTab.state.id, url = recoverableTab.state.url)
val otherTab = createTab(id = "otherTab", url = "https://mozilla.org") val otherTab = createTab(id = "otherTab", url = "https://mozilla.org")
store.dispatch(TabListAction.AddTabAction(otherTab)).joinBlocking() store.dispatch(TabListAction.AddTabAction(otherTab)).joinBlocking()
store.dispatch(TabListAction.SelectTabAction(otherTab.id)).joinBlocking() store.dispatch(TabListAction.SelectTabAction(otherTab.id)).joinBlocking()
@ -243,22 +246,24 @@ class DefaultSessionControlControllerTest {
@Test @Test
fun `handleCollectionOpenTabClicked without existing selected tab`() { fun `handleCollectionOpenTabClicked without existing selected tab`() {
val recoverableTab = RecoverableTab( val recoverableTab = RecoverableTab(
id = "test", engineSessionState = null,
parentId = null, state = TabState(
url = "https://www.mozilla.org", id = "test",
title = "Mozilla", parentId = null,
state = null, url = "https://www.mozilla.org",
contextId = null, title = "Mozilla",
readerState = ReaderState(), contextId = null,
lastAccess = 0, readerState = ReaderState(),
private = false lastAccess = 0,
private = false
)
) )
val tab = mockk<ComponentTab> { val tab = mockk<ComponentTab> {
every { restore(activity, engine, restoreSessionId = false) } returns recoverableTab every { restore(activity, engine, restoreSessionId = false) } returns recoverableTab
} }
val restoredTab = createTab(id = recoverableTab.id, url = recoverableTab.url) val restoredTab = createTab(id = recoverableTab.state.id, url = recoverableTab.state.url)
store.dispatch(TabListAction.AddTabAction(restoredTab)).joinBlocking() store.dispatch(TabListAction.AddTabAction(restoredTab)).joinBlocking()
createController().handleCollectionOpenTabClicked(tab) createController().handleCollectionOpenTabClicked(tab)

@ -6,20 +6,25 @@ package org.mozilla.fenix.library.recentlyclosed
import androidx.navigation.NavController import androidx.navigation.NavController
import androidx.navigation.NavOptions import androidx.navigation.NavOptions
import io.mockk.Runs import io.mockk.mockk
import io.mockk.coEvery
import io.mockk.verifyAll
import io.mockk.clearMocks import io.mockk.clearMocks
import io.mockk.every import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.verify import io.mockk.verify
import io.mockk.verifyAll import io.mockk.coVerify
import kotlinx.coroutines.test.TestCoroutineDispatcher import io.mockk.just
import io.mockk.Runs
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
import mozilla.components.browser.state.action.RecentlyClosedAction import mozilla.components.browser.state.action.RecentlyClosedAction
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.feature.recentlyclosed.RecentlyClosedTabsStorage
import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.feature.tabs.TabsUseCases
import org.junit.After import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
@ -35,7 +40,6 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@RunWith(FenixRobolectricTestRunner::class) @RunWith(FenixRobolectricTestRunner::class)
class DefaultRecentlyClosedControllerTest { class DefaultRecentlyClosedControllerTest {
private val dispatcher = TestCoroutineDispatcher()
private val navController: NavController = mockk(relaxed = true) private val navController: NavController = mockk(relaxed = true)
private val activity: HomeActivity = mockk(relaxed = true) private val activity: HomeActivity = mockk(relaxed = true)
private val browserStore: BrowserStore = mockk(relaxed = true) private val browserStore: BrowserStore = mockk(relaxed = true)
@ -45,39 +49,34 @@ class DefaultRecentlyClosedControllerTest {
@Before @Before
fun setUp() { fun setUp() {
every { tabsUseCases.restore.invoke(any(), true) } just Runs coEvery { tabsUseCases.restore.invoke(any(), any(), true) } just Runs
}
@After
fun tearDown() {
dispatcher.cleanupTestCoroutines()
} }
@Test @Test
fun handleOpen() { fun handleOpen() {
val item: RecoverableTab = mockk(relaxed = true) val item: TabState = mockk(relaxed = true)
var actualtab: RecoverableTab? = null var tabUrl: String? = null
var actualBrowsingMode: BrowsingMode? = null var actualBrowsingMode: BrowsingMode? = null
val controller = createController( val controller = createController(
openToBrowser = { tab, browsingMode -> openToBrowser = { url, browsingMode ->
actualtab = tab tabUrl = url
actualBrowsingMode = browsingMode actualBrowsingMode = browsingMode
} }
) )
controller.handleOpen(item, BrowsingMode.Private) controller.handleOpen(item, BrowsingMode.Private)
assertEquals(item, actualtab) assertEquals(item.url, tabUrl)
assertEquals(actualBrowsingMode, BrowsingMode.Private) assertEquals(actualBrowsingMode, BrowsingMode.Private)
actualtab = null tabUrl = null
actualBrowsingMode = null actualBrowsingMode = null
controller.handleOpen(item, BrowsingMode.Normal) controller.handleOpen(item, BrowsingMode.Normal)
assertEquals(item, actualtab) assertEquals(item.url, tabUrl)
assertEquals(actualBrowsingMode, BrowsingMode.Normal) assertEquals(actualBrowsingMode, BrowsingMode.Normal)
} }
@ -85,34 +84,34 @@ class DefaultRecentlyClosedControllerTest {
fun `open multiple tabs`() { fun `open multiple tabs`() {
val tabs = createFakeTabList(2) val tabs = createFakeTabList(2)
val actualTabs = mutableListOf<RecoverableTab>() val tabUrls = mutableListOf<String>()
val actualBrowsingModes = mutableListOf<BrowsingMode?>() val actualBrowsingModes = mutableListOf<BrowsingMode?>()
val controller = createController( val controller = createController(
openToBrowser = { tab, mode -> openToBrowser = { url, mode ->
actualTabs.add(tab) tabUrls.add(url)
actualBrowsingModes.add(mode) actualBrowsingModes.add(mode)
} }
) )
controller.handleOpen(tabs.toSet(), BrowsingMode.Normal) controller.handleOpen(tabs.toSet(), BrowsingMode.Normal)
assertEquals(2, actualTabs.size) assertEquals(2, tabUrls.size)
assertEquals(tabs[0], actualTabs[0]) assertEquals(tabs[0].url, tabUrls[0])
assertEquals(tabs[1], actualTabs[1]) assertEquals(tabs[1].url, tabUrls[1])
assertEquals(BrowsingMode.Normal, actualBrowsingModes[0]) assertEquals(BrowsingMode.Normal, actualBrowsingModes[0])
assertEquals(BrowsingMode.Normal, actualBrowsingModes[1]) assertEquals(BrowsingMode.Normal, actualBrowsingModes[1])
verifyAll { metrics.track(Event.RecentlyClosedTabsMenuOpenInNormalTab) } verifyAll { metrics.track(Event.RecentlyClosedTabsMenuOpenInNormalTab) }
clearMocks(metrics) clearMocks(metrics)
actualTabs.clear() tabUrls.clear()
actualBrowsingModes.clear() actualBrowsingModes.clear()
controller.handleOpen(tabs.toSet(), BrowsingMode.Private) controller.handleOpen(tabs.toSet(), BrowsingMode.Private)
assertEquals(2, actualTabs.size) assertEquals(2, tabUrls.size)
assertEquals(tabs[0], actualTabs[0]) assertEquals(tabs[0].url, tabUrls[0])
assertEquals(tabs[1], actualTabs[1]) assertEquals(tabs[1].url, tabUrls[1])
assertEquals(BrowsingMode.Private, actualBrowsingModes[0]) assertEquals(BrowsingMode.Private, actualBrowsingModes[0])
assertEquals(BrowsingMode.Private, actualBrowsingModes[1]) assertEquals(BrowsingMode.Private, actualBrowsingModes[1])
verifyAll { metrics.track(Event.RecentlyClosedTabsMenuOpenInPrivateTab) } verifyAll { metrics.track(Event.RecentlyClosedTabsMenuOpenInPrivateTab) }
@ -164,7 +163,7 @@ class DefaultRecentlyClosedControllerTest {
@Test @Test
fun handleDelete() { fun handleDelete() {
val item: RecoverableTab = mockk(relaxed = true) val item: TabState = mockk(relaxed = true)
createController().handleDelete(item) createController().handleDelete(item)
@ -221,14 +220,12 @@ class DefaultRecentlyClosedControllerTest {
} }
@Test @Test
fun handleRestore() { fun handleRestore() = runBlocking {
val item: RecoverableTab = mockk(relaxed = true) val item: TabState = mockk(relaxed = true)
createController().handleRestore(item)
dispatcher.advanceUntilIdle() createController(scope = this).handleRestore(item)
verify { tabsUseCases.restore.invoke(item, true) } coVerify { tabsUseCases.restore.invoke(eq(item), any(), true) }
verify { metrics.track(Event.RecentlyClosedTabsOpenTab) } verify { metrics.track(Event.RecentlyClosedTabsOpenTab) }
} }
@ -253,24 +250,27 @@ class DefaultRecentlyClosedControllerTest {
} }
private fun createController( private fun createController(
openToBrowser: (RecoverableTab, BrowsingMode?) -> Unit = { _, _ -> } scope: CoroutineScope = CoroutineScope(Dispatchers.IO),
openToBrowser: (String, BrowsingMode?) -> Unit = { _, _ -> },
): RecentlyClosedController { ): RecentlyClosedController {
return DefaultRecentlyClosedController( return DefaultRecentlyClosedController(
navController, navController,
browserStore, browserStore,
recentlyClosedStore, recentlyClosedStore,
RecentlyClosedTabsStorage(testContext, mockk(), mockk()),
tabsUseCases, tabsUseCases,
activity, activity,
metrics, metrics,
scope,
openToBrowser openToBrowser
) )
} }
private fun createFakeTab(id: String = "FakeId", url: String = "www.fake.com"): RecoverableTab = private fun createFakeTab(id: String = "FakeId", url: String = "www.fake.com"): TabState =
RecoverableTab(id, url) TabState(id, url)
private fun createFakeTabList(size: Int): List<RecoverableTab> { private fun createFakeTabList(size: Int): List<TabState> {
val fakeTabs = mutableListOf<RecoverableTab>() val fakeTabs = mutableListOf<TabState>()
for (i in 0 until size) { for (i in 0 until size) {
fakeTabs.add(createFakeTab(id = "FakeId$i")) fakeTabs.add(createFakeTab(id = "FakeId$i"))
} }

@ -6,7 +6,7 @@ package org.mozilla.fenix.library.recentlyclosed
import io.mockk.mockk import io.mockk.mockk
import io.mockk.verify import io.mockk.verify
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.TabState
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
@ -26,7 +26,7 @@ class RecentlyClosedFragmentInteractorTest {
@Test @Test
fun onDelete() { fun onDelete() {
val tab = RecoverableTab(id = "tab-id", title = "Mozilla", url = "mozilla.org", lastAccess = 1L) val tab = TabState(id = "tab-id", title = "Mozilla", url = "mozilla.org", lastAccess = 1L)
interactor.onDelete(tab) interactor.onDelete(tab)
verify { verify {

@ -16,6 +16,7 @@ import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabGroup import mozilla.components.browser.state.state.TabGroup
import mozilla.components.browser.state.state.TabPartition import mozilla.components.browser.state.state.TabPartition
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.RecoverableTab
import mozilla.components.browser.state.state.recover.TabState
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.storage.HistoryMetadataKey import mozilla.components.concept.storage.HistoryMetadataKey
import mozilla.components.lib.state.MiddlewareContext import mozilla.components.lib.state.MiddlewareContext
@ -86,9 +87,12 @@ class SearchTermTabGroupMiddlewareTest {
TabListAction.RestoreAction( TabListAction.RestoreAction(
listOf( listOf(
RecoverableTab( RecoverableTab(
id = "testId", engineSessionState = null,
url = "url", state = TabState(
historyMetadata = HistoryMetadataKey("url", "search term", "url") id = "testId",
url = "url",
historyMetadata = HistoryMetadataKey("url", "search term", "url")
)
) )
), ),
restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING

@ -14,6 +14,7 @@ import mozilla.components.browser.state.engine.EngineMiddleware
import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.recover.RecoverableTab
import mozilla.components.browser.state.state.recover.TabState
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.service.glean.testing.GleanTestRule import mozilla.components.service.glean.testing.GleanTestRule
import mozilla.components.support.base.android.Clock import mozilla.components.support.base.android.Clock
@ -165,8 +166,8 @@ class TelemetryMiddlewareTest {
fun `WHEN tabs are restored THEN the open tab count is updated`() { fun `WHEN tabs are restored THEN the open tab count is updated`() {
assertEquals(0, settings.openTabsCount) assertEquals(0, settings.openTabsCount)
val tabsToRestore = listOf( val tabsToRestore = listOf(
RecoverableTab(url = "https://mozilla.org", id = "1"), RecoverableTab(null, TabState(url = "https://mozilla.org", id = "1")),
RecoverableTab(url = "https://firefox.com", id = "2") RecoverableTab(null, TabState(url = "https://firefox.com", id = "2"))
) )
store.dispatch( store.dispatch(
@ -206,9 +207,9 @@ class TelemetryMiddlewareTest {
store.dispatch( store.dispatch(
TabListAction.RestoreAction( TabListAction.RestoreAction(
listOf( listOf(
RecoverableTab(url = "https://www.mozilla.org", id = "foreground"), RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")),
RecoverableTab(url = "https://getpocket.com", id = "background_pocket"), RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket")),
RecoverableTab(url = "https://theverge.com", id = "background_verge") RecoverableTab(null, TabState(url = "https://theverge.com", id = "background_verge"))
), ),
selectedTabId = "foreground", selectedTabId = "foreground",
restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING
@ -230,9 +231,9 @@ class TelemetryMiddlewareTest {
store.dispatch( store.dispatch(
TabListAction.RestoreAction( TabListAction.RestoreAction(
listOf( listOf(
RecoverableTab(url = "https://www.mozilla.org", id = "foreground"), RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")),
RecoverableTab(url = "https://getpocket.com", id = "background_pocket"), RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket")),
RecoverableTab(url = "https://theverge.com", id = "background_verge") RecoverableTab(null, TabState(url = "https://theverge.com", id = "background_verge"))
), ),
selectedTabId = "foreground", selectedTabId = "foreground",
restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING
@ -264,9 +265,9 @@ class TelemetryMiddlewareTest {
store.dispatch( store.dispatch(
TabListAction.RestoreAction( TabListAction.RestoreAction(
listOf( listOf(
RecoverableTab(url = "https://www.mozilla.org", id = "foreground"), RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")),
RecoverableTab(url = "https://getpocket.com", id = "background_pocket"), RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket")),
RecoverableTab(url = "https://theverge.com", id = "background_verge") RecoverableTab(null, TabState(url = "https://theverge.com", id = "background_verge"))
), ),
selectedTabId = "foreground", selectedTabId = "foreground",
restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING
@ -301,9 +302,9 @@ class TelemetryMiddlewareTest {
store.dispatch( store.dispatch(
TabListAction.RestoreAction( TabListAction.RestoreAction(
listOf( listOf(
RecoverableTab(url = "https://www.mozilla.org", id = "foreground"), RecoverableTab(null, TabState(url = "https://www.mozilla.org", id = "foreground")),
RecoverableTab(url = "https://getpocket.com", id = "background_pocket"), RecoverableTab(null, TabState(url = "https://getpocket.com", id = "background_pocket")),
RecoverableTab(url = "https://theverge.com", id = "background_verge") RecoverableTab(null, TabState(url = "https://theverge.com", id = "background_verge"))
), ),
selectedTabId = "foreground", selectedTabId = "foreground",
restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING restoreLocation = TabListAction.RestoreAction.RestoreLocation.BEGINNING

Loading…
Cancel
Save