Integrate OAuth2 login token generation into TokenService:

Integrate OAuth2 login token generation into TokenService:
- Start Oauth2 login token fetch when "lso" auth token is fetched successfully.
- Store the refresh and access login tokens.
- Generate a token available notification for login token.
- Add tests.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=111882

[Rick Campbell, 2011-11-28 22:51:07]

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
File chrome/browser/net/gaia/token_service.cc (right):

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
chrome/browser/net/gaia/token_service.cc:251: bool
TokenService::HasOAuthLoginToken() const {
See comment in header.  I recommend removing these.

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
chrome/browser/net/gaia/token_service.cc:303: if (service ==
GaiaConstants::kLSOService) {
Is there another way to accomplish this?  Splicing in an explicit test here is
troubling to me.  I would be happier if this could be done by code that already
handles the issuance of the LSOService token.

At a minimum, this should be documented inline, but hopefully this logic can
live elsewhere.

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
File chrome/browser/net/gaia/token_service.h (right):

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
chrome/browser/net/gaia/token_service.h:158: virtual bool HasOAuthLoginToken()
const;
I think that these should not be singled out for special access, but rather,
users of this class can call HasTokenForService and/or GetTokenForService with
the service identifier as they do for other services.

[Munjal (Google), 2011-11-28 23:41:14]

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
File chrome/browser/net/gaia/token_service.cc (right):

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
chrome/browser/net/gaia/token_service.cc:251: bool
TokenService::HasOAuthLoginToken() const {
On 2011/11/28 22:51:07, Rick Campbell wrote:
> See comment in header.  I recommend removing these.

Replied there.

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
chrome/browser/net/gaia/token_service.cc:303: if (service ==
GaiaConstants::kLSOService) {
On 2011/11/28 22:51:07, Rick Campbell wrote:
> Is there another way to accomplish this?  Splicing in an explicit test here is
> troubling to me.  I would be happier if this could be done by code that
already
> handles the issuance of the LSOService token.
> 
> At a minimum, this should be documented inline, but hopefully this logic can
> live elsewhere.

I thought about that but decided that this was the best option right now since
the fact that creating an oauth login scoped token depends on "lso" service
token is implementation detail which calelrs should not worry about.

I added a comment here.

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
File chrome/browser/net/gaia/token_service.h (right):

http://codereview.chromium.org/8680019/diff/5001/chrome/browser/net/gaia/toke...
chrome/browser/net/gaia/token_service.h:158: virtual bool HasOAuthLoginToken()
const;
On 2011/11/28 22:51:07, Rick Campbell wrote:
> I think that these should not be singled out for special access, but rather,
> users of this class can call HasTokenForService and/or GetTokenForService with
> the service identifier as they do for other services.

These methods are more like OAuth1 methods and less like client login tokens for
each service.

Also, methods and code to create/store tokens for other OAuth scopes are not
added yet.

[Rick Campbell, 2011-11-28 23:56:12]

LGTM