[Webapp Refactor] Remove remoting.SessionConnector.

remoting.SessionConnector is currently responsible for creating
the plugin, the signal strategy and the clientSession.  It also
magically disposes the plugin when the clientSession finishes.

However, the session is not exposed to the caller elsewhere, which makes
it hard for the caller to dispose of the session before it is connected,
e.g. Cancel a PIN entry.

It is currently a stateful object that keeps track of the host,
credentials provider, strategy of the created session, which is redundant.

This CL
1. Offloads the creation of the clientSession to the
   remoting.ClientSessionFactory.  It also allow the caller to configure
   a clientSession prior to connecting.  The remoting.ClientSessionFactory
   is essentially stateless except for a few predefined ClientSession
   construction parameters.
2. Allow the caller to have a different instance of
   ClientSession.EventHandler for each instance ClientSession created.
3. Revives remoting.MockClientPlugin and uses it for remoting.ClientSessionFactory
   unittests.

Ownership graph before:
Activity -> SessionConenctor -> ClientSession

Ownership graph after:
Activity -> ClientSession

BUG=477522
TEST=All browser tests passed on
https://chromium-swarm.appspot.com/user/tasks?sort=created_ts&state=all&limit=10&task_name=chromoting_integration_tests

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

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

remoting/remoting_webapp_files.gypi

@@ -39,7 +39,6 @@
       'webapp/crd/js/mock_host_list_api.js',
       'webapp/crd/js/mock_identity.js',
       'webapp/crd/js/mock_oauth2_api.js',
-      'webapp/crd/js/mock_session_connector.js',
       'webapp/crd/js/mock_signal_strategy.js',
     ],
     'remoting_webapp_browsertest_js_proto_files': [
@@ -74,6 +73,7 @@
       'webapp/base/js/protocol_extension_manager_unittest.js',
       'webapp/crd/js/apps_v2_migration_unittest.js',
       'webapp/crd/js/desktop_viewport_unittest.js',
+      'webapp/crd/js/client_session_factory_unittest.js',
       'webapp/crd/js/dns_blackhole_checker_unittest.js',
       'webapp/crd/js/error_unittest.js',
       'webapp/crd/js/fallback_signal_strategy_unittest.js',
@@ -95,6 +95,7 @@
     'remoting_webapp_unittests_js_mock_files': [
       # Some proto files can be repurposed as simple mocks for the unittests.
       # Note that some defs in chrome_proto are overwritten by chrome_mocks.
+      'webapp/crd/js/mock_client_plugin.js',
       'webapp/crd/js/mock_host_daemon_facade.js',
       'webapp/crd/js/mock_signal_strategy.js',
       'webapp/js_proto/chrome_proto.js',
@@ -157,14 +158,13 @@
       'webapp/crd/js/client_plugin_impl.js',
       'webapp/crd/js/client_plugin_host_desktop_impl.js',
       'webapp/crd/js/client_session.js',
+      'webapp/crd/js/client_session_factory.js',
       'webapp/crd/js/clipboard.js',
       'webapp/crd/js/connected_view.js',
       'webapp/crd/js/connection_info.js',
       'webapp/crd/js/credentials_provider.js',
       'webapp/crd/js/desktop_connected_view.js',
       'webapp/crd/js/host_desktop.js',
-      'webapp/crd/js/session_connector.js',
-      'webapp/crd/js/session_connector_impl.js',
       'webapp/crd/js/smart_reconnector.js',
       'webapp/crd/js/video_frame_recorder.js',
     ],

remoting/webapp/app_remoting/js/app_remoting_activity.js

@@ -35,9 +35,9 @@ remoting.AppRemotingActivity = function(appCapabilities) {
   this.connectedView_ = null;
 
   /** @private */
-  this.connector_ = remoting.SessionConnector.factory.createConnector(
-      document.getElementById('client-container'), appCapabilities,
-      this);
+  this.sessionFactory_ = new remoting.ClientSessionFactory(
+      document.querySelector('#client-container .client-plugin-container'),
+      appCapabilities);
 
   /** @private {remoting.ClientSession} */
   this.session_ = null;
@@ -69,6 +69,7 @@ remoting.AppRemotingActivity.prototype.stop = function() {
 remoting.AppRemotingActivity.prototype.cleanup_ = function() {
   base.dispose(this.connectedView_);
   this.connectedView_ = null;
+  base.dispose(this.session_);
   this.session_ = null;
 };
 
@@ -127,11 +128,17 @@ remoting.AppRemotingActivity.prototype.onAppHostResponse_ =
                                  host['sharedSecret']);
       };
 
-      this.connector_.connect(
-          remoting.Application.Mode.APP_REMOTING, host,
-          new remoting.CredentialsProvider(
-              {fetchThirdPartyToken: fetchThirdPartyToken}));
-
+      remoting.app.setConnectionMode(remoting.Application.Mode.APP_REMOTING);
+      var credentialsProvider = new remoting.CredentialsProvider(
+              {fetchThirdPartyToken: fetchThirdPartyToken});
+      var that = this;
+
+      this.sessionFactory_.createSession(this).then(
+        function(/** remoting.ClientSession */ session) {
+          that.session_ = session;
+          session.logHostOfflineErrors(true);
+          session.connect(host, credentialsProvider);
+      });
     } else if (response && response.status == 'pending') {
       this.onError(new remoting.Error(
           remoting.Error.Tag.SERVICE_UNAVAILABLE));
@@ -149,7 +156,6 @@ remoting.AppRemotingActivity.prototype.onConnected = function(connectionInfo) {
   this.connectedView_ = new remoting.AppConnectedView(
       document.getElementById('client-container'), connectionInfo);
 
-  this.session_ = connectionInfo.session();
   var idleDetector = new remoting.IdleDetector(
       document.getElementById('idle-dialog'), this.stop.bind(this));
 

remoting/webapp/base/js/application.js

@@ -25,8 +25,6 @@ remoting.testEvents;
 remoting.Application = function() {
   // Create global factories.
   remoting.ClientPlugin.factory = new remoting.DefaultClientPluginFactory();
-  remoting.SessionConnector.factory =
-      new remoting.DefaultSessionConnectorFactory();
 
   /** @protected {remoting.Application.Mode} */
   this.connectionMode_ = remoting.Application.Mode.ME2ME;

remoting/webapp/base/js/protocol_extension.js

@@ -5,7 +5,7 @@
 /**
  * @fileoverview
  * Interface abstracting the protocol extension functionality.
- * Instances of this class can be registered with the SessionConnector
+ * Instances of this class can be registered with the ProtocolExtensionManager
  * to enhance the communication protocol between the host and client.
  * Note that corresponding support on the host side is required.
  */

remoting/webapp/crd/js/client_session.js

@@ -8,9 +8,8 @@
  *
  * The ClientSession class controls lifetime of the client plugin
  * object and provides the plugin with the functionality it needs to
- * establish connection. Specifically it:
- *  - Delivers incoming/outgoing signaling messages,
- *  - Adjusts plugin size and position when destop resolution changes,
+ * establish connection, e.g. delivers incoming/outgoing signaling
+ * messages.
  *
  * This class should not access the plugin directly, instead it should
  * do it through ClientPlugin class which abstracts plugin version
@@ -24,15 +23,15 @@ var remoting = remoting || {};
 
 /**
  * @param {remoting.ClientPlugin} plugin
- * @param {remoting.Host} host The host to connect to.
  * @param {remoting.SignalStrategy} signalStrategy Signal strategy.
+ * @param {remoting.ClientSession.EventHandler} listener
  *
  * @constructor
  * @extends {base.EventSourceImpl}
  * @implements {base.Disposable}
  * @implements {remoting.ClientPlugin.ConnectionEventHandler}
  */
-remoting.ClientSession = function(plugin, host, signalStrategy) {
+remoting.ClientSession = function(plugin, signalStrategy, listener) {
   base.inherits(this, base.EventSourceImpl);
 
   /** @private */
@@ -41,12 +40,18 @@ remoting.ClientSession = function(plugin, host, signalStrategy) {
   /** @private {!remoting.Error} */
   this.error_ = remoting.Error.none();
 
-  /** @private */
-  this.host_ = host;
+  /** @private {remoting.Host} */
+  this.host_ = null;
+
+  /** @private {remoting.CredentialsProvider} */
+  this.credentialsProvider_ = null;
 
   /** @private */
   this.sessionId_ = '';
 
+  /** @private */
+  this.listener_ = listener;
+
   /** @private */
   this.hasReceivedFrame_ = false;
   this.logToServer = new remoting.LogToServer(signalStrategy);
@@ -58,9 +63,8 @@ remoting.ClientSession = function(plugin, host, signalStrategy) {
   this.signalStrategy_.setIncomingStanzaCallback(
       this.onIncomingMessage_.bind(this));
 
-  /** @private */
-  this.iqFormatter_ =
-      new remoting.FormatIq(this.signalStrategy_.getJid(), host.jabberId);
+ /** @private {remoting.FormatIq} */
+  this.iqFormatter_ = null;
 
   /**
    * Allow host-offline error reporting to be suppressed in situations where it
@@ -254,6 +258,21 @@ remoting.ClientSession.Capability = {
   CAST: 'casting',
 };
 
+/**
+ * Connects to |host| using |credentialsProvider| as the credentails.
+ *
+ * @param {remoting.Host} host
+ * @param {remoting.CredentialsProvider} credentialsProvider
+ */
+remoting.ClientSession.prototype.connect = function(host, credentialsProvider) {
+  this.host_ = host;
+  this.credentialsProvider_ = credentialsProvider;
+  this.iqFormatter_ =
+      new remoting.FormatIq(this.signalStrategy_.getJid(), host.jabberId);
+  this.plugin_.connect(this.host_, this.signalStrategy_.getJid(),
+                       credentialsProvider);
+};
+
 /**
  * Disconnect the current session with a particular |error|.  The session will
  * raise a |stateChanged| event in response to it.  The caller should then call
@@ -297,6 +316,7 @@ remoting.ClientSession.prototype.disconnect = function(error) {
 remoting.ClientSession.prototype.dispose = function() {
   base.dispose(this.connectedDisposables_);
   this.connectedDisposables_ = null;
+  base.dispose(this.plugin_);
   this.plugin_ = null;
 };
 
@@ -488,7 +508,65 @@ remoting.ClientSession.prototype.setState_ = function(newState) {
     this.connectedDisposables_ = null;
   }
 
+  this.notifyStateChanges_(oldState, this.state_);
   this.logToServer.logClientSessionStateChange(this.state_, this.error_);
+};
+
+/**
+ * @param {remoting.ClientSession.State} oldState The new state for the session.
+ * @param {remoting.ClientSession.State} newState The new state for the session.
+ * @private
+ */
+remoting.ClientSession.prototype.notifyStateChanges_ =
+    function(oldState, newState) {
+  /** @type {remoting.Error} */
+  var error;
+  switch (this.state_) {
+    case remoting.ClientSession.State.CONNECTED:
+      console.log('Connection established.');
+      var connectionInfo = new remoting.ConnectionInfo(
+          this.host_, this.credentialsProvider_, this, this.plugin_);
+      this.listener_.onConnected(connectionInfo);
+      break;
+
+    case remoting.ClientSession.State.CONNECTING:
+      remoting.identity.getEmail().then(function(/** string */ email) {
+        console.log('Connecting as ' + email);
+      });
+      break;
+
+    case remoting.ClientSession.State.AUTHENTICATED:
+      console.log('Connection authenticated.');
+      break;
+
+    case remoting.ClientSession.State.INITIALIZING:
+      console.log('Connection initializing .');
+      break;
+
+    case remoting.ClientSession.State.CLOSED:
+      console.log('Connection closed.');
+      this.listener_.onDisconnected();
+      break;
+
+    case remoting.ClientSession.State.FAILED:
+      error = this.getError();
+      console.error('Connection failed: ' + error.toString());
+      this.listener_.onConnectionFailed(error);
+      break;
+
+    case remoting.ClientSession.State.CONNECTION_DROPPED:
+      error = this.getError();
+      console.error('Connection dropped: ' + error.toString());
+      this.listener_.onError(error);
+      break;
+
+    default:
+      console.error('Unexpected client plugin state: ' + newState);
+      // This should only happen if the web-app and client plugin get out of
+      // sync, and even then the version check should ensure compatibility.
+      this.listener_.onError(
+          new remoting.Error(remoting.Error.Tag.MISSING_PLUGIN));
+  }
 
   this.raiseEvent(remoting.ClientSession.Events.stateChanged,
     new remoting.ClientSession.StateEvent(newState, oldState)