Cleaned up OAuth refresh error handling.

remoting/webapp/oauth2.js

Index: remoting/webapp/oauth2.js
diff --git a/remoting/webapp/oauth2.js b/remoting/webapp/oauth2.js
index ece2619959398c7e9ce323a97687d89377fc8c4b..e5cee10d0de600b106ecb28182940e779736c222 100644
--- a/remoting/webapp/oauth2.js
+++ b/remoting/webapp/oauth2.js
@@ -62,7 +62,7 @@ remoting.OAuth2.prototype.isAuthenticated = function() {
  */
 remoting.OAuth2.prototype.clear = function() {
   window.localStorage.removeItem(this.KEY_EMAIL_);
-  this.clearAccessToken();
+  this.clearAccessToken_();
   this.clearRefreshToken_();
 };
 
@@ -78,7 +78,7 @@ remoting.OAuth2.prototype.clear = function() {
 remoting.OAuth2.prototype.setRefreshToken = function(token) {
   window.localStorage.setItem(this.KEY_REFRESH_TOKEN_, escape(token));
   window.localStorage.setItem(this.KEY_REFRESH_TOKEN_REVOKABLE_, true);
-  this.clearAccessToken();
+  this.clearAccessToken_();
 };
 
 /**
@@ -163,8 +163,9 @@ remoting.OAuth2.prototype.getAccessTokenInternal_ = function() {
  * Will throw if !isAuthenticated().
  *
  * @return {boolean} True if a new access token is needed.
+ * @private
  */
-remoting.OAuth2.prototype.needsNewAccessToken = function() {
+remoting.OAuth2.prototype.needsNewAccessToken_ = function() {
   if (!this.isAuthenticated()) {
     throw 'Not Authenticated.';
   }
@@ -179,22 +180,10 @@ remoting.OAuth2.prototype.needsNewAccessToken = function() {
 };
 
 /**
- * Returns the current access token.
- *
- * Will throw if !isAuthenticated() or needsNewAccessToken().
- *
- * @return {string} The access token.
+ * @return {void} Nothing.
  * @private
  */
-remoting.OAuth2.prototype.getAccessToken_ = function() {

[Jamie, 2012/07/17 00:25:38]

Only used in two places, one of which was a potential time-related bug as
mentioned in the CL description. Now inlined in callWithToken.

-  if (this.needsNewAccessToken()) {
-    throw 'Access Token expired.';
-  }
-  return this.getAccessTokenInternal_()['token'];
-};
-
-/** @return {void} Nothing. */
-remoting.OAuth2.prototype.clearAccessToken = function() {
+remoting.OAuth2.prototype.clearAccessToken_ = function() {
   window.localStorage.removeItem(this.KEY_ACCESS_TOKEN_);
 };
 
@@ -202,7 +191,7 @@ remoting.OAuth2.prototype.clearAccessToken = function() {
  * Update state based on token response from the OAuth2 /token endpoint.
  *
  * @private
- * @param {function(XMLHttpRequest): void} onDone Callback to invoke on
+ * @param {function(XMLHttpRequest, string): void} onDone Callback to invoke on
  *     completion.
  * @param {XMLHttpRequest} xhr The XHR object for this request.
  * @return {void} Nothing.
@@ -235,7 +224,7 @@ remoting.OAuth2.prototype.processTokenResponse_ = function(onDone, xhr) {
     console.error('Failed to get tokens. Status: ' + xhr.status +
                   ' response: ' + xhr.responseText);
   }
-  onDone(xhr);
+  onDone(xhr, tokens['access_token']);

[simonmorris, 2012/07/17 00:52:50]

Is 'tokens' in scope here? What happens if xhr.status isn't 200?

[Jamie, 2012/07/17 01:14:11]

You're right, it will fail. Good catch!

 };
 
 /**
@@ -330,11 +319,7 @@ remoting.OAuth2.prototype.revokeToken_ = function(token) {
 };
 
 /**
- * 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.
- *
+ * Call a function with an access token, refreshing it first if necessary.
  * The access token will remain valid for at least 2 minutes.
  *
  * @param {function(string):void} onOk Function to invoke with access token if
@@ -344,13 +329,13 @@ remoting.OAuth2.prototype.revokeToken_ = function(token) {
  * @return {void} Nothing.
  */
 remoting.OAuth2.prototype.callWithToken = function(onOk, onError) {
-  try {
-    if (this.needsNewAccessToken()) {
+  if (this.isAuthenticated()) {
+    if (this.needsNewAccessToken_()) {
       this.refreshAccessToken_(this.onRefreshToken_.bind(this, onOk, onError));
     } else {
-      onOk(this.getAccessToken_());
+      onOk(this.getAccessTokenInternal_()['token']);
     }
-  } catch (error) {
+  } else {
     onError(remoting.Error.NOT_AUTHENTICATED);
   }
 };
@@ -363,12 +348,14 @@ remoting.OAuth2.prototype.callWithToken = function(onOk, onError) {
  * @param {function(remoting.Error):void} onError Function to invoke with an
  *     error code on failure.
  * @param {XMLHttpRequest} xhr The result of the refresh operation.
+ * @param {string} accessToken The fresh access token.
  * @private
  */
-remoting.OAuth2.prototype.onRefreshToken_ = function(onOk, onError, xhr) {
+remoting.OAuth2.prototype.onRefreshToken_ = function(onOk, onError, xhr,
+                                                     accessToken) {
   var error = remoting.Error.UNEXPECTED;
   if (xhr.status == 200) {
-    onOk(this.getAccessToken_());
+    onOk(accessToken);
     return;
   } else if (xhr.status == 400) {
     var result =