[Passwords] Support Account Password Store in SavedPasswordsPresenter

Bug: 1108422
Change-Id: I6a9996b0bd8245e3289acf4d8c3a6469ddb85827
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362923
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799749}

components/password_manager/core/browser/ui/saved_passwords_presenter.cc

@@ -11,34 +11,44 @@
 #include "base/check.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
+#include "base/util/ranges/algorithm.h"
 #include "components/autofill/core/common/password_form.h"
 
 namespace password_manager {
 
 SavedPasswordsPresenter::SavedPasswordsPresenter(
-    scoped_refptr<PasswordStore> store)
-    : store_(std::move(store)) {
-  DCHECK(store_);
-  store_->AddObserver(this);
+    scoped_refptr<PasswordStore> profile_store,
+    scoped_refptr<PasswordStore> account_store)
+    : profile_store_(std::move(profile_store)),
+      account_store_(std::move(account_store)) {
+  DCHECK(profile_store_);
+  profile_store_->AddObserver(this);
+  if (account_store_)
+    account_store_->AddObserver(this);
 }
 
 SavedPasswordsPresenter::~SavedPasswordsPresenter() {
-  store_->RemoveObserver(this);
+  if (account_store_)
+    account_store_->RemoveObserver(this);
+  profile_store_->RemoveObserver(this);
 }
 
 void SavedPasswordsPresenter::Init() {
-  store_->GetAllLoginsWithAffiliationAndBrandingInformation(this);
+  profile_store_->GetAllLoginsWithAffiliationAndBrandingInformation(this);
+  if (account_store_)
+    account_store_->GetAllLoginsWithAffiliationAndBrandingInformation(this);
 }
 
-bool SavedPasswordsPresenter::EditPassword(
-    const autofill::PasswordForm& password,
-    base::string16 new_password) {
-  auto found = std::find(passwords_.begin(), passwords_.end(), password);
+bool SavedPasswordsPresenter::EditPassword(const autofill::PasswordForm& form,
+                                           base::string16 new_password) {
+  auto found = util::ranges::find(passwords_, form);
   if (found == passwords_.end())
     return false;
 
   found->password_value = std::move(new_password);
-  store_->UpdateLogin(*found);
+  PasswordStore& store =
+      form.IsUsingAccountStore() ? *account_store_ : *profile_store_;
+  store.UpdateLogin(*found);
   NotifyEdited(*found);
   return true;
 }
@@ -69,21 +79,59 @@ void SavedPasswordsPresenter::NotifySavedPasswordsChanged() {
 
 void SavedPasswordsPresenter::OnLoginsChanged(
     const PasswordStoreChangeList& changes) {
-  // Cancel ongoing requests to the password store and issue a new request.
-  cancelable_task_tracker()->TryCancelAll();
-  store_->GetAllLoginsWithAffiliationAndBrandingInformation(this);
+  // This class overrides OnLoginsChangedIn() (the version of this
+  // method that also receives the originating store), so the store-less version
+  // never gets called.
+  NOTREACHED();
+}
+
+void SavedPasswordsPresenter::OnLoginsChangedIn(
+    PasswordStore* store,
+    const PasswordStoreChangeList& changes) {
+  store->GetAllLoginsWithAffiliationAndBrandingInformation(this);
 }
 
 void SavedPasswordsPresenter::OnGetPasswordStoreResults(
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
+  // This class overrides OnGetPasswordStoreResultsFrom() (the version of this
+  // method that also receives the originating store), so the store-less version
+  // never gets called.
+  NOTREACHED();
+}
+
+void SavedPasswordsPresenter::OnGetPasswordStoreResultsFrom(
+    PasswordStore* store,
+    std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   // Ignore blocked or federated credentials.
   base::EraseIf(results, [](const auto& form) {
     return form->blocked_by_user || form->IsFederatedCredential();
   });
 
-  passwords_.resize(results.size());
-  std::transform(results.begin(), results.end(), passwords_.begin(),
-                 [](auto& result) { return std::move(*result); });
+  // Profile store passwords are always stored first in `passwords_`.
+  auto account_passwords_it = util::ranges::partition_point(
+      passwords_,
+      [](auto& password) { return !password.IsUsingAccountStore(); });
+  if (store == profile_store_) {
+    // Old profile store passwords are in front. Create a temporary buffer for
+    // the new passwords and replace existing passwords.
+    std::vector<autofill::PasswordForm> new_passwords;
+    new_passwords.reserve(results.size() + passwords_.end() -
+                          account_passwords_it);
+    auto new_passwords_back_inserter = std::back_inserter(new_passwords);
+    util::ranges::transform(results, new_passwords_back_inserter,
+                            [](auto& result) { return std::move(*result); });
+    std::move(account_passwords_it, passwords_.end(),
+              new_passwords_back_inserter);
+    passwords_ = std::move(new_passwords);
+  } else {
+    // Need to replace existing account passwords at the end. Can re-use
+    // existing `passwords_` vector.
+    passwords_.erase(account_passwords_it, passwords_.end());
+    if (passwords_.capacity() < passwords_.size() + results.size())
+      passwords_.reserve(passwords_.size() + results.size());
+    util::ranges::transform(results, std::back_inserter(passwords_),
+                            [](auto& result) { return std::move(*result); });
+  }
   NotifySavedPasswordsChanged();
 }
 

components/password_manager/core/browser/ui/saved_passwords_presenter.h

@@ -56,19 +56,21 @@ class SavedPasswordsPresenter : public PasswordStore::Observer,
     virtual void OnSavedPasswordsChanged(SavedPasswordsView passwords) {}
   };
 
-  explicit SavedPasswordsPresenter(scoped_refptr<PasswordStore> store);
+  explicit SavedPasswordsPresenter(
+      scoped_refptr<PasswordStore> profile_store,
+      scoped_refptr<PasswordStore> account_store = nullptr);
   ~SavedPasswordsPresenter() override;
 
   // Initializes the presenter and makes it issue the first request for all
   // saved passwords.
   void Init();
 
-  // Tries to edit |password|. After checking whether |password| is present in
+  // Tries to edit |password|. After checking whether |form| is present in
   // |passwords_|, this will ask the password store to change the underlying
   // password_value to |new_password| in case it was found. This will also
-  // notify clients that an edit event happened in case |password| was present
+  // notify clients that an edit event happened in case |form| was present
   // in |passwords_|.
-  bool EditPassword(const autofill::PasswordForm& password,
+  bool EditPassword(const autofill::PasswordForm& form,
                     base::string16 new_password);
 
   // Returns a list of the currently saved credentials.
@@ -81,19 +83,26 @@ class SavedPasswordsPresenter : public PasswordStore::Observer,
  private:
   // PasswordStore::Observer
   void OnLoginsChanged(const PasswordStoreChangeList& changes) override;
+  void OnLoginsChangedIn(PasswordStore* store,
+                         const PasswordStoreChangeList& changes) override;
 
   // PasswordStoreConsumer:
   void OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
+  void OnGetPasswordStoreResultsFrom(
+      PasswordStore* store,
+      std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
   // Notify observers about changes in the compromised credentials.
   void NotifyEdited(const autofill::PasswordForm& password);
   void NotifySavedPasswordsChanged();
 
-  // The password store containing the saved passwords.
-  scoped_refptr<PasswordStore> store_;
+  // The password stores containing the saved passwords.
+  scoped_refptr<PasswordStore> profile_store_;
+  scoped_refptr<PasswordStore> account_store_;
 
-  // Cache of the most recently obtained saved passwords.
+  // Cache of the most recently obtained saved passwords. Profile store
+  // passwords are always stored first, and then account store passwords if any.
   std::vector<autofill::PasswordForm> passwords_;
 
   base::ObserverList<Observer, /*check_empty=*/true> observers_;

components/password_manager/core/browser/ui/saved_passwords_presenter_unittest.cc

@@ -22,6 +22,7 @@ using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
 using ::testing::Pair;
+using ::testing::UnorderedElementsAre;
 
 struct MockSavedPasswordsPresenterObserver : SavedPasswordsPresenter::Observer {
   MOCK_METHOD(void, OnEdited, (const autofill::PasswordForm&), (override));
@@ -86,7 +87,7 @@ TEST_F(SavedPasswordsPresenterTest, NotifyObservers) {
   EXPECT_FALSE(store().IsEmpty());
 }
 
-// Tests whether adding and removing an observer works as expected.
+// Tests whether adding federated credentials doesn't inform the observers.
 TEST_F(SavedPasswordsPresenterTest, IgnoredCredentials) {
   PasswordForm federated_form;
   federated_form.federation_origin =
@@ -150,4 +151,85 @@ TEST_F(SavedPasswordsPresenterTest, EditPassword) {
   presenter().RemoveObserver(&observer);
 }
 
+namespace {
+
+class SavedPasswordsPresenterWithTwoStoresTest : public ::testing::Test {
+ protected:
+  SavedPasswordsPresenterWithTwoStoresTest() {
+    profile_store_->Init(/*prefs=*/nullptr);
+    account_store_->Init(/*prefs=*/nullptr);
+  }
+
+  ~SavedPasswordsPresenterWithTwoStoresTest() override {
+    account_store_->ShutdownOnUIThread();
+    profile_store_->ShutdownOnUIThread();
+    task_env_.RunUntilIdle();
+  }
+
+  TestPasswordStore& profile_store() { return *profile_store_; }
+  TestPasswordStore& account_store() { return *account_store_; }
+  SavedPasswordsPresenter& presenter() { return presenter_; }
+
+  void RunUntilIdle() { task_env_.RunUntilIdle(); }
+
+ private:
+  base::test::SingleThreadTaskEnvironment task_env_;
+  scoped_refptr<TestPasswordStore> profile_store_ =
+      base::MakeRefCounted<TestPasswordStore>(/*is_account_store=*/false);
+  scoped_refptr<TestPasswordStore> account_store_ =
+      base::MakeRefCounted<TestPasswordStore>(/*is_account_store=*/true);
+  SavedPasswordsPresenter presenter_{profile_store_, account_store_};
+};
+
+}  // namespace
+
+// Tests whether adding credentials to profile or account store notifies
+// observers with credentials in both stores.
+TEST_F(SavedPasswordsPresenterWithTwoStoresTest, AddCredentialsToBothStores) {
+  PasswordForm profile_store_form;
+  profile_store_form.username_value = base::ASCIIToUTF16("profile@gmail.com");
+  profile_store_form.password_value = base::ASCIIToUTF16("profile_pass");
+  profile_store_form.in_store = PasswordForm::Store::kProfileStore;
+
+  PasswordForm account_store_form1;
+  account_store_form1.username_value = base::ASCIIToUTF16("account@gmail.com");
+  account_store_form1.password_value = base::ASCIIToUTF16("account_pass");
+  account_store_form1.in_store = PasswordForm::Store::kAccountStore;
+
+  PasswordForm account_store_form2 = account_store_form1;
+  account_store_form2.username_value = base::ASCIIToUTF16("account2@gmail.com");
+
+  StrictMockSavedPasswordsPresenterObserver observer;
+  presenter().AddObserver(&observer);
+
+  EXPECT_CALL(observer, OnSavedPasswordsChanged(
+                            UnorderedElementsAre(profile_store_form)));
+  profile_store().AddLogin(profile_store_form);
+  RunUntilIdle();
+
+  EXPECT_CALL(observer, OnSavedPasswordsChanged(UnorderedElementsAre(
+                            profile_store_form, account_store_form1)));
+  account_store().AddLogin(account_store_form1);
+  RunUntilIdle();
+
+  EXPECT_CALL(observer, OnSavedPasswordsChanged(UnorderedElementsAre(
+                            profile_store_form, account_store_form1,
+                            account_store_form2)));
+  account_store().AddLogin(account_store_form2);
+  RunUntilIdle();
+
+  EXPECT_CALL(observer, OnSavedPasswordsChanged(UnorderedElementsAre(
+                            account_store_form1, account_store_form2)));
+  profile_store().RemoveLogin(profile_store_form);
+  RunUntilIdle();
+
+  EXPECT_CALL(observer, OnSavedPasswordsChanged(UnorderedElementsAre(
+                            profile_store_form, account_store_form1,
+                            account_store_form2)));
+  profile_store().AddLogin(profile_store_form);
+  RunUntilIdle();
+
+  presenter().RemoveObserver(&observer);
+}
+
 }  // namespace password_manager