[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..de3b4b82a9cafd3470423688f4a6ee062f9c957d 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -376,19 +376,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)
@@ -403,11 +390,12 @@ bool PasswordFormManager::HasCompletedMatching() const {
   return state_ == POST_MATCHING_PHASE;
 }
 
-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;
+bool PasswordFormManager::IsIgnorableChangePasswordForm(
+    const autofill::PasswordForm& candidate) const {
+  bool is_change_password_form = !candidate.new_password_element.empty() &&
+                                 !candidate.password_element.empty();
+  return is_change_password_form && !candidate.username_marked_by_site &&
+         !HasMatchingCredentialsInStore(candidate);
 }
 
 void PasswordFormManager::OnRequestDone(
@@ -551,6 +539,19 @@ void PasswordFormManager::ProcessFrame(
     manager_action_ = kManagerActionNone;
   else
     manager_action_ = kManagerActionAutofilled;
+
+  // Do not autofill on sign-up or change password forms (until we have some

[vabr (Chromium), 2015/02/10 18:54:56]

Could you move the whole added block above line 530, which says "Proceed to
autofill"?

[Pritam Nikam, 2015/02/19 11:18:47]

Done.

+  // working change password functionality).
+  if (!observed_form_.new_password_element.empty()) {
+    scoped_ptr<BrowserSavePasswordProgressLogger> logger;

[vabr (Chromium), 2015/02/10 18:54:56]

You only need the |logger| in the if-block below, so don't use scoped_ptr,
create it simply as a stack variable.
Also, please add a log message identifying the current method (see the *_METHOD
constants in components/autofill/core/common/save_password_progress_logger.cc
for the pattern to use). In other words, please do this:
if (...IsLoggingActive...) {
  BrowserSavePasswordProgressLogger logger(client_);
  logger.LogMessage(Logger::PROCESS_FRAME_METHOD);
  logger.LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED);
}

[Pritam Nikam, 2015/02/19 11:18:47]

Done.

+    if (client_->IsLoggingActive()) {
+      logger.reset(new BrowserSavePasswordProgressLogger(client_));
+      logger->LogMessage(Logger::STRING_FORM_NOT_AUTOFILLED);
+    }
+    client_->AutofillResultsComputed();

[vabr (Chromium), 2015/02/10 18:54:57]

Remove this line. AutofillResultsComputed has already been called when the
password store results were received.

[Pritam Nikam, 2015/02/19 11:18:47]

Done.

+    return;
+  }
+
   password_manager_->Autofill(driver.get(), observed_form_, best_matches_,
                               *preferred_match_, wait_for_username);
 }
@@ -868,4 +869,14 @@ void PasswordFormManager::SubmitFailed() {
     LogPasswordGenerationSubmissionEvent(PASSWORD_SUBMISSION_FAILED);
 }
 
+bool PasswordFormManager::HasMatchingCredentialsInStore(
+    const autofill::PasswordForm& candidate) const {
+  for (auto match : best_matches_) {
+    if (match.second->username_value == candidate.username_value &&
+        match.second->password_value == candidate.password_value)
+      return true;
+  }
+  return false;
+}
+
 }  // namespace password_manager