Webapp changes to support asking for a PIN

remoting/webapp/client_session.js

Index: remoting/webapp/client_session.js
diff --git a/remoting/webapp/client_session.js b/remoting/webapp/client_session.js
index 89fcc82eb70c599e38ebb407f7156e06890978b4..9d8733f28a58ccbba55993ea5816a210285671b7 100644
--- a/remoting/webapp/client_session.js
+++ b/remoting/webapp/client_session.js
@@ -27,8 +27,9 @@ var remoting = remoting || {};
  * @param {string} clientJid The jid of the WCS client.
  * @param {string} hostPublicKey The base64 encoded version of the host's
  *     public key.
- * @param {string} sharedSecret The access code for IT2Me or the PIN
- *     for Me2Me.
+ * @param {string} passPhrase The pre-determined pass phrase for connection.

[Jamie, 2013/03/18 20:28:23]

I think it's worth keeping the style of the original comment here. In other
words, explain what this parameter is for IT2Me and for Me2Me.

[rmsousa, 2013/03/18 21:58:01]

Done.

+ * @param {function(function(string): void): void} fetchPin Called
+ *     when a PIN needs to be obtained interactively.

[Jamie, 2013/03/18 20:28:23]

I think we should be consistent with referring to PIN vs. pass phrase.
Reintroducing the term PIN in the previous parameter description is probably
sufficient

[rmsousa, 2013/03/18 21:58:01]

I tried to differentiate between a user-generated PIN (Me2Me), and a non-user
pass phrase (It2Me). ut since the latter is only used for IT2Me anyway, we might
as well call it accessCode explicitly.

  * @param {string} authenticationMethods Comma-separated list of
  *     authentication methods the client should attempt to use.
  * @param {string} hostId The host identifier for Me2Me, or empty for IT2Me.
@@ -37,14 +38,16 @@ var remoting = remoting || {};
  * @constructor
  */
 remoting.ClientSession = function(hostJid, clientJid,
-                                  hostPublicKey, sharedSecret,
+                                  hostPublicKey, passPhrase,
+                                  fetchPin,

[Jamie, 2013/03/18 20:28:23]

Nit: I'm not sure what the intended line-break scheme was here. Either
one-per-line or only as needed would be fine, but there's no need for fetchPin
to have a line to itself (hostJid and clientJid are sufficiently closely-related
that they can stay as they are if you prefer).

[rmsousa, 2013/03/18 21:58:01]

Done.

                                   authenticationMethods, hostId, mode) {
   this.state = remoting.ClientSession.State.CREATED;
 
   this.hostJid = hostJid;
   this.clientJid = clientJid;
   this.hostPublicKey = hostPublicKey;
-  this.sharedSecret = sharedSecret;
+  this.passPhrase_ = passPhrase;
+  this.fetchPin_ = fetchPin;

[Jamie, 2013/03/18 20:28:23]

Please add @private annotations to these members.

[rmsousa, 2013/03/18 21:58:01]

Done.

   this.authenticationMethods = authenticationMethods;
   this.hostId = hostId;
   /** @type {remoting.ClientSession.Mode} */
@@ -290,14 +293,19 @@ remoting.ClientSession.prototype.onHostSettingsLoaded_ = function(options) {
         options[remoting.ClientSession.KEY_SHRINK_TO_FIT];
   }
 
-  this.plugin.element().focus();
-
   /** @param {boolean} result */
   this.plugin.initialize(this.onPluginInitialized_.bind(this));
+};
+
+/**
+ * Constrains the focus to the plugin element. 

[Jamie, 2013/03/18 20:28:23]

Add @private

[rmsousa, 2013/03/18 21:58:01]

Done.

+ */
+remoting.ClientSession.prototype.setFocusHandlers_ = function() {
   this.plugin.element().addEventListener(
       'focus', this.callPluginGotFocus_, false);
   this.plugin.element().addEventListener(
       'blur', this.callPluginLostFocus_, false);
+  this.plugin.element().focus();
 };
 
 /**
@@ -588,6 +596,7 @@ remoting.ClientSession.prototype.sendIq_ = function(msg) {
  */
 remoting.ClientSession.prototype.connectPluginToWcs_ = function() {
   remoting.formatIq.setJids(this.clientJid, this.hostJid);
+  /** @type remoting.ClientPlugin */

[Jamie, 2013/03/18 20:28:23]

Brackets around the type, please.

[rmsousa, 2013/03/18 21:58:01]

Done.

   var plugin = this.plugin;
   var forwardIq = plugin.onIncomingIq.bind(plugin);
   /** @param {string} stanza The IQ stanza received. */
@@ -611,8 +620,36 @@ remoting.ClientSession.prototype.connectPluginToWcs_ = function() {
     forwardIq(stanza);
   }
   remoting.wcsSandbox.setOnIq(onIncomingIq);

[Jamie, 2013/03/18 20:28:23]

Blank line here, please.

[rmsousa, 2013/03/18 21:58:01]

Done.

+  // Pass phrase was already supplied before connecting (It2Me case).

[Jamie, 2013/03/18 20:28:23]

Can you move this comment inside the if, for consistency with the else clauses?
Also consider blank lines before the elses for readability.

[rmsousa, 2013/03/18 21:58:01]

Done.

+  if (this.passPhrase_) {
+    this.connectToHost_(this.passPhrase_);
+  } else if (plugin.hasFeature(
+      remoting.ClientPlugin.Feature.USE_ASYNC_PIN_DIALOG)) {
+    // Plugin supports asynchronously asking for passphrase.
+    plugin.useAsyncPinDialog();
+    /** @type remoting.ClientSession */
+    var that = this;
+    var fetchPin = function() {
+      that.fetchPin_(plugin.onPinFetched.bind(plugin));
+    };
+    plugin.fetchPinHandler = fetchPin;
+    this.connectToHost_('');
+  } else {
+    // Plugin doesn't support asynchronously asking for passphrase, ask now.
+    this.fetchPin_(this.connectToHost_.bind(this));
+  }
+};
+
+/**
+ * Connects to the host.
+ *
+ * @private

[Jamie, 2013/03/18 20:28:23]

Nit: We're pretty consistent about @private being at the end of the annotations.

[rmsousa, 2013/03/18 21:58:01]

Done.

+ * @param {string} sharedSecret Shared secret for SPAKE negotiation.
+ * @return {void} Nothing.
+ */
+remoting.ClientSession.prototype.connectToHost_ = function(sharedSecret) {
   this.plugin.connect(this.hostJid, this.hostPublicKey, this.clientJid,
-                      this.sharedSecret, this.authenticationMethods,
+                      sharedSecret, this.authenticationMethods,
                       this.hostId);
 };
 
@@ -627,6 +664,7 @@ remoting.ClientSession.prototype.connectPluginToWcs_ = function() {
 remoting.ClientSession.prototype.onConnectionStatusUpdate_ =
     function(status, error) {
   if (status == remoting.ClientSession.State.CONNECTED) {
+    this.setFocusHandlers_();
     this.onDesktopSizeChanged_();
     if (this.resizeToClient_) {
       this.plugin.notifyClientResolution(window.innerWidth,