Skip to content

Commit

Permalink
Fix event listeners memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
WesselKroos committed Sep 15, 2024
1 parent e372d64 commit 62937f2
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 43 deletions.
1 change: 0 additions & 1 deletion src/scripts/content-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ const detectPageTransitions = (ytdAppElem) => {
}
},
undefined,
undefined,
true
);
};
Expand Down
2 changes: 0 additions & 2 deletions src/scripts/libs/ambientlight.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,6 @@ export default class Ambientlight {
this.resetAverageVideoFramesDifference();
},
undefined,
undefined,
true
);
on(
Expand All @@ -492,7 +491,6 @@ export default class Ambientlight {
this.calculateAverageVideoFramesDifference();
},
undefined,
undefined,
true
);
} catch (ex) {
Expand Down
13 changes: 11 additions & 2 deletions src/scripts/libs/bar-detection.js
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,9 @@ export default class BarDetection {
if (!this.worker) {
this.worker = workerFromCode(workerCode);
this.worker.onmessage = (e) => {
if(this.onWorkerMessageListener) {
return this.onWorkerMessageListener(e);
}
if (e.data.id !== -1) {
// console.warn('Ignoring old bar detection message:', e.data)
return;
Expand All @@ -1282,6 +1285,12 @@ export default class BarDetection {
SentryReporter.captureException(e.data.error);
}
};
this.worker.onerror = (err) => {
if(this.onWorkerRejectListener) {
return this.onWorkerRejectListener(err);
}
SentryReporter.captureException(err);
}
}

// Ignore previous percentages in cases: new video src, seeked or setting changed
Expand Down Expand Up @@ -1570,8 +1579,8 @@ export default class BarDetection {
const stack = new Error().stack;
const onMessagePromise = new Promise(
function onMessagePromise(resolve, reject) {
this.worker.onerror = (err) => reject(err);
this.worker.onmessage = async (e) => {
this.onWorkerRejectListener = (err) => reject(err);
this.onWorkerMessageListener = async (e) => {
try {
if (e.data.id !== this.runId) {
// console.warn('Ignoring old bar detection percentage:',
Expand Down
48 changes: 42 additions & 6 deletions src/scripts/libs/generic.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ export const setTimeout = (handler, timeout) => {
return window.setTimeout(wrapErrorHandler(handler), timeout);
};

const eventListenerCallbacks = [];
export function on(
elem,
eventNames,
callback,
options,
getListenerCallback,
reportOnce = false
) {
try {
Expand Down Expand Up @@ -168,13 +168,36 @@ export function on(
},
};
const eventListenerCallback = namedCallbacks[callbacksName];

const list = eventNames.split(' ');
list.forEach(function eventNamesAddEventListener(eventName) {
const eventNamesList = eventNames.split(' ');

const existingEventListenerCallback = eventListenerCallbacks.find(e => (
e.args.elem === elem &&
e.args.callback === callback &&
JSON.stringify(e.args.options) === JSON.stringify(options)
));

eventNamesList.forEach(function eventNamesAddEventListener(eventName) {
if(existingEventListenerCallback) {
if(existingEventListenerCallback.args.eventNamesList.includes(eventName)) {
return;
} else {
existingEventListenerCallback.args.eventNamesList.push(eventName);
}
}
elem.addEventListener(eventName, eventListenerCallback, options);
});

if (getListenerCallback) getListenerCallback(eventListenerCallback);
if(!existingEventListenerCallback) {
eventListenerCallbacks.push({
args: {
elem,
eventNamesList,
callback,
options,
},
callback: eventListenerCallback
});
}
} catch (ex) {
ex.details = {
eventNames,
Expand Down Expand Up @@ -204,7 +227,20 @@ export function off(elem, eventNames, callback) {
try {
const list = eventNames.split(' ');
list.forEach(function eventNamesRemoveEventListener(eventName) {
elem.removeEventListener(eventName, callback);
const eventListenerCallback = eventListenerCallbacks.find(e => (
e.args.elem === elem &&
e.args.callback === callback &&
e.args.eventNamesList.includes(eventName)
));
if(!eventListenerCallback) return;

eventListenerCallback.args.eventNamesList.splice(eventListenerCallback.args.eventNamesList.indexOf(eventName), 1);

if (eventListenerCallback.args.eventNamesList.length === 0) {
eventListenerCallbacks.splice(eventListenerCallbacks.indexOf(eventListenerCallback), 1);
}

elem.removeEventListener(eventName, eventListenerCallback.callback, eventListenerCallback.args.options);
});
} catch (ex) {
ex.details = {
Expand Down
54 changes: 23 additions & 31 deletions src/scripts/libs/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,9 +628,7 @@ export default class Settings {
on(
this.menuBtn,
'click',
this.onSettingsBtnClicked,
undefined,
(listener) => (this.onSettingsBtnClickedListener = listener)
this.onSettingsBtnClicked
);

const settingsMenuBtnTooltip = document.createElement('div');
Expand Down Expand Up @@ -1459,11 +1457,12 @@ export default class Settings {
menuOnCloseScrollBottom = -1;
menuOnCloseScrollHeight = 1;
onSettingsBtnClicked = async () => {
const isOpen = this.menuElem.classList.contains('is-visible');
const isOpen = (
this.menuElem.classList.contains('is-visible') ||
this.menuElem.classList.contains('fade-out')
);
if (isOpen) return;

off(this.menuBtn, 'click', this.onSettingsBtnClickedListener);

while (!this.ambientlight.initializedTime) {
await new Promise((resolve) => setTimeout(resolve, 500));
}
Expand All @@ -1487,21 +1486,26 @@ export default class Settings {
'ytp-ambientlight-settings-shown'
);
}

on(
document.body,
'click',
this.onCloseMenu,
{ capture: true }
);

setTimeout(() => {
on(
document.body,
'click',
this.onCloseMenu,
{ capture: true },
(listener) => (this.onCloseMenuListener = listener)
);
this.scrollToWarning();
}, 100);
};

onCloseMenu = (e) => {
if (!this.onCloseMenuListener) return;
const isOpen = (
this.menuElem.classList.contains('is-visible') ||
this.menuElem.classList.contains('fade-out')
);
if (!isOpen) return;

if (this.menuElem === e.target || this.menuElem.contains(e.target)) return;

e.stopPropagation();
Expand All @@ -1516,11 +1520,10 @@ export default class Settings {
on(
this.menuElem,
'animationend',
this.onSettingsFadeOutEnd,
undefined,
(listener) => (this.onSettingsFadeOutEndListener = listener)
this.onSettingsFadeOutEnd
);
this.onSettingsFadeOutEndTimeout = setTimeout(() => {
if(!this.onSettingsFadeOutEndTimeout) return;
this.onSettingsFadeOutEndTimeout = undefined;
this.onSettingsFadeOutEnd();
}, 500);
Expand All @@ -1534,23 +1537,13 @@ export default class Settings {
);
}

off(document.body, 'click', this.onCloseMenuListener);
this.onCloseMenuListener = undefined;
setTimeout(() => {
on(
this.menuBtn,
'click',
this.onSettingsBtnClicked,
undefined,
(listener) => (this.onSettingsBtnClickedListener = listener)
);
}, 100);
off(document.body, 'click', this.onCloseMenu);

this.hideUpdatesBadge();
};

onSettingsFadeOutEnd = () => {
off(this.menuElem, 'animationend', this.onSettingsFadeOutEndListener);
off(this.menuElem, 'animationend', this.onSettingsFadeOutEnd);
if (this.onSettingsFadeOutEndTimeout) {
clearTimeout(this.onSettingsFadeOutEndTimeout);
this.onSettingsFadeOutEndTimeout = undefined;
Expand Down Expand Up @@ -2104,8 +2097,7 @@ export default class Settings {
scrollToWarning() {
if (
!this.warningElem.textContent ||
!this.scrollToWarningQueued ||
!this.onCloseMenuListener
!this.scrollToWarningQueued
)
return;

Expand Down
1 change: 0 additions & 1 deletion src/scripts/libs/theming.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export default class Theming {
}
},
undefined,
undefined,
true
);

Expand Down

0 comments on commit 62937f2

Please sign in to comment.