[FIX] mail, web: fix memory leak in QUnit test suite
With the discuss refactoring, two memory leaks were introduced: - the activity service opens a broadcast channel for cross-tab communication. Because services have no destruction mechanism, the broadcast channel is never closed, and since most tests create their own test environment with services, each test would leak an instance of the activity service as it is captured by the bound onmessage function. The activity service holds a reference to the mail store which results in a large leak - the RelativeTime component creates a timeout so that it can update its time every minute or every hour depending on how old the message is, this timeout is cleared when the component is destroyed. This component tries to initialize the value of its timeout to `null`, but it does so *after* calling the method that actually sets the timeout, causing the timeout to be overriden with null and never cleared The BroadcastChannel problem has been solved by adding BroadcastChannel to the browser object, and patching it in the test setup code of web, as it's a generic problem with BroadcastChannels. The timeout problem has been fixed by simply reordering the lines so that we don't override the timeout and it gets correctly cleared when the component is destroyed. closes odoo/odoo#128768 X-original-commit: bd25f14d7a746fe48e09edea7ebb4aaa4634fbdc Signed-off-by: Aaron Bohy (aab) <aab@odoo.com> Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
@@ -12,8 +12,8 @@ export class RelativeTime extends Component {
|
||||
static template = xml`<t t-esc="relativeTime"/>`;
|
||||
|
||||
setup() {
|
||||
this.computeRelativeTime();
|
||||
this.timeout = null;
|
||||
this.computeRelativeTime();
|
||||
onWillDestroy(() => clearTimeout(this.timeout));
|
||||
}
|
||||
|
||||
|
||||
@@ -5,12 +5,13 @@ import { assignDefined } from "@mail/utils/common/misc";
|
||||
|
||||
import { _t } from "@web/core/l10n/translation";
|
||||
import { registry } from "@web/core/registry";
|
||||
import { browser } from "@web/core/browser/browser";
|
||||
|
||||
export class ActivityService {
|
||||
constructor(env, services) {
|
||||
try {
|
||||
// useful for synchronizing activity data between multiple tabs
|
||||
this.broadcastChannel = new BroadcastChannel("mail.activity.channel");
|
||||
this.broadcastChannel = new browser.BroadcastChannel("mail.activity.channel");
|
||||
this.broadcastChannel.onmessage = this._onBroadcastChannelMessage.bind(this);
|
||||
} catch {
|
||||
// BroadcastChannel API is not supported (e.g. Safari < 15.4), so disabling it.
|
||||
|
||||
@@ -45,6 +45,7 @@ export const browser = {
|
||||
innerHeight: window.innerHeight,
|
||||
innerWidth: window.innerWidth,
|
||||
ontouchstart: window.ontouchstart,
|
||||
BroadcastChannel: window.BroadcastChannel,
|
||||
};
|
||||
|
||||
Object.defineProperty(browser, "location", {
|
||||
|
||||
@@ -192,6 +192,13 @@ function patchBrowserWithCleanup() {
|
||||
cancelAnimationFrame: (handle) => {
|
||||
animationFrameHandles.delete(handle);
|
||||
},
|
||||
// BroadcastChannels need to be closed to be garbage collected
|
||||
BroadcastChannel: class SelfClosingBroadcastChannel extends BroadcastChannel {
|
||||
constructor() {
|
||||
super(...arguments);
|
||||
registerCleanup(() => this.close());
|
||||
}
|
||||
},
|
||||
},
|
||||
{ pure: true }
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user