[FIX] bus, mail: correctly synchronize document chat windows

Before this commit, there could be an infinite loop when changing
the chat window state of a document chat window.

This is caused when mutiple tabs overwrite the chat window state
with a different value everytime, resulting to an infinite loop.
Accross multiple tabs, `setItem` and `onStorage` are not synchronous.
That's why tabs should communicate through local storage events
instead of actual value in the local storage.

To illustrate the issue, suppose there are 2 tabs (T1 and T2) with
one document chat windows (C) and the following user interactions:

     -  T1 and T2 both have C open
     -  on T1, user folds C
     -  on T2, user folds C

Now let's add an example of local storage (LS) logics that could
apply from above example:

     -  T1 and T2 both have C open
     -  on T1, user folds C
    [1] T1 writes 'C folded' in LS
     -  on T2, user folds C
    [2] T2 writes 'C folded' in LS
    [3] T2 detects 'C folded' in LS (from [1])
           => T2 writes 'C folded' in LS
    [4] T1 detects 'C folded' in LS (from [2])
           => T1 writes 'C folded' in LS
    [5] T1 detects 'C folded' in LS (from [3])
           => T1 writes 'C folded' in LS
    [6] T2 detects 'C folded' in LS (from [4])
           => T2 writes 'C folded' in LS
     etc...

This scenario could have been easily prevented by not writing on
local storage when the chat window state has not changed. However,
this wouldn't fix this scenario that also introduces an infinite
loop:

    -  T1 and T2 both have C open
    -  on T1, user folds C
   [1] T1 writes 'C folded' in LS
    -  on T2, user folds C
   [2] T2 writes 'C folded' in LS
   -   on T2, user unfolds C
   [3] T2 writes 'C unfolded' in LS
   [4] T2 detects 'C folded' in LS (from [1])
          => T2 writes 'C folded' in LS
   [5] T1 detects 'C unfolded' in LS (from [3])
          => T1 writes 'C unfolded' in LS
   [6] T2 detects 'C unfolded' in LS (from [5])
          => T2 writes 'C unfolded' in LS
   [7] T1 detects 'C folded' in LS (from [4])
          => T1 writes 'C folded' in LS
   [8] T2 detects 'C folded' in LS (from [7])
          => T2 writes 'C folded' in LS
   [9] T1 detects 'C unfolded' in LS (from [6])
          => T1 writes 'C folded' in LS
   etc...

This commit fixes the infinite loop issue:

  - by turning local storage read during cross-tab communication into
    local storage event read instead. This should prevent race
    conditions based on read/write operations in local, since their
    order is not guaranteed accros tabs.
  - by isolating document chat window state to their own local
    storage entry. This should prevent a tab from notifying a chat
    window state that it didn't change, which may also result in
    an infinite loop.

Some tests were adapted from the changes above. Also, some tests did
pass thanks to local storage being mocked by a ram storage: the
handler `_onStorage` should have been called, but it didn't because
it was registered on the actual local storage instead of the ram
storage. In order to make tests pass again, the following additional
changes were required:

  - Cross-tab bus service now exports the unique tab ID with
    `getTabId`.
  - document thread entries in local storage now track the tab ID, so
    that self-tab ignore their own local storage writes.

Task-Id 2034997

Closes odoo#36177

addons/bus/static/src/js/crosstab_bus.js

@@ -79,6 +79,12 @@ var CrossTabBus = Longpolling.extend({
         this._super.apply(this, arguments);
         this._callLocalStorage('setItem', 'channels', this._channels);
     },
+    /**
+     * @return {string}
+     */
+    getTabId: function () {
+        return this._id;
+    },
     /**
      * Tells whether this bus is related to the master tab.
      *

addons/mail/static/src/js/models/threads/document_thread.js

@@ -62,9 +62,16 @@ var DocumentThread = Thread.extend({
      * shared between tabs, and restored on F5.
      *
      * @override
+     * @param {Object} [options]
+     * @param {boolean} [options.skipCrossTabSync=false] if set, it should
+     *   not notify other tabs from updating document thread state. This happens
+     *   in case the `close` operation comes from local storage event.
      */
-    close: function () {
+    close: function (options) {
         this._super.apply(this, arguments);
+        if (options && options.skipCrossTabSync) {
+            return;
+        }
         this.call('mail_service', 'updateDocumentThreadState', this._id, {
             name: this.getName(),
             windowState: 'closed',
@@ -75,9 +82,16 @@ var DocumentThread = Thread.extend({
      * shared between tabs, and restored on F5.
      *
      * @override
+     * @param {Object} [options]
+     * @param {boolean} [options.skipCrossTabSync=false] if set, it should
+     *   not notify other tabs from updating document thread state. This happens
+     *   in case the `detach` operation comes from local storage event.
      */
-    detach: function () {
+    detach: function (options) {
         this._super.apply(this, arguments);
+        if (options && options.skipCrossTabSync) {
+            return;
+        }
         var windowState = this._folded ? 'folded' : 'open';
         this.call('mail_service', 'updateDocumentThreadState', this._id, {
             name: this.getName(),
@@ -105,9 +119,16 @@ var DocumentThread = Thread.extend({
      * shared between tabs, and restored on F5.
      *
      * @override
+     * @param {Object} [options]
+     * @param {boolean} [options.skipCrossTabSync=false] if set, it should
+     *   not notify other tabs from updating document thread state. This happens
+     *   in case the `detach` operation comes from local storage event.
      */
-    fold: function () {
+    fold: function (folded, options) {
         this._super.apply(this, arguments);
+        if (options && options.skipCrossTabSync) {
+            return;
+        }
         var windowState = this._folded ? 'folded' : 'open';
         this.call('mail_service', 'updateDocumentThreadState', this._id, {
             name: this.getName(),

addons/mail/static/src/js/services/mail_document_thread_manager.js

@@ -21,17 +21,23 @@ MailManager.include({
 
     start: function () {
         this._super.apply(this, arguments);
-
+        this._documentThreadWindowStates = {};
+        this._tabId = this.call('bus_service', 'getTabId');
         // retrieve the open DocumentThreads from the localStorage
-        var state = this.call('local_storage', 'getItem', this.DOCUMENT_THREAD_STATE_KEY);
-        if (!state) {
-            this.call('local_storage', 'setItem', this.DOCUMENT_THREAD_STATE_KEY, {});
-        } else {
-            this.isReady().then(this._updateDocumentThreadWindows.bind(this, state));
+        var localStorageLength = this.call('local_storage', 'length');
+        for (var i = 0; i < localStorageLength; i++) {
+            var key = this.call('local_storage', 'key', i);
+            if (key.indexOf(this.DOCUMENT_THREAD_STATE_KEY) === 0) {
+                // key format: DOCUMENT_THREAD_STATE_KEY + '/' + threadId
+                var documentThreadId = key.substring(this.DOCUMENT_THREAD_STATE_KEY.length+1);
+                this._documentThreadWindowStates[documentThreadId] =
+                    this.call('local_storage', 'getItem', key);
+            }
         }
+        this.isReady().then(this._updateDocumentThreadWindows.bind(this, this._documentThreadWindowStates));
         // listen to localStorage changes to synchronize DocumentThread's
         // windows between tabs
-        window.addEventListener('storage', this._onStorage.bind(this));
+        this.call('local_storage', 'onStorage', this, this._onStorage);
     },
 
     //--------------------------------------------------------------------------
@@ -60,7 +66,10 @@ MailManager.include({
             options.postedFromDocumentThread
         ) {
             var key = this.DOCUMENT_THREAD_MESSAGE_KEY;
-            this.call('local_storage', 'setItem', key, data);
+            this.call('local_storage', 'setItem', key, {
+                messageData: data,
+                tabId: this._tabId,
+            });
         }
         return this._super.apply(this, arguments);
     },
@@ -144,12 +153,11 @@ MailManager.include({
      * @param {string} state.windowState ('closed', 'folded' or 'open')
      */
     updateDocumentThreadState: function (threadID, state) {
-        var states = this.call('local_storage', 'getItem', this.DOCUMENT_THREAD_STATE_KEY);
-        states = _.omit(states, function (state) {
-            return state.windowState === 'closed';
+        this._documentThreadWindowStates[threadID] = state;
+        this.call('local_storage', 'setItem', this.DOCUMENT_THREAD_STATE_KEY + '/' + threadID, {
+            state: state,
+            tabId: this._tabId,
         });
-        states[threadID] = state;
-        this.call('local_storage', 'setItem', this.DOCUMENT_THREAD_STATE_KEY, states);
     },
 
     //--------------------------------------------------------------------------
@@ -194,10 +202,17 @@ MailManager.include({
                 resModel: info[0],
             });
             if (state.windowState === 'closed') {
-                documentThread.close();
+                documentThread.close({
+                    skipCrossTabSync: true,
+                });
             } else {
-                documentThread.fold(state.windowState === 'folded');
-                self.openThreadWindow(documentThread.getID(), { keepFoldState: true });
+                documentThread.fold(state.windowState === 'folded', {
+                    skipCrossTabSync: true,
+                });
+                self.openThreadWindow(documentThread.getID(), {
+                    keepFoldState: true,
+                    skipCrossTabSync: true,
+                });
             }
         });
     },
@@ -213,13 +228,27 @@ MailManager.include({
      * @param {StorageEvent} ev
      */
     _onStorage: function (ev) {
-        if (ev.key === this.DOCUMENT_THREAD_STATE_KEY) {
-            var state = this.call('local_storage', 'getItem', this.DOCUMENT_THREAD_STATE_KEY);
-            this._updateDocumentThreadWindows(state);
+        var value;
+        try {
+            value = JSON.parse(ev.newValue);
+        } catch (err) {
+            return;
+        }
+        if (ev.key.indexOf(this.DOCUMENT_THREAD_STATE_KEY) === 0) {
+            if (value.tabId === this._tabId) {
+                return;
+            }
+            // key format: DOCUMENT_THREAD_STATE_KEY + '/' + threadId
+            var documentThreadId = ev.key.substring(this.DOCUMENT_THREAD_STATE_KEY.length+1);
+            var param = {};
+            param[documentThreadId] = value.state;
+            this._updateDocumentThreadWindows(param);
         } else if (ev.key === this.DOCUMENT_THREAD_MESSAGE_KEY) {
-            var message = this.call('local_storage', 'getItem', this.DOCUMENT_THREAD_MESSAGE_KEY);
-            if (message) {
-                this.addMessage(message);
+            if (value.tabId === this._tabId) {
+                return;
+            }
+            if (value.messageData) {
+                this.addMessage(value.messageData);
             }
         }
     },

addons/mail/static/src/js/services/mail_window_manager.js

@@ -94,6 +94,8 @@ MailManager.include({
      *   is not open yet, and do nothing otherwise.
      * @param {boolean} [options.keepFoldState=false] if set to true, keep the
      *   fold state of the thread
+     * @param {boolean} [options.skipCrossTabSync=false] if set, thread
+     *   should not notify other tabs from new document thread chat window state.
      */
     openThreadWindow: function (threadID, options) {
         var self = this;
@@ -130,7 +132,9 @@ MailManager.include({
                 // thread window could not be open, which may happen due to
                 // access error while fetching messages to the document.
                 // abort opening the thread window in this case.
-                thread.close();
+                thread.close({
+                    skipCrossTabSync: true,
+                });
             }).always(function () {
                 thread.isCreatingWindow = false;
             });

addons/mail/static/tests/document_thread_window_tests.js

@@ -201,7 +201,7 @@ QUnit.test('post messages in a document thread window', function (assert) {
     testUtils.intercept(messagingMenu, 'call_service', function (ev) {
         if (ev.data.service === 'local_storage' && ev.data.method === 'setItem' &&
             ev.data.args[0] === 'mail.document_threads_last_message') {
-            assert.deepEqual(ev.data.args[1], newMessage,
+            assert.deepEqual(ev.data.args[1].messageData, newMessage,
                 "should write sent message in local storage, to share info with other tabs");
         }
     }, true);
@@ -229,7 +229,7 @@ QUnit.test('post messages in a document thread window', function (assert) {
 });
 
 QUnit.test('open, fold, unfold and close a document thread window', function (assert) {
-    assert.expect(8);
+    assert.expect(24);
 
     var messagingMenu = new MessagingMenu();
     testUtils.addMockEnvironment(messagingMenu, {
@@ -239,7 +239,13 @@ QUnit.test('open, fold, unfold and close a document thread window', function (as
     });
     testUtils.intercept(messagingMenu, 'call_service', function (ev) {
         if (ev.data.service === 'local_storage' && ev.data.method === 'setItem') {
-            assert.step(ev.data.args);
+            const key = ev.data.args[0];
+            const state = ev.data.args[1].state;
+            assert.strictEqual(key, 'mail.document_threads_state/some.res.model_1');
+            assert.ok(state);
+            assert.ok(state.name);
+            assert.ok(state.windowState);
+            assert.step(`${key}: { name: "${state.name}", windowState: '${state.windowState}' }`);
         }
     }, true);
     messagingMenu.appendTo($('#qunit-fixture'));
@@ -262,10 +268,10 @@ QUnit.test('open, fold, unfold and close a document thread window', function (as
     $('.o_thread_window .o_thread_window_close').click();
 
     assert.verifySteps([
-        ['mail.document_threads_state', {"some.res.model_1": {"name": "Some Record", "windowState": "open"}}],
-        ['mail.document_threads_state', {"some.res.model_1": {"name": "Some Record", "windowState": "folded"}}],
-        ['mail.document_threads_state', {"some.res.model_1": {"name": "Some Record", "windowState": "open"}}],
-        ['mail.document_threads_state', {"some.res.model_1": {"name": "Some Record", "windowState": "closed"}}],
+        `mail.document_threads_state/some.res.model_1: { name: "Some Record", windowState: 'open' }`,
+        `mail.document_threads_state/some.res.model_1: { name: "Some Record", windowState: 'folded' }`,
+        `mail.document_threads_state/some.res.model_1: { name: "Some Record", windowState: 'open' }`,
+        `mail.document_threads_state/some.res.model_1: { name: "Some Record", windowState: 'closed' }`,
     ]);
 
     messagingMenu.destroy();