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

components/password_manager/core/browser/password_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/password_manager/core/browser/password_manager.h"
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
@@ skipping 322 matching lines @@
 }
 
 bool PasswordManager::ShouldPromptUserToSavePassword() const {
   return !client_->IsAutomaticPasswordSavingEnabled() &&
          provisional_save_manager_->IsNewLogin() &&
          !provisional_save_manager_->HasGeneratedPassword() &&
          !provisional_save_manager_->IsPendingCredentialsPublicSuffixMatch();
 }
 
 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_));
     logger->LogMessage(Logger::STRING_ON_PASSWORD_FORMS_RENDERED_METHOD);
   }
 
   if (!provisional_save_manager_.get()) {
     if (logger) {
       logger->LogMessage(Logger::STRING_NO_PROVISIONAL_SAVE_MANAGER);
       logger->LogMessage(Logger::STRING_DECISION_DROP);
     }
     return;
   }
 
   DCHECK(IsSavingEnabled());
 
   if (logger) {
     logger->LogNumber(Logger::STRING_NUMBER_OF_VISIBLE_FORMS,
                       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);
-      }
-      provisional_save_manager_->SubmitFailed();
-      provisional_save_manager_.reset();
-      return;
-    }
+    all_visible_forms_.push_back(visible_forms[i]);

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

nit: consider using std::vector::insert

   }
 
-  // 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 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;
+      }
+    }
 
-  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();
+    // Clear all_visible_forms_ after checking all the forms.
+    while (all_visible_forms_.size()) {
+      all_visible_forms_.resize(all_visible_forms_.size() - 1);
+    }
+
+    // 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();
+    }
   }
 }
 
 void PasswordManager::PossiblyInitializeUsernamesExperiment(
     const PasswordFormMap& best_matches) const {
   if (base::FieldTrialList::Find(kOtherPossibleUsernamesExperiment))
     return;
 
   bool other_possible_usernames_exist = false;
   for (autofill::PasswordFormMap::const_iterator it = best_matches.begin();
@@ skipping 68 matching lines @@
           observers_,
           OnAutofillDataAvailable(preferred_match.username_value,
                                   preferred_match.password_value));
       break;
   }
 
   client_->PasswordWasAutofilled(best_matches);
 }
 
 }  // namespace password_manager