Report priority of PasswordGenerationRequirementsSpec

Report the priority of a PasswordGenerationRequirementsSpec for a
generated password. This can be used for debugging as a 0 means that no
spec was used, a 10 means that the spec came from autofill and was crowd
sourced, a 20 means that it was overrideen per domain and a 30 means that
is was overridden for the form.

Bug: 846694
Change-Id: Ic64433d23dece4f9d6d511254cc85fdb2a746646
Reviewed-on: https://chromium-review.googlesource.com/1101211
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571865}

chrome/browser/password_manager/password_accessory_controller.cc

@@ -30,13 +30,18 @@ using Item = PasswordAccessoryViewInterface::AccessoryItem;
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(PasswordAccessoryController);
 
 struct PasswordAccessoryController::GenerationElementData {
-  GenerationElementData(autofill::FormSignature form_signature,
+  GenerationElementData(autofill::PasswordForm form,
+                        autofill::FormSignature form_signature,
                         autofill::FieldSignature field_signature,
                         uint32_t max_password_length)
-      : form_signature(form_signature),
+      : form(std::move(form)),
+        form_signature(form_signature),
         field_signature(field_signature),
         max_password_length(max_password_length) {}
 
+  // Form for which password generation is triggered.
+  autofill::PasswordForm form;
+
   // Signature of the form for which password generation is triggered.
   autofill::FormSignature form_signature;
 
@@ -136,6 +141,7 @@ void PasswordAccessoryController::OnAutomaticGenerationStatusChanged(
   if (available) {
     DCHECK(ui_data.has_value());
     generation_element_data_ = std::make_unique<GenerationElementData>(
+        ui_data.value().password_form,
         autofill::CalculateFormSignature(
             ui_data.value().password_form.form_data),
         autofill::CalculateFieldSignatureByNameAndType(
@@ -179,12 +185,18 @@ void PasswordAccessoryController::OnGenerationRequested() {
   // TODO(crbug.com/835234): Take the modal dialog logic out of the accessory
   // controller.
   dialog_view_ = create_dialog_factory_.Run(this);
+  uint32_t spec_priority = 0;
   base::string16 password =
       target_frame_driver_->GetPasswordGenerationManager()->GeneratePassword(
           web_contents_->GetLastCommittedURL().GetOrigin(),
           generation_element_data_->form_signature,
           generation_element_data_->field_signature,
-          generation_element_data_->max_password_length);
+          generation_element_data_->max_password_length, &spec_priority);
+  if (target_frame_driver_ && target_frame_driver_->GetPasswordManager()) {
+    target_frame_driver_->GetPasswordManager()
+        ->ReportSpecPriorityForGeneratedPassword(generation_element_data_->form,
+                                                 spec_priority);
+  }
   dialog_view_->Show(password);
 }
 

chrome/browser/password_manager/password_accessory_controller_unittest.cc

@@ -82,11 +82,12 @@ class MockPasswordGenerationManager
                                 password_manager::PasswordManagerDriver* driver)
       : password_manager::PasswordGenerationManager(client, driver) {}
 
-  MOCK_METHOD4(GeneratePassword,
+  MOCK_METHOD5(GeneratePassword,
                base::string16(const GURL&,
                               autofill::FormSignature,
                               autofill::FieldSignature,
-                              uint32_t));
+                              uint32_t,
+                              uint32_t*));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockPasswordGenerationManager);
@@ -423,7 +424,7 @@ TEST_F(PasswordAccessoryControllerTest,
       .WillOnce(Return(&mock_generation_manager));
   EXPECT_CALL(mock_generation_manager,
               GeneratePassword(_, form_signature, field_signature,
-                               uint32_t(new_ui_data.max_length)))
+                               uint32_t(new_ui_data.max_length), _))
       .WillOnce(Return(generated_password));
   EXPECT_CALL(*raw_dialog_view, Show(generated_password));
   controller()->OnGenerationRequested();

chrome/browser/ui/passwords/password_generation_popup_controller_impl.cc

@@ -178,10 +178,15 @@ void PasswordGenerationPopupControllerImpl::Show(GenerationState state) {
   // When switching from editing to generation state, regenerate the password.
   if (state == kOfferGeneration &&
       (state_ != state || current_password_.empty())) {
+    uint32_t spec_priority = 0;
     current_password_ =
         driver_->GetPasswordGenerationManager()->GeneratePassword(
             web_contents_->GetLastCommittedURL().GetOrigin(), form_signature_,
-            field_signature_, max_length_);
+            field_signature_, max_length_, &spec_priority);
+    if (driver_ && driver_->GetPasswordManager()) {
+      driver_->GetPasswordManager()->ReportSpecPriorityForGeneratedPassword(
+          form_, spec_priority);
+    }
   }
   state_ = state;
 

components/password_manager/core/browser/password_form_metrics_recorder.cc

@@ -139,6 +139,10 @@ PasswordFormMetricsRecorder::~PasswordFormMetricsRecorder() {
     ukm_entry_builder_.SetGeneration_PopupShown(
         static_cast<int64_t>(password_generation_popup_shown_));
   }
+  if (spec_priority_of_generated_password_) {
+    ukm_entry_builder_.SetGeneration_SpecPriority(
+        spec_priority_of_generated_password_.value());
+  }
 
   ukm_entry_builder_.Record(ukm::UkmRecorder::Get());
 }
@@ -157,6 +161,11 @@ void PasswordFormMetricsRecorder::SetHasGeneratedPasswordChanged(
   has_generated_password_changed_ = has_generated_password_changed;
 }
 
+void PasswordFormMetricsRecorder::ReportSpecPriorityForGeneratedPassword(
+    uint32_t spec_priority) {
+  spec_priority_of_generated_password_ = spec_priority;
+}
+
 void PasswordFormMetricsRecorder::SetManagerAction(
     ManagerAction manager_action) {
   manager_action_ = manager_action;

components/password_manager/core/browser/password_form_metrics_recorder.h

@@ -13,6 +13,7 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/optional.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/autofill/core/common/signatures_util.h"
 #include "components/password_manager/core/browser/password_form_user_action.h"
@@ -202,6 +203,13 @@ class PasswordFormMetricsRecorder
   // Stores whether the a generated password has been modified by the user.
   void SetHasGeneratedPasswordChanged(bool has_changed_generated_password);
 
+  // Reports the priority of a PasswordGenerationRequirementsSpec for a
+  // generated password. This can be used for debugging as a 0 means that
+  // no spec was used, a 10 means that the spec came from autofill and was crowd
+  // sourced, a 20 means that it was overrideen per domain and a 30 means that
+  // is was overridden for the form.
+  void ReportSpecPriorityForGeneratedPassword(uint32_t spec_priority);
+
   // Stores the password manager and user actions and logs them.
   void SetManagerAction(ManagerAction manager_action);
   void SetUserAction(UserAction user_action);
@@ -323,6 +331,8 @@ class PasswordFormMetricsRecorder
   // user.
   bool has_generated_password_changed_ = false;
 
+  base::Optional<uint32_t> spec_priority_of_generated_password_;
+
   // Tracks which bubble is currently being displayed to the user.
   CurrentBubbleOfInterest current_bubble_ = CurrentBubbleOfInterest::kNone;
 

components/password_manager/core/browser/password_generation_manager.cc

@@ -141,7 +141,8 @@ base::string16 PasswordGenerationManager::GeneratePassword(
     const GURL& last_committed_url,
     autofill::FormSignature form_signature,
     autofill::FieldSignature field_signature,
-    uint32_t max_length) {
+    uint32_t max_length,
+    uint32_t* spec_priority) {
   autofill::PasswordRequirementsSpec spec;
 
   // Lookup password requirements.
@@ -152,6 +153,9 @@ base::string16 PasswordGenerationManager::GeneratePassword(
         last_committed_url.GetOrigin(), form_signature, field_signature);
   }
 
+  if (spec_priority)
+    *spec_priority = spec.priority();
+
   // Choose the password length as the minimum of default length, what website
   // allows, and what the autofill server suggests.
   uint32_t target_length = autofill::kDefaultPasswordLength;

components/password_manager/core/browser/password_generation_manager.h

@@ -82,7 +82,8 @@ class PasswordGenerationManager {
       const GURL& last_committed_url,
       autofill::FormSignature form_signature,
       autofill::FieldSignature field_signature,
-      uint32_t max_length);
+      uint32_t max_length,
+      uint32_t* spec_priority);
 
  private:
   friend class PasswordGenerationManagerTest;

components/password_manager/core/browser/password_manager.cc

@@ -710,6 +710,16 @@ void PasswordManager::ProcessSubmittedForm(
   }
 }
 
+void PasswordManager::ReportSpecPriorityForGeneratedPassword(
+    const autofill::PasswordForm& password_form,
+    uint32_t spec_priority) {
+  PasswordFormManager* form_manager = GetMatchingPendingManager(password_form);
+  if (form_manager && form_manager->GetMetricsRecorder()) {
+    form_manager->GetMetricsRecorder()->ReportSpecPriorityForGeneratedPassword(
+        spec_priority);
+  }
+}
+
 void PasswordManager::OnStartNavigation(PasswordManagerDriver* driver) {
   // TODO(crbug/842643): use this signal instead of DidStartProvisionalLoad in
   // the renderer.

components/password_manager/core/browser/password_manager.h

@@ -180,6 +180,13 @@ class PasswordManager : public LoginModel, public FormSubmissionObserver {
 
   NavigationEntryToCheck entry_to_check() const { return entry_to_check_; }
 
+  // Reports the priority of a PasswordGenerationRequirementsSpec for a
+  // generated password. See
+  // PasswordFormMetricsRecorder::ReportSpecPriorityForGeneratedPassword.
+  void ReportSpecPriorityForGeneratedPassword(
+      const autofill::PasswordForm& password_form,
+      uint32_t spec_priority);
+
  private:
   FRIEND_TEST_ALL_PREFIXES(
       PasswordManagerTest,

tools/metrics/ukm/ukm.xml

@@ -2451,6 +2451,15 @@ be describing additional metrics about the same event.
       if (and only if) a popup was shown.
     </summary>
   </metric>
+  <metric name="Generation.SpecPriority">
+    <summary>
+      Reports the priority of a PasswordGenerationRequirementsSpec for a
+      generated password. This can be used for debugging as a 0 means that no
+      spec was used, a 10 means that the spec came from autofill and was crowd
+      sourced, a 20 means that it was overridden per domain and a 30 means that
+      is was overridden for the form.
+    </summary>
+  </metric>
   <metric name="ManagerFill.Action">
     <summary>
       Records for each password form (and HTTP auth), whether the password