For #18160: Show a prompt when trying to leave private browsing with active downloads (#22912)

Co-authored-by: mike a <mavduevskiy@gmail.com>
upstream-sync
mavduevskiy 2 years ago committed by GitHub
parent 105a2e5fc6
commit 89f5e96d73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -7,7 +7,6 @@ package org.mozilla.fenix.tabstray
import android.content.Context
import androidx.navigation.NavController
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.launch
import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
import mozilla.components.browser.state.selector.normalTabs
@ -26,11 +25,13 @@ import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.tabstray.ext.getTabSessionState
import org.mozilla.fenix.tabstray.ext.isActiveDownload
import kotlin.coroutines.CoroutineContext
/**
* An interactor that helps with navigating to different parts of the app from the tabs tray.
*/
@Suppress("TooManyFunctions")
interface NavigationInteractor {
/**
@ -63,6 +64,11 @@ interface NavigationInteractor {
*/
fun onCloseAllTabsClicked(private: Boolean)
/**
* Called when cancelling private downloads confirmed.
*/
fun onCloseAllPrivateTabsWarningConfirmed(private: Boolean)
/**
* Called when opening the recently closed tabs menu button.
*/
@ -87,7 +93,7 @@ interface NavigationInteractor {
/**
* A default implementation of [NavigationInteractor].
*/
@Suppress("LongParameterList")
@Suppress("LongParameterList", "TooManyFunctions")
class DefaultNavigationInteractor(
private val context: Context,
private val activity: HomeActivity,
@ -95,7 +101,7 @@ class DefaultNavigationInteractor(
private val navController: NavController,
private val metrics: MetricController,
private val dismissTabTray: () -> Unit,
private val dismissTabTrayAndNavigateHome: (String) -> Unit,
private val dismissTabTrayAndNavigateHome: (sessionId: String) -> Unit,
private val bookmarksUseCase: BookmarksUseCase,
private val tabsTrayStore: TabsTrayStore,
private val collectionStorage: TabCollectionStorage,
@ -105,6 +111,7 @@ class DefaultNavigationInteractor(
collectionToSelect: Long?
) -> Unit,
private val showBookmarkSnackbar: (tabSize: Int) -> Unit,
private val showCancelledDownloadWarning: (downloadCount: Int, tabId: String?, source: String?) -> Unit,
private val accountManager: FxaAccountManager,
private val ioDispatcher: CoroutineContext
) : NavigationInteractor {
@ -159,14 +166,30 @@ class DefaultNavigationInteractor(
navController.navigate(directions)
}
@OptIn(ExperimentalCoroutinesApi::class)
override fun onCloseAllTabsClicked(private: Boolean) {
closeAllTabs(private, isConfirmed = false)
}
override fun onCloseAllPrivateTabsWarningConfirmed(private: Boolean) {
closeAllTabs(private, isConfirmed = true)
}
private fun closeAllTabs(private: Boolean, isConfirmed: Boolean) {
val sessionsToClose = if (private) {
HomeFragment.ALL_PRIVATE_TABS
} else {
HomeFragment.ALL_NORMAL_TABS
}
if (private && !isConfirmed) {
val privateDownloads = browserStore.state.downloads.filter {
it.value.private && it.value.isActiveDownload()
}
if (privateDownloads.isNotEmpty()) {
showCancelledDownloadWarning(privateDownloads.size, null, null)
return
}
}
dismissTabTrayAndNavigateHome(sessionsToClose)
}

@ -23,6 +23,7 @@ import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.ext.DEFAULT_ACTIVE_DAYS
import org.mozilla.fenix.ext.potentialInactiveTabs
import org.mozilla.fenix.tabstray.ext.isActiveDownload
import java.util.concurrent.TimeUnit
interface TabsTrayController {
@ -46,11 +47,23 @@ interface TabsTrayController {
fun handleNavigateToBrowser()
/**
* Deletes the [TabSessionState] with the specified [tabId].
* Deletes the [TabSessionState] with the specified [tabId] or calls [DownloadCancelDialogFragment]
* if user tries to close the last private tab while private downloads are active.
* Tracks [Event.ClosedExistingTab] in case of deletion.
*
* @param tabId The id of the [TabSessionState] to be removed from TabsTray.
* @param source app feature from which the tab with [tabId] was closed.
*/
fun handleTabDeletion(tabId: String, source: String? = null)
/**
* Deletes the [TabSessionState] with the specified [tabId]
* Tracks [Event.ClosedExistingTab] in case of deletion.
*
* @param tabId The id of the [TabSessionState] to be removed from TabsTray.
* @param source app feature from which the tab with [tabId] was closed.
*/
fun handleTabDeletion(tabId: String)
fun handleDeleteTabWarningAccepted(tabId: String, source: String? = null)
/**
* Deletes a list of [tabs].
@ -104,7 +117,9 @@ class DefaultTabsTrayController(
private val tabsUseCases: TabsUseCases,
private val selectTabPosition: (Int, Boolean) -> Unit,
private val dismissTray: () -> Unit,
private val showUndoSnackbarForTab: (Boolean) -> Unit
private val showUndoSnackbarForTab: (Boolean) -> Unit,
@VisibleForTesting
internal val showCancelledDownloadWarning: (downloadCount: Int, tabId: String?, source: String?) -> Unit,
) : TabsTrayController {
@ -144,18 +159,37 @@ class DefaultTabsTrayController(
* Deletes the [TabSessionState] with the specified [tabId].
*
* @param tabId The id of the [TabSessionState] to be removed from TabsTray.
* @param source app feature from which the tab with [tabId] was closed.
* This method has no effect if the tab does not exist.
*/
override fun handleTabDeletion(tabId: String) {
override fun handleTabDeletion(tabId: String, source: String?) {
deleteTab(tabId, source, isConfirmed = false)
}
override fun handleDeleteTabWarningAccepted(tabId: String, source: String?) {
deleteTab(tabId, source, isConfirmed = true)
}
private fun deleteTab(tabId: String, source: String?, isConfirmed: Boolean) {
val tab = browserStore.state.findTab(tabId)
tab?.let {
if (browserStore.state.getNormalOrPrivateTabs(it.content.private).size != 1) {
val isLastTab = browserStore.state.getNormalOrPrivateTabs(it.content.private).size == 1
if (!isLastTab) {
tabsUseCases.removeTab(tabId)
showUndoSnackbarForTab(it.content.private)
} else {
dismissTabsTrayAndNavigateHome(tabId)
val privateDownloads = browserStore.state.downloads.filter { map ->
map.value.private && map.value.isActiveDownload()
}
if (!isConfirmed && privateDownloads.isNotEmpty()) {
showCancelledDownloadWarning(privateDownloads.size, tabId, source)
return
} else {
dismissTabsTrayAndNavigateHome(tabId)
}
}
metrics.track(Event.ClosedExistingTab(source ?: "unknown"))
}
}

@ -8,6 +8,7 @@ import android.content.Context
import android.content.res.Configuration
import android.os.Build
import android.os.Bundle
import android.view.Gravity
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
@ -26,6 +27,7 @@ import mozilla.appservices.places.BookmarkRoot
import mozilla.components.browser.state.selector.normalTabs
import mozilla.components.browser.state.selector.privateTabs
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.feature.downloads.ui.DownloadCancelDialogFragment
import mozilla.components.feature.tabs.tabstray.TabsFeature
import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
import org.mozilla.fenix.HomeActivity
@ -58,6 +60,7 @@ import org.mozilla.fenix.tabstray.ext.collectionMessage
import org.mozilla.fenix.tabstray.ext.make
import org.mozilla.fenix.tabstray.ext.orDefault
import org.mozilla.fenix.tabstray.ext.showWithTheme
import org.mozilla.fenix.theme.ThemeManager
import org.mozilla.fenix.utils.allowUndo
import kotlin.math.max
@ -67,6 +70,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
private lateinit var browserTrayInteractor: BrowserTrayInteractor
private lateinit var tabsTrayInteractor: TabsTrayInteractor
private lateinit var tabsTrayController: DefaultTabsTrayController
private lateinit var navigationInteractor: DefaultNavigationInteractor
@VisibleForTesting internal lateinit var trayBehaviorManager: TabSheetBehaviorManager
private val tabLayoutMediator = ViewBoundFeatureWrapper<TabLayoutMediator>()
@ -142,6 +146,13 @@ class TabsTrayFragment : AppCompatDialogFragment() {
return tabsTrayDialogBinding.root
}
override fun onStart() {
super.onStart()
findPreviousDialogFragment()?.let { dialog ->
dialog.onAcceptClicked = ::onCancelDownloadWarningAccepted
}
}
override fun onDestroyView() {
super.onDestroyView()
_tabsTrayBinding = null
@ -160,7 +171,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
}
requireComponents.analytics.metrics.track(Event.TabsTrayOpened)
val navigationInteractor =
navigationInteractor =
DefaultNavigationInteractor(
context = requireContext(),
activity = activity,
@ -174,6 +185,7 @@ class TabsTrayFragment : AppCompatDialogFragment() {
collectionStorage = requireComponents.core.tabCollectionStorage,
showCollectionSnackbar = ::showCollectionSnackbar,
showBookmarkSnackbar = ::showBookmarkSnackbar,
showCancelledDownloadWarning = ::showCancelledDownloadWarning,
accountManager = requireComponents.backgroundServices.accountManager,
ioDispatcher = Dispatchers.IO
)
@ -190,7 +202,8 @@ class TabsTrayFragment : AppCompatDialogFragment() {
tabsUseCases = requireComponents.useCases.tabsUseCases,
selectTabPosition = ::selectTabPosition,
dismissTray = ::dismissTabsTray,
showUndoSnackbarForTab = ::showUndoSnackbarForTab
showUndoSnackbarForTab = ::showUndoSnackbarForTab,
showCancelledDownloadWarning = ::showCancelledDownloadWarning
)
tabsTrayInteractor = DefaultTabsTrayInteractor(tabsTrayController)
@ -371,6 +384,40 @@ class TabsTrayFragment : AppCompatDialogFragment() {
}
}
@VisibleForTesting
internal fun onCancelDownloadWarningAccepted(tabId: String?, source: String?) {
if (tabId != null) {
tabsTrayInteractor.onDeletePrivateTabWarningAccepted(tabId, source)
} else {
navigationInteractor.onCloseAllPrivateTabsWarningConfirmed(private = true)
}
}
@VisibleForTesting
internal fun showCancelledDownloadWarning(downloadCount: Int, tabId: String?, source: String?) {
val dialog = DownloadCancelDialogFragment.newInstance(
downloadCount = downloadCount,
tabId = tabId,
source = source,
promptStyling = DownloadCancelDialogFragment.PromptStyling(
gravity = Gravity.BOTTOM,
shouldWidthMatchParent = true,
positiveButtonBackgroundColor = ThemeManager.resolveAttribute(
R.attr.accent,
requireContext()
),
positiveButtonTextColor = ThemeManager.resolveAttribute(
R.attr.contrastText,
requireContext()
),
positiveButtonRadius = (resources.getDimensionPixelSize(R.dimen.tab_corner_radius)).toFloat()
),
onPositiveButtonClicked = ::onCancelDownloadWarningAccepted
)
dialog.show(parentFragmentManager, DOWNLOAD_CANCEL_DIALOG_FRAGMENT_TAG)
}
@VisibleForTesting
internal fun showUndoSnackbarForTab(isPrivate: Boolean) {
val snackbarMessage =
@ -518,6 +565,11 @@ class TabsTrayFragment : AppCompatDialogFragment() {
.show()
}
@Suppress("MaxLineLength")
private fun findPreviousDialogFragment(): DownloadCancelDialogFragment? {
return parentFragmentManager.findFragmentByTag(DOWNLOAD_CANCEL_DIALOG_FRAGMENT_TAG) as? DownloadCancelDialogFragment
}
private fun getSnackbarAnchor(): View? {
return if (requireComponents.settings.accessibilityServicesEnabled) {
null
@ -527,6 +579,8 @@ class TabsTrayFragment : AppCompatDialogFragment() {
}
companion object {
private const val DOWNLOAD_CANCEL_DIALOG_FRAGMENT_TAG = "DOWNLOAD_CANCEL_DIALOG_FRAGMENT_TAG"
// Minimum number of list items for which to show the tabs tray as expanded.
const val EXPAND_AT_LIST_SIZE = 4

@ -22,8 +22,15 @@ interface TabsTrayInteractor {
/**
* Invoked when a tab is removed from the tabs tray with the given [tabId].
* @param source app feature from which the [TabSessionState] with [tabId] was closed.
*/
fun onDeleteTab(tabId: String)
fun onDeleteTab(tabId: String, source: String? = null)
/**
* Invoked when the user confirmed tab removal that would lead to cancelled private downloads.
* @param source is the app feature from which the [TabSessionState] with [tabId] was closed.
*/
fun onDeletePrivateTabWarningAccepted(tabId: String, source: String? = null)
/**
* Invoked when [TabSessionState]s need to be deleted.
@ -66,8 +73,12 @@ class DefaultTabsTrayInteractor(
controller.handleNavigateToBrowser()
}
override fun onDeleteTab(tabId: String) {
controller.handleTabDeletion(tabId)
override fun onDeleteTab(tabId: String, source: String?) {
controller.handleTabDeletion(tabId, source)
}
override fun onDeletePrivateTabWarningAccepted(tabId: String, source: String?) {
controller.handleDeleteTabWarningAccepted(tabId, source)
}
override fun onDeleteTabs(tabs: Collection<TabSessionState>) {

@ -67,13 +67,6 @@ class DefaultBrowserTrayInteractor(
}
}
private val removeTabWrapper by lazy {
RemoveTabUseCaseWrapper(metrics) {
// Handle removal from the interactor where we can also handle "undo" visuals.
trayInteractor.onDeleteTab(it)
}
}
/**
* See [SelectionInteractor.open]
*/
@ -149,6 +142,6 @@ class DefaultBrowserTrayInteractor(
}
private fun closeTab(tab: TabSessionState, source: String? = null) {
removeTabWrapper.invoke(tab.id, source)
trayInteractor.onDeleteTab(tab.id, source)
}
}

@ -0,0 +1,13 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.tabstray.ext
import mozilla.components.browser.state.state.content.DownloadState
fun DownloadState.isActiveDownload(): Boolean {
return status == DownloadState.Status.INITIATED ||
status == DownloadState.Status.DOWNLOADING ||
status == DownloadState.Status.PAUSED
}

@ -19,6 +19,7 @@ import io.mockk.verifyOrder
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.base.profiler.Profiler
@ -122,6 +123,55 @@ class DefaultTabsTrayControllerTest {
verify { metrics.track(Event.NewTabTapped) }
}
@Test
fun `WHEN handleTabDeletion is called THEN Event#ClosedExistingTab is added to telemetry`() {
val tab: TabSessionState = mockk { every { content.private } returns true }
every { browserStore.state } returns mockk()
try {
mockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
every { browserStore.state.findTab(any()) } returns tab
every { browserStore.state.getNormalOrPrivateTabs(any()) } returns listOf(tab)
createController().handleTabDeletion("testTabId", "unknown")
verify { metrics.track(Event.ClosedExistingTab("unknown")) }
} finally {
unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
}
}
@Test
fun `GIVEN active private download WHEN handleTabDeletion is called for the last private tab THEN showCancelledDownloadWarning is called`() {
var showCancelledDownloadWarningInvoked = false
val controller = spyk(
createController(
showCancelledDownloadWarning = { _, _, _ ->
showCancelledDownloadWarningInvoked = true
}
)
)
val tab: TabSessionState = mockk { every { content.private } returns true }
every { browserStore.state } returns mockk()
every { browserStore.state.downloads } returns mapOf(
"1" to DownloadState(
"https://mozilla.org/download",
private = true,
destinationDirectory = "Download",
status = DownloadState.Status.DOWNLOADING
)
)
try {
mockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
every { browserStore.state.findTab(any()) } returns tab
every { browserStore.state.getNormalOrPrivateTabs(any()) } returns listOf(tab)
controller.handleTabDeletion("testTabId", "unknown")
assertTrue(showCancelledDownloadWarningInvoked)
} finally {
unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
}
}
@Test
fun `WHEN handleTrayScrollingToPosition is called with smoothScroll=true THEN it scrolls to that position with smoothScroll`() {
var selectTabPositionInvoked = false
@ -447,7 +497,8 @@ class DefaultTabsTrayControllerTest {
navigateToHomeAndDeleteSession: (String) -> Unit = { },
selectTabPosition: (Int, Boolean) -> Unit = { _, _ -> },
dismissTray: () -> Unit = { },
showUndoSnackbarForTab: (Boolean) -> Unit = { _ -> }
showUndoSnackbarForTab: (Boolean) -> Unit = { _ -> },
showCancelledDownloadWarning: (Int, String?, String?) -> Unit = { _, _, _ -> }
): DefaultTabsTrayController {
return DefaultTabsTrayController(
trayStore,
@ -461,7 +512,8 @@ class DefaultTabsTrayControllerTest {
tabsUseCases,
selectTabPosition,
dismissTray,
showUndoSnackbarForTab
showUndoSnackbarForTab,
showCancelledDownloadWarning = showCancelledDownloadWarning
)
}
}

@ -34,6 +34,13 @@ class DefaultTabsTrayInteractorTest {
verifySequence { controller.handleTabDeletion("testTabId") }
}
@Test
fun `GIVEN user confirmed downloads cancellation WHEN onDeletePrivateTabWarningAccepted is called THEN the Interactor delegates the controller`() {
trayInteractor.onDeletePrivateTabWarningAccepted("testTabId")
verifySequence { controller.handleDeleteTabWarningAccepted("testTabId") }
}
@Test
fun `GIVEN user deleted multiple browser tabs WHEN onDeleteTabs is called THEN the Interactor delegates the controller`() {
val tabsToDelete = listOf<TabSessionState>(mockk(), mockk())

@ -7,17 +7,21 @@ package org.mozilla.fenix.tabstray
import android.content.Context
import androidx.navigation.NavController
import androidx.navigation.NavDirections
import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import io.mockk.mockkStatic
import io.mockk.unmockkStatic
import io.mockk.verify
import io.mockk.coVerify
import io.mockk.verifyOrder
import kotlinx.coroutines.test.TestCoroutineDispatcher
import kotlinx.coroutines.test.runBlockingTest
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.browser.storage.sync.TabEntry
import mozilla.components.service.fxa.manager.FxaAccountManager
@ -117,6 +121,41 @@ class NavigationInteractorTest {
assertTrue(dismissTabTrayAndNavigateHomeInvoked)
}
@Test
fun `GIVEN active private download WHEN onCloseAllTabsClicked is called for private tabs THEN showCancelledDownloadWarning is called`() {
var showCancelledDownloadWarningInvoked = false
val mockedStore: BrowserStore = mockk()
val controller = spyk(
createInteractor(
browserStore = mockedStore,
showCancelledDownloadWarning = { _, _, _ ->
showCancelledDownloadWarningInvoked = true
}
)
)
val tab: TabSessionState = mockk { every { content.private } returns true }
every { mockedStore.state } returns mockk()
every { mockedStore.state.downloads } returns mapOf(
"1" to DownloadState(
"https://mozilla.org/download",
private = true,
destinationDirectory = "Download",
status = DownloadState.Status.DOWNLOADING
)
)
try {
mockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
every { mockedStore.state.findTab(any()) } returns tab
every { mockedStore.state.getNormalOrPrivateTabs(any()) } returns listOf(tab)
controller.onCloseAllTabsClicked(true)
assertTrue(showCancelledDownloadWarningInvoked)
} finally {
unmockkStatic("mozilla.components.browser.state.selector.SelectorsKt")
}
}
@Test
fun `onShareTabsOfType calls navigation on DefaultNavigationInteractor`() {
createInteractor().onShareTabsOfTypeClicked(false)
@ -182,15 +221,17 @@ class NavigationInteractorTest {
@Suppress("LongParameterList")
private fun createInteractor(
browserStore: BrowserStore = store,
dismissTabTray: () -> Unit = { },
dismissTabTrayAndNavigateHome: (String) -> Unit = { _ -> },
showCollectionSnackbar: (Int, Boolean, Long?) -> Unit = { _, _, _ -> },
showBookmarkSnackbar: (Int) -> Unit = { _ -> }
showBookmarkSnackbar: (Int) -> Unit = { _ -> },
showCancelledDownloadWarning: (Int, String?, String?) -> Unit = { _, _, _ -> }
): NavigationInteractor {
return DefaultNavigationInteractor(
context,
activity,
store,
browserStore,
navController,
metrics,
dismissTabTray,
@ -200,6 +241,7 @@ class NavigationInteractorTest {
collectionStorage,
showCollectionSnackbar,
showBookmarkSnackbar,
showCancelledDownloadWarning,
accountManager,
testDispatcher
)

Loading…
Cancel
Save