Fixed callWithToken error handling.

remoting/webapp/oauth2.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
  * OAuth2 class that handles retrieval/storage of an OAuth2 token.
  *
  * Uses a content script to trampoline the OAuth redirect page back into the
  * extension context.  This works around the lack of native support for
@@ skipping 321 matching lines @@
 };
 
 /**
  * Call myfunc with an access token as the only parameter.
  *
  * This will refresh the access token if necessary.  If the access token
  * cannot be refreshed, an error is thrown.
  *
  * The access token will remain valid for at least 2 minutes.
  *
- * @param {function(string):void} myfunc
- *        Function to invoke with access token.
+ * @param {function(string?):void} myfunc Function to invoke with access token.
  * @return {void} Nothing.
  */
 remoting.OAuth2.prototype.callWithToken = function(myfunc) {
-  /** @type {remoting.OAuth2} */
-  var that = this;
-  if (remoting.oauth2.needsNewAccessToken()) {
-    remoting.oauth2.refreshAccessToken(function() {
-      if (remoting.oauth2.needsNewAccessToken()) {
-        // If we still need it, we're going to infinite loop.
-        throw 'Unable to get access token.';
+  try {
+    myfunc(this.getAccessToken());

[simonmorris, 2012/04/25 16:59:41]

If myfunc throws an exception, the following code will wrongly refresh the
access token. It'd be clearer to explicitly check whether a refresh is needed,
as the existing code does. Is there any reason to change the existing code so
much in this CL?

[Jamie, 2012/04/25 17:29:09]

As discussed off-line, it's makes the test less racy, but I'll follow-up with a
CL to do that more comprehensively.

+  } catch (err) {
+    /** @type {remoting.OAuth2} */
+    var that = this;
+    var onRefresh = function() {
+      try {
+        myfunc(that.getAccessToken());
+      } catch (err) {
+        myfunc(null);
       }
-      myfunc(that.getAccessToken());
-    });
-    return;
+    };
+    remoting.oauth2.refreshAccessToken(onRefresh);
   }
-
-  myfunc(this.getAccessToken());
 };
 
 /**
  * Get the user's email address.
  *
  * @param {function(?string):void} setEmail Callback invoked when the email
  *     address is available, or on error.
  * @return {void} Nothing.
  */
 remoting.OAuth2.prototype.getEmail = function(setEmail) {
   /** @type {remoting.OAuth2} */
   var that = this;
   /** @param {XMLHttpRequest} xhr The XHR response. */
   var onResponse = function(xhr) {
     that.email = null;
     if (xhr.status == 200) {
       // TODO(ajwong): See if we can't find a JSON endpoint.
       that.email = xhr.responseText.split('&')[0].split('=')[1];
     }
     window.localStorage.setItem(that.KEY_EMAIL_, that.email);
     setEmail(that.email);
   };
 
-  /** @param {string} token The access token. */
+  /** @param {string?} token The access token. */
   var getEmailFromToken = function(token) {
-    var headers = { 'Authorization': 'OAuth ' + token };
-    // TODO(ajwong): Update to new v2 API.
-    remoting.xhr.get('https://www.googleapis.com/userinfo/email',
-                     onResponse, '', headers);
+    if (token) {
+      var headers = { 'Authorization': 'OAuth ' + token };
+      // TODO(ajwong): Update to new v2 API.
+      remoting.xhr.get('https://www.googleapis.com/userinfo/email',
+                       onResponse, '', headers);
+    } else {
+      setEmail(null);
+    }
   };
 
   this.callWithToken(getEmailFromToken);
 };
 
 /**
  * If the user's email address is cached, return it, otherwise return null.
  *
  * @return {?string} The email address, if it has been cached by a previous call
  *     to getEmail, otherwise null.
  */
 remoting.OAuth2.prototype.getCachedEmail = function() {
   var value = window.localStorage.getItem(this.KEY_EMAIL_);
   if (typeof value == 'string') {
     return value;
   }
   return null;
 };