Merge pull request #1537 from TimSBSquare/bugfix/existing-channel-race-condition

Fixed a race condition when attaching to an existing socket

core/rfb.js

@@ -286,8 +286,17 @@ export default class RFB extends EventTargetMixin {
         this._sock.on('error', e => Log.Warn("WebSocket on-error event"));
 
         // Slight delay of the actual connection so that the caller has
-        // time to set up callbacks
-        setTimeout(this._updateConnectionState.bind(this, 'connecting'));
+        // time to set up callbacks.
+        // This it not possible when a pre-existing socket is passed in and is just opened.
+        // If the caller creates this object in the open() callback of a socket and there's
+        // data pending doing it next tick causes a packet to be lost.
+        // This is particularly noticable for RTCDataChannel's where the other end creates
+        // the channel and the client, this end, gets notified it exists.
+        if (typeof urlOrChannel === 'string') {
+            setTimeout(this._updateConnectionState.bind(this, 'connecting'));
+        } else {
+            this._updateConnectionState('connecting');
+        }
 
         Log.Debug("<< RFB.constructor");
 

docs/API.md

@@ -168,6 +168,11 @@ connection to a specified VNC server.
 **`urlOrDataChannel`**
   - A `DOMString` specifying the VNC server to connect to. This must be
     a valid WebSocket URL. This can also be a `WebSocket` or `RTCDataChannel`.
+    If a `DOMString` is supplied then the connection will be delayed until the next tick to
+    allow allow adding event listeners that fire on connection. If an existing object
+    is supplied then the connection logic happens the same tick. For instance if passing
+    in an existing open WebSocket then it will not be possible to listen for the `connect`
+    event. This is to avoid dropping data on a connection that has data as soon as its opened.
 
 **`options`** *Optional*
   - An `Object` specifying extra details about how the connection