diff options
author | Samuel Mannehed <samuel@cendio.se> | 2016-11-08 16:56:35 +0100 |
---|---|---|
committer | Samuel Mannehed <samuel@cendio.se> | 2016-11-10 15:17:31 +0100 |
commit | b2e961d48d6c4c05f695437ebf410ca56c46b8fe (patch) | |
tree | f7f56bb26758b94dbe396c90e2b3261c5ce8c29e | |
parent | b45905ab863903ba0d1894fca9b4110a1a7d1004 (diff) | |
download | novnc-b2e961d48d6c4c05f695437ebf410ca56c46b8fe.tar.gz |
Ensure proper connection state transitions
Makes the state machine more rubust and clear.
-rw-r--r-- | core/rfb.js | 63 | ||||
-rw-r--r-- | tests/test.rfb.js | 40 |
2 files changed, 75 insertions, 28 deletions
diff --git a/core/rfb.js b/core/rfb.js index 9d968bb..91d9a6f 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -239,7 +239,7 @@ this._updateConnectionState('disconnected'); break; case 'disconnected': - Util.Error("Received onclose while disconnected" + msg); + this._fail("Received onclose while disconnected" + msg); break; default: this._fail("Unexpected server disconnect" + msg); @@ -473,19 +473,7 @@ return; } - this._rfb_connection_state = state; - - var smsg = "New state '" + state + "', was '" + oldstate + "'."; - Util.Debug(smsg); - - if (this._disconnTimer && state !== 'disconnecting') { - Util.Debug("Clearing disconnect timer"); - clearTimeout(this._disconnTimer); - this._disconnTimer = null; - this._sock.off('close'); // make sure we don't get a double event - } - - this._onUpdateState(this, state, oldstate); + // Ensure proper transitions before doing anything switch (state) { case 'connected': if (oldstate !== 'connecting') { @@ -501,7 +489,50 @@ "previous connection state: " + oldstate); return; } + break; + + case 'connecting': + if (oldstate !== '') { + Util.Error("Bad transition to connecting state, " + + "previous connection state: " + oldstate); + return; + } + break; + + case 'disconnecting': + if (oldstate !== 'connected' && oldstate !== 'connecting') { + Util.Error("Bad transition to disconnecting state, " + + "previous connection state: " + oldstate); + return; + } + break; + default: + Util.Error("Unknown connection state: " + state); + return; + } + + // State change actions + + this._rfb_connection_state = state; + this._onUpdateState(this, state, oldstate); + + var smsg = "New state '" + state + "', was '" + oldstate + "'."; + Util.Debug(smsg); + + if (this._disconnTimer && state !== 'disconnecting') { + Util.Debug("Clearing disconnect timer"); + clearTimeout(this._disconnTimer); + this._disconnTimer = null; + + // make sure we don't get a double event + this._sock.off('close'); + } + + switch (state) { + case 'disconnected': + // Call onDisconnected callback after onUpdateState since + // we don't know if the UI only displays the latest message if (this._rfb_disconnect_reason !== "") { this._onDisconnected(this, this._rfb_disconnect_reason); } else { @@ -522,10 +553,6 @@ this._updateConnectionState('disconnected'); }.bind(this), this._disconnectTimeout * 1000); break; - - default: - Util.Error("Unknown connection state: " + state); - return; } }, diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 828456a..60bbe9a 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -330,7 +330,7 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should clear the disconnect timer if the state is not "disconnecting"', function () { var spy = sinon.spy(); client._disconnTimer = setTimeout(spy, 50); - client._updateConnectionState('connected'); + client._updateConnectionState('connecting'); this.clock.tick(51); expect(spy).to.not.have.been.called; expect(client._disconnTimer).to.be.null; @@ -338,27 +338,37 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should call the updateState callback', function () { client.set_onUpdateState(sinon.spy()); - client._updateConnectionState('a specific state'); + client._updateConnectionState('connecting'); var spy = client.get_onUpdateState(); expect(spy).to.have.been.calledOnce; - expect(spy.args[0][1]).to.equal('a specific state'); + expect(spy.args[0][1]).to.equal('connecting'); }); it('should set the rfb_connection_state', function () { - client._updateConnectionState('a specific state'); - expect(client._rfb_connection_state).to.equal('a specific state'); + client._rfb_connection_state = 'disconnecting'; + client._updateConnectionState('disconnected'); + expect(client._rfb_connection_state).to.equal('disconnected'); }); it('should not change the state when we are disconnected', function () { client._rfb_connection_state = 'disconnected'; - client._updateConnectionState('a specific state'); - expect(client._rfb_connection_state).to.not.equal('a specific state'); + client._updateConnectionState('connecting'); + expect(client._rfb_connection_state).to.not.equal('connecting'); }); it('should ignore state changes to the same state', function () { client.set_onUpdateState(sinon.spy()); - client._rfb_connection_state = 'a specific state'; - client._updateConnectionState('a specific state'); + client._rfb_connection_state = 'connecting'; + client._updateConnectionState('connecting'); + var spy = client.get_onUpdateState(); + expect(spy).to.not.have.been.called; + }); + + it('should ignore illegal state changes', function () { + client.set_onUpdateState(sinon.spy()); + client._rfb_connection_state = 'connected'; + client._updateConnectionState('disconnected'); + expect(client._rfb_connection_state).to.not.equal('disconnected'); var spy = client.get_onUpdateState(); expect(spy).to.not.have.been.called; }); @@ -391,11 +401,13 @@ describe('Remote Frame Buffer Protocol Client', function() { }); it('should set disconnect_reason', function () { + client._rfb_connection_state = 'connected'; client._fail('a reason'); expect(client._rfb_disconnect_reason).to.equal('a reason'); }); it('should result in disconnect callback with message when reason given', function () { + client._rfb_connection_state = 'connected'; client.set_onDisconnected(sinon.spy()); client._fail('a reason'); var spy = client.get_onDisconnected(); @@ -542,7 +554,7 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should call the updateState callback before the disconnect callback', function () { client.set_onDisconnected(sinon.spy()); client.set_onUpdateState(sinon.spy()); - client._rfb_connection_state = 'other state'; + client._rfb_connection_state = 'disconnecting'; client._updateConnectionState('disconnected'); var updateStateSpy = client.get_onUpdateState(); var disconnectSpy = client.get_onDisconnected(); @@ -2120,6 +2132,14 @@ describe('Remote Frame Buffer Protocol Client', function() { expect(client._fail).to.have.been.calledOnce; }); + it('should fail if we get a close event while disconnected', function () { + sinon.spy(client, "_fail"); + client.connect('host', 8675); + client._rfb_connection_state = 'disconnected'; + client._sock._websocket.close(); + expect(client._fail).to.have.been.calledOnce; + }); + it('should unregister close event handler', function () { sinon.spy(client._sock, 'off'); client.connect('host', 8675); |