Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swiping tabs: Refactoring #5264

Open
wants to merge 19 commits into
base: feature/ondrej/swiping-tabs
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 21 additions & 153 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -43,15 +42,18 @@ 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
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
Expand All @@ -62,7 +64,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
Expand All @@ -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

Expand Down Expand Up @@ -129,11 +129,19 @@ open class BrowserActivity : DuckDuckGoActivity() {
@Inject
lateinit var appBuildConfig: AppBuildConfig

private val lastActiveTabs = TabList()
@Inject
lateinit var swipingTabsFeature: SwipingTabsFeature

@Inject
lateinit var tabManager: TabManager

private var currentTab: BrowserTabFragment? = null
private var currentTab: BrowserTabFragment?
get() = tabManager.currentTab
set(value) {
tabManager.currentTab = value
}

private val viewModel: BrowserViewModel by bindViewModel()
val viewModel: BrowserViewModel by bindViewModel()

private var instanceStateBundles: CombinedInstanceState? = null

Expand All @@ -145,8 +153,6 @@ open class BrowserActivity : DuckDuckGoActivity() {

private lateinit var toolbarMockupBinding: IncludeOmnibarToolbarMockupBinding

private var openMessageInNewTabJob: Job? = null

@VisibleForTesting
var destroyedByBackPress: Boolean = false

Expand Down Expand Up @@ -196,7 +202,7 @@ open class BrowserActivity : DuckDuckGoActivity() {
}

override fun onStop() {
openMessageInNewTabJob?.cancel()
tabManager.onCleanup()
super.onStop()
}

Expand Down Expand Up @@ -232,71 +238,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)

viewModel.onTabActivated(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<BrowserTabFragment>) {
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()
Expand Down Expand Up @@ -347,13 +288,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
}

Expand Down Expand Up @@ -404,14 +345,10 @@ open class BrowserActivity : DuckDuckGoActivity() {
processCommand(it)
}
viewModel.selectedTab.observe(this) {
if (it != null) {
selectTab(it)
}
tabManager.onSelectedTabChanged(it)
}
viewModel.tabs.observe(this) {
clearStaleTabs(it)
removeOldTabs()
lifecycleScope.launch { viewModel.onTabsUpdated(it) }
tabManager.onTabsUpdated(it)
}
}

Expand All @@ -421,33 +358,6 @@ open class BrowserActivity : DuckDuckGoActivity() {
viewModel.tabs.removeObservers(this)
}

private fun clearStaleTabs(updatedTabs: List<TabEntity>?) {
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) {
Expand Down Expand Up @@ -497,36 +407,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?,
) {
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
}
}

fun openExistingTab(tabId: String) {
lifecycleScope.launch {
viewModel.onTabSelected(tabId)
}
}

fun launchSettings() {
startActivity(SettingsActivity.intent(this))
}
Expand Down Expand Up @@ -615,10 +495,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 {
Expand Down Expand Up @@ -762,13 +640,3 @@ open class BrowserActivity : DuckDuckGoActivity() {
val newInstanceState: Bundle?,
)
}

// Temporary class to keep track of latest visited tabs, keeping unique ids.
private class TabList() : ArrayList<String>() {
override fun add(element: String): Boolean {
if (this.contains(element)) {
this.remove(element)
}
return super.add(element)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,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
Expand Down Expand Up @@ -521,6 +522,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
Expand Down Expand Up @@ -1486,7 +1490,7 @@ class BrowserTabFragment :
when (it) {
is NavigationCommand.Refresh -> refresh()
is Command.OpenInNewTab -> {
browserActivity?.openInNewTab(it.query, it.sourceTabId)
tabManager.openQueryInNewTab(it.query, it.sourceTabId)
}

is Command.OpenMessageInNewTab -> {
Expand All @@ -1498,15 +1502,15 @@ class BrowserTabFragment :
isLaunchedFromExternalApp,
)
} else {
browserActivity?.openMessageInNewTab(it.message, it.sourceTabId)
tabManager.openMessageInNewTab(it.message, it.sourceTabId)
}
}

is Command.OpenInNewBackgroundTab -> {
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(
Expand Down Expand Up @@ -1739,7 +1743,7 @@ class BrowserTabFragment :
viewModel.autoCompleteSuggestionsGone()
}
binding.autoCompleteSuggestionsList.gone()
browserActivity?.openExistingTab(it.tabId)
tabManager.openExistingTab(it.tabId)
}
else -> {
// NO OP
Expand Down
Loading
Loading