Allow gaia auth fetcher to work with arbitrary clients.

google_apis/gaia/gaia_auth_fetcher.cc

Index: google_apis/gaia/gaia_auth_fetcher.cc
diff --git a/google_apis/gaia/gaia_auth_fetcher.cc b/google_apis/gaia/gaia_auth_fetcher.cc
index c807e139ede3b7e2c06c3b62b36267534b65e69f..50f4916ae6da2e6e21284912a44c15846c39c81d 100644
--- a/google_apis/gaia/gaia_auth_fetcher.cc
+++ b/google_apis/gaia/gaia_auth_fetcher.cc
@@ -183,7 +183,8 @@ GaiaAuthFetcher::GaiaAuthFetcher(GaiaAuthConsumer* consumer,
       oauth2_iframe_url_(GaiaUrls::GetInstance()->oauth2_iframe_url()),
       client_login_to_oauth2_gurl_(
           GaiaUrls::GetInstance()->client_login_to_oauth2_url()),
-      fetch_pending_(false) {
+      fetch_pending_(false),
+      fetch_token_from_auth_code_(false) {

[xiyuan, 2016/01/27 22:51:29]

nit: use the non-static class member initializers and also update
fetch_pending_.

[khmel, 2016/01/27 23:16:13]

Done.

 }
 
 GaiaAuthFetcher::~GaiaAuthFetcher() {}
@@ -526,14 +527,25 @@ void GaiaAuthFetcher::StartCookieForOAuthLoginTokenExchange(
 void GaiaAuthFetcher::StartCookieForOAuthLoginTokenExchangeWithDeviceId(
     const std::string& session_index,
     const std::string& device_id) {
+  StartCookieForOAuthLoginTokenExchangeWithClientIdAndDeviceId(
+      true,
+      session_index,
+      GaiaUrls::GetInstance()->oauth2_chrome_client_id(),
+      device_id);
+}
+
+void GaiaAuthFetcher::StartCookieForOAuthLoginTokenExchangeWithClientIdAndDeviceId(

[xiyuan, 2016/01/27 22:51:30]

nit: The name is insane. Can we call it StartCookieForOAuthLoginTokenExchange
and rely on function overloading?

[khmel, 2016/01/27 23:16:13]

Agree :)
But I followed style used here. BTW, I tried to rename function above too
(StartCookieForOAuthLoginTokenExchangeWithDeviceId). But it is used in legacy
code. And if we rename StartCookieForOAuthLoginTokenExchangeWithDeviceId we need
to rename several function here ending with DeviceId as well which will cause a
massive change. I think renaming of newly added function is fine.

[xiyuan, 2016/01/27 23:19:31]

Understand. I would try to follow existing convention as well. But this name is
inhumane. :p

I am okay to leave StartCookieForOAuthLoginTokenExchangeWithDeviceId for another
day.

+    bool fetch_token_from_auth_code,
+    const std::string& session_index,
+    const std::string& client_id,
+    const std::string& device_id) {
   DCHECK(!fetch_pending_) << "Tried to fetch two things at once!";
 
   VLOG(1) << "Starting OAuth login token fetch with cookie jar";
 
   std::string encoded_scope = net::EscapeUrlEncodedData(
       GaiaConstants::kOAuth1LoginScope, true);
-  std::string encoded_client_id = net::EscapeUrlEncodedData(
-      GaiaUrls::GetInstance()->oauth2_chrome_client_id(), true);
+  std::string encoded_client_id = net::EscapeUrlEncodedData(client_id, true);
   std::string query_string =
       base::StringPrintf(kClientLoginToOAuth2URLFormat, encoded_scope.c_str(),
                          encoded_client_id.c_str());
@@ -548,6 +560,7 @@ void GaiaAuthFetcher::StartCookieForOAuthLoginTokenExchangeWithDeviceId(
         base::StringPrintf(kDeviceIdHeaderFormat, device_id.c_str());
   }
 
+  fetch_token_from_auth_code_ = fetch_token_from_auth_code;
   CreateAndStartGaiaFetcher(std::string(), device_id_header,
                             client_login_to_oauth2_gurl_.Resolve(query_string),
                             net::LOAD_NORMAL);
@@ -738,7 +751,16 @@ void GaiaAuthFetcher::OnClientLoginToOAuth2Fetched(
   if (status.is_success() && response_code == net::HTTP_OK) {
     std::string auth_code;
     if (ParseClientLoginToOAuth2Response(cookies, &auth_code)) {
-      StartAuthCodeForOAuth2TokenExchange(auth_code);
+      // In case fetch_token_from_auth_code_ is not set OnClientOAuthCode
+      // callback for consumer_ must be last call in context of this object
+      // because consumer may delete GaiaAuthFetcher after getting the final
+      // result.
+      if (fetch_token_from_auth_code_) {
+        consumer_->OnClientOAuthCode(auth_code);

[xiyuan, 2016/01/27 22:51:30]

I wonder whether we should call this. The auth code would be used to exchange
for RT in this case. We probably should not pass it out and let consumer to mess
with it.

[khmel, 2016/01/27 23:16:13]

Yes, this would be better.

+        StartAuthCodeForOAuth2TokenExchange(auth_code);
+      } else {
+        consumer_->OnClientOAuthCode(auth_code);
+      }
     } else {
       GoogleServiceAuthError auth_error(
           GoogleServiceAuthError::FromUnexpectedServiceResponse(