[Passwords] Support Account Store in CompromisedCredentialsReader

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

components/password_manager/core/browser/BUILD.gn

@@ -632,6 +632,7 @@ source_set("unit_tests") {
     "sync_username_test_base.h",
     "ui/bulk_leak_check_service_adapter_unittest.cc",
     "ui/compromised_credentials_manager_unittest.cc",
+    "ui/compromised_credentials_reader_unittest.cc",
     "ui/post_save_compromised_helper_unittest.cc",
     "ui/saved_passwords_presenter_unittest.cc",
     "vote_uploads_test_matchers.h",

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

@@ -4,29 +4,70 @@
 
 #include "components/password_manager/core/browser/ui/compromised_credentials_reader.h"
 
+#include <iterator>
+
+#include "base/stl_util.h"
+#include "base/util/ranges/algorithm.h"
+#include "components/autofill/core/common/password_form.h"
+
 namespace password_manager {
-CompromisedCredentialsReader::CompromisedCredentialsReader(PasswordStore* store)
-    : store_(store) {
-  DCHECK(store_);
-  observed_password_store_.Add(store_);
+CompromisedCredentialsReader::CompromisedCredentialsReader(
+    PasswordStore* profile_store,
+    PasswordStore* account_store)
+    : profile_store_(profile_store), account_store_(account_store) {
+  DCHECK(profile_store_);
+  observed_password_store_.Add(profile_store_);
+  if (account_store_)
+    observed_password_store_.Add(account_store_);
 }
 
 CompromisedCredentialsReader::~CompromisedCredentialsReader() = default;
 
 void CompromisedCredentialsReader::Init() {
-  store_->GetAllCompromisedCredentials(this);
+  profile_store_->GetAllCompromisedCredentials(this);
+  if (account_store_)
+    account_store_->GetAllCompromisedCredentials(this);
 }
 
 void CompromisedCredentialsReader::OnCompromisedCredentialsChanged() {
-  // Cancel ongoing requests to the password store and issue a new request.
-  cancelable_task_tracker()->TryCancelAll();
-  store_->GetAllCompromisedCredentials(this);
+  // This class overrides OnCompromisedCredentialsChangedIn() (the version of
+  // this method that also receives the originating store), so the store-less
+  // version never gets called.
+  NOTREACHED();
+}
+
+void CompromisedCredentialsReader::OnCompromisedCredentialsChangedIn(
+    PasswordStore* store) {
+  store->GetAllCompromisedCredentials(this);
 }
 
 void CompromisedCredentialsReader::OnGetCompromisedCredentials(
     std::vector<CompromisedCredentials> compromised_credentials) {
+  // This class overrides OnGetCompromisedCredentialFrom() (the version of this
+  // method that also receives the originating store), so the store-less version
+  // never gets called.
+  NOTREACHED();
+}
+
+void CompromisedCredentialsReader::OnGetCompromisedCredentialsFrom(
+    PasswordStore* store,
+    std::vector<CompromisedCredentials> compromised_credentials) {
+  // Remove all previously cached credentials from `store` and then insert
+  // the just received `compromised_credentials`.
+  autofill::PasswordForm::Store to_remove =
+      store == profile_store_ ? autofill::PasswordForm::Store::kProfileStore
+                              : autofill::PasswordForm::Store::kAccountStore;
+
+  base::EraseIf(compromised_credentials_, [to_remove](const auto& credential) {
+    return credential.in_store == to_remove;
+  });
+
+  util::ranges::move(compromised_credentials,
+                     std::back_inserter(compromised_credentials_));
+
+  // Inform the observers
   for (auto& observer : observers_)
-    observer.OnCompromisedCredentialsChanged(compromised_credentials);
+    observer.OnCompromisedCredentialsChanged(compromised_credentials_);
 }
 
 void CompromisedCredentialsReader::AddObserver(Observer* observer) {

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

@@ -27,8 +27,9 @@ class CompromisedCredentialsReader
         const std::vector<CompromisedCredentials>& compromised_credentials) = 0;
   };
 
-  // |store| cannot be null, and must outlive this class.
-  explicit CompromisedCredentialsReader(PasswordStore* store);
+  // |profile_store| cannot be null, and must outlive this class.
+  explicit CompromisedCredentialsReader(PasswordStore* profile_store,
+                                        PasswordStore* account_store = nullptr);
   CompromisedCredentialsReader(const CompromisedCredentialsReader&) = delete;
   CompromisedCredentialsReader& operator=(const CompromisedCredentialsReader&) =
       delete;
@@ -43,17 +44,25 @@ class CompromisedCredentialsReader
  private:
   // PasswordStore::DatabaseCompromisedCredentialsObserver:
   void OnCompromisedCredentialsChanged() override;
+  void OnCompromisedCredentialsChangedIn(PasswordStore* store) override;
 
   // CompromisedCredentialsConsumer:
   void OnGetCompromisedCredentials(
       std::vector<CompromisedCredentials> compromised_credentials) override;
+  void OnGetCompromisedCredentialsFrom(
+      PasswordStore* store,
+      std::vector<CompromisedCredentials> compromised_credentials) override;
+
+  // The password stores containing the compromised credentials.
+  // |profile_store_| must not be null and must outlive this class.
+  PasswordStore* profile_store_;
+  PasswordStore* account_store_;
 
-  // The password store containing the compromised credentials. It must outlive
-  // this class.
-  PasswordStore* store_;
+  // Cache of the most recently obtained compromised credentials.
+  std::vector<CompromisedCredentials> compromised_credentials_;
 
-  // A scoped observer for |store_| to listen changes related to
-  // CompromisedCredentials only.
+  // A scoped observer for |profile_store_|, and |account_store_| that listens
+  // to changes related to CompromisedCredentials only.
   ScopedObserver<PasswordStore,
                  PasswordStore::DatabaseCompromisedCredentialsObserver,
                  &PasswordStore::AddDatabaseCompromisedCredentialsObserver,

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/password_manager/core/browser/ui/compromised_credentials_reader.h"
+
+#include "base/memory/scoped_refptr.h"
+#include "base/scoped_observer.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/task_environment.h"
+#include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace password_manager {
+
+namespace {
+
+using autofill::PasswordForm;
+using ::testing::UnorderedElementsAre;
+
+constexpr const char kTestWebRealm[] = "https://one.example.com/";
+
+class MockCompromisedCredentialsReaderObserver
+    : public CompromisedCredentialsReader::Observer {
+ public:
+  MOCK_METHOD(void,
+              OnCompromisedCredentialsChanged,
+              (const std::vector<CompromisedCredentials>&),
+              (override));
+};
+
+class CompromisedCredentialsReaderTest : public ::testing::Test {
+ protected:
+  CompromisedCredentialsReaderTest() {
+    profile_store_->Init(/*prefs=*/nullptr);
+    account_store_->Init(/*prefs=*/nullptr);
+  }
+
+  ~CompromisedCredentialsReaderTest() override {
+    account_store_->ShutdownOnUIThread();
+    profile_store_->ShutdownOnUIThread();
+    task_env_.RunUntilIdle();
+  }
+
+  TestPasswordStore& profile_store() { return *profile_store_; }
+  TestPasswordStore& account_store() { return *account_store_; }
+  CompromisedCredentialsReader& reader() { return reader_; }
+
+  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);
+  CompromisedCredentialsReader reader_{profile_store_.get(),
+                                       account_store_.get()};
+};
+
+}  // namespace
+
+TEST_F(CompromisedCredentialsReaderTest, AddCredentialsToBothStores) {
+  CompromisedCredentials profile_cred;
+  profile_cred.signon_realm = kTestWebRealm;
+  profile_cred.username = base::ASCIIToUTF16("profile@gmail.com");
+  profile_cred.in_store = PasswordForm::Store::kProfileStore;
+
+  CompromisedCredentials account_cred1;
+  account_cred1.signon_realm = kTestWebRealm;
+  account_cred1.username = base::ASCIIToUTF16("account1@gmail.com");
+  account_cred1.in_store = PasswordForm::Store::kAccountStore;
+
+  CompromisedCredentials account_cred2;
+  account_cred2.signon_realm = kTestWebRealm;
+  account_cred2.username = base::ASCIIToUTF16("account2@gmail.com");
+  account_cred2.in_store = PasswordForm::Store::kAccountStore;
+
+  ::testing::NiceMock<MockCompromisedCredentialsReaderObserver> mock_observer;
+  reader().AddObserver(&mock_observer);
+
+  EXPECT_CALL(mock_observer, OnCompromisedCredentialsChanged(
+                                 UnorderedElementsAre(profile_cred)));
+  profile_store().AddCompromisedCredentials(profile_cred);
+  RunUntilIdle();
+
+  EXPECT_CALL(mock_observer,
+              OnCompromisedCredentialsChanged(
+                  UnorderedElementsAre(profile_cred, account_cred1)));
+  account_store().AddCompromisedCredentials(account_cred1);
+  RunUntilIdle();
+
+  EXPECT_CALL(mock_observer,
+              OnCompromisedCredentialsChanged(UnorderedElementsAre(
+                  profile_cred, account_cred1, account_cred2)));
+  account_store().AddCompromisedCredentials(account_cred2);
+  RunUntilIdle();
+
+  EXPECT_CALL(mock_observer,
+              OnCompromisedCredentialsChanged(
+                  UnorderedElementsAre(account_cred1, account_cred2)));
+  profile_store().RemoveCompromisedCredentials(
+      profile_cred.signon_realm, profile_cred.username,
+      RemoveCompromisedCredentialsReason::kRemove);
+  RunUntilIdle();
+
+  EXPECT_CALL(mock_observer,
+              OnCompromisedCredentialsChanged(UnorderedElementsAre(
+                  profile_cred, account_cred1, account_cred2)));
+  profile_store().AddCompromisedCredentials(profile_cred);
+  RunUntilIdle();
+
+  reader().RemoveObserver(&mock_observer);
+}
+
+}  // namespace password_manager