This CL fixes a couple of related things:

* The linked bug, caused by the use of try...catch to check for the "not signed in" case.
* A couple of theoretical bugs caused by the use of a time-sensitive "is valid" method followed by an accessor that throws if it not valid.

It also cleans up the interface by enforcing stricter use of @private.

BUG=137613
TEST=Manual

Review URL: https://chromiumcodereview.appspot.com/10704240

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@147023 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 21 insertions, 31 deletions

diff --git a/remoting/webapp/oauth2.js b/remoting/webapp/oauth2.js
index ece2619..50bc38b 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() {
-  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,12 +191,14 @@ 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.
  */
 remoting.OAuth2.prototype.processTokenResponse_ = function(onDone, xhr) {
+  /** @type {string} */
+  var accessToken = '';
   if (xhr.status == 200) {
     try {
       // Don't use jsonParseSafe here unless you move the definition out of
@@ -225,7 +216,8 @@ remoting.OAuth2.prototype.processTokenResponse_ = function(onDone, xhr) {
       // The choice of 2 minutes is arbitrary, but that length of time
       // is part of the contract satisfied by callWithToken().
       // Offset by a further 30 seconds to account for RTT issues.
-      this.setAccessToken(tokens['access_token'],
+      accessToken = /** @type {string} */ (tokens['access_token']);
+      this.setAccessToken(accessToken,
           (tokens['expires_in'] - (120 + 30)) * 1000 + Date.now());
     } catch (err) {
       console.error('Invalid "token" response from server:',
@@ -235,7 +227,7 @@ remoting.OAuth2.prototype.processTokenResponse_ = function(onDone, xhr) {
     console.error('Failed to get tokens. Status: ' + xhr.status +
                   ' response: ' + xhr.responseText);
   }
-  onDone(xhr);
+  onDone(xhr, accessToken);
 };
 
 /**
@@ -330,11 +322,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 +332,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 +351,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 =