[Chromoting] Move ownership of ClientPlugin into SessionConnector.

remoting/webapp/crd/js/client_session.js

Index: remoting/webapp/crd/js/client_session.js
diff --git a/remoting/webapp/crd/js/client_session.js b/remoting/webapp/crd/js/client_session.js
index 4fa47c926192b68632a9f5b41536c6f981c8e6c9..19a2d8b995aa6ddfd43a936e76711fda3f8a8f81 100644
--- a/remoting/webapp/crd/js/client_session.js
+++ b/remoting/webapp/crd/js/client_session.js
@@ -36,16 +36,13 @@ remoting.ACCESS_TOKEN_RESEND_INTERVAL_MS = 15 * 60 * 1000;
  * @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;
 
@@ -59,12 +56,6 @@ remoting.ClientSession = function(host, signalStrategy, credentialsProvider,
   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 */
@@ -250,30 +241,9 @@ remoting.ClientSession.prototype.hasCapability = function(capability) {
 };
 
 /**
- * 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));
@@ -293,16 +263,6 @@ remoting.ClientSession.prototype.onPluginInitialized_ = function(
 };
 
 /**
- * @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.
@@ -310,9 +270,7 @@ remoting.ClientSession.prototype.resetWithError_ = function(error) {
  * @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;
 };
 
 /**
@@ -450,7 +408,7 @@ remoting.ClientSession.prototype.onIncomingMessage_ = function(message) {
 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) {
@@ -509,7 +467,7 @@ remoting.ClientSession.prototype.onConnectionReady_ = function(ready) {
     return;
   }
 
-  this.uiHandler_.onConnectionReady(ready);
+  remoting.desktopConnectedView.onConnectionReady(ready);
 
   this.raiseEvent(remoting.ClientSession.Events.videoChannelStateChanged,
                   ready);
@@ -536,7 +494,7 @@ remoting.ClientSession.prototype.onSetCapabilities_ = function(capabilities) {
   }
   if (this.hasCapability(
       remoting.ClientSession.Capability.VIDEO_RECORDER)) {
-    this.uiHandler_.initVideoFrameRecorder();
+    remoting.desktopConnectedView.initVideoFrameRecorder();
   }
 };
 
@@ -675,7 +633,8 @@ remoting.ClientSession.prototype.processGnubbyAuthMessage_ = function(data) {
  * @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'});
@@ -742,7 +701,8 @@ remoting.ClientSession.prototype.processCastExtensionMessage_ = function(data) {
  */
 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);
   }
 };
@@ -755,7 +715,7 @@ remoting.ClientSession.prototype.createCastExtensionHandler_ = function() {
  */
 remoting.ClientSession.prototype.handleExtensionMessage =
     function(type, message) {
-  if (this.uiHandler_.handleExtensionMessage(type, message)) {
+  if (remoting.desktopConnectedView.handleExtensionMessage(type, message)) {
     return true;
   }
   return false;
@@ -768,7 +728,8 @@ remoting.ClientSession.prototype.handleExtensionMessage =
 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);
   }