[Passwords] Add CompromisedPasswordsObserver

This change adds a CompromisedPasswordsObserver interface to the
password store allowing subscribers to be notified about changes to the
stored compromised credentials.

Bug: 1047726
Change-Id: I10e8938bad7ad7503791f8c109df23384b4392c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037574
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738639}

components/password_manager/core/browser/compromised_credentials_table.cc

@@ -4,6 +4,8 @@
 
 #include "components/password_manager/core/browser/compromised_credentials_table.h"
 
+#include "base/bind.h"
+#include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "components/password_manager/core/browser/sql_table_builder.h"
 #include "components/password_manager/core/common/password_manager_features.h"
@@ -104,11 +106,20 @@ bool CompromisedCredentialsTable::AddRow(
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.CompromisedCredentials.Add",
                             compromised_credentials.compromise_type);
 
-  sql::Statement s(
-      db_->GetCachedStatement(SQL_FROM_HERE,
-                              "INSERT OR IGNORE INTO compromised_credentials "
-                              "(url, username, create_time, compromise_type) "
-                              "VALUES (?, ?, ?, ?)"));
+  // In case there is an error, expect it to be a constraint violation.
+  db_->set_error_callback(base::BindRepeating([](int error, sql::Statement*) {
+    constexpr int kSqliteErrorMask = 0xFF;
+    constexpr int kSqliteConstraint = 19;
+    if ((error & kSqliteErrorMask) != kSqliteConstraint) {
+      DLOG(ERROR) << "Got unexpected SQL error code: " << error;
+    }
+  }));
+
+  sql::Statement s(db_->GetCachedStatement(
+      SQL_FROM_HERE,
+      "INSERT INTO compromised_credentials (url, username, create_time, "
+      "compromise_type) VALUES (?, ?, ?, ?)"));
+
   s.BindString(GetColumnNumber(CompromisedCredentialsTableColumn::kSignonRealm),
                compromised_credentials.signon_realm);
   s.BindString16(GetColumnNumber(CompromisedCredentialsTableColumn::kUsername),
@@ -119,7 +130,10 @@ bool CompromisedCredentialsTable::AddRow(
   s.BindInt64(
       GetColumnNumber(CompromisedCredentialsTableColumn::kCompromiseType),
       static_cast<int>(compromised_credentials.compromise_type));
-  return s.Run();
+
+  bool result = s.Run();
+  db_->reset_error_callback();
+  return result;
 }
 
 std::vector<CompromisedCredentials> CompromisedCredentialsTable::GetRows(

components/password_manager/core/browser/compromised_credentials_table_unittest.cc

@@ -140,10 +140,8 @@ TEST_F(CompromisedCredentialsTableTest,
   compromised_credentials2.create_time = base::Time::FromTimeT(2);
 
   EXPECT_TRUE(db()->AddRow(compromised_credentials1));
-  // It should return true as the sql statement ran correctly. It ignored
-  // new row though because of unique constraints, hence there is only one
-  // record in the database.
-  EXPECT_TRUE(db()->AddRow(compromised_credentials2));
+  // It should return false because of unique constraints.
+  EXPECT_FALSE(db()->AddRow(compromised_credentials2));
   EXPECT_THAT(db()->GetAllRows(), ElementsAre(compromised_credentials1));
 }
 

components/password_manager/core/browser/mock_password_store.h

@@ -74,15 +74,15 @@ class MockPasswordStore : public PasswordStore {
   MOCK_METHOD1(AddSiteStatsImpl, void(const InteractionsStats&));
   MOCK_METHOD1(RemoveSiteStatsImpl, void(const GURL&));
   MOCK_METHOD1(AddCompromisedCredentialsImpl,
-               void(const CompromisedCredentials&));
+               bool(const CompromisedCredentials&));
   MOCK_METHOD3(RemoveCompromisedCredentialsImpl,
-               void(const std::string&,
+               bool(const std::string&,
                     const base::string16&,
                     RemoveCompromisedCredentialsReason));
   MOCK_METHOD0(GetAllCompromisedCredentialsImpl,
                std::vector<CompromisedCredentials>());
   MOCK_METHOD3(RemoveCompromisedCredentialsByUrlAndTimeImpl,
-               void(const base::RepeatingCallback<bool(const GURL&)>&,
+               bool(const base::RepeatingCallback<bool(const GURL&)>&,
                     base::Time,
                     base::Time));
   MOCK_METHOD1(AddFieldInfoImpl, void(const FieldInfo&));

components/password_manager/core/browser/password_store.cc

@@ -373,8 +373,11 @@ void PasswordStore::GetSiteStats(const GURL& origin_domain,
 void PasswordStore::AddCompromisedCredentials(
     const CompromisedCredentials& compromised_credentials) {
   DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
-  ScheduleTask(base::BindOnce(&PasswordStore::AddCompromisedCredentialsImpl,
-                              this, compromised_credentials));
+  auto callback = base::BindOnce(&PasswordStore::AddCompromisedCredentialsImpl,
+                                 this, compromised_credentials);
+  ScheduleTask(base::BindOnce(
+      &PasswordStore::InvokeAndNotifyAboutCompromisedPasswordsChange, this,
+      std::move(callback)));
 }
 
 void PasswordStore::RemoveCompromisedCredentials(
@@ -382,8 +385,12 @@ void PasswordStore::RemoveCompromisedCredentials(
     const base::string16& username,
     RemoveCompromisedCredentialsReason reason) {
   DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
-  ScheduleTask(base::BindOnce(&PasswordStore::RemoveCompromisedCredentialsImpl,
-                              this, signon_realm, username, reason));
+  auto callback =
+      base::BindOnce(&PasswordStore::RemoveCompromisedCredentialsImpl, this,
+                     signon_realm, username, reason);
+  ScheduleTask(base::BindOnce(
+      &PasswordStore::InvokeAndNotifyAboutCompromisedPasswordsChange, this,
+      std::move(callback)));
 }
 
 void PasswordStore::GetAllCompromisedCredentials(
@@ -400,9 +407,13 @@ void PasswordStore::RemoveCompromisedCredentialsByUrlAndTime(
     base::Time remove_end,
     base::OnceClosure completion) {
   DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
-  ScheduleTask(base::BindOnce(
+  auto callback = base::BindOnce(
       &PasswordStore::RemoveCompromisedCredentialsByUrlAndTimeInternal, this,
-      std::move(url_filter), remove_begin, remove_end, std::move(completion)));
+      std::move(url_filter), remove_begin, remove_end, std::move(completion));
+
+  ScheduleTask(base::BindOnce(
+      &PasswordStore::InvokeAndNotifyAboutCompromisedPasswordsChange, this,
+      std::move(callback)));
 }
 
 void PasswordStore::AddFieldInfo(const FieldInfo& field_info) {
@@ -439,10 +450,19 @@ void PasswordStore::RemoveObserver(Observer* observer) {
   observers_->RemoveObserver(observer);
 }
 
+void PasswordStore::AddCompromisedPasswordsObserver(
+    CompromisedPasswordsObserver* observer) {
+  compromised_passwords_observers_->AddObserver(observer);
+}
+
+void PasswordStore::RemoveCompromisedPasswordsObserver(
+    CompromisedPasswordsObserver* observer) {
+  compromised_passwords_observers_->RemoveObserver(observer);
+}
+
 bool PasswordStore::ScheduleTask(base::OnceClosure task) {
-  if (background_task_runner_)
-    return background_task_runner_->PostTask(FROM_HERE, std::move(task));
-  return false;
+  return background_task_runner_ &&
+         background_task_runner_->PostTask(FROM_HERE, std::move(task));
 }
 
 bool PasswordStore::IsAbleToSavePasswords() const {
@@ -692,6 +712,16 @@ void PasswordStore::NotifyLoginsChanged(
   }
 }
 
+void PasswordStore::InvokeAndNotifyAboutCompromisedPasswordsChange(
+    base::OnceCallback<bool()> callback) {
+  DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
+  if (std::move(callback).Run()) {
+    compromised_passwords_observers_->Notify(
+        FROM_HERE,
+        &CompromisedPasswordsObserver::OnCompromisedPasswordsChanged);
+  }
+}
+
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
 void PasswordStore::CheckReuseImpl(std::unique_ptr<CheckReuseRequest> request,
                                    const base::string16& input,
@@ -953,16 +983,17 @@ void PasswordStore::UnblacklistInternal(
     main_task_runner_->PostTask(FROM_HERE, std::move(completion));
 }
 
-void PasswordStore::RemoveCompromisedCredentialsByUrlAndTimeInternal(
+bool PasswordStore::RemoveCompromisedCredentialsByUrlAndTimeInternal(
     const base::RepeatingCallback<bool(const GURL&)>& url_filter,
     base::Time remove_begin,
     base::Time remove_end,
     base::OnceClosure completion) {
   DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
-  RemoveCompromisedCredentialsByUrlAndTimeImpl(url_filter, remove_begin,
-                                               remove_end);
+  bool result = RemoveCompromisedCredentialsByUrlAndTimeImpl(
+      url_filter, remove_begin, remove_end);
   if (!completion.is_null())
     main_task_runner_->PostTask(FROM_HERE, std::move(completion));
+  return result;
 }
 
 void PasswordStore::RemoveFieldInfoByTimeInternal(

components/password_manager/core/browser/password_store.h

@@ -14,6 +14,7 @@
 #include "base/callback_list.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/observer_list_threadsafe.h"
 #include "base/sequenced_task_runner.h"
 #include "base/time/time.h"
@@ -91,7 +92,20 @@ class PasswordStore : protected PasswordStoreSync,
     virtual void OnLoginsChanged(const PasswordStoreChangeList& changes) = 0;
 
    protected:
-    virtual ~Observer() {}
+    virtual ~Observer() = default;
+  };
+
+  class CompromisedPasswordsObserver {
+    // An interface used to notify clients (observers) of this object that the
+    // list of comrpomised crendentials in the password store has changed.
+    // Register the observer via PasswordStore::AddCompromisedPasswordsObserver.
+   public:
+    // Notifies the observer that the list of compromised credentials changed.
+    // Will be called from the UI thread.
+    virtual void OnCompromisedPasswordsChanged() = 0;
+
+   protected:
+    virtual ~CompromisedPasswordsObserver() = default;
   };
 
   // Represents a subset of autofill::PasswordForm needed for credential
@@ -293,6 +307,14 @@ class PasswordStore : protected PasswordStoreSync,
   // Removes |observer| from the observer list.
   void RemoveObserver(Observer* observer);
 
+  // Adds an observer to be notified when the list of compromised passwords in
+  // the password store changes.
+  void AddCompromisedPasswordsObserver(CompromisedPasswordsObserver* observer);
+
+  // Removes |observer| from the list of compromised credentials observer.
+  void RemoveCompromisedPasswordsObserver(
+      CompromisedPasswordsObserver* observer);
+
   // Schedules the given |task| to be run on the PasswordStore's TaskRunner.
   bool ScheduleTask(base::OnceClosure task);
 
@@ -487,15 +509,15 @@ class PasswordStore : protected PasswordStoreSync,
 
   // Synchronous implementation for manipulating with information about
   // compromised credentials.
-  virtual void AddCompromisedCredentialsImpl(
+  virtual bool AddCompromisedCredentialsImpl(
       const CompromisedCredentials& compromised_credentials) = 0;
-  virtual void RemoveCompromisedCredentialsImpl(
+  virtual bool RemoveCompromisedCredentialsImpl(
       const std::string& signon_realm,
       const base::string16& username,
       RemoveCompromisedCredentialsReason reason) = 0;
   virtual std::vector<CompromisedCredentials>
   GetAllCompromisedCredentialsImpl() = 0;
-  virtual void RemoveCompromisedCredentialsByUrlAndTimeImpl(
+  virtual bool RemoveCompromisedCredentialsByUrlAndTimeImpl(
       const base::RepeatingCallback<bool(const GURL&)>& url_filter,
       base::Time remove_begin,
       base::Time remove_end) = 0;
@@ -520,6 +542,11 @@ class PasswordStore : protected PasswordStoreSync,
   // been changed.
   void NotifyLoginsChanged(const PasswordStoreChangeList& changes) override;
 
+  // Invokes callback and notifies observers if there was a change to the list
+  // of compromised passwords.
+  void InvokeAndNotifyAboutCompromisedPasswordsChange(
+      base::OnceCallback<bool()> callback);
+
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
   // Saves |username| and a hash of |password| for password reuse checking.
   // |is_gaia_password| indicates if it is a Gaia account. |event| is used for
@@ -652,7 +679,7 @@ class PasswordStore : protected PasswordStoreSync,
       const base::Closure& completion);
   void UnblacklistInternal(const PasswordStore::FormDigest& form_digest,
                            base::OnceClosure completion);
-  void RemoveCompromisedCredentialsByUrlAndTimeInternal(
+  bool RemoveCompromisedCredentialsByUrlAndTimeInternal(
       const base::RepeatingCallback<bool(const GURL&)>& url_filter,
       base::Time remove_begin,
       base::Time remove_end,
@@ -761,6 +788,9 @@ class PasswordStore : protected PasswordStoreSync,
 
   // The observers.
   scoped_refptr<base::ObserverListThreadSafe<Observer>> observers_;
+  scoped_refptr<base::ObserverListThreadSafe<CompromisedPasswordsObserver>>
+      compromised_passwords_observers_ = base::MakeRefCounted<
+          base::ObserverListThreadSafe<CompromisedPasswordsObserver>>();
 
   // Either of two below would actually be set based on a feature flag.
   std::unique_ptr<PasswordSyncableService> syncable_service_;

components/password_manager/core/browser/password_store_default.cc

@@ -222,22 +222,20 @@ std::vector<InteractionsStats> PasswordStoreDefault::GetSiteStatsImpl(
                    : std::vector<InteractionsStats>();
 }
 
-void PasswordStoreDefault::AddCompromisedCredentialsImpl(
-    const CompromisedCredentials& compromised_credentials) {
+bool PasswordStoreDefault::AddCompromisedCredentialsImpl(
+    const CompromisedCredentials& credentials) {
   DCHECK(background_task_runner()->RunsTasksInCurrentSequence());
-  if (login_db_)
-    login_db_->compromised_credentials_table().AddRow(compromised_credentials);
+  return login_db_ &&
+         login_db_->compromised_credentials_table().AddRow(credentials);
 }
 
-void PasswordStoreDefault::RemoveCompromisedCredentialsImpl(
+bool PasswordStoreDefault::RemoveCompromisedCredentialsImpl(
     const std::string& signon_realm,
     const base::string16& username,
     RemoveCompromisedCredentialsReason reason) {
   DCHECK(background_task_runner()->RunsTasksInCurrentSequence());
-  if (login_db_) {
-    login_db_->compromised_credentials_table().RemoveRow(signon_realm, username,
-                                                         reason);
-  }
+  return login_db_ && login_db_->compromised_credentials_table().RemoveRow(
+                          signon_realm, username, reason);
 }
 
 std::vector<CompromisedCredentials>
@@ -247,14 +245,14 @@ PasswordStoreDefault::GetAllCompromisedCredentialsImpl() {
                    : std::vector<CompromisedCredentials>();
 }
 
-void PasswordStoreDefault::RemoveCompromisedCredentialsByUrlAndTimeImpl(
+bool PasswordStoreDefault::RemoveCompromisedCredentialsByUrlAndTimeImpl(
     const base::RepeatingCallback<bool(const GURL&)>& url_filter,
     base::Time remove_begin,
     base::Time remove_end) {
-  if (login_db_) {
+  DCHECK(background_task_runner()->RunsTasksInCurrentSequence());
+  return login_db_ &&
          login_db_->compromised_credentials_table().RemoveRowsByUrlAndTime(
              url_filter, remove_begin, remove_end);
-  }
 }
 
 void PasswordStoreDefault::AddFieldInfoImpl(const FieldInfo& field_info) {

components/password_manager/core/browser/password_store_default.h

@@ -73,15 +73,15 @@ class PasswordStoreDefault : public PasswordStore {
   std::vector<InteractionsStats> GetAllSiteStatsImpl() override;
   std::vector<InteractionsStats> GetSiteStatsImpl(
       const GURL& origin_domain) override;
-  void AddCompromisedCredentialsImpl(
+  bool AddCompromisedCredentialsImpl(
       const CompromisedCredentials& compromised_credentials) override;
-  void RemoveCompromisedCredentialsImpl(
+  bool RemoveCompromisedCredentialsImpl(
       const std::string& signon_realm,
       const base::string16& username,
       RemoveCompromisedCredentialsReason reason) override;
   std::vector<CompromisedCredentials> GetAllCompromisedCredentialsImpl()
       override;
-  void RemoveCompromisedCredentialsByUrlAndTimeImpl(
+  bool RemoveCompromisedCredentialsByUrlAndTimeImpl(
       const base::RepeatingCallback<bool(const GURL&)>& url_filter,
       base::Time remove_begin,
       base::Time remove_end) override;

components/password_manager/core/browser/password_store_unittest.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
@@ -26,6 +27,7 @@
 #include "components/password_manager/core/browser/form_parsing/form_parser.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/password_reuse_detector.h"
+#include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
 #include "components/password_manager/core/browser/password_store_default.h"
 #include "components/password_manager/core/browser/password_store_signin_notifier.h"
@@ -109,6 +111,11 @@ class MockPasswordStoreConsumer : public PasswordStoreConsumer {
   DISALLOW_COPY_AND_ASSIGN(MockPasswordStoreConsumer);
 };
 
+struct MockCompromisedPasswordsObserver
+    : PasswordStore::CompromisedPasswordsObserver {
+  MOCK_METHOD0(OnCompromisedPasswordsChanged, void());
+};
+
 class MockPasswordStoreSigninNotifier : public PasswordStoreSigninNotifier {
  public:
   MOCK_METHOD1(SubscribeToSigninEvents, void(PasswordStore* store));
@@ -500,6 +507,94 @@ TEST_F(PasswordStoreTest, CompromisedCredentialsObserverOnLoginAdded) {
   store->ShutdownOnUIThread();
 }
 
+TEST_F(PasswordStoreTest,
+       CompromisedPasswordObserverOnCompromisedCredentialAdded) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kPasswordCheck);
+  MockCompromisedPasswordsObserver observer;
+
+  CompromisedCredentials compromised_credentials(
+      kTestWebRealm1, base::ASCIIToUTF16("username_value_1"),
+      base::Time::FromTimeT(1), CompromiseType::kLeaked);
+
+  scoped_refptr<PasswordStoreDefault> store = CreatePasswordStore();
+  store->Init(syncer::SyncableService::StartSyncFlare(), nullptr);
+  store->AddCompromisedPasswordsObserver(&observer);
+
+  // Expect a notification after adding a credential.
+  EXPECT_CALL(observer, OnCompromisedPasswordsChanged);
+  store->AddCompromisedCredentials(compromised_credentials);
+  WaitForPasswordStore();
+
+  // Adding the same credential should not result in another notification.
+  EXPECT_CALL(observer, OnCompromisedPasswordsChanged).Times(0);
+  store->AddCompromisedCredentials(compromised_credentials);
+  WaitForPasswordStore();
+
+  store->ShutdownOnUIThread();
+}
+
+TEST_F(PasswordStoreTest,
+       CompromisedPasswordObserverOnCompromisedCredentialRemoved) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kPasswordCheck);
+  MockCompromisedPasswordsObserver observer;
+
+  CompromisedCredentials compromised_credentials(
+      kTestWebRealm1, base::ASCIIToUTF16("username_value_1"),
+      base::Time::FromTimeT(1), CompromiseType::kLeaked);
+
+  scoped_refptr<PasswordStoreDefault> store = CreatePasswordStore();
+  store->Init(syncer::SyncableService::StartSyncFlare(), nullptr);
+  store->AddCompromisedCredentials(compromised_credentials);
+  WaitForPasswordStore();
+
+  store->AddCompromisedPasswordsObserver(&observer);
+
+  // Expect a notification after removing a credential.
+  EXPECT_CALL(observer, OnCompromisedPasswordsChanged);
+  store->RemoveCompromisedCredentials(
+      compromised_credentials.signon_realm, compromised_credentials.username,
+      RemoveCompromisedCredentialsReason::kRemove);
+  WaitForPasswordStore();
+
+  // Removing the same credential should not result in another notification.
+  EXPECT_CALL(observer, OnCompromisedPasswordsChanged).Times(0);
+  store->RemoveCompromisedCredentials(
+      compromised_credentials.signon_realm, compromised_credentials.username,
+      RemoveCompromisedCredentialsReason::kRemove);
+  WaitForPasswordStore();
+
+  store->ShutdownOnUIThread();
+}
+
+TEST_F(PasswordStoreTest,
+       CompromisedPasswordObserverOnCompromisedCredentialRemovedByTime) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kPasswordCheck);
+  MockCompromisedPasswordsObserver observer;
+
+  CompromisedCredentials compromised_credentials(
+      kTestWebRealm1, base::ASCIIToUTF16("username_value_1"),
+      base::Time::FromTimeT(1), CompromiseType::kLeaked);
+
+  scoped_refptr<PasswordStoreDefault> store = CreatePasswordStore();
+  store->Init(syncer::SyncableService::StartSyncFlare(), nullptr);
+  store->AddCompromisedCredentials(compromised_credentials);
+  WaitForPasswordStore();
+
+  store->AddCompromisedPasswordsObserver(&observer);
+
+  // Expect a notification after removing all credential.
+  EXPECT_CALL(observer, OnCompromisedPasswordsChanged);
+  store->RemoveCompromisedCredentialsByUrlAndTime(
+      base::NullCallback(), base::Time::Min(), base::Time::Max(),
+      base::NullCallback());
+  WaitForPasswordStore();
+
+  store->ShutdownOnUIThread();
+}
+
 // When no Android applications are actually affiliated with the realm of the
 // observed form, GetLoginsWithAffiliations() should still return the exact and
 // PSL matching results, but not any stored Android credentials.

components/password_manager/core/browser/test_password_store.cc

@@ -225,16 +225,18 @@ std::vector<InteractionsStats> TestPasswordStore::GetAllSiteStatsImpl() {
   return std::vector<InteractionsStats>();
 }
 
-void TestPasswordStore::AddCompromisedCredentialsImpl(
+bool TestPasswordStore::AddCompromisedCredentialsImpl(
     const CompromisedCredentials& stats) {
   NOTIMPLEMENTED();
+  return false;
 }
 
-void TestPasswordStore::RemoveCompromisedCredentialsImpl(
+bool TestPasswordStore::RemoveCompromisedCredentialsImpl(
     const std::string& signon_realm,
     const base::string16& username,
     RemoveCompromisedCredentialsReason reason) {
   NOTIMPLEMENTED();
+  return false;
 }
 
 std::vector<CompromisedCredentials>
@@ -243,11 +245,12 @@ TestPasswordStore::GetAllCompromisedCredentialsImpl() {
   return std::vector<CompromisedCredentials>();
 }
 
-void TestPasswordStore::RemoveCompromisedCredentialsByUrlAndTimeImpl(
+bool TestPasswordStore::RemoveCompromisedCredentialsByUrlAndTimeImpl(
     const base::RepeatingCallback<bool(const GURL&)>& url_filter,
     base::Time remove_begin,
     base::Time remove_end) {
   NOTIMPLEMENTED();
+  return false;
 }
 
 void TestPasswordStore::AddFieldInfoImpl(const FieldInfo& field_info) {

components/password_manager/core/browser/test_password_store.h

@@ -86,15 +86,15 @@ class TestPasswordStore : public PasswordStore {
   void AddSiteStatsImpl(const InteractionsStats& stats) override;
   void RemoveSiteStatsImpl(const GURL& origin_domain) override;
   std::vector<InteractionsStats> GetAllSiteStatsImpl() override;
-  void AddCompromisedCredentialsImpl(
+  bool AddCompromisedCredentialsImpl(
       const CompromisedCredentials& compromised_credentials) override;
-  void RemoveCompromisedCredentialsImpl(
+  bool RemoveCompromisedCredentialsImpl(
       const std::string& signon_realm,
       const base::string16& username,
       RemoveCompromisedCredentialsReason reason) override;
   std::vector<CompromisedCredentials> GetAllCompromisedCredentialsImpl()
       override;
-  void RemoveCompromisedCredentialsByUrlAndTimeImpl(
+  bool RemoveCompromisedCredentialsByUrlAndTimeImpl(
       const base::RepeatingCallback<bool(const GURL&)>& url_filter,
       base::Time remove_begin,
       base::Time remove_end) override;