Closes #3527: browser-state: Add parent tab functionality

components/browser/state/build.gradle

@@ -28,14 +28,15 @@ dependencies {
     implementation project(':support-utils')
     implementation project(':support-ktx')
 
-    implementation Dependencies.kotlin_stdlib
-    implementation Dependencies.kotlin_coroutines
     implementation Dependencies.androidx_browser
+    implementation Dependencies.kotlin_coroutines
+    implementation Dependencies.kotlin_stdlib
 
     testImplementation project(':support-test')
+    testImplementation Dependencies.androidx_test_junit
     testImplementation Dependencies.testing_junit
-    testImplementation Dependencies.testing_robolectric
     testImplementation Dependencies.testing_mockito
+    testImplementation Dependencies.testing_robolectric
 }
 
 apply from: '../../../publish.gradle'

components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt

@@ -16,27 +16,40 @@ import mozilla.components.lib.state.Action
  * [Action] implementation related to [BrowserState].
  */
 sealed class BrowserAction : Action
+
 /**
  * [BrowserAction] implementations related to updating the list of [TabSessionState] inside [BrowserState].
  */
 sealed class TabListAction : BrowserAction() {
     /**
      * Adds a new [TabSessionState] to the list.
+     *
+     * @property tab the [TabSessionState] to add
+     * @property select whether or not to the tab should be selected.
      */
     data class AddTabAction(val tab: TabSessionState, val select: Boolean = false) : TabListAction()
 
     /**
      * Marks the [TabSessionState] with the given [tabId] as selected tab.
+     *
+     * @property tabId the ID of the tab to select.
      */
     data class SelectTabAction(val tabId: String) : TabListAction()
 
     /**
      * Removes the [TabSessionState] with the given [tabId] from the list of sessions.
+     *
+     * @property tabId the ID of the tab to remove.
+     * @property selectParentIfExists whether or not a parent tab should be
+     * selected if one exists, defaults to true.
      */
-    data class RemoveTabAction(val tabId: String) : TabListAction()
+    data class RemoveTabAction(val tabId: String, val selectParentIfExists: Boolean = true) : TabListAction()
 
     /**
      * Restores state from a (partial) previous state.
+     *
+     * @property tabs the [TabSessionState]s to restore.
+     * @property selectedTabId the ID of the tab to select.
      */
     data class RestoreAction(val tabs: List<TabSessionState>, val selectedTabId: String? = null) : TabListAction()
 
@@ -62,11 +75,15 @@ sealed class TabListAction : BrowserAction() {
 sealed class CustomTabListAction : BrowserAction() {
     /**
      * Adds a new [CustomTabSessionState] to [BrowserState.customTabs].
+     *
+     * @property tab the [CustomTabSessionState] to add.
      */
     data class AddCustomTabAction(val tab: CustomTabSessionState) : CustomTabListAction()
 
     /**
      * Removes an existing [CustomTabSessionState] to [BrowserState.customTabs].
+     *
+     * @property tabId the ID of the custom tab to remove.
      */
     data class RemoveCustomTabAction(val tabId: String) : CustomTabListAction()
 

components/browser/state/src/main/java/mozilla/components/browser/state/reducer/TabListReducer.kt

@@ -18,8 +18,22 @@ internal object TabListReducer {
     fun reduce(state: BrowserState, action: TabListAction): BrowserState {
         return when (action) {
             is TabListAction.AddTabAction -> {
+
+                val updatedTabList = if (action.tab.parentId != null) {
+                    val parentIndex = state.tabs.indexOfFirst { it.id == action.tab.parentId }
+                    if (parentIndex == -1) {
+                        throw IllegalArgumentException("The parent does not exist")
+                    }
+
+                    // Add the child tab next to its parent
+                    val childIndex = parentIndex + 1
+                    state.tabs.subList(0, childIndex) + action.tab + state.tabs.subList(childIndex, state.tabs.size)
+                } else {
+                    state.tabs + action.tab
+                }
+
                 state.copy(
-                    tabs = state.tabs + action.tab,
+                    tabs = updatedTabList,
                     selectedTabId = if (action.select || state.selectedTabId == null) {
                         action.tab.id
                     } else {
@@ -33,16 +47,25 @@ internal object TabListReducer {
             }
 
             is TabListAction.RemoveTabAction -> {
-                val tab = state.findTab(action.tabId)
+                val tabToRemove = state.findTab(action.tabId)
 
-                if (tab == null) {
+                if (tabToRemove == null) {
                     state
                 } else {
-                    val updatedTabList = state.tabs - tab
-                    val updatedSelection = if (state.selectedTabId == tab.id) {
-                        val previousIndex = state.tabs.indexOf(tab)
-                        findNewSelectedTabId(updatedTabList, tab.content.private, previousIndex)
+                    // Remove tab and update child tabs in case their parent was removed
+                    val updatedTabList = (state.tabs - tabToRemove).map {
+                        if (it.parentId == tabToRemove.id) it.copy(parentId = tabToRemove.parentId) else it
+                    }
+
+                    val updatedSelection = if (action.selectParentIfExists && tabToRemove.parentId != null) {
+                        // The parent tab should be selected if one exists
+                        tabToRemove.parentId
+                    } else if (state.selectedTabId == tabToRemove.id) {
+                        // The selected tab was removed and we need to find a new one
+                        val previousIndex = state.tabs.indexOf(tabToRemove)
+                        findNewSelectedTabId(updatedTabList, tabToRemove.content.private, previousIndex)
                     } else {
+                        // The selected tab is not affected and can stay the same
                         state.selectedTabId
                     }
 

components/browser/state/src/main/java/mozilla/components/browser/state/state/TabSessionState.kt

@@ -11,20 +11,27 @@ import java.util.UUID
  *
  * @property id the ID of this tab and session.
  * @property content the [ContentState] of this tab.
+ * @property parentId the parent ID of this tab or null if this tab has no
+ * parent. The parent tab is usually the tab that initiated opening this
+ * tab (e.g. the user clicked a link with target="_blank" or selected
+ * "open in new tab" or a "window.open" was triggered).
  */
 data class TabSessionState(
     override val id: String = UUID.randomUUID().toString(),
-    override val content: ContentState
+    override val content: ContentState,
+    val parentId: String? = null
 ) : SessionState
 
 internal fun createTab(
     url: String,
     private: Boolean = false,
-    id: String = UUID.randomUUID().toString()
+    id: String = UUID.randomUUID().toString(),
+    parent: TabSessionState? = null
 ): TabSessionState {
     return TabSessionState(
         id = id,
-        content = ContentState(url, private)
+        content = ContentState(url, private),
+        parentId = parent?.id
     )
 }
 

components/browser/state/src/test/java/mozilla/components/browser/state/action/TabListActionTest.kt

@@ -4,6 +4,7 @@
 
 package mozilla.components.browser.state.action
 
+import mozilla.components.browser.state.selector.selectedTab
 import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.createCustomTab
 import mozilla.components.browser.state.state.createTab
@@ -25,8 +26,7 @@ class TabListActionTest {
 
         val tab = createTab(url = "https://www.mozilla.org")
 
-        store.dispatch(TabListAction.AddTabAction(tab))
-            .joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
 
         assertEquals(1, store.state.tabs.size)
         assertEquals(tab.id, store.state.selectedTabId)
@@ -70,6 +70,56 @@ class TabListActionTest {
         assertEquals(newTab.id, store.state.selectedTabId)
     }
 
+    @Test
+    fun `AddTabAction - Specify parent tab`() {
+        val store = BrowserStore()
+
+        val tab1 = createTab("https://www.mozilla.org")
+        val tab2 = createTab("https://www.firefox.com")
+        val tab3 = createTab("https://wiki.mozilla.org", parent = tab1)
+        val tab4 = createTab("https://github.com/mozilla-mobile/android-components", parent = tab2)
+
+        store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab2)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab3)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab4)).joinBlocking()
+
+        assertEquals(4, store.state.tabs.size)
+        assertNull(store.state.tabs[0].parentId)
+        assertNull(store.state.tabs[2].parentId)
+        assertEquals(tab1.id, store.state.tabs[1].parentId)
+        assertEquals(tab2.id, store.state.tabs[3].parentId)
+    }
+
+    @Test
+    fun `AddTabAction - Tabs with parent are added after (next to) parent`() {
+        val store = BrowserStore()
+
+        val parent01 = createTab("https://www.mozilla.org")
+        val parent02 = createTab("https://getpocket.com")
+        val tab1 = createTab("https://www.firefox.com")
+        val tab2 = createTab("https://developer.mozilla.org/en-US/")
+        val child001 = createTab("https://www.mozilla.org/en-US/internet-health/", parent = parent01)
+        val child002 = createTab("https://www.mozilla.org/en-US/technology/", parent = parent01)
+        val child003 = createTab("https://getpocket.com/add/", parent = parent02)
+
+        store.dispatch(TabListAction.AddTabAction(parent01)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(child001)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab2)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(parent02)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(child002)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(child003)).joinBlocking()
+
+        assertEquals(parent01.id, store.state.tabs[0].id) // ├── parent 1
+        assertEquals(child002.id, store.state.tabs[1].id) // │   ├── child 2
+        assertEquals(child001.id, store.state.tabs[2].id) // │   └── child 1
+        assertEquals(tab1.id, store.state.tabs[3].id) //     ├──tab 1
+        assertEquals(tab2.id, store.state.tabs[4].id) //     ├──tab 2
+        assertEquals(parent02.id, store.state.tabs[5].id) // └── parent 2
+        assertEquals(child003.id, store.state.tabs[6].id) //     └── child 3
+    }
+
     @Test
     fun `SelectTabAction - Selects SessionState by id`() {
         val state = BrowserState(
@@ -260,6 +310,116 @@ class TabListActionTest {
         assertNull(store.state.selectedTabId)
     }
 
+    @Test
+    fun `RemoveTabAction - Parent will be selected if child is removed and flag is set to true (default)`() {
+        val store = BrowserStore()
+
+        val parent = createTab("https://www.mozilla.org")
+        val tab1 = createTab("https://www.firefox.com")
+        val tab2 = createTab("https://getpocket.com")
+        val child = createTab("https://www.mozilla.org/en-US/internet-health/", parent = parent)
+
+        store.dispatch(TabListAction.AddTabAction(parent)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab2)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(child)).joinBlocking()
+
+        store.dispatch(TabListAction.SelectTabAction(child.id)).joinBlocking()
+        store.dispatch(TabListAction.RemoveTabAction(child.id, selectParentIfExists = true)).joinBlocking()
+
+        assertEquals(parent.id, store.state.selectedTabId)
+        assertEquals("https://www.mozilla.org", store.state.selectedTab?.content?.url)
+    }
+
+    @Test
+    fun `RemoveTabAction - Parent will not be selected if child is removed and flag is set to false`() {
+        val store = BrowserStore()
+
+        val parent = createTab("https://www.mozilla.org")
+
+        val tab1 = createTab("https://www.firefox.com")
+        val tab2 = createTab("https://getpocket.com")
+        val child1 = createTab("https://www.mozilla.org/en-US/internet-health/", parent = parent)
+        val child2 = createTab("https://www.mozilla.org/en-US/technology/", parent = parent)
+
+        store.dispatch(TabListAction.AddTabAction(parent)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab2)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(child1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(child2)).joinBlocking()
+
+        store.dispatch(TabListAction.SelectTabAction(child1.id)).joinBlocking()
+        store.dispatch(TabListAction.RemoveTabAction(child1.id, selectParentIfExists = false)).joinBlocking()
+
+        assertEquals(tab1.id, store.state.selectedTabId)
+        assertEquals("https://www.firefox.com", store.state.selectedTab?.content?.url)
+    }
+
+    @Test
+    fun `RemoveTabAction - Providing selectParentIfExists when removing tab without parent has no effect`() {
+        val store = BrowserStore()
+
+        val tab1 = createTab("https://www.firefox.com")
+        val tab2 = createTab("https://getpocket.com")
+        val tab3 = createTab("https://www.mozilla.org/en-US/internet-health/")
+
+        store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab2)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab3)).joinBlocking()
+
+        store.dispatch(TabListAction.SelectTabAction(tab3.id)).joinBlocking()
+        store.dispatch(TabListAction.RemoveTabAction(tab3.id, selectParentIfExists = true)).joinBlocking()
+
+        assertEquals(tab2.id, store.state.selectedTabId)
+        assertEquals("https://getpocket.com", store.state.selectedTab?.content?.url)
+    }
+
+    @Test
+    fun `RemoveTabAction - Children are updated when parent is removed`() {
+        val store = BrowserStore()
+
+        val tab0 = createTab("https://www.firefox.com")
+        val tab1 = createTab("https://developer.mozilla.org/en-US/", parent = tab0)
+        val tab2 = createTab("https://www.mozilla.org/en-US/internet-health/", parent = tab1)
+        val tab3 = createTab("https://www.mozilla.org/en-US/technology/", parent = tab2)
+
+        store.dispatch(TabListAction.AddTabAction(tab0)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab1)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab2)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab3)).joinBlocking()
+
+        // tab0 <- tab1 <- tab2 <- tab3
+        assertEquals(tab0.id, store.state.tabs[0].id)
+        assertEquals(tab1.id, store.state.tabs[1].id)
+        assertEquals(tab2.id, store.state.tabs[2].id)
+        assertEquals(tab3.id, store.state.tabs[3].id)
+
+        assertNull(store.state.tabs[0].parentId)
+        assertEquals(tab0.id, store.state.tabs[1].parentId)
+        assertEquals(tab1.id, store.state.tabs[2].parentId)
+        assertEquals(tab2.id, store.state.tabs[3].parentId)
+
+        store.dispatch(TabListAction.RemoveTabAction(tab2.id)).joinBlocking()
+
+        // tab0 <- tab1 <- tab3
+        assertEquals(tab0.id, store.state.tabs[0].id)
+        assertEquals(tab1.id, store.state.tabs[1].id)
+        assertEquals(tab3.id, store.state.tabs[2].id)
+
+        assertNull(store.state.tabs[0].parentId)
+        assertEquals(tab0.id, store.state.tabs[1].parentId)
+        assertEquals(tab1.id, store.state.tabs[2].parentId)
+
+        store.dispatch(TabListAction.RemoveTabAction(tab0.id)).joinBlocking()
+
+        // tab1 <- tab3
+        assertEquals(tab1.id, store.state.tabs[0].id)
+        assertEquals(tab3.id, store.state.tabs[1].id)
+
+        assertNull(store.state.tabs[0].parentId)
+        assertEquals(tab1.id, store.state.tabs[1].parentId)
+    }
+
     @Test
     fun `RestoreAction - Adds restored tabs and updates selected tab`() {
         val store = BrowserStore()

components/browser/state/src/test/java/mozilla/components/browser/state/store/BrowserStoreExceptionTest.kt

+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.browser.state.store
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import mozilla.components.browser.state.action.TabListAction
+import mozilla.components.browser.state.state.createTab
+import mozilla.components.support.test.ext.joinBlocking
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.shadows.ShadowLooper
+import java.lang.IllegalStateException
+
+// These tests are in a separate class because they needs to run with
+// Robolectric (different runner, slower) while all other tests only
+// need a Java VM (fast).
+@RunWith(AndroidJUnit4::class)
+class BrowserStoreExceptionTest {
+
+    @Test(expected = java.lang.IllegalArgumentException::class)
+    fun `AddTabAction - Exception is thrown if parent doesn't exist`() {
+        try {
+            val store = BrowserStore()
+            val parent = createTab("https://www.mozilla.org")
+            val child = createTab("https://www.firefox.com", parent = parent)
+
+            store.dispatch(TabListAction.AddTabAction(child)).joinBlocking()
+
+            // Wait for the main looper to process the re-thrown exception.
+            ShadowLooper.idleMainLooper()
+        } catch (e: IllegalStateException) {
+            val cause = e.cause
+            if (cause != null) {
+                throw cause
+            }
+        }
+    }
+}