[Chromoting] SessionLogger refactor.

Motivation:

When a remoting connection fails, we often need to collect extra
debugging information in additional to the error tag.  For example,
when the host goes offline, we would like to report the corresponding
XMPP error stanza as well.

In the current design, we pass the XMPP error stanza to the SessionLogger
as a separate argument in logSessionStateChange().

In this pattern, we need to add a new argument to logSessionStateChange()
for each error specific detail.  As we collect more and more extra info
(e.g. PNaCl error, script error callstack), this pattern quickly becomes
unmanageable.

Summary of changes:

1. When the connection fails, the caller set the |detail| field in the
   remoting.Error object. When logSessionStateChange() is called,
   SessionLogger calls error.fillLogEntry() to translate the
   |detail| field into the corresponding fields in the log entry.
2. Consolidates logClientSessionStateChange() and logSessionStateChange()
3. Remove the dependency of remoting.ClientSession.State from
   SessionLogger.js
4. Report bad version as a connection failure instead of user
   cancellation

BUG=552693

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

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

9 files changed, 126 insertions, 163 deletions

diff --git a/remoting/webapp/base/js/chromoting_event.js b/remoting/webapp/base/js/chromoting_event.js
index 88cd4f9..7bd21df 100644
--- a/remoting/webapp/base/js/chromoting_event.js
+++ b/remoting/webapp/base/js/chromoting_event.js
@@ -122,6 +122,29 @@ remoting.ChromotingEvent.prototype.init_ = function() {
 };
 
 /**
+ * Populates the corresponding fields in the logEntry based on |error|.
+ *
+ * @param {remoting.Error} error
+ */
+remoting.ChromotingEvent.prototype.setError = function(error) {
+  var Tag = remoting.Error.Tag;
+  var detail = /** @type {string} */ (error.getDetail());
+
+  switch (error.getTag()) {
+    case Tag.HOST_IS_OFFLINE:
+      if (detail) {
+        this.xmpp_error = new remoting.ChromotingEvent.XmppError(detail);
+      }
+      break;
+    case Tag.MISSING_PLUGIN:
+      console.assert(detail, 'Missing PNaCl plugin last error string.');
+      this.raw_plugin_error = detail;
+  }
+
+  this.connection_error = error.toConnectionError();
+};
+
+/**
  * @param {remoting.ChromotingEvent} event
  * @return {boolean}
  */
diff --git a/remoting/webapp/base/js/client_session.js b/remoting/webapp/base/js/client_session.js
index 7b3f0cb..38532c6 100644
--- a/remoting/webapp/base/js/client_session.js
+++ b/remoting/webapp/base/js/client_session.js
@@ -456,34 +456,34 @@ remoting.ClientSession.prototype.onIncomingMessage_ = function(message) {
 remoting.ClientSession.prototype.onConnectionStatusUpdate =
     function(status, error) {
   if (status == remoting.ClientSession.State.FAILED) {
+    var errorTag = remoting.Error.Tag.UNEXPECTED;
     switch (error) {
       case remoting.ClientSession.ConnectionError.HOST_IS_OFFLINE:
-        this.error_ = new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE);
+        errorTag = remoting.Error.Tag.HOST_IS_OFFLINE;
         break;
       case remoting.ClientSession.ConnectionError.SESSION_REJECTED:
-        this.error_ =
-            new remoting.Error(remoting.Error.Tag.INVALID_ACCESS_CODE);
+        errorTag = remoting.Error.Tag.INVALID_ACCESS_CODE;
         break;
       case remoting.ClientSession.ConnectionError.INCOMPATIBLE_PROTOCOL:
-        this.error_ =
-            new remoting.Error(remoting.Error.Tag.INCOMPATIBLE_PROTOCOL);
+        errorTag = remoting.Error.Tag.INCOMPATIBLE_PROTOCOL;
         break;
       case remoting.ClientSession.ConnectionError.NETWORK_FAILURE:
-        this.error_ = new remoting.Error(remoting.Error.Tag.P2P_FAILURE);
+        errorTag = remoting.Error.Tag.P2P_FAILURE;
         break;
       case remoting.ClientSession.ConnectionError.HOST_OVERLOAD:
-        this.error_ = new remoting.Error(remoting.Error.Tag.HOST_OVERLOAD);
+        errorTag = remoting.Error.Tag.HOST_OVERLOAD;
         break;
       case remoting.ClientSession.ConnectionError.MAX_SESSION_LENGTH:
-        this.error_ = new remoting.Error(remoting.Error.Tag.MAX_SESSION_LENGTH);
+        errorTag = remoting.Error.Tag.MAX_SESSION_LENGTH;
         break;
       case remoting.ClientSession.ConnectionError.HOST_CONFIGURATION_ERROR:
-        this.error_ =
-            new remoting.Error(remoting.Error.Tag.HOST_CONFIGURATION_ERROR);
+        errorTag = remoting.Error.Tag.HOST_CONFIGURATION_ERROR;
         break;
       default:
         this.error_ = remoting.Error.unexpected();
     }
+    this.error_ = new remoting.Error(
+        errorTag, this.xmppErrorCache_.getFirstErrorStanza());
   }
   this.setState_(status);
 };
@@ -562,8 +562,7 @@ remoting.ClientSession.prototype.setState_ = function(newState) {
   this.notifyStateChanges_(oldState, this.state_);
   // Record state count in an UMA enumerated histogram.
   recordState(this.state_);
-  this.logger_.logClientSessionStateChange(
-      this.state_, this.error_, this.xmppErrorCache_.getFirstError());
+  this.logger_.logSessionStateChange(toSessionState(this.state_), this.error_);
 };
 
 /** @private */
@@ -661,6 +660,37 @@ remoting.ClientSession.prototype.notifyStateChanges_ =
 };
 
 /**
+ * TODO(kelvinp): Consolidate the two enums (crbug.com/504200)
+ * @param {remoting.ClientSession.State} state
+ * @return {remoting.ChromotingEvent.SessionState}
+ */
+function toSessionState(state) {
+  var SessionState = remoting.ChromotingEvent.SessionState;
+  switch(state) {
+    case remoting.ClientSession.State.UNKNOWN:
+      return SessionState.UNKNOWN;
+    case remoting.ClientSession.State.INITIALIZING:
+      return SessionState.INITIALIZING;
+    case remoting.ClientSession.State.CONNECTING:
+      return SessionState.CONNECTING;
+    case remoting.ClientSession.State.AUTHENTICATED:
+      return SessionState.AUTHENTICATED;
+    case remoting.ClientSession.State.CONNECTED:
+      return SessionState.CONNECTED;
+    case remoting.ClientSession.State.CLOSED:
+      return SessionState.CLOSED;
+    case remoting.ClientSession.State.FAILED:
+      return SessionState.CONNECTION_FAILED;
+    case remoting.ClientSession.State.CONNECTION_DROPPED:
+      return SessionState.CONNECTION_DROPPED;
+    case remoting.ClientSession.State.CONNECTION_CANCELED:
+      return SessionState.CONNECTION_CANCELED;
+    default:
+      throw new Error('Unknown session state : ' + state);
+  }
+}
+
+/**
  * @param {remoting.ClientSession.State} previous
  * @param {remoting.ClientSession.State} current
  * @return {remoting.ClientSession.State}
diff --git a/remoting/webapp/base/js/client_session_factory.js b/remoting/webapp/base/js/client_session_factory.js
index 249864e..658d490 100644
--- a/remoting/webapp/base/js/client_session_factory.js
+++ b/remoting/webapp/base/js/client_session_factory.js
@@ -52,7 +52,8 @@ remoting.ClientSessionFactory.prototype.createSession =
   var clientPlugin;
 
   function OnError(/** !remoting.Error */ error) {
-    logError(logger, error);
+    logger.logSessionStateChange(
+        remoting.ChromotingEvent.SessionState.CONNECTION_FAILED, error);
     base.dispose(signalStrategy);
     base.dispose(clientPlugin);
     throw error;
@@ -64,14 +65,12 @@ remoting.ClientSessionFactory.prototype.createSession =
     return remoting.identity.getUserInfo();
   }).then(function(/** {email: string, name: string} */ userInfo) {
     logger.logSessionStateChange(
-        remoting.ChromotingEvent.SessionState.SIGNALING,
-        remoting.ChromotingEvent.ConnectionError.NONE);
+        remoting.ChromotingEvent.SessionState.SIGNALING);
     return connectSignaling(userInfo.email, token);
   }).then(function(/** remoting.SignalStrategy */ strategy) {
     signalStrategy = strategy;
     logger.logSessionStateChange(
-        remoting.ChromotingEvent.SessionState.CREATING_PLUGIN,
-        remoting.ChromotingEvent.ConnectionError.NONE);
+        remoting.ChromotingEvent.SessionState.CREATING_PLUGIN);
     return createPlugin(that.container_, that.requiredCapabilities_);
   }).then(function(/** remoting.ClientPlugin */ plugin) {
     clientPlugin = plugin;
@@ -124,31 +123,4 @@ function createPlugin(container, capabilities) {
   });
 }
 
-/**
- * Converts |e| to remoting.ChromotingEvent.ConnectionError and logs
- * it to the telemetry service.
- *
- * TODO(kelvinp): Move this block to remoting.SessionLogger and consolidate
- * the code path with xmpp_error.
- *
- * @param {remoting.SessionLogger} logger
- * @param {remoting.Error} e
- */
-function logError(logger, e) {
-  var error = remoting.ChromotingEvent.ConnectionError.UNEXPECTED;
-
-  if (e instanceof remoting.Error) {
-    error = e.toConnectionError();
-
-    if (e.hasTag(remoting.Error.Tag.MISSING_PLUGIN)) {
-      var pluginError = /** @type {string} */ (e.getDetail());
-      console.assert(Boolean(pluginError), 'Missing plugin error string.');
-      logger.setPluginError(pluginError);
-    }
-  }
-
-  logger.logSessionStateChange(
-      remoting.ChromotingEvent.SessionState.CONNECTION_FAILED, error);
-}
-
 })();
diff --git a/remoting/webapp/base/js/client_session_unittest.js b/remoting/webapp/base/js/client_session_unittest.js
index f4a3c99..6f6273a 100644
--- a/remoting/webapp/base/js/client_session_unittest.js
+++ b/remoting/webapp/base/js/client_session_unittest.js
@@ -76,7 +76,7 @@ QUnit.module('ClientSession', {
     listener = new SessionListener();
     logger = new remoting.SessionLogger(remoting.ChromotingEvent.Role.CLIENT,
                                         base.doNothing);
-    logToServerStub = sinon.stub(logger, 'logClientSessionStateChange');
+    logToServerStub = sinon.stub(logger, 'logSessionStateChange');
   },
   afterEach: function() {
     session.dispose();
@@ -121,13 +121,13 @@ QUnit.test(
   'Connection error after CONNECTED should raise the CONNECTION_DROPPED event',
   function(assert) {
 
-  var State = remoting.ClientSession.State;
+  var State = remoting.ChromotingEvent.SessionState;
 
   return connect().then(function() {
     var onDisconnected = sinon.stub(listener, 'onDisconnected');
     session.disconnect(new remoting.Error(remoting.Error.Tag.P2P_FAILURE));
     assert.equal(onDisconnected.callCount, 1);
-    assert.equal(logToServerStub.args[2][0], State.CONNECTION_DROPPED);
+    assert.equal(logToServerStub.args[4][0], State.CONNECTION_DROPPED);
   });
 });
 
@@ -136,14 +136,14 @@ QUnit.test(
   function(assert) {
 
   var PluginError = remoting.ClientSession.ConnectionError;
-  var State = remoting.ClientSession.State;
+  var State = remoting.ChromotingEvent.SessionState;
 
   return connect(PluginError.SESSION_REJECTED).then(function() {
     assert.ok(false, 'Expect connection to fail');
   }).catch(function(/** remoting.Error */ error) {
     assert.ok(error.hasTag(remoting.Error.Tag.INVALID_ACCESS_CODE));
-    assert.equal(logToServerStub.args[1][0], State.FAILED);
-    var errorLogged = /** @type {remoting.Error} */(logToServerStub.args[1][1]);
+    assert.equal(logToServerStub.args[3][0], State.CONNECTION_FAILED);
+    var errorLogged = /** @type {remoting.Error} */(logToServerStub.args[3][1]);
     assert.equal(errorLogged.getTag(), remoting.Error.Tag.INVALID_ACCESS_CODE);
   });
 });
diff --git a/remoting/webapp/base/js/session_logger.js b/remoting/webapp/base/js/session_logger.js
index 3165da2..95eaa73 100644
--- a/remoting/webapp/base/js/session_logger.js
+++ b/remoting/webapp/base/js/session_logger.js
@@ -52,8 +52,6 @@ remoting.SessionLogger = function(role, writeLogEntry) {
   this.mode_ = remoting.ChromotingEvent.Mode.ME2ME;
   /** @private {remoting.ChromotingEvent.AuthMethod} */
   this.authMethod_;
-  /** @private */
-  this.pluginError_ = '';
 
   this.setSessionId_();
 };
@@ -131,13 +129,6 @@ remoting.SessionLogger.prototype.setAuthMethod = function(method) {
 };
 
 /**
- * @param {string} error  The error string of the plugin error.
- */
-remoting.SessionLogger.prototype.setPluginError = function(error) {
-  this.pluginError_ = error;
-};
-
-/**
  * @return {string} The current session id. This is random GUID, refreshed
  *     every 24hrs.
  */
@@ -162,34 +153,14 @@ remoting.SessionLogger.prototype.logSignalStrategyProgress =
 };
 
 /**
- * Logs a client session state change.
- *
- * @param {remoting.ClientSession.State} state
- * @param {!remoting.Error} stateError
- * @param {?remoting.ChromotingEvent.XmppError} xmppError The XMPP error
- *     as described in http://xmpp.org/rfcs/rfc6120.html#stanzas-error.
- *     Set if the connecton error originates from the an XMPP stanza error.
- */
-remoting.SessionLogger.prototype.logClientSessionStateChange = function(
-    state, stateError, xmppError) {
-  this.logSessionStateChange(
-      toSessionState(state),
-      stateError.toConnectionError(),
-      xmppError);
-};
-
-/**
  * @param {remoting.ChromotingEvent.SessionState} state
- * @param {remoting.ChromotingEvent.ConnectionError} error
- * @param {remoting.ChromotingEvent.XmppError=} opt_XmppError
+ * @param {remoting.Error=} opt_error
  */
-remoting.SessionLogger.prototype.logSessionStateChange = function(
-    state, error, opt_XmppError) {
+remoting.SessionLogger.prototype.logSessionStateChange =
+    function(state, opt_error) {
   this.maybeExpireSessionId_();
 
-  var entry = this.makeSessionStateChange_(
-      state, error,
-      /** @type {?remoting.ChromotingEvent.XmppError} */ (opt_XmppError));
+  var entry = this.makeSessionStateChange_(state, opt_error);
   entry.previous_session_state = this.previousSessionState_;
   this.previousSessionState_ = state;
 
@@ -219,22 +190,27 @@ remoting.SessionLogger.prototype.logStatistics = function(stats) {
 
 /**
  * @param {remoting.ChromotingEvent.SessionState} state
- * @param {remoting.ChromotingEvent.ConnectionError} error
- * @param {?remoting.ChromotingEvent.XmppError} xmppError
+ * @param {remoting.Error=} opt_error
  * @return {remoting.ChromotingEvent}
  * @private
  */
 remoting.SessionLogger.prototype.makeSessionStateChange_ =
-    function(state, error, xmppError) {
+    function(state, opt_error) {
   var entry = new remoting.ChromotingEvent(
       remoting.ChromotingEvent.Type.SESSION_STATE);
-  entry.connection_error = error;
-  entry.session_state = state;
 
-  if (Boolean(xmppError)) {
-    entry.xmpp_error = xmppError;
+  var ConnectionError = remoting.ChromotingEvent.ConnectionError;
+
+  if (!opt_error) {
+    entry.connection_error = ConnectionError.NONE;
+  } else if (opt_error instanceof remoting.Error) {
+    entry.setError(opt_error);
+  } else {
+    entry.connection_error = ConnectionError.UNKNOWN_ERROR;
   }
 
+  entry.session_state = state;
+
   this.fillEvent_(entry);
   return entry;
 };
@@ -320,9 +296,6 @@ remoting.SessionLogger.prototype.fillEvent_ = function(entry) {
   if (this.authMethod_ != undefined) {
     entry.auth_method = this.authMethod_;
   }
-  if (Boolean(this.pluginError_)) {
-    entry.raw_plugin_error = this.pluginError_;
-  }
   entry.host_version = this.hostVersion_;
   entry.host_os = this.hostOs_;
   entry.host_os_version = this.hostOsVersion_;
@@ -390,37 +363,6 @@ remoting.SessionLogger.createForClient = function() {
 };
 
 /**
- * TODO(kelvinp): Consolidate the two enums (crbug.com/504200)
- * @param {remoting.ClientSession.State} state
- * @return {remoting.ChromotingEvent.SessionState}
- */
-function toSessionState(state) {
-  var SessionState = remoting.ChromotingEvent.SessionState;
-  switch(state) {
-    case remoting.ClientSession.State.UNKNOWN:
-      return SessionState.UNKNOWN;
-    case remoting.ClientSession.State.INITIALIZING:
-      return SessionState.INITIALIZING;
-    case remoting.ClientSession.State.CONNECTING:
-      return SessionState.CONNECTING;
-    case remoting.ClientSession.State.AUTHENTICATED:
-      return SessionState.AUTHENTICATED;
-    case remoting.ClientSession.State.CONNECTED:
-      return SessionState.CONNECTED;
-    case remoting.ClientSession.State.CLOSED:
-      return SessionState.CLOSED;
-    case remoting.ClientSession.State.FAILED:
-      return SessionState.CONNECTION_FAILED;
-    case remoting.ClientSession.State.CONNECTION_DROPPED:
-      return SessionState.CONNECTION_DROPPED;
-    case remoting.ClientSession.State.CONNECTION_CANCELED:
-      return SessionState.CONNECTION_CANCELED;
-    default:
-      throw new Error('Unknown session state : ' + state);
-  }
-}
-
-/**
  * @param {remoting.SignalStrategy.Type} type
  * @return {remoting.ChromotingEvent.SignalStrategyType}
  */
diff --git a/remoting/webapp/base/js/session_logger_unittest.js b/remoting/webapp/base/js/session_logger_unittest.js
index ba8122e..78f7e45 100644
--- a/remoting/webapp/base/js/session_logger_unittest.js
+++ b/remoting/webapp/base/js/session_logger_unittest.js
@@ -74,7 +74,7 @@ QUnit.test('logSignalStrategyProgress()', function(assert) {
   });
 });
 
-QUnit.test('logClientSessionStateChange()', function(assert){
+QUnit.test('logSessionStateChange()', function(assert){
   var Event = remoting.ChromotingEvent;
 
   logger = new remoting.SessionLogger(Event.Role.CLIENT, logWriter);
@@ -84,9 +84,9 @@ QUnit.test('logClientSessionStateChange()', function(assert){
   logger.setHostOs(remoting.ChromotingEvent.Os.OTHER);
   logger.setHostOsVersion('host_os_version');
 
-  logger.logClientSessionStateChange(
-      remoting.ClientSession.State.FAILED,
-      new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE), null);
+  logger.logSessionStateChange(
+      remoting.ChromotingEvent.SessionState.CONNECTION_FAILED,
+      new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE));
   var sessionId = logger.getSessionId();
 
   assert.ok(sessionId !== null);
@@ -110,7 +110,7 @@ QUnit.test('logClientSessionStateChange()', function(assert){
   });
 });
 
-QUnit.test('logClientSessionStateChange() should handle XMPP error',
+QUnit.test('logSessionStateChange() should handle XMPP error',
     function(assert){
   var Event = remoting.ChromotingEvent;
 
@@ -121,11 +121,9 @@ QUnit.test('logClientSessionStateChange() should handle XMPP error',
   logger.setHostOs(remoting.ChromotingEvent.Os.OTHER);
   logger.setHostOsVersion('host_os_version');
 
-  var xmppError = new remoting.ChromotingEvent.XmppError('<fake-stanza/>');
-
-  logger.logClientSessionStateChange(
-      remoting.ClientSession.State.FAILED,
-      new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE), xmppError);
+  logger.logSessionStateChange(
+      remoting.ChromotingEvent.SessionState.CONNECTION_FAILED,
+      new remoting.Error(remoting.Error.Tag.HOST_IS_OFFLINE, '<fake-stanza/>'));
   var sessionId = logger.getSessionId();
 
   assert.ok(sessionId !== null);
@@ -152,7 +150,7 @@ QUnit.test('logClientSessionStateChange() should handle XMPP error',
   });
 });
 
-QUnit.test('logClientSessionStateChange() should handle sessionId change.',
+QUnit.test('logSessionStateChange() should handle sessionId change.',
   function(assert){
   var clock = sinon.useFakeTimers();
   var Event = remoting.ChromotingEvent;
@@ -170,8 +168,8 @@ QUnit.test('logClientSessionStateChange() should handle sessionId change.',
   clock.tick(remoting.SessionLogger.MAX_SESSION_ID_AGE + 100);
 
   // Logs the event.
-  logger.logClientSessionStateChange(
-      remoting.ClientSession.State.AUTHENTICATED, remoting.Error.none(), null);
+  logger.logSessionStateChange(
+      remoting.ChromotingEvent.SessionState.AUTHENTICATED);
 
   var newSessionId = logger.getSessionId();
   verifyEvent(assert, 0, {
@@ -225,7 +223,7 @@ QUnit.test('logClientSessionStateChange() should handle sessionId change.',
   });
 });
 
-QUnit.test('logClientSessionStateChange() should log session_duration.',
+QUnit.test('logSessionStateChange() should log session_duration.',
   function(assert){
   var clock = sinon.useFakeTimers();
   var Event = remoting.ChromotingEvent;
@@ -241,8 +239,8 @@ QUnit.test('logClientSessionStateChange() should log session_duration.',
   clock.tick(2500);
 
   // Logs the event.
-  logger.logClientSessionStateChange(
-      remoting.ClientSession.State.CONNECTED, remoting.Error.none(), null);
+  logger.logSessionStateChange(
+    remoting.ChromotingEvent.SessionState.CONNECTED);
 
   verifyEvent(assert, 0, {
     type: Event.Type.SESSION_STATE,
diff --git a/remoting/webapp/base/js/telemetry_event_writer_unittest.js b/remoting/webapp/base/js/telemetry_event_writer_unittest.js
index cf41c9f..0a17ae6 100644
--- a/remoting/webapp/base/js/telemetry_event_writer_unittest.js
+++ b/remoting/webapp/base/js/telemetry_event_writer_unittest.js
@@ -92,8 +92,7 @@ QUnit.test('should send CANCELED event when window is closed while started.',
     chrome.app.window.current().id = 'fake-window-id';
   }).then(function() {
     logger.logSessionStateChange(
-        remoting.ChromotingEvent.SessionState.STARTED,
-        remoting.ChromotingEvent.ConnectionError.NONE);
+        remoting.ChromotingEvent.SessionState.STARTED);
   }).then(function() {
     return service.unbindSession('fake-window-id');
   }).then(function() {
@@ -119,8 +118,8 @@ QUnit.test('should send CANCELED event when window is closed while connecting.',
   return service.init().then(function() {
     chrome.app.window.current().id = 'fake-window-id';
   }).then(function() {
-    logger.logClientSessionStateChange(
-        remoting.ClientSession.State.CONNECTING, remoting.Error.none(), null);
+    logger.logSessionStateChange(
+        remoting.ChromotingEvent.SessionState.CONNECTING);
   }).then(function() {
     return service.unbindSession('fake-window-id');
   }).then(function() {
@@ -147,11 +146,11 @@ QUnit.test('should send CLOSED event when window is closed while connected.',
   return service.init().then(function() {
     chrome.app.window.current().id = 'fake-window-id';
   }).then(function() {
-    logger.logClientSessionStateChange(
-        remoting.ClientSession.State.CONNECTING, remoting.Error.none(), null);
+    logger.logSessionStateChange(
+        remoting.ChromotingEvent.SessionState.CONNECTING);
   }).then(function() {
-    logger.logClientSessionStateChange(
-        remoting.ClientSession.State.CONNECTED, remoting.Error.none(), null);
+    logger.logSessionStateChange(
+        remoting.ChromotingEvent.SessionState.CONNECTED);
   }).then(function() {
     return service.unbindSession('fake-window-id');
   }).then(function() {
@@ -180,11 +179,11 @@ QUnit.test('should not send CLOSED event when window is closed unconnected.',
   return service.init().then(function() {
     chrome.app.window.current().id = 'fake-window-id';
   }).then(function() {
-    logger.logClientSessionStateChange(
-        remoting.ClientSession.State.CONNECTING, remoting.Error.none(), null);
+    logger.logSessionStateChange(
+        remoting.ChromotingEvent.SessionState.CONNECTING);
   }).then(function() {
-    logger.logClientSessionStateChange(
-        remoting.ClientSession.State.FAILED, remoting.Error.none(), null);
+    logger.logSessionStateChange(
+        remoting.ChromotingEvent.SessionState.CONNECTION_FAILED);
   }).then(function() {
     return service.unbindSession('fake-window-id');
   }).then(function() {
diff --git a/remoting/webapp/base/js/xmpp_error_cache.js b/remoting/webapp/base/js/xmpp_error_cache.js
index 79a9138..eabbdb3 100644
--- a/remoting/webapp/base/js/xmpp_error_cache.js
+++ b/remoting/webapp/base/js/xmpp_error_cache.js
@@ -19,17 +19,16 @@ var remoting = remoting || {};
  */
 
 remoting.XmppErrorCache = function() {
-  /** @private {remoting.ChromotingEvent.XmppError} */
-  this.firstError_ = null;
+  /** @private {string} */
+  this.firstErrorStanza_ = '';
 };
 
 /**
- * @return {remoting.ChromotingEvent.XmppError} The first XMPP error that the
- *     monitor encountered.  Returns null if no errors have been encountered so
- *     far.
+ * @return {string} The first XMPP error stanza that the monitor encountered.
+ *     Returns an empty string if no errors have been encountered so far.
  */
-remoting.XmppErrorCache.prototype.getFirstError = function() {
-  return this.firstError_;
+remoting.XmppErrorCache.prototype.getFirstErrorStanza = function() {
+  return this.firstErrorStanza_;
 };
 
 /**
@@ -38,7 +37,7 @@ remoting.XmppErrorCache.prototype.getFirstError = function() {
  * @param {Element} iqNode
  */
 remoting.XmppErrorCache.prototype.processStanza = function(iqNode) {
-  if (this.firstError_ != null) {
+  if (this.firstErrorStanza_ != '') {
     return;
   }
   // The XML structure is as follows:
@@ -51,7 +50,7 @@ remoting.XmppErrorCache.prototype.processStanza = function(iqNode) {
   }
 
   var strippedStanza = this.stripPII_(iqNode);
-  this.firstError_ = new remoting.ChromotingEvent.XmppError(strippedStanza);
+  this.firstErrorStanza_ = strippedStanza;
 };
 
 /**
diff --git a/remoting/webapp/base/js/xmpp_error_cache_unittest.js b/remoting/webapp/base/js/xmpp_error_cache_unittest.js
index f06aa15..8170cd3 100644
--- a/remoting/webapp/base/js/xmpp_error_cache_unittest.js
+++ b/remoting/webapp/base/js/xmpp_error_cache_unittest.js
@@ -18,8 +18,8 @@ function runStanzaTest(stanza, expected, assert) {
   var parser = new DOMParser();
   var xml = parser.parseFromString(stanza, 'text/xml');
   monitor.processStanza(xml.firstElementChild);
-  var error = monitor.getFirstError();
-  assert.equal(error.raw_stanza, expected);
+  var errorStanza = monitor.getFirstErrorStanza();
+  assert.equal(errorStanza, expected);
 }
 
 QUnit.test('should strip PII from session-initiate.', function(assert) {