Refactor FXIOS-9519 - Private Tab Persistence with "Close Private tab…

…s" option as Opt In (#21240) Default: ON (Meaning: Private tabs are not persisted after the app is killed)
mozilla-mobile · Jul 24, 2024 · 6c9215c · 6c9215c
1 parent 2ec016f
commit 6c9215c
Show file tree

Hide file tree

Showing 17 changed files with 135 additions and 12 deletions.
diff --git a/firefox-ios/Client/Application/AccessibilityIdentifiers.swift b/firefox-ios/Client/Application/AccessibilityIdentifiers.swift
@@ -574,6 +574,10 @@ public struct AccessibilityIdentifiers {
             static let title = "showLinkPreviews"
         }
 
+        struct ClosePrivateTabs {
+            static let title = "ClosePrivateTabs"
+        }
+
         struct SearchBar {
             static let searchBarSetting = "SearchBarSetting"
             static let topSetting = "TopSearchBar"

diff --git a/firefox-ios/Client/Application/SceneDelegate.swift b/firefox-ios/Client/Application/SceneDelegate.swift
@@ -42,7 +42,12 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
             Experiments.shared.initializeTooling(url: url)
         }
 
-        routeBuilder.configure(prefs: profile.prefs)
+        routeBuilder.configure(
+            isPrivate: UserDefaults.standard.bool(
+                forKey: PrefsKeys.LastSessionWasPrivate
+            ),
+            prefs: profile.prefs
+        )
 
         let sceneCoordinator = SceneCoordinator(scene: scene)
         self.sceneCoordinator = sceneCoordinator
@@ -140,7 +145,12 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
     }
 
     func handleOpenURL(_ url: URL) {
-        routeBuilder.configure(prefs: profile.prefs)
+        routeBuilder.configure(
+            isPrivate: UserDefaults.standard.bool(
+                forKey: PrefsKeys.LastSessionWasPrivate
+            ),
+            prefs: profile.prefs
+        )
 
         // Before processing the incoming URL, check if it is a widget that is opening a tab via UUID.
         // If so, we want to be sure that we select the tab in the correct iPad window.

diff --git a/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift b/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift
@@ -7,9 +7,12 @@ import CoreSpotlight
 import Shared
 
 final class RouteBuilder {
+    private var isPrivate = false
     private var prefs: Prefs?
 
-    func configure(prefs: Prefs) {
+    func configure(isPrivate: Bool,
+                   prefs: Prefs) {
+        self.isPrivate = isPrivate
         self.prefs = prefs
     }
 
@@ -25,7 +28,7 @@ final class RouteBuilder {
             let urlQuery = urlScanner.fullURLQueryItem()?.asURL
             // Unless the `open-url` URL specifies a `private` parameter,
             // use the last browsing mode the user was in.
-            let isPrivate = Bool(urlScanner.value(query: "private") ?? "") ?? false
+            let isPrivate = Bool(urlScanner.value(query: "private") ?? "") ?? isPrivate
 
             recordTelemetry(input: host, isPrivate: isPrivate)
 
@@ -123,7 +126,7 @@ final class RouteBuilder {
             TelemetryWrapper.gleanRecordEvent(category: .action, method: .open, object: .asDefaultBrowser)
             RatingPromptManager.isBrowserDefault = true
             // Use the last browsing mode the user was in
-            return .search(url: url, isPrivate: false, options: [.focusLocationField])
+            return .search(url: url, isPrivate: isPrivate, options: [.focusLocationField])
         } else {
             return nil
         }

diff --git a/firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift b/firefox-ios/Client/Frontend/Browser/TabDisplayManager.swift
@@ -290,6 +290,8 @@ class LegacyTabDisplayManager: NSObject, FeatureFlaggable {
 
         isPrivate = isOn
 
+        UserDefaults.standard.set(isPrivate, forKey: PrefsKeys.LastSessionWasPrivate)
+
         TelemetryWrapper.recordEvent(
             category: .action,
             method: .tap,

diff --git a/firefox-ios/Client/Frontend/Browser/Tabs/Legacy/LegacyTabTrayViewController.swift b/firefox-ios/Client/Frontend/Browser/Tabs/Legacy/LegacyTabTrayViewController.swift
@@ -223,6 +223,7 @@ class LegacyTabTrayViewController: UIViewController, Themeable, TabTrayControlle
 
         viewSetup()
         listenForThemeChange(view)
+        updatePrivateUIState()
         applyTheme()
         changePanel()
     }
@@ -272,6 +273,13 @@ class LegacyTabTrayViewController: UIViewController, Themeable, TabTrayControlle
         showPanel(viewModel.tabTrayView)
     }
 
+    func updatePrivateUIState() {
+        UserDefaults.standard.set(
+            viewModel.tabManager.selectedTab?.isPrivate ?? false,
+            forKey: PrefsKeys.LastSessionWasPrivate
+        )
+    }
+
     private func updateTitle() {
         if let newTitle = viewModel.navTitle(for: segmentedControlIphone.selectedSegmentIndex) {
             // iPhone
@@ -334,6 +342,7 @@ class LegacyTabTrayViewController: UIViewController, Themeable, TabTrayControlle
         }
         updateToolbarItems(forSyncTabs: viewModel.profile.hasSyncableAccount())
         viewModel.tabTrayView.didTogglePrivateMode(privateMode)
+        updatePrivateUIState()
         updateTitle()
     }
 
@@ -583,6 +592,7 @@ extension LegacyTabTrayViewController {
         notificationCenter.post(name: .TabsTrayDidClose, withUserInfo: windowUUID.userInfo)
         // Update Private mode when closing TabTray, if the mode toggle but
         // no tab is pressed with return to previous state
+        updatePrivateUIState()
         viewModel.tabTrayView.didTogglePrivateMode(viewModel.tabManager.selectedTab?.isPrivate ?? false)
         if viewModel.segmentToFocus == .privateTabs {
             TelemetryWrapper.recordEvent(category: .action,

diff --git a/firefox-ios/Client/Frontend/Browser/TopTabsViewController.swift b/firefox-ios/Client/Frontend/Browser/TopTabsViewController.swift
@@ -195,6 +195,12 @@ class TopTabsViewController: UIViewController, Themeable, Notifiable, FeatureFla
         topTabDisplayManager.refreshStore()
     }
 
+    override func viewDidDisappear(_ animated: Bool) {
+        super.viewDidDisappear(animated)
+        UserDefaults.standard.set(tabManager.selectedTab?.isPrivate ?? false,
+                                  forKey: PrefsKeys.LastSessionWasPrivate)
+    }
+
     func updateTabCount(_ count: Int, animated: Bool = true) {
         tabsButton.updateTabCount(count, animated: animated)
     }

diff --git a/firefox-ios/Client/Frontend/Settings/Main/AppSettingsTableViewController.swift b/firefox-ios/Client/Frontend/Settings/Main/AppSettingsTableViewController.swift
@@ -293,6 +293,15 @@ class AppSettingsTableViewController: SettingsTableViewController,
 
         privacySettings.append(ClearPrivateDataSetting(settings: self, settingsDelegate: parentCoordinator))
 
+        privacySettings += [
+            BoolSetting(prefs: profile.prefs,
+                        theme: themeManager.getCurrentTheme(for: windowUUID),
+                        prefKey: PrefsKeys.Settings.closePrivateTabs,
+                        defaultValue: true,
+                        titleText: .AppSettingsClosePrivateTabsTitle,
+                        statusText: .AppSettingsClosePrivateTabsDescription)
+        ]
+
         privacySettings.append(ContentBlockerSetting(settings: self, settingsDelegate: parentCoordinator))
 
         privacySettings.append(NotificationsSetting(theme: themeManager.getCurrentTheme(for: windowUUID),

diff --git a/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift b/firefox-ios/Client/TabManagement/Legacy/LegacyTabManager.swift
@@ -274,6 +274,11 @@ class LegacyTabManager: NSObject, FeatureFlaggable, TabManager, TabEventHandler
     // TODO: FXIOS-7596 Remove when moving the TabManager protocol to TabManagerImplementation
     func preserveTabs() { fatalError("should never be called") }
 
+    func shouldClearPrivateTabs() -> Bool {
+        // FXIOS-9519: By default if no bool value is set we close the private tabs and mark it true
+        return profile.prefs.boolForKey(PrefsKeys.Settings.closePrivateTabs) ?? true
+    }
+
     func cleanupClosedTabs(_ closedTabs: [Tab], previous: Tab?, isPrivate: Bool = false) {
         DispatchQueue.main.async { [unowned self] in
             // select normal tab if there are no private tabs, we need to do this

diff --git a/firefox-ios/Client/TabManagement/TabManagerImplementation.swift b/firefox-ios/Client/TabManagement/TabManagerImplementation.swift
@@ -152,9 +152,11 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
     /// Creates the webview so needs to live on the main thread
     @MainActor
     private func generateTabs(from windowData: WindowData) async {
+        let filteredTabs = filterPrivateTabs(from: windowData,
+                                             clearPrivateTabs: shouldClearPrivateTabs())
         var tabToSelect: Tab?
 
-        for tabData in windowData.tabData {
+        for tabData in filteredTabs {
             let newTab = addTab(flushToDisk: false, zombie: true, isPrivate: tabData.isPrivate)
             newTab.url = URL(string: tabData.siteUrl, invalidCharacters: false)
             newTab.lastTitle = tabData.title
@@ -184,7 +186,7 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
             }
         }
 
-        logger.log("There was \(windowData.tabData.count) tabs restored",
+        logger.log("There was \(filteredTabs.count) tabs restored",
                    level: .debug,
                    category: .tabs)
 
@@ -201,6 +203,14 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
         }
     }
 
+    private func filterPrivateTabs(from windowData: WindowData, clearPrivateTabs: Bool) -> [TabData] {
+        var savedTabs = windowData.tabData
+        if clearPrivateTabs {
+            savedTabs = windowData.tabData.filter { !$0.isPrivate }
+        }
+        return savedTabs
+    }
+
     /// Creates the webview so needs to live on the main thread
     @MainActor
     private func generateEmptyTab() {
@@ -243,7 +253,12 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
     }
 
     private func generateTabDataForSaving() -> [TabData] {
-        let tabData = normalTabs.map { tab in
+        var tabsToSave = tabs
+        if shouldClearPrivateTabs() {
+            tabsToSave = normalTabs
+        }
+
+        let tabData = tabsToSave.map { tab in
             let oldTabGroupData = tab.metadataManager?.tabGroupData
             let state = TabGroupTimerState(rawValue: oldTabGroupData?.tabHistoryCurrentState ?? "")
             let groupData = TabGroupData(searchTerm: oldTabGroupData?.tabAssociatedSearchTerm,
@@ -289,7 +304,6 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
 
     override func saveSessionData(forTab tab: Tab?) {
         guard let tab = tab,
-              !tab.isPrivate,
               let tabSession = tab.webView?.interactionState as? Data,
               let tabID = UUID(uuidString: tab.tabUUID)
         else { return }
@@ -333,6 +347,11 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
         previous?.metadataManager?.updateTimerAndObserving(state: .tabSwitched, isPrivate: previous?.isPrivate ?? false)
         tab.metadataManager?.updateTimerAndObserving(state: .tabSelected, isPrivate: tab.isPrivate)
 
+        // Make sure to wipe the private tabs if the user has the pref turned on
+        if shouldClearPrivateTabs(), !tab.isPrivate {
+            removeAllPrivateTabs()
+        }
+
         _selectedIndex = tabs.firstIndex(of: tab) ?? -1
 
         preserveTabs()
@@ -362,6 +381,17 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
         updateMenuItemsForSelectedTab()
     }
 
+    private func removeAllPrivateTabs() {
+        // reset the selectedTabIndex if we are on a private tab because we will be removing it.
+        if selectedTab?.isPrivate ?? false {
+            _selectedIndex = -1
+        }
+        privateTabs.forEach { $0.close() }
+        tabs = normalTabs
+
+        privateConfiguration = LegacyTabManager.makeWebViewConfig(isPrivate: true, prefs: profile.prefs)
+    }
+
     private func willSelectTab(_ url: URL?) {
         tabsTelemetry.startTabSwitchMeasurement()
     }
@@ -396,6 +426,12 @@ class TabManagerImplementation: LegacyTabManager, Notifiable, WindowSimpleTabsPr
             TabEvent.post(.didGainFocus, for: tab)
         }
         TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .tab)
+
+        // Note: we setup last session private case as the session is tied to user's selected
+        // tab but there are times when tab manager isn't available and we need to know
+        // users's last state (Private vs Regular)
+        UserDefaults.standard.set(selectedTab?.isPrivate ?? false,
+                                  forKey: PrefsKeys.LastSessionWasPrivate)
     }
 
     // MARK: - Screenshots

diff --git a/firefox-ios/Client/Telemetry/TelemetryWrapper.swift b/firefox-ios/Client/Telemetry/TelemetryWrapper.swift
@@ -209,6 +209,13 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
             GleanMetrics.Preferences.showClipboardBar.set(false)
         }
 
+        // Close private tabs
+        if let closePrivateTabs = prefs.boolForKey(PrefsKeys.Settings.closePrivateTabs) {
+            GleanMetrics.Preferences.closePrivateTabs.set(closePrivateTabs)
+        } else {
+            GleanMetrics.Preferences.closePrivateTabs.set(false)
+        }
+
         // Tracking protection - enabled
         if let tpEnabled = prefs.boolForKey(ContentBlockingConfig.Prefs.EnabledKey) {
             GleanMetrics.TrackingProtection.enabled.set(tpEnabled)

diff --git a/firefox-ios/Shared/Prefs.swift b/firefox-ios/Shared/Prefs.swift
@@ -111,6 +111,11 @@ public struct PrefsKeys {
         case shoppingOnboardingCFRsCounterKey = "ShoppingOnboardingCFRsCounterKey"
     }
 
+    // Firefox settings
+    public struct Settings {
+        public static let closePrivateTabs = "ClosePrivateTabs"
+    }
+
     // Activity Stream
     public static let KeyTopSitesCacheIsValid = "topSitesCacheIsValid"
     public static let KeyTopSitesCacheSize = "topSitesCacheSize"
@@ -163,6 +168,9 @@ public struct PrefsKeys {
     // The last recorded CFR timestamp
     public static let FakespotLastCFRTimestamp = "FakespotLastCFRTimestamp"
 
+    // Representing whether or not the last user session was private
+    public static let LastSessionWasPrivate = "wasLastSessionPrivate"
+
     // Only used to force nimbus features to true with tests
     public static let NimbusFeatureTestsOverride = "NimbusFeatureTestsOverride"
 

diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/RouteTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/RouteTests.swift
@@ -362,7 +362,7 @@ class RouteTests: XCTestCase {
 
     func createSubject() -> RouteBuilder {
         let subject = RouteBuilder()
-        subject.configure(prefs: MockProfile().prefs)
+        subject.configure(isPrivate: false, prefs: MockProfile().prefs)
         trackForMemoryLeaks(subject)
         return subject
     }

diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShortcutRouteTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/ShortcutRouteTests.swift
@@ -80,7 +80,7 @@ final class ShortcutRouteTests: XCTestCase {
 
     func createSubject() -> RouteBuilder {
         let subject = RouteBuilder()
-        subject.configure(prefs: MockProfile().prefs)
+        subject.configure(isPrivate: false, prefs: MockProfile().prefs)
         trackForMemoryLeaks(subject)
         return subject
     }

diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/UserActivityRouteTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Coordinators/UserActivityRouteTests.swift
@@ -54,7 +54,7 @@ class UserActivityRouteTests: XCTestCase {
 
     func createSubject() -> RouteBuilder {
         let subject = RouteBuilder()
-        subject.configure(prefs: MockProfile().prefs)
+        subject.configure(isPrivate: false, prefs: MockProfile().prefs)
         trackForMemoryLeaks(subject)
         return subject
     }

diff --git a/...firefox-ios-tests/Tests/ClientTests/TabTray/Legacy/LegacyTabTrayViewControllerTests.swift b/...firefox-ios-tests/Tests/ClientTests/TabTray/Legacy/LegacyTabTrayViewControllerTests.swift
@@ -72,4 +72,15 @@ final class LegacyTabTrayViewControllerTests: XCTestCase {
 
         waitForExpectations(timeout: 3.0)
     }
+
+    func testTabTrayRevertToRegular_ForNoPrivateTabSelected() {
+        // If the user selects Private mode but doesn't focus or creates a new tab
+        // we considered that regular is actually active
+        tabTray.viewModel.segmentToFocus = TabTrayPanelType.privateTabs
+        tabTray.viewDidLoad()
+        tabTray.didTapDone()
+
+        let privateState = UserDefaults.standard.bool(forKey: PrefsKeys.LastSessionWasPrivate)
+        XCTAssertFalse(privateState)
+    }
 }
diff --git a/firefox-ios/firefox-ios-tests/Tests/XCUITests/PrivateBrowsingTest.swift b/firefox-ios/firefox-ios-tests/Tests/XCUITests/PrivateBrowsingTest.swift
@@ -283,6 +283,17 @@ fileprivate extension BaseTestCase {
             "The number of tabs is not correct"
         )
     }
+
+    func enableClosePrivateBrowsingOptionWhenLeaving() {
+        navigator.goto(SettingsScreen)
+        let settingsTableView = app.tables[AccessibilityIdentifiers.Settings.tableViewController]
+
+        while settingsTableView.staticTexts["Close Private Tabs"].isHittable == false {
+            settingsTableView.swipeUp()
+        }
+        let closePrivateTabsSwitch = settingsTableView.switches["ClosePrivateTabs"]
+        closePrivateTabsSwitch.tap()
+    }
 }
 
 class PrivateBrowsingTestIphone: IphoneOnlyTestCase {

diff --git a/firefox-ios/firefox-ios-tests/Tests/XCUITests/SettingsTests.swift b/firefox-ios/firefox-ios-tests/Tests/XCUITests/SettingsTests.swift
@@ -153,6 +153,7 @@ class SettingsTests: BaseTestCase {
             table.cells[settingsQuery.NoImageMode.title], app.switches[settingsQuery.OfferToOpen.title],
             table.cells[settingsQuery.Logins.title], app.switches[settingsQuery.ShowLink.title],
             table.cells[settingsQuery.CreditCards.title], table.cells[settingsQuery.Address.title],
+            table.cells[settingsQuery.ClearData.title], app.switches[settingsQuery.ClosePrivateTabs.title],
             table.cells[settingsQuery.ContentBlocker.title], table.cells[settingsQuery.Notifications.title],
             table.cells[settingsQuery.ShowIntroduction.title], table.cells[settingsQuery.SendAnonymousUsageData.title],
             table.cells[settingsQuery.StudiesToggle.title], table.cells[settingsQuery.Version.title],