[Autofill] Splits logging of SuggestionAcceptedIndex by form type.

The Autofill.SuggestionAcceptIndex metric encompasses credit card and
address forms. Note that "address forms" is generally used to refer to
forms that use data from AutofillProfiles--including forms without
addresses, e.g. a sign-up form with name, phone, and email fields.

The present logging does not allow us to answer form-specific questions
such as "What percentage of users select one of the first three
suggestions for credit card forms?" "For address forms?"

Given our findings about the differences between the number of credit
cards and the number of profiles that users have, which influences the
number of suggestions we show in each context, and hence, the suggestion
accepted index, it would be more useful to split this metric by form
type:

(1) CreditCard
(2) Profile
(3) Other

Please refer to the findings in the table on page 3 (Googlers only):
https://docs.google.com/document/d/1qU7jJ6DmNgC1WLG9HrxGdgDoPWmQSAi9mAldIy9X8rU/edit

Bug: 964422
Change-Id: I422af5fd995b4032ec3bc931697e43a8eac1cab9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617092
Auto-Submit: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Nik Bhagat <nikunjb@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Cr-Commit-Position: refs/heads/master@{#665992}

components/autofill/core/browser/autofill_external_delegate.cc

@@ -228,7 +228,8 @@ void AutofillExternalDelegate::DidAcceptSuggestion(const base::string16& value,
     manager_->OnUserAcceptedCardsFromAccountOption();
   } else {
     if (identifier > 0)  // Denotes an Autofill suggestion.
-      AutofillMetrics::LogAutofillSuggestionAcceptedIndex(position);
+      AutofillMetrics::LogAutofillSuggestionAcceptedIndex(position,
+                                                          popup_type_);
 
     FillAutofillFormData(identifier, false);
   }

components/autofill/core/browser/autofill_external_delegate_unittest.cc

@@ -595,8 +595,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
                                           0);
 }
 
-// Test that an accepted autofill suggestion will fill the form and log the
-// proper metric.
+// Test that an accepted autofill suggestion will fill the form.
 TEST_F(AutofillExternalDelegateUnitTest,
        ExternalDelegateAcceptAutofillSuggestion) {
   EXPECT_CALL(autofill_client_, HideAutofillPopup());
@@ -605,11 +604,9 @@ TEST_F(AutofillExternalDelegateUnitTest,
               FillOrPreviewForm(
                   AutofillDriver::FORM_DATA_ACTION_FILL, _, _, _,
                   kAutofillProfileId));
-  base::HistogramTester histogram;
   external_delegate_->DidAcceptSuggestion(dummy_string,
                                           kAutofillProfileId,
                                           2);  // Row 2
-  histogram.ExpectUniqueSample("Autofill.SuggestionAcceptedIndex", 2, 1);
 }
 
 // Test that the driver is directed to clear the form after being notified that

components/autofill/core/browser/autofill_metrics.cc

@@ -1459,10 +1459,23 @@ void AutofillMetrics::LogAddressSuggestionsCount(size_t num_suggestions) {
 }
 
 // static
-void AutofillMetrics::LogAutofillSuggestionAcceptedIndex(int index) {
+void AutofillMetrics::LogAutofillSuggestionAcceptedIndex(int index,
+                                                         PopupType popup_type) {
   base::UmaHistogramSparse("Autofill.SuggestionAcceptedIndex",
                            std::min(index, kMaxBucketsCount));
 
+  if (popup_type == PopupType::kCreditCards) {
+    base::UmaHistogramSparse("Autofill.SuggestionAcceptedIndex.CreditCard",
+                             std::min(index, kMaxBucketsCount));
+  } else if (popup_type == PopupType::kAddresses ||
+             popup_type == PopupType::kPersonalInformation) {
+    base::UmaHistogramSparse("Autofill.SuggestionAcceptedIndex.Profile",
+                             std::min(index, kMaxBucketsCount));
+  } else {
+    base::UmaHistogramSparse("Autofill.SuggestionAcceptedIndex.Other",
+                             std::min(index, kMaxBucketsCount));
+  }
+
   base::RecordAction(base::UserMetricsAction("Autofill_SelectedSuggestion"));
 }
 

components/autofill/core/browser/autofill_metrics.h

@@ -20,6 +20,7 @@
 #include "components/autofill/core/browser/form_types.h"
 #include "components/autofill/core/browser/metrics/form_events.h"
 #include "components/autofill/core/browser/sync_utils.h"
+#include "components/autofill/core/browser/ui/popup_types.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/signatures_util.h"
 #include "components/security_state/core/security_state.h"
@@ -1159,7 +1160,8 @@ class AutofillMetrics {
   static void LogAddressSuggestionsCount(size_t num_suggestions);
 
   // Log the index of the selected Autofill suggestion in the popup.
-  static void LogAutofillSuggestionAcceptedIndex(int index);
+  static void LogAutofillSuggestionAcceptedIndex(int index,
+                                                 PopupType popup_type);
 
   // Logs that the user cleared the form.
   static void LogAutofillFormCleared();

components/autofill/core/browser/autofill_metrics_unittest.cc

@@ -38,6 +38,7 @@
 #include "components/autofill/core/browser/test_form_structure.h"
 #include "components/autofill/core/browser/test_personal_data_manager.h"
 #include "components/autofill/core/browser/ui/popup_item_ids.h"
+#include "components/autofill/core/browser/ui/popup_types.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/autofill/core/common/autofill_clock.h"
 #include "components/autofill/core/common/autofill_features.h"
@@ -63,6 +64,7 @@ using base::ASCIIToUTF16;
 using base::Bucket;
 using base::TimeTicks;
 using ::testing::ElementsAre;
+using ::testing::HasSubstr;
 using ::testing::Matcher;
 using ::testing::UnorderedPointwise;
 
@@ -8606,6 +8608,59 @@ TEST_F(AutofillMetricsTest, LogAutocompleteSuggestionAcceptedIndex_IndexCap) {
       /*expected_count=*/1);
 }
 
+TEST_F(AutofillMetricsTest, LogSuggestionAcceptedIndex_CreditCard) {
+  const int index = 2;
+  const PopupType popup_type = PopupType::kCreditCards;
+
+  base::HistogramTester histogram_tester;
+  AutofillMetrics::LogAutofillSuggestionAcceptedIndex(index, popup_type);
+  histogram_tester.ExpectUniqueSample(
+      "Autofill.SuggestionAcceptedIndex.CreditCard", index, 1);
+
+  const std::string histograms = histogram_tester.GetAllHistogramsRecorded();
+  EXPECT_THAT(
+      histograms,
+      Not(AnyOf(HasSubstr("Autofill.SuggestionAcceptedIndex.Other"),
+                HasSubstr("Autofill.SuggestionAcceptedIndex.Profile"))));
+}
+
+TEST_F(AutofillMetricsTest, LogSuggestionAcceptedIndex_Profile) {
+  const int index = 1;
+  const PopupType popup_type1 = PopupType::kPersonalInformation;
+  const PopupType popup_type2 = PopupType::kAddresses;
+
+  base::HistogramTester histogram_tester;
+  AutofillMetrics::LogAutofillSuggestionAcceptedIndex(index, popup_type1);
+  AutofillMetrics::LogAutofillSuggestionAcceptedIndex(index, popup_type2);
+  histogram_tester.ExpectUniqueSample(
+      "Autofill.SuggestionAcceptedIndex.Profile", index, 2);
+
+  const std::string histograms = histogram_tester.GetAllHistogramsRecorded();
+  EXPECT_THAT(
+      histograms,
+      Not(AnyOf(HasSubstr("Autofill.SuggestionAcceptedIndex.CreditCard"),
+                HasSubstr("Autofill.SuggestionAcceptedIndex.Other"))));
+}
+
+TEST_F(AutofillMetricsTest, LogSuggestionAcceptedIndex_Other) {
+  const int index = 0;
+  const PopupType popup_type1 = PopupType::kUnspecified;
+  const PopupType popup_type2 = PopupType::kPasswords;
+
+  base::HistogramTester histogram_tester;
+  AutofillMetrics::LogAutofillSuggestionAcceptedIndex(index, popup_type1);
+  AutofillMetrics::LogAutofillSuggestionAcceptedIndex(index, popup_type2);
+
+  histogram_tester.ExpectUniqueSample("Autofill.SuggestionAcceptedIndex.Other",
+                                      index, 2);
+
+  const std::string histograms = histogram_tester.GetAllHistogramsRecorded();
+  EXPECT_THAT(
+      histograms,
+      Not(AnyOf(HasSubstr("Autofill.SuggestionAcceptedIndex.CreditCard"),
+                HasSubstr("Autofill.SuggestionAcceptedIndex.Profile"))));
+}
+
 TEST_F(AutofillMetricsTest, OnAutocompleteSuggestionsShown) {
   base::HistogramTester histogram_tester;
   AutofillMetrics::OnAutocompleteSuggestionsShown();

tools/metrics/histograms/histograms.xml

@@ -147658,6 +147658,16 @@ should be kept until we use this API.  -->
       name="Autofill.StrikeDatabase.StrikesPresentWhenStrikeExpired"/>
 </histogram_suffixes>
 
+<histogram_suffixes name="AutofillSuggestionAcceptedIndexSuggestionType"
+    separator=".">
+  <suffix name="CreditCard" label="Suggestions with credit card data"/>
+  <suffix name="Other" label="Unspecified suggestions"/>
+  <suffix name="Profile"
+      label="Suggestions with personal data from AutofillProfiles, e.g. name,
+             address, email address, and or phone number"/>
+  <affected-histogram name="Autofill.SuggestionAcceptedIndex"/>
+</histogram_suffixes>
+
 <histogram_suffixes name="AutofillSyncState" separator=".">
   <suffix name="SignedIn" label="Signed in"/>
   <suffix name="SignedInAndSyncFeature" label="Signed in and sync feature">