pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[MERGE][FIX] bus, mail, web: several cross-tab communication fixes

Browse Source 
### [FIX] bus: background slave tabs should not loose notifications

Slave tabs missed some notifications in case the local storage
event's `newValue` did not match actual value in local storage. This
happens when master tab overwrites a value that slave tabs did not
take the time to read. Slave tabs should now always handle all local
storage events.

Task-ID 2044231

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

Cross-tab synchronization of document chat windows resulted sometimes
with an infinite loop. This happens due to tabs relying on actual
value in the local storage and all document chat windows state being
set as a single local storage entry. Cross-tab communication of
document chat windows now is relying on event's `newValue`, and each
document chat window has its own local storage entry. Changes from
local storage no longer writes again in the local storage, so that
it no longer makes an echo (= infinite loop).

Task-Id 2034997

closes odoo/odoo#36177

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
This commit is contained in:
Odoo's Mergebot
2019-09-03 13:06:23 +02:00
committed by GitHub
parent 82c0e9b620 40b57ca824
commit 85c5d0a4be
8 changed files with 130 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/bus/static/src/js/crosstab_bus.js
+6
View File
@@ -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
+24 -3
View File
@@ -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
+51 -22
View File
@@ -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
+5 -1
View File
@@ -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
+13 -7
View File
@@ -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();
addons/web/static/src/js/core/abstract_storage_service.js
+13
View File
@@ -44,6 +44,19 @@ var AbstractStorageService = AbstractService.extend({
var val = this.storage.getItem(key);
return val ? JSON.parse(val) : defaultValue;
},
/**
* @param {integer} index
* @return {string}
*/
key: function (index) {
return this.storage.key(index);
},
/**
* @return {integer}
*/
length: function () {
return this.storage.length;
},
/**
* Removes the given key from the storage
*
addons/web/static/src/js/core/local_storage.js
+4 -6
View File
@@ -35,12 +35,10 @@ try {
var newValue = e.originalEvent.newValue;
try {
JSON.parse(newValue);
if (localStorage.getItem(key) === newValue) {
storage.trigger('storage', {
key: key,
newValue: newValue,
});
}
storage.trigger('storage', {
key: key,
newValue: newValue,
});
} catch (error) {}
});
return storage;
addons/web/static/src/js/core/ram_storage.js
+14
View File
@@ -31,6 +31,7 @@ var RamStorage = Class.extend(mixins.EventDispatcherMixin, {
*/
clear: function () {
this.storage = Object.create(null);
this.length = 0;
},
/**
* Returns the value associated with a given key in the storage
@@ -41,12 +42,22 @@ var RamStorage = Class.extend(mixins.EventDispatcherMixin, {
getItem: function (key) {
return this.storage[key];
},
/**
* @param {integer} index
* @return {string}
*/
key: function (index) {
return _.keys(this.storage)[index];
},
/**
* Removes the given key from the storage
*
* @param {string} key
*/
removeItem: function (key) {
if (key in this.storage) {
this.length--;
}
delete this.storage[key];
this.trigger('storage', {key: key, newValue: null});
},
@@ -58,6 +69,9 @@ var RamStorage = Class.extend(mixins.EventDispatcherMixin, {
* @param {string} value
*/
setItem: function (key, value) {
if (!(key in this.storage)) {
this.length++;
}
this.storage[key] = value;
this.trigger('storage', {key: key, newValue: value});
},