[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>
This commit is contained in:
co-authored by
tsm-odoo
parent
d367580f82
commit
3f60066283
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -151,6 +151,8 @@ export const multiTabService = {
|
||||
|
||||
// Unload main.
|
||||
if (_isOnMainTab) {
|
||||
_isOnMainTab = false;
|
||||
bus.trigger('no_longer_main_tab');
|
||||
browser.localStorage.removeItem(generateLocalStorageKey('main'));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user