Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

clean up clear data code #2449

Merged
merged 24 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6b615e4
use direct and async api for consuming cookies (validated in browser)
brindy Feb 7, 2024
b871bb6
make cookie clearing / fireproofing async
brindy Feb 7, 2024
5e24409
remove (basically unused) protocol
brindy Feb 7, 2024
b8c7672
make more async
brindy Feb 7, 2024
d863a64
Merge branch 'main' into brindy/clean-up-clear-data-code
brindy Feb 7, 2024
f7640fc
fix test
brindy Feb 7, 2024
f8145be
remove unused code
brindy Feb 7, 2024
1f1581f
more clean up
brindy Feb 7, 2024
e29b4ed
tidy up
brindy Feb 7, 2024
8bbdcc5
tidy up
brindy Feb 7, 2024
7a5f0d5
don't try and cancel the task, just fire the pixel when it takes too …
brindy Feb 7, 2024
b76ea46
only update cookies if they have been consumed otherwise they will ge…
brindy Feb 8, 2024
7095d3d
Update the documentation for the CookieStorage class
brindy Feb 8, 2024
07904cf
use legacy container for container tests / reference test and remembe…
brindy Feb 8, 2024
0340f01
add maestro tests to validate fire proofing and ddg cookie retention
brindy Feb 8, 2024
00d9257
Load a webview to ensure the data store is properly available before …
brindy Feb 15, 2024
d361469
move data store loader to share code
brindy Feb 15, 2024
079e409
move things around and fix ui when no animation
brindy Feb 16, 2024
68a6840
remove print statements
brindy Feb 16, 2024
7f08981
fix tests
brindy Feb 16, 2024
33b3a52
Merge branch 'main' into brindy/clean-up-clear-data-code
brindy Feb 16, 2024
3ca935e
address feedback
brindy Feb 19, 2024
36385cf
only clear data when auto clear is on if 'clearData' flag is set
brindy Feb 19, 2024
dd5b376
add a note and fix tests
brindy Feb 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions .maestro/data_clearing_tests/01_fire_proofing.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
appId: com.duckduckgo.mobile.ios
tags:
- dataclearing

---

# Set up
- clearState
- launchApp
- runFlow:
when:
visible:
text: "Let’s Do It!"
index: 0
file: ../shared/onboarding.yaml

# Load Site
- assertVisible:
id: "searchEntry"
- tapOn:
id: "searchEntry"
- inputText: "https://setcookie.net"
- pressKey: Enter

# Set a cookie
- assertVisible: "Cookie Test"
- tapOn: "Cookie name"
- inputText: "TestName"
- tapOn: "Cookie value"
- inputText: "TestValue"
- scrollUntilVisible:
centerElement: true
element:
text: "Submit"
- tapOn: "Submit"

# Fireproof the site
- tapOn: "Browsing Menu"
- tapOn: "Fireproof This Site"
- tapOn: "Fireproof"
- assertVisible: "setcookie.net is now Fireproof"

# Fire Button - twice, just to be sure
- tapOn: "Close Tabs and Clear Data"
- tapOn:
id: "alert.forget-data.confirm"
- assertVisible:
id: "searchEntry"
- tapOn: "Close Tabs and Clear Data"
- tapOn:
id: "alert.forget-data.confirm"

# Validate Cookie was retained
- tapOn:
id: "searchEntry"
- inputText: "https://setcookie.net"
- pressKey: Enter
- assertVisible: "TestName = TestValue"

# Remove fireproofing
- tapOn: "Browsing Menu"
- tapOn: "Remove Fireproofing"

# Fire Button
- tapOn: "Close Tabs and Clear Data"
- tapOn:
id: "alert.forget-data.confirm"

# Validate Cookie was removed
- tapOn:
id: "searchEntry"
- inputText: "https://setcookie.net"
- pressKey: Enter
- assertVisible: "Cookie Test"
- assertVisible: "Received no cookies."
44 changes: 44 additions & 0 deletions .maestro/data_clearing_tests/02_duckduckgo_settings.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
appId: com.duckduckgo.mobile.ios
tags:
- privacy

---

# Set up
- clearState
- launchApp
- runFlow:
when:
visible:
text: "Let’s Do It!"
index: 0
file: ../shared/onboarding.yaml

# Load Site
- assertVisible:
id: "searchEntry"
- tapOn:
id: "searchEntry"
- inputText: "privacy blogs"
- pressKey: Enter

# Change settings
- tapOn: "Safe search: moderate ▼"
- tapOn: "Off"

# Fire Button - twice, just to be sure
- tapOn: "Close Tabs and Clear Data"
- tapOn:
id: "alert.forget-data.confirm"
- assertVisible:
id: "searchEntry"
- tapOn: "Close Tabs and Clear Data"
- tapOn:
id: "alert.forget-data.confirm"

# Validate Cookie was retained
- tapOn:
id: "searchEntry"
- inputText: "creepy trackers"
- pressKey: Enter
- assertVisible: "Safe search: off ▼"
23 changes: 18 additions & 5 deletions Core/CookieStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
import Common
import Foundation

/// 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 {
Expand All @@ -31,7 +37,7 @@ public class CookieStorage {

var isConsumed: Bool {
get {
userDefaults.bool(forKey: Keys.consumed, defaultValue: false)
return userDefaults.bool(forKey: Keys.consumed, defaultValue: false)
}
set {
userDefaults.set(newValue, forKey: Keys.consumed)
Expand Down Expand Up @@ -77,15 +83,20 @@ public class CookieStorage {
self.userDefaults = userDefaults
}

enum CookieDomainsOnUpdate {
/// 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], keepingPreservedLogins preservedLogins: PreserveLogins) -> CookieDomainsOnUpdate {
func updateCookies(_ cookies: [HTTPCookie], keepingPreservedLogins preservedLogins: PreserveLogins) -> CookieDomainsOnUpdateDiagnostic {
guard isConsumed else { return .notConsumed }

isConsumed = false

let persisted = self.cookies
Expand All @@ -109,7 +120,9 @@ public class CookieStorage {
persistedDomains: persistedCookiesByDomain.keys.sorted()
)

updatedCookiesByDomain.keys.forEach {
let cookieDomains = Set(updatedCookiesByDomain.keys.map { $0 } + persistedCookiesByDomain.keys.map { $0 })

cookieDomains.forEach {
persistedCookiesByDomain[$0] = updatedCookiesByDomain[$0]
}

Expand All @@ -128,7 +141,7 @@ public class CookieStorage {
return diagnosticResult
}

private func evaluateDomains(updatedDomains: [String], persistedDomains: [String]) -> CookieDomainsOnUpdate {
private func evaluateDomains(updatedDomains: [String], persistedDomains: [String]) -> CookieDomainsOnUpdateDiagnostic {
if persistedDomains.isEmpty {
return .empty
} else if updatedDomains.count < persistedDomains.count {
Expand Down
72 changes: 72 additions & 0 deletions Core/DataStoreWarmup.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//
// DataStoreWarmup.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 Combine
import WebKit

/// WKWebsiteDataStore is basically non-functional until a web view has been instanciated and a page is successfully loaded.
public class DataStoreWarmup {

public init() { }

@MainActor
public func ensureReady() async {
await BlockingNavigationDelegate().loadInBackgroundWebView(url: URL(string: "about:blank")!)
}

}

private class BlockingNavigationDelegate: NSObject, WKNavigationDelegate {

let finished = PassthroughSubject<Void, Never>()

func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction) async -> WKNavigationActionPolicy {
return .allow
}

func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
finished.send()
}

func webViewWebContentProcessDidTerminate(_ webView: WKWebView) {
Pixel.fire(pixel: .webKitDidTerminateDuringWarmup)
// We won't get a `didFinish` if the webview crashes
finished.send()
}

var cancellable: AnyCancellable?
func waitForLoad() async {
await withCheckedContinuation { continuation in
cancellable = finished.sink { _ in
continuation.resume()
}
}
}

@MainActor
func loadInBackgroundWebView(url: URL) async {
let config = WKWebViewConfiguration.persistent()
let webView = WKWebView(frame: .zero, configuration: config)
webView.navigationDelegate = self
let request = URLRequest(url: url)
webView.load(request)
await waitForLoad()
}

}
6 changes: 4 additions & 2 deletions Core/PixelEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ extension Pixel {

case webKitDidTerminate
case webKitTerminationDidReloadCurrentTab

case webKitDidTerminateDuringWarmup

case backgroundTaskSubmissionFailed

case blankOverlayNotDismissed
Expand Down Expand Up @@ -891,6 +892,7 @@ extension Pixel.Event {
case .ampBlockingRulesCompilationFailed: return "m_debug_amp_rules_compilation_failed"

case .webKitDidTerminate: return "m_d_wkt"
case .webKitDidTerminateDuringWarmup: return "m_d_webkit-terminated-during-warmup"
case .webKitTerminationDidReloadCurrentTab: return "m_d_wktct"

case .backgroundTaskSubmissionFailed: return "m_bt_rf"
Expand Down Expand Up @@ -926,7 +928,7 @@ extension Pixel.Event {
case .debugCannotClearObservationsDatabase: return "m_d_cannot_clear_observations_database"
case .debugWebsiteDataStoresNotClearedMultiple: return "m_d_wkwebsitedatastoresnotcleared_multiple"
case .debugWebsiteDataStoresNotClearedOne: return "m_d_wkwebsitedatastoresnotcleared_one"
case .debugCookieCleanupError: return "m_cookie_cleanup_error"
case .debugCookieCleanupError: return "m_d_cookie-cleanup-error"

// MARK: Ad Attribution

Expand Down
1 change: 0 additions & 1 deletion Core/PreserveLogins.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public class PreserveLogins {
return allowedDomains.contains(where: { $0 == cookieDomain
|| ".\($0)" == cookieDomain
|| (cookieDomain.hasPrefix(".") && $0.hasSuffix(cookieDomain)) })

}

public func remove(domain: String) {
Expand Down
Loading
Loading