Adds UKM for autofill attributes in form_structure.

components/autofill/core/browser/form_structure.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/form_structure.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 18 matching lines @@
 #include "components/autofill/core/browser/form_field.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/autofill_util.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/form_data_predictions.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/autofill/core/common/form_field_data_predictions.h"
 #include "components/autofill/core/common/signatures_util.h"
 #include "components/rappor/public/rappor_utils.h"
 #include "components/rappor/rappor_service_impl.h"
+#include "components/ukm/ukm_service.h"
 
 namespace autofill {
 namespace {
 
 const char kClientVersion[] = "6.1.1715.1442/en (GGLL)";
 const char kBillingMode[] = "billing";
 const char kShippingMode[] = "shipping";
 
 // A form is considered to have a high prediction mismatch rate if the number of
 // mismatches exceeds this threshold.
@@ skipping 276 matching lines @@
     fields_.push_back(base::MakeUnique<AutofillField>(field, unique_name));
   }
 
   form_signature_ = autofill::CalculateFormSignature(form);
   // Do further processing on the fields, as needed.
   ProcessExtractedFields();
 }
 
 FormStructure::~FormStructure() {}
 
-void FormStructure::DetermineHeuristicTypes() {
+void FormStructure::DetermineHeuristicTypes(ukm::UkmService* ukm_service) {
   const auto determine_heuristic_types_start_time = base::TimeTicks::Now();
 
   // First, try to detect field types based on each field's |autocomplete|
   // attribute value.
   if (!was_parsed_for_autocomplete_attributes_)
     ParseFieldTypesFromAutocompleteAttributes();
 
   // Then if there are enough active fields, and if we are dealing with either a
   // proper <form> or a <form>-less checkout, run the heuristics and server
   // prediction routines.
@@ skipping 10 matching lines @@
 
   UpdateAutofillCount();
   IdentifySections(has_author_specified_sections_);
 
   if (IsAutofillable()) {
     AutofillMetrics::LogDeveloperEngagementMetric(
         AutofillMetrics::FILLABLE_FORM_PARSED);
     if (has_author_specified_types_) {
       AutofillMetrics::LogDeveloperEngagementMetric(
           AutofillMetrics::FILLABLE_FORM_CONTAINS_TYPE_HINTS);
+      AutofillMetrics::LogDeveloperEngagementUkm(
+          ukm_service, source_url(),
+          {AutofillMetrics::FILLABLE_FORM_CONTAINS_TYPE_HINTS,

[sebsg, 2017/03/29 13:52:58]

by looking at the comments of these constants, it seems that
FILLABLE_FOMR_CONTAINS_TYPE_HINTS is a superset of FILLABLE_FORM_PARSED.

https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil...

Do you think we could only log one? Maybe Math knows why it's logged this way
now?

That would remove the need to have multiple times the same metric in a
UKM_Entry, which is something I find a bit weird.

Thanks!

[csashi, 2017/03/29 16:15:11]

Yes, CONTAINS_TYPE_HINTS are a subset of FORM_PARSED. I am logging both to be
consistent with UMA metrics. If consistency with UMA is not important, I can log
just one.

BTW, the log schema seems to be specifically designed for multiple enums for the
same metric. See
https://codesearch.chromium.org/chromium/src/components/metrics/proto/ukm/ent...

[sebsg, 2017/03/29 17:55:13]

I see, I really prefer logging only once if it's a subset, since it's then
simpler to parse when analyzing the metrics. I just want to wait for Math to see
if he has another reason to keep them separated. I'll ping him :)

[sebsg, 2017/03/29 18:46:09]

Ok I talked with Math. Would you mind renaming the enum and changing the logic a
bit? The idea would be to make them mutually exclusive, so something like
FILLABLE_FORM_PARSED_WITH_HINTS and FILLABLE_FORM_PARSED_NO_HINT. WDYT?

[csashi, 2017/03/30 00:52:10]

Done.

+           AutofillMetrics::FILLABLE_FORM_PARSED});
+    } else {
+      AutofillMetrics::LogDeveloperEngagementUkm(
+          ukm_service, source_url(), {AutofillMetrics::FILLABLE_FORM_PARSED});
     }
   }
 
   AutofillMetrics::LogDetermineHeuristicTypesTiming(
       base::TimeTicks::Now() - determine_heuristic_types_start_time);
 }
 
 bool FormStructure::EncodeUploadRequest(
     const ServerFieldTypeSet& available_field_types,
     bool form_was_autofilled,
@@ skipping 943 matching lines @@
               filtered_strings[0].at(prefix_len)) {
         // Mismatch found.
         return filtered_strings[i].substr(0, prefix_len);
       }
     }
   }
   return filtered_strings[0];
 }
 
 }  // namespace autofill