[PasswordManager] Improve detection of ignorable change 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 096257b8d1a3114a819949087cb7350fd5c5c466..ad2defff3a948e8cae3461060a1c11becb8f624c 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <set>
 
+#include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/user_metrics.h"
 #include "base/strings/string_split.h"
@@ -15,6 +16,7 @@
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/validation.h"
+#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
 #include "components/password_manager/core/browser/password_manager.h"
@@ -68,6 +70,11 @@ PasswordForm CopyAndModifySSLValidity(const PasswordForm& orig,
   return result;
 }
 
+bool IsChangePasswordForm(const PasswordForm& candidate) {
+  return !candidate.new_password_element.empty() &&
+         !candidate.password_element.empty();
+}
+
 }  // namespace
 
 PasswordFormManager::PasswordFormManager(
@@ -246,13 +253,15 @@ void PasswordFormManager::ProvisionallySave(
   base::string16 password_to_save(credentials.new_password_element.empty() ?
       credentials.password_value : credentials.new_password_value);
 
+  bool is_username_certainly_correct = credentials.username_marked_by_site;
+
   // Make sure the important fields stay the same as the initially observed or
   // 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(credentials.username_value);
-  if (it != best_matches_.end()) {
+  if (!is_username_certainly_correct && it != best_matches_.end()) {
     // The user signed in with a login we autofilled.
     pending_credentials_ = *it->second;
     bool password_changed =
@@ -303,6 +312,16 @@ void PasswordFormManager::ProvisionallySave(
       is_new_login_ = false;
       if (password_changed)
         user_action_ = kUserActionOverridePassword;
+
+      // Check whether its a change-password form with matching credentials in
+      // stored form.
+      PasswordForm matching_form;
+      if (IsChangePasswordForm(credentials) &&
+          HasMatchingCredentialsInStore(credentials, &matching_form)) {
+        pending_credentials_ = matching_form;
+        selected_username_ = matching_form.username_value;
+        user_action_ = kUserActionOverridePassword;
+      }
     }
   } else if (action == ALLOW_OTHER_POSSIBLE_USERNAMES &&
              UpdatePendingCredentialsIfOtherPossibleUsername(
@@ -376,19 +395,6 @@ void PasswordFormManager::FetchMatchingLoginsFromPasswordStore(
     logger->LogMessage(Logger::STRING_FETCH_LOGINS_METHOD);
   }
 
-  // Do not autofill on sign-up or change password forms (until we have some
-  // working change password functionality).
-  if (!observed_form_.new_password_element.empty()) {
-    if (logger)
-      logger->LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED);
-    client_->AutofillResultsComputed();
-    // There is no point in looking for the credentials in the store when they
-    // won't be autofilled, so pretend there were none.
-    std::vector<autofill::PasswordForm*> dummy_results;
-    OnGetPasswordStoreResults(dummy_results);
-    return;
-  }
-
   PasswordStore* password_store = client_->GetPasswordStore();
   if (!password_store) {
     if (logger)
@@ -404,10 +410,8 @@ bool PasswordFormManager::HasCompletedMatching() const {
 }
 
 bool PasswordFormManager::IsIgnorableChangePasswordForm() const {
-  bool is_change_password_form = !observed_form_.new_password_element.empty() &&
-                                 !observed_form_.password_element.empty();
   bool is_username_certainly_correct = observed_form_.username_marked_by_site;
-  return is_change_password_form && !is_username_certainly_correct;
+  return IsChangePasswordForm(observed_form_) && !is_username_certainly_correct;
 }
 
 void PasswordFormManager::OnRequestDone(
@@ -551,8 +555,13 @@ void PasswordFormManager::ProcessFrame(
     manager_action_ = kManagerActionNone;
   else
     manager_action_ = kManagerActionAutofilled;
-  password_manager_->Autofill(driver.get(), observed_form_, best_matches_,
-                              *preferred_match_, wait_for_username);
+
+  // Do not autofill on sign-up or change password forms (until we have some
+  // working change password functionality).
+  if (observed_form_.new_password_element.empty()) {
+    password_manager_->Autofill(driver.get(), observed_form_, best_matches_,
+                                *preferred_match_, wait_for_username);
+  }
 }
 
 void PasswordFormManager::OnGetPasswordStoreResults(
@@ -868,4 +877,25 @@ void PasswordFormManager::SubmitFailed() {
     LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED);
 }
 
+bool PasswordFormManager::HasMatchingCredentialsInStore(
+    const autofill::PasswordForm& candidate,
+    autofill::PasswordForm* matching_form) const {
+  bool match_found = false;
+  for (const auto& match : best_matches_) {
+    // Make sure the username, password and origin signon realm matches to
+    // ensure that the candidate form is one available in the store.
+    if (match.second->username_value == candidate.username_value &&
+        match.second->password_value == candidate.password_value &&
+        match.second->original_signon_realm ==
+            candidate.original_signon_realm) {
+      if (matching_form)
+        *matching_form = *(match.second);
+      match_found = true;
+      break;
+    }
+  }
+
+  return match_found;
+}
+
 }  // namespace password_manager