Skip to content

Commit 590bda6

Browse files
committed
Merge-70 [AF] Get account from PDM instead of IdentityManager
Actually Two changes bundled here 1) Get account from Personal data manager instead of IdentityManager 2) (Small helper method) Personal datamanager changes to denote fullsync user or not that can be propagated to rest of the Autofill module. Goal is to send it to payment RPCs. [email protected] (cherry picked from commit 7ba5ea8) Bug: 880861, 840703 Change-Id: I61422018c9fe6755f7315ccc692ab6245ed754da Reviewed-on: https://chromium-review.googlesource.com/1184030 Commit-Queue: Sebastien Seguin-Gagnon <[email protected]> Reviewed-by: Mathieu Perreault <[email protected]> Reviewed-by: Sebastien Seguin-Gagnon <[email protected]> Reviewed-by: David Roger <[email protected]> Reviewed-by: Marc Treib <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#588608} Reviewed-on: https://chromium-review.googlesource.com/1208137 Cr-Commit-Position: refs/branch-heads/3538@{#67} Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
1 parent 557dccd commit 590bda6

24 files changed

+186
-44
lines changed

chrome/browser/ui/views/payments/cvc_unmask_view_controller.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "components/autofill/core/browser/autofill_client.h"
2020
#include "components/autofill/core/browser/credit_card.h"
2121
#include "components/autofill/core/browser/payments/full_card_request.h"
22+
#include "components/autofill/core/browser/personal_data_manager.h"
2223
#include "components/autofill/core/browser/validation.h"
2324
#include "components/autofill/core/common/autofill_clock.h"
2425
#include "components/grit/components_scaled_resources.h"
@@ -66,6 +67,7 @@ CvcUnmaskViewController::CvcUnmaskViewController(
6667
IdentityManagerFactory::GetForProfile(
6768
Profile::FromBrowserContext(web_contents_->GetBrowserContext())
6869
->GetOriginalProfile()),
70+
state->GetPersonalDataManager(),
6971
Profile::FromBrowserContext(web_contents_->GetBrowserContext())
7072
->IsOffTheRecord()),
7173
full_card_request_(this,

components/autofill/core/browser/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import("//testing/libfuzzer/fuzzer_test.gni")
88

99
jumbo_static_library("browser") {
1010
sources = [
11+
"account_info_getter.h",
1112
"address.cc",
1213
"address.h",
1314
"address_combobox_model.cc",

components/autofill/core/browser/DEPS

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,7 @@ include_rules = [
66
"+components/metrics",
77
"+components/policy",
88
"+components/security_state",
9-
10-
# Autofill depends on //services/identity/public/cpp for its core Google
11-
# identity dependencies, but depends on this isolated file for recording
12-
# some browser-specific signin metrics.
13-
"+components/signin/core/browser/signin_metrics.h",
14-
9+
"+components/signin/core/browser",
1510
"+components/sync",
1611
"+components/variations",
1712
"+components/version_info",
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2018 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_ACCOUNT_INFO_GETTER_H_
6+
#define COMPONENTS_AUTOFILL_CORE_BROWSER_ACCOUNT_INFO_GETTER_H_
7+
8+
#include <string>
9+
10+
namespace autofill {
11+
12+
// Interface to get account information in Autofill.
13+
class AccountInfoGetter {
14+
public:
15+
// Returns the account id of the active signed-in user irrespective of
16+
// whether they enabled sync or not.
17+
virtual std::string GetActiveSignedInAccountId() const = 0;
18+
19+
// Returns true - When user is both signed-in and enabled sync.
20+
// Returns false - When user is not signed-in or does not have sync the
21+
// feature enabled.
22+
virtual bool IsSyncFeatureEnabled() const = 0;
23+
24+
protected:
25+
virtual ~AccountInfoGetter() {}
26+
};
27+
28+
} // namespace autofill
29+
30+
#endif // COMPONENTS_AUTOFILL_CORE_BROWSER_ACCOUNT_INFO_GETTER_H_

components/autofill/core/browser/autofill_manager.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,7 @@ AutofillManager::AutofillManager(
11211121
driver->GetURLLoaderFactory(),
11221122
client->GetPrefs(),
11231123
client->GetIdentityManager(),
1124+
client->GetPersonalDataManager(),
11241125
driver->IsIncognito())),
11251126
app_locale_(app_locale),
11261127
personal_data_(personal_data),

components/autofill/core/browser/credit_card_save_manager_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class CreditCardSaveManagerTest : public testing::Test {
113113
autofill_driver_->SetURLRequestContext(request_context_.get());
114114
payments_client_ = new payments::TestPaymentsClient(
115115
autofill_driver_->GetURLLoaderFactory(), autofill_client_.GetPrefs(),
116-
autofill_client_.GetIdentityManager());
116+
autofill_client_.GetIdentityManager(), &personal_data_);
117117
credit_card_save_manager_ =
118118
new TestCreditCardSaveManager(autofill_driver_.get(), &autofill_client_,
119119
payments_client_, &personal_data_);

components/autofill/core/browser/local_card_migration_manager_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class LocalCardMigrationManagerTest : public testing::Test {
6969
autofill_driver_->SetURLRequestContext(request_context_.get());
7070
payments_client_ = new payments::TestPaymentsClient(
7171
autofill_driver_->GetURLLoaderFactory(), autofill_client_.GetPrefs(),
72-
autofill_client_.GetIdentityManager());
72+
autofill_client_.GetIdentityManager(), &personal_data_);
7373
credit_card_save_manager_ =
7474
new TestCreditCardSaveManager(autofill_driver_.get(), &autofill_client_,
7575
payments_client_, &personal_data_);

components/autofill/core/browser/payments/full_card_request_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class FullCardRequestTest : public testing::Test {
8181
autofill_client_.SetPrefs(std::move(pref_service));
8282
payments_client_ = std::make_unique<PaymentsClient>(
8383
test_shared_loader_factory_, autofill_client_.GetPrefs(),
84-
autofill_client_.GetIdentityManager());
84+
autofill_client_.GetIdentityManager(), &personal_data_);
8585
request_ = std::make_unique<FullCardRequest>(
8686
&autofill_client_, payments_client_.get(), &personal_data_);
8787
// Silence the warning from PaymentsClient about matching sync and Payments

components/autofill/core/browser/payments/payments_client.cc

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "base/strings/utf_string_conversions.h"
1818
#include "base/values.h"
1919
#include "build/build_config.h"
20+
#include "components/autofill/core/browser/account_info_getter.h"
2021
#include "components/autofill/core/browser/autofill_data_model.h"
2122
#include "components/autofill/core/browser/autofill_type.h"
2223
#include "components/autofill/core/browser/credit_card.h"
@@ -226,6 +227,13 @@ void SetActiveExperiments(const std::vector<const char*>& active_experiments,
226227
std::move(active_chrome_experiments));
227228
}
228229

230+
bool ShouldUseActiveSignedInAccount() {
231+
return base::FeatureList::IsEnabled(
232+
features::kAutofillEnableAccountWalletStorage) ||
233+
base::FeatureList::IsEnabled(
234+
features::kAutofillGetPaymentsIdentityFromSync);
235+
}
236+
229237
class UnmaskCardRequest : public PaymentsRequest {
230238
public:
231239
UnmaskCardRequest(const PaymentsClient::UnmaskRequestDetails& request_details,
@@ -625,10 +633,12 @@ PaymentsClient::PaymentsClient(
625633
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
626634
PrefService* pref_service,
627635
identity::IdentityManager* identity_manager,
636+
AccountInfoGetter* account_info_getter,
628637
bool is_off_the_record)
629638
: url_loader_factory_(url_loader_factory),
630639
pref_service_(pref_service),
631640
identity_manager_(identity_manager),
641+
account_info_getter_(account_info_getter),
632642
is_off_the_record_(is_off_the_record),
633643
has_retried_authorization_(false),
634644
weak_ptr_factory_(this) {}
@@ -843,20 +853,25 @@ void PaymentsClient::StartTokenFetch(bool invalidate_old) {
843853
if (!invalidate_old && token_fetcher_)
844854
return;
845855

856+
DCHECK(account_info_getter_);
857+
846858
OAuth2TokenService::ScopeSet payments_scopes;
847859
payments_scopes.insert(kPaymentsOAuth2Scope);
860+
std::string account_id =
861+
ShouldUseActiveSignedInAccount()
862+
? account_info_getter_->GetActiveSignedInAccountId()
863+
: identity_manager_->GetPrimaryAccountInfo().account_id;
848864
if (invalidate_old) {
849865
DCHECK(!access_token_.empty());
850-
identity_manager_->RemoveAccessTokenFromCache(
851-
identity_manager_->GetPrimaryAccountInfo().account_id, payments_scopes,
852-
access_token_);
866+
identity_manager_->RemoveAccessTokenFromCache(account_id, payments_scopes,
867+
access_token_);
853868
}
854869
access_token_.clear();
855-
token_fetcher_ = std::make_unique<identity::PrimaryAccountAccessTokenFetcher>(
856-
kTokenFetchId, identity_manager_, payments_scopes,
870+
token_fetcher_ = identity_manager_->CreateAccessTokenFetcherForAccount(
871+
account_id, kTokenFetchId, payments_scopes,
857872
base::BindOnce(&PaymentsClient::AccessTokenFetchFinished,
858873
base::Unretained(this)),
859-
identity::PrimaryAccountAccessTokenFetcher::Mode::kImmediate);
874+
identity::AccessTokenFetcher::Mode::kImmediate);
860875
}
861876

862877
void PaymentsClient::SetOAuth2TokenAndStartRequest() {

components/autofill/core/browser/payments/payments_client.h

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
#include "components/autofill/core/browser/credit_card.h"
1616
#include "components/prefs/pref_service.h"
1717
#include "google_apis/gaia/google_service_auth_error.h"
18+
#include "services/identity/public/cpp/access_token_fetcher.h"
1819
#include "services/identity/public/cpp/access_token_info.h"
1920

2021
namespace identity {
2122
class IdentityManager;
22-
class PrimaryAccountAccessTokenFetcher;
2323
} // namespace identity
2424

2525
namespace network {
@@ -30,6 +30,7 @@ class SharedURLLoaderFactory;
3030

3131
namespace autofill {
3232

33+
class AccountInfoGetter;
3334
class MigratableCreditCard;
3435

3536
namespace payments {
@@ -110,13 +111,14 @@ class PaymentsClient {
110111

111112
// |url_loader_factory| is reference counted so it has no lifetime or
112113
// ownership requirements. |pref_service| is used to get the registered
113-
// preference value, |identity_manager|, |unmask_delegate| and |save_delegate|
114+
// preference value, |identity_manager| and |account_info_getter|
114115
// must all outlive |this|. Either delegate might be nullptr.
115116
// |is_off_the_record| denotes incognito mode.
116117
PaymentsClient(
117118
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
118-
PrefService* pref_service,
119-
identity::IdentityManager* identity_manager,
119+
PrefService* const pref_service,
120+
identity::IdentityManager* const identity_manager,
121+
AccountInfoGetter* const account_info_getter,
120122
bool is_off_the_record = false);
121123

122124
virtual ~PaymentsClient();
@@ -221,8 +223,12 @@ class PaymentsClient {
221223
// The pref service for this client.
222224
PrefService* const pref_service_;
223225

226+
// Provided in constructor; not owned by PaymentsClient.
224227
identity::IdentityManager* const identity_manager_;
225228

229+
// Provided in constructor; not owned by PaymentsClient.
230+
AccountInfoGetter* const account_info_getter_;
231+
226232
// The current request.
227233
std::unique_ptr<PaymentsRequest> request_;
228234

@@ -232,8 +238,8 @@ class PaymentsClient {
232238
// The URL loader being used to issue the current request.
233239
std::unique_ptr<network::SimpleURLLoader> simple_url_loader_;
234240

235-
// The current OAuth2 token fetcher.
236-
std::unique_ptr<identity::PrimaryAccountAccessTokenFetcher> token_fetcher_;
241+
// The OAuth2 token fetcher for any account.
242+
std::unique_ptr<identity::AccessTokenFetcher> token_fetcher_;
237243

238244
// The OAuth2 token, or empty if not fetched.
239245
std::string access_token_;

components/autofill/core/browser/payments/payments_client_unittest.cc

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "components/autofill/core/browser/credit_card_save_manager.h"
2020
#include "components/autofill/core/browser/local_card_migration_manager.h"
2121
#include "components/autofill/core/browser/payments/payments_client.h"
22+
#include "components/autofill/core/browser/test_personal_data_manager.h"
2223
#include "components/autofill/core/common/autofill_features.h"
2324
#include "components/autofill/core/common/autofill_switches.h"
2425
#include "components/prefs/pref_registry_simple.h"
@@ -73,9 +74,13 @@ class PaymentsClientTest : public testing::Test {
7374
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
7475
&test_url_loader_factory_);
7576
TestingPrefServiceSimple pref_service_;
76-
client_.reset(new PaymentsClient(test_shared_loader_factory_,
77-
&pref_service_,
78-
identity_test_env_.identity_manager()));
77+
client_ = std::make_unique<PaymentsClient>(
78+
test_shared_loader_factory_, &pref_service_,
79+
identity_test_env_.identity_manager(), &test_personal_data_);
80+
const std::string& account_id =
81+
identity_test_env_.MakePrimaryAccountAvailable("[email protected]")
82+
.account_id;
83+
test_personal_data_.SetActiveAccountId(account_id);
7984
}
8085

8186
void TearDown() override { client_.reset(); }
@@ -90,6 +95,11 @@ class PaymentsClientTest : public testing::Test {
9095
features::kAutofillSendExperimentIdsInPaymentsRPCs);
9196
}
9297

98+
void EnableAutofillGetPaymentsIdentityFromSync() {
99+
scoped_feature_list_.InitAndEnableFeature(
100+
features::kAutofillGetPaymentsIdentityFromSync);
101+
}
102+
93103
void DisableAutofillSendExperimentIdsInPaymentsRPCs() {
94104
scoped_feature_list_.InitAndDisableFeature(
95105
features::kAutofillSendExperimentIdsInPaymentsRPCs);
@@ -141,9 +151,6 @@ class PaymentsClientTest : public testing::Test {
141151
// Issue an UnmaskCard request. This requires an OAuth token before starting
142152
// the request.
143153
void StartUnmasking() {
144-
if (!identity_test_env_.identity_manager()->HasPrimaryAccount())
145-
identity_test_env_.MakePrimaryAccountAvailable("[email protected]");
146-
147154
PaymentsClient::UnmaskRequestDetails request_details;
148155
request_details.billing_customer_number = 111222333444;
149156
request_details.card = test::GetMaskedServerCard();
@@ -156,9 +163,6 @@ class PaymentsClientTest : public testing::Test {
156163

157164
// Issue a GetUploadDetails request.
158165
void StartGettingUploadDetails() {
159-
if (!identity_test_env_.identity_manager()->HasPrimaryAccount())
160-
identity_test_env_.MakePrimaryAccountAvailable("[email protected]");
161-
162166
client_->GetUploadDetails(
163167
BuildTestProfiles(), kAllDetectableValues,
164168
/*pan_first_six=*/"411111", std::vector<const char*>(),
@@ -171,9 +175,6 @@ class PaymentsClientTest : public testing::Test {
171175
// Issue an UploadCard request. This requires an OAuth token before starting
172176
// the request.
173177
void StartUploading(bool include_cvc) {
174-
if (!identity_test_env_.identity_manager()->HasPrimaryAccount())
175-
identity_test_env_.MakePrimaryAccountAvailable("[email protected]");
176-
177178
PaymentsClient::UploadRequestDetails request_details;
178179
request_details.billing_customer_number = 111222333444;
179180
request_details.card = test::GetCreditCard();
@@ -189,9 +190,6 @@ class PaymentsClientTest : public testing::Test {
189190
}
190191

191192
void StartMigrating(bool uncheck_last_card, bool has_cardholder_name) {
192-
if (!identity_test_env_.identity_manager()->HasPrimaryAccount())
193-
identity_test_env_.MakePrimaryAccountAvailable("[email protected]");
194-
195193
PaymentsClient::MigrationRequestDetails request_details;
196194
request_details.context_token = base::ASCIIToUTF16("context token");
197195
request_details.risk_data = "some risk data";
@@ -251,6 +249,7 @@ class PaymentsClientTest : public testing::Test {
251249
base::test::ScopedTaskEnvironment scoped_task_environment_;
252250
network::TestURLLoaderFactory test_url_loader_factory_;
253251
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
252+
TestPersonalDataManager test_personal_data_;
254253
std::unique_ptr<PaymentsClient> client_;
255254
identity::IdentityTestEnvironment identity_test_env_;
256255

@@ -319,6 +318,15 @@ TEST_F(PaymentsClientTest, UnmaskSuccess) {
319318
EXPECT_EQ("1234", real_pan_);
320319
}
321320

321+
TEST_F(PaymentsClientTest, UnmaskSuccessAccountFromSyncTest) {
322+
EnableAutofillGetPaymentsIdentityFromSync();
323+
StartUnmasking();
324+
IssueOAuthToken();
325+
ReturnResponse(net::HTTP_OK, "{ \"pan\": \"1234\" }");
326+
EXPECT_EQ(AutofillClient::SUCCESS, result_);
327+
EXPECT_EQ("1234", real_pan_);
328+
}
329+
322330
TEST_F(PaymentsClientTest, GetDetailsSuccess) {
323331
StartGettingUploadDetails();
324332
ReturnResponse(
@@ -361,6 +369,29 @@ TEST_F(PaymentsClientTest, GetDetailsIncludesDetectedValuesInRequest) {
361369
std::string::npos);
362370
}
363371

372+
TEST_F(PaymentsClientTest, GetUploadAccountFromSyncTest) {
373+
EnableAutofillGetPaymentsIdentityFromSync();
374+
// Set up a different account.
375+
const std::string& secondary_account_id =
376+
identity_test_env_.MakeAccountAvailable("[email protected]").account_id;
377+
test_personal_data_.SetActiveAccountId(secondary_account_id);
378+
379+
StartUploading(/*include_cvc=*/true);
380+
ReturnResponse(net::HTTP_OK, "{}");
381+
382+
// Issue a token for the secondary account.
383+
identity_test_env_.WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
384+
secondary_account_id, "secondary_account_token",
385+
base::Time::Now() + base::TimeDelta::FromDays(10));
386+
387+
// Verify the auth header.
388+
std::string auth_header_value;
389+
EXPECT_TRUE(intercepted_headers_.GetHeader(
390+
net::HttpRequestHeaders::kAuthorization, &auth_header_value))
391+
<< intercepted_headers_.ToString();
392+
EXPECT_EQ("Bearer secondary_account_token", auth_header_value);
393+
}
394+
364395
TEST_F(PaymentsClientTest, GetUploadDetailsVariationsTest) {
365396
// Register a trial and variation id, so that there is data in variations
366397
// headers. Also, the variations header provider may have been registered to

components/autofill/core/browser/payments/payments_util.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager,
2323
DCHECK(pref_service);
2424

2525
if (base::FeatureList::IsEnabled(
26-
features::kAutofillUsePaymentsCustomerData)) {
26+
features::kAutofillUsePaymentsCustomerData) ||
27+
base::FeatureList::IsEnabled(
28+
features::kAutofillEnableAccountWalletStorage)) {
2729
// Get billing customer ID from the synced PaymentsCustomerData.
2830
PaymentsCustomerData* customer_data =
2931
personal_data_manager->GetPaymentsCustomerData();

components/autofill/core/browser/payments/test_payments_client.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "components/autofill/core/browser/payments/test_payments_client.h"
66

77
#include "base/strings/utf_string_conversions.h"
8+
#include "components/autofill/core/browser/personal_data_manager.h"
89
#include "services/network/public/cpp/shared_url_loader_factory.h"
910

1011
namespace autofill {
@@ -13,8 +14,12 @@ namespace payments {
1314
TestPaymentsClient::TestPaymentsClient(
1415
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_,
1516
PrefService* pref_service,
16-
identity::IdentityManager* identity_manager)
17-
: PaymentsClient(url_loader_factory_, pref_service, identity_manager) {}
17+
identity::IdentityManager* identity_manager,
18+
PersonalDataManager* personal_data_manager)
19+
: PaymentsClient(url_loader_factory_,
20+
pref_service,
21+
identity_manager,
22+
personal_data_manager) {}
1823

1924
TestPaymentsClient::~TestPaymentsClient() {}
2025

0 commit comments

Comments
 (0)