From 0692a80ab947b6bb68e8a7dc829292684ecd4870 Mon Sep 17 00:00:00 2001 From: Jacob Sikorski Date: Fri, 29 Mar 2024 01:00:24 +0100 Subject: [PATCH] Fix issues --- .../FilterLists/FilterListsView.swift | 17 ++++ .../AdBlock/AdBlockEngineManager.swift | 90 +++++++++++-------- .../AdBlock/AdBlockGroupsManager.swift | 87 +++++++++++++----- .../ContentBlockerManager.swift | 45 ++++++++-- .../FilterListCustomURLDownloader.swift | 7 +- .../FilterListResourceDownloader.swift | 2 +- .../AdBlockEngineManagerTests.swift | 7 +- .../AdBlockGroupsManagerTests.swift | 9 +- 8 files changed, 187 insertions(+), 77 deletions(-) diff --git a/ios/brave-ios/Sources/Brave/Frontend/Settings/Features/ShieldsPrivacy/FilterLists/FilterListsView.swift b/ios/brave-ios/Sources/Brave/Frontend/Settings/Features/ShieldsPrivacy/FilterLists/FilterListsView.swift index 3d577fe98df6..d48238565208 100644 --- a/ios/brave-ios/Sources/Brave/Frontend/Settings/Features/ShieldsPrivacy/FilterLists/FilterListsView.swift +++ b/ios/brave-ios/Sources/Brave/Frontend/Settings/Features/ShieldsPrivacy/FilterLists/FilterListsView.swift @@ -71,6 +71,23 @@ struct FilterListsView: View { } @ViewBuilder private var filterListView: some View { + #if DEBUG + let allEnabled = Binding { + filterListStorage.filterLists.allSatisfy({ $0.isEnabled }) + } set: { isEnabled in + filterListStorage.filterLists.enumerated().forEach { index, filterList in + let isEnabled = filterList.entry.hidden ? filterList.entry.defaultEnabled : isEnabled + filterListStorage.filterLists[index].isEnabled = isEnabled + } + } + + Toggle(isOn: allEnabled) { + VStack(alignment: .leading) { + Text("All").foregroundColor(Color(.bravePrimary)) + } + } + #endif + ForEach($filterListStorage.filterLists) { $filterList in if !filterList.isHidden { Toggle(isOn: $filterList.isEnabled) { diff --git a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift index 6cbe353c641a..1e933cc7eb6b 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockEngineManager.swift @@ -76,7 +76,10 @@ import os for enabledSources: [GroupedAdBlockEngine.Source] ) -> [FileInfo] { return enabledSources.compactMap { source in - return availableFiles.first(where: { $0.filterListInfo.source == source }) + return availableFiles.first(where: { + FileManager.default.fileExists(atPath: $0.localFileURL.path) + && $0.filterListInfo.source == source + }) } } @@ -98,9 +101,13 @@ import os } /// Checks to see if we need to compile or recompile the engine based on the available info - func checkNeedsCompile(for enabledSources: [GroupedAdBlockEngine.Source]) -> Bool { - let compilableInfos = compilableFiles(for: enabledSources).map({ $0.filterListInfo }) - guard !compilableInfos.isEmpty else { return false } + func checkNeedsCompile(for fileInfos: [AdBlockEngineManager.FileInfo]) -> Bool { + if let pendingGroup = pendingGroup { + return pendingGroup.infos == fileInfos.map({ $0.filterListInfo }) + } + + let compilableInfos = fileInfos.map({ $0.filterListInfo }) + guard !compilableInfos.isEmpty else { return engine?.group.infos.isEmpty == false } guard let engine = engine else { return true } return compilableInfos != engine.group.infos } @@ -145,22 +152,19 @@ import os /// Especially needed during launch when we have a bunch of downloads coming at the same time. func compileDelayedIfNeeded( for enabledSources: [GroupedAdBlockEngine.Source], - resourcesInfo: GroupedAdBlockEngine.ResourcesInfo?, - priority: TaskPriority + resourcesInfo: GroupedAdBlockEngine.ResourcesInfo? ) { - guard self.checkNeedsCompile(for: enabledSources) else { return } - // Cancel the previous task delayTask?.cancel() // Restart the task delayTask = Task { let hasAllInfo = checkHasAllInfo(for: enabledSources) - try await Task.sleep(seconds: hasAllInfo ? 1 : 60) + try await Task.sleep(seconds: hasAllInfo ? 5 : 60) + await compileAvailableIfNeeded( for: enabledSources, - resourcesInfo: resourcesInfo, - priority: priority + resourcesInfo: resourcesInfo ) } } @@ -168,15 +172,13 @@ import os /// This will compile available data right away if it is needed and cancel any delayedTasks func compileImmediatelyIfNeeded( for enabledSources: [GroupedAdBlockEngine.Source], - resourcesInfo: GroupedAdBlockEngine.ResourcesInfo?, - priority: TaskPriority + resourcesInfo: GroupedAdBlockEngine.ResourcesInfo? ) async { delayTask?.cancel() await self.compileAvailableIfNeeded( for: enabledSources, - resourcesInfo: resourcesInfo, - priority: priority + resourcesInfo: resourcesInfo ) } @@ -192,29 +194,37 @@ import os } func needsCompile(for filterListInfo: GroupedAdBlockEngine.FilterListInfo) -> Bool { - guard let info = engine?.group.infos.first(where: { $0.source == filterListInfo.source }) else { + var infos = availableFiles.map(\.filterListInfo) + if let engineInfos = engine?.group.infos { + // This is an optimization because during launch we don't have file infos. + // But we will have an engine loaded from cache. + // So we also check the engine infos as well + infos.append(contentsOf: engineInfos) + } + + guard let info = infos.first(where: { $0.source == filterListInfo.source }) else { return true } + return filterListInfo.version < info.version } - /// Checks to see if we need to compile or recompile the engine based on the available info - private func checkHasAllInfo(for sources: [GroupedAdBlockEngine.Source]) -> Bool { - return Set(sources) == Set(availableFiles.map(\.filterListInfo.source)) + func checkHasAllInfo(for sources: [GroupedAdBlockEngine.Source]) -> Bool { + let availableSources = compilableFiles(for: sources).map({ $0.filterListInfo.source }) + return sources.allSatisfy({ availableSources.contains($0) }) } /// This will compile available data right away if it is needed private func compileAvailableIfNeeded( for enabledSources: [GroupedAdBlockEngine.Source], - resourcesInfo: GroupedAdBlockEngine.ResourcesInfo?, - priority: TaskPriority + resourcesInfo: GroupedAdBlockEngine.ResourcesInfo? ) async { do { - guard self.checkNeedsCompile(for: enabledSources) else { return } + let compilableFiles = compilableFiles(for: enabledSources) + guard self.checkNeedsCompile(for: compilableFiles) else { return } try await compileAvailable( - for: enabledSources, - resourcesInfo: resourcesInfo, - priority: priority + for: compilableFiles, + resourcesInfo: resourcesInfo ) } catch { ContentBlockerManager.log.error( @@ -225,16 +235,23 @@ import os /// Compile an engine from all available data private func compileAvailable( - for enabledSources: [GroupedAdBlockEngine.Source], - resourcesInfo: GroupedAdBlockEngine.ResourcesInfo?, - priority: TaskPriority + for files: [AdBlockEngineManager.FileInfo], + resourcesInfo: GroupedAdBlockEngine.ResourcesInfo? ) async throws { + let infosString = files.map({ " \($0.filterListInfo.debugDescription)" }) + .joined(separator: "\n") + let count = files.count + + ContentBlockerManager.log.debug( + "Compiling `\(self.cacheFolderName)` engine from \(count) sources:\n\(infosString)" + ) + let engineType = self.engineType - let group = try combineRules(for: enabledSources) + let group = try combineRules(for: files) self.pendingGroup = group // 2. Compile the engine - let groupedEngine = try await Task.detached(priority: priority) { + let groupedEngine = try await Task.detached(priority: .high) { let engine = try GroupedAdBlockEngine.compile(group: group, type: engineType) if let resourcesInfo { @@ -264,17 +281,14 @@ import os /// Take all the filter lists and combine them into one then save them into a cache folder. private func combineRules( - for enabledSources: [GroupedAdBlockEngine.Source] + for compilableFiles: [AdBlockEngineManager.FileInfo] ) throws -> GroupedAdBlockEngine.FilterListGroup { - // 1. Grab all the needed files - let compilableFiles = compilableFiles(for: enabledSources) - - // 2. Create a file url + // 1. Create a file url let cachedFolder = try getOrCreateCacheFolder() let fileURL = cachedFolder.appendingPathComponent("list.txt", conformingTo: .text) var compiledInfos: [GroupedAdBlockEngine.FilterListInfo] = [] var unifiedRules = "" - // 3. Join all the rules together + // 2. Join all the rules together compilableFiles.forEach { fileInfo in do { let fileContents = try String(contentsOf: fileInfo.localFileURL) @@ -282,12 +296,12 @@ import os unifiedRules = [unifiedRules, fileContents].joined(separator: "\n") } catch { ContentBlockerManager.log.error( - "Could not load rules for `\(fileInfo.filterListInfo.debugDescription)`: \(error)" + "Could not load rules for \(fileInfo.filterListInfo.debugDescription): \(error)" ) } } - // 4. Save the files into storage + // 3. Save the files into storage if FileManager.default.fileExists(atPath: fileURL.path) { try FileManager.default.removeItem(at: fileURL) } diff --git a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift index 1e7faf5993c4..0a48e310a68d 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/AdBlock/AdBlockGroupsManager.swift @@ -96,8 +96,7 @@ import os await manager.compileImmediatelyIfNeeded( for: sourceProvider.enabledSources, - resourcesInfo: self.resourcesInfo, - priority: .high + resourcesInfo: self.resourcesInfo ) } } @@ -118,27 +117,75 @@ import os updateIfNeeded(resourcesInfo: resourcesInfo) } - /// Handle updated filter list info - func updated( - fileInfo: AdBlockEngineManager.FileInfo, + func update( + fileInfos: [AdBlockEngineManager.FileInfo], engineType: GroupedAdBlockEngine.EngineType - ) async { + ) { let manager = getManager(for: engineType) let enabledSources = sourceProvider.enabledSources // Compile content blockers if this filter list is enabled - if enabledSources.contains(fileInfo.filterListInfo.source) { - await ensureContentBlockers(for: fileInfo, engineType: engineType) + for fileInfo in fileInfos { + if enabledSources.contains(fileInfo.filterListInfo.source) { + Task { + await ensureContentBlockers(for: fileInfo, engineType: engineType) + } + } + + manager.add(fileInfo: fileInfo) + } + + let sources = fileInfos.map({ $0.filterListInfo.source }) + + if manager.checkHasAllInfo(for: sources) { + Task { + await manager.compileImmediatelyIfNeeded( + for: enabledSources, + resourcesInfo: self.resourcesInfo + ) + } + } else { + manager.compileDelayedIfNeeded( + for: enabledSources, + resourcesInfo: resourcesInfo + ) + } + } + + /// Handle updated filter list info + func update( + fileInfo: AdBlockEngineManager.FileInfo, + engineType: GroupedAdBlockEngine.EngineType + ) { + update(fileInfos: [fileInfo], engineType: engineType) + } + + func removeFileInfos( + for sources: [GroupedAdBlockEngine.Source], + engineType: GroupedAdBlockEngine.EngineType + ) { + let manager = getManager(for: engineType) + for source in sources { + manager.removeInfo(for: source) } - // Always update the info on the manager - manager.add(fileInfo: fileInfo) manager.compileDelayedIfNeeded( - for: enabledSources, - resourcesInfo: resourcesInfo, - priority: .background + for: sourceProvider.enabledSources, + resourcesInfo: resourcesInfo ) + } + func removeFileInfo( + for source: GroupedAdBlockEngine.Source, + engineType: GroupedAdBlockEngine.EngineType + ) { + let manager = getManager(for: engineType) + manager.removeInfo(for: source) + + manager.compileDelayedIfNeeded( + for: sourceProvider.enabledSources, + resourcesInfo: resourcesInfo + ) } /// Ensure all engines and content blockers are compiled @@ -148,8 +195,7 @@ import os let manager = self.getManager(for: engineType) await manager.compileImmediatelyIfNeeded( for: enabledSources, - resourcesInfo: self.resourcesInfo, - priority: .high + resourcesInfo: self.resourcesInfo ) self.ensureContentBlockers(for: enabledSources, engineType: engineType) @@ -354,12 +400,13 @@ extension AdBlockEngineManager.FileInfo { return nil } - let filterListInfo = GroupedAdBlockEngine.FilterListInfo( - source: source, - version: version + self.init( + filterListInfo: GroupedAdBlockEngine.FilterListInfo( + source: source, + version: version + ), + localFileURL: localFileURL ) - - self.init(filterListInfo: filterListInfo, localFileURL: localFileURL) } } diff --git a/ios/brave-ios/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift b/ios/brave-ios/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift index 416c7fd79e6e..04033a2a326d 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift @@ -70,6 +70,20 @@ actor ContentBlockerManager { } } + enum CompileResult { + case empty + case success(WKContentRuleList) + case failure(Error) + + func get() throws -> WKContentRuleList? { + switch self { + case .empty: return nil + case .success(let ruleList): return ruleList + case .failure(let error): throw error + } + } + } + /// An object representing the type of block list public enum BlocklistType: Hashable, CustomDebugStringConvertible { fileprivate static let genericPrifix = "stored-type" @@ -139,7 +153,7 @@ actor ContentBlockerManager { /// The store in which these rule lists should be compiled let ruleStore: WKContentRuleListStore /// We cached the rule lists so that we can return them quicker if we need to - private var cachedRuleLists: [String: Result] + private var cachedRuleLists: [String: CompileResult] /// A list of etld+1s that are always aggressive /// TODO: @JS Replace this with the 1st party ad-block list let alwaysAggressiveETLDs: Set = ["youtube.com"] @@ -210,7 +224,7 @@ actor ContentBlockerManager { try await compile(encodedContentRuleList: result.rulesJSON, for: blocklistType, modes: modes) } catch { ContentBlockerManager.log.error( - "Failed to compile rule list for \(blocklistType.debugDescription)" + "Failed to compile rule list for `\(blocklistType.debugDescription)`" ) } } @@ -223,13 +237,25 @@ actor ContentBlockerManager { ) async throws { guard !modes.isEmpty else { return } var foundError: Error? + let ruleList = try decode(encodedContentRuleList: encodedContentRuleList) + + guard !ruleList.isEmpty else { + for mode in modes { + self.cachedRuleLists[type.makeIdentifier(for: mode)] = .empty + } + + ContentBlockerManager.log.debug( + "Empty filter set for `\(type.debugDescription)`" + ) + return + } for mode in modes { let identifier = type.makeIdentifier(for: mode) do { let moddedRuleList = try self.modify( - encodedContentRuleList: encodedContentRuleList, + ruleList: ruleList, for: mode ) let ruleList = try await compile( @@ -237,6 +263,7 @@ actor ContentBlockerManager { for: type, mode: mode ) + self.cachedRuleLists[identifier] = .success(ruleList) Self.log.debug("Compiled rule list for `\(identifier)`") } catch { @@ -253,16 +280,18 @@ actor ContentBlockerManager { } } - private func modify(encodedContentRuleList: String, for mode: BlockingMode) throws -> String? { + private func modify( + ruleList: [[String: Any?]], + for mode: BlockingMode + ) throws -> String? { switch mode { case .aggressive, .general: // Aggressive mode and general mode has no modification to the rules - return nil + let modifiedData = try JSONSerialization.data(withJSONObject: ruleList) + return String(bytes: modifiedData, encoding: .utf8) case .standard: - // Add the ignore first party rule to make it standard - var ruleList = try decode(encodedContentRuleList: encodedContentRuleList) - + var ruleList = ruleList // We need to make sure we are not going over the limit // So we make space for the added rule if ruleList.count >= (Self.maxContentBlockerSize) { diff --git a/ios/brave-ios/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift b/ios/brave-ios/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift index c9cd286fafdd..02ce3094e50d 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift @@ -76,7 +76,7 @@ actor FilterListCustomURLDownloader: ObservableObject { localFileURL: downloadResult.fileURL ) - await AdBlockGroupsManager.shared.updated( + await AdBlockGroupsManager.shared.update( fileInfo: fileInfo, engineType: .aggressive ) @@ -142,6 +142,11 @@ actor FilterListCustomURLDownloader: ObservableObject { let resource = await filterListCustomURL.setting.resource fetchTasks[resource]?.cancel() fetchTasks.removeValue(forKey: resource) + + await AdBlockGroupsManager.shared.removeFileInfo( + for: filterListCustomURL.setting.engineSource, + engineType: .aggressive + ) } } diff --git a/ios/brave-ios/Sources/Brave/WebFilters/FilterListResourceDownloader.swift b/ios/brave-ios/Sources/Brave/WebFilters/FilterListResourceDownloader.swift index 5daf32a3f5f8..b6b3ca8fb81c 100644 --- a/ios/brave-ios/Sources/Brave/WebFilters/FilterListResourceDownloader.swift +++ b/ios/brave-ios/Sources/Brave/WebFilters/FilterListResourceDownloader.swift @@ -94,7 +94,7 @@ public actor FilterListResourceDownloader { return } - await AdBlockGroupsManager.shared.updated( + await AdBlockGroupsManager.shared.update( fileInfo: fileInfo, engineType: engineType ) diff --git a/ios/brave-ios/Tests/ClientTests/AdBlockEngineManagerTests.swift b/ios/brave-ios/Tests/ClientTests/AdBlockEngineManagerTests.swift index 470298bb7848..e4a2002a3fb9 100644 --- a/ios/brave-ios/Tests/ClientTests/AdBlockEngineManagerTests.swift +++ b/ios/brave-ios/Tests/ClientTests/AdBlockEngineManagerTests.swift @@ -49,7 +49,7 @@ final class AdBlockEngineManagerTests: XCTestCase { // Then // Needs compile returns true and there is no engine - var needsCompile = await engineManager.checkNeedsCompile(for: sources) + var needsCompile = await engineManager.checkNeedsCompile(for: fileInfos) var engine = await engineManager.engine XCTAssertTrue(needsCompile) XCTAssertNil(engine) @@ -68,13 +68,12 @@ final class AdBlockEngineManagerTests: XCTestCase { // We compile engine await engineManager.compileImmediatelyIfNeeded( for: sources, - resourcesInfo: resourcesInfo, - priority: .high + resourcesInfo: resourcesInfo ) // Then // Needs compile returns false and engine is correctly created - needsCompile = await engineManager.checkNeedsCompile(for: sources) + needsCompile = await engineManager.checkNeedsCompile(for: fileInfos) engine = await engineManager.engine let compiledResources = await engine?.resourcesInfo let group = await engine?.group diff --git a/ios/brave-ios/Tests/ClientTests/AdBlockGroupsManagerTests.swift b/ios/brave-ios/Tests/ClientTests/AdBlockGroupsManagerTests.swift index 27d358d1473c..a90e673586c1 100644 --- a/ios/brave-ios/Tests/ClientTests/AdBlockGroupsManagerTests.swift +++ b/ios/brave-ios/Tests/ClientTests/AdBlockGroupsManagerTests.swift @@ -71,9 +71,8 @@ final class AdBlockGroupsManagerTests: XCTestCase { enabled: true ) await groupsManager.updateIfNeeded(resourcesInfo: resourcesInfo) - await groupsManager.updated(fileInfo: fileInfos[1], engineType: .standard) - await groupsManager.updated(fileInfo: fileInfos[0], engineType: .aggressive) - await groupsManager.updated(fileInfo: fileInfos[1], engineType: .aggressive) + await groupsManager.update(fileInfo: fileInfos[1], engineType: .standard) + await groupsManager.update(fileInfos: fileInfos, engineType: .aggressive) await groupsManager.compileEnginesIfNeeded() // Then @@ -154,7 +153,7 @@ final class AdBlockGroupsManagerTests: XCTestCase { enabled: true ) groupsManager.updateIfNeeded(resourcesInfo: resourcesInfo) - await groupsManager.updated(fileInfo: fileInfo, engineType: .standard) + groupsManager.update(fileInfo: fileInfo, engineType: .standard) await groupsManager.compileEnginesIfNeeded() let mainFrameURL = URL(string: "https://dev-pages.bravesoftware.com")! @@ -218,7 +217,7 @@ final class AdBlockGroupsManagerTests: XCTestCase { // When // Adding an aggressive filter list - await groupsManager.updated(fileInfo: fileInfo, engineType: .aggressive) + groupsManager.update(fileInfo: fileInfo, engineType: .aggressive) await groupsManager.compileEnginesIfNeeded() // Then