Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(390)

Side by Side Diff: components/autofill/core/browser/autofill_manager.cc

Issue 2789843004: [Payments] Upload card UI now has a CVC prompt (Closed)
Patch Set: Addressing code review comments Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/autofill/core/browser/autofill_manager.h" 5 #include "components/autofill/core/browser/autofill_manager.h"
6 6
7 #include <stddef.h> 7 #include <stddef.h>
8 #include <stdint.h> 8 #include <stdint.h>
9 9
10 #include <algorithm> 10 #include <algorithm>
(...skipping 216 matching lines...) Expand 10 before | Expand all | Expand 10 after
227 new AutofillMetrics::FormEventLogger(false /* is_for_credit_card */)), 227 new AutofillMetrics::FormEventLogger(false /* is_for_credit_card */)),
228 credit_card_form_event_logger_( 228 credit_card_form_event_logger_(
229 new AutofillMetrics::FormEventLogger(true /* is_for_credit_card */)), 229 new AutofillMetrics::FormEventLogger(true /* is_for_credit_card */)),
230 has_logged_autofill_enabled_(false), 230 has_logged_autofill_enabled_(false),
231 has_logged_address_suggestions_count_(false), 231 has_logged_address_suggestions_count_(false),
232 did_show_suggestions_(false), 232 did_show_suggestions_(false),
233 user_did_type_(false), 233 user_did_type_(false),
234 user_did_autofill_(false), 234 user_did_autofill_(false),
235 user_did_edit_autofilled_field_(false), 235 user_did_edit_autofilled_field_(false),
236 user_did_accept_upload_prompt_(false), 236 user_did_accept_upload_prompt_(false),
237 should_cvc_be_requested_(false),
237 external_delegate_(NULL), 238 external_delegate_(NULL),
238 test_delegate_(NULL), 239 test_delegate_(NULL),
239 #if defined(OS_ANDROID) || defined(OS_IOS) 240 #if defined(OS_ANDROID) || defined(OS_IOS)
240 autofill_assistant_(this), 241 autofill_assistant_(this),
241 #endif 242 #endif
242 weak_ptr_factory_(this) { 243 weak_ptr_factory_(this) {
243 if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) { 244 if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) {
244 download_manager_.reset(new AutofillDownloadManager(driver, this)); 245 download_manager_.reset(new AutofillDownloadManager(driver, this));
245 } 246 }
246 CountryNames::SetLocaleString(app_locale_); 247 CountryNames::SetLocaleString(app_locale_);
(...skipping 779 matching lines...) Expand 10 before | Expand all | Expand 10 after
1026 const base::string16& context_token, 1027 const base::string16& context_token,
1027 std::unique_ptr<base::DictionaryValue> legal_message) { 1028 std::unique_ptr<base::DictionaryValue> legal_message) {
1028 // TODO(jdonnelly): Log duration. 1029 // TODO(jdonnelly): Log duration.
1029 if (result == AutofillClient::SUCCESS) { 1030 if (result == AutofillClient::SUCCESS) {
1030 // Do *not* call payments_client_->Prepare() here. We shouldn't send 1031 // Do *not* call payments_client_->Prepare() here. We shouldn't send
1031 // credentials until the user has explicitly accepted a prompt to upload. 1032 // credentials until the user has explicitly accepted a prompt to upload.
1032 upload_request_.context_token = context_token; 1033 upload_request_.context_token = context_token;
1033 user_did_accept_upload_prompt_ = false; 1034 user_did_accept_upload_prompt_ = false;
1034 client_->ConfirmSaveCreditCardToCloud( 1035 client_->ConfirmSaveCreditCardToCloud(
1035 upload_request_.card, std::move(legal_message), 1036 upload_request_.card, std::move(legal_message),
1037 should_cvc_be_requested_,
1036 base::Bind(&AutofillManager::OnUserDidAcceptUpload, 1038 base::Bind(&AutofillManager::OnUserDidAcceptUpload,
1037 weak_ptr_factory_.GetWeakPtr())); 1039 weak_ptr_factory_.GetWeakPtr()));
1038 client_->LoadRiskData(base::Bind(&AutofillManager::OnDidGetUploadRiskData, 1040 client_->LoadRiskData(base::Bind(&AutofillManager::OnDidGetUploadRiskData,
1039 weak_ptr_factory_.GetWeakPtr())); 1041 weak_ptr_factory_.GetWeakPtr()));
1040 AutofillMetrics::LogCardUploadDecisionMetric( 1042 AutofillMetrics::CardUploadDecisionMetric cardUploadDecisionMetric =
Mathieu 2017/04/03 19:46:09 variable_casing
Jared Saul 2017/04/03 21:19:18 Done, good catch.
1041 AutofillMetrics::UPLOAD_OFFERED); 1043 should_cvc_be_requested_ ? AutofillMetrics::UPLOAD_OFFERED_NO_CVC
1042 LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_OFFERED); 1044 : AutofillMetrics::UPLOAD_OFFERED;
1045 AutofillMetrics::LogCardUploadDecisionMetric(cardUploadDecisionMetric);
1046 LogCardUploadDecisionUkm(cardUploadDecisionMetric);
1043 } else { 1047 } else {
1044 // If the upload details request failed, fall back to a local save. The 1048 // If the upload details request failed, fall back to a local save. The
1045 // reasoning here is as follows: 1049 // reasoning here is as follows:
1046 // - This will sometimes fail intermittently, in which case it might be 1050 // - This will sometimes fail intermittently, in which case it might be
1047 // better to not fall back, because sometimes offering upload and sometimes 1051 // better to not fall back, because sometimes offering upload and sometimes
1048 // offering local save is a poor user experience. 1052 // offering local save is a poor user experience.
1049 // - However, in some cases, our local configuration limiting the feature to 1053 // - However, in some cases, our local configuration limiting the feature to
1050 // countries that Payments is known to support will not match Payments' own 1054 // countries that Payments is known to support will not match Payments' own
1051 // determination of what country the user is located in. In these cases, 1055 // determination of what country the user is located in. In these cases,
1052 // the upload details request will consistently fail and if we don't fall 1056 // the upload details request will consistently fail and if we don't fall
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
1092 1096
1093 void AutofillManager::OnUnmaskVerificationResult( 1097 void AutofillManager::OnUnmaskVerificationResult(
1094 AutofillClient::PaymentsRpcResult result) { 1098 AutofillClient::PaymentsRpcResult result) {
1095 client_->OnUnmaskVerificationResult(result); 1099 client_->OnUnmaskVerificationResult(result);
1096 } 1100 }
1097 1101
1098 void AutofillManager::OnUserDidAcceptUpload() { 1102 void AutofillManager::OnUserDidAcceptUpload() {
1099 user_did_accept_upload_prompt_ = true; 1103 user_did_accept_upload_prompt_ = true;
1100 if (!upload_request_.risk_data.empty()) { 1104 if (!upload_request_.risk_data.empty()) {
1101 upload_request_.app_locale = app_locale_; 1105 upload_request_.app_locale = app_locale_;
1106 // If the upload request does not have card CVC, populate it with the value
1107 // provided by the user:
1108 if (upload_request_.cvc.empty()) {
1109 upload_request_.cvc =
Mathieu 2017/04/03 19:46:09 DCHECK(client_->GetSaveCardBubbleController()) ?
Jared Saul 2017/04/03 21:19:18 Done.
1110 client_->GetSaveCardBubbleController()->GetCvcEnteredByUser();
1111 }
1102 payments_client_->UploadCard(upload_request_); 1112 payments_client_->UploadCard(upload_request_);
1103 } 1113 }
1104 } 1114 }
1105 1115
1106 void AutofillManager::OnDidGetUploadRiskData(const std::string& risk_data) { 1116 void AutofillManager::OnDidGetUploadRiskData(const std::string& risk_data) {
1107 upload_request_.risk_data = risk_data; 1117 upload_request_.risk_data = risk_data;
1108 if (user_did_accept_upload_prompt_) { 1118 if (user_did_accept_upload_prompt_) {
1109 upload_request_.app_locale = app_locale_; 1119 upload_request_.app_locale = app_locale_;
1120 // If the upload request does not have card CVC, populate it with the value
1121 // provided by the user:
1122 if (upload_request_.cvc.empty()) {
1123 upload_request_.cvc =
1124 client_->GetSaveCardBubbleController()->GetCvcEnteredByUser();
1125 }
1110 payments_client_->UploadCard(upload_request_); 1126 payments_client_->UploadCard(upload_request_);
1111 } 1127 }
1112 } 1128 }
1113 1129
1114 void AutofillManager::OnDidEndTextFieldEditing() { 1130 void AutofillManager::OnDidEndTextFieldEditing() {
1115 external_delegate_->DidEndTextFieldEditing(); 1131 external_delegate_->DidEndTextFieldEditing();
1116 } 1132 }
1117 1133
1118 bool AutofillManager::IsAutofillEnabled() const { 1134 bool AutofillManager::IsAutofillEnabled() const {
1119 return ::autofill::IsAutofillEnabled(client_->GetPrefs()); 1135 return ::autofill::IsAutofillEnabled(client_->GetPrefs());
(...skipping 72 matching lines...) Expand 10 before | Expand all | Expand 10 after
1192 upload_request_.card = *imported_credit_card; 1208 upload_request_.card = *imported_credit_card;
1193 1209
1194 // In order to prompt the user to upload their card, we must have both: 1210 // In order to prompt the user to upload their card, we must have both:
1195 // 1) Card with CVC 1211 // 1) Card with CVC
1196 // 2) 1+ recently-used or modified addresses that meet the client-side 1212 // 2) 1+ recently-used or modified addresses that meet the client-side
1197 // validation rules 1213 // validation rules
1198 // Here we perform both checks before returning or logging anything, 1214 // Here we perform both checks before returning or logging anything,
1199 // because if only one of the two is missing, it may be fixable. 1215 // because if only one of the two is missing, it may be fixable.
1200 1216
1201 // Check for a CVC to determine whether we can prompt the user to upload 1217 // Check for a CVC to determine whether we can prompt the user to upload
1202 // their card. If no CVC is present, do nothing. We could fall back to a 1218 // their card. If no CVC is present and the experiment is off, do nothing.
1203 // local save but we believe that sometimes offering upload and sometimes 1219 // We could fall back to a local save but we believe that sometimes offering
1204 // offering local save is a confusing user experience. 1220 // upload and sometimes offering local save is a confusing user experience.
1221 // If no CVC and the experiment is on, request CVC from the user in the
1222 // bubble and save using the provided value.
1205 for (const auto& field : submitted_form) { 1223 for (const auto& field : submitted_form) {
1206 if (field->Type().GetStorableType() == CREDIT_CARD_VERIFICATION_CODE && 1224 if (field->Type().GetStorableType() == CREDIT_CARD_VERIFICATION_CODE &&
1207 IsValidCreditCardSecurityCode(field->value, 1225 IsValidCreditCardSecurityCode(field->value,
1208 upload_request_.card.type())) { 1226 upload_request_.card.type())) {
1209 upload_request_.cvc = field->value; 1227 upload_request_.cvc = field->value;
1210 break; 1228 break;
1211 } 1229 }
1212 } 1230 }
1213 1231
1214 // Upload requires that recently used or modified addresses meet the 1232 // Upload requires that recently used or modified addresses meet the
1215 // client-side validation rules. 1233 // client-side validation rules.
1216 autofill::AutofillMetrics::CardUploadDecisionMetric 1234 autofill::AutofillMetrics::CardUploadDecisionMetric
1217 get_profiles_decision_metric = AutofillMetrics::UPLOAD_OFFERED; 1235 get_profiles_decision_metric = AutofillMetrics::UPLOAD_OFFERED;
1218 std::string rappor_metric_name; 1236 std::string rappor_metric_name;
1219 bool get_profiles_succeeded = 1237 bool get_profiles_succeeded =
1220 GetProfilesForCreditCardUpload(*imported_credit_card, 1238 GetProfilesForCreditCardUpload(*imported_credit_card,
1221 &upload_request_.profiles, 1239 &upload_request_.profiles,
1222 &get_profiles_decision_metric, 1240 &get_profiles_decision_metric,
1223 &rappor_metric_name); 1241 &rappor_metric_name);
1224 1242
1225 pending_upload_request_url_ = GURL(submitted_form.source_url()); 1243 pending_upload_request_url_ = GURL(submitted_form.source_url());
1226 1244
1227 // Both the CVC and address checks are done. Conform to the legacy order of 1245 // Both the CVC and address checks are done. Conform to the legacy order of
1228 // reporting on CVC then address. 1246 // reporting on CVC then address.
1247 should_cvc_be_requested_ = false;
1229 if (upload_request_.cvc.empty()) { 1248 if (upload_request_.cvc.empty()) {
1230 AutofillMetrics::LogCardUploadDecisionMetric( 1249 if (IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled()) {
1231 AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC); 1250 should_cvc_be_requested_ = true;
1232 LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC); 1251 } else {
1233 pending_upload_request_url_ = GURL(); 1252 AutofillMetrics::LogCardUploadDecisionMetric(
1234 CollectRapportSample(submitted_form.source_url(), 1253 AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
1235 "Autofill.CardUploadNotOfferedNoCvc"); 1254 LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
1236 return; 1255 pending_upload_request_url_ = GURL();
1256 CollectRapportSample(submitted_form.source_url(),
1257 "Autofill.CardUploadNotOfferedNoCvc");
1258 return;
1259 }
1237 } 1260 }
1238 if (!get_profiles_succeeded) { 1261 if (!get_profiles_succeeded) {
1239 DCHECK(get_profiles_decision_metric != AutofillMetrics::UPLOAD_OFFERED); 1262 DCHECK(get_profiles_decision_metric != AutofillMetrics::UPLOAD_OFFERED);
1240 AutofillMetrics::LogCardUploadDecisionMetric( 1263 AutofillMetrics::LogCardUploadDecisionMetric(
1241 get_profiles_decision_metric); 1264 get_profiles_decision_metric);
1242 LogCardUploadDecisionUkm(get_profiles_decision_metric); 1265 LogCardUploadDecisionUkm(get_profiles_decision_metric);
1243 pending_upload_request_url_ = GURL(); 1266 pending_upload_request_url_ = GURL();
1244 if (!rappor_metric_name.empty()) { 1267 if (!rappor_metric_name.empty()) {
1245 CollectRapportSample(submitted_form.source_url(), rappor_metric_name); 1268 CollectRapportSample(submitted_form.source_url(), rappor_metric_name);
1246 } 1269 }
(...skipping 951 matching lines...) Expand 10 before | Expand all | Expand 10 after
2198 } 2221 }
2199 #endif // ENABLE_FORM_DEBUG_DUMP 2222 #endif // ENABLE_FORM_DEBUG_DUMP
2200 2223
2201 void AutofillManager::LogCardUploadDecisionUkm( 2224 void AutofillManager::LogCardUploadDecisionUkm(
2202 AutofillMetrics::CardUploadDecisionMetric upload_decision) { 2225 AutofillMetrics::CardUploadDecisionMetric upload_decision) {
2203 AutofillMetrics::LogCardUploadDecisionUkm( 2226 AutofillMetrics::LogCardUploadDecisionUkm(
2204 client_->GetUkmService(), pending_upload_request_url_, upload_decision); 2227 client_->GetUkmService(), pending_upload_request_url_, upload_decision);
2205 } 2228 }
2206 2229
2207 } // namespace autofill 2230 } // namespace autofill
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698