Commit 6e4ba44e authored by MozLando's avatar MozLando
Browse files

Merge #6153 



6153: Closes #6145: Do not invoke close callback on start r=rocketsroger a=jonalmeida

In the tabs tray Flow, `ifChanged` is invoked because the initial state starts with `null` and that in turn invokes the close tabs callback.

We also need to pass the initial filter of tabs from the feature to satisfy changes on the app's side that we need to use here.



Co-authored-by: default avatarJonathan Almeida <jalmeida@mozilla.com>
parents 3f35ecab 0b6dee17

components/feature/tabs/src/main/java/mozilla/components/feature/tabs/tabstray/TabsFeature.kt

+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

components/feature/tabs/src/main/java/mozilla/components/feature/tabs/tabstray/TabsTrayPresenter.kt

+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)

            }

    }

components/feature/tabs/src/test/java/mozilla/components/feature/tabs/tabstray/TabsFeatureTest.kt

+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

    }

}

components/feature/tabs/src/test/java/mozilla/components/feature/tabs/tabstray/TabsTrayPresenterTest.kt

+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 {

docs/changelog.md

+5 −0
Original line number Diff line number Diff line
@@ -41,6 +41,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)