[Webapp Refactor] Reliably cancels a connection.

remoting/webapp/crd/js/client_session.js

Index: remoting/webapp/crd/js/client_session.js
diff --git a/remoting/webapp/crd/js/client_session.js b/remoting/webapp/crd/js/client_session.js
index 8a6bdb870f63bcd1b15258e48dc3c7dd16452a25..6941b420b4a26cb6d61a58fb79fe82ee24c5514c 100644
--- a/remoting/webapp/crd/js/client_session.js
+++ b/remoting/webapp/crd/js/client_session.js
@@ -88,17 +88,15 @@ remoting.ClientSession = function(plugin, signalStrategy, listener) {
 
 /** @enum {string} */
 remoting.ClientSession.Events = {
-  stateChanged: 'stateChanged', // deprecated.
-  videoChannelStateChanged: 'videoChannelStateChanged',
+  videoChannelStateChanged: 'videoChannelStateChanged'
 };
 
 /**
  * @interface
  * [START]-------> [onConnected] ------> [onDisconnected]
- *    |                              |
- *    |-----> [OnConnectionFailed]   |----> [onError]
+ *    |
+ *    |-----> [OnConnectionFailed]
  *
- * TODO(kelvinp): Route session state changes through this interface.
  */
 remoting.ClientSession.EventHandler = function() {};
 
@@ -112,7 +110,7 @@ remoting.ClientSession.EventHandler.prototype.onConnectionFailed =
 
 /**
  * Called when a new session has been connected.  The |connectionInfo| will be
- * valid until onDisconnected() or onError() is called.
+ * valid until onDisconnected() is called.
  *
  * @param {!remoting.ConnectionInfo} connectionInfo
  */
@@ -121,14 +119,12 @@ remoting.ClientSession.EventHandler.prototype.onConnected =
 
 /**
  * Called when the current session has been disconnected.
+ *
+ * @param {!remoting.Error} reason Reason that the session is disconnected.
+ *     Set to remoting.Error.none() if there is no error.
  */
-remoting.ClientSession.EventHandler.prototype.onDisconnected = function() {};
-
-/**
- * Called when an error needs to be displayed to the user.
- * @param {!remoting.Error} error
- */
-remoting.ClientSession.EventHandler.prototype.onError = function(error) {};
+remoting.ClientSession.EventHandler.prototype.onDisconnected =
+    function(reason) {};
 
 // Note that the positive values in both of these enums are copied directly
 // from connection_to_host.h and must be kept in sync. Code in
@@ -165,19 +161,6 @@ remoting.ClientSession.State.fromString = function(state) {
   return remoting.ClientSession.State[state];
 };
 
-/**
-  @param {remoting.ClientSession.State} current
-  @param {remoting.ClientSession.State} previous
-  @constructor
-*/
-remoting.ClientSession.StateEvent = function(current, previous) {
-  /** @type {remoting.ClientSession.State} */
-  this.previous = previous
-
-  /** @type {remoting.ClientSession.State} */
-  this.current = current;
-};
-
 /** @enum {number} */
 remoting.ClientSession.ConnectionError = {
   UNKNOWN: -1,
@@ -285,6 +268,10 @@ remoting.ClientSession.prototype.connect = function(host, credentialsProvider) {
  * @return {void} Nothing.
  */
 remoting.ClientSession.prototype.disconnect = function(error) {
+  if (this.isFinished()) {

[Jamie, 2015/04/22 23:11:55]

Does the sendIq call (below) still happen in all cases?

[kelvinp, 2015/04/23 01:17:23]

No and it shoudn't happen if the session is already finished.

For most connection state changes, the client session merely responds to
notifications from the plugin.  Disconnect is a special case as the
clientSession is actively sending the IQ termination and faking a state
transition.

In a cleaner design, the clientSession should expose a disconnect() method which
the clientSession can call into and responds to the CLOSED state changes.

[Jamie, 2015/04/24 17:59:18]

This message is needed so that the host knows that the client has gone away and
can clean up (for example, resetting the desktop size and dismissing the
"connected" dialog). Are you saying that this will no longer happen in some
cases?

[kelvinp, 2015/04/24 19:03:52]

Discussed offline and comments added.

+    return;
+  }
+
   this.sendIq_(
       '<cli:iq ' +
           'to="' + this.host_.jabberId + '" ' +
@@ -551,7 +538,7 @@ remoting.ClientSession.prototype.notifyStateChanges_ =
 
     case remoting.ClientSession.State.CLOSED:
       console.log('Connection closed.');
-      this.listener_.onDisconnected();
+      this.listener_.onDisconnected(remoting.Error.none());
       break;
 
     case remoting.ClientSession.State.CONNECTION_CANCELED:
@@ -566,20 +553,12 @@ remoting.ClientSession.prototype.notifyStateChanges_ =
     case remoting.ClientSession.State.CONNECTION_DROPPED:
       error = this.getError();
       console.error('Connection dropped: ' + error.toString());
-      this.listener_.onError(error);
+      this.listener_.onDisconnected(error);
       break;
 
     default:
       console.error('Unexpected client plugin state: ' + newState);
-      // This should only happen if the web-app and client plugin get out of
-      // sync, and even then the version check should ensure compatibility.
-      this.listener_.onError(
-          new remoting.Error(remoting.Error.Tag.MISSING_PLUGIN));
   }
-
-  this.raiseEvent(remoting.ClientSession.Events.stateChanged,
-    new remoting.ClientSession.StateEvent(newState, oldState)
-  );
 };
 
 /**