From d2272291ad42beed1d547d07b31eab8162e85b5e Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Fri, 19 Apr 2024 15:51:56 -0300 Subject: [PATCH] Revert "fix double clearing on autoclear (#2666)" This reverts commit cebfbf5e24294846a07e980ca949149c19f56f6f. --- DuckDuckGo/AppDelegate.swift | 33 +---------- DuckDuckGo/AutoClear.swift | 9 ++- DuckDuckGo/MainViewController.swift | 84 +++++++++++++++------------- DuckDuckGo/TabManager.swift | 4 +- DuckDuckGoTests/AutoClearTests.swift | 17 ++---- 5 files changed, 60 insertions(+), 87 deletions(-) diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 1bf54d8d4b..655081c6cb 100644 --- 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 index bda1c46614..eca57dce95 100644 --- 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 index d4d92afa17..f0bd94d1e8 100644 --- 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 index 7fd9b05d53..a2da65dea7 100644 --- 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 index 45e66212ba..2edb1e52ee 100644 --- 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,