Update Confirmation UI for saved password change

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 e331c63d6f9258a5b7bbbad44d303628fc5622f5..c7bf35f7b14f63cb88486f3a26ef2d5a1eab5abe 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -294,6 +294,16 @@ void PasswordFormManager::Save() {
     UpdateLogin();
 }
 
+void PasswordFormManager::Update(
+    const autofill::PasswordForm& credentials_to_update) {
+  base::string16 password_to_save = pending_credentials_.password_value;
+  pending_credentials_ = credentials_to_update;
+  pending_credentials_.password_value = password_to_save;
+  pending_credentials_.preferred = true;
+  is_new_login_ = false;
+  UpdateLogin();
+}
+
 void PasswordFormManager::FetchMatchingLoginsFromPasswordStore(
     PasswordStore::AuthorizationPromptPolicy prompt_policy) {
   DCHECK_EQ(state_, PRE_MATCHING_PHASE);
@@ -323,7 +333,8 @@ void PasswordFormManager::SetSubmittedForm(const autofill::PasswordForm& form) {
   bool is_change_password_form =
       !form.new_password_value.empty() && !form.password_value.empty();
   is_ignorable_change_password_form_ =
-      is_change_password_form && !form.username_marked_by_site &&
+      !client_->IsUpdatePasswordUIEnabled() && is_change_password_form &&

[vasilii, 2015/07/31 14:38:54]

I'd put this new expression to the end of the condition for clarity.

[dvadym, 2015/08/03 15:44:08]

Done.

+      !form.username_marked_by_site &&
       !DoesUsenameAndPasswordMatchCredentials(
           form.username_value, form.password_value, best_matches_);
   bool is_signup_form =
@@ -865,6 +876,19 @@ void PasswordFormManager::CreatePendingCredentials() {
     // credential.
     selected_username_ = provisionally_saved_form_->username_value;
     is_new_login_ = false;
+  } else if (client_->IsUpdatePasswordUIEnabled() && !best_matches_.empty() &&
+             provisionally_saved_form_
+                 ->IsPossibleChangePasswordFormWithoutUsername()) {
+    PasswordForm* best_update_match = FindBestMatchForUpdatePassword(
+        provisionally_saved_form_->password_value);
+
+    if (best_update_match) {
+      pending_credentials_ = *best_update_match;

[vasilii, 2015/07/31 14:38:54]

Here you overwrite pending_credentials_ so it contains the old password. Later
the UI calls Update(pending_credentials) and the old password goes to the store.

[dvadym, 2015/08/03 15:44:08]

This code is part of a function CreatePendingCredentials(), this is not
overwriting, this is exactly the place where pending_credentials is created.
Before this assignment pending_credentials is an empty object. A password field
is set on the line 925. 

+    }

[vasilii, 2015/07/31 14:38:54]

no need in brackets. The surrounding code doesn't use them.

[dvadym, 2015/08/03 15:44:08]

Done.

+    pending_credentials_.origin = provisionally_saved_form_->origin;

[vasilii, 2015/07/31 14:38:54]

Why overwriting the origin?

[dvadym, 2015/08/03 15:44:08]

Good point, we need to set origin only in case when no best match found, because
origin can't be empty.

+    is_new_login_ = false;
+    // We don't care about |pending_credentials_| if we didn't find the best
+    // match, since the user will select the correct one.
   } else {
     // User typed in a new, unknown username.
     user_action_ = kUserActionOverrideUsernameAndPassword;
@@ -970,6 +994,31 @@ int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const {
   return score;
 }
 
+PasswordForm* PasswordFormManager::FindBestMatchForUpdatePassword(
+    const base::string16& password) const {
+  if (password.empty()) {
+    if (best_matches_.size() == 1) {
+      // In case when there is no old password on the form and the user has only
+      // one piece of credentials, consider it the same as is being saved.
+      return best_matches_.begin()->second;
+    }
+    return nullptr;
+  }
+  PasswordFormMap::const_iterator best_password_match_it = best_matches_.end();
+  for (auto it = best_matches_.begin(); it != best_matches_.end(); ++it) {
+    if (it->second->password_value == password) {
+      if (best_password_match_it != best_matches_.end()) {
+        // Found a second credential with the same password, do nothing.
+        return nullptr;
+      }
+      best_password_match_it = it;
+    }
+  }
+  return best_password_match_it == best_matches_.end()
+             ? nullptr
+             : best_password_match_it->second;
+}
+
 void PasswordFormManager::SubmitPassed() {
   submit_result_ = kSubmitResultPassed;
   if (has_generated_password_)