[Webapp Refactor] Cleans up the ClientSession.EventHandler interface.

This CL provides a cleaner way to report session state changes with the following:
1. Removes onError() callback on ClientSession Handler.  Instead, if an error
   is encountered before the session connects, onConnectionFailed() will be called,
   and similarly, onDisconnected() will be called for errors encountered after
   a session is connected.
2. Removes the sessionStateChanged event on clientSession, so session state
   are reported consistently through the ClientSession.EventHandler interface.

BUG=477522

Review URL: https://codereview.chromium.org/1093373005

Cr-Commit-Position: refs/heads/master@{#326853}

remoting/webapp/app_remoting/js/app_remoting_activity.js

@@ -140,12 +140,12 @@ remoting.AppRemotingActivity.prototype.onAppHostResponse_ =
           session.connect(host, credentialsProvider);
       });
     } else if (response && response.status == 'pending') {
-      this.onError(new remoting.Error(
+      this.onConnectionFailed(new remoting.Error(
           remoting.Error.Tag.SERVICE_UNAVAILABLE));
     }
   } else {
     console.error('Invalid "runApplication" response from server.');
-    this.onError(remoting.Error.fromHttpStatus(xhrResponse.status));
+    this.onConnectionFailed(remoting.Error.fromHttpStatus(xhrResponse.status));
   }
 };
 
@@ -166,28 +166,36 @@ remoting.AppRemotingActivity.prototype.onConnected = function(connectionInfo) {
   }
 };
 
-remoting.AppRemotingActivity.prototype.onDisconnected = function() {
+/**
+ * @param {remoting.Error} error
+ */
+remoting.AppRemotingActivity.prototype.onDisconnected = function(error) {
+  if (error.isNone()) {
+    chrome.app.window.current().close();
+  } else {
+    this.showErrorMessage_(error);
+  }
   this.cleanup_();
-  chrome.app.window.current().close();
 };
 
 /**
  * @param {!remoting.Error} error
  */
 remoting.AppRemotingActivity.prototype.onConnectionFailed = function(error) {
-  this.onError(error);
+  remoting.LoadingWindow.close();
+  this.showErrorMessage_(error);
+  this.cleanup_();
 };
 
 /**
  * @param {!remoting.Error} error The error to be localized and displayed.
+ * @private
  */
-remoting.AppRemotingActivity.prototype.onError = function(error) {
+remoting.AppRemotingActivity.prototype.showErrorMessage_ = function(error) {
   console.error('Connection failed: ' + error.toString());
-  remoting.LoadingWindow.close();
   remoting.MessageWindow.showErrorMessage(
       chrome.i18n.getMessage(/*i18n-content*/'CONNECTION_FAILED'),
       chrome.i18n.getMessage(error.getTag()));
-  this.cleanup_();
 };
 
 })();

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()) {
+    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)
-  );
 };
 
 /**

remoting/webapp/crd/js/client_session_factory_unittest.js

@@ -20,7 +20,7 @@ var listener;
 var SessionListener = function() {};
 SessionListener.prototype.onConnectionFailed = function(error) {};
 SessionListener.prototype.onConnected = function(connectionInfo) {};
-SessionListener.prototype.onDisconnected = function() {};
+SessionListener.prototype.onDisconnected = function(reason) {};
 SessionListener.prototype.onError = function(error) {};
 
 QUnit.module('ClientSessionFactory', {

remoting/webapp/crd/js/client_session_unittest.js

@@ -22,8 +22,7 @@ var logToServerStub;
 var SessionListener = function() {};
 SessionListener.prototype.onConnectionFailed = function(error) {};
 SessionListener.prototype.onConnected = function(connectionInfo) {};
-SessionListener.prototype.onDisconnected = function() {};
-SessionListener.prototype.onError = function(error) {};
+SessionListener.prototype.onDisconnected = function(error) {};
 
 /**
  * @param {remoting.ClientSession.ConnectionError=} opt_error
@@ -142,9 +141,9 @@ QUnit.test(
   var State = remoting.ClientSession.State;
 
   return connect().then(function() {
-    var onError = sinon.stub(listener, 'onError');
+    var onDisconnected = sinon.stub(listener, 'onDisconnected');
     session.disconnect(new remoting.Error(remoting.Error.Tag.P2P_FAILURE));
-    assert.equal(onError.callCount, 1);
+    assert.equal(onDisconnected.callCount, 1);
     assert.equal(logToServerStub.args[2][0], State.CONNECTION_DROPPED);
   });
 });

remoting/webapp/crd/js/desktop_remoting_activity.js

@@ -88,9 +88,11 @@ remoting.DesktopRemotingActivity.prototype.onConnected =
   this.parentActivity_.onConnected(connectionInfo);
 };
 
-remoting.DesktopRemotingActivity.prototype.onDisconnected = function() {
-  this.parentActivity_.onDisconnected();
-  this.dispose();
+remoting.DesktopRemotingActivity.prototype.onDisconnected = function(reason) {
+  if (this.handleError_(reason)) {
+    return;
+  }
+  this.parentActivity_.onDisconnected(reason);
 };
 
 /**
@@ -98,24 +100,24 @@ remoting.DesktopRemotingActivity.prototype.onDisconnected = function() {
  */
 remoting.DesktopRemotingActivity.prototype.onConnectionFailed =
     function(error) {
+  if (this.handleError_(error)) {
+    return;
+  }
   this.parentActivity_.onConnectionFailed(error);
 };
 
 /**
  * @param {!remoting.Error} error The error to be localized and displayed.
+ * @return {boolean} returns true if the error is handled.
+ * @private
  */
-remoting.DesktopRemotingActivity.prototype.onError = function(error) {
-  console.error('Connection failed: ' + error.toString());
-
+remoting.DesktopRemotingActivity.prototype.handleError_ = function(error) {
   if (error.hasTag(remoting.Error.Tag.AUTHENTICATION_FAILED)) {
     remoting.setMode(remoting.AppMode.HOME);
     remoting.handleAuthFailureAndRelaunch();
-    return;
+    return true;
   }
-
-  this.parentActivity_.onError(error);
-
-  this.dispose();
+  return false;
 };
 
 remoting.DesktopRemotingActivity.prototype.dispose = function() {

remoting/webapp/crd/js/it2me_activity.js

@@ -74,7 +74,9 @@ remoting.It2MeActivity.prototype.stop = function() {
  * @param {!remoting.Error} error
  */
 remoting.It2MeActivity.prototype.onConnectionFailed = function(error) {
-  this.onError(error);
+  this.showErrorMessage_(error);
+  base.dispose(this.desktopActivity_);
+  this.desktopActivity_ = null;
 };
 
 /**
@@ -84,14 +86,22 @@ remoting.It2MeActivity.prototype.onConnected = function(connectionInfo) {
   this.accessCodeDialog_.inputField().value = '';
 };
 
-remoting.It2MeActivity.prototype.onDisconnected = function() {
-  this.showFinishDialog_(remoting.AppMode.CLIENT_SESSION_FINISHED_IT2ME);
+remoting.It2MeActivity.prototype.onDisconnected = function(error) {
+  if (error.isNone()) {
+    this.showFinishDialog_(remoting.AppMode.CLIENT_SESSION_FINISHED_IT2ME);
+  } else {
+    this.showErrorMessage_(error);
+  }
+
+  base.dispose(this.desktopActivity_);
+  this.desktopActivity_ = null;
 };
 
 /**
  * @param {!remoting.Error} error
+ * @private
  */
-remoting.It2MeActivity.prototype.onError = function(error) {
+remoting.It2MeActivity.prototype.showErrorMessage_ = function(error) {
   var errorDiv = document.getElementById('connect-error-message');
   l10n.localizeElementFromTag(errorDiv, error.getTag());
   this.showFinishDialog_(remoting.AppMode.CLIENT_CONNECT_FAILED_IT2ME);

remoting/webapp/crd/js/me2me_activity.js

@@ -140,15 +140,18 @@ remoting.Me2MeActivity.prototype.onConnectionFailed = function(error) {
         that.connect_(false);
         return;
       }
-      that.onError(error);
+      that.showErrorMessage_(error);
     };
     this.retryOnHostOffline_ = false;
 
     // The plugin will be re-created when the host finished refreshing
     remoting.hostList.refresh(onHostListRefresh);
-  } else {
-    this.onError(error);
+  } else if (!error.isNone()) {
+    this.showErrorMessage_(error);
   }
+
+  base.dispose(this.desktopActivity_);
+  this.desktopActivity_ = null;
 };
 
 /**
@@ -172,14 +175,23 @@ remoting.Me2MeActivity.prototype.onConnected = function(connectionInfo) {
       connectionInfo.session());
 };
 
-remoting.Me2MeActivity.prototype.onDisconnected = function() {
-  this.showFinishDialog_(remoting.AppMode.CLIENT_SESSION_FINISHED_ME2ME);
+remoting.Me2MeActivity.prototype.onDisconnected = function(error) {
+  if (error.isNone()) {
+    this.showFinishDialog_(remoting.AppMode.CLIENT_SESSION_FINISHED_ME2ME);
+  } else {
+    this.reconnector_.onConnectionDropped(error);
+    this.showErrorMessage_(error);
+  }
+
+  base.dispose(this.desktopActivity_);
+  this.desktopActivity_ = null;
 };
 
 /**
  * @param {!remoting.Error} error
+ * @private
  */
-remoting.Me2MeActivity.prototype.onError = function(error) {
+remoting.Me2MeActivity.prototype.showErrorMessage_ = function(error) {
   var errorDiv = document.getElementById('connect-error-message');
   l10n.localizeElementFromTag(errorDiv, error.getTag());
   this.showFinishDialog_(remoting.AppMode.CLIENT_CONNECT_FAILED_ME2ME);

remoting/webapp/crd/js/smart_reconnector.js

@@ -49,11 +49,9 @@ remoting.SmartReconnector =
 
   var Events = remoting.ClientSession.Events;
   /** @private */
-  this.eventHooks_ = new base.Disposables(
-      new base.EventHook(clientSession, Events.stateChanged,
-                         this.stateChanged_.bind(this)),
+  this.eventHook_ =
       new base.EventHook(clientSession, Events.videoChannelStateChanged,
-                         this.videoChannelStateChanged_.bind(this)));
+                         this.videoChannelStateChanged_.bind(this));
 };
 
 // The online event only means the network adapter is enabled, but
@@ -80,18 +78,15 @@ remoting.SmartReconnector.prototype.reconnectAsync_ = function() {
 };
 
 /**
- * @param {remoting.ClientSession.StateEvent=} event
+ * @param {!remoting.Error} reason
  */
-remoting.SmartReconnector.prototype.stateChanged_ = function(event) {
-  var State = remoting.ClientSession.State;
-  if (event.previous === State.CONNECTED && event.current === State.FAILED) {
-    this.cancelPending_();
-    if (navigator.onLine) {
-      this.reconnect_();
-    } else {
-      this.pending_ = new base.DomEventHook(
-          window, 'online', this.reconnectAsync_.bind(this), false);
-    }
+remoting.SmartReconnector.prototype.onConnectionDropped = function(reason) {
+  this.cancelPending_();
+  if (navigator.onLine) {
+    this.reconnect_();
+  } else {
+    this.pending_ = new base.DomEventHook(
+        window, 'online', this.reconnectAsync_.bind(this), false);
   }
 };
 
@@ -121,8 +116,8 @@ remoting.SmartReconnector.prototype.cancelPending_ = function() {
 
 remoting.SmartReconnector.prototype.dispose = function() {
   this.cancelPending_();
-  base.dispose(this.eventHooks_);
-  this.eventHooks_ = null;
+  base.dispose(this.eventHook_);
+  this.eventHook_ = null;
 };
 
 })();