Don't show "Save password" prompt for a failed login

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 73c54ef1461df217249b70870773d1dc8bcfb83c..7abefede12b5569f91f5fd4664dba641f7ec794f 100644
--- a/components/password_manager/core/browser/password_manager.cc
+++ b/components/password_manager/core/browser/password_manager.cc
@@ -340,7 +340,8 @@ bool PasswordManager::ShouldPromptUserToSavePassword() const {
 }
 
 void PasswordManager::OnPasswordFormsRendered(
-    const std::vector<PasswordForm>& visible_forms) {
+    const std::vector<PasswordForm>& visible_forms,
+    bool did_stop_loading) {
   scoped_ptr<BrowserSavePasswordProgressLogger> logger;
   if (client_->IsLoggingActive()) {
     logger.reset(new BrowserSavePasswordProgressLogger(client_));
@@ -362,40 +363,56 @@ void PasswordManager::OnPasswordFormsRendered(
                       visible_forms.size());
   }
 
-  // If we see the login form again, then the login failed.
+  // Records all visible forms in the page.

[vabr (Chromium), 2014/05/23 12:18:36]

page or frame?

   for (size_t i = 0; i < visible_forms.size(); ++i) {
-    // TODO(vabr): The similarity check is just action equality for now. If it
-    // becomes more complex, it may make sense to consider modifying and using
-    // PasswordFormManager::DoesManage for it.
-    if (visible_forms[i].action.is_valid() &&
-        provisional_save_manager_->pending_credentials().action ==
-            visible_forms[i].action) {
-      if (logger) {
-        logger->LogPasswordForm(Logger::STRING_PASSWORD_FORM_REAPPEARED,
-                                visible_forms[i]);
-        logger->LogMessage(Logger::STRING_DECISION_DROP);
+    all_visible_forms_.push_back(visible_forms[i]);

[vabr (Chromium), 2014/05/23 12:18:36]

nit: consider using std::vector::insert

+  }
+
+  // If we see the login form again, then the login failed.
+  if (did_stop_loading) {
+    for (size_t i = 0; i < all_visible_forms_.size(); ++i) {
+      // TODO(vabr): The similarity check is just action equality for now. If it
+      // becomes more complex, it may make sense to consider modifying and using
+      // PasswordFormManager::DoesManage for it.
+      if (all_visible_forms_[i].action.is_valid() &&
+          provisional_save_manager_->pending_credentials().action ==
+              all_visible_forms_[i].action) {
+        if (logger) {
+          logger->LogPasswordForm(Logger::STRING_PASSWORD_FORM_REAPPEARED,
+                                  visible_forms[i]);
+          logger->LogMessage(Logger::STRING_DECISION_DROP);
+        }
+        provisional_save_manager_->SubmitFailed();
+        provisional_save_manager_.reset();
+        // Clear all_visible_forms_ once we found the match.
+        while (all_visible_forms_.size()) {

[vabr (Chromium), 2014/05/23 12:18:36]

Did you just mean all_visible_forms_.clear()?

This while loop looks very inefficient to achieve clear(), but maybe I'm missing
something?

Also below.

+          all_visible_forms_.resize(all_visible_forms_.size() - 1);
+        }
+        return;
       }
-      provisional_save_manager_->SubmitFailed();
-      provisional_save_manager_.reset();
-      return;
     }
-  }
 
-  // Looks like a successful login attempt. Either show an infobar or
-  // automatically save the login data. We prompt when the user hasn't already
-  // given consent, either through previously accepting the infobar or by having
-  // the browser generate the password.
-  provisional_save_manager_->SubmitPassed();
+    // Clear all_visible_forms_ after checking all the forms.
+    while (all_visible_forms_.size()) {
+      all_visible_forms_.resize(all_visible_forms_.size() - 1);
+    }
 
-  if (ShouldPromptUserToSavePassword()) {
-    if (logger)
-      logger->LogMessage(Logger::STRING_DECISION_ASK);
-    client_->PromptUserToSavePassword(provisional_save_manager_.release());
-  } else {
-    if (logger)
-      logger->LogMessage(Logger::STRING_DECISION_SAVE);
-    provisional_save_manager_->Save();
-    provisional_save_manager_.reset();
+    // Looks like a successful login attempt. Either show an infobar or
+    // automatically save the login data. We prompt when the user hasn't
+    // already given consent, either through previously accepting the infobar
+    // or by having the browser generate the password.
+    provisional_save_manager_->SubmitPassed();
+
+    if (ShouldPromptUserToSavePassword()) {
+      if (logger)
+        logger->LogMessage(Logger::STRING_DECISION_ASK);
+      client_->PromptUserToSavePassword(provisional_save_manager_.release());
+    } else {
+      if (logger)
+        logger->LogMessage(Logger::STRING_DECISION_SAVE);
+      provisional_save_manager_->Save();
+      provisional_save_manager_.reset();
+    }
   }
 }