[Chromoting] SessionLogger refactor.

remoting/webapp/base/js/client_session_factory.js

Index: remoting/webapp/base/js/client_session_factory.js
diff --git a/remoting/webapp/base/js/client_session_factory.js b/remoting/webapp/base/js/client_session_factory.js
index 249864e5bcfb71a1d445db5827d0ae692342ebe4..658d490c16f84d78a0447f33866022e05e2e29ab 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);

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

Don't you want to associate the pluginError with the detail field in the Error
instance?

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

It is now set in |detail| when the error is first thrown at
client_plugin_impl.js

[Jamie, 2015/11/09 23:23:09]

I don't see that in this CL. Was it duplicated logic previously?

[kelvinp, 2015/11/10 00:02:44]

Yup, it is implemented in the previous CL.

https://code.google.com/p/chromium/codesearch#chromium/src/remoting/webapp/ba...

And we write it to the log entry in the new ChromotingEvent.prototype.setError()
method.

-    }
-  }
-
-  logger.logSessionStateChange(
-      remoting.ChromotingEvent.SessionState.CONNECTION_FAILED, error);
-}
-
 })();