Back navigation to previous tab (#897)

* only store sourceTabId when tab is opened after clicking on a link

* Store sourceTabId only when tab opened from current tab

* store sourceTabId when tab opened in background from currentTab

* When back pressed, if sourceTab of current tab is different to null, navigate to that tab

* use room to nullify sourceTabId when tab deleted

* Rename method to be explicit about what sourceTab will be added.
Cleanup a isDefaultTab as it was never used in some parts of the code.

* Passing around sourceTabId from tab fragment. Adds flexibility and it's more explicit about what's happening.

* Removing isDefaultTab param from multiple methods where it was always false

* Add necessary migration to create foreign key in table tabs

Author: cmonfortep

app/schemas/com.duckduckgo.app.global.db.AppDatabase/24.json

@@ -2,7 +2,7 @@
   "formatVersion": 1,
   "database": {
     "version": 24,
-    "identityHash": "03673f6d5937091013955e2520b94c0e",
+    "identityHash": "d6df0e21b463f404e4ea0a430f7d905c",
     "entities": [
       {
         "tableName": "tds_tracker",
@@ -258,7 +258,7 @@
       },
       {
         "tableName": "tabs",
-        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`tabId` TEXT NOT NULL, `url` TEXT, `title` TEXT, `skipHome` INTEGER NOT NULL, `viewed` INTEGER NOT NULL, `position` INTEGER NOT NULL, `tabPreviewFile` TEXT, `sourceTabId` TEXT, PRIMARY KEY(`tabId`))",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`tabId` TEXT NOT NULL, `url` TEXT, `title` TEXT, `skipHome` INTEGER NOT NULL, `viewed` INTEGER NOT NULL, `position` INTEGER NOT NULL, `tabPreviewFile` TEXT, `sourceTabId` TEXT, PRIMARY KEY(`tabId`), FOREIGN KEY(`sourceTabId`) REFERENCES `tabs`(`tabId`) ON UPDATE SET NULL ON DELETE SET NULL )",
         "fields": [
           {
             "fieldPath": "tabId",
@@ -325,7 +325,19 @@
             "createSql": "CREATE INDEX IF NOT EXISTS `index_tabs_tabId` ON `${TABLE_NAME}` (`tabId`)"
           }
         ],
-        "foreignKeys": []
+        "foreignKeys": [
+          {
+            "table": "tabs",
+            "onDelete": "SET NULL",
+            "onUpdate": "SET NULL",
+            "columns": [
+              "sourceTabId"
+            ],
+            "referencedColumns": [
+              "tabId"
+            ]
+          }
+        ]
       },
       {
         "tableName": "tab_selection",
@@ -746,7 +758,7 @@
     "views": [],
     "setupQueries": [
       "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
-      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '03673f6d5937091013955e2520b94c0e')"
+      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'd6df0e21b463f404e4ea0a430f7d905c')"
     ]
   }
 }

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt

@@ -476,7 +476,9 @@ class BrowserTabViewModelTest {
         givenOneActiveTabSelected()
         givenInvalidatedGlobalLayout()
         testee.onUserSubmittedQuery("foo")
-        assertCommandIssued<Command.OpenInNewTab>()
+        assertCommandIssued<Command.OpenInNewTab> {
+            assertNull(sourceTabId)
+        }
     }
 
     @Test
@@ -1093,7 +1095,9 @@ class BrowserTabViewModelTest {
 
         testee.onRefreshRequested()
 
-        assertCommandIssued<Command.OpenInNewTab>()
+        assertCommandIssued<Command.OpenInNewTab>() {
+            assertNull(sourceTabId)
+        }
     }
 
     @Test
@@ -1188,6 +1192,10 @@ class BrowserTabViewModelTest {
         testee.userSelectedItemFromLongPressMenu(longPressTarget, mockMenItem)
         val command = captureCommands().value as Command.OpenInNewTab
         assertEquals("http://example.com", command.query)
+
+        assertCommandIssued<Command.OpenInNewTab> {
+            assertNotNull(sourceTabId)
+        }
     }
 
     @Test
@@ -1319,18 +1327,6 @@ class BrowserTabViewModelTest {
         assertEquals(true, testee.browserViewState.value?.browserShowing)
     }
 
-    @Test
-    fun whenOpenInNewTabThenOpenInNewTabCommandWithCorrectUrlSent() {
-        val url = "https://example.com"
-        testee.openInNewTab(url)
-        verify(mockCommandObserver).onChanged(commandCaptor.capture())
-
-        val command = commandCaptor.lastValue
-        assertTrue(command is Command.OpenInNewTab)
-        command as Command.OpenInNewTab
-        assertEquals(url, command.query)
-    }
-
     @Test
     fun whenRecoveringFromProcessGoneThenShowErrorWithAction() {
         testee.recoverFromRenderProcessGone()
@@ -1348,6 +1344,7 @@ class BrowserTabViewModelTest {
 
         assertCommandIssued<Command.OpenInNewTab> {
             assertEquals("https://example.com", query)
+            assertNull(sourceTabId)
         }
     }
 
@@ -1558,6 +1555,16 @@ class BrowserTabViewModelTest {
         assertTrue(commandCaptor.allValues.contains(Command.ShowKeyboard))
     }
 
+    @Test
+    fun whenUserPressesBackOnATabWithASourceTabThenDeleteCurrentAndSelectSource() = coroutineRule.runBlocking {
+        selectedTabLiveData.value = TabEntity("TAB_ID", "https://example.com", position = 0, sourceTabId = "TAB_ID_SOURCE")
+        setupNavigation(isBrowsing = true)
+
+        testee.onUserPressedBack()
+
+        verify(mockTabsRepository).deleteCurrentTabAndSelectSource()
+    }
+
     @Test
     fun whenScheduledSurveyChangesAndInstalledDaysMatchThenCtaIsSurvey() {
         testee.onSurveyChanged(Survey("abc", "http://example.com", daysInstalled = 1, status = Survey.Status.SCHEDULED))

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt

@@ -123,7 +123,14 @@ class BrowserViewModelTest {
     fun whenNewTabRequestedThenTabAddedToRepository() = runBlocking<Unit> {
         whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData())
         testee.onNewTabRequested()
-        verify(mockTabRepository).addWithSource()
+        verify(mockTabRepository).add()
+    }
+
+    @Test
+    fun whenNewTabRequestedFromSourceTabThenTabAddedToRepositoryWithSourceTabId() = runBlocking<Unit> {
+        whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData())
+        testee.onNewTabRequested("sourceTabId")
+        verify(mockTabRepository).addFromSourceTab(sourceTabId = "sourceTabId")
     }
 
     @Test
@@ -132,13 +139,22 @@ class BrowserViewModelTest {
         whenever(mockOmnibarEntryConverter.convertQueryToUrl(url)).thenReturn(url)
         whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData())
         testee.onOpenInNewTabRequested(url)
-        verify(mockTabRepository).addWithSource(url)
+        verify(mockTabRepository).add(url = url, skipHome = false)
+    }
+
+    @Test
+    fun whenOpenInNewTabRequestedWithSourceTabIdThenTabAddedToRepositoryWithSourceTabId() = runBlocking<Unit> {
+        val url = "http://example.com"
+        whenever(mockOmnibarEntryConverter.convertQueryToUrl(url)).thenReturn(url)
+        whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData())
+        testee.onOpenInNewTabRequested(url, sourceTabId = "tabId")
+        verify(mockTabRepository).addFromSourceTab(url = url, skipHome = false, sourceTabId = "tabId")
     }
 
     @Test
-    fun whenTabsUpdatedAndNoTabsThenNewTabAddedToRepository() = runBlocking<Unit> {
+    fun whenTabsUpdatedAndNoTabsThenDefaultTabAddedToRepository() = runBlocking<Unit> {
         testee.onTabsUpdated(ArrayList())
-        verify(mockTabRepository).add(null, false, true)
+        verify(mockTabRepository).addDefaultTab()
     }
 
     @Test

app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt

@@ -206,7 +206,6 @@ class TabsDaoTest {
 
     @Test
     fun whenTabInsertedAtPositionThenOtherTabsReordered() {
-
         testee.insertTab(TabEntity("TAB_ID1", position = 0))
         testee.insertTab(TabEntity("TAB_ID2", position = 1))
         testee.insertTab(TabEntity("TAB_ID3", position = 2))
@@ -226,7 +225,18 @@ class TabsDaoTest {
 
         assertEquals(3, tabs[3].position)
         assertEquals("TAB_ID3", tabs[3].tabId)
-
     }
 
+    @Test
+    fun whenSourceTabDeletedThenRelatedTabsUpdated() {
+        val firstTab = TabEntity("TAB_ID", "http//updatedexample.com", position = 0)
+        val secondTab = TabEntity("TAB_ID_1", "http//updatedexample.com", position = 1, sourceTabId = "TAB_ID")
+        testee.insertTab(firstTab)
+        testee.insertTab(secondTab)
+
+        testee.deleteTab(firstTab)
+
+        assertNotNull(testee.tab("TAB_ID_1"))
+        assertNull(testee.tab("TAB_ID_1")?.sourceTabId)
+    }
 }