Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Merge #7171
Browse files Browse the repository at this point in the history 
7171: Issue 7170: Make ThumbnailsUseCases optional in TabsFeature and TabsTrayPresenter r=jonalmeida a=gabrielluong




Co-authored-by: Gabriel Luong <[email protected]>
  • Loading branch information
@MozLando @gabrielluong
MozLando and gabrielluong committed May 29, 2020
2 parents ecb7881 + b674ac1 commit 2a18bde
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 71 deletions.
2 changes: 1 addition & 1 deletion ...onents/feature/tabs/src/main/java/mozilla/components/feature/tabs/tabstray/TabsFeature.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class TabsFeature(
tabsTray: TabsTray,
private val store: BrowserStore,
tabsUseCases: TabsUseCases,
thumbnailsUseCases: ThumbnailsUseCases,
thumbnailsUseCases: ThumbnailsUseCases? = null,
private val defaultTabsFilter: (TabSessionState) -> Boolean = { true },
closeTabsTray: () -> Unit
) : LifecycleAwareFeature {
Expand Down
14 changes: 9 additions & 5 deletions .../feature/tabs/src/main/java/mozilla/components/feature/tabs/tabstray/TabsTrayPresenter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
class TabsTrayPresenter(
private val tabsTray: TabsTray,
private val store: BrowserStore,
private val thumbnailsUseCases: ThumbnailsUseCases,
private val thumbnailsUseCases: ThumbnailsUseCases? = null,
internal var tabsFilter: (TabSessionState) -> Boolean,
private val closeTabsTray: () -> Unit
) {
Expand All @@ -46,10 +46,14 @@ class TabsTrayPresenter(
private suspend fun collect(flow: Flow<BrowserState>) {
flow.map { it.toTabs(tabsFilter) }
.map { tabs ->
// Load the tab thumbnail from the memory or disk caches.
tabs.copy(list = tabs.list.map { tab ->
tab.copy(thumbnail = thumbnailsUseCases.loadThumbnail(tab.id))
})
if (thumbnailsUseCases != null) {
// Load the tab thumbnail from the memory or disk caches.
tabs.copy(list = tabs.list.map { tab ->
tab.copy(thumbnail = thumbnailsUseCases.loadThumbnail(tab.id))
})
} else {
tabs
}
}
.ifChanged()
.collect { tabs ->
Expand Down
9 changes: 8 additions & 1 deletion ...ts/feature/tabs/src/test/java/mozilla/components/feature/tabs/tabstray/TabsFeatureTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ class TabsFeatureTest {
val filter: (TabSessionState) -> Boolean = { false }
val sessionManager = SessionManager(engine = mock())
val useCases = TabsUseCases(sessionManager)
val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock(), filter, mock()))
val tabsFeature = spy(TabsFeature(
mock(),
store,
useCases,
mock(),
defaultTabsFilter = filter,
closeTabsTray = mock()
))
val presenter: TabsTrayPresenter = mock()
val interactor: TabsTrayInteractor = mock()

Expand Down
147 changes: 85 additions & 62 deletions ...ture/tabs/src/test/java/mozilla/components/feature/tabs/tabstray/TabsTrayPresenterTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,12 @@ import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.MediaState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.browser.thumbnails.ThumbnailsUseCases
import mozilla.components.browser.thumbnails.storage.ThumbnailStorage
import mozilla.components.concept.tabstray.Tabs
import mozilla.components.concept.tabstray.TabsTray
import mozilla.components.feature.tabs.ext.toTabs
import mozilla.components.support.test.any
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
Expand Down Expand Up @@ -61,16 +58,20 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

verifyNoMoreInteractions(tabsTray)

Expand All @@ -93,16 +94,20 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

presenter.start()

Expand All @@ -112,7 +117,7 @@ class TabsTrayPresenterTest {

store.dispatch(
TabListAction.AddTabAction(
createTab("https://developer.mozilla.org/", thumbnail = mock())
createTab("https://developer.mozilla.org/")
)
).joinBlocking()

Expand All @@ -128,16 +133,20 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

presenter.start()

Expand Down Expand Up @@ -165,16 +174,20 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

presenter.start()

Expand All @@ -197,19 +210,23 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock()),
createTab("https://developer.mozilla.org", id = "c", thumbnail = mock()),
createTab("https://www.firefox.com", id = "d", thumbnail = mock()),
createTab("https://www.google.com", id = "e", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b"),
createTab("https://developer.mozilla.org", id = "c"),
createTab("https://www.firefox.com", id = "d"),
createTab("https://www.google.com", id = "e")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

presenter.start()
testDispatcher.advanceUntilIdle()
Expand All @@ -232,29 +249,30 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock()),
createTab("https://developer.mozilla.org", id = "c", thumbnail = mock()),
createTab("https://www.firefox.com", id = "d", thumbnail = mock()),
createTab("https://www.google.com", id = "e", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b"),
createTab("https://developer.mozilla.org", id = "c"),
createTab("https://www.firefox.com", id = "d"),
createTab("https://www.google.com", id = "e")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

presenter.start()
testDispatcher.advanceUntilIdle()

store.dispatch(
MediaAction.UpdateMediaAggregateAction(
store.state.media.aggregate.copy(
activeTabId = "a",
state = MediaState.State.PLAYING
)
store.state.media.aggregate.copy(activeTabId = "a", state = MediaState.State.PLAYING)
)
).joinBlocking()

Expand All @@ -268,11 +286,11 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock()),
createTab("https://developer.mozilla.org", id = "c", thumbnail = mock()),
createTab("https://www.firefox.com", id = "d", thumbnail = mock()),
createTab("https://www.google.com", id = "e", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b"),
createTab("https://developer.mozilla.org", id = "c"),
createTab("https://www.firefox.com", id = "d"),
createTab("https://www.google.com", id = "e")
),
selectedTabId = "a"
)
Expand All @@ -281,11 +299,9 @@ class TabsTrayPresenterTest {
var closed = false

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(
tabsTray,
store,
thumbnailsUseCases,
tabsFilter = { true },
closeTabsTray = { closed = true })

Expand All @@ -307,8 +323,8 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b")
),
selectedTabId = "a"
)
Expand All @@ -317,11 +333,9 @@ class TabsTrayPresenterTest {
var closed = false

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(
tabsTray,
store,
thumbnailsUseCases,
tabsFilter = { true },
closeTabsTray = { closed = true })

Expand All @@ -348,16 +362,20 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b")
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter = TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { true }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { true },
closeTabsTray = mock()
)

presenter.start()
testDispatcher.advanceUntilIdle()
Expand All @@ -379,17 +397,20 @@ class TabsTrayPresenterTest {
val store = BrowserStore(
BrowserState(
tabs = listOf(
createTab("https://www.mozilla.org", id = "a", thumbnail = mock()),
createTab("https://getpocket.com", id = "b", private = true, thumbnail = mock())
createTab("https://www.mozilla.org", id = "a"),
createTab("https://getpocket.com", id = "b", private = true)
),
selectedTabId = "a"
)
)

val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val thumbnailsUseCases = ThumbnailsUseCases(store, ThumbnailStorage(testContext))
val presenter =
TabsTrayPresenter(tabsTray, store, thumbnailsUseCases, { it.content.private }, mock())
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { it.content.private },
closeTabsTray = mock()
)

presenter.start()
testDispatcher.advanceUntilIdle()
Expand All @@ -408,8 +429,11 @@ class TabsTrayPresenterTest {

var invoked = false
val tabsTray: MockedTabsTray = spy(MockedTabsTray())
val presenter =
TabsTrayPresenter(tabsTray, store, mock(), { it.content.private }, { invoked = true })
val presenter = TabsTrayPresenter(
tabsTray,
store,
tabsFilter = { it.content.private },
closeTabsTray = { invoked = true })

presenter.start()
testDispatcher.advanceUntilIdle()
Expand Down Expand Up @@ -447,8 +471,7 @@ private class MockedTabsTray : TabsTray {

override fun notifyAtLeastOneObserver(block: TabsTray.Observer.() -> Unit) {}

override fun <R> wrapConsumers(block: TabsTray.Observer.(R) -> Boolean): List<(R) -> Boolean> =
emptyList()
override fun <R> wrapConsumers(block: TabsTray.Observer.(R) -> Boolean): List<(R) -> Boolean> = emptyList()

override fun isObserved(): Boolean = false

Expand Down
4 changes: 2 additions & 2 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ permalink: /changelog/
* Added support for [onbeforeunload prompt](https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload)

* **feature-tabs**
* ⚠️ **This is a breaking change**: Added a dependency on `ThumbnailsUseCases` to `TabsFeature` and `TabsTrayPresenter`
for loading a tab's thumbnail.
* Added an optional `ThumbnailsUseCases` to `TabsFeature` and `TabsTrayPresenter` for loading a
tab's thumbnail.

* **browser-thumbnails**
* Adds `LoadThumbnailUseCase` in `ThumbnailsUseCases` for loading the thumbnail of a tab.
Expand Down

0 comments on commit 2a18bde

Please sign in to comment.