Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #8656: Remove max number of filter lists limit (#8657)
Browse files Browse the repository at this point in the history
  • Loading branch information
cuba authored Feb 2, 2024
1 parent 6c13ee0 commit 9f3feef
Show file tree
Hide file tree
Showing 15 changed files with 164,345 additions and 28,500 deletions.
3 changes: 1 addition & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,7 @@ var package = Package(
.copy("Resources/google-search-plugin.xml"),
.copy("Resources/duckduckgo-search-plugin.xml"),
.copy("Resources/ad-block-resources/resources.json"),
.copy("Resources/filter-rules/cdbbhgbmjhfnhnmgeddbliobbofkgdhe.txt"),
.copy("Resources/filter-rules/cffkpbalmllkdoenhmdmpbkajipdjfam.dat"),
.copy("Resources/filter-rules/iodkpdagapdfkphljnddpjlldadblomo.txt"),
.copy("Resources/html/index.html"),
.copy("Resources/scripts/farbling-tests.js"),
.copy("Resources/scripts/request-blocking-tests.js"),
Expand Down
23 changes: 1 addition & 22 deletions Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,27 +87,6 @@ public actor LaunchHelper {
let remainingModes = ContentBlockerManager.BlockingMode.allCases.filter({ !loadedBlockModes.contains($0) })

Task.detached(priority: .low) {
// Let's disable filter lists if we have reached a maxumum amount
let enabledSources = await AdBlockStats.shared.enabledPrioritizedSources

if enabledSources.count > AdBlockStats.maxNumberOfAllowedFilterLists {
let toDisableSources = enabledSources[AdBlockStats.maxNumberOfAllowedFilterLists...]

for source in toDisableSources {
switch source {
case .adBlock:
// This should never be in the list because the order of enabledSources places this as the first item
continue
case .filterList(let componentId):
ContentBlockerManager.log.debug("Disabling filter list \(source.debugDescription)")
await FilterListStorage.shared.ensureFilterList(for: componentId, isEnabled: false)
case .filterListURL(let uuid):
ContentBlockerManager.log.debug("Disabling custom filter list \(source.debugDescription)")
await CustomFilterListStorage.shared.ensureFilterList(for: uuid, isEnabled: false)
}
}
}

let signpostID = Self.signpost.makeSignpostID()
let state = Self.signpost.beginInterval("nonBlockingLaunchTask", id: signpostID)
await FilterListResourceDownloader.shared.start(with: adBlockService)
Expand Down Expand Up @@ -135,7 +114,7 @@ public actor LaunchHelper {
.validBlocklistTypes
// All generic types
.union(
ContentBlockerManager.GenericBlocklistType.allCases.map { .generic($0) }
ContentBlockerManager.BlocklistType.allStaticTypes
)
// All custom filter list urls
.union(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ struct FilterListsView: View {
@State private var expectedEnabledSources: Set<CachedAdBlockEngine.Source> = Set(AdBlockStats.shared.enabledSources)
private let dateFormatter = RelativeDateTimeFormatter()

private var reachedMaxLimit: Bool {
expectedEnabledSources.count >= AdBlockStats.maxNumberOfAllowedFilterLists
}

var body: some View {
List {
Section {
Expand Down Expand Up @@ -85,7 +81,6 @@ struct FilterListsView: View {
.foregroundColor(Color(.secondaryBraveLabel))
}
}
.disabled(!filterList.isEnabled && reachedMaxLimit)
.onChange(of: filterList.isEnabled) { isEnabled in
if isEnabled {
expectedEnabledSources.insert(filterList.engineSource)
Expand Down Expand Up @@ -124,7 +119,6 @@ struct FilterListsView: View {
}
}
}
.disabled(reachedMaxLimit && !filterListURL.setting.isEnabled)
.onChange(of: filterListURL.setting.isEnabled) { isEnabled in
if isEnabled {
expectedEnabledSources.insert(filterListURL.setting.engineSource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ actor ContentBlockerManager {
fileprivate static let filterListPrefix = "filter-list"
fileprivate static let filterListURLPrefix = "filter-list-url"

/// These are all types that are non-configurable by the user
/// and don't need additional stored or fetched catalogues to get a complete list.
static var allStaticTypes: Set<BlocklistType> {
return Set(ContentBlockerManager.GenericBlocklistType.allCases.map { .generic($0) })
}

case generic(GenericBlocklistType)
case filterList(componentId: String, isAlwaysAggressive: Bool)
case customFilterList(uuid: String)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ actor FilterListCustomURLDownloader: ObservableObject {
let uuid = await filterListCustomURL.setting.uuid

// Add/remove the resource depending on if it is enabled/disabled
guard let resourcesInfo = await FilterListResourceDownloader.shared.resourcesInfo else {
guard let resourcesInfo = await AdBlockStats.shared.resourcesInfo else {
assertionFailure("This should not have been called if the resources are not ready")
return
}
Expand Down
96 changes: 57 additions & 39 deletions Sources/Brave/WebFilters/FilterListResourceDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ public actor FilterListResourceDownloader {
/// A marker that says that we loaded shield components for the first time.
/// This boolean is used to configure this downloader only once after `AdBlockService` generic shields have been loaded.
private var registeredFilterLists = false
/// The path to the resources file
private(set) var resourcesInfo: CachedAdBlockEngine.ResourcesInfo?

init() {
self.adBlockServiceTasks = [:]
Expand All @@ -40,27 +38,37 @@ public actor FilterListResourceDownloader {
/// - Warning: This method loads filter list settings.
/// You need to wait for `DataController.shared.initializeOnce()` to be called first before invoking this method
public func loadFilterListSettingsAndCachedData() async {
if let defaultFilterListFolderURL = await FilterListSetting.makeFolderURL(
forFilterListFolderPath: Preferences.AppState.lastFilterListCatalogueComponentFolderPath.value
), FileManager.default.fileExists(atPath: defaultFilterListFolderURL.path),
let resourcesFolderURL = await FilterListSetting.makeFolderURL(
forFilterListFolderPath: Preferences.AppState.lastAdBlockResourcesFolderPath.value
), FileManager.default.fileExists(atPath: resourcesFolderURL.path) {
let resourcesInfo = await didUpdateResourcesComponent(folderURL: resourcesFolderURL)
async let startedCustomFilterListsDownloader: Void = FilterListCustomURLDownloader.shared.startIfNeeded()
async let cachedFilterLists: Void = compileCachedFilterLists(resourcesInfo: resourcesInfo)
async let compileDefaultEngine: Void = compileDefaultEngine(defaultFilterListFolderURL: defaultFilterListFolderURL, resourcesInfo: resourcesInfo)
_ = await (startedCustomFilterListsDownloader, cachedFilterLists, compileDefaultEngine)
} else if let legacyComponentFolderURL = await FilterListSetting.makeFolderURL(
forFilterListFolderPath: Preferences.AppState.lastLegacyDefaultFilterListFolderPath.value
), FileManager.default.fileExists(atPath: legacyComponentFolderURL.path) {
// TODO: @JS Remove this after this release. Its here just so users can upgrade without a pause to their adblocking
let resourcesInfo = await didUpdateResourcesComponent(folderURL: legacyComponentFolderURL)
async let startedCustomFilterListsDownloader: Void = FilterListCustomURLDownloader.shared.startIfNeeded()
async let cachedFilterLists: Void = compileCachedFilterLists(resourcesInfo: resourcesInfo)
async let compileDefaultEngine: Void = compileDefaultEngine(defaultFilterListFolderURL: legacyComponentFolderURL, resourcesInfo: resourcesInfo)
_ = await (startedCustomFilterListsDownloader, cachedFilterLists, compileDefaultEngine)
guard let resourcesFolderURL = await FilterListSetting.makeFolderURL(
forComponentFolderPath: Preferences.AppState.lastAdBlockResourcesFolderPath.value
), FileManager.default.fileExists(atPath: resourcesFolderURL.path) else {
// We need this for all filter lists so we can't compile anything until we download it
return
}

let resourcesInfo = await didUpdateResourcesComponent(folderURL: resourcesFolderURL)
async let startedCustomFilterListsDownloader: Void = FilterListCustomURLDownloader.shared.startIfNeeded()
async let cachedFilterLists: Void = compileCachedFilterLists(resourcesInfo: resourcesInfo)
async let compileDefaultEngine: Void = compileDefaultFilterList(resourcesInfo: resourcesInfo)

_ = await (startedCustomFilterListsDownloader, cachedFilterLists, compileDefaultEngine)
}

/// Compile the default filter list from cache
private func compileDefaultFilterList(resourcesInfo: CachedAdBlockEngine.ResourcesInfo) async {
guard let defaultFilterListFolderURL = await FilterListSetting.makeFolderURL(
forComponentFolderPath: Preferences.AppState.lastFilterListCatalogueComponentFolderPath.value
), FileManager.default.fileExists(atPath: defaultFilterListFolderURL.path) else {
// We don't really need this but its the most important filter list
// so without this not much point compiling anything else because we probably don't have it
return
}

await compileEngine(
filterListFolderURL: defaultFilterListFolderURL, resourcesInfo: resourcesInfo,
engineSource: .adBlock, isAlwaysAggressive: false,
// This is false because we use a slim-list version of the list downloaded from S3
compileContentBlockers: false
)
}

/// This function adds engine resources to `AdBlockManager` from cached data representing the enabled filter lists.
Expand Down Expand Up @@ -106,7 +114,11 @@ public actor FilterListResourceDownloader {
public func start(with adBlockService: AdblockService) {
self.adBlockService = adBlockService

// Start listening to changes to the install url
// Here we register components in a specific order in order to avoid race conditions:
// 1. All our filter lists need the resources component. So we register this first
// 2. We also register the catalogue component since we don't need the resources for this
// 3. When either of the above triggers we check if we have our resources and catalogue
// and if we do have both we register all of the filter lists.
Task { @MainActor in
for await folderURL in adBlockService.resourcesComponentStream() {
guard let folderURL = folderURL else {
Expand All @@ -127,7 +139,7 @@ public actor FilterListResourceDownloader {
for await filterListEntries in adBlockService.filterListCatalogComponentStream() {
FilterListStorage.shared.loadFilterLists(from: filterListEntries)

if await self.resourcesInfo != nil {
if await AdBlockStats.shared.resourcesInfo != nil {
await registerAllFilterListsIfNeeded(with: adBlockService)
}
}
Expand Down Expand Up @@ -156,12 +168,16 @@ public actor FilterListResourceDownloader {
}

await Task { @MainActor in
let folderSubPath = FilterListSetting.extractFolderPath(fromFilterListFolderURL: folderURL)
let folderSubPath = FilterListSetting.extractFolderPath(fromComponentFolderURL: folderURL)
Preferences.AppState.lastFilterListCatalogueComponentFolderPath.value = folderSubPath
}.value

if let resourcesInfo = await self.resourcesInfo {
await compileDefaultEngine(defaultFilterListFolderURL: folderURL, resourcesInfo: resourcesInfo)
if let resourcesInfo = await AdBlockStats.shared.resourcesInfo {
await compileEngine(
filterListFolderURL: folderURL, resourcesInfo: resourcesInfo,
engineSource: .adBlock, isAlwaysAggressive: false,
compileContentBlockers: false // This is set to false because we unfortunately have to use the slim list version of these lists
)
}
}
}
Expand All @@ -171,7 +187,7 @@ public actor FilterListResourceDownloader {
/// When the
private func didUpdateResourcesComponent(folderURL: URL) async -> CachedAdBlockEngine.ResourcesInfo {
await Task { @MainActor in
let folderSubPath = FilterListSetting.extractFolderPath(fromFilterListFolderURL: folderURL)
let folderSubPath = FilterListSetting.extractFolderPath(fromComponentFolderURL: folderURL)
Preferences.AppState.lastAdBlockResourcesFolderPath.value = folderSubPath
}.value

Expand All @@ -181,29 +197,31 @@ public actor FilterListResourceDownloader {
version: version
)

self.resourcesInfo = resourcesInfo
await AdBlockStats.shared.updateIfNeeded(resourcesInfo: resourcesInfo)
return resourcesInfo
}

/// Compile the general engine from the given `AdblockService` `shieldsInstallPath` `URL`.
private func compileDefaultEngine(defaultFilterListFolderURL folderURL: URL, resourcesInfo: CachedAdBlockEngine.ResourcesInfo) async {
// TODO: @JS Remove this on the next update. This is here so users don't have a pause to their ad-blocking
let isLegacy = folderURL.pathExtension == "dat"
let localFileURL = isLegacy ? folderURL.appendingPathComponent("rs-ABPFilterParserData.dat", conformingTo: .data) : folderURL.appendingPathComponent("list.txt", conformingTo: .text)
/// Compile the engine from the given `AdblockService` `shieldsInstallPath` `URL`.
private func compileEngine(
filterListFolderURL folderURL: URL, resourcesInfo: CachedAdBlockEngine.ResourcesInfo,
engineSource: CachedAdBlockEngine.Source,
isAlwaysAggressive: Bool, compileContentBlockers: Bool
) async {
let localFileURL = folderURL.appendingPathComponent("list.txt", conformingTo: .text)

let version = folderURL.lastPathComponent
let filterListInfo = CachedAdBlockEngine.FilterListInfo(
source: .adBlock,
source: engineSource,
localFileURL: localFileURL,
version: version, fileType: isLegacy ? .dat : .text
version: version, fileType: .text
)
let lazyInfo = AdBlockStats.LazyFilterListInfo(
filterListInfo: filterListInfo, isAlwaysAggressive: false
filterListInfo: filterListInfo, isAlwaysAggressive: isAlwaysAggressive
)

await AdBlockStats.shared.compile(
lazyInfo: lazyInfo, resourcesInfo: resourcesInfo,
compileContentBlockers: false
compileContentBlockers: compileContentBlockers
)
}

Expand Down Expand Up @@ -253,7 +271,7 @@ public actor FilterListResourceDownloader {
adBlockServiceTasks[filterList.entry.componentId] = Task { @MainActor in
for await folderURL in adBlockService.register(filterList: filterList) {
guard let folderURL = folderURL else { continue }
guard let resourcesInfo = await self.resourcesInfo else {
guard let resourcesInfo = await AdBlockStats.shared.resourcesInfo else {
assertionFailure("We shouldn't have started downloads before getting this value")
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,6 @@ extension AdblockEngine {
try useResources(fromFileURL: resourcesFileURL)
}

convenience init(datFileURL fileURL: URL, resourcesFileURL: URL) throws {
self.init()

if try !deserialize(data: Data(contentsOf: fileURL)) {
throw CompileError.couldNotDeserializeDATFile
}

try useResources(fromFileURL: resourcesFileURL)
}

/// Combine all resources of type rule lists to one single string
private static func combineAllRuleLists(from infos: [CachedAdBlockEngine.FilterListInfo]) -> String {
// Combine all rule lists that need to be injected during initialization
Expand All @@ -39,8 +29,6 @@ extension AdblockEngine {
}

return String(data: data, encoding: .utf8)
case .dat:
return nil
}
}

Expand Down
Loading

0 comments on commit 9f3feef

Please sign in to comment.