[Payments] Upload card UI now has a CVC prompt

components/autofill/core/browser/autofill_manager.cc

 // Copyright 2013 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/autofill/core/browser/autofill_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 216 matching lines @@
           new AutofillMetrics::FormEventLogger(false /* is_for_credit_card */)),
       credit_card_form_event_logger_(
           new AutofillMetrics::FormEventLogger(true /* is_for_credit_card */)),
       has_logged_autofill_enabled_(false),
       has_logged_address_suggestions_count_(false),
       did_show_suggestions_(false),
       user_did_type_(false),
       user_did_autofill_(false),
       user_did_edit_autofilled_field_(false),
       user_did_accept_upload_prompt_(false),
+      upload_cvc_should_be_requested_(false),
       external_delegate_(NULL),
       test_delegate_(NULL),
 #if defined(OS_ANDROID) || defined(OS_IOS)
       autofill_assistant_(this),
 #endif
       weak_ptr_factory_(this) {
   if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) {
     download_manager_.reset(new AutofillDownloadManager(driver, this));
   }
   CountryNames::SetLocaleString(app_locale_);
@@ skipping 777 matching lines @@
 void AutofillManager::OnDidGetUploadDetails(
     AutofillClient::PaymentsRpcResult result,
     const base::string16& context_token,
     std::unique_ptr<base::DictionaryValue> legal_message) {
   // TODO(jdonnelly): Log duration.
   if (result == AutofillClient::SUCCESS) {
     // Do *not* call payments_client_->Prepare() here. We shouldn't send
     // credentials until the user has explicitly accepted a prompt to upload.
     upload_request_.context_token = context_token;
     user_did_accept_upload_prompt_ = false;
+
     client_->ConfirmSaveCreditCardToCloud(
         upload_request_.card, std::move(legal_message),
+        upload_cvc_should_be_requested_,
         base::Bind(&AutofillManager::OnUserDidAcceptUpload,
                    weak_ptr_factory_.GetWeakPtr()));
     client_->LoadRiskData(base::Bind(&AutofillManager::OnDidGetUploadRiskData,
                                      weak_ptr_factory_.GetWeakPtr()));
-    AutofillMetrics::LogCardUploadDecisionMetric(
-        AutofillMetrics::UPLOAD_OFFERED);
-    LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_OFFERED);
+    AutofillMetrics::CardUploadDecisionMetric cardUploadDecisionMetric =
+        upload_cvc_should_be_requested_ ? AutofillMetrics::UPLOAD_OFFERED_NO_CVC

[csashi, 2017/03/31 21:48:15]

s/NO_CVC/CVC_MISSING or CVC_REQUESTED ?

[Jared Saul, 2017/04/01 04:18:02]

Ooh, I really like UPLOAD_OFFERED_CVC_MISSING, but I did it this way to be
consistent with UPLOAD_NOT_OFFERED_NO_CVC.  I can go either way on this one. 
What do you think, better name or consistency?

+                                        : AutofillMetrics::UPLOAD_OFFERED;
+    AutofillMetrics::LogCardUploadDecisionMetric(cardUploadDecisionMetric);
+    LogCardUploadDecisionUkm(cardUploadDecisionMetric);
   } else {
     // If the upload details request failed, fall back to a local save. The
     // reasoning here is as follows:
     // - This will sometimes fail intermittently, in which case it might be
     // better to not fall back, because sometimes offering upload and sometimes
     // offering local save is a poor user experience.
     // - However, in some cases, our local configuration limiting the feature to
     // countries that Payments is known to support will not match Payments' own
     // determination of what country the user is located in. In these cases,
     // the upload details request will consistently fail and if we don't fall
@@ skipping 39 matching lines @@
 
 void AutofillManager::OnUnmaskVerificationResult(
     AutofillClient::PaymentsRpcResult result) {
   client_->OnUnmaskVerificationResult(result);
 }
 
 void AutofillManager::OnUserDidAcceptUpload() {
   user_did_accept_upload_prompt_ = true;
   if (!upload_request_.risk_data.empty()) {
     upload_request_.app_locale = app_locale_;
+    // If the upload request does not have card CVC, populate it with the value
+    // provided by the user:
+    if (upload_request_.cvc.empty()) {
+      upload_request_.cvc =
+          client_->GetSaveCardBubbleController()->GetUserProvidedCvc();
+    }
     payments_client_->UploadCard(upload_request_);
   }
 }
 
 void AutofillManager::OnDidGetUploadRiskData(const std::string& risk_data) {
   upload_request_.risk_data = risk_data;
   if (user_did_accept_upload_prompt_) {
     upload_request_.app_locale = app_locale_;
+    // If the upload request does not have card CVC, populate it with the value
+    // provided by the user:
+    if (upload_request_.cvc.empty()) {
+      upload_request_.cvc =
+          client_->GetSaveCardBubbleController()->GetUserProvidedCvc();
+    }
     payments_client_->UploadCard(upload_request_);
   }
 }
 
 void AutofillManager::OnDidEndTextFieldEditing() {
   external_delegate_->DidEndTextFieldEditing();
 }
 
 bool AutofillManager::IsAutofillEnabled() const {
   return ::autofill::IsAutofillEnabled(client_->GetPrefs());
@@ skipping 72 matching lines @@
     upload_request_.card = *imported_credit_card;
 
     // In order to prompt the user to upload their card, we must have both:
     //  1) Card with CVC
     //  2) 1+ recently-used or modified addresses that meet the client-side
     //     validation rules
     // Here we perform both checks before returning or logging anything,
     // because if only one of the two is missing, it may be fixable.
 
     // Check for a CVC to determine whether we can prompt the user to upload
-    // their card. If no CVC is present, do nothing. We could fall back to a
-    // local save but we believe that sometimes offering upload and sometimes
-    // offering local save is a confusing user experience.
+    // their card. If no CVC is present and the experiment is off, do nothing.
+    // We could fall back to a local save but we believe that sometimes offering
+    // upload and sometimes offering local save is a confusing user experience.

[csashi, 2017/03/31 21:48:15]

You can remove these 2 lines (rationale for these decisions outside the scope of
the code comments?)

[Jared Saul, 2017/04/01 04:18:02]

These lines were already here, they just got moved over a little.  I could
reorder them if you want, but I don't want to get rid of them entirely.

+    // If no CVC and the experiment is on, request CVC from the user in the
+    // bubble and save using the provided value.
     for (const auto& field : submitted_form) {
       if (field->Type().GetStorableType() == CREDIT_CARD_VERIFICATION_CODE &&
           IsValidCreditCardSecurityCode(field->value,
                                         upload_request_.card.type())) {
         upload_request_.cvc = field->value;
         break;
       }
     }
 
     // Upload requires that recently used or modified addresses meet the
     // client-side validation rules.
     autofill::AutofillMetrics::CardUploadDecisionMetric
         get_profiles_decision_metric = AutofillMetrics::UPLOAD_OFFERED;
     std::string rappor_metric_name;
     bool get_profiles_succeeded =
         GetProfilesForCreditCardUpload(*imported_credit_card,
                                        &upload_request_.profiles,
                                        &get_profiles_decision_metric,
                                        &rappor_metric_name);
 
     pending_upload_request_url_ = GURL(submitted_form.source_url());
 
     // Both the CVC and address checks are done.  Conform to the legacy order of
     // reporting on CVC then address.
+    upload_cvc_should_be_requested_ = false;
     if (upload_request_.cvc.empty()) {
-      AutofillMetrics::LogCardUploadDecisionMetric(
-          AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
-      LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
-      pending_upload_request_url_ = GURL();
-      CollectRapportSample(submitted_form.source_url(),
-                           "Autofill.CardUploadNotOfferedNoCvc");
-      return;
+      if (IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled()) {
+        upload_cvc_should_be_requested_ = true;
+      } else {
+        AutofillMetrics::LogCardUploadDecisionMetric(
+            AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
+        LogCardUploadDecisionUkm(AutofillMetrics::UPLOAD_NOT_OFFERED_NO_CVC);
+        pending_upload_request_url_ = GURL();
+        CollectRapportSample(submitted_form.source_url(),
+                             "Autofill.CardUploadNotOfferedNoCvc");
+        return;
+      }
     }
     if (!get_profiles_succeeded) {
       DCHECK(get_profiles_decision_metric != AutofillMetrics::UPLOAD_OFFERED);
       AutofillMetrics::LogCardUploadDecisionMetric(
           get_profiles_decision_metric);
       LogCardUploadDecisionUkm(get_profiles_decision_metric);
       pending_upload_request_url_ = GURL();
       if (!rappor_metric_name.empty()) {
         CollectRapportSample(submitted_form.source_url(), rappor_metric_name);
       }
@@ skipping 951 matching lines @@
 }
 #endif  // ENABLE_FORM_DEBUG_DUMP
 
 void AutofillManager::LogCardUploadDecisionUkm(
     AutofillMetrics::CardUploadDecisionMetric upload_decision) {
   AutofillMetrics::LogCardUploadDecisionUkm(
       client_->GetUkmService(), pending_upload_request_url_, upload_decision);
 }
 
 }  // namespace autofill