From 1e73bcad84a51d1b960b629e8ac218ac13356c48 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Fri, 22 Nov 2024 13:48:45 +0000 Subject: [PATCH 01/14] initial refactoring and migration code --- Core/CookieStorage.swift | 157 --------------- Core/DataStoreIdManager.swift | 57 ++++++ Core/MigratableCookieStorage.swift | 72 +++++++ Core/PixelEvent.swift | 4 +- Core/WKWebViewConfigurationExtension.swift | 35 +--- ...acheManager+ObservationsDataClearing.swift | 64 ++++++ Core/WebCacheManager.swift | 185 +++++++----------- DuckDuckGo.xcodeproj/project.pbxproj | 20 +- DuckDuckGo/MainViewController.swift | 2 +- DuckDuckGoTests/CookieStorageTests.swift | 6 +- .../FireButtonReferenceTests.swift | 4 +- DuckDuckGoTests/WebCacheManagerTests.swift | 14 +- 12 files changed, 292 insertions(+), 328 deletions(-) delete mode 100644 Core/CookieStorage.swift create mode 100644 Core/DataStoreIdManager.swift create mode 100644 Core/MigratableCookieStorage.swift create mode 100644 Core/WebCacheManager+ObservationsDataClearing.swift diff --git a/Core/CookieStorage.swift b/Core/CookieStorage.swift deleted file mode 100644 index 978d69197a..0000000000 --- a/Core/CookieStorage.swift +++ /dev/null @@ -1,157 +0,0 @@ -// -// CookieStorage.swift -// DuckDuckGo -// -// Copyright © 2018 DuckDuckGo. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Common -import Foundation -import os.log - -/// Class for persisting cookies for fire proofed sites to work around a WKWebView / DataStore bug which does not let data get persisted until the webview has loaded. -/// -/// Privacy information: -/// * The Fire Button does not delete the user's DuckDuckGo search settings, which are saved as cookies. Removing these cookies would reset them and have undesired consequences, i.e. changing the theme, default language, etc. -/// * The Fire Button also does not delete temporary cookies associated with 'surveys.duckduckgo.com'. When we launch surveys to help us understand issues that impact users over time, we use this cookie to temporarily store anonymous survey answers, before deleting the cookie. Cookie storage duration is communicated to users before they opt to submit survey answers. -/// * These cookies are not stored in a personally identifiable way. For example, the large size setting is stored as 's=l.' More info in https://duckduckgo.com/privacy -public class CookieStorage { - - struct Keys { - static let allowedCookies = "com.duckduckgo.allowedCookies" - static let consumed = "com.duckduckgo.consumedCookies" - } - - private var userDefaults: UserDefaults - - var isConsumed: Bool { - get { - return userDefaults.bool(forKey: Keys.consumed, defaultValue: false) - } - set { - userDefaults.set(newValue, forKey: Keys.consumed) - } - } - - /// Use the `updateCookies` function rather than the setter which is only visible for testing. - var cookies: [HTTPCookie] { - get { - var storedCookies = [HTTPCookie]() - if let cookies = userDefaults.object(forKey: Keys.allowedCookies) as? [[String: Any?]] { - for cookieData in cookies { - var properties = [HTTPCookiePropertyKey: Any]() - cookieData.forEach({ - properties[HTTPCookiePropertyKey(rawValue: $0.key)] = $0.value - }) - - if let cookie = HTTPCookie(properties: properties) { - Logger.general.debug("read cookie \(cookie.domain) \(cookie.name) \(cookie.value)") - storedCookies.append(cookie) - } - } - } - - return storedCookies - } - - set { - var cookies = [[String: Any?]]() - newValue.forEach { cookie in - var mappedCookie = [String: Any?]() - cookie.properties?.forEach { - mappedCookie[$0.key.rawValue] = $0.value - } - cookies.append(mappedCookie) - } - userDefaults.setValue(cookies, forKey: Keys.allowedCookies) - } - - } - - public init(userDefaults: UserDefaults = UserDefaults.app) { - self.userDefaults = userDefaults - } - - /// Used when debugging (e.g. on the simulator). - enum CookieDomainsOnUpdateDiagnostic { - case empty - case match - case missing - case different - case notConsumed - } - - /// Update ALL cookies. The absence of cookie domains here indicateds they have been removed by the website, so be sure to call this with all cookies that might need to be persisted even if those websites have not been visited yet. - @discardableResult - func updateCookies(_ cookies: [HTTPCookie], preservingFireproofedDomains fireproofing: Fireproofing) -> CookieDomainsOnUpdateDiagnostic { - guard isConsumed else { return .notConsumed } - - isConsumed = false - - let persisted = self.cookies - - func cookiesByDomain(_ cookies: [HTTPCookie]) -> [String: [HTTPCookie]] { - var byDomain = [String: [HTTPCookie]]() - cookies.forEach { cookie in - var cookies = byDomain[cookie.domain, default: []] - cookies.append(cookie) - byDomain[cookie.domain] = cookies - } - return byDomain - } - - let updatedCookiesByDomain = cookiesByDomain(cookies) - var persistedCookiesByDomain = cookiesByDomain(persisted) - - // Do the diagnostics before the dicts get changed. - let diagnosticResult = evaluateDomains( - updatedDomains: updatedCookiesByDomain.keys.sorted(), - persistedDomains: persistedCookiesByDomain.keys.sorted() - ) - - let cookieDomains = Set(updatedCookiesByDomain.keys.map { $0 } + persistedCookiesByDomain.keys.map { $0 }) - - cookieDomains.forEach { - persistedCookiesByDomain[$0] = updatedCookiesByDomain[$0] - } - - persistedCookiesByDomain.keys.forEach { - guard !URL.isDuckDuckGo(domain: $0) else { return } // DDG cookies are for SERP settings only - - if !fireproofing.isAllowed(cookieDomain: $0) { - persistedCookiesByDomain.removeValue(forKey: $0) - } - } - - let now = Date() - self.cookies = persistedCookiesByDomain.map { $0.value }.joined().compactMap { $0 } - .filter { $0.expiresDate == nil || $0.expiresDate! > now } - - return diagnosticResult - } - - private func evaluateDomains(updatedDomains: [String], persistedDomains: [String]) -> CookieDomainsOnUpdateDiagnostic { - if persistedDomains.isEmpty { - return .empty - } else if updatedDomains.count < persistedDomains.count { - return .missing - } else if updatedDomains == persistedDomains { - return .match - } else { - return .different - } - } - -} diff --git a/Core/DataStoreIdManager.swift b/Core/DataStoreIdManager.swift new file mode 100644 index 0000000000..a033d0b155 --- /dev/null +++ b/Core/DataStoreIdManager.swift @@ -0,0 +1,57 @@ +// +// DataStoreIdManager.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + + +import WebKit +import Persistence + +/// Supports an existing ID set in previous versions of the app, but moving forward does not allocate an ID. We have gone back to using the default +/// peristence for the webview storage so that we can fireproof types that don't have an API for accessing their data. (e.g. localStorage) +public protocol DataStoreIdManaging { + + var currentId: UUID? { get } + + func invalidateCurrentId() +} + +public class DataStoreIdManager: DataStoreIdManaging { + + enum Constants: String { + case currentWebContainerId = "com.duckduckgo.ios.webcontainer.id" + } + + public static let shared = DataStoreIdManager() + + private let store: KeyValueStoring + init(store: KeyValueStoring = UserDefaults.app) { + self.store = store + } + + public var currentId: UUID? { + guard let uuidString = store.object(forKey: Constants.currentWebContainerId.rawValue) as? String else { + return nil + } + return UUID(uuidString: uuidString) + } + + public func invalidateCurrentId() { + store.removeObject(forKey: Constants.currentWebContainerId.rawValue) + } + +} diff --git a/Core/MigratableCookieStorage.swift b/Core/MigratableCookieStorage.swift new file mode 100644 index 0000000000..2cd69420cf --- /dev/null +++ b/Core/MigratableCookieStorage.swift @@ -0,0 +1,72 @@ +// +// MigratableCookieStorage.swift +// DuckDuckGo +// +// Copyright © 2018 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Common +import Foundation +import os.log + +/// This class was used to store cookies when moving between persistences containers, but now we are clearing the default container, this class is only used for storing cookies that existed previously and need to be migrated. +public class MigratableCookieStorage { + + struct Keys { + static let allowedCookies = "com.duckduckgo.allowedCookies" + static let consumed = "com.duckduckgo.consumedCookies" + } + + private var userDefaults: UserDefaults + + var isConsumed: Bool { + get { + return userDefaults.bool(forKey: Keys.consumed, defaultValue: false) + } + set { + userDefaults.set(newValue, forKey: Keys.consumed) + } + } + + var cookies: [HTTPCookie] { + var storedCookies = [HTTPCookie]() + if let cookies = userDefaults.object(forKey: Keys.allowedCookies) as? [[String: Any?]] { + for cookieData in cookies { + var properties = [HTTPCookiePropertyKey: Any]() + cookieData.forEach({ + properties[HTTPCookiePropertyKey(rawValue: $0.key)] = $0.value + }) + + if let cookie = HTTPCookie(properties: properties) { + Logger.general.debug("read cookie \(cookie.domain) \(cookie.name) \(cookie.value)") + storedCookies.append(cookie) + } + } + } + + return storedCookies + } + + public init(userDefaults: UserDefaults = UserDefaults.app) { + self.userDefaults = userDefaults + } + + /// Called when migration is completed to clean up + func migrationComplete() { + userDefaults.removeObject(forKey: Keys.allowedCookies) + userDefaults.removeObject(forKey: Keys.consumed) + } + +} diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 85416a001c..0145644968 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -533,7 +533,7 @@ extension Pixel { case cookieDeletionTime(_ time: BucketAggregation) case cookieDeletionLeftovers - case legacyDataClearingTime(_ time: BucketAggregation) + case clearDataInDefaultPersistence(_ time: BucketAggregation) case webkitWarmupStart(appState: String) case webkitWarmupFinished(appState: String) @@ -1369,7 +1369,7 @@ extension Pixel.Event { case .cookieDeletionTime(let aggregation): return "m_debug_cookie-clearing-time-\(aggregation)" - case .legacyDataClearingTime(let aggregation): + case .clearDataInDefaultPersistence(let aggregation): return "m_debug_legacy-data-clearing-time-\(aggregation)" case .cookieDeletionLeftovers: return "m_cookie_deletion_leftovers" diff --git a/Core/WKWebViewConfigurationExtension.swift b/Core/WKWebViewConfigurationExtension.swift index 18de2c2dad..810b08f6a5 100644 --- a/Core/WKWebViewConfigurationExtension.swift +++ b/Core/WKWebViewConfigurationExtension.swift @@ -26,7 +26,7 @@ extension WKWebViewConfiguration { public static func persistent(idManager: DataStoreIdManaging = DataStoreIdManager.shared) -> WKWebViewConfiguration { let config = configuration(persistsData: true) - // Only use a container if there's an id which will be allocated next time the fire button is used. + // Only use a container if there's an id. We no longer allocate ids so this should not happen. if #available(iOS 17, *), let containerId = idManager.currentId { config.websiteDataStore = WKWebsiteDataStore(forIdentifier: containerId) } @@ -56,36 +56,3 @@ extension WKWebViewConfiguration { } } - -public protocol DataStoreIdManaging { - - var currentId: UUID? { get } - - func invalidateCurrentIdAndAllocateNew() -} - -public class DataStoreIdManager: DataStoreIdManaging { - - enum Constants: String { - case currentWebContainerId = "com.duckduckgo.ios.webcontainer.id" - } - - public static let shared = DataStoreIdManager() - - private let store: KeyValueStoring - init(store: KeyValueStoring = UserDefaults.app) { - self.store = store - } - - public var currentId: UUID? { - guard let uuidString = store.object(forKey: Constants.currentWebContainerId.rawValue) as? String else { - return nil - } - return UUID(uuidString: uuidString) - } - - public func invalidateCurrentIdAndAllocateNew() { - store.set(UUID().uuidString, forKey: Constants.currentWebContainerId.rawValue) - } - -} diff --git a/Core/WebCacheManager+ObservationsDataClearing.swift b/Core/WebCacheManager+ObservationsDataClearing.swift new file mode 100644 index 0000000000..ada6aa974f --- /dev/null +++ b/Core/WebCacheManager+ObservationsDataClearing.swift @@ -0,0 +1,64 @@ +// +// WebCacheManager+ObservationsDataClearing.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Common +import GRDB +import os.log + +extension WebCacheManager { + + func removeObservationsData() { + if let pool = getValidDatabasePool() { + removeObservationsData(from: pool) + } else { + Logger.general.debug("Could not find valid pool to clear observations data") + } + } + + private func getValidDatabasePool() -> DatabasePool? { + let bundleID = Bundle.main.bundleIdentifier ?? "" + + let databaseURLs = [ + FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask)[0] + .appendingPathComponent("WebKit/WebsiteData/ResourceLoadStatistics/observations.db"), + FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask)[0] + .appendingPathComponent("WebKit/\(bundleID)/WebsiteData/ResourceLoadStatistics/observations.db") + ] + + guard let validURL = databaseURLs.first(where: { FileManager.default.fileExists(atPath: $0.path) }) else { return nil } + + return try? DatabasePool(path: validURL.absoluteString) + } + + private func removeObservationsData(from pool: DatabasePool) { + do { + try pool.write { database in + try database.execute(sql: "PRAGMA wal_checkpoint(TRUNCATE);") + + let tables = try String.fetchAll(database, sql: "SELECT name FROM sqlite_master WHERE type='table'") + + for table in tables { + try database.execute(sql: "DELETE FROM \(table)") + } + } + } catch { + Pixel.fire(pixel: .debugCannotClearObservationsDatabase, error: error) + } + } +} diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 13ce952ba8..1abead7ddd 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -19,7 +19,6 @@ import Common import WebKit -import GRDB import os.log extension WKWebsiteDataStore { @@ -52,7 +51,7 @@ public class WebCacheManager { /// We save cookies from the current container rather than copying them to a new container because /// the container only persists cookies to disk when the web view is used. If the user presses the fire button /// twice then the fire proofed cookies will be lost and the user will be logged out any sites they're logged in to. - public func consumeCookies(cookieStorage: CookieStorage = CookieStorage(), + public func consumeCookies(cookieStorage: MigratableCookieStorage = MigratableCookieStorage(), httpCookieStore: WKHTTPCookieStore) async { guard !cookieStorage.isConsumed else { return } @@ -77,42 +76,72 @@ public class WebCacheManager { Pixel.fire(pixel: .cookieDeletionTime(.init(number: totalTime))) } - public func clear(cookieStorage: CookieStorage = CookieStorage(), + public func clear(fromDataStore dataStore: WKWebsiteDataStore, + cookieStorage: MigratableCookieStorage = MigratableCookieStorage(), fireproofing: Fireproofing = UserDefaultsFireproofing.shared, dataStoreIdManager: DataStoreIdManaging = DataStoreIdManager.shared) async { - var cookiesToUpdate = [HTTPCookie]() - var leftoverContainerIDs = [UUID]() - if #available(iOS 17, *) { - let result = await containerBasedClearing(storeIdManager: dataStoreIdManager) - cookiesToUpdate += result.cookies - leftoverContainerIDs = result.leftoverContainerIDs + await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, destinationStore: dataStore) + await clearData(fromDataStore: dataStore, withFireproofing: fireproofing) + removeContainersIfNeeded() + + } + +} + +extension WebCacheManager { + + private func performMigrationIfNeeded(dataStoreIdManager: DataStoreIdManaging, + cookieStorage: MigratableCookieStorage = MigratableCookieStorage(), + destinationStore: WKWebsiteDataStore) async { + + // Check version here rather than on function so that we don't need complicated logic related to verison in the calling function + guard #available(iOS 17, *) else { return } + + // If there's no id, then migration has been done or isn't needed + guard dataStoreIdManager.currentId != nil else { return } + + // Get all cookies, we'll clean them later to keep all that logic in the same place + let cookies = cookieStorage.cookies + + // The returned cookies should be kept so move them to the data store + for cookie in cookies { + await destinationStore.httpCookieStore.setCookie(cookie) } - // Perform legacy clearing to migrate to new container - cookiesToUpdate += await legacyDataClearing() ?? [] + cookieStorage.migrationComplete() + dataStoreIdManager.invalidateCurrentId() + } - cookieStorage.updateCookies(cookiesToUpdate, preservingFireproofedDomains: fireproofing) + private func removeContainersIfNeeded() { + // Check version here rather than on function so that we don't need complicated logic related to verison in the calling function + guard #available(iOS 17, *) else { return } - // Attempt to clean up leftover stores again after a delay - // This should not be a problem as these containers are not supposed to be used anymore. + // Attempt to clean up all previous stores, but wait for a few seconds. // If this fails, we are going to still clean them next time as WebKit keeps track of all stores for us. - if #available(iOS 17, *), !leftoverContainerIDs.isEmpty { - Task { - try? await Task.sleep(for: .seconds(3)) - for uuid in leftoverContainerIDs { - try? await WKWebsiteDataStore.remove(forIdentifier: uuid) - } + Task { + try? await Task.sleep(for: .seconds(3)) + for uuid in await WKWebsiteDataStore.allDataStoreIdentifiers { + try? await WKWebsiteDataStore.remove(forIdentifier: uuid) } - } - } -} + let count = await WKWebsiteDataStore.allDataStoreIdentifiers.count + switch count { + case 0: + Pixel.fire(pixel: .debugWebsiteDataStoresCleared) -extension WebCacheManager { + case 1: + Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedOne) + + default: + Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedMultiple) + } + } + } - @available(iOS 17, *) private func checkForLeftBehindDataStores(previousLeftOversCount: Int) async { + guard #available(iOS 17, *) else { return } + let params = [ "left_overs_count": "\(previousLeftOversCount)" ] @@ -127,54 +156,17 @@ extension WebCacheManager { } } - @available(iOS 17, *) - private func containerBasedClearing(storeIdManager: DataStoreIdManaging) async -> (cookies: [HTTPCookie], - leftoverContainerIDs: [UUID]) { - guard let containerId = storeIdManager.currentId else { - storeIdManager.invalidateCurrentIdAndAllocateNew() - return ([], []) - } - storeIdManager.invalidateCurrentIdAndAllocateNew() - - var leftoverContainerIDs = [UUID]() - - var dataStore: WKWebsiteDataStore? = WKWebsiteDataStore(forIdentifier: containerId) - let cookies = await dataStore?.httpCookieStore.allCookies() ?? [] - dataStore = nil - - var uuids = await WKWebsiteDataStore.allDataStoreIdentifiers - - // There may be a timing issue related to fetching current container, so append previous UUID as a precaution - if uuids.firstIndex(of: containerId) == nil { - uuids.append(containerId) - } - - if let newContainerID = storeIdManager.currentId, - let newIdIndex = uuids.firstIndex(of: newContainerID) { - assertionFailure("Attempted to cleanup current Data Store") - uuids.remove(at: newIdIndex) - } - - let previousLeftOversCount = max(0, uuids.count - 1) // -1 because one store is expected to be cleared - for uuid in uuids { - do { - try await WKWebsiteDataStore.remove(forIdentifier: uuid) - } catch { - leftoverContainerIDs.append(uuid) - } - } - await checkForLeftBehindDataStores(previousLeftOversCount: previousLeftOversCount) - - return (cookies, leftoverContainerIDs) - } - - private func legacyDataClearing() async -> [HTTPCookie]? { - - let dataStore = WKWebsiteDataStore.default() + private func clearData(fromDataStore dataStore: WKWebsiteDataStore, withFireproofing fireproofing: Fireproofing) async { let startTime = CACurrentMediaTime() - let cookies = await dataStore.httpCookieStore.allCookies() + // Start with all types var types = WKWebsiteDataStore.allWebsiteDataTypes() + + // Remove types we want to retain + types.remove(WKWebsiteDataTypeCookies) + types.remove(WKWebsiteDataTypeLocalStorage) + + // Add types without an API constant that we also want to clear types.insert("_WKWebsiteDataTypeMediaKeys") types.insert("_WKWebsiteDataTypeHSTSCache") types.insert("_WKWebsiteDataTypeSearchFieldRecentSearches") @@ -184,55 +176,16 @@ extension WebCacheManager { types.insert("_WKWebsiteDataTypePrivateClickMeasurements") types.insert("_WKWebsiteDataTypeAlternativeServices") - await dataStore.removeData(ofTypes: types, modifiedSince: .distantPast) - - self.removeObservationsData() - let totalTime = CACurrentMediaTime() - startTime - Pixel.fire(pixel: .legacyDataClearingTime(.init(number: totalTime))) - - return cookies - } - - private func removeObservationsData() { - if let pool = getValidDatabasePool() { - removeObservationsData(from: pool) - } else { - Logger.general.debug("Could not find valid pool to clear observations data") + // Get a list of records that are NOT fireproofed + let removableRecords = await dataStore.dataRecords(ofTypes: types).filter { record in + !fireproofing.isAllowed(fireproofDomain: record.displayName) } - } - func getValidDatabasePool() -> DatabasePool? { - let bundleID = Bundle.main.bundleIdentifier ?? "" + await dataStore.removeData(ofTypes: types, for: removableRecords) - let databaseURLs = [ - FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask)[0] - .appendingPathComponent("WebKit/WebsiteData/ResourceLoadStatistics/observations.db"), - FileManager.default.urls(for: .libraryDirectory, in: .userDomainMask)[0] - .appendingPathComponent("WebKit/\(bundleID)/WebsiteData/ResourceLoadStatistics/observations.db") - ] - - guard let validURL = databaseURLs.first(where: { FileManager.default.fileExists(atPath: $0.path) }), - let pool = try? DatabasePool(path: validURL.absoluteString) else { - return nil - } - - return pool + self.removeObservationsData() + let totalTime = CACurrentMediaTime() - startTime + Pixel.fire(pixel: .clearDataInDefaultPersistence(.init(number: totalTime))) } - private func removeObservationsData(from pool: DatabasePool) { - do { - try pool.write { database in - try database.execute(sql: "PRAGMA wal_checkpoint(TRUNCATE);") - - let tables = try String.fetchAll(database, sql: "SELECT name FROM sqlite_master WHERE type='table'") - - for table in tables { - try database.execute(sql: "DELETE FROM \(table)") - } - } - } catch { - Pixel.fire(pixel: .debugCannotClearObservationsDatabase, error: error) - } - } - } diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 654bc03135..052e9699ed 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -540,7 +540,7 @@ 859DB8182CE6263C001F7210 /* TextZoomCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 859DB8122CE6263C001F7210 /* TextZoomCoordinator.swift */; }; 859DB81C2CE6268C001F7210 /* MockTextZoomCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 859DB81A2CE6268C001F7210 /* MockTextZoomCoordinator.swift */; }; 859DB81E2CE62766001F7210 /* TextZoomTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 859DB81B2CE6268C001F7210 /* TextZoomTests.swift */; }; - 85A1B3B220C6CD9900C18F15 /* CookieStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85A1B3B120C6CD9900C18F15 /* CookieStorage.swift */; }; + 85A1B3B220C6CD9900C18F15 /* MigratableCookieStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */; }; 85A313972028E78A00327D00 /* release_notes.txt in Resources */ = {isa = PBXBuildFile; fileRef = 85A313962028E78A00327D00 /* release_notes.txt */; }; 85A9C37920E0E00C00073340 /* HomeRow.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85A9C37820E0E00C00073340 /* HomeRow.xcassets */; }; 85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */; }; @@ -563,6 +563,8 @@ 85C29708247BDD060063A335 /* DaxDialogsBrowsingSpecTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29706247BDCFF0063A335 /* DaxDialogsBrowsingSpecTests.swift */; }; 85C2970A247EB7AA0063A335 /* Text.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85C29709247EB7AA0063A335 /* Text.xcassets */; }; 85C2971A248162CA0063A335 /* DaxOnboardingPadViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */; }; + 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */; }; + 85C503FB2CF0ADCE0075DF6F /* WebCacheManager+ObservationsDataClearing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */; }; 85C8E61D2B0E47380029A6BD /* BookmarksDatabaseSetup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */; }; 85C91CA224671F4C00A11132 /* AppDeepLinkSchemes.swift in Sources */ = {isa = PBXBuildFile; fileRef = F17D723B1E8BB374003E8B0E /* AppDeepLinkSchemes.swift */; }; 85D2187024BF24DB004373D2 /* FaviconRequestModifierTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85D2186F24BF24DB004373D2 /* FaviconRequestModifierTests.swift */; }; @@ -1851,7 +1853,7 @@ 859DB8122CE6263C001F7210 /* TextZoomCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextZoomCoordinator.swift; sourceTree = ""; }; 859DB81A2CE6268C001F7210 /* MockTextZoomCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockTextZoomCoordinator.swift; sourceTree = ""; }; 859DB81B2CE6268C001F7210 /* TextZoomTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextZoomTests.swift; sourceTree = ""; }; - 85A1B3B120C6CD9900C18F15 /* CookieStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CookieStorage.swift; sourceTree = ""; }; + 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MigratableCookieStorage.swift; sourceTree = ""; }; 85A313962028E78A00327D00 /* release_notes.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = release_notes.txt; path = fastlane/metadata/default/release_notes.txt; sourceTree = ""; }; 85A53EC9200D1FA20010D13F /* FileStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileStore.swift; sourceTree = ""; }; 85A9C37820E0E00C00073340 /* HomeRow.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = HomeRow.xcassets; sourceTree = ""; }; @@ -1876,6 +1878,8 @@ 85C29706247BDCFF0063A335 /* DaxDialogsBrowsingSpecTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxDialogsBrowsingSpecTests.swift; sourceTree = ""; }; 85C29709247EB7AA0063A335 /* Text.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Text.xcassets; sourceTree = ""; }; 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxOnboardingPadViewController.swift; sourceTree = ""; }; + 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIdManager.swift; sourceTree = ""; }; + 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WebCacheManager+ObservationsDataClearing.swift"; sourceTree = ""; }; 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksDatabaseSetup.swift; sourceTree = ""; }; 85CA53A324B9F2BD00A6288C /* Favicons.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Favicons.swift; sourceTree = ""; }; 85CA53A924BB376800A6288C /* NotFoundCachingDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = NotFoundCachingDownloader.swift; path = ../Core/NotFoundCachingDownloader.swift; sourceTree = ""; }; @@ -6015,18 +6019,20 @@ F143C3311E4A9A6A00CFDE3A /* Web */ = { isa = PBXGroup; children = ( - 85A1B3B120C6CD9900C18F15 /* CookieStorage.swift */, + 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */, + 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */, + 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */, + 8596C30C2B7EB1800058EF90 /* DataStoreWarmup.swift */, 85BDC3132434D8F80053DB07 /* DebugUserScript.swift */, + 850559CF23CF647C0055C0D5 /* Fireproofing.swift */, 4B60ACA0252EC0B100E8D219 /* FullScreenVideoUserScript.swift */, 85BDC3182436161C0053DB07 /* LoginFormDetectionUserScript.swift */, - 850559CF23CF647C0055C0D5 /* Fireproofing.swift */, 4B75EA9126A266CB00018634 /* PrintingUserScript.swift */, 988F3DCE237D5C0F00AEE34C /* SchemeHandler.swift */, 836A941C247F23C600BF8EF5 /* UserAgentManager.swift */, F1A886771F29394E0096251E /* WebCacheManager.swift */, 83004E7F2193BB8200DA013C /* WKNavigationExtension.swift */, 830381BF1F850AAF00863075 /* WKWebViewConfigurationExtension.swift */, - 8596C30C2B7EB1800058EF90 /* DataStoreWarmup.swift */, ); name = Web; sourceTree = ""; @@ -8346,7 +8352,7 @@ 85E065BC2C73A54700D73E2A /* UsageSegmentationStorage.swift in Sources */, F143C3271E4A9A0E00CFDE3A /* Logger+Multiple.swift in Sources */, 85372447220DD103009D09CD /* UIKeyCommandExtension.swift in Sources */, - 85A1B3B220C6CD9900C18F15 /* CookieStorage.swift in Sources */, + 85A1B3B220C6CD9900C18F15 /* MigratableCookieStorage.swift in Sources */, 9856A1992933D2EB00ACB44F /* BookmarksModelsErrorHandling.swift in Sources */, 850559D023CF647C0055C0D5 /* Fireproofing.swift in Sources */, 4B27FBAE2C924EC6007E21A7 /* PersistentPixelStoring.swift in Sources */, @@ -8376,6 +8382,7 @@ 85528AA72C7CA95D0017BCCA /* UsageSegmentationCalculator.swift in Sources */, 8598D2DC2CEB93AD00C45685 /* FaviconsCacheType.swift in Sources */, 1E6A4D692984208800A371D3 /* LocaleExtension.swift in Sources */, + 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */, 98F6EA472863124100720957 /* ContentBlockerRulesLists.swift in Sources */, 566B73732BECE4F200FF1959 /* SyncErrorHandling.swift in Sources */, F1134EB01F40AC6300B73467 /* AtbParser.swift in Sources */, @@ -8393,6 +8400,7 @@ 1D8F727F2BA86D8000E31493 /* PixelExperiment.swift in Sources */, 56D8556C2BEA91C4009F9698 /* SyncAlertsPresenting.swift in Sources */, 37FD780F2A29E28B00B36DB1 /* SyncErrorHandler.swift in Sources */, + 85C503FB2CF0ADCE0075DF6F /* WebCacheManager+ObservationsDataClearing.swift in Sources */, 85F21DC621145DD5002631A6 /* global.swift in Sources */, 372A0FF02B2389590033BF7F /* SyncMetricsEventsHandler.swift in Sources */, F41C2DA326C1925700F9A760 /* BookmarksAndFolders.xcdatamodeld in Sources */, diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index c6b5f59729..a952094b60 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -2660,7 +2660,7 @@ extension MainViewController: AutoClearWorker { URLSession.shared.configuration.urlCache?.removeAllCachedResponses() let pixel = TimedPixel(.forgetAllDataCleared) - await WebCacheManager.shared.clear() + await WebCacheManager.shared.clear(fromDataStore: WKWebsiteDataStore.default()) pixel.fire(withAdditionalParameters: [PixelParameters.tabCount: "\(self.tabManager.count)"]) AutoconsentManagement.shared.clearCache() diff --git a/DuckDuckGoTests/CookieStorageTests.swift b/DuckDuckGoTests/CookieStorageTests.swift index f200fc251e..7094a4daeb 100644 --- a/DuckDuckGoTests/CookieStorageTests.swift +++ b/DuckDuckGoTests/CookieStorageTests.swift @@ -23,7 +23,7 @@ import WebKit public class CookieStorageTests: XCTestCase { - var storage: CookieStorage! + var storage: MigratableCookieStorage! // This is updated by the `make` function which preserves any cookies added as part of this test let fireproofing = UserDefaultsFireproofing.shared @@ -34,7 +34,7 @@ public class CookieStorageTests: XCTestCase { super.setUp() let defaults = UserDefaults(suiteName: Self.userDefaultsSuiteName)! defaults.removePersistentDomain(forName: Self.userDefaultsSuiteName) - storage = CookieStorage(userDefaults: defaults) + storage = MigratableCookieStorage(userDefaults: defaults) storage.isConsumed = true fireproofing.clearAll() } @@ -158,7 +158,7 @@ public class CookieStorageTests: XCTestCase { ], preservingFireproofedDomains: fireproofing) storage.isConsumed = true - let otherStorage = CookieStorage(userDefaults: UserDefaults(suiteName: Self.userDefaultsSuiteName)!) + let otherStorage = MigratableCookieStorage(userDefaults: UserDefaults(suiteName: Self.userDefaultsSuiteName)!) XCTAssertEqual(1, otherStorage.cookies.count) XCTAssertTrue(otherStorage.isConsumed) } diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index 5ab2d225c3..c786184548 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -61,7 +61,7 @@ final class FireButtonReferenceTests: XCTestCase { $0.exceptPlatforms.contains("ios-browser") == false } - let cookieStorage = CookieStorage() + let cookieStorage = MigratableCookieStorage() for test in referenceTests { let cookie = try XCTUnwrap(cookie(for: test)) @@ -105,7 +105,7 @@ final class FireButtonReferenceTests: XCTestCase { $0.exceptPlatforms.contains("ios-browser") == false } - let cookieStorage = CookieStorage() + let cookieStorage = MigratableCookieStorage() cookieStorage.isConsumed = true for test in referenceTests { let cookie = try XCTUnwrap(cookie(for: test)) diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index 196762db89..0628268239 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -26,8 +26,8 @@ class WebCacheManagerTests: XCTestCase { override func setUp() { super.setUp() - CookieStorage().cookies = [] - CookieStorage().isConsumed = true + MigratableCookieStorage().cookies = [] + MigratableCookieStorage().isConsumed = true if #available(iOS 17, *) { WKWebsiteDataStore.fetchAllDataStoreIdentifiers { uuids in @@ -43,7 +43,7 @@ class WebCacheManagerTests: XCTestCase { func testEnsureIdAllocatedAfterClearing() async throws { let fireproofing = MockFireproofing(domains: []) - let storage = CookieStorage() + let storage = MigratableCookieStorage() let inMemoryDataStoreIdManager = DataStoreIdManager(store: MockKeyValueStore()) XCTAssertNil(inMemoryDataStoreIdManager.currentId) @@ -80,7 +80,7 @@ class WebCacheManagerTests: XCTestCase { let loadedCount = await defaultStore.httpCookieStore.allCookies().count XCTAssertEqual(5, loadedCount) - let cookieStore = CookieStorage() + let cookieStore = MigratableCookieStorage() await WebCacheManager.shared.clear(cookieStorage: cookieStore, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) let cookies = await defaultStore.httpCookieStore.allCookies() @@ -124,7 +124,7 @@ class WebCacheManagerTests: XCTestCase { await defaultStore.httpCookieStore.setCookie(.make(domain: "example.com")) await defaultStore.httpCookieStore.setCookie(.make(domain: ".example.com")) - let cookieStorage = CookieStorage() + let cookieStorage = MigratableCookieStorage() await WebCacheManager.shared.clear(cookieStorage: cookieStorage, fireproofing: fireproofing, @@ -153,7 +153,7 @@ class WebCacheManagerTests: XCTestCase { await cookieStore.setCookie(.make(name: "name", value: "value", domain: "duckduckgo.com")) await cookieStore.setCookie(.make(name: "name", value: "value", domain: "subdomain.duckduckgo.com")) - let storage = CookieStorage() + let storage = MigratableCookieStorage() storage.isConsumed = true await WebCacheManager.shared.clear(cookieStorage: storage, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) @@ -179,7 +179,7 @@ class WebCacheManagerTests: XCTestCase { let loadedCount = await defaultStore.httpCookieStore.allCookies().count XCTAssertEqual(2, loadedCount) - let cookieStore = CookieStorage() + let cookieStore = MigratableCookieStorage() await WebCacheManager.shared.clear(cookieStorage: cookieStore, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) From fcbdf7292fd372f7caea7e542d154e1c6613d0e7 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Fri, 22 Nov 2024 16:22:23 +0000 Subject: [PATCH 02/14] migration logic and refactoring --- Core/Fireproofing.swift | 4 +- Core/WebCacheManager.swift | 35 +++++++++----- DuckDuckGo/AppDelegate.swift | 15 +++++- DuckDuckGo/ContentBlockingUpdating.swift | 6 ++- DuckDuckGo/CookieDebugViewController.swift | 2 +- DuckDuckGo/Favicons.swift | 2 +- .../FireproofingSettingsViewController.swift | 10 ++-- .../ImageCacheDebugViewController.swift | 2 +- DuckDuckGo/MainViewController+Segues.swift | 7 ++- DuckDuckGo/MainViewController.swift | 13 +++-- ...otDebugViewController+VanillaBrowser.swift | 2 +- DuckDuckGo/RootDebugViewController.swift | 48 ++++++++----------- DuckDuckGo/ScriptSourceProviding.swift | 2 +- DuckDuckGo/SettingsLegacyViewProvider.swift | 35 ++++++++------ .../HeadlessWebView.swift | 3 +- .../PrivacyProDataReporting.swift | 2 +- DuckDuckGo/TabManager.swift | 16 +++++-- DuckDuckGo/TabViewController.swift | 15 ++++-- ...ViewControllerLongPressMenuExtension.swift | 4 +- 19 files changed, 140 insertions(+), 83 deletions(-) diff --git a/Core/Fireproofing.swift b/Core/Fireproofing.swift index 166c04c562..745787900d 100644 --- a/Core/Fireproofing.swift +++ b/Core/Fireproofing.swift @@ -35,7 +35,9 @@ public protocol Fireproofing { // This class is not final because we override allowed domains in WebCacheManagerTests public class UserDefaultsFireproofing: Fireproofing { - public static let shared: Fireproofing = UserDefaultsFireproofing() + /// This is only here because there are some places that don't support injection at this time. DO NOT USE IT. + /// If you find you really need to use it, ping Apple Devs channel first. + public static let xshared: Fireproofing = UserDefaultsFireproofing() public struct Notifications { public static let loginDetectionStateChanged = Foundation.Notification.Name("com.duckduckgo.ios.PreserveLogins.loginDetectionStateChanged") diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 1abead7ddd..7e66ef0ed8 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -42,17 +42,31 @@ extension HTTPCookie { } @MainActor -public class WebCacheManager { +public protocol WebsiteDataManaging { - public static var shared = WebCacheManager() + func removeCookies(forDomains domains: [String], fromDataStore: WKWebsiteDataStore) async + func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async + func clear(dataStore: WKWebsiteDataStore) async - private init() { } +} + +@MainActor +public class WebCacheManager: WebsiteDataManaging { + + let cookieStorage: MigratableCookieStorage + let fireproofing: Fireproofing + let dataStoreIdManager: DataStoreIdManaging + + public init(cookieStorage: MigratableCookieStorage, fireproofing: Fireproofing, dataStoreIdManager: DataStoreIdManaging) { + self.cookieStorage = cookieStorage + self.fireproofing = fireproofing + self.dataStoreIdManager = dataStoreIdManager + } /// We save cookies from the current container rather than copying them to a new container because /// the container only persists cookies to disk when the web view is used. If the user presses the fire button /// twice then the fire proofed cookies will be lost and the user will be logged out any sites they're logged in to. - public func consumeCookies(cookieStorage: MigratableCookieStorage = MigratableCookieStorage(), - httpCookieStore: WKHTTPCookieStore) async { + public func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async { guard !cookieStorage.isConsumed else { return } let cookies = cookieStorage.cookies @@ -65,7 +79,7 @@ public class WebCacheManager { } public func removeCookies(forDomains domains: [String], - dataStore: WKWebsiteDataStore) async { + fromDataStore dataStore: WKWebsiteDataStore) async { let startTime = CACurrentMediaTime() let cookieStore = dataStore.httpCookieStore let cookies = await cookieStore.allCookies() @@ -76,13 +90,10 @@ public class WebCacheManager { Pixel.fire(pixel: .cookieDeletionTime(.init(number: totalTime))) } - public func clear(fromDataStore dataStore: WKWebsiteDataStore, - cookieStorage: MigratableCookieStorage = MigratableCookieStorage(), - fireproofing: Fireproofing = UserDefaultsFireproofing.shared, - dataStoreIdManager: DataStoreIdManaging = DataStoreIdManager.shared) async { + public func clear(dataStore: WKWebsiteDataStore) async { await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, destinationStore: dataStore) - await clearData(fromDataStore: dataStore, withFireproofing: fireproofing) + await clearData(inDataStore: dataStore, withFireproofing: fireproofing) removeContainersIfNeeded() } @@ -156,7 +167,7 @@ extension WebCacheManager { } } - private func clearData(fromDataStore dataStore: WKWebsiteDataStore, withFireproofing fireproofing: Fireproofing) async { + private func clearData(inDataStore dataStore: WKWebsiteDataStore, withFireproofing fireproofing: Fireproofing) async { let startTime = CACurrentMediaTime() // Start with all types diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 8020b7ad4b..05bc844c0b 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -285,7 +285,8 @@ import os.log syncService.initializeIfNeeded() self.syncService = syncService - privacyProDataReporter = PrivacyProDataReporter() + let fireproofing = UserDefaultsFireproofing.xshared + privacyProDataReporter = PrivacyProDataReporter(fireproofing: fireproofing) isSyncInProgressCancellable = syncService.isSyncInProgressPublisher .filter { $0 } @@ -357,8 +358,11 @@ import os.log subscriptionFeatureAvailability: subscriptionFeatureAvailability, voiceSearchHelper: voiceSearchHelper, featureFlagger: AppDependencyProvider.shared.featureFlagger, + fireproofing: fireproofing, subscriptionCookieManager: subscriptionCookieManager, - textZoomCoordinator: makeTextZoomCoordinator()) + textZoomCoordinator: makeTextZoomCoordinator(), + websiteDataManager: makeWebsiteDataManager(fireproofing: fireproofing) + ) main.loadViewIfNeeded() syncErrorHandler.alertPresenter = main @@ -411,6 +415,13 @@ import os.log return true } + private func makeWebsiteDataManager(fireproofing: Fireproofing, + dataStoreIdManager: DataStoreIdManaging = DataStoreIdManager.shared) -> WebsiteDataManaging { + return WebCacheManager(cookieStorage: MigratableCookieStorage(), + fireproofing: fireproofing, + dataStoreIdManager: dataStoreIdManager) + } + private func makeTextZoomCoordinator() -> TextZoomCoordinator { let provider = AppDependencyProvider.shared let storage = TextZoomStorage() diff --git a/DuckDuckGo/ContentBlockingUpdating.swift b/DuckDuckGo/ContentBlockingUpdating.swift index dc3b8a720d..4147bbef44 100644 --- a/DuckDuckGo/ContentBlockingUpdating.swift +++ b/DuckDuckGo/ContentBlockingUpdating.swift @@ -57,12 +57,14 @@ public final class ContentBlockingUpdating { init(appSettings: AppSettings = AppUserDefaults(), contentBlockerRulesManager: ContentBlockerRulesManagerProtocol = ContentBlocking.shared.contentBlockingManager, - privacyConfigurationManager: PrivacyConfigurationManaging = ContentBlocking.shared.privacyConfigurationManager) { + privacyConfigurationManager: PrivacyConfigurationManaging = ContentBlocking.shared.privacyConfigurationManager, + fireproofing: Fireproofing = UserDefaultsFireproofing.xshared) { let makeValue: (Update) -> NewContent = { rulesUpdate in let sourceProvider = DefaultScriptSourceProvider(appSettings: appSettings, privacyConfigurationManager: privacyConfigurationManager, - contentBlockingManager: contentBlockerRulesManager) + contentBlockingManager: contentBlockerRulesManager, + fireproofing: fireproofing) return NewContent(rulesUpdate: rulesUpdate, sourceProvider: sourceProvider) } diff --git a/DuckDuckGo/CookieDebugViewController.swift b/DuckDuckGo/CookieDebugViewController.swift index 4efc382742..577bc50192 100644 --- a/DuckDuckGo/CookieDebugViewController.swift +++ b/DuckDuckGo/CookieDebugViewController.swift @@ -41,7 +41,7 @@ class CookieDebugViewController: UITableViewController { var loaded = false let fireproofing: Fireproofing - init(fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + init(fireproofing: Fireproofing) { self.fireproofing = fireproofing super.init() } diff --git a/DuckDuckGo/Favicons.swift b/DuckDuckGo/Favicons.swift index 719b1fa2cd..fafe8ef3a6 100644 --- a/DuckDuckGo/Favicons.swift +++ b/DuckDuckGo/Favicons.swift @@ -57,7 +57,7 @@ public class Favicons { init(sourcesProvider: FaviconSourcesProvider = DefaultFaviconSourcesProvider(), downloader: NotFoundCachingDownloader = NotFoundCachingDownloader(), - fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + fireproofing: Fireproofing = UserDefaultsFireproofing.xshared) { self.sourcesProvider = sourcesProvider self.downloader = downloader self.fireproofing = fireproofing diff --git a/DuckDuckGo/FireproofingSettingsViewController.swift b/DuckDuckGo/FireproofingSettingsViewController.swift index 08bc88b0c7..0fe2175144 100644 --- a/DuckDuckGo/FireproofingSettingsViewController.swift +++ b/DuckDuckGo/FireproofingSettingsViewController.swift @@ -37,9 +37,13 @@ class FireproofingSettingsViewController: UITableViewController { private var shouldShowRemoveAll = false private let fireproofing: Fireproofing + private let websiteDataManager: WebsiteDataManaging - init?(coder: NSCoder, fireproofing: Fireproofing) { + init?(coder: NSCoder, + fireproofing: Fireproofing, + websiteDataManager: WebsiteDataManaging) { self.fireproofing = fireproofing + self.websiteDataManager = websiteDataManager super.init(coder: coder) } @@ -166,7 +170,7 @@ class FireproofingSettingsViewController: UITableViewController { } Task { @MainActor in - await WebCacheManager.shared.removeCookies(forDomains: [domain], dataStore: WKWebsiteDataStore.current()) + await websiteDataManager.removeCookies(forDomains: [domain], fromDataStore: WKWebsiteDataStore.current()) } } @@ -225,7 +229,7 @@ class FireproofingSettingsViewController: UITableViewController { self?.refreshModel() }, confirmed: { [weak self] in Task { @MainActor in - await WebCacheManager.shared.removeCookies(forDomains: self?.model ?? [], dataStore: WKWebsiteDataStore.current()) + await self?.websiteDataManager.removeCookies(forDomains: self?.model ?? [], fromDataStore: WKWebsiteDataStore.current()) self?.fireproofing.clearAll() self?.refreshModel() self?.endEditing() diff --git a/DuckDuckGo/ImageCacheDebugViewController.swift b/DuckDuckGo/ImageCacheDebugViewController.swift index 37c6a838fe..f7be2275bd 100644 --- a/DuckDuckGo/ImageCacheDebugViewController.swift +++ b/DuckDuckGo/ImageCacheDebugViewController.swift @@ -61,7 +61,7 @@ class ImageCacheDebugViewController: UITableViewController { init?(coder: NSCoder, bookmarksDatabase: CoreDataDatabase, - fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + fireproofing: Fireproofing) { bookmarksContext = bookmarksDatabase.makeContext(concurrencyType: .mainQueueConcurrencyType) self.fireproofing = fireproofing diff --git a/DuckDuckGo/MainViewController+Segues.swift b/DuckDuckGo/MainViewController+Segues.swift index 3ab8722e7f..5eafe2cdea 100644 --- a/DuckDuckGo/MainViewController+Segues.swift +++ b/DuckDuckGo/MainViewController+Segues.swift @@ -292,7 +292,9 @@ extension MainViewController { appSettings: appSettings, bookmarksDatabase: bookmarksDatabase, tabManager: tabManager, - syncPausedStateManager: syncPausedStateManager) + syncPausedStateManager: syncPausedStateManager, + fireproofing: fireproofing, + websiteDataManager: websiteDataManager) let settingsViewModel = SettingsViewModel(legacyViewProvider: legacyViewProvider, subscriptionManager: AppDependencyProvider.shared.subscriptionManager, @@ -331,7 +333,8 @@ extension MainViewController { sync: self.syncService, bookmarksDatabase: self.bookmarksDatabase, internalUserDecider: AppDependencyProvider.shared.internalUserDecider, - tabManager: self.tabManager) + tabManager: self.tabManager, + fireproofing: self.fireproofing) } let controller = UINavigationController(rootViewController: settings) diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index a952094b60..b961dc0d78 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -178,6 +178,7 @@ class MainViewController: UIViewController { } let fireproofing: Fireproofing + let websiteDataManager: WebsiteDataManaging let textZoomCoordinator: TextZoomCoordinating var historyManager: HistoryManaging @@ -204,9 +205,10 @@ class MainViewController: UIViewController { subscriptionFeatureAvailability: SubscriptionFeatureAvailability, voiceSearchHelper: VoiceSearchHelperProtocol, featureFlagger: FeatureFlagger, - fireproofing: Fireproofing = UserDefaultsFireproofing.shared, + fireproofing: Fireproofing, subscriptionCookieManager: SubscriptionCookieManaging, - textZoomCoordinator: TextZoomCoordinating + textZoomCoordinator: TextZoomCoordinating, + websiteDataManager: WebsiteDataManaging ) { self.bookmarksDatabase = bookmarksDatabase self.bookmarksDatabaseCleaner = bookmarksDatabaseCleaner @@ -232,7 +234,9 @@ class MainViewController: UIViewController { featureFlagger: featureFlagger, subscriptionCookieManager: subscriptionCookieManager, appSettings: appSettings, - textZoomCoordinator: textZoomCoordinator) + textZoomCoordinator: textZoomCoordinator, + websiteDataManager: websiteDataManager, + fireproofing: fireproofing) self.syncPausedStateManager = syncPausedStateManager self.privacyProDataReporter = privacyProDataReporter self.homeTabManager = NewTabPageManager() @@ -246,6 +250,7 @@ class MainViewController: UIViewController { self.fireproofing = fireproofing self.subscriptionCookieManager = subscriptionCookieManager self.textZoomCoordinator = textZoomCoordinator + self.websiteDataManager = websiteDataManager super.init(nibName: nil, bundle: nil) @@ -2660,7 +2665,7 @@ extension MainViewController: AutoClearWorker { URLSession.shared.configuration.urlCache?.removeAllCachedResponses() let pixel = TimedPixel(.forgetAllDataCleared) - await WebCacheManager.shared.clear(fromDataStore: WKWebsiteDataStore.default()) + await websiteDataManager.clear(dataStore: WKWebsiteDataStore.default()) pixel.fire(withAdditionalParameters: [PixelParameters.tabCount: "\(self.tabManager.count)"]) AutoconsentManagement.shared.clearCache() diff --git a/DuckDuckGo/RootDebugViewController+VanillaBrowser.swift b/DuckDuckGo/RootDebugViewController+VanillaBrowser.swift index c2900f153e..1de4437194 100644 --- a/DuckDuckGo/RootDebugViewController+VanillaBrowser.swift +++ b/DuckDuckGo/RootDebugViewController+VanillaBrowser.swift @@ -29,7 +29,7 @@ extension RootDebugViewController { fileprivate static let ddgURL = URL(string: "https://duckduckgo.com/")! @objc func openVanillaBrowser(_ sender: Any?) { - let homeURL = tabManager?.current()?.tabModel.link?.url ?? RootDebugViewController.ddgURL + let homeURL = tabManager.current()?.tabModel.link?.url ?? RootDebugViewController.ddgURL openVanillaBrowser(url: homeURL) } diff --git a/DuckDuckGo/RootDebugViewController.swift b/DuckDuckGo/RootDebugViewController.swift index 58258a5a07..9c833739de 100644 --- a/DuckDuckGo/RootDebugViewController.swift +++ b/DuckDuckGo/RootDebugViewController.swift @@ -55,14 +55,15 @@ class RootDebugViewController: UITableViewController { weak var reportGatheringActivity: UIView? @IBAction func onShareTapped() { - presentShareSheet(withItems: [DiagnosticReportDataSource(delegate: self)], fromButtonItem: shareButton) + presentShareSheet(withItems: [DiagnosticReportDataSource(delegate: self, fireproofing: fireproofing)], fromButtonItem: shareButton) } - private var bookmarksDatabase: CoreDataDatabase? - private var sync: DDGSyncing? - private var internalUserDecider: DefaultInternalUserDecider? - var tabManager: TabManager? - private var tipKitUIActionHandler: TipKitDebugOptionsUIActionHandling? + private let bookmarksDatabase: CoreDataDatabase + private let sync: DDGSyncing + private let internalUserDecider: InternalUserDecider + let tabManager: TabManager + private let tipKitUIActionHandler: TipKitDebugOptionsUIActionHandling + private let fireproofing: Fireproofing @UserDefaultsWrapper(key: .lastConfigurationRefreshDate, defaultValue: .distantPast) private var lastConfigurationRefreshDate: Date @@ -72,33 +73,27 @@ class RootDebugViewController: UITableViewController { bookmarksDatabase: CoreDataDatabase, internalUserDecider: InternalUserDecider, tabManager: TabManager, - tipKitUIActionHandler: TipKitDebugOptionsUIActionHandling = TipKitDebugOptionsUIActionHandler()) { + tipKitUIActionHandler: TipKitDebugOptionsUIActionHandling = TipKitDebugOptionsUIActionHandler(), + fireproofing: Fireproofing) { self.sync = sync self.bookmarksDatabase = bookmarksDatabase - self.internalUserDecider = internalUserDecider as? DefaultInternalUserDecider + self.internalUserDecider = internalUserDecider self.tabManager = tabManager self.tipKitUIActionHandler = tipKitUIActionHandler + self.fireproofing = fireproofing super.init(coder: coder) } required init?(coder: NSCoder) { - super.init(coder: coder) - } - - func configure(sync: DDGSyncing, bookmarksDatabase: CoreDataDatabase, internalUserDecider: InternalUserDecider, tabManager: TabManager, tipKitUIActionHandler: TipKitDebugOptionsUIActionHandling = TipKitDebugOptionsUIActionHandler()) { - - self.sync = sync - self.bookmarksDatabase = bookmarksDatabase - self.internalUserDecider = internalUserDecider as? DefaultInternalUserDecider - self.tabManager = tabManager - self.tipKitUIActionHandler = tipKitUIActionHandler + fatalError("init not implemented") } @IBSegueAction func onCreateImageCacheDebugScreen(_ coder: NSCoder) -> ImageCacheDebugViewController? { guard let controller = ImageCacheDebugViewController(coder: coder, - bookmarksDatabase: self.bookmarksDatabase!) else { + bookmarksDatabase: self.bookmarksDatabase, + fireproofing: fireproofing) else { fatalError("Failed to create controller") } @@ -107,8 +102,8 @@ class RootDebugViewController: UITableViewController { @IBSegueAction func onCreateSyncDebugScreen(_ coder: NSCoder, sender: Any?, segueIdentifier: String?) -> SyncDebugViewController { guard let controller = SyncDebugViewController(coder: coder, - sync: self.sync!, - bookmarksDatabase: self.bookmarksDatabase!) else { + sync: self.sync, + bookmarksDatabase: self.bookmarksDatabase) else { fatalError("Failed to create controller") } @@ -127,7 +122,7 @@ class RootDebugViewController: UITableViewController { if cell.tag == Row.toggleInspectableWebViews.rawValue { cell.accessoryType = AppUserDefaults().inspectableWebViewEnabled ? .checkmark : .none } else if cell.tag == Row.toggleInternalUserState.rawValue { - cell.accessoryType = (internalUserDecider?.isInternalUser ?? false) ? .checkmark : .none + cell.accessoryType = (internalUserDecider.isInternalUser) ? .checkmark : .none } } @@ -163,8 +158,8 @@ class RootDebugViewController: UITableViewController { cell.accessoryType = defaults.inspectableWebViewEnabled ? .checkmark : .none NotificationCenter.default.post(Notification(name: AppUserDefaults.Notifications.inspectableWebViewsToggled)) case .toggleInternalUserState: - let newState = !(internalUserDecider?.isInternalUser ?? false) - internalUserDecider?.debugSetInternalUserState(newState) + let newState = !internalUserDecider.isInternalUser + (internalUserDecider as? DefaultInternalUserDecider)?.debugSetInternalUserState(newState) cell.accessoryType = newState ? .checkmark : .none NotificationCenter.default.post(Notification(name: AppUserDefaults.Notifications.inspectableWebViewsToggled)) case .openVanillaBrowser: @@ -184,12 +179,11 @@ class RootDebugViewController: UITableViewController { let controller = UIHostingController(rootView: OnboardingDebugView(onNewOnboardingIntroStartAction: action)) show(controller, sender: nil) case .resetSyncPromoPrompts: - guard let sync = sync else { return } let syncPromoPresenter = SyncPromoManager(syncService: sync) syncPromoPresenter.resetPromos() ActionMessageView.present(message: "Sync Promos reset") case .resetTipKit: - tipKitUIActionHandler?.resetTipKitTapped() + tipKitUIActionHandler.resetTipKitTapped() } } } @@ -255,7 +249,7 @@ class DiagnosticReportDataSource: UIActivityItemProvider { @UserDefaultsWrapper(key: .lastConfigurationRefreshDate, defaultValue: .distantPast) private var lastRefreshDate: Date - convenience init(delegate: DiagnosticReportDataSourceDelegate, fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + convenience init(delegate: DiagnosticReportDataSourceDelegate, fireproofing: Fireproofing) { self.init(placeholderItem: "") self.delegate = delegate self.fireproofing = fireproofing diff --git a/DuckDuckGo/ScriptSourceProviding.swift b/DuckDuckGo/ScriptSourceProviding.swift index 61a47e794f..549b538b45 100644 --- a/DuckDuckGo/ScriptSourceProviding.swift +++ b/DuckDuckGo/ScriptSourceProviding.swift @@ -55,7 +55,7 @@ struct DefaultScriptSourceProvider: ScriptSourceProviding { init(appSettings: AppSettings = AppDependencyProvider.shared.appSettings, privacyConfigurationManager: PrivacyConfigurationManaging = ContentBlocking.shared.privacyConfigurationManager, contentBlockingManager: ContentBlockerRulesManagerProtocol = ContentBlocking.shared.contentBlockingManager, - fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + fireproofing: Fireproofing) { sendDoNotSell = appSettings.sendDoNotSell diff --git a/DuckDuckGo/SettingsLegacyViewProvider.swift b/DuckDuckGo/SettingsLegacyViewProvider.swift index 3b74346f78..2ce76ef5d6 100644 --- a/DuckDuckGo/SettingsLegacyViewProvider.swift +++ b/DuckDuckGo/SettingsLegacyViewProvider.swift @@ -41,6 +41,7 @@ class SettingsLegacyViewProvider: ObservableObject { let tabManager: TabManager let syncPausedStateManager: any SyncPausedStateManaging let fireproofing: Fireproofing + let websiteDataManager: WebsiteDataManaging init(syncService: any DDGSyncing, syncDataProviders: SyncDataProviders, @@ -48,7 +49,8 @@ class SettingsLegacyViewProvider: ObservableObject { bookmarksDatabase: CoreDataDatabase, tabManager: TabManager, syncPausedStateManager: any SyncPausedStateManaging, - fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + fireproofing: Fireproofing, + websiteDataManager: WebsiteDataManaging) { self.syncService = syncService self.syncDataProviders = syncDataProviders self.appSettings = appSettings @@ -56,6 +58,7 @@ class SettingsLegacyViewProvider: ObservableObject { self.tabManager = tabManager self.syncPausedStateManager = syncPausedStateManager self.fireproofing = fireproofing + self.websiteDataManager = websiteDataManager } enum LegacyView { @@ -81,7 +84,7 @@ class SettingsLegacyViewProvider: ObservableObject { private func instantiateFireproofingController() -> UIViewController { let storyboard = UIStoryboard(name: StoryboardName.settings, bundle: nil) return storyboard.instantiateViewController(identifier: "FireProofSites") { coder in - return FireproofingSettingsViewController(coder: coder, fireproofing: self.fireproofing) + return FireproofingSettingsViewController(coder: coder, fireproofing: self.fireproofing, websiteDataManager: self.websiteDataManager) } } @@ -92,6 +95,18 @@ class SettingsLegacyViewProvider: ObservableObject { }) } + private func instantiateDebugController() -> UIViewController { + let storyboard = UIStoryboard(name: "Debug", bundle: nil) + return storyboard.instantiateViewController(identifier: "DebugMenu") { coder in + RootDebugViewController(coder: coder, + sync: self.syncService, + bookmarksDatabase: self.bookmarksDatabase, + internalUserDecider: AppDependencyProvider.shared.internalUserDecider, + tabManager: self.tabManager, + fireproofing: self.fireproofing) + } + } + // Legacy UIKit Views (Pushed unmodified) var addToDock: UIViewController { instantiate( "instructions", fromStoryboard: StoryboardName.homeRow) } var appIcon: UIViewController { instantiate("AppIcon", fromStoryboard: StoryboardName.settings) } @@ -102,6 +117,8 @@ class SettingsLegacyViewProvider: ObservableObject { var keyboard: UIViewController { instantiate("Keyboard", fromStoryboard: StoryboardName.settings) } var feedback: UIViewController { instantiate("Feedback", fromStoryboard: StoryboardName.feedback) } var autoclearData: UIViewController { instantiateAutoClearController() } + var debug: UIViewController { instantiateDebugController() } + @MainActor func syncSettings(source: String? = nil) -> SyncSettingsViewController { @@ -121,17 +138,5 @@ class SettingsLegacyViewProvider: ObservableObject { selectedAccount: selectedAccount, source: .settings) } - - var debug: UIViewController { - let storyboard = UIStoryboard(name: "Debug", bundle: nil) - if let viewController = storyboard.instantiateViewController(withIdentifier: "DebugMenu") as? RootDebugViewController { - viewController.configure(sync: syncService, - bookmarksDatabase: bookmarksDatabase, - internalUserDecider: AppDependencyProvider.shared.internalUserDecider, - tabManager: tabManager) - return viewController - } - return UIViewController() - } - + } diff --git a/DuckDuckGo/Subscription/AsyncHeadlessWebview/HeadlessWebView.swift b/DuckDuckGo/Subscription/AsyncHeadlessWebview/HeadlessWebView.swift index 5af147b4a5..f7246b44f0 100644 --- a/DuckDuckGo/Subscription/AsyncHeadlessWebview/HeadlessWebView.swift +++ b/DuckDuckGo/Subscription/AsyncHeadlessWebview/HeadlessWebView.swift @@ -22,6 +22,7 @@ import SwiftUI import WebKit import UserScript import BrowserServicesKit +import Core struct HeadlessWebView: UIViewRepresentable { weak var userScript: UserScriptMessaging? @@ -85,7 +86,7 @@ struct HeadlessWebView: UIViewRepresentable { // Enable content blocking rules if settings.contentBlocking { - let sourceProvider = DefaultScriptSourceProvider() + let sourceProvider = DefaultScriptSourceProvider(fireproofing: UserDefaultsFireproofing.xshared) let contentBlockerUserScript = ContentBlockerRulesUserScript(configuration: sourceProvider.contentBlockerRulesConfig) let contentScopeUserScript = ContentScopeUserScript(sourceProvider.privacyConfigurationManager, properties: sourceProvider.contentScopeProperties) diff --git a/DuckDuckGo/Subscription/PrivacyProDataReporting.swift b/DuckDuckGo/Subscription/PrivacyProDataReporting.swift index 43c2150504..86a65a29ed 100644 --- a/DuckDuckGo/Subscription/PrivacyProDataReporting.swift +++ b/DuckDuckGo/Subscription/PrivacyProDataReporting.swift @@ -127,7 +127,7 @@ final class PrivacyProDataReporter: PrivacyProDataReporting { secureVaultMaker: @escaping () -> (any AutofillSecureVault)? = { try? AutofillSecureVaultFactory.makeVault(reporter: SecureVaultReporter()) }, syncService: DDGSyncing? = nil, tabsModel: TabsModel? = nil, - fireproofing: Fireproofing = UserDefaultsFireproofing.shared, + fireproofing: Fireproofing, dateGenerator: @escaping () -> Date = Date.init) { self.configurationManager = configurationManager self.variantManager = variantManager diff --git a/DuckDuckGo/TabManager.swift b/DuckDuckGo/TabManager.swift index 2690c7b518..998ff5c25a 100644 --- a/DuckDuckGo/TabManager.swift +++ b/DuckDuckGo/TabManager.swift @@ -44,6 +44,8 @@ class TabManager { private let onboardingPixelReporter: OnboardingPixelReporting private let featureFlagger: FeatureFlagger private let textZoomCoordinator: TextZoomCoordinating + private let fireproofing: Fireproofing + private let websiteDataManager: WebsiteDataManaging private let subscriptionCookieManager: SubscriptionCookieManaging private let appSettings: AppSettings @@ -66,7 +68,9 @@ class TabManager { featureFlagger: FeatureFlagger, subscriptionCookieManager: SubscriptionCookieManaging, appSettings: AppSettings, - textZoomCoordinator: TextZoomCoordinating) { + textZoomCoordinator: TextZoomCoordinating, + websiteDataManager: WebsiteDataManaging, + fireproofing: Fireproofing) { self.model = model self.previewsSource = previewsSource self.bookmarksDatabase = bookmarksDatabase @@ -81,6 +85,8 @@ class TabManager { self.subscriptionCookieManager = subscriptionCookieManager self.appSettings = appSettings self.textZoomCoordinator = textZoomCoordinator + self.websiteDataManager = websiteDataManager + self.fireproofing = fireproofing registerForNotifications() } @@ -105,7 +111,9 @@ class TabManager { onboardingPixelReporter: onboardingPixelReporter, featureFlagger: featureFlagger, subscriptionCookieManager: subscriptionCookieManager, - textZoomCoordinator: textZoomCoordinator) + textZoomCoordinator: textZoomCoordinator, + websiteDataManager: websiteDataManager, + fireproofing: fireproofing) controller.applyInheritedAttribution(inheritedAttribution) controller.attachWebView(configuration: configuration, andLoadRequest: url == nil ? nil : URLRequest.userInitiated(url!), @@ -185,7 +193,9 @@ class TabManager { onboardingPixelReporter: onboardingPixelReporter, featureFlagger: featureFlagger, subscriptionCookieManager: subscriptionCookieManager, - textZoomCoordinator: textZoomCoordinator) + textZoomCoordinator: textZoomCoordinator, + websiteDataManager: websiteDataManager, + fireproofing: fireproofing) controller.attachWebView(configuration: configCopy, andLoadRequest: request, consumeCookies: !model.hasActiveTabs, diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index a5bbd41846..e96f54f302 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -332,7 +332,9 @@ class TabViewController: UIViewController { urlCredentialCreator: URLCredentialCreating = URLCredentialCreator(), featureFlagger: FeatureFlagger, subscriptionCookieManager: SubscriptionCookieManaging, - textZoomCoordinator: TextZoomCoordinating) -> TabViewController { + textZoomCoordinator: TextZoomCoordinating, + websiteDataManager: WebsiteDataManaging, + fireproofing: Fireproofing) -> TabViewController { let storyboard = UIStoryboard(name: "Tab", bundle: nil) let controller = storyboard.instantiateViewController(identifier: "TabViewController", creator: { coder in @@ -350,7 +352,9 @@ class TabViewController: UIViewController { urlCredentialCreator: urlCredentialCreator, featureFlagger: featureFlagger, subscriptionCookieManager: subscriptionCookieManager, - textZoomCoordinator: textZoomCoordinator + textZoomCoordinator: textZoomCoordinator, + fireproofing: fireproofing, + websiteDataManager: websiteDataManager ) }) return controller @@ -371,6 +375,7 @@ class TabViewController: UIViewController { let onboardingPixelReporter: OnboardingCustomInteractionPixelReporting let textZoomCoordinator: TextZoomCoordinating let fireproofing: Fireproofing + let websiteDataManager: WebsiteDataManaging required init?(coder aDecoder: NSCoder, tabModel: Tab, @@ -388,7 +393,8 @@ class TabViewController: UIViewController { featureFlagger: FeatureFlagger, subscriptionCookieManager: SubscriptionCookieManaging, textZoomCoordinator: TextZoomCoordinating, - fireproofing: Fireproofing = UserDefaultsFireproofing.shared) { + fireproofing: Fireproofing, + websiteDataManager: WebsiteDataManaging) { self.tabModel = tabModel self.appSettings = appSettings self.bookmarksDatabase = bookmarksDatabase @@ -410,6 +416,7 @@ class TabViewController: UIViewController { self.subscriptionCookieManager = subscriptionCookieManager self.textZoomCoordinator = textZoomCoordinator self.fireproofing = fireproofing + self.websiteDataManager = websiteDataManager super.init(coder: aDecoder) @@ -664,7 +671,7 @@ class TabViewController: UIViewController { Task { @MainActor in await webView.configuration.websiteDataStore.dataRecords(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes()) let cookieStore = webView.configuration.websiteDataStore.httpCookieStore - await WebCacheManager.shared.consumeCookies(httpCookieStore: cookieStore) + await websiteDataManager.consumeCookies(intoHTTPCookieStore: cookieStore) subscriptionCookieManager.resetLastRefreshDate() await subscriptionCookieManager.refreshSubscriptionCookie() doLoad() diff --git a/DuckDuckGo/TabViewControllerLongPressMenuExtension.swift b/DuckDuckGo/TabViewControllerLongPressMenuExtension.swift index 974fbbf895..0227757b55 100644 --- a/DuckDuckGo/TabViewControllerLongPressMenuExtension.swift +++ b/DuckDuckGo/TabViewControllerLongPressMenuExtension.swift @@ -114,7 +114,9 @@ extension TabViewController { onboardingPixelReporter: onboardingPixelReporter, featureFlagger: featureFlagger, subscriptionCookieManager: subscriptionCookieManager, - textZoomCoordinator: textZoomCoordinator) + textZoomCoordinator: textZoomCoordinator, + websiteDataManager: websiteDataManager, + fireproofing: fireproofing) tabController.isLinkPreview = true let configuration = WKWebViewConfiguration.nonPersistent() From f80bf7b7c1a4ded5aba26ff64314ed890b860ee5 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Fri, 22 Nov 2024 19:24:13 +0000 Subject: [PATCH 03/14] update unit tests --- Core/WebCacheManager.swift | 1 - DuckDuckGo.xcodeproj/project.pbxproj | 8 + DuckDuckGoTests/CookieStorageTests.swift | 219 ++++-------------- DuckDuckGoTests/DataStoreIdManagerTests.swift | 31 +-- .../FireButtonReferenceTests.swift | 59 +---- DuckDuckGoTests/MockFireproofing.swift | 37 +++ DuckDuckGoTests/MockTabDelegate.swift | 4 +- DuckDuckGoTests/MockWebsiteDataManager.swift | 35 +++ .../OnboardingDaxFavouritesTests.swift | 6 +- .../OnboardingNavigationDelegateTests.swift | 5 +- .../PrivacyProDataReporterTests.swift | 4 +- DuckDuckGoTests/WebCacheManagerTests.swift | 16 +- 12 files changed, 156 insertions(+), 269 deletions(-) create mode 100644 DuckDuckGoTests/MockFireproofing.swift create mode 100644 DuckDuckGoTests/MockWebsiteDataManager.swift diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 7e66ef0ed8..7390983add 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -41,7 +41,6 @@ extension HTTPCookie { } -@MainActor public protocol WebsiteDataManaging { func removeCookies(forDomains domains: [String], fromDataStore: WKWebsiteDataStore) async diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 052e9699ed..836714553a 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -565,6 +565,8 @@ 85C2971A248162CA0063A335 /* DaxOnboardingPadViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */; }; 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */; }; 85C503FB2CF0ADCE0075DF6F /* WebCacheManager+ObservationsDataClearing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */; }; + 85C503FD2CF0E7B10075DF6F /* MockFireproofing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */; }; + 85C503FF2CF0F4DC0075DF6F /* MockWebsiteDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */; }; 85C8E61D2B0E47380029A6BD /* BookmarksDatabaseSetup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */; }; 85C91CA224671F4C00A11132 /* AppDeepLinkSchemes.swift in Sources */ = {isa = PBXBuildFile; fileRef = F17D723B1E8BB374003E8B0E /* AppDeepLinkSchemes.swift */; }; 85D2187024BF24DB004373D2 /* FaviconRequestModifierTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85D2186F24BF24DB004373D2 /* FaviconRequestModifierTests.swift */; }; @@ -1880,6 +1882,8 @@ 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxOnboardingPadViewController.swift; sourceTree = ""; }; 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIdManager.swift; sourceTree = ""; }; 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WebCacheManager+ObservationsDataClearing.swift"; sourceTree = ""; }; + 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockFireproofing.swift; sourceTree = ""; }; + 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockWebsiteDataManager.swift; sourceTree = ""; }; 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksDatabaseSetup.swift; sourceTree = ""; }; 85CA53A324B9F2BD00A6288C /* Favicons.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Favicons.swift; sourceTree = ""; }; 85CA53A924BB376800A6288C /* NotFoundCachingDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = NotFoundCachingDownloader.swift; path = ../Core/NotFoundCachingDownloader.swift; sourceTree = ""; }; @@ -6080,6 +6084,7 @@ F17669A91E412A17003D3222 /* Mocks */ = { isa = PBXGroup; children = ( + 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */, 8598D2E52CEBAA1B00C45685 /* MockFaviconStore.swift */, 9F4CC51A2C48C0C7006A96EB /* MockTabDelegate.swift */, C14882E927F20DD000D59F0C /* MockBookmarksCoreDataStorage.swift */, @@ -6099,6 +6104,7 @@ 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */, 852409302C78030D00CB28FC /* MockUsageSegmentation.swift */, 4B27FBB72C93F53B007E21A7 /* MockPersistentPixel.swift */, + 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */, ); name = Mocks; sourceTree = ""; @@ -8146,6 +8152,7 @@ 316790E52C9352190090B0A2 /* MarketplaceAdPostbackManagerTests.swift in Sources */, F194FAFB1F14E622009B4DF8 /* UIFontExtensionTests.swift in Sources */, BBFF18B12C76448100C48D7D /* QuerySubmittedTests.swift in Sources */, + 85C503FF2CF0F4DC0075DF6F /* MockWebsiteDataManager.swift in Sources */, 854A8D812C7F4452001D62E5 /* AtbTests.swift in Sources */, 9F23B8092C2BE9B700950875 /* MockURLOpener.swift in Sources */, 9F8007262C5261AF003EDAF4 /* MockPrivacyDataReporter.swift in Sources */, @@ -8173,6 +8180,7 @@ C185ED672BD43DA100BAE9DC /* ImportPasswordsStatusHandlerTests.swift in Sources */, 6FF9AD452CE766F700C5A406 /* NewTabPageControllerPixelTests.swift in Sources */, F198D7981E3A45D90088DA8A /* WKWebViewConfigurationExtensionTests.swift in Sources */, + 85C503FD2CF0E7B10075DF6F /* MockFireproofing.swift in Sources */, 6F3529FF2CDCEDFF00A59170 /* OmniBarLoadingStateBearerTests.swift in Sources */, 564DE45E2C45218500D23241 /* OnboardingNavigationDelegateTests.swift in Sources */, C14E2F7729DE14EA002AC515 /* AutofillInterfaceUsernameTruncatorTests.swift in Sources */, diff --git a/DuckDuckGoTests/CookieStorageTests.swift b/DuckDuckGoTests/CookieStorageTests.swift index 7094a4daeb..5443c591bf 100644 --- a/DuckDuckGoTests/CookieStorageTests.swift +++ b/DuckDuckGoTests/CookieStorageTests.swift @@ -22,186 +22,59 @@ import XCTest import WebKit public class CookieStorageTests: XCTestCase { - - var storage: MigratableCookieStorage! - - // This is updated by the `make` function which preserves any cookies added as part of this test - let fireproofing = UserDefaultsFireproofing.shared - static let userDefaultsSuiteName = "test" - - public override func setUp() { - super.setUp() - let defaults = UserDefaults(suiteName: Self.userDefaultsSuiteName)! - defaults.removePersistentDomain(forName: Self.userDefaultsSuiteName) - storage = MigratableCookieStorage(userDefaults: defaults) - storage.isConsumed = true - fireproofing.clearAll() + func testLoadCookiesFromDefaultsAndRemovalWhenMigrationCompletes() { + let defaults = UserDefaults(suiteName: "test")! + defaults.removeSuite(named: "test") + + addCookies([ + make("example.com", name: "test1", value: "value1"), + make("example.com", name: "test2", value: "value2"), + make("facebook.com", name: "test3", value: "value3"), + ], defaults) + + let storage = MigratableCookieStorage(userDefaults: defaults) + XCTAssertEqual(storage.cookies.count, 3) + + XCTAssertTrue(storage.cookies.contains(where: { + $0.domain == "example.com" && + $0.name == "test1" && + $0.value == "value1" + })) + + XCTAssertTrue(storage.cookies.contains(where: { + $0.domain == "example.com" && + $0.name == "test2" && + $0.value == "value2" + })) + + XCTAssertTrue(storage.cookies.contains(where: { + $0.domain == "facebook.com" && + $0.name == "test3" && + $0.value == "value3" + })) + + // Now remove them all + storage.migrationComplete() + + XCTAssertTrue(storage.cookies.isEmpty) } - - func testWhenDomainRemovesAllCookesThenTheyAreClearedFromPersisted() { - fireproofing.addToAllowed(domain: "example.com") - - XCTAssertEqual(storage.updateCookies([ - make("example.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing), .empty) - XCTAssertEqual(1, storage.cookies.count) - - storage.isConsumed = true - storage.updateCookies([], preservingFireproofedDomains: fireproofing) + func addCookies(_ cookies: [HTTPCookie], _ defaults: UserDefaults) { - XCTAssertEqual(0, storage.cookies.count) + var cookieData = [[String: Any?]]() + cookies.forEach { cookie in + var mappedCookie = [String: Any?]() + cookie.properties?.forEach { + mappedCookie[$0.key.rawValue] = $0.value + } + cookieData.append(mappedCookie) + } + defaults.setValue(cookieData, forKey: MigratableCookieStorage.Keys.allowedCookies) } - func testWhenUpdatedThenDuckDuckGoCookiesAreNotRemoved() { - storage.updateCookies([ - make("duckduckgo.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(1, storage.cookies.count) - - storage.isConsumed = true - storage.updateCookies([ - make("duckduckgo.com", name: "x", value: "1"), - make("test.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(2, storage.cookies.count) - - storage.isConsumed = true - storage.updateCookies([ - make("usedev1.duckduckgo.com", name: "x", value: "1"), - make("duckduckgo.com", name: "x", value: "1"), - make("test.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(3, storage.cookies.count) - - } - - func testWhenUpdatedThenCookiesWithFutureExpirationAreNotRemoved() { - storage.updateCookies([ - make("test.com", name: "x", value: "1", expires: .distantFuture), - make("example.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(2, storage.cookies.count) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" })) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" })) - - } - - func testWhenUpdatingThenExistingExpiredCookiesAreRemoved() { - storage.cookies = [ - make("test.com", name: "x", value: "1", expires: Date(timeIntervalSinceNow: -100)), - ] - XCTAssertEqual(1, storage.cookies.count) - - storage.isConsumed = true - storage.updateCookies([ - make("example.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(1, storage.cookies.count) - XCTAssertFalse(storage.cookies.contains(where: { $0.domain == "test.com" })) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" })) - - } - - func testWhenExpiredCookieIsAddedThenItIsNotPersisted() { - - storage.updateCookies([ - make("example.com", name: "x", value: "1", expires: Date(timeIntervalSinceNow: -100)), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(0, storage.cookies.count) - - } - - func testWhenUpdatedThenNoLongerFireproofedDomainsAreCleared() { - storage.updateCookies([ - make("test.com", name: "x", value: "1"), - make("example.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - fireproofing.remove(domain: "test.com") - - storage.isConsumed = true - storage.updateCookies([ - make("example.com", name: "x", value: "1"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(1, storage.cookies.count) - XCTAssertFalse(storage.cookies.contains(where: { $0.domain == "test.com" })) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "example.com" })) - } - - func testWhenStorageInitialiedThenItIsEmptyAndIsReadyToBeUpdated() { - XCTAssertEqual(0, storage.cookies.count) - XCTAssertTrue(storage.isConsumed) - } - - func testWhenStorageIsUpdatedThenConsumedIsResetToFalse() { - storage.isConsumed = true - XCTAssertTrue(storage.isConsumed) - storage.updateCookies([ - make("test.com", name: "x", value: "1") - ], preservingFireproofedDomains: fireproofing) - XCTAssertFalse(storage.isConsumed) - } - - func testWhenStorageIsReinstanciatedThenUsesStoredData() { - storage.updateCookies([ - make("test.com", name: "x", value: "1") - ], preservingFireproofedDomains: fireproofing) - storage.isConsumed = true - - let otherStorage = MigratableCookieStorage(userDefaults: UserDefaults(suiteName: Self.userDefaultsSuiteName)!) - XCTAssertEqual(1, otherStorage.cookies.count) - XCTAssertTrue(otherStorage.isConsumed) - } - - func testWhenStorageIsUpdatedThenUpdatingAddsNewCookies() { - storage.updateCookies([ - make("test.com", name: "x", value: "1") - ], preservingFireproofedDomains: fireproofing) - XCTAssertEqual(1, storage.cookies.count) - } - - func testWhenStorageHasMatchingDOmainThenUpdatingReplacesCookies() { - storage.updateCookies([ - make("test.com", name: "x", value: "1") - ], preservingFireproofedDomains: fireproofing) - - storage.isConsumed = true - storage.updateCookies([ - make("test.com", name: "x", value: "2"), - make("test.com", name: "y", value: "3"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(2, storage.cookies.count) - XCTAssertFalse(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "x" && $0.value == "1" })) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "x" && $0.value == "2" })) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "y" && $0.value == "3" })) - } - - func testWhenStorageUpdatedAndNotConsumedThenNothingHappens() { - storage.updateCookies([ - make("test.com", name: "x", value: "1") - ], preservingFireproofedDomains: fireproofing) - - storage.updateCookies([ - make("example.com", name: "y", value: "3"), - ], preservingFireproofedDomains: fireproofing) - - XCTAssertEqual(1, storage.cookies.count) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "test.com" && $0.name == "x" && $0.value == "1" })) - } - func make(_ domain: String, name: String, value: String, expires: Date? = nil) -> HTTPCookie { - fireproofing.addToAllowed(domain: domain) return HTTPCookie(properties: [ .domain: domain, .name: name, @@ -210,5 +83,5 @@ public class CookieStorageTests: XCTestCase { .expires: expires as Any ])! } - + } diff --git a/DuckDuckGoTests/DataStoreIdManagerTests.swift b/DuckDuckGoTests/DataStoreIdManagerTests.swift index f4d1e3d2df..d507fb41da 100644 --- a/DuckDuckGoTests/DataStoreIdManagerTests.swift +++ b/DuckDuckGoTests/DataStoreIdManagerTests.swift @@ -26,45 +26,26 @@ import TestUtils class DataStoreIdManagerTests: XCTestCase { - func testWhenFreshlyInstalledThenIdIsAllocated() { + func testWhenFreshlyInstalledThenNoIdIsAllocated() { let manager = DataStoreIdManager(store: MockKeyValueStore()) - XCTAssertNil(manager.currentId) - manager.invalidateCurrentIdAndAllocateNew() - XCTAssertNotNil(manager.currentId) } - func testWhenIdIsInvalidatedThenNewIsCreated() { - + func testWhenIdIsInvalidatedThenNoNewIdIsCreated() { let manager = DataStoreIdManager(store: MockKeyValueStore()) - XCTAssertNil(manager.currentId) - manager.invalidateCurrentIdAndAllocateNew() - - let firstID = manager.currentId - XCTAssertNotNil(firstID) - - manager.invalidateCurrentIdAndAllocateNew() - let secondID = manager.currentId - - XCTAssertNotEqual(firstID, secondID) - - manager.invalidateCurrentIdAndAllocateNew() - let thirdID = manager.currentId - - XCTAssertNotEqual(firstID, thirdID) - XCTAssertNotEqual(secondID, thirdID) + manager.invalidateCurrentId() + XCTAssertNil(manager.currentId) } func testWhenIdAlreadyExistsThenItIsRedFromTheStore() { - let storedUUID = UUID().uuidString let store = MockKeyValueStore() store.set(storedUUID, forKey: DataStoreIdManager.Constants.currentWebContainerId.rawValue) - let manager = DataStoreIdManager(store: store) - XCTAssertEqual(manager.currentId?.uuidString, storedUUID) + manager.invalidateCurrentId() + XCTAssertNil(manager.currentId) } } diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index c786184548..93e7801d04 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -48,7 +48,7 @@ final class FireButtonReferenceTests: XCTestCase { @MainActor func testClearDataUsingLegacyContainer() async throws { // Using WKWebsiteDataStore(forIdentifier:) doesn't persist cookies in a testable way, so use the legacy container here. - let fireproofing = UserDefaultsFireproofing.shared + let fireproofing = UserDefaultsFireproofing.xshared fireproofing.clearAll() for site in testData.fireButtonFireproofing.fireproofedSites { @@ -60,8 +60,6 @@ final class FireButtonReferenceTests: XCTestCase { let referenceTests = testData.fireButtonFireproofing.tests.filter { $0.exceptPlatforms.contains("ios-browser") == false } - - let cookieStorage = MigratableCookieStorage() for test in referenceTests { let cookie = try XCTUnwrap(cookie(for: test)) @@ -69,15 +67,13 @@ final class FireButtonReferenceTests: XCTestCase { let warmup = DataStoreWarmup() await warmup.ensureReady(applicationState: .unknown) - let cookieStore = WKWebsiteDataStore.default().httpCookieStore + let dataStore = WKWebsiteDataStore.default() + let cookieStore = dataStore.httpCookieStore await cookieStore.setCookie(cookie) - - // Pretend the webview was loaded and the cookies were previously consumed - cookieStorage.isConsumed = true - - await WebCacheManager.shared.clear(cookieStorage: cookieStorage, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) - let testCookie = cookieStorage.cookies.filter { $0.name == test.cookieName }.first + await WebCacheManager(cookieStorage: MigratableCookieStorage(), fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())).clear(dataStore: dataStore) + + let testCookie = await cookieStore.allCookies().filter { $0.name == test.cookieName }.first if test.expectCookieRemoved { XCTAssertNil(testCookie, "Cookie should not exist for test: \(test.name)") @@ -86,52 +82,11 @@ final class FireButtonReferenceTests: XCTestCase { } // Reset cache - cookieStorage.cookies = [] + // cookieStorage.cookies = [] } } - - func testCookieStorage() throws { - let fireproofing = UserDefaultsFireproofing.shared - fireproofing.clearAll() - - for site in testData.fireButtonFireproofing.fireproofedSites { - let sanitizedSite = sanitizedSite(site) - print("Adding %s to fireproofed sites", sanitizedSite) - fireproofing.addToAllowed(domain: sanitizedSite) - } - - let referenceTests = testData.fireButtonFireproofing.tests.filter { - $0.exceptPlatforms.contains("ios-browser") == false - } - - let cookieStorage = MigratableCookieStorage() - cookieStorage.isConsumed = true - for test in referenceTests { - let cookie = try XCTUnwrap(cookie(for: test)) - - // Pretend the webview was loaded and the cookies were previously consumed - cookieStorage.isConsumed = true - - // This simulates loading the cookies from the current web view data stores and updating the storage - cookieStorage.updateCookies([ - cookie - ], preservingFireproofedDomains: fireproofing) - let testCookie = cookieStorage.cookies.filter { $0.name == test.cookieName }.first - - if test.expectCookieRemoved { - XCTAssertNil(testCookie, "Cookie should not exist for test: \(test.name)") - } else { - XCTAssertNotNil(testCookie, "Cookie should exist for test: \(test.name)") - } - - // Reset cache - cookieStorage.cookies = [] - cookieStorage.isConsumed = true - } - } - private func cookie(for test: Test) -> HTTPCookie? { HTTPCookie(properties: [.name: test.cookieName, .path: "", diff --git a/DuckDuckGoTests/MockFireproofing.swift b/DuckDuckGoTests/MockFireproofing.swift new file mode 100644 index 0000000000..15e4bd3e24 --- /dev/null +++ b/DuckDuckGoTests/MockFireproofing.swift @@ -0,0 +1,37 @@ +// +// MockFireproofing.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import XCTest +@testable import Core + +// MARK: Mocks + +class MockFireproofing: UserDefaultsFireproofing { + override var allowedDomains: [String] { + return domains + } + + let domains: [String] + init(domains: [String] = []) { + self.domains = domains + } + +} + diff --git a/DuckDuckGoTests/MockTabDelegate.swift b/DuckDuckGoTests/MockTabDelegate.swift index fefb80faa7..6b50f42973 100644 --- a/DuckDuckGoTests/MockTabDelegate.swift +++ b/DuckDuckGoTests/MockTabDelegate.swift @@ -138,7 +138,9 @@ extension TabViewController { urlCredentialCreator: MockCredentialCreator(), featureFlagger: featureFlagger, subscriptionCookieManager: SubscriptionCookieManagerMock(), - textZoomCoordinator: MockTextZoomCoordinator() + textZoomCoordinator: MockTextZoomCoordinator(), + websiteDataManager: MockWebsiteDataManager(), + fireproofing: MockFireproofing() ) tab.attachWebView(configuration: .nonPersistent(), andLoadRequest: nil, consumeCookies: false, customWebView: customWebView) return tab diff --git a/DuckDuckGoTests/MockWebsiteDataManager.swift b/DuckDuckGoTests/MockWebsiteDataManager.swift new file mode 100644 index 0000000000..441b28cb76 --- /dev/null +++ b/DuckDuckGoTests/MockWebsiteDataManager.swift @@ -0,0 +1,35 @@ +// +// MockWebsiteDataManager.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import WebKit + +@testable import Core + +class MockWebsiteDataManager: WebsiteDataManaging { + + func removeCookies(forDomains domains: [String], fromDataStore: WKWebsiteDataStore) async { + } + + func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async { + } + + func clear(dataStore: WKWebsiteDataStore) async { + } + +} diff --git a/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift b/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift index 3aa6866fb5..b266186908 100644 --- a/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift +++ b/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift @@ -34,6 +34,8 @@ final class OnboardingDaxFavouritesTests: XCTestCase { private var tutorialSettingsMock: MockTutorialSettings! private var contextualOnboardingLogicMock: ContextualOnboardingLogicMock! + let mockWebsiteDataManager = MockWebsiteDataManager() + override func setUpWithError() throws { try super.setUpWithError() let db = CoreDataDatabase.bookmarksMock @@ -82,8 +84,10 @@ final class OnboardingDaxFavouritesTests: XCTestCase { subscriptionFeatureAvailability: SubscriptionFeatureAvailabilityMock.enabled, voiceSearchHelper: MockVoiceSearchHelper(isSpeechRecognizerAvailable: true, voiceSearchEnabled: true), featureFlagger: MockFeatureFlagger(), + fireproofing: MockFireproofing(), subscriptionCookieManager: SubscriptionCookieManagerMock(), - textZoomCoordinator: MockTextZoomCoordinator() + textZoomCoordinator: MockTextZoomCoordinator(), + websiteDataManager: mockWebsiteDataManager ) let window = UIWindow(frame: UIScreen.main.bounds) window.rootViewController = UIViewController() diff --git a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift index 52e2da5b9a..7c1f47bc6b 100644 --- a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift +++ b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift @@ -80,8 +80,11 @@ final class OnboardingNavigationDelegateTests: XCTestCase { subscriptionFeatureAvailability: SubscriptionFeatureAvailabilityMock.enabled, voiceSearchHelper: MockVoiceSearchHelper(isSpeechRecognizerAvailable: true, voiceSearchEnabled: true), featureFlagger: MockFeatureFlagger(), + fireproofing: MockFireproofing(), subscriptionCookieManager: SubscriptionCookieManagerMock(), - textZoomCoordinator: MockTextZoomCoordinator()) + textZoomCoordinator: MockTextZoomCoordinator(), + websiteDataManager: MockWebsiteDataManager() + ) let window = UIWindow(frame: UIScreen.main.bounds) window.rootViewController = UIViewController() window.makeKeyAndVisible() diff --git a/DuckDuckGoTests/Subscription/PrivacyProDataReporterTests.swift b/DuckDuckGoTests/Subscription/PrivacyProDataReporterTests.swift index 65597fd1a0..b54c2f20fa 100644 --- a/DuckDuckGoTests/Subscription/PrivacyProDataReporterTests.swift +++ b/DuckDuckGoTests/Subscription/PrivacyProDataReporterTests.swift @@ -73,6 +73,7 @@ final class PrivacyProDataReporterTests: XCTestCase { autofillCheck: { true }, secureVaultMaker: { nil }, tabsModel: TabsModel(tabs: [], desktop: false), + fireproofing: MockFireproofing(), dateGenerator: mockCalendar.now ) anotherReporter = PrivacyProDataReporter( @@ -85,7 +86,8 @@ final class PrivacyProDataReporterTests: XCTestCase { featureFlagger: MockFeatureFlagger(), autofillCheck: { true }, secureVaultMaker: { nil }, - tabsModel: TabsModel(tabs: [Tab(), Tab(), Tab(), Tab()], desktop: false) + tabsModel: TabsModel(tabs: [Tab(), Tab(), Tab(), Tab()], desktop: false), + fireproofing: MockFireproofing() ) } diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index 0628268239..b2b9d253a6 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -22,6 +22,7 @@ import XCTest import WebKit import TestUtils +/* class WebCacheManagerTests: XCTestCase { override func setUp() { @@ -195,18 +196,5 @@ class WebCacheManagerTests: XCTestCase { let pool = WebCacheManager.shared.getValidDatabasePool() XCTAssertNotNil(pool, "DatabasePool should not be nil") } - - // MARK: Mocks - - class MockFireproofing: UserDefaultsFireproofing { - override var allowedDomains: [String] { - return domains - } - - let domains: [String] - init(domains: [String]) { - self.domains = domains - } - } - } +*/ From c52d9a5a9227b5afd3b1527bc5ac882dae000db0 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Fri, 22 Nov 2024 19:41:52 +0000 Subject: [PATCH 04/14] swiftlint --- DuckDuckGoTests/MockFireproofing.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/DuckDuckGoTests/MockFireproofing.swift b/DuckDuckGoTests/MockFireproofing.swift index 15e4bd3e24..ced235d287 100644 --- a/DuckDuckGoTests/MockFireproofing.swift +++ b/DuckDuckGoTests/MockFireproofing.swift @@ -34,4 +34,3 @@ class MockFireproofing: UserDefaultsFireproofing { } } - From 1f9458eda8784f5359219aa6b78dec92f35b555e Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Mon, 25 Nov 2024 13:39:58 +0000 Subject: [PATCH 05/14] re-use macos implementation --- Core/Fireproofing.swift | 12 ++- Core/MigratableCookieStorage.swift | 13 +-- Core/WebCacheManager.swift | 94 ++++++++++++++----- .../UserDefaultsFireproofingTests.swift | 9 ++ 4 files changed, 96 insertions(+), 32 deletions(-) diff --git a/Core/Fireproofing.swift b/Core/Fireproofing.swift index 745787900d..3fcb5e4e75 100644 --- a/Core/Fireproofing.swift +++ b/Core/Fireproofing.swift @@ -18,6 +18,7 @@ // import Foundation +import Subscription public protocol Fireproofing { @@ -53,13 +54,20 @@ public class UserDefaultsFireproofing: Fireproofing { } } + var allowedDomainsIncludingDuckDuckGo: [String] { + allowedDomains + [ + URL.ddg.host ?? "", + SubscriptionCookieManager.cookieDomain + ] + } + public func addToAllowed(domain: String) { allowedDomains += [domain] } public func isAllowed(cookieDomain: String) -> Bool { - return allowedDomains.contains(where: { $0 == cookieDomain + return allowedDomainsIncludingDuckDuckGo.contains(where: { $0 == cookieDomain || ".\($0)" == cookieDomain || (cookieDomain.hasPrefix(".") && $0.hasSuffix(cookieDomain)) }) } @@ -73,7 +81,7 @@ public class UserDefaultsFireproofing: Fireproofing { } public func isAllowed(fireproofDomain domain: String) -> Bool { - return allowedDomains.contains(domain) + return allowedDomainsIncludingDuckDuckGo.contains(domain) } } diff --git a/Core/MigratableCookieStorage.swift b/Core/MigratableCookieStorage.swift index 2cd69420cf..1c9e07b537 100644 --- a/Core/MigratableCookieStorage.swift +++ b/Core/MigratableCookieStorage.swift @@ -32,12 +32,13 @@ public class MigratableCookieStorage { private var userDefaults: UserDefaults var isConsumed: Bool { - get { - return userDefaults.bool(forKey: Keys.consumed, defaultValue: false) - } - set { - userDefaults.set(newValue, forKey: Keys.consumed) - } + // The default is now true because if this key does not exist then there are no + // cookies to consume as they have been migrated. + // Nothing sets this to false explicitly now. If the stored value is false then there + // are cookies from a previous version of the app which still need to be migrated. This + // could happen if the user hit the fire button and then an update happened + // before they browsed again. + return userDefaults.bool(forKey: Keys.consumed, defaultValue: true) } var cookies: [HTTPCookie] { diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 7390983add..d6a3abd733 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -52,6 +52,39 @@ public protocol WebsiteDataManaging { @MainActor public class WebCacheManager: WebsiteDataManaging { + static let safelyRemovableWebsiteDataTypes: Set = { + var types = WKWebsiteDataStore.allWebsiteDataTypes() + + types.insert("_WKWebsiteDataTypeMediaKeys") + types.insert("_WKWebsiteDataTypeHSTSCache") + types.insert("_WKWebsiteDataTypeSearchFieldRecentSearches") + types.insert("_WKWebsiteDataTypeResourceLoadStatistics") + types.insert("_WKWebsiteDataTypeCredentials") + types.insert("_WKWebsiteDataTypeAdClickAttributions") + types.insert("_WKWebsiteDataTypePrivateClickMeasurements") + types.insert("_WKWebsiteDataTypeAlternativeServices") + + fireproofableDataTypes.forEach { + types.remove($0) + } + + return types + }() + + static let fireproofableDataTypes: Set = { + Set([ + WKWebsiteDataTypeLocalStorage, + WKWebsiteDataTypeIndexedDBDatabases, + WKWebsiteDataTypeCookies, + ]) + }() + + static let fireproofableDataTypesExceptCookies: Set = { + var dataTypes = fireproofableDataTypes + dataTypes.remove(WKWebsiteDataTypeCookies) + return dataTypes + }() + let cookieStorage: MigratableCookieStorage let fireproofing: Fireproofing let dataStoreIdManager: DataStoreIdManaging @@ -62,10 +95,13 @@ public class WebCacheManager: WebsiteDataManaging { self.dataStoreIdManager = dataStoreIdManager } - /// We save cookies from the current container rather than copying them to a new container because - /// the container only persists cookies to disk when the web view is used. If the user presses the fire button - /// twice then the fire proofed cookies will be lost and the user will be logged out any sites they're logged in to. + /// The previous version saved cookies externally to the data so we can move them between containers. We now use + /// the default persistence so this only needs to happen once when the fire button is pressed. + /// + /// The migration code removes the key that is used to check for the isConsumed flag so will only be + /// true if the data needs to be migrated. public func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async { + // This can only be true if the data has not yet been migrated. guard !cookieStorage.isConsumed else { return } let cookies = cookieStorage.cookies @@ -74,7 +110,6 @@ public class WebCacheManager: WebsiteDataManaging { consumedCookiesCount += 1 await httpCookieStore.setCookie(cookie) } - cookieStorage.isConsumed = true } public func removeCookies(forDomains domains: [String], @@ -169,33 +204,44 @@ extension WebCacheManager { private func clearData(inDataStore dataStore: WKWebsiteDataStore, withFireproofing fireproofing: Fireproofing) async { let startTime = CACurrentMediaTime() - // Start with all types - var types = WKWebsiteDataStore.allWebsiteDataTypes() + await clearDataForSafelyRemovableDataTypes(fromStore: dataStore) + await clearFireproofableDataForNonFireproofDomains(fromStore: dataStore, usingFireproofing: fireproofing) + await clearCookiesForNonFireproofedDomains(fromStore: dataStore, usingFireproofing: fireproofing) + self.removeObservationsData() - // Remove types we want to retain - types.remove(WKWebsiteDataTypeCookies) - types.remove(WKWebsiteDataTypeLocalStorage) + let totalTime = CACurrentMediaTime() - startTime + Pixel.fire(pixel: .clearDataInDefaultPersistence(.init(number: totalTime))) + } - // Add types without an API constant that we also want to clear - types.insert("_WKWebsiteDataTypeMediaKeys") - types.insert("_WKWebsiteDataTypeHSTSCache") - types.insert("_WKWebsiteDataTypeSearchFieldRecentSearches") - types.insert("_WKWebsiteDataTypeResourceLoadStatistics") - types.insert("_WKWebsiteDataTypeCredentials") - types.insert("_WKWebsiteDataTypeAdClickAttributions") - types.insert("_WKWebsiteDataTypePrivateClickMeasurements") - types.insert("_WKWebsiteDataTypeAlternativeServices") + @MainActor + private func clearDataForSafelyRemovableDataTypes(fromStore dataStore: WKWebsiteDataStore) async { + await dataStore.removeData(ofTypes: Self.safelyRemovableWebsiteDataTypes, modifiedSince: Date.distantPast) + } - // Get a list of records that are NOT fireproofed - let removableRecords = await dataStore.dataRecords(ofTypes: types).filter { record in + @MainActor + private func clearFireproofableDataForNonFireproofDomains(fromStore dataStore: WKWebsiteDataStore, usingFireproofing fireproofing: Fireproofing) async { + let allRecords = await dataStore.dataRecords(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes()) + let removableRecords = allRecords.filter { record in !fireproofing.isAllowed(fireproofDomain: record.displayName) } - await dataStore.removeData(ofTypes: types, for: removableRecords) + var fireproofableTypesExceptCookies = Self.fireproofableDataTypesExceptCookies + fireproofableTypesExceptCookies.remove(WKWebsiteDataTypeCookies) + await dataStore.removeData(ofTypes: fireproofableTypesExceptCookies, for: removableRecords) + } + + @MainActor + private func clearCookiesForNonFireproofedDomains(fromStore dataStore: WKWebsiteDataStore, usingFireproofing fireproofing: Fireproofing) async { + let cookieStore = dataStore.httpCookieStore + let cookies = await cookieStore.allCookies() - self.removeObservationsData() - let totalTime = CACurrentMediaTime() - startTime - Pixel.fire(pixel: .clearDataInDefaultPersistence(.init(number: totalTime))) + let cookiesToRemove = cookies.filter { cookie in + !fireproofing.isAllowed(cookieDomain: cookie.domain) + } + + for cookie in cookiesToRemove { + await cookieStore.deleteCookie(cookie) + } } } diff --git a/DuckDuckGoTests/UserDefaultsFireproofingTests.swift b/DuckDuckGoTests/UserDefaultsFireproofingTests.swift index 58154219d1..93efa25976 100644 --- a/DuckDuckGoTests/UserDefaultsFireproofingTests.swift +++ b/DuckDuckGoTests/UserDefaultsFireproofingTests.swift @@ -19,6 +19,7 @@ import XCTest @testable import Core +@testable import Subscription class UserDefaultsFireproofingTests: XCTestCase { @@ -40,4 +41,12 @@ class UserDefaultsFireproofingTests: XCTestCase { XCTAssertTrue(fireproofing.allowedDomains.isEmpty) } + func testDuckDuckGoIsFireproofed() { + let fireproofing = UserDefaultsFireproofing() + XCTAssertTrue(fireproofing.isAllowed(fireproofDomain: "duckduckgo.com")) + XCTAssertTrue(fireproofing.isAllowed(cookieDomain: "duckduckgo.com")) + XCTAssertTrue(fireproofing.isAllowed(cookieDomain: SubscriptionCookieManager.cookieDomain)) + XCTAssertFalse(fireproofing.isAllowed(cookieDomain: "test.duckduckgo.com")) + } + } From 327a12f9c976bfde79b2eb3d6083102c8f4873a8 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Mon, 25 Nov 2024 18:08:56 +0000 Subject: [PATCH 06/14] added tests but need an abstraction for the data store container --- ...acheManager+ObservationsDataClearing.swift | 3 +- Core/WebCacheManager.swift | 10 +- DuckDuckGoTests/CookieStorageTests.swift | 27 +- .../UserDefaultsFireproofingTests.swift | 11 +- DuckDuckGoTests/WebCacheManagerTests.swift | 238 ++++++++---------- 5 files changed, 133 insertions(+), 156 deletions(-) diff --git a/Core/WebCacheManager+ObservationsDataClearing.swift b/Core/WebCacheManager+ObservationsDataClearing.swift index ada6aa974f..5f11a2bc4e 100644 --- a/Core/WebCacheManager+ObservationsDataClearing.swift +++ b/Core/WebCacheManager+ObservationsDataClearing.swift @@ -31,7 +31,8 @@ extension WebCacheManager { } } - private func getValidDatabasePool() -> DatabasePool? { + /// Only has internal so that it can be accessed by test + func getValidDatabasePool() -> DatabasePool? { let bundleID = Bundle.main.bundleIdentifier ?? "" let databaseURLs = [ diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index d6a3abd733..9366391de3 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -88,11 +88,13 @@ public class WebCacheManager: WebsiteDataManaging { let cookieStorage: MigratableCookieStorage let fireproofing: Fireproofing let dataStoreIdManager: DataStoreIdManaging + let dataStoreClearingDelay: TimeInterval - public init(cookieStorage: MigratableCookieStorage, fireproofing: Fireproofing, dataStoreIdManager: DataStoreIdManaging) { + public init(cookieStorage: MigratableCookieStorage, fireproofing: Fireproofing, dataStoreIdManager: DataStoreIdManaging, dataStoreClearingDelay: TimeInterval = 3.0) { self.cookieStorage = cookieStorage self.fireproofing = fireproofing self.dataStoreIdManager = dataStoreIdManager + self.dataStoreClearingDelay = dataStoreClearingDelay } /// The previous version saved cookies externally to the data so we can move them between containers. We now use @@ -126,7 +128,7 @@ public class WebCacheManager: WebsiteDataManaging { public func clear(dataStore: WKWebsiteDataStore) async { - await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, destinationStore: dataStore) + await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, cookieStorage: cookieStorage, destinationStore: dataStore) await clearData(inDataStore: dataStore, withFireproofing: fireproofing) removeContainersIfNeeded() @@ -137,7 +139,7 @@ public class WebCacheManager: WebsiteDataManaging { extension WebCacheManager { private func performMigrationIfNeeded(dataStoreIdManager: DataStoreIdManaging, - cookieStorage: MigratableCookieStorage = MigratableCookieStorage(), + cookieStorage: MigratableCookieStorage, destinationStore: WKWebsiteDataStore) async { // Check version here rather than on function so that we don't need complicated logic related to verison in the calling function @@ -165,7 +167,7 @@ extension WebCacheManager { // Attempt to clean up all previous stores, but wait for a few seconds. // If this fails, we are going to still clean them next time as WebKit keeps track of all stores for us. Task { - try? await Task.sleep(for: .seconds(3)) + try? await Task.sleep(interval: dataStoreClearingDelay) for uuid in await WKWebsiteDataStore.allDataStoreIdentifiers { try? await WKWebsiteDataStore.remove(forIdentifier: uuid) } diff --git a/DuckDuckGoTests/CookieStorageTests.swift b/DuckDuckGoTests/CookieStorageTests.swift index 5443c591bf..5c0b3d3666 100644 --- a/DuckDuckGoTests/CookieStorageTests.swift +++ b/DuckDuckGoTests/CookieStorageTests.swift @@ -27,11 +27,11 @@ public class CookieStorageTests: XCTestCase { let defaults = UserDefaults(suiteName: "test")! defaults.removeSuite(named: "test") - addCookies([ - make("example.com", name: "test1", value: "value1"), - make("example.com", name: "test2", value: "value2"), - make("facebook.com", name: "test3", value: "value3"), - ], defaults) + MigratableCookieStorage.addCookies([ + .make(name: "test1", value: "value1", domain: "example.com"), + .make(name: "test2", value: "value2", domain: "example.com"), + .make(name: "test3", value: "value3", domain: "facebook.com"), + ], defaults) let storage = MigratableCookieStorage(userDefaults: defaults) XCTAssertEqual(storage.cookies.count, 3) @@ -60,7 +60,12 @@ public class CookieStorageTests: XCTestCase { XCTAssertTrue(storage.cookies.isEmpty) } - func addCookies(_ cookies: [HTTPCookie], _ defaults: UserDefaults) { + +} + +extension MigratableCookieStorage { + + static func addCookies(_ cookies: [HTTPCookie], _ defaults: UserDefaults) { var cookieData = [[String: Any?]]() cookies.forEach { cookie in @@ -74,14 +79,4 @@ public class CookieStorageTests: XCTestCase { } - func make(_ domain: String, name: String, value: String, expires: Date? = nil) -> HTTPCookie { - return HTTPCookie(properties: [ - .domain: domain, - .name: name, - .value: value, - .path: "/", - .expires: expires as Any - ])! - } - } diff --git a/DuckDuckGoTests/UserDefaultsFireproofingTests.swift b/DuckDuckGoTests/UserDefaultsFireproofingTests.swift index 93efa25976..28d94e61e7 100644 --- a/DuckDuckGoTests/UserDefaultsFireproofingTests.swift +++ b/DuckDuckGoTests/UserDefaultsFireproofingTests.swift @@ -35,7 +35,16 @@ class UserDefaultsFireproofingTests: XCTestCase { fireproofing.addToAllowed(domain: "example.com") XCTAssertTrue(fireproofing.isAllowed(fireproofDomain: "example.com")) } - + + func testAllowedCookieDomains() { + let fireproofing = UserDefaultsFireproofing() + XCTAssertFalse(fireproofing.isAllowed(fireproofDomain: "example.com")) + fireproofing.addToAllowed(domain: "example.com") + XCTAssertTrue(fireproofing.isAllowed(cookieDomain: ".example.com")) + XCTAssertFalse(fireproofing.isAllowed(cookieDomain: "subdomain.example.com")) + XCTAssertFalse(fireproofing.isAllowed(cookieDomain: ".subdomain.example.com")) + } + func testWhenNewThenAllowedDomainsIsEmpty() { let fireproofing = UserDefaultsFireproofing() XCTAssertTrue(fireproofing.allowedDomains.isEmpty) diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index b2b9d253a6..0a8fe24f53 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -22,179 +22,149 @@ import XCTest import WebKit import TestUtils -/* +// TODO DataStoreIDManager assert new install has nil id for container + class WebCacheManagerTests: XCTestCase { + let defaults = UserDefaults(suiteName: "Test")! + let dataStoreIdStore = MockKeyValueStore() + + lazy var cookieStorage = MigratableCookieStorage(userDefaults: defaults) + lazy var fireproofing = MockFireproofing() + lazy var dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) + override func setUp() { super.setUp() - MigratableCookieStorage().cookies = [] - MigratableCookieStorage().isConsumed = true - - if #available(iOS 17, *) { - WKWebsiteDataStore.fetchAllDataStoreIdentifiers { uuids in - uuids.forEach { - WKWebsiteDataStore.remove(forIdentifier: $0, completionHandler: { _ in }) - } - } - } + defaults.removePersistentDomain(forName: "Test") } - @available(iOS 17, *) - @MainActor - func testEnsureIdAllocatedAfterClearing() async throws { - let fireproofing = MockFireproofing(domains: []) - - let storage = MigratableCookieStorage() - - let inMemoryDataStoreIdManager = DataStoreIdManager(store: MockKeyValueStore()) - XCTAssertNil(inMemoryDataStoreIdManager.currentId) + func test_whenNewInstall_ThenUsesDefaultPersistence() async { + let dataStore = await WKWebsiteDataStore.current() + let defaultStore = await WKWebsiteDataStore.default() + XCTAssertTrue(dataStore === defaultStore) + } - await WebCacheManager.shared.clear(cookieStorage: storage, fireproofing: fireproofing, dataStoreIdManager: inMemoryDataStoreIdManager) + func test_whenClearingData_ThenCookiesAreRemoved() async { + let dataStore = await WKWebsiteDataStore.default() + await dataStore.httpCookieStore.setCookie(.make(name: "Test", value: "Value", domain: "example.com")) - XCTAssertNotNil(inMemoryDataStoreIdManager.currentId) - let oldId = inMemoryDataStoreIdManager.currentId?.uuidString - XCTAssertNotNil(oldId) + var cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(1, cookies.count) - await WebCacheManager.shared.clear(cookieStorage: storage, fireproofing: fireproofing, dataStoreIdManager: inMemoryDataStoreIdManager) + let webCacheManager = await makeWebCacheManager() + await webCacheManager.clear(dataStore: dataStore) - XCTAssertNotNil(inMemoryDataStoreIdManager.currentId) - XCTAssertNotEqual(inMemoryDataStoreIdManager.currentId?.uuidString, oldId) + cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(0, cookies.count) } - @available(iOS 17, *) - @MainActor - func testWhenCookiesHaveSubDomainsOnSubDomainsAndWidlcardsThenOnlyMatchingCookiesRetained() async throws { - let fireproofing = MockFireproofing(domains: ["mobile.twitter.com"]) - - let defaultStore = WKWebsiteDataStore.default() - await defaultStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes(), modifiedSince: .distantPast) - - let initialCount = await defaultStore.httpCookieStore.allCookies().count - XCTAssertEqual(0, initialCount) + func test_WhenClearingDefaultPersistence_ThenLeaveFireproofedCookies() async { + fireproofing = MockFireproofing(domains: ["example.com"]) - await defaultStore.httpCookieStore.setCookie(.make(domain: "twitter.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: ".twitter.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: "mobile.twitter.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: "fake.mobile.twitter.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: ".fake.mobile.twitter.com")) + let dataStore = await WKWebsiteDataStore.default() + await dataStore.httpCookieStore.setCookie(.make(name: "Test1", value: "Value", domain: "example.com")) + await dataStore.httpCookieStore.setCookie(.make(name: "Test2", value: "Value", domain: ".example.com")) + await dataStore.httpCookieStore.setCookie(.make(name: "Test3", value: "Value", domain: "facebook.com")) - let loadedCount = await defaultStore.httpCookieStore.allCookies().count - XCTAssertEqual(5, loadedCount) + var cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(3, cookies.count) - let cookieStore = MigratableCookieStorage() - await WebCacheManager.shared.clear(cookieStorage: cookieStore, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) + let webCacheManager = await makeWebCacheManager() + await webCacheManager.clear(dataStore: dataStore) - let cookies = await defaultStore.httpCookieStore.allCookies() - XCTAssertEqual(cookies.count, 0) - - XCTAssertEqual(2, cookieStore.cookies.count) - XCTAssertTrue(cookieStore.cookies.contains(where: { $0.domain == ".twitter.com" })) - XCTAssertTrue(cookieStore.cookies.contains(where: { $0.domain == "mobile.twitter.com" })) - } - - @MainActor - func testWhenRemovingCookieForDomainThenItIsRemovedFromCookieStorage() async { - let defaultStore = WKWebsiteDataStore.default() - await defaultStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes(), modifiedSince: .distantPast) - - let initialCount = await defaultStore.httpCookieStore.allCookies().count - XCTAssertEqual(0, initialCount) - - await defaultStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes(), modifiedSince: .distantPast) - await defaultStore.httpCookieStore.setCookie(.make(domain: "www.example.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: ".example.com")) - let cookies = await defaultStore.httpCookieStore.allCookies() - XCTAssertEqual(cookies.count, 2) - - await WebCacheManager.shared.removeCookies(forDomains: ["www.example.com"], dataStore: WKWebsiteDataStore.default()) - let cleanCookies = await defaultStore.httpCookieStore.allCookies() - XCTAssertEqual(cleanCookies.count, 0) + cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(2, cookies.count) + XCTAssertTrue(cookies.contains(where: { $0.domain == "example.com" })) + XCTAssertTrue(cookies.contains(where: { $0.domain == ".example.com" })) } + // TODO create an abstraction for WKWebsiteDataStore management and pass it into the webcache manager @MainActor - func testWhenClearedThenCookiesWithParentDomainsAreRetained() async { - let fireproofing = MockFireproofing(domains: ["www.example.com"]) - - let defaultStore = WKWebsiteDataStore.default() - await defaultStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes(), modifiedSince: .distantPast) + @available(iOS 17, *) + func test_WhenClearingDataAfterUsingContainer_ThenCookiesAreMigratedAndOldContainersAreRemoved() async { + fireproofing = MockFireproofing(domains: ["example.com"]) - let initialCount = await defaultStore.httpCookieStore.allCookies().count - XCTAssertEqual(0, initialCount) + MigratableCookieStorage.addCookies([ + .make(name: "Test1", value: "Value", domain: "example.com"), + .make(name: "Test2", value: "Value", domain: ".example.com"), + .make(name: "Test3", value: "Value", domain: "facebook.com"), + ], defaults) - await defaultStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes(), modifiedSince: .distantPast) - await defaultStore.httpCookieStore.setCookie(.make(domain: "example.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: ".example.com")) + // Setup a new container and add something to it just to make it real + let uuid = await createContainer() - let cookieStorage = MigratableCookieStorage() - - await WebCacheManager.shared.clear(cookieStorage: cookieStorage, - fireproofing: fireproofing, - dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) - let cookies = await defaultStore.httpCookieStore.allCookies() + var dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers + XCTAssertEqual(1, dataStoreIds.count) - XCTAssertEqual(cookies.count, 0) - XCTAssertEqual(cookieStorage.cookies.count, 1) - XCTAssertEqual(cookieStorage.cookies[0].domain, ".example.com") - } + // Use the UUID we just created for the store id + dataStoreIdStore.store = [DataStoreIdManager.Constants.currentWebContainerId.rawValue: uuid.uuidString] + dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) - @MainActor - @available(iOS 17, *) - func testWhenClearedWithDataStoreContainerThenDDGCookiesAreRetained() async throws { - throw XCTSkip("WKWebsiteDataStore(forIdentifier:) does not persist cookies properly until attached to a running webview") - - // This test should look like `testWhenClearedWithLegacyContainerThenDDGCookiesAreRetained` but - // with a container ID set on the `dataStoreIdManager`. - } + let dataStore = WKWebsiteDataStore.default() + var cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(0, cookies.count) - @MainActor - func testWhenClearedWithLegacyContainerThenDDGCookiesAreRetained() async { - let fireproofing = MockFireproofing(domains: ["www.example.com"]) + let webCacheManager = makeWebCacheManager() + await webCacheManager.clear(dataStore: dataStore) - let cookieStore = WKWebsiteDataStore.default().httpCookieStore - await cookieStore.setCookie(.make(name: "name", value: "value", domain: "duckduckgo.com")) - await cookieStore.setCookie(.make(name: "name", value: "value", domain: "subdomain.duckduckgo.com")) + try? await Task.sleep(interval: 0.3) - let storage = MigratableCookieStorage() - storage.isConsumed = true - - await WebCacheManager.shared.clear(cookieStorage: storage, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) + cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(2, cookies.count) + XCTAssertTrue(cookies.contains(where: { $0.domain == "example.com" })) + XCTAssertTrue(cookies.contains(where: { $0.domain == ".example.com" })) - XCTAssertEqual(storage.cookies.count, 2) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "duckduckgo.com" })) - XCTAssertTrue(storage.cookies.contains(where: { $0.domain == "subdomain.duckduckgo.com" })) + dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers + XCTAssertTrue(dataStoreIds.isEmpty) } - - @MainActor - func testWhenClearedThenCookiesForLoginsAreRetained() async { - let fireproofing = MockFireproofing(domains: ["www.example.com"]) - let defaultStore = WKWebsiteDataStore.default() - await defaultStore.removeData(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes(), modifiedSince: .distantPast) + @MainActor + @available(iOS 17, *) + func test_WhenClearingData_ThenOldContainersAreRemoved() async { + _ = await createContainer() + _ = await createContainer() + _ = await createContainer() + _ = await createContainer() + _ = await createContainer() - let initialCount = await defaultStore.httpCookieStore.allCookies().count - XCTAssertEqual(0, initialCount) + try? await Task.sleep(interval: 0.3) - await defaultStore.httpCookieStore.setCookie(.make(domain: "www.example.com")) - await defaultStore.httpCookieStore.setCookie(.make(domain: "facebook.com")) + var dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers + XCTAssertEqual(5, dataStoreIds.count) - let loadedCount = await defaultStore.httpCookieStore.allCookies().count - XCTAssertEqual(2, loadedCount) + await makeWebCacheManager().clear(dataStore: .default()) - let cookieStore = MigratableCookieStorage() - - await WebCacheManager.shared.clear(cookieStorage: cookieStore, fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())) + try? await Task.sleep(interval: 0.3) - let cookies = await defaultStore.httpCookieStore.allCookies() - XCTAssertEqual(cookies.count, 0) - - XCTAssertEqual(1, cookieStore.cookies.count) - XCTAssertEqual(cookieStore.cookies[0].domain, "www.example.com") + dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers + XCTAssertEqual(0, dataStoreIds.count) } + /// Temporarily disabled. @MainActor func x_testWhenAccessingObservationsDbThenValidDatabasePoolIsReturned() { - let pool = WebCacheManager.shared.getValidDatabasePool() + let pool = makeWebCacheManager().getValidDatabasePool() XCTAssertNotNil(pool, "DatabasePool should not be nil") } + + @MainActor + private func makeWebCacheManager() -> WebCacheManager { + return WebCacheManager( + cookieStorage: cookieStorage, + fireproofing: fireproofing, + dataStoreIdManager: dataStoreIdManager, + dataStoreClearingDelay: 0.01 + ) + } + + @available(iOS 17, *) + @MainActor private func createContainer() async -> UUID { + let uuid = UUID() + let containerStore = WKWebsiteDataStore(forIdentifier: uuid) + await containerStore.httpCookieStore.setCookie(.make(name: "Not", value: "Used")) + let cookies = await containerStore.httpCookieStore.allCookies() + XCTAssertEqual(1, cookies.count) + return uuid + } + } -*/ From 7ec9cded0e3bd25be7399de60ec99b605c70dec7 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Tue, 26 Nov 2024 12:03:31 +0000 Subject: [PATCH 07/14] abstract datastore removal to make it testable --- Core/WebCacheManager.swift | 55 +++------------- Core/WebsiteDataStoreCleaning.swift | 70 ++++++++++++++++++++ DuckDuckGo.xcodeproj/project.pbxproj | 8 ++- DuckDuckGoTests/WebCacheManagerTests.swift | 75 +++++++++------------- 4 files changed, 115 insertions(+), 93 deletions(-) create mode 100644 Core/WebsiteDataStoreCleaning.swift diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 9366391de3..84242e90ff 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -88,13 +88,16 @@ public class WebCacheManager: WebsiteDataManaging { let cookieStorage: MigratableCookieStorage let fireproofing: Fireproofing let dataStoreIdManager: DataStoreIdManaging - let dataStoreClearingDelay: TimeInterval + let dataStoreCleaner: WebsiteDataStoreCleaning - public init(cookieStorage: MigratableCookieStorage, fireproofing: Fireproofing, dataStoreIdManager: DataStoreIdManaging, dataStoreClearingDelay: TimeInterval = 3.0) { + public init(cookieStorage: MigratableCookieStorage, + fireproofing: Fireproofing, + dataStoreIdManager: DataStoreIdManaging, + dataStoreCleaner: WebsiteDataStoreCleaning = DefaultWebsiteDataStoreCleaner()) { self.cookieStorage = cookieStorage self.fireproofing = fireproofing self.dataStoreIdManager = dataStoreIdManager - self.dataStoreClearingDelay = dataStoreClearingDelay + self.dataStoreCleaner = dataStoreCleaner } /// The previous version saved cookies externally to the data so we can move them between containers. We now use @@ -128,9 +131,10 @@ public class WebCacheManager: WebsiteDataManaging { public func clear(dataStore: WKWebsiteDataStore) async { + let count = await dataStoreCleaner.countContainers() await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, cookieStorage: cookieStorage, destinationStore: dataStore) await clearData(inDataStore: dataStore, withFireproofing: fireproofing) - removeContainersIfNeeded() + dataStoreCleaner.removeAllContainersAfterDelay(previousCount: count) } @@ -160,47 +164,8 @@ extension WebCacheManager { dataStoreIdManager.invalidateCurrentId() } - private func removeContainersIfNeeded() { - // Check version here rather than on function so that we don't need complicated logic related to verison in the calling function - guard #available(iOS 17, *) else { return } - - // Attempt to clean up all previous stores, but wait for a few seconds. - // If this fails, we are going to still clean them next time as WebKit keeps track of all stores for us. - Task { - try? await Task.sleep(interval: dataStoreClearingDelay) - for uuid in await WKWebsiteDataStore.allDataStoreIdentifiers { - try? await WKWebsiteDataStore.remove(forIdentifier: uuid) - } - - let count = await WKWebsiteDataStore.allDataStoreIdentifiers.count - switch count { - case 0: - Pixel.fire(pixel: .debugWebsiteDataStoresCleared) - - case 1: - Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedOne) - - default: - Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedMultiple) - } - } - } - - private func checkForLeftBehindDataStores(previousLeftOversCount: Int) async { - guard #available(iOS 17, *) else { return } - - let params = [ - "left_overs_count": "\(previousLeftOversCount)" - ] - - let ids = await WKWebsiteDataStore.allDataStoreIdentifiers - if ids.count > 1 { - Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedMultiple, withAdditionalParameters: params) - } else if ids.count > 0 { - Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedOne, withAdditionalParameters: params) - } else if previousLeftOversCount > 0 { - Pixel.fire(pixel: .debugWebsiteDataStoresCleared, withAdditionalParameters: params) - } + private func removeContainersIfNeeded(previousCount: Int) { + dataStoreCleaner.removeAllContainersAfterDelay(previousCount: previousCount) } private func clearData(inDataStore dataStore: WKWebsiteDataStore, withFireproofing fireproofing: Fireproofing) async { diff --git a/Core/WebsiteDataStoreCleaning.swift b/Core/WebsiteDataStoreCleaning.swift new file mode 100644 index 0000000000..7b4fbef1e8 --- /dev/null +++ b/Core/WebsiteDataStoreCleaning.swift @@ -0,0 +1,70 @@ +// +// WebsiteDataStoreCleaning.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import WebKit + +public protocol WebsiteDataStoreCleaning { + + func countContainers() async -> Int + func removeAllContainersAfterDelay(previousCount: Int) + +} + +public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { + + public init() { } + + public func countContainers() async -> Int { + guard #available(iOS 17, *) else { return 0 } + return await WKWebsiteDataStore.allDataStoreIdentifiers.count + } + + public func removeAllContainersAfterDelay(previousCount: Int) { + guard #available(iOS 17, *) else { return } + + // Attempt to clean up all previous stores, but wait for a few seconds. + // If this fails, we are going to still clean them next time as WebKit keeps track of all stores for us. + Task { + try? await Task.sleep(interval: 3.0) + for uuid in await WKWebsiteDataStore.allDataStoreIdentifiers { + try? await WKWebsiteDataStore.remove(forIdentifier: uuid) + } + + await checkForLeftBehindDataStores(previousLeftOversCount: previousCount) + } + } + + private func checkForLeftBehindDataStores(previousLeftOversCount: Int) async { + guard #available(iOS 17, *) else { return } + + let params = [ + "left_overs_count": "\(previousLeftOversCount)" + ] + + let ids = await WKWebsiteDataStore.allDataStoreIdentifiers + if ids.count > 1 { + Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedMultiple, withAdditionalParameters: params) + } else if ids.count > 0 { + Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedOne, withAdditionalParameters: params) + } else if previousLeftOversCount > 0 { + Pixel.fire(pixel: .debugWebsiteDataStoresCleared, withAdditionalParameters: params) + } + } + +} diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 2e0a661dd8..6e13744d9a 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -365,9 +365,9 @@ 6FEC0B852C999352006B4F6E /* FavoriteItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FEC0B842C999352006B4F6E /* FavoriteItem.swift */; }; 6FEC0B882C999961006B4F6E /* FavoritesListInteractingAdapter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FEC0B872C999961006B4F6E /* FavoritesListInteractingAdapter.swift */; }; 6FF915822B88E07A0042AC87 /* AdAttributionFetcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF915802B88E0750042AC87 /* AdAttributionFetcherTests.swift */; }; - 6FF9AD452CE766F700C5A406 /* NewTabPageControllerPixelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */; }; 6FF9AD3F2CE63DD800C5A406 /* TabSwitcherOpenDailyPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF9AD3E2CE63DC200C5A406 /* TabSwitcherOpenDailyPixel.swift */; }; 6FF9AD412CE6610F00C5A406 /* TabSwitcherDailyPixelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF9AD402CE6610600C5A406 /* TabSwitcherDailyPixelTests.swift */; }; + 6FF9AD452CE766F700C5A406 /* NewTabPageControllerPixelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */; }; 7B1604E82CB685B400A44EC6 /* Logger+TipKit.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B1604E72CB685B400A44EC6 /* Logger+TipKit.swift */; }; 7B1604EC2CB68BDA00A44EC6 /* TipKitController+ConvenienceInitializers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B1604EB2CB68BDA00A44EC6 /* TipKitController+ConvenienceInitializers.swift */; }; 7B1604EE2CB68D2600A44EC6 /* TipKitDebugOptionsUIActionHandling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B1604ED2CB68D2600A44EC6 /* TipKitDebugOptionsUIActionHandling.swift */; }; @@ -545,6 +545,7 @@ 85A1B3B220C6CD9900C18F15 /* MigratableCookieStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */; }; 85A313972028E78A00327D00 /* release_notes.txt in Resources */ = {isa = PBXBuildFile; fileRef = 85A313962028E78A00327D00 /* release_notes.txt */; }; 85A9C37920E0E00C00073340 /* HomeRow.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85A9C37820E0E00C00073340 /* HomeRow.xcassets */; }; + 85AB84C12CF5DDB4007E679F /* WebsiteDataStoreCleaning.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */; }; 85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */; }; 85AE668E2097206E0014CF04 /* NotificationView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 85AE668D2097206E0014CF04 /* NotificationView.xib */; }; 85AE6690209724120014CF04 /* NotificationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AE668F209724120014CF04 /* NotificationView.swift */; }; @@ -1683,9 +1684,9 @@ 6FEC0B842C999352006B4F6E /* FavoriteItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoriteItem.swift; sourceTree = ""; }; 6FEC0B872C999961006B4F6E /* FavoritesListInteractingAdapter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoritesListInteractingAdapter.swift; sourceTree = ""; }; 6FF915802B88E0750042AC87 /* AdAttributionFetcherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdAttributionFetcherTests.swift; sourceTree = ""; }; - 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageControllerPixelTests.swift; sourceTree = ""; }; 6FF9AD3E2CE63DC200C5A406 /* TabSwitcherOpenDailyPixel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabSwitcherOpenDailyPixel.swift; sourceTree = ""; }; 6FF9AD402CE6610600C5A406 /* TabSwitcherDailyPixelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabSwitcherDailyPixelTests.swift; sourceTree = ""; }; + 6FF9AD442CE766F700C5A406 /* NewTabPageControllerPixelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageControllerPixelTests.swift; sourceTree = ""; }; 7B1604E72CB685B400A44EC6 /* Logger+TipKit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Logger+TipKit.swift"; sourceTree = ""; }; 7B1604EB2CB68BDA00A44EC6 /* TipKitController+ConvenienceInitializers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TipKitController+ConvenienceInitializers.swift"; sourceTree = ""; }; 7B1604ED2CB68D2600A44EC6 /* TipKitDebugOptionsUIActionHandling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TipKitDebugOptionsUIActionHandling.swift; sourceTree = ""; }; @@ -1863,6 +1864,7 @@ 85A313962028E78A00327D00 /* release_notes.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = release_notes.txt; path = fastlane/metadata/default/release_notes.txt; sourceTree = ""; }; 85A53EC9200D1FA20010D13F /* FileStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileStore.swift; sourceTree = ""; }; 85A9C37820E0E00C00073340 /* HomeRow.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = HomeRow.xcassets; sourceTree = ""; }; + 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebsiteDataStoreCleaning.swift; sourceTree = ""; }; 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CookieStorageTests.swift; sourceTree = ""; }; 85AE668D2097206E0014CF04 /* NotificationView.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = NotificationView.xib; sourceTree = ""; }; 85AE668F209724120014CF04 /* NotificationView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationView.swift; sourceTree = ""; }; @@ -6029,6 +6031,7 @@ F143C3311E4A9A6A00CFDE3A /* Web */ = { isa = PBXGroup; children = ( + 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */, 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */, 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */, 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */, @@ -8368,6 +8371,7 @@ 85E065BC2C73A54700D73E2A /* UsageSegmentationStorage.swift in Sources */, F143C3271E4A9A0E00CFDE3A /* Logger+Multiple.swift in Sources */, 85372447220DD103009D09CD /* UIKeyCommandExtension.swift in Sources */, + 85AB84C12CF5DDB4007E679F /* WebsiteDataStoreCleaning.swift in Sources */, 85A1B3B220C6CD9900C18F15 /* MigratableCookieStorage.swift in Sources */, 9856A1992933D2EB00ACB44F /* BookmarksModelsErrorHandling.swift in Sources */, 850559D023CF647C0055C0D5 /* Fireproofing.swift in Sources */, diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index 0a8fe24f53..c33d45f800 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -22,8 +22,6 @@ import XCTest import WebKit import TestUtils -// TODO DataStoreIDManager assert new install has nil id for container - class WebCacheManagerTests: XCTestCase { let defaults = UserDefaults(suiteName: "Test")! @@ -32,6 +30,7 @@ class WebCacheManagerTests: XCTestCase { lazy var cookieStorage = MigratableCookieStorage(userDefaults: defaults) lazy var fireproofing = MockFireproofing() lazy var dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) + let dataStoreCleaner = MockDataStoreCleaner() override func setUp() { super.setUp() @@ -78,10 +77,15 @@ class WebCacheManagerTests: XCTestCase { XCTAssertTrue(cookies.contains(where: { $0.domain == ".example.com" })) } - // TODO create an abstraction for WKWebsiteDataStore management and pass it into the webcache manager - @MainActor @available(iOS 17, *) func test_WhenClearingDataAfterUsingContainer_ThenCookiesAreMigratedAndOldContainersAreRemoved() async { + // Mock having a single container so we can validate cleaning it gets called + dataStoreCleaner.countContainersReturnValue = 1 + + // Mock a data store id to force migration to happen + dataStoreIdStore.store = [DataStoreIdManager.Constants.currentWebContainerId.rawValue: UUID().uuidString] + dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) + fireproofing = MockFireproofing(domains: ["example.com"]) MigratableCookieStorage.addCookies([ @@ -90,54 +94,29 @@ class WebCacheManagerTests: XCTestCase { .make(name: "Test3", value: "Value", domain: "facebook.com"), ], defaults) - // Setup a new container and add something to it just to make it real - let uuid = await createContainer() - - var dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers - XCTAssertEqual(1, dataStoreIds.count) - - // Use the UUID we just created for the store id - dataStoreIdStore.store = [DataStoreIdManager.Constants.currentWebContainerId.rawValue: uuid.uuidString] - dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) - - let dataStore = WKWebsiteDataStore.default() + let dataStore = await WKWebsiteDataStore.default() var cookies = await dataStore.httpCookieStore.allCookies() XCTAssertEqual(0, cookies.count) - let webCacheManager = makeWebCacheManager() + let webCacheManager = await makeWebCacheManager() await webCacheManager.clear(dataStore: dataStore) - try? await Task.sleep(interval: 0.3) - cookies = await dataStore.httpCookieStore.allCookies() XCTAssertEqual(2, cookies.count) XCTAssertTrue(cookies.contains(where: { $0.domain == "example.com" })) XCTAssertTrue(cookies.contains(where: { $0.domain == ".example.com" })) - dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers - XCTAssertTrue(dataStoreIds.isEmpty) + XCTAssertEqual(1, dataStoreCleaner.removeAllContainersAfterDelayCalls.count) + XCTAssertEqual(1, dataStoreCleaner.removeAllContainersAfterDelayCalls[0]) } - @MainActor @available(iOS 17, *) func test_WhenClearingData_ThenOldContainersAreRemoved() async { - _ = await createContainer() - _ = await createContainer() - _ = await createContainer() - _ = await createContainer() - _ = await createContainer() - - try? await Task.sleep(interval: 0.3) - - var dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers - XCTAssertEqual(5, dataStoreIds.count) - + // Mock existence of 5 containers so we can validate that cleaning it is called even without migrations + dataStoreCleaner.countContainersReturnValue = 5 await makeWebCacheManager().clear(dataStore: .default()) - - try? await Task.sleep(interval: 0.3) - - dataStoreIds = await WKWebsiteDataStore.allDataStoreIdentifiers - XCTAssertEqual(0, dataStoreIds.count) + XCTAssertEqual(1, dataStoreCleaner.removeAllContainersAfterDelayCalls.count) + XCTAssertEqual(5, dataStoreCleaner.removeAllContainersAfterDelayCalls[0]) } /// Temporarily disabled. @@ -153,18 +132,22 @@ class WebCacheManagerTests: XCTestCase { cookieStorage: cookieStorage, fireproofing: fireproofing, dataStoreIdManager: dataStoreIdManager, - dataStoreClearingDelay: 0.01 + dataStoreCleaner: dataStoreCleaner ) } +} - @available(iOS 17, *) - @MainActor private func createContainer() async -> UUID { - let uuid = UUID() - let containerStore = WKWebsiteDataStore(forIdentifier: uuid) - await containerStore.httpCookieStore.setCookie(.make(name: "Not", value: "Used")) - let cookies = await containerStore.httpCookieStore.allCookies() - XCTAssertEqual(1, cookies.count) - return uuid +class MockDataStoreCleaner: WebsiteDataStoreCleaning { + + var countContainersReturnValue = 0 + var removeAllContainersAfterDelayCalls: [Int] = [] + + func countContainers() async -> Int { + return countContainersReturnValue + } + + func removeAllContainersAfterDelay(previousCount: Int) { + removeAllContainersAfterDelayCalls.append(previousCount) } } From 5de2087e28317f19df49379b985ea21fb6b5abcc Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Tue, 26 Nov 2024 12:36:26 +0000 Subject: [PATCH 08/14] mock the observations data --- ...g.swift => ObservationsDataClearing.swift} | 17 ++++++++++--- Core/WebCacheManager.swift | 7 ++++-- DuckDuckGo.xcodeproj/project.pbxproj | 12 ++++----- DuckDuckGoTests/WebCacheManagerTests.swift | 25 +++++++++++++------ 4 files changed, 41 insertions(+), 20 deletions(-) rename Core/{WebCacheManager+ObservationsDataClearing.swift => ObservationsDataClearing.swift} (84%) diff --git a/Core/WebCacheManager+ObservationsDataClearing.swift b/Core/ObservationsDataClearing.swift similarity index 84% rename from Core/WebCacheManager+ObservationsDataClearing.swift rename to Core/ObservationsDataClearing.swift index 5f11a2bc4e..a4cdb0915d 100644 --- a/Core/WebCacheManager+ObservationsDataClearing.swift +++ b/Core/ObservationsDataClearing.swift @@ -1,5 +1,5 @@ // -// WebCacheManager+ObservationsDataClearing.swift +// ObservationsDataCleaning.swift // DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. @@ -21,9 +21,18 @@ import Common import GRDB import os.log -extension WebCacheManager { +public protocol ObservationsDataCleaning { - func removeObservationsData() { + func removeObservationsData() async + +} + +/// Used by data clearing. Has no unit tests just now because getting the observation data is flakey, so we inject this for testing. +public class DefaultObservationsDataCleaner: ObservationsDataCleaning { + + public init() { } + + public func removeObservationsData() async { if let pool = getValidDatabasePool() { removeObservationsData(from: pool) } else { @@ -31,7 +40,6 @@ extension WebCacheManager { } } - /// Only has internal so that it can be accessed by test func getValidDatabasePool() -> DatabasePool? { let bundleID = Bundle.main.bundleIdentifier ?? "" @@ -62,4 +70,5 @@ extension WebCacheManager { Pixel.fire(pixel: .debugCannotClearObservationsDatabase, error: error) } } + } diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 84242e90ff..1b9f3f85c3 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -89,15 +89,18 @@ public class WebCacheManager: WebsiteDataManaging { let fireproofing: Fireproofing let dataStoreIdManager: DataStoreIdManaging let dataStoreCleaner: WebsiteDataStoreCleaning + let observationsCleaner: ObservationsDataCleaning public init(cookieStorage: MigratableCookieStorage, fireproofing: Fireproofing, dataStoreIdManager: DataStoreIdManaging, - dataStoreCleaner: WebsiteDataStoreCleaning = DefaultWebsiteDataStoreCleaner()) { + dataStoreCleaner: WebsiteDataStoreCleaning = DefaultWebsiteDataStoreCleaner(), + observationsCleaner: ObservationsDataCleaning = DefaultObservationsDataCleaner()) { self.cookieStorage = cookieStorage self.fireproofing = fireproofing self.dataStoreIdManager = dataStoreIdManager self.dataStoreCleaner = dataStoreCleaner + self.observationsCleaner = observationsCleaner } /// The previous version saved cookies externally to the data so we can move them between containers. We now use @@ -174,7 +177,7 @@ extension WebCacheManager { await clearDataForSafelyRemovableDataTypes(fromStore: dataStore) await clearFireproofableDataForNonFireproofDomains(fromStore: dataStore, usingFireproofing: fireproofing) await clearCookiesForNonFireproofedDomains(fromStore: dataStore, usingFireproofing: fireproofing) - self.removeObservationsData() + await observationsCleaner.removeObservationsData() let totalTime = CACurrentMediaTime() - startTime Pixel.fire(pixel: .clearDataInDefaultPersistence(.init(number: totalTime))) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 6e13744d9a..16722665d0 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -567,7 +567,7 @@ 85C2970A247EB7AA0063A335 /* Text.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85C29709247EB7AA0063A335 /* Text.xcassets */; }; 85C2971A248162CA0063A335 /* DaxOnboardingPadViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */; }; 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */; }; - 85C503FB2CF0ADCE0075DF6F /* WebCacheManager+ObservationsDataClearing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */; }; + 85C503FB2CF0ADCE0075DF6F /* ObservationsDataClearing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* ObservationsDataClearing.swift */; }; 85C503FD2CF0E7B10075DF6F /* MockFireproofing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */; }; 85C503FF2CF0F4DC0075DF6F /* MockWebsiteDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */; }; 85C8E61D2B0E47380029A6BD /* BookmarksDatabaseSetup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */; }; @@ -1887,7 +1887,7 @@ 85C29709247EB7AA0063A335 /* Text.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Text.xcassets; sourceTree = ""; }; 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxOnboardingPadViewController.swift; sourceTree = ""; }; 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIdManager.swift; sourceTree = ""; }; - 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WebCacheManager+ObservationsDataClearing.swift"; sourceTree = ""; }; + 85C503FA2CF0ADBD0075DF6F /* ObservationsDataClearing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ObservationsDataClearing.swift; sourceTree = ""; }; 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockFireproofing.swift; sourceTree = ""; }; 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockWebsiteDataManager.swift; sourceTree = ""; }; 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksDatabaseSetup.swift; sourceTree = ""; }; @@ -6031,19 +6031,19 @@ F143C3311E4A9A6A00CFDE3A /* Web */ = { isa = PBXGroup; children = ( - 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */, - 85C503FA2CF0ADBD0075DF6F /* WebCacheManager+ObservationsDataClearing.swift */, - 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */, 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */, 8596C30C2B7EB1800058EF90 /* DataStoreWarmup.swift */, 85BDC3132434D8F80053DB07 /* DebugUserScript.swift */, 850559CF23CF647C0055C0D5 /* Fireproofing.swift */, 4B60ACA0252EC0B100E8D219 /* FullScreenVideoUserScript.swift */, 85BDC3182436161C0053DB07 /* LoginFormDetectionUserScript.swift */, + 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */, + 85C503FA2CF0ADBD0075DF6F /* ObservationsDataClearing.swift */, 4B75EA9126A266CB00018634 /* PrintingUserScript.swift */, 988F3DCE237D5C0F00AEE34C /* SchemeHandler.swift */, 836A941C247F23C600BF8EF5 /* UserAgentManager.swift */, F1A886771F29394E0096251E /* WebCacheManager.swift */, + 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */, 83004E7F2193BB8200DA013C /* WKNavigationExtension.swift */, 830381BF1F850AAF00863075 /* WKWebViewConfigurationExtension.swift */, ); @@ -8420,7 +8420,7 @@ 1D8F727F2BA86D8000E31493 /* PixelExperiment.swift in Sources */, 56D8556C2BEA91C4009F9698 /* SyncAlertsPresenting.swift in Sources */, 37FD780F2A29E28B00B36DB1 /* SyncErrorHandler.swift in Sources */, - 85C503FB2CF0ADCE0075DF6F /* WebCacheManager+ObservationsDataClearing.swift in Sources */, + 85C503FB2CF0ADCE0075DF6F /* ObservationsDataClearing.swift in Sources */, 85F21DC621145DD5002631A6 /* global.swift in Sources */, 372A0FF02B2389590033BF7F /* SyncMetricsEventsHandler.swift in Sources */, F41C2DA326C1925700F9A760 /* BookmarksAndFolders.xcdatamodeld in Sources */, diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index c33d45f800..30fa177122 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -31,6 +31,7 @@ class WebCacheManagerTests: XCTestCase { lazy var fireproofing = MockFireproofing() lazy var dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) let dataStoreCleaner = MockDataStoreCleaner() + let observationsCleaner = MockObservationsCleaner() override func setUp() { super.setUp() @@ -77,7 +78,6 @@ class WebCacheManagerTests: XCTestCase { XCTAssertTrue(cookies.contains(where: { $0.domain == ".example.com" })) } - @available(iOS 17, *) func test_WhenClearingDataAfterUsingContainer_ThenCookiesAreMigratedAndOldContainersAreRemoved() async { // Mock having a single container so we can validate cleaning it gets called dataStoreCleaner.countContainersReturnValue = 1 @@ -110,7 +110,6 @@ class WebCacheManagerTests: XCTestCase { XCTAssertEqual(1, dataStoreCleaner.removeAllContainersAfterDelayCalls[0]) } - @available(iOS 17, *) func test_WhenClearingData_ThenOldContainersAreRemoved() async { // Mock existence of 5 containers so we can validate that cleaning it is called even without migrations dataStoreCleaner.countContainersReturnValue = 5 @@ -119,11 +118,10 @@ class WebCacheManagerTests: XCTestCase { XCTAssertEqual(5, dataStoreCleaner.removeAllContainersAfterDelayCalls[0]) } - /// Temporarily disabled. - @MainActor - func x_testWhenAccessingObservationsDbThenValidDatabasePoolIsReturned() { - let pool = makeWebCacheManager().getValidDatabasePool() - XCTAssertNotNil(pool, "DatabasePool should not be nil") + func test_WhenClearingData_ThenObservationsDatabaseIsCleared() async { + XCTAssertEqual(0, observationsCleaner.removeObservationsDataCallCount) + await makeWebCacheManager().clear(dataStore: .default()) + XCTAssertEqual(1, observationsCleaner.removeObservationsDataCallCount) } @MainActor @@ -132,7 +130,8 @@ class WebCacheManagerTests: XCTestCase { cookieStorage: cookieStorage, fireproofing: fireproofing, dataStoreIdManager: dataStoreIdManager, - dataStoreCleaner: dataStoreCleaner + dataStoreCleaner: dataStoreCleaner, + observationsCleaner: observationsCleaner ) } } @@ -151,3 +150,13 @@ class MockDataStoreCleaner: WebsiteDataStoreCleaning { } } + +class MockObservationsCleaner: ObservationsDataCleaning { + + var removeObservationsDataCallCount = 0 + + func removeObservationsData() async { + removeObservationsDataCallCount += 1 + } + +} From 7ea99d13b73523e250d014b33adf3e6c3955f3e7 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Tue, 26 Nov 2024 17:50:23 +0000 Subject: [PATCH 09/14] re-use domain cookie matching code to fix long-standing bug --- Core/Fireproofing.swift | 5 +- Core/HTTPCookieExtension.swift | 30 ++++++++++ Core/MigratableCookieStorage.swift | 29 ++++++--- ...g.swift => ObservationsDataCleaning.swift} | 0 Core/WebCacheManager.swift | 12 +--- DuckDuckGo.xcodeproj/project.pbxproj | 12 ++-- DuckDuckGoTests/CookieStorageTests.swift | 15 +++-- DuckDuckGoTests/WebCacheManagerTests.swift | 59 +++++++++++++++---- 8 files changed, 117 insertions(+), 45 deletions(-) create mode 100644 Core/HTTPCookieExtension.swift rename Core/{ObservationsDataClearing.swift => ObservationsDataCleaning.swift} (100%) diff --git a/Core/Fireproofing.swift b/Core/Fireproofing.swift index 3fcb5e4e75..0a56d7d54a 100644 --- a/Core/Fireproofing.swift +++ b/Core/Fireproofing.swift @@ -66,10 +66,7 @@ public class UserDefaultsFireproofing: Fireproofing { } public func isAllowed(cookieDomain: String) -> Bool { - - return allowedDomainsIncludingDuckDuckGo.contains(where: { $0 == cookieDomain - || ".\($0)" == cookieDomain - || (cookieDomain.hasPrefix(".") && $0.hasSuffix(cookieDomain)) }) + return allowedDomainsIncludingDuckDuckGo.contains(where: { HTTPCookie.cookieDomain(cookieDomain, matchesTestDomain: $0) }) } public func remove(domain: String) { diff --git a/Core/HTTPCookieExtension.swift b/Core/HTTPCookieExtension.swift new file mode 100644 index 0000000000..c6f7fa445f --- /dev/null +++ b/Core/HTTPCookieExtension.swift @@ -0,0 +1,30 @@ +// +// HTTPCookieExtension.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import WebKit + +extension HTTPCookie { + + static func cookieDomain(_ cookieDomain: String, matchesTestDomain testDomain: String) -> Bool { + return testDomain == cookieDomain + || ".\(testDomain)" == cookieDomain + || (cookieDomain.hasPrefix(".") && testDomain.hasSuffix(cookieDomain)) + } + +} diff --git a/Core/MigratableCookieStorage.swift b/Core/MigratableCookieStorage.swift index 1c9e07b537..b850f4173e 100644 --- a/Core/MigratableCookieStorage.swift +++ b/Core/MigratableCookieStorage.swift @@ -20,6 +20,7 @@ import Common import Foundation import os.log +import Persistence /// This class was used to store cookies when moving between persistences containers, but now we are clearing the default container, this class is only used for storing cookies that existed previously and need to be migrated. public class MigratableCookieStorage { @@ -29,7 +30,7 @@ public class MigratableCookieStorage { static let consumed = "com.duckduckgo.consumedCookies" } - private var userDefaults: UserDefaults + private var keyValueStore: KeyValueStoring var isConsumed: Bool { // The default is now true because if this key does not exist then there are no @@ -38,12 +39,12 @@ public class MigratableCookieStorage { // are cookies from a previous version of the app which still need to be migrated. This // could happen if the user hit the fire button and then an update happened // before they browsed again. - return userDefaults.bool(forKey: Keys.consumed, defaultValue: true) + return (keyValueStore.object(forKey: Keys.consumed) as? Bool) ?? true } var cookies: [HTTPCookie] { var storedCookies = [HTTPCookie]() - if let cookies = userDefaults.object(forKey: Keys.allowedCookies) as? [[String: Any?]] { + if let cookies = keyValueStore.object(forKey: Keys.allowedCookies) as? [[String: Any?]] { for cookieData in cookies { var properties = [HTTPCookiePropertyKey: Any]() cookieData.forEach({ @@ -56,18 +57,30 @@ public class MigratableCookieStorage { } } } - return storedCookies } - public init(userDefaults: UserDefaults = UserDefaults.app) { - self.userDefaults = userDefaults + public init(store: KeyValueStoring = UserDefaults.app) { + self.keyValueStore = store + } + + /// Called after consuming cookies but migration has not happened. + /// This can happen in the following flow + /// * User presses fire button on container based app and then backgrounds it/switches to another app + /// * Installs update to this version of the app without using the browser + /// * Starts browsing + func setConsumed() { + resetStore() } /// Called when migration is completed to clean up func migrationComplete() { - userDefaults.removeObject(forKey: Keys.allowedCookies) - userDefaults.removeObject(forKey: Keys.consumed) + resetStore() + } + + private func resetStore() { + keyValueStore.removeObject(forKey: Keys.allowedCookies) + keyValueStore.removeObject(forKey: Keys.consumed) } } diff --git a/Core/ObservationsDataClearing.swift b/Core/ObservationsDataCleaning.swift similarity index 100% rename from Core/ObservationsDataClearing.swift rename to Core/ObservationsDataCleaning.swift diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 1b9f3f85c3..36ec80ddbe 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -33,14 +33,6 @@ extension WKWebsiteDataStore { } -extension HTTPCookie { - - func matchesDomain(_ domain: String) -> Bool { - return self.domain == domain || (self.domain.hasPrefix(".") && domain.hasSuffix(self.domain)) - } - -} - public protocol WebsiteDataManaging { func removeCookies(forDomains domains: [String], fromDataStore: WKWebsiteDataStore) async @@ -118,6 +110,8 @@ public class WebCacheManager: WebsiteDataManaging { consumedCookiesCount += 1 await httpCookieStore.setCookie(cookie) } + + cookieStorage.setConsumed() } public func removeCookies(forDomains domains: [String], @@ -125,7 +119,7 @@ public class WebCacheManager: WebsiteDataManaging { let startTime = CACurrentMediaTime() let cookieStore = dataStore.httpCookieStore let cookies = await cookieStore.allCookies() - for cookie in cookies where domains.contains(where: { cookie.matchesDomain($0) }) { + for cookie in cookies where domains.contains(where: { HTTPCookie.cookieDomain(cookie.domain, matchesTestDomain: $0) }) { await cookieStore.deleteCookie(cookie) } let totalTime = CACurrentMediaTime() - startTime diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 16722665d0..f164004c69 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -546,6 +546,7 @@ 85A313972028E78A00327D00 /* release_notes.txt in Resources */ = {isa = PBXBuildFile; fileRef = 85A313962028E78A00327D00 /* release_notes.txt */; }; 85A9C37920E0E00C00073340 /* HomeRow.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85A9C37820E0E00C00073340 /* HomeRow.xcassets */; }; 85AB84C12CF5DDB4007E679F /* WebsiteDataStoreCleaning.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */; }; + 85AB84C32CF624D8007E679F /* HTTPCookieExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AB84C22CF624CE007E679F /* HTTPCookieExtension.swift */; }; 85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */; }; 85AE668E2097206E0014CF04 /* NotificationView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 85AE668D2097206E0014CF04 /* NotificationView.xib */; }; 85AE6690209724120014CF04 /* NotificationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AE668F209724120014CF04 /* NotificationView.swift */; }; @@ -567,7 +568,7 @@ 85C2970A247EB7AA0063A335 /* Text.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85C29709247EB7AA0063A335 /* Text.xcassets */; }; 85C2971A248162CA0063A335 /* DaxOnboardingPadViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */; }; 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */; }; - 85C503FB2CF0ADCE0075DF6F /* ObservationsDataClearing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* ObservationsDataClearing.swift */; }; + 85C503FB2CF0ADCE0075DF6F /* ObservationsDataCleaning.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* ObservationsDataCleaning.swift */; }; 85C503FD2CF0E7B10075DF6F /* MockFireproofing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */; }; 85C503FF2CF0F4DC0075DF6F /* MockWebsiteDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */; }; 85C8E61D2B0E47380029A6BD /* BookmarksDatabaseSetup.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */; }; @@ -1865,6 +1866,7 @@ 85A53EC9200D1FA20010D13F /* FileStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileStore.swift; sourceTree = ""; }; 85A9C37820E0E00C00073340 /* HomeRow.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = HomeRow.xcassets; sourceTree = ""; }; 85AB84C02CF5DDAF007E679F /* WebsiteDataStoreCleaning.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebsiteDataStoreCleaning.swift; sourceTree = ""; }; + 85AB84C22CF624CE007E679F /* HTTPCookieExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPCookieExtension.swift; sourceTree = ""; }; 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CookieStorageTests.swift; sourceTree = ""; }; 85AE668D2097206E0014CF04 /* NotificationView.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = NotificationView.xib; sourceTree = ""; }; 85AE668F209724120014CF04 /* NotificationView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationView.swift; sourceTree = ""; }; @@ -1887,7 +1889,7 @@ 85C29709247EB7AA0063A335 /* Text.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Text.xcassets; sourceTree = ""; }; 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxOnboardingPadViewController.swift; sourceTree = ""; }; 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIdManager.swift; sourceTree = ""; }; - 85C503FA2CF0ADBD0075DF6F /* ObservationsDataClearing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ObservationsDataClearing.swift; sourceTree = ""; }; + 85C503FA2CF0ADBD0075DF6F /* ObservationsDataCleaning.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ObservationsDataCleaning.swift; sourceTree = ""; }; 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockFireproofing.swift; sourceTree = ""; }; 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockWebsiteDataManager.swift; sourceTree = ""; }; 85C8E61C2B0E47380029A6BD /* BookmarksDatabaseSetup.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksDatabaseSetup.swift; sourceTree = ""; }; @@ -6031,6 +6033,7 @@ F143C3311E4A9A6A00CFDE3A /* Web */ = { isa = PBXGroup; children = ( + 85AB84C22CF624CE007E679F /* HTTPCookieExtension.swift */, 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */, 8596C30C2B7EB1800058EF90 /* DataStoreWarmup.swift */, 85BDC3132434D8F80053DB07 /* DebugUserScript.swift */, @@ -6038,7 +6041,7 @@ 4B60ACA0252EC0B100E8D219 /* FullScreenVideoUserScript.swift */, 85BDC3182436161C0053DB07 /* LoginFormDetectionUserScript.swift */, 85A1B3B120C6CD9900C18F15 /* MigratableCookieStorage.swift */, - 85C503FA2CF0ADBD0075DF6F /* ObservationsDataClearing.swift */, + 85C503FA2CF0ADBD0075DF6F /* ObservationsDataCleaning.swift */, 4B75EA9126A266CB00018634 /* PrintingUserScript.swift */, 988F3DCE237D5C0F00AEE34C /* SchemeHandler.swift */, 836A941C247F23C600BF8EF5 /* UserAgentManager.swift */, @@ -8420,7 +8423,7 @@ 1D8F727F2BA86D8000E31493 /* PixelExperiment.swift in Sources */, 56D8556C2BEA91C4009F9698 /* SyncAlertsPresenting.swift in Sources */, 37FD780F2A29E28B00B36DB1 /* SyncErrorHandler.swift in Sources */, - 85C503FB2CF0ADCE0075DF6F /* ObservationsDataClearing.swift in Sources */, + 85C503FB2CF0ADCE0075DF6F /* ObservationsDataCleaning.swift in Sources */, 85F21DC621145DD5002631A6 /* global.swift in Sources */, 372A0FF02B2389590033BF7F /* SyncMetricsEventsHandler.swift in Sources */, F41C2DA326C1925700F9A760 /* BookmarksAndFolders.xcdatamodeld in Sources */, @@ -8453,6 +8456,7 @@ B652DF0D287C2A6300C12A9C /* PrivacyFeatures.swift in Sources */, F10E522D1E946F8800CE1253 /* NSAttributedStringExtension.swift in Sources */, 9F678B892C9BAA4800CA0E19 /* Debouncer.swift in Sources */, + 85AB84C32CF624D8007E679F /* HTTPCookieExtension.swift in Sources */, 9887DC252354D2AA005C85F5 /* Database.swift in Sources */, F143C3171E4A99D200CFDE3A /* AppURLs.swift in Sources */, C1963863283794A000298D4D /* BookmarksCachingSearch.swift in Sources */, diff --git a/DuckDuckGoTests/CookieStorageTests.swift b/DuckDuckGoTests/CookieStorageTests.swift index 5c0b3d3666..62d56e30aa 100644 --- a/DuckDuckGoTests/CookieStorageTests.swift +++ b/DuckDuckGoTests/CookieStorageTests.swift @@ -20,20 +20,20 @@ import XCTest @testable import Core import WebKit +import Persistence +import TestUtils public class CookieStorageTests: XCTestCase { func testLoadCookiesFromDefaultsAndRemovalWhenMigrationCompletes() { - let defaults = UserDefaults(suiteName: "test")! - defaults.removeSuite(named: "test") - + let store = MockKeyValueStore() MigratableCookieStorage.addCookies([ .make(name: "test1", value: "value1", domain: "example.com"), .make(name: "test2", value: "value2", domain: "example.com"), .make(name: "test3", value: "value3", domain: "facebook.com"), - ], defaults) + ], store) - let storage = MigratableCookieStorage(userDefaults: defaults) + let storage = MigratableCookieStorage(store: store) XCTAssertEqual(storage.cookies.count, 3) XCTAssertTrue(storage.cookies.contains(where: { @@ -65,7 +65,7 @@ public class CookieStorageTests: XCTestCase { extension MigratableCookieStorage { - static func addCookies(_ cookies: [HTTPCookie], _ defaults: UserDefaults) { + static func addCookies(_ cookies: [HTTPCookie], _ store: KeyValueStoring) { var cookieData = [[String: Any?]]() cookies.forEach { cookie in @@ -75,8 +75,7 @@ extension MigratableCookieStorage { } cookieData.append(mappedCookie) } - defaults.setValue(cookieData, forKey: MigratableCookieStorage.Keys.allowedCookies) - + store.set(cookieData, forKey: MigratableCookieStorage.Keys.allowedCookies) } } diff --git a/DuckDuckGoTests/WebCacheManagerTests.swift b/DuckDuckGoTests/WebCacheManagerTests.swift index 30fa177122..9196525579 100644 --- a/DuckDuckGoTests/WebCacheManagerTests.swift +++ b/DuckDuckGoTests/WebCacheManagerTests.swift @@ -24,20 +24,14 @@ import TestUtils class WebCacheManagerTests: XCTestCase { - let defaults = UserDefaults(suiteName: "Test")! - let dataStoreIdStore = MockKeyValueStore() + let keyValueStore = MockKeyValueStore() - lazy var cookieStorage = MigratableCookieStorage(userDefaults: defaults) + lazy var cookieStorage = MigratableCookieStorage(store: keyValueStore) lazy var fireproofing = MockFireproofing() - lazy var dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) + lazy var dataStoreIdManager = DataStoreIdManager(store: keyValueStore) let dataStoreCleaner = MockDataStoreCleaner() let observationsCleaner = MockObservationsCleaner() - override func setUp() { - super.setUp() - defaults.removePersistentDomain(forName: "Test") - } - func test_whenNewInstall_ThenUsesDefaultPersistence() async { let dataStore = await WKWebsiteDataStore.current() let defaultStore = await WKWebsiteDataStore.default() @@ -83,8 +77,8 @@ class WebCacheManagerTests: XCTestCase { dataStoreCleaner.countContainersReturnValue = 1 // Mock a data store id to force migration to happen - dataStoreIdStore.store = [DataStoreIdManager.Constants.currentWebContainerId.rawValue: UUID().uuidString] - dataStoreIdManager = DataStoreIdManager(store: dataStoreIdStore) + keyValueStore.store = [DataStoreIdManager.Constants.currentWebContainerId.rawValue: UUID().uuidString] + dataStoreIdManager = DataStoreIdManager(store: keyValueStore) fireproofing = MockFireproofing(domains: ["example.com"]) @@ -92,7 +86,7 @@ class WebCacheManagerTests: XCTestCase { .make(name: "Test1", value: "Value", domain: "example.com"), .make(name: "Test2", value: "Value", domain: ".example.com"), .make(name: "Test3", value: "Value", domain: "facebook.com"), - ], defaults) + ], keyValueStore) let dataStore = await WKWebsiteDataStore.default() var cookies = await dataStore.httpCookieStore.allCookies() @@ -124,6 +118,47 @@ class WebCacheManagerTests: XCTestCase { XCTAssertEqual(1, observationsCleaner.removeObservationsDataCallCount) } + func test_WhenCookiesAreFromPreviousAppWithContainers_ThenTheyAreConsumed() async { + + MigratableCookieStorage.addCookies([ + .make(name: "Test1", value: "Value", domain: "example.com"), + .make(name: "Test2", value: "Value", domain: ".example.com"), + .make(name: "Test3", value: "Value", domain: "facebook.com"), + ], keyValueStore) + + keyValueStore.set(false, forKey: MigratableCookieStorage.Keys.consumed) + + cookieStorage = MigratableCookieStorage(store: keyValueStore) + + let dataStore = await WKWebsiteDataStore.default() + let httpCookieStore = await dataStore.httpCookieStore + await makeWebCacheManager().consumeCookies(intoHTTPCookieStore: httpCookieStore) + + XCTAssertTrue(self.cookieStorage.isConsumed) + XCTAssertTrue(self.cookieStorage.cookies.isEmpty) + + let cookies = await httpCookieStore.allCookies() + XCTAssertEqual(3, cookies.count) + } + + func test_WhenRemoveCookiesForDomains_ThenUnaffectedLeftBehind() async { + let dataStore = await WKWebsiteDataStore.default() + await dataStore.httpCookieStore.setCookie(.make(name: "Test1", value: "Value", domain: "example.com")) + await dataStore.httpCookieStore.setCookie(.make(name: "Test4", value: "Value", domain: "sample.com")) + await dataStore.httpCookieStore.setCookie(.make(name: "Test2", value: "Value", domain: ".example.com")) + await dataStore.httpCookieStore.setCookie(.make(name: "Test3", value: "Value", domain: "facebook.com")) + + var cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(4, cookies.count) + + let webCacheManager = await makeWebCacheManager() + await webCacheManager.removeCookies(forDomains: ["example.com", "sample.com"], fromDataStore: dataStore) + + cookies = await dataStore.httpCookieStore.allCookies() + XCTAssertEqual(1, cookies.count) + XCTAssertTrue(cookies.contains(where: { $0.domain == "facebook.com" })) + } + @MainActor private func makeWebCacheManager() -> WebCacheManager { return WebCacheManager( From b3e58e1c0659af5897c939a0cd01e47b8057fa6d Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Thu, 28 Nov 2024 22:01:09 +0000 Subject: [PATCH 10/14] containers need to be removed on the main actor --- Core/WebCacheManager.swift | 6 +++--- Core/WebsiteDataStoreCleaning.swift | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 36ec80ddbe..2a60f12969 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -131,7 +131,7 @@ public class WebCacheManager: WebsiteDataManaging { let count = await dataStoreCleaner.countContainers() await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, cookieStorage: cookieStorage, destinationStore: dataStore) await clearData(inDataStore: dataStore, withFireproofing: fireproofing) - dataStoreCleaner.removeAllContainersAfterDelay(previousCount: count) + await dataStoreCleaner.removeAllContainersAfterDelay(previousCount: count) } @@ -161,8 +161,8 @@ extension WebCacheManager { dataStoreIdManager.invalidateCurrentId() } - private func removeContainersIfNeeded(previousCount: Int) { - dataStoreCleaner.removeAllContainersAfterDelay(previousCount: previousCount) + private func removeContainersIfNeeded(previousCount: Int) async { + await dataStoreCleaner.removeAllContainersAfterDelay(previousCount: previousCount) } private func clearData(inDataStore dataStore: WKWebsiteDataStore, withFireproofing fireproofing: Fireproofing) async { diff --git a/Core/WebsiteDataStoreCleaning.swift b/Core/WebsiteDataStoreCleaning.swift index 7b4fbef1e8..08f0fa26ed 100644 --- a/Core/WebsiteDataStoreCleaning.swift +++ b/Core/WebsiteDataStoreCleaning.swift @@ -22,7 +22,7 @@ import WebKit public protocol WebsiteDataStoreCleaning { func countContainers() async -> Int - func removeAllContainersAfterDelay(previousCount: Int) + func removeAllContainersAfterDelay(previousCount: Int) async } @@ -30,18 +30,22 @@ public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { public init() { } + @MainActor public func countContainers() async -> Int { guard #available(iOS 17, *) else { return 0 } + print("***", #function) return await WKWebsiteDataStore.allDataStoreIdentifiers.count } - public func removeAllContainersAfterDelay(previousCount: Int) { + @MainActor + public func removeAllContainersAfterDelay(previousCount: Int) async { guard #available(iOS 17, *) else { return } // Attempt to clean up all previous stores, but wait for a few seconds. // If this fails, we are going to still clean them next time as WebKit keeps track of all stores for us. Task { try? await Task.sleep(interval: 3.0) + print("***", #function) for uuid in await WKWebsiteDataStore.allDataStoreIdentifiers { try? await WKWebsiteDataStore.remove(forIdentifier: uuid) } @@ -50,6 +54,7 @@ public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { } } + @MainActor private func checkForLeftBehindDataStores(previousLeftOversCount: Int) async { guard #available(iOS 17, *) else { return } @@ -57,6 +62,7 @@ public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { "left_overs_count": "\(previousLeftOversCount)" ] + print("***", #function) let ids = await WKWebsiteDataStore.allDataStoreIdentifiers if ids.count > 1 { Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedMultiple, withAdditionalParameters: params) From 04545522dcd54288dfda93664e777d3f69743c1e Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Thu, 28 Nov 2024 22:51:51 +0000 Subject: [PATCH 11/14] remove print statements --- Core/WebsiteDataStoreCleaning.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/Core/WebsiteDataStoreCleaning.swift b/Core/WebsiteDataStoreCleaning.swift index 08f0fa26ed..d2b3d472a9 100644 --- a/Core/WebsiteDataStoreCleaning.swift +++ b/Core/WebsiteDataStoreCleaning.swift @@ -33,7 +33,6 @@ public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { @MainActor public func countContainers() async -> Int { guard #available(iOS 17, *) else { return 0 } - print("***", #function) return await WKWebsiteDataStore.allDataStoreIdentifiers.count } @@ -45,7 +44,6 @@ public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { // If this fails, we are going to still clean them next time as WebKit keeps track of all stores for us. Task { try? await Task.sleep(interval: 3.0) - print("***", #function) for uuid in await WKWebsiteDataStore.allDataStoreIdentifiers { try? await WKWebsiteDataStore.remove(forIdentifier: uuid) } @@ -62,7 +60,6 @@ public class DefaultWebsiteDataStoreCleaner: WebsiteDataStoreCleaning { "left_overs_count": "\(previousLeftOversCount)" ] - print("***", #function) let ids = await WKWebsiteDataStore.allDataStoreIdentifiers if ids.count > 1 { Pixel.fire(pixel: .debugWebsiteDataStoresNotClearedMultiple, withAdditionalParameters: params) From f54fb192e931d53782e0992d57bf70ef468c15d8 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Thu, 28 Nov 2024 23:40:38 +0000 Subject: [PATCH 12/14] update maestro and add local storage test --- .maestro/release_tests/local-storage.yaml | 72 +++++++++++++++++++++++ .maestro/setup_ui_tests.sh | 2 +- 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 .maestro/release_tests/local-storage.yaml diff --git a/.maestro/release_tests/local-storage.yaml b/.maestro/release_tests/local-storage.yaml new file mode 100644 index 0000000000..715c0d2c21 --- /dev/null +++ b/.maestro/release_tests/local-storage.yaml @@ -0,0 +1,72 @@ +# local-storage.yaml +appId: com.duckduckgo.mobile.ios +tags: + - release + +--- + +# Set up +- runFlow: + file: ../shared/setup.yaml + +# Load Site +- assertVisible: + id: "searchEntry" +- tapOn: + id: "searchEntry" +- inputText: "https://privacy-test-pages.site/features/local-storage.html" +- pressKey: Enter + +# Add a cookie +- assertVisible: "Storage Counter: undefined" +- assertVisible: "Cookie Counter:" +- assertNotVisible: "Cookie Counter: 1" +- assertNotVisible: "Storage Counter: 1" +- assertVisible: "Manual Increment" +- tapOn: "Manual Increment" +- tapOn: "Manual Increment" +- assertVisible: "Cookie Counter: 2" +- assertVisible: "Storage Counter: 2" + +# Fireproofing +- tapOn: "Browsing menu" +- tapOn: "Fireproof This Site" +- tapOn: "Fireproof" + +# Fire button +- tapOn: "Close Tabs and Clear Data" +- tapOn: "Close Tabs and Clear Data" + +- assertNotVisible: "https://privacy-test-pages.site/features/local-storage.html" + +# Load Site +- assertVisible: + id: "searchEntry" +- tapOn: + id: "searchEntry" + +- inputText: "https://privacy-test-pages.site/features/local-storage.html" +- pressKey: Enter +- assertVisible: "Storage Counter: 2" +- assertVisible: "Cookie Counter: 2" + +# Remove Fireproofing +- tapOn: "Browsing menu" +- tapOn: "Remove Fireproofing" + +# Fire button +- tapOn: "Close Tabs and Clear Data" +- tapOn: "Close Tabs and Clear Data" + +- assertNotVisible: "https://privacy-test-pages.site/features/local-storage.html" + +# Load Site +- assertVisible: + id: "searchEntry" +- tapOn: + id: "searchEntry" + +- inputText: "https://privacy-test-pages.site/features/local-storage.html" +- pressKey: Enter +- assertVisible: "Storage Counter: undefined" +- assertVisible: "Cookie Counter:" diff --git a/.maestro/setup_ui_tests.sh b/.maestro/setup_ui_tests.sh index 2e2c0d3014..8da1584978 100755 --- a/.maestro/setup_ui_tests.sh +++ b/.maestro/setup_ui_tests.sh @@ -15,7 +15,7 @@ target_os="iOS-18-1" check_maestro() { local command_name="maestro" - local known_version="1.39.1" + local known_version="1.39.2" if command -v $command_name > /dev/null 2>&1; then local version_output=$($command_name -v 2>&1 | tail -n 1) From 42b70634c7f6e7f9f4487d8fd330c594c9d953b6 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Mon, 2 Dec 2024 16:46:34 +0000 Subject: [PATCH 13/14] address feedback comments --- ...Manager.swift => DataStoreIDManager.swift} | 22 ++++++------- Core/Fireproofing.swift | 2 +- Core/HTTPCookieExtension.swift | 5 +++ Core/WKWebViewConfigurationExtension.swift | 4 +-- Core/WebCacheManager.swift | 25 ++++++++------- DuckDuckGo.xcodeproj/project.pbxproj | 16 +++++----- DuckDuckGo/AppDelegate.swift | 4 +-- DuckDuckGo/TabViewController.swift | 2 +- .../FireButtonReferenceTests.swift | 2 +- ...ts.swift => DataStoreIDManagerTests.swift} | 32 +++++++++---------- WebViewUnitTests/WebCacheManagerTests.swift | 10 +++--- 11 files changed, 65 insertions(+), 59 deletions(-) rename Core/{DataStoreIdManager.swift => DataStoreIDManager.swift} (75%) rename WebViewUnitTests/{DataStoreIdManagerTests.swift => DataStoreIDManagerTests.swift} (50%) diff --git a/Core/DataStoreIdManager.swift b/Core/DataStoreIDManager.swift similarity index 75% rename from Core/DataStoreIdManager.swift rename to Core/DataStoreIDManager.swift index a033d0b155..3c9709c64b 100644 --- a/Core/DataStoreIdManager.swift +++ b/Core/DataStoreIDManager.swift @@ -1,5 +1,5 @@ // -// DataStoreIdManager.swift +// DataStoreIDManager.swift // DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. @@ -23,35 +23,35 @@ import Persistence /// Supports an existing ID set in previous versions of the app, but moving forward does not allocate an ID. We have gone back to using the default /// peristence for the webview storage so that we can fireproof types that don't have an API for accessing their data. (e.g. localStorage) -public protocol DataStoreIdManaging { +public protocol DataStoreIDManaging { - var currentId: UUID? { get } + var currentID: UUID? { get } - func invalidateCurrentId() + func invalidateCurrentID() } -public class DataStoreIdManager: DataStoreIdManaging { +public class DataStoreIDManager: DataStoreIDManaging { enum Constants: String { - case currentWebContainerId = "com.duckduckgo.ios.webcontainer.id" + case currentWebContainerID = "com.duckduckgo.ios.webcontainer.id" } - public static let shared = DataStoreIdManager() + public static let shared = DataStoreIDManager() private let store: KeyValueStoring init(store: KeyValueStoring = UserDefaults.app) { self.store = store } - public var currentId: UUID? { - guard let uuidString = store.object(forKey: Constants.currentWebContainerId.rawValue) as? String else { + public var currentID: UUID? { + guard let uuidString = store.object(forKey: Constants.currentWebContainerID.rawValue) as? String else { return nil } return UUID(uuidString: uuidString) } - public func invalidateCurrentId() { - store.removeObject(forKey: Constants.currentWebContainerId.rawValue) + public func invalidateCurrentID() { + store.removeObject(forKey: Constants.currentWebContainerID.rawValue) } } diff --git a/Core/Fireproofing.swift b/Core/Fireproofing.swift index 0a56d7d54a..d8f515bbb9 100644 --- a/Core/Fireproofing.swift +++ b/Core/Fireproofing.swift @@ -54,7 +54,7 @@ public class UserDefaultsFireproofing: Fireproofing { } } - var allowedDomainsIncludingDuckDuckGo: [String] { + private var allowedDomainsIncludingDuckDuckGo: [String] { allowedDomains + [ URL.ddg.host ?? "", SubscriptionCookieManager.cookieDomain diff --git a/Core/HTTPCookieExtension.swift b/Core/HTTPCookieExtension.swift index c6f7fa445f..5475c54b43 100644 --- a/Core/HTTPCookieExtension.swift +++ b/Core/HTTPCookieExtension.swift @@ -21,6 +21,11 @@ import WebKit extension HTTPCookie { + /// Checks that the `cookieDomain` (provided by a cookie) matches a given domain. e.g. + /// * cookie domain example.com would match a domain called example.com + /// * cookie domain `.example.com` would also match a domain called example.com and also any subdomain, e.g. `docs.example.com` + /// + /// See `UserDefaultsFireproofingTests` for more examples. static func cookieDomain(_ cookieDomain: String, matchesTestDomain testDomain: String) -> Bool { return testDomain == cookieDomain || ".\(testDomain)" == cookieDomain diff --git a/Core/WKWebViewConfigurationExtension.swift b/Core/WKWebViewConfigurationExtension.swift index 810b08f6a5..be557a68be 100644 --- a/Core/WKWebViewConfigurationExtension.swift +++ b/Core/WKWebViewConfigurationExtension.swift @@ -23,11 +23,11 @@ import Persistence extension WKWebViewConfiguration { @MainActor - public static func persistent(idManager: DataStoreIdManaging = DataStoreIdManager.shared) -> WKWebViewConfiguration { + public static func persistent(idManager: DataStoreIDManaging = DataStoreIDManager.shared) -> WKWebViewConfiguration { let config = configuration(persistsData: true) // Only use a container if there's an id. We no longer allocate ids so this should not happen. - if #available(iOS 17, *), let containerId = idManager.currentId { + if #available(iOS 17, *), let containerId = idManager.currentID { config.websiteDataStore = WKWebsiteDataStore(forIdentifier: containerId) } return config diff --git a/Core/WebCacheManager.swift b/Core/WebCacheManager.swift index 2a60f12969..7e1af854dd 100644 --- a/Core/WebCacheManager.swift +++ b/Core/WebCacheManager.swift @@ -23,8 +23,8 @@ import os.log extension WKWebsiteDataStore { - public static func current(dataStoreIdManager: DataStoreIdManaging = DataStoreIdManager.shared) -> WKWebsiteDataStore { - if #available(iOS 17, *), let id = dataStoreIdManager.currentId { + public static func current(dataStoreIDManager: DataStoreIDManaging = DataStoreIDManager.shared) -> WKWebsiteDataStore { + if #available(iOS 17, *), let id = dataStoreIDManager.currentID { return WKWebsiteDataStore(forIdentifier: id) } else { return WKWebsiteDataStore.default() @@ -36,7 +36,7 @@ extension WKWebsiteDataStore { public protocol WebsiteDataManaging { func removeCookies(forDomains domains: [String], fromDataStore: WKWebsiteDataStore) async - func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async + func consumeCookies(into httpCookieStore: WKHTTPCookieStore) async func clear(dataStore: WKWebsiteDataStore) async } @@ -79,18 +79,18 @@ public class WebCacheManager: WebsiteDataManaging { let cookieStorage: MigratableCookieStorage let fireproofing: Fireproofing - let dataStoreIdManager: DataStoreIdManaging + let dataStoreIDManager: DataStoreIDManaging let dataStoreCleaner: WebsiteDataStoreCleaning let observationsCleaner: ObservationsDataCleaning public init(cookieStorage: MigratableCookieStorage, fireproofing: Fireproofing, - dataStoreIdManager: DataStoreIdManaging, + dataStoreIDManager: DataStoreIDManaging, dataStoreCleaner: WebsiteDataStoreCleaning = DefaultWebsiteDataStoreCleaner(), observationsCleaner: ObservationsDataCleaning = DefaultObservationsDataCleaner()) { self.cookieStorage = cookieStorage self.fireproofing = fireproofing - self.dataStoreIdManager = dataStoreIdManager + self.dataStoreIDManager = dataStoreIDManager self.dataStoreCleaner = dataStoreCleaner self.observationsCleaner = observationsCleaner } @@ -100,7 +100,7 @@ public class WebCacheManager: WebsiteDataManaging { /// /// The migration code removes the key that is used to check for the isConsumed flag so will only be /// true if the data needs to be migrated. - public func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async { + public func consumeCookies(into httpCookieStore: WKHTTPCookieStore) async { // This can only be true if the data has not yet been migrated. guard !cookieStorage.isConsumed else { return } @@ -129,7 +129,7 @@ public class WebCacheManager: WebsiteDataManaging { public func clear(dataStore: WKWebsiteDataStore) async { let count = await dataStoreCleaner.countContainers() - await performMigrationIfNeeded(dataStoreIdManager: dataStoreIdManager, cookieStorage: cookieStorage, destinationStore: dataStore) + await performMigrationIfNeeded(dataStoreIDManager: dataStoreIDManager, cookieStorage: cookieStorage, destinationStore: dataStore) await clearData(inDataStore: dataStore, withFireproofing: fireproofing) await dataStoreCleaner.removeAllContainersAfterDelay(previousCount: count) @@ -139,15 +139,16 @@ public class WebCacheManager: WebsiteDataManaging { extension WebCacheManager { - private func performMigrationIfNeeded(dataStoreIdManager: DataStoreIdManaging, + private func performMigrationIfNeeded(dataStoreIDManager: DataStoreIDManaging, cookieStorage: MigratableCookieStorage, destinationStore: WKWebsiteDataStore) async { - // Check version here rather than on function so that we don't need complicated logic related to verison in the calling function + // Check version here rather than on function so that we don't need complicated logic related to verison in the calling function. + // Also, migration will not be needed if we are on a version lower than this. guard #available(iOS 17, *) else { return } // If there's no id, then migration has been done or isn't needed - guard dataStoreIdManager.currentId != nil else { return } + guard dataStoreIDManager.currentID != nil else { return } // Get all cookies, we'll clean them later to keep all that logic in the same place let cookies = cookieStorage.cookies @@ -158,7 +159,7 @@ extension WebCacheManager { } cookieStorage.migrationComplete() - dataStoreIdManager.invalidateCurrentId() + dataStoreIDManager.invalidateCurrentID() } private func removeContainersIfNeeded(previousCount: Int) async { diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index c93723a10b..b69f6fb6e2 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -565,7 +565,7 @@ 85C29708247BDD060063A335 /* DaxDialogsBrowsingSpecTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29706247BDCFF0063A335 /* DaxDialogsBrowsingSpecTests.swift */; }; 85C2970A247EB7AA0063A335 /* Text.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85C29709247EB7AA0063A335 /* Text.xcassets */; }; 85C2971A248162CA0063A335 /* DaxOnboardingPadViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */; }; - 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */; }; + 85C503F92CEF78C10075DF6F /* DataStoreIDManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503F82CEF78C00075DF6F /* DataStoreIDManager.swift */; }; 85C503FB2CF0ADCE0075DF6F /* ObservationsDataCleaning.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FA2CF0ADBD0075DF6F /* ObservationsDataCleaning.swift */; }; 85C503FD2CF0E7B10075DF6F /* MockFireproofing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */; }; 85C503FF2CF0F4DC0075DF6F /* MockWebsiteDataManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */; }; @@ -654,7 +654,7 @@ 98424A9C2CED4F430071C7DB /* NetworkProtectionTestUtils in Frameworks */ = {isa = PBXBuildFile; productRef = 984249C32CED4F430071C7DB /* NetworkProtectionTestUtils */; }; 98424A9D2CED4F430071C7DB /* Common in Frameworks */ = {isa = PBXBuildFile; productRef = 984249C72CED4F430071C7DB /* Common */; }; 98424AAB2CED4FF10071C7DB /* CookieStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */; }; - 98424AAC2CED4FF10071C7DB /* DataStoreIdManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 981C49AF2C8FA61D00DF11E8 /* DataStoreIdManagerTests.swift */; }; + 98424AAC2CED4FF10071C7DB /* DataStoreIDManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 981C49AF2C8FA61D00DF11E8 /* DataStoreIDManagerTests.swift */; }; 98424AAD2CED4FF10071C7DB /* WKWebViewConfigurationExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F198D7971E3A45D90088DA8A /* WKWebViewConfigurationExtensionTests.swift */; }; 98424AAE2CED4FF10071C7DB /* WebCacheManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 850559D123CF710C0055C0D5 /* WebCacheManagerTests.swift */; }; 98424AAF2CED4FF10071C7DB /* UserAgentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 834DF990248FDDF60075EA48 /* UserAgentTests.swift */; }; @@ -1909,7 +1909,7 @@ 85C29706247BDCFF0063A335 /* DaxDialogsBrowsingSpecTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxDialogsBrowsingSpecTests.swift; sourceTree = ""; }; 85C29709247EB7AA0063A335 /* Text.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Text.xcassets; sourceTree = ""; }; 85C29719248162CA0063A335 /* DaxOnboardingPadViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DaxOnboardingPadViewController.swift; sourceTree = ""; }; - 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIdManager.swift; sourceTree = ""; }; + 85C503F82CEF78C00075DF6F /* DataStoreIDManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIDManager.swift; sourceTree = ""; }; 85C503FA2CF0ADBD0075DF6F /* ObservationsDataCleaning.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ObservationsDataCleaning.swift; sourceTree = ""; }; 85C503FC2CF0E7B10075DF6F /* MockFireproofing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockFireproofing.swift; sourceTree = ""; }; 85C503FE2CF0F4D60075DF6F /* MockWebsiteDataManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockWebsiteDataManager.swift; sourceTree = ""; }; @@ -1993,7 +1993,7 @@ 981685572521EEF600FA91A1 /* nb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nb; path = nb.lproj/MainInterface.strings; sourceTree = ""; }; 981685A825221ACF00FA91A1 /* nb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = nb; path = nb.lproj/Localizable.stringsdict; sourceTree = ""; }; 9817C9C221EF594700884F65 /* AutoClear.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutoClear.swift; sourceTree = ""; }; - 981C49AF2C8FA61D00DF11E8 /* DataStoreIdManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIdManagerTests.swift; sourceTree = ""; }; + 981C49AF2C8FA61D00DF11E8 /* DataStoreIDManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataStoreIDManagerTests.swift; sourceTree = ""; }; 981CA7E92617797500E119D5 /* MainViewController+AddFavoriteFlow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MainViewController+AddFavoriteFlow.swift"; sourceTree = ""; }; 981DCA922521EFAB00CD4C18 /* nb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nb; path = nb.lproj/InfoPlist.strings; sourceTree = ""; }; 981DCA932521EFAB00CD4C18 /* nb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nb; path = nb.lproj/InfoPlist.strings; sourceTree = ""; }; @@ -4797,7 +4797,7 @@ 834DF990248FDDF60075EA48 /* UserAgentTests.swift */, 8540BD5123D8C2220057FDD2 /* UserDefaultsFireproofingTests.swift */, 850559D123CF710C0055C0D5 /* WebCacheManagerTests.swift */, - 981C49AF2C8FA61D00DF11E8 /* DataStoreIdManagerTests.swift */, + 981C49AF2C8FA61D00DF11E8 /* DataStoreIDManagerTests.swift */, F198D7971E3A45D90088DA8A /* WKWebViewConfigurationExtensionTests.swift */, 85AD49ED2B6149110085D2D1 /* CookieStorageTests.swift */, ); @@ -6091,7 +6091,7 @@ isa = PBXGroup; children = ( 85AB84C22CF624CE007E679F /* HTTPCookieExtension.swift */, - 85C503F82CEF78C00075DF6F /* DataStoreIdManager.swift */, + 85C503F82CEF78C00075DF6F /* DataStoreIDManager.swift */, 8596C30C2B7EB1800058EF90 /* DataStoreWarmup.swift */, 85BDC3132434D8F80053DB07 /* DebugUserScript.swift */, 850559CF23CF647C0055C0D5 /* Fireproofing.swift */, @@ -8406,7 +8406,7 @@ buildActionMask = 2147483647; files = ( 98424AAB2CED4FF10071C7DB /* CookieStorageTests.swift in Sources */, - 98424AAC2CED4FF10071C7DB /* DataStoreIdManagerTests.swift in Sources */, + 98424AAC2CED4FF10071C7DB /* DataStoreIDManagerTests.swift in Sources */, 98424AAD2CED4FF10071C7DB /* WKWebViewConfigurationExtensionTests.swift in Sources */, 8567E29D2CF77A700035926F /* MockFireproofing.swift in Sources */, 98424AAE2CED4FF10071C7DB /* WebCacheManagerTests.swift in Sources */, @@ -8496,7 +8496,7 @@ 85528AA72C7CA95D0017BCCA /* UsageSegmentationCalculator.swift in Sources */, 8598D2DC2CEB93AD00C45685 /* FaviconsCacheType.swift in Sources */, 1E6A4D692984208800A371D3 /* LocaleExtension.swift in Sources */, - 85C503F92CEF78C10075DF6F /* DataStoreIdManager.swift in Sources */, + 85C503F92CEF78C10075DF6F /* DataStoreIDManager.swift in Sources */, 98F6EA472863124100720957 /* ContentBlockerRulesLists.swift in Sources */, 566B73732BECE4F200FF1959 /* SyncErrorHandling.swift in Sources */, F1134EB01F40AC6300B73467 /* AtbParser.swift in Sources */, diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 2677ef2fdc..8ef177dd59 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -442,10 +442,10 @@ import os.log } private func makeWebsiteDataManager(fireproofing: Fireproofing, - dataStoreIdManager: DataStoreIdManaging = DataStoreIdManager.shared) -> WebsiteDataManaging { + dataStoreIDManager: DataStoreIDManaging = DataStoreIDManager.shared) -> WebsiteDataManaging { return WebCacheManager(cookieStorage: MigratableCookieStorage(), fireproofing: fireproofing, - dataStoreIdManager: dataStoreIdManager) + dataStoreIDManager: dataStoreIDManager) } private func makeTextZoomCoordinator() -> TextZoomCoordinator { diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index e96f54f302..529f49da2b 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -671,7 +671,7 @@ class TabViewController: UIViewController { Task { @MainActor in await webView.configuration.websiteDataStore.dataRecords(ofTypes: WKWebsiteDataStore.allWebsiteDataTypes()) let cookieStore = webView.configuration.websiteDataStore.httpCookieStore - await websiteDataManager.consumeCookies(intoHTTPCookieStore: cookieStore) + await websiteDataManager.consumeCookies(into: cookieStore) subscriptionCookieManager.resetLastRefreshDate() await subscriptionCookieManager.refreshSubscriptionCookie() doLoad() diff --git a/DuckDuckGoTests/FireButtonReferenceTests.swift b/DuckDuckGoTests/FireButtonReferenceTests.swift index 50a5356911..66d1c32688 100644 --- a/DuckDuckGoTests/FireButtonReferenceTests.swift +++ b/DuckDuckGoTests/FireButtonReferenceTests.swift @@ -75,7 +75,7 @@ final class FireButtonReferenceTests: XCTestCase { let cookieStore = dataStore.httpCookieStore await cookieStore.setCookie(cookie) - await WebCacheManager(cookieStorage: MigratableCookieStorage(), fireproofing: fireproofing, dataStoreIdManager: DataStoreIdManager(store: MockKeyValueStore())).clear(dataStore: dataStore) + await WebCacheManager(cookieStorage: MigratableCookieStorage(), fireproofing: fireproofing, dataStoreIDManager: DataStoreIDManager(store: MockKeyValueStore())).clear(dataStore: dataStore) let testCookie = await cookieStore.allCookies().filter { $0.name == test.cookieName }.first diff --git a/WebViewUnitTests/DataStoreIdManagerTests.swift b/WebViewUnitTests/DataStoreIDManagerTests.swift similarity index 50% rename from WebViewUnitTests/DataStoreIdManagerTests.swift rename to WebViewUnitTests/DataStoreIDManagerTests.swift index d507fb41da..ea32e99b48 100644 --- a/WebViewUnitTests/DataStoreIdManagerTests.swift +++ b/WebViewUnitTests/DataStoreIDManagerTests.swift @@ -1,5 +1,5 @@ // -// DataStoreIdManagerTests.swift +// DataStoreIDManagerTests.swift // DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. @@ -24,28 +24,28 @@ import XCTest import WebKit import TestUtils -class DataStoreIdManagerTests: XCTestCase { +class DataStoreIDManagerTests: XCTestCase { - func testWhenFreshlyInstalledThenNoIdIsAllocated() { - let manager = DataStoreIdManager(store: MockKeyValueStore()) - XCTAssertNil(manager.currentId) + func testWhenFreshlyInstalledThenNoIDIsAllocated() { + let manager = DataStoreIDManager(store: MockKeyValueStore()) + XCTAssertNil(manager.currentID) } - func testWhenIdIsInvalidatedThenNoNewIdIsCreated() { - let manager = DataStoreIdManager(store: MockKeyValueStore()) - XCTAssertNil(manager.currentId) - manager.invalidateCurrentId() - XCTAssertNil(manager.currentId) + func testWhenIDIsInvalidatedThenNoNewIDIsCreated() { + let manager = DataStoreIDManager(store: MockKeyValueStore()) + XCTAssertNil(manager.currentID) + manager.invalidateCurrentID() + XCTAssertNil(manager.currentID) } - func testWhenIdAlreadyExistsThenItIsRedFromTheStore() { + func testWhenIDAlreadyExistsThenItIsRedFromTheStore() { let storedUUID = UUID().uuidString let store = MockKeyValueStore() - store.set(storedUUID, forKey: DataStoreIdManager.Constants.currentWebContainerId.rawValue) - let manager = DataStoreIdManager(store: store) - XCTAssertEqual(manager.currentId?.uuidString, storedUUID) - manager.invalidateCurrentId() - XCTAssertNil(manager.currentId) + store.set(storedUUID, forKey: DataStoreIDManager.Constants.currentWebContainerID.rawValue) + let manager = DataStoreIDManager(store: store) + XCTAssertEqual(manager.currentID?.uuidString, storedUUID) + manager.invalidateCurrentID() + XCTAssertNil(manager.currentID) } } diff --git a/WebViewUnitTests/WebCacheManagerTests.swift b/WebViewUnitTests/WebCacheManagerTests.swift index 5253a4575a..6fa7be86c1 100644 --- a/WebViewUnitTests/WebCacheManagerTests.swift +++ b/WebViewUnitTests/WebCacheManagerTests.swift @@ -51,7 +51,7 @@ class WebCacheManagerTests: XCTestCase { lazy var cookieStorage = MigratableCookieStorage(store: keyValueStore) lazy var fireproofing = MockFireproofing() - lazy var dataStoreIdManager = DataStoreIdManager(store: keyValueStore) + lazy var dataStoreIDManager = DataStoreIDManager(store: keyValueStore) let dataStoreCleaner = MockDataStoreCleaner() let observationsCleaner = MockObservationsCleaner() @@ -100,8 +100,8 @@ class WebCacheManagerTests: XCTestCase { dataStoreCleaner.countContainersReturnValue = 1 // Mock a data store id to force migration to happen - keyValueStore.store = [DataStoreIdManager.Constants.currentWebContainerId.rawValue: UUID().uuidString] - dataStoreIdManager = DataStoreIdManager(store: keyValueStore) + keyValueStore.store = [DataStoreIDManager.Constants.currentWebContainerID.rawValue: UUID().uuidString] + dataStoreIDManager = DataStoreIDManager(store: keyValueStore) fireproofing = MockFireproofing(domains: ["example.com"]) @@ -155,7 +155,7 @@ class WebCacheManagerTests: XCTestCase { let dataStore = await WKWebsiteDataStore.default() let httpCookieStore = await dataStore.httpCookieStore - await makeWebCacheManager().consumeCookies(intoHTTPCookieStore: httpCookieStore) + await makeWebCacheManager().consumeCookies(into: httpCookieStore) XCTAssertTrue(self.cookieStorage.isConsumed) XCTAssertTrue(self.cookieStorage.cookies.isEmpty) @@ -187,7 +187,7 @@ class WebCacheManagerTests: XCTestCase { return WebCacheManager( cookieStorage: cookieStorage, fireproofing: fireproofing, - dataStoreIdManager: dataStoreIdManager, + dataStoreIDManager: dataStoreIDManager, dataStoreCleaner: dataStoreCleaner, observationsCleaner: observationsCleaner ) From ffe34487c4539835a58ec3bce34b8f9155bd58a9 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Mon, 2 Dec 2024 16:52:01 +0000 Subject: [PATCH 14/14] fix api in mock --- DuckDuckGoTests/MockWebsiteDataManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGoTests/MockWebsiteDataManager.swift b/DuckDuckGoTests/MockWebsiteDataManager.swift index 441b28cb76..c64c71f456 100644 --- a/DuckDuckGoTests/MockWebsiteDataManager.swift +++ b/DuckDuckGoTests/MockWebsiteDataManager.swift @@ -26,7 +26,7 @@ class MockWebsiteDataManager: WebsiteDataManaging { func removeCookies(forDomains domains: [String], fromDataStore: WKWebsiteDataStore) async { } - func consumeCookies(intoHTTPCookieStore httpCookieStore: WKHTTPCookieStore) async { + func consumeCookies(into httpCookieStore: WKHTTPCookieStore) async { } func clear(dataStore: WKWebsiteDataStore) async {