Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix iOS auto-clearing fails to remove cookies #2767

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 3 additions & 30 deletions DuckDuckGo/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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) {
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
}
}
84 changes: 45 additions & 39 deletions DuckDuckGo/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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()
}
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -287,6 +264,7 @@ class MainViewController: UIViewController {
initTabButton()
initMenuButton()
initBookmarksButton()
configureTabManager()
loadInitialView()
previewsSource.prepare()
addLaunchTabNotificationObserver()
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -2301,10 +2312,6 @@ extension MainViewController: AutoClearWorker {
}

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

@MainActor
func forgetData() async {
guard !clearInProgress else {
assertionFailure("Shouldn't get called multiple times")
Expand Down
4 changes: 2 additions & 2 deletions 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 All @@ -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
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
Loading