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

Revert "Fix #6254: Fix engine crashes by using serial queue for cosmetic filters" #7107

Merged
merged 1 commit into from
Mar 20, 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
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,6 @@ extension BrowserViewController: WKNavigationDelegate {
guard let url = navigationAction.request.url else {
return (.cancel, preferences)
}

#if DEBUG
ContentBlockerManager.log.debug("Nav: \(url.absoluteString)")
#endif

if InternalURL.isValid(url: url) {
if navigationAction.navigationType != .backForward, navigationAction.isInternalUnprivileged {
Expand Down Expand Up @@ -283,7 +279,7 @@ extension BrowserViewController: WKNavigationDelegate {
// Check if custom user scripts must be added to or removed from the web view.
if let targetFrame = navigationAction.targetFrame {
tab?.currentPageData?.addSubframeURL(forRequestURL: url, isForMainFrame: targetFrame.isMainFrame)
let scriptTypes = await tab?.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
let scriptTypes = tab?.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
tab?.setCustomUserScript(scripts: scriptTypes)
}
}
Expand Down Expand Up @@ -354,8 +350,9 @@ extension BrowserViewController: WKNavigationDelegate {
let domainForShields = Domain.getOrCreate(forUrl: mainDocumentURL, persistent: !isPrivateBrowsing)

// Load rule lists
let ruleLists = ContentBlockerManager.shared.ruleLists(for: domainForShields)
tab?.contentBlocker.set(ruleLists: ruleLists)
tab?.contentBlocker.ruleListTypes = ContentBlockerManager.shared.compiledRuleTypes(
for: domainForShields
)

let isScriptsEnabled = !domainForShields.isShieldExpected(.NoScript, considerAllShieldsOption: true)
preferences.allowsContentJavaScript = isScriptsEnabled
Expand Down Expand Up @@ -405,7 +402,7 @@ extension BrowserViewController: WKNavigationDelegate {
if let responseURL = responseURL,
let domain = tab?.currentPageData?.domain(persistent: !isPrivateBrowsing),
tab?.currentPageData?.upgradeFrameURL(forResponseURL: responseURL, isForMainFrame: navigationResponse.isForMainFrame) == true {
let scriptTypes = await tab?.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
let scriptTypes = tab?.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
tab?.setCustomUserScript(scripts: scriptTypes)
}

Expand Down
9 changes: 0 additions & 9 deletions Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ public actor LaunchHelper {

// Otherwise prepare the services and await the task
let task = Task {
#if DEBUG
let startTime = CFAbsoluteTimeGetCurrent()
#endif

// Load cached data
// This is done first because compileResources and loadCachedRuleLists need their results
async let loadBundledResources: Void = ContentBlockerManager.shared.loadBundledResources()
Expand All @@ -43,11 +39,6 @@ public actor LaunchHelper {
async let cachedRuleListLoad: Void = ContentBlockerManager.shared.loadCachedRuleLists()
_ = await (compiledResourcesCompile, cachedRuleListLoad)

#if DEBUG
let timeElapsed = CFAbsoluteTimeGetCurrent() - startTime
ContentBlockerManager.log.debug("Adblock loaded: \(timeElapsed)s")
#endif

// This one is non-blocking
performPostLoadTasks(adBlockService: adBlockService)
markAdBlockReady()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ class LinkPreviewViewController: UIViewController {
}

let domain = Domain.getOrCreate(forUrl: url, persistent: !PrivateBrowsingManager.shared.isPrivateBrowsing)
let compiledRuleTypes = ContentBlockerManager.shared.compiledRuleTypes(for: domain)

Task(priority: .userInitiated) {
let ruleLists = await ContentBlockerManager.shared.ruleLists(for: domain)

let ruleLists = await ContentBlockerManager.shared.loadRuleLists(for: Set(compiledRuleTypes.map({ $0.ruleType })))
for ruleList in ruleLists {
webView.configuration.userContentController.add(ruleList)
}
Expand Down
22 changes: 7 additions & 15 deletions Sources/Brave/Frontend/Browser/PageData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ struct PageData {
}

/// Return all the user script types for this page. The number of script types grows as more frames are loaded.
@MainActor func makeUserScriptTypes(domain: Domain) async -> Set<UserScriptType> {
@MainActor func makeUserScriptTypes(domain: Domain) -> Set<UserScriptType> {
var userScriptTypes: Set<UserScriptType> = [.siteStateListener]

// Handle dynamic domain level scripts on the main document.
Expand Down Expand Up @@ -98,26 +98,18 @@ struct PageData {
}
}

let allEngineScriptTypes = await makeAllEngineScripts(for: domain)
return userScriptTypes.union(allEngineScriptTypes)
}

func makeMainFrameEngineScriptTypes(domain: Domain) async -> Set<UserScriptType> {
return await adBlockStats.makeEngineScriptTypes(frameURL: mainFrameURL, isMainFrame: true, domain: domain)
}

func makeAllEngineScripts(for domain: Domain) async -> Set<UserScriptType> {
// Add engine scripts for the main frame
async let engineScripts = adBlockStats.makeEngineScriptTypes(frameURL: mainFrameURL, isMainFrame: true, domain: domain)
userScriptTypes = userScriptTypes.union(
adBlockStats.makeEngineScriptTypes(frameURL: mainFrameURL, isMainFrame: true, domain: domain)
)

// Add engine scripts for all of the known sub-frames
async let additionalScriptTypes = allSubframeURLs.asyncConcurrentCompactMap({ frameURL in
return await self.adBlockStats.makeEngineScriptTypes(frameURL: frameURL, isMainFrame: false, domain: domain)
let additionalScriptTypes = allSubframeURLs.map({ frameURL in
return self.adBlockStats.makeEngineScriptTypes(frameURL: frameURL, isMainFrame: false, domain: domain)
}).reduce(Set<UserScriptType>(), { partialResult, scriptTypes in
return partialResult.union(scriptTypes)
})

let allEngineScripts = await (mainFrame: engineScripts, subFrames: additionalScriptTypes)
return allEngineScripts.mainFrame.union(allEngineScripts.subFrames)
return userScriptTypes.union(additionalScriptTypes)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ extension PlaylistWebLoader: WKNavigationDelegate {
if let requestURL = navigationAction.request.url,
let targetFrame = navigationAction.targetFrame {
tab.currentPageData?.addSubframeURL(forRequestURL: requestURL, isForMainFrame: targetFrame.isMainFrame)
let scriptTypes = await tab.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
let scriptTypes = tab.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
tab.setCustomUserScript(scripts: scriptTypes)
}

Expand All @@ -584,15 +584,16 @@ extension PlaylistWebLoader: WKNavigationDelegate {
domainForShields.shield_adblockAndTp = true

// Load block lists
let ruleLists = await ContentBlockerManager.shared.ruleLists(for: domainForShields)
tab.contentBlocker.set(ruleLists: ruleLists)
let enabledRuleTypes = ContentBlockerManager.shared.compiledRuleTypes(for: domainForShields)
tab.contentBlocker.ruleListTypes = enabledRuleTypes

let isScriptsEnabled = !domainForShields.isShieldExpected(.NoScript, considerAllShieldsOption: true)
preferences.allowsContentJavaScript = isScriptsEnabled
}

// Cookie Blocking code below
tab.setScript(script: .cookieBlocking, enabled: Preferences.Privacy.blockAllCookies.value)

return (.allow, preferences)
}

Expand All @@ -613,7 +614,7 @@ extension PlaylistWebLoader: WKNavigationDelegate {
if let responseURL = responseURL,
tab.currentPageData?.upgradeFrameURL(forResponseURL: responseURL, isForMainFrame: navigationResponse.isForMainFrame) == true,
let domain = tab.currentPageData?.domain(persistent: false) {
let scriptTypes = await tab.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
let scriptTypes = tab.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
tab.setCustomUserScript(scripts: scriptTypes)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ extension ContentBlockerHelper: TabContentScript {
guard let requestURL = NSURL(idnString: dto.data.resourceURL) as URL? else { return }
guard let sourceURL = NSURL(idnString: dto.data.sourceURL) as URL? else { return }
guard let domainURLString = domain.url else { return }
let loadedRuleTypes = ContentBlockerManager.shared.enabledRuleTypes(for: domain)
let loadedRuleTypes = Set(self.loadedRuleTypeWithSourceTypes.map({ $0.ruleType }))

let blockedType = await TPStatsBlocklistChecker.shared.blockedTypes(
requestURL: requestURL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class CosmeticFiltersScriptHandler: TabContentScript {

let selectorArrays = await cachedEngines.asyncConcurrentMap { cachedEngine -> [String] in
do {
return try await cachedEngine.selectorsForCosmeticRules(
return try cachedEngine.selectorsForCosmeticRules(
frameURL: frameURL,
ids: dto.data.ids,
classes: dto.data.classes
Expand Down
36 changes: 19 additions & 17 deletions Sources/Brave/WebFilters/AdBlockEngineManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,39 +184,41 @@ public actor AdBlockEngineManager: Sendable {
}

#if DEBUG
private extension AdBlockEngineManager {
extension AdBlockEngineManager {
/// A method that logs info on the given resources
func debug(compiledResults: [ResourceWithVersion: Result<Void, Error>]) {
log.debug("Loaded \(compiledResults.count) (total) engine resources:")

compiledResults.sorted(by: { $0.key.order < $1.key.order })
.forEach { (resourceWithVersion, compileResult) in
fileprivate func debug(compiledResults: [ResourceWithVersion: Result<Void, Error>]) {
let resourcesString = compiledResults.sorted(by: { $0.key.order < $1.key.order })
.map { (resourceWithVersion, compileResult) -> String in
let resultString: String

switch compileResult {
case .success:
resultString = "✔︎"
resultString = "success"
case .failure(let error):
resultString = "\(error)"
resultString = error.localizedDescription
}

let sourceDebugString = [
"", resourceWithVersion.debugDescription,
"\(resultString)",
].joined(separator: " ")
resourceWithVersion.debugDescription,
"result: \(resultString)",
].joined(separator: ", ")

log.debug("\(sourceDebugString)")
}
return ["{", sourceDebugString, "}"].joined()
}.joined(separator: ", ")

log.debug("Loaded \(self.enabledResources.count, privacy: .public) (total) engine resources: \(resourcesString, privacy: .public)")
}
}

extension AdBlockEngineManager.ResourceWithVersion: CustomDebugStringConvertible {
public var debugDescription: String {
return [
"#\(order)",
"\(resource.source.debugDescription).\(resource.type.debugDescription)",
"v\(version ?? "nil")",
].joined(separator: " ")
"order: \(order)",
"fileName: \(fileURL.lastPathComponent)",
"source: \(resource.source.debugDescription)",
"version: \(version ?? "nil")",
"type: \(resource.type.debugDescription)"
].joined(separator: ", ")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,12 @@ enum BlockingStrength: String {
class ContentBlockerHelper {
private(set) weak var tab: Tab?

/// The rule lists that are loaded into the current tab
private var setRuleLists: Set<WKContentRuleList> = []
/// The rule types and source types that are currently loaded in this tab
private(set) var loadedRuleTypeWithSourceTypes: Set<ContentBlockerManager.RuleTypeWithSourceType> = []
/// The rule types with their source types that should be loaded in this tab
var ruleListTypes: Set<ContentBlockerManager.RuleTypeWithSourceType> = [] {
didSet { reloadNeededRuleLists() }
}

var stats: TPPageStats = TPPageStats() {
didSet {
Expand All @@ -65,31 +69,58 @@ class ContentBlockerHelper {
self.tab = tab
}

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

// 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)
private func reloadNeededRuleLists() {
// Remove old values that were previously added
for loadedRuleTypeWithSourceType in loadedRuleTypeWithSourceTypes {
// Check if should be removed or if the source type doesn't match, otherwise don't do anything
guard !ruleListTypes.contains(loadedRuleTypeWithSourceType) else {
continue
}

guard let ruleList = ContentBlockerManager.shared.cachedRuleList(for: loadedRuleTypeWithSourceType.ruleType) else {
// For some reason the rule is not cached. Shouldn't happen.
// But if it does we have to remove all the rule lists
// We will add back all the necessary ones below
tab?.webView?.configuration.userContentController.removeAllContentRuleLists()
loadedRuleTypeWithSourceTypes = []
assertionFailure("This shouldn't happen!")
break
}

#if DEBUG
ContentBlockerManager.log.debug(" - \(ruleList.identifier)")
#endif
// Since either it shouldn't be included or the source type doesn't match, we remove it
tab?.webView?.configuration.userContentController.remove(ruleList)
loadedRuleTypeWithSourceTypes.remove(loadedRuleTypeWithSourceType)
}

// Add missing rule lists
for ruleList in ruleLists.subtracting(setRuleLists) {
// Add new values that are not yet added (or were removed above because the source type didn't match)
for ruleTypeWithSourceType in ruleListTypes {
// Only add rule lists that are missing
guard !loadedRuleTypeWithSourceTypes.contains(ruleTypeWithSourceType) else { continue }
guard let ruleList = ContentBlockerManager.shared.cachedRuleList(for: ruleTypeWithSourceType.ruleType) else { continue }
tab?.webView?.configuration.userContentController.add(ruleList)
setRuleLists.insert(ruleList)

#if DEBUG
ContentBlockerManager.log.debug(" + \(ruleList.identifier)")
#endif
loadedRuleTypeWithSourceTypes.insert(ruleTypeWithSourceType)
}

#if DEBUG
let rulesString = loadedRuleTypeWithSourceTypes.map { ruleTypeWithSourceType -> String in
let ruleTypeString: String

switch ruleTypeWithSourceType.ruleType {
case .general(let type):
ruleTypeString = type.rawValue
case .filterList(let uuid):
ruleTypeString = "filterList(\(uuid))"
}

let rulesDebugString = [
"ruleType: \(ruleTypeString)",
"sourceType: \(ruleTypeWithSourceType.sourceType)"
].joined(separator: ", ")

return ["{", rulesDebugString, "}"].joined()
}.joined(separator: ", ")

log.debug("Loaded \(self.loadedRuleTypeWithSourceTypes.count, privacy: .public) tab rules: \(rulesString, privacy: .public)")
#endif
}
}
Loading