[Passwords] PasswordSaveManager::MoveCredentialsToAccountStore()

This patch introduces the method
PasswordSaveManager::MoveCredentialsToAccountStore()
and provide an implementation for the
MultiStorePasswordSaveManager.

Bug: 1032992
Change-Id: Ib6fee5f7bb0dae53a2a5b6372f4299200c80778c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1962067
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733567}

components/password_manager/core/browser/multi_store_password_save_manager.cc

@@ -135,4 +135,32 @@ bool MultiStorePasswordSaveManager::IsAccountStoreActive() {
          password_manager::ACCOUNT_PASSWORDS_ACTIVE_NORMAL_ENCRYPTION;
 }
 
+void MultiStorePasswordSaveManager::MoveCredentialsToAccountStore() {
+  // TODO(crbug.com/1032992): There are other rare corner cases that should
+  // still be handled: 0. Moving PSL matched credentials doesn't work now
+  // because of
+  // https://cs.chromium.org/chromium/src/components/password_manager/core/browser/login_database.cc?l=1318&rcl=e32055d4843e9fc1fa920c5f1f83c1313607e28a
+  // 1. Credential exists only in the profile store but with an outdated
+  // password.
+  // 2. Credentials exist in both stores.
+  // 3. Credentials exist in both stores while one of them of outdated. (profile
+  // or remote).
+  // 4. Credential exists only in the profile store but a PSL matched one exists
+  // in both profile and account store.
+
+  const std::vector<const PasswordForm*> account_store_matches =
+      AccountStoreMatches(form_fetcher_->GetBestMatches());
+  for (const PasswordForm* match :
+       ProfileStoreMatches(form_fetcher_->GetBestMatches())) {
+    DCHECK(!match->IsUsingAccountStore());
+    // Ignore credentials matches for other usernames.
+    if (match->username_value != pending_credentials_.username_value)
+      continue;
+
+    account_store_form_saver_->Save(*match, account_store_matches,
+                                    /*old_password=*/base::string16());
+    form_saver_->Remove(*match);
+  }
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/multi_store_password_save_manager.h

@@ -39,6 +39,8 @@ class MultiStorePasswordSaveManager : public PasswordSaveManagerImpl {
 
   std::unique_ptr<PasswordSaveManager> Clone() override;
 
+  void MoveCredentialsToAccountStore() override;
+
  protected:
   FormSaver* GetFormSaverForGeneration() override;
 

components/password_manager/core/browser/multi_store_password_save_manager_unittest.cc

@@ -501,4 +501,45 @@ TEST_F(MultiStorePasswordSaveManagerTest,
   password_save_manager()->PermanentlyBlacklist(form_digest);
 }
 
+TEST_F(MultiStorePasswordSaveManagerTest,
+       MoveCredentialsFromProfileToAccountStoreWhenExistsOnlyInProfileStore) {
+  PasswordForm saved_match_in_profile_store(saved_match_);
+  saved_match_in_profile_store.in_store = PasswordForm::Store::kProfileStore;
+  SetNonFederatedAndNotifyFetchCompleted({&saved_match_in_profile_store});
+
+  password_save_manager()->CreatePendingCredentials(
+      saved_match_in_profile_store, observed_form_, submitted_form_,
+      /*is_http_auth=*/false,
+      /*is_credential_api_save=*/false);
+
+  EXPECT_CALL(*mock_profile_form_saver(), Remove(saved_match_in_profile_store));
+  EXPECT_CALL(*mock_account_form_saver(),
+              Save(saved_match_in_profile_store, _, _));
+
+  password_save_manager()->MoveCredentialsToAccountStore();
+}
+
+TEST_F(
+    MultiStorePasswordSaveManagerTest,
+    DoNotMoveCredentialsFromProfileToAccountStoreWhenExistsOnlyInProfileStoreWithDifferentUserName) {
+  PasswordForm saved_match_in_profile_store(saved_match_);
+  saved_match_in_profile_store.in_store = PasswordForm::Store::kProfileStore;
+  SetNonFederatedAndNotifyFetchCompleted({&saved_match_in_profile_store});
+  PasswordForm credentials_with_diffrent_username(saved_match_in_profile_store);
+  credentials_with_diffrent_username.username_value =
+      ASCIIToUTF16("different_username");
+  password_save_manager()->CreatePendingCredentials(
+      credentials_with_diffrent_username, observed_form_, submitted_form_,
+      /*is_http_auth=*/false,
+      /*is_credential_api_save=*/false);
+
+  EXPECT_CALL(*mock_profile_form_saver(), Remove(saved_match_in_profile_store))
+      .Times(0);
+  EXPECT_CALL(*mock_account_form_saver(),
+              Save(saved_match_in_profile_store, _, _))
+      .Times(0);
+
+  password_save_manager()->MoveCredentialsToAccountStore();
+}
+
 }  // namespace password_manager

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -2330,6 +2330,7 @@ class MockPasswordSaveManager : public PasswordSaveManager {
   std::unique_ptr<PasswordSaveManager> Clone() override {
     return std::make_unique<MockPasswordSaveManager>();
   }
+  MOCK_METHOD0(MoveCredentialsToAccountStore, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockPasswordSaveManager);

components/password_manager/core/browser/password_save_manager.h

@@ -76,6 +76,10 @@ class PasswordSaveManager {
   // Signals that the user cancels password generation.
   virtual void PasswordNoLongerGenerated() = 0;
 
+  // Moves the pending credentials together with any other PSL matched ones from
+  // the profile store to the account store.
+  virtual void MoveCredentialsToAccountStore() = 0;
+
   virtual bool IsNewLogin() const = 0;
   virtual bool IsPasswordUpdate() const = 0;
   virtual bool HasGeneratedPassword() const = 0;

components/password_manager/core/browser/password_save_manager_impl.cc

@@ -331,6 +331,11 @@ void PasswordSaveManagerImpl::PasswordNoLongerGenerated() {
       PasswordFormMetricsRecorder::GeneratedPasswordStatus::kPasswordDeleted);
 }
 
+void PasswordSaveManagerImpl::MoveCredentialsToAccountStore() {
+  // Moving credentials is only supported in MultiStorePasswordSaveManager.
+  NOTREACHED();
+}
+
 bool PasswordSaveManagerImpl::IsNewLogin() const {
   return pending_credentials_state_ == PendingCredentialsState::NEW_LOGIN ||
          pending_credentials_state_ == PendingCredentialsState::AUTOMATIC_SAVE;

components/password_manager/core/browser/password_save_manager_impl.h

@@ -63,6 +63,8 @@ class PasswordSaveManagerImpl : public PasswordSaveManager {
   // Signals that the user cancels password generation.
   void PasswordNoLongerGenerated() override;
 
+  void MoveCredentialsToAccountStore() override;
+
   bool IsNewLogin() const override;
   bool IsPasswordUpdate() const override;
   bool HasGeneratedPassword() const override;
@@ -105,6 +107,9 @@ class PasswordSaveManagerImpl : public PasswordSaveManager {
   PendingCredentialsState pending_credentials_state_ =
       PendingCredentialsState::NONE;
 
+  // FormFetcher instance which owns the login data from PasswordStore.
+  const FormFetcher* form_fetcher_;
+
  private:
   // Create pending credentials from provisionally saved form when this form
   // represents credentials that were not previously saved.
@@ -128,9 +133,6 @@ class PasswordSaveManagerImpl : public PasswordSaveManager {
   // Handles the user flows related to the generation.
   std::unique_ptr<PasswordGenerationManager> generation_manager_;
 
-  // FormFetcher instance which owns the login data from PasswordStore.
-  const FormFetcher* form_fetcher_;
-
   // Takes care of recording metrics and events for |*this|.
   scoped_refptr<PasswordFormMetricsRecorder> metrics_recorder_;