Update PasswordManager when a relevant credential is removed by the user.

Before this CL this behavior is possible:
- a credential is autofilled.
- user removes the credential via settings or "Manage passwords" bubble.
- user clicks "Login" or reloads the page. In other words makes an action that we consider a successful form submission.
- password manager updates the credential in the store by reviving it.

After this CL the password manager is informed as soon as the password is removed. Thus, the copy of the credential in the memory should go away.

Bug: 821763,841853
Change-Id: If6c371312a9ed55217f5998989dd0457296a538a
Reviewed-on: https://chromium-review.googlesource.com/1064118Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559539}

chrome/browser/password_manager/password_manager_browsertest.cc

@@ -1747,6 +1747,47 @@ IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithViewsFeature,
   WaitForJsElementValue("document.body.children[1].children[1]", "random");
 }
 
+// Test that an autofilled credential is deleted then the password manager
+// doesn't try to resurrect it on navigation.
+IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithViewsFeature,
+                       DeletedPasswordIsNotRevived) {
+  // At first let us save a credential to the password store.
+  scoped_refptr<password_manager::TestPasswordStore> password_store =
+      static_cast<password_manager::TestPasswordStore*>(
+          PasswordStoreFactory::GetForProfile(
+              browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS)
+              .get());
+  autofill::PasswordForm signin_form;
+  signin_form.signon_realm = embedded_test_server()->base_url().spec();
+  signin_form.origin = embedded_test_server()->base_url();
+  signin_form.action = embedded_test_server()->base_url();
+  signin_form.username_value = base::ASCIIToUTF16("admin");
+  signin_form.password_value = base::ASCIIToUTF16("1234");
+  password_store->AddLogin(signin_form);
+
+  NavigateToFile("/password/password_form.html");
+  // Let the user interact with the page.
+  content::SimulateMouseClickAt(
+      WebContents(), 0, blink::WebMouseEvent::Button::kLeft, gfx::Point(1, 1));
+  // Wait until that interaction causes the username and the password value to
+  // be revealed.
+  WaitForElementValue("username_field", "admin");
+
+  // Now the credential is removed via the settings or the bubble.
+  password_store->RemoveLogin(signin_form);
+  WaitForPasswordStore();
+
+  // Submit the form. It shouldn't revive the credential in the store.
+  NavigationObserver observer(WebContents());
+  ASSERT_TRUE(content::ExecuteScript(
+      RenderViewHost(),
+      "document.getElementById('input_submit_button').click()"));
+  observer.Wait();
+
+  WaitForPasswordStore();
+  EXPECT_TRUE(password_store->IsEmpty());
+}
+
 IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithViewsFeature,
                        PromptForPushStateWhenFormDisappears) {
   NavigateToFile("/password/password_push_state.html");

chrome/browser/ui/passwords/manage_passwords_state.cc

@@ -57,7 +57,8 @@ bool UpdateFormInVector(
 }
 
 // Removes a form from |forms| that has the same unique key as |form_to_delete|.
-void RemoveFormFromVector(
+// Returns true iff the form was deleted.
+bool RemoveFormFromVector(
     const autofill::PasswordForm& form_to_delete,
     std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) {
   auto it = std::find_if(
@@ -65,8 +66,11 @@ void RemoveFormFromVector(
       [&form_to_delete](const std::unique_ptr<autofill::PasswordForm>& form) {
         return ArePasswordFormUniqueKeyEqual(*form, form_to_delete);
       });
-  if (it != forms->end())
+  if (it != forms->end()) {
     forms->erase(it);
+    return true;
+  }
+  return false;
 }
 
 }  // namespace
@@ -196,19 +200,27 @@ void ManagePasswordsState::ProcessLoginsChanged(
   if (state() == password_manager::ui::INACTIVE_STATE)
     return;
 
+  bool applied_change = false;
   for (const password_manager::PasswordStoreChange& change : changes) {
     const autofill::PasswordForm& changed_form = change.form();
     if (changed_form.blacklisted_by_user)
       continue;
     if (change.type() == password_manager::PasswordStoreChange::REMOVE) {
-      DeleteForm(changed_form);
+      if (RemoveFormFromVector(changed_form, &local_credentials_forms_))
+        applied_change = true;
+    } else if (change.type() == password_manager::PasswordStoreChange::UPDATE) {
+      if (UpdateFormInVector(changed_form, &local_credentials_forms_))
+        applied_change = true;
     } else {
-      if (change.type() == password_manager::PasswordStoreChange::UPDATE)
-        UpdateForm(changed_form);
-      else
-        AddForm(changed_form);
+      DCHECK_EQ(password_manager::PasswordStoreChange::ADD, change.type());
+      if (AddForm(changed_form))
+        applied_change = true;
     }
   }
+  // Let the password manager know that it should update the list of the
+  // credentials.
+  if (applied_change && client_->GetPasswordManager())
+    client_->GetPasswordManager()->UpdateFormManagers();
 }
 
 void ManagePasswordsState::ChooseCredential(
@@ -226,21 +238,14 @@ void ManagePasswordsState::ClearData() {
   credentials_callback_.Reset();
 }
 
-void ManagePasswordsState::AddForm(const autofill::PasswordForm& form) {
+bool ManagePasswordsState::AddForm(const autofill::PasswordForm& form) {
   if (form.origin.GetOrigin() != origin_.GetOrigin())
-    return;
-  if (UpdateForm(form))
-    return;
+    return false;
+  if (UpdateFormInVector(form, &local_credentials_forms_))
+    return true;
   local_credentials_forms_.push_back(
       std::make_unique<autofill::PasswordForm>(form));
-}
-
-bool ManagePasswordsState::UpdateForm(const autofill::PasswordForm& form) {
-  return UpdateFormInVector(form, &local_credentials_forms_);
-}
-
-void ManagePasswordsState::DeleteForm(const autofill::PasswordForm& form) {
-  RemoveFormFromVector(form, &local_credentials_forms_);
+  return true;
 }
 
 void ManagePasswordsState::SetState(password_manager::ui::State state) {

chrome/browser/ui/passwords/manage_passwords_state.h

@@ -113,12 +113,8 @@ class ManagePasswordsState {
   // Removes all the PasswordForms stored in this object.
   void ClearData();
 
-  // Add |form| to the internal state.
-  void AddForm(const autofill::PasswordForm& form);
-  // Updates |form| in the internal state.
-  bool UpdateForm(const autofill::PasswordForm& form);
-  // Removes |form| from the internal state.
-  void DeleteForm(const autofill::PasswordForm& form);
+  // Adds |form| to the internal state if it's relevant.
+  bool AddForm(const autofill::PasswordForm& form);
 
   void SetState(password_manager::ui::State state);