PasswordFormManager should not read from store for signup and change password forms

We do not autofill such forms, but currently, we only check if an observed form should be ignored after we ask the password store for corresponding results.

This CL pulls out that check before asking the store. It should not result in any change in behaviour, hence no tests added.

BUG=447611
R=engedy@chromium.org

Review URL: https://codereview.chromium.org/845763002

Cr-Commit-Position: refs/heads/master@{#310843}

4 files changed, 16 insertions, 11 deletions

diff --git a/components/autofill/core/common/save_password_progress_logger.cc b/components/autofill/core/common/save_password_progress_logger.cc
index 686c834..872ac79 100644
--- a/components/autofill/core/common/save_password_progress_logger.cc
+++ b/components/autofill/core/common/save_password_progress_logger.cc
@@ -207,6 +207,8 @@ std::string GetStringFromID(SavePasswordProgressLogger::StringID id) {
       return "ShowLoginPrompt";
     case SavePasswordProgressLogger::STRING_NEW_UI_STATE:
       return "The new state of the UI";
+    case SavePasswordProgressLogger::STRING_FORM_NOT_AUTOFILLED:
+      return "The observed form will not be autofilled";
     case SavePasswordProgressLogger::STRING_INVALID:
       return "INVALID";
       // Intentionally no default: clause here -- all IDs need to get covered.
diff --git a/components/autofill/core/common/save_password_progress_logger.h b/components/autofill/core/common/save_password_progress_logger.h
index 93bf321..cf1ed6f 100644
--- a/components/autofill/core/common/save_password_progress_logger.h
+++ b/components/autofill/core/common/save_password_progress_logger.h
@@ -120,6 +120,7 @@ class SavePasswordProgressLogger {
     STRING_CLIENT_CHECK_PRESENT,
     STRING_SHOW_LOGIN_PROMPT_METHOD,
     STRING_NEW_UI_STATE,
+    STRING_FORM_NOT_AUTOFILLED,
     STRING_INVALID,  // Represents a string returned in a case of an error.
     STRING_MAX = STRING_INVALID
   };
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index 9f2bd8e..ecb2c37 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -375,6 +375,19 @@ 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)
@@ -557,10 +570,6 @@ void PasswordFormManager::OnGetPasswordStoreResults(
 }
 
 bool PasswordFormManager::ShouldIgnoreResult(const PasswordForm& form) const {
-  // 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())
-    return true;
   // Don't match an invalid SSL form with one saved under secure circumstances.
   if (form.ssl_valid && !observed_form_.ssl_valid)
     return true;
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc
index f04083f..ce787d0 100644
--- a/components/password_manager/core/browser/password_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -279,10 +279,7 @@ TEST_F(PasswordManagerTest, FormSubmitWithOnlyNewPasswordField) {
   // This test is the same as FormSubmitEmptyStore, except that it simulates the
   // user entering credentials into a sign-up form that only has a new password
   // field.
-  std::vector<PasswordForm*> result;  // Empty password store.
   EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
-  EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
-      .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
   PasswordForm form(MakeFormWithOnlyNewPasswordField());
   observed.push_back(form);
@@ -824,9 +821,6 @@ TEST_F(PasswordManagerTest,
 // Create a form with a new_password_element. Submit the form with the empty
 // new password value. It shouldn't overwrite the existing password.
 TEST_F(PasswordManagerTest, DoNotUpdateWithEmptyPassword) {
-  std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
-      .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
   PasswordForm form(MakeSimpleForm());
   form.new_password_element = ASCIIToUTF16("new_password_element");
@@ -839,7 +833,6 @@ TEST_F(PasswordManagerTest, DoNotUpdateWithEmptyPassword) {
   // And the form submit contract is to call ProvisionallySavePassword.
   OnPasswordFormSubmitted(form);
 
-  scoped_ptr<PasswordFormManager> form_to_save;
   EXPECT_CALL(client_, PromptUserToSavePasswordPtr(_)).Times(0);
 
   // Now the password manager waits for the login to complete successfully.