devol
/
bitwarden-estensione-browser
mirror of https://github.com/bitwarden/browser
1
0
Fork 0
Code Issues 1 Releases Wiki Activity

Ensure chrome.storage listeners also get cleaned up in Safari to avoid memory leak (#6354)

This commit is contained in:
Daniel García 2023-09-21 07:04:17 -06:00 committed by GitHub
parent 5d14afb97f
commit 5b69d52f02
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
apps/browser/src/platform/browser/browser-api.ts
View File

@ -199,7 +199,10 @@ export class BrowserApi {
BrowserApi.removeTab(tabToClose.id);
}
// 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
private static registeredMessageListeners: any[] = [];
private static registeredStorageChangeListeners: any[] = [];
static messageListener(
name: string,
@ -207,21 +210,38 @@ export class BrowserApi {
) {
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);
}
};
BrowserApi.setupUnloadListeners();
}
}
static storageChangeListener(
callback: Parameters<typeof chrome.storage.onChanged.addListener>[0]
) {
chrome.storage.onChanged.addListener(callback);
if (BrowserApi.isSafariApi && !BrowserApi.isBackgroundPage(window)) {
BrowserApi.registeredStorageChangeListeners.push(callback);
BrowserApi.setupUnloadListeners();
}
}
// Setup the event to destroy all the listeners when the popup gets unloaded in Safari, otherwise we get a memory leak
private static setupUnloadListeners() {
// 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);
}
for (const callback of BrowserApi.registeredStorageChangeListeners) {
chrome.storage.onChanged.removeListener(callback);
}
};
}
static sendMessage(subscriber: string, arg: any = {}) {
const message = Object.assign({}, { command: subscriber }, arg);
return chrome.runtime.sendMessage(message);

3
apps/browser/src/platform/services/browser-state.service.ts
View File

@ -14,6 +14,7 @@ import { Account } from "../../models/account";
import { BrowserComponentState } from "../../models/browserComponentState";
import { BrowserGroupingsComponentState } from "../../models/browserGroupingsComponentState";
import { BrowserSendComponentState } from "../../models/browserSendComponentState";
import { BrowserApi } from "../browser/browser-api";
import { browserSession, sessionSync } from "../decorators/session-sync-observable";
import { BrowserStateService as StateServiceAbstraction } from "./abstractions/browser-state.service";
@ -56,7 +57,7 @@ export class BrowserStateService
// the background page that can get out of sync. We need to work out the
// best way to handle caching with multiple instances of the state service.
if (useAccountCache) {
chrome.storage.onChanged.addListener((changes, namespace) => {
BrowserApi.storageChangeListener((changes, namespace) => {
if (namespace === "local") {
for (const key of Object.keys(changes)) {
if (key !== "accountActivity" && this.accountDiskCache.value[key]) {