Commit 473d9a34 authored by Mathieu Perreault's avatar Mathieu Perreault Committed by Commit Bot

[Payments] Restrict when the validity of PaymentsCustomerData is logged.

Will only log the validity of PaymentsCustomerData's billing customer id during
the full card request. This is the only moment we can be sure that the user
has an account.

Bug: 884689
Change-Id: I74699f14827e2954ac03480f17ac7f51b391ae73
Reviewed-on: https://chromium-review.googlesource.com/1246289
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: default avatarSebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594485}
parent 947bd6b7
...@@ -69,7 +69,8 @@ void FullCardRequest::GetFullCard(const CreditCard& card, ...@@ -69,7 +69,8 @@ void FullCardRequest::GetFullCard(const CreditCard& card,
if (should_unmask_card_) { if (should_unmask_card_) {
payments_client_->Prepare(); payments_client_->Prepare();
request_->billing_customer_number = GetBillingCustomerId( request_->billing_customer_number = GetBillingCustomerId(
personal_data_manager_, payments_client_->GetPrefService()); personal_data_manager_, payments_client_->GetPrefService(),
/*should_log_validity=*/true);
} }
ui_delegate_->ShowUnmaskPrompt(request_->card, reason, ui_delegate_->ShowUnmaskPrompt(request_->card, reason,
......
...@@ -18,7 +18,8 @@ namespace autofill { ...@@ -18,7 +18,8 @@ namespace autofill {
namespace payments { namespace payments {
int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager, int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager,
PrefService* pref_service) { PrefService* pref_service,
bool should_log_validity) {
DCHECK(personal_data_manager); DCHECK(personal_data_manager);
DCHECK(pref_service); DCHECK(pref_service);
...@@ -33,16 +34,22 @@ int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager, ...@@ -33,16 +34,22 @@ int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager,
int64_t billing_customer_id = 0; int64_t billing_customer_id = 0;
if (base::StringToInt64(base::StringPiece(customer_data->customer_id), if (base::StringToInt64(base::StringPiece(customer_data->customer_id),
&billing_customer_id)) { &billing_customer_id)) {
AutofillMetrics::LogPaymentsCustomerDataBillingIdStatus( if (should_log_validity) {
AutofillMetrics::BillingIdStatus::VALID); AutofillMetrics::LogPaymentsCustomerDataBillingIdStatus(
AutofillMetrics::BillingIdStatus::VALID);
}
return billing_customer_id; return billing_customer_id;
} else { } else {
AutofillMetrics::LogPaymentsCustomerDataBillingIdStatus( if (should_log_validity) {
AutofillMetrics::BillingIdStatus::PARSE_ERROR); AutofillMetrics::LogPaymentsCustomerDataBillingIdStatus(
AutofillMetrics::BillingIdStatus::PARSE_ERROR);
}
} }
} else { } else {
AutofillMetrics::LogPaymentsCustomerDataBillingIdStatus( if (should_log_validity) {
AutofillMetrics::BillingIdStatus::MISSING); AutofillMetrics::LogPaymentsCustomerDataBillingIdStatus(
AutofillMetrics::BillingIdStatus::MISSING);
}
} }
} }
......
...@@ -17,9 +17,11 @@ namespace payments { ...@@ -17,9 +17,11 @@ namespace payments {
// Returns the billing customer ID (a.k.a. the customer number) for the Google // Returns the billing customer ID (a.k.a. the customer number) for the Google
// Payments account for this user. Obtains it from the synced data. Returns 0 // Payments account for this user. Obtains it from the synced data. Returns 0
// if the customer ID was not found. // if the customer ID was not found. If |should_log_validity| is true, will
// report on the validity state of the customer ID in PaymentsCustomerData.
int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager, int64_t GetBillingCustomerId(PersonalDataManager* personal_data_manager,
PrefService* pref_service); PrefService* pref_service,
bool should_log_validity = false);
} // namespace payments } // namespace payments
} // namespace autofill } // namespace autofill
......
...@@ -46,7 +46,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PaymentsCustomerData_Normal) { ...@@ -46,7 +46,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PaymentsCustomerData_Normal) {
std::make_unique<PaymentsCustomerData>(/*customer_id=*/"123456")); std::make_unique<PaymentsCustomerData>(/*customer_id=*/"123456"));
EXPECT_EQ(123456, EXPECT_EQ(123456,
GetBillingCustomerId(&personal_data_manager_, &pref_service_)); GetBillingCustomerId(&personal_data_manager_, &pref_service_,
/*should_log_validity=*/true));
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"Autofill.PaymentsCustomerDataBillingIdStatus", "Autofill.PaymentsCustomerDataBillingIdStatus",
...@@ -61,7 +62,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PaymentsCustomerData_Garbage) { ...@@ -61,7 +62,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PaymentsCustomerData_Garbage) {
personal_data_manager_.SetPaymentsCustomerData( personal_data_manager_.SetPaymentsCustomerData(
std::make_unique<PaymentsCustomerData>(/*customer_id=*/"garbage")); std::make_unique<PaymentsCustomerData>(/*customer_id=*/"garbage"));
EXPECT_EQ(0, GetBillingCustomerId(&personal_data_manager_, &pref_service_)); EXPECT_EQ(0, GetBillingCustomerId(&personal_data_manager_, &pref_service_,
/*should_log_validity=*/true));
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"Autofill.PaymentsCustomerDataBillingIdStatus", "Autofill.PaymentsCustomerDataBillingIdStatus",
...@@ -75,7 +77,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PaymentsCustomerData_NoData) { ...@@ -75,7 +77,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PaymentsCustomerData_NoData) {
// Explictly do not set PaymentsCustomerData. Nothing crashes and the returned // Explictly do not set PaymentsCustomerData. Nothing crashes and the returned
// customer ID is 0. // customer ID is 0.
EXPECT_EQ(0, GetBillingCustomerId(&personal_data_manager_, &pref_service_)); EXPECT_EQ(0, GetBillingCustomerId(&personal_data_manager_, &pref_service_,
/*should_log_validity=*/true));
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"Autofill.PaymentsCustomerDataBillingIdStatus", "Autofill.PaymentsCustomerDataBillingIdStatus",
AutofillMetrics::BillingIdStatus::MISSING, 1); AutofillMetrics::BillingIdStatus::MISSING, 1);
...@@ -93,7 +96,8 @@ TEST_F(PaymentsUtilTest, ...@@ -93,7 +96,8 @@ TEST_F(PaymentsUtilTest,
// We got the data from prefs and log that the PaymentsCustomerData is // We got the data from prefs and log that the PaymentsCustomerData is
// invalid. // invalid.
EXPECT_EQ(123456, EXPECT_EQ(123456,
GetBillingCustomerId(&personal_data_manager_, &pref_service_)); GetBillingCustomerId(&personal_data_manager_, &pref_service_,
/*should_log_validity=*/true));
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"Autofill.PaymentsCustomerDataBillingIdStatus", "Autofill.PaymentsCustomerDataBillingIdStatus",
AutofillMetrics::BillingIdStatus::MISSING, 1); AutofillMetrics::BillingIdStatus::MISSING, 1);
...@@ -106,7 +110,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PriorityPrefs_Normal) { ...@@ -106,7 +110,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PriorityPrefs_Normal) {
pref_service_.SetDouble(prefs::kAutofillBillingCustomerNumber, 123456.0); pref_service_.SetDouble(prefs::kAutofillBillingCustomerNumber, 123456.0);
EXPECT_EQ(123456, EXPECT_EQ(123456,
GetBillingCustomerId(&personal_data_manager_, &pref_service_)); GetBillingCustomerId(&personal_data_manager_, &pref_service_,
/*should_log_validity=*/true));
} }
TEST_F(PaymentsUtilTest, GetBillingCustomerId_PriorityPrefs_NoData) { TEST_F(PaymentsUtilTest, GetBillingCustomerId_PriorityPrefs_NoData) {
...@@ -115,7 +120,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PriorityPrefs_NoData) { ...@@ -115,7 +120,8 @@ TEST_F(PaymentsUtilTest, GetBillingCustomerId_PriorityPrefs_NoData) {
// Explictly do not set Prefs data. Nothing crashes and the returned customer // Explictly do not set Prefs data. Nothing crashes and the returned customer
// ID is 0. // ID is 0.
EXPECT_EQ(0, GetBillingCustomerId(&personal_data_manager_, &pref_service_)); EXPECT_EQ(0, GetBillingCustomerId(&personal_data_manager_, &pref_service_,
/*should_log_validity=*/true));
} }
} // namespace payments } // namespace payments
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment