Part 1 of work to do user sign in based on OAuth2:

Part 1 of work to do user sign in based on OAuth2:
- Add a couple of more flow methods to GaiaAuthFetcher.
- Add a new fetcher class to get a new access token from refresh token.
- Full set o tests.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=111270

[Rick Campbell, 2011-11-22 21:32:57]

Mostly good.  I'm open to discussion on the naming, but I do think it would be
good to explicitly distinguish the OAuth2 stuff given the number of ClientLogin
and OAuth1 identifiers the live in or nearby this code.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
File chrome/common/net/gaia/gaia_auth_fetcher.h (right):

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:120: // The format of the POST body
to get OAuth2 auth code from auth token.
Just because there are so many similar names and overlapping name-parts, I'd be
inclined to explicitly bake OAuth2 into this name ... perhaps
kGetOAuth2CodeBodyFormat, etc.?

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:147: // Constants for
request/response for OAtuh2 requests.
Similarly here, I'd like to see OAuth2 in the name just to help future
developers who may need to slice and dice this a bit.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:166: void OnGetAuthCodeFetched(const
std::string& data,
Maybe OnGetOAuth2CodeFetched?

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:273: const GURL get_auth_code_gurl_;
Even here, I'd say maybe get_oauth2_code_gurl.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/oauth...
File chrome/common/net/gaia/oauth2_access_token_fetcher.h (right):

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/oauth...
chrome/common/net/gaia/oauth2_access_token_fetcher.h:53: virtual void
OnURLFetchComplete(const content::URLFetcher* source);
Add OVERRIDE directive

[Munjal (Google), 2011-11-22 22:05:58]

All done. Please take another look.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
File chrome/common/net/gaia/gaia_auth_fetcher.h (right):

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:120: // The format of the POST body
to get OAuth2 auth code from auth token.
On 2011/11/22 21:32:57, Rick Campbell wrote:
> Just because there are so many similar names and overlapping name-parts, I'd
be
> inclined to explicitly bake OAuth2 into this name ... perhaps
> kGetOAuth2CodeBodyFormat, etc.?

Done.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:147: // Constants for
request/response for OAtuh2 requests.
On 2011/11/22 21:32:57, Rick Campbell wrote:
> Similarly here, I'd like to see OAuth2 in the name just to help future
> developers who may need to slice and dice this a bit.

Done.

Note that I did not rename kAuthHeaderFormat on purpose since that is not OAuth2
specific.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:166: void OnGetAuthCodeFetched(const
std::string& data,
On 2011/11/22 21:32:57, Rick Campbell wrote:
> Maybe OnGetOAuth2CodeFetched?

Done.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:273: const GURL get_auth_code_gurl_;
On 2011/11/22 21:32:57, Rick Campbell wrote:
> Even here, I'd say maybe get_oauth2_code_gurl.

Done.

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/oauth...
File chrome/common/net/gaia/oauth2_access_token_fetcher.h (right):

http://codereview.chromium.org/8632005/diff/5005/chrome/common/net/gaia/oauth...
chrome/common/net/gaia/oauth2_access_token_fetcher.h:53: virtual void
OnURLFetchComplete(const content::URLFetcher* source);
On 2011/11/22 21:32:57, Rick Campbell wrote:
> Add OVERRIDE directive

Done.

[Rick Campbell, 2011-11-22 22:22:52]

LGTM with two nits

http://codereview.chromium.org/8632005/diff/9001/chrome/common/net/gaia/gaia_...
File chrome/common/net/gaia/gaia_auth_fetcher.h (right):

http://codereview.chromium.org/8632005/diff/9001/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:147: // Constants for
request/response for OAtuh2 requests.
typo OAtuh2 --> OAuth2

http://codereview.chromium.org/8632005/diff/9001/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:258: bool use_cookies,
Nit -- this is called send_cookies in the source file and use_cookies in the
header.

[Munjal (Google), 2011-11-22 22:29:24]

http://codereview.chromium.org/8632005/diff/9001/chrome/common/net/gaia/gaia_...
File chrome/common/net/gaia/gaia_auth_fetcher.h (right):

http://codereview.chromium.org/8632005/diff/9001/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:147: // Constants for
request/response for OAtuh2 requests.
On 2011/11/22 22:22:53, Rick Campbell wrote:
> typo OAtuh2 --> OAuth2

Done.

http://codereview.chromium.org/8632005/diff/9001/chrome/common/net/gaia/gaia_...
chrome/common/net/gaia/gaia_auth_fetcher.h:258: bool use_cookies,
On 2011/11/22 22:22:53, Rick Campbell wrote:
> Nit -- this is called send_cookies in the source file and use_cookies in the
> header.

Done.