From 8e61184c0f8d94cdc0d0b33bd5668ce4cb473fb0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa?=
 <dani-garcia@users.noreply.github.com>
Date: Tue, 16 May 2023 16:26:01 +0200
Subject: [PATCH] [PM-2197] Fix memory leaks in Safari (#5451)

* Remove reference cycle between ThemingService and the global window object

* Deregister messageListeners on a safari popup to avoid mem leaks

* Use pagehide event instead of unload
---
 apps/browser/src/browser/browserApi.ts        | 22 ++++++++++++++-----
 .../src/services/theming/theming.service.ts   |  2 +-
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/apps/browser/src/browser/browserApi.ts b/apps/browser/src/browser/browserApi.ts
index e93fcceb0df..b1313ebda6f 100644
--- a/apps/browser/src/browser/browserApi.ts
+++ b/apps/browser/src/browser/browserApi.ts
@@ -168,15 +168,27 @@ export class BrowserApi {
     chrome.tabs.remove(tabToClose.id);
   }
 
+  private static registeredMessageListeners: any[] = [];
+
   static messageListener(
     name: string,
     callback: (message: any, sender: chrome.runtime.MessageSender, response: any) => void
   ) {
-    chrome.runtime.onMessage.addListener(
-      (msg: any, sender: chrome.runtime.MessageSender, response: any) => {
-        callback(msg, sender, response);
-      }
-    );
+    chrome.runtime.onMessage.addListener(callback);
+
+    // Keep track of all the events registered in a Safari popup so we can remove
+    // them when the popup gets unloaded, otherwise we cause a memory leak
+    if (BrowserApi.isSafariApi && !BrowserApi.isBackgroundPage(window)) {
+      BrowserApi.registeredMessageListeners.push(callback);
+
+      // The MDN recommend using 'visibilitychange' but that event is fired any time the popup window is obscured as well
+      // 'pagehide' works just like 'unload' but is compatible with the back/forward cache, so we prefer using that one
+      window.onpagehide = () => {
+        for (const callback of BrowserApi.registeredMessageListeners) {
+          chrome.runtime.onMessage.removeListener(callback);
+        }
+      };
+    }
   }
 
   static sendMessage(subscriber: string, arg: any = {}) {
diff --git a/libs/angular/src/services/theming/theming.service.ts b/libs/angular/src/services/theming/theming.service.ts
index 270cd92c4fe..fbae277cabb 100644
--- a/libs/angular/src/services/theming/theming.service.ts
+++ b/libs/angular/src/services/theming/theming.service.ts
@@ -63,7 +63,7 @@ export class ThemingService implements AbstractThemingService {
 
   protected monitorSystemThemeChanges(): void {
     fromEvent<MediaQueryListEvent>(
-      this.window.matchMedia("(prefers-color-scheme: dark)"),
+      window.matchMedia("(prefers-color-scheme: dark)"),
       "change"
     ).subscribe((event) => {
       this.updateSystemTheme(event.matches ? ThemeType.Dark : ThemeType.Light);