From 744b956e5f41638ad7277232475ee2d2e725eabc Mon Sep 17 00:00:00 2001 From: iamEvan <47493765+iamEvanYT@users.noreply.github.com> Date: Wed, 16 Jul 2025 13:08:21 +0100 Subject: [PATCH] refactor: Enhance ContentBlocker with improved error handling, memory management, and session management - Introduced BlockerConfig interface for better configuration handling. - Replaced array with Set for blocked sessions to optimize performance. - Added error handling in blocker instance creation and session management methods. - Implemented cleanup method to handle resource deallocation on app shutdown. - Updated initialization process to ensure proper event listener binding. --- src/main/modules/content-blocker.ts | 227 +++++++++++++++++++++------- 1 file changed, 174 insertions(+), 53 deletions(-) diff --git a/src/main/modules/content-blocker.ts b/src/main/modules/content-blocker.ts index 7ae62c17..8cfbd09e 100644 --- a/src/main/modules/content-blocker.ts +++ b/src/main/modules/content-blocker.ts @@ -9,13 +9,21 @@ type BlockerInstanceType = "all" | "adsAndTrackers" | "adsOnly"; const SESSION_KEY = "content-blocker"; +interface BlockerConfig { + type: BlockerInstanceType; + enabled: boolean; +} + /** * ContentBlocker class manages ad and tracking content blocking functionality + * with improved memory management, error handling, and performance optimizations */ class ContentBlocker { private blockerInstancePromise: Promise | undefined = undefined; private blockerInstanceType: BlockerInstanceType | undefined = undefined; - private blockedSessions: Session[] = []; + private blockedSessions = new Set(); + private isInitialized = false; + private updateTimeout: NodeJS.Timeout | undefined; /** * Creates or returns existing blocker instance of the specified type @@ -30,26 +38,37 @@ class ContentBlocker { } debugPrint("CONTENT_BLOCKER", "Creating blocker instance:", type); - switch (type) { - case "all": - this.blockerInstancePromise = ElectronBlocker.fromPrebuiltFull(); - break; - case "adsAndTrackers": - this.blockerInstancePromise = ElectronBlocker.fromPrebuiltAdsAndTracking(); - break; - case "adsOnly": - this.blockerInstancePromise = ElectronBlocker.fromPrebuiltAdsOnly(); - break; - } - this.blockerInstancePromise.then((blocker) => { + try { + let promise: Promise; + + switch (type) { + case "all": + promise = ElectronBlocker.fromPrebuiltFull(); + break; + case "adsAndTrackers": + promise = ElectronBlocker.fromPrebuiltAdsAndTracking(); + break; + case "adsOnly": + promise = ElectronBlocker.fromPrebuiltAdsOnly(); + break; + } + + this.blockerInstancePromise = promise; + this.blockerInstanceType = type; + + const blocker = await promise; blocker.on("request-blocked", (request) => { debugPrint("CONTENT_BLOCKER", "Request blocked:", request.url); }); - }); - this.blockerInstanceType = type; - return this.blockerInstancePromise as Promise; + return blocker; + } catch (error) { + debugPrint("CONTENT_BLOCKER", "Failed to create blocker instance:", error); + this.blockerInstancePromise = undefined; + this.blockerInstanceType = undefined; + throw error; + } } /** @@ -58,74 +77,171 @@ class ContentBlocker { private async disableBlocker(): Promise { if (!this.blockerInstancePromise) return; - const blocker = await this.blockerInstancePromise; - for (const session of this.blockedSessions) { - blocker.disableBlockingInSession(createBetterSession(session, SESSION_KEY)); - } + try { + const blocker = await this.blockerInstancePromise; + + // Disable blocking for all sessions + const disablePromises = Array.from(this.blockedSessions).map((session) => + blocker.disableBlockingInSession(createBetterSession(session, SESSION_KEY)) + ); - this.blockedSessions = []; - this.blockerInstancePromise = undefined; - this.blockerInstanceType = undefined; + await Promise.allSettled(disablePromises); + + this.blockedSessions.clear(); + this.blockerInstancePromise = undefined; + this.blockerInstanceType = undefined; + + debugPrint("CONTENT_BLOCKER", "Content blocker disabled for all sessions"); + } catch (error) { + debugPrint("CONTENT_BLOCKER", "Error disabling blocker:", error); + } } /** * Enables content blocking for a specific session */ private async enableBlockerForSession(blockerType: BlockerInstanceType, session: Session): Promise { - const blocker = await this.createBlockerInstance(blockerType); - if (!blocker) return; + try { + const blocker = await this.createBlockerInstance(blockerType); - // check if session is already blocked - if (this.blockedSessions.includes(session)) return; + // Skip if already blocked + if (this.blockedSessions.has(session)) return; - // add session to blocked sessions - this.blockedSessions.push(session); + // Enable blocking + await blocker.enableBlockingInSession(createBetterSession(session, SESSION_KEY)); - // enable blocking in session - blocker.enableBlockingInSession(createBetterSession(session, SESSION_KEY)); + // Track blocked session + this.blockedSessions.add(session); + + debugPrint("CONTENT_BLOCKER", `Enabled ${blockerType} blocking for session`); + } catch (error) { + debugPrint("CONTENT_BLOCKER", "Failed to enable blocker for session:", error); + } } /** - * Updates content blocker configuration based on user settings + * Removes a session from blocking */ - public async updateConfig(): Promise { - if (!browser) return; + public async removeSession(session: Session): Promise { + if (!this.blockedSessions.has(session)) return; + + try { + if (this.blockerInstancePromise) { + const blocker = await this.blockerInstancePromise; + await blocker.disableBlockingInSession(createBetterSession(session, SESSION_KEY)); + } + this.blockedSessions.delete(session); + debugPrint("CONTENT_BLOCKER", "Removed session from content blocking"); + } catch (error) { + debugPrint("CONTENT_BLOCKER", "Error removing session:", error); + } + } + + /** + * Gets current blocker configuration + */ + private getBlockerConfig(): BlockerConfig { const contentBlocker = getSettingValueById("contentBlocker") as string | undefined; - const profiles = browser.getLoadedProfiles(); switch (contentBlocker) { case "all": case "adsAndTrackers": case "adsOnly": - for (const profile of profiles) { - this.enableBlockerForSession(contentBlocker as BlockerInstanceType, profile.session); - } - break; + return { type: contentBlocker as BlockerInstanceType, enabled: true }; default: - this.disableBlocker(); + return { type: "adsOnly", enabled: false }; } + } - debugPrint("CONTENT_BLOCKER", "Content blocker configuration updated:", contentBlocker); + /** + * Updates content blocker configuration based on user settings + */ + public async updateConfig(): Promise { + if (!browser) return; + + // Debounce rapid configuration changes + if (this.updateTimeout) { + clearTimeout(this.updateTimeout); + } + + this.updateTimeout = setTimeout(async () => { + try { + const config = this.getBlockerConfig(); + const profiles = browser?.getLoadedProfiles() ?? []; + + if (config.enabled) { + // Enable blocking for all profiles + const enablePromises = profiles.map((profile) => this.enableBlockerForSession(config.type, profile.session)); + await Promise.allSettled(enablePromises); + } else { + // Disable blocking entirely + await this.disableBlocker(); + } + + debugPrint("CONTENT_BLOCKER", "Content blocker configuration updated:", config); + } catch (error) { + debugPrint("CONTENT_BLOCKER", "Error updating configuration:", error); + } + }, 100); + } + + /** + * Cleans up resources and event listeners + */ + public async cleanup(): Promise { + if (this.updateTimeout) { + clearTimeout(this.updateTimeout); + this.updateTimeout = undefined; + } + + await this.disableBlocker(); + this.isInitialized = false; } /** * Initializes content blocker and sets up event listeners */ public async initialize(): Promise { - // Initial configuration - await this.updateConfig(); - - // Listen for setting changes - settingsEmitter.on("settings-changed", () => { - this.updateConfig(); - }); - - // Listen for profile changes - browser?.on("profile-loaded", () => { - this.updateConfig(); - }); + if (this.isInitialized) return; + + try { + // Initial configuration + await this.updateConfig(); + + // Listen for setting changes + settingsEmitter.on("settings-changed", this.handleSettingsChanged); + + // Listen for profile changes + browser?.on("profile-loaded", this.handleProfileLoaded); + browser?.on("profile-unloaded", this.handleProfileRemoved); + + this.isInitialized = true; + debugPrint("CONTENT_BLOCKER", "Content blocker initialized successfully"); + } catch (error) { + debugPrint("CONTENT_BLOCKER", "Failed to initialize content blocker:", error); + } } + + /** + * Event handlers bound to maintain proper context + */ + private handleSettingsChanged = () => { + this.updateConfig(); + }; + + private handleProfileLoaded = () => { + this.updateConfig(); + }; + + private handleProfileRemoved = (profileId: string) => { + // Find the session for this profile and remove it + if (!browser) return; + const profile = browser.getLoadedProfile(profileId); + if (profile) { + this.removeSession(profile.session); + } + }; } // Export singleton instance @@ -136,3 +252,8 @@ onSettingsCached().then(() => { debugPrint("CONTENT_BLOCKER", "Initializing content blocker"); contentBlocker.initialize(); }); + +// Handle app shutdown +process.on("beforeExit", async () => { + await contentBlocker.cleanup(); +});