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

Commit

Permalink
Fix #8221: Cleanup logs for ad-block (#8223)
Browse files Browse the repository at this point in the history
  • Loading branch information
cuba authored Oct 17, 2023
1 parent 178fb2b commit 0db7215
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 44 deletions.
19 changes: 9 additions & 10 deletions Sources/Brave/Frontend/Browser/UserScriptManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -280,16 +280,15 @@ class UserScriptManager {
return
}

#if DEBUG
ContentBlockerManager.log.debug("Loaded \(userScripts.count + customScripts.count) scripts:")
userScripts.sorted(by: { $0.rawValue < $1.rawValue}).forEach { scriptType in
ContentBlockerManager.log.debug(" \(scriptType.debugDescription)")
}
customScripts.sorted(by: { $0.order < $1.order}).forEach { scriptType in
ContentBlockerManager.log.debug(" #\(scriptType.order) \(scriptType.debugDescription)")
}
#endif

let logComponents = [
userScripts.sorted(by: { $0.rawValue < $1.rawValue}).map { scriptType in
" \(scriptType.debugDescription)"
}.joined(separator: "\n"),
customScripts.sorted(by: { $0.order < $1.order}).map { scriptType in
" #\(scriptType.order) \(scriptType.debugDescription)"
}.joined(separator: "\n")
]
ContentBlockerManager.log.debug("Loaded \(userScripts.count + customScripts.count) script(s): \n\(logComponents.joined(separator: "\n"))")
loadScripts(into: webView, scripts: userScripts)

webView.configuration.userContentController.do { scriptController in
Expand Down
3 changes: 1 addition & 2 deletions Sources/Brave/WebFilters/AdblockResourceDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public actor AdblockResourceDownloader: Sendable {
// .blockAds is special because it can be replaced by a downloaded file.
// Hence we need to first check if it already exists.
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
Expand Down Expand Up @@ -154,7 +153,7 @@ public actor AdblockResourceDownloader: Sendable {
)
} catch {
ContentBlockerManager.log.error(
"Failed to compile downloaded content blocker resource: \(error.localizedDescription)"
"Failed to compile rule lists for `\(blocklistType.debugDescription)`: \(error.localizedDescription)"
)
}

Expand Down
22 changes: 11 additions & 11 deletions Sources/Brave/WebFilters/ContentBlocker/ContentBlockerHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,29 +67,29 @@ class ContentBlockerHelper {

@MainActor func set(ruleLists: Set<WKContentRuleList>) {
guard ruleLists != setRuleLists else { return }
#if DEBUG
ContentBlockerManager.log.debug("Set rule lists:")
#endif
var addedIds: [String] = []
var removedIds: [String] = []

// Remove unwanted rule lists
for ruleList in setRuleLists.subtracting(ruleLists) {
// It's added but we don't want it. So we remove it.
tab?.webView?.configuration.userContentController.remove(ruleList)
setRuleLists.remove(ruleList)

#if DEBUG
ContentBlockerManager.log.debug(" - \(ruleList.identifier)")
#endif
removedIds.append(ruleList.identifier)
}

// Add missing rule lists
for ruleList in ruleLists.subtracting(setRuleLists) {
tab?.webView?.configuration.userContentController.add(ruleList)
setRuleLists.insert(ruleList)

#if DEBUG
ContentBlockerManager.log.debug(" + \(ruleList.identifier)")
#endif
addedIds.append(ruleList.identifier)
}

let parts = [
addedIds.sorted(by: { $0 < $1 }).map({ " + \($0)" }).joined(separator: "\n"),
removedIds.sorted(by: { $0 < $1 }).map({ " - \($0)" }).joined(separator: "\n")
]

ContentBlockerManager.log.debug("Set rule lists:\n\(parts.joined(separator: "\n"))")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ actor ContentBlockerManager {
}

/// An object representing the type of block list
public enum BlocklistType: Hashable {
public enum BlocklistType: Hashable, CustomDebugStringConvertible {
fileprivate static let genericPrifix = "stored-type"
fileprivate static let filterListPrefix = "filter-list"
fileprivate static let filterListURLPrefix = "filter-list-url"
Expand Down Expand Up @@ -126,6 +126,10 @@ actor ContentBlockerManager {
return [self.identifier, "standard"].joined(separator: "-")
}
}

public var debugDescription: String {
return identifier
}
}

public static var shared = ContentBlockerManager()
Expand Down Expand Up @@ -176,7 +180,7 @@ actor ContentBlockerManager {
let cleanedRuleList: [[String: Any?]]

do {
cleanedRuleList = try await process(encodedContentRuleList: encodedContentRuleList, with: options)
cleanedRuleList = try await process(encodedContentRuleList: encodedContentRuleList, for: type, with: options)
} catch {
for mode in modes {
self.cachedRuleLists[type.makeIdentifier(for: mode)] = .failure(error)
Expand All @@ -193,10 +197,10 @@ actor ContentBlockerManager {
do {
let ruleList = try await compile(ruleList: moddedRuleList, for: type, mode: mode)
self.cachedRuleLists[identifier] = .success(ruleList)
Self.log.debug("Compiled content blockers for `\(identifier)`")
Self.log.debug("Compiled rule list for `\(identifier)`")
} catch {
self.cachedRuleLists[identifier] = .failure(error)
Self.log.debug("Failed to compile content blockers for `\(identifier)`: \(String(describing: error))")
Self.log.debug("Failed to compile rule list for `\(identifier)`: \(String(describing: error))")
foundError = error
}
}
Expand Down Expand Up @@ -378,7 +382,9 @@ actor ContentBlockerManager {
do {
return try await self.ruleList(for: blocklistType, mode: mode)
} catch {
Self.log.error("Missing rule list for `\(blocklistType.makeIdentifier(for: mode))`")
// We can't log the error because some rules have empty rules. This is normal
// But on relaunches we try to reload the filter list and this will give us an error.
// Need to find a more graceful way of handling this so error here can be logged properly
return nil
}
}))
Expand Down Expand Up @@ -406,13 +412,12 @@ actor ContentBlockerManager {
}

/// Perform operations of the rule list given by the provided options
func process(encodedContentRuleList: String, with options: CompileOptions) async throws -> [[String: Any?]] {
func process(encodedContentRuleList: String, for type: BlocklistType, with options: CompileOptions) async throws -> [[String: Any?]] {
var ruleList = try decode(encodedContentRuleList: encodedContentRuleList)
if options.isEmpty { return ruleList }

#if DEBUG
let originalCount = ruleList.count
ContentBlockerManager.log.debug("Cleanining up \(originalCount) rules")
#endif

if options.contains(.stripContentBlockers) {
Expand All @@ -425,7 +430,9 @@ actor ContentBlockerManager {

#if DEBUG
let count = originalCount - ruleList.count
ContentBlockerManager.log.debug("Filtered out \(count) rules")
if count > 0 {
Self.log.debug("Filtered out \(count) rules for `\(type.debugDescription)`")
}
#endif

return ruleList
Expand Down
10 changes: 3 additions & 7 deletions Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,22 @@ 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<DownloadResource>.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)
let type = ContentBlockerManager.BlocklistType.customFilterList(uuid: uuid)

try await ContentBlockerManager.shared.compile(
encodedContentRuleList: result.rulesJSON,
for: type,
for: blocklistType,
options: .all
)

ContentBlockerManager.log.debug(
"Loaded custom filter list content blockers: \(String(describing: type))"
)
} catch {
ContentBlockerManager.log.error(
"Failed to convert custom filter list to content blockers: \(error.localizedDescription)"
"Failed to compile rule lists for `\(blocklistType.debugDescription)`: \(error.localizedDescription)"
)
}
}
Expand Down
7 changes: 1 addition & 6 deletions Sources/Brave/WebFilters/FilterListResourceDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -298,13 +298,8 @@ public actor FilterListResourceDownloader {
}
} catch {
ContentBlockerManager.log.error(
"Failed to create content blockers for `\(componentId)` v\(version): \(error)"
"Failed to create content blockers for `\(blocklistType.debugDescription)` v\(version): \(error)"
)
#if DEBUG
ContentBlockerManager.log.debug(
"`\(componentId)`: \(filterListURL.absoluteString)"
)
#endif
}
}
}
Expand Down

0 comments on commit 0db7215

Please sign in to comment.