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

Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController+WKNavigationDelegate.swift

@@ -129,6 +129,10 @@ 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 {
@@ -279,7 +283,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 = tab?.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
+        let scriptTypes = await tab?.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
         tab?.setCustomUserScript(scripts: scriptTypes)
       }
     }
@@ -350,9 +354,8 @@ extension BrowserViewController: WKNavigationDelegate {
         let domainForShields = Domain.getOrCreate(forUrl: mainDocumentURL, persistent: !isPrivateBrowsing)
 
         // Load rule lists
-        tab?.contentBlocker.ruleListTypes = ContentBlockerManager.shared.compiledRuleTypes(
-          for: domainForShields
-        )
+        let ruleLists = ContentBlockerManager.shared.ruleLists(for: domainForShields)
+        tab?.contentBlocker.set(ruleLists: ruleLists)
 
         let isScriptsEnabled = !domainForShields.isShieldExpected(.NoScript, considerAllShieldsOption: true)
         preferences.allowsContentJavaScript = isScriptsEnabled
@@ -402,7 +405,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 = tab?.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
+      let scriptTypes = await tab?.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
       tab?.setCustomUserScript(scripts: scriptTypes)
     }
 

Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift

@@ -27,6 +27,10 @@ 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()
@@ -39,6 +43,11 @@ 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()

Sources/Brave/Frontend/Browser/LinkPreviewViewController.swift

@@ -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.loadRuleLists(for: Set(compiledRuleTypes.map({ $0.ruleType })))
+      let ruleLists = await ContentBlockerManager.shared.ruleLists(for: domain)
+
       for ruleList in ruleLists {
         webView.configuration.userContentController.add(ruleList)
       }

Sources/Brave/Frontend/Browser/PageData.swift

@@ -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) -> Set<UserScriptType> {
+  @MainActor func makeUserScriptTypes(domain: Domain) async -> Set<UserScriptType> {
     var userScriptTypes: Set<UserScriptType> = [.siteStateListener]
 
     // Handle dynamic domain level scripts on the main document.
@@ -98,18 +98,26 @@ 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
-    userScriptTypes = userScriptTypes.union(
-      adBlockStats.makeEngineScriptTypes(frameURL: mainFrameURL, isMainFrame: true, domain: domain)
-    )
+    async let engineScripts = adBlockStats.makeEngineScriptTypes(frameURL: mainFrameURL, isMainFrame: true, domain: domain)
 
     // Add engine scripts for all of the known sub-frames
-    let additionalScriptTypes = allSubframeURLs.map({ frameURL in
-      return self.adBlockStats.makeEngineScriptTypes(frameURL: frameURL, isMainFrame: false, domain: domain)
+    async let additionalScriptTypes = allSubframeURLs.asyncConcurrentCompactMap({ frameURL in
+      return await self.adBlockStats.makeEngineScriptTypes(frameURL: frameURL, isMainFrame: false, domain: domain)
     }).reduce(Set<UserScriptType>(), { partialResult, scriptTypes in
       return partialResult.union(scriptTypes)
     })
 
-    return userScriptTypes.union(additionalScriptTypes)
+    let allEngineScripts = await (mainFrame: engineScripts, subFrames: additionalScriptTypes)
+    return allEngineScripts.mainFrame.union(allEngineScripts.subFrames)
   }
 }

Sources/Brave/Frontend/Browser/Playlist/Managers & Cache/PlaylistCacheLoader.swift

@@ -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 = tab.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
+        let scriptTypes = await tab.currentPageData?.makeUserScriptTypes(domain: domainForMainFrame) ?? []
         tab.setCustomUserScript(scripts: scriptTypes)
       }
 
@@ -584,16 +584,15 @@ extension PlaylistWebLoader: WKNavigationDelegate {
         domainForShields.shield_adblockAndTp = true
 
         // Load block lists
-        let enabledRuleTypes = ContentBlockerManager.shared.compiledRuleTypes(for: domainForShields)
-        tab.contentBlocker.ruleListTypes = enabledRuleTypes
+        let ruleLists = await ContentBlockerManager.shared.ruleLists(for: domainForShields)
+        tab.contentBlocker.set(ruleLists: ruleLists)
 
         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)
     }
 
@@ -614,7 +613,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 = tab.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
+      let scriptTypes = await tab.currentPageData?.makeUserScriptTypes(domain: domain) ?? []
       tab.setCustomUserScript(scripts: scriptTypes)
     }
 

Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Paged/ContentBlockerScriptHandler.swift

@@ -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 = Set(self.loadedRuleTypeWithSourceTypes.map({ $0.ruleType }))
+        let loadedRuleTypes = ContentBlockerManager.shared.enabledRuleTypes(for: domain)
 
         let blockedType = await TPStatsBlocklistChecker.shared.blockedTypes(
           requestURL: requestURL,

Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/ScriptHandlers/Paged/CosmeticFiltersScriptHandler.swift

@@ -58,7 +58,7 @@ class CosmeticFiltersScriptHandler: TabContentScript {
 
         let selectorArrays = await cachedEngines.asyncConcurrentMap { cachedEngine -> [String] in
           do {
-            return try cachedEngine.selectorsForCosmeticRules(
+            return try await cachedEngine.selectorsForCosmeticRules(
               frameURL: frameURL,
               ids: dto.data.ids,
               classes: dto.data.classes

Sources/Brave/WebFilters/AdBlockEngineManager.swift

@@ -184,41 +184,39 @@ public actor AdBlockEngineManager: Sendable {
 }
 
 #if DEBUG
-extension AdBlockEngineManager {
+private extension AdBlockEngineManager {
   /// A method that logs info on the given resources
-  fileprivate func debug(compiledResults: [ResourceWithVersion: Result<Void, Error>]) {
-    let resourcesString = compiledResults.sorted(by: { $0.key.order < $1.key.order })
-      .map { (resourceWithVersion, compileResult) -> String in
+  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
         let resultString: String
 
         switch compileResult {
         case .success:
-          resultString = "success"
+          resultString = "✔︎"
         case .failure(let error):
-          resultString = error.localizedDescription
+          resultString = "\(error)"
         }
 
         let sourceDebugString = [
-          resourceWithVersion.debugDescription,
-          "result: \(resultString)",
-        ].joined(separator: ", ")
+          "", resourceWithVersion.debugDescription,
+          "\(resultString)",
+        ].joined(separator: " ")
 
-        return ["{", sourceDebugString, "}"].joined()
-      }.joined(separator: ", ")
-
-    log.debug("Loaded \(self.enabledResources.count, privacy: .public) (total) engine resources: \(resourcesString, privacy: .public)")
+        log.debug("\(sourceDebugString)")
+      }
   }
 }
 
 extension AdBlockEngineManager.ResourceWithVersion: CustomDebugStringConvertible {
   public var debugDescription: String {
     return [
-      "order: \(order)",
-      "fileName: \(fileURL.lastPathComponent)",
-      "source: \(resource.source.debugDescription)",
-      "version: \(version ?? "nil")",
-      "type: \(resource.type.debugDescription)"
-    ].joined(separator: ", ")
+      "#\(order)",
+      "\(resource.source.debugDescription).\(resource.type.debugDescription)",
+      "v\(version ?? "nil")",
+    ].joined(separator: " ")
   }
 }
 

Sources/Brave/WebFilters/ContentBlocker/ContentBlockerHelper.swift

@@ -45,12 +45,8 @@ enum BlockingStrength: String {
 class ContentBlockerHelper {
   private(set) weak var tab: Tab?
 
-  /// 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() }
-  }
+  /// The rule lists that are loaded into the current tab
+  private var setRuleLists: Set<WKContentRuleList> = []
 
   var stats: TPPageStats = TPPageStats() {
     didSet {
@@ -69,58 +65,31 @@ class ContentBlockerHelper {
     self.tab = tab
   }
 
-  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
-      }
-
-      // Since either it shouldn't be included or the source type doesn't match, we remove it
+  @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)
-      loadedRuleTypeWithSourceTypes.remove(loadedRuleTypeWithSourceType)
+      setRuleLists.remove(ruleList)
+
+      #if DEBUG
+      ContentBlockerManager.log.debug(" - \(ruleList.identifier)")
+      #endif
     }
 
-    // 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 }
+    // Add missing rule lists
+    for ruleList in ruleLists.subtracting(setRuleLists) {
       tab?.webView?.configuration.userContentController.add(ruleList)
-      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))"
-      }
+      setRuleLists.insert(ruleList)
 
-      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
+      #if DEBUG
+      ContentBlockerManager.log.debug(" + \(ruleList.identifier)")
+      #endif
+    }
   }
 }