Use the same metric recorder for PasswordFormManager and NewPasswordFormManager.

When flag #new-password-form-parsing is on, PasswordFormManager is used for
saving and NewPasswordFormManager for filling. Now they have separate metric
recorders. Metric PasswordManager.ActionsTakenV3 (and others) is inconsistent
because filling and saving parts of this metric written with different recorders.

This CL implements usage of the same metric recorder for PasswordFormManager
and NewPasswordFormManager that corresponds for the same form.

Bug: 882432, 831123

Change-Id: I84b480c86ec8172fb9d055cd8189529c5e4f42fb
Reviewed-on: https://chromium-review.googlesource.com/1215965
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590234}

components/password_manager/core/browser/new_password_form_manager.cc

@@ -15,7 +15,6 @@
 #include "components/password_manager/core/browser/form_parsing/form_parser.h"
 #include "components/password_manager/core/browser/form_saver.h"
 #include "components/password_manager/core/browser/password_form_filling.h"
-#include "components/password_manager/core/browser/password_form_metrics_recorder.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
 #include "components/password_manager/core/browser/password_manager_util.h"

components/password_manager/core/browser/new_password_form_manager.h

@@ -18,6 +18,7 @@
 #include "components/password_manager/core/browser/form_fetcher.h"
 #include "components/password_manager/core/browser/form_parsing/password_field_prediction.h"
 #include "components/password_manager/core/browser/password_form_manager_for_ui.h"
+#include "components/password_manager/core/browser/password_form_metrics_recorder.h"
 #include "components/password_manager/core/browser/password_form_user_action.h"
 #include "components/password_manager/core/browser/votes_uploader.h"
 
@@ -129,6 +130,12 @@ class NewPasswordFormManager : public PasswordFormManagerInterface,
 
 #endif
 
+  // TODO(https://crbug.com/831123): Remove it when the old form parsing is
+  // removed.
+  scoped_refptr<PasswordFormMetricsRecorder> metrics_recorder() {
+    return metrics_recorder_;
+  }
+
  protected:
   // FormFetcher::Consumer:
   void ProcessMatches(

components/password_manager/core/browser/password_manager.cc

@@ -702,7 +702,8 @@ void PasswordManager::CreatePendingLoginManagers(
         (driver ? driver->AsWeakPtr() : base::WeakPtr<PasswordManagerDriver>()),
         form, std::make_unique<FormSaverImpl>(client_->GetPasswordStore()),
         nullptr);
-    manager->Init(nullptr);
+    manager->Init(
+        GetMetricRecorderFromNewPasswordFormManager(form.form_data, driver));
     pending_login_managers_.push_back(std::move(manager));
   }
 
@@ -1193,4 +1194,16 @@ void PasswordManager::RecordProvisionalSaveFailure(
   }
 }
 
+scoped_refptr<PasswordFormMetricsRecorder>
+PasswordManager::GetMetricRecorderFromNewPasswordFormManager(
+    const autofill::FormData& form,
+    const PasswordManagerDriver* driver) {
+  for (auto& form_manager : form_managers_) {
+    if (form_manager->DoesManage(form, driver))
+      return form_manager->metrics_recorder();
+  }
+
+  return nullptr;
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_manager.h

@@ -264,6 +264,11 @@ class PasswordManager : public LoginModel, public FormSubmissionObserver {
       const GURL& form_origin,
       BrowserSavePasswordProgressLogger* logger);
 
+  scoped_refptr<PasswordFormMetricsRecorder>
+  GetMetricRecorderFromNewPasswordFormManager(
+      const autofill::FormData& form,
+      const PasswordManagerDriver* driver);
+
   // Note about how a PasswordFormManager can transition from
   // pending_login_managers_ to provisional_save_manager_ and the infobar.
   //

components/password_manager/core/browser/password_manager_unittest.cc

@@ -2650,4 +2650,35 @@ TEST_F(PasswordManagerTest, SubmittedGaiaFormWithoutVisiblePasswordField) {
   manager()->OnPasswordFormSubmittedNoChecks(&driver_, form);
 }
 
+// Tests that PasswordFormManager and NewPasswordFormManager for the same form
+// have the same metrics recorder.
+TEST_F(PasswordManagerTest, CheckMetricsRecorder) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  TurnOnNewParsingForSaving(&scoped_feature_list);
+
+  EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
+      .WillRepeatedly(Return(true));
+
+  PasswordForm form(MakeSimpleForm());
+  EXPECT_CALL(*store_, GetLogins(_, _))
+      .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms()));
+
+  std::vector<PasswordForm> observed;
+  observed.push_back(form);
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+
+  const std::vector<std::unique_ptr<PasswordFormManager>>&
+      password_form_managers = manager()->pending_login_managers();
+
+  const std::vector<std::unique_ptr<NewPasswordFormManager>>&
+      new_password_form_managers = manager()->form_managers();
+
+  ASSERT_EQ(1u, password_form_managers.size());
+  ASSERT_EQ(1u, new_password_form_managers.size());
+
+  EXPECT_TRUE(password_form_managers[0]->GetMetricsRecorder());
+  EXPECT_EQ(password_form_managers[0]->GetMetricsRecorder(),
+            new_password_form_managers[0]->GetMetricsRecorder());
+}
+
 }  // namespace password_manager