Fix: NTP creation crash with tab swiping (#6007)

Task/Issue URL:
https://app.asana.com/1/137249556945/project/1207418217763355/task/1210095537840719?focus=true

### Description

This PR fixes the NTP creation. There was a logical error in the
previous implementation when the wrong boolean was being returned in
`transformWhile()` when requesting a new tab, which in some situation
might result in the flow completion without emitting any value, which
resulted in a crash.

I added code that throws an exception when a new tab is not created
because it's an invalid state in the DB that I'm not sure how we should
recover from. I don't think that's what was happening but in case it
does, we'll know where to start looking.

### Steps to test this PR

I wasn't able to reproduce it, however, I added unit tests that should
verify the fix works as expected.

Author: 0nko

app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt

@@ -18,20 +18,27 @@ package com.duckduckgo.app.browser.tabs
 
 import com.duckduckgo.app.browser.SkipUrlConversionOnNewTabFeature
 import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter
+import com.duckduckgo.app.browser.tabs.TabManager.Companion.NEW_TAB_CREATION_TIMEOUT_LIMIT
 import com.duckduckgo.app.tabs.model.TabEntity
 import com.duckduckgo.app.tabs.model.TabRepository
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.ActivityScope
 import com.squareup.anvil.annotations.ContributesBinding
 import javax.inject.Inject
-import kotlinx.coroutines.flow.first
+import kotlin.time.Duration.Companion.seconds
+import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.TimeoutCancellationException
+import kotlinx.coroutines.flow.catch
+import kotlinx.coroutines.flow.firstOrNull
+import kotlinx.coroutines.flow.timeout
 import kotlinx.coroutines.flow.transformWhile
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 
 interface TabManager {
     companion object {
         const val MAX_ACTIVE_TABS = 20
+        const val NEW_TAB_CREATION_TIMEOUT_LIMIT = 2 // seconds
     }
 
     fun registerCallbacks(onTabsUpdated: (List<String>) -> Unit)
@@ -76,15 +83,27 @@ class DefaultTabManager @Inject constructor(
         }
     }
 
+    @OptIn(FlowPreview::class)
     override suspend fun requestAndWaitForNewTab(): TabEntity = withContext(dispatchers.io()) {
         val tabId = openNewTab()
-        return@withContext tabRepository.flowTabs.transformWhile { result ->
-            result.firstOrNull { it.tabId == tabId }?.let { entity ->
-                emit(entity)
-                return@transformWhile true
+        return@withContext tabRepository.flowTabs
+            .transformWhile { result ->
+                result.firstOrNull { it.tabId == tabId }?.let { entity ->
+                    emit(entity)
+                    return@transformWhile false // stop after finding the tab
+                }
+                return@transformWhile true // continue looking if not found
             }
-            return@transformWhile false
-        }.first()
+            .timeout(NEW_TAB_CREATION_TIMEOUT_LIMIT.seconds)
+            .catch { e ->
+                if (e is TimeoutCancellationException) {
+                    // timeout expired and the new tab was not found
+                    throw IllegalStateException("A new tab failed to be created within $NEW_TAB_CREATION_TIMEOUT_LIMIT second")
+                } else {
+                    throw e
+                }
+            }
+            .firstOrNull() ?: throw IllegalStateException("Tabs flow completed before finding the new tab")
     }
 
     override suspend fun switchToTab(tabId: String) = withContext(dispatchers.io()) {

app/src/test/java/com/duckduckgo/app/browser/tabs/DefaultTabManagerTest.kt

@@ -3,14 +3,18 @@ package com.duckduckgo.app.browser.tabs
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.duckduckgo.app.browser.SkipUrlConversionOnNewTabFeature
 import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter
+import com.duckduckgo.app.browser.tabs.TabManager.Companion.NEW_TAB_CREATION_TIMEOUT_LIMIT
 import com.duckduckgo.app.tabs.model.TabEntity
 import com.duckduckgo.app.tabs.model.TabRepository
 import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
 import com.duckduckgo.feature.toggles.api.Toggle.State
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
+import org.junit.Assert.fail
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -164,4 +168,73 @@ class DefaultTabManagerTest {
         verify(tabRepository).add(url = query, skipHome = false)
         verify(omnibarEntryConverter, never()).convertQueryToUrl(query)
     }
+
+    @Test
+    fun whenRequestAndWaitForNewTabAndTabIsFoundImmediatelyThenReturnTabEntity() = runTest {
+        val tabId = "tabId"
+        val tabEntity = TabEntity(tabId = tabId, position = 0)
+        whenever(tabRepository.flowTabs).thenReturn(flowOf(listOf(tabEntity)))
+        whenever(tabRepository.add()).thenReturn(tabId)
+
+        val result = testee.requestAndWaitForNewTab()
+
+        assertEquals(tabEntity, result)
+    }
+
+    @Test
+    fun whenRequestAndWaitForNewTabAndTabIsFoundAfterDelayThenReturnTabEntity() = runTest {
+        val tabId = "tabId"
+        val tabEntity = TabEntity(tabId = tabId, position = 0)
+
+        val flow = flow {
+            emit(emptyList())
+            delay(500) // Delay less than the timeout
+            emit(listOf(tabEntity))
+        }
+        whenever(tabRepository.flowTabs).thenReturn(flow)
+        whenever(tabRepository.add()).thenReturn(tabId)
+
+        val result = testee.requestAndWaitForNewTab()
+
+        assertEquals(tabEntity, result)
+    }
+
+    @Test
+    fun whenRequestAndWaitForNewTabAndTimeoutOccursThenThrowException() = runTest {
+        val tabId = "tabId"
+
+        val flow = flow {
+            while (true) {
+                emit(emptyList<TabEntity>())
+                delay(300)
+            }
+        }
+        whenever(tabRepository.flowTabs).thenReturn(flow)
+        whenever(tabRepository.add()).thenReturn(tabId)
+
+        try {
+            testee.requestAndWaitForNewTab()
+            fail("Expected IllegalStateException was not thrown")
+        } catch (e: IllegalStateException) {
+            assertEquals("A new tab failed to be created within $NEW_TAB_CREATION_TIMEOUT_LIMIT second", e.message)
+        }
+    }
+
+    @Test
+    fun whenRequestAndWaitForNewTabAndFlowCompletesWithoutFindingTabThenThrowException() = runTest {
+        val tabId = "tabId"
+        val otherTabId = "otherTabId"
+        val otherTabEntity = TabEntity(tabId = otherTabId, position = 0)
+
+        val flow = flowOf(listOf(otherTabEntity))
+        whenever(tabRepository.flowTabs).thenReturn(flow)
+        whenever(tabRepository.add()).thenReturn(tabId)
+
+        try {
+            testee.requestAndWaitForNewTab()
+            fail("Expected IllegalStateException was not thrown")
+        } catch (e: IllegalStateException) {
+            assertEquals("Tabs flow completed before finding the new tab", e.message)
+        }
+    }
 }