[Signin] Pass externalCcResult to multilogin requests
Pass externalCcResult in the multilogin params to make sure it correctly
sets cookies for secondary domains.
This had conflicts with https://crrev.com/c/1572131 which were resolved
manually.
TBR=bsazonov@chromium.org
(cherry picked from commit 0aec7c82277205ddd4bf8e8a3a6604fe186d5019)
Bug: 971393, b/133516269
Change-Id: Iab7d6779e8137fd677e19929113464aa3acd9be6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1658572
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#669320}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660334
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3770@{#1029}
Cr-Branched-From: a9eee1c7c727ef42a15d86e9fa7b77ff0e63840a-refs/heads/master@{#652427}
diff --git a/components/signin/core/browser/gaia_cookie_manager_service.cc b/components/signin/core/browser/gaia_cookie_manager_service.cc
index ff0011b..93fd888 100644
--- a/components/signin/core/browser/gaia_cookie_manager_service.cc
+++ b/components/signin/core/browser/gaia_cookie_manager_service.cc
@@ -261,9 +261,11 @@
return base::JoinString(results, ",");
}
-void GaiaCookieManagerService::ExternalCcResultFetcher::Start() {
+void GaiaCookieManagerService::ExternalCcResultFetcher::Start(
+ base::OnceClosure callback) {
DCHECK(!helper_->external_cc_result_fetched_);
m_external_cc_result_start_time_ = base::Time::Now();
+ callback_ = std::move(callback);
CleanupTransientState();
results_.clear();
@@ -453,11 +455,7 @@
time_to_check_connections);
helper_->external_cc_result_fetched_ = true;
- // Since the ExternalCCResultFetcher is only Started in place of calling
- // StartFetchingMergeSession, we can assume we need to call
- // StartFetchingMergeSession. If this assumption becomes invalid, a Callback
- // will need to be passed to Start() and Run() here.
- helper_->StartFetchingMergeSession();
+ std::move(callback_).Run();
}
GaiaCookieManagerService::GaiaCookieManagerService(
@@ -808,7 +806,9 @@
if (!external_cc_result_fetched_ &&
!external_cc_result_fetcher_.IsRunning()) {
- external_cc_result_fetcher_.Start();
+ external_cc_result_fetcher_.Start(
+ base::BindOnce(&GaiaCookieManagerService::StartFetchingMergeSession,
+ weak_ptr_factory_.GetWeakPtr()));
return;
}
@@ -1064,6 +1064,15 @@
DCHECK_EQ(SET_ACCOUNTS, requests_.front().request_type());
VLOG(1) << "GaiaCookieManagerService::StartFetchingAccessToken account_id ="
<< base::JoinString(requests_.front().account_ids(), " ");
+
+ if (!external_cc_result_fetched_ &&
+ !external_cc_result_fetcher_.IsRunning()) {
+ external_cc_result_fetcher_.Start(base::BindOnce(
+ &GaiaCookieManagerService::StartFetchingAccessTokensForMultilogin,
+ weak_ptr_factory_.GetWeakPtr()));
+ return;
+ }
+
token_requests_.clear();
access_tokens_.clear();
for (const std::string& account_id : requests_.front().account_ids()) {
@@ -1091,10 +1100,12 @@
void GaiaCookieManagerService::StartFetchingMultiLogin(
const std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>& accounts) {
+ DCHECK(external_cc_result_fetched_);
gaia_auth_fetcher_ = signin_client_->CreateGaiaAuthFetcher(
this, requests_.front().source(), GetURLLoaderFactory());
- gaia_auth_fetcher_->StartOAuthMultilogin(accounts);
+ gaia_auth_fetcher_->StartOAuthMultilogin(
+ accounts, external_cc_result_fetcher_.GetExternalCcResult());
}
void GaiaCookieManagerService::StartFetchingMergeSession() {
diff --git a/components/signin/core/browser/gaia_cookie_manager_service.h b/components/signin/core/browser/gaia_cookie_manager_service.h
index 854d00a..8073b55 100644
--- a/components/signin/core/browser/gaia_cookie_manager_service.h
+++ b/components/signin/core/browser/gaia_cookie_manager_service.h
@@ -184,7 +184,7 @@
// Start fetching the external CC result. If a fetch is already in progress
// it is canceled.
- void Start();
+ void Start(base::OnceClosure callback);
// Are external URLs still being checked?
bool IsRunning();
@@ -221,6 +221,7 @@
LoaderToToken loaders_;
ResultMap results_;
base::Time m_external_cc_result_start_time_;
+ base::OnceClosure callback_;
DISALLOW_COPY_AND_ASSIGN(ExternalCcResultFetcher);
};
diff --git a/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc b/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
index 04b96f42..2b71d611 100644
--- a/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
+++ b/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
@@ -1372,7 +1372,9 @@
InstrumentedGaiaCookieManagerService helper(token_service(), signin_client());
GaiaCookieManagerService::ExternalCcResultFetcher result_fetcher(&helper);
EXPECT_CALL(helper, StartFetchingMergeSession());
- result_fetcher.Start();
+ result_fetcher.Start(base::BindOnce(
+ &InstrumentedGaiaCookieManagerService::StartFetchingMergeSession,
+ base::Unretained(&helper)));
// Simulate a successful completion of GetCheckConnctionInfo.
SimulateGetCheckConnectionInfoSuccess(
@@ -1397,7 +1399,9 @@
InstrumentedGaiaCookieManagerService helper(token_service(), signin_client());
GaiaCookieManagerService::ExternalCcResultFetcher result_fetcher(&helper);
EXPECT_CALL(helper, StartFetchingMergeSession());
- result_fetcher.Start();
+ result_fetcher.Start(base::BindOnce(
+ &InstrumentedGaiaCookieManagerService::StartFetchingMergeSession,
+ base::Unretained(&helper)));
// Simulate a successful completion of GetCheckConnctionInfo.
SimulateGetCheckConnectionInfoSuccess(
@@ -1426,7 +1430,9 @@
InstrumentedGaiaCookieManagerService helper(token_service(), signin_client());
GaiaCookieManagerService::ExternalCcResultFetcher result_fetcher(&helper);
EXPECT_CALL(helper, StartFetchingMergeSession());
- result_fetcher.Start();
+ result_fetcher.Start(base::BindOnce(
+ &InstrumentedGaiaCookieManagerService::StartFetchingMergeSession,
+ base::Unretained(&helper)));
// Simulate a successful completion of GetCheckConnctionInfo.
SimulateGetCheckConnectionInfoSuccess(
@@ -1468,7 +1474,9 @@
TEST_F(GaiaCookieManagerServiceTest, UbertokenSuccessFetchesExternalCCOnce) {
InstrumentedGaiaCookieManagerService helper(token_service(), signin_client());
- helper.external_cc_result_fetcher_for_testing()->Start();
+ helper.external_cc_result_fetcher_for_testing()->Start(base::BindOnce(
+ &InstrumentedGaiaCookieManagerService::StartFetchingMergeSession,
+ base::Unretained(&helper)));
EXPECT_CALL(helper, StartFetchingUbertoken());
helper.AddAccountToCookie(
diff --git a/google_apis/gaia/gaia_auth_fetcher.cc b/google_apis/gaia/gaia_auth_fetcher.cc
index 52da1db8..dd258f076 100644
--- a/google_apis/gaia/gaia_auth_fetcher.cc
+++ b/google_apis/gaia/gaia_auth_fetcher.cc
@@ -751,7 +751,8 @@
}
void GaiaAuthFetcher::StartOAuthMultilogin(
- const std::vector<MultiloginTokenIDPair>& accounts) {
+ const std::vector<MultiloginTokenIDPair>& accounts,
+ const std::string& external_cc_result) {
DCHECK(!fetch_pending_) << "Tried to fetch two things at once!";
UMA_HISTOGRAM_COUNTS_100("Signin.Multilogin.NumberOfAccounts",
@@ -767,8 +768,13 @@
kOAuthMultiBearerHeaderFormat,
base::JoinString(authorization_header_parts, ",").c_str());
- std::string parameters = base::StringPrintf(
- "?source=%s", net::EscapeUrlEncodedData(source_, true).c_str());
+ std::string source_string = net::EscapeUrlEncodedData(source_, true);
+ std::string parameters =
+ external_cc_result.empty()
+ ? base::StringPrintf("?source=%s", source_string.c_str())
+ : base::StringPrintf(
+ "?source=%s&externalCcResult=%s", source_string.c_str(),
+ net::EscapeUrlEncodedData(external_cc_result, true).c_str());
net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("gaia_auth_multilogin", R"(
diff --git a/google_apis/gaia/gaia_auth_fetcher.h b/google_apis/gaia/gaia_auth_fetcher.h
index a5d2bd6..f375b22 100644
--- a/google_apis/gaia/gaia_auth_fetcher.h
+++ b/google_apis/gaia/gaia_auth_fetcher.h
@@ -159,7 +159,8 @@
const std::string& service);
// Starts a request to get the cookie for list of accounts.
- void StartOAuthMultilogin(const std::vector<MultiloginTokenIDPair>& accounts);
+ void StartOAuthMultilogin(const std::vector<MultiloginTokenIDPair>& accounts,
+ const std::string& external_cc_result);
// Starts a request to list the accounts in the GAIA cookie.
void StartListAccounts();
diff --git a/google_apis/gaia/gaia_auth_fetcher_unittest.cc b/google_apis/gaia/gaia_auth_fetcher_unittest.cc
index eb0d4c9..a73e9e6 100644
--- a/google_apis/gaia/gaia_auth_fetcher_unittest.cc
+++ b/google_apis/gaia/gaia_auth_fetcher_unittest.cc
@@ -394,7 +394,7 @@
TestGaiaAuthFetcher auth(&consumer, GetURLLoaderFactory());
auth.StartOAuthMultilogin(
- std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>());
+ std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>(), std::string());
EXPECT_TRUE(auth.HasPendingFetch());
auth.TestOnURLLoadCompleteInternal(net::OK, net::HTTP_OK, {},
@@ -429,7 +429,7 @@
TestGaiaAuthFetcher auth(&consumer, GetURLLoaderFactory());
auth.StartOAuthMultilogin(
- std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>());
+ std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>(), std::string());
EXPECT_TRUE(auth.HasPendingFetch());
auth.TestOnURLLoadCompleteInternal(net::ERR_ABORTED, net::HTTP_OK, {},
@@ -464,7 +464,7 @@
TestGaiaAuthFetcher auth(&consumer, GetURLLoaderFactory());
auth.StartOAuthMultilogin(
- std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>());
+ std::vector<GaiaAuthFetcher::MultiloginTokenIDPair>(), std::string());
EXPECT_TRUE(auth.HasPendingFetch());
auth.TestOnURLLoadCompleteInternal(net::OK, net::HTTP_OK, {},