Commit 0b6dee17 authored by Jonathan Almeida's avatar Jonathan Almeida
Browse files

Closes #6145: Do not invoke close callback on start

parent 2b68e914
Loading
Loading
Loading
Loading
+7 −2
Original line number Diff line number Diff line
@@ -14,17 +14,22 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature

/**
 * Feature implementation for connecting a tabs tray implementation with the session module.
 *
 * @param defaultTabsFilter A tab filter that is used for the initial presenting of tabs that will be used by
 * [TabsFeature.filterTabs] by default as well.
 */
class TabsFeature(
    tabsTray: TabsTray,
    private val store: BrowserStore,
    tabsUseCases: TabsUseCases,
    private val defaultTabsFilter: (TabSessionState) -> Boolean = { true },
    closeTabsTray: () -> Unit
) : LifecycleAwareFeature {
    @VisibleForTesting
    internal var presenter = TabsTrayPresenter(
        tabsTray,
        store,
        defaultTabsFilter,
        closeTabsTray
    )

@@ -49,9 +54,9 @@ class TabsFeature(
     * Filter the list of tabs using [tabsFilter].
     *
     * @param tabsFilter A filter function returning `true` for all tabs that should be displayed in
     * the tabs tray.
     * the tabs tray. Uses the [defaultTabsFilter] if none is provided.
     */
    fun filterTabs(tabsFilter: (TabSessionState) -> Boolean) {
    fun filterTabs(tabsFilter: (TabSessionState) -> Boolean = defaultTabsFilter) {
        presenter.tabsFilter = tabsFilter

        val state = store.state
+6 −4
Original line number Diff line number Diff line
@@ -27,8 +27,8 @@ import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
class TabsTrayPresenter(
    private val tabsTray: TabsTray,
    private val store: BrowserStore,
    private val closeTabsTray: () -> Unit,
    internal var tabsFilter: (TabSessionState) -> Boolean = { true }
    internal var tabsFilter: (TabSessionState) -> Boolean,
    private val closeTabsTray: () -> Unit
) {
    private var tabs: Tabs? = null
    private var scope: CoroutineScope? = null
@@ -45,11 +45,13 @@ class TabsTrayPresenter(
        flow.map { state -> state.toTabs(tabsFilter) }
            .ifChanged()
            .collect { tabs ->
                updateTabs(tabs)

                if (tabs.list.isEmpty()) {
                // Do not invoke the callback on start if this is the initial state.
                if (tabs.list.isEmpty() && this.tabs != null) {
                    closeTabsTray.invoke()
                }

                updateTabs(tabs)
            }
    }

+22 −4
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ class TabsFeatureTest {
        val presenter: TabsTrayPresenter = mock()
        val interactor: TabsTrayInteractor = mock()
        val useCases = TabsUseCases(sessionManager)
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock()))
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock(), mock()))

        assertNotEquals(tabsFeature.interactor, interactor)
        assertNotEquals(tabsFeature.presenter, presenter)
@@ -44,7 +44,7 @@ class TabsFeatureTest {
        val presenter: TabsTrayPresenter = mock()
        val interactor: TabsTrayInteractor = mock()
        val useCases = TabsUseCases(sessionManager)
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock()))
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock(), mock()))

        tabsFeature.presenter = presenter
        tabsFeature.interactor = interactor
@@ -62,7 +62,7 @@ class TabsFeatureTest {
        val presenter: TabsTrayPresenter = mock()
        val interactor: TabsTrayInteractor = mock()
        val useCases = TabsUseCases(sessionManager)
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock()))
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock(), mock()))

        tabsFeature.presenter = presenter
        tabsFeature.interactor = interactor
@@ -80,7 +80,7 @@ class TabsFeatureTest {
        val presenter: TabsTrayPresenter = mock()
        val interactor: TabsTrayInteractor = mock()
        val useCases = TabsUseCases(sessionManager)
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock()))
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, mock(), mock()))

        tabsFeature.presenter = presenter
        tabsFeature.interactor = interactor
@@ -92,4 +92,22 @@ class TabsFeatureTest {
        verify(presenter).tabsFilter = filter
        verify(presenter).updateTabs(Tabs(emptyList(), -1))
    }

    @Test
    fun `filterTabs uses default filter if a new one is not provided`() {
        val store = BrowserStore()
        val filter: (TabSessionState) -> Boolean = { false }
        val sessionManager = SessionManager(engine = mock())
        val useCases = TabsUseCases(sessionManager)
        val tabsFeature = spy(TabsFeature(mock(), store, useCases, filter, mock()))
        val presenter: TabsTrayPresenter = mock()
        val interactor: TabsTrayInteractor = mock()

        tabsFeature.presenter = presenter
        tabsFeature.interactor = interactor

        tabsFeature.filterTabs()

        verify(presenter).tabsFilter = filter
    }
}
+28 −9
Original line number Diff line number Diff line
@@ -63,7 +63,7 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock())
        val presenter = TabsTrayPresenter(tabsTray, store, { true }, mock())

        verifyNoMoreInteractions(tabsTray)

@@ -94,7 +94,7 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock())
        val presenter = TabsTrayPresenter(tabsTray, store, { true }, mock())

        presenter.start()

@@ -128,7 +128,7 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock())
        val presenter = TabsTrayPresenter(tabsTray, store, { true }, mock())

        presenter.start()

@@ -164,7 +164,7 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock())
        val presenter = TabsTrayPresenter(tabsTray, store, { true }, mock())

        presenter.start()

@@ -198,7 +198,7 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock())
        val presenter = TabsTrayPresenter(tabsTray, store, { true }, mock())

        presenter.start()
        testDispatcher.advanceUntilIdle()
@@ -234,7 +234,7 @@ class TabsTrayPresenterTest {
        var closed = false

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

        presenter.start()
        testDispatcher.advanceUntilIdle()
@@ -264,7 +264,7 @@ class TabsTrayPresenterTest {
        var closed = false

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

        presenter.start()
        testDispatcher.advanceUntilIdle()
@@ -297,7 +297,7 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock())
        val presenter = TabsTrayPresenter(tabsTray, store, { true }, mock())

        presenter.start()
        testDispatcher.advanceUntilIdle()
@@ -327,13 +327,32 @@ class TabsTrayPresenterTest {
        )

        val tabsTray: MockedTabsTray = spy(MockedTabsTray())
        val presenter = TabsTrayPresenter(tabsTray, store, mock(), { it.content.private })
        val presenter = TabsTrayPresenter(tabsTray, store, { it.content.private }, mock())

        presenter.start()
        testDispatcher.advanceUntilIdle()

        assertTrue(tabsTray.updateTabs?.list?.size == 1)
    }

    @Test
    fun `tabs tray should not invoke the close callback on start`() {
        val store = BrowserStore(
            BrowserState(
                tabs = emptyList(),
                selectedTabId = null
            )
        )

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

        presenter.start()
        testDispatcher.advanceUntilIdle()

        assertFalse(invoked)
    }
}

private class MockedTabsTray : TabsTray {
+5 −0
Original line number Diff line number Diff line
@@ -38,6 +38,11 @@ permalink: /changelog/
* **service-sync-logins**
  * ⚠️ **This is a breaking change**: Refactored `AsyncLoginsStorage`, which is now called `SyncableLoginsStorage`. New class caches the db connection, and removes lock/unlock operations from the public API.

* **feature-tabs**
  * Fixed close tabs callback incorrectly invoked on start.
  * ⚠️ **This is a breaking change**: Added `defaultTabsFilter` to `TabsFeature` for initial presenting of tabs.
    * `TabsFeature.filterTabs` also uses the same filter if no new filter is provided.

# 34.0.0

* [Commits](https://github.com/mozilla-mobile/android-components/compare/v33.0.0...v34.0.0)
Loading