Changed identity API to use promises instead of callbacks.

remoting/webapp/crd/js/identity.js

 // Copyright (c) 2012 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.
 
 /**
  * @fileoverview
  * Wrapper class for Chrome's identity API.
  */
 
 'use strict';
@@ skipping 13 matching lines @@
  * @param {remoting.Identity.ConsentDialog=} opt_consentDialog
  * @constructor
  */
 remoting.Identity = function(opt_consentDialog) {
   /** @private */
   this.consentDialog_ = opt_consentDialog;
   /** @type {string} @private */
   this.email_ = '';
   /** @type {string} @private */
   this.fullName_ = '';
-  /** @type {Array.<remoting.Identity.Callbacks>} */
-  this.pendingCallbacks_ = [];
+  /** @type {Promise<string>} */
+  this.pendingPromise_ = null;
 };
 
 /**
  * chrome.identity.getAuthToken should be initiated from user interactions if
  * called with interactive equals true.  This interface prompts a dialog for
  * the user's consent.
  *
  * @interface
  */
 remoting.Identity.ConsentDialog = function() {};
 
 /**
  * @return {Promise} A Promise that resolves when permission to start an
  *   interactive flow is granted.
  */
 remoting.Identity.ConsentDialog.prototype.show = function() {};
 
 /**
- * Call a function with an access token.
+ * Get an access token.
  *
- * @param {function(string):void} onOk Function to invoke with access token if
- *     an access token was successfully retrieved.
- * @param {function(remoting.Error):void} onError Function to invoke with an
- *     error code on failure.
- * @return {void} Nothing.
+ * @return {!Promise<string>} A promise resolved the an access token or
+ *     rejected with a remoting.Error.
  */
-remoting.Identity.prototype.callWithToken = function(onOk, onError) {
-  this.pendingCallbacks_.push(new remoting.Identity.Callbacks(onOk, onError));
-  if (this.pendingCallbacks_.length == 1) {
-    chrome.identity.getAuthToken(
-        { 'interactive': false },
-        this.onAuthComplete_.bind(this, false));
+remoting.Identity.prototype.getToken = function() {
+  /** @const */
+  var that = this;
+
+  if (this.pendingPromise_ == null) {
+    this.pendingPromise_ = new Promise(function(

[kelvinp, 2015/02/18 19:45:24]

I think base.Deferred will work better as you want to return a promise in one
function but resolves/rejects it in other. 

var that = this;
if (!this.getAuthTokenDeferred_) {
  this.getAuthTokenDeferred_ = new base.Deferred();
  chrome.identity.getAuthToken({ interactive: false },
                               that.onAuthComplete.bind(that, false)) 
}

return this.getAuthTokenDeferred_.promise();
... 

In onAuthComplete
getAuthTokenDeferred_.resolves(token) or
getAuthTokenDeferred_.reject(error)

[John Williams, 2015/02/18 20:28:22]

Done.

+        /** function(string) */ resolve,

[kelvinp, 2015/02/18 19:45:24]

Is this a new syntax of annotating parameters? I like it as it seems to be
shorter. However, we use @param elsewhere.  Let's talk about it and we should be
consistent throughout the codebase.

[John Williams, 2015/02/18 20:28:22]

It's a recent addition to the syntax.  I like it for documenting parameter types
for functions that are nested inside another expression because the @param
syntax is very heavy and easy to get wrong.  The syntax used in a lot of the
codebase is actually wrong because when the doc comment doesn't immediately
precede the "function" keyword, the version of jscompiler in git just ignores it
silently.  Newer versions of jscompiler actually report this case as an error,
so when you use the correct syntax and indent according to standard rules, you
get this mess:

  new Promise(
      /**
       * @param {function(string)} resolve
       * @param {function(remoting.Error)} reject
       */
      function(resolve, reject) {
        ...
      });

Since the new syntax is shorter by about 2 lines, and less indented by 4
columns, I greatly prefer it.

As for consistency, I think the new syntax should be preferred when it avoids
excessive indentation, but the old form should be used elsewhere.  I don't think
it's worth changing existing uses except the ones that are actually broken.

+        /** function(remoting.Error) */ reject) {
+      chrome.identity.getAuthToken(
+          { 'interactive': false },
+          that.onAuthComplete_.bind(that, resolve, reject, false));
+    });
   }
+  return this.pendingPromise_;
 };
 
 /**
- * Call a function with a fresh access token.
+ * Get a fresh access token.

[kelvinp, 2015/02/18 19:45:24]

Nit: Gets

[John Williams, 2015/02/18 20:28:22]

Done.

  *
- * @param {function(string):void} onOk Function to invoke with access token if
- *     an access token was successfully retrieved.
- * @param {function(remoting.Error):void} onError Function to invoke with an
- *     error code on failure.
- * @return {void} Nothing.
+ * @return {!Promise<string>} A promise resolved the an access token or

[kelvinp, 2015/02/18 19:45:24]

s/resolved the an/resolved with an/

+ *     rejected with a remoting.Error.
  */
-remoting.Identity.prototype.callWithNewToken = function(onOk, onError) {
+remoting.Identity.prototype.getNewToken = function() {
   /** @type {remoting.Identity} */
   var that = this;
 
-  /**
-   * @param {string} token
-   */
-  function revokeToken(token) {
-    chrome.identity.removeCachedAuthToken(
-        {'token': token },
-        that.callWithToken.bind(that, onOk, onError));
-  }
-
-  this.callWithToken(revokeToken, onError);
+  return this.getToken().then(function(/** string */ token) {
+    return new Promise(function(resolve, reject) {
+      chrome.identity.removeCachedAuthToken({'token': token }, function() {
+        resolve(that.getToken());
+      });
+    });
+  });
 };
 
 /**
  * Remove the cached auth token, if any.
  *
- * @param {function():void=} opt_onDone Completion callback.
- * @return {void} Nothing.
+ * @return {!Promise<null>} A promise resolved with the operation completes.
  */
-remoting.Identity.prototype.removeCachedAuthToken = function(opt_onDone) {
-  var onDone = (opt_onDone) ? opt_onDone : base.doNothing;
-
-  /** @param {string} token */
-  var onToken = function(token) {
-    if (token) {
-      chrome.identity.removeCachedAuthToken({'token': token}, onDone);
-    } else {
-      onDone();
-    }
-  };
-  chrome.identity.getAuthToken({'interactive': false}, onToken);
+remoting.Identity.prototype.removeCachedAuthToken = function() {
+  return new Promise(function(resolve, reject) {
+    /** @param {string} token */
+    var onToken = function(token) {
+      if (token) {
+        chrome.identity.removeCachedAuthToken(
+            {'token': token}, resolve.bind(null, null));
+      } else {
+        resolve(null);
+      }
+    };
+    chrome.identity.getAuthToken({'interactive': false}, onToken);
+  });
 };
 
 /**
- * Get the user's email address and full name.
- * The full name will be null unless the webapp has requested and been
- * granted the userinfo.profile permission.
+ * Get the user's email address and full name.  The full name will be
+ * null unless the webapp has requested and been granted the
+ * userinfo.profile permission.
  *
- * @param {function(string,string):void} onOk Callback invoked when the user's
- *     email address and full name are available.
- * @param {function(remoting.Error):void} onError Callback invoked if an
- *     error occurs.
- * @return {void} Nothing.
+ * TODO(jrw): Type declarations say the name can't be null.  Are the
+ * types wrong, or is the documentation wrong?
+ *
+ * @return {!Promise<{email: string, name: string}>} Promise

[kelvinp, 2015/02/18 19:45:24]

no space between : and type name

[John Williams, 2015/02/18 20:28:22]

Done.

+ *     resolved with the user's email address and full name, or rejected
+ *     with a remoting.Error.
  */
-remoting.Identity.prototype.getUserInfo = function(onOk, onError) {
+remoting.Identity.prototype.getUserInfo = function() {
   if (this.isAuthenticated()) {
-    onOk(this.email_, this.fullName_);
-    return;
+    /**
+     * The temp variable is needed to work around a compiler bug.
+     * @type {{email: string, name: string}}
+     */
+    var result = {email: this.email_, name: this.fullName_};
+    return Promise.resolve(result);
   }
 
   /** @type {remoting.Identity} */
   var that = this;
 
-  /**
-   * @param {string} email
-   * @param {string} name
-   */
-  var onResponse = function(email, name) {
-    that.email_ = email;
-    that.fullName_ = name;
-    onOk(email, name);
-  };
+  return this.getToken().then(function(token) {
+    return new Promise(function(resolve, reject) {
+      /**
+       * @param {string} email
+       * @param {string} name
+       */
+      var onResponse = function(email, name) {
+        that.email_ = email;
+        that.fullName_ = name;
+        resolve({email: email, name: name});
+      };
 
-  this.callWithToken(
-      remoting.oauth2Api.getUserInfo.bind(
-          remoting.oauth2Api, onResponse, onError),
-      onError);
+      remoting.oauth2Api.getUserInfo(onResponse, reject, token);
+    });
+  });
 };
 
 /**
  * Get the user's email address.
  *
- * @param {function(string):void} onOk Callback invoked when the email
- *     address is available.
- * @param {function(remoting.Error):void} onError Callback invoked if an
- *     error occurs.
- * @return {void} Nothing.
+ * @return {!Promise<string>} Promise resolved with the user's email
+ *     address or rejected with a remoting.Error.
  */
-remoting.Identity.prototype.getEmail = function(onOk, onError) {
-  this.getUserInfo(function(email, name) {
-    onOk(email);
-  }, onError);
+remoting.Identity.prototype.getEmail = function() {
+  this.getUserInfo().then(function(userInfo) {
+    return userInfo.email;
+  });
 };
 
 /**
  * Get the user's email address, or null if no successful call to getUserInfo
  * has been made.
  *
  * @return {?string} The cached email address, if available.
  */
 remoting.Identity.prototype.getCachedEmail = function() {
   return this.email_;
 };
 
 /**
  * Get the user's full name.
  * This will return null if either:
  *   No successful call to getUserInfo has been made, or
  *   The webapp doesn't have permission to access this value.
  *
  * @return {?string} The cached user's full name, if available.
  */
 remoting.Identity.prototype.getCachedUserFullName = function() {
   return this.fullName_;
 };
 
 /**
  * Callback for the getAuthToken API.
  *
+ * @param {function(string):void} resolve
+ * @param {function(remoting.Error):void} reject
  * @param {boolean} interactive The value of the "interactive" parameter to
  *     getAuthToken.
  * @param {?string} token The auth token, or null if the request failed.
  * @private
  */
-remoting.Identity.prototype.onAuthComplete_ = function(interactive, token) {
+remoting.Identity.prototype.onAuthComplete_ = function(
+    resolve, reject, interactive, token) {

[kelvinp, 2015/02/18 19:45:24]

As commented earlier, use base.Deferred instead

[John Williams, 2015/02/18 20:28:22]

Done.

   // Pass the token to the callback(s) if it was retrieved successfully.
   if (token) {
-    while (this.pendingCallbacks_.length > 0) {
-      var callback = /** @type {remoting.Identity.Callbacks} */
-          (this.pendingCallbacks_.shift());
-      callback.onOk(token);
-    }
+    resolve(token);
+    this.pendingPromise_ = null;
     return;
   }
 
   // If not, pass an error back to the callback(s) if we've already prompted the
   // user for permission.
   if (interactive) {
     var error_message =
         chrome.runtime.lastError ? chrome.runtime.lastError.message
                                  : 'Unknown error.';
     console.error(error_message);
-    while (this.pendingCallbacks_.length > 0) {
-      var callback = /** @type {remoting.Identity.Callbacks} */
-          (this.pendingCallbacks_.shift());
-      callback.onError(remoting.Error.NOT_AUTHENTICATED);
-    }
+    reject(remoting.Error.NOT_AUTHENTICATED);
     return;
   }
 
   // If there's no token, but we haven't yet prompted for permission, do so
   // now.
   var that = this;
   var showConsentDialog =
       (this.consentDialog_) ? this.consentDialog_.show() : Promise.resolve();
   showConsentDialog.then(function() {
     chrome.identity.getAuthToken({'interactive': true},
-                                 that.onAuthComplete_.bind(that, true));
+                                 that.onAuthComplete_.bind(
+                                     that, resolve, reject, true));

[kelvinp, 2015/02/18 19:45:24]

indentation

[John Williams, 2015/02/18 20:28:22]

Done.

   });
 };
 
 /**
- * Internal representation for pair of callWithToken callbacks.
- *
- * @param {function(string):void} onOk
- * @param {function(remoting.Error):void} onError
- * @constructor
- * @private
- */
-remoting.Identity.Callbacks = function(onOk, onError) {
-  /** @type {function(string):void} */
-  this.onOk = onOk;
-  /** @type {function(remoting.Error):void} */
-  this.onError = onError;
-};
-
-/**
  * Returns whether the web app has authenticated with the Google services.
  *
  * @return {boolean}
  */
 remoting.Identity.prototype.isAuthenticated = function() {
   return remoting.identity.email_ !== '';
 };