From ce36c496800f4367462df3537c53a51e268f243a Mon Sep 17 00:00:00 2001 From: Nicolas Bayet Date: Tue, 26 Sep 2023 14:05:09 +0200 Subject: [PATCH] [FIX] web_editor: reconnect peers after when reconnecting internet Prior to this commit, when the user experienced an internet disconnection, after few seconds the PeerToPeer connection state would change to "closed" and the PeerToPeer connection would be removed permanently. Consequently, when the user reestablished their internet connection, the peers would remain disconnected to each other. With this commit, peers are now re-connecting to each other. task-3488166 closes odoo/odoo#138510 X-original-commit: b9d96629df5adf37f362eb63107fa5a9a6cd3e0d Signed-off-by: David Monjoie (dmo) --- .../js/editor/odoo-editor/src/OdooEditor.js | 3 + .../static/src/js/wysiwyg/PeerToPeer.js | 6 +- .../static/src/js/wysiwyg/wysiwyg.js | 23 ++-- .../tests/test_wysiwyg_collaboration.js | 101 ++++++++++++++++++ 4 files changed, 117 insertions(+), 16 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js index 1ad2cd47eb1..8d512de86c5 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js @@ -1202,6 +1202,9 @@ export class OdooEditor extends EventTarget { this.observerActive(); this.dispatchEvent(new Event('historyResetFromSteps')); } + historyGetSteps() { + return this._historySteps; + } historyGetMissingSteps({fromStepId, toStepId}) { const fromIndex = this._historySteps.findIndex(x => x.id === fromStepId); const toIndex = toStepId ? this._historySteps.findIndex(x => x.id === toStepId) : this._historySteps.length; diff --git a/addons/web_editor/static/src/js/wysiwyg/PeerToPeer.js b/addons/web_editor/static/src/js/wysiwyg/PeerToPeer.js index 0ddcd15b81e..72d7bd4fe5f 100644 --- a/addons/web_editor/static/src/js/wysiwyg/PeerToPeer.js +++ b/addons/web_editor/static/src/js/wysiwyg/PeerToPeer.js @@ -41,7 +41,11 @@ const baseNotificationMethods = { }, ptp_join: async function (notification) { - this._createClient(notification.fromClientId); + const clientId = notification.fromClientId; + if (this.clientsInfos[clientId] && this.clientsInfos[clientId].peerConnection) { + return this.clientsInfos[clientId]; + } + this._createClient(clientId); }, rtc_signal_icecandidate: async function (notification) { diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 1e9e6b40b0b..85ac6e0feb1 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -2686,19 +2686,11 @@ export class Wysiwyg extends Component { this._isOnline = true; if (!this.ptp) return; - // Ask for potential missing steps from all peers. - return Promise.all(this._getPtpClients().map(client => { - return this.requestClient( - client.id, - 'get_missing_steps', { - fromStepId: peek(this.odooEditor.historyGetBranchIds()).id, - }, - { transport: 'rtc' } - ).then(missingSteps => { - if (missingSteps === REQUEST_ERROR) return; - this._processMissingSteps(missingSteps); - }); - })); + // If it was disconnected to some peers, send the join signal again. + this.ptp.notifyAllClients('ptp_join'); + // Send last step to all peers. If the peers cannot add the step, they + // will ask for missing steps. + this.ptp.notifyAllClients('oe_history_step', peek(this.odooEditor.historyGetSteps()), { transport: 'rtc' }); } /** * Process missing steps received from a peer. @@ -2821,9 +2813,10 @@ export class Wysiwyg extends Component { } this._historySyncFinished = true; } else { - const currentStep = this.odooEditor._historySteps[this.odooEditor._historySteps.length - 1]; + // Make both send their last step to each other to + // ensure they are in sync. + this.ptp.notifyAllClients('oe_history_step', peek(this.odooEditor.historyGetSteps()), { transport: 'rtc' }); this._setCollaborativeSelection(fromClientId); - this.ptp.notifyClient(fromClientId, 'oe_history_step', currentStep, { transport: 'rtc' }); } this.requestClient(fromClientId, 'get_client_name', undefined, { transport: 'rtc' }).then((clientName) => { diff --git a/addons/web_editor/static/tests/test_wysiwyg_collaboration.js b/addons/web_editor/static/tests/test_wysiwyg_collaboration.js index e9fb8e88f8f..b58e345bf64 100644 --- a/addons/web_editor/static/tests/test_wysiwyg_collaboration.js +++ b/addons/web_editor/static/tests/test_wysiwyg_collaboration.js @@ -86,6 +86,16 @@ class PeerTest { connectionClientId: this.peerId, }); } + async removeDataChannel(peer) { + this._connections.delete(peer); + peer._connections.delete(this); + const ptpFrom = this.wysiwyg.ptp; + const ptpTo = peer.wysiwyg.ptp; + delete ptpFrom.clientsInfos[peer.peerId]; + delete ptpTo.clientsInfos[this.peerId]; + this.onlineMutex = new Mutex(); + this._onlineResolver = undefined; + } makeStep(fn) { fn(this); } @@ -149,6 +159,8 @@ class PeerPool { async function createPeers(peers) { const pool = new PeerPool(); + let lastGeneratedId = 0; + for (const peerId of peers) { const iframe = document.createElement('iframe'); if (navigator.userAgent.toLowerCase().indexOf('firefox') > -1) { @@ -263,6 +275,18 @@ async function createPeers(peers) { _getCollaborationClientAvatarUrl() { return ''; }, + async startEdition() { + await super.startEdition(...arguments); + patch(this.odooEditor, { + _generateId() { + // Ensure the id are deterministically gererated for + // when we need to sort by them. (eg. in the + // callaboration sorting of steps) + lastGeneratedId++; + return lastGeneratedId.toString(); + }, + }); + }, _hasICEServers() { return true; }, @@ -807,6 +831,83 @@ QUnit.module('web_editor', { }); }); }); + QUnit.module('Disconnect & reconnect', {}, () => { + QUnit.test('should sync history when disconnecting and reconnecting to internet', async (assert) => { + assert.expect(2); + const pool = await createPeers(['p1', 'p2']); + const peers = pool.peers; + + await peers.p1.startEditor(); + await peers.p2.startEditor(); + + await peers.p1.focus(); + await peers.p2.focus(); + await peers.p1.openDataChannel(peers.p2); + + await peers.p1.makeStep(insert('b')); + + await peers.p1.setOffline(); + peers.p1.removeDataChannel(peers.p2); + + const setSelection = (peer) => { + const selection = peer.document.getSelection(); + const pElement = peer.wysiwyg.odooEditor.editable.querySelector('p') + const range = new Range(); + range.setStart(pElement, 1); + range.setEnd(pElement, 1); + selection.removeAllRanges(); + selection.addRange(range); + } + const addP = (peer, content) => { + const p = document.createElement('p'); + p.textContent = content; + peer.wysiwyg.odooEditor.editable.append(p); + peer.wysiwyg.odooEditor.historyStep(); + } + + setSelection(peers.p1); + await peers.p1.makeStep(insert('c')); + addP(peers.p1, 'd'); + + setSelection(peers.p2); + await peers.p2.makeStep(insert('e')); + addP(peers.p2, 'f'); + + peers.p1.setOnline(); + peers.p2.setOnline(); + + // todo: p1PromiseForMissingStep and p2PromiseForMissingStep + // should be removed when the fix of undetected missing step + // will be merged. (task-3208277) + const p1PromiseForMissingStep = new Promise((resolve) => { + patch(peers.p2.wysiwyg, { + async _processMissingSteps() { + // Wait for the p2PromiseForMissingStep to resolve + // to avoid undetected missing step. + await p2PromiseForMissingStep; + super._processMissingSteps(...arguments); + resolve(); + } + }) + }); + const p2PromiseForMissingStep = new Promise((resolve) => { + patch(peers.p1.wysiwyg, { + async _processMissingSteps() { + super._processMissingSteps(...arguments); + resolve(); + } + }) + }); + + await peers.p1.openDataChannel(peers.p2); + await p1PromiseForMissingStep; + + assert.equal(peers.p1.getValue(), `

ac[]eb

d

f

`, 'p1 should have the value merged with p2'); + assert.equal(peers.p2.getValue(), `

ace[]b

d

f

`, 'p2 should have the value merged with p1'); + + removePeers(peers); + }); + }); }); });