Implementation of new behaviour of Password Manager on retry password forms.

components/password_manager/core/browser/password_form_manager.cc

Index: components/password_manager/core/browser/password_form_manager.cc
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index bb1b4b872a2da1d575182bfdcf326f09b529637e..012162b1c4d62cbeafef92098708d8639c15b1c5 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -105,6 +105,7 @@ PasswordFormManager::PasswordFormManager(
       is_new_login_(true),
       has_generated_password_(false),
       password_overridden_(false),
+      retry_password_form_password_update_(false),
       generation_available_(false),
       password_manager_(password_manager),
       preferred_match_(nullptr),
@@ -975,11 +976,11 @@ void PasswordFormManager::CreatePendingCredentials() {
   // autofilled ones, as they may have changed if the user experienced a login
   // failure.
   // Look for these credentials in the list containing auto-fill entries.
-  PasswordFormMap::const_iterator it =
-      best_matches_.find(provisionally_saved_form_->username_value);
-  if (it != best_matches_.end()) {
+  PasswordForm* saved_form =
+      FindBestSavedMatch(provisionally_saved_form_.get());
+  if (saved_form != nullptr) {
     // The user signed in with a login we autofilled.
-    pending_credentials_ = *it->second;
+    pending_credentials_ = *saved_form;
     password_overridden_ =
         pending_credentials_.password_value != password_to_save;
     if (IsPendingCredentialsPublicSuffixMatch() ||
@@ -1053,11 +1054,16 @@ void PasswordFormManager::CreatePendingCredentials() {
     selected_username_ = provisionally_saved_form_->username_value;
     is_new_login_ = false;
   } else if (client_->IsUpdatePasswordUIEnabled() && !best_matches_.empty() &&
-             provisionally_saved_form_
-                 ->IsPossibleChangePasswordFormWithoutUsername()) {
+             (provisionally_saved_form_
+                  ->IsPossibleChangePasswordFormWithoutUsername() ||
+              provisionally_saved_form_->username_element.empty())) {
     PasswordForm* best_update_match = FindBestMatchForUpdatePassword(
         provisionally_saved_form_->password_value);
 
+    retry_password_form_password_update_ =
+        provisionally_saved_form_->username_element.empty() &&
+        provisionally_saved_form_->new_password_element.empty();
+
     if (best_update_match)
       pending_credentials_ = *best_update_match;
     else
@@ -1252,6 +1258,21 @@ PasswordForm* PasswordFormManager::FindBestMatchForUpdatePassword(
              : best_password_match_it->second;
 }
 
+PasswordForm* PasswordFormManager::FindBestSavedMatch(
+    const PasswordForm* form) const {
+  PasswordFormMap::const_iterator it =
+      best_matches_.find(provisionally_saved_form_->username_value);
+  if (it != best_matches_.end())
+    return it->second;
+  if (!form->username_value.empty())
+    return nullptr;
+  for (auto it = best_matches_.begin(); it != best_matches_.end(); ++it) {

[vabr (Chromium), 2015/11/26 14:35:54]

nit: You are occluding the |it| from line 1263. Please use a more meaningful
name for each of the two |it|s.

[vabr (Chromium), 2015/11/26 14:35:54]

From this point until the end of the method, the code works similar to
FindBestMatchForUpdatePassword. The difference is that the other method requires
a unique match, and checks for empty password to speed-up searching. The second
one does not change the result, but the first one does. Are you not requiring a
unique match here on purpose, or could you reuse the
FindBestMatchForUpdatePassword code to avoid duplication?

[vabr (Chromium), 2015/11/26 14:35:54]

nit: Please use a range-based loop for better readability, and const for clean
code:

for (const auto stored_match : best_matches_) {...}

[dvadym, 2015/11/26 15:19:55]

Yeah, I saw that this functions are quite similar, but they are have slightly
different logic, so I don't think that it's worth to do one function. In this
part the difference is that unique match is not required, because I want to
verify that we already have forms with such passwords and not to show bubble, it
doesn't matter if there are multiple stored credentials with this password.

[dvadym, 2015/11/26 15:19:55]

Thanks it will make code cleaner

[dvadym, 2015/11/26 15:19:55]

Done.

[vabr (Chromium), 2015/11/26 17:10:37]

Acknowledged.

+    if (it->second->password_value == form->password_value)
+      return it->second;
+  }
+  return nullptr;
+}
+
 void PasswordFormManager::OnNopeUpdateClicked() {
   UploadChangePasswordForm(autofill::NOT_NEW_PASSWORD, std::string());
 }