ChromeOS: Use OAuth2 refresh token to download GAIA info

ChromeOS: Use OAuth2 refresh token to download GAIA info

BUG=91241
TEST=

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

[Ivan Korotkov, 2011-11-30 11:21:58]

I'm gonna test if CrOS is susceptible to the 24hrs bug. If it is, I suggest
submitting it before the branch point.

http://codereview.chromium.org/8742008/diff/1/chrome/browser/net/gaia/token_s...
File chrome/browser/net/gaia/token_service.cc (right):

http://codereview.chromium.org/8742008/diff/1/chrome/browser/net/gaia/token_s...
chrome/browser/net/gaia/token_service.cc:24: const char*
TokenService::kServices[] = {
You should update kNumServices in .h as well, otherwise it segfaults.

[Munjal (Google), 2011-11-30 19:29:29]

http://codereview.chromium.org/8742008/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_downloader.cc (left):

http://codereview.chromium.org/8742008/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_downloader.cc:219: } else {
In else you don't want to just wait for token available or failed notification.
That will put you into an infinite wait for users who are already logged in
pre-M17 (at least in Chrome, not sure about ChromeOS).

If the oauth login token is not available there are two cases:
1. The user was already logged in.
2. The user has not logged in yet.

If it is okay for this code to wait for a long time until the user logs in
again, then I guess it is ok.

[sail, 2011-12-01 20:01:49]

http://codereview.chromium.org/8742008/diff/1/chrome/browser/net/gaia/token_s...
File chrome/browser/net/gaia/token_service.cc (right):

http://codereview.chromium.org/8742008/diff/1/chrome/browser/net/gaia/token_s...
chrome/browser/net/gaia/token_service.cc:24: const char*
TokenService::kServices[] = {
On 2011/11/30 11:21:58, Ivan Korotkov wrote:
> You should update kNumServices in .h as well, otherwise it segfaults.

Done.

http://codereview.chromium.org/8742008/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_downloader.cc (left):

http://codereview.chromium.org/8742008/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_downloader.cc:219: } else {
On 2011/11/30 19:29:29, munjal wrote:
> In else you don't want to just wait for token available or failed
notification.
> That will put you into an infinite wait for users who are already logged in
> pre-M17 (at least in Chrome, not sure about ChromeOS).
> 
> If the oauth login token is not available there are two cases:
> 1. The user was already logged in.
> 2. The user has not logged in yet.
> 
> If it is okay for this code to wait for a long time until the user logs in
> again, then I guess it is ok.

Ahh, you're right. I'm not sure if this is a problem for ChromeOS but it's
definitely a problem for the browser code.

The old code actually had the same problem. It would wait forever for M16 users
who never got the picasa token. If you don't mind I'd like to fix this in a
separate change.

[Munjal (Google), 2011-12-01 23:25:58]

LGTM for integration with the oauth2 fetcher.

http://codereview.chromium.org/8742008/diff/1/chrome/browser/profiles/profile...
File chrome/browser/profiles/profile_downloader.cc (left):

http://codereview.chromium.org/8742008/diff/1/chrome/browser/profiles/profile...
chrome/browser/profiles/profile_downloader.cc:219: } else {
On 2011/12/01 20:01:49, sail wrote:
> On 2011/11/30 19:29:29, munjal wrote:
> > In else you don't want to just wait for token available or failed
> notification.
> > That will put you into an infinite wait for users who are already logged in
> > pre-M17 (at least in Chrome, not sure about ChromeOS).
> > 
> > If the oauth login token is not available there are two cases:
> > 1. The user was already logged in.
> > 2. The user has not logged in yet.
> > 
> > If it is okay for this code to wait for a long time until the user logs in
> > again, then I guess it is ok.
> 
> Ahh, you're right. I'm not sure if this is a problem for ChromeOS but it's
> definitely a problem for the browser code.
> 
> The old code actually had the same problem. It would wait forever for M16
users
> who never got the picasa token. If you don't mind I'd like to fix this in a
> separate change.

Sure, you can fix it in a separate CL.

[Ivan Korotkov, 2011-12-02 14:16:39]

Works on CrOS. LGTM

[commit-bot: I haz the power, 2011-12-02 16:32:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/8742008/5001

[commit-bot: I haz the power, 2011-12-02 16:32:34]

Presubmit check for 8742008-5001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
chrome/browser/net/gaia/token_service.h,chrome/browser/net/gaia/token_service.cc

Presubmit checks took 1.4s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Ivan Korotkov, 2011-12-02 16:36:15]

Zel, can you take a look at token_service changes?
We don't need the Picasa token now as OAuth2 refresh token is used instead.

[sail, 2011-12-05 18:53:34]

rickcam, rogerta, sanjeevr, tim, zelidrag: chrome/browser/net/gaia OWNERS review

[zel, 2011-12-05 20:57:14]

LGTM

[commit-bot: I haz the power, 2011-12-05 20:57:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/8742008/5001

[Roger Tawa OOO till Jul 10th, 2011-12-05 20:58:06]

lgtm for changes in net/gaia.

[commit-bot: I haz the power, 2011-12-05 23:54:30]

Can't apply patch for file chrome/browser/net/gaia/token_service.h.
While running patch -p1 --forward --force;
patching file chrome/browser/net/gaia/token_service.h
Hunk #1 FAILED at 249.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/net/gaia/token_service.h.rej

[commit-bot: I haz the power, 2011-12-06 01:38:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/8742008/11001