From 791e897c0d0f7606289e486c4f2bd9d314748c3f Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Thu, 7 May 2015 11:26:48 -0700 Subject: [PATCH] Conpherence - improve reliability of message delivery Summary: Ref T7755. T7755#107290 reproduced for me reliably and now it does not. Cleaned up the logic around in flight updates as it was not correct. Not sure this is enough to close T7755, but maybe? Test Plan: T7755#107290 no longer reproduces! Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6713, T7755 Differential Revision: https://secure.phabricator.com/D12755 --- resources/celerity/map.php | 26 +++++++++---------- .../conpherence/ConpherenceThreadManager.js | 16 ++++++++---- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 30dbefb7bc..8f2e5af1a4 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'ca3f6a60', - 'core.pkg.js' => 'ff529dc7', + 'core.pkg.js' => 'f6b48b53', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => 'bb338e4b', 'differential.pkg.js' => '895b8d62', @@ -346,7 +346,7 @@ return array( 'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761', 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', - 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '6709c934', + 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'f8dace3b', 'rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js' => 'cf86d16a', 'rsrc/js/application/conpherence/behavior-durable-column.js' => '657c2b50', 'rsrc/js/application/conpherence/behavior-menu.js' => '804b0773', @@ -513,7 +513,7 @@ return array( 'conpherence-menu-css' => 'f389e048', 'conpherence-message-pane-css' => '3150e2a2', 'conpherence-notification-css' => 'd208f806', - 'conpherence-thread-manager' => '6709c934', + 'conpherence-thread-manager' => 'f8dace3b', 'conpherence-transaction-css' => '25138b7f', 'conpherence-update-css' => '1099a660', 'conpherence-widget-pane-css' => '2af42ebe', @@ -1288,16 +1288,6 @@ return array( 'phabricator-keyboard-shortcut', 'conpherence-thread-manager', ), - '6709c934' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - ), '6882e80a' => array( 'javelin-dom', ), @@ -1997,6 +1987,16 @@ return array( 'javelin-util', 'phabricator-busy', ), + 'f8dace3b' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + ), 'f9539603' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js b/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js index b1694e19ca..2f4c72bd16 100644 --- a/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js +++ b/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js @@ -164,15 +164,19 @@ JX.install('ConpherenceThreadManager', { // Message event for something we already know about. return; } - // If we're currently updating, wait for the update to complete. + // If this notification tells us about a message which is newer than - // the newest one we know to exist, keep track of it so we can - // update once the in-flight update finishes. + // the newest one we know to exist, update our latest knownID so we + // can properly update later. if (this._updating && this._updating.threadPHID == this._loadedThreadPHID) { if (message.messageID > this._updating.knownID) { this._updating.knownID = message.messageID; - return; + // We're currently updating, so wait for the update to complete. + // this.syncWorkflow has us covered in this case. + if (this._updating.active) { + return; + } } } @@ -226,7 +230,8 @@ JX.install('ConpherenceThreadManager', { syncWorkflow: function(workflow, stage) { this._updating = { threadPHID: this._loadedThreadPHID, - knownID: this._latestTransactionID + knownID: this._latestTransactionID, + active: true }; workflow.listen(stage, JX.bind(this, function() { // TODO - do we need to handle if we switch threads somehow? @@ -235,6 +240,7 @@ JX.install('ConpherenceThreadManager', { if (need_sync) { return this._updateThread(); } + this._updating.active = false; })); workflow.start(); },