Cleaned up OAuth refresh error handling.

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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147023

[Jamie, 2012-07-17 00:25:37]

https://chromiumcodereview.appspot.com/10704240/diff/1/remoting/webapp/oauth2.js
File remoting/webapp/oauth2.js (left):

https://chromiumcodereview.appspot.com/10704240/diff/1/remoting/webapp/oauth2...
remoting/webapp/oauth2.js:189: remoting.OAuth2.prototype.getAccessToken_ =
function() {
Only used in two places, one of which was a potential time-related bug as
mentioned in the CL description. Now inlined in callWithToken.

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

http://codereview.chromium.org/10704240/diff/1/remoting/webapp/oauth2.js
File remoting/webapp/oauth2.js (right):

http://codereview.chromium.org/10704240/diff/1/remoting/webapp/oauth2.js#newc...
remoting/webapp/oauth2.js:227: onDone(xhr, tokens['access_token']);
Is 'tokens' in scope here? What happens if xhr.status isn't 200?

[Jamie, 2012-07-17 01:14:10]

ptal

http://codereview.chromium.org/10704240/diff/1/remoting/webapp/oauth2.js
File remoting/webapp/oauth2.js (right):

http://codereview.chromium.org/10704240/diff/1/remoting/webapp/oauth2.js#newc...
remoting/webapp/oauth2.js:227: onDone(xhr, tokens['access_token']);
On 2012/07/17 00:52:50, simonmorris wrote:
> Is 'tokens' in scope here? What happens if xhr.status isn't 200?

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

[simonmorris, 2012-07-17 01:25:32]

http://codereview.chromium.org/10704240/diff/4001/remoting/webapp/oauth2.js
File remoting/webapp/oauth2.js (right):

http://codereview.chromium.org/10704240/diff/4001/remoting/webapp/oauth2.js#n...
remoting/webapp/oauth2.js:194: * @param {function(XMLHttpRequest, string): void}
onDone Callback to invoke on
Doesn't "string" need to be something like "string?" if the parameter can be
null? And shouldn't the prototypes in the processTokenResponse_ call sites also
be changed?

[Jamie, 2012-07-17 01:51:57]

ptal

http://codereview.chromium.org/10704240/diff/4001/remoting/webapp/oauth2.js
File remoting/webapp/oauth2.js (right):

http://codereview.chromium.org/10704240/diff/4001/remoting/webapp/oauth2.js#n...
remoting/webapp/oauth2.js:194: * @param {function(XMLHttpRequest, string): void}
onDone Callback to invoke on
On 2012/07/17 01:25:33, simonmorris wrote:
> Doesn't "string" need to be something like "string?" if the parameter can be
> null? And shouldn't the prototypes in the processTokenResponse_ call sites
also
> be changed?

I've changed the default to '', since that minimizes the amount of casting
required.

Regarding the other call sites, I think they're fine as they are: if they don't
use the access token parameter, it's fine for them not to declare it.

[simonmorris, 2012-07-17 15:32:38]

lgtm