From 20469bbeeff33fcd18e78f672a544ee82b4a741c Mon Sep 17 00:00:00 2001 From: Michal Smaga Date: Fri, 27 Sep 2024 16:12:30 +0200 Subject: [PATCH 1/2] For third party requests differentiate if they are affiliated with first party (#1003) Please review the release process for BrowserServicesKit [here](https://app.asana.com/0/1200194497630846/1200837094583426). **Required**: Task/Issue URL: https://app.asana.com/0/414709148257752/1208376794617030/f iOS PR: https://github.com/duckduckgo/iOS/pull/3386 macOS PR: https://github.com/duckduckgo/macos-browser/pull/3343 What kind of version bump will this require?: Minor **Description**: When loading a website and some of the allowed third party requests may not be recognized as trackers but by their URL they may belong to the same entity as the website. For that cases we should mark their state as `.allowed(reason: .ownedByFirstParty)` (instead of `.allowed(reason: .otherThirdPartyRequest)`). **Steps to test this PR**: See parent task for testing steps. **OS Testing**: * [ ] iOS 14 * [ ] iOS 15 * [ ] iOS 16 * [ ] macOS 10.15 * [ ] macOS 11 * [ ] macOS 12 --- ###### Internal references: [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) --- .../ContentBlockerRulesUserScript.swift | 4 +- .../UserScripts/TrackerResolver.swift | 2 +- .../ContentBlockerRulesUserScriptsTests.swift | 51 ++++++++++++------- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/ContentBlockerRulesUserScript.swift b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/ContentBlockerRulesUserScript.swift index ad2bc44e8..0278ed64e 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/ContentBlockerRulesUserScript.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/ContentBlockerRulesUserScript.swift @@ -187,11 +187,13 @@ open class ContentBlockerRulesUserScript: NSObject, UserScript { !isFirstParty(requestURL: trackerUrlString, websiteURL: pageUrlStr) else { return } let entity = currentTrackerData.findEntity(forHost: requestETLDp1) ?? Entity(displayName: requestETLDp1, domains: nil, prevalence: nil) + let isAffiliated = resolver.isPageAffiliatedWithTrackerEntity(pageUrlString: pageUrlStr, trackerEntity: entity) + let thirdPartyRequest = DetectedRequest(url: trackerUrlString, eTLDplus1: requestETLDp1, knownTracker: nil, entity: entity, - state: .allowed(reason: .otherThirdPartyRequest), + state: .allowed(reason: isAffiliated ? .ownedByFirstParty : .otherThirdPartyRequest), pageUrl: pageUrlStr) delegate.contentBlockerRulesUserScript(self, detectedThirdPartyRequest: thirdPartyRequest) } diff --git a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/TrackerResolver.swift b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/TrackerResolver.swift index 8e1b2baae..36a5974d7 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/TrackerResolver.swift +++ b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/TrackerResolver.swift @@ -85,7 +85,7 @@ public class TrackerResolver { pageUrl: pageUrlString) } - private func isPageAffiliatedWithTrackerEntity(pageUrlString: String, trackerEntity: Entity) -> Bool { + public func isPageAffiliatedWithTrackerEntity(pageUrlString: String, trackerEntity: Entity) -> Bool { guard let pageHost = URL(string: pageUrlString)?.host, let pageEntity = tds.findEntity(forHost: pageHost) else { return false } diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesUserScriptsTests.swift b/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesUserScriptsTests.swift index f6b4440e5..8d48a1ff4 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesUserScriptsTests.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/ContentBlockerRulesUserScriptsTests.swift @@ -59,14 +59,16 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { "entities": { "Fake Tracking Inc": { "domains": [ - "tracker.com" + "tracker.com", + "trackeraffiliated.com" ], "displayName": "Fake Tracking Inc", "prevalence": 0.1 } }, "domains": { - "tracker.com": "Fake Tracking Inc" + "tracker.com": "Fake Tracking Inc", + "trackeraffiliated.com": "Fake Tracking Inc" } } """ @@ -79,6 +81,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { var webView: WKWebView? let nonTrackerURL = URL(string: "test://nontracker.com/1.png")! + let nonTrackerAffiliatedURL = URL(string: "test://trackeraffiliated.com/1.png")! let trackerURL = URL(string: "test://tracker.com/1.png")! let subTrackerURL = URL(string: "test://sub.tracker.com/1.png")! @@ -89,7 +92,8 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { website = MockWebsite(resources: [.init(type: .image, url: nonTrackerURL), .init(type: .image, url: trackerURL), - .init(type: .image, url: subTrackerURL)]) + .init(type: .image, url: subTrackerURL), + .init(type: .image, url: nonTrackerAffiliatedURL)]) } private func setupWebViewForUserScripTests(trackerData: TrackerData, @@ -186,7 +190,11 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let blockedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.filter { $0.isBlocked }.map { $0.domain }) XCTAssertEqual(expectedTrackers, blockedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL] + let expected3rdParty: Set = ["nontracker.com", "trackeraffiliated.com"] + let detected3rdParty = Set(self.userScriptDelegateMock.detectedThirdPartyRequests.map { $0.domain }) + XCTAssertEqual(detected3rdParty, expected3rdParty) + + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -216,11 +224,19 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssert(detectedTrackers.isEmpty) - let expected3rdParty: Set = ["nontracker.com"] + let expected3rdParty: Set = ["nontracker.com", "trackeraffiliated.com"] let detected3rdParty = Set(self.userScriptDelegateMock.detectedThirdPartyRequests.map { $0.domain }) XCTAssertEqual(detected3rdParty, expected3rdParty) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedOwnedBy1stPartyRequests: Set = ["trackeraffiliated.com"] + let detectedOwnedBy1stPartyRequests = Set(self.userScriptDelegateMock.detectedThirdPartyRequests.filter { $0.state == .allowed(reason: .ownedByFirstParty) }.map { $0.domain }) + XCTAssertEqual(detectedOwnedBy1stPartyRequests, expectedOwnedBy1stPartyRequests) + + let expectedOther3rdPartyRequests: Set = ["nontracker.com"] + let detectedOther3rdPartyRequests = Set(self.userScriptDelegateMock.detectedThirdPartyRequests.filter { $0.state == .allowed(reason: .otherThirdPartyRequest) }.map { $0.domain }) + XCTAssertEqual(detectedOther3rdPartyRequests, expectedOther3rdPartyRequests) + + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -247,10 +263,11 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let blockedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.filter { $0.isBlocked }.map { $0.domain }) XCTAssertEqual(blockedTrackers, expectedTrackers) + let expected3rdParty: Set = ["trackeraffiliated.com"] let detected3rdParty = Set(self.userScriptDelegateMock.detectedThirdPartyRequests.map { $0.domain }) - XCTAssert(detected3rdParty.isEmpty) + XCTAssertEqual(detected3rdParty, expected3rdParty) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -280,7 +297,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssertEqual(expectedTrackers, detectedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -312,7 +329,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssertEqual(expectedTrackers, detectedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -339,7 +356,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let blockedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.filter { $0.isBlocked }.map { $0.domain }) XCTAssertEqual(expectedTrackers, blockedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -366,7 +383,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let blockedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.filter { $0.isBlocked }.map { $0.domain }) XCTAssertEqual(expectedTrackers, blockedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -396,7 +413,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssertEqual(expectedTrackers, detectedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -426,7 +443,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssertEqual(expectedTrackers, detectedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -453,7 +470,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let blockedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.filter { $0.isBlocked }.map { $0.domain }) XCTAssertEqual(expectedTrackers, blockedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -483,7 +500,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssertEqual(expectedTrackers, detectedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } @@ -513,7 +530,7 @@ class ContentBlockerRulesUserScriptsTests: XCTestCase { let detectedTrackers = Set(self.userScriptDelegateMock.detectedTrackers.map { $0.domain }) XCTAssertEqual(expectedTrackers, detectedTrackers) - let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.trackerURL, self.subTrackerURL] + let expectedRequests: Set = [websiteURL, self.nonTrackerURL, self.nonTrackerAffiliatedURL, self.trackerURL, self.subTrackerURL] XCTAssertEqual(Set(self.schemeHandler.handledRequests), expectedRequests) } From b60b38bace7262e0c4a006018b7e4b060ba4b754 Mon Sep 17 00:00:00 2001 From: Christopher Brind Date: Fri, 27 Sep 2024 15:38:26 +0100 Subject: [PATCH 2/2] fix suggestions performance (#1008) **Required**: Task/Issue URL: https://app.asana.com/0/1201048563534612/1208413716679959/f iOS PR: https://github.com/duckduckgo/iOS/pull/3405 macOS PR: https://github.com/duckduckgo/macos-browser/pull/3353 What kind of version bump will this require?: Major/Minor/Patch **Optional**: Tech Design URL: CC: **Description**: Fix suggestions performance issue by just looking at the first 100 sorted local results. **Steps to test this PR**: 1. Run the app on a local large data source (history entries > 1000 entries ideally) 1. Use autocomplete and check that it doesn't time out for a broad range of matches --- Sources/Suggestions/SuggestionProcessing.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/Suggestions/SuggestionProcessing.swift b/Sources/Suggestions/SuggestionProcessing.swift index 2425a0ed5..b0ff5eacb 100644 --- a/Sources/Suggestions/SuggestionProcessing.swift +++ b/Sources/Suggestions/SuggestionProcessing.swift @@ -58,7 +58,8 @@ final class SuggestionProcessing { } // Get best matches from history and bookmarks - let allLocalSuggestions = localSuggestions(from: history, bookmarks: bookmarks, internalPages: internalPages, openTabs: openTabs, query: query) + let allLocalSuggestions = Array(localSuggestions(from: history, bookmarks: bookmarks, internalPages: internalPages, openTabs: openTabs, query: query) + .prefix(100)) // temporary optimsiation // Combine HaB and domains into navigational suggestions and remove duplicates let navigationalSuggestions = allLocalSuggestions + duckDuckGoDomainSuggestions