diff --git a/components/autofill/core/browser/autofill_form_test_utils.cc b/components/autofill/core/browser/autofill_form_test_utils.cc index 3acab8e9ef3e26..c58f17db0c2956 100644 --- a/components/autofill/core/browser/autofill_form_test_utils.cc +++ b/components/autofill/core/browser/autofill_form_test_utils.cc @@ -115,6 +115,7 @@ FormData GetFormData(const FormDataDescription& d) { ff.is_autofilled = dd.is_autofilled.value_or(false); ff.origin = dd.origin.value_or(f.main_frame_origin); ff.should_autocomplete = dd.should_autocomplete; + ff.properties_mask = dd.properties_mask; f.fields.push_back(ff); } return f; diff --git a/components/autofill/core/browser/autofill_form_test_utils.h b/components/autofill/core/browser/autofill_form_test_utils.h index 8da4714b0f9b19..498bfdff9ab7ad 100644 --- a/components/autofill/core/browser/autofill_form_test_utils.h +++ b/components/autofill/core/browser/autofill_form_test_utils.h @@ -47,6 +47,7 @@ struct FieldDataDescription { absl::optional is_autofilled; absl::optional origin; std::vector select_options = {}; + FieldPropertiesMask properties_mask = 0; }; // Attributes provided to the test form. diff --git a/components/autofill/core/browser/browser_autofill_manager.cc b/components/autofill/core/browser/browser_autofill_manager.cc index 1df3392a96b75d..8d9e064187cc26 100644 --- a/components/autofill/core/browser/browser_autofill_manager.cc +++ b/components/autofill/core/browser/browser_autofill_manager.cc @@ -695,8 +695,12 @@ void BrowserAutofillManager::OnFormSubmittedImpl(const FormData& form, form_for_autocomplete.fields[i].should_autocomplete = false; } + // If the field was edited by the user and there existed an autofillable + // value for the field, log whether the value on submission is same as the + // autofillable value. if (submitted_form->field(i) - ->value_not_autofilled_over_existing_value_hash()) { + ->value_not_autofilled_over_existing_value_hash() && + (submitted_form->field(i)->properties_mask & kUserTyped)) { // Compare and record if the currently filled value is same as the // non-empty value that was to be autofilled in the field. AutofillMetrics:: @@ -1922,6 +1926,8 @@ void BrowserAutofillManager::FillOrPreviewDataModelForm( // case. if (base::FeatureList::IsEnabled( features::kAutofillPreventOverridingPrefilledValues)) { + form_structure->field(i) + ->set_value_not_autofilled_over_existing_value_hash(absl::nullopt); if (form.fields[i].form_control_type != "select-one" && !form.fields[i].value.empty() && !has_override && !FormFieldData::DeepEqual(form.fields[i], field)) { @@ -1933,18 +1939,16 @@ void BrowserAutofillManager::FillOrPreviewDataModelForm( optional_cvc ? *optional_cvc : kEmptyCvc, action, &unused_failure_to_fill); if (action == mojom::RendererFormDataAction::kFill && - !fill_value.empty() && fill_value != form.fields[i].value) { - // Save the value that was supposed to be autofilled for this field. + !fill_value.empty() && fill_value != form.fields[i].value && + !form_structure->field(i)->value.empty()) { + // Save the value that was supposed to be autofilled for this field if + // the field contained an initial value. form_structure->field(i) ->set_value_not_autofilled_over_existing_value_hash( base::FastHash(base::UTF16ToUTF8(fill_value))); } continue; } - - // Clear out the value in case the autofill happens for the field. - form_structure->field(i) - ->set_value_not_autofilled_over_existing_value_hash(absl::nullopt); } // Do not fill fields that have been edited by the user, except if the field diff --git a/components/autofill/core/browser/metrics/autofill_metrics.cc b/components/autofill/core/browser/metrics/autofill_metrics.cc index a02cffe11b510d..ef58aa90c1f01f 100644 --- a/components/autofill/core/browser/metrics/autofill_metrics.cc +++ b/components/autofill/core/browser/metrics/autofill_metrics.cc @@ -3598,7 +3598,7 @@ void AutofillMetrics::LogOtpInputDialogNewOtpRequested() { void AutofillMetrics:: LogIsValueNotAutofilledOverExistingValueSameAsSubmittedValue(bool is_same) { base::UmaHistogramBoolean( - "Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue", + "Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2", is_same); } diff --git a/components/autofill/core/browser/metrics/autofill_metrics_unittest.cc b/components/autofill/core/browser/metrics/autofill_metrics_unittest.cc index 0ec290430306ed..d4e08254d97b98 100644 --- a/components/autofill/core/browser/metrics/autofill_metrics_unittest.cc +++ b/components/autofill/core/browser/metrics/autofill_metrics_unittest.cc @@ -11863,22 +11863,22 @@ TEST_F(AutofillMetricsTest, AutofilledStateFieldSource) { // Tests the following 4 cases when |kAutofillPreventOverridingPrefilledValues| // is enabled: -// 1. The field is not autofilled since it has a prefilled value but the value +// 1. The field is not autofilled since it has an initial value but the value // is edited before the form submission and is same as the value that was // to be autofilled in the field. -// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue| +// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2| // should emit true for this case. -// 2. The field is not autofilled since it has a prefilled value but the value +// 2. The field is not autofilled since it has an initial value but the value // is edited before the form submission and is different than the value that // was to be autofilled in the field. -// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue| +// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2| // should emit false for this case. -// 3. The field had a prefilled value that was similar to the value to be +// 3. The field had an initial value that was similar to the value to be // autofilled in the field. -// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue| +// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2| // should not record anything in this case. // 4. Selection fields are always overridden by Autofill. -// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue| +// |Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2| // should not record anything in this case. TEST_F(AutofillMetricsTest, IsValueNotAutofilledOverExistingValueSameAsSubmittedValue) { @@ -11889,20 +11889,23 @@ TEST_F(AutofillMetricsTest, FormData form = test::GetFormData( {.description_for_logging = "AutofilledStateFieldSource", - .fields = {{.role = ServerFieldType::NAME_FULL}, - {.role = ServerFieldType::ADDRESS_HOME_CITY, - .value = u"Sacremento"}, // Case #1 - {.role = ServerFieldType::ADDRESS_HOME_STATE, - .value = u"CA", - .form_control_type = "select-one", - .select_options = {{u"TN", u"Tennesse"}, - {u"CA", u"California"}, - {u"WA", u"Washington DC"}}}, // Case #4 - {.role = ServerFieldType::ADDRESS_HOME_ZIP, - .value = u"00000"}, // Case #2 - {.role = ServerFieldType::PHONE_HOME_WHOLE_NUMBER, - .value = u"12345678901"}, // Case #3 - {.role = ServerFieldType::ADDRESS_HOME_COUNTRY}}}); + .fields = { + {.role = ServerFieldType::NAME_FULL}, + {.role = ServerFieldType::ADDRESS_HOME_CITY, + .value = u"Sacremento", + .properties_mask = FieldPropertiesFlags::kUserTyped}, // Case #1 + {.role = ServerFieldType::ADDRESS_HOME_STATE, + .value = u"CA", + .form_control_type = "select-one", + .select_options = {{u"TN", u"Tennesse"}, + {u"CA", u"California"}, + {u"WA", u"Washington DC"}}}, // Case #4 + {.role = ServerFieldType::ADDRESS_HOME_ZIP, + .value = u"00000", + .properties_mask = FieldPropertiesFlags::kUserTyped}, // Case #2 + {.role = ServerFieldType::PHONE_HOME_WHOLE_NUMBER, + .value = u"12345678901"}, // Case #3 + {.role = ServerFieldType::ADDRESS_HOME_COUNTRY}}}); std::vector heuristic_types = { NAME_FULL, ADDRESS_HOME_CITY, ADDRESS_HOME_STATE, @@ -11912,7 +11915,8 @@ TEST_F(AutofillMetricsTest, ADDRESS_HOME_ZIP, PHONE_HOME_WHOLE_NUMBER, ADDRESS_HOME_COUNTRY}; // Simulate having seen this form on page load. - autofill_manager().AddSeenForm(form, heuristic_types, server_types); + autofill_manager().AddSeenForm(form, heuristic_types, server_types, + /*preserve_values_in_form_structure=*/true); autofill_manager().OnAskForValuesToFillTest(form, form.fields[0]); autofill_manager().DidShowSuggestions( @@ -11942,10 +11946,10 @@ TEST_F(AutofillMetricsTest, SubmissionSource::FORM_SUBMISSION); histogram_tester.ExpectBucketCount( - "Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue", + "Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2", true, 1); histogram_tester.ExpectBucketCount( - "Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue", + "Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2", false, 1); } diff --git a/components/autofill/core/browser/test_browser_autofill_manager.cc b/components/autofill/core/browser/test_browser_autofill_manager.cc index 92cf287120fcbf..da912c15ac07b6 100644 --- a/components/autofill/core/browser/test_browser_autofill_manager.cc +++ b/components/autofill/core/browser/test_browser_autofill_manager.cc @@ -107,7 +107,8 @@ int TestBrowserAutofillManager::GetPackedCreditCardID(int credit_card_id) { void TestBrowserAutofillManager::AddSeenForm( const FormData& form, const std::vector& heuristic_types, - const std::vector& server_types) { + const std::vector& server_types, + bool preserve_values_in_form_structure) { std::vector>> all_heuristic_types; @@ -118,21 +119,24 @@ void TestBrowserAutofillManager::AddSeenForm( return {{GetActivePatternSource(), type}}; }); - AddSeenForm(form, all_heuristic_types, server_types); + AddSeenForm(form, all_heuristic_types, server_types, + preserve_values_in_form_structure); } void TestBrowserAutofillManager::AddSeenForm( const FormData& form, const std::vector>>& heuristic_types, - const std::vector& server_types) { - FormData empty_form = form; - for (auto& field : empty_form.fields) { + const std::vector& server_types, + bool preserve_values_in_form_structure) { + FormData form_with_empty_fields = form; + for (auto& field : form_with_empty_fields.fields) { field.value = std::u16string(); } std::unique_ptr form_structure = - std::make_unique(empty_form); + std::make_unique( + preserve_values_in_form_structure ? form : form_with_empty_fields); form_structure->SetFieldTypes(heuristic_types, server_types); form_structure->identify_sections_for_testing(); AddSeenFormStructure(std::move(form_structure)); diff --git a/components/autofill/core/browser/test_browser_autofill_manager.h b/components/autofill/core/browser/test_browser_autofill_manager.h index 422bf0e0d492fd..dcd782523aa3ad 100644 --- a/components/autofill/core/browser/test_browser_autofill_manager.h +++ b/components/autofill/core/browser/test_browser_autofill_manager.h @@ -58,13 +58,15 @@ class TestBrowserAutofillManager : public BrowserAutofillManager { void AddSeenForm(const FormData& form, const std::vector& heuristic_types, - const std::vector& server_types); + const std::vector& server_types, + bool preserve_values_in_form_structure = false); void AddSeenForm( const FormData& form, const std::vector>>& heuristic_types, - const std::vector& server_types); + const std::vector& server_types, + bool preserve_values_in_form_structure = false); void AddSeenFormStructure(std::unique_ptr form_structure); diff --git a/tools/metrics/histograms/metadata/autofill/histograms.xml b/tools/metrics/histograms/metadata/autofill/histograms.xml index 6a993f50e68d73..4298cec04419c0 100644 --- a/tools/metrics/histograms/metadata/autofill/histograms.xml +++ b/tools/metrics/histograms/metadata/autofill/histograms.xml @@ -1730,6 +1730,10 @@ chromium-metrics-reviews@google.com. + + Deprecated in July 2022. Subsumed by + Autofill.IsValueNotAutofilledOverExistingValueSameAsSubmittedValue2. + vidhanj@google.com koerber@google.com chrome-autofill-alerts@google.com @@ -1741,6 +1745,20 @@ chromium-metrics-reviews@google.com. + + vidhanj@google.com + koerber@google.com + chrome-autofill-alerts@google.com + + This metric is recorderd on form submission for each field that had an + initial value on page load and was edited by the user later on. The result + of the equality comparison between the submitted field value and supposedly + autofillable value is emitted by this metric. + + + battre@chromium.org