[Chromoting] SessionLogger refactor.

remoting/webapp/base/js/client_session_factory.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
 (function() {
 
 'use strict';
@@ skipping 34 matching lines @@
     function(listener, logger) {
   var that = this;
   /** @type {string} */
   var token;
   /** @type {remoting.SignalStrategy} */
   var signalStrategy;
   /** @type {remoting.ClientPlugin} */
   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;
   }
 
   var promise = remoting.identity.getToken().then(
     function(/** string */ authToken) {
     token = authToken;
     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;
     return new remoting.ClientSession(plugin, signalStrategy, logger, listener);
   }).catch(
     remoting.Error.handler(OnError)
   );
 
   return /** @type {Promise<!remoting.ClientSession>} */ (promise);
 };
@@ skipping 32 matching lines @@
  * @return {Promise<!remoting.ClientPlugin>}
  */
 function createPlugin(container, capabilities) {
   var plugin = remoting.ClientPlugin.factory.createPlugin(
       container, capabilities);
   return plugin.initialize().then(function() {
     return plugin;
   });
 }
 
-/**
- * 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);
-}
-
 })();