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

Commit

Permalink
Fix #8277: Crash on old devices due to too many filter lists enabled (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
cuba authored and iccub committed Oct 18, 2023
1 parent e71008c commit 863cfc8
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 49 deletions.
20 changes: 20 additions & 0 deletions Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,26 @@ 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.enabledSources
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ struct FilterListsView: View {
} header: {
Text(Strings.customFilterLists)
}
.listRowBackground(Color(.secondaryBraveGroupedBackground))
.toggleStyle(SwitchToggleStyle(tint: .accentColor))

Section {
Expand All @@ -52,10 +53,9 @@ struct FilterListsView: View {
Text(Strings.filterListsDescription)
.textCase(.none)
}
}
}.listRowBackground(Color(.secondaryBraveGroupedBackground))
}
.toggleStyle(SwitchToggleStyle(tint: .accentColor))
.listRowBackground(Color(.secondaryBraveGroupedBackground))
.animation(.default, value: customFilterListStorage.filterListsURLs)
.listBackgroundColor(Color(UIColor.braveGroupedBackground))
.listStyle(.insetGrouped)
Expand Down
11 changes: 11 additions & 0 deletions Sources/Brave/WebFilters/CustomFilterListStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ import Data
}
}
}

/// Ensures that the settings for a filter list are stored
/// - Parameters:
/// - uuid: The uuid of the filter list to update
/// - isEnabled: A boolean indicating if the filter list is enabled or not
public func ensureFilterList(for uuid: String, isEnabled: Bool) {
// Enable the setting
if let index = filterListsURLs.firstIndex(where: { $0.setting.uuid == uuid }) {
filterListsURLs[index].setting.isEnabled = isEnabled
}
}

func update(filterListId id: ObjectIdentifier, with result: Result<Date, Error>) {
guard let index = filterListsURLs.firstIndex(where: { $0.id == id }) else {
Expand Down
12 changes: 4 additions & 8 deletions Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,10 @@ actor FilterListCustomURLDownloader: ObservableObject {
return
}

do {
try await AdBlockStats.shared.compileDelayed(
filterListInfo: filterListInfo, resourcesInfo: resourcesInfo,
isAlwaysAggressive: true, delayed: true
)
} catch {
// Don't handle cancellation errors
}
await AdBlockStats.shared.compile(
filterListInfo: filterListInfo, resourcesInfo: resourcesInfo,
isAlwaysAggressive: true
)
}

/// Start fetching the resource for the given filter list. Once a new version is downloaded, the file will be processed using the `handle` method
Expand Down
14 changes: 4 additions & 10 deletions Sources/Brave/WebFilters/FilterListResourceDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,10 @@ public actor FilterListResourceDownloader {
return
}

let isImportant = await AdBlockStats.shared.criticalSources.contains(source)

do {
try await AdBlockStats.shared.compileDelayed(
filterListInfo: filterListInfo, resourcesInfo: resourcesInfo,
isAlwaysAggressive: isAlwaysAggressive, delayed: !isImportant
)
} catch {
// Don't handle cancellation errors
}
await AdBlockStats.shared.compile(
filterListInfo: filterListInfo, resourcesInfo: resourcesInfo,
isAlwaysAggressive: isAlwaysAggressive
)
}

/// Register all enabled filter lists with the `AdBlockService`
Expand Down
40 changes: 11 additions & 29 deletions Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ import os.log
/// This object holds on to our adblock engines and returns information needed for stats tracking as well as some conveniences
/// for injected scripts needed during web navigation and cosmetic filters models needed by the `SelectorsPollerScript.js` script.
public actor AdBlockStats {
static let maxNumberOfAllowedFilterLists = 30
/// The max number of enabled filter lists depending on the amount memory available to the device
static var maxNumberOfAllowedFilterLists: Int = {
let memory = Int(ProcessInfo.processInfo.physicalMemory / 1073741824)
ContentBlockerManager.log.debug("Memory: \(memory)")
return min(5 * memory, 40)
}()

typealias CosmeticFilterModelTuple = (isAlwaysAggressive: Bool, model: CosmeticFilterModel)
public static let shared = AdBlockStats()

Expand Down Expand Up @@ -63,33 +69,13 @@ public actor AdBlockStats {
await removeDisabledEngines()
}

public func compileDelayed(
filterListInfo: CachedAdBlockEngine.FilterListInfo, resourcesInfo: CachedAdBlockEngine.ResourcesInfo,
isAlwaysAggressive: Bool, delayed: Bool
) async throws {
if delayed {
Task.detached(priority: .background) {
ContentBlockerManager.log.debug("Delaying \(filterListInfo.source.debugDescription)")
await self.compile(
filterListInfo: filterListInfo, resourcesInfo: resourcesInfo,
isAlwaysAggressive: isAlwaysAggressive
)
}
} else {
await self.compile(
filterListInfo: filterListInfo, resourcesInfo: resourcesInfo,
isAlwaysAggressive: isAlwaysAggressive
)
}
}

/// Create and add an engine from the given resources.
/// If an engine already exists for the given source, it will be replaced.
///
/// - Note: This method will ensure syncronous compilation
public func compile(
filterListInfo: CachedAdBlockEngine.FilterListInfo, resourcesInfo: CachedAdBlockEngine.ResourcesInfo,
isAlwaysAggressive: Bool
isAlwaysAggressive: Bool, ignoreMaximum: Bool = false
) async {
await currentCompileTask?.value

Expand All @@ -99,7 +85,7 @@ public actor AdBlockStats {
}

currentCompileTask = Task {
if reachedMaxLimit && cachedEngines[filterListInfo.source] == nil {
if !ignoreMaximum && reachedMaxLimit && cachedEngines[filterListInfo.source] == nil {
ContentBlockerManager.log.error("Failed to compile engine for \(filterListInfo.source.debugDescription): Reached maximum!")
return
}
Expand Down Expand Up @@ -171,22 +157,18 @@ public actor AdBlockStats {
/// Remove all engines that have disabled sources
func ensureEnabledEngines() async {
do {
var count = 0

for source in await enabledSources {
guard cachedEngines[source] == nil else { continue }
guard let availableFilterList = availableFilterLists[source] else { continue }
guard let resourcesInfo = self.resourcesInfo else { continue }

try await compileDelayed(
await compile(
filterListInfo: availableFilterList.filterListInfo,
resourcesInfo: resourcesInfo,
isAlwaysAggressive: availableFilterList.isAlwaysAggressive,
delayed: count > 5
isAlwaysAggressive: availableFilterList.isAlwaysAggressive
)

// Sleep for 1ms. This drastically reduces memory usage without much impact to usability
count += 1
try await Task.sleep(nanoseconds: 1000000)
}
} catch {
Expand Down

0 comments on commit 863cfc8

Please sign in to comment.