From 2d42501a5b94de45e68a0b7ee8573aefc29500f0 Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Fri, 14 Jul 2023 11:37:36 +0000
Subject: [PATCH] [FIX] mail, web: fix memory leak in QUnit test suite
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 addons/mail/static/src/core/common/relative_time.js | 2 +-
 addons/mail/static/src/core/web/activity_service.js | 3 ++-
 addons/web/static/src/core/browser/browser.js       | 1 +
 addons/web/static/tests/setup.js                    | 7 +++++++
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/addons/mail/static/src/core/common/relative_time.js b/addons/mail/static/src/core/common/relative_time.js
index 9441ef8e8c3..47b317f1634 100644
--- a/addons/mail/static/src/core/common/relative_time.js
+++ b/addons/mail/static/src/core/common/relative_time.js
@@ -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));
     }
 
diff --git a/addons/mail/static/src/core/web/activity_service.js b/addons/mail/static/src/core/web/activity_service.js
index 140114981dd..c2c733004b5 100644
--- a/addons/mail/static/src/core/web/activity_service.js
+++ b/addons/mail/static/src/core/web/activity_service.js
@@ -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.
diff --git a/addons/web/static/src/core/browser/browser.js b/addons/web/static/src/core/browser/browser.js
index b0e03aad509..fc7562c04c2 100644
--- a/addons/web/static/src/core/browser/browser.js
+++ b/addons/web/static/src/core/browser/browser.js
@@ -45,6 +45,7 @@ export const browser = {
     innerHeight: window.innerHeight,
     innerWidth: window.innerWidth,
     ontouchstart: window.ontouchstart,
+    BroadcastChannel: window.BroadcastChannel,
 };
 
 Object.defineProperty(browser, "location", {
diff --git a/addons/web/static/tests/setup.js b/addons/web/static/tests/setup.js
index 5c5dc7084d8..43f0bf257ff 100644
--- a/addons/web/static/tests/setup.js
+++ b/addons/web/static/tests/setup.js
@@ -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 }
     );