Password manager: improve the check for login failure

chrome/browser/password_manager/password_manager.cc

Index: chrome/browser/password_manager/password_manager.cc
diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc
index eb6adfc9ba7557a439379f098998a076c363657d..f6f8e0d4e6ac60bf7e12a34912f1c9c997aafc6d 100644
--- a/chrome/browser/password_manager/password_manager.cc
+++ b/chrome/browser/password_manager/password_manager.cc
@@ -350,12 +350,15 @@ void PasswordManager::OnPasswordFormsRendered(
 
   DCHECK(IsSavingEnabled());
 
-  // We now assume that if there is at least one visible password form
-  // that means that the previous login attempt failed.
-  if (!visible_forms.empty()) {
-    provisional_save_manager_->SubmitFailed();
-    provisional_save_manager_.reset();
-    return;
+  // If we see the login form again, then the login failed.
+  for (size_t i = 0; i < visible_forms.size(); ++i) {
+    if (visible_forms[i].action.is_valid() &&
+        provisional_save_manager_->pending_credentials().action ==
+            visible_forms[i].action) {

[Garrett Casto, 2014/01/15 02:28:27]

One thing to note is that it looks like at some point
PasswordFormManager::DoesManage() was used to check this (see
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pas...).
 This was before my time so I'm not sure why this was removed, but if it turns
out that your change makes matching too loose switching to DoesManage seems
reasonable.

[vabr (Chromium), 2014/01/15 08:37:53]

I actually started this CL with DoesManage + ACTION_MATCH_REQUIRED, but it
always returned false, because the names of the username or password elements
were not the same.

For now, I think just checking the actions is obvious enough to leave there
directly. If we end up with a more complex similarity check, it may make sense
to modify and use DoesManage for it. I'll put a comment saying this in the code.

On 2014/01/15 02:28:27, Garrett Casto wrote:

+      provisional_save_manager_->SubmitFailed();
+      provisional_save_manager_.reset();
+      return;
+    }
   }
 
   // Looks like a successful login attempt. Either show an infobar or