Skip to content

Commit

Permalink
Revert "fix double clearing on autoclear (#2666)"
Browse files Browse the repository at this point in the history
This reverts commit cebfbf5.
  • Loading branch information
jotaemepereira committed Apr 19, 2024
1 parent 11b781d commit 8a513d1
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 85 deletions.
33 changes: 3 additions & 30 deletions DuckDuckGo/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,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,
Expand All @@ -294,18 +292,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()
Expand Down Expand Up @@ -360,27 +354,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(),
Expand Down Expand Up @@ -763,7 +736,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
}

Task { @MainActor in
// Autoclear should have happened by now
await autoClear?.applicationWillMoveToForeground()
showKeyboardIfSettingOn = false

if !handleAppDeepLink(app, mainViewController, url) {
Expand Down
9 changes: 4 additions & 5 deletions DuckDuckGo/AutoClear.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
82 changes: 44 additions & 38 deletions DuckDuckGo/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,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
Expand Down Expand Up @@ -138,7 +136,7 @@ class MainViewController: UIViewController {

lazy var tabSwitcherTransition = TabSwitcherTransitionDelegate()
var currentTab: TabViewController? {
return tabManager.current(createIfNeeded: false)
return tabManager?.current(createIfNeeded: false)
}

var searchBarRect: CGRect {
Expand Down Expand Up @@ -173,9 +171,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
Expand All @@ -186,17 +182,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()
}
Expand All @@ -207,9 +195,7 @@ class MainViewController: UIViewController {
historyManager: HistoryManager,
syncService: DDGSyncing,
syncDataProviders: SyncDataProviders,
appSettings: AppSettings,
previewsSource: TabPreviewsSource,
tabsModel: TabsModel
appSettings: AppSettings
) {
self.bookmarksDatabase = bookmarksDatabase
self.bookmarksDatabaseCleaner = bookmarksDatabaseCleaner
Expand All @@ -219,18 +205,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
Expand Down Expand Up @@ -284,6 +261,7 @@ class MainViewController: UIViewController {
initTabButton()
initMenuButton()
initBookmarksButton()
configureTabManager()
loadInitialView()
previewsSource.prepare()
addLaunchTabNotificationObserver()
Expand Down Expand Up @@ -736,6 +714,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 }
Expand Down Expand Up @@ -893,7 +905,6 @@ class MainViewController: UIViewController {
selectTab(existing)
return
} else if reuseExisting, let existing = tabManager.firstHomeTab() {
doRefreshAfterClear = false
tabManager.selectTab(existing)
loadUrl(url, fromExternalLink: fromExternalLink)
} else {
Expand Down Expand Up @@ -2069,7 +2080,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
Expand Down Expand Up @@ -2375,10 +2386,6 @@ extension MainViewController: AutoClearWorker {
}

func refreshUIAfterClear() {
guard doRefreshAfterClear else {
doRefreshAfterClear = true
return
}
showBars()
attachHomeScreen()
tabsBarController?.refresh(tabsModel: tabManager.model)
Expand All @@ -2391,7 +2398,6 @@ extension MainViewController: AutoClearWorker {
refreshUIAfterClear()
}

@MainActor
func forgetData() async {
guard !clearInProgress else {
assertionFailure("Shouldn't get called multiple times")
Expand Down
1 change: 0 additions & 1 deletion DuckDuckGo/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 6 additions & 11 deletions DuckDuckGoTests/AutoClearTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down

0 comments on commit 8a513d1

Please sign in to comment.