Implement FallbackSignalStrategy.

remoting/webapp/crd/js/fallback_signal_strategy.js

Index: remoting/webapp/crd/js/fallback_signal_strategy.js
diff --git a/remoting/webapp/crd/js/fallback_signal_strategy.js b/remoting/webapp/crd/js/fallback_signal_strategy.js
new file mode 100644
index 0000000000000000000000000000000000000000..86e38accf2446bb31657721200cdd791be135801
--- /dev/null
+++ b/remoting/webapp/crd/js/fallback_signal_strategy.js
@@ -0,0 +1,327 @@
+// Copyright 2014 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.
+
+'use strict';
+
+/** @suppress {duplicate} */
+var remoting = remoting || {};
+
+/**
+ * A signal strategy encapsulating a primary and a back-up strategy. If the
+ * 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 {function(remoting.FallbackSignalStrategy.Result)} onProgressCallback
+ *
+ * @implements {remoting.SignalStrategy}
+ * @constructor
+ */
+remoting.FallbackSignalStrategy = function(
+    primaryFactory, secondaryFactory,
+    onStateChangedCallback, onProgressCallback) {
+  /**
+   * @type {remoting.SignalStrategy}
+   * @private
+   */
+  this.primary_ = primaryFactory(this.onStateChanged_.bind(this, true));
+
+  /**
+   * @type {remoting.SignalStrategy}
+   * @private
+   */
+  this.secondary_ = secondaryFactory(this.onStateChanged_.bind(this, false));
+
+  /**
+   * @type {function(remoting.SignalStrategy.State)}
+   * @private
+   */
+  this.onStateChangedCallback_ = onStateChangedCallback;
+
+  /**
+   * @type {function(remoting.FallbackSignalStrategy.Result)}
+   * @private
+   */
+  this.onProgressCallback_ = onProgressCallback;
+
+  /**
+   * @type {?function(Element):void}
+   * @private
+   */
+  this.onIncomingStanzaCallback_ = null;
+
+  /**
+   * @type {number}
+   * @private
+   * @const
+   */
+  this.PRIMARY_CONNECT_TIMEOUT_MS_ = 10 * 1000;
+
+  /**
+   * @enum {string}
+   * @private
+   */
+  this.State = {

[kelvinp, 2015/01/13 19:22:47]

Enum should be defined on the 'class' remoting.FallbackSignalStrategy and not
the instance. e.g.
remoting.FallbackSignalStrategy.State = { .. } ;

[Jamie, 2015/01/14 01:36:26]

I considered that, but it ends up being incredibly verbose at all the
call-sites, and for an internal private enum, I felt that this was more
idiomatic JavaScript. With the suggested name change for ProgressResult, I've
ended up having to use a "typedef" for shorter lines, so I could do the same
with this type if you feel strongly that it should be defined on the class.

+    NOT_CONNECTED: 'not-connected',
+    PRIMARY_PENDING: 'primary-pending',
+    PRIMARY_SUCCEEDED: 'primary-succeeded',
+    SECONDARY_PENDING: 'secondary-pending',
+    SECONDARY_SUCCEEDED: 'secondary-succeeded',
+    SECONDARY_FAILED: 'secondary-failed',
+    CLOSED: 'closed'
+  };
+
+  /**
+   * @type {string}
+   * @private
+   */
+  this.state_ = this.State.NOT_CONNECTED;
+
+  /**
+   * @type {?remoting.SignalStrategy.State}
+   * @private
+   */
+  this.externalState_ = null;
+
+  /**
+   * @type {string}
+   * @private
+   */
+  this.server_ = '';
+
+  /**
+   * @type {string}
+   * @private
+   */
+  this.username_ = '';
+
+  /**
+   * @type {string}
+   * @private
+   */
+  this.authToken_ = '';
+
+  /**
+   * @type {number}
+   * @private
+   */
+  this.primaryConnectTimerId_ = 0;
+};
+
+/**
+ * @enum {string}
+ */
+remoting.FallbackSignalStrategy.Result = {

[kelvinp, 2015/01/13 19:22:47]

Nit: ProgressResult.  To make it clear that this is used as the return value for
the progressCallback. 

[Jamie, 2015/01/14 01:36:26]

I changed it to Progress, since it's not always a final "result" (the callback
might see primary-failed, then secondary-succeeded, maybe even followed by
primary-succeeded-late).

[kelvinp, 2015/01/14 19:46:22]

Done.

+  PRIMARY_SUCCEEDED: 'primary-succeeded',
+  PRIMARY_FAILED: 'primary-failed',
+  PRIMARY_TIMED_OUT: 'primary-timed-out',
+  PRIMARY_SUCCEEDED_LATE: 'primary-succeeded-late',
+  PRIMARY_FAILED_LATE: 'primary-failed-late',
+  SECONDARY_SUCCEEDED: 'secondary-succeeded',
+  SECONDARY_FAILED: 'secondary-failed'
+};
+
+remoting.FallbackSignalStrategy.prototype.dispose = function() {
+  this.primary_.dispose();
+  this.secondary_.dispose();
+};
+
+/**
+ * @param {?function(Element):void} onIncomingStanzaCallback Callback to call on
+ *     incoming messages.
+ */
+remoting.FallbackSignalStrategy.prototype.setIncomingStanzaCallback =
+    function(onIncomingStanzaCallback) {
+  this.onIncomingStanzaCallback_ = onIncomingStanzaCallback;
+  if (this.state_ == this.State.PRIMARY_PENDING ||
+      this.state_ == this.State.PRIMARY_SUCCEEDED) {
+    this.primary_.setIncomingStanzaCallback(onIncomingStanzaCallback);
+  } else if (this.state_ == this.State.SECONDARY_PENDING ||
+             this.state_ == this.State.SECONDARY_SUCCEEDED) {
+    this.secondary_.setIncomingStanzaCallback(onIncomingStanzaCallback);
+  }
+};
+
+/**
+ * @param {string} server
+ * @param {string} username
+ * @param {string} authToken
+ */
+remoting.FallbackSignalStrategy.prototype.connect =
+    function(server, username, authToken) {
+  base.debug.assert(this.state_ == this.State.NOT_CONNECTED);
+  this.server_ = server;
+  this.username_ = username;
+  this.authToken_ = authToken;
+  this.state_ = this.State.PRIMARY_PENDING;
+  this.primary_.setIncomingStanzaCallback(this.onIncomingStanzaCallback_);
+  this.primary_.connect(server, username, authToken);
+  this.primaryConnectTimerId_ =
+      window.setTimeout(this.onPrimaryTimeout_.bind(this),
+                        this.PRIMARY_CONNECT_TIMEOUT_MS_);
+};
+
+/**
+ * Sends a message. Can be called only in CONNECTED state.
+ * @param {string} message
+ */
+remoting.FallbackSignalStrategy.prototype.sendMessage = function(message) {
+  this.getConnectedSignalStrategy_().sendMessage(message);
+};
+
+/** @return {remoting.SignalStrategy.State} Current state */
+remoting.FallbackSignalStrategy.prototype.getState = function() {
+  return (this.externalState_ === null)
+      ? remoting.SignalStrategy.State.NOT_CONNECTED
+      : this.externalState_;
+};
+
+/** @return {remoting.Error} Error when in FAILED state. */
+remoting.FallbackSignalStrategy.prototype.getError = function() {
+  base.debug.assert(this.state_ == this.State.SECONDARY_FAILED);
+  base.debug.assert(
+      this.secondary_.getState() == remoting.SignalStrategy.State.FAILED);
+  return this.secondary_.getError();
+};
+
+/** @return {string} Current JID when in CONNECTED state. */
+remoting.FallbackSignalStrategy.prototype.getJid = function() {
+  return this.getConnectedSignalStrategy_().getJid();
+};
+
+/**
+ * @return {remoting.SignalStrategy} The active signal strategy, if the
+ *     connection has succeeded.
+ * @private
+ */
+remoting.FallbackSignalStrategy.prototype.getConnectedSignalStrategy_ =
+    function() {
+  if (this.state_ == this.State.PRIMARY_SUCCEEDED) {
+    base.debug.assert(
+        this.primary_.getState() == remoting.SignalStrategy.State.CONNECTED);
+    return this.primary_;
+  } else if (this.state_ == this.State.SECONDARY_SUCCEEDED) {
+    base.debug.assert(
+        this.secondary_.getState() == remoting.SignalStrategy.State.CONNECTED);
+    return this.secondary_;
+  } else {
+    base.debug.assert(
+        false,
+        'getConnectedSignalStrategy called in unconnected state');
+    return null;
+  }
+};
+
+/**
+ * @param {boolean} isPrimary
+ * @param {remoting.SignalStrategy.State} state
+ * @private
+ */
+remoting.FallbackSignalStrategy.prototype.onStateChanged_ =

[Sergey Ulanov, 2015/01/13 19:43:51]

Maybe separate this into two callbacks, one for primary and one for secondary
connection? I think that would simplify state handling.

[Jamie, 2015/01/14 01:36:26]

Done.

+    function(isPrimary, state) {
+  /**
+   * @type {boolean} True to notify the external callback of the new state
+   *     (ie, if it represents a later stage of the connection process than
+   *     has previously been notified). This ensures that, for example, the
+   *     external callback won't see a CONNECTING -> NOT_CONNECTED transition
+   *     when we switch from the primary to the secondary signal strategy.
+   */
+  var notifyExternalCallback = (this.externalState_ === null) ||
+                               (state > this.externalState_);
+
+  switch (state) {
+    case remoting.SignalStrategy.State.CONNECTED:
+      if (isPrimary) {
+        if (this.state_ == this.State.PRIMARY_PENDING) {
+          window.clearTimeout(this.primaryConnectTimerId_);
+          this.onProgressCallback_(
+              remoting.FallbackSignalStrategy.Result.PRIMARY_SUCCEEDED);
+          this.state_ = this.State.PRIMARY_SUCCEEDED;
+        } else {
+          this.onProgressCallback_(
+              remoting.FallbackSignalStrategy.Result.PRIMARY_SUCCEEDED_LATE);
+        }
+      } else {
+        this.onProgressCallback_(

[kelvinp, 2015/01/13 19:22:48]

base.debug.assert(this.state_ == this.State.SECONDARY_PENDING);

[Jamie, 2015/01/14 01:36:26]

I've factored this into two separate functions. LMK if you still think this
assert would make it clearer.

[kelvinp, 2015/01/14 19:46:22]

I think the code is much clearer with two separate function :)

+            remoting.FallbackSignalStrategy.Result.SECONDARY_SUCCEEDED);
+        this.state_ = this.State.SECONDARY_SUCCEEDED;
+      }
+      break;
+
+    case remoting.SignalStrategy.State.FAILED:
+      notifyExternalCallback = !isPrimary;
+      if (isPrimary) {
+        if (this.state_ == this.State.PRIMARY_PENDING) {
+          window.clearTimeout(this.primaryConnectTimerId_);
+          this.onProgressCallback_(
+              remoting.FallbackSignalStrategy.Result.PRIMARY_FAILED);
+          this.reconnect_();
+        } else {
+          this.onProgressCallback_(
+              remoting.FallbackSignalStrategy.Result.PRIMARY_FAILED_LATE);
+        }
+      } else {
+        this.onProgressCallback_(
+            remoting.FallbackSignalStrategy.Result.SECONDARY_FAILED);
+      }
+      break;
+
+    case remoting.SignalStrategy.State.CLOSED:
+      this.state_ = this.State.CLOSED;
+      break;
+  }
+
+  if (notifyExternalCallback) {
+    this.externalState_ = state;
+    this.onStateChangedCallback_(state);
+  }
+};
+
+/**
+ * @private
+ */
+remoting.FallbackSignalStrategy.prototype.reconnect_ = function() {

[Sergey Ulanov, 2015/01/13 19:43:51]

maybe call it connectSecondary_()?

[Jamie, 2015/01/14 01:36:26]

Done.

+  base.debug.assert(this.state_ == this.State.PRIMARY_PENDING);
+  base.debug.assert(this.server_ != '');
+  base.debug.assert(this.username_ != '');
+  base.debug.assert(this.authToken_ != '');
+
+  this.state_ = this.State.SECONDARY_PENDING;
+  this.primary_.setIncomingStanzaCallback(null);
+  this.secondary_.setIncomingStanzaCallback(this.onIncomingStanzaCallback_);
+  this.secondary_.connect(this.server_, this.username_, this.authToken_);

[kelvinp, 2015/01/13 19:22:48]

Does the primary and the secondary signal strategy shares the same authToken
even they are different services?

[Sergey Ulanov, 2015/01/13 19:43:51]

we use the same token for xmpp and wcs

+};
+
+/**
+ * @private
+ */
+remoting.FallbackSignalStrategy.prototype.onPrimaryTimeout_ = function() {
+  this.onProgressCallback_(
+      remoting.FallbackSignalStrategy.Result.PRIMARY_TIMED_OUT);
+  this.reconnect_();
+};
+
+/**
+ * @return {remoting.SignalStrategy} The primary signal strategy.
+ * @private

[Sergey Ulanov, 2015/01/13 19:43:51]

This is called from tests so it isn't really private. 

[Jamie, 2015/01/14 01:36:26]

True, but marking it as private means that the compiler will complain if
production code calls it (we don't run the compiler over our tests). That said,
the same test that calls this accesses PRIMARY_CONNECT_TIMEOUT_MS_ directly, so
I could simply remove these methods. WDYT?

[Sergey Ulanov, 2015/01/14 19:42:43]

sgtm

+ */
+remoting.FallbackSignalStrategy.prototype.getPrimaryStrategyForTesting_ =
+    function() {
+  return this.primary_;
+};
+
+/**
+ * @return {remoting.SignalStrategy} The primary signal strategy.
+ * @private
+ */
+remoting.FallbackSignalStrategy.prototype.getSecondaryStrategyForTesting_ =
+    function() {
+  return this.secondary_;
+};