Fix iOS auto-clearing fails to remove cookies (#2767)

Task/Issue URL: https://app.asana.com/0/414235014887631/1207126219690101/f Description: This reverts commit cebfbf5.
duckduckgo · Apr 19, 2024 · 685776f · 685776f
1 parent 4365462
commit 685776f
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 87 deletions.
diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift
@@ -266,9 +266,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
                 })
             }
 
-        let previewsSource = TabPreviewsSource()
         let historyManager = makeHistoryManager()
-        let tabsModel = prepareTabsModel(previewsSource: previewsSource)
 
 #if APP_TRACKING_PROTECTION
         let main = MainViewController(bookmarksDatabase: bookmarksDatabase,
@@ -277,18 +275,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
                                       historyManager: historyManager,
                                       syncService: syncService,
                                       syncDataProviders: syncDataProviders,
-                                      appSettings: AppDependencyProvider.shared.appSettings,
-                                      previewsSource: previewsSource,
-                                      tabsModel: tabsModel)
+                                      appSettings: AppDependencyProvider.shared.appSettings)
 #else
         let main = MainViewController(bookmarksDatabase: bookmarksDatabase,
                                       bookmarksDatabaseCleaner: syncDataProviders.bookmarksAdapter.databaseCleaner,
                                       historyManager: historyManager,
                                       syncService: syncService,
                                       syncDataProviders: syncDataProviders,
-                                      appSettings: AppDependencyProvider.shared.appSettings,
-                                      previewsSource: previewsSource,
-                                      tabsModel: tabsModel)
+                                      appSettings: AppDependencyProvider.shared.appSettings)
 #endif
 
         main.loadViewIfNeeded()
@@ -345,27 +339,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         return true
     }
 
-    private func prepareTabsModel(previewsSource: TabPreviewsSource = TabPreviewsSource(),
-                                  appSettings: AppSettings = AppDependencyProvider.shared.appSettings,
-                                  isDesktop: Bool = UIDevice.current.userInterfaceIdiom == .pad) -> TabsModel {
-        let isPadDevice = UIDevice.current.userInterfaceIdiom == .pad
-        let tabsModel: TabsModel
-        if AutoClearSettingsModel(settings: appSettings) != nil {
-            tabsModel = TabsModel(desktop: isPadDevice)
-            tabsModel.save()
-            previewsSource.removeAllPreviews()
-        } else {
-            if let storedModel = TabsModel.get() {
-                // Save new model in case of migration
-                storedModel.save()
-                tabsModel = storedModel
-            } else {
-                tabsModel = TabsModel(desktop: isPadDevice)
-            }
-        }
-        return tabsModel
-    }
-
     private func makeHistoryManager() -> HistoryManager {
         let historyManager = HistoryManager(privacyConfigManager: ContentBlocking.shared.privacyConfigurationManager,
                               variantManager: DefaultVariantManager(),
@@ -766,7 +739,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         }
 
         Task { @MainActor in
-            // Autoclear should have happened by now
+            await autoClear?.applicationWillMoveToForeground()
             showKeyboardIfSettingOn = false
 
             if !handleAppDeepLink(app, mainViewController, url) {

diff --git a/DuckDuckGo/AutoClear.swift b/DuckDuckGo/AutoClear.swift
@@ -33,15 +33,14 @@ class AutoClear {
     private let worker: AutoClearWorker
     private var timestamp: TimeInterval?
 
-    private let appSettings: AppSettings
-
+    private lazy var appSettings = AppDependencyProvider.shared.appSettings
+    
     var isClearingEnabled: Bool {
         return AutoClearSettingsModel(settings: appSettings) != nil
     }
 
-    init(worker: AutoClearWorker, appSettings: AppSettings = AppDependencyProvider.shared.appSettings) {
+    init(worker: AutoClearWorker) {
         self.worker = worker
-        self.appSettings = appSettings
     }
 
     @MainActor
@@ -87,8 +86,8 @@ class AutoClear {
             let timestamp = timestamp,
             shouldClearData(elapsedTime: Date().timeIntervalSince1970 - timestamp) else { return }
 
-        self.timestamp = nil
         worker.clearNavigationStack()
         await clearData()
+        self.timestamp = nil
     }
 }
diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift
@@ -84,13 +84,11 @@ class MainViewController: UIViewController {
     var tabsBarController: TabsBarViewController?
     var suggestionTrayController: SuggestionTrayViewController?
 
-    let tabManager: TabManager
-    let previewsSource: TabPreviewsSource
+    var tabManager: TabManager!
+    let previewsSource = TabPreviewsSource()
     let appSettings: AppSettings
     private var launchTabObserver: LaunchTabNotification.Observer?
 
-    var doRefreshAfterClear = true
-
 #if APP_TRACKING_PROTECTION
     private let appTrackingProtectionDatabase: CoreDataDatabase
 #endif
@@ -141,7 +139,7 @@ class MainViewController: UIViewController {
 
     lazy var tabSwitcherTransition = TabSwitcherTransitionDelegate()
     var currentTab: TabViewController? {
-        return tabManager.current(createIfNeeded: false)
+        return tabManager?.current(createIfNeeded: false)
     }
 
     var searchBarRect: CGRect {
@@ -176,9 +174,7 @@ class MainViewController: UIViewController {
         historyManager: HistoryManager,
         syncService: DDGSyncing,
         syncDataProviders: SyncDataProviders,
-        appSettings: AppSettings = AppUserDefaults(),
-        previewsSource: TabPreviewsSource,
-        tabsModel: TabsModel
+        appSettings: AppSettings = AppUserDefaults()
     ) {
         self.appTrackingProtectionDatabase = appTrackingProtectionDatabase
         self.bookmarksDatabase = bookmarksDatabase
@@ -189,17 +185,9 @@ class MainViewController: UIViewController {
         self.favoritesViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, favoritesDisplayMode: appSettings.favoritesDisplayMode)
         self.bookmarksCachingSearch = BookmarksCachingSearch(bookmarksStore: CoreDataBookmarksSearchStore(bookmarksStore: bookmarksDatabase))
         self.appSettings = appSettings
-        self.previewsSource = previewsSource
-
-        self.tabManager = TabManager(model: tabsModel,
-                                     previewsSource: previewsSource,
-                                     bookmarksDatabase: bookmarksDatabase,
-                                     historyManager: historyManager,
-                                     syncService: syncService)
-
+
         super.init(nibName: nil, bundle: nil)
-
-        tabManager.delegate = self
+
         bindFavoritesDisplayMode()
         bindSyncService()
     }
@@ -210,9 +198,7 @@ class MainViewController: UIViewController {
         historyManager: HistoryManager,
         syncService: DDGSyncing,
         syncDataProviders: SyncDataProviders,
-        appSettings: AppSettings,
-        previewsSource: TabPreviewsSource,
-        tabsModel: TabsModel
+        appSettings: AppSettings
     ) {
         self.bookmarksDatabase = bookmarksDatabase
         self.bookmarksDatabaseCleaner = bookmarksDatabaseCleaner
@@ -222,18 +208,9 @@ class MainViewController: UIViewController {
         self.favoritesViewModel = FavoritesListViewModel(bookmarksDatabase: bookmarksDatabase, favoritesDisplayMode: appSettings.favoritesDisplayMode)
         self.bookmarksCachingSearch = BookmarksCachingSearch(bookmarksStore: CoreDataBookmarksSearchStore(bookmarksStore: bookmarksDatabase))
         self.appSettings = appSettings
-        self.previewsSource = previewsSource
-
-        self.tabManager = TabManager(model: tabsModel,
-                                     previewsSource: previewsSource,
-                                     bookmarksDatabase: bookmarksDatabase,
-                                     historyManager: historyManager,
-                                     syncService: syncService)
-
-
+
         super.init(nibName: nil, bundle: nil)
 
-        tabManager.delegate = self
         bindSyncService()
     }
 #endif
@@ -287,6 +264,7 @@ class MainViewController: UIViewController {
         initTabButton()
         initMenuButton()
         initBookmarksButton()
+        configureTabManager()
         loadInitialView()
         previewsSource.prepare()
         addLaunchTabNotificationObserver()
@@ -712,6 +690,40 @@ class MainViewController: UIViewController {
         dismissOmniBar()
     }
 
+    private func configureTabManager() {
+
+        let isPadDevice = UIDevice.current.userInterfaceIdiom == .pad
+
+        let tabsModel: TabsModel
+        if let settings = AutoClearSettingsModel(settings: appSettings) {
+            // This needs to be refactored so that tabs model is injected and cleared before view did load,
+            //  but for now, ensure this happens in the right order by clearing data here too, if needed.
+            tabsModel = TabsModel(desktop: isPadDevice)
+            tabsModel.save()
+            previewsSource.removeAllPreviews()
+
+            if settings.action.contains(.clearData) {
+                Task { @MainActor in
+                    await forgetData()
+                }
+            }
+        } else {
+            if let storedModel = TabsModel.get() {
+                // Save new model in case of migration
+                storedModel.save()
+                tabsModel = storedModel
+            } else {
+                tabsModel = TabsModel(desktop: isPadDevice)
+            }
+        }
+        tabManager = TabManager(model: tabsModel,
+                                previewsSource: previewsSource,
+                                bookmarksDatabase: bookmarksDatabase,
+                                historyManager: historyManager,
+                                syncService: syncService,
+                                delegate: self)
+    }
+
     private func addLaunchTabNotificationObserver() {
         launchTabObserver = LaunchTabNotification.addObserver(handler: { [weak self] urlString in
             guard let self = self else { return }
@@ -869,7 +881,6 @@ class MainViewController: UIViewController {
                 selectTab(existing)
                 return
             } else if reuseExisting, let existing = tabManager.firstHomeTab() {
-                doRefreshAfterClear = false
                 tabManager.selectTab(existing)
                 loadUrl(url, fromExternalLink: fromExternalLink)
             } else {
@@ -1997,7 +2008,7 @@ extension MainViewController: TabDelegate {
         if currentTab == tab {
             refreshControls()
         }
-        tabManager.save()
+        tabManager?.save()
         tabsBarController?.refresh(tabsModel: tabManager.model)
         // note: model in swipeTabsCoordinator doesn't need to be updated here
         // https://app.asana.com/0/414235014887631/1206847376910045/f
@@ -2244,7 +2255,7 @@ extension MainViewController: TabSwitcherButtonDelegate {
     }
 
     func showTabSwitcher() {
-        guard let currentTab = currentTab ?? tabManager.current(createIfNeeded: true) else {
+        guard let currentTab = currentTab ?? tabManager?.current(createIfNeeded: true) else {
             fatalError("Unable to get current tab")
         }
 
@@ -2301,10 +2312,6 @@ extension MainViewController: AutoClearWorker {
     }
 
     func refreshUIAfterClear() {
-        guard doRefreshAfterClear else {
-            doRefreshAfterClear = true
-            return
-        }
         showBars()
         attachHomeScreen()
         tabsBarController?.refresh(tabsModel: tabManager.model)
@@ -2317,7 +2324,6 @@ extension MainViewController: AutoClearWorker {
         refreshUIAfterClear()
     }
 
-    @MainActor
     func forgetData() async {
         guard !clearInProgress else {
             assertionFailure("Shouldn't get called multiple times")

diff --git a/DuckDuckGo/TabManager.swift b/DuckDuckGo/TabManager.swift
@@ -35,7 +35,6 @@ class TabManager {
     private let historyManager: HistoryManager
     private let syncService: DDGSyncing
     private var previewsSource: TabPreviewsSource
-
     weak var delegate: TabDelegate?
 
     @UserDefaultsWrapper(key: .faviconTabsCacheNeedsCleanup, defaultValue: true)
@@ -46,7 +45,8 @@ class TabManager {
          previewsSource: TabPreviewsSource,
          bookmarksDatabase: CoreDataDatabase,
          historyManager: HistoryManager,
-         syncService: DDGSyncing) {
+         syncService: DDGSyncing,
+         delegate: TabDelegate) {
         self.model = model
         self.previewsSource = previewsSource
         self.bookmarksDatabase = bookmarksDatabase

diff --git a/DuckDuckGoTests/AutoClearTests.swift b/DuckDuckGoTests/AutoClearTests.swift
@@ -49,20 +49,19 @@ class AutoClearTests: XCTestCase {
 
     private var worker: MockWorker!
     private var logic: AutoClear!
-    private var appSettings: AppSettingsMock!
-
-    override func setUp() async throws {
-        try await super.setUp()
 
+    override func setUp() {
+        super.setUp()
+
         worker = MockWorker()
-        appSettings = AppSettingsMock()
-        logic = AutoClear(worker: worker, appSettings: appSettings)
+        logic = AutoClear(worker: worker)
     }
 
     // Note: applicationDidLaunch based clearing has moved to "configureTabManager" function of
     //  MainViewController to ensure that tabs are removed before the data is cleared.
 
     func testWhenTimingIsSetToTerminationThenOnlyRestartClearsData() async {
+        let appSettings = AppUserDefaults()
         appSettings.autoClearAction = .clearData
         appSettings.autoClearTiming = .termination
 
@@ -71,14 +70,10 @@ class AutoClearTests: XCTestCase {
 
         XCTAssertEqual(worker.clearNavigationStackInvocationCount, 0)
         XCTAssertEqual(worker.forgetDataInvocationCount, 0)
-
-        await logic.applicationWillMoveToForeground()
-
-        XCTAssertEqual(worker.clearNavigationStackInvocationCount, 0)
-        XCTAssertEqual(worker.forgetDataInvocationCount, 0)
     }
 
     func testWhenDesiredTimingIsSetThenDataIsClearedOnceTimeHasElapsed() async {
+        let appSettings = AppUserDefaults()
         appSettings.autoClearAction = .clearData
 
         let cases: [AutoClearSettingsModel.Timing: TimeInterval] = [.delay5min: 5 * 60,