Handle ListAccount fetches from within the GaiaCookieManagerService.

components/signin/core/browser/gaia_cookie_manager_service.cc

 // Copyright 2014 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 "components/signin/core/browser/gaia_cookie_manager_service.h"
 
 #include <queue>
 #include <vector>
 
 #include "base/json/json_reader.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "components/signin/core/browser/signin_metrics.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
 namespace {
 
@@ skipping 37 matching lines @@
   ADD_ACCOUNT,
   LOG_OUT_ALL_ACCOUNTS,
   LOG_OUT_ONE_ACCOUNT,
   LIST_ACCOUNTS
 };
 
 }  // namespace
 
 GaiaCookieManagerService::GaiaCookieRequest::GaiaCookieRequest(
     GaiaCookieRequestType request_type,
-    const std::string& account_id,
-    const GaiaCookieManagerService::ListAccountsCallback&
-        list_accounts_callback)
+    const std::string& account_id)
   : request_type_(request_type),
-    account_id_(account_id),
-    list_accounts_callback_(list_accounts_callback) {}
+    account_id_(account_id) {}
 
 GaiaCookieManagerService::GaiaCookieRequest::~GaiaCookieRequest() {
 }
 
 // static
 GaiaCookieManagerService::GaiaCookieRequest
 GaiaCookieManagerService::GaiaCookieRequest::CreateAddAccountRequest(
       const std::string& account_id) {
   return GaiaCookieManagerService::GaiaCookieRequest(
-      GaiaCookieManagerService::GaiaCookieRequestType::ADD_ACCOUNT,
-      account_id,
-      GaiaCookieManagerService::ListAccountsCallback());
+      GaiaCookieManagerService::GaiaCookieRequestType::ADD_ACCOUNT, account_id);
 }
 
 // static
 GaiaCookieManagerService::GaiaCookieRequest
 GaiaCookieManagerService::GaiaCookieRequest::CreateLogOutRequest() {
   return GaiaCookieManagerService::GaiaCookieRequest(
-      GaiaCookieManagerService::GaiaCookieRequestType::LOG_OUT,
-      std::string(),
-      GaiaCookieManagerService::ListAccountsCallback());
+      GaiaCookieManagerService::GaiaCookieRequestType::LOG_OUT, std::string());
 }
 
 GaiaCookieManagerService::GaiaCookieRequest
-GaiaCookieManagerService::GaiaCookieRequest::CreateListAccountsRequest(
-    const GaiaCookieManagerService::ListAccountsCallback&
-        list_accounts_callback) {
+GaiaCookieManagerService::GaiaCookieRequest::CreateListAccountsRequest() {
   return GaiaCookieManagerService::GaiaCookieRequest(
       GaiaCookieManagerService::GaiaCookieRequestType::LIST_ACCOUNTS,
-      std::string(),
-      list_accounts_callback);
+      std::string());
 }
 
 GaiaCookieManagerService::ExternalCcResultFetcher::ExternalCcResultFetcher(
     GaiaCookieManagerService* helper)
     : helper_(helper) {
   DCHECK(helper_);
 }
 
 GaiaCookieManagerService::ExternalCcResultFetcher::~ExternalCcResultFetcher() {
   CleanupTransientState();
@@ skipping 166 matching lines @@
 GaiaCookieManagerService::GaiaCookieManagerService(
     OAuth2TokenService* token_service,
     const std::string& source,
     SigninClient* signin_client)
     : token_service_(token_service),
       signin_client_(signin_client),
       external_cc_result_fetcher_(this),
       gaia_auth_fetcher_backoff_(&kBackoffPolicy),
       gaia_auth_fetcher_retries_(0),
       source_(source),
-      external_cc_result_fetched_(false) {
+      external_cc_result_fetched_(false),
+      list_accounts_fetched_once_(false) {
 }
 
 GaiaCookieManagerService::~GaiaCookieManagerService() {
   CancelAll();
   DCHECK(requests_.empty());
 }
 
+void GaiaCookieManagerService::Init() {
+  cookie_changed_subscription_ = signin_client_->AddCookieChangedCallback(
+      GURL("http://.google.com"),

[Roger Tawa OOO till Jul 10th, 2015/04/15 18:51:06]

A constant very similar to this is also used on chrome_signin_client.cc.  Maybe
this should be moved to GaiaUrls.  Should it be https instead, or in addition
to?

[Mike Lerman, 2015/04/16 13:13:46]

Done. Also, it's supposed to be http not https - the APISID token isn't secure.

+      "APISID",
+      base::Bind(&GaiaCookieManagerService::OnCookieChanged,
+                 base::Unretained(this)));
+}
+
+void GaiaCookieManagerService::Shutdown() {
+  cookie_changed_subscription_.reset();
+}
+
 void GaiaCookieManagerService::AddAccountToCookie(
     const std::string& account_id) {
+  if (!signin_client_->AreSigninCookiesAllowed()) {
+    SignalComplete(account_id,
+        GoogleServiceAuthError(GoogleServiceAuthError::REQUEST_CANCELED));
+    return;
+  }
+
   DCHECK(!account_id.empty());
   VLOG(1) << "GaiaCookieManagerService::AddAccountToCookie: " << account_id;
   requests_.push_back(GaiaCookieRequest::CreateAddAccountRequest(account_id));
   if (requests_.size() == 1)
     StartFetchingUbertoken();
 }
 
-void GaiaCookieManagerService::ListAccounts(
-    const ListAccountsCallback& callback) {
-  // Not implemented yet.
-  NOTREACHED();
+bool GaiaCookieManagerService::ListAccounts(
+    std::vector<std::pair<std::string,bool> >* accounts) {
+  DCHECK(accounts);
+  accounts->assign(listed_accounts_.begin(), listed_accounts_.end());

[Roger Tawa OOO till Jul 10th, 2015/04/15 18:51:06]

Should do this only if the return value is true. Should clean |accounts| if the
return value is false.

[Mike Lerman, 2015/04/16 13:13:46]

Done.

 
-  // TODO(mlerman): Once this service listens to all GAIA cookie changes, cache
-  // the results of ListAccounts, and return them here if the GAIA cookie
-  // hasn't changed since the last call.
+  // There is a fetch currently executing (the results being provided in the
+  // parameter don't align with the fetches that have been started), or the list
+  // of accounts haven't been fetched even once.
+  if (!requests_.empty())
+    return false;
 
-  // If there's a GAIA call being executed, wait for it to complete. If it was
-  // another /ListAccounts then we'll use the results it caches.
-  if (gaia_auth_fetcher_)
-    return;
-
-  VLOG(1) << "GaiaCookieManagerService::ListAccounts";
-  gaia_auth_fetcher_.reset(
-      new GaiaAuthFetcher(this, source_,
-                          signin_client_->GetURLRequestContext()));
-  gaia_auth_fetcher_->StartListAccounts();
+  if (!list_accounts_fetched_once_) {
+    gaia_auth_fetcher_retries_ = 0;
+    requests_.push_back(GaiaCookieRequest::CreateListAccountsRequest());
+    StartFetchingListAccounts();
+    return false;
+  }
+  return true;
 }
 
 void GaiaCookieManagerService::LogOutAllAccounts() {
   VLOG(1) << "GaiaCookieManagerService::LogOutAllAccounts";
 
   bool log_out_queued = false;
   if (!requests_.empty()) {
     // Track requests to keep; all other unstarted requests will be removed.
     std::vector<GaiaCookieRequest> requests_to_keep;
 
@@ skipping 43 matching lines @@
 }
 
 void GaiaCookieManagerService::CancelAll() {
   VLOG(1) << "GaiaCookieManagerService::CancelAll";
   gaia_auth_fetcher_.reset();
   uber_token_fetcher_.reset();
   requests_.clear();
   gaia_auth_fetcher_timer_.Stop();
 }
 
+// It is unknown if the cookie was changed because of processing initiated by
+// this class or other (such as the user clearing all cookies or a cookie being
+// evicted).
+void GaiaCookieManagerService::OnCookieChanged(
+    const net::CanonicalCookie& cookie,
+    bool removed) {
+  DCHECK_EQ("APISID", cookie.Name());
+  DCHECK(GaiaUrls::GetInstance()->gaia_url().host().find(cookie.Domain()) !=
+      std::string::npos);

[Roger Tawa OOO till Jul 10th, 2015/04/15 18:51:06]

nit: use DCHECK_NE ?

[Mike Lerman, 2015/04/16 13:13:46]

Now that I have a GaiaURL for google.com, this is much cleaner.

+  gaia_auth_fetcher_retries_ = 0;
+  if (requests_.empty()) {
+    requests_.push_back(GaiaCookieRequest::CreateListAccountsRequest());
+    StartFetchingListAccounts();
+  } else {
+    // Remove all pending ListAccount calls; for efficiency, only call
+    // after all pending requests are processed.
+    // Track requests to keep; all other unstarted requests will be removed.
+    std::vector<GaiaCookieRequest> requests_to_keep;
+
+    // Check all pending, non-executing requests.
+    for (auto it = requests_.begin() + 1; it != requests_.end(); ++it) {
+      // Keep all requests except for LIST_ACCOUNTS.
+      if (it->request_type() != GaiaCookieRequestType::LIST_ACCOUNTS)
+        requests_to_keep.push_back(*it);
+    }
+
+    // Remove all but the executing request. Re-add all requests being kept.
+    if (requests_.size() > 1) {
+      requests_.erase(requests_.begin() + 1, requests_.end());
+      requests_.insert(
+          requests_.end(), requests_to_keep.begin(), requests_to_keep.end());
+    }
+    requests_.push_back(GaiaCookieRequest::CreateListAccountsRequest());
+  }
+}
+
 void GaiaCookieManagerService::SignalComplete(
     const std::string& account_id,
     const GoogleServiceAuthError& error) {
   // Its possible for the observer to delete |this| object.  Don't access
   // access any members after this calling the observer.  This method should
   // be the last call in any other method.
   FOR_EACH_OBSERVER(Observer, observer_list_,
                     OnAddAccountToCookieCompleted(account_id, error));
 }
 
@@ skipping 32 matching lines @@
           << " account=" << requests_.front().account_id()
           << " error=" << error.ToString();
   const std::string account_id = requests_.front().account_id();
   HandleNextRequest();
   SignalComplete(account_id, error);
 }
 
 void GaiaCookieManagerService::OnMergeSessionSuccess(const std::string& data) {
   VLOG(1) << "MergeSession successful account="
           << requests_.front().account_id();
+  DCHECK(requests_.front().request_type() ==
+         GaiaCookieRequestType::ADD_ACCOUNT);
   const std::string account_id = requests_.front().account_id();
   HandleNextRequest();
   SignalComplete(account_id, GoogleServiceAuthError::AuthErrorNone());
 
   gaia_auth_fetcher_backoff_.InformOfRequest(true);
   uber_token_ = std::string();
 }
 
 void GaiaCookieManagerService::OnMergeSessionFailure(
     const GoogleServiceAuthError& error) {
+  DCHECK(requests_.front().request_type() ==
+         GaiaCookieRequestType::ADD_ACCOUNT);
   VLOG(1) << "Failed MergeSession"
           << " account=" << requests_.front().account_id()
           << " error=" << error.ToString();
-
   if (++gaia_auth_fetcher_retries_ < kMaxGaiaAuthFetcherRetries &&
       IsTransientError(error)) {
     gaia_auth_fetcher_backoff_.InformOfRequest(false);
     gaia_auth_fetcher_timer_.Start(
         FROM_HERE, gaia_auth_fetcher_backoff_.GetTimeUntilRelease(), this,
         &GaiaCookieManagerService::StartFetchingMergeSession);
     return;
   }
 
   uber_token_ = std::string();
   const std::string account_id = requests_.front().account_id();
   HandleNextRequest();
   SignalComplete(account_id, error);
 }
 
+void GaiaCookieManagerService::OnListAccountsSuccess(const std::string& data) {
+  VLOG(1) << "ListAccounts successful";
+  DCHECK(requests_.front().request_type() ==
+         GaiaCookieRequestType::LIST_ACCOUNTS);

[Roger Tawa OOO till Jul 10th, 2015/04/15 18:51:06]

If |requests_.size() > 1| then maybe we should just ignore this result.  We are
about to change the cookie anyway, so might as well wait.  Maybe queue up
another /ListAccounts at the very end if one is not already there?

[Mike Lerman, 2015/04/16 13:13:46]

I'm hesitant if that's the best thing to do. We've already executed the
expensive network request - why not use the result? Also, we may be about to
execute a request which will fail, or is invalid, or otherwise won't change the
cookie. If the cookie ends up changing, then we'll do another ListAccounts to
update the world with The New Truth.

+  gaia_auth_fetcher_backoff_.InformOfRequest(true);
+
+  if (!gaia::ParseListAccountsData(data, &listed_accounts_)) {
+    listed_accounts_.clear();
+    OnListAccountsFailure(GoogleServiceAuthError(
+        GoogleServiceAuthError::UNEXPECTED_SERVICE_RESPONSE));
+    return;
+  }
+
+  list_accounts_fetched_once_ = true;
+  FOR_EACH_OBSERVER(Observer, observer_list_,
+      OnGaiaAccountsInCookieUpdated(
+          listed_accounts_,
+          GoogleServiceAuthError(GoogleServiceAuthError::NONE)));
+  HandleNextRequest();
+}
+
+void GaiaCookieManagerService::OnListAccountsFailure(
+    const GoogleServiceAuthError& error) {
+  VLOG(1) << "ListAccounts failed";
+  DCHECK(requests_.front().request_type() ==
+         GaiaCookieRequestType::LIST_ACCOUNTS);
+  if (++gaia_auth_fetcher_retries_ < kMaxGaiaAuthFetcherRetries &&
+      IsTransientError(error)) {
+    gaia_auth_fetcher_backoff_.InformOfRequest(false);
+    gaia_auth_fetcher_timer_.Start(
+        FROM_HERE, gaia_auth_fetcher_backoff_.GetTimeUntilRelease(), this,
+        &GaiaCookieManagerService::StartFetchingListAccounts);
+    return;
+  }
+
+  HandleNextRequest();
+}
+
 void GaiaCookieManagerService::StartFetchingUbertoken() {
   VLOG(1) << "GaiaCookieManagerService::StartFetching account_id="
           << requests_.front().account_id();
   uber_token_fetcher_.reset(
       new UbertokenFetcher(token_service_, this, source_,
                            signin_client_->GetURLRequestContext()));
   uber_token_fetcher_->StartFetchingToken(requests_.front().account_id());
 }
 
 void GaiaCookieManagerService::StartFetchingMergeSession() {
   DCHECK(!uber_token_.empty());
   gaia_auth_fetcher_.reset(
       new GaiaAuthFetcher(this, source_,
                           signin_client_->GetURLRequestContext()));
 
   gaia_auth_fetcher_->StartMergeSession(uber_token_,
       external_cc_result_fetcher_.GetExternalCcResult());
 }
 
+void GaiaCookieManagerService::StartFetchingListAccounts() {
+  VLOG(1) << "GaiaCookieManagerService::ListAccounts";
+  gaia_auth_fetcher_.reset(
+      new GaiaAuthFetcher(this, source_,
+                          signin_client_->GetURLRequestContext()));
+  gaia_auth_fetcher_->StartListAccounts();
+}
+
 void GaiaCookieManagerService::OnURLFetchComplete(
     const net::URLFetcher* source) {
   DCHECK(requests_.front().request_type() == GaiaCookieRequestType::LOG_OUT);
   VLOG(1) << "GaiaCookieManagerService::OnURLFetchComplete";
   HandleNextRequest();
 }
 
 void GaiaCookieManagerService::HandleNextRequest() {
   VLOG(1) << "GaiaCookieManagerService::HandleNextRequest";
-  requests_.pop_front();
+  if (requests_.front().request_type() ==
+      GaiaCookieRequestType::LIST_ACCOUNTS) {
+    // This and any directly subsequent list accounts would return the same.
+    while (!requests_.empty() && requests_.front().request_type() ==
+           GaiaCookieRequestType::LIST_ACCOUNTS) {
+      requests_.pop_front();
+    }
+  } else {
+    // Pop the completed request.
+    requests_.pop_front();
+  }
+
   gaia_auth_fetcher_.reset();
+  gaia_auth_fetcher_retries_ = 0;
   if (requests_.empty()) {
     VLOG(1) << "GaiaCookieManagerService::HandleNextRequest: no more";
     uber_token_fetcher_.reset();
   } else {
     switch (requests_.front().request_type()) {
       case GaiaCookieRequestType::ADD_ACCOUNT:
         StartFetchingUbertoken();
         break;
       case GaiaCookieRequestType::LOG_OUT:
         StartLogOutUrlFetch();
         break;
       case GaiaCookieRequestType::LIST_ACCOUNTS:
+        uber_token_fetcher_.reset();
+        StartFetchingListAccounts();
         break;
     };
   }
 }