Fixed callWithToken error handling.

Fixed callWithToken error handling.

BUG=124898
TEST=Manual

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

[Jamie, 2012-04-25 01:20:47]

callWithToken was previously just throwing an exception if the token refresh
failed, resulting in the callback not being invoked. I've changed it so that the
callback is invoked with a null token in this case, and updated all call sites
to handle this correctly.

https://chromiumcodereview.appspot.com/10207028/diff/1/remoting/webapp/host_c...
File remoting/webapp/host_controller.js (right):

https://chromiumcodereview.appspot.com/10207028/diff/1/remoting/webapp/host_c...
remoting/webapp/host_controller.js:211:
callback(remoting.HostController.AsyncResult.FAILED);
We could probably use a more specific error code here, but it's unlikely that
this is the first OAuth failure the user has seen--the host list refresh will
probably fail first and that now has a very clear error message.

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

http://codereview.chromium.org/10207028/diff/1/remoting/webapp/host_controlle...
File remoting/webapp/host_controller.js (right):

http://codereview.chromium.org/10207028/diff/1/remoting/webapp/host_controlle...
remoting/webapp/host_controller.js:211:
callback(remoting.HostController.AsyncResult.FAILED);
On 2012/04/25 01:20:48, Jamie wrote:
> We could probably use a more specific error code here, but it's unlikely that
> this is the first OAuth failure the user has seen--the host list refresh will
> probably fail first and that now has a very clear error message.

Maybe a TODO to add a more specific error code?

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

http://codereview.chromium.org/10207028/diff/1/remoting/webapp/oauth2.js#newc...
remoting/webapp/oauth2.js:347: myfunc(this.getAccessToken());
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]

http://codereview.chromium.org/10207028/diff/1/remoting/webapp/host_controlle...
File remoting/webapp/host_controller.js (right):

http://codereview.chromium.org/10207028/diff/1/remoting/webapp/host_controlle...
remoting/webapp/host_controller.js:211:
callback(remoting.HostController.AsyncResult.FAILED);
On 2012/04/25 16:59:41, simonmorris wrote:
> On 2012/04/25 01:20:48, Jamie wrote:
> > We could probably use a more specific error code here, but it's unlikely
that
> > this is the first OAuth failure the user has seen--the host list refresh
will
> > probably fail first and that now has a very clear error message.
> 
> Maybe a TODO to add a more specific error code?

Done.

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

http://codereview.chromium.org/10207028/diff/1/remoting/webapp/oauth2.js#newc...
remoting/webapp/oauth2.js:347: myfunc(this.getAccessToken());
On 2012/04/25 16:59:41, simonmorris wrote:
> 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?

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

[simonmorris, 2012-04-25 18:12:34]

lgtm