Allow gaia auth fetcher to work with arbitrary clients.

google_apis/gaia/gaia_auth_fetcher.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 
 #include <string>
 #include <utility>
 #include <vector>
 
@@ skipping 165 matching lines @@
           base::StringPrintf(kUberAuthTokenURLFormat, source.c_str()))),
       oauth_login_gurl_(GaiaUrls::GetInstance()->oauth1_login_url()),
       list_accounts_gurl_(
           GaiaUrls::GetInstance()->ListAccountsURLWithSource(source)),
       logout_gurl_(GaiaUrls::GetInstance()->LogOutURLWithSource(source)),
       get_check_connection_info_url_(
           GaiaUrls::GetInstance()->GetCheckConnectionInfoURLWithSource(source)),
       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() {}
 
 bool GaiaAuthFetcher::HasPendingFetch() {
   return fetch_pending_;
 }
 
 void GaiaAuthFetcher::SetPendingFetch(bool pending_fetch) {
   fetch_pending_ = pending_fetch;
@@ skipping 322 matching lines @@
 
 void GaiaAuthFetcher::StartCookieForOAuthLoginTokenExchange(
     const std::string& session_index) {
   StartCookieForOAuthLoginTokenExchangeWithDeviceId(session_index,
                                                     std::string());
 }
 
 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());
   if (!device_id.empty())
     query_string += "&device_type=chrome";
   if (!session_index.empty())
     query_string += "&authuser=" + session_index;
 
   std::string device_id_header;
   if (!device_id.empty()) {
     device_id_header =
         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);
 }
 
 void GaiaAuthFetcher::StartAuthCodeForOAuth2TokenExchange(
     const std::string& auth_code) {
   StartAuthCodeForOAuth2TokenExchangeWithDeviceId(auth_code, std::string());
 }
 
@@ skipping 170 matching lines @@
 }
 
 void GaiaAuthFetcher::OnClientLoginToOAuth2Fetched(
     const std::string& data,
     const net::ResponseCookies& cookies,
     const net::URLRequestStatus& status,
     int response_code) {
   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(
               "ClientLogin response cookies didn't contain an auth code"));
       consumer_->OnClientOAuthFailure(auth_error);
     }
   } else {
     GoogleServiceAuthError auth_error(GenerateAuthError(data, status));
     consumer_->OnClientOAuthFailure(auth_error);
   }
@@ skipping 231 matching lines @@
   return alleged_error.find(kSecondFactor) !=
       std::string::npos;
 }
 
 // static
 bool GaiaAuthFetcher::IsWebLoginRequiredSuccess(
     const std::string& alleged_error) {
   return alleged_error.find(kWebLoginRequired) !=
       std::string::npos;
 }