Add UMA stats for Chromoting connection details.

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 329decea3d212eefa74c3b3093d4bd65f85df4c4..411afd9c94e815fb2a359eb284c9910fd7c71e48 100644
--- a/remoting/webapp/base/js/client_session.js
+++ b/remoting/webapp/base/js/client_session.js
@@ -139,8 +139,11 @@ remoting.ClientSession.EventHandler.prototype.onDisconnected =
 // the names given here.
 // The negative values represent state transitions that occur within the
 // web-app that have no corresponding plugin state transition.
+// NOTE: The enums here are also defined in histograms.xml for our UMA
+// Chromoting.Connections histogram. The 2 lists should be kept in sync.

[Ilya Sherman, 2015/08/21 19:48:29]

Please also document that the enum should be treated as append-only for this
reason.

[anandc, 2015/08/21 21:30:03]

Done.

 /** @enum {number} */
 remoting.ClientSession.State = {
+  MIN_STATE_ENUM: -3,
   CONNECTION_CANCELED: -3,  // Connection closed (gracefully) before connecting.
   CONNECTION_DROPPED: -2,  // Succeeded, but subsequently closed with an error.
   CREATED: -1,
@@ -150,7 +153,9 @@ remoting.ClientSession.State = {
   AUTHENTICATED: 3,
   CONNECTED: 4,
   CLOSED: 5,
-  FAILED: 6
+  FAILED: 6,
+  MAX_STATE_ENUM: 7 // The upper limit for a UMA enumerated histogram should be
+                    // 1 above the max-enum. See src/base/metrics/histogram.h.
 };
 
 /**
@@ -566,11 +571,32 @@ 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());
 };
 
 /**
+ * Records a Chromoting Connection State, stored in an UMA enumerated histogram.
+ * @param {remoting.ClientSession.State} state State identifier.
+ */
+function recordState(state) {
+  var metricDescription = {
+    metricName: 'Chromoting.Connections',
+    type: 'histogram-linear',
+    min: remoting.ClientSession.State.MIN_STATE_ENUM,

[Ilya Sherman, 2015/08/21 19:48:29]

Hmm, I'm not actually sure whether negative buckets work smoothly for
histograms.  Could you please check that chrome://histograms shows what you'd
expect?  If it doesn't, an easy tweak is to simply add 3 (or, equivalently,
subtract MIN_STATE_ENUM) to the logged values.  (Though, this tweak does mean
that you'd need to freeze MIN_STATE_ENUM at -3 for all time; otherwise, the
semantics of the recorded enum values would change each time that MIN_STATE_ENUM
changes.)

[anandc, 2015/08/21 21:30:03]

Thanks. Negative values were actually not working in chrome://histograms.
Updated to use only non-negative enum values.

+    max: remoting.ClientSession.State.MAX_STATE_ENUM,
+    // The # of buckets should include 1 for overflow. Use the boundary values
+    // for the ClientSession.State enum to set the # of buckets.
+    buckets: (remoting.ClientSession.State.MAX_STATE_ENUM) -
+    (remoting.ClientSession.State.MIN_STATE_ENUM) + 2

[Ilya Sherman, 2015/08/21 19:48:29]

nit: Is the indentation correct here?

[Ilya Sherman, 2015/08/21 19:48:29]

MAX_STATE_ENUM Is already reserving an extra bucket, so I'm not sure why this is
+2 rather than +1.

[anandc, 2015/08/21 21:30:03]

Done.

[anandc, 2015/08/21 21:30:03]

Thanks for catching. Forgot to update here when changing the value in the
SessionState enum.

+  };
+
+  chrome.metricsPrivate.recordValue(metricDescription, state);
+}
+
+/**
  * @param {remoting.ClientSession.State} oldState The new state for the session.
  * @param {remoting.ClientSession.State} newState The new state for the session.
  * @private