diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt index e3726066df8f..63bffee9b443 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt @@ -18,13 +18,19 @@ 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 @@ -32,6 +38,7 @@ import timber.log.Timber interface TabManager { companion object { const val MAX_ACTIVE_TABS = 20 + const val NEW_TAB_CREATION_TIMEOUT_LIMIT = 1 // seconds } fun registerCallbacks(onTabsUpdated: (List) -> 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 1 second") + } else { + throw e + } + } + .firstOrNull() ?: throw IllegalStateException("Tabs flow completed before finding the new tab") } override suspend fun switchToTab(tabId: String) = withContext(dispatchers.io()) { diff --git a/app/src/test/java/com/duckduckgo/app/browser/tabs/DefaultTabManagerTest.kt b/app/src/test/java/com/duckduckgo/app/browser/tabs/DefaultTabManagerTest.kt index 37dc1406e8c1..da6aefe8343f 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/tabs/DefaultTabManagerTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/tabs/DefaultTabManagerTest.kt @@ -8,9 +8,12 @@ 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 +167,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()) + 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 1 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) + } + } }