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

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

@@ -128,6 +128,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 {
@@ -278,7 +282,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)
       }
     }
@@ -364,9 +368,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 = await ContentBlockerManager.shared.ruleLists(for: domainForShields)
+        tab?.contentBlocker.set(ruleLists: ruleLists)
 
         let isScriptsEnabled = !domainForShields.isShieldExpected(.NoScript, considerAllShieldsOption: true)
         preferences.allowsContentJavaScript = isScriptsEnabled
@@ -416,7 +419,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/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

@@ -591,7 +591,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)
       }
 
@@ -633,16 +633,13 @@ 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)
     }
 
@@ -663,7 +660,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/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
+    }
   }
 }

Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift

@@ -313,53 +313,45 @@ final public class ContentBlockerManager: Sendable {
     return ruleList
   }
 
-  /// Return the enabled rule types for this domain and the enabled settings
-  @MainActor public func compiledRuleTypes(for domain: Domain) -> Set<RuleTypeWithSourceType> {
+  @MainActor public func enabledRuleTypes(for domain: Domain) -> Set<ContentBlockerManager.BlocklistRuleType> {
     let filterLists = FilterListResourceDownloader.shared.filterLists
 
     if domain.shield_allOff == 1 {
       return []
     }
 
-    var results = Set<RuleTypeWithSourceType>()
+    var results = Set<ContentBlockerManager.BlocklistRuleType>()
 
     // Get domain specific rule types
     if domain.isShieldExpected(.AdblockAndTp, considerAllShieldsOption: true) {
-      if let sourceType = self.sourceType(for: .general(.blockAds)) {
-        results.insert(RuleTypeWithSourceType(ruleType: .general(.blockAds), sourceType: sourceType))
-      }
-
-      if let sourceType = self.sourceType(for: .general(.blockTrackers)) {
-        results.insert(RuleTypeWithSourceType(ruleType: .general(.blockTrackers), sourceType: sourceType))
-      }
+      results.insert(.general(.blockAds))
+      results.insert(.general(.blockTrackers))
     }
 
     // Get filter list specific rule types
     filterLists.forEach { filterList in
       guard filterList.isEnabled else { return }
-      let ruleType = filterList.makeRuleType()
-
-      guard let sourceType = self.sourceType(for: ruleType) else {
-        return
-      }
-
-      guard cachedRuleList(for: ruleType) != nil else {
-        return
-      }
-
-      results.insert(RuleTypeWithSourceType(ruleType: ruleType, sourceType: sourceType))
+      results.insert(filterList.makeRuleType())
     }
 
     // Get global rule types
     if Preferences.Privacy.blockAllCookies.value {
-      if let sourceType = sourceType(for: .general(.blockCookies)) {
-        results.insert(RuleTypeWithSourceType(ruleType: .general(.blockCookies), sourceType: sourceType))
-      }
+      results.insert(.general(.blockCookies))
     }
 
     return results
   }
 
+  /// Return the enabled rule types for this domain and the enabled settings
+  @MainActor public func ruleLists(for domain: Domain) async -> Set<WKContentRuleList> {
+    let filterLists = FilterListResourceDownloader.shared.filterLists
+    let ruleTypes = enabledRuleTypes(for: domain)
+
+    return await Set(ruleTypes.asyncConcurrentCompactMap { ruleType in
+      return self.cachedRuleList(for: ruleType)
+    })
+  }
+
   /// Return a source type for the given rule type
   public func sourceType(for ruleType: BlocklistRuleType) -> BlocklistSourceType? {
     guard let compileResult = cachedCompileResults[ruleType.identifier] else { return nil }

Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift

@@ -43,13 +43,13 @@ public class AdBlockStats {
   }
 
   /// This returns all the user script types for the given frame
-  @MainActor func makeEngineScriptTypes(frameURL: URL, isMainFrame: Bool, domain: Domain) -> Set<UserScriptType> {
+  @MainActor func makeEngineScriptTypes(frameURL: URL, isMainFrame: Bool, domain: Domain) async -> Set<UserScriptType> {
     // Add any engine scripts for this frame
-    return cachedEngines.enumerated().map({ (index, cachedEngine) -> Set<UserScriptType> in
+    return await cachedEngines.enumerated().asyncMap({ (index, cachedEngine) -> Set<UserScriptType> in
       guard cachedEngine.isEnabled(for: domain) else { return [] }
 
       do {
-        return try cachedEngine.makeEngineScriptTypes(
+        return try await cachedEngine.makeEngineScriptTypes(
           frameURL: frameURL, isMainFrame: isMainFrame, domain: domain, index: index
         )
       } catch {

Sources/Brave/WebFilters/ShieldStats/Adblock/CachedAdBlockEngine.swift

@@ -40,19 +40,28 @@ public class CachedAdBlockEngine {
 
   /// Returns all the models for this frame URL
   /// The results are cached per url, so you may call this method as many times for the same url without any performance implications.
-  @MainActor func cosmeticFilterModel(forFrameURL frameURL: URL) throws -> CosmeticFilterModel? {
-    if let model = self.cachedCosmeticFilterModels.getElement(frameURL) {
-      return model
+  func cosmeticFilterModel(forFrameURL frameURL: URL) async throws -> CosmeticFilterModel? {
+    return try await withCheckedThrowingContinuation { continuation in
+      serialQueue.async { [weak self] in
+        if let model = self?.cachedCosmeticFilterModels.getElement(frameURL) {
+          continuation.resume(returning: model)
+          return
+        }
+
+        do {
+          let model = try self?.engine.cosmeticFilterModel(forFrameURL: frameURL)
+          self?.cachedCosmeticFilterModels.addElement(model, forKey: frameURL)
+          continuation.resume(returning: model)
+        } catch {
+          continuation.resume(throwing: error)
+        }
+      }
     }
-
-    let model = try self.engine.cosmeticFilterModel(forFrameURL: frameURL)
-    self.cachedCosmeticFilterModels.addElement(model, forKey: frameURL)
-    return model
   }
 
   /// Return the selectors that need to be hidden given the frameURL, ids and classes
-  @MainActor func selectorsForCosmeticRules(frameURL: URL, ids: [String], classes: [String]) throws -> [String] {
-    let model = try cosmeticFilterModel(forFrameURL: frameURL)
+  @MainActor func selectorsForCosmeticRules(frameURL: URL, ids: [String], classes: [String]) async throws -> [String] {
+    let model = try await cosmeticFilterModel(forFrameURL: frameURL)
     let selectorsJSON = self.engine.stylesheetForCosmeticRulesIncluding(
       classes: classes,
       ids: ids,
@@ -86,15 +95,15 @@ public class CachedAdBlockEngine {
   }
 
   /// This returns all the user script types for the given frame
-  @MainActor func makeEngineScriptTypes(frameURL: URL, isMainFrame: Bool, domain: Domain, index: Int) throws -> Set<UserScriptType> {
+  @MainActor func makeEngineScriptTypes(frameURL: URL, isMainFrame: Bool, domain: Domain, index: Int) async throws -> Set<UserScriptType> {
     if let userScriptTypes = cachedFrameScriptTypes.getElement(frameURL) {
       return userScriptTypes
     }
 
     // Add the selectors poller scripts for this frame
     var userScriptTypes: Set<UserScriptType> = []
 
-    if let source = try cosmeticFilterModel(forFrameURL: frameURL)?.injectedScript, !source.isEmpty {
+    if let source = try await cosmeticFilterModel(forFrameURL: frameURL)?.injectedScript, !source.isEmpty {
       let configuration = UserScriptType.EngineScriptConfiguration(
         frameURL: frameURL, isMainFrame: isMainFrame, source: source, order: index,
         isDeAMPEnabled: Preferences.Shields.autoRedirectAMPPages.value