From 3f60066283523ae1f32ba68bed7172dfb9dc266e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= <lul@odoo.com>
Date: Wed, 3 Aug 2022 07:13:30 +0000
Subject: [PATCH] [FIX] bus: stop listening when tab unloads

When a tab is closed (unloads), the main tab is still considered the main tab
and event listeners are not properly removed.

In practice, it's probably not an issue because the tab is killed by the browser,
but it could still potentially have unexpected and undesired side effects.
It's safer to clean everything properly.

The issue is much more visible in tests because the mutli_tab and bus services
are still "active" after we simulated a tab unload.

Part-of: odoo/odoo#97036
Co-authored-by: tsm-odoo <tsm@odoo.com>
---
 addons/bus/static/src/crosstab_bus.js         | 34 ++++++++-----------
 addons/bus/static/src/multi_tab_service.js    |  2 ++
 .../static/tests/multi_tab_service_tests.js   | 10 +++++-
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/addons/bus/static/src/crosstab_bus.js b/addons/bus/static/src/crosstab_bus.js
index d19742e907e..f89a83bda5b 100644
--- a/addons/bus/static/src/crosstab_bus.js
+++ b/addons/bus/static/src/crosstab_bus.js
@@ -33,11 +33,21 @@ export class CrossTab extends Longpolling {
             this.env.services['multi_tab'].removeSharedValue('last');
         }
         this._lastNotificationID = this.env.services['multi_tab'].getSharedValue('last', 0);
-        this._registerWindowUnload();
 
-        services['multi_tab'].bus.addEventListener('shared_value_updated', this._onSharedValueUpdated.bind(this));
-        services['multi_tab'].bus.addEventListener('no_longer_main_tab', () => this.stopPolling());
-        services['multi_tab'].bus.addEventListener('become_main_tab', () => this.startPolling());
+        const onSharedValueUpdated = (event) => this._onSharedValueUpdated(event);
+        const stopPolling = () => this.stopPolling();
+        const startPolling = () => this.startPolling();
+        services['multi_tab'].bus.addEventListener('shared_value_updated', onSharedValueUpdated);
+        services['multi_tab'].bus.addEventListener('no_longer_main_tab', stopPolling);
+        services['multi_tab'].bus.addEventListener('become_main_tab', startPolling);
+
+        browser.addEventListener('unload', () => {
+            services['multi_tab'].bus.removeEventListener('shared_value_updated', onSharedValueUpdated);
+            services['multi_tab'].bus.removeEventListener('no_longer_main_tab', stopPolling);
+            services['multi_tab'].bus.removeEventListener('become_main_tab', startPolling);
+            this._currentTabChannels.clear();
+            this._updateChannels();
+        });
     }
 
     //--------------------------------------------------------------------------
@@ -102,13 +112,6 @@ export class CrossTab extends Longpolling {
     // Private
     //--------------------------------------------------------------------------
 
-    /**
-     * @private
-     */
-    _registerWindowUnload() {
-        browser.addEventListener('unload', this._onUnload.bind(this));
-    }
-
     /**
      * Update localstorage channels of with the channels of this tab.
      *
@@ -203,13 +206,4 @@ export class CrossTab extends Longpolling {
         }
     }
 
-    /**
-     * Handler when unload the window
-     *
-     * @private
-     */
-    _onUnload() {
-        this._currentTabChannels.clear();
-        this._updateChannels();
-    }
 }
diff --git a/addons/bus/static/src/multi_tab_service.js b/addons/bus/static/src/multi_tab_service.js
index 108f83122e7..67d18943f3d 100644
--- a/addons/bus/static/src/multi_tab_service.js
+++ b/addons/bus/static/src/multi_tab_service.js
@@ -151,6 +151,8 @@ export const multiTabService = {
 
             // Unload main.
             if (_isOnMainTab) {
+                _isOnMainTab = false;
+                bus.trigger('no_longer_main_tab');
                 browser.localStorage.removeItem(generateLocalStorageKey('main'));
             }
         }
diff --git a/addons/bus/static/tests/multi_tab_service_tests.js b/addons/bus/static/tests/multi_tab_service_tests.js
index 6ba6aaf2f80..5cae1e03aa3 100644
--- a/addons/bus/static/tests/multi_tab_service_tests.js
+++ b/addons/bus/static/tests/multi_tab_service_tests.js
@@ -11,7 +11,7 @@ QUnit.module('bus', function () {
     QUnit.module('multi_tab_service_tests.js');
 
     QUnit.test('multi tab service elects new master on unload', async function (assert) {
-        assert.expect(2);
+        assert.expect(5);
 
         registry.category('services').add('multi_tab', multiTabService);
 
@@ -28,11 +28,19 @@ QUnit.module('bus', function () {
             },
         });
         const secondTabEnv = await makeTestEnv();
+        firstTabEnv.services["multi_tab"].bus.addEventListener("no_longer_main_tab", () =>
+            assert.step("tab1 no_longer_main_tab")
+        );
+        secondTabEnv.services["multi_tab"].bus.addEventListener("no_longer_main_tab", () =>
+            assert.step("tab2 no_longer_main_tab")
+        );
         window.dispatchEvent(new Event('unload'));
 
         // Let the multi tab elect a new main.
         await nextTick();
+        assert.notOk(firstTabEnv.services['multi_tab'].isOnMainTab());
         assert.ok(secondTabEnv.services['multi_tab'].isOnMainTab());
+        assert.verifySteps(['tab1 no_longer_main_tab']);
     });
 
     QUnit.test('multi tab allow to share values between tabs', async function (assert) {