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 b83e1c0127d01440c882f90f931207abd081837a..5c6d122c1e3a8fed8b5b4ae0decb2a2fa1f4d1a7 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -282,6 +282,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);
@@ -844,6 +854,22 @@ void PasswordFormManager::CreatePendingCredentials() {
     // credential.
     selected_username_ = provisionally_saved_form_->username_value;
     is_new_login_ = false;
+  } else if (client_->IsPasswordUpdateUIEnabled() &&
+             !provisionally_saved_form_->new_password_element.empty() &&
+             provisionally_saved_form_->username_value.empty()) {
+    // This is password change form without username. If username is present

[vabr (Chromium), 2015/06/19 18:03:16]

Please keep the comments short. If this block is for forms without username, it
has no sense to comment on what happens to forms with usernames.

And actually, since the last if condition is verbose enough and covers the first
sentence of the comment completely, I suggest to just drop the whole comment.

[dvadym, 2015/06/22 14:43:33]

Done.

+    // processing of password change form is almost the same as usual sign-in
+    // forms.
+    PasswordFormMap::const_iterator best_password_match_it =
+        FindBestMatchForPasswordChange(
+            provisionally_saved_form_->password_value);
+
+    if (best_password_match_it != best_matches_.end()) {
+      pending_credentials_ = *best_password_match_it->second;
+    }
+    pending_credentials_.is_password_change_form_without_username = true;
+    // We don't care about |pending_credentials| if we didn't find best match,

[vabr (Chromium), 2015/06/19 18:03:17]

If we don't care about |pending_credentials_| if there is no best match, why
don't we just modify is_password_change_form_without_username inside the if
block above, and spare us the comment here?

[vabr (Chromium), 2015/06/19 18:03:17]

nit: |pending_credentials| -> |pending_credentials_|

[vabr (Chromium), 2015/06/19 18:03:17]

grammar: best match -> the best match

[dvadym, 2015/06/22 14:43:33]

We need to set is_password_change_form_without_username=true anyway, have we
best match or not. If we don't have best match, we will allow the user to choose
correct one later. That's why we can't set it in if block. And I think comment
is needed in order to get hint why we don't set |pending_credentials_|, but
probably we need to change wording. I would propose to discuss it offline, it
would be faster.

[dvadym, 2015/06/22 14:43:33]

Done.

[dvadym, 2015/06/22 14:43:33]

Done.

+    // since the user will select the correct one.
   } else {
     // User typed in a new, unknown username.
     user_action_ = kUserActionOverrideUsernameAndPassword;
@@ -860,11 +886,12 @@ void PasswordFormManager::CreatePendingCredentials() {
     pending_credentials_.password_value.clear();
     pending_credentials_.new_password_value.clear();
 
-    // If this was a sign-up or change password form, the names of the elements
-    // are likely different than those on a login form, so do not bother saving
-    // them. We will fill them with meaningful values in UpdateLogin() when the
-    // user goes onto a real login form for the first time.
     if (!provisionally_saved_form_->new_password_element.empty()) {
+      // If this was a sign-up or change password form, the names of the

[vabr (Chromium), 2015/06/19 18:03:16]

nit: Is there a real reason to move the block of comments? It is already clear
that it belongs to the if-block, and plan moving and reformating it messes up
the git blame, and increases the change of this CL.

[dvadym, 2015/06/22 14:43:33]

Just traces of intermediate implementation, where else block existed. Comments
were moved to previous place.

+      // elements are likely different than those on a login form, so do not
+      // bother saving them. We will fill them with meaningful values in
+      // UpdateLogin() when the user goes onto a real login form for the first
+      // time.
       pending_credentials_.password_element.clear();
       pending_credentials_.new_password_element.clear();
     }
@@ -946,6 +973,30 @@ int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const {
   return score;
 }
 
+PasswordFormMap::const_iterator
+PasswordFormManager::FindBestMatchForPasswordChange(
+    const base::string16& password) {
+  if (password.empty()) {
+    if (best_matches_.size() == 1) {
+      // In case when there is no old password on the form and the user has only
+      // 1 credentials, consider this as the same piece of credentials.

[vabr (Chromium), 2015/06/19 18:03:16]

nit: Consider replacing line 982 with
// one piece of credentials, consider it the same as is being saved.

[dvadym, 2015/06/22 14:43:33]

Done.

+      return best_matches_.begin();
+    }
+    return best_matches_.end();
+  }
+  PasswordFormMap::const_iterator best_password_match_it = best_matches_.end();
+  for (auto it = best_matches_.cbegin(); it != best_matches_.cend(); ++it) {
+    if (it->second->password_value == password) {
+      if (best_password_match_it != best_matches_.end()) {
+        // Found the second credential with the same password, do nothing.
+        return best_matches_.end();
+      }
+      best_password_match_it = it;
+    }
+  }
+  return best_password_match_it;
+}
+
 void PasswordFormManager::SubmitPassed() {
   submit_result_ = kSubmitResultPassed;
   if (has_generated_password_)