Fix a crash when using OptionalBooleanToBool on undefined value

if we type in a field which has not been autofilled, |was_autofilled| is
undefined, so |filled_value_was_modified| is also undefined. For
|filled_value_was_modified| we should set it in both filling and
typing cases, we want to set it to be true when filling happens first
and the user types into the field, if autofill happens without a user
editing to it, it should be false.

(cherry picked from commit 35c2078)

Bug: 1410898
Change-Id: I6c37b82c02fb85d7f647ff88ca4d795b3b7b986d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4201202
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1098530}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4209028
Auto-Submit: Lan Wei <lanwei@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Lan Wei <lanwei@chromium.org>
Cr-Commit-Position: refs/branch-heads/5563@{#55}
Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}

components/autofill/core/browser/metrics/autofill_metrics.cc

@@ -2576,12 +2576,19 @@ void AutofillMetrics::FormInteractionsUkmLogger::
       had_value_before_filling |= event->had_value_before_filling;
       autofill_skipped_status.insert(event->autofill_skipped_status);
       had_value_after_filling = event->had_value_after_filling;
+      if (was_autofilled == OptionalBoolean::kTrue &&
+          filled_value_was_modified == OptionalBoolean::kUndefined) {
+        // Only switch from unknown to false on the first filling.
+        filled_value_was_modified = OptionalBoolean::kFalse;
+      }
       ++autofill_count;
     }
 
     if (auto* event = absl::get_if<TypingFieldLogEvent>(&log_event)) {
       user_typed_into_field = OptionalBoolean::kTrue;
-      filled_value_was_modified |= was_autofilled;
+      if (was_autofilled == OptionalBoolean::kTrue) {
+        filled_value_was_modified = OptionalBoolean::kTrue;
+      }
       has_value_after_typing = event->has_value_after_typing;
     }
 
@@ -2674,9 +2681,12 @@ void AutofillMetrics::FormInteractionsUkmLogger::
   }
 
   if (user_typed_into_field == OptionalBoolean::kTrue) {
-    builder.SetUserTypedIntoField(OptionalBooleanToBool(user_typed_into_field))
-        .SetFilledValueWasModified(
-            OptionalBooleanToBool(filled_value_was_modified));
+    builder.SetUserTypedIntoField(OptionalBooleanToBool(user_typed_into_field));
+  }
+
+  if (filled_value_was_modified != OptionalBoolean::kUndefined) {
+    builder.SetFilledValueWasModified(
+        OptionalBooleanToBool(filled_value_was_modified));
   }
 
   if (had_typed_or_filled_value_at_submission != OptionalBoolean::kUndefined) {

components/autofill/core/browser/metrics/autofill_metrics_unittest.cc

@@ -10588,6 +10588,7 @@ TEST_F(AutofillMetricsFromLogEventsTest, AddressSubmittedFormLogEvents) {
            DenseSet<SkipStatus>{SkipStatus::kNotSkipped}.to_uint64()},
           {UFIT::kWasRefillName, false},
           {UFIT::kHadValueBeforeFillingName, false},
+          {UFIT::kFilledValueWasModifiedName, false},
           {UFIT::kHadTypedOrFilledValueAtSubmissionName, true},
       };
       if (i == 0) {
@@ -10608,7 +10609,7 @@ TEST_F(AutofillMetricsFromLogEventsTest, AddressSubmittedFormLogEvents) {
 
 // Test if we have recorded UKM metrics correctly about field types after
 // parsing the form by the local heuristic prediction.
-TEST_F(AutofillMetricsFromLogEventsTest, AutofillFieldInfoMetrics_FieldType) {
+TEST_F(AutofillMetricsFromLogEventsTest, AutofillFieldInfoMetricsFieldType) {
   FormData form = CreateForm(
       {// Heuristic value will match with Autocomplete attribute.
        CreateField("Last Name", "lastname", "", "text", "family-name"),
@@ -10718,6 +10719,64 @@ TEST_F(AutofillMetricsFromLogEventsTest, AutofillFieldInfoMetrics_FieldType) {
   }
 }
 
+// Test if we have recorded FieldInfo UKM metrics correctly after typing in
+// fields without autofilling first.
+TEST_F(AutofillMetricsFromLogEventsTest,
+       AutofillFieldInfoMetricsEditedFieldWithoutFill) {
+  test::FormDescription form_description = {
+      .description_for_logging = "NumberOfAutofilledFields",
+      .fields = {{.role = NAME_FULL,
+                  .value = u"Elvis Aaron Presley",
+                  .is_autofilled = false},
+                 {.role = EMAIL_ADDRESS,
+                  .value = u"buddy@gmail.com",
+                  .is_autofilled = false},
+                 {.role = PHONE_HOME_CITY_AND_NUMBER, .is_autofilled = true}},
+      .unique_renderer_id = test::MakeFormRendererId(),
+      .main_frame_origin =
+          url::Origin::Create(autofill_client_->form_origin())};
+
+  FormData form = GetAndAddSeenForm(form_description);
+
+  base::HistogramTester histogram_tester;
+  // Simulate text input in the first and second fields.
+  SimulateUserChangedTextField(form, form.fields[0]);
+  SimulateUserChangedTextField(form, form.fields[1]);
+
+  SubmitForm(form);
+
+  // Record Autofill2.FieldInfo UKM event at autofill manager reset.
+  autofill_manager().Reset();
+
+  auto entries =
+      test_ukm_recorder_->GetEntriesByName(UkmFieldInfoType::kEntryName);
+  ASSERT_EQ(2u, entries.size());
+
+  for (size_t i = 0; i < entries.size(); ++i) {
+    SCOPED_TRACE(testing::Message() << i);
+    using UFIT = UkmFieldInfoType;
+    const auto* const entry = entries[i];
+
+    std::map<std::string, int64_t> expected = {
+        {UFIT::kFormSessionIdentifierName,
+         AutofillMetrics::FormGlobalIdToHash64Bit(form.global_id())},
+        {UFIT::kFieldSessionIdentifierName,
+         AutofillMetrics::FieldGlobalIdToHash64Bit(form.fields[i].global_id())},
+        {UFIT::kFieldSignatureName,
+         Collapse(CalculateFieldSignatureForField(form.fields[i])).value()},
+        {UFIT::kWasFocusedName, false},
+        {UFIT::kIsFocusableName, true},
+        {UFIT::kUserTypedIntoFieldName, true},
+        {UFIT::kHadTypedOrFilledValueAtSubmissionName, true},
+    };
+
+    EXPECT_EQ(expected.size(), entry->metrics.size());
+    for (const auto& [metric, value] : expected) {
+      test_ukm_recorder_->ExpectEntryMetric(entry, metric, value);
+    }
+  }
+}
+
 // TODO(crbug.com/1352826) Delete this after collecting the metrics.
 struct LaxLocalHeuristicsTestCase {
   test::FormDescription form;