[Chromoting] SessionLogger refactor.

remoting/webapp/base/js/client_session.js

Index: remoting/webapp/base/js/client_session.js
diff --git a/remoting/webapp/base/js/client_session.js b/remoting/webapp/base/js/client_session.js
index 7b3f0cb854149904d21c07097aa006eac77e834e..2051a5bf2c78be8e3d0fc255b5dc31b98c22f65d 100644
--- a/remoting/webapp/base/js/client_session.js
+++ b/remoting/webapp/base/js/client_session.js
@@ -472,7 +472,9 @@ remoting.ClientSession.prototype.onConnectionStatusUpdate =
         this.error_ = new remoting.Error(remoting.Error.Tag.P2P_FAILURE);
         break;
       case remoting.ClientSession.ConnectionError.HOST_OVERLOAD:
-        this.error_ = new remoting.Error(remoting.Error.Tag.HOST_OVERLOAD);
+        this.error_ = new remoting.Error(
+            remoting.Error.Tag.HOST_OVERLOAD,
+            this.xmppErrorCache_.getFirstErrorStanza());
         break;
       case remoting.ClientSession.ConnectionError.MAX_SESSION_LENGTH:
         this.error_ = new remoting.Error(remoting.Error.Tag.MAX_SESSION_LENGTH);
@@ -562,8 +564,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_);

[Jamie, 2015/11/09 18:46:02]

This is a change in semantics. Previously, getFirstError was being included for
all state changes; now it's only included for HOST_OVERLOAD, which seems wrong
(HOST_OVERLOAD is a pretty narrowly-defined error--I'm surprised we need any
additional detail in that case).

[kelvinp, 2015/11/09 22:59:10]

Good catch.

 };
 
 /** @private */
@@ -661,6 +662,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}