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

Fix #8221: Cleanup logs for ad-block #8223

Merged
merged 1 commit into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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