From 82460848afbe36b14a1486c9dce92d678edd2e64 Mon Sep 17 00:00:00 2001 From: Jacob Sikorski Date: Wed, 7 Feb 2024 15:49:55 -0700 Subject: [PATCH] Fix compiling only necessary files during launch --- .../BVC+WKNavigationDelegate.swift | 8 +- .../Browser/Helpers/LaunchHelper.swift | 4 +- .../Browser/User Scripts/UserScriptType.swift | 2 +- .../ContentBlockerManager.swift | 4 + Sources/Brave/WebFilters/FilterList.swift | 4 + .../FilterListCustomURLDownloader.swift | 29 +------- .../FilterListResourceDownloader.swift | 73 +++++++------------ .../Brave/WebFilters/FilterListStorage.swift | 2 +- .../ShieldStats/Adblock/AdBlockStats.swift | 39 ++-------- .../Adblock/CachedAdBlockEngine.swift | 24 ++++-- Sources/Data/models/FilterListSetting.swift | 11 +++ 11 files changed, 87 insertions(+), 113 deletions(-) diff --git a/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift b/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift index 0a58c67f48e..d23055457fc 100644 --- a/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift +++ b/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+WKNavigationDelegate.swift @@ -149,7 +149,6 @@ extension BrowserViewController: WKNavigationDelegate { @MainActor public func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, preferences: WKWebpagePreferences) async -> (WKNavigationActionPolicy, WKWebpagePreferences) { - guard var requestURL = navigationAction.request.url else { return (.cancel, preferences) } @@ -255,6 +254,9 @@ extension BrowserViewController: WKNavigationDelegate { return (.cancel, preferences) } + let signpostID = ContentBlockerManager.signpost.makeSignpostID() + let state = ContentBlockerManager.signpost.beginInterval("decidePolicyFor", id: signpostID) + // before loading any ad-block scripts // await the preparation of the ad-block services await LaunchHelper.shared.prepareAdBlockServices( @@ -279,6 +281,7 @@ extension BrowserViewController: WKNavigationDelegate { ContentBlockerManager.log.debug("Redirected user to `\(url.absoluteString, privacy: .private)`") } + ContentBlockerManager.signpost.endInterval("decidePolicyFor", state, "Redirected navigation") return (.cancel, preferences) } else { tab?.isInternalRedirect = false @@ -321,6 +324,7 @@ extension BrowserViewController: WKNavigationDelegate { var modifiedRequest = URLRequest(url: requestURL) modifiedRequest.setValue("1", forHTTPHeaderField: "X-Brave-Ads-Enabled") tab?.loadRequest(modifiedRequest) + ContentBlockerManager.signpost.endInterval("decidePolicyFor", state, "Redirected to search") return (.cancel, preferences) } @@ -373,6 +377,7 @@ extension BrowserViewController: WKNavigationDelegate { if let url = components?.url { let request = PrivilegedRequest(url: url) as URLRequest tab?.loadRequest(request) + ContentBlockerManager.signpost.endInterval("decidePolicyFor", state, "Blocked navigation") return (.cancel, preferences) } } @@ -437,6 +442,7 @@ extension BrowserViewController: WKNavigationDelegate { self.shouldDownloadNavigationResponse = true } + ContentBlockerManager.signpost.endInterval("decidePolicyFor", state) return (.allow, preferences) } diff --git a/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift b/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift index 0fde7b93212..2038eabc856 100644 --- a/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift +++ b/Sources/Brave/Frontend/Browser/Helpers/LaunchHelper.swift @@ -39,6 +39,7 @@ public actor LaunchHelper { // Otherwise prepare the services and await the task let task = Task { let signpostID = Self.signpost.makeSignpostID() + ContentBlockerManager.log.debug("Loading blocking launch data") let state = Self.signpost.beginInterval("blockingLaunchTask", id: signpostID) // We only want to compile the necessary content blockers during launch // We will compile other ones after launch @@ -50,6 +51,7 @@ public actor LaunchHelper { async let adblockResourceCache: Void = AdblockResourceDownloader.shared.loadCachedAndBundledDataIfNeeded(allowedModes: launchBlockModes) _ = await (filterListCache, adblockResourceCache) Self.signpost.emitEvent("loadedCachedData", id: signpostID, "Loaded cached data") + ContentBlockerManager.log.debug("Loaded blocking launch data") // This one is non-blocking performPostLoadTasks(adBlockService: adBlockService, loadedBlockModes: launchBlockModes) @@ -128,7 +130,7 @@ private extension FilterListStorage { var validBlocklistTypes: Set { if filterLists.isEmpty { // If we don't have filter lists yet loaded, use the settings - return Set(allFilterListSettings.compactMap { setting in + return Set(allFilterListSettings.compactMap { setting -> ContentBlockerManager.BlocklistType? in guard let componentId = setting.componentId else { return nil } return .filterList( componentId: componentId, diff --git a/Sources/Brave/Frontend/Browser/User Scripts/UserScriptType.swift b/Sources/Brave/Frontend/Browser/User Scripts/UserScriptType.swift index a6f997083ed..f4774d5aa39 100644 --- a/Sources/Brave/Frontend/Browser/User Scripts/UserScriptType.swift +++ b/Sources/Brave/Frontend/Browser/User Scripts/UserScriptType.swift @@ -92,7 +92,7 @@ extension UserScriptType: CustomDebugStringConvertible { case .nacl: return "nacl" case .gpc(let isEnabled): - return "gpc(\(isEnabled)" + return "gpc(\(isEnabled))" case .siteStateListener: return "siteStateListener" case .selectorsPoller: diff --git a/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift b/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift index 2650b4ee013..610e3711734 100644 --- a/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift +++ b/Sources/Brave/WebFilters/ContentBlocker/ContentBlockerManager.swift @@ -153,6 +153,8 @@ actor ContentBlockerManager { /// Should be used only as a cleanup once during launch to get rid of unecessary/old data. /// This is mostly for custom filter lists a user may have added. public func cleaupInvalidRuleLists(validTypes: Set) async { + let signpostID = Self.signpost.makeSignpostID() + let state = Self.signpost.beginInterval("cleaupInvalidRuleLists", id: signpostID) let availableIdentifiers = await ruleStore.availableIdentifiers() ?? [] await availableIdentifiers.asyncConcurrentForEach { identifier in @@ -170,6 +172,8 @@ actor ContentBlockerManager { assertionFailure() } } + + Self.signpost.endInterval("cleaupInvalidRuleLists", state) } /// Compile the rule list found in the given local URL using the specified modes diff --git a/Sources/Brave/WebFilters/FilterList.swift b/Sources/Brave/WebFilters/FilterList.swift index 3ed03dcfa6c..898f5c9f969 100644 --- a/Sources/Brave/WebFilters/FilterList.swift +++ b/Sources/Brave/WebFilters/FilterList.swift @@ -46,6 +46,10 @@ struct FilterList: Identifiable { let entry: AdblockFilterListCatalogEntry var isEnabled: Bool = false + var isHidden: Bool { + // TODO: @JS get this from entry.hidden once it is available. + return false + } /// Tells us if this filter list is regional (i.e. if it contains language restrictions) var isRegional: Bool { return !entry.languages.isEmpty diff --git a/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift b/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift index a2b87de4221..5567e5f60c9 100644 --- a/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift +++ b/Sources/Brave/WebFilters/FilterListCustomURLDownloader.swift @@ -58,29 +58,9 @@ actor FilterListCustomURLDownloader: ObservableObject { self.startedService = true await CustomFilterListStorage.shared.loadCachedFilterLists() - do { - try await CustomFilterListStorage.shared.filterListsURLs.asyncConcurrentForEach { customURL in - let resource = await customURL.setting.resource - - do { - if let cachedResult = try resource.cachedResult() { - await self.handle(downloadResult: cachedResult, for: customURL) - } - } catch { - let uuid = await customURL.setting.uuid - ContentBlockerManager.log.error( - "Failed to cached data for custom filter list `\(uuid)`: \(error)" - ) - } - - // Always fetch this resource so it's ready if the user enables it. - await self.startFetching(filterListCustomURL: customURL) - - // Sleep for 1ms. This drastically reduces memory usage without much impact to usability - try await Task.sleep(nanoseconds: 1000000) - } - } catch { - // Ignore cancellation errors + await CustomFilterListStorage.shared.filterListsURLs.asyncForEach { customURL in + // Always fetch this resource so it's ready if the user enables it. + await self.startFetching(filterListCustomURL: customURL) } } @@ -105,9 +85,8 @@ actor FilterListCustomURLDownloader: ObservableObject { filterListInfo: filterListInfo, isAlwaysAggressive: true ) - guard await AdBlockStats.shared.isEagerlyLoaded(source: source) else { + guard await AdBlockStats.shared.isEnabled(source: source) else { // Don't compile unless eager - await AdBlockStats.shared.updateIfNeeded(resourcesInfo: resourcesInfo) await AdBlockStats.shared.updateIfNeeded(filterListInfo: filterListInfo, isAlwaysAggressive: true) // To free some space, remove any rule lists that are not needed diff --git a/Sources/Brave/WebFilters/FilterListResourceDownloader.swift b/Sources/Brave/WebFilters/FilterListResourceDownloader.swift index 726b9746bda..890ee7e683e 100644 --- a/Sources/Brave/WebFilters/FilterListResourceDownloader.swift +++ b/Sources/Brave/WebFilters/FilterListResourceDownloader.swift @@ -46,11 +46,9 @@ public actor FilterListResourceDownloader { } let resourcesInfo = await didUpdateResourcesComponent(folderURL: resourcesFolderURL) - async let startedCustomFilterListsDownloader: Void = FilterListCustomURLDownloader.shared.startIfNeeded() async let cachedFilterLists: Void = compileCachedFilterLists(resourcesInfo: resourcesInfo) async let compileDefaultEngine: Void = compileDefaultFilterList(resourcesInfo: resourcesInfo) - - _ = await (startedCustomFilterListsDownloader, cachedFilterLists, compileDefaultEngine) + _ = await (cachedFilterLists, compileDefaultEngine) } /// Compile the default filter list from cache @@ -86,9 +84,10 @@ public actor FilterListResourceDownloader { do { try await filterListSettings.asyncConcurrentForEach { setting in - guard await setting.isEnabled == true else { return } + guard await setting.isEagerlyLoaded == true else { return } guard let componentId = await setting.componentId else { return } - guard FilterList.disabledComponentIDs.contains(componentId) else { return } + guard !FilterList.disabledComponentIDs.contains(componentId) else { return } + guard let source = await setting.engineSource else { return } // Try to load the filter list folder. We always have to compile this at start guard let folderURL = await setting.folderURL, FileManager.default.fileExists(atPath: folderURL.path) else { @@ -96,7 +95,7 @@ public actor FilterListResourceDownloader { } await self.compileFilterListEngineIfNeeded( - fromComponentId: componentId, folderURL: folderURL, + source: source, folderURL: folderURL, isAlwaysAggressive: setting.isAlwaysAggressive, resourcesInfo: resourcesInfo, compileContentBlockers: false @@ -138,7 +137,7 @@ public actor FilterListResourceDownloader { Task { @MainActor in for await filterListEntries in adBlockService.filterListCatalogComponentStream() { FilterListStorage.shared.loadFilterLists(from: filterListEntries) - + ContentBlockerManager.log.debug("Loaded filter list catalog") if await AdBlockStats.shared.resourcesInfo != nil { await registerAllFilterListsIfNeeded(with: adBlockService) } @@ -227,22 +226,30 @@ public actor FilterListResourceDownloader { /// Load general filter lists (shields) from the given `AdblockService` `shieldsInstallPath` `URL`. private func compileFilterListEngineIfNeeded( - fromComponentId componentId: String, folderURL: URL, + source: CachedAdBlockEngine.Source, + folderURL: URL, isAlwaysAggressive: Bool, resourcesInfo: CachedAdBlockEngine.ResourcesInfo, compileContentBlockers: Bool ) async { let version = folderURL.lastPathComponent - let source = CachedAdBlockEngine.Source.filterList(componentId: componentId) + let filterListURL = folderURL.appendingPathComponent("list.txt", conformingTo: .text) + + guard FileManager.default.fileExists(atPath: filterListURL.relativePath) else { + // We are loading the old component from cache. We don't want this file to be loaded. + // When we download the new component shortly we will update our cache. + // This should only trigger after an app update and eventually this check can be removed. + return + } + let filterListInfo = CachedAdBlockEngine.FilterListInfo( - source: source, - localFileURL: folderURL.appendingPathComponent("list.txt", conformingTo: .text), + source: source, localFileURL: filterListURL, version: version, fileType: .text ) let lazyInfo = AdBlockStats.LazyFilterListInfo(filterListInfo: filterListInfo, isAlwaysAggressive: isAlwaysAggressive) - guard await AdBlockStats.shared.isEagerlyLoaded(source: source) else { - // Don't compile unless eager - await AdBlockStats.shared.updateIfNeeded(resourcesInfo: resourcesInfo) + + // Check if we should load these rules + guard await AdBlockStats.shared.isEnabled(source: source) else { await AdBlockStats.shared.updateIfNeeded(filterListInfo: filterListInfo, isAlwaysAggressive: isAlwaysAggressive) // To free some space, remove any rule lists that are not needed @@ -275,12 +282,12 @@ public actor FilterListResourceDownloader { assertionFailure("We shouldn't have started downloads before getting this value") return } + let source = filterList.engineSource - await self.loadShields( - fromComponentId: filterList.entry.componentId, folderURL: folderURL, relativeOrder: filterList.order, - loadContentBlockers: true, - isAlwaysAggressive: filterList.isAlwaysAggressive, - resourcesInfo: resourcesInfo + // Add or remove the filter list from the engine depending if it's been enabled or not + await self.compileFilterListEngineIfNeeded( + source: source, folderURL: folderURL, isAlwaysAggressive: filterList.isAlwaysAggressive, + resourcesInfo: resourcesInfo, compileContentBlockers: true ) // Save the downloaded folder for later (caching) purposes @@ -288,34 +295,6 @@ public actor FilterListResourceDownloader { } } } - - /// Handle the downloaded component folder url of a filter list. - /// - /// The folder URL should point to a `AdblockFilterListEntry` download location as given by the `AdBlockService`. - /// - /// If `loadContentBlockers` is set to `true`, this method will compile the rule lists to content blocker format and load them into the `WKContentRuleListStore`. - /// As both these procedures are expensive, this should be set to `false` if this method is called on a blocking UI process such as the launch of the application. - private func loadShields( - fromComponentId componentId: String, folderURL: URL, relativeOrder: Int, loadContentBlockers: Bool, isAlwaysAggressive: Bool, - resourcesInfo: CachedAdBlockEngine.ResourcesInfo - ) async { - // Check if we're loading the new component or an old component from cache. - // The new component has a file `list.txt` which we check the presence of. - let filterListURL = folderURL.appendingPathComponent("list.txt", conformingTo: .text) - - guard FileManager.default.fileExists(atPath: filterListURL.relativePath) else { - // We are loading the old component from cache. We don't want this file to be loaded. - // When we download the new component shortly we will update our cache. - // This should only trigger after an app update and eventually this check can be removed. - return - } - - // Add or remove the filter list from the engine depending if it's been enabled or not - await self.compileFilterListEngineIfNeeded( - fromComponentId: componentId, folderURL: folderURL, isAlwaysAggressive: isAlwaysAggressive, - resourcesInfo: resourcesInfo, compileContentBlockers: loadContentBlockers - ) - } } /// Helpful extension to the AdblockService diff --git a/Sources/Brave/WebFilters/FilterListStorage.swift b/Sources/Brave/WebFilters/FilterListStorage.swift index b10e274b754..8d99171ef48 100644 --- a/Sources/Brave/WebFilters/FilterListStorage.swift +++ b/Sources/Brave/WebFilters/FilterListStorage.swift @@ -134,7 +134,7 @@ import Combine upsertSetting( uuid: filterList.entry.uuid, isEnabled: filterList.isEnabled, - isHidden: false, + isHidden: filterList.isHidden, componentId: filterList.entry.componentId, allowCreation: true, order: filterList.order, diff --git a/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift b/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift index 20e473cf14d..14fb173d995 100644 --- a/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift +++ b/Sources/Brave/WebFilters/ShieldStats/Adblock/AdBlockStats.swift @@ -29,16 +29,6 @@ public actor AdBlockStats { /// The current task that is compiling. private var currentCompileTask: Task<(), Never>? - /// Return all the critical sources - /// - /// Critical sources are those that are enabled and are "on" by default. Giving us the most important sources. - /// Used for memory managment so we know which filter lists to disable upon a memory warning - @MainActor var criticalSources: [CachedAdBlockEngine.Source] { - var enabledSources: [CachedAdBlockEngine.Source] = [.adBlock] - enabledSources.append(contentsOf: FilterListStorage.shared.criticalSources) - return enabledSources - } - /// Return an array of all sources that are enabled according to user's settings /// - Note: This does not take into account the domain or global adblock toggle @MainActor var enabledSources: [CachedAdBlockEngine.Source] { @@ -125,6 +115,12 @@ public actor AdBlockStats { func updateIfNeeded(resourcesInfo: CachedAdBlockEngine.ResourcesInfo) { guard self.resourcesInfo == nil || resourcesInfo.version > self.resourcesInfo!.version else { return } self.resourcesInfo = resourcesInfo + + if #available(iOS 16.0, *) { + ContentBlockerManager.log.debug( + "Updated resources component: `\(resourcesInfo.localFileURL.path(percentEncoded: false))`" + ) + } } /// Remove all the engines @@ -177,12 +173,8 @@ public actor AdBlockStats { } } - /// Tells us if this source should be eagerly loaded. - /// - /// Eagerness is determined by several factors: - /// * If the source represents a fitler list or a custom filter list, it is eager if it is enabled - /// * If the source represents the `adblock` default filter list, it is always eager regardless of shield settings - @MainActor func isEagerlyLoaded(source: CachedAdBlockEngine.Source) -> Bool { + /// Tells us if this source should be loaded. + @MainActor func isEnabled(source: CachedAdBlockEngine.Source) -> Bool { return enabledSources.contains(source) } @@ -275,21 +267,6 @@ extension FilterList { } private extension FilterListStorage { - /// Gives us source representations of all the critical filter lists - /// - /// Critical filter lists are those that are enabled and are "on" by default. Giving us the most important filter lists. - /// Used for memory managment so we know which filter lists to disable upon a memory warning - @MainActor var criticalSources: [CachedAdBlockEngine.Source] { - return enabledSources.filter { source in - switch source { - case .filterList(let componentId): - return FilterList.defaultOnComponentIds.contains(componentId) - default: - return false - } - } - } - /// Gives us source representations of all the enabled filter lists @MainActor var enabledSources: [CachedAdBlockEngine.Source] { if !filterLists.isEmpty { diff --git a/Sources/Brave/WebFilters/ShieldStats/Adblock/CachedAdBlockEngine.swift b/Sources/Brave/WebFilters/ShieldStats/Adblock/CachedAdBlockEngine.swift index 65125beb07c..ca9fbba1daf 100644 --- a/Sources/Brave/WebFilters/ShieldStats/Adblock/CachedAdBlockEngine.swift +++ b/Sources/Brave/WebFilters/ShieldStats/Adblock/CachedAdBlockEngine.swift @@ -7,6 +7,7 @@ import Foundation import BraveCore import Data import Preferences +import os /// An object that wraps around an `AdblockEngine` and caches some results /// and ensures information is always returned on the correct thread on the engine. @@ -51,6 +52,7 @@ public class CachedAdBlockEngine { let version: String } + static let signpost = OSSignposter(logger: ContentBlockerManager.log) /// We cache the models so that they load faster when we need to poll information about the frame private var cachedCosmeticFilterModels = FifoDict() /// We cache the models so that they load faster when doing stats tracking or request blocking @@ -201,12 +203,22 @@ public class CachedAdBlockEngine { ) throws -> CachedAdBlockEngine { switch filterListInfo.fileType { case .text: - let engine = try AdblockEngine(textFileURL: filterListInfo.localFileURL, resourcesFileURL: resourcesInfo.localFileURL) - let serialQueue = DispatchQueue(label: "com.brave.WrappedAdBlockEngine.\(UUID().uuidString)") - return CachedAdBlockEngine( - engine: engine, filterListInfo: filterListInfo, resourcesInfo: resourcesInfo, - serialQueue: serialQueue, isAlwaysAggressive: isAlwaysAggressive - ) + let signpostID = Self.signpost.makeSignpostID() + let state = Self.signpost.beginInterval("compileEngine", id: signpostID, "\(filterListInfo.debugDescription)") + + do { + let engine = try AdblockEngine(textFileURL: filterListInfo.localFileURL, resourcesFileURL: resourcesInfo.localFileURL) + let serialQueue = DispatchQueue(label: "com.brave.WrappedAdBlockEngine.\(UUID().uuidString)") + Self.signpost.endInterval("compileEngine", state) + + return CachedAdBlockEngine( + engine: engine, filterListInfo: filterListInfo, resourcesInfo: resourcesInfo, + serialQueue: serialQueue, isAlwaysAggressive: isAlwaysAggressive + ) + } catch { + Self.signpost.endInterval("compileEngine", state, "\(error.localizedDescription)") + throw error + } } } } diff --git a/Sources/Data/models/FilterListSetting.swift b/Sources/Data/models/FilterListSetting.swift index 8a1df6b188e..6b0810a5e60 100644 --- a/Sources/Data/models/FilterListSetting.swift +++ b/Sources/Data/models/FilterListSetting.swift @@ -22,6 +22,17 @@ public final class FilterListSetting: NSManagedObject, CRUD { @MainActor @NSManaged public var isAlwaysAggressive: Bool @MainActor @NSManaged public var order: NSNumber? @MainActor @NSManaged private var folderPath: String? + + /// Tells us which filter lists should be compiled during launch. + /// + /// The filter lists that will be eagerly loaded are ones that are: + /// 1. Enabled + /// 2. Hidden (i.e. there is no UI to disable/enable them). + /// This includes the "default" and "first party" filter lists. + /// These are not available when using the regional catalog (i.e. `regional_catalog.json`). + @MainActor public var isEagerlyLoaded: Bool { + return isEnabled && isHidden + } @MainActor public var folderURL: URL? { get {