[ChromeOS] Make GaiaAutheFetcher parsing logic default to service_unavailable

[ChromeOS] Make GaiaAutheFetcher parsing logic default to service_unavailable

The logic that parses responses from Google Accounts servers was
defaulting to INVALID_GAIA_CREDENTIALS, which indicate a successful
communication with teh backend that resulted in a failing
authentication attempt.  Gibberish was thus interpreted this way.  We
should instead interpret gibberish as an unsuccessful communication
attempt.

BUG=chromium-os:8943
TEST=unit tests, installed on device and ran successful login and failed login autotests.  Also tried live login.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=68104

[John Gregg, 2010-12-01 00:58:15]

Can you also try this on the linux_sync try bot (and other trybots)?

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
File chrome/common/net/gaia/gaia_auth_fetcher.cc (right):

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
chrome/common/net/gaia/gaia_auth_fetcher.cc:360:
GoogleServiceAuthError::SERVICE_UNAVAILABLE);
Neither one of these is natural as the default.  If we are going to change the
default now, perhaps it's an opportunity to add "UNKNOWN_ERROR".

[Chris Masone, 2010-12-01 16:51:26]

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
File chrome/common/net/gaia/gaia_auth_fetcher.cc (right):

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
chrome/common/net/gaia/gaia_auth_fetcher.cc:360:
GoogleServiceAuthError::SERVICE_UNAVAILABLE);
On 2010/12/01 00:58:15, John Gregg wrote:
> Neither one of these is natural as the default.  If we are going to change the
> default now, perhaps it's an opportunity to add "UNKNOWN_ERROR".

I dunno...it seems to me that what this means is we can't successfully talk to
the service, so SERVICE_UNAVAILABLE makes sense.  I don't know that using
UNKNOWN_ERROR would allow us to show a more useful error to the user in this
case.  I suppose we could add UNKNOWN_ERROR for logging purposes, or to allow
callers of this API to distinguish these cases, but conflate this error with
something like SERVICE_UNAVAILABLE for UI messaging purposes?

[Chris Masone, 2010-12-02 18:56:00]

ping?

On 2010/12/01 16:51:26, Chris Masone wrote:
>
http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
> File chrome/common/net/gaia/gaia_auth_fetcher.cc (right):
> 
>
http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
> chrome/common/net/gaia/gaia_auth_fetcher.cc:360:
> GoogleServiceAuthError::SERVICE_UNAVAILABLE);
> On 2010/12/01 00:58:15, John Gregg wrote:
> > Neither one of these is natural as the default.  If we are going to change
the
> > default now, perhaps it's an opportunity to add "UNKNOWN_ERROR".
> 
> I dunno...it seems to me that what this means is we can't successfully talk to
> the service, so SERVICE_UNAVAILABLE makes sense.  I don't know that using
> UNKNOWN_ERROR would allow us to show a more useful error to the user in this
> case.  I suppose we could add UNKNOWN_ERROR for logging purposes, or to allow
> callers of this API to distinguish these cases, but conflate this error with
> something like SERVICE_UNAVAILABLE for UI messaging purposes?

[John Gregg, 2010-12-02 19:02:16]

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
File chrome/common/net/gaia/gaia_auth_fetcher.cc (right):

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
chrome/common/net/gaia/gaia_auth_fetcher.cc:360:
GoogleServiceAuthError::SERVICE_UNAVAILABLE);
On 2010/12/01 16:51:26, Chris Masone wrote:
> On 2010/12/01 00:58:15, John Gregg wrote:
> > Neither one of these is natural as the default.  If we are going to change
the
> > default now, perhaps it's an opportunity to add "UNKNOWN_ERROR".
> 
> I dunno...it seems to me that what this means is we can't successfully talk to
> the service, so SERVICE_UNAVAILABLE makes sense.  I don't know that using
> UNKNOWN_ERROR would allow us to show a more useful error to the user in this
> case.  I suppose we could add UNKNOWN_ERROR for logging purposes, or to allow
> callers of this API to distinguish these cases, but conflate this error with
> something like SERVICE_UNAVAILABLE for UI messaging purposes?

I was thinking SERVICE_UNAVAILABLE implies "try again later" where as this
incomprensible response means something is more seriously wrong, like we're
talking to a different version of the server.

But this is probably fine.

[Chris Masone, 2010-12-02 22:59:49]

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
File chrome/common/net/gaia/gaia_auth_fetcher.cc (right):

http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
chrome/common/net/gaia/gaia_auth_fetcher.cc:360:
GoogleServiceAuthError::SERVICE_UNAVAILABLE);
On 2010/12/02 19:02:16, John Gregg wrote:
> On 2010/12/01 16:51:26, Chris Masone wrote:
> > On 2010/12/01 00:58:15, John Gregg wrote:
> > > Neither one of these is natural as the default.  If we are going to change
> the
> > > default now, perhaps it's an opportunity to add "UNKNOWN_ERROR".
> > 
> > I dunno...it seems to me that what this means is we can't successfully talk
to
> > the service, so SERVICE_UNAVAILABLE makes sense.  I don't know that using
> > UNKNOWN_ERROR would allow us to show a more useful error to the user in this
> > case.  I suppose we could add UNKNOWN_ERROR for logging purposes, or to
allow
> > callers of this API to distinguish these cases, but conflate this error with
> > something like SERVICE_UNAVAILABLE for UI messaging purposes?
> 
> I was thinking SERVICE_UNAVAILABLE implies "try again later" where as this
> incomprensible response means something is more seriously wrong, like we're
> talking to a different version of the server.
> 
> But this is probably fine.

We've seen this in a captive portal situation as well, so really... "try again
later" is the only thing the system can do in this case, kinda no matter what.

Given that, and the trybot run...how do you feel about the change?

[John Gregg, 2010-12-02 23:01:24]

On 2010/12/02 22:59:49, Chris Masone wrote:
>
http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
> File chrome/common/net/gaia/gaia_auth_fetcher.cc (right):
> 
>
http://codereview.chromium.org/5309007/diff/1/chrome/common/net/gaia/gaia_aut...
> chrome/common/net/gaia/gaia_auth_fetcher.cc:360:
> GoogleServiceAuthError::SERVICE_UNAVAILABLE);
> On 2010/12/02 19:02:16, John Gregg wrote:
> > On 2010/12/01 16:51:26, Chris Masone wrote:
> > > On 2010/12/01 00:58:15, John Gregg wrote:
> > > > Neither one of these is natural as the default.  If we are going to
change
> > the
> > > > default now, perhaps it's an opportunity to add "UNKNOWN_ERROR".
> > > 
> > > I dunno...it seems to me that what this means is we can't successfully
talk
> to
> > > the service, so SERVICE_UNAVAILABLE makes sense.  I don't know that using
> > > UNKNOWN_ERROR would allow us to show a more useful error to the user in
this
> > > case.  I suppose we could add UNKNOWN_ERROR for logging purposes, or to
> allow
> > > callers of this API to distinguish these cases, but conflate this error
with
> > > something like SERVICE_UNAVAILABLE for UI messaging purposes?
> > 
> > I was thinking SERVICE_UNAVAILABLE implies "try again later" where as this
> > incomprensible response means something is more seriously wrong, like we're
> > talking to a different version of the server.
> > 
> > But this is probably fine.
> 
> We've seen this in a captive portal situation as well, so really... "try again
> later" is the only thing the system can do in this case, kinda no matter what.
> 
> Given that, and the trybot run...how do you feel about the change?

LGTM