1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-09-29 04:17:41 +02:00

[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
This commit is contained in:
Daniel García 2023-05-16 16:26:01 +02:00 committed by GitHub
parent c58b0c0753
commit 8e61184c0f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 6 deletions

View File

@ -168,15 +168,27 @@ export class BrowserApi {
chrome.tabs.remove(tabToClose.id); chrome.tabs.remove(tabToClose.id);
} }
private static registeredMessageListeners: any[] = [];
static messageListener( static messageListener(
name: string, name: string,
callback: (message: any, sender: chrome.runtime.MessageSender, response: any) => void callback: (message: any, sender: chrome.runtime.MessageSender, response: any) => void
) { ) {
chrome.runtime.onMessage.addListener( chrome.runtime.onMessage.addListener(callback);
(msg: any, sender: chrome.runtime.MessageSender, response: any) => {
callback(msg, sender, response); // 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 = {}) { static sendMessage(subscriber: string, arg: any = {}) {

View File

@ -63,7 +63,7 @@ export class ThemingService implements AbstractThemingService {
protected monitorSystemThemeChanges(): void { protected monitorSystemThemeChanges(): void {
fromEvent<MediaQueryListEvent>( fromEvent<MediaQueryListEvent>(
this.window.matchMedia("(prefers-color-scheme: dark)"), window.matchMedia("(prefers-color-scheme: dark)"),
"change" "change"
).subscribe((event) => { ).subscribe((event) => {
this.updateSystemTheme(event.matches ? ThemeType.Dark : ThemeType.Light); this.updateSystemTheme(event.matches ? ThemeType.Dark : ThemeType.Light);