[Autofill] Fix local save metric for dynamic and accordion form

Set the |from_dynamic_change_form| and |has_non_focusable_field| in SaveCreditCardOptions when Chrome shows local save prompt to user from a dynamic/accordion form so that sub-histogram can be logged correctly.

We add tests to ensure these two fields are correctly populated in SaveCreditCardOptions when a user was prompted local-save/upload from a dynamic/accordion form. The tests for the local save sub-histogram were already introduced in save_card_bubble_controller_impl_unittest.cc

Bug: 989771
Change-Id: I286416e2d8a4f90f2e5740658cf414531ba0a574
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1730853
Commit-Queue: Sujie Zhu <sujiezhu@google.com>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#684363}

components/autofill/core/browser/form_data_importer.cc

@@ -475,6 +475,8 @@ CreditCard FormDataImporter::ExtractCreditCardFromForm(
     const FormStructure& form,
     bool* has_duplicate_field_type) {
   *has_duplicate_field_type = false;
+  has_non_focusable_field_ = false;
+  from_dynamic_change_form_ = false;
 
   CreditCard candidate_credit_card;
 

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

@@ -421,8 +421,10 @@ void CreditCardSaveManager::OfferCardLocalSave() {
       observer_for_testing_->OnOfferLocalSave();
     client_->ConfirmSaveCreditCardLocally(
         local_card_save_candidate_,
-        AutofillClient::SaveCreditCardOptions().with_show_prompt(
-            show_save_prompt_.value_or(true)),
+        AutofillClient::SaveCreditCardOptions()
+            .with_show_prompt(show_save_prompt_.value_or(true))
+            .with_from_dynamic_change_form(from_dynamic_change_form_)
+            .with_has_non_focusable_field(has_non_focusable_field_),
         base::BindOnce(&CreditCardSaveManager::OnUserDidDecideOnLocalSave,
                        weak_ptr_factory_.GetWeakPtr()));
 

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

@@ -759,11 +759,350 @@ TEST_F(CreditCardSaveManagerTest, LocalCreditCard_WithNonFocusableField) {
   credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
   credit_card_form.fields[5].value = ASCIIToUTF16("123");
 
-  base::HistogramTester histogram_tester;
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+}
+
+// Tests that |has_non_focusable_field| is correctly sent to AutofillClient when
+// offering local save.
+TEST_F(CreditCardSaveManagerTest,
+       Local_UploadDisabled_SaveCreditCardOptions_WithNonFocusableField) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(false);
+
+  // Set up our credit card form data with non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions()
+                                   .with_split_names(true)
+                                   .with_is_from_non_focusable_form(true));
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
 
   FormSubmitted(credit_card_form);
   EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
   EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().has_non_focusable_field);
+}
+
+// Tests that |has_non_focusable_field| is correctly sent to AutofillClient when
+// upload failed because of unsupported bin and falling back to local save.
+TEST_F(CreditCardSaveManagerTest,
+       Local_UnsupportedCard_SaveCreditCardOptions_WithNonFocusableField) {
+  scoped_feature_list_.InitAndEnableFeature(
+      features::kAutofillDoNotUploadSaveUnsupportedCards);
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+  // Set supported bin range so that the used card is unsupported.
+  std::vector<std::pair<int, int>> supported_card_bin_ranges{
+      std::make_pair(34, 34), std::make_pair(300, 305)};
+  payments_client_->SetSupportedBINRanges(supported_card_bin_ranges);
+
+  // Set up our credit card form data with non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions()
+                                   .with_split_names(true)
+                                   .with_is_from_non_focusable_form(true));
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().has_non_focusable_field);
+}
+
+// Tests that |has_non_focusable_field| is correctly sent to AutofillClient when
+// GetDetailsForSaveCard failed and falling back to local save.
+TEST_F(
+    CreditCardSaveManagerTest,
+    Local_GetDetailsForSaveCardFails_SaveCreditCardOptions_WithNonFocusableField) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+  // Anything other than "en-US" will cause GetUploadDetails to return a failure
+  // response.
+  credit_card_save_manager_->SetAppLocale("pt-BR");
+
+  // Create, fill and submit an address form in order to establish a recent
+  // profile which can be selected for the upload request.
+  FormData address_form;
+  test::CreateTestAddressFormData(&address_form);
+  FormsSeen(std::vector<FormData>(1, address_form));
+  ExpectUniqueFillableFormParsedUkm();
+
+  ManuallyFillAddressForm("Flo", "Master", "77401", "US", &address_form);
+  FormSubmitted(address_form);
+
+  // Set up our credit card form data with non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions()
+                                   .with_split_names(true)
+                                   .with_is_from_non_focusable_form(true));
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().has_non_focusable_field);
+}
+
+// Tests that |has_non_focusable_field| is correctly sent to AutofillClient when
+// offering upload save.
+TEST_F(CreditCardSaveManagerTest,
+       Upload_SaveCreditCardOptions_WithNonFocusableField) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+
+  // Create, fill and submit an address form in order to establish a recent
+  // profile which can be selected for the upload request.
+  FormData address_form;
+  test::CreateTestAddressFormData(&address_form);
+  FormsSeen(std::vector<FormData>(1, address_form));
+  ExpectUniqueFillableFormParsedUkm();
+
+  ManuallyFillAddressForm("Flo", "Master", "77401", "US", &address_form);
+  FormSubmitted(address_form);
+
+  // Set up our credit card form data with non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions()
+                                   .with_split_names(true)
+                                   .with_is_from_non_focusable_form(true));
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().has_non_focusable_field);
+}
+
+// Tests that |has_non_focusable_field| is not sent to AutofillClient when the
+// form does not have any non-focusable fields.
+TEST_F(CreditCardSaveManagerTest,
+       SaveCreditCardOptions_WithoutNonFocusableField) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(false);
+
+  // Set up our credit card form data without any non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions().with_split_names(true));
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_FALSE(
+      autofill_client_.get_save_credit_card_options().has_non_focusable_field);
+}
+
+// Tests that |from_dynamic_change_form| is correctly sent to AutofillClient
+// when offering local save.
+TEST_F(CreditCardSaveManagerTest,
+       Local_UploadDisabled_SaveCreditCardOptions_WithDynamicForms) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(false);
+
+  // Set up our credit card form data without any non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions().with_split_names(true));
+  // Use the two same forms for FormsSeen to mock the dynamic change forms.
+  FormsSeen(std::vector<FormData>(2, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().from_dynamic_change_form);
+}
+
+// Tests that |from_dynamic_change_form| is correctly sent to AutofillClient
+// when upload failed because of unsupported bin and falling back to local save.
+TEST_F(CreditCardSaveManagerTest,
+       Local_UnsupportedCard_SaveCreditCardOptions_WithDynamicForms) {
+  scoped_feature_list_.InitAndEnableFeature(
+      features::kAutofillDoNotUploadSaveUnsupportedCards);
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+  // Set supported bin range so that the used card is unsupported.
+  std::vector<std::pair<int, int>> supported_card_bin_ranges{
+      std::make_pair(34, 34), std::make_pair(300, 305)};
+  payments_client_->SetSupportedBINRanges(supported_card_bin_ranges);
+
+  // Set up our credit card form data without any non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions().with_split_names(true));
+  // Use the two same forms for FormsSeen to mock the dynamic change forms.
+  FormsSeen(std::vector<FormData>(2, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().from_dynamic_change_form);
+}
+
+// Tests that |from_dynamic_change_form| is correctly sent to AutofillClient
+// when GetDetailsForSaveCard failed and falling back to local save.
+TEST_F(
+    CreditCardSaveManagerTest,
+    Local_GetDetailsForSaveCardFails_SaveCreditCardOptions_WithDynamicForms) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+  // Anything other than "en-US" will cause GetUploadDetails to return a failure
+  // response.
+  credit_card_save_manager_->SetAppLocale("pt-BR");
+
+  // Create, fill and submit an address form in order to establish a recent
+  // profile which can be selected for the upload request.
+  FormData address_form;
+  test::CreateTestAddressFormData(&address_form);
+  FormsSeen(std::vector<FormData>(1, address_form));
+  ExpectUniqueFillableFormParsedUkm();
+
+  ManuallyFillAddressForm("Flo", "Master", "77401", "US", &address_form);
+  FormSubmitted(address_form);
+
+  // Set up our credit card form data without any non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions().with_split_names(true));
+  // Use the two same forms for FormsSeen to mock the dynamic change forms.
+  FormsSeen(std::vector<FormData>(2, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().from_dynamic_change_form);
+}
+
+// Tests that |from_dynamic_change_form| is correctly sent to AutofillClient
+// when offering upload save.
+TEST_F(CreditCardSaveManagerTest,
+       Upload_SaveCreditCardOptions_WithDynamicForms) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+
+  // Create, fill and submit an address form in order to establish a recent
+  // profile which can be selected for the upload request.
+  FormData address_form;
+  test::CreateTestAddressFormData(&address_form);
+  FormsSeen(std::vector<FormData>(1, address_form));
+  ExpectUniqueFillableFormParsedUkm();
+
+  ManuallyFillAddressForm("Flo", "Master", "77401", "US", &address_form);
+  FormSubmitted(address_form);
+
+  // Set up our credit card form data without any non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions().with_split_names(true));
+  // Use the two same forms for FormsSeen to mock the dynamic change forms.
+  FormsSeen(std::vector<FormData>(2, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_TRUE(
+      autofill_client_.get_save_credit_card_options().from_dynamic_change_form);
+}
+
+// Tests that |from_dynamic_change_form| is not sent to AutofillClient when the
+// form is not dynamically changing.
+TEST_F(CreditCardSaveManagerTest, SaveCreditCardOptions_WithoutDynamicForms) {
+  credit_card_save_manager_->SetCreditCardUploadEnabled(false);
+
+  // Set up our credit card form data without any non_focusable form field.
+  FormData credit_card_form;
+  CreateTestCreditCardFormData(&credit_card_form,
+                               CreditCardFormOptions().with_split_names(true));
+  // Only using one form for FormsSeen will not be treated as dynamic change
+  // form.
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  credit_card_form.fields[0].value = ASCIIToUTF16("Flo");
+  credit_card_form.fields[1].value = ASCIIToUTF16("Master");
+  credit_card_form.fields[2].value = ASCIIToUTF16("4111111111111111");
+  credit_card_form.fields[3].value = ASCIIToUTF16(NextMonth());
+  credit_card_form.fields[4].value = ASCIIToUTF16(NextYear());
+  credit_card_form.fields[5].value = ASCIIToUTF16("123");
+
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(autofill_client_.ConfirmSaveCardLocallyWasCalled());
+  EXPECT_FALSE(credit_card_save_manager_->CreditCardWasUploaded());
+  EXPECT_FALSE(
+      autofill_client_.get_save_credit_card_options().from_dynamic_change_form);
 }
 
 // Tests that a credit card inferred from a form with a credit card first and

components/autofill/core/browser/test_autofill_client.cc

@@ -121,6 +121,7 @@ void TestAutofillClient::ConfirmSaveCreditCardLocally(
     LocalSaveCardPromptCallback callback) {
   confirm_save_credit_card_locally_called_ = true;
   offer_to_save_credit_card_bubble_was_shown_ = options.show_prompt;
+  save_credit_card_options_ = options;
   std::move(callback).Run(AutofillClient::ACCEPTED);
 }
 
@@ -148,6 +149,7 @@ void TestAutofillClient::ConfirmSaveCreditCardToCloud(
     SaveCreditCardOptions options,
     UploadSaveCardPromptCallback callback) {
   offer_to_save_credit_card_bubble_was_shown_ = options.show_prompt;
+  save_credit_card_options_ = options;
   std::move(callback).Run(AutofillClient::ACCEPTED, {});
 }
 

components/autofill/core/browser/test_autofill_client.h

@@ -159,6 +159,10 @@ class TestAutofillClient : public AutofillClient {
     return offer_to_save_credit_card_bubble_was_shown_.value();
   }
 
+  SaveCreditCardOptions get_save_credit_card_options() {
+    return save_credit_card_options_.value();
+  }
+
   MockAutocompleteHistoryManager* GetMockAutocompleteHistoryManager() {
     return &mock_autocomplete_history_manager_;
   }
@@ -200,6 +204,9 @@ class TestAutofillClient : public AutofillClient {
   // otherwise.
   base::Optional<bool> credit_card_name_fix_flow_bubble_was_shown_;
 
+  // Populated if local save or upload was offered.
+  base::Optional<SaveCreditCardOptions> save_credit_card_options_;
+
   std::vector<std::string> migration_card_selection_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAutofillClient);