From e8d7c82a6b2d1791e7b39e2833b0ac2f34ef053f Mon Sep 17 00:00:00 2001 From: Jacob Sikorski Date: Thu, 19 Oct 2023 12:15:09 -0600 Subject: [PATCH] Compile max content blockers --- .../Browser/Helpers/LaunchHelper.swift | 3 +- .../AdblockResourceDownloader.swift | 15 +-- .../ContentBlockerManager.swift | 23 +++- .../FilterListCustomURLDownloader.swift | 37 +++---- .../FilterListResourceDownloader.swift | 77 ++++--------- .../Brave/WebFilters/FilterListStorage.swift | 32 ++++-- .../ShieldStats/Adblock/AdBlockStats.swift | 101 ++++++++++++------ 7 files changed, 155 insertions(+), 133 deletions(-) diff --git a/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift b/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift index a8a843ca664..1e9985d0600 100644 --- a/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift +++ b/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift @@ -88,7 +88,8 @@ public actor LaunchHelper { Task.detached(priority: .low) { // Let's disable filter lists if we have reached a maxumum amount - let enabledSources = await AdBlockStats.shared.enabledSources + let enabledSources = await AdBlockStats.shared.enabledPrioritizedSources + if enabledSources.count > AdBlockStats.maxNumberOfAllowedFilterLists { let toDisableSources = enabledSources[AdBlockStats.maxNumberOfAllowedFilterLists...] diff --git a/Sources/Brave/WebFilters/AdblockResourceDownloader.swift b/Sources/Brave/WebFilters/AdblockResourceDownloader.swift index 83c8f22cdd9..8a3395f0d98 100644 --- a/Sources/Brave/WebFilters/AdblockResourceDownloader.swift +++ b/Sources/Brave/WebFilters/AdblockResourceDownloader.swift @@ -121,19 +121,8 @@ public actor AdblockResourceDownloader: Sendable { switch resource { case .adBlockRules: let blocklistType = ContentBlockerManager.BlocklistType.generic(.blockAds) - let modes = await blocklistType.allowedModes.asyncFilter { mode in - guard allowedModes.contains(mode) else { return false } - if downloadResult.isModified { return true } - - // If the file wasn't modified, make sure we have something compiled. - // We should, but this can be false during upgrades if the identifier changed for some reason. - if await ContentBlockerManager.shared.hasRuleList(for: blocklistType, mode: mode) { - ContentBlockerManager.log.debug("Rule list already compiled for `\(blocklistType.makeIdentifier(for: mode))`") - return false - } else { - return true - } - } + let neededModes = downloadResult.isModified ? blocklistType.allowedModes : await ContentBlockerManager.shared.missingModes(for: blocklistType) + let modes = neededModes.filter({ allowedModes.contains($0) }) // No modes are needed to be compiled guard !modes.isEmpty else { return } diff --git a/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift b/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift index 98623896cf5..1bf877422f4 100644 --- a/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift +++ b/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift @@ -9,6 +9,7 @@ import Data import Shared import Preferences import BraveShields +import BraveCore import os.log /// A class that aids in the managment of rule lists on the rule store. @@ -169,9 +170,11 @@ actor ContentBlockerManager { } } - /// Compile the given resource and store it in cache for the given blocklist type using all allowed modes - func compile(encodedContentRuleList: String, for type: BlocklistType, options: CompileOptions = []) async throws { - try await self.compile(encodedContentRuleList: encodedContentRuleList, for: type, modes: type.allowedModes) + /// Compile the rule list found in the given local URL using the specified modes + func compileRuleList(at localFileURL: URL, for type: BlocklistType, options: CompileOptions = [], modes: [BlockingMode]) async throws { + let filterSet = try String(contentsOf: localFileURL) + let result = try AdblockEngine.contentBlockerRules(fromFilterSet: filterSet) + try await compile(encodedContentRuleList: result.rulesJSON, for: type, options: options, modes: modes) } /// Compile the given resource and store it in cache for the given blocklist type and specified modes @@ -257,6 +260,20 @@ actor ContentBlockerManager { return ruleList } + /// Return all the modes that need to be compiled for the given type + func missingModes(for type: BlocklistType) async -> [BlockingMode] { + return await type.allowedModes.asyncFilter { mode in + // If the file wasn't modified, make sure we have something compiled. + // We should, but this can be false during upgrades if the identifier changed for some reason. + if await hasRuleList(for: type, mode: mode) { + ContentBlockerManager.log.debug("Rule list already compiled for `\(type.makeIdentifier(for: mode))`") + return false + } else { + return true + } + } + } + /// Check if a rule list is compiled for this type func hasRuleList(for type: BlocklistType, mode: BlockingMode) async -> Bool { do { diff --git a/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift b/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift index 4caf134bfa9..30098586b29 100644 --- a/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift +++ b/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift @@ -87,50 +87,43 @@ actor FilterListCustomURLDownloader: ObservableObject { /// Handle the download results of a custom filter list. This will process the download by compiling iOS rule lists and adding the rule list to the `AdblockEngineManager`. private func handle(downloadResult: ResourceDownloader.DownloadResult, for filterListCustomURL: FilterListCustomURL) async { let uuid = await filterListCustomURL.setting.uuid - let blocklistType = ContentBlockerManager.BlocklistType.customFilterList(uuid: uuid) - - // Compile this rule list if we haven't already or if the file has been modified - if downloadResult.isModified { - do { - let filterSet = try String(contentsOf: downloadResult.fileURL, encoding: .utf8) - let result = try AdblockEngine.contentBlockerRules(fromFilterSet: filterSet) - - try await ContentBlockerManager.shared.compile( - encodedContentRuleList: result.rulesJSON, - for: blocklistType, - options: .all - ) - } catch { - ContentBlockerManager.log.error( - "Failed to compile rule lists for `\(blocklistType.debugDescription)`: \(error.localizedDescription)" - ) - } - } // Add/remove the resource depending on if it is enabled/disabled - let source = CachedAdBlockEngine.Source.filterListURL(uuid: uuid) guard let resourcesInfo = await FilterListResourceDownloader.shared.resourcesInfo else { assertionFailure("This should not have been called if the resources are not ready") return } + let source = await filterListCustomURL.setting.engineSource let version = fileVersionDateFormatter.string(from: downloadResult.date) let filterListInfo = CachedAdBlockEngine.FilterListInfo( source: .filterListURL(uuid: uuid), localFileURL: downloadResult.fileURL, version: version, fileType: .text ) + let lazyInfo = AdBlockStats.LazyFilterListInfo( + filterListInfo: filterListInfo, isAlwaysAggressive: true + ) guard await AdBlockStats.shared.isEagerlyLoaded(source: source) else { // Don't compile unless eager await AdBlockStats.shared.updateIfNeeded(resourcesInfo: resourcesInfo) await AdBlockStats.shared.updateIfNeeded(filterListInfo: filterListInfo, isAlwaysAggressive: true) + + // To free some space, remove any rule lists that are not needed + if let blocklistType = lazyInfo.blocklistType { + do { + try await ContentBlockerManager.shared.removeRuleLists(for: blocklistType) + } catch { + ContentBlockerManager.log.error("Failed to remove rule lists for \(filterListInfo.debugDescription)") + } + } return } await AdBlockStats.shared.compile( - filterListInfo: filterListInfo, resourcesInfo: resourcesInfo, - isAlwaysAggressive: true + lazyInfo: lazyInfo, resourcesInfo: resourcesInfo, + compileContentBlockers: downloadResult.isModified ) } diff --git a/Sources/Brave/WebFilters/FilterListResourceDownloader.swift b/Sources/Brave/WebFilters/FilterListResourceDownloader.swift index 3ec37276406..2a9dfa8579d 100644 --- a/Sources/Brave/WebFilters/FilterListResourceDownloader.swift +++ b/Sources/Brave/WebFilters/FilterListResourceDownloader.swift @@ -86,7 +86,8 @@ public actor FilterListResourceDownloader { await self.compileFilterListEngineIfNeeded( fromComponentId: componentId, folderURL: folderURL, isAlwaysAggressive: setting.isAlwaysAggressive, - resourcesInfo: resourcesInfo + resourcesInfo: resourcesInfo, + compileContentBlockers: false ) // Sleep for 1ms. This drastically reduces memory usage without much impact to usability @@ -157,14 +158,13 @@ public actor FilterListResourceDownloader { localFileURL: folderURL.appendingPathComponent("rs-ABPFilterParserData.dat", conformingTo: .data), version: version, fileType: .dat ) - - guard await AdBlockStats.shared.needsCompilation(for: filterListInfo, resourcesInfo: resourcesInfo) else { - return - } + let lazyInfo = AdBlockStats.LazyFilterListInfo( + filterListInfo: filterListInfo, isAlwaysAggressive: false + ) await AdBlockStats.shared.compile( - filterListInfo: filterListInfo, resourcesInfo: resourcesInfo, - isAlwaysAggressive: false + lazyInfo: lazyInfo, resourcesInfo: resourcesInfo, + compileContentBlockers: false ) } @@ -172,7 +172,8 @@ public actor FilterListResourceDownloader { private func compileFilterListEngineIfNeeded( fromComponentId componentId: String, folderURL: URL, isAlwaysAggressive: Bool, - resourcesInfo: CachedAdBlockEngine.ResourcesInfo + resourcesInfo: CachedAdBlockEngine.ResourcesInfo, + compileContentBlockers: Bool ) async { let version = folderURL.lastPathComponent let source = CachedAdBlockEngine.Source.filterList(componentId: componentId) @@ -181,22 +182,26 @@ public actor FilterListResourceDownloader { localFileURL: folderURL.appendingPathComponent("list.txt", conformingTo: .text), version: version, fileType: .text ) - + let lazyInfo = AdBlockStats.LazyFilterListInfo(filterListInfo: filterListInfo, isAlwaysAggressive: isAlwaysAggressive) guard await AdBlockStats.shared.isEagerlyLoaded(source: source) else { // Don't compile unless eager await AdBlockStats.shared.updateIfNeeded(resourcesInfo: resourcesInfo) await AdBlockStats.shared.updateIfNeeded(filterListInfo: filterListInfo, isAlwaysAggressive: isAlwaysAggressive) - return - } - - guard await AdBlockStats.shared.needsCompilation(for: filterListInfo, resourcesInfo: resourcesInfo) else { - // Don't compile unless needed + + // To free some space, remove any rule lists that are not needed + if let blocklistType = lazyInfo.blocklistType { + do { + try await ContentBlockerManager.shared.removeRuleLists(for: blocklistType) + } catch { + ContentBlockerManager.log.error("Failed to remove rule lists for \(filterListInfo.debugDescription)") + } + } return } await AdBlockStats.shared.compile( - filterListInfo: filterListInfo, resourcesInfo: resourcesInfo, - isAlwaysAggressive: isAlwaysAggressive + lazyInfo: lazyInfo, resourcesInfo: resourcesInfo, + compileContentBlockers: compileContentBlockers ) } @@ -229,7 +234,7 @@ public actor FilterListResourceDownloader { ) // Save the downloaded folder for later (caching) purposes - FilterListStorage.shared.set(folderURL: folderURL, forUUID: filterList.uuid) + FilterListStorage.shared.set(folderURL: folderURL, forComponentId: filterList.entry.componentId) } } } @@ -258,44 +263,8 @@ public actor FilterListResourceDownloader { // Add or remove the filter list from the engine depending if it's been enabled or not await self.compileFilterListEngineIfNeeded( fromComponentId: componentId, folderURL: folderURL, isAlwaysAggressive: isAlwaysAggressive, - resourcesInfo: resourcesInfo + resourcesInfo: resourcesInfo, compileContentBlockers: loadContentBlockers ) - - // Compile this rule list if we haven't already or if the file has been modified - // We also don't load them if they are loading from cache because this will cost too much during launch - if loadContentBlockers { - let version = folderURL.lastPathComponent - let blocklistType = ContentBlockerManager.BlocklistType.filterList(componentId: componentId, isAlwaysAggressive: isAlwaysAggressive) - let modes = await blocklistType.allowedModes.asyncFilter { mode in - if let loadedVersion = await FilterListStorage.shared.loadedRuleListVersions.value[componentId] { - // if we know the loaded version we can just check it (optimization) - return loadedVersion != version - } else { - return true - } - } - - // No modes need to be compiled - guard !modes.isEmpty else { return } - - do { - let filterSet = try String(contentsOf: filterListURL, encoding: .utf8) - let result = try AdblockEngine.contentBlockerRules(fromFilterSet: filterSet) - - try await ContentBlockerManager.shared.compile( - encodedContentRuleList: result.rulesJSON, for: blocklistType, - options: .all, modes: modes - ) - - await MainActor.run { - FilterListStorage.shared.loadedRuleListVersions.value[componentId] = version - } - } catch { - ContentBlockerManager.log.error( - "Failed to create content blockers for `\(blocklistType.debugDescription)` v\(version): \(error)" - ) - } - } } } diff --git a/Sources/Brave/WebFilters/FilterListStorage.swift b/Sources/Brave/WebFilters/FilterListStorage.swift index eb6ae068e8d..75accd392c3 100644 --- a/Sources/Brave/WebFilters/FilterListStorage.swift +++ b/Sources/Brave/WebFilters/FilterListStorage.swift @@ -45,7 +45,16 @@ import Combine /// Load the filter list settings func loadFilterListSettings() { - allFilterListSettings = FilterListSetting.loadAllSettings(fromMemory: !persistChanges) + var componentIds: Set = [] + allFilterListSettings = FilterListSetting.loadAllSettings(fromMemory: !persistChanges).filter({ setting in + guard let componentId = setting.componentId, !componentId.isEmpty, !componentIds.contains(componentId) else { + setting.delete(inMemory: persistChanges) + return false + } + + componentIds.insert(componentId) + return true + }) } /// Load filter lists from the ad block service and subscribe to any filter list changes @@ -149,7 +158,7 @@ import Combine uuid: String, isEnabled: Bool, componentId: String, allowCreation: Bool, order: Int, isAlwaysAggressive: Bool ) { - if allFilterListSettings.contains(where: { $0.uuid == uuid }) { + if allFilterListSettings.contains(where: { $0.componentId == componentId }) { updateSetting( uuid: uuid, componentId: componentId, @@ -171,8 +180,8 @@ import Combine /// Set the folder url of the /// /// - Warning: Do not call this before we load core data - public func set(folderURL: URL, forUUID uuid: String) { - guard let index = allFilterListSettings.firstIndex(where: { $0.uuid == uuid }) else { + public func set(folderURL: URL, forComponentId componentId: String) { + guard let index = allFilterListSettings.firstIndex(where: { $0.componentId == componentId }) else { return } @@ -184,20 +193,23 @@ import Combine /// Update the filter list settings with the given `componentId` and `isEnabled` status /// Will not write unless one of these two values have changed private func updateSetting(uuid: String, componentId: String, isEnabled: Bool, order: Int, isAlwaysAggressive: Bool) { - guard let index = allFilterListSettings.firstIndex(where: { $0.uuid == uuid }) else { + guard let index = allFilterListSettings.firstIndex(where: { $0.componentId == componentId }) else { return } - guard allFilterListSettings[index].isEnabled != isEnabled || allFilterListSettings[index].componentId != componentId || allFilterListSettings[index].order?.intValue != order || allFilterListSettings[index].isAlwaysAggressive != isAlwaysAggressive else { - // Ensure we stop if this is already in sync in order to avoid an event loop - // And things hanging for too long. - // This happens because we care about UI changes but not when our downloads finish + // Ensure we stop if this is already in sync in order to avoid an event loop + // And things hanging for too long. + guard allFilterListSettings[index].isEnabled != isEnabled + || allFilterListSettings[index].uuid != uuid + || allFilterListSettings[index].order?.intValue != order + || allFilterListSettings[index].isAlwaysAggressive != isAlwaysAggressive + else { return } allFilterListSettings[index].isEnabled = isEnabled allFilterListSettings[index].isAlwaysAggressive = isAlwaysAggressive - allFilterListSettings[index].componentId = componentId + allFilterListSettings[index].uuid = uuid allFilterListSettings[index].order = NSNumber(value: order) FilterListSetting.save(inMemory: !persistChanges) } diff --git a/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift b/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift index 11df6cd2724..58c52f962e3 100644 --- a/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift +++ b/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift @@ -25,7 +25,7 @@ public actor AdBlockStats { public static let shared = AdBlockStats() /// An object containing the basic information to allow us to compile an engine - struct LazyFilterListInfo { + public struct LazyFilterListInfo { let filterListInfo: CachedAdBlockEngine.FilterListInfo let isAlwaysAggressive: Bool } @@ -56,6 +56,16 @@ public actor AdBlockStats { return enabledSources } + @MainActor var enabledPrioritizedSources: [CachedAdBlockEngine.Source] { + let criticalSources = Set(self.criticalSources) + var enabledSources: [CachedAdBlockEngine.Source] = [.adBlock] + enabledSources.append(contentsOf: FilterListStorage.shared.enabledSources.sorted { left, right in + return criticalSources.contains(left) && !criticalSources.contains(right) + }) + enabledSources.append(contentsOf: CustomFilterListStorage.shared.enabledSources) + return enabledSources + } + /// Tells us if we reached the max limit of already compiled filter lists var reachedMaxLimit: Bool { return cachedEngines.count >= Self.maxNumberOfAllowedFilterLists @@ -77,30 +87,42 @@ public actor AdBlockStats { /// /// - Note: This method will ensure syncronous compilation public func compile( - filterListInfo: CachedAdBlockEngine.FilterListInfo, resourcesInfo: CachedAdBlockEngine.ResourcesInfo, - isAlwaysAggressive: Bool, ignoreMaximum: Bool = false + lazyInfo: LazyFilterListInfo, resourcesInfo: CachedAdBlockEngine.ResourcesInfo, + ignoreMaximum: Bool = false, compileContentBlockers: Bool ) async { await currentCompileTask?.value - guard needsCompilation(for: filterListInfo, resourcesInfo: resourcesInfo) else { - // Ensure we only compile if we need to. This prevents two lazy loads from recompiling - return - } - currentCompileTask = Task { - if !ignoreMaximum && reachedMaxLimit && cachedEngines[filterListInfo.source] == nil { - ContentBlockerManager.log.error("Failed to compile engine for \(filterListInfo.source.debugDescription): Reached maximum!") + if !ignoreMaximum && reachedMaxLimit && cachedEngines[lazyInfo.filterListInfo.source] == nil { + ContentBlockerManager.log.error("Failed to compile engine for \(lazyInfo.filterListInfo.source.debugDescription): Reached maximum!") return } - do { - let engine = try CachedAdBlockEngine.compile( - filterListInfo: filterListInfo, resourcesInfo: resourcesInfo, isAlwaysAggressive: isAlwaysAggressive - ) + // Compile engine + if needsCompilation(for: lazyInfo.filterListInfo, resourcesInfo: resourcesInfo) { + do { + let engine = try CachedAdBlockEngine.compile( + filterListInfo: lazyInfo.filterListInfo, resourcesInfo: resourcesInfo, isAlwaysAggressive: lazyInfo.isAlwaysAggressive + ) + + add(engine: engine) + } catch { + ContentBlockerManager.log.error("Failed to compile engine for \(lazyInfo.filterListInfo.source.debugDescription)") + } + } + + // Compile content blockers + if compileContentBlockers, let blocklistType = lazyInfo.blocklistType { + let modes = await ContentBlockerManager.shared.missingModes(for: blocklistType) + guard !modes.isEmpty else { return } - add(engine: engine) - } catch { - ContentBlockerManager.log.error("Failed to compile engine for \(filterListInfo.source.debugDescription)") + do { + try await ContentBlockerManager.shared.compileRuleList( + at: lazyInfo.filterListInfo.localFileURL, for: blocklistType, modes: modes + ) + } catch { + ContentBlockerManager.log.error("Failed to compile rule list for \(lazyInfo.filterListInfo.source.debugDescription)") + } } } @@ -133,15 +155,6 @@ public actor AdBlockStats { self.resourcesInfo = resourcesInfo } - /// Remove an engine for the given source. - func removeEngine(for source: CachedAdBlockEngine.Source) { - if let filterListInfo = cachedEngines[source]?.filterListInfo { - ContentBlockerManager.log.debug("Removed engine for \(filterListInfo.debugDescription)") - } - - cachedEngines.removeValue(forKey: source) - } - /// Remove all the engines func removeAllEngines() { cachedEngines.removeAll() @@ -153,22 +166,35 @@ public actor AdBlockStats { for source in cachedEngines.keys { guard !sources.contains(source) else { continue } - removeEngine(for: source) + // Remove the engine + if let filterListInfo = cachedEngines[source]?.filterListInfo { + cachedEngines.removeValue(forKey: source) + ContentBlockerManager.log.debug("Removed engine for \(filterListInfo.debugDescription)") + } + + // Delete the Content blockers + if let lazyInfo = availableFilterLists[source], let blocklistType = lazyInfo.blocklistType { + do { + try await ContentBlockerManager.shared.removeRuleLists(for: blocklistType) + } catch { + ContentBlockerManager.log.error("Failed to remove rule lists for \(lazyInfo.filterListInfo.debugDescription)") + } + } } } /// Remove all engines that have disabled sources func ensureEnabledEngines() async { do { - for source in await enabledSources { + for source in await enabledPrioritizedSources { guard cachedEngines[source] == nil else { continue } guard let availableFilterList = availableFilterLists[source] else { continue } guard let resourcesInfo = self.resourcesInfo else { continue } await compile( - filterListInfo: availableFilterList.filterListInfo, + lazyInfo: availableFilterList, resourcesInfo: resourcesInfo, - isAlwaysAggressive: availableFilterList.isAlwaysAggressive + compileContentBlockers: true ) // Sleep for 1ms. This drastically reduces memory usage without much impact to usability @@ -338,3 +364,18 @@ private extension CachedAdBlockEngine.Source { } } } + +extension AdBlockStats.LazyFilterListInfo { + var blocklistType: ContentBlockerManager.BlocklistType? { + switch filterListInfo.source { + case .adBlock: + // Normally this should be .generic(.blockAds) + // but this content blocker is coming from slim-list + return nil + case .filterList(let componentId): + return .filterList(componentId: componentId, isAlwaysAggressive: isAlwaysAggressive) + case .filterListURL(let uuid): + return .customFilterList(uuid: uuid) + } + } +}