From 02a1023025a7721728f17350b79aaff1aca7af20 Mon Sep 17 00:00:00 2001 From: ekager Date: Mon, 2 Nov 2020 13:25:56 -0800 Subject: [PATCH] [components] For https://github.com/mozilla-mobile/android-components/issues/8831 - Adds ability to remove (and restore) a list of tabs --- .../components/browser/session/SessionManager.kt | 13 ++ .../engine/middleware/TabsRemovedMiddleware.kt | 4 + .../browser/session/undo/UndoMiddleware.kt | 5 + .../browser/session/SessionManagerMigrationTest.kt | 132 +++++++++++++++++++++ .../engine/middleware/TabsRemovedMiddlewareTest.kt | 29 +++++ .../browser/session/undo/UndoMiddlewareTest.kt | 46 +++++++ .../browser/state/action/BrowserAction.kt | 7 ++ .../browser/state/reducer/TabListReducer.kt | 55 +++++++++ .../browser/state/action/TabListActionTest.kt | 18 +++ .../browser/thumbnails/ThumbnailsMiddleware.kt | 4 + .../browser/thumbnails/ThumbnailsMiddlewareTest.kt | 16 +++ .../feature/media/middleware/MediaMiddleware.kt | 7 ++ .../media/middleware/MediaMiddlewareTest.kt | 43 +++++++ .../recentlyclosed/RecentlyClosedMiddleware.kt | 9 ++ .../recentlyclosed/RecentlyClosedMiddlewareTest.kt | 43 +++++++ .../components/feature/tabs/TabsUseCases.kt | 17 ++- .../components/feature/tabs/TabsUseCasesTest.kt | 19 +++ .../android/android-components/docs/changelog.md | 5 + 18 files changed, 471 insertions(+), 1 deletion(-) diff --git a/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/SessionManager.kt b/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/SessionManager.kt index cdfb4d75de50..30691e4ce6ae 100644 --- a/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/SessionManager.kt +++ b/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/SessionManager.kt @@ -261,6 +261,19 @@ class SessionManager( } /** + * Removes a list of sessions by id. + */ + fun removeListOfSessions(ids: List) { + for (id in ids) { + sessions.firstOrNull { it.id == id }?.let { delegate.remove(it) } + } + + store?.syncDispatch( + TabListAction.RemoveTabsAction(ids) + ) + } + + /** * Removes all sessions but CustomTab sessions. */ fun removeSessions() { diff --git a/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddleware.kt b/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddleware.kt index b8d239ed5c0f..b4da2bc85bb6 100644 --- a/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddleware.kt +++ b/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddleware.kt @@ -27,6 +27,7 @@ import mozilla.components.lib.state.MiddlewareContext internal class TabsRemovedMiddleware( private val scope: CoroutineScope ) : Middleware { + @Suppress("ComplexMethod") override fun invoke( context: MiddlewareContext, next: (BrowserAction) -> Unit, @@ -39,6 +40,9 @@ internal class TabsRemovedMiddleware( is TabListAction.RemoveTabAction -> context.state.findTab(action.tabId)?.let { onTabsRemoved(context, listOf(it)) } + is TabListAction.RemoveTabsAction -> action.tabIds.mapNotNull { context.state.findTab(it) }.let { + onTabsRemoved(context, it) + } is CustomTabListAction.RemoveAllCustomTabsAction -> onTabsRemoved(context, context.state.customTabs) is CustomTabListAction.RemoveCustomTabAction -> context.state.findCustomTab(action.tabId)?.let { onTabsRemoved(context, listOf(it)) diff --git a/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/undo/UndoMiddleware.kt b/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/undo/UndoMiddleware.kt index c1e3a54a0d6e..a52bb6decf74 100644 --- a/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/undo/UndoMiddleware.kt +++ b/mobile/android/android-components/components/browser/session/src/main/java/mozilla/components/browser/session/undo/UndoMiddleware.kt @@ -61,6 +61,11 @@ class UndoMiddleware( is TabListAction.RemoveTabAction -> state.findTab(action.tabId)?.let { onTabsRemoved(context, listOf(it), state.selectedTabId) } + is TabListAction.RemoveTabsAction -> { + action.tabIds.mapNotNull { state.findTab(it) }.let { + onTabsRemoved(context, it, state.selectedTabId) + } + } // Restore is UndoAction.RestoreRecoverableTabs -> restore(context.state) diff --git a/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt b/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt index c58809dfa825..a1829cedbfb3 100644 --- a/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt +++ b/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/SessionManagerMigrationTest.kt @@ -943,4 +943,136 @@ class SessionManagerMigrationTest { assertEquals("https://www.mozilla.org", manager.selectedSessionOrThrow.url) assertEquals("https://www.mozilla.org", store.state.selectedTab!!.content.url) } + + @Test + fun `State in sync after removingListOfSessions`() { + val store = BrowserStore() + val manager = SessionManager(engine = mock(), store = store) + + manager.add(Session("https://www.mozilla.org", id = "1")) + manager.add(Session("https://getpocket.com", private = true, id = "2")) + manager.add(Session("https://www.theverge.com", private = true, id = "3")) + manager.add(Session("https://www.reddit.com/r/firefox/", id = "4")) + manager.add(Session("https://github.com", id = "5")) + manager.add(Session("https://twitch.tv", private = true, id = "6")) + + assertEquals(6, manager.sessions.size) + assertEquals(3, manager.sessions.filter { !it.private }.size) + assertEquals(3, manager.sessions.filter { it.private }.size) + + assertEquals(6, store.state.tabs.size) + assertEquals(3, store.state.normalTabs.size) + assertEquals(3, store.state.privateTabs.size) + + manager.removeListOfSessions(listOf("1", "3", "5")) + + assertEquals(3, manager.sessions.size) + assertEquals(1, manager.sessions.filter { !it.private }.size) + assertEquals(2, manager.sessions.filter { it.private }.size) + + assertEquals(3, store.state.tabs.size) + assertEquals(1, store.state.normalTabs.size) + assertEquals(2, store.state.privateTabs.size) + + manager.sessions.filter { !it.private }.let { tabs -> + assertEquals("https://www.reddit.com/r/firefox/", tabs[0].url) + } + + manager.sessions.filter { it.private }.let { tabs -> + assertEquals("https://getpocket.com", tabs[0].url) + assertEquals("https://twitch.tv", tabs[1].url) + } + + assertEquals("https://www.reddit.com/r/firefox/", store.state.normalTabs[0].content.url) + assertEquals("https://getpocket.com", store.state.privateTabs[0].content.url) + assertEquals("https://twitch.tv", store.state.privateTabs[1].content.url) + + assertEquals("https://www.reddit.com/r/firefox/", manager.selectedSessionOrThrow.url) + assertEquals("https://www.reddit.com/r/firefox/", store.state.selectedTab!!.content.url) + } + + @Test + fun `State in sync after removingListOfSessions and all parent tabs`() { + val store = BrowserStore() + val manager = SessionManager(engine = mock(), store = store) + + manager.add(Session("https://www.mozilla.org", id = "1")) + manager.add(Session("https://www.reddit.com/r/firefox/", id = "2").apply { parentId = "1" }) + manager.add(Session("https://github.com", id = "3").apply { parentId = "2" }) + + assertEquals(3, manager.sessions.size) + assertEquals(3, manager.sessions.filter { !it.private }.size) + assertEquals(0, manager.sessions.filter { it.private }.size) + + assertEquals(3, store.state.tabs.size) + assertEquals(3, store.state.normalTabs.size) + assertEquals(0, store.state.privateTabs.size) + + manager.removeListOfSessions(listOf("1", "2")) + + assertEquals(1, manager.sessions.size) + assertEquals(1, manager.sessions.filter { !it.private }.size) + assertEquals(0, manager.sessions.filter { it.private }.size) + + assertEquals(1, store.state.tabs.size) + assertEquals(1, store.state.normalTabs.size) + assertEquals(0, store.state.privateTabs.size) + + manager.sessions.filter { !it.private }.let { tabs -> + assertEquals("https://github.com", tabs[0].url) + } + + assertEquals("https://github.com", store.state.normalTabs[0].content.url) + + assertEquals("https://github.com", manager.selectedSessionOrThrow.url) + assertEquals("https://github.com", store.state.selectedTab!!.content.url) + + assertNull(manager.selectedSessionOrThrow.parentId) + assertNull(store.state.selectedTab!!.parentId) + } + + @Test + fun `State in sync after removingListOfSessions and middle parent tab`() { + val store = BrowserStore() + val manager = SessionManager(engine = mock(), store = store) + + manager.add(Session("https://www.mozilla.org", id = "1")) + manager.add(Session("https://www.reddit.com/r/firefox/", id = "2").apply { parentId = "1" }) + manager.add(Session("https://github.com", id = "3").apply { parentId = "2" }) + + assertEquals(3, manager.sessions.size) + assertEquals(3, manager.sessions.filter { !it.private }.size) + assertEquals(0, manager.sessions.filter { it.private }.size) + + assertEquals(3, store.state.tabs.size) + assertEquals(3, store.state.normalTabs.size) + assertEquals(0, store.state.privateTabs.size) + + manager.removeListOfSessions(listOf("2")) + + assertEquals(2, manager.sessions.size) + assertEquals(2, manager.sessions.filter { !it.private }.size) + assertEquals(0, manager.sessions.filter { it.private }.size) + + assertEquals(2, store.state.tabs.size) + assertEquals(2, store.state.normalTabs.size) + assertEquals(0, store.state.privateTabs.size) + + manager.sessions.filter { !it.private }.let { tabs -> + assertEquals("https://www.mozilla.org", tabs[0].url) + assertEquals("https://github.com", tabs[1].url) + } + + assertEquals("https://www.mozilla.org", store.state.normalTabs[0].content.url) + assertEquals("https://github.com", store.state.normalTabs[1].content.url) + + assertEquals("https://www.mozilla.org", manager.selectedSessionOrThrow.url) + assertEquals("https://www.mozilla.org", store.state.selectedTab!!.content.url) + + assertNull(manager.selectedSessionOrThrow.parentId) + assertNull(store.state.selectedTab!!.parentId) + + assertEquals("1", store.state.normalTabs[1].parentId) + assertEquals("1", manager.sessions.filter { !it.private }[1].parentId) + } } diff --git a/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddlewareTest.kt b/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddlewareTest.kt index 7b34e8133fac..23bbae8a995b 100644 --- a/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddlewareTest.kt +++ b/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/engine/middleware/TabsRemovedMiddlewareTest.kt @@ -64,6 +64,35 @@ class TabsRemovedMiddlewareTest { } @Test + fun `closes and unlinks engine session when list of tabs are removed`() = runBlocking { + val middleware = TabsRemovedMiddleware(scope) + + val tab1 = createTab("https://www.mozilla.org", id = "1", private = false) + val tab2 = createTab("https://www.firefox.com", id = "2", private = false) + val tab3 = createTab("https://www.getpocket.com", id = "3", private = false) + + val store = BrowserStore( + initialState = BrowserState(tabs = listOf(tab1, tab2, tab3)), + middleware = listOf(middleware, ConsumeRemoveTabActionsMiddleware()) + ) + + val engineSession1 = linkEngineSession(store, tab1.id) + val engineSession2 = linkEngineSession(store, tab2.id) + val engineSession3 = linkEngineSession(store, tab3.id) + + store.dispatch(TabListAction.RemoveTabsAction(listOf(tab1.id, tab2.id))).joinBlocking() + store.waitUntilIdle() + dispatcher.advanceUntilIdle() + + assertNull(store.state.findTab(tab1.id)?.engineState?.engineSession) + assertNull(store.state.findTab(tab2.id)?.engineState?.engineSession) + assertNotNull(store.state.findTab(tab3.id)?.engineState?.engineSession) + verify(engineSession1).close() + verify(engineSession2).close() + verify(engineSession3, never()).close() + } + + @Test fun `closes and unlinks engine session when all normal tabs are removed`() = runBlocking { val middleware = TabsRemovedMiddleware(scope) diff --git a/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/undo/UndoMiddlewareTest.kt b/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/undo/UndoMiddlewareTest.kt index ceefcc1beeef..7c1ffd3662e3 100644 --- a/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/undo/UndoMiddlewareTest.kt +++ b/mobile/android/android-components/components/browser/session/src/test/java/mozilla/components/browser/session/undo/UndoMiddlewareTest.kt @@ -84,6 +84,52 @@ class UndoMiddlewareTest { } @Test + fun `Undo scenario - Removing list of tabs`() { + val lookup = SessionManagerLookup() + val store = BrowserStore(middleware = listOf( + UndoMiddleware(lookup, clearAfterMillis = 60000) + )) + val manager = SessionManager(engine = mock(), store = store).apply { + lookup.sessionManager = this + } + + val mozilla = Session("https://www.mozilla.org") + val pocket = Session("https://getpocket.com") + val firefox = Session("https://firefox.com") + + manager.add(mozilla) + manager.add(pocket) + manager.add(firefox) + + assertEquals(3, manager.size) + assertEquals(3, store.state.tabs.size) + assertEquals("https://www.mozilla.org", manager.selectedSessionOrThrow.url) + assertEquals("https://www.mozilla.org", store.state.selectedTab!!.content.url) + + manager.removeListOfSessions(listOf(mozilla.id, pocket.id)) + + assertEquals(1, manager.size) + assertEquals(1, store.state.tabs.size) + assertEquals("https://firefox.com", manager.selectedSessionOrThrow.url) + assertEquals("https://firefox.com", store.state.selectedTab!!.content.url) + + testDispatcher.withDispatchingPaused { + // We need to pause the test dispatcher here to avoid it dispatching immediately. + // Otherwise we deadlock the test here when we wait for the store to complete and + // at the same time the middleware dispatches a coroutine on the dispatcher which will + // also block on the store in SessionManager.restore(). + store.dispatch(UndoAction.RestoreRecoverableTabs).joinBlocking() + } + + store.waitUntilIdle() + + assertEquals(3, manager.size) + assertEquals(3, store.state.tabs.size) + assertEquals("https://www.mozilla.org", manager.selectedSessionOrThrow.url) + assertEquals("https://www.mozilla.org", store.state.selectedTab!!.content.url) + } + + @Test fun `Undo scenario - Removing all normal tabs`() { val lookup = SessionManagerLookup() val store = BrowserStore(middleware = listOf( diff --git a/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt b/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt index 7a408e26e47a..0de0ea5312c9 100644 --- a/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt +++ b/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt @@ -154,6 +154,13 @@ sealed class TabListAction : BrowserAction() { TabListAction() /** + * Removes the [TabSessionState]s with the given [tabId]s from the list of sessions. + * + * @property tabIds the IDs of the tabs to remove. + */ + data class RemoveTabsAction(val tabIds: List) : TabListAction() + + /** * Restores state from a (partial) previous state. * * @property tabs the [TabSessionState]s to restore. diff --git a/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TabListReducer.kt b/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TabListReducer.kt index 2c0bf45776d9..024b4d2ad4a7 100644 --- a/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TabListReducer.kt +++ b/mobile/android/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TabListReducer.kt @@ -95,6 +95,42 @@ internal object TabListReducer { } } + is TabListAction.RemoveTabsAction -> { + val tabsToRemove = action.tabIds.mapNotNull { state.findTab(it) } + + if (tabsToRemove.isNullOrEmpty()) { + state + } else { + // Remove tabs and update child tabs' parentId if their parent is in removed list + val updatedTabList = (state.tabs - tabsToRemove).map { tabState -> + tabsToRemove.firstOrNull { removedTab -> tabState.parentId == removedTab.id } + ?.let { tabState.copy(parentId = findNewParentId(it, tabsToRemove)) } + ?: tabState + } + + val updatedSelection = + if (action.tabIds.contains(state.selectedTabId)) { + val removedSelectedTab = + tabsToRemove.first { it.id == state.selectedTabId } + // The selected tab was removed and we need to find a new one + val previousIndex = state.tabs.indexOf(removedSelectedTab) + findNewSelectedTabId( + updatedTabList, + removedSelectedTab.content.private, + previousIndex + ) + } else { + // The selected tab is not affected and can stay the same + state.selectedTabId + } + + state.copy( + tabs = updatedTabList, + selectedTabId = updatedSelection + ) + } + } + is TabListAction.RestoreAction -> { // Verify that none of the tabs to restore already exist action.tabs.forEach { requireUniqueTab(state, it) } @@ -155,6 +191,25 @@ internal object TabListReducer { } /** + * Looks for an appropriate new parentId for a tab by checking if the parent is being removed, + * and if so, will recursively check the parent's parent and so on. + */ +private fun findNewParentId( + tabToFindNewParent: TabSessionState, + tabsToBeRemoved: List +): String? { + return if (tabsToBeRemoved.map { it.id }.contains(tabToFindNewParent.parentId)) { + // The parent tab is being removed, let's check the parent's parent + findNewParentId( + tabsToBeRemoved.first { tabToFindNewParent.parentId == it.id }, + tabsToBeRemoved + ) + } else { + tabToFindNewParent.parentId + } +} + +/** * Find a new selected tab and return its id after the tab at [previousIndex] was removed. */ private fun findNewSelectedTabId( diff --git a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TabListActionTest.kt b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TabListActionTest.kt index 513a678b50f9..af2702ea6d29 100644 --- a/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TabListActionTest.kt +++ b/mobile/android/android-components/components/browser/state/src/test/java/mozilla/components/browser/state/action/TabListActionTest.kt @@ -173,6 +173,24 @@ class TabListActionTest { } @Test + fun `RemoveTabsAction - Removes SessionState`() { + val state = BrowserState( + tabs = listOf( + createTab(id = "a", url = "https://www.mozilla.org"), + createTab(id = "b", url = "https://www.firefox.com"), + createTab(id = "c", url = "https://www.getpocket.com") + ) + ) + val store = BrowserStore(state) + + store.dispatch(TabListAction.RemoveTabsAction(listOf("a", "b"))) + .joinBlocking() + + assertEquals(1, store.state.tabs.size) + assertEquals("https://www.getpocket.com", store.state.tabs[0].content.url) + } + + @Test fun `RemoveTabAction - Noop for unknown id`() { val state = BrowserState( tabs = listOf( diff --git a/mobile/android/android-components/components/browser/thumbnails/src/main/java/mozilla/components/browser/thumbnails/ThumbnailsMiddleware.kt b/mobile/android/android-components/components/browser/thumbnails/src/main/java/mozilla/components/browser/thumbnails/ThumbnailsMiddleware.kt index 03a6db7cc33d..45d381132905 100644 --- a/mobile/android/android-components/components/browser/thumbnails/src/main/java/mozilla/components/browser/thumbnails/ThumbnailsMiddleware.kt +++ b/mobile/android/android-components/components/browser/thumbnails/src/main/java/mozilla/components/browser/thumbnails/ThumbnailsMiddleware.kt @@ -19,6 +19,7 @@ import mozilla.components.lib.state.MiddlewareContext class ThumbnailsMiddleware( private val thumbnailStorage: ThumbnailStorage ) : Middleware { + @Suppress("ComplexMethod") override fun invoke( context: MiddlewareContext, next: (BrowserAction) -> Unit, @@ -47,6 +48,9 @@ class ThumbnailsMiddleware( // Delete the tab screenshot from the storage when the tab is removed. thumbnailStorage.deleteThumbnail(action.tabId) } + is TabListAction.RemoveTabsAction -> { + action.tabIds.forEach { thumbnailStorage.deleteThumbnail(it) } + } } next(action) diff --git a/mobile/android/android-components/components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/ThumbnailsMiddlewareTest.kt b/mobile/android/android-components/components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/ThumbnailsMiddlewareTest.kt index 34a378e52491..88c7e33dcf4a 100644 --- a/mobile/android/android-components/components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/ThumbnailsMiddlewareTest.kt +++ b/mobile/android/android-components/components/browser/thumbnails/src/test/java/mozilla/components/browser/thumbnails/ThumbnailsMiddlewareTest.kt @@ -104,4 +104,20 @@ class ThumbnailsMiddlewareTest { store.dispatch(TabListAction.RemoveTabAction(sessionIdOrUrl)).joinBlocking() verify(thumbnailStorage).deleteThumbnail(sessionIdOrUrl) } + + @Test + fun `thumbnail storage removes the thumbnail on remove tabs action`() { + val sessionIdOrUrl = "test-tab1" + val thumbnailStorage: ThumbnailStorage = mock() + val store = BrowserStore( + initialState = BrowserState(tabs = listOf( + createTab("https://www.mozilla.org", id = "test-tab1"), + createTab("https://www.firefox.com", id = "test-tab2") + )), + middleware = listOf(ThumbnailsMiddleware(thumbnailStorage)) + ) + + store.dispatch(TabListAction.RemoveTabsAction(listOf(sessionIdOrUrl))).joinBlocking() + verify(thumbnailStorage).deleteThumbnail(sessionIdOrUrl) + } } diff --git a/mobile/android/android-components/components/feature/media/src/main/java/mozilla/components/feature/media/middleware/MediaMiddleware.kt b/mobile/android/android-components/components/feature/media/src/main/java/mozilla/components/feature/media/middleware/MediaMiddleware.kt index c1ba71aba88f..5e045f7519e9 100644 --- a/mobile/android/android-components/components/feature/media/src/main/java/mozilla/components/feature/media/middleware/MediaMiddleware.kt +++ b/mobile/android/android-components/components/feature/media/src/main/java/mozilla/components/feature/media/middleware/MediaMiddleware.kt @@ -69,6 +69,13 @@ class MediaMiddleware( listOf(action.tabId) )) + is TabListAction.RemoveTabsAction -> + store.dispatch( + MediaAction.RemoveTabMediaAction( + action.tabIds + ) + ) + is CustomTabListAction.RemoveAllCustomTabsAction -> store.dispatch(MediaAction.RemoveTabMediaAction( store.state.customTabs.map { it.id } diff --git a/mobile/android/android-components/components/feature/media/src/test/java/mozilla/components/feature/media/middleware/MediaMiddlewareTest.kt b/mobile/android/android-components/components/feature/media/src/test/java/mozilla/components/feature/media/middleware/MediaMiddlewareTest.kt index a3045438a047..ee515ad36d39 100644 --- a/mobile/android/android-components/components/feature/media/src/test/java/mozilla/components/feature/media/middleware/MediaMiddlewareTest.kt +++ b/mobile/android/android-components/components/feature/media/src/test/java/mozilla/components/feature/media/middleware/MediaMiddlewareTest.kt @@ -184,6 +184,49 @@ class MediaMiddlewareTest { } @Test + fun `State is updated after sessions are removed`() { + val middleware = MediaMiddleware(testContext, AbstractMediaService::class.java) + + val store = BrowserStore( + initialState = BrowserState( + tabs = listOf( + createTab("https://www.mozilla.org", id = "test-tab"), + createTab("https://www.firefox.com", id = "test-tab-2") + ) + ), + middleware = listOf(middleware) + ) + + val media = createMockMediaElement( + id = "test-media", + state = Media.State.PLAYING + ) + + store.dispatch( + MediaAction.AddMediaAction("test-tab", media) + ).joinBlocking() + + middleware.mediaAggregateUpdate.updateAggregateJob!!.joinBlocking() + store.waitUntilIdle() + + assertEquals(MediaState.State.PLAYING, store.state.media.aggregate.state) + assertEquals("test-tab", store.state.media.aggregate.activeTabId) + assertEquals(1, store.state.media.aggregate.activeMedia.size) + assertEquals("test-media", store.state.media.aggregate.activeMedia[0]) + + store.dispatch( + TabListAction.RemoveTabsAction(listOf("test-tab", "test-tab-2")) + ).joinBlocking() + + middleware.mediaAggregateUpdate.updateAggregateJob!!.joinBlocking() + store.waitUntilIdle() + + assertEquals(MediaState.State.NONE, store.state.media.aggregate.state) + assertEquals(null, store.state.media.aggregate.activeTabId) + assertEquals(0, store.state.media.aggregate.activeMedia.size) + } + + @Test fun `Multiple media of session start playing and stop`() { val middleware = MediaMiddleware(testContext, AbstractMediaService::class.java) diff --git a/mobile/android/android-components/components/feature/recentlyclosed/src/main/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddleware.kt b/mobile/android/android-components/components/feature/recentlyclosed/src/main/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddleware.kt index 1d51619a5cb7..88950de3f39b 100644 --- a/mobile/android/android-components/components/feature/recentlyclosed/src/main/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddleware.kt +++ b/mobile/android/android-components/components/feature/recentlyclosed/src/main/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddleware.kt @@ -71,6 +71,15 @@ class RecentlyClosedMiddleware( ) } } + is TabListAction.RemoveTabsAction -> { + val tabs = action.tabIds.mapNotNull { context.state.findTab(it) } + .filterNot { it.content.private } + context.store.dispatch( + RecentlyClosedAction.AddClosedTabsAction( + tabs.toClosedTab() + ) + ) + } is RecentlyClosedAction.AddClosedTabsAction -> { addTabsToStorage( action.tabs diff --git a/mobile/android/android-components/components/feature/recentlyclosed/src/test/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddlewareTest.kt b/mobile/android/android-components/components/feature/recentlyclosed/src/test/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddlewareTest.kt index 3ed995aed02a..4572d3ae42c5 100644 --- a/mobile/android/android-components/components/feature/recentlyclosed/src/test/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddlewareTest.kt +++ b/mobile/android/android-components/components/feature/recentlyclosed/src/test/java/mozilla/components/feature/recentlyclosed/RecentlyClosedMiddlewareTest.kt @@ -84,6 +84,49 @@ class RecentlyClosedMiddlewareTest { } @Test + fun `closed tab storage adds normal tabs removed with TabListAction`() = + runBlockingTest { + val storage: RecentlyClosedTabsStorage = mock() + val middleware = spy(RecentlyClosedMiddleware(testContext, 5, engine, scope)) + whenever(middleware.recentlyClosedTabsStorage).thenReturn(storage) + + val tab = createTab("https://www.mozilla.org", private = false, id = "1234") + val tab2 = createTab("https://www.firefox.com", private = false, id = "5678") + + val store = spy( + BrowserStore( + initialState = BrowserState( + tabs = listOf(tab, tab2) + ), + middleware = listOf(middleware) + ) + ) + + store.dispatch(TabListAction.RemoveTabsAction(listOf("1234", "5678"))).joinBlocking() + dispatcher.advanceUntilIdle() + store.waitUntilIdle() + + val closedTabCaptor = argumentCaptor>() + verify(storage).addTabsToCollectionWithMax( + closedTabCaptor.capture(), + eq(5) + ) + assertEquals(2, closedTabCaptor.value.size) + assertEquals(tab.content.title, closedTabCaptor.value[0].title) + assertEquals(tab.content.url, closedTabCaptor.value[0].url) + assertEquals(tab2.content.title, closedTabCaptor.value[1].title) + assertEquals(tab2.content.url, closedTabCaptor.value[1].url) + assertEquals( + tab.engineState.engineSessionState, + closedTabCaptor.value[0].engineSessionState + ) + assertEquals( + tab2.engineState.engineSessionState, + closedTabCaptor.value[1].engineSessionState + ) + } + + @Test fun `closed tab storage adds a normal tab removed with TabListAction`() = runBlockingTest { val storage: RecentlyClosedTabsStorage = mock() diff --git a/mobile/android/android-components/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt b/mobile/android/android-components/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt index a5c02c19f918..971b22f1d307 100644 --- a/mobile/android/android-components/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt +++ b/mobile/android/android-components/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt @@ -5,10 +5,10 @@ package mozilla.components.feature.tabs import mozilla.components.browser.session.Session -import mozilla.components.browser.state.state.SessionState.Source import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.action.EngineAction import mozilla.components.browser.state.action.UndoAction +import mozilla.components.browser.state.state.SessionState.Source import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession.LoadUrlFlags @@ -217,6 +217,20 @@ class TabsUseCases( } } + /** + * Use case for removing a list of tabs. + */ + class RemoveTabsUseCase internal constructor( + private val sessionManager: SessionManager + ) { + /** + * Removes a specified list of tabs. + */ + operator fun invoke(ids: List) { + sessionManager.removeListOfSessions(ids) + } + } + class RemoveAllTabsUseCase internal constructor( private val sessionManager: SessionManager ) { @@ -272,6 +286,7 @@ class TabsUseCases( val addTab: AddNewTabUseCase by lazy { AddNewTabUseCase(store, sessionManager) } val addPrivateTab: AddNewPrivateTabUseCase by lazy { AddNewPrivateTabUseCase(store, sessionManager) } val removeAllTabs: RemoveAllTabsUseCase by lazy { RemoveAllTabsUseCase(sessionManager) } + val removeTabs: RemoveTabsUseCase by lazy { RemoveTabsUseCase(sessionManager) } val removeNormalTabs: RemoveNormalTabsUseCase by lazy { RemoveNormalTabsUseCase(sessionManager) } val removePrivateTabs: RemovePrivateTabsUseCase by lazy { RemovePrivateTabsUseCase(sessionManager) } val undo by lazy { UndoTabRemovalUseCase(store) } diff --git a/mobile/android/android-components/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt b/mobile/android/android-components/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt index fbc26422b6cf..5f915f511d69 100644 --- a/mobile/android/android-components/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt +++ b/mobile/android/android-components/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt @@ -61,6 +61,25 @@ class TabsUseCasesTest { } @Test + fun `RemoveTabsUseCase - list of sessions can be removed`() { + val sessionManager = spy(SessionManager(mock())) + val useCases = TabsUseCases(BrowserStore(), sessionManager) + + val session = Session(id = "test", initialUrl = "http://mozilla.org") + val session2 = Session(id = "test2", initialUrl = "http://pocket.com") + val session3 = Session(id = "test3", initialUrl = "http://firefox.com") + + sessionManager.add(listOf(session, session2, session3)) + assertEquals(3, sessionManager.size) + + useCases.removeTabs.invoke(listOf(session.id, session2.id)) + + verify(sessionManager).removeListOfSessions(listOf(session.id, session2.id)) + + assertEquals(1, sessionManager.size) + } + + @Test fun `AddNewTabUseCase - session will be added to session manager`() { val sessionManager = spy(SessionManager(mock())) val store: BrowserStore = mock() diff --git a/mobile/android/android-components/docs/changelog.md b/mobile/android/android-components/docs/changelog.md index e175aea3ab7d..18aa80d0d8c5 100644 --- a/mobile/android/android-components/docs/changelog.md +++ b/mobile/android/android-components/docs/changelog.md @@ -12,6 +12,11 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/.config.yml) +* **feature-tabs** + * Added `TabsUseCases.RemoveTabsUseCase` for removing an arbitrary list of tabs. +* **browser-state** + * Added `TabListAction.RemoveTabsAction` to `BrowserAction`. + * **feature-downloads** * 🚒 Bug fixed [issue #8823](https://github.com/mozilla-mobile/android-components/issues/8823) Downloads for data URLs were failing on nightly and beta more details in the [Fenix issue](https://github.com/mozilla-mobile/fenix/issues/16228#issuecomment-717976737). -- 2.11.4.GIT