Improve web-app type safety.

remoting/webapp/me2mom/remoting.js

Index: remoting/webapp/me2mom/remoting.js
diff --git a/remoting/webapp/me2mom/remoting.js b/remoting/webapp/me2mom/remoting.js
index e7c549681dd8f4f6703aab9e07876bc6056c8422..1c7cceb027c47da3262efe4361d6237aed19ae93 100644
--- a/remoting/webapp/me2mom/remoting.js
+++ b/remoting/webapp/me2mom/remoting.js
@@ -5,18 +5,24 @@
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
-(function() {
-'use strict';
+/**
+ * Whether or not the plugin should scale itself.
+ * @type {boolean}
+ */
+remoting.scaleToFit = false;
 
-window.addEventListener('blur', pluginLostFocus_, false);
+/** @type {remoting.ClientSession} */
+remoting.session = null;
 
-function pluginLostFocus_() {
-  // If the plug loses input focus, release all keys as a precaution against
-  // leaving them 'stuck down' on the host.
-  if (remoting.session && remoting.session.plugin) {
-    remoting.session.plugin.releaseAllKeys();
-  }
-}
+/** @type {string} */ remoting.accessCode = '';
+/** @type {number} */ remoting.accessCodeTimerId = 0;
+/** @type {number} */ remoting.accessCodeExpiresIn = 0;
+/** @type {remoting.AppMode} */ remoting.currentMode;
+/** @type {string} */ remoting.hostJid = '';
+/** @type {string} */ remoting.hostPublicKey = '';
+/** @type {boolean} */ remoting.lastShareWasCancelled = false;
+/** @type {boolean} */ remoting.timerRunning = false;
+/** @type {string} */ remoting.username = '';

[Jamie, 2011/10/18 00:19:27]

This while file needs refactoring, but for now just pre-declaring these globals
make the intent more explicit that having them created at first use.

 
 /** @enum {string} */
 remoting.AppMode = {
@@ -36,7 +42,20 @@ remoting.AppMode = {
   IN_SESSION: 'in-session'
 };
 
-remoting.EMAIL = 'email';

[Jamie, 2011/10/18 00:19:27]

This is no longer used.

+(function() {
+'use strict';

[simonmorris, 2011/10/18 00:55:36]

Is it better to have 'use strict' at the top of the file?

[Jamie, 2011/10/18 01:42:01]

It doesn't matter, but yes, probably. Done.

+
+window.addEventListener('blur', pluginLostFocus_, false);
+
+function pluginLostFocus_() {
+  // If the plug loses input focus, release all keys as a precaution against
+  // leaving them 'stuck down' on the host.
+  if (remoting.session && remoting.session.plugin) {
+    remoting.session.plugin.releaseAllKeys();
+  }
+}
+
+/** @type {string} */
 remoting.HOST_PLUGIN_ID = 'host-plugin-id';
 
 /** @enum {string} */
@@ -50,27 +69,31 @@ remoting.ClientError = {
   OTHER_ERROR: /*i18n-content*/'ERROR_GENERIC'
 };
 
-/**
- * Whether or not the plugin should scale itself.
- * @type {boolean}
- */
-remoting.scaleToFit = false;
-
 // Constants representing keys used for storing persistent application state.
 var KEY_APP_MODE_ = 'remoting-app-mode';
 var KEY_EMAIL_ = 'remoting-email';
 var KEY_USE_P2P_API_ = 'remoting-use-p2p-api';
 
 // Some constants for pretty-printing the access code.
-var kSupportIdLen = 7;
-var kHostSecretLen = 5;
-var kAccessCodeLen = kSupportIdLen + kHostSecretLen;
-var kDigitsPerGroup = 4;
+/** @type {number} */ var kSupportIdLen = 7;
+/** @type {number} */ var kHostSecretLen = 5;
+/** @type {number} */ var kAccessCodeLen = kSupportIdLen + kHostSecretLen;
+/** @type {number} */ var kDigitsPerGroup = 4;
 
+/**
+ * @param {string} classes A space-separated list of classes.
+ * @param {string} cls The class to check for.
+ * @return {boolean} True if |cls| is found within |classes|.
+ */
 function hasClass(classes, cls) {
-  return classes.match(new RegExp('(\\s|^)' + cls + '(\\s|$)'));
+  return classes.match(new RegExp('(\\s|^)' + cls + '(\\s|$)')) != null;
 }
 
+/**
+ * @param {Element} element The element to which to add the class.
+ * @param {string} cls The new class.
+ * @return {void} Nothing.
+ */
 function addClass(element, cls) {
   if (!hasClass(element.className, cls)) {
     var padded = element.className == '' ? '' : element.className + ' ';
@@ -78,6 +101,11 @@ function addClass(element, cls) {
   }
 }
 
+/**
+ * @param {Element} element The element from which to remove the class.
+ * @param {string} cls The new class.
+ * @return {void} Nothing.
+ */
 function removeClass(element, cls) {
   element.className =
       element.className.replace(new RegExp('\\b' + cls + '\\b', 'g'), '')
@@ -89,6 +117,7 @@ function retrieveEmail_(access_token) {
     'Authorization': 'OAuth ' + remoting.oauth2.getAccessToken()
   };
 
+  /** @param {XMLHttpRequest} xhr The XHR response. */
   var onResponse = function(xhr) {
     if (xhr.status != 200) {
       // TODO(ajwong): Have a better way of showing an error.
@@ -112,6 +141,10 @@ function refreshEmail_() {
   }
 }
 
+/**
+ * @param {string} value The email address to place in local storage.
+ * @return {void} Nothing.
+ */
 function setEmail(value) {
   window.localStorage.setItem(KEY_EMAIL_, value);
   document.getElementById('current-email').innerText = value;
@@ -129,9 +162,9 @@ function exchangedCodeForToken_() {
   if (!remoting.oauth2.isAuthenticated()) {
     alert('Your OAuth2 token was invalid. Please try again.');
   }
-  remoting.oauth2.callWithToken(function(token) {
-      retrieveEmail_(token);
-  });
+  /** @param {string} token The auth token. */
+  var retrieveEmail = function(token) { retrieveEmail_(token); }
+  remoting.oauth2.callWithToken(retrieveEmail);
 }
 
 remoting.clearOAuth2 = function() {
@@ -193,7 +226,7 @@ remoting.setMode = function(mode) {
     modes[i] = modes[i - 1] + '.' + modes[i];
   var elements = document.querySelectorAll('[data-ui-mode]');
   for (var i = 0; i < elements.length; ++i) {
-    var element = elements[i];
+    /** @type {Element} */ var element = elements[i];
     var hidden = true;
     for (var m = 0; m < modes.length; ++m) {
       if (hasClass(element.getAttribute('data-ui-mode'), modes[m])) {
@@ -214,7 +247,7 @@ remoting.setMode = function(mode) {
 
 /**
  * Get the major mode that the app is running in.
- * @return {remoting.AppMode} The app's current major mode.
+ * @return {string} The app's current major mode.
  */
 remoting.getMajorMode = function() {
   return remoting.currentMode.split('.')[0];
@@ -308,6 +341,11 @@ remoting.updateAccessCodeTimeoutElement_ = function() {
   }
 }
 
+/**
+ * Callback to show or hide the NAT traversal warning when the policy changes.
+ * @param {boolean} enabled True if NAT traversal is enabled.
+ * @return {void} Nothing.
+ */
 function onNatTraversalPolicyChanged_(enabled) {
   var container = document.getElementById('nat-box-container');
   container.hidden = enabled;
@@ -414,7 +452,8 @@ remoting.cancelShare = function() {
   try {
     plugin.disconnect();
   } catch (error) {
-    remoting.debug.log('Error disconnecting: ' + error.description +
+    var tmp = /** @type {{description: string}} */ error;

[Jamie, 2011/10/18 00:19:27]

This is horrible, but I couldn't find any other way of adding type annotations
to an exception.

[simonmorris, 2011/10/18 00:55:36]

It's OK - maybe rename tmp to errorTyped, to make your
intention clearer?

[Jamie, 2011/10/18 01:42:01]

Done.

+    remoting.debug.log('Error disconnecting: ' + tmp.description +
                        '. The host plugin probably crashed.');
     // TODO(jamiewalch): Clean this up. We should have a class representing
     // the host plugin, like we do for the client, which should handle crash
@@ -483,6 +522,7 @@ function showToolbarPreview_() {
   window.setTimeout(removeClass, 3000, toolbar, 'toolbar-preview');
 }
 
+/** @param {number} oldState The previous state of the plugin. */
 function onClientStateChange_(oldState) {
   if (!remoting.session) {
     // If the connection has been cancelled, then we no longer have a reference
@@ -552,11 +592,13 @@ function startSession_() {
       new remoting.ClientSession(remoting.hostJid, remoting.hostPublicKey,
                                  remoting.accessCode, remoting.username,
                                  onClientStateChange_);
-  remoting.oauth2.callWithToken(function(token) {
+  /** @param {string} token The auth token. */
+  var createPluginAndConnect = function(token) {
     remoting.session.createPluginAndConnect(
         document.getElementById('session-mode'),
         token);
-  });
+  };
+  remoting.oauth2.callWithToken(createPluginAndConnect);
 }
 
 /**
@@ -586,8 +628,9 @@ function parseServerResponse_(xhr) {
   remoting.supportHostsXhr = null;
   remoting.debug.log('parseServerResponse: status = ' + xhr.status);
   if (xhr.status == 200) {
-    var host = JSON.parse(xhr.responseText);
-    if (host.data && host.data.jabberId) {
+    var host = /** @type {{data: {jabberId: string, publicKey: string}}} */
+        JSON.parse(xhr.responseText);
+    if (host.data && host.data.jabberId && host.data.publicKey) {

[Jamie, 2011/10/18 00:19:27]

I've updated the test to match the type cast above so that it's safe.

[simonmorris, 2011/10/18 00:55:36]

OK.

       remoting.hostJid = host.data.jabberId;
       remoting.hostPublicKey = host.data.publicKey;
       var split = remoting.hostJid.split('/');
@@ -607,12 +650,15 @@ function parseServerResponse_(xhr) {
   showConnectError_(errorMsg);
 }
 
+/** @param {string} accessCode The access code, as entered by the user.
+ *  @return {string} The normalized form of the code (whitespace removed). */
 function normalizeAccessCode_(accessCode) {
   // Trim whitespace.
   // TODO(sergeyu): Do we need to do any other normalization here?
   return accessCode.replace(/\s/g, '');
 }
 
+/** @param {string} supportId The canonicalized support ID. */
 function resolveSupportId(supportId) {
   var headers = {
     'Authorization': 'OAuth ' + remoting.oauth2.getAccessToken()
@@ -647,11 +693,13 @@ remoting.tryConnectWithAccessToken = function() {
   if (!remoting.wcsLoader) {
     remoting.wcsLoader = new remoting.WcsLoader();
   }
+  /** @param {function(string):void} setToken The callback function. */
+  var callWithToken = function(setToken) {
+    remoting.oauth2.callWithToken(setToken);
+  };
   remoting.wcsLoader.start(
       remoting.oauth2.getAccessToken(),
-      function(setToken) {
-        remoting.oauth2.callWithToken(setToken);
-      },
+      callWithToken,
       remoting.tryConnectWithWcs);
 }
 
@@ -769,6 +817,10 @@ remoting.promptClose = function() {
   }
 }
 
+/**
+ * @param {Event} event The keyboard event.
+ * @return {void} Nothing.
+ */
 remoting.checkHotkeys = function(event) {
   if (String.fromCharCode(event.which) == 'D') {
     remoting.toggleDebugLog();