From 3c7663a965fe050ef038d6dd34b91c4f15591731 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa?=
 <dani-garcia@users.noreply.github.com>
Date: Fri, 28 Jun 2024 21:58:15 +0200
Subject: [PATCH] [PM-7809] Fix memory leak in AngularThemingService for Safari
 extension (#9434)

* [PM-7809] Fix memory leak in AngularThemingService for Safari

* Use getSystemThemeFromWindow in createSystemThemeFromWindow
---
 .../src/popup/services/services.module.ts       | 13 +++++++------
 .../services/theming/angular-theming.service.ts | 17 ++++++++++++-----
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts
index c7b5ca9b416..7c187d00514 100644
--- a/apps/browser/src/popup/services/services.module.ts
+++ b/apps/browser/src/popup/services/services.module.ts
@@ -1,6 +1,6 @@
 import { APP_INITIALIZER, NgModule, NgZone } from "@angular/core";
 import { Router } from "@angular/router";
-import { Subject, merge } from "rxjs";
+import { Subject, merge, of } from "rxjs";
 
 import { UnauthGuard as BaseUnauthGuardService } from "@bitwarden/angular/auth/guards";
 import { AngularThemingService } from "@bitwarden/angular/platform/services/theming/angular-theming.service";
@@ -485,14 +485,15 @@ const safeProviders: SafeProvider[] = [
     provide: SYSTEM_THEME_OBSERVABLE,
     useFactory: (platformUtilsService: PlatformUtilsService) => {
       // Safari doesn't properly handle the (prefers-color-scheme) media query in the popup window, it always returns light.
-      // In Safari, we have to use the background page instead, which comes with limitations like not dynamically changing the extension theme when the system theme is changed.
-      let windowContext = window;
+      // This means we have to use the background page instead, which comes with limitations like not dynamically
+      // changing the extension theme when the system theme is changed. We also have issues with memory leaks when
+      // holding the reference to the background page.
       const backgroundWindow = BrowserApi.getBackgroundPage();
       if (platformUtilsService.isSafari() && backgroundWindow) {
-        windowContext = backgroundWindow;
+        return of(AngularThemingService.getSystemThemeFromWindow(backgroundWindow));
+      } else {
+        return AngularThemingService.createSystemThemeFromWindow(window);
       }
-
-      return AngularThemingService.createSystemThemeFromWindow(windowContext);
     },
     deps: [PlatformUtilsService],
   }),
diff --git a/libs/angular/src/platform/services/theming/angular-theming.service.ts b/libs/angular/src/platform/services/theming/angular-theming.service.ts
index d0f96eb4a7a..e8b78c90c46 100644
--- a/libs/angular/src/platform/services/theming/angular-theming.service.ts
+++ b/libs/angular/src/platform/services/theming/angular-theming.service.ts
@@ -18,11 +18,7 @@ export class AngularThemingService implements AbstractThemingService {
   static createSystemThemeFromWindow(window: Window): Observable<ThemeType> {
     return merge(
       // This observable should always emit at least once, so go and get the current system theme designation
-      of(
-        window.matchMedia("(prefers-color-scheme: dark)").matches
-          ? ThemeType.Dark
-          : ThemeType.Light,
-      ),
+      of(AngularThemingService.getSystemThemeFromWindow(window)),
       // Start listening to changes
       fromEvent<MediaQueryListEvent>(
         window.matchMedia("(prefers-color-scheme: dark)"),
@@ -31,6 +27,17 @@ export class AngularThemingService implements AbstractThemingService {
     );
   }
 
+  /**
+   * Gets the currently active system theme based on the given window.
+   * @param window The window to query for the current theme.
+   * @returns The active system theme.
+   */
+  static getSystemThemeFromWindow(window: Window): ThemeType {
+    return window.matchMedia("(prefers-color-scheme: dark)").matches
+      ? ThemeType.Dark
+      : ThemeType.Light;
+  }
+
   readonly theme$ = this.themeStateService.selectedTheme$.pipe(
     switchMap((configuredTheme) => {
       if (configuredTheme === ThemeType.System) {