From 990c8068e81bf33dad5372a8d5d7dbbda81c3e7f Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Wed, 30 Oct 2024 14:02:34 +0100 Subject: [PATCH 01/47] Add a new swiping tabs feature flag --- .../app/browser/SwipingTabsFeature.kt | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt b/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt new file mode 100644 index 000000000000..cc47d0977e1b --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser + +import com.duckduckgo.anvil.annotations.ContributesRemoteFeature +import com.duckduckgo.di.scopes.AppScope +import com.duckduckgo.feature.toggles.api.Toggle + +@ContributesRemoteFeature( + scope = AppScope::class, + featureName = "swipingTabs", +) +interface SwipingTabsFeature { + @Toggle.DefaultValue(false) + fun self(): Toggle +} From 31521ba071db3bae02d831218be37f1b0f83ba94 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Wed, 30 Oct 2024 14:03:30 +0100 Subject: [PATCH 02/47] Add a class for checking if tab swiping is enabled --- .../app/browser/RealTabSwipingAvailability.kt | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt new file mode 100644 index 000000000000..296845524ccb --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser + +import com.duckduckgo.app.cta.db.DismissedCtaDao +import com.duckduckgo.app.cta.model.CtaId +import com.duckduckgo.di.scopes.AppScope +import com.squareup.anvil.annotations.ContributesBinding +import javax.inject.Inject + +@ContributesBinding(AppScope::class) +class RealTabSwipingAvailability @Inject constructor( + swipingTabsFeature: SwipingTabsFeature, + private val dismissedCtaDao: DismissedCtaDao, +) : TabSwipingAvailability { + private val isOnboardingComplete: Boolean + get() = dismissedCtaDao.exists(CtaId.DAX_END) + + override val isSwipingTabsEnabled: Boolean = swipingTabsFeature.self().isEnabled() && isOnboardingComplete +} From 214b4df4027f326bc279a1c9d580b0b431c7d461 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Wed, 30 Oct 2024 16:59:53 +0100 Subject: [PATCH 03/47] Add an interface for checking if tab swiping is enabled --- .../app/browser/TabSwipingAvailability.kt | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt diff --git a/browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt b/browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt new file mode 100644 index 000000000000..078f4fa74d00 --- /dev/null +++ b/browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser + +interface TabSwipingAvailability { + val isSwipingTabsEnabled: Boolean +} From ce74105e6a4e968a30f7fedeb7b5245afd6f0c7e Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay <0nko@users.noreply.github.com> Date: Sat, 9 Nov 2024 11:12:13 +0100 Subject: [PATCH 04/47] Update app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt Co-authored-by: Noelia Alcala --- .../duckduckgo/app/browser/RealTabSwipingAvailability.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt index 296845524ccb..5003d7776e19 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt @@ -25,10 +25,7 @@ import javax.inject.Inject @ContributesBinding(AppScope::class) class RealTabSwipingAvailability @Inject constructor( swipingTabsFeature: SwipingTabsFeature, - private val dismissedCtaDao: DismissedCtaDao, + private val userStageStore: UserStageStore, ) : TabSwipingAvailability { - private val isOnboardingComplete: Boolean - get() = dismissedCtaDao.exists(CtaId.DAX_END) - - override val isSwipingTabsEnabled: Boolean = swipingTabsFeature.self().isEnabled() && isOnboardingComplete + override val isSwipingTabsEnabled: Boolean = swipingTabsFeature.self().isEnabled() && userStageStore.daxOnboardingActive() } From 25fce10fd4dc4dc44985ed157617a7ce72023fbc Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Sat, 9 Nov 2024 14:39:50 +0100 Subject: [PATCH 05/47] Optimize imports --- .../com/duckduckgo/app/browser/RealTabSwipingAvailability.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt index 5003d7776e19..2c701a9d108e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt @@ -16,8 +16,6 @@ package com.duckduckgo.app.browser -import com.duckduckgo.app.cta.db.DismissedCtaDao -import com.duckduckgo.app.cta.model.CtaId import com.duckduckgo.di.scopes.AppScope import com.squareup.anvil.annotations.ContributesBinding import javax.inject.Inject From 273bbb2079e290295137bfe4ac45f4d50a0de71c Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 10:27:14 +0100 Subject: [PATCH 06/47] Remove the extra class since onboarding check needs to happen reactively --- .../app/browser/RealTabSwipingAvailability.kt | 29 ------------------- .../app/browser/TabSwipingAvailability.kt | 21 -------------- 2 files changed, 50 deletions(-) delete mode 100644 app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt delete mode 100644 browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt b/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt deleted file mode 100644 index 2c701a9d108e..000000000000 --- a/app/src/main/java/com/duckduckgo/app/browser/RealTabSwipingAvailability.kt +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (c) 2024 DuckDuckGo - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.duckduckgo.app.browser - -import com.duckduckgo.di.scopes.AppScope -import com.squareup.anvil.annotations.ContributesBinding -import javax.inject.Inject - -@ContributesBinding(AppScope::class) -class RealTabSwipingAvailability @Inject constructor( - swipingTabsFeature: SwipingTabsFeature, - private val userStageStore: UserStageStore, -) : TabSwipingAvailability { - override val isSwipingTabsEnabled: Boolean = swipingTabsFeature.self().isEnabled() && userStageStore.daxOnboardingActive() -} diff --git a/browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt b/browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt deleted file mode 100644 index 078f4fa74d00..000000000000 --- a/browser-api/src/main/java/com/duckduckgo/app/browser/TabSwipingAvailability.kt +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright (c) 2024 DuckDuckGo - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.duckduckgo.app.browser - -interface TabSwipingAvailability { - val isSwipingTabsEnabled: Boolean -} From df8b2f6b56ea312e3487c0676bfbafcbe3739dff Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Sat, 9 Nov 2024 14:47:25 +0100 Subject: [PATCH 07/47] Move the existing tab management functionality to a separate class --- .../duckduckgo/app/browser/BrowserActivity.kt | 139 +++----------- .../duckduckgo/app/browser/tabs/TabManager.kt | 175 ++++++++++++++++++ 2 files changed, 197 insertions(+), 117 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index f9a1c5199cbf..8a292e2a86f1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -43,6 +43,7 @@ import com.duckduckgo.app.browser.databinding.IncludeOmnibarToolbarMockupBinding import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP import com.duckduckgo.app.browser.shortcut.ShortcutBuilder +import com.duckduckgo.app.browser.tabs.TabManager import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.downloads.DownloadsScreens.DownloadsScreenNoParams import com.duckduckgo.app.feedback.ui.common.FeedbackActivity @@ -79,7 +80,6 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParam import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA import javax.inject.Inject import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job import kotlinx.coroutines.launch import timber.log.Timber @@ -129,9 +129,22 @@ open class BrowserActivity : DuckDuckGoActivity() { @Inject lateinit var appBuildConfig: AppBuildConfig - private val lastActiveTabs = TabList() + @Inject + lateinit var swipingTabsFeature: SwipingTabsFeature + + private val tabManager by lazy { + TabManager( + activity = this, + swipingTabsFeature = swipingTabsFeature, + onNewTabRequested = viewModel::onNewTabRequested, + ) + } - private var currentTab: BrowserTabFragment? = null + private var currentTab: BrowserTabFragment? + get() = tabManager.currentTab + set(value) { + tabManager.currentTab = value + } private val viewModel: BrowserViewModel by bindViewModel() @@ -145,8 +158,6 @@ open class BrowserActivity : DuckDuckGoActivity() { private lateinit var toolbarMockupBinding: IncludeOmnibarToolbarMockupBinding - private var openMessageInNewTabJob: Job? = null - @VisibleForTesting var destroyedByBackPress: Boolean = false @@ -196,7 +207,7 @@ open class BrowserActivity : DuckDuckGoActivity() { } override fun onStop() { - openMessageInNewTabJob?.cancel() + tabManager.cancelOpenMessageInNewTab() super.onStop() } @@ -232,69 +243,6 @@ open class BrowserActivity : DuckDuckGoActivity() { } } - private fun openNewTab( - tabId: String, - url: String? = null, - skipHome: Boolean, - isExternal: Boolean, - ): BrowserTabFragment { - Timber.i("Opening new tab, url: $url, tabId: $tabId") - val fragment = BrowserTabFragment.newInstance(tabId, url, skipHome, isExternal) - addOrReplaceNewTab(fragment, tabId) - currentTab = fragment - return fragment - } - - private fun addOrReplaceNewTab( - fragment: BrowserTabFragment, - tabId: String, - ) { - if (supportFragmentManager.isStateSaved) { - return - } - val transaction = supportFragmentManager.beginTransaction() - val tab = currentTab - if (tab == null) { - transaction.replace(R.id.fragmentContainer, fragment, tabId) - } else { - transaction.hide(tab) - transaction.add(R.id.fragmentContainer, fragment, tabId) - } - transaction.commit() - } - - private fun selectTab(tab: TabEntity?) { - Timber.v("Select tab: $tab") - - if (tab == null) return - - if (tab.tabId == currentTab?.tabId) return - - lastActiveTabs.add(tab.tabId) - - val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment - if (fragment == null) { - openNewTab(tab.tabId, tab.url, tab.skipHome, intent?.getBooleanExtra(LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false) - return - } - val transaction = supportFragmentManager.beginTransaction() - currentTab?.let { - transaction.hide(it) - } - transaction.show(fragment) - transaction.commit() - currentTab = fragment - } - - private fun removeTabs(fragments: List) { - val transaction = supportFragmentManager.beginTransaction() - fragments.forEach { - transaction.remove(it) - lastActiveTabs.remove(it.tabId) - } - transaction.commit() - } - override fun onKeyLongPress(keyCode: Int, event: KeyEvent?): Boolean { return if (keyCode == KeyEvent.KEYCODE_BACK) { currentTab?.onLongPressBackButton() @@ -403,12 +351,12 @@ open class BrowserActivity : DuckDuckGoActivity() { } viewModel.selectedTab.observe(this) { if (it != null) { - selectTab(it) + tabManager.selectTab(it) } } viewModel.tabs.observe(this) { - clearStaleTabs(it) - removeOldTabs() + tabManager.clearStaleTabs(it) + tabManager.removeOldTabs() lifecycleScope.launch { viewModel.onTabsUpdated(it) } } } @@ -419,33 +367,6 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.tabs.removeObservers(this) } - private fun clearStaleTabs(updatedTabs: List?) { - if (updatedTabs == null) { - return - } - - val stale = supportFragmentManager - .fragments.mapNotNull { it as? BrowserTabFragment } - .filter { fragment -> updatedTabs.none { it.tabId == fragment.tabId } } - - if (stale.isNotEmpty()) { - removeTabs(stale) - } - } - - private fun removeOldTabs() { - val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) - if (candidatesToRemove.isEmpty()) return - - val tabsToRemove = supportFragmentManager.fragments - .mapNotNull { it as? BrowserTabFragment } - .filter { candidatesToRemove.contains(it.tabId) } - - if (tabsToRemove.isNotEmpty()) { - removeTabs(tabsToRemove) - } - } - private fun processCommand(command: Command) { Timber.i("Processing command: $command") when (command) { @@ -512,11 +433,7 @@ open class BrowserActivity : DuckDuckGoActivity() { message: Message, sourceTabId: String?, ) { - openMessageInNewTabJob = lifecycleScope.launch { - val tabId = viewModel.onNewTabRequested(sourceTabId = sourceTabId) - val fragment = openNewTab(tabId, null, false, intent?.getBooleanExtra(LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false) - fragment.messageFromPreviousTab = message - } + tabManager.openMessageInNewTab(message, sourceTabId) } fun openExistingTab(tabId: String) { @@ -613,10 +530,8 @@ open class BrowserActivity : DuckDuckGoActivity() { private const val LAUNCH_FROM_INTERSTITIAL_EXTRA = "INTERSTITIAL_SCREEN_EXTRA" const val OPEN_EXISTING_TAB_ID_EXTRA = "OPEN_EXISTING_TAB_ID_EXTRA" - private const val LAUNCH_FROM_EXTERNAL_EXTRA = "LAUNCH_FROM_EXTERNAL_EXTRA" + const val LAUNCH_FROM_EXTERNAL_EXTRA = "LAUNCH_FROM_EXTERNAL_EXTRA" private const val LAUNCH_FROM_CLEAR_DATA_ACTION = "LAUNCH_FROM_CLEAR_DATA_ACTION" - - private const val MAX_ACTIVE_TABS = 40 } inner class BrowserStateRenderer { @@ -760,13 +675,3 @@ open class BrowserActivity : DuckDuckGoActivity() { val newInstanceState: Bundle?, ) } - -// Temporary class to keep track of latest visited tabs, keeping unique ids. -private class TabList() : ArrayList() { - override fun add(element: String): Boolean { - if (this.contains(element)) { - this.remove(element) - } - return super.add(element) - } -} diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt new file mode 100644 index 000000000000..9aedde9a20ad --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -0,0 +1,175 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.tabs + +import android.os.Message +import androidx.lifecycle.lifecycleScope +import com.duckduckgo.app.browser.BrowserActivity +import com.duckduckgo.app.browser.BrowserTabFragment +import com.duckduckgo.app.browser.R +import com.duckduckgo.app.browser.SwipingTabsFeature +import com.duckduckgo.app.tabs.model.TabEntity +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import timber.log.Timber + +class TabManager( + private val activity: BrowserActivity, + private val swipingTabsFeature: SwipingTabsFeature, + private val onNewTabRequested: suspend (sourceTabId: String?) -> String, +) { + companion object { + private const val MAX_ACTIVE_TABS = 40 + } + + private val lastActiveTabs = TabList() + private val supportFragmentManager = activity.supportFragmentManager + private var openMessageInNewTabJob: Job? = null + + private var _currentTab: BrowserTabFragment? = null + var currentTab: BrowserTabFragment? + get() { + return if (swipingTabsFeature.self().isEnabled()) { + null + } else { + _currentTab + } + } + set(value) { + _currentTab = value + } + + fun selectTab(tab: TabEntity?) { + Timber.v("Select tab: $tab") + + if (tab == null) return + + if (tab.tabId == currentTab?.tabId) return + + lastActiveTabs.add(tab.tabId) + + val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment + if (fragment == null) { + openNewTab(tab.tabId, tab.url, tab.skipHome, activity.intent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false) + return + } + val transaction = supportFragmentManager.beginTransaction() + currentTab?.let { + transaction.hide(it) + } + transaction.show(fragment) + transaction.commit() + currentTab = fragment + } + + private fun openNewTab( + tabId: String, + url: String? = null, + skipHome: Boolean, + isExternal: Boolean, + ): BrowserTabFragment { + Timber.i("Opening new tab, url: $url, tabId: $tabId") + val fragment = BrowserTabFragment.newInstance(tabId, url, skipHome, isExternal) + addOrReplaceNewTab(fragment, tabId) + currentTab = fragment + return fragment + } + + private fun addOrReplaceNewTab( + fragment: BrowserTabFragment, + tabId: String, + ) { + if (supportFragmentManager.isStateSaved) { + return + } + val transaction = supportFragmentManager.beginTransaction() + val tab = currentTab + if (tab == null) { + transaction.replace(R.id.fragmentContainer, fragment, tabId) + } else { + transaction.hide(tab) + transaction.add(R.id.fragmentContainer, fragment, tabId) + } + transaction.commit() + } + + fun openMessageInNewTab( + message: Message, + sourceTabId: String?, + ) { + openMessageInNewTabJob = activity.lifecycleScope.launch { + val tabId = onNewTabRequested(sourceTabId) + val fragment = openNewTab( + tabId = tabId, + url = null, + skipHome = false, + isExternal = activity.intent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false, + ) + fragment.messageFromPreviousTab = message + } + } + + fun removeOldTabs() { + val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) + if (candidatesToRemove.isEmpty()) return + + val tabsToRemove = supportFragmentManager.fragments + .mapNotNull { it as? BrowserTabFragment } + .filter { candidatesToRemove.contains(it.tabId) } + + if (tabsToRemove.isNotEmpty()) { + removeTabs(tabsToRemove) + } + } + + fun clearStaleTabs(updatedTabs: List?) { + if (updatedTabs == null) { + return + } + + val stale = supportFragmentManager + .fragments.mapNotNull { it as? BrowserTabFragment } + .filter { fragment -> updatedTabs.none { it.tabId == fragment.tabId } } + + if (stale.isNotEmpty()) { + removeTabs(stale) + } + } + + private fun removeTabs(fragments: List) { + val transaction = supportFragmentManager.beginTransaction() + fragments.forEach { + transaction.remove(it) + lastActiveTabs.remove(it.tabId) + } + transaction.commit() + } + + fun cancelOpenMessageInNewTab() { + openMessageInNewTabJob?.cancel() + } + + // Temporary class to keep track of latest visited tabs, keeping unique ids. + private class TabList : ArrayList() { + override fun add(element: String): Boolean { + if (this.contains(element)) { + this.remove(element) + } + return super.add(element) + } + } +} From 3579d31693031ef4693844e905e56e826d69d58b Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 10:15:12 +0100 Subject: [PATCH 08/47] Move the tab-related calls to TabManager --- .../duckduckgo/app/browser/BrowserActivity.kt | 24 +++++--- .../duckduckgo/app/browser/tabs/TabManager.kt | 61 +++++++++++++------ 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 8a292e2a86f1..2f6de5eb50b4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -31,6 +31,7 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting +import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat @@ -56,6 +57,7 @@ import com.duckduckgo.app.global.rating.PromptCount import com.duckduckgo.app.global.view.ClearDataAction import com.duckduckgo.app.global.view.FireDialog import com.duckduckgo.app.global.view.renderIfChanged +import com.duckduckgo.app.onboarding.store.UserStageStore import com.duckduckgo.app.onboarding.ui.page.DefaultBrowserPage import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.AppPixelName.FIRE_DIALOG_CANCEL @@ -63,7 +65,6 @@ import com.duckduckgo.app.settings.SettingsActivity import com.duckduckgo.app.settings.db.SettingsDataStore import com.duckduckgo.app.sitepermissions.SitePermissionsActivity import com.duckduckgo.app.statistics.pixels.Pixel -import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.appbuildconfig.api.AppBuildConfig import com.duckduckgo.autofill.api.emailprotection.EmailProtectionLinkVerifier import com.duckduckgo.browser.api.ui.BrowserScreens.BookmarksScreenNoParams @@ -80,6 +81,7 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParam import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA import javax.inject.Inject import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber @@ -132,6 +134,9 @@ open class BrowserActivity : DuckDuckGoActivity() { @Inject lateinit var swipingTabsFeature: SwipingTabsFeature + @Inject + lateinit var userStageStore: UserStageStore + private val tabManager by lazy { TabManager( activity = this, @@ -350,14 +355,19 @@ open class BrowserActivity : DuckDuckGoActivity() { processCommand(it) } viewModel.selectedTab.observe(this) { - if (it != null) { - tabManager.selectTab(it) - } + tabManager.onSelectedTabChanged(it) } viewModel.tabs.observe(this) { - tabManager.clearStaleTabs(it) - tabManager.removeOldTabs() - lifecycleScope.launch { viewModel.onTabsUpdated(it) } + tabManager.onTabsUpdated(it) + } + + lifecycleScope.launch { + viewModel.isOnboardingCompleted.flowWithLifecycle(lifecycle).collectLatest { isOnboardingCompleted -> + if (isOnboardingCompleted) { + userStageStore.completeStage(UserStageStore.Stage.DAX_ONBOARDING) + } else { + } + } } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index 9aedde9a20ad..eeab1747d9ae 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -53,7 +53,31 @@ class TabManager( _currentTab = value } - fun selectTab(tab: TabEntity?) { + fun onSelectedTabChanged(tab: TabEntity?) { + if (swipingTabsFeature.self().isEnabled()) { + return + } else if (tab != null) { + selectTab(tab) + } + } + + fun onTabsUpdated(updatedTabs: List) { + if (swipingTabsFeature.self().isEnabled()) { + return + } else { + clearStaleTabs(updatedTabs) + } + } + + fun cancelOpenMessageInNewTab() { + openMessageInNewTabJob?.cancel() + } + + private fun selectTab(tab: TabEntity?) { + if (swipingTabsFeature.self().isEnabled()) { + return + } + Timber.v("Select tab: $tab") if (tab == null) return @@ -107,6 +131,7 @@ class TabManager( transaction.commit() } + // TODO: Handle the FF case fun openMessageInNewTab( message: Message, sourceTabId: String?, @@ -123,20 +148,11 @@ class TabManager( } } - fun removeOldTabs() { - val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) - if (candidatesToRemove.isEmpty()) return - - val tabsToRemove = supportFragmentManager.fragments - .mapNotNull { it as? BrowserTabFragment } - .filter { candidatesToRemove.contains(it.tabId) } - - if (tabsToRemove.isNotEmpty()) { - removeTabs(tabsToRemove) + private fun clearStaleTabs(updatedTabs: List?) { + if (swipingTabsFeature.self().isEnabled()) { + return } - } - fun clearStaleTabs(updatedTabs: List?) { if (updatedTabs == null) { return } @@ -148,6 +164,21 @@ class TabManager( if (stale.isNotEmpty()) { removeTabs(stale) } + + removeOldTabs() + } + + private fun removeOldTabs() { + val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) + if (candidatesToRemove.isEmpty()) return + + val tabsToRemove = supportFragmentManager.fragments + .mapNotNull { it as? BrowserTabFragment } + .filter { candidatesToRemove.contains(it.tabId) } + + if (tabsToRemove.isNotEmpty()) { + removeTabs(tabsToRemove) + } } private fun removeTabs(fragments: List) { @@ -159,10 +190,6 @@ class TabManager( transaction.commit() } - fun cancelOpenMessageInNewTab() { - openMessageInNewTabJob?.cancel() - } - // Temporary class to keep track of latest visited tabs, keeping unique ids. private class TabList : ArrayList() { override fun add(element: String): Boolean { From eb1c6071c1ab0b36696e5704919618ebe6b49195 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 10:50:38 +0100 Subject: [PATCH 09/47] Remove unused code --- .../java/com/duckduckgo/app/browser/BrowserActivity.kt | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 2f6de5eb50b4..4eb4b750b4bf 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -360,15 +360,6 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.tabs.observe(this) { tabManager.onTabsUpdated(it) } - - lifecycleScope.launch { - viewModel.isOnboardingCompleted.flowWithLifecycle(lifecycle).collectLatest { isOnboardingCompleted -> - if (isOnboardingCompleted) { - userStageStore.completeStage(UserStageStore.Stage.DAX_ONBOARDING) - } else { - } - } - } } private fun removeObservers() { From b29d7611aaf5dab5f1fec4f12f43b661add89b63 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 10:59:42 +0100 Subject: [PATCH 10/47] Clean up code --- .../java/com/duckduckgo/app/browser/BrowserActivity.kt | 10 +++------- .../java/com/duckduckgo/app/browser/tabs/TabManager.kt | 1 - 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 4eb4b750b4bf..203f723412b1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -31,7 +31,6 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting -import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat @@ -51,13 +50,14 @@ import com.duckduckgo.app.feedback.ui.common.FeedbackActivity import com.duckduckgo.app.fire.DataClearer import com.duckduckgo.app.fire.DataClearerForegroundAppRestartPixel import com.duckduckgo.app.firebutton.FireButtonStore -import com.duckduckgo.app.global.* +import com.duckduckgo.app.global.ApplicationClearDataState import com.duckduckgo.app.global.events.db.UserEventsStore +import com.duckduckgo.app.global.intentText import com.duckduckgo.app.global.rating.PromptCount +import com.duckduckgo.app.global.sanitize import com.duckduckgo.app.global.view.ClearDataAction import com.duckduckgo.app.global.view.FireDialog import com.duckduckgo.app.global.view.renderIfChanged -import com.duckduckgo.app.onboarding.store.UserStageStore import com.duckduckgo.app.onboarding.ui.page.DefaultBrowserPage import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.AppPixelName.FIRE_DIALOG_CANCEL @@ -81,7 +81,6 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParam import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA import javax.inject.Inject import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber @@ -134,9 +133,6 @@ open class BrowserActivity : DuckDuckGoActivity() { @Inject lateinit var swipingTabsFeature: SwipingTabsFeature - @Inject - lateinit var userStageStore: UserStageStore - private val tabManager by lazy { TabManager( activity = this, diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index eeab1747d9ae..b8ab33a49f8e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -131,7 +131,6 @@ class TabManager( transaction.commit() } - // TODO: Handle the FF case fun openMessageInNewTab( message: Message, sourceTabId: String?, From d9f13c4b469bfb6345955e761048a07126b3c7a6 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 15:10:45 +0100 Subject: [PATCH 11/47] Add onCleanup function --- .../duckduckgo/app/browser/BrowserActivity.kt | 2 +- .../duckduckgo/app/browser/tabs/TabManager.kt | 34 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 203f723412b1..022917c34370 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -208,7 +208,7 @@ open class BrowserActivity : DuckDuckGoActivity() { } override fun onStop() { - tabManager.cancelOpenMessageInNewTab() + tabManager.onCleanup() super.onStop() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index b8ab33a49f8e..9e61d4a7c36d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -69,7 +69,23 @@ class TabManager( } } - fun cancelOpenMessageInNewTab() { + fun openMessageInNewTab( + message: Message, + sourceTabId: String?, + ) { + openMessageInNewTabJob = activity.lifecycleScope.launch { + val tabId = onNewTabRequested(sourceTabId) + val fragment = openNewTab( + tabId = tabId, + url = null, + skipHome = false, + isExternal = activity.intent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false, + ) + fragment.messageFromPreviousTab = message + } + } + + fun onCleanup() { openMessageInNewTabJob?.cancel() } @@ -131,22 +147,6 @@ class TabManager( transaction.commit() } - fun openMessageInNewTab( - message: Message, - sourceTabId: String?, - ) { - openMessageInNewTabJob = activity.lifecycleScope.launch { - val tabId = onNewTabRequested(sourceTabId) - val fragment = openNewTab( - tabId = tabId, - url = null, - skipHome = false, - isExternal = activity.intent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false, - ) - fragment.messageFromPreviousTab = message - } - } - private fun clearStaleTabs(updatedTabs: List?) { if (swipingTabsFeature.self().isEnabled()) { return From a6b8d7e738dd757d97e6bc51dee6907bae3b4ada Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 16:01:20 +0100 Subject: [PATCH 12/47] Extract a TabManager interface --- .../app/browser/tabs/RealTabManager.kt | 241 ++++++++++++++++++ .../duckduckgo/app/browser/tabs/TabManager.kt | 182 +------------ 2 files changed, 252 insertions(+), 171 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt new file mode 100644 index 000000000000..e3a724b924b9 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt @@ -0,0 +1,241 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.tabs + +import android.os.Message +import androidx.lifecycle.lifecycleScope +import com.duckduckgo.app.browser.BrowserActivity +import com.duckduckgo.app.browser.BrowserTabFragment +import com.duckduckgo.app.browser.R +import com.duckduckgo.app.browser.SwipingTabsFeature +import com.duckduckgo.app.tabs.model.TabEntity +import com.duckduckgo.di.scopes.ActivityScope +import com.squareup.anvil.annotations.ContributesBinding +import dagger.SingleInstanceIn +import dagger.android.DaggerActivity +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import timber.log.Timber +import javax.inject.Inject + +@ContributesBinding(ActivityScope::class) +@SingleInstanceIn(ActivityScope::class) +class RealTabManager @Inject constructor( + activity: DaggerActivity, + private val swipingTabsFeature: SwipingTabsFeature, +) : TabManager { + companion object { + private const val MAX_ACTIVE_TABS = 40 + } + + private val browserActivity = activity as BrowserActivity + private val lastActiveTabs = TabList() + private val supportFragmentManager = activity.supportFragmentManager + private var openMessageInNewTabJob: Job? = null + + private var _currentTab: BrowserTabFragment? = null + override var currentTab: BrowserTabFragment? + get() { + return if (swipingTabsFeature.self().isEnabled()) { + null + } else { + _currentTab + } + } + set(value) { + _currentTab = value + } + + override fun onSelectedTabChanged(tab: TabEntity?) { + if (swipingTabsFeature.self().isEnabled()) { + return + } else if (tab != null) { + selectTab(tab) + } + } + + override fun onTabsUpdated(updatedTabs: List) { + if (swipingTabsFeature.self().isEnabled()) { + return + } else { + clearStaleTabs(updatedTabs) + } + } + + override fun openMessageInNewTab( + message: Message, + sourceTabId: String?, + ) { + openMessageInNewTabJob = browserActivity.lifecycleScope.launch { + val tabId = browserActivity.viewModel.onNewTabRequested(sourceTabId) + val fragment = openNewTab( + tabId = tabId, + url = null, + skipHome = false, + isExternal = browserActivity.intent?.getBooleanExtra( + BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, + false, + ) ?: false, + ) + fragment.messageFromPreviousTab = message + } + } + + override fun openExistingTab(tabId: String) { + browserActivity.lifecycleScope.launch { + browserActivity.viewModel.onTabSelected(tabId) + } + } + + override fun launchNewTab() { + browserActivity.lifecycleScope.launch { browserActivity.viewModel.onNewTabRequested() } + } + + override fun openInNewTab( + query: String, + sourceTabId: String?, + ) { + browserActivity.lifecycleScope.launch { + browserActivity.viewModel.onOpenInNewTabRequested( + query = query, + sourceTabId = sourceTabId, + ) + } + } + + override fun onCleanup() { + openMessageInNewTabJob?.cancel() + } + + private fun selectTab(tab: TabEntity?) { + if (swipingTabsFeature.self().isEnabled()) { + return + } + + Timber.v("Select tab: $tab") + + if (tab == null) return + + if (tab.tabId == currentTab?.tabId) return + + lastActiveTabs.add(tab.tabId) + + val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment + if (fragment == null) { + openNewTab( + tabId = tab.tabId, + url = tab.url, + skipHome = tab.skipHome, + isExternal = browserActivity.intent?.getBooleanExtra( + BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, + false, + ) ?: false, + ) + return + } + val transaction = supportFragmentManager.beginTransaction() + currentTab?.let { + transaction.hide(it) + } + transaction.show(fragment) + transaction.commit() + currentTab = fragment + } + + private fun openNewTab( + tabId: String, + url: String? = null, + skipHome: Boolean, + isExternal: Boolean, + ): BrowserTabFragment { + Timber.i("Opening new tab, url: $url, tabId: $tabId") + val fragment = BrowserTabFragment.newInstance(tabId, url, skipHome, isExternal) + addOrReplaceNewTab(fragment, tabId) + currentTab = fragment + return fragment + } + + private fun addOrReplaceNewTab( + fragment: BrowserTabFragment, + tabId: String, + ) { + if (supportFragmentManager.isStateSaved) { + return + } + val transaction = supportFragmentManager.beginTransaction() + val tab = currentTab + if (tab == null) { + transaction.replace(R.id.fragmentContainer, fragment, tabId) + } else { + transaction.hide(tab) + transaction.add(R.id.fragmentContainer, fragment, tabId) + } + transaction.commit() + } + + private fun clearStaleTabs(updatedTabs: List?) { + if (swipingTabsFeature.self().isEnabled()) { + return + } + + if (updatedTabs == null) { + return + } + + val stale = supportFragmentManager + .fragments.mapNotNull { it as? BrowserTabFragment } + .filter { fragment -> updatedTabs.none { it.tabId == fragment.tabId } } + + if (stale.isNotEmpty()) { + removeTabs(stale) + } + + removeOldTabs() + } + + private fun removeOldTabs() { + val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) + if (candidatesToRemove.isEmpty()) return + + val tabsToRemove = supportFragmentManager.fragments + .mapNotNull { it as? BrowserTabFragment } + .filter { candidatesToRemove.contains(it.tabId) } + + if (tabsToRemove.isNotEmpty()) { + removeTabs(tabsToRemove) + } + } + + private fun removeTabs(fragments: List) { + val transaction = supportFragmentManager.beginTransaction() + fragments.forEach { + transaction.remove(it) + lastActiveTabs.remove(it.tabId) + } + transaction.commit() + } + + // Temporary class to keep track of latest visited tabs, keeping unique ids. + private class TabList : ArrayList() { + override fun add(element: String): Boolean { + if (this.contains(element)) { + this.remove(element) + } + return super.add(element) + } + } +} diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index 9e61d4a7c36d..4d5777fe8f15 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -17,185 +17,25 @@ package com.duckduckgo.app.browser.tabs import android.os.Message -import androidx.lifecycle.lifecycleScope -import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment -import com.duckduckgo.app.browser.R -import com.duckduckgo.app.browser.SwipingTabsFeature import com.duckduckgo.app.tabs.model.TabEntity -import kotlinx.coroutines.Job -import kotlinx.coroutines.launch -import timber.log.Timber -class TabManager( - private val activity: BrowserActivity, - private val swipingTabsFeature: SwipingTabsFeature, - private val onNewTabRequested: suspend (sourceTabId: String?) -> String, -) { - companion object { - private const val MAX_ACTIVE_TABS = 40 - } - - private val lastActiveTabs = TabList() - private val supportFragmentManager = activity.supportFragmentManager - private var openMessageInNewTabJob: Job? = null - - private var _currentTab: BrowserTabFragment? = null +interface TabManager { var currentTab: BrowserTabFragment? - get() { - return if (swipingTabsFeature.self().isEnabled()) { - null - } else { - _currentTab - } - } - set(value) { - _currentTab = value - } - - fun onSelectedTabChanged(tab: TabEntity?) { - if (swipingTabsFeature.self().isEnabled()) { - return - } else if (tab != null) { - selectTab(tab) - } - } - - fun onTabsUpdated(updatedTabs: List) { - if (swipingTabsFeature.self().isEnabled()) { - return - } else { - clearStaleTabs(updatedTabs) - } - } + fun onSelectedTabChanged(tab: TabEntity?) + fun onTabsUpdated(updatedTabs: List) fun openMessageInNewTab( message: Message, sourceTabId: String?, - ) { - openMessageInNewTabJob = activity.lifecycleScope.launch { - val tabId = onNewTabRequested(sourceTabId) - val fragment = openNewTab( - tabId = tabId, - url = null, - skipHome = false, - isExternal = activity.intent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false, - ) - fragment.messageFromPreviousTab = message - } - } - - fun onCleanup() { - openMessageInNewTabJob?.cancel() - } - - private fun selectTab(tab: TabEntity?) { - if (swipingTabsFeature.self().isEnabled()) { - return - } - - Timber.v("Select tab: $tab") - - if (tab == null) return - - if (tab.tabId == currentTab?.tabId) return - - lastActiveTabs.add(tab.tabId) - - val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment - if (fragment == null) { - openNewTab(tab.tabId, tab.url, tab.skipHome, activity.intent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false) - return - } - val transaction = supportFragmentManager.beginTransaction() - currentTab?.let { - transaction.hide(it) - } - transaction.show(fragment) - transaction.commit() - currentTab = fragment - } + ) - private fun openNewTab( - tabId: String, - url: String? = null, - skipHome: Boolean, - isExternal: Boolean, - ): BrowserTabFragment { - Timber.i("Opening new tab, url: $url, tabId: $tabId") - val fragment = BrowserTabFragment.newInstance(tabId, url, skipHome, isExternal) - addOrReplaceNewTab(fragment, tabId) - currentTab = fragment - return fragment - } - - private fun addOrReplaceNewTab( - fragment: BrowserTabFragment, - tabId: String, - ) { - if (supportFragmentManager.isStateSaved) { - return - } - val transaction = supportFragmentManager.beginTransaction() - val tab = currentTab - if (tab == null) { - transaction.replace(R.id.fragmentContainer, fragment, tabId) - } else { - transaction.hide(tab) - transaction.add(R.id.fragmentContainer, fragment, tabId) - } - transaction.commit() - } - - private fun clearStaleTabs(updatedTabs: List?) { - if (swipingTabsFeature.self().isEnabled()) { - return - } - - if (updatedTabs == null) { - return - } - - val stale = supportFragmentManager - .fragments.mapNotNull { it as? BrowserTabFragment } - .filter { fragment -> updatedTabs.none { it.tabId == fragment.tabId } } - - if (stale.isNotEmpty()) { - removeTabs(stale) - } - - removeOldTabs() - } - - private fun removeOldTabs() { - val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) - if (candidatesToRemove.isEmpty()) return - - val tabsToRemove = supportFragmentManager.fragments - .mapNotNull { it as? BrowserTabFragment } - .filter { candidatesToRemove.contains(it.tabId) } - - if (tabsToRemove.isNotEmpty()) { - removeTabs(tabsToRemove) - } - } - - private fun removeTabs(fragments: List) { - val transaction = supportFragmentManager.beginTransaction() - fragments.forEach { - transaction.remove(it) - lastActiveTabs.remove(it.tabId) - } - transaction.commit() - } + fun openExistingTab(tabId: String) + fun launchNewTab() + fun openInNewTab( + query: String, + sourceTabId: String?, + ) - // Temporary class to keep track of latest visited tabs, keeping unique ids. - private class TabList : ArrayList() { - override fun add(element: String): Boolean { - if (this.contains(element)) { - this.remove(element) - } - return super.add(element) - } - } + fun onCleanup() } From c2b0cd2c6f882913a891e891c8043980f90d85ef Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 16:05:38 +0100 Subject: [PATCH 13/47] Use an injected TabManager for tab-related operations --- .../duckduckgo/app/browser/BrowserActivity.kt | 46 ++++--------------- .../app/browser/BrowserTabFragment.kt | 12 +++-- 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 022917c34370..1fbffec56cc0 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -23,7 +23,6 @@ import android.content.Intent.EXTRA_TEXT import android.os.Bundle import android.os.Handler import android.os.Looper -import android.os.Message import android.view.KeyEvent import android.view.View import android.widget.Toast @@ -31,6 +30,7 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting +import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat @@ -43,7 +43,7 @@ import com.duckduckgo.app.browser.databinding.IncludeOmnibarToolbarMockupBinding import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP import com.duckduckgo.app.browser.shortcut.ShortcutBuilder -import com.duckduckgo.app.browser.tabs.TabManager +import com.duckduckgo.app.browser.tabs.RealTabManager import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.downloads.DownloadsScreens.DownloadsScreenNoParams import com.duckduckgo.app.feedback.ui.common.FeedbackActivity @@ -58,6 +58,7 @@ import com.duckduckgo.app.global.sanitize import com.duckduckgo.app.global.view.ClearDataAction import com.duckduckgo.app.global.view.FireDialog import com.duckduckgo.app.global.view.renderIfChanged +import com.duckduckgo.app.onboarding.store.UserStageStore import com.duckduckgo.app.onboarding.ui.page.DefaultBrowserPage import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.AppPixelName.FIRE_DIALOG_CANCEL @@ -133,13 +134,8 @@ open class BrowserActivity : DuckDuckGoActivity() { @Inject lateinit var swipingTabsFeature: SwipingTabsFeature - private val tabManager by lazy { - TabManager( - activity = this, - swipingTabsFeature = swipingTabsFeature, - onNewTabRequested = viewModel::onNewTabRequested, - ) - } + @Inject + lateinit var tabManager: RealTabManager private var currentTab: BrowserTabFragment? get() = tabManager.currentTab @@ -147,7 +143,7 @@ open class BrowserActivity : DuckDuckGoActivity() { tabManager.currentTab = value } - private val viewModel: BrowserViewModel by bindViewModel() + val viewModel: BrowserViewModel by bindViewModel() private var instanceStateBundles: CombinedInstanceState? = null @@ -294,13 +290,13 @@ open class BrowserActivity : DuckDuckGoActivity() { if (launchNewSearch(intent)) { Timber.w("new tab requested") - lifecycleScope.launch { viewModel.onNewTabRequested() } + tabManager.launchNewTab() return } val existingTabId = intent.getStringExtra(OPEN_EXISTING_TAB_ID_EXTRA) if (existingTabId != null) { - openExistingTab(existingTabId) + tabManager.openExistingTab(existingTabId) return } @@ -413,32 +409,6 @@ open class BrowserActivity : DuckDuckGoActivity() { dialog.show() } - fun launchNewTab() { - lifecycleScope.launch { viewModel.onNewTabRequested() } - } - - fun openInNewTab( - query: String, - sourceTabId: String?, - ) { - lifecycleScope.launch { - viewModel.onOpenInNewTabRequested(query = query, sourceTabId = sourceTabId) - } - } - - fun openMessageInNewTab( - message: Message, - sourceTabId: String?, - ) { - tabManager.openMessageInNewTab(message, sourceTabId) - } - - fun openExistingTab(tabId: String) { - lifecycleScope.launch { - viewModel.onTabSelected(tabId) - } - } - fun launchSettings() { startActivity(SettingsActivity.intent(this)) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 8aa39b14d8ea..8eae1876e486 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -151,6 +151,7 @@ import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.shortcut.ShortcutBuilder import com.duckduckgo.app.browser.tabpreview.WebViewPreviewGenerator import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister +import com.duckduckgo.app.browser.tabs.TabManager import com.duckduckgo.app.browser.ui.dialogs.AutomaticFireproofDialogOptions import com.duckduckgo.app.browser.ui.dialogs.LaunchInExternalAppOptions import com.duckduckgo.app.browser.urlextraction.DOMUrlExtractor @@ -530,6 +531,9 @@ class BrowserTabFragment : @Inject lateinit var webViewCapabilityChecker: WebViewCapabilityChecker + @Inject + lateinit var tabManager: TabManager + /** * We use this to monitor whether the user was seeing the in-context Email Protection signup prompt * This is needed because the activity stack will be cleared if an external link is opened in our browser @@ -1495,7 +1499,7 @@ class BrowserTabFragment : when (it) { is NavigationCommand.Refresh -> refresh() is Command.OpenInNewTab -> { - browserActivity?.openInNewTab(it.query, it.sourceTabId) + tabManager.openInNewTab(it.query, it.sourceTabId) } is Command.OpenMessageInNewTab -> { @@ -1507,7 +1511,7 @@ class BrowserTabFragment : isLaunchedFromExternalApp, ) } else { - browserActivity?.openMessageInNewTab(it.message, it.sourceTabId) + tabManager.openMessageInNewTab(it.message, it.sourceTabId) } } @@ -1515,7 +1519,7 @@ class BrowserTabFragment : openInNewBackgroundTab() } - is Command.LaunchNewTab -> browserActivity?.launchNewTab() + is Command.LaunchNewTab -> tabManager.launchNewTab() is Command.ShowSavedSiteAddedConfirmation -> savedSiteAdded(it.savedSiteChangedViewState) is Command.ShowEditSavedSiteDialog -> editSavedSite(it.savedSiteChangedViewState) is Command.DeleteFavoriteConfirmation -> confirmDeleteSavedSite( @@ -1750,7 +1754,7 @@ class BrowserTabFragment : viewModel.autoCompleteSuggestionsGone() } binding.autoCompleteSuggestionsList.gone() - browserActivity?.openExistingTab(it.tabId) + tabManager.openExistingTab(it.tabId) } else -> { // NO OP From d865d184028071f5b91759111ad1b3118720a5c8 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 16:12:23 +0100 Subject: [PATCH 14/47] Fix ktlint issues --- app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt | 2 -- .../main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 1fbffec56cc0..61c9bdb4e29e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -30,7 +30,6 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting -import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat @@ -58,7 +57,6 @@ import com.duckduckgo.app.global.sanitize import com.duckduckgo.app.global.view.ClearDataAction import com.duckduckgo.app.global.view.FireDialog import com.duckduckgo.app.global.view.renderIfChanged -import com.duckduckgo.app.onboarding.store.UserStageStore import com.duckduckgo.app.onboarding.ui.page.DefaultBrowserPage import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.AppPixelName.FIRE_DIALOG_CANCEL diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt index e3a724b924b9..4d3828109983 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt @@ -27,10 +27,10 @@ import com.duckduckgo.di.scopes.ActivityScope import com.squareup.anvil.annotations.ContributesBinding import dagger.SingleInstanceIn import dagger.android.DaggerActivity +import javax.inject.Inject import kotlinx.coroutines.Job import kotlinx.coroutines.launch import timber.log.Timber -import javax.inject.Inject @ContributesBinding(ActivityScope::class) @SingleInstanceIn(ActivityScope::class) From 23cf2d48431e605830112b5b445c8884e1f41591 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 16:18:24 +0100 Subject: [PATCH 15/47] Rename function for easier readability --- .../main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- .../main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt | 2 +- app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 8eae1876e486..c161ad3d49e1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -1499,7 +1499,7 @@ class BrowserTabFragment : when (it) { is NavigationCommand.Refresh -> refresh() is Command.OpenInNewTab -> { - tabManager.openInNewTab(it.query, it.sourceTabId) + tabManager.openQueryInNewTab(it.query, it.sourceTabId) } is Command.OpenMessageInNewTab -> { diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt index 4d3828109983..c51d55304692 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt @@ -105,7 +105,7 @@ class RealTabManager @Inject constructor( browserActivity.lifecycleScope.launch { browserActivity.viewModel.onNewTabRequested() } } - override fun openInNewTab( + override fun openQueryInNewTab( query: String, sourceTabId: String?, ) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index 4d5777fe8f15..e6d4887d5e76 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -32,7 +32,7 @@ interface TabManager { fun openExistingTab(tabId: String) fun launchNewTab() - fun openInNewTab( + fun openQueryInNewTab( query: String, sourceTabId: String?, ) From a0dc5cc6f9588991b4a341931ceb3eca7badac9d Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 16:29:31 +0100 Subject: [PATCH 16/47] Inject the TabManager interface instead of the implementation class --- .../main/java/com/duckduckgo/app/browser/BrowserActivity.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 61c9bdb4e29e..f1e869b91acc 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -42,7 +42,7 @@ import com.duckduckgo.app.browser.databinding.IncludeOmnibarToolbarMockupBinding import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP import com.duckduckgo.app.browser.shortcut.ShortcutBuilder -import com.duckduckgo.app.browser.tabs.RealTabManager +import com.duckduckgo.app.browser.tabs.TabManager import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.downloads.DownloadsScreens.DownloadsScreenNoParams import com.duckduckgo.app.feedback.ui.common.FeedbackActivity @@ -133,7 +133,7 @@ open class BrowserActivity : DuckDuckGoActivity() { lateinit var swipingTabsFeature: SwipingTabsFeature @Inject - lateinit var tabManager: RealTabManager + lateinit var tabManager: TabManager private var currentTab: BrowserTabFragment? get() = tabManager.currentTab From 16af89e27861ecf521d61e968d20155d0cde1467 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 17:02:44 +0100 Subject: [PATCH 17/47] Remove unnecessary null checks --- .../app/browser/BrowserViewModel.kt | 67 ++++++++++++------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index fcf92bc0d38f..0d80519e696b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -33,6 +33,8 @@ import com.duckduckgo.app.global.rating.AppEnjoymentPromptEmitter import com.duckduckgo.app.global.rating.AppEnjoymentPromptOptions import com.duckduckgo.app.global.rating.AppEnjoymentUserEventRecorder import com.duckduckgo.app.global.rating.PromptCount +import com.duckduckgo.app.onboarding.store.AppStage +import com.duckduckgo.app.onboarding.store.UserStageStore import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.pixels.AppPixelName.APP_ENJOYMENT_DIALOG_SHOWN import com.duckduckgo.app.pixels.AppPixelName.APP_ENJOYMENT_DIALOG_USER_CANCELLED @@ -58,6 +60,11 @@ import com.duckduckgo.feature.toggles.api.Toggle import javax.inject.Inject import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import timber.log.Timber @@ -74,6 +81,7 @@ class BrowserViewModel @Inject constructor( private val skipUrlConversionOnNewTabFeature: SkipUrlConversionOnNewTabFeature, private val showOnAppLaunchFeature: ShowOnAppLaunchFeature, private val showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler, + private val userStageStore: UserStageStore, ) : ViewModel(), CoroutineScope { @@ -105,35 +113,42 @@ class BrowserViewModel @Inject constructor( var selectedTab: LiveData = tabRepository.liveSelectedTab val command: SingleLiveEvent = SingleLiveEvent() - private var dataClearingObserver = Observer { - it?.let { state -> - when (state) { - ApplicationClearDataState.INITIALIZING -> { - Timber.i("App clear state initializing") - viewState.value = currentViewState.copy(hideWebContent = true) - } - ApplicationClearDataState.FINISHED -> { - Timber.i("App clear state finished") - viewState.value = currentViewState.copy(hideWebContent = false) - } + val tabIds = tabRepository.flowTabs + .map { tabs -> tabs.map { it.tabId } } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), emptyList()) + .onEach { + onTabsUpdated(it) + } + + val isOnboardingCompleted: Flow = userStageStore.currentAppStage + .map { it != AppStage.DAX_ONBOARDING } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false) + + private var dataClearingObserver = Observer { state -> + when (state) { + ApplicationClearDataState.INITIALIZING -> { + Timber.i("App clear state initializing") + viewState.value = currentViewState.copy(hideWebContent = true) + } + ApplicationClearDataState.FINISHED -> { + Timber.i("App clear state finished") + viewState.value = currentViewState.copy(hideWebContent = false) } } } - private val appEnjoymentObserver = Observer { - it?.let { promptType -> - when (promptType) { - is AppEnjoymentPromptOptions.ShowEnjoymentPrompt -> { - command.value = Command.ShowAppEnjoymentPrompt(promptType.promptCount) - } - is AppEnjoymentPromptOptions.ShowRatingPrompt -> { - command.value = Command.ShowAppRatingPrompt(promptType.promptCount) - } - is AppEnjoymentPromptOptions.ShowFeedbackPrompt -> { - command.value = Command.ShowAppFeedbackPrompt(promptType.promptCount) - } - else -> {} + private val appEnjoymentObserver = Observer { promptType -> + when (promptType) { + is AppEnjoymentPromptOptions.ShowEnjoymentPrompt -> { + command.value = Command.ShowAppEnjoymentPrompt(promptType.promptCount) + } + is AppEnjoymentPromptOptions.ShowRatingPrompt -> { + command.value = Command.ShowAppRatingPrompt(promptType.promptCount) + } + is AppEnjoymentPromptOptions.ShowFeedbackPrompt -> { + command.value = Command.ShowAppFeedbackPrompt(promptType.promptCount) } + else -> {} } } @@ -186,8 +201,8 @@ class BrowserViewModel @Inject constructor( ) } - suspend fun onTabsUpdated(tabs: List?) { - if (tabs.isNullOrEmpty()) { + suspend fun onTabsUpdated(tabs: List) { + if (tabs.isEmpty()) { Timber.i("Tabs list is null or empty; adding default tab") tabRepository.addDefaultTab() return From d3454d59279b6ab8a4d9a12ab929342b609e280a Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 21:12:05 +0100 Subject: [PATCH 18/47] Add view pager to the layout and create an adapter --- .../duckduckgo/app/browser/BrowserActivity.kt | 42 ++++++ .../app/browser/tabs/TabPagerAdapter.kt | 125 ++++++++++++++++++ app/src/main/res/layout/activity_browser.xml | 6 + 3 files changed, 173 insertions(+) create mode 100644 app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index f1e869b91acc..1da4d07bb62c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -30,7 +30,9 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting +import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope +import androidx.viewpager2.widget.ViewPager2 import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat import androidx.webkit.WebViewFeature @@ -43,6 +45,7 @@ import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP import com.duckduckgo.app.browser.shortcut.ShortcutBuilder import com.duckduckgo.app.browser.tabs.TabManager +import com.duckduckgo.app.browser.tabs.TabPagerAdapter import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.downloads.DownloadsScreens.DownloadsScreenNoParams import com.duckduckgo.app.feedback.ui.common.FeedbackActivity @@ -80,6 +83,7 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParam import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA import javax.inject.Inject import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber @@ -151,8 +155,19 @@ open class BrowserActivity : DuckDuckGoActivity() { private val binding: ActivityBrowserBinding by viewBinding() + val tabPager: ViewPager2 by lazy { + binding.tabPager + } + private lateinit var toolbarMockupBinding: IncludeOmnibarToolbarMockupBinding + private val onTabPageChangeListener = object : ViewPager2.OnPageChangeCallback() { + override fun onPageSelected(position: Int) { + super.onPageSelected(position) + tabManager.tabPagerAdapter.onPageChanged(position) + } + } + @VisibleForTesting var destroyedByBackPress: Boolean = false @@ -208,6 +223,10 @@ open class BrowserActivity : DuckDuckGoActivity() { override fun onDestroy() { currentTab = null + + binding.tabPager.adapter = null + binding.tabPager.unregisterOnPageChangeCallback(onTabPageChangeListener) + super.onDestroy() } @@ -350,6 +369,13 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.tabs.observe(this) { tabManager.onTabsUpdated(it) } + + // listen to onboarding completion to enable/disable swiping + lifecycleScope.launch { + viewModel.isOnboardingCompleted.flowWithLifecycle(lifecycle).collectLatest { isOnboardingCompleted -> + binding.tabPager.isUserInputEnabled = isOnboardingCompleted + } + } } private fun removeObservers() { @@ -518,6 +544,7 @@ open class BrowserActivity : DuckDuckGoActivity() { private fun showWebContent() { Timber.d("BrowserActivity can now start displaying web content. instance state is $instanceStateBundles") + initializeTabs() configureObservers() binding.clearingInProgressView.gone() @@ -536,6 +563,21 @@ open class BrowserActivity : DuckDuckGoActivity() { } } + @SuppressLint("ClickableViewAccessibility", "WrongConstant") + private fun initializeTabs() { + if (swipingTabsFeature.self().isEnabled()) { + binding.tabPager.adapter = tabManager.tabPagerAdapter + binding.tabPager.offscreenPageLimit = 1 + binding.tabPager.registerOnPageChangeCallback(onTabPageChangeListener) + + binding.fragmentContainer.gone() + binding.tabPager.show() + } else { + binding.fragmentContainer.show() + binding.tabPager.gone() + } + } + private val Intent.launchedFromRecents: Boolean get() = (flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY) == Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt new file mode 100644 index 000000000000..638d86874a11 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt @@ -0,0 +1,125 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.tabs + +import androidx.fragment.app.Fragment +import androidx.fragment.app.FragmentManager +import androidx.lifecycle.Lifecycle +import androidx.recyclerview.widget.DiffUtil +import androidx.viewpager2.adapter.FragmentStateAdapter +import com.duckduckgo.app.browser.BrowserTabFragment +import com.duckduckgo.app.tabs.model.TabEntity + +class TabPagerAdapter( + lifecycle: Lifecycle, + private val fragmentManager: FragmentManager, + private val moveToTabIndex: (Int, Boolean) -> Unit, + private val getCurrentTabIndex: () -> Int, + private val getSelectedTabId: () -> String?, + private val getTabById: (String) -> TabEntity?, + private val onTabSelected: (String) -> Unit, + private val getOffScreenPageLimit: () -> Int, + private val setOffScreenPageLimit: (Int) -> Unit, +) : FragmentStateAdapter(fragmentManager, lifecycle) { + private val tabIds = mutableListOf() + + override fun getItemCount(): Int = tabIds.size + + override fun getItemId(position: Int) = tabIds[position].hashCode().toLong() + + override fun containsItem(itemId: Long) = tabIds.any { it.hashCode().toLong() == itemId } + + val currentFragment: BrowserTabFragment? + get() = fragmentManager.fragments + .filterIsInstance() + .firstOrNull { it.tabId == getSelectedTabId() } + + private val activeTabCount + get() = fragmentManager.fragments + .filterIsInstance() + .filter { it.isInitialized }.size + + override fun createFragment(position: Int): Fragment { + increaseOffscreenTabLimitIfNeeded() + + val tab = getTabById(tabIds[position])!! + return BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, false) + } + + // init { + // lifecycleScope.launch { + // delay(5000) + // binding.fragmentPager.setCurrentItem(0, true) + // repeat(MAX_ACTIVE_TABS) { + // Timber.d("$$$ moving to #$it") + // binding.fragmentPager.setCurrentItem(it, true) + // delay(1000) + // } + // } + // } + + fun onTabsUpdated(newTabs: List) { + if (tabIds != newTabs) { + val diffUtil = PagerDiffUtil(tabIds, newTabs) + val diff = DiffUtil.calculateDiff(diffUtil) + tabIds.clear() + tabIds.addAll(newTabs) + diff.dispatchUpdatesTo(this) + + getSelectedTabId()?.let { + onSelectedTabChanged(it) + } + } + } + + fun onSelectedTabChanged(tabId: String) { + val selectedTabIndex = tabIds.indexOfFirst { it == tabId } + if (selectedTabIndex != -1 && selectedTabIndex != getCurrentTabIndex()) { + moveToTabIndex(selectedTabIndex, false) + } + } + + fun onPageChanged(position: Int) { + if (position < tabIds.size) { + onTabSelected(tabIds[position]) + } + } + + private fun increaseOffscreenTabLimitIfNeeded() { + val offscreenPageLimit = getOffScreenPageLimit() + if (activeTabCount >= offscreenPageLimit * 2 - 1 && activeTabCount < TabManager.MAX_ACTIVE_TABS) { + setOffScreenPageLimit(offscreenPageLimit + 1) + } + } + + inner class PagerDiffUtil( + private val oldList: List, + private val newList: List, + ) : DiffUtil.Callback() { + override fun getOldListSize() = oldList.size + + override fun getNewListSize() = newList.size + + override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { + return oldList[oldItemPosition] == newList[newItemPosition] + } + + override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { + return true + } + } +} diff --git a/app/src/main/res/layout/activity_browser.xml b/app/src/main/res/layout/activity_browser.xml index 49022e924971..13f7a7d20271 100644 --- a/app/src/main/res/layout/activity_browser.xml +++ b/app/src/main/res/layout/activity_browser.xml @@ -34,6 +34,12 @@ android:layout_width="match_parent" android:layout_height="match_parent" /> + + Date: Mon, 11 Nov 2024 21:13:13 +0100 Subject: [PATCH 19/47] Initialize the adapter in the tab manager --- .../app/browser/tabs/RealTabManager.kt | 37 ++++++++++++------- .../duckduckgo/app/browser/tabs/TabManager.kt | 15 ++++---- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt index c51d55304692..4b7744418e9e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt @@ -22,6 +22,7 @@ import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.SwipingTabsFeature +import com.duckduckgo.app.browser.tabs.TabManager.Companion.MAX_ACTIVE_TABS import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.di.scopes.ActivityScope import com.squareup.anvil.annotations.ContributesBinding @@ -38,20 +39,30 @@ class RealTabManager @Inject constructor( activity: DaggerActivity, private val swipingTabsFeature: SwipingTabsFeature, ) : TabManager { - companion object { - private const val MAX_ACTIVE_TABS = 40 - } - private val browserActivity = activity as BrowserActivity private val lastActiveTabs = TabList() private val supportFragmentManager = activity.supportFragmentManager private var openMessageInNewTabJob: Job? = null + override val tabPagerAdapter by lazy { + TabPagerAdapter( + fragmentManager = supportFragmentManager, + lifecycle = browserActivity.lifecycle, + moveToTabIndex = { index, smoothScroll -> browserActivity.tabPager.setCurrentItem(index, smoothScroll) }, + getCurrentTabIndex = { browserActivity.tabPager.currentItem }, + getSelectedTabId = { browserActivity.viewModel.selectedTab.value?.tabId }, + getTabById = { tabId -> browserActivity.viewModel.getTabById(tabId) }, + onTabSelected = { tabId -> browserActivity.viewModel.onTabSelected(tabId) }, + setOffScreenPageLimit = { limit -> browserActivity.tabPager.offscreenPageLimit = limit }, + getOffScreenPageLimit = { browserActivity.tabPager.offscreenPageLimit }, + ) + } + private var _currentTab: BrowserTabFragment? = null override var currentTab: BrowserTabFragment? get() { return if (swipingTabsFeature.self().isEnabled()) { - null + tabPagerAdapter.currentFragment } else { _currentTab } @@ -61,16 +72,18 @@ class RealTabManager @Inject constructor( } override fun onSelectedTabChanged(tab: TabEntity?) { - if (swipingTabsFeature.self().isEnabled()) { - return - } else if (tab != null) { - selectTab(tab) + if (tab != null) { + if (swipingTabsFeature.self().isEnabled()) { + tabPagerAdapter.onSelectedTabChanged(tab.tabId) + } else { + selectTab(tab) + } } } override fun onTabsUpdated(updatedTabs: List) { if (swipingTabsFeature.self().isEnabled()) { - return + tabPagerAdapter.onTabsUpdated(updatedTabs.map { it.tabId }) } else { clearStaleTabs(updatedTabs) } @@ -121,15 +134,13 @@ class RealTabManager @Inject constructor( openMessageInNewTabJob?.cancel() } - private fun selectTab(tab: TabEntity?) { + private fun selectTab(tab: TabEntity) { if (swipingTabsFeature.self().isEnabled()) { return } Timber.v("Select tab: $tab") - if (tab == null) return - if (tab.tabId == currentTab?.tabId) return lastActiveTabs.add(tab.tabId) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index e6d4887d5e76..a7bafeea705a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -21,21 +21,20 @@ import com.duckduckgo.app.browser.BrowserTabFragment import com.duckduckgo.app.tabs.model.TabEntity interface TabManager { + companion object { + const val MAX_ACTIVE_TABS = 40 + } + var currentTab: BrowserTabFragment? + val tabPagerAdapter: TabPagerAdapter fun onSelectedTabChanged(tab: TabEntity?) fun onTabsUpdated(updatedTabs: List) - fun openMessageInNewTab( - message: Message, - sourceTabId: String?, - ) + fun openMessageInNewTab(message: Message, sourceTabId: String?) fun openExistingTab(tabId: String) fun launchNewTab() - fun openQueryInNewTab( - query: String, - sourceTabId: String?, - ) + fun openQueryInNewTab(query: String, sourceTabId: String?) fun onCleanup() } From c443b8d804abddf523980e30280a25e8212ff5af Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 21:14:29 +0100 Subject: [PATCH 20/47] Add method to get tab by ID --- .../app/browser/BrowserViewModel.kt | 20 +++++++++---------- .../app/tabs/model/TabDataRepository.kt | 4 ++++ .../app/tabs/model/TabRepository.kt | 2 ++ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 0d80519e696b..6a20a349e0eb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -20,6 +20,7 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer import androidx.lifecycle.ViewModel +import androidx.lifecycle.distinctUntilChanged import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesRemoteFeature import com.duckduckgo.anvil.annotations.ContributesViewModel @@ -63,9 +64,9 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import timber.log.Timber @ContributesViewModel(ActivityScope::class) @@ -81,7 +82,7 @@ class BrowserViewModel @Inject constructor( private val skipUrlConversionOnNewTabFeature: SkipUrlConversionOnNewTabFeature, private val showOnAppLaunchFeature: ShowOnAppLaunchFeature, private val showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler, - private val userStageStore: UserStageStore, + userStageStore: UserStageStore, ) : ViewModel(), CoroutineScope { @@ -109,17 +110,10 @@ class BrowserViewModel @Inject constructor( private val currentViewState: ViewState get() = viewState.value!! - var tabs: LiveData> = tabRepository.liveTabs - var selectedTab: LiveData = tabRepository.liveSelectedTab + val tabs: LiveData> = tabRepository.liveTabs.distinctUntilChanged() + val selectedTab: LiveData = tabRepository.liveSelectedTab.distinctUntilChanged() val command: SingleLiveEvent = SingleLiveEvent() - val tabIds = tabRepository.flowTabs - .map { tabs -> tabs.map { it.tabId } } - .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), emptyList()) - .onEach { - onTabsUpdated(it) - } - val isOnboardingCompleted: Flow = userStageStore.currentAppStage .map { it != AppStage.DAX_ONBOARDING } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false) @@ -318,6 +312,10 @@ class BrowserViewModel @Inject constructor( } } } + + fun getTabById(tabId: String): TabEntity? = runBlocking(dispatchers.io()) { + tabRepository.getTabById(tabId) + } } /** 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 index 0338a282c400..bcc40c50e890 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt @@ -370,4 +370,8 @@ class TabDataRepository @Inject constructor( private fun databaseExecutor(): Scheduler { return Schedulers.single() } + + override fun getTabById(tabId: String): TabEntity? { + return tabsDao.tab(tabId) + } } 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 index 0e7e877c8020..e7b50aefc599 100644 --- 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 @@ -114,4 +114,6 @@ interface TabRepository { suspend fun setIsUserNew(isUserNew: Boolean) suspend fun setTabLayoutType(layoutType: LayoutType) + + fun getTabById(tabId: String): TabEntity? } From 57016fdd525797d238333f517c736820dad40775 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 21:15:04 +0100 Subject: [PATCH 21/47] Optimize the fragment initialization for the view pager use case --- .../app/browser/BrowserTabFragment.kt | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index c161ad3d49e1..66605552c599 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -615,6 +615,8 @@ class BrowserTabFragment : private var webView: DuckDuckGoWebView? = null + var isInitialized = false + private val activityResultHandlerEmailProtectionInContextSignup = registerForActivityResult(StartActivityForResult()) { result: ActivityResult -> when (result.resultCode) { EmailProtectionInContextSignUpScreenResult.SUCCESS -> { @@ -845,9 +847,8 @@ class BrowserTabFragment : configureOmnibar() if (savedInstanceState == null) { - viewModel.onViewReady() - messageFromPreviousTab?.let { - processMessage(it) + if (isActiveTab) { + initializeFragment() } } else { viewModel.onViewRecreated() @@ -856,7 +857,7 @@ class BrowserTabFragment : lifecycle.addObserver( @SuppressLint("NoLifecycleObserver") // we don't observe app lifecycle object : DefaultLifecycleObserver { - override fun onStop(owner: LifecycleOwner) { + override fun onPause(owner: LifecycleOwner) { if (isVisible) { updateOrDeleteWebViewPreview() } @@ -1145,9 +1146,22 @@ class BrowserTabFragment : startActivity(TabSwitcherActivity.intent(activity, tabId)) } + private fun initializeFragment() { + if (!isInitialized) { + isInitialized = true + + viewModel.onViewReady() + messageFromPreviousTab?.let { + processMessage(it) + } + } + } + override fun onResume() { super.onResume() + initializeFragment() + if (viewModel.hasOmnibarPositionChanged(omnibar.omnibarPosition)) { requireActivity().recreate() return @@ -1158,7 +1172,7 @@ class BrowserTabFragment : viewModel.onViewResumed() // onResume can be called for a hidden/backgrounded fragment, ensure this tab is visible. - if (fragmentIsVisible()) { + if (fragmentIsVisible() && isActiveTab) { viewModel.onViewVisible() } @@ -2545,7 +2559,11 @@ class BrowserTabFragment : cancelPendingAutofillRequestsToChooseCredentials() } else { omnibar.omnibarTextInput.hideKeyboard() - binding.focusDummy.requestFocus() + + // prevent a crash when the view is not initiliazed yet + if (view != null) { + binding.focusDummy.requestFocus() + } } } From 84ba9447f65ca9ece952cada30370fc0294a3c4c Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 11 Nov 2024 21:15:40 +0100 Subject: [PATCH 22/47] Add methods to observe the onboarding status --- .../java/com/duckduckgo/app/onboarding/store/UserStageDao.kt | 4 ++++ .../com/duckduckgo/app/onboarding/store/UserStageStore.kt | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageDao.kt b/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageDao.kt index d2effdbb646e..b3141fb6bf84 100644 --- a/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageDao.kt +++ b/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageDao.kt @@ -17,6 +17,7 @@ package com.duckduckgo.app.onboarding.store import androidx.room.* +import kotlinx.coroutines.flow.Flow @Dao interface UserStageDao { @@ -24,6 +25,9 @@ interface UserStageDao { @Query("select * from $USER_STAGE_TABLE_NAME limit 1") suspend fun currentUserAppStage(): UserStage? + @Query("select * from $USER_STAGE_TABLE_NAME limit 1") + fun currentAppStage(): Flow + @Insert(onConflict = OnConflictStrategy.REPLACE) fun insert(userStage: UserStage) diff --git a/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageStore.kt b/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageStore.kt index 84bf15267e9d..a72241c4df07 100644 --- a/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageStore.kt +++ b/app/src/main/java/com/duckduckgo/app/onboarding/store/UserStageStore.kt @@ -18,12 +18,15 @@ package com.duckduckgo.app.onboarding.store import com.duckduckgo.common.utils.DispatcherProvider import javax.inject.Inject +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map import kotlinx.coroutines.withContext interface UserStageStore { suspend fun getUserAppStage(): AppStage suspend fun stageCompleted(appStage: AppStage): AppStage suspend fun moveToStage(appStage: AppStage) + val currentAppStage: Flow } class AppUserStageStore @Inject constructor( @@ -57,6 +60,8 @@ class AppUserStageStore @Inject constructor( override suspend fun moveToStage(appStage: AppStage) { userStageDao.updateUserStage(appStage) } + + override val currentAppStage: Flow = userStageDao.currentAppStage().map { it.appStage } } suspend fun UserStageStore.isNewUser(): Boolean { From d1f14132384b1e6d2c5b343592e93ea51ca33552 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Tue, 12 Nov 2024 12:06:56 +0100 Subject: [PATCH 23/47] Fix the new tab creation --- .../com/duckduckgo/app/browser/BrowserViewModel.kt | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 6a20a349e0eb..5da1319f9f9b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -20,6 +20,7 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer import androidx.lifecycle.ViewModel +import androidx.lifecycle.asLiveData import androidx.lifecycle.distinctUntilChanged import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesRemoteFeature @@ -63,7 +64,9 @@ import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking @@ -110,9 +113,12 @@ class BrowserViewModel @Inject constructor( private val currentViewState: ViewState get() = viewState.value!! - val tabs: LiveData> = tabRepository.liveTabs.distinctUntilChanged() - val selectedTab: LiveData = tabRepository.liveSelectedTab.distinctUntilChanged() val command: SingleLiveEvent = SingleLiveEvent() + val selectedTab: LiveData = tabRepository.liveSelectedTab.distinctUntilChanged() + val tabs: LiveData> = tabRepository.flowTabs + .distinctUntilChanged() + .onEach { onTabsUpdated(it.isEmpty()) } + .asLiveData() val isOnboardingCompleted: Flow = userStageStore.currentAppStage .map { it != AppStage.DAX_ONBOARDING } @@ -195,8 +201,8 @@ class BrowserViewModel @Inject constructor( ) } - suspend fun onTabsUpdated(tabs: List) { - if (tabs.isEmpty()) { + suspend fun onTabsUpdated(areTabsEmpty: Boolean) { + if (areTabsEmpty) { Timber.i("Tabs list is null or empty; adding default tab") tabRepository.addDefaultTab() return From 9b665a04d776423da25b6cc346ca30b565e71393 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Tue, 12 Nov 2024 12:07:44 +0100 Subject: [PATCH 24/47] Handle the new tab creation using a message from another tab --- .../app/browser/tabs/RealTabManager.kt | 36 ++++++++++--------- .../app/browser/tabs/TabPagerAdapter.kt | 21 ++++++++++- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt index 4b7744418e9e..ab422bf16be5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt @@ -48,6 +48,7 @@ class RealTabManager @Inject constructor( TabPagerAdapter( fragmentManager = supportFragmentManager, lifecycle = browserActivity.lifecycle, + activityIntent = browserActivity.intent, moveToTabIndex = { index, smoothScroll -> browserActivity.tabPager.setCurrentItem(index, smoothScroll) }, getCurrentTabIndex = { browserActivity.tabPager.currentItem }, getSelectedTabId = { browserActivity.viewModel.selectedTab.value?.tabId }, @@ -93,18 +94,25 @@ class RealTabManager @Inject constructor( message: Message, sourceTabId: String?, ) { - openMessageInNewTabJob = browserActivity.lifecycleScope.launch { - val tabId = browserActivity.viewModel.onNewTabRequested(sourceTabId) - val fragment = openNewTab( - tabId = tabId, - url = null, - skipHome = false, - isExternal = browserActivity.intent?.getBooleanExtra( - BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, - false, - ) ?: false, - ) - fragment.messageFromPreviousTab = message + if (swipingTabsFeature.self().isEnabled()) { + openMessageInNewTabJob = browserActivity.lifecycleScope.launch { + tabPagerAdapter.setMessageForNewFragment(message) + browserActivity.viewModel.onNewTabRequested(sourceTabId) + } + } else { + openMessageInNewTabJob = browserActivity.lifecycleScope.launch { + val tabId = browserActivity.viewModel.onNewTabRequested(sourceTabId) + val fragment = openNewTab( + tabId = tabId, + url = null, + skipHome = false, + isExternal = browserActivity.intent?.getBooleanExtra( + BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, + false, + ) ?: false, + ) + fragment.messageFromPreviousTab = message + } } } @@ -135,10 +143,6 @@ class RealTabManager @Inject constructor( } private fun selectTab(tab: TabEntity) { - if (swipingTabsFeature.self().isEnabled()) { - return - } - Timber.v("Select tab: $tab") if (tab.tabId == currentTab?.tabId) return diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt index 638d86874a11..152df81104f0 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt @@ -16,17 +16,21 @@ package com.duckduckgo.app.browser.tabs +import android.content.Intent +import android.os.Message import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager import androidx.lifecycle.Lifecycle import androidx.recyclerview.widget.DiffUtil import androidx.viewpager2.adapter.FragmentStateAdapter +import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment import com.duckduckgo.app.tabs.model.TabEntity class TabPagerAdapter( lifecycle: Lifecycle, private val fragmentManager: FragmentManager, + private val activityIntent: Intent?, private val moveToTabIndex: (Int, Boolean) -> Unit, private val getCurrentTabIndex: () -> Int, private val getSelectedTabId: () -> String?, @@ -36,6 +40,7 @@ class TabPagerAdapter( private val setOffScreenPageLimit: (Int) -> Unit, ) : FragmentStateAdapter(fragmentManager, lifecycle) { private val tabIds = mutableListOf() + private var messageForNewFragment: Message? = null override fun getItemCount(): Int = tabIds.size @@ -57,7 +62,17 @@ class TabPagerAdapter( increaseOffscreenTabLimitIfNeeded() val tab = getTabById(tabIds[position])!! - return BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, false) + val isExternal = activityIntent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false + + if (messageForNewFragment != null) { + val message = messageForNewFragment + messageForNewFragment = null + return BrowserTabFragment.newInstance(tab.tabId, null, false, isExternal).apply { + this.messageFromPreviousTab = message + } + } else { + return BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, isExternal) + } } // init { @@ -72,6 +87,10 @@ class TabPagerAdapter( // } // } + fun setMessageForNewFragment(message: Message) { + messageForNewFragment = message + } + fun onTabsUpdated(newTabs: List) { if (tabIds != newTabs) { val diffUtil = PagerDiffUtil(tabIds, newTabs) From b581903cf38fe8fbba721781dcb3e3520dc471e8 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Sun, 17 Nov 2024 15:27:59 +0100 Subject: [PATCH 25/47] Initialize the tab fragment when it becomes active --- .../duckduckgo/app/browser/BrowserTabFragment.kt | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 66605552c599..fc2c611c1c2b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -848,7 +848,7 @@ class BrowserTabFragment : if (savedInstanceState == null) { if (isActiveTab) { - initializeFragment() + initFragmentIfNecessary() } } else { viewModel.onViewRecreated() @@ -869,6 +869,11 @@ class BrowserTabFragment : (dialog as EditSavedSiteDialogFragment).listener = viewModel dialog.deleteBookmarkListener = viewModel } + + newBrowserTab.newTabLayout.setOnTouchListener { v, event -> + (v as FrameLayout).requestDisallowInterceptTouchEvent(true) + return@setOnTouchListener true + } } private fun configureOmnibar() { @@ -1146,7 +1151,7 @@ class BrowserTabFragment : startActivity(TabSwitcherActivity.intent(activity, tabId)) } - private fun initializeFragment() { + private fun initFragmentIfNecessary() { if (!isInitialized) { isInitialized = true @@ -1160,7 +1165,7 @@ class BrowserTabFragment : override fun onResume() { super.onResume() - initializeFragment() + initFragmentIfNecessary() if (viewModel.hasOmnibarPositionChanged(omnibar.omnibarPosition)) { requireActivity().recreate() @@ -1331,6 +1336,10 @@ class BrowserTabFragment : // want to ensure that we aren't offering to inject credentials from an inactive tab hideDialogWithTag(CredentialAutofillPickerDialog.TAG) } + + if (isActiveTab) { + initFragmentIfNecessary() + } } }, ) From 9e4c066c1ca4db64128b83d32ca83912b128bbe9 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 18 Nov 2024 10:48:03 +0100 Subject: [PATCH 26/47] Rename the tab manager impelemntation --- .../tabs/{RealTabManager.kt => DefaultTabManager.kt} | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) rename app/src/main/java/com/duckduckgo/app/browser/tabs/{RealTabManager.kt => DefaultTabManager.kt} (96%) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt similarity index 96% rename from app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt rename to app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt index ab422bf16be5..b5a70a7a117c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt @@ -31,11 +31,12 @@ import dagger.android.DaggerActivity import javax.inject.Inject import kotlinx.coroutines.Job import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import timber.log.Timber @ContributesBinding(ActivityScope::class) @SingleInstanceIn(ActivityScope::class) -class RealTabManager @Inject constructor( +class DefaultTabManager @Inject constructor( activity: DaggerActivity, private val swipingTabsFeature: SwipingTabsFeature, ) : TabManager { @@ -53,6 +54,7 @@ class RealTabManager @Inject constructor( getCurrentTabIndex = { browserActivity.tabPager.currentItem }, getSelectedTabId = { browserActivity.viewModel.selectedTab.value?.tabId }, getTabById = { tabId -> browserActivity.viewModel.getTabById(tabId) }, + requestNewTab = ::requestNewTab, onTabSelected = { tabId -> browserActivity.viewModel.onTabSelected(tabId) }, setOffScreenPageLimit = { limit -> browserActivity.tabPager.offscreenPageLimit = limit }, getOffScreenPageLimit = { browserActivity.tabPager.offscreenPageLimit }, @@ -142,6 +144,11 @@ class RealTabManager @Inject constructor( openMessageInNewTabJob?.cancel() } + private fun requestNewTab(): TabEntity { + val tabId = runBlocking { browserActivity.viewModel.onNewTabRequested() } + return browserActivity.viewModel.getTabById(tabId)!! + } + private fun selectTab(tab: TabEntity) { Timber.v("Select tab: $tab") From 1402075d2b6905c8b87105d38022973d62559e20 Mon Sep 17 00:00:00 2001 From: Ondrej Ruttkay Date: Mon, 18 Nov 2024 10:50:16 +0100 Subject: [PATCH 27/47] Disable tab deletion when there is just one --- .../duckduckgo/app/browser/tabs/TabPagerAdapter.kt | 3 ++- .../duckduckgo/app/tabs/ui/TabSwitcherActivity.kt | 12 ++++++++++-- .../duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt | 13 ++++++++++++- .../duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt | 5 +++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt index 152df81104f0..9192159273bb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt @@ -35,6 +35,7 @@ class TabPagerAdapter( private val getCurrentTabIndex: () -> Int, private val getSelectedTabId: () -> String?, private val getTabById: (String) -> TabEntity?, + private val requestNewTab: () -> TabEntity, private val onTabSelected: (String) -> Unit, private val getOffScreenPageLimit: () -> Int, private val setOffScreenPageLimit: (Int) -> Unit, @@ -61,7 +62,7 @@ class TabPagerAdapter( override fun createFragment(position: Int): Fragment { increaseOffscreenTabLimitIfNeeded() - val tab = getTabById(tabIds[position])!! + val tab = getTabById(tabIds[position]) ?: requestNewTab() val isExternal = activityIntent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false if (messageForNewFragment != null) { 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 index 4b2529dbd8c5..f39c72c6d2c8 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherActivity.kt @@ -129,6 +129,7 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine private var layoutTypeMenuItem: MenuItem? = null private var layoutType: LayoutType? = null + private var swipeListener: ItemTouchHelper? = null override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -174,8 +175,8 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine onTabDraggingFinished = this::onTabDraggingFinished, ) - val swipeListener = ItemTouchHelper(tabTouchHelper) - swipeListener.attachToRecyclerView(tabsRecycler) + swipeListener = ItemTouchHelper(tabTouchHelper) + swipeListener?.attachToRecyclerView(tabsRecycler) tabItemDecorator = TabItemDecorator(this, selectedTabId) tabsRecycler.addItemDecoration(tabItemDecorator) @@ -208,6 +209,13 @@ class TabSwitcherActivity : DuckDuckGoActivity(), TabSwitcherListener, Coroutine } } + lifecycleScope.launch { + viewModel.isDeletingEnabled.flowWithLifecycle(lifecycle, Lifecycle.State.STARTED).collect { isEnabled -> + tabsAdapter.onIsDeletingEnabledChanged(isEnabled) + swipeListener?.attachToRecyclerView(if (isEnabled) tabsRecycler else null) + } + } + viewModel.command.observe(this) { processCommand(it) } diff --git a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt index b33be228fc2a..6a613753f1da 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherAdapter.kt @@ -49,7 +49,6 @@ import com.duckduckgo.app.tabs.ui.TabSwitcherAdapter.TabViewHolder.ListTabViewHo import com.duckduckgo.common.ui.view.show import com.duckduckgo.common.utils.swap import java.io.File -import java.util.Collections.swap import kotlinx.coroutines.launch import timber.log.Timber @@ -63,6 +62,7 @@ class TabSwitcherAdapter( private val list = mutableListOf() private var isDragging: Boolean = false private var layoutType: LayoutType = LayoutType.GRID + private var isDeletingEnabled: Boolean = true init { setHasStableIds(true) @@ -129,6 +129,12 @@ class TabSwitcherAdapter( } override fun onBindViewHolder(holder: TabViewHolder, position: Int, payloads: MutableList) { + if (isDeletingEnabled) { + holder.close.show() + } else { + holder.close.visibility = View.GONE + } + if (payloads.isEmpty()) { onBindViewHolder(holder, position) return @@ -202,6 +208,11 @@ class TabSwitcherAdapter( diffResult.dispatchUpdatesTo(this) } + fun onIsDeletingEnabledChanged(isDeletingEnabled: Boolean) { + this.isDeletingEnabled = isDeletingEnabled + notifyDataSetChanged() + } + fun getTab(position: Int): TabEntity? = list.getOrNull(position) fun adapterPositionForTab(tabId: String?): Int = list.indexOfFirst { it.tabId == tabId } 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 index 6121400e4375..5bd82a5e805f 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt @@ -35,6 +35,7 @@ import com.duckduckgo.common.utils.SingleLiveEvent import com.duckduckgo.di.scopes.ActivityScope import javax.inject.Inject import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -54,6 +55,10 @@ class TabSwitcherViewModel @Inject constructor( context = viewModelScope.coroutineContext, ) + val isDeletingEnabled = tabRepository.flowTabs.map { it.size > 1 } + .distinctUntilChanged() + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), true) + val layoutType = tabRepository.tabSwitcherData .map { it.layoutType } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), null) From a311686d33b11f268f38e8315e2201e60cf9fa3f Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 20 Nov 2024 16:53:39 +0100 Subject: [PATCH 28/47] Switch to the existing empty tab instead of opening a new one --- .../duckduckgo/app/browser/BrowserTabViewModel.kt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 7f4f2b4d5b50..cfa7091bbcb4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -2797,10 +2797,17 @@ class BrowserTabViewModel @Inject constructor( } fun userRequestedOpeningNewTab(longPress: Boolean = false) { - command.value = GenerateWebViewPreviewImage - command.value = LaunchNewTab - if (longPress) { - pixel.fire(AppPixelName.TAB_MANAGER_NEW_TAB_LONG_PRESSED) + val emptyTab = tabs.value?.firstOrNull { it.url.isNullOrBlank() }?.tabId + if (emptyTab != null) { + viewModelScope.launch { + tabRepository.select(tabId = emptyTab) + } + } else if (!url.isNullOrEmpty() || (tabs.value?.size ?: 0) > 1) { + command.value = GenerateWebViewPreviewImage + command.value = LaunchNewTab + if (longPress) { + pixel.fire(AppPixelName.TAB_MANAGER_NEW_TAB_LONG_PRESSED) + } } } From 1635f4d638716ed4be60ef44e91c21fde32385bc Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 16:34:30 +0100 Subject: [PATCH 29/47] Update tab callbacks --- .../duckduckgo/app/browser/BrowserActivity.kt | 32 +++++++++++++++++-- .../app/browser/tabs/DefaultTabManager.kt | 16 +++++++--- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 1da4d07bb62c..62c9281172e8 100644 --- 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,10 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting +import androidx.core.view.postDelayed import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope +import androidx.recyclerview.widget.RecyclerView import androidx.viewpager2.widget.ViewPager2 import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat @@ -45,7 +47,6 @@ import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.BOTTOM import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition.TOP import com.duckduckgo.app.browser.shortcut.ShortcutBuilder import com.duckduckgo.app.browser.tabs.TabManager -import com.duckduckgo.app.browser.tabs.TabPagerAdapter import com.duckduckgo.app.di.AppCoroutineScope import com.duckduckgo.app.downloads.DownloadsScreens.DownloadsScreenNoParams import com.duckduckgo.app.feedback.ui.common.FeedbackActivity @@ -67,6 +68,7 @@ import com.duckduckgo.app.settings.SettingsActivity import com.duckduckgo.app.settings.db.SettingsDataStore import com.duckduckgo.app.sitepermissions.SitePermissionsActivity import com.duckduckgo.app.statistics.pixels.Pixel +import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.appbuildconfig.api.AppBuildConfig import com.duckduckgo.autofill.api.emailprotection.EmailProtectionLinkVerifier import com.duckduckgo.browser.api.ui.BrowserScreens.BookmarksScreenNoParams @@ -168,6 +170,18 @@ open class BrowserActivity : DuckDuckGoActivity() { } } + init { + // lifecycleScope.launch { + // delay(5000) + // binding.tabPager.setCurrentItem(0, true) + // repeat(MAX_ACTIVE_TABS) { + // Timber.d("$$$ moving to #$it") + // binding.tabPager.setCurrentItem(it, true) + // delay(1000) + // } + // } + } + @VisibleForTesting var destroyedByBackPress: Boolean = false @@ -367,7 +381,7 @@ open class BrowserActivity : DuckDuckGoActivity() { tabManager.onSelectedTabChanged(it) } viewModel.tabs.observe(this) { - tabManager.onTabsUpdated(it) + updateTabs(it) } // listen to onboarding completion to enable/disable swiping @@ -677,8 +691,22 @@ open class BrowserActivity : DuckDuckGoActivity() { playStoreUtils.launchPlayStore() } + fun onMoveToTabRequested(index: Int, smoothScroll: Boolean) { + binding.tabPager.unregisterOnPageChangeCallback(onTabPageChangeListener) + binding.tabPager.setCurrentItem(index, smoothScroll) + binding.tabPager.registerOnPageChangeCallback(onTabPageChangeListener) + } + private data class CombinedInstanceState( val originalInstanceState: Bundle?, val newInstanceState: Bundle?, ) + + private fun updateTabs(entities: List) { + val recyclerView = tabPager.getChildAt(0) as RecyclerView + val state = recyclerView.layoutManager?.onSaveInstanceState() + recyclerView.itemAnimator = null + tabManager.onTabsUpdated(entities) + recyclerView.layoutManager?.onRestoreInstanceState(state) + } } 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 b5a70a7a117c..4eca150b4b45 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 @@ -24,12 +24,15 @@ import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.SwipingTabsFeature import com.duckduckgo.app.browser.tabs.TabManager.Companion.MAX_ACTIVE_TABS import com.duckduckgo.app.tabs.model.TabEntity +import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.di.scopes.ActivityScope import com.squareup.anvil.annotations.ContributesBinding import dagger.SingleInstanceIn import dagger.android.DaggerActivity import javax.inject.Inject import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import timber.log.Timber @@ -39,6 +42,7 @@ import timber.log.Timber class DefaultTabManager @Inject constructor( activity: DaggerActivity, private val swipingTabsFeature: SwipingTabsFeature, + private val tabRepository: TabRepository, ) : TabManager { private val browserActivity = activity as BrowserActivity private val lastActiveTabs = TabList() @@ -50,10 +54,10 @@ class DefaultTabManager @Inject constructor( fragmentManager = supportFragmentManager, lifecycle = browserActivity.lifecycle, activityIntent = browserActivity.intent, - moveToTabIndex = { index, smoothScroll -> browserActivity.tabPager.setCurrentItem(index, smoothScroll) }, + moveToTabIndex = { index, smoothScroll -> browserActivity.onMoveToTabRequested(index, smoothScroll) }, getCurrentTabIndex = { browserActivity.tabPager.currentItem }, - getSelectedTabId = { browserActivity.viewModel.selectedTab.value?.tabId }, - getTabById = { tabId -> browserActivity.viewModel.getTabById(tabId) }, + getSelectedTabId = { runBlocking { tabRepository.flowSelectedTab.firstOrNull()?.tabId } }, + getTabById = { tabId -> runBlocking { tabRepository.flowTabs.first().firstOrNull { it.tabId == tabId } } }, requestNewTab = ::requestNewTab, onTabSelected = { tabId -> browserActivity.viewModel.onTabSelected(tabId) }, setOffScreenPageLimit = { limit -> browserActivity.tabPager.offscreenPageLimit = limit }, @@ -145,8 +149,10 @@ class DefaultTabManager @Inject constructor( } private fun requestNewTab(): TabEntity { - val tabId = runBlocking { browserActivity.viewModel.onNewTabRequested() } - return browserActivity.viewModel.getTabById(tabId)!! + return runBlocking { + val tabId = browserActivity.viewModel.onNewTabRequested() + tabRepository.flowTabs.first().first { it.tabId == tabId } + } } private fun selectTab(tab: TabEntity) { From e92dababd344975bfa263a895c3488f361750e5e Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 16:35:40 +0100 Subject: [PATCH 30/47] Change the onActivityCreated to onViewCreated --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index fc2c611c1c2b..4dbded31962e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -825,13 +825,16 @@ class BrowserTabFragment : } private fun resumeWebView() { + Timber.d("Resuming webview: $tabId") webView?.let { if (it.isShown) it.onResume() } } - override fun onActivityCreated(savedInstanceState: Bundle?) { - super.onActivityCreated(savedInstanceState) + override fun onViewCreated( + view: View, + savedInstanceState: Bundle?, + ) { omnibar = Omnibar(settingsDataStore.omnibarPosition, changeOmnibarPositionFeature.refactor().isEnabled(), binding) webViewContainer = binding.webViewContainer @@ -874,6 +877,7 @@ class BrowserTabFragment : (v as FrameLayout).requestDisallowInterceptTouchEvent(true) return@setOnTouchListener true } + binding.legacyOmnibar.setOnTouchListener { v, event -> false } } private fun configureOmnibar() { From c5af1d0db8b548cec77540004b20db4e86dfd260 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 16:37:05 +0100 Subject: [PATCH 31/47] Disable viewpager swiping over the webview --- .../main/java/com/duckduckgo/app/browser/DuckDuckGoWebView.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/DuckDuckGoWebView.kt b/app/src/main/java/com/duckduckgo/app/browser/DuckDuckGoWebView.kt index dc1fe3011316..717215f8190d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DuckDuckGoWebView.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DuckDuckGoWebView.kt @@ -227,7 +227,9 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 { @SuppressLint("ClickableViewAccessibility") override fun onTouchEvent(ev: MotionEvent): Boolean { - var returnValue = false + requestDisallowInterceptTouchEvent(true) + + val returnValue: Boolean val event = MotionEvent.obtain(ev) val action = event.actionMasked From 2b4abaee487fcf36cadcee822dc95164c5d648d9 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 16:37:49 +0100 Subject: [PATCH 32/47] Check if the tab has changed before calling the callback --- .../duckduckgo/app/browser/BrowserViewModel.kt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 5da1319f9f9b..3ab357fc0e3a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -69,7 +69,6 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking import timber.log.Timber @ContributesViewModel(ActivityScope::class) @@ -154,6 +153,13 @@ class BrowserViewModel @Inject constructor( init { appEnjoymentPromptEmitter.promptType.observeForever(appEnjoymentObserver) + + // viewModelScope.launch { + // repeat(50) { + // delay(1000) + // tabRepository.add("cnn.com") + // } + // } } suspend fun onNewTabRequested(sourceTabId: String? = null): String { @@ -307,7 +313,9 @@ class BrowserViewModel @Inject constructor( fun onTabSelected(tabId: String) { launch(dispatchers.io()) { - tabRepository.select(tabId) + if (tabId != tabRepository.getSelectedTab()?.tabId) { + tabRepository.select(tabId) + } } } @@ -318,10 +326,6 @@ class BrowserViewModel @Inject constructor( } } } - - fun getTabById(tabId: String): TabEntity? = runBlocking(dispatchers.io()) { - tabRepository.getTabById(tabId) - } } /** From cbf1df29f16e32bd965516330a4d2738a83f42ed Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 16:38:30 +0100 Subject: [PATCH 33/47] Clean up --- .../app/browser/tabs/TabPagerAdapter.kt | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt index 9192159273bb..ec3d8778ea79 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt @@ -26,6 +26,7 @@ import androidx.viewpager2.adapter.FragmentStateAdapter import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment import com.duckduckgo.app.tabs.model.TabEntity +import timber.log.Timber class TabPagerAdapter( lifecycle: Lifecycle, @@ -63,7 +64,7 @@ class TabPagerAdapter( increaseOffscreenTabLimitIfNeeded() val tab = getTabById(tabIds[position]) ?: requestNewTab() - val isExternal = activityIntent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) ?: false + val isExternal = activityIntent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) == true if (messageForNewFragment != null) { val message = messageForNewFragment @@ -76,24 +77,15 @@ class TabPagerAdapter( } } - // init { - // lifecycleScope.launch { - // delay(5000) - // binding.fragmentPager.setCurrentItem(0, true) - // repeat(MAX_ACTIVE_TABS) { - // Timber.d("$$$ moving to #$it") - // binding.fragmentPager.setCurrentItem(it, true) - // delay(1000) - // } - // } - // } - fun setMessageForNewFragment(message: Message) { messageForNewFragment = message } fun onTabsUpdated(newTabs: List) { if (tabIds != newTabs) { + Timber.i("$$$ oldIds: ${tabIds.joinToString(",")}") + Timber.i("$$$ newIds: ${newTabs.joinToString(",")}") + val diffUtil = PagerDiffUtil(tabIds, newTabs) val diff = DiffUtil.calculateDiff(diffUtil) tabIds.clear() @@ -108,13 +100,17 @@ class TabPagerAdapter( fun onSelectedTabChanged(tabId: String) { val selectedTabIndex = tabIds.indexOfFirst { it == tabId } + Timber.i("$$$ onSelectedTabChanged: selectedTabIndex $selectedTabIndex; currentTabIndex ${getCurrentTabIndex()}") if (selectedTabIndex != -1 && selectedTabIndex != getCurrentTabIndex()) { + Timber.i("$$$ onSelectedTabChanged:tabIds ${tabIds.joinToString(",")}") + Timber.i("$$$ onSelectedTabChanged: moving to index $selectedTabIndex ($tabId)") moveToTabIndex(selectedTabIndex, false) } } fun onPageChanged(position: Int) { if (position < tabIds.size) { + Timber.i("$$$ onPageChanged: moving to position $position (${tabIds[position]})") onTabSelected(tabIds[position]) } } @@ -139,7 +135,7 @@ class TabPagerAdapter( } override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { - return true + return areItemsTheSame(oldItemPosition, newItemPosition) } } } From dedd69a6278539a47df2335f663b7e28cf50da32 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 16:39:08 +0100 Subject: [PATCH 34/47] Use an existing empty tab instead of opening a new one --- .../com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 index 5bd82a5e805f..7ee57e3f6a62 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/ui/TabSwitcherViewModel.kt @@ -71,7 +71,12 @@ class TabSwitcherViewModel @Inject constructor( } suspend fun onNewTabRequested(fromOverflowMenu: Boolean) { - tabRepository.add() + val emptyTab = tabs.value?.firstOrNull { it.url.isNullOrBlank() }?.tabId + if (emptyTab != null) { + tabRepository.select(tabId = emptyTab) + } else { + tabRepository.add() + } command.value = Command.Close if (fromOverflowMenu) { pixel.fire(AppPixelName.TAB_MANAGER_MENU_NEW_TAB_PRESSED) From 24c8fa8480d2e8ac77ba2075cc404d6d80f9cfce Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 18:01:11 +0100 Subject: [PATCH 35/47] Use a flow to access the currently selected tab --- .../app/browser/tabs/DefaultTabManager.kt | 26 ++++++++++++------- .../com/duckduckgo/app/tabs/db/TabsDao.kt | 3 +++ .../app/tabs/model/TabDataRepository.kt | 7 +++-- .../app/tabs/model/TabRepository.kt | 4 +-- 4 files changed, 24 insertions(+), 16 deletions(-) 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 4eca150b4b45..bcbf50754af5 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 @@ -56,10 +56,10 @@ class DefaultTabManager @Inject constructor( activityIntent = browserActivity.intent, moveToTabIndex = { index, smoothScroll -> browserActivity.onMoveToTabRequested(index, smoothScroll) }, getCurrentTabIndex = { browserActivity.tabPager.currentItem }, - getSelectedTabId = { runBlocking { tabRepository.flowSelectedTab.firstOrNull()?.tabId } }, - getTabById = { tabId -> runBlocking { tabRepository.flowTabs.first().firstOrNull { it.tabId == tabId } } }, + getSelectedTabId = ::getSelectedTab, + getTabById = ::getTabById, requestNewTab = ::requestNewTab, - onTabSelected = { tabId -> browserActivity.viewModel.onTabSelected(tabId) }, + onTabSelected = ::getTabById, setOffScreenPageLimit = { limit -> browserActivity.tabPager.offscreenPageLimit = limit }, getOffScreenPageLimit = { browserActivity.tabPager.offscreenPageLimit }, ) @@ -115,7 +115,7 @@ class DefaultTabManager @Inject constructor( isExternal = browserActivity.intent?.getBooleanExtra( BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false, - ) ?: false, + ) == true, ) fragment.messageFromPreviousTab = message } @@ -148,11 +148,17 @@ class DefaultTabManager @Inject constructor( openMessageInNewTabJob?.cancel() } - private fun requestNewTab(): TabEntity { - return runBlocking { - val tabId = browserActivity.viewModel.onNewTabRequested() - tabRepository.flowTabs.first().first { it.tabId == tabId } - } + private fun requestNewTab(): TabEntity = runBlocking { + val tabId = browserActivity.viewModel.onNewTabRequested() + tabRepository.flowTabs.first().first { it.tabId == tabId } + } + + private fun getSelectedTab(): String? = runBlocking { + tabRepository.flowSelectedTab.firstOrNull()?.tabId + } + + private fun getTabById(tabId: String): TabEntity? = runBlocking { + tabRepository.flowTabs.first().firstOrNull { it.tabId == tabId } } private fun selectTab(tab: TabEntity) { @@ -171,7 +177,7 @@ class DefaultTabManager @Inject constructor( isExternal = browserActivity.intent?.getBooleanExtra( BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false, - ) ?: false, + ) == true, ) return } diff --git a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt index caa0cb4caa9f..4003c7b3929b 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt @@ -38,6 +38,9 @@ abstract class TabsDao { @Query("select * from tabs inner join tab_selection on tabs.tabId = tab_selection.tabId order by position limit 1") abstract fun liveSelectedTab(): LiveData + @Query("select * from tabs inner join tab_selection on tabs.tabId = tab_selection.tabId order by position limit 1") + abstract fun flowSelectedTab(): Flow + @Query("select * from tabs where deletable is 0 order by position") abstract fun tabs(): List 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 index bcc40c50e890..037b0de95b31 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt @@ -75,6 +75,8 @@ class TabDataRepository @Inject constructor( override val liveSelectedTab: LiveData = tabsDao.liveSelectedTab() + override val flowSelectedTab: Flow = tabsDao.flowSelectedTab() + override val tabSwitcherData: Flow = tabSwitcherDataStore.data private val siteData: LinkedHashMap> = LinkedHashMap() @@ -307,6 +309,7 @@ class TabDataRepository @Inject constructor( databaseExecutor().scheduleDirect { val selection = TabSelectionEntity(tabId = tabId) tabsDao.insertTabSelection(selection) + Timber.d("$$$ select(): Tab selected: $tabId") } } @@ -370,8 +373,4 @@ class TabDataRepository @Inject constructor( private fun databaseExecutor(): Scheduler { return Schedulers.single() } - - override fun getTabById(tabId: String): TabEntity? { - return tabsDao.tab(tabId) - } } 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 index e7b50aefc599..551f54ca8f51 100644 --- 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 @@ -41,6 +41,8 @@ interface TabRepository { val liveSelectedTab: LiveData + val flowSelectedTab: Flow + val tabSwitcherData: Flow /** @@ -114,6 +116,4 @@ interface TabRepository { suspend fun setIsUserNew(isUserNew: Boolean) suspend fun setTabLayoutType(layoutType: LayoutType) - - fun getTabById(tabId: String): TabEntity? } From 7dc5e9696fda0ec8070624d0188ca9b5e66054c2 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 21:15:46 +0100 Subject: [PATCH 36/47] Use LiveData instead of Flows --- .../duckduckgo/app/browser/BrowserActivity.kt | 27 +++++++------------ .../app/browser/BrowserViewModel.kt | 23 +++++----------- 2 files changed, 15 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 62c9281172e8..71119f33b0d3 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -31,7 +31,6 @@ import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting import androidx.core.view.postDelayed -import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.RecyclerView import androidx.viewpager2.widget.ViewPager2 @@ -85,7 +84,6 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParam import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA import javax.inject.Inject import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber @@ -170,18 +168,6 @@ open class BrowserActivity : DuckDuckGoActivity() { } } - init { - // lifecycleScope.launch { - // delay(5000) - // binding.tabPager.setCurrentItem(0, true) - // repeat(MAX_ACTIVE_TABS) { - // Timber.d("$$$ moving to #$it") - // binding.tabPager.setCurrentItem(it, true) - // delay(1000) - // } - // } - } - @VisibleForTesting var destroyedByBackPress: Boolean = false @@ -377,18 +363,22 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.command.observe(this) { processCommand(it) } + viewModel.selectedTab.observe(this) { tabManager.onSelectedTabChanged(it) } + viewModel.tabs.observe(this) { updateTabs(it) + + lifecycleScope.launch { + viewModel.onTabsUpdated(it.isEmpty()) + } } // listen to onboarding completion to enable/disable swiping - lifecycleScope.launch { - viewModel.isOnboardingCompleted.flowWithLifecycle(lifecycle).collectLatest { isOnboardingCompleted -> - binding.tabPager.isUserInputEnabled = isOnboardingCompleted - } + viewModel.isOnboardingCompleted.observe(this) { isOnboardingCompleted -> + binding.tabPager.isUserInputEnabled = isOnboardingCompleted } } @@ -396,6 +386,7 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.command.removeObservers(this) viewModel.selectedTab.removeObservers(this) viewModel.tabs.removeObservers(this) + viewModel.isOnboardingCompleted.removeObservers(this) } private fun processCommand(command: Command) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 3ab357fc0e3a..8b1cbfa57e94 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -62,12 +62,8 @@ import com.duckduckgo.feature.toggles.api.Toggle import javax.inject.Inject import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import timber.log.Timber @@ -113,15 +109,15 @@ class BrowserViewModel @Inject constructor( get() = viewState.value!! val command: SingleLiveEvent = SingleLiveEvent() + val selectedTab: LiveData = tabRepository.liveSelectedTab.distinctUntilChanged() - val tabs: LiveData> = tabRepository.flowTabs - .distinctUntilChanged() - .onEach { onTabsUpdated(it.isEmpty()) } - .asLiveData() - val isOnboardingCompleted: Flow = userStageStore.currentAppStage + val tabs: LiveData> = tabRepository.liveTabs.distinctUntilChanged() + + val isOnboardingCompleted: LiveData = userStageStore.currentAppStage + .distinctUntilChanged() .map { it != AppStage.DAX_ONBOARDING } - .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), false) + .asLiveData() private var dataClearingObserver = Observer { state -> when (state) { @@ -153,13 +149,6 @@ class BrowserViewModel @Inject constructor( init { appEnjoymentPromptEmitter.promptType.observeForever(appEnjoymentObserver) - - // viewModelScope.launch { - // repeat(50) { - // delay(1000) - // tabRepository.add("cnn.com") - // } - // } } suspend fun onNewTabRequested(sourceTabId: String? = null): String { From f9e92554955b863ef9ca1d2f3930b3a897cc1989 Mon Sep 17 00:00:00 2001 From: 0nko Date: Thu, 21 Nov 2024 21:48:35 +0100 Subject: [PATCH 37/47] Fix the merge --- .../main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt index c51d55304692..3347c40d4530 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt @@ -134,6 +134,8 @@ class RealTabManager @Inject constructor( lastActiveTabs.add(tab.tabId) + browserActivity.viewModel.onTabActivated(tab.tabId) + val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment if (fragment == null) { openNewTab( From ca31c567a607bb188a07ff715c093f9b5a0bce56 Mon Sep 17 00:00:00 2001 From: 0nko Date: Mon, 25 Nov 2024 11:15:30 +0100 Subject: [PATCH 38/47] Make selected tab flow nullable --- app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt | 2 +- .../java/com/duckduckgo/app/tabs/model/TabDataRepository.kt | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt index 6a2a89238dac..149e0569e362 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt @@ -41,7 +41,7 @@ abstract class TabsDao { abstract fun liveSelectedTab(): LiveData @Query("select * from tabs inner join tab_selection on tabs.tabId = tab_selection.tabId order by position limit 1") - abstract fun flowSelectedTab(): Flow + abstract fun flowSelectedTab(): Flow @Query("select * from tabs where deletable is 0 order by position") abstract fun tabs(): List 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 index 12f9aed26490..3ed4f17fff40 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt @@ -77,7 +77,7 @@ class TabDataRepository @Inject constructor( override val liveSelectedTab: LiveData = tabsDao.liveSelectedTab() - override val flowSelectedTab: Flow = tabsDao.flowSelectedTab() + override val flowSelectedTab: Flow = tabsDao.flowSelectedTab() override val tabSwitcherData: Flow = tabSwitcherDataStore.data @@ -330,7 +330,6 @@ class TabDataRepository @Inject constructor( databaseExecutor().scheduleDirect { val selection = TabSelectionEntity(tabId = tabId) tabsDao.insertTabSelection(selection) - Timber.d("$$$ select(): Tab selected: $tabId") } } From f186a6c5ec40ba2f3d6a20e2566609ecb691767f Mon Sep 17 00:00:00 2001 From: 0nko Date: Mon, 25 Nov 2024 11:30:27 +0100 Subject: [PATCH 39/47] Fix the tab update mechanism and only use 1 tab when swiping disabled --- .../duckduckgo/app/browser/BrowserActivity.kt | 13 ++-- .../app/browser/BrowserViewModel.kt | 5 +- .../app/browser/tabs/DefaultTabManager.kt | 48 ++++++++++---- .../duckduckgo/app/browser/tabs/TabManager.kt | 2 +- .../app/browser/tabs/TabPagerAdapter.kt | 63 ++++++++++++------- 5 files changed, 92 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 71119f33b0d3..fee45e93f936 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -30,7 +30,6 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting -import androidx.core.view.postDelayed import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.RecyclerView import androidx.viewpager2.widget.ViewPager2 @@ -67,7 +66,6 @@ import com.duckduckgo.app.settings.SettingsActivity import com.duckduckgo.app.settings.db.SettingsDataStore import com.duckduckgo.app.sitepermissions.SitePermissionsActivity import com.duckduckgo.app.statistics.pixels.Pixel -import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.appbuildconfig.api.AppBuildConfig import com.duckduckgo.autofill.api.emailprotection.EmailProtectionLinkVerifier import com.duckduckgo.browser.api.ui.BrowserScreens.BookmarksScreenNoParams @@ -214,6 +212,13 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.onLaunchedFromNotification(it) } configureOnBackPressedListener() + + // lifecycleScope.launch { + // repeat(100) { + // viewModel.onOpenInNewTabRequested("cnn.com") + // kotlinx.coroutines.delay(1000) + // } + // } } override fun onStop() { @@ -693,11 +698,11 @@ open class BrowserActivity : DuckDuckGoActivity() { val newInstanceState: Bundle?, ) - private fun updateTabs(entities: List) { + private fun updateTabs(tabIds: List) { val recyclerView = tabPager.getChildAt(0) as RecyclerView val state = recyclerView.layoutManager?.onSaveInstanceState() recyclerView.itemAnimator = null - tabManager.onTabsUpdated(entities) + tabManager.onTabsUpdated(tabIds) recyclerView.layoutManager?.onRestoreInstanceState(state) } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 141b3f25167c..f8ab8f57f230 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -22,6 +22,7 @@ import androidx.lifecycle.Observer import androidx.lifecycle.ViewModel import androidx.lifecycle.asLiveData import androidx.lifecycle.distinctUntilChanged +import androidx.lifecycle.map import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesRemoteFeature import com.duckduckgo.anvil.annotations.ContributesViewModel @@ -112,7 +113,9 @@ class BrowserViewModel @Inject constructor( val selectedTab: LiveData = tabRepository.liveSelectedTab.distinctUntilChanged() - val tabs: LiveData> = tabRepository.liveTabs.distinctUntilChanged() + val tabs: LiveData> = tabRepository.liveTabs + .map { tabs -> tabs.map { tab -> tab.tabId } } + .distinctUntilChanged() val isOnboardingCompleted: LiveData = userStageStore.currentAppStage .distinctUntilChanged() 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 f1bbc041a479..513cc30aacf9 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 @@ -25,6 +25,7 @@ import com.duckduckgo.app.browser.SwipingTabsFeature import com.duckduckgo.app.browser.tabs.TabManager.Companion.MAX_ACTIVE_TABS 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 dagger.SingleInstanceIn @@ -33,6 +34,7 @@ import javax.inject.Inject import kotlinx.coroutines.Job import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import timber.log.Timber @@ -43,23 +45,27 @@ class DefaultTabManager @Inject constructor( activity: DaggerActivity, private val swipingTabsFeature: SwipingTabsFeature, private val tabRepository: TabRepository, + private val dispatchers: DispatcherProvider, ) : TabManager { private val browserActivity = activity as BrowserActivity private val lastActiveTabs = TabList() private val supportFragmentManager = activity.supportFragmentManager private var openMessageInNewTabJob: Job? = null + private val keepSingleTab: Boolean + get() = !browserActivity.tabPager.isUserInputEnabled + override val tabPagerAdapter by lazy { TabPagerAdapter( fragmentManager = supportFragmentManager, - lifecycle = browserActivity.lifecycle, + lifecycleOwner = browserActivity, activityIntent = browserActivity.intent, moveToTabIndex = { index, smoothScroll -> browserActivity.onMoveToTabRequested(index, smoothScroll) }, getCurrentTabIndex = { browserActivity.tabPager.currentItem }, getSelectedTabId = ::getSelectedTab, getTabById = ::getTabById, requestNewTab = ::requestNewTab, - onTabSelected = ::getTabById, + onTabSelected = { tabId -> browserActivity.viewModel.onTabSelected(tabId) }, setOffScreenPageLimit = { limit -> browserActivity.tabPager.offscreenPageLimit = limit }, getOffScreenPageLimit = { browserActivity.tabPager.offscreenPageLimit }, ) @@ -82,17 +88,26 @@ class DefaultTabManager @Inject constructor( if (tab != null) { if (swipingTabsFeature.self().isEnabled()) { tabPagerAdapter.onSelectedTabChanged(tab.tabId) + if (keepSingleTab) { + tabPagerAdapter.onTabsUpdated(listOf(tab.tabId)) + } } else { selectTab(tab) } } } - override fun onTabsUpdated(updatedTabs: List) { + override fun onTabsUpdated(updatedTabIds: List) { if (swipingTabsFeature.self().isEnabled()) { - tabPagerAdapter.onTabsUpdated(updatedTabs.map { it.tabId }) + if (keepSingleTab) { + updatedTabIds.firstOrNull { it == getSelectedTab() }?.let { + tabPagerAdapter.onTabsUpdated(listOf(it)) + } + } else { + tabPagerAdapter.onTabsUpdated(updatedTabIds.map { it }) + } } else { - clearStaleTabs(updatedTabs) + clearStaleTabs(updatedTabIds) } } @@ -148,16 +163,27 @@ class DefaultTabManager @Inject constructor( openMessageInNewTabJob?.cancel() } - private fun requestNewTab(): TabEntity = runBlocking { + private fun requestNewTab(): TabEntity = runBlocking(dispatchers.io()) { + Timber.d("$$$ runBlocking requestNewTab") val tabId = browserActivity.viewModel.onNewTabRequested() - tabRepository.flowTabs.first().first { it.tabId == tabId } + return@runBlocking tabRepository.flowTabs.transformWhile { result -> + result.firstOrNull { it.tabId == tabId }?.let { entity -> + emit(entity) + Timber.d("$$$ requestNewTab: TAB FOUND") + return@transformWhile true + } + Timber.d("$$$ requestNewTab: TAB NOT FOUND") + return@transformWhile false + }.first() } - private fun getSelectedTab(): String? = runBlocking { + private fun getSelectedTab(): String? = runBlocking(dispatchers.io()) { + Timber.d("$$$ runBlocking getSelectedTab") tabRepository.flowSelectedTab.firstOrNull()?.tabId } - private fun getTabById(tabId: String): TabEntity? = runBlocking { + private fun getTabById(tabId: String): TabEntity? = runBlocking(dispatchers.io()) { + Timber.d("$$$ runBlocking getTabById") tabRepository.flowTabs.first().firstOrNull { it.tabId == tabId } } @@ -223,7 +249,7 @@ class DefaultTabManager @Inject constructor( transaction.commit() } - private fun clearStaleTabs(updatedTabs: List?) { + private fun clearStaleTabs(updatedTabs: List?) { if (swipingTabsFeature.self().isEnabled()) { return } @@ -234,7 +260,7 @@ class DefaultTabManager @Inject constructor( val stale = supportFragmentManager .fragments.mapNotNull { it as? BrowserTabFragment } - .filter { fragment -> updatedTabs.none { it.tabId == fragment.tabId } } + .filter { fragment -> updatedTabs.none { it == fragment.tabId } } if (stale.isNotEmpty()) { removeTabs(stale) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index a7bafeea705a..e3d7147ef1e9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -29,7 +29,7 @@ interface TabManager { val tabPagerAdapter: TabPagerAdapter fun onSelectedTabChanged(tab: TabEntity?) - fun onTabsUpdated(updatedTabs: List) + fun onTabsUpdated(updatedTabIds: List) fun openMessageInNewTab(message: Message, sourceTabId: String?) fun openExistingTab(tabId: String) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt index ec3d8778ea79..1582cd7eb8f9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt @@ -20,16 +20,15 @@ import android.content.Intent import android.os.Message import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager -import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner import androidx.recyclerview.widget.DiffUtil import androidx.viewpager2.adapter.FragmentStateAdapter import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment import com.duckduckgo.app.tabs.model.TabEntity -import timber.log.Timber class TabPagerAdapter( - lifecycle: Lifecycle, + lifecycleOwner: LifecycleOwner, private val fragmentManager: FragmentManager, private val activityIntent: Intent?, private val moveToTabIndex: (Int, Boolean) -> Unit, @@ -40,7 +39,7 @@ class TabPagerAdapter( private val onTabSelected: (String) -> Unit, private val getOffScreenPageLimit: () -> Int, private val setOffScreenPageLimit: (Int) -> Unit, -) : FragmentStateAdapter(fragmentManager, lifecycle) { +) : FragmentStateAdapter(fragmentManager, lifecycleOwner.lifecycle) { private val tabIds = mutableListOf() private var messageForNewFragment: Message? = null @@ -73,7 +72,10 @@ class TabPagerAdapter( this.messageFromPreviousTab = message } } else { - return BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, isExternal) + val existingFragment = fragmentManager.fragments + .filterIsInstance() + .firstOrNull { it.tabId == tab.tabId } + return existingFragment ?: BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, isExternal) } } @@ -81,36 +83,53 @@ class TabPagerAdapter( messageForNewFragment = message } + // var waiting = false + // fun onTabsUpdated(newTabs: List) = lifecycleOwner.lifecycleScope.launch { + // fun updateTabs(tabs: List, selectedId: String?) { + // val diffUtil = PagerDiffUtil(tabIds, tabs) + // val diff = DiffUtil.calculateDiff(diffUtil) + // tabIds.clear() + // tabIds.addAll(tabs) + // diff.dispatchUpdatesTo(this@TabPagerAdapter) + // + // if (selectedId != null) { + // onSelectedTabChanged(selectedId) + // } + // } + // + // if (tabIds != newTabs && !waiting) { + // val selectedId = getSelectedTabId() + // if (tabIds.isEmpty() && selectedId != null) { + // updateTabs(listOf(selectedId), selectedId) + // waiting = true + // delay(5000) + // } + // updateTabs(newTabs, selectedId) + // waiting = false + // } + // } + fun onTabsUpdated(newTabs: List) { - if (tabIds != newTabs) { - Timber.i("$$$ oldIds: ${tabIds.joinToString(",")}") - Timber.i("$$$ newIds: ${newTabs.joinToString(",")}") - - val diffUtil = PagerDiffUtil(tabIds, newTabs) - val diff = DiffUtil.calculateDiff(diffUtil) - tabIds.clear() - tabIds.addAll(newTabs) - diff.dispatchUpdatesTo(this) - - getSelectedTabId()?.let { - onSelectedTabChanged(it) - } + val diffUtil = PagerDiffUtil(tabIds, newTabs) + val diff = DiffUtil.calculateDiff(diffUtil) + tabIds.clear() + tabIds.addAll(newTabs) + diff.dispatchUpdatesTo(this) + + getSelectedTabId()?.let { selectedId -> + onSelectedTabChanged(selectedId) } } fun onSelectedTabChanged(tabId: String) { val selectedTabIndex = tabIds.indexOfFirst { it == tabId } - Timber.i("$$$ onSelectedTabChanged: selectedTabIndex $selectedTabIndex; currentTabIndex ${getCurrentTabIndex()}") if (selectedTabIndex != -1 && selectedTabIndex != getCurrentTabIndex()) { - Timber.i("$$$ onSelectedTabChanged:tabIds ${tabIds.joinToString(",")}") - Timber.i("$$$ onSelectedTabChanged: moving to index $selectedTabIndex ($tabId)") moveToTabIndex(selectedTabIndex, false) } } fun onPageChanged(position: Int) { if (position < tabIds.size) { - Timber.i("$$$ onPageChanged: moving to position $position (${tabIds[position]})") onTabSelected(tabIds[position]) } } From d7805770713409cce13e3b2393f951bb8d54c428 Mon Sep 17 00:00:00 2001 From: 0nko Date: Tue, 26 Nov 2024 22:36:28 +0100 Subject: [PATCH 40/47] Refactor the tab switching mechanism --- .../duckduckgo/app/browser/BrowserActivity.kt | 69 +++++++------ .../app/browser/BrowserTabFragment.kt | 10 +- .../app/browser/BrowserViewModel.kt | 14 ++- .../app/browser/tabs/DefaultTabManager.kt | 97 +++++++++---------- .../duckduckgo/app/browser/tabs/TabManager.kt | 3 +- .../app/browser/tabs/TabPagerAdapter.kt | 57 +++-------- .../app/tabs/model/TabDataRepository.kt | 4 + .../app/tabs/model/TabRepository.kt | 2 + 8 files changed, 120 insertions(+), 136 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index fee45e93f936..b9c3be9f4dad 100644 --- 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,9 @@ import androidx.activity.OnBackPressedCallback import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.VisibleForTesting +import androidx.lifecycle.flowWithLifecycle import androidx.lifecycle.lifecycleScope -import androidx.recyclerview.widget.RecyclerView +import androidx.viewpager2.widget.MarginPageTransformer import androidx.viewpager2.widget.ViewPager2 import androidx.webkit.ServiceWorkerClientCompat import androidx.webkit.ServiceWorkerControllerCompat @@ -73,6 +74,7 @@ import com.duckduckgo.common.ui.DuckDuckGoActivity import com.duckduckgo.common.ui.view.dialog.TextAlertDialogBuilder import com.duckduckgo.common.ui.view.gone import com.duckduckgo.common.ui.view.show +import com.duckduckgo.common.ui.view.toPx import com.duckduckgo.common.ui.viewbinding.viewBinding import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.playstore.PlayStoreUtils @@ -82,6 +84,7 @@ import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParam import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA import javax.inject.Inject import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber @@ -160,9 +163,23 @@ open class BrowserActivity : DuckDuckGoActivity() { private lateinit var toolbarMockupBinding: IncludeOmnibarToolbarMockupBinding private val onTabPageChangeListener = object : ViewPager2.OnPageChangeCallback() { + private var wasSwipingStarted = false + override fun onPageSelected(position: Int) { super.onPageSelected(position) - tabManager.tabPagerAdapter.onPageChanged(position) + Timber.d("### onPageChanged requested for: $position") + if (wasSwipingStarted) { + Timber.d("### onPageChanged: $position") + tabManager.tabPagerAdapter.onPageChanged(position) + wasSwipingStarted = false + } + } + + override fun onPageScrollStateChanged(state: Int) { + super.onPageScrollStateChanged(state) + if (state == ViewPager2.SCROLL_STATE_DRAGGING) { + wasSwipingStarted = true + } } } @@ -369,28 +386,29 @@ open class BrowserActivity : DuckDuckGoActivity() { processCommand(it) } - viewModel.selectedTab.observe(this) { - tabManager.onSelectedTabChanged(it) + lifecycleScope.launch { + viewModel.tabs.flowWithLifecycle(lifecycle).collectLatest { + tabManager.onTabsUpdated(it) + lifecycleScope.launch { + viewModel.onTabsUpdated(it.isEmpty()) + } + } } - viewModel.tabs.observe(this) { - updateTabs(it) - - lifecycleScope.launch { - viewModel.onTabsUpdated(it.isEmpty()) + lifecycleScope.launch { + viewModel.selectedTab.flowWithLifecycle(lifecycle).collectLatest { + tabManager.onSelectedTabChanged(it) } } // listen to onboarding completion to enable/disable swiping viewModel.isOnboardingCompleted.observe(this) { isOnboardingCompleted -> - binding.tabPager.isUserInputEnabled = isOnboardingCompleted + tabPager.isUserInputEnabled = isOnboardingCompleted } } private fun removeObservers() { viewModel.command.removeObservers(this) - viewModel.selectedTab.removeObservers(this) - viewModel.tabs.removeObservers(this) viewModel.isOnboardingCompleted.removeObservers(this) } @@ -576,15 +594,16 @@ open class BrowserActivity : DuckDuckGoActivity() { @SuppressLint("ClickableViewAccessibility", "WrongConstant") private fun initializeTabs() { if (swipingTabsFeature.self().isEnabled()) { - binding.tabPager.adapter = tabManager.tabPagerAdapter - binding.tabPager.offscreenPageLimit = 1 - binding.tabPager.registerOnPageChangeCallback(onTabPageChangeListener) + tabPager.adapter = tabManager.tabPagerAdapter + tabPager.offscreenPageLimit = 1 + tabPager.registerOnPageChangeCallback(onTabPageChangeListener) + tabPager.setPageTransformer(MarginPageTransformer(resources.getDimension(com.duckduckgo.mobile.android.R.dimen.keyline_2).toPx().toInt())) binding.fragmentContainer.gone() - binding.tabPager.show() + tabPager.show() } else { binding.fragmentContainer.show() - binding.tabPager.gone() + tabPager.gone() } } @@ -687,22 +706,14 @@ open class BrowserActivity : DuckDuckGoActivity() { playStoreUtils.launchPlayStore() } - fun onMoveToTabRequested(index: Int, smoothScroll: Boolean) { - binding.tabPager.unregisterOnPageChangeCallback(onTabPageChangeListener) - binding.tabPager.setCurrentItem(index, smoothScroll) - binding.tabPager.registerOnPageChangeCallback(onTabPageChangeListener) + fun onMoveToTabRequested(index: Int) { + Timber.d("### onMoveToTabRequested: $index") + + tabPager.currentItem = index } private data class CombinedInstanceState( val originalInstanceState: Bundle?, val newInstanceState: Bundle?, ) - - private fun updateTabs(tabIds: List) { - val recyclerView = tabPager.getChildAt(0) as RecyclerView - val state = recyclerView.layoutManager?.onSaveInstanceState() - recyclerView.itemAnimator = null - tabManager.onTabsUpdated(tabIds) - recyclerView.layoutManager?.onRestoreInstanceState(state) - } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index cee3b9b79a59..38e4503ac8a5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -1172,7 +1172,7 @@ class BrowserTabFragment : viewModel.onViewResumed() // onResume can be called for a hidden/backgrounded fragment, ensure this tab is visible. - if (fragmentIsVisible() && isActiveTab) { + if (fragmentIsVisible()) { viewModel.onViewVisible() } @@ -1358,7 +1358,7 @@ class BrowserTabFragment : newBrowserTab.newTabContainerLayout.show() binding.browserLayout.gone() webViewContainer.gone() - omnibar.setViewMode(Omnibar.ViewMode.NewTab) + omnibar.setViewMode(ViewMode.NewTab) webView?.onPause() webView?.hide() errorView.errorLayout.gone() @@ -1374,7 +1374,7 @@ class BrowserTabFragment : webView?.onResume() errorView.errorLayout.gone() sslErrorView.gone() - omnibar.setViewMode(Omnibar.ViewMode.Browser(viewModel.url)) + omnibar.setViewMode(ViewMode.Browser(viewModel.url)) } private fun showError( @@ -1385,7 +1385,7 @@ class BrowserTabFragment : newBrowserTab.newTabLayout.gone() newBrowserTab.newTabContainerLayout.gone() sslErrorView.gone() - omnibar.setViewMode(Omnibar.ViewMode.Error) + omnibar.setViewMode(ViewMode.Error) webView?.onPause() webView?.hide() errorView.errorMessage.text = getString(errorType.errorId, url).html(requireContext()) @@ -1406,7 +1406,7 @@ class BrowserTabFragment : newBrowserTab.newTabContainerLayout.gone() webView?.onPause() webView?.hide() - omnibar.setViewMode(Omnibar.ViewMode.SSLWarning) + omnibar.setViewMode(ViewMode.SSLWarning) errorView.errorLayout.gone() binding.browserLayout.gone() sslErrorView.bind(handler, errorResponse) { action -> diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index f8ab8f57f230..299265df570e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -21,8 +21,6 @@ import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer import androidx.lifecycle.ViewModel import androidx.lifecycle.asLiveData -import androidx.lifecycle.distinctUntilChanged -import androidx.lifecycle.map import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesRemoteFeature import com.duckduckgo.anvil.annotations.ContributesViewModel @@ -53,7 +51,6 @@ import com.duckduckgo.app.pixels.AppPixelName.APP_RATING_DIALOG_USER_DECLINED_RA import com.duckduckgo.app.pixels.AppPixelName.APP_RATING_DIALOG_USER_GAVE_RATING import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter -import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.SingleLiveEvent @@ -63,7 +60,10 @@ import com.duckduckgo.feature.toggles.api.Toggle import javax.inject.Inject import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filterNot +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import timber.log.Timber @@ -111,10 +111,14 @@ class BrowserViewModel @Inject constructor( val command: SingleLiveEvent = SingleLiveEvent() - val selectedTab: LiveData = tabRepository.liveSelectedTab.distinctUntilChanged() + val selectedTab: Flow = tabRepository.flowSelectedTab + .map { tab -> tab?.tabId } + .filterNotNull() + .distinctUntilChanged() - val tabs: LiveData> = tabRepository.liveTabs + val tabs: Flow> = tabRepository.flowTabs .map { tabs -> tabs.map { tab -> tab.tabId } } + .filterNot { it.isEmpty() } .distinctUntilChanged() val isOnboardingCompleted: LiveData = userStageStore.currentAppStage 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 513cc30aacf9..d29f94b7d496 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 @@ -33,7 +33,6 @@ import dagger.android.DaggerActivity import javax.inject.Inject import kotlinx.coroutines.Job import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking @@ -60,9 +59,8 @@ class DefaultTabManager @Inject constructor( fragmentManager = supportFragmentManager, lifecycleOwner = browserActivity, activityIntent = browserActivity.intent, - moveToTabIndex = { index, smoothScroll -> browserActivity.onMoveToTabRequested(index, smoothScroll) }, - getCurrentTabIndex = { browserActivity.tabPager.currentItem }, - getSelectedTabId = ::getSelectedTab, + moveToTabIndex = { index -> browserActivity.onMoveToTabRequested(index) }, + getSelectedTabId = ::getSelectedTabId, getTabById = ::getTabById, requestNewTab = ::requestNewTab, onTabSelected = { tabId -> browserActivity.viewModel.onTabSelected(tabId) }, @@ -84,27 +82,27 @@ class DefaultTabManager @Inject constructor( _currentTab = value } - override fun onSelectedTabChanged(tab: TabEntity?) { - if (tab != null) { - if (swipingTabsFeature.self().isEnabled()) { - tabPagerAdapter.onSelectedTabChanged(tab.tabId) - if (keepSingleTab) { - tabPagerAdapter.onTabsUpdated(listOf(tab.tabId)) - } - } else { - selectTab(tab) + override fun onSelectedTabChanged(tabId: String) { + if (swipingTabsFeature.self().isEnabled()) { + Timber.d("### TabManager.onSelectedTabChanged: $tabId") + tabPagerAdapter.onSelectedTabChanged(tabId) + if (keepSingleTab) { + tabPagerAdapter.onTabsUpdated(listOf(tabId)) } + } else { + selectTab(tabId) } } override fun onTabsUpdated(updatedTabIds: List) { + Timber.d("### TabManager.onTabsUpdated: $updatedTabIds") if (swipingTabsFeature.self().isEnabled()) { if (keepSingleTab) { - updatedTabIds.firstOrNull { it == getSelectedTab() }?.let { + updatedTabIds.firstOrNull { it == getSelectedTabId() }?.let { tabPagerAdapter.onTabsUpdated(listOf(it)) } } else { - tabPagerAdapter.onTabsUpdated(updatedTabIds.map { it }) + tabPagerAdapter.onTabsUpdated(updatedTabIds) } } else { clearStaleTabs(updatedTabIds) @@ -164,58 +162,53 @@ class DefaultTabManager @Inject constructor( } private fun requestNewTab(): TabEntity = runBlocking(dispatchers.io()) { - Timber.d("$$$ runBlocking requestNewTab") val tabId = browserActivity.viewModel.onNewTabRequested() return@runBlocking tabRepository.flowTabs.transformWhile { result -> result.firstOrNull { it.tabId == tabId }?.let { entity -> emit(entity) - Timber.d("$$$ requestNewTab: TAB FOUND") return@transformWhile true } - Timber.d("$$$ requestNewTab: TAB NOT FOUND") return@transformWhile false }.first() } - private fun getSelectedTab(): String? = runBlocking(dispatchers.io()) { - Timber.d("$$$ runBlocking getSelectedTab") - tabRepository.flowSelectedTab.firstOrNull()?.tabId + private fun getSelectedTabId(): String? = runBlocking { + tabRepository.getSelectedTab()?.tabId } - private fun getTabById(tabId: String): TabEntity? = runBlocking(dispatchers.io()) { - Timber.d("$$$ runBlocking getTabById") - tabRepository.flowTabs.first().firstOrNull { it.tabId == tabId } + private fun getTabById(tabId: String): TabEntity? = runBlocking { + tabRepository.getTab(tabId) } - private fun selectTab(tab: TabEntity) { - Timber.v("Select tab: $tab") - - if (tab.tabId == currentTab?.tabId) return - - lastActiveTabs.add(tab.tabId) - - browserActivity.viewModel.onTabActivated(tab.tabId) - - val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment - if (fragment == null) { - openNewTab( - tabId = tab.tabId, - url = tab.url, - skipHome = tab.skipHome, - isExternal = browserActivity.intent?.getBooleanExtra( - BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, - false, - ) == true, - ) - return - } - val transaction = supportFragmentManager.beginTransaction() - currentTab?.let { - transaction.hide(it) + private fun selectTab(tabId: String) = browserActivity.lifecycleScope.launch { + if (tabId != currentTab?.tabId) { + lastActiveTabs.add(tabId) + + browserActivity.viewModel.onTabActivated(tabId) + + val fragment = supportFragmentManager.findFragmentByTag(tabId) as? BrowserTabFragment + if (fragment == null) { + tabRepository.getTab(tabId)?.let { tab -> + openNewTab( + tabId = tab.tabId, + url = tab.url, + skipHome = tab.skipHome, + isExternal = browserActivity.intent?.getBooleanExtra( + BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, + false, + ) == true, + ) + } + return@launch + } + val transaction = supportFragmentManager.beginTransaction() + currentTab?.let { + transaction.hide(it) + } + transaction.show(fragment) + transaction.commit() + currentTab = fragment } - transaction.show(fragment) - transaction.commit() - currentTab = fragment } private fun openNewTab( diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt index e3d7147ef1e9..0c0c450eaee7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabManager.kt @@ -18,7 +18,6 @@ package com.duckduckgo.app.browser.tabs import android.os.Message import com.duckduckgo.app.browser.BrowserTabFragment -import com.duckduckgo.app.tabs.model.TabEntity interface TabManager { companion object { @@ -28,7 +27,7 @@ interface TabManager { var currentTab: BrowserTabFragment? val tabPagerAdapter: TabPagerAdapter - fun onSelectedTabChanged(tab: TabEntity?) + fun onSelectedTabChanged(tabId: String) fun onTabsUpdated(updatedTabIds: List) fun openMessageInNewTab(message: Message, sourceTabId: String?) diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt index 1582cd7eb8f9..f9192601aba3 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/tabs/TabPagerAdapter.kt @@ -16,6 +16,7 @@ package com.duckduckgo.app.browser.tabs +import android.annotation.SuppressLint import android.content.Intent import android.os.Message import androidx.fragment.app.Fragment @@ -26,16 +27,16 @@ import androidx.viewpager2.adapter.FragmentStateAdapter import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment import com.duckduckgo.app.tabs.model.TabEntity +import timber.log.Timber class TabPagerAdapter( lifecycleOwner: LifecycleOwner, private val fragmentManager: FragmentManager, private val activityIntent: Intent?, - private val moveToTabIndex: (Int, Boolean) -> Unit, - private val getCurrentTabIndex: () -> Int, - private val getSelectedTabId: () -> String?, + private val moveToTabIndex: (Int) -> Unit, private val getTabById: (String) -> TabEntity?, private val requestNewTab: () -> TabEntity, + private val getSelectedTabId: () -> String?, private val onTabSelected: (String) -> Unit, private val getOffScreenPageLimit: () -> Int, private val setOffScreenPageLimit: (Int) -> Unit, @@ -43,7 +44,7 @@ class TabPagerAdapter( private val tabIds = mutableListOf() private var messageForNewFragment: Message? = null - override fun getItemCount(): Int = tabIds.size + override fun getItemCount() = tabIds.size override fun getItemId(position: Int) = tabIds[position].hashCode().toLong() @@ -60,22 +61,20 @@ class TabPagerAdapter( .filter { it.isInitialized }.size override fun createFragment(position: Int): Fragment { + Timber.d("### TabPagerAdapter.createFragment: $position") increaseOffscreenTabLimitIfNeeded() val tab = getTabById(tabIds[position]) ?: requestNewTab() val isExternal = activityIntent?.getBooleanExtra(BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, false) == true - if (messageForNewFragment != null) { + return if (messageForNewFragment != null) { val message = messageForNewFragment messageForNewFragment = null return BrowserTabFragment.newInstance(tab.tabId, null, false, isExternal).apply { this.messageFromPreviousTab = message } } else { - val existingFragment = fragmentManager.fragments - .filterIsInstance() - .firstOrNull { it.tabId == tab.tabId } - return existingFragment ?: BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, isExternal) + BrowserTabFragment.newInstance(tab.tabId, tab.url, tab.skipHome, isExternal) } } @@ -83,48 +82,20 @@ class TabPagerAdapter( messageForNewFragment = message } - // var waiting = false - // fun onTabsUpdated(newTabs: List) = lifecycleOwner.lifecycleScope.launch { - // fun updateTabs(tabs: List, selectedId: String?) { - // val diffUtil = PagerDiffUtil(tabIds, tabs) - // val diff = DiffUtil.calculateDiff(diffUtil) - // tabIds.clear() - // tabIds.addAll(tabs) - // diff.dispatchUpdatesTo(this@TabPagerAdapter) - // - // if (selectedId != null) { - // onSelectedTabChanged(selectedId) - // } - // } - // - // if (tabIds != newTabs && !waiting) { - // val selectedId = getSelectedTabId() - // if (tabIds.isEmpty() && selectedId != null) { - // updateTabs(listOf(selectedId), selectedId) - // waiting = true - // delay(5000) - // } - // updateTabs(newTabs, selectedId) - // waiting = false - // } - // } - + @SuppressLint("NotifyDataSetChanged") fun onTabsUpdated(newTabs: List) { - val diffUtil = PagerDiffUtil(tabIds, newTabs) - val diff = DiffUtil.calculateDiff(diffUtil) + val diff = DiffUtil.calculateDiff(PagerDiffUtil(tabIds, newTabs)) + diff.dispatchUpdatesTo(this) tabIds.clear() tabIds.addAll(newTabs) - diff.dispatchUpdatesTo(this) - getSelectedTabId()?.let { selectedId -> - onSelectedTabChanged(selectedId) - } + onSelectedTabChanged(getSelectedTabId() ?: tabIds.first()) } fun onSelectedTabChanged(tabId: String) { val selectedTabIndex = tabIds.indexOfFirst { it == tabId } - if (selectedTabIndex != -1 && selectedTabIndex != getCurrentTabIndex()) { - moveToTabIndex(selectedTabIndex, false) + if (selectedTabIndex != -1) { + moveToTabIndex(selectedTabIndex) } } 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 index 3ed4f17fff40..452834a954ad 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt @@ -333,6 +333,10 @@ class TabDataRepository @Inject constructor( } } + override suspend fun getTab(tabId: String): TabEntity? { + return withContext(dispatchers.io()) { tabsDao.tab(tabId) } + } + override fun updateTabFavicon( tabId: String, fileName: String?, 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 index 84ed5aa8fdff..310c1f92f5d8 100644 --- 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 @@ -101,6 +101,8 @@ interface TabRepository { suspend fun select(tabId: String) + suspend fun getTab(tabId: String): TabEntity? + fun updateTabPreviewImage( tabId: String, fileName: String?, From fda2287e455d69ccccabfa22b15748898045ca96 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 01:12:37 +0100 Subject: [PATCH 41/47] Fix the merge --- .../duckduckgo/app/browser/BrowserActivity.kt | 7 +- .../app/browser/tabs/DefaultTabManager.kt | 16 +- .../app/browser/tabs/RealTabManager.kt | 243 ------------------ 3 files changed, 13 insertions(+), 253 deletions(-) delete mode 100644 app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 64d7d71192c5..ae3bc0e49ec1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -82,11 +82,11 @@ import com.duckduckgo.di.scopes.ActivityScope import com.duckduckgo.navigation.api.GlobalActivityStarter import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.PrivacyDashboardPrimaryScreen import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA -import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber +import javax.inject.Inject // open class so that we can test BrowserApplicationStateInfo @InjectWith(ActivityScope::class) @@ -134,6 +134,9 @@ open class BrowserActivity : DuckDuckGoActivity() { @Inject lateinit var appBuildConfig: AppBuildConfig + @Inject + lateinit var isSwipingTabsFeatureEnabled: IsSwipingTabsFeatureEnabled + @Inject lateinit var tabManager: TabManager @@ -590,7 +593,7 @@ open class BrowserActivity : DuckDuckGoActivity() { @SuppressLint("ClickableViewAccessibility", "WrongConstant") private fun initializeTabs() { - if (swipingTabsFeature.self().isEnabled()) { + if (isSwipingTabsFeatureEnabled()) { tabPager.adapter = tabManager.tabPagerAdapter tabPager.offscreenPageLimit = 1 tabPager.registerOnPageChangeCallback(onTabPageChangeListener) 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 d29f94b7d496..cd6dd95ed9bd 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 @@ -20,8 +20,8 @@ import android.os.Message import androidx.lifecycle.lifecycleScope import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.BrowserTabFragment +import com.duckduckgo.app.browser.IsSwipingTabsFeatureEnabled import com.duckduckgo.app.browser.R -import com.duckduckgo.app.browser.SwipingTabsFeature import com.duckduckgo.app.browser.tabs.TabManager.Companion.MAX_ACTIVE_TABS import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.app.tabs.model.TabRepository @@ -30,19 +30,19 @@ import com.duckduckgo.di.scopes.ActivityScope import com.squareup.anvil.annotations.ContributesBinding import dagger.SingleInstanceIn import dagger.android.DaggerActivity -import javax.inject.Inject import kotlinx.coroutines.Job import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import timber.log.Timber +import javax.inject.Inject @ContributesBinding(ActivityScope::class) @SingleInstanceIn(ActivityScope::class) class DefaultTabManager @Inject constructor( activity: DaggerActivity, - private val swipingTabsFeature: SwipingTabsFeature, + private val isSwipingTabsFeatureEnabled: IsSwipingTabsFeatureEnabled, private val tabRepository: TabRepository, private val dispatchers: DispatcherProvider, ) : TabManager { @@ -72,7 +72,7 @@ class DefaultTabManager @Inject constructor( private var _currentTab: BrowserTabFragment? = null override var currentTab: BrowserTabFragment? get() { - return if (swipingTabsFeature.self().isEnabled()) { + return if (isSwipingTabsFeatureEnabled()) { tabPagerAdapter.currentFragment } else { _currentTab @@ -83,7 +83,7 @@ class DefaultTabManager @Inject constructor( } override fun onSelectedTabChanged(tabId: String) { - if (swipingTabsFeature.self().isEnabled()) { + if (isSwipingTabsFeatureEnabled()) { Timber.d("### TabManager.onSelectedTabChanged: $tabId") tabPagerAdapter.onSelectedTabChanged(tabId) if (keepSingleTab) { @@ -96,7 +96,7 @@ class DefaultTabManager @Inject constructor( override fun onTabsUpdated(updatedTabIds: List) { Timber.d("### TabManager.onTabsUpdated: $updatedTabIds") - if (swipingTabsFeature.self().isEnabled()) { + if (isSwipingTabsFeatureEnabled()) { if (keepSingleTab) { updatedTabIds.firstOrNull { it == getSelectedTabId() }?.let { tabPagerAdapter.onTabsUpdated(listOf(it)) @@ -113,7 +113,7 @@ class DefaultTabManager @Inject constructor( message: Message, sourceTabId: String?, ) { - if (swipingTabsFeature.self().isEnabled()) { + if (isSwipingTabsFeatureEnabled()) { openMessageInNewTabJob = browserActivity.lifecycleScope.launch { tabPagerAdapter.setMessageForNewFragment(message) browserActivity.viewModel.onNewTabRequested(sourceTabId) @@ -243,7 +243,7 @@ class DefaultTabManager @Inject constructor( } private fun clearStaleTabs(updatedTabs: List?) { - if (swipingTabsFeature.self().isEnabled()) { + if (isSwipingTabsFeatureEnabled()) { return } diff --git a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt b/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt deleted file mode 100644 index a9cecb61e5cd..000000000000 --- a/app/src/main/java/com/duckduckgo/app/browser/tabs/RealTabManager.kt +++ /dev/null @@ -1,243 +0,0 @@ -/* - * Copyright (c) 2024 DuckDuckGo - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.duckduckgo.app.browser.tabs - -import android.os.Message -import androidx.lifecycle.lifecycleScope -import com.duckduckgo.app.browser.BrowserActivity -import com.duckduckgo.app.browser.BrowserTabFragment -import com.duckduckgo.app.browser.IsSwipingTabsFeatureEnabled -import com.duckduckgo.app.browser.R -import com.duckduckgo.app.tabs.model.TabEntity -import com.duckduckgo.di.scopes.ActivityScope -import com.squareup.anvil.annotations.ContributesBinding -import dagger.SingleInstanceIn -import dagger.android.DaggerActivity -import javax.inject.Inject -import kotlinx.coroutines.Job -import kotlinx.coroutines.launch -import timber.log.Timber - -@ContributesBinding(ActivityScope::class) -@SingleInstanceIn(ActivityScope::class) -class RealTabManager @Inject constructor( - activity: DaggerActivity, - private val isSwipingTabsFeatureEnabled: IsSwipingTabsFeatureEnabled, -) : TabManager { - companion object { - private const val MAX_ACTIVE_TABS = 40 - } - - private val browserActivity = activity as BrowserActivity - private val lastActiveTabs = TabList() - private val supportFragmentManager = activity.supportFragmentManager - private var openMessageInNewTabJob: Job? = null - - private var _currentTab: BrowserTabFragment? = null - override var currentTab: BrowserTabFragment? - get() { - return if (isSwipingTabsFeatureEnabled()) { - null - } else { - _currentTab - } - } - set(value) { - _currentTab = value - } - - override fun onSelectedTabChanged(tab: TabEntity?) { - if (isSwipingTabsFeatureEnabled()) { - return - } else if (tab != null) { - selectTab(tab) - } - } - - override fun onTabsUpdated(updatedTabs: List) { - if (isSwipingTabsFeatureEnabled()) { - return - } else { - clearStaleTabs(updatedTabs) - } - } - - override fun openMessageInNewTab( - message: Message, - sourceTabId: String?, - ) { - openMessageInNewTabJob = browserActivity.lifecycleScope.launch { - val tabId = browserActivity.viewModel.onNewTabRequested(sourceTabId) - val fragment = openNewTab( - tabId = tabId, - url = null, - skipHome = false, - isExternal = browserActivity.intent?.getBooleanExtra( - BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, - false, - ) ?: false, - ) - fragment.messageFromPreviousTab = message - } - } - - override fun openExistingTab(tabId: String) { - browserActivity.lifecycleScope.launch { - browserActivity.viewModel.onTabSelected(tabId) - } - } - - override fun launchNewTab() { - browserActivity.lifecycleScope.launch { browserActivity.viewModel.onNewTabRequested() } - } - - override fun openQueryInNewTab( - query: String, - sourceTabId: String?, - ) { - browserActivity.lifecycleScope.launch { - browserActivity.viewModel.onOpenInNewTabRequested( - query = query, - sourceTabId = sourceTabId, - ) - } - } - - override fun onCleanup() { - openMessageInNewTabJob?.cancel() - } - - private fun selectTab(tab: TabEntity?) { - if (isSwipingTabsFeatureEnabled()) { - return - } - - Timber.v("Select tab: $tab") - - if (tab == null) return - - if (tab.tabId == currentTab?.tabId) return - - lastActiveTabs.add(tab.tabId) - - browserActivity.viewModel.onTabActivated(tab.tabId) - - val fragment = supportFragmentManager.findFragmentByTag(tab.tabId) as? BrowserTabFragment - if (fragment == null) { - openNewTab( - tabId = tab.tabId, - url = tab.url, - skipHome = tab.skipHome, - isExternal = browserActivity.intent?.getBooleanExtra( - BrowserActivity.LAUNCH_FROM_EXTERNAL_EXTRA, - false, - ) ?: false, - ) - return - } - val transaction = supportFragmentManager.beginTransaction() - currentTab?.let { - transaction.hide(it) - } - transaction.show(fragment) - transaction.commit() - currentTab = fragment - } - - private fun openNewTab( - tabId: String, - url: String? = null, - skipHome: Boolean, - isExternal: Boolean, - ): BrowserTabFragment { - Timber.i("Opening new tab, url: $url, tabId: $tabId") - val fragment = BrowserTabFragment.newInstance(tabId, url, skipHome, isExternal) - addOrReplaceNewTab(fragment, tabId) - currentTab = fragment - return fragment - } - - private fun addOrReplaceNewTab( - fragment: BrowserTabFragment, - tabId: String, - ) { - if (supportFragmentManager.isStateSaved) { - return - } - val transaction = supportFragmentManager.beginTransaction() - val tab = currentTab - if (tab == null) { - transaction.replace(R.id.fragmentContainer, fragment, tabId) - } else { - transaction.hide(tab) - transaction.add(R.id.fragmentContainer, fragment, tabId) - } - transaction.commit() - } - - private fun clearStaleTabs(updatedTabs: List?) { - if (isSwipingTabsFeatureEnabled()) { - return - } - - if (updatedTabs == null) { - return - } - - val stale = supportFragmentManager - .fragments.mapNotNull { it as? BrowserTabFragment } - .filter { fragment -> updatedTabs.none { it.tabId == fragment.tabId } } - - if (stale.isNotEmpty()) { - removeTabs(stale) - } - - removeOldTabs() - } - - private fun removeOldTabs() { - val candidatesToRemove = lastActiveTabs.dropLast(MAX_ACTIVE_TABS) - if (candidatesToRemove.isEmpty()) return - - val tabsToRemove = supportFragmentManager.fragments - .mapNotNull { it as? BrowserTabFragment } - .filter { candidatesToRemove.contains(it.tabId) } - - if (tabsToRemove.isNotEmpty()) { - removeTabs(tabsToRemove) - } - } - - private fun removeTabs(fragments: List) { - val transaction = supportFragmentManager.beginTransaction() - fragments.forEach { - transaction.remove(it) - lastActiveTabs.remove(it.tabId) - } - transaction.commit() - } - - // Temporary class to keep track of latest visited tabs, keeping unique ids. - private class TabList : ArrayList() { - override fun add(element: String): Boolean { - if (this.contains(element)) { - this.remove(element) - } - return super.add(element) - } - } -} From dcef654b4654fb39cb34a0d86fed854bb831ec3a Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 01:13:57 +0100 Subject: [PATCH 42/47] Enable the tab swiping internally --- .../main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt b/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt index cc47d0977e1b..b59c6c35ddd2 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/SwipingTabsFeature.kt @@ -26,5 +26,6 @@ import com.duckduckgo.feature.toggles.api.Toggle ) interface SwipingTabsFeature { @Toggle.DefaultValue(false) + @Toggle.InternalAlwaysEnabled fun self(): Toggle } From b5b0df7dd89f4e0f32f0dc9ae3933a4ae1dd305b Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 01:15:12 +0100 Subject: [PATCH 43/47] Optimize imports --- app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt | 2 +- .../java/com/duckduckgo/app/browser/tabs/DefaultTabManager.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index ae3bc0e49ec1..4fab45f8bd92 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -82,11 +82,11 @@ import com.duckduckgo.di.scopes.ActivityScope import com.duckduckgo.navigation.api.GlobalActivityStarter import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.PrivacyDashboardPrimaryScreen import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA +import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.launch import timber.log.Timber -import javax.inject.Inject // open class so that we can test BrowserApplicationStateInfo @InjectWith(ActivityScope::class) 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 cd6dd95ed9bd..d8228cae29e3 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 @@ -30,13 +30,13 @@ import com.duckduckgo.di.scopes.ActivityScope import com.squareup.anvil.annotations.ContributesBinding import dagger.SingleInstanceIn import dagger.android.DaggerActivity +import javax.inject.Inject import kotlinx.coroutines.Job import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import timber.log.Timber -import javax.inject.Inject @ContributesBinding(ActivityScope::class) @SingleInstanceIn(ActivityScope::class) From dbe281ccdd65d4cc15c7670244a7aa1da027fc62 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 01:22:29 +0100 Subject: [PATCH 44/47] Clean up --- .../java/com/duckduckgo/app/browser/BrowserActivity.kt | 7 ------- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++++ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 4fab45f8bd92..d296f3f654c4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -229,13 +229,6 @@ open class BrowserActivity : DuckDuckGoActivity() { viewModel.onLaunchedFromNotification(it) } configureOnBackPressedListener() - - // lifecycleScope.launch { - // repeat(100) { - // viewModel.onOpenInNewTabRequested("cnn.com") - // kotlinx.coroutines.delay(1000) - // } - // } } override fun onStop() { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 18306932dab3..f9a2ec73a65d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -864,6 +864,10 @@ class BrowserTabFragment : dialog.deleteBookmarkListener = viewModel } + disableSwipingOutsideTheOmnibar() + } + + private fun disableSwipingOutsideTheOmnibar() { newBrowserTab.newTabLayout.setOnTouchListener { v, event -> (v as FrameLayout).requestDisallowInterceptTouchEvent(true) return@setOnTouchListener true From bb80ee39d88a6f50af7e0af5b21eedaf201d74a2 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 01:42:25 +0100 Subject: [PATCH 45/47] Fix unit tests --- .../com/duckduckgo/app/browser/BrowserViewModelTest.kt | 9 ++++++--- .../ShowOnAppLaunchOptionHandlerImplTest.kt | 6 ++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index 5eda04d9d419..97617b523428 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -29,10 +29,10 @@ import com.duckduckgo.app.global.rating.AppEnjoymentPromptEmitter import com.duckduckgo.app.global.rating.AppEnjoymentPromptOptions import com.duckduckgo.app.global.rating.AppEnjoymentUserEventRecorder import com.duckduckgo.app.global.rating.PromptCount +import com.duckduckgo.app.onboarding.store.UserStageStore import com.duckduckgo.app.pixels.AppPixelName import com.duckduckgo.app.statistics.pixels.Pixel import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter -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 @@ -81,6 +81,8 @@ class BrowserViewModelTest { @Mock private lateinit var showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler + @Mock private lateinit var userStageStore: UserStageStore + private val fakeShowOnAppLaunchFeatureToggle = FakeFeatureToggleFactory.create(ShowOnAppLaunchFeature::class.java) private lateinit var testee: BrowserViewModel @@ -146,13 +148,13 @@ class BrowserViewModelTest { @Test fun whenTabsUpdatedAndNoTabsThenDefaultTabAddedToRepository() = runTest { - testee.onTabsUpdated(ArrayList()) + testee.onTabsUpdated(areTabsEmpty = true) verify(mockTabRepository).addDefaultTab() } @Test fun whenTabsUpdatedWithTabsThenNewTabNotLaunched() = runTest { - testee.onTabsUpdated(listOf(TabEntity(TAB_ID, "", "", skipHome = false, viewed = true, position = 0))) + testee.onTabsUpdated(areTabsEmpty = false) verify(mockCommandObserver, never()).onChanged(any()) } @@ -299,6 +301,7 @@ class BrowserViewModelTest { skipUrlConversionOnNewTabFeature = skipUrlConversionOnNewTabFeature, showOnAppLaunchFeature = fakeShowOnAppLaunchFeatureToggle, showOnAppLaunchOptionHandler = showOnAppLaunchOptionHandler, + userStageStore = userStageStore, ) } diff --git a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt index 0067de644847..9070e673141d 100644 --- a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt +++ b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt @@ -754,9 +754,15 @@ class ShowOnAppLaunchOptionHandlerImplTest { get() = TODO("Not yet implemented") override val liveSelectedTab: LiveData get() = TODO("Not yet implemented") + override val flowSelectedTab: Flow + get() = TODO("Not yet implemented") override val tabSwitcherData: Flow get() = TODO("Not yet implemented") + override suspend fun getTab(tabId: String): TabEntity? { + TODO("Not yet implemented") + } + override suspend fun addDefaultTab(): String { TODO("Not yet implemented") } From 181dbb18a3e2575636fdb63caf481372500574a5 Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 11:34:10 +0100 Subject: [PATCH 46/47] Fix the logic for new tab creation on long press --- .../com/duckduckgo/app/browser/BrowserTabViewModel.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 8575a2b1f2c4..0be337eff5b5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -2617,17 +2617,18 @@ class BrowserTabViewModel @Inject constructor( } fun userRequestedOpeningNewTab(longPress: Boolean = false) { + command.value = GenerateWebViewPreviewImage + val emptyTab = tabs.value?.firstOrNull { it.url.isNullOrBlank() }?.tabId if (emptyTab != null) { viewModelScope.launch { tabRepository.select(tabId = emptyTab) } - } else if (!url.isNullOrEmpty() || (tabs.value?.size ?: 0) > 1) { - command.value = GenerateWebViewPreviewImage + } else { command.value = LaunchNewTab - if (longPress) { - pixel.fire(AppPixelName.TAB_MANAGER_NEW_TAB_LONG_PRESSED) - } + } + if (longPress) { + pixel.fire(AppPixelName.TAB_MANAGER_NEW_TAB_LONG_PRESSED) } onUserDismissedCta(ctaViewState.value?.cta) } From 75535ba01d366ca14bc7921101b1a8341bba36ce Mon Sep 17 00:00:00 2001 From: 0nko Date: Wed, 27 Nov 2024 11:35:08 +0100 Subject: [PATCH 47/47] Fix the uni tests --- .../app/browser/BrowserTabViewModelTest.kt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index d88ee4acadc2..b8f330911929 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -2423,7 +2423,8 @@ class BrowserTabViewModelTest { } @Test - fun whenUserRequestedToOpenNewTabThenNewTabCommandIssued() { + fun whenUserRequestedToOpenNewTabAndNoEmptyTabExistsThenNewTabCommandIssued() { + tabsLiveData.value = listOf(TabEntity("1", "https://example.com", position = 0)) testee.userRequestedOpeningNewTab() verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) val command = commandCaptor.lastValue @@ -2431,6 +2432,20 @@ class BrowserTabViewModelTest { verify(mockPixel, never()).fire(AppPixelName.TAB_MANAGER_NEW_TAB_LONG_PRESSED) } + @Test + fun whenUserRequestedToOpenNewTabAndEmptyTabExistsThenSelectTheEmptyTab() = runTest { + val emptyTabId = "EMPTY_TAB" + tabsLiveData.value = listOf(TabEntity(emptyTabId)) + testee.userRequestedOpeningNewTab() + + verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) + val command = commandCaptor.lastValue + assertFalse(command is Command.LaunchNewTab) + + verify(mockTabRepository).select(emptyTabId) + verify(mockPixel, never()).fire(AppPixelName.TAB_MANAGER_NEW_TAB_LONG_PRESSED) + } + @Test fun whenUserRequestedToOpenNewTabByLongPressThenPixelFired() { testee.userRequestedOpeningNewTab(longPress = true)