[Chromoting] Move ownership of ClientPlugin into SessionConnector.

remoting/webapp/crd/js/client_session.js

 // Copyright (c) 2012 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.
 
 /**
  * @fileoverview
  * Class handling creation and teardown of a remoting client session.
  *
  * The ClientSession class controls lifetime of the client plugin
  * object and provides the plugin with the functionality it needs to
@@ skipping 18 matching lines @@
  * @const
  * @type {number}
  */
 remoting.ACCESS_TOKEN_RESEND_INTERVAL_MS = 15 * 60 * 1000;
 
 /**
  * @param {remoting.Host} host The host to connect to.
  * @param {remoting.SignalStrategy} signalStrategy Signal strategy.
  * @param {remoting.CredentialsProvider} credentialsProvider
  *     The credentialsProvider to authenticate the client with the host.
- * @param {HTMLElement} container Container element for the client view.
  * @param {remoting.DesktopConnectedView.Mode} mode The mode of this connection.
- * @param {string} defaultRemapKeys The default set of remap keys, to use
- *     when the client doesn't define any.
  * @constructor
  * @extends {base.EventSourceImpl}
  * @implements {base.Disposable}
  */
 remoting.ClientSession = function(host, signalStrategy, credentialsProvider,

[kelvinp, 2015/03/06 02:58:37]

I think we should create the clientSession only after the plugin is initialized
and passed it (the plugin) in the client session constructor.  In this way, we
don't have to worry about remove the plugin in SessionConnector if plugin
initialization failed.

-                                  container, mode, defaultRemapKeys) {
+                                  mode) {
   /** @private */
   this.state_ = remoting.ClientSession.State.CREATED;
 
   /** @private */
   this.error_ = remoting.Error.NONE;
 
   /** @private */
   this.host_ = host;
 
   /** @private */
   this.credentialsProvider_ = credentialsProvider;
 
   /** @private */
-  this.uiHandler_ = new remoting.DesktopConnectedView(
-      this, container, this.host_, mode, defaultRemapKeys,
-      this.onPluginInitialized_.bind(this));
-  remoting.desktopConnectedView = this.uiHandler_;
-
-  /** @private */
   this.sessionId_ = '';
   /** @type {remoting.ClientPlugin}
     * @private */
   this.plugin_ = null;
   /** @private */
   this.hasReceivedFrame_ = false;
   this.logToServer = new remoting.LogToServer(signalStrategy, mode);
 
   /** @private */
   this.signalStrategy_ = signalStrategy;
@@ skipping 165 matching lines @@
  *     the client and host.
  */
 remoting.ClientSession.prototype.hasCapability = function(capability) {
   if (this.capabilities_ == null)
     return false;
 
   return this.capabilities_.indexOf(capability) > -1;
 };
 
 /**
- * Adds <embed> element to the UI container and readies the session object.
- *
- * @param {function(string, string):boolean} onExtensionMessage The handler for
- *     protocol extension messages. Returns true if a message is recognized;
- *     false otherwise.
- * @param {Array<string>} requiredCapabilities A list of capabilities
- *     required by this application.
- */
-remoting.ClientSession.prototype.createPluginAndConnect =
-    function(onExtensionMessage, requiredCapabilities) {
-  this.uiHandler_.createPluginAndConnect(onExtensionMessage,
-                                         requiredCapabilities);
-};
-
-/**
- * @param {remoting.Error} error
  * @param {remoting.ClientPlugin} plugin
  */
-remoting.ClientSession.prototype.onPluginInitialized_ = function(
-    error, plugin) {
-  if (error != remoting.Error.NONE) {
-    this.resetWithError_(error);
-  }
-
+remoting.ClientSession.prototype.onPluginInitialized = function(plugin) {
   this.plugin_ = plugin;
   plugin.setOnOutgoingIqHandler(this.sendIq_.bind(this));
   plugin.setOnDebugMessageHandler(this.onDebugMessage_.bind(this));
 
   plugin.setConnectionStatusUpdateHandler(
       this.onConnectionStatusUpdate_.bind(this));
   plugin.setRouteChangedHandler(this.onRouteChanged_.bind(this));
   plugin.setConnectionReadyHandler(this.onConnectionReady_.bind(this));
   plugin.setCapabilitiesHandler(this.onSetCapabilities_.bind(this));
   plugin.setGnubbyAuthHandler(
       this.processGnubbyAuthMessage_.bind(this));
   plugin.setCastExtensionHandler(
       this.processCastExtensionMessage_.bind(this));
 
   this.plugin_.connect(
       this.host_, this.signalStrategy_.getJid(), this.credentialsProvider_);
 };
 
 /**
- * @param {remoting.Error} error
- */
-remoting.ClientSession.prototype.resetWithError_ = function(error) {
-  this.signalStrategy_.setIncomingStanzaCallback(null);
-  this.removePlugin();
-  this.error_ = error;
-  this.setState_(remoting.ClientSession.State.FAILED);
-};
-
-/**
  * Deletes the <embed> element from the container, without sending a

[kelvinp, 2015/03/06 02:58:37]

This comment needs to be updated.

[garykac, 2015/03/06 23:38:33]

Done.

  * session_terminate request.  This is to be called when the session was
  * disconnected by the Host.
  *
  * @return {void} Nothing.
  */
 remoting.ClientSession.prototype.removePlugin = function() {
-  this.uiHandler_.removePlugin();

[kelvinp, 2015/03/06 02:58:37]

I think after all, it may still make sense for the clientSession to own the
plugin, due to life time reason.

I think here are the lifetime of the objects
|<----------- Application ---------------->|
|<---------- SessionConnector ------------>|
     |<-----   ClientPlugin  ---------->|
     |<-----   ClientSession ---------->|
        |<--DesktopConnectedView->|

[garykac, 2015/03/06 23:38:33]

ClientPlugin and ClientSession have similar lifetimes, but the plugin needs to
be shared with the UX code (DesktopConnectedView) and we want to move all the UX
code out of here (into the app-specific code).

Moving the ClientPlugin up into SessionConnector makes it slightly easier to
share the plugin with the app-specific code.

   this.plugin_ = null;
-  remoting.desktopConnectedView = null;
 };
 
 /**
  * Disconnect the current session with a particular |error|.  The session will
  * raise a |stateChanged| event in response to it.  The caller should then call
  * dispose() to remove and destroy the <embed> element.
  *
  * @param {remoting.Error} error The reason for the disconnection.  Use
  *    remoting.Error.NONE if there is no error.
  * @return {void} Nothing.
@@ skipping 117 matching lines @@
  * Callback that the plugin invokes to indicate that the connection
  * status has changed.
  *
  * @param {number} status The plugin's status.
  * @param {number} error The plugin's error state, if any.
  * @private
  */
 remoting.ClientSession.prototype.onConnectionStatusUpdate_ =
     function(status, error) {
   if (status == remoting.ClientSession.State.CONNECTED) {
-    this.uiHandler_.updateClientSessionUi_(this);
+    remoting.desktopConnectedView.updateClientSessionUi_(this);
 
   } else if (status == remoting.ClientSession.State.FAILED) {
     switch (error) {
       case remoting.ClientSession.ConnectionError.HOST_IS_OFFLINE:
         this.error_ = remoting.Error.HOST_IS_OFFLINE;
         break;
       case remoting.ClientSession.ConnectionError.SESSION_REJECTED:
         this.error_ = remoting.Error.INVALID_ACCESS_CODE;
         break;
       case remoting.ClientSession.ConnectionError.INCOMPATIBLE_PROTOCOL:
@@ skipping 38 matching lines @@
   // TODO(jamiewalch): Currently, the logic for determining whether or not the
   // connection is available is based solely on whether or not any video frames
   // have been received recently. which leads to poor UX on slow connections.
   // Re-enable this once crbug.com/435315 has been fixed.
   var ignoreVideoChannelState = true;
   if (ignoreVideoChannelState) {
     console.log('Video channel ' + (ready ? '' : 'not ') + 'ready.');
     return;
   }
 
-  this.uiHandler_.onConnectionReady(ready);
+  remoting.desktopConnectedView.onConnectionReady(ready);
 
   this.raiseEvent(remoting.ClientSession.Events.videoChannelStateChanged,
                   ready);
 };
 
 /**
  * Called when the client-host capabilities negotiation is complete.
  * TODO(kelvinp): Move this function out of ClientSession.
  *
  * @param {!Array<string>} capabilities The set of capabilities negotiated
  *     between the client and host.
  * @return {void} Nothing.
  * @private
  */
 remoting.ClientSession.prototype.onSetCapabilities_ = function(capabilities) {
   if (this.capabilities_ != null) {
     console.error('onSetCapabilities_() is called more than once');
     return;
   }
 
   this.capabilities_ = capabilities;
   if (this.hasCapability(remoting.ClientSession.Capability.GOOGLE_DRIVE)) {
     this.sendGoogleDriveAccessToken_();
   }
   if (this.hasCapability(
       remoting.ClientSession.Capability.VIDEO_RECORDER)) {
-    this.uiHandler_.initVideoFrameRecorder();
+    remoting.desktopConnectedView.initVideoFrameRecorder();
   }
 };
 
 /**
  * @param {remoting.ClientSession.State} newState The new state for the session.
  * @return {void} Nothing.
  * @private
  */
 remoting.ClientSession.prototype.setState_ = function(newState) {
   var oldState = this.state_;
@@ skipping 118 matching lines @@
     console.error('Received unexpected gnubby message');
   }
 };
 
 /**
  * Create a gnubby auth handler and inform the host that gnubby auth is
  * supported.
  * @private
  */
 remoting.ClientSession.prototype.createGnubbyAuthHandler_ = function() {
-  if (this.uiHandler_.getMode() == remoting.DesktopConnectedView.Mode.ME2ME) {
+  if (remoting.desktopConnectedView.getMode() ==
+      remoting.DesktopConnectedView.Mode.ME2ME) {
     this.gnubbyAuthHandler_ = new remoting.GnubbyAuthHandler(this);
     // TODO(psj): Move to more generic capabilities mechanism.
     this.sendGnubbyAuthMessage({'type': 'control', 'option': 'auth-v1'});
   }
 };
 
 /**
  * Timer callback to send the access token to the host.
  * @private
  */
@@ skipping 46 matching lines @@
   }
 };
 
 /**
  * Create a CastExtensionHandler and inform the host that cast extension
  * is supported.
  * @private
  */
 remoting.ClientSession.prototype.createCastExtensionHandler_ = function() {
   if (remoting.app.hasCapability(remoting.ClientSession.Capability.CAST) &&
-      this.uiHandler_.getMode() == remoting.DesktopConnectedView.Mode.ME2ME) {
+      remoting.desktopConnectedView.getMode() ==
+          remoting.DesktopConnectedView.Mode.ME2ME) {
     this.castExtensionHandler_ = new remoting.CastExtensionHandler(this);
   }
 };
 
 /**
  * Handles protocol extension messages.
  * @param {string} type Type of extension message.
  * @param {Object} message The parsed extension message data.
  * @return {boolean} True if the message was recognized, false otherwise.
  */
 remoting.ClientSession.prototype.handleExtensionMessage =
     function(type, message) {
-  if (this.uiHandler_.handleExtensionMessage(type, message)) {
+  if (remoting.desktopConnectedView.handleExtensionMessage(type, message)) {
     return true;
   }
   return false;
 };
 
 /**
  * Enables or disables rendering of dirty regions for debugging.
  * @param {boolean} enable True to enable rendering.
  */
 remoting.ClientSession.prototype.enableDebugRegion = function(enable) {
   if (enable) {
     this.plugin_.setDebugDirtyRegionHandler(
-        this.uiHandler_.handleDebugRegion.bind(this.uiHandler_));
+        remoting.desktopConnectedView.handleDebugRegion.bind(
+            remoting.desktopConnectedView));
   } else {
     this.plugin_.setDebugDirtyRegionHandler(null);
   }
 }