Update Confirmation UI for saved password change

components/password_manager/core/browser/password_form_manager.h

Index: components/password_manager/core/browser/password_form_manager.h
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h
index 70eeccf9aea470cf1ddf94d31e96d6e65cc4b808..ee50f3df10b828f372d0fd69c4a82a1d970f7e66 100644
--- a/components/password_manager/core/browser/password_form_manager.h
+++ b/components/password_manager/core/browser/password_form_manager.h
@@ -146,6 +146,9 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // TODO: Make this private once we switch to the new UI.
   void Save();
 
+  // Update |credentials_to_update| with the user typed password.

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

It is not clear what it means to update a PasswordForm. Literally, it would mean
that this method just changes something in credentials_to_update, but that's
obviously false (it is a const ref).

Perhaps you wanted to say:
// Update password store entry for |credentials_to_update|, using the password
from |pending_credentials_|.
?

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

Done.

+  void Update(const autofill::PasswordForm& credentials_to_update);
+
   // Call these if/when we know the form submission worked or failed.
   // These routines are used to update internal statistics ("ActionsTaken").
   void SubmitPassed();
@@ -332,6 +335,11 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // from password store.
   void CreatePendingCredentials();
 
+  // Return best candidate for password update during processing the password

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

I would not mention change password forms, the method is oblivious to them.
Rather, mention the criteria for selecting a result. What about:

// For non-empty |password| returns the unique entry in |best_matches_| with the
same password, if it exists, and nullptr otherwise. For empty passwords, if
there is just one entry in |best_matches_|, returns that entry, otherwise
nullptr.

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

I changed comments to explaining how it works. 

+  // change form.
+  autofill::PasswordFormMap::const_iterator FindBestMatchForPasswordChange(

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

The "Change" in the name sounds confusing. What about
FindUniqueBestMatchWithPassword
?

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

I suggest changing the return type to
const PasswordForm*
with nullptr representing "no match found". The reason follows.

I don't think the const_iterator is useful here. Consider this snippet:

#include <iostream>
#include <map>

int main() {
  std::map<int, int*> m;
  int a = 4;
  m[5] = &a;
  auto it = m.cbegin();
  *(it->second) = 7;
  std::cout << "a=" << a << "\n";
}

It returns "a=7", because while it->second is a const pointer, it is not a
pointer to a const object. The same is true of pointers to PasswordForm, which
are the values here.

You only care about the PasswordForm pointers, not about the iterator itself, so
let's make it simpler.

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

Done.

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

Change to FindBestMatchForUpdatePassword. It's not fully correct, since we
matching not only by passwords. And I believe that name shouldn't contain too
much information about implementation, since implementation probably will
change.

+      const base::string16& password);
+
   // Set of PasswordForms from the DB that best match the form
   // being managed by this. Use a map instead of vector, because we most
   // frequently require lookups by username value in IsNewLogin.