UKM metric for password form parsing comparison.

This metric is for checking correctness and improving the new password
form parsing algorithm (go/new-cpm-design-refactoring).

The privacy review of password manager UKM metrics was done on
crbug.com/728707 and https://goto.google.com/gkmnc

Bug: 831123
Change-Id: I05426e4af88af13af9abef883da14da0f8f501b3
Reviewed-on: https://chromium-review.googlesource.com/1097398Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566869}

components/password_manager/core/browser/new_password_form_manager.cc

@@ -200,9 +200,6 @@ void NewPasswordFormManager::ProcessServerPredictions(
 }
 
 void NewPasswordFormManager::Fill() {
-  if (!driver_ || best_matches_.empty())
-    return;
-
   // There are additional signals (server-side data) and parse results in
   // filling and saving mode might be different so it is better not to cache
   // parse result, but to parse each time again.
@@ -211,6 +208,13 @@ void NewPasswordFormManager::Fill() {
   if (!observed_password_form)
     return;
 
+  RecordMetricOnCompareParsingResult(*observed_password_form);
+
+  // TODO(https://crbug.com/831123). Move this lines to the beginning of the
+  // function when the old parsing is removed.
+  if (!driver_ || best_matches_.empty())
+    return;
+
   // TODO(https://crbug.com/831123). Implement correct treating of federated
   // matches.
   std::vector<const autofill::PasswordForm*> federated_matches;
@@ -220,4 +224,42 @@ void NewPasswordFormManager::Fill() {
       preferred_match_, metrics_recorder_.get());
 }
 
+void NewPasswordFormManager::RecordMetricOnCompareParsingResult(
+    const autofill::PasswordForm& parsed_form) {
+  bool same =
+      parsed_form.username_element == old_parsing_result_.username_element &&
+      parsed_form.password_element == old_parsing_result_.password_element &&
+      parsed_form.new_password_element ==
+          old_parsing_result_.new_password_element &&
+      parsed_form.confirmation_password_element ==
+          old_parsing_result_.confirmation_password_element;
+  if (same) {
+    metrics_recorder_->RecordParsingsComparisonResult(
+        PasswordFormMetricsRecorder::ParsingComparisonResult::kSame);
+    return;
+  }
+
+  // In the old parsing for fields with empty name, placeholders are used. The
+  // reason for this is that an empty "..._element" attribute in a PasswordForm
+  // means that no corresponding input element exists. The new form parsing sets
+  // empty string in that case because renderer ids are used instead of element
+  // names for fields identification. Hence in case of anonymous fields, the
+  // results will be different for sure. Compare to placeholders and record this
+  // case.
+  if (old_parsing_result_.username_element ==
+          base::ASCIIToUTF16("anonymous_username") ||
+      old_parsing_result_.password_element ==
+          base::ASCIIToUTF16("anonymous_password") ||
+      old_parsing_result_.new_password_element ==
+          base::ASCIIToUTF16("anonymous_new_password") ||
+      old_parsing_result_.confirmation_password_element ==
+          base::ASCIIToUTF16("anonymous_confirmation_password")) {
+    metrics_recorder_->RecordParsingsComparisonResult(
+        PasswordFormMetricsRecorder::ParsingComparisonResult::kAnonymousFields);
+  } else {
+    metrics_recorder_->RecordParsingsComparisonResult(
+        PasswordFormMetricsRecorder::ParsingComparisonResult::kDifferent);
+  }
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/new_password_form_manager.h

@@ -12,6 +12,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/password_manager/core/browser/form_fetcher.h"
 #include "components/password_manager/core/browser/form_parsing/password_field_prediction.h"
@@ -59,6 +60,10 @@ class NewPasswordFormManager : public PasswordFormManagerForUI,
   bool is_submitted() { return is_submitted_; }
   void set_not_submitted() { is_submitted_ = false; }
 
+  void set_old_parsing_result(const autofill::PasswordForm& form) {
+    old_parsing_result_ = form;
+  }
+
   // Selects from |predictions| predictions that corresponds to
   // |observed_form_|, initiates filling and stores predictions in
   // |predictions_|.
@@ -100,6 +105,12 @@ class NewPasswordFormManager : public PasswordFormManagerForUI,
   // Sends fill data to the renderer.
   void Fill();
 
+  // Compares |parsed_form| with |old_parsing_result_| and records UKM metric.
+  // TODO(https://crbug.com/831123): Remove it when the old form parsing is
+  // removed.
+  void RecordMetricOnCompareParsingResult(
+      const autofill::PasswordForm& parsed_form);
+
   // The client which implements embedder-specific PasswordManager operations.
   PasswordManagerClient* client_;
 
@@ -134,6 +145,11 @@ class NewPasswordFormManager : public PasswordFormManagerForUI,
 
   base::Optional<FormPredictions> predictions_;
 
+  // Used for comparison metrics.
+  // TODO(https://crbug.com/831123): Remove it when the old form parsing is
+  // removed.
+  autofill::PasswordForm old_parsing_result_;
+
   DISALLOW_COPY_AND_ASSIGN(NewPasswordFormManager);
 };
 

components/password_manager/core/browser/password_form_metrics_recorder.cc

@@ -235,6 +235,12 @@ void PasswordFormMetricsRecorder::RecordFormSignature(
       HashFormSignature(form_signature));
 }
 
+void PasswordFormMetricsRecorder::RecordParsingsComparisonResult(
+    ParsingComparisonResult comparison_result) {
+  ukm_entry_builder_.SetParsingComparison(
+      static_cast<uint64_t>(comparison_result));
+}
+
 int PasswordFormMetricsRecorder::GetActionsTaken() const {
   return static_cast<int>(user_action_) +
          static_cast<int>(UserAction::kMax) *

components/password_manager/core/browser/password_form_metrics_recorder.h

@@ -157,6 +157,16 @@ class PasswordFormMetricsRecorder
     kCorrectedUsernameInForm = 200,
   };
 
+  // Old and new form parsings comparison result.
+  enum class ParsingComparisonResult {
+    kSame,
+    kDifferent,
+    // Old and new parsers use different identification mechanism for unnamed
+    // fields, so the difference in parsing of anonymous fields is expected.
+    kAnonymousFields,
+    kMax
+  };
+
   // The maximum number of combinations of the ManagerAction, UserAction and
   // SubmitResult enums.
   // This is used when recording the actions taken by the form in UMA.
@@ -236,6 +246,10 @@ class PasswordFormMetricsRecorder
   // distinguish two forms on the same site.
   void RecordFormSignature(autofill::FormSignature form_signature);
 
+  // Records old and new form parsings comparison result.
+  void RecordParsingsComparisonResult(
+      ParsingComparisonResult comparison_result);
+
  private:
   friend class base::RefCounted<PasswordFormMetricsRecorder>;
 

components/password_manager/core/browser/password_manager.cc

@@ -667,23 +667,24 @@ void PasswordManager::CreateFormManagers(
     password_manager::PasswordManagerDriver* driver,
     const std::vector<autofill::PasswordForm>& forms) {
   // Find new forms.
-  std::vector<const FormData*> new_forms;
-  for (const autofill::PasswordForm& form : forms) {
+  std::vector<const PasswordForm*> new_forms;
+  for (const PasswordForm& form : forms) {
     bool form_manager_exists =
         std::any_of(form_managers_.begin(), form_managers_.end(),
                     [&form, driver](const auto& form_manager) {
                       return form_manager->DoesManage(form.form_data, driver);
                     });
     if (!form_manager_exists)
-      new_forms.push_back(&form.form_data);
+      new_forms.push_back(&form);
   }
 
   // Create form manager for new forms.
-  for (const FormData* new_form : new_forms) {
+  for (const PasswordForm* new_form : new_forms) {
     form_managers_.push_back(std::make_unique<NewPasswordFormManager>(
         client_,
         driver ? driver->AsWeakPtr() : base::WeakPtr<PasswordManagerDriver>(),
-        *new_form, nullptr));
+        new_form->form_data, nullptr));
+    form_managers_.back()->set_old_parsing_result(*new_form);
   }
 }
 

tools/metrics/ukm/ukm.xml

@@ -2298,6 +2298,14 @@ be describing additional metrics about the same event.
       enum ManagerAutofillEvent.
     </summary>
   </metric>
+  <metric name="ParsingComparison">
+    <summary>
+      Records comparison result of old and new password form parsing algorithms.
+      Metric is recorded on a password form detection. Recorded values
+      correspond to the enum
+      password_manager::PasswordFormMetricsRecorder::ParsingComparisonResult.
+    </summary>
+  </metric>
   <metric name="Saving.Prompt.Interaction">
     <summary>
       Records how the user interacted with a saving prompt. Recorded values