Merge branch 'hotfix/5.196.1'

duckduckgo · Apr 8, 2024 · dd6fbf0 · dd6fbf0
2 parents d64eece + c1c53ee
commit dd6fbf0
Show file tree

Hide file tree

Showing 17 changed files with 83 additions and 78 deletions.
diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
@@ -454,6 +454,7 @@ class BrowserTabViewModelTest {
 
         whenever(mockDismissedCtaDao.dismissedCtas()).thenReturn(dismissedCtaDaoChannel.consumeAsFlow())
         whenever(mockTabRepository.flowTabs).thenReturn(flowOf(emptyList()))
+        whenever(mockTabRepository.liveTabs).thenReturn(tabsLiveData)
         whenever(mockEmailManager.signedInFlow()).thenReturn(emailStateFlow.asStateFlow())
         whenever(mockSavedSitesRepository.getFavorites()).thenReturn(favoriteListFlow.consumeAsFlow())
         whenever(mockSavedSitesRepository.getBookmarks()).thenReturn(bookmarksListFlow.consumeAsFlow())

diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt
@@ -19,9 +19,9 @@
 package com.duckduckgo.app.tabs.model
 
 import androidx.arch.core.executor.testing.InstantTaskExecutorRule
+import androidx.lifecycle.MutableLiveData
 import androidx.room.Room
 import androidx.test.platform.app.InstrumentationRegistry
-import app.cash.turbine.test
 import com.duckduckgo.app.blockingObserve
 import com.duckduckgo.app.browser.favicon.FaviconManager
 import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister
@@ -216,9 +216,7 @@ class TabDataRepositoryTest {
 
         testee.addDefaultTab()
 
-        testee.flowTabs.test {
-            assertTrue(awaitItem().size == 1)
-        }
+        assertTrue(testee.liveTabs.blockingObserve()?.size == 1)
     }
 
     @Test
@@ -229,9 +227,7 @@ class TabDataRepositoryTest {
 
         testee.addDefaultTab()
 
-        testee.flowTabs.test {
-            assertTrue(awaitItem().size == 1)
-        }
+        assertTrue(testee.liveTabs.blockingObserve()?.size == 1)
     }
 
     @Test
@@ -437,6 +433,8 @@ class TabDataRepositoryTest {
     private fun mockDatabase(): TabsDao {
         whenever(mockDao.flowDeletableTabs())
             .thenReturn(daoDeletableTabs.consumeAsFlow())
+        whenever(mockDao.liveTabs())
+            .thenReturn(MutableLiveData())
 
         return mockDao
     }

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt
@@ -30,8 +30,6 @@ import androidx.activity.OnBackPressedCallback
 import androidx.activity.result.ActivityResult
 import androidx.activity.result.contract.ActivityResultContracts
 import androidx.annotation.VisibleForTesting
-import androidx.lifecycle.Lifecycle.State.STARTED
-import androidx.lifecycle.flowWithLifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.webkit.ServiceWorkerClientCompat
 import androidx.webkit.ServiceWorkerControllerCompat
@@ -79,8 +77,6 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreen.Priv
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Job
-import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import timber.log.Timber
 
@@ -375,17 +371,17 @@ open class BrowserActivity : DuckDuckGoActivity() {
                 selectTab(it)
             }
         }
-        viewModel.tabs.onEach {
+        viewModel.tabs.observe(this) {
             clearStaleTabs(it)
             removeOldTabs()
-            viewModel.onTabsUpdated(it)
-        }.flowWithLifecycle(lifecycle, STARTED)
-            .launchIn(lifecycleScope)
+            lifecycleScope.launch { viewModel.onTabsUpdated(it) }
+        }
     }
 
     private fun removeObservers() {
         viewModel.command.removeObservers(this)
         viewModel.selectedTab.removeObservers(this)
+        viewModel.tabs.removeObservers(this)
     }
 
     private fun clearStaleTabs(updatedTabs: List<TabEntity>?) {

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
@@ -1109,11 +1109,14 @@ class BrowserTabFragment :
     }
 
     private fun addTabsObserver() {
-        viewModel.tabs.flowWithLifecycle(viewLifecycleOwner.lifecycle)
-            .onEach {
-                decorator.renderTabIcon(it)
-            }
-            .launchIn(viewLifecycleOwner.lifecycleScope)
+        viewModel.tabs.observe(
+            viewLifecycleOwner,
+            Observer {
+                it?.let {
+                    decorator.renderTabIcon(it)
+                }
+            },
+        )
 
         viewModel.liveSelectedTab.distinctUntilChanged().observe(
             viewLifecycleOwner,
@@ -1721,6 +1724,7 @@ class BrowserTabFragment :
 
     private fun openInNewBackgroundTab() {
         omnibar.appBarLayout.setExpanded(true, true)
+        viewModel.tabs.removeObservers(this)
         decorator.incrementTabs()
     }
 

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
@@ -273,7 +273,7 @@ class BrowserTabViewModel @Inject constructor(
 
     var skipHome = false
     var hasCtaBeenShownForCurrentPage: AtomicBoolean = AtomicBoolean(false)
-    val tabs: Flow<List<TabEntity>> = tabRepository.flowTabs
+    val tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
     val liveSelectedTab: LiveData<TabEntity> = tabRepository.liveSelectedTab
     val survey: LiveData<Survey> = ctaViewModel.surveyLiveData
     val command: SingleLiveEvent<Command> = SingleLiveEvent()
@@ -1128,11 +1128,9 @@ class BrowserTabViewModel @Inject constructor(
     }
 
     private fun setAdClickActiveTabData(url: String?) {
-        viewModelScope.launch {
-            val sourceTabId = tabRepository.liveSelectedTab.value?.sourceTabId
-            val sourceTabUrl = tabRepository.flowTabs.firstOrNull()?.firstOrNull { it.tabId == sourceTabId }?.url
-            adClickManager.setActiveTabId(tabId, url, sourceTabId, sourceTabUrl)
-        }
+        val sourceTabId = tabRepository.liveSelectedTab.value?.sourceTabId
+        val sourceTabUrl = tabRepository.liveTabs.value?.firstOrNull { it.tabId == sourceTabId }?.url
+        adClickManager.setActiveTabId(tabId, url, sourceTabId, sourceTabUrl)
     }
 
     private fun cacheAppLink(url: String?) {

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt
@@ -52,7 +52,6 @@ import com.duckduckgo.di.scopes.ActivityScope
 import javax.inject.Inject
 import kotlin.coroutines.CoroutineContext
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.launch
 import timber.log.Timber
 
@@ -93,7 +92,7 @@ class BrowserViewModel @Inject constructor(
     private val currentViewState: ViewState
         get() = viewState.value!!
 
-    var tabs: Flow<List<TabEntity>> = tabRepository.flowTabs
+    var tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
     var selectedTab: LiveData<TabEntity> = tabRepository.liveSelectedTab
     val command: SingleLiveEvent<Command> = SingleLiveEvent()
 

diff --git a/app/src/main/java/com/duckduckgo/app/browser/customtabs/IntentDispatcherViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/customtabs/IntentDispatcherViewModel.kt
@@ -47,20 +47,24 @@ class IntentDispatcherViewModel @Inject constructor(
 
     fun onIntentReceived(intent: Intent?, defaultColor: Int) {
         viewModelScope.launch(dispatcherProvider.io()) {
-            val hasSession = intent?.hasExtra(CustomTabsIntent.EXTRA_SESSION) == true
-            val intentText = intent?.intentText
-            val toolbarColor = intent?.getIntExtra(CustomTabsIntent.EXTRA_TOOLBAR_COLOR, defaultColor) ?: defaultColor
+            runCatching {
+                val hasSession = intent?.hasExtra(CustomTabsIntent.EXTRA_SESSION) == true
+                val intentText = intent?.intentText
+                val toolbarColor = intent?.getIntExtra(CustomTabsIntent.EXTRA_TOOLBAR_COLOR, defaultColor) ?: defaultColor
 
-            Timber.d("Intent $intent received. Has extra session=$hasSession. Intent text=$intentText. Toolbar color=$toolbarColor")
+                Timber.d("Intent $intent received. Has extra session=$hasSession. Intent text=$intentText. Toolbar color=$toolbarColor")
 
-            customTabDetector.setCustomTab(false)
-            _viewState.emit(
-                viewState.value.copy(
-                    customTabRequested = hasSession,
-                    intentText = intentText,
-                    toolbarColor = toolbarColor,
-                ),
-            )
+                customTabDetector.setCustomTab(false)
+                _viewState.emit(
+                    viewState.value.copy(
+                        customTabRequested = hasSession,
+                        intentText = intentText,
+                        toolbarColor = toolbarColor,
+                    ),
+                )
+            }.onFailure {
+                Timber.w("Error handling custom tab intent %s", it.message)
+            }
         }
     }
 }
diff --git a/app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt b/app/src/main/java/com/duckduckgo/app/cta/ui/CtaViewModel.kt
@@ -336,7 +336,7 @@ class CtaViewModel @Inject constructor(
         }
     }
 
-    private fun forceStopFireButtonPulseAnimationFlow() = tabRepository.flowTabs
+    private fun forceStopFireButtonPulseAnimationFlow() = tabRepository.flowTabs.distinctUntilChanged()
         .map { tabs ->
             if (tabs.size >= MAX_TABS_OPEN_FIRE_EDUCATION) return@map true
             return@map false

diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt
@@ -19,6 +19,7 @@ package com.duckduckgo.app.tabs.model
 import android.net.Uri
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
+import androidx.lifecycle.distinctUntilChanged
 import com.duckduckgo.app.browser.favicon.FaviconManager
 import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister
 import com.duckduckgo.app.di.AppCoroutineScope
@@ -53,7 +54,9 @@ class TabDataRepository @Inject constructor(
     private val dispatchers: DispatcherProvider,
 ) : TabRepository {
 
-    override val flowTabs: Flow<List<TabEntity>> = tabsDao.flowTabs().distinctUntilChanged()
+    override val liveTabs: LiveData<List<TabEntity>> = tabsDao.liveTabs().distinctUntilChanged()
+
+    override val flowTabs: Flow<List<TabEntity>> = tabsDao.flowTabs()
 
     private val childTabClosedSharedFlow = MutableSharedFlow<String>()
 

diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt
@@ -23,9 +23,6 @@ import android.view.Menu
 import android.view.MenuItem
 import android.widget.TextView
 import androidx.appcompat.widget.Toolbar
-import androidx.lifecycle.Lifecycle.State.STARTED
-import androidx.lifecycle.flowWithLifecycle
-import androidx.lifecycle.lifecycleScope
 import androidx.recyclerview.widget.GridLayoutManager
 import androidx.recyclerview.widget.ItemTouchHelper
 import androidx.recyclerview.widget.RecyclerView
@@ -57,11 +54,7 @@ import javax.inject.Inject
 import kotlin.coroutines.CoroutineContext
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.SupervisorJob
-import kotlinx.coroutines.flow.firstOrNull
-import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.runBlocking
 
 @InjectWith(ActivityScope::class)
 class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, CoroutineScope {
@@ -159,11 +152,9 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
     }
 
     private fun configureObservers() {
-        viewModel.tabs.onEach {
+        viewModel.tabs.observe(this) {
             render(it)
-        }.flowWithLifecycle(lifecycle, STARTED)
-            .launchIn(lifecycleScope)
-
+        }
         viewModel.deletableTabs.observe(this) {
             if (it.isNotEmpty()) {
                 onDeletableTab(it.last())
@@ -214,7 +205,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
 
     override fun onPrepareOptionsMenu(menu: Menu?): Boolean {
         val closeAllTabsMenuItem = menu?.findItem(R.id.closeAllTabs)
-        runBlocking { closeAllTabsMenuItem?.isVisible = viewModel.tabs.firstOrNull()?.isNotEmpty() == true }
+        closeAllTabsMenuItem?.isVisible = viewModel.tabs.value?.isNotEmpty() == true
         return super.onPrepareOptionsMenu(menu)
     }
 
@@ -311,6 +302,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine
     }
 
     private fun clearObserversEarlyToStopViewUpdates() {
+        viewModel.tabs.removeObservers(this)
         viewModel.deletableTabs.removeObservers(this)
     }
 

diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt
@@ -29,8 +29,6 @@ import com.duckduckgo.app.tabs.model.TabRepository
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.ActivityScope
 import javax.inject.Inject
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.firstOrNull
 import kotlinx.coroutines.launch
 
 @ContributesViewModel(ActivityScope::class)
@@ -41,7 +39,7 @@ class TabSwitcherViewModel @Inject constructor(
     private val dispatcherProvider: DispatcherProvider,
 ) : ViewModel() {
 
-    var tabs: Flow<List<TabEntity>> = tabRepository.flowTabs
+    var tabs: LiveData<List<TabEntity>> = tabRepository.liveTabs
     var deletableTabs: LiveData<List<TabEntity>> = tabRepository.flowDeletableTabs.asLiveData(
         context = viewModelScope.coroutineContext,
     )
@@ -87,7 +85,7 @@ class TabSwitcherViewModel @Inject constructor(
 
     fun onCloseAllTabsConfirmed() {
         viewModelScope.launch(dispatcherProvider.io()) {
-            tabs.firstOrNull()?.forEach {
+            tabs.value?.forEach {
                 onTabDeleted(it)
             }
         }

diff --git a/app/src/test/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModelTest.kt
@@ -19,6 +19,7 @@
 package com.duckduckgo.app.tabs.ui
 
 import androidx.arch.core.executor.testing.InstantTaskExecutorRule
+import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.Observer
 import com.duckduckgo.adclick.api.AdClickManager
 import com.duckduckgo.app.browser.session.WebViewSessionStorage
@@ -27,7 +28,6 @@ import com.duckduckgo.app.tabs.model.TabRepository
 import com.duckduckgo.app.tabs.ui.TabSwitcherViewModel.Command
 import com.duckduckgo.common.test.CoroutineTestRule
 import kotlinx.coroutines.channels.Channel
-import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.consumeAsFlow
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runTest
@@ -69,15 +69,15 @@ class TabSwitcherViewModelTest {
     private lateinit var testee: TabSwitcherViewModel
 
     private val repoDeletableTabs = Channel<List<TabEntity>>()
-    private val tabs = MutableStateFlow<List<TabEntity>>(emptyList())
+    private val tabs = MutableLiveData<List<TabEntity>>()
 
     @Before
     fun before() {
         MockitoAnnotations.openMocks(this)
         runBlocking {
             whenever(mockTabRepository.flowDeletableTabs)
                 .thenReturn(repoDeletableTabs.consumeAsFlow())
-            whenever(mockTabRepository.flowTabs)
+            whenever(mockTabRepository.liveTabs)
                 .thenReturn(tabs)
             whenever(mockTabRepository.add()).thenReturn("TAB_ID")
             testee = TabSwitcherViewModel(
@@ -180,12 +180,16 @@ class TabSwitcherViewModelTest {
     @Test
     fun whenOnCloseAllTabsConfirmedThenTabDeletedAndTabIdClearedAndSessionDeleted() = runTest {
         val tab = TabEntity("ID", position = 0)
-        tabs.emit(listOf(tab))
+        tabs.postValue(listOf(tab))
 
         testee.onCloseAllTabsConfirmed()
 
-        verify(mockTabRepository).delete(tab)
-        verify(mockAdClickManager).clearTabId(tab.tabId)
-        verify(mockWebViewSessionStorage).deleteSession(tab.tabId)
+        testee.tabs.observeForever {
+            runBlocking {
+                verify(mockTabRepository).delete(tab)
+            }
+            verify(mockAdClickManager).clearTabId(tab.tabId)
+            verify(mockWebViewSessionStorage).deleteSession(tab.tabId)
+        }
     }
 }
diff --git a/app/version/version.properties b/app/version/version.properties
@@ -1 +1 @@
-VERSION=5.196.0
+VERSION=5.196.1
diff --git a/browser-api/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/browser-api/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt
@@ -27,6 +27,8 @@ interface TabRepository {
     /**
      * @return the tabs that are NOT marked as deletable in the DB
      */
+    val liveTabs: LiveData<List<TabEntity>>
+
     val flowTabs: Flow<List<TabEntity>>
 
     val childClosedTabs: SharedFlow<String>

diff --git a/...ubscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt b/...ubscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt
@@ -68,7 +68,6 @@ import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.onSubscription
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.withContext
 import logcat.LogPriority
 import logcat.logcat
 import retrofit2.HttpException
@@ -478,13 +477,11 @@ class RealSubscriptionsManager @Inject constructor(
 
             logcat(LogPriority.DEBUG) { "Subs: external id is ${authRepository.getAccount()!!.externalId}" }
             _currentPurchaseState.emit(CurrentPurchase.PreFlowFinished)
-            withContext(dispatcherProvider.main()) {
-                playBillingManager.launchBillingFlow(
-                    activity = activity,
-                    planId = planId,
-                    externalId = authRepository.getAccount()!!.externalId,
-                )
-            }
+            playBillingManager.launchBillingFlow(
+                activity = activity,
+                planId = planId,
+                externalId = authRepository.getAccount()!!.externalId,
+            )
         } catch (e: Exception) {
             val error = extractError(e)
             logcat(LogPriority.ERROR) { "Subs: $error" }