Allow deleting autofill password suggestions on Shift+Delete

components/password_manager/core/browser/password_manager.cc

Index: components/password_manager/core/browser/password_manager.cc
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc
index 16ba3f4c78134008e21ea486775a7d0992c3d763..6970f34bbb94e9c78ce0d7a6612604b944f35a65 100644
--- a/components/password_manager/core/browser/password_manager.cc
+++ b/components/password_manager/core/browser/password_manager.cc
@@ -267,6 +267,64 @@ void PasswordManager::OnPasswordFormsParsed(
   }
 }
 
+void PasswordManager::OnRemovePasswordSuggestion(

[vabr (Chromium), 2014/04/04 10:51:40]

I suggest the following simplification of this method:

1) Erase the stored password, identified by |password_form| from the password
store before updating any PasswordFormManagers.
2) Then go through all the PasswordFormManagers, and call Update on them,
regardless of what they manage.

Step 2) should not make a lot of difference, as far as I understand the code,
the PasswordFormManagers should actually correspond 1:1 to
|updated_password_forms|.

As a bonus, you can get rid of the vector of PasswordForms in the argument and
the IPC message.

[vabr (Chromium), 2014/04/04 10:51:40]

Also, I think we are missing a test for this method yet. Could you add
something?

+    const PasswordForm& password_form,
+    const std::vector<PasswordForm>& updated_password_forms) {
+

[vabr (Chromium), 2014/04/04 10:51:40]

nit: unnecessary blank line

+  // Deleting the PasswordFormManager for the current |password_form| to avoid

[vabr (Chromium), 2014/04/04 10:51:40]

I did not understand what this comment warned about, and I suggest dropping it
completely. Feel free to push back on this, if you find it useful.

+  // that we have two PasswordFormManager for it:
+  // AutofillHostMsg_PasswordFormsParsed is going to be received in the near
+  // future to update the current form and this is going to generate a new
+  // PasswordFormManager.
+
+  // Avoid prompting the user for access to a password if they don't have

[vabr (Chromium), 2014/04/04 10:51:40]

nit: instead of duplicating this piece of code from
PasswordManager::OnPasswordFormsParsed, consider putting it in a helper method
and use on those two places.

+  // password saving enabled.
+  PasswordStore::AuthorizationPromptPolicy prompt_policy =
+      *password_manager_enabled_ ? PasswordStore::ALLOW_PROMPT
+                                 : PasswordStore::DISALLOW_PROMPT;
+  std::vector<PasswordFormManager*> not_yet_updated;
+  bool removed = false;
+  for (ScopedVector<PasswordFormManager>::iterator iter =
+           pending_login_managers_.begin();
+       iter != pending_login_managers_.end();
+       ++iter) {
+    if ((*iter)->DoesManage(password_form,
+                            PasswordFormManager::ACTION_MATCH_REQUIRED)) {
+      if (!removed) {
+        (*iter)->Update(password_form.username_value, prompt_policy, true);
+        removed = true;
+      } else {
+        (*iter)->Update(password_form.username_value, prompt_policy, false);
+      }
+    } else {
+      not_yet_updated.push_back(*iter);
+    }
+  }
+
+  if (removed) {
+    std::set<const PasswordForm*> already_updated;
+    for (std::vector<PasswordFormManager*>::iterator
+             form_manager_update_candidate = not_yet_updated.begin();
+         form_manager_update_candidate != not_yet_updated.end();
+         ++form_manager_update_candidate) {
+      for (std::vector<PasswordForm>::const_iterator form_update_condidate =
+               updated_password_forms.begin();
+           form_update_condidate != updated_password_forms.end();
+           ++form_update_condidate) {
+        if ((already_updated.find(&(*form_update_condidate)) ==
+             already_updated.end()) &&
+            ((*form_manager_update_candidate)
+                 ->DoesManage(*form_update_condidate,
+                              PasswordFormManager::ACTION_MATCH_REQUIRED))) {
+          (*form_manager_update_candidate)
+              ->Update(password_form.username_value, prompt_policy, false);
+          already_updated.insert(&(*form_update_condidate));
+        }
+      }
+    }
+  }
+}
+
 bool PasswordManager::ShouldPromptUserToSavePassword() const {
   return provisional_save_manager_->IsNewLogin() &&
          !provisional_save_manager_->HasGeneratedPassword() &&