Make race condition window for Conpherence smaller and rarer

Summary:
Ref T6713. This isn't very clean, and primarily unblocks D11143.

After D11143, I have a reliable local race where I submit, get a notification immediately, then get a double update (form submission + notification-triggered update).

Instead, make the notification updates wait for form submissions.

This doesn't resolve the race completely. The notification updates don't block chat submission (only the other way around), so if you're really fast you can submit at the same time someone else sends chat and race. But this fixes the most glaring issue.

The overall structure here is still pretty shaky but I tried to improve things a little, at least.

Test Plan: Chatted with myself, saw 0 races instead of 100% races.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T6713

Differential Revision: https://secure.phabricator.com/D11277

resources/celerity/map.php

@@ -349,7 +349,7 @@ return array(
     'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18',
     'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de',
     'rsrc/js/application/conpherence/behavior-menu.js' => 'f0a41b9f',
-    'rsrc/js/application/conpherence/behavior-pontificate.js' => '85ab3c8e',
+    'rsrc/js/application/conpherence/behavior-pontificate.js' => 'e23dfe0f',
     'rsrc/js/application/conpherence/behavior-widget-pane.js' => '40b1ff90',
     'rsrc/js/application/countdown/timer.js' => 'e4cc26b3',
     'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '469c0d9e',
@@ -553,7 +553,7 @@ return array(
     'javelin-behavior-choose-control' => '6153c708',
     'javelin-behavior-config-reorder-fields' => '14a827de',
     'javelin-behavior-conpherence-menu' => 'f0a41b9f',
-    'javelin-behavior-conpherence-pontificate' => '85ab3c8e',
+    'javelin-behavior-conpherence-pontificate' => 'e23dfe0f',
     'javelin-behavior-conpherence-widget-pane' => '40b1ff90',
     'javelin-behavior-countdown-timer' => 'e4cc26b3',
     'javelin-behavior-dark-console' => '08883e8b',
@@ -1342,13 +1342,6 @@ return array(
       'javelin-workflow',
       'phabricator-draggable-list',
     ),
-    '85ab3c8e' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-workflow',
-      'javelin-stratcom',
-    ),
     '85ea0626' => array(
       'javelin-install',
     ),
@@ -1777,6 +1770,13 @@ return array(
       'javelin-stratcom',
       'javelin-dom',
     ),
+    'e23dfe0f' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-workflow',
+      'javelin-stratcom',
+    ),
     'e32d14ab' => array(
       'javelin-behavior',
       'javelin-stratcom',

webroot/rsrc/js/application/conpherence/behavior-pontificate.js

@@ -9,6 +9,30 @@
 
 JX.behavior('conpherence-pontificate', function() {
 
+  // TODO: This isn't very clean. When you submit a message, you may get a
+  // notification about it back before you get the rendered message back. To
+  // prevent this, we keep track of whether we're currently updating the
+  // thread. If we are, we hold further updates until the response comes
+  // back.
+
+  // After the response returns, we'll do another update if we know about
+  // a transaction newer than the one we got back from the server.
+  var updating = null;
+
+  function get_thread_data() {
+    // TODO: This is really, really gross.
+    var infonode = JX.DOM.find(document, 'input', 'latest-transaction-id');
+    var data = JX.Stratcom.getData(infonode);
+    data.latestID = infonode.value;
+    return data;
+  }
+
+  function update_latest_transaction_id(id) {
+    // TODO: Continued grossness from above.
+    var infonode = JX.DOM.find(document, 'input', 'latest-transaction-id');
+    infonode.value = id;
+  }
+
   JX.Stratcom.listen('aphlict-receive-message', null, function(e) {
     var message = e.getData();
 
@@ -17,42 +41,72 @@ JX.behavior('conpherence-pontificate', function() {
       return;
     }
 
-    // TODO: This is really, really gross.
+    var data = get_thread_data();
-    var infonode = JX.DOM.find(document, 'input', 'latest-transaction-id');
-    var data = JX.Stratcom.getData(infonode);
 
-    var latest_id = infonode.value;
+    if (message.threadPHID != data.threadPHID) {
-    var thread_phid = data.threadPHID;
-    var thread_id = data.threadID;
-
-    if (message.threadPHID != thread_phid) {
       // Message event for some thread other than the visible one.
       return;
     }
 
-    if (message.messageID <= latest_id) {
+    if (message.messageID <= data.latestID) {
       // 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.
+    if (updating && updating.threadPHID == data.threadPHID) {
+      if (message.messageID > updating.knownID) {
+        updating.knownID = message.messageID;
+        return;
+      }
+    }
+
+    update_thread(data);
+  });
+
+  function update_thread(data) {
     var params = {
       action: 'load',
-      latest_transaction_id: latest_id
+      latest_transaction_id: data.latestID
     };
 
-    new JX.Workflow('/conpherence/update/' + thread_id + '/')
+    var uri = '/conpherence/update/' + data.threadID + '/';
+
+    var workflow = new JX.Workflow(uri)
       .setData(params)
       .setHandler(function(r) {
         var messages = JX.DOM.find(document, 'div', 'conpherence-messages');
         JX.DOM.appendContent(messages, JX.$H(r.transactions));
         messages.scrollTop = messages.scrollHeight;
 
-        // TODO: Continued grossness from above.
+        update_latest_transaction_id(r.latest_transaction_id);
-        infonode.value = r.latest_transaction_id;
+      });
-      })
-      .start();
-  });
 
+    sync_workflow(workflow, data);
+  }
+
+  function sync_workflow(workflow, data) {
+    updating = {
+      threadPHID: data.threadPHID,
+      knownID: data.latestID
+    };
+
+    workflow.listen('finally', function() {
+      var new_data = get_thread_data();
+      var need_sync = (updating.knownID > new_data.latestID);
+
+      updating = null;
+
+      if (need_sync) {
+        update_thread(new_data);
+      }
+    });
+
+    workflow.start();
+  }
 
   var onsubmit = function(e) {
     e.kill();
@@ -71,7 +125,7 @@ JX.behavior('conpherence-pontificate', function() {
     }
     JX.DOM.alterClass(form_root, 'loading', true);
 
-    JX.Workflow.newFromForm(form)
+    var workflow = JX.Workflow.newFromForm(form)
       .setHandler(JX.bind(this, function(r) {
         JX.DOM.appendContent(messages, JX.$H(r.transactions));
         messages.scrollTop = messages.scrollHeight;
@@ -83,11 +137,7 @@ JX.behavior('conpherence-pontificate', function() {
           );
         }
 
-        var latest_transaction_dom = JX.DOM.find(
+        update_latest_transaction_id(r.latest_transaction_id);
-          root,
-          'input',
-          'latest-transaction-id');
-        latest_transaction_dom.value = r.latest_transaction_id;
 
         var textarea = JX.DOM.find(form, 'textarea');
         textarea.value = '';
@@ -99,8 +149,9 @@ JX.behavior('conpherence-pontificate', function() {
           );
 
         JX.DOM.alterClass(form_root, 'loading', false);
-      }))
+      }));
-    .start();
+
+    sync_workflow(workflow, get_thread_data());
   };
 
   JX.Stratcom.listen(