Fix for no password save when action URL is modified

chrome/browser/password_manager/password_manager.cc

Index: chrome/browser/password_manager/password_manager.cc
===================================================================
--- chrome/browser/password_manager/password_manager.cc	(revision 152419)
+++ chrome/browser/password_manager/password_manager.cc	(working copy)
@@ -80,7 +80,8 @@
   for (ScopedVector<PasswordFormManager>::iterator iter =
            pending_login_managers_.begin();
        iter != pending_login_managers_.end(); ++iter) {
-    if ((*iter)->DoesManage(form)) {
+    if ((*iter)->DoesManage(
+        form, PasswordFormManager::kActionMatchRequired)) {
       (*iter)->SetHasGeneratedPassword();
       return;
     }
@@ -114,22 +115,36 @@
     return;
 
   scoped_ptr<PasswordFormManager> manager;
+  ScopedVector<PasswordFormManager>::iterator matched_manager_it =
+      pending_login_managers_.end();
   for (ScopedVector<PasswordFormManager>::iterator iter =
            pending_login_managers_.begin();
        iter != pending_login_managers_.end(); ++iter) {
-    if ((*iter)->DoesManage(form)) {
-      // Transfer ownership of the manager from |pending_login_managers_| to
-      // |manager|.
-      manager.reset(*iter);
-      pending_login_managers_.weak_erase(iter);
+    // If we find a manager that exactly matches the submitted form including
+    // the action URL, exit the loop.
+    if ((*iter)->DoesManage(
+        form, PasswordFormManager::kActionMatchRequired)) {
+      matched_manager_it = iter;
       break;
+    // If the current manager matches the submitted form excluding the action
+    // URL, remember it as a candidate and continue searching for an exact
+    // match.
+    } else if ((*iter)->DoesManage(
+        form, PasswordFormManager::kActionMatchNotRequired)) {

[tim (not reviewing), 2012/09/05 21:40:59]

Can we add a PasswordManagerTest that verifies matches with action mismatch?

[guohui, 2012/09/17 13:28:55]

Done.

+      matched_manager_it = iter;
     }
   }
   // If we didn't find a manager, this means a form was submitted without
   // first loading the page containing the form. Don't offer to save
   // passwords in this case.
-  if (!manager.get())
+  if (matched_manager_it != pending_login_managers_.end()) {
+    // Transfer ownership of the manager from |pending_login_managers_| to
+    // |manager|.
+    manager.reset(*matched_manager_it);
+    pending_login_managers_.weak_erase(matched_manager_it);
+  } else {
     return;
+  }
 
   // If we found a manager but it didn't finish matching yet, the user has
   // tried to submit credentials before we had time to even find matching
@@ -219,7 +234,8 @@
   // First, check for a failed login attempt.
   for (std::vector<PasswordForm>::const_iterator iter = visible_forms.begin();
        iter != visible_forms.end(); ++iter) {
-    if (provisional_save_manager_->DoesManage(*iter)) {
+    if (provisional_save_manager_->DoesManage(
+        *iter, PasswordFormManager::kActionMatchRequired)) {
       // The form trying to be saved has immediately re-appeared. Assume login
       // failure and abort this save, by clearing provisional_save_manager_.
       provisional_save_manager_->SubmitFailed();