Add SignalStrategy.setStateChangedCallback()

Previous SignalStrategy implementations had to get state callback in the
constructor. This makes it harder to implement wrappers such as
FallbackSignalStrategy. Added SignalStrategy.setStateChangedCallback,
which simplified it and the code that creates FallbackSignalStrategy.

This will also be useful for bug 443397, which I'm planning to fix by
addind a SignalStrategy filter.

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

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

remoting/webapp/crd/js/fallback_signal_strategy.js

@@ -12,39 +12,37 @@ var remoting = remoting || {};
  * primary fails or times out, then the secondary is used. Information about
  * which strategy was used, and why, is returned via |onProgressCallback|.
  *
- * @param {function(
- *             function(remoting.SignalStrategy.State)
- *         ):remoting.SignalStrategy} primaryFactory
- * @param {function(
- *             function(remoting.SignalStrategy.State)
- *         ):remoting.SignalStrategy} secondaryFactory
- * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
+ * @param {remoting.SignalStrategy} primary
+ * @param {remoting.SignalStrategy} secondary
  * @param {function(remoting.FallbackSignalStrategy.Progress)}
  *     onProgressCallback
  *
  * @implements {remoting.SignalStrategy}
  * @constructor
  */
-remoting.FallbackSignalStrategy = function(
-    primaryFactory, secondaryFactory,
-    onStateChangedCallback, onProgressCallback) {
+remoting.FallbackSignalStrategy = function(primary,
+                                           secondary,
+                                           onProgressCallback) {
   /**
    * @type {remoting.SignalStrategy}
    * @private
    */
-  this.primary_ = primaryFactory(this.onPrimaryStateChanged_.bind(this));
+  this.primary_ = primary;
+  this.primary_.setStateChangedCallback(this.onPrimaryStateChanged_.bind(this));
 
   /**
    * @type {remoting.SignalStrategy}
    * @private
    */
-  this.secondary_ = secondaryFactory(this.onSecondaryStateChanged_.bind(this));
+  this.secondary_ = secondary;
+  this.secondary_.setStateChangedCallback(
+      this.onSecondaryStateChanged_.bind(this));
 
   /**
-   * @type {function(remoting.SignalStrategy.State)}
+   * @type {?function(remoting.SignalStrategy.State)}
    * @private
    */
-  this.onStateChangedCallback_ = onStateChangedCallback;
+  this.onStateChangedCallback_ = null;
 
   /**
    * @type {function(remoting.FallbackSignalStrategy.Progress)}
@@ -134,6 +132,15 @@ remoting.FallbackSignalStrategy.prototype.dispose = function() {
   this.secondary_.dispose();
 };
 
+/**
+ * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
+ *   Callback to call on state change.
+ */
+remoting.FallbackSignalStrategy.prototype.setStateChangedCallback = function(
+    onStateChangedCallback) {
+  this.onStateChangedCallback_ = onStateChangedCallback;
+};
+
 /**
  * @param {?function(Element):void} onIncomingStanzaCallback Callback to call on
  *     incoming messages.
@@ -158,6 +165,7 @@ remoting.FallbackSignalStrategy.prototype.setIncomingStanzaCallback =
 remoting.FallbackSignalStrategy.prototype.connect =
     function(server, username, authToken) {
   base.debug.assert(this.state_ == this.State.NOT_CONNECTED);
+  base.debug.assert(this.onStateChangedCallback_ != null);
   this.server_ = server;
   this.username_ = username;
   this.authToken_ = authToken;

remoting/webapp/crd/js/host_controller.js

@@ -554,7 +554,8 @@ remoting.HostController.prototype.getClientBaseJid_ = function(
     }
   };
 
-  signalStrategy = remoting.SignalStrategy.create(onState);
+  signalStrategy = remoting.SignalStrategy.create();
+  signalStrategy.setStateChangedCallback(onState);
 
   /** @param {string} token */
   function connectSignalingWithToken(token) {

remoting/webapp/crd/js/session_connector_impl.js

@@ -432,8 +432,9 @@ remoting.SessionConnectorImpl.prototype.connectSignaling_ = function() {
         remoting.settings.XMPP_SERVER_FOR_CLIENT, email, token);
   }
 
-  this.signalStrategy_ =
-      remoting.SignalStrategy.create(this.onSignalingState_.bind(this));
+  this.signalStrategy_ = remoting.SignalStrategy.create();
+  this.signalStrategy_.setStateChangedCallback(
+      this.onSignalingState_.bind(this));
 
   remoting.identity.callWithToken(connectSignalingWithToken, this.onError_);
 };

remoting/webapp/crd/js/signal_strategy.js

@@ -37,6 +37,13 @@ remoting.SignalStrategy.State = {
 
 remoting.SignalStrategy.prototype.dispose = function() {};
 
+/**
+ * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
+ *   Callback to call on state change.
+ */
+remoting.SignalStrategy.prototype.setStateChangedCallback =
+    function(onStateChangedCallback) {};
+
 /**
  * @param {?function(Element):void} onIncomingStanzaCallback Callback to call on
  *     incoming messages.
@@ -70,39 +77,25 @@ remoting.SignalStrategy.prototype.getJid = function() {};
 
 /**
  * Creates the appropriate signal strategy for the current environment.
- * @param {function(remoting.SignalStrategy.State): void} onStateChangedCallback
  * @return {remoting.SignalStrategy} New signal strategy object.
  */
-remoting.SignalStrategy.create = function(onStateChangedCallback) {
+remoting.SignalStrategy.create = function() {
   // Only use XMPP when TCP API is available and TLS support is enabled. That's
   // not the case for V1 app (socket API is available only to platform apps)
   // and for Chrome releases before 38.
   if (chrome.socket && chrome.socket.secure) {
-    /**
-     * @param {function(remoting.SignalStrategy.State): void} onStateChanged
-     */
-    var xmppFactory = function(onStateChanged) {
-      return new remoting.XmppConnection(onStateChanged);
-    };
-
-    /**
-     * @param {function(remoting.SignalStrategy.State): void} onStateChanged
-     */
-    var wcsFactory = function(onStateChanged) {
-      return new remoting.WcsAdapter(onStateChanged);
-    };
-
     /**
      * @param {remoting.FallbackSignalStrategy.Progress} progress
      */
     var progressCallback = function(progress) {
       console.log('FallbackSignalStrategy progress: ' + progress);
     };
 
-    return new remoting.FallbackSignalStrategy(
-        xmppFactory, wcsFactory, onStateChangedCallback, progressCallback);
+    return new remoting.FallbackSignalStrategy(new remoting.XmppConnection(),
+                                               new remoting.WcsAdapter(),
+                                               progressCallback);
 
   } else {
-    return new remoting.WcsAdapter(onStateChangedCallback);
+    return new remoting.WcsAdapter();
   }
 };

remoting/webapp/crd/js/wcs_adapter.js

@@ -11,15 +11,12 @@ var remoting = remoting || {};
  * WCS-based SignalStrategy implementation. Used instead of XMPPConnection
  * when XMPP cannot be used (e.g. in V1 app).
  *
- * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
- *   Callback to call on state change.
  * @constructor
  * @implements {remoting.SignalStrategy}
  */
-remoting.WcsAdapter =
-    function(onStateChangedCallback) {
-  /** @private */
-  this.onStateChangedCallback_ = onStateChangedCallback;
+remoting.WcsAdapter = function() {
+  /** @type {?function(remoting.SignalStrategy.State):void} @private */
+  this.onStateChangedCallback_ = null;
   /** @type {?function(Element):void} @private */
   this.onIncomingStanzaCallback_ = null;
   /** @private */
@@ -30,6 +27,14 @@ remoting.WcsAdapter =
   this.error_ = remoting.Error.NONE;
 }
 
+/**
+ * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
+ */
+remoting.WcsAdapter.prototype.setStateChangedCallback = function(
+    onStateChangedCallback) {
+  this.onStateChangedCallback_ = onStateChangedCallback;
+};
+
 /**
  * @param {?function(Element):void} onIncomingStanzaCallback Callback to call on
  *     incoming messages.
@@ -39,9 +44,15 @@ remoting.WcsAdapter.prototype.setIncomingStanzaCallback =
   this.onIncomingStanzaCallback_ = onIncomingStanzaCallback;
 }
 
-remoting.WcsAdapter.prototype.connect = function(server, authToken) {
-  remoting.wcsSandbox.setOnIq(this.onIncomingStanza_.bind(this));
+/**
+ * @param {string} server
+ * @param {string} username
+ * @param {string} authToken
+ */
+remoting.WcsAdapter.prototype.connect = function(server, username, authToken) {
+  base.debug.assert(this.onStateChangedCallback_ != null);
 
+  remoting.wcsSandbox.setOnIq(this.onIncomingStanza_.bind(this));
   remoting.wcsSandbox.connect(this.onWcsConnected_.bind(this),
                               this.onError_.bind(this));
 }

remoting/webapp/crd/js/xmpp_connection.js

@@ -15,18 +15,16 @@ var remoting = remoting || {};
  * because TLS support in chrome.sockets.tcp is currently broken, see
  * crbug.com/403076 .
  *
- * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
- *   Callback to call on state change.
  * @constructor
  * @implements {remoting.SignalStrategy}
  */
-remoting.XmppConnection = function(onStateChangedCallback) {
+remoting.XmppConnection = function() {
   /** @private */
   this.server_ = '';
   /** @private */
   this.port_ = 0;
-  /** @private */
-  this.onStateChangedCallback_ = onStateChangedCallback;
+  /** @type {?function(remoting.SignalStrategy.State):void} @private */
+  this.onStateChangedCallback_ = null;
   /** @type {?function(Element):void} @private */
   this.onIncomingStanzaCallback_ = null;
   /** @private */
@@ -96,6 +94,14 @@ remoting.XmppConnection.FAKE_SSL_RESPONSE_ = [
   0x00                                             // null compression
 ];
 
+/**
+ * @param {function(remoting.SignalStrategy.State):void} onStateChangedCallback
+ */
+remoting.XmppConnection.prototype.setStateChangedCallback = function(
+    onStateChangedCallback) {
+  this.onStateChangedCallback_ = onStateChangedCallback;
+};
+
 /**
  * @param {?function(Element):void} onIncomingStanzaCallback Callback to call on
  *     incoming messages.
@@ -113,6 +119,7 @@ remoting.XmppConnection.prototype.setIncomingStanzaCallback =
 remoting.XmppConnection.prototype.connect =
     function(server, username, authToken) {
   base.debug.assert(this.state_ == remoting.SignalStrategy.State.NOT_CONNECTED);
+  base.debug.assert(this.onStateChangedCallback_ != null);
 
   this.error_ = remoting.Error.NONE;
   var hostnameAndPort = server.split(':', 2);

remoting/webapp/unittests/fallback_signal_strategy_unittest.js

@@ -6,25 +6,31 @@
 
 'use strict';
 
-var ControllableSignalStrategy = function(jid, stateChangeCallback) {
+var ControllableSignalStrategy = function(jid) {
   this.jid = jid;
-  this.stateChangeCallback_ = stateChangeCallback;
-  this.state_ = null;
+  this.onStateChangedCallback = function() {};
+  this.state = null;
   this.onIncomingStanzaCallback = function() {};
   this.dispose = sinon.spy();
   this.connect = sinon.spy();
   this.sendMessage = sinon.spy();
 };
 
+ControllableSignalStrategy.prototype.setStateChangedCallback = function(
+    onStateChangedCallback) {
+  this.onStateChangedCallback =
+      onStateChangedCallback ? onStateChangedCallback : function() {};
+};
+
 ControllableSignalStrategy.prototype.setIncomingStanzaCallback =
     function(onIncomingStanzaCallback) {
   this.onIncomingStanzaCallback =
       onIncomingStanzaCallback ? onIncomingStanzaCallback
                                : function() {};
-}
+};
 
 ControllableSignalStrategy.prototype.getState = function(message) {
-  return this.state_;
+  return this.state;
 };
 
 ControllableSignalStrategy.prototype.getError = function(message) {
@@ -42,9 +48,9 @@ ControllableSignalStrategy.prototype.setExternalCallbackForTesting =
 
 ControllableSignalStrategy.prototype.setStateForTesting =
     function(state, expectExternalCallback) {
-  this.state_ = state;
+  this.state = state;
   this.externalCallback_.reset();
-  this.stateChangeCallback_(state);
+  this.onStateChangedCallback(state);
   if (expectExternalCallback) {
     equal(this.externalCallback_.callCount, 1);
     ok(this.externalCallback_.calledWith(state));
@@ -54,10 +60,6 @@ ControllableSignalStrategy.prototype.setStateForTesting =
   }
 };
 
-var createControllableSignalStrategy = function(jid, callback) {
-  return new ControllableSignalStrategy(jid, callback);
-};
-
 var onStateChange = null;
 var onProgressCallback = null;
 var onIncomingStanzaCallback = null;
@@ -71,10 +73,10 @@ module('fallback_signal_strategy', {
     onProgressCallback = sinon.spy();
     onIncomingStanzaCallback = sinon.spy();
     strategy = new remoting.FallbackSignalStrategy(
-        createControllableSignalStrategy.bind(null, 'primary-jid'),
-        createControllableSignalStrategy.bind(null, 'secondary-jid'),
-        onStateChange,
+        new ControllableSignalStrategy('primary-jid'),
+        new ControllableSignalStrategy('secondary-jid'),
         onProgressCallback);
+    strategy.setStateChangedCallback(onStateChange);
     strategy.setIncomingStanzaCallback(onIncomingStanzaCallback);
     primary = strategy.primary_;
     secondary = strategy.secondary_;

remoting/webapp/unittests/xmpp_connection_unittest.js

@@ -29,7 +29,8 @@ module('XmppConnection', {
     sinon.stub(chrome.socket, 'destroy');
     sinon.stub(chrome.socket, 'secure');
 
-    connection = new remoting.XmppConnection(onStateChange);
+    connection = new remoting.XmppConnection();
+    connection.setStateChangedCallback(onStateChange);
     connection.setIncomingStanzaCallback(onStanza);
   },