PasswordSaveManagerImpl: Split FillPendingCredentials

...into ComputePendingCredentialsState and BuildPendingCredentials.
This will allow us to call ComputePendingCredentialsState twice in
MultiStorePasswordSaveManager - separately for the account matches and
the profile matches.

Bug: 1012203
Change-Id: Ie017474b84e4dc8b0b82bb4bb35a483dfb1a9175
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116039
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753537}

components/password_manager/core/browser/password_save_manager_impl.cc

@@ -152,82 +152,101 @@ void PasswordSaveManagerImpl::Init(
 }
 
 // static
-PendingCredentialsState PasswordSaveManagerImpl::FillPendingCredentials(
+PendingCredentialsState PasswordSaveManagerImpl::ComputePendingCredentialsState(
+    const PasswordForm& parsed_submitted_form,
+    const PasswordForm* similar_saved_form) {
+  ValueElementPair password_to_save(PasswordToSave(parsed_submitted_form));
+  // Check if there are previously saved credentials (that were available to
+  // autofilling) matching the actually submitted credentials.
+  if (!similar_saved_form)
+    return PendingCredentialsState::NEW_LOGIN;
+
+  // A similar credential exists in the store already.
+  if (similar_saved_form->password_value != password_to_save.first)
+    return PendingCredentialsState::UPDATE;
+
+  // If the autofilled credentials were a PSL match, store a copy with the
+  // current origin and signon realm. This ensures that on the next visit, a
+  // precise match is found.
+  if (similar_saved_form->is_public_suffix_match)
+    return PendingCredentialsState::AUTOMATIC_SAVE;
+
+  return PendingCredentialsState::EQUAL_TO_SAVED_MATCH;
+}
+
+// static
+PasswordForm PasswordSaveManagerImpl::BuildPendingCredentials(
+    PendingCredentialsState pending_credentials_state,
     const PasswordForm& parsed_submitted_form,
     const FormData& observed_form,
     const FormData& submitted_form,
     const base::Optional<base::string16>& generated_password,
     bool is_http_auth,
     bool is_credential_api_save,
-    const PasswordForm* similar_saved_form,
-    PasswordForm* pending_credentials) {
-  DCHECK(pending_credentials);
-
-  PendingCredentialsState pending_credentials_state =
-      PendingCredentialsState::NONE;
+    const PasswordForm* similar_saved_form) {
+  PasswordForm pending_credentials;
 
   ValueElementPair password_to_save(PasswordToSave(parsed_submitted_form));
-  // Check if there are previously saved credentials (that were available to
-  // autofilling) matching the actually submitted credentials.
-  if (similar_saved_form) {
-    // A similar credential exists in the store already.
-    *pending_credentials = *similar_saved_form;
-    pending_credentials_state = PendingCredentialsState::EQUAL_TO_SAVED_MATCH;
-    if (pending_credentials->password_value != password_to_save.first) {
-      pending_credentials_state = PendingCredentialsState::UPDATE;
-    } else if (pending_credentials->is_public_suffix_match) {
-      // If the autofilled credentials were a PSL match, store a copy with the
-      // current origin and signon realm. This ensures that on the next visit, a
-      // precise match is found.
-      pending_credentials_state = PendingCredentialsState::AUTOMATIC_SAVE;
+
+  switch (pending_credentials_state) {
+    case PendingCredentialsState::NEW_LOGIN:
+      // No stored credentials can be matched to the submitted form. Offer to
+      // save new credentials.
+      pending_credentials = PendingCredentialsForNewCredentials(
+          parsed_submitted_form, observed_form, password_to_save.second,
+          is_http_auth, is_credential_api_save);
+      break;
+    case PendingCredentialsState::EQUAL_TO_SAVED_MATCH:
+    case PendingCredentialsState::UPDATE:
+      pending_credentials = *similar_saved_form;
+      break;
+    case PendingCredentialsState::AUTOMATIC_SAVE:
+      pending_credentials = *similar_saved_form;
+
       // Update credential to reflect that it has been used for submission.
       // If this isn't updated, then password generation uploads are off for
       // sites where PSL matching is required to fill the login form, as two
       // PASSWORD votes are uploaded per saved password instead of one.
-      password_manager_util::UpdateMetadataForUsage(pending_credentials);
+      password_manager_util::UpdateMetadataForUsage(&pending_credentials);
 
       // Update |pending_credentials| in order to be able correctly save it.
-      pending_credentials->origin = parsed_submitted_form.origin;
-      pending_credentials->signon_realm = parsed_submitted_form.signon_realm;
-      pending_credentials->action = parsed_submitted_form.action;
-    }
-  } else {
-    pending_credentials_state = PendingCredentialsState::NEW_LOGIN;
-    // No stored credentials can be matched to the submitted form. Offer to
-    // save new credentials.
-    *pending_credentials = PendingCredentialsForNewCredentials(
-        parsed_submitted_form, observed_form, password_to_save.second,
-        is_http_auth, is_credential_api_save);
+      pending_credentials.origin = parsed_submitted_form.origin;
+      pending_credentials.signon_realm = parsed_submitted_form.signon_realm;
+      pending_credentials.action = parsed_submitted_form.action;
+      break;
+    case PendingCredentialsState::NONE:
+      NOTREACHED();
+      break;
   }
-  pending_credentials->password_value =
+
+  pending_credentials.password_value =
       generated_password.value_or(password_to_save.first);
-  pending_credentials->date_last_used = base::Time::Now();
-  pending_credentials->form_has_autofilled_value =
+  pending_credentials.date_last_used = base::Time::Now();
+  pending_credentials.form_has_autofilled_value =
       parsed_submitted_form.form_has_autofilled_value;
-  pending_credentials->all_possible_passwords =
+  pending_credentials.all_possible_passwords =
       parsed_submitted_form.all_possible_passwords;
-  CopyFieldPropertiesMasks(submitted_form, &pending_credentials->form_data);
+  CopyFieldPropertiesMasks(submitted_form, &pending_credentials.form_data);
 
   // If we're dealing with an API-driven provisionally saved form, then take
   // the server provided values. We don't do this for non-API forms, as
   // those will never have those members set.
   if (parsed_submitted_form.type == PasswordForm::Type::kApi) {
-    pending_credentials->skip_zero_click =
-        parsed_submitted_form.skip_zero_click;
-    pending_credentials->display_name = parsed_submitted_form.display_name;
-    pending_credentials->federation_origin =
+    pending_credentials.skip_zero_click = parsed_submitted_form.skip_zero_click;
+    pending_credentials.display_name = parsed_submitted_form.display_name;
+    pending_credentials.federation_origin =
         parsed_submitted_form.federation_origin;
-    pending_credentials->icon_url = parsed_submitted_form.icon_url;
+    pending_credentials.icon_url = parsed_submitted_form.icon_url;
     // It's important to override |signon_realm| for federated credentials
     // because it has format "federation://" + origin_host + "/" +
     // federation_host
-    pending_credentials->signon_realm = parsed_submitted_form.signon_realm;
+    pending_credentials.signon_realm = parsed_submitted_form.signon_realm;
   }
 
   if (generated_password.has_value())
-    pending_credentials->type = PasswordForm::Type::kGenerated;
+    pending_credentials.type = PasswordForm::Type::kGenerated;
 
-  return pending_credentials_state;
+  return pending_credentials;
 }
 
 void PasswordSaveManagerImpl::CreatePendingCredentials(
@@ -243,16 +262,21 @@ void PasswordSaveManagerImpl::CreatePendingCredentials(
   pending_credentials_ = PasswordForm();
   votes_uploader_->set_password_overridden(false);
 
+  const PasswordForm* similar_saved_form =
+      password_manager_util::GetMatchForUpdating(
+          parsed_submitted_form, form_fetcher_->GetBestMatches());
+
+  pending_credentials_state_ =
+      ComputePendingCredentialsState(parsed_submitted_form, similar_saved_form);
+
   base::Optional<base::string16> generated_password;
   if (HasGeneratedPassword())
     generated_password = generation_manager_->generated_password();
 
-  pending_credentials_state_ = FillPendingCredentials(
-      parsed_submitted_form, observed_form, submitted_form, generated_password,
-      is_http_auth, is_credential_api_save,
-      password_manager_util::GetMatchForUpdating(
-          parsed_submitted_form, form_fetcher_->GetBestMatches()),
-      &pending_credentials_);
+  pending_credentials_ = BuildPendingCredentials(
+      pending_credentials_state_, parsed_submitted_form, observed_form,
+      submitted_form, generated_password, is_http_auth, is_credential_api_save,
+      similar_saved_form);
 
   switch (pending_credentials_state_) {
     case PendingCredentialsState::NEW_LOGIN: {

components/password_manager/core/browser/password_save_manager_impl.h

@@ -84,15 +84,18 @@ class PasswordSaveManagerImpl : public PasswordSaveManager {
 #endif
 
  protected:
-  static PendingCredentialsState FillPendingCredentials(
+  static PendingCredentialsState ComputePendingCredentialsState(
+      const autofill::PasswordForm& parsed_submitted_form,
+      const autofill::PasswordForm* similar_saved_form);
+  static autofill::PasswordForm BuildPendingCredentials(
+      PendingCredentialsState pending_credentials_state,
       const autofill::PasswordForm& parsed_submitted_form,
       const autofill::FormData& observed_form,
       const autofill::FormData& submitted_form,
       const base::Optional<base::string16>& generated_password,
       bool is_http_auth,
       bool is_credential_api_save,
-      const autofill::PasswordForm* similar_saved_form,
-      autofill::PasswordForm* pending_credentials);
+      const autofill::PasswordForm* similar_saved_form);
 
   // Returns the form_saver to be used for generated passwords. Subclasses will
   // override this method to provide different logic for get the form saver.