Clean up client state callback.

remoting/webapp/me2mom/client_session.js

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * Class handling creation and teardown of a remoting client session.
  *
  * This abstracts a <embed> element and controls the plugin which does the
  * actual remoting work.  There should be no UI code inside this class.  It
  * should be purely thought of as a controller of sorts.
  */
 
 'use strict';
 
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
 /**
  * @param {string} hostJid The jid of the host to connect to.
  * @param {string} hostPublicKey The base64 encoded version of the host's
  *     public key.
  * @param {string} accessCode The access code for the IT2Me connection.
  * @param {string} email The username for the talk network.
- * @param {function(remoting.ClientSession.State):void} onStateChange
- *     The callback to invoke when the session changes state. This callback
- *     occurs after the state changes and is passed the previous state; the
- *     new state is accessible via ClientSession's |state| property.
+ * @param {function(remoting.ClientSession.State,
+                    remoting.ClientSession.State):void} onStateChange
+ *     The callback to invoke when the session changes state.
  * @constructor
  */
 remoting.ClientSession = function(hostJid, hostPublicKey, accessCode, email,
                                   onStateChange) {
   this.state = remoting.ClientSession.State.CREATED;
 
   this.hostJid = hostJid;
   this.hostPublicKey = hostPublicKey;
   this.accessCode = accessCode;
   this.email = email;
   this.clientJid = '';
   this.sessionId = '';
   /** @type {remoting.ViewerPlugin} */ this.plugin = null;
   this.logToServer = new remoting.LogToServer();
   this.onStateChange = onStateChange;
 };
 
+// Note that the (positive) values in both of these enums are copied directly

[Wez, 2011/11/16 21:58:26]

Why is positive in brackets?  Are the negative values copied or not?

[Jamie, 2011/11/17 22:06:18]

No. I've removed the brackets and expanded a bit.

+// from chromoting_scriptable_object.h and must be kept in sync.
 /** @enum {number} */
 remoting.ClientSession.State = {
+  CREATED: -3,
+  BAD_PLUGIN_VERSION: -2,
+  UNKNOWN_PLUGIN_ERROR: -1,
   UNKNOWN: 0,
-  CREATED: 1,
-  BAD_PLUGIN_VERSION: 2,
-  UNKNOWN_PLUGIN_ERROR: 3,
-  CONNECTING: 4,
-  INITIALIZING: 5,
-  CONNECTED: 6,
-  CLOSED: 7,
-  CONNECTION_FAILED: 8
+  CONNECTING: 1,
+  INITIALIZING: 2,
+  CONNECTED: 3,
+  CLOSED: 4,
+  CONNECTION_FAILED: 5
 };
 
 /** @enum {number} */
 remoting.ClientSession.ConnectionError = {
   NONE: 0,
   HOST_IS_OFFLINE: 1,
   SESSION_REJECTED: 2,
   INCOMPATIBLE_PROTOCOL: 3,
-  NETWORK_FAILURE: 4,
-  OTHER: 5
+  NETWORK_FAILURE: 4
 };
 
 /**
  * The current state of the session.
  * @type {remoting.ClientSession.State}
  */
 remoting.ClientSession.prototype.state = remoting.ClientSession.State.UNKNOWN;
 
 /**
  * The last connection error. Set when state is set to CONNECTION_FAILED.
@@ skipping 35 matching lines @@
 /**
  * The id of the client plugin
  *
  * @const
  */
 remoting.ClientSession.prototype.PLUGIN_ID = 'session-client-plugin';
 
 /**
  * Callback to invoke when the state is changed.
  *
- * @param {remoting.ClientSession.State} state The previous state.
+ * @param {remoting.ClientSession.State} oldState The previous state.
+ * @param {remoting.ClientSession.State} newState The current state.
  */
-remoting.ClientSession.prototype.onStateChange = function(state) { };
+remoting.ClientSession.prototype.onStateChange =
+    function(oldState, newState) { };
 
 /**
  * Adds <embed> element to |container| and readies the sesion object.
  *
  * @param {Element} container The element to add the plugin to.
  * @param {string} oauth2AccessToken A valid OAuth2 access token.
  * @return {void} Nothing.
  */
 remoting.ClientSession.prototype.createPluginAndConnect =
     function(container, oauth2AccessToken) {
@@ skipping 17 matching lines @@
   /** @param {string} msg The IQ stanza to send. */
   this.plugin.sendIq = function(msg) { that.sendIq_(msg); };
   /** @param {string} msg The message to log. */
   this.plugin.debugInfo = function(msg) {
     remoting.debug.log('plugin: ' + msg);
   };
 
   // TODO(ajwong): Is it even worth having this class handle these events?
   // Or would it be better to just allow users to pass in their own handlers
   // and leave these blank by default?
-  this.plugin.connectionInfoUpdate = function() {
-    that.connectionInfoUpdateCallback();
+  /**
+   * @param {number} status The plugin status.
+   * @param {number} error The plugin error status, if any.
+   */
+  this.plugin.connectionInfoUpdate = function(status, error) {
+    that.connectionInfoUpdateCallback(status, error);
   };
   this.plugin.desktopSizeUpdate = function() { that.onDesktopSizeChanged_(); };
 
   // TODO(garykac): Clean exit if |connect| isn't a function.
   if (typeof this.plugin.connect === 'function') {
     this.connectPluginToWcs_(oauth2AccessToken);
   } else {
     remoting.debug.log('ERROR: remoting plugin not loaded');
     this.setState_(remoting.ClientSession.State.UNKNOWN_PLUGIN_ERROR);
   }
@@ skipping 101 matching lines @@
     that.plugin.onIq(stanza);
   }
   remoting.wcs.setOnIq(onIq);
   that.plugin.connect(this.hostJid, this.hostPublicKey, this.clientJid,
                       this.accessCode);
 };
 
 /**
  * Callback that the plugin invokes to indicate that the connection
  * status has changed.
+ *
+ * @param {number} status The plugin's status.
+ * @param {number} error The plugin's error state, if any.
  */
-remoting.ClientSession.prototype.connectionInfoUpdateCallback = function() {
-  var state = this.plugin.status;
+remoting.ClientSession.prototype.connectionInfoUpdateCallback =
+    function(status, error) {
+  // Old plugins didn't pass the status and error values, so get them directly.
+  // Note that there is a race condition inherent in this approach.
+  if (typeof(status) == 'undefined') {
+    status = this.plugin.status;
+  }
+  if (typeof(error) == 'undefined') {
+    error = this.plugin.error;
+  }
 
-  // TODO(ajwong): We're doing silly type translation here. Any way to avoid?
-  if (state == this.plugin.STATUS_UNKNOWN) {
-    this.setState_(remoting.ClientSession.State.UNKNOWN);
-  } else if (state == this.plugin.STATUS_CONNECTING) {
-    this.setState_(remoting.ClientSession.State.CONNECTING);
-  } else if (state == this.plugin.STATUS_INITIALIZING) {
-    this.setState_(remoting.ClientSession.State.INITIALIZING);
-  } else if (state == this.plugin.STATUS_CONNECTED) {
+  if (status == this.plugin.STATUS_CONNECTED) {
     this.onDesktopSizeChanged_();
-    this.setState_(remoting.ClientSession.State.CONNECTED);
-  } else if (state == this.plugin.STATUS_CLOSED) {
-    this.setState_(remoting.ClientSession.State.CLOSED);
-  } else if (state == this.plugin.STATUS_FAILED) {
-    var error = this.plugin.error;
-    if (error == this.plugin.ERROR_HOST_IS_OFFLINE) {
-      this.error = remoting.ClientSession.ConnectionError.HOST_IS_OFFLINE;
-    } else if (error == this.plugin.ERROR_SESSION_REJECTED) {
-      this.error = remoting.ClientSession.ConnectionError.SESSION_REJECTED;
-    } else if (error == this.plugin.ERROR_INCOMPATIBLE_PROTOCOL) {
-      this.error = remoting.ClientSession.ConnectionError.INCOMPATIBLE_PROTOCOL;
-    } else if (error == this.plugin.ERROR_NETWORK_FAILURE) {
-      this.error = remoting.ClientSession.ConnectionError.NETWORK_FAILURE;
-    } else {
-      this.error = remoting.ClientSession.ConnectionError.OTHER;
-    }
-    this.setState_(remoting.ClientSession.State.CONNECTION_FAILED);
+  } else if (status == this.plugin.STATUS_FAILED) {
+    this.error = /** @type {remoting.ClientSession.ConnectionError} */ (error);
   }
+  this.setState_(/** @type {remoting.ClientSession.State} */ (status));
 };
 
 /**
  * @private
- * @param {remoting.ClientSession.State} state The new state for the session.
+ * @param {remoting.ClientSession.State} newState The new state for the session.
  * @return {void} Nothing.
  */
-remoting.ClientSession.prototype.setState_ = function(state) {
+remoting.ClientSession.prototype.setState_ = function(newState) {
   var oldState = this.state;
-  this.state = state;
+  this.state = newState;
   if (this.onStateChange) {
-    this.onStateChange(oldState);
+    this.onStateChange(oldState, newState);
   }
   this.logToServer.logClientSessionStateChange(this.state, this.error);
 };
 
 /**
  * This is a callback that gets called when the window is resized.
  *
  * @return {void} Nothing.
  */
 remoting.ClientSession.prototype.onWindowSizeChanged = function() {
-  remoting.debug.log('window size changed: ' +
-                     window.innerWidth + 'x' +
-                     window.innerHeight);

[Jamie, 2011/11/16 00:17:13]

Unrelated to this CL; it was just bugging me.

   this.updateDimensions();
 };
 
 /**
  * This is a callback that gets called when the plugin notifies us of a change
  * in the size of the remote desktop.
  *
  * @private
  * @return {void} Nothing.
  */
@@ skipping 57 matching lines @@
   return {
     'video_bandwidth': this.plugin.videoBandwidth,
     'video_frame_rate': this.plugin.videoFrameRate,
     'capture_latency': this.plugin.videoCaptureLatency,
     'encode_latency': this.plugin.videoEncodeLatency,
     'decode_latency': this.plugin.videoDecodeLatency,
     'render_latency': this.plugin.videoRenderLatency,
     'roundtrip_latency': this.plugin.roundTripLatency
   };
 };