[Passwords] Reset pending credentials when Autofill Assistant has

handled submission

This will prevent confusing prompts when a script has already handled
the submission.

Bug: 1100330, 1063347
Change-Id: I60bfc326c8d32a478710d270bceba3837b668f37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2270079
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Auto-Submit: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784416}

chrome/browser/android/autofill_assistant/client_android.cc

@@ -81,6 +81,19 @@ std::unique_ptr<TriggerContextImpl> CreateTriggerContext(
       base::android::ConvertJavaStringToUTF8(env, jexperiment_ids));
 }
 
+// Notifies Chrome's Password Manager that Autofill Assistant is running or
+// not. No-op if the script is not a password change script.
+void NotifyPasswordManagerIfApplicable(
+    ClientAndroid* client,
+    password_manager::AutofillAssistantMode mode) {
+  auto* password_manager_client = client->GetPasswordManagerClient();
+  if (password_manager_client &&
+      password_manager_client->WasCredentialLeakDialogShown()) {
+    password_manager_client->GetPasswordManager()->SetAutofillAssistantMode(
+        mode);
+  }
+}
+
 }  // namespace
 
 static base::android::ScopedJavaLocalRef<jobject>
@@ -185,8 +198,8 @@ void ClientAndroid::TransferUITo(
   // From this point on, the UIController, in ui_ptr, is either transferred or
   // deleted.
 
-  GetPasswordManagerClient()->GetPasswordManager()->SetAutofillAssistantMode(
-      password_manager::AutofillAssistantMode::kNotRunning);
+  NotifyPasswordManagerIfApplicable(
+      this, password_manager::AutofillAssistantMode::kNotRunning);
 
   if (!jother_web_contents)
     return;
@@ -423,12 +436,8 @@ void ClientAndroid::AttachUI(
 
     // Suppress password manager's prompts while running a password change
     // script.
-    auto* password_manager_client = GetPasswordManagerClient();
-    if (password_manager_client &&
-        password_manager_client->WasCredentialLeakDialogShown()) {
-      password_manager_client->GetPasswordManager()->SetAutofillAssistantMode(
-          password_manager::AutofillAssistantMode::kRunning);
-    }
+    NotifyPasswordManagerIfApplicable(
+        this, password_manager::AutofillAssistantMode::kRunning);
   }
 }
 
@@ -531,8 +540,8 @@ void ClientAndroid::Shutdown(Metrics::DropOutReason reason) {
   if (!controller_)
     return;
 
-  GetPasswordManagerClient()->GetPasswordManager()->SetAutofillAssistantMode(
-      password_manager::AutofillAssistantMode::kNotRunning);
+  NotifyPasswordManagerIfApplicable(
+      this, password_manager::AutofillAssistantMode::kNotRunning);
 
   if (ui_controller_android_ && ui_controller_android_->IsAttached())
     DestroyUI();

components/password_manager/core/browser/password_form_manager.cc

@@ -674,6 +674,13 @@ void PasswordFormManager::CreatePendingCredentials() {
       IsCredentialAPISave());
 }
 
+void PasswordFormManager::ResetState() {
+  parsed_submitted_form_.reset();
+  submitted_form_ = FormData();
+  password_save_manager_->ResetPendingCredentials();
+  is_submitted_ = false;
+}
+
 bool PasswordFormManager::ProvisionallySave(
     const FormData& submitted_form,
     const PasswordManagerDriver* driver,
@@ -690,10 +697,7 @@ bool PasswordFormManager::ProvisionallySave(
 
   if (!have_password_to_save) {
     // In case of error during parsing, reset the state.
-    parsed_submitted_form_.reset();
-    submitted_form_ = FormData();
-    password_save_manager_->ResetPendingCrednetials();
-    is_submitted_ = false;
+    ResetState();
     return false;
   }
 

components/password_manager/core/browser/password_form_manager.h

@@ -82,6 +82,10 @@ class PasswordFormManager : public PasswordFormManagerForUI,
   // for success detection.
   bool IsEqualToSubmittedForm(const autofill::FormData& form) const;
 
+  // Clears potentially submitted or pending form. Used to forget the state when
+  // Autofill Assistant has handled a submission.
+  void ResetState();
+
   // If |submitted_form| is managed by *this (i.e. DoesManage returns true for
   // |submitted_form| and |driver|) then saves |submitted_form| to
   // |submitted_form_| field, sets |is_submitted| = true and returns true.

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -658,6 +658,20 @@ TEST_P(PasswordFormManagerTest, SetSubmittedMultipleTimes) {
   EXPECT_EQ(PasswordForm(), form_manager_->GetPendingCredentials());
 }
 
+TEST_P(PasswordFormManagerTest, ResetState) {
+  EXPECT_TRUE(
+      form_manager_->ProvisionallySave(submitted_form_, &driver_, nullptr));
+  EXPECT_TRUE(form_manager_->is_submitted());
+  EXPECT_TRUE(form_manager_->GetSubmittedForm());
+  EXPECT_NE(PasswordForm(), form_manager_->GetPendingCredentials());
+
+  form_manager_->ResetState();
+
+  EXPECT_FALSE(form_manager_->is_submitted());
+  EXPECT_FALSE(form_manager_->GetSubmittedForm());
+  EXPECT_EQ(PasswordForm(), form_manager_->GetPendingCredentials());
+}
+
 // Tests that when PasswordFormManager receives saved matches it waits for
 // server predictions and fills on receiving them.
 TEST_P(PasswordFormManagerTest, ServerPredictionsWithinDelay) {
@@ -2343,7 +2357,7 @@ class MockPasswordSaveManager : public PasswordSaveManager {
                     const autofill::FormData&,
                     bool,
                     bool));
-  MOCK_METHOD0(ResetPendingCrednetials, void());
+  MOCK_METHOD0(ResetPendingCredentials, void());
   MOCK_METHOD2(Save,
                void(const autofill::FormData&, const autofill::PasswordForm&));
   MOCK_METHOD3(Update,

components/password_manager/core/browser/password_manager.cc

@@ -1215,16 +1215,24 @@ void PasswordManager::ShowManualFallbackForSavingImpl(
 }
 
 void PasswordManager::SetAutofillAssistantMode(AutofillAssistantMode mode) {
+  if (autofill_assistant_mode_ == mode) {
+    NOTREACHED()
+        << "Autofill Assistant tried to disable/enable prompts twice in a row.";
+    return;
+  }
   autofill_assistant_mode_ = mode;
 
   if (autofill_assistant_mode_ == AutofillAssistantMode::kRunning) {
-    DCHECK(!disable_prompts_timer_.IsRunning())
-        << "Autofill Assistant tried to disable prompts twice in a row.";
     disable_prompts_timer_.Start(FROM_HERE, GetTimeoutForDisablingPrompts(),
                                  this,
                                  &PasswordManager::ResetAutofillAssistantMode);
   } else {
     disable_prompts_timer_.Stop();
+    // Reset pending credentials as Autofill Assistant has handled the pending
+    // submission.
+    for (auto& form_manager : form_managers_)
+      form_manager->ResetState();
+    owned_submitted_form_manager_.reset();
   }
 }
 

components/password_manager/core/browser/password_manager.h

@@ -214,8 +214,10 @@ class PasswordManager : public FormSubmissionObserver {
   // Notifies that Credential Management API function store() is called.
   void NotifyStorePasswordCalled();
 
-  // Sets the autofill-assistant mode. Certain prompts will be disabled while
-  // autofill-assistant is running. See |AutofillAssistantMode|.
+  // Sets the Autofill Assistant mode to disable prompts while |mode=kRunning|.
+  // A script start triggers a timer that will reset the mode to |kNotRunning|
+  // (default) to prevent disabling the password manager forever. A script
+  // finish will clear pending credentials in all form managers.
   void SetAutofillAssistantMode(AutofillAssistantMode mode);
 
   // Returns the currently set autofill-assistant mode.

components/password_manager/core/browser/password_manager_unittest.cc

@@ -3665,6 +3665,61 @@ TEST_P(PasswordManagerTest, NoPromptAutofillAssistantManuallyCuratedScript) {
   }
 }
 
+// Password Manager may store a pending credential that will cause a prompt when
+// Autofill Assistant has already handled the submission. This test ensures that
+// Password Manager forgots the pending credential and doesn't prompt to update
+// the password later (e.g., after navigation).
+TEST_P(PasswordManagerTest,
+       NoPromptAfterAutofillAssistantManuallyCuratedScript) {
+  EXPECT_CALL(*store_, GetLogins)
+      .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms(store_.get())));
+
+  for (bool set_owned_form_manager : {false, true}) {
+    SCOPED_TRACE(testing::Message("set_owned_form_manager = ")
+                 << set_owned_form_manager);
+
+    EXPECT_CALL(client_, IsSavingAndFillingEnabled)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr).Times(0);
+    manager()->SetAutofillAssistantMode(AutofillAssistantMode::kRunning);
+
+    // Make several forms ready for saving.
+    PasswordForm form1(MakeFormWithOnlyNewPasswordField());
+    PasswordForm form2(MakeSimpleForm());
+    manager()->OnPasswordFormsParsed(&driver_,
+                                     {form1.form_data, form2.form_data});
+    manager()->ShowManualFallbackForSaving(&driver_, form1.form_data);
+    manager()->ShowManualFallbackForSaving(&driver_, form2.form_data);
+
+    // Simulate submission in different ways depending on whether
+    // |owned_submitted_form_manager_| should be set and |form_managers_| should
+    // be cleared OR the submitted form manager should be in |form_managers_|.
+    if (set_owned_form_manager)
+      manager()->DidNavigateMainFrame(true /* form_may_be_submitted */);
+    else
+      OnPasswordFormSubmitted(form2.form_data);
+
+    manager()->SetAutofillAssistantMode(AutofillAssistantMode::kNotRunning);
+
+    manager()->OnPasswordFormsRendered(&driver_, {} /* observed */,
+                                       true /* did stop loading */);
+    // No form manager is ready for saving.
+    EXPECT_FALSE(manager()->GetSubmittedManagerForTest());
+    Mock::VerifyAndClearExpectations(&client_);
+
+    // A form reappears again and a user submits it manually. Now expect a
+    // prompt.
+    EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr);
+    EXPECT_CALL(client_, IsSavingAndFillingEnabled)
+        .WillRepeatedly(Return(true));
+    manager()->OnPasswordFormsParsed(&driver_, {form2.form_data});
+    OnPasswordFormSubmitted(form2.form_data);
+    manager()->OnPasswordFormsRendered(&driver_, {} /* observed */,
+                                       true /* did stop loading */);
+    Mock::VerifyAndClearExpectations(&client_);
+  }
+}
+
 // Tests the following scenario:
 // 1. Password Manager's prompts are disabled by Autofill Assistant because it
 // runs a script.

components/password_manager/core/browser/password_save_manager.h

@@ -59,7 +59,7 @@ class PasswordSaveManager {
       bool is_http_auth,
       bool is_credential_api_save) = 0;
 
-  virtual void ResetPendingCrednetials() = 0;
+  virtual void ResetPendingCredentials() = 0;
 
   virtual void Save(const autofill::FormData& observed_form,
                     const autofill::PasswordForm& parsed_submitted_form) = 0;

components/password_manager/core/browser/password_save_manager_impl.cc

@@ -207,7 +207,7 @@ void PasswordSaveManagerImpl::SetVotesAndRecordMetricsForPendingCredentials(
   }
 }
 
-void PasswordSaveManagerImpl::ResetPendingCrednetials() {
+void PasswordSaveManagerImpl::ResetPendingCredentials() {
   pending_credentials_ = PasswordForm();
   pending_credentials_state_ = PendingCredentialsState::NONE;
 }

components/password_manager/core/browser/password_save_manager_impl.h

@@ -48,7 +48,7 @@ class PasswordSaveManagerImpl : public PasswordSaveManager {
       bool is_http_auth,
       bool is_credential_api_save) override;
 
-  void ResetPendingCrednetials() override;
+  void ResetPendingCredentials() override;
 
   void Save(const autofill::FormData& observed_form,
             const autofill::PasswordForm& parsed_submitted_form) override;

components/password_manager/core/browser/password_save_manager_impl_unittest.cc

@@ -557,7 +557,7 @@ TEST_P(PasswordSaveManagerImplTest, CreatePendingCredentialsEmptyName) {
 }
 
 // Tests creating pending credentials when the password store is empty.
-TEST_P(PasswordSaveManagerImplTest, ResetPendingCrednetials) {
+TEST_P(PasswordSaveManagerImplTest, ResetPendingCredentials) {
   fetcher()->NotifyFetchCompleted();
 
   password_save_manager_impl()->CreatePendingCredentials(
@@ -565,7 +565,7 @@ TEST_P(PasswordSaveManagerImplTest, ResetPendingCrednetials) {
       /*is_http_auth=*/false,
       /*is_credential_api_save=*/false);
 
-  password_save_manager_impl()->ResetPendingCrednetials();
+  password_save_manager_impl()->ResetPendingCredentials();
 
   // Check that save manager is in None state.
   EXPECT_FALSE(password_save_manager_impl()->IsNewLogin());