From ea5e28211a9d78fa083b0d461035f63643246c46 Mon Sep 17 00:00:00 2001 From: bwaresiak Date: Sat, 25 May 2024 00:15:42 +0200 Subject: [PATCH] Add debug pixels for data cleanup (#2890) Task/Issue URL: https://app.asana.com/0/856498667320406/1207313758437850/f Tech Design URL: CC: Description: Add debug information around data clearing logic. --- Core/DataStoreWarmup.swift | 12 +++- Core/PixelEvent.swift | 62 ++++++++++++------- Core/WebCacheManager.swift | 30 +++------ DuckDuckGo/AppDelegate.swift | 23 ++++++- DuckDuckGo/AutoClear.swift | 31 +++++----- DuckDuckGo/MainViewController.swift | 7 ++- DuckDuckGo/RulesCompilationMonitor.swift | 2 +- DuckDuckGoTests/AutoClearTests.swift | 17 +++-- .../FireButtonReferenceTests.swift | 2 +- 9 files changed, 116 insertions(+), 70 deletions(-) diff --git a/Core/DataStoreWarmup.swift b/Core/DataStoreWarmup.swift index 0ef67ac0b6..298a8b929f 100644 --- a/Core/DataStoreWarmup.swift +++ b/Core/DataStoreWarmup.swift @@ -23,11 +23,21 @@ import WebKit /// WKWebsiteDataStore is basically non-functional until a web view has been instanciated and a page is successfully loaded. public class DataStoreWarmup { + public enum ApplicationState: String { + case active + case inactive + case background + case handlingShortcut + case unknown + } + public init() { } @MainActor - public func ensureReady() async { + public func ensureReady(applicationState: ApplicationState) async { + Pixel.fire(pixel: .webkitWarmupStart(appState: applicationState.rawValue)) await BlockingNavigationDelegate().loadInBackgroundWebView(url: URL(string: "about:blank")!) + Pixel.fire(pixel: .webkitWarmupFinished(appState: applicationState.rawValue)) } } diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 81c7c654de..464c2e3a67 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -453,16 +453,20 @@ extension Pixel { case blankOverlayNotDismissed - case cookieDeletionTimedOut + case cookieDeletionTime(_ time: BucketAggregation) case cookieDeletionLeftovers - + case legacyDataClearingTime(_ time: BucketAggregation) + + case webkitWarmupStart(appState: String) + case webkitWarmupFinished(appState: String) + case cachedTabPreviewsExceedsTabCount case cachedTabPreviewRemovalError case missingDownloadedFile case unhandledDownload - case compilationResult(result: CompileRulesResult, waitTime: CompileRulesWaitTime, appState: AppState) + case compilationResult(result: CompileRulesResult, waitTime: BucketAggregation, appState: AppState) case emailAutofillKeychainError @@ -495,7 +499,6 @@ extension Pixel { case debugCannotClearObservationsDatabase case debugWebsiteDataStoresNotClearedMultiple case debugWebsiteDataStoresNotClearedOne - case debugCookieCleanupError case debugBookmarksMigratedMoreThanOnce @@ -1126,9 +1129,17 @@ extension Pixel.Event { case .blankOverlayNotDismissed: return "m_d_ovs" - case .cookieDeletionTimedOut: return "m_debug_cookie-clearing-timeout" + case .cookieDeletionTime(let aggregation): + return "m_debug_cookie-clearing-time-\(aggregation)" + case .legacyDataClearingTime(let aggregation): + return "m_debug_legacy-data-clearing-time-\(aggregation)" case .cookieDeletionLeftovers: return "m_cookie_deletion_leftovers" - + + case .webkitWarmupStart(let appState): + return "m_webkit-warmup-start-\(appState)" + case .webkitWarmupFinished(let appState): + return "m_webkit-warmup-finished-\(appState)" + case .cachedTabPreviewsExceedsTabCount: return "m_d_tpetc" case .cachedTabPreviewRemovalError: return "m_d_tpre" @@ -1155,7 +1166,6 @@ extension Pixel.Event { case .debugCannotClearObservationsDatabase: return "m_d_cannot_clear_observations_database" case .debugWebsiteDataStoresNotClearedMultiple: return "m_d_wkwebsitedatastoresnotcleared_multiple" case .debugWebsiteDataStoresNotClearedOne: return "m_d_wkwebsitedatastoresnotcleared_one" - case .debugCookieCleanupError: return "m_d_cookie-cleanup-error" // MARK: Ad Attribution @@ -1404,32 +1414,38 @@ extension Pixel.Event { // swiftlint:disable file_length extension Pixel.Event { - public enum CompileRulesWaitTime: String, CustomStringConvertible { - + public enum BucketAggregation: String, CustomStringConvertible { + public var description: String { rawValue } - case noWait = "0" - case lessThan1s = "1" - case lessThan5s = "5" - case lessThan10s = "10" - case lessThan20s = "20" - case lessThan40s = "40" + case zero = "0" + case lessThan01 = "0.1" + case lessThan05 = "0.5" + case lessThan1 = "1" + case lessThan5 = "5" + case lessThan10 = "10" + case lessThan20 = "20" + case lessThan40 = "40" case more - public init(waitTime: TimeInterval) { - switch waitTime { + public init(number: Double) { + switch number { case 0: - self = .noWait + self = .zero + case ...0.1: + self = .lessThan01 + case ...0.5: + self = .lessThan05 case ...1: - self = .lessThan1s + self = .lessThan1 case ...5: - self = .lessThan5s + self = .lessThan5 case ...10: - self = .lessThan10s + self = .lessThan10 case ...20: - self = .lessThan20s + self = .lessThan20 case ...40: - self = .lessThan40s + self = .lessThan40 default: self = .more } diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index faefc24b3b..682afae536 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -66,22 +66,14 @@ public class WebCacheManager { public func removeCookies(forDomains domains: [String], dataStore: WKWebsiteDataStore) async { - - let timeoutTask = Task.detached { - try? await Task.sleep(interval: 5.0) - if !Task.isCancelled { - Pixel.fire(pixel: .cookieDeletionTimedOut, withAdditionalParameters: [ - PixelParameters.removeCookiesTimedOut: "1" - ]) - } - } - + let startTime = CACurrentMediaTime() let cookieStore = dataStore.httpCookieStore let cookies = await cookieStore.allCookies() for cookie in cookies where domains.contains(where: { cookie.matchesDomain($0) }) { await cookieStore.deleteCookie(cookie) } - timeoutTask.cancel() + let totalTime = CACurrentMediaTime() - startTime + Pixel.fire(pixel: .cookieDeletionTime(.init(number: totalTime))) } public func clear(cookieStorage: CookieStorage = CookieStorage(), @@ -131,15 +123,10 @@ extension WebCacheManager { } private func legacyDataClearing() async -> [HTTPCookie]? { - let timeoutTask = Task.detached { - try? await Task.sleep(interval: 5.0) - if !Task.isCancelled { - Pixel.fire(pixel: .cookieDeletionTimedOut, withAdditionalParameters: [ - PixelParameters.clearWebDataTimedOut: "1" - ]) - } - } + let dataStore = WKWebsiteDataStore.default() + let startTime = CACurrentMediaTime() + let cookies = await dataStore.httpCookieStore.allCookies() var types = WKWebsiteDataStore.allWebsiteDataTypes() types.insert("_WKWebsiteDataTypeMediaKeys") @@ -152,8 +139,11 @@ extension WebCacheManager { types.insert("_WKWebsiteDataTypeAlternativeServices") await dataStore.removeData(ofTypes: types, modifiedSince: .distantPast) + self.removeObservationsData() - timeoutTask.cancel() + let totalTime = CACurrentMediaTime() - startTime + Pixel.fire(pixel: .legacyDataClearingTime(.init(number: totalTime))) + return cookies } diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 442c26084c..e9ea518b31 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -300,8 +300,9 @@ import WebKit } autoClear = AutoClear(worker: main) + let applicationState = application.applicationState Task { - await autoClear?.clearDataIfEnabled(launching: true) + await autoClear?.clearDataIfEnabled(applicationState: .init(with: applicationState)) } AppDependencyProvider.shared.voiceSearchHelper.migrateSettingsFlagIfNecessary() @@ -663,7 +664,7 @@ import WebKit Task { @MainActor in await beginAuthentication() - await autoClear?.clearDataIfEnabledAndTimeExpired() + await autoClear?.clearDataIfEnabledAndTimeExpired(applicationState: .active) showKeyboardIfSettingOn = true syncService.scheduler.resumeSyncQueue() } @@ -843,7 +844,7 @@ import WebKit if appIsLaunching { await autoClear?.clearDataIfEnabled() } else { - await autoClear?.clearDataIfEnabledAndTimeExpired() + await autoClear?.clearDataIfEnabledAndTimeExpired(applicationState: .active) } if shortcutItem.type == ShortcutKey.clipboard, let query = UIPasteboard.general.string { @@ -1050,6 +1051,22 @@ extension AppDelegate: UNUserNotificationCenterDelegate { } } +extension DataStoreWarmup.ApplicationState { + + init(with state: UIApplication.State) { + switch state { + case .inactive: + self = .inactive + case .active: + self = .active + case .background: + self = .background + @unknown default: + self = .unknown + } + } +} + private extension Error { var isDiskFull: Bool { diff --git a/DuckDuckGo/AutoClear.swift b/DuckDuckGo/AutoClear.swift index 10acc03518..5f507f809a 100644 --- a/DuckDuckGo/AutoClear.swift +++ b/DuckDuckGo/AutoClear.swift @@ -19,56 +19,58 @@ import Foundation import UIKit +import Core protocol AutoClearWorker { - + func clearNavigationStack() func forgetData() async + func forgetData(applicationState: DataStoreWarmup.ApplicationState) async func forgetTabs() func clearDataFinished(_: AutoClear) } class AutoClear { - + private let worker: AutoClearWorker private var timestamp: TimeInterval? - + private let appSettings: AppSettings var isClearingEnabled: Bool { return AutoClearSettingsModel(settings: appSettings) != nil } - + init(worker: AutoClearWorker, appSettings: AppSettings = AppDependencyProvider.shared.appSettings) { self.worker = worker self.appSettings = appSettings } - + @MainActor - func clearDataIfEnabled(launching: Bool = false) async { + func clearDataIfEnabled(launching: Bool = false, applicationState: DataStoreWarmup.ApplicationState = .unknown) async { guard let settings = AutoClearSettingsModel(settings: appSettings) else { return } - + if settings.action.contains(.clearTabs) { worker.forgetTabs() } if settings.action.contains(.clearData) { - await worker.forgetData() + await worker.forgetData(applicationState: applicationState) } if !launching { worker.clearDataFinished(self) } } - + /// Note: function is parametrised because of tests. func startClearingTimer(_ time: TimeInterval = Date().timeIntervalSince1970) { timestamp = time } - + private func shouldClearData(elapsedTime: TimeInterval) -> Bool { guard let settings = AutoClearSettingsModel(settings: appSettings) else { return false } - + switch settings.timing { case .termination: return false @@ -82,15 +84,16 @@ class AutoClear { return elapsedTime > 60 * 60 } } - + @MainActor - func clearDataIfEnabledAndTimeExpired(baseTimeInterval: TimeInterval = Date().timeIntervalSince1970) async { + func clearDataIfEnabledAndTimeExpired(baseTimeInterval: TimeInterval = Date().timeIntervalSince1970, + applicationState: DataStoreWarmup.ApplicationState) async { guard isClearingEnabled, let timestamp = timestamp, shouldClearData(elapsedTime: baseTimeInterval - timestamp) else { return } self.timestamp = nil worker.clearNavigationStack() - await clearDataIfEnabled() + await clearDataIfEnabled(applicationState: applicationState) } } diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 0d066b858e..78b3589de1 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -2435,6 +2435,11 @@ extension MainViewController: AutoClearWorker { @MainActor func forgetData() async { + await forgetData(applicationState: .unknown) + } + + @MainActor + func forgetData(applicationState: DataStoreWarmup.ApplicationState) async { guard !clearInProgress else { assertionFailure("Shouldn't get called multiple times") return @@ -2443,7 +2448,7 @@ extension MainViewController: AutoClearWorker { // This needs to happen only once per app launch if let dataStoreWarmup { - await dataStoreWarmup.ensureReady() + await dataStoreWarmup.ensureReady(applicationState: applicationState) self.dataStoreWarmup = nil } diff --git a/DuckDuckGo/RulesCompilationMonitor.swift b/DuckDuckGo/RulesCompilationMonitor.swift index bf85ceda92..1282bff99f 100644 --- a/DuckDuckGo/RulesCompilationMonitor.swift +++ b/DuckDuckGo/RulesCompilationMonitor.swift @@ -92,7 +92,7 @@ final class RulesCompilationMonitor { private func reportWaitTime(_ waitTime: TimeInterval, result: Pixel.Event.CompileRulesResult) { didReport = true Pixel.fire(pixel: .compilationResult(result: result, - waitTime: Pixel.Event.CompileRulesWaitTime(waitTime: waitTime), + waitTime: Pixel.Event.BucketAggregation(number: waitTime), appState: isOnboarding ? .onboarding : .regular), withAdditionalParameters: [Const.waitTime: String(waitTime)]) } diff --git a/DuckDuckGoTests/AutoClearTests.swift b/DuckDuckGoTests/AutoClearTests.swift index b20c744a38..1cc8383bf9 100644 --- a/DuckDuckGoTests/AutoClearTests.swift +++ b/DuckDuckGoTests/AutoClearTests.swift @@ -24,7 +24,7 @@ import XCTest class AutoClearTests: XCTestCase { class MockWorker: AutoClearWorker { - + var clearNavigationStackInvocationCount = 0 var forgetDataInvocationCount = 0 var forgetTabsInvocationCount = 0 @@ -37,7 +37,12 @@ class AutoClearTests: XCTestCase { func forgetData() { forgetDataInvocationCount += 1 } - + + func forgetData(applicationState: Core.DataStoreWarmup.ApplicationState) { + forgetDataInvocationCount += 1 + } + + func forgetTabs() { forgetTabsInvocationCount += 1 } @@ -59,13 +64,13 @@ class AutoClearTests: XCTestCase { appSettings.autoClearAction = .clearData appSettings.autoClearTiming = .termination - await logic.clearDataIfEnabledAndTimeExpired() + await logic.clearDataIfEnabledAndTimeExpired(applicationState: .unknown) logic.startClearingTimer() XCTAssertEqual(worker.clearNavigationStackInvocationCount, 0) XCTAssertEqual(worker.forgetDataInvocationCount, 0) - await logic.clearDataIfEnabledAndTimeExpired() + await logic.clearDataIfEnabledAndTimeExpired(applicationState: .unknown) XCTAssertEqual(worker.clearNavigationStackInvocationCount, 0) XCTAssertEqual(worker.forgetDataInvocationCount, 0) @@ -89,13 +94,13 @@ class AutoClearTests: XCTestCase { // Swift Concurrency appears to sometimes get delayed so we pass the base time internal to use just for tests // otherwise it's not computed until the functional is called - await logic.clearDataIfEnabledAndTimeExpired(baseTimeInterval: Date().timeIntervalSince1970) + await logic.clearDataIfEnabledAndTimeExpired(baseTimeInterval: Date().timeIntervalSince1970, applicationState: .unknown) XCTAssertEqual(worker.clearNavigationStackInvocationCount, iterationCount) XCTAssertEqual(worker.forgetDataInvocationCount, iterationCount) logic.startClearingTimer(Date().timeIntervalSince1970 - delay - 1) - await logic.clearDataIfEnabledAndTimeExpired(baseTimeInterval: Date().timeIntervalSince1970) + await logic.clearDataIfEnabledAndTimeExpired(baseTimeInterval: Date().timeIntervalSince1970, applicationState: .unknown) iterationCount += 1 XCTAssertEqual(worker.clearNavigationStackInvocationCount, iterationCount) diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index bf90916820..c24ff9a818 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -69,7 +69,7 @@ final class FireButtonReferenceTests: XCTestCase { XCTAssertFalse(idManager.hasId) let warmup = DataStoreWarmup() - await warmup.ensureReady() + await warmup.ensureReady(applicationState: .unknown) let cookieStore = WKWebsiteDataStore.default().httpCookieStore await cookieStore.setCookie(cookie)