[Passwords]Replace Add/RemoveInsecure in the password protection service

Add/RemoveInsecureCredentials will be removed from PasswordStore. For
phished credentials the insecure_credentials_helper should be used
instead.

Bug: 1223022
Change-Id: Iafad6a1cfb760e9fa7bebff5d888ab4381540df6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3044114
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#905383}

chrome/browser/safe_browsing/chrome_password_protection_service.cc

@@ -44,7 +44,7 @@
 #include "components/google/core/common/google_util.h"
 #include "components/omnibox/common/omnibox_features.h"
 #include "components/password_manager/core/browser/form_parsing/form_parser.h"
-#include "components/password_manager/core/browser/insecure_credentials_table.h"
+#include "components/password_manager/core/browser/insecure_credentials_helper.h"
 #include "components/password_manager/core/browser/leak_detection_dialog_utils.h"
 #include "components/password_manager/core/browser/ui/password_check_referrer.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -286,6 +286,10 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
           &ChromePasswordProtectionService::OnEnterprisePasswordUrlChanged,
           base::Unretained(this)));
 
+  add_phished_credentials_ =
+      base::BindRepeating(&password_manager::AddPhishedCredentials);
+  remove_phished_credentials_ =
+      base::BindRepeating(&password_manager::RemovePhishedCredentials);
   // TODO(nparker) Move the rest of the above code into Init()
   // without crashing unittests.
   Init();
@@ -1672,7 +1676,9 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
     Profile* profile,
     scoped_refptr<SafeBrowsingUIManager> ui_manager,
     StringProvider sync_password_hash_provider,
-    VerdictCacheManager* cache_manager)
+    VerdictCacheManager* cache_manager,
+    ChangePhishedCredentialsCallback add_phished_credentials,
+    ChangePhishedCredentialsCallback remove_phished_credentials)
     : PasswordProtectionService(nullptr,
                                 nullptr,
                                 nullptr,
@@ -1685,6 +1691,8 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
       trigger_manager_(nullptr),
       profile_(profile),
       cache_manager_(cache_manager),
+      add_phished_credentials_(std::move(add_phished_credentials)),
+      remove_phished_credentials_(std::move(remove_phished_credentials)),
       sync_password_hash_provider_for_testing_(sync_password_hash_provider) {
   Init();
 }
@@ -1751,10 +1759,7 @@ void ChromePasswordProtectionService::PersistPhishedSavedPasswordCredential(
     }
     LogCredentialPhishedStatusChanged(
         CredentialPhishedStatus::kMarkedAsPhished);
-    password_store->AddInsecureCredential(password_manager::InsecureCredential(
+    add_phished_credentials_.Run(password_store, credential);
-        credential.signon_realm, credential.username, base::Time::Now(),
-        password_manager::InsecureType::kPhished,
-        password_manager::IsMuted(false)));
   }
 }
 
@@ -1773,10 +1778,7 @@ void ChromePasswordProtectionService::RemovePhishedSavedPasswordCredential(
     }
     LogCredentialPhishedStatusChanged(
         CredentialPhishedStatus::kSiteMarkedAsLegitimate);
-    password_store->RemoveInsecureCredentials(
+    remove_phished_credentials_.Run(password_store, credential);
-        credential.signon_realm, credential.username,
-        password_manager::RemoveInsecureCredentialsReason::
-            kMarkSiteAsLegitimate);
   }
 }
 

chrome/browser/safe_browsing/chrome_password_protection_service.h

@@ -7,7 +7,7 @@
 
 #include <map>
 
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/callback_list.h"
 #include "base/observer_list.h"
 #include "base/timer/timer.h"
@@ -20,6 +20,7 @@
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_reuse_manager.h"
 #include "components/password_manager/core/browser/password_store.h"
+#include "components/password_manager/core/browser/password_store_interface.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/safe_browsing/buildflags.h"
 #include "components/safe_browsing/content/browser/password_protection/password_protection_service.h"
@@ -72,6 +73,9 @@ MaybeCreateNavigationThrottle(content::NavigationHandle* navigation_handle);
 class ChromePasswordProtectionService : public PasswordProtectionService,
                                         public KeyedService {
  public:
+  using ChangePhishedCredentialsCallback = base::RepeatingCallback<void(
+      password_manager::PasswordStoreInterface*,
+      const password_manager::MatchingReusedCredential&)>;
   // Observer is used to coordinate password protection UIs (e.g. modal warning,
   // change password card, etc) in reaction to user events.
   class Observer {
@@ -549,7 +553,9 @@ class ChromePasswordProtectionService : public PasswordProtectionService,
       Profile* profile,
       scoped_refptr<SafeBrowsingUIManager> ui_manager,
       StringProvider sync_password_hash_provider,
-      VerdictCacheManager* cache_manager);
+      VerdictCacheManager* cache_manager,
+      ChangePhishedCredentialsCallback add_phished_credentials,
+      ChangePhishedCredentialsCallback remove_phished_credentials);
 
   // Code shared by both ctors.
   void Init();
@@ -578,6 +584,14 @@ class ChromePasswordProtectionService : public PasswordProtectionService,
   // Schedules the next time to log the PasswordCaptured event.
   base::OneShotTimer log_password_capture_timer_;
 
+  // Calls `password_manager::AddPhishedCredentials`. Used to facilitate
+  // testing.
+  ChangePhishedCredentialsCallback add_phished_credentials_;
+
+  // Calls `password_manager::RemovePhishedCredentials`. Used to facilitate
+  // testing.
+  ChangePhishedCredentialsCallback remove_phished_credentials_;
+
   // Bypasses the check for probability when sending sample pings.
   bool bypass_probability_for_tests_ = false;
 

chrome/browser/safe_browsing/chrome_password_protection_service_browsertest.cc

@@ -23,11 +23,13 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/password_manager/core/browser/fake_password_store_backend.h"
 #include "components/password_manager/core/browser/hash_password_manager.h"
-#include "components/password_manager/core/browser/mock_password_store.h"
+#include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/password_reuse_manager.h"
+#include "components/password_manager/core/browser/test_password_store.h"
 #include "components/password_manager/core/browser/ui/password_check_referrer.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/prefs/pref_service.h"
@@ -43,13 +45,18 @@
 #include "components/signin/public/identity_manager/identity_test_environment.h"
 #include "components/user_manager/user_names.h"
 #include "components/variations/service/variations_service.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+using password_manager::FakePasswordStoreBackend;
+using password_manager::PasswordForm;
+using password_manager::PasswordStore;
 using ::testing::_;
+using ::testing::ElementsAre;
 
 namespace {
 
@@ -58,6 +65,31 @@ const char kGaiaPasswordChangeHistogramName[] =
 const char kLoginPageUrl[] = "/safe_browsing/login_page.html";
 const char kChangePasswordUrl[] = "/safe_browsing/change_password_page.html";
 
+PasswordForm CreatePasswordFormWithPhishedEntry(std::string signon_realm,
+                                                std::u16string username) {
+  PasswordForm form;
+  form.signon_realm = signon_realm;
+  form.username_value = username;
+  form.password_value = u"password";
+  form.in_store = PasswordForm::Store::kProfileStore;
+  form.password_issues = {
+      {password_manager::InsecureType::kPhished,
+       password_manager::InsecurityMetadata(base::Time::FromTimeT(1),
+                                            password_manager::IsMuted(false))}};
+
+  return form;
+}
+
+void AddFormToStore(PasswordStore* password_store, const PasswordForm& form) {
+  password_store->AddLogin(form);
+  base::RunLoop().RunUntilIdle();
+  FakePasswordStoreBackend* fake_backend =
+      static_cast<FakePasswordStoreBackend*>(
+          password_store->GetBackendForTesting());
+  ASSERT_THAT(fake_backend->stored_passwords().at(form.signon_realm),
+              ElementsAre(form));
+}
+
 }  // namespace
 
 namespace safe_browsing {
@@ -339,20 +371,28 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
   // Simulate removing the compromised credentials on mark site as legitimate
   // action.
-  scoped_refptr<password_manager::MockPasswordStore> password_store =
+  scoped_refptr<password_manager::PasswordStore> password_store =
-      base::WrapRefCounted(static_cast<password_manager::MockPasswordStore*>(
+      base::WrapRefCounted(static_cast<password_manager::PasswordStore*>(
           PasswordStoreFactory::GetInstance()
               ->SetTestingFactoryAndUse(
                   browser()->profile(),
-                  base::BindRepeating(&password_manager::BuildPasswordStore<
+                  base::BindRepeating(
-                                      content::BrowserContext,
+                      &password_manager::BuildPasswordStoreWithFakeBackend<
-                                      password_manager::MockPasswordStore>))
+                          content::BrowserContext>))
               .get()));
+
+  // In order to test removal, we need to make sure it was added first.
+  const std::string kSignonRealm = "https://example.test";
+  const std::u16string kUsername = u"username1";
+  password_manager::PasswordForm form =
+      CreatePasswordFormWithPhishedEntry(kSignonRealm, kUsername);
+  AddFormToStore(password_store.get(), form);
+
   std::vector<password_manager::MatchingReusedCredential> credentials = {
-      {"https://example.test", u"username1"}};
+      {kSignonRealm, kUsername}};
+
   service->set_saved_passwords_matching_reused_credentials({credentials});
 
-  EXPECT_CALL(*password_store, RemoveInsecureCredentialsImpl(_, _, _)).Times(1);
   // Simulates clicking on "Mark site legitimate". Site is no longer dangerous.
   service->OnUserAction(web_contents, account_type, RequestOutcome::UNKNOWN,
                         LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED,
@@ -365,6 +405,13 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
   EXPECT_EQ(security_state::NONE, GetSecurityLevel(web_contents));
   EXPECT_EQ(security_state::MALICIOUS_CONTENT_STATUS_NONE,
             GetVisibleSecurityState(web_contents)->malicious_content_status);
+  FakePasswordStoreBackend* fake_backend =
+      static_cast<FakePasswordStoreBackend*>(
+          password_store->GetBackendForTesting());
+  EXPECT_TRUE(fake_backend->stored_passwords()
+                  .at(kSignonRealm)
+                  .at(0)
+                  .password_issues->empty());
 }
 #endif
 

chrome/browser/safe_browsing/chrome_password_protection_service_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
+#include "base/test/mock_callback.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/password_manager/account_password_store_factory.h"
@@ -33,6 +34,7 @@
 #include "components/password_manager/core/browser/mock_password_store.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.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/common/password_manager_features.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
@@ -65,6 +67,7 @@
 #include "extensions/browser/test_event_router.h"
 #endif
 
+using password_manager::MatchingReusedCredential;
 using sync_pb::UserEventSpecifics;
 using GaiaPasswordReuse = sync_pb::GaiaPasswordReuse;
 using GaiaPasswordCaptured = UserEventSpecifics::GaiaPasswordCaptured;
@@ -153,11 +156,15 @@ class MockChromePasswordProtectionService
       Profile* profile,
       scoped_refptr<SafeBrowsingUIManager> ui_manager,
       StringProvider sync_password_hash_provider,
-      VerdictCacheManager* cache_manager)
+      VerdictCacheManager* cache_manager,
+      ChangePhishedCredentialsCallback add_phished_credentials,
+      ChangePhishedCredentialsCallback remove_phished_credentials)
       : ChromePasswordProtectionService(profile,
                                         ui_manager,
                                         sync_password_hash_provider,
-                                        cache_manager),
+                                        cache_manager,
+                                        add_phished_credentials,
+                                        remove_phished_credentials),
         is_incognito_(false),
         is_extended_reporting_(false),
         is_syncing_(false),
@@ -314,7 +321,8 @@ class ChromePasswordProtectionServiceTest
             std::make_unique<ChromeSafeBrowsingUIManagerDelegate>(),
             std::make_unique<ChromeSafeBrowsingBlockingPageFactory>(),
             GURL(chrome::kChromeUINewTabURL)),
-        sync_password_hash_provider, cache_manager_.get());
+        sync_password_hash_provider, cache_manager_.get(),
+        mock_add_callback_.Get(), mock_remove_callback_.Get());
   }
 
   TestingProfile::TestingFactories GetTestingFactories() const override {
@@ -416,6 +424,12 @@ class ChromePasswordProtectionServiceTest
 #endif
   std::unique_ptr<VerdictCacheManager> cache_manager_;
   ScopedTestingLocalState local_state_;
+  base::MockCallback<
+      ChromePasswordProtectionService::ChangePhishedCredentialsCallback>
+      mock_add_callback_;
+  base::MockCallback<
+      ChromePasswordProtectionService::ChangePhishedCredentialsCallback>
+      mock_remove_callback_;
 };
 
 TEST_F(ChromePasswordProtectionServiceTest,
@@ -598,7 +612,8 @@ TEST_F(ChromePasswordProtectionServiceTest,
   std::vector<password_manager::MatchingReusedCredential> credentials = {
       {"http://example.test"}, {"http://2.example.com"}};
 
-  EXPECT_CALL(*password_store_, AddInsecureCredentialImpl(_)).Times(2);
+  EXPECT_CALL(mock_add_callback_, Run(password_store_.get(), credentials[0]));
+  EXPECT_CALL(mock_add_callback_, Run(password_store_.get(), credentials[1]));
   service_->PersistPhishedSavedPasswordCredential(credentials);
 }
 
@@ -610,12 +625,11 @@ TEST_F(ChromePasswordProtectionServiceTest,
       {"http://example.test", u"username1"},
       {"http://2.example.test", u"username2"}};
 
-  EXPECT_CALL(*password_store_,
+  EXPECT_CALL(mock_remove_callback_,
-              RemoveInsecureCredentialsImpl(
+              Run(password_store_.get(), credentials[0]));
-                  _, _,
+  EXPECT_CALL(mock_remove_callback_,
-                  password_manager::RemoveInsecureCredentialsReason::
+              Run(password_store_.get(), credentials[1]));
-                      kMarkSiteAsLegitimate))
+
-      .Times(2);
   service_->RemovePhishedSavedPasswordCredential(credentials);
 }
 
@@ -1568,7 +1582,10 @@ TEST_F(ChromePasswordProtectionServiceWithAccountPasswordStoreTest,
       {.signon_realm = "http://2.example.test",
        .in_store = password_manager::PasswordForm::Store::kAccountStore}};
 
-  EXPECT_CALL(*account_password_store_, AddInsecureCredentialImpl(_)).Times(2);
+  EXPECT_CALL(mock_add_callback_,
+              Run(account_password_store_.get(), credentials[0]));
+  EXPECT_CALL(mock_add_callback_,
+              Run(account_password_store_.get(), credentials[1]));
   service_->PersistPhishedSavedPasswordCredential(credentials);
 }
 
@@ -1582,12 +1599,11 @@ TEST_F(ChromePasswordProtectionServiceWithAccountPasswordStoreTest,
       {"http://2.example.test", u"username2",
        password_manager::PasswordForm::Store::kAccountStore}};
 
-  EXPECT_CALL(*account_password_store_,
+  EXPECT_CALL(mock_remove_callback_,
-              RemoveInsecureCredentialsImpl(
+              Run(account_password_store_.get(), credentials[0]));
-                  _, _,
+  EXPECT_CALL(mock_remove_callback_,
-                  password_manager::RemoveInsecureCredentialsReason::
+              Run(account_password_store_.get(), credentials[1]));
-                      kMarkSiteAsLegitimate))
+
-      .Times(2);
   service_->RemovePhishedSavedPasswordCredential(credentials);
 }
 

components/password_manager/core/browser/fake_password_store_backend.cc

@@ -8,6 +8,7 @@
 #include "base/sequenced_task_runner.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "components/password_manager/core/browser/password_form.h"
+#include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/browser/psl_matching_helper.h"
 
 namespace password_manager {
@@ -84,10 +85,20 @@ void FakePasswordStoreBackend::RemoveLoginsCreatedBetweenAsync(
   NOTIMPLEMENTED();
 }
 
+void FakePasswordStoreBackend::DisableAutoSignInForOriginsAsync(
+    const base::RepeatingCallback<bool(const GURL&)>& origin_filter,
+    base::OnceClosure completion) {
+  NOTIMPLEMENTED();
+}
+
 SmartBubbleStatsStore* FakePasswordStoreBackend::GetSmartBubbleStatsStore() {
   return nullptr;
 }
 
+FieldInfoStore* FakePasswordStoreBackend::GetFieldInfoStore() {
+  return nullptr;
+}
+
 LoginsResult FakePasswordStoreBackend::FillMatchingLoginsInternal(
     const std::vector<PasswordFormDigest>& forms) {
   std::vector<std::unique_ptr<PasswordForm>> results;

components/password_manager/core/browser/fake_password_store_backend.h

@@ -52,7 +52,11 @@ class FakePasswordStoreBackend : public PasswordStoreBackend {
       base::Time delete_begin,
       base::Time delete_end,
       PasswordStoreChangeListReply callback) override;
+  void DisableAutoSignInForOriginsAsync(
+      const base::RepeatingCallback<bool(const GURL&)>& origin_filter,
+      base::OnceClosure completion) override;
   SmartBubbleStatsStore* GetSmartBubbleStatsStore() override;
+  FieldInfoStore* GetFieldInfoStore() override;
 
   LoginsResult FillMatchingLoginsInternal(
       const std::vector<PasswordFormDigest>& forms);

components/password_manager/core/browser/insecure_credentials_helper.cc

@@ -58,10 +58,10 @@ void InsecureCredentialsHelper::AddPhishedCredentials(
   PasswordFormDigest digest = {PasswordForm::Scheme::kHtml,
                                credential.signon_realm,
                                GURL(credential.signon_realm)};
-  store_->GetLogins(digest, this);
   operation_ =
       base::BindOnce(&InsecureCredentialsHelper::AddPhishedCredentialsInternal,
                      base::Owned(this), credential);
+  store_->GetLogins(digest, this);
 }
 
 void InsecureCredentialsHelper::RemovePhishedCredentials(
@@ -69,10 +69,10 @@ void InsecureCredentialsHelper::RemovePhishedCredentials(
   PasswordFormDigest digest = {PasswordForm::Scheme::kHtml,
                                credential.signon_realm,
                                GURL(credential.signon_realm)};
-  store_->GetLogins(digest, this);
   operation_ = base::BindOnce(
       &InsecureCredentialsHelper::RemovePhishedCredentialsInternal,
       base::Owned(this), credential);
+  store_->GetLogins(digest, this);
 }
 
 void InsecureCredentialsHelper::OnGetPasswordStoreResults(

components/password_manager/core/browser/password_manager_test_utils.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/memory/ref_counted.h"
+#include "components/password_manager/core/browser/fake_password_store_backend.h"
 #include "components/password_manager/core/browser/origin_credential_store.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_hash_data.h"
@@ -48,6 +49,17 @@ scoped_refptr<RefcountedKeyedService> BuildPasswordStoreWithArgs(
   return store;
 }
 
+// Helper function that builds a real password store with a fake backend.
+// Context is the browser context prescribed by TestingFactory.
+template <class Context>
+scoped_refptr<RefcountedKeyedService> BuildPasswordStoreWithFakeBackend(
+    Context* context) {
+  return password_manager::BuildPasswordStoreWithArgs<
+      Context, password_manager::PasswordStore,
+      std::unique_ptr<password_manager::FakePasswordStoreBackend>>(
+      std::make_unique<password_manager::FakePasswordStoreBackend>(), context);
+}
+
 // Struct used for creation of PasswordForms from static arrays of data.
 // Note: This is only meant to be used in unit test.
 struct PasswordFormData {

components/password_manager/core/browser/password_store.h

@@ -161,6 +161,10 @@ class PasswordStore : public PasswordStoreInterface {
   std::unique_ptr<syncer::ProxyModelTypeControllerDelegate>
   CreateSyncControllerDelegate();
 
+#if defined(UNIT_TEST)
+  PasswordStoreBackend* GetBackendForTesting() { return backend_; }
+#endif
+
  protected:
   using LoginsTask = base::OnceCallback<LoginsResult()>;
   using LoginsResultProcessor =

ios/chrome/browser/safe_browsing/chrome_password_protection_service.h

@@ -11,7 +11,9 @@
 
 #include "base/memory/weak_ptr.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/password_manager/core/browser/insecure_credentials_helper.h"
 #include "components/password_manager/core/browser/password_reuse_detector.h"
+#include "components/password_manager/core/browser/password_store_interface.h"
 #include "components/safe_browsing/core/browser/password_protection/metrics_util.h"
 #include "components/safe_browsing/core/common/proto/csd.pb.h"
 #import "components/safe_browsing/ios/browser/password_protection/password_protection_service.h"
@@ -38,8 +40,16 @@ class ChromePasswordProtectionService
     : public safe_browsing::PasswordProtectionService,
       public KeyedService {
  public:
-  ChromePasswordProtectionService(SafeBrowsingService* sb_service,
+  using ChangePhishedCredentialsCallback = base::RepeatingCallback<void(
-                                  ChromeBrowserState* browser_state);
+      password_manager::PasswordStoreInterface*,
+      const password_manager::MatchingReusedCredential&)>;
+  ChromePasswordProtectionService(
+      SafeBrowsingService* sb_service,
+      ChromeBrowserState* browser_state,
+      ChangePhishedCredentialsCallback add_phished_credentials =
+          base::BindRepeating(&password_manager::AddPhishedCredentials),
+      ChangePhishedCredentialsCallback remove_phished_credentials =
+          base::BindRepeating(&password_manager::RemovePhishedCredentials));
   ~ChromePasswordProtectionService() override;
 
   // PasswordProtectionServiceBase:
@@ -269,6 +279,14 @@ class ChromePasswordProtectionService
 
   ChromeBrowserState* browser_state_;
 
+  // Calls `password_manager::AddPhishedCredentials`. Used to facilitate
+  // testing.
+  ChangePhishedCredentialsCallback add_phished_credentials_;
+
+  // Calls `password_manager::RemovePhishedCredentials`. Used to facilitate
+  // testing.
+  ChangePhishedCredentialsCallback remove_phished_credentials_;
+
   base::WeakPtrFactory<ChromePasswordProtectionService> weak_factory_{this};
 };
 

ios/chrome/browser/safe_browsing/chrome_password_protection_service.mm

@@ -15,6 +15,7 @@
 #include "base/time/time.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/omnibox/common/omnibox_features.h"
+#include "components/password_manager/core/browser/insecure_credentials_helper.h"
 #include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/browser/ui/password_check_referrer.h"
 #include "components/prefs/pref_service.h"
@@ -136,7 +137,9 @@ std::unique_ptr<UserEventSpecifics> GetUserEventSpecifics(
 
 ChromePasswordProtectionService::ChromePasswordProtectionService(
     SafeBrowsingService* sb_service,
-    ChromeBrowserState* browser_state)
+    ChromeBrowserState* browser_state,
+    ChangePhishedCredentialsCallback add_phished_credentials,
+    ChangePhishedCredentialsCallback remove_phished_credentials)
     : safe_browsing::PasswordProtectionService(
           sb_service->GetDatabaseManager(),
           sb_service->GetURLLoaderFactory(),
@@ -148,7 +151,9 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
           browser_state->IsOffTheRecord(),
           /*identity_manager=*/nullptr,
           /*try_token_fetch=*/false),
-      browser_state_(browser_state) {}
+      browser_state_(browser_state),
+      add_phished_credentials_(std::move(add_phished_credentials)),
+      remove_phished_credentials_(std::move(remove_phished_credentials)) {}
 
 ChromePasswordProtectionService::~ChromePasswordProtectionService() = default;
 
@@ -265,10 +270,7 @@ void ChromePasswordProtectionService::PersistPhishedSavedPasswordCredential(
     }
     LogCredentialPhishedStatusChanged(
         safe_browsing::CredentialPhishedStatus::kMarkedAsPhished);
-    password_store->AddInsecureCredential(password_manager::InsecureCredential(
+    add_phished_credentials_.Run(password_store, credential);
-        credential.signon_realm, credential.username, base::Time::Now(),
-        password_manager::InsecureType::kPhished,
-        password_manager::IsMuted(false)));
   }
 }
 
@@ -287,10 +289,7 @@ void ChromePasswordProtectionService::RemovePhishedSavedPasswordCredential(
     }
     LogCredentialPhishedStatusChanged(
         safe_browsing::CredentialPhishedStatus::kSiteMarkedAsLegitimate);
-    password_store->RemoveInsecureCredentials(
+    remove_phished_credentials_.Run(password_store, credential);
-        credential.signon_realm, credential.username,
-        password_manager::RemoveInsecureCredentialsReason::
-            kMarkSiteAsLegitimate);
   }
 }
 

ios/chrome/browser/safe_browsing/chrome_password_protection_service_unittest.mm

@@ -10,9 +10,9 @@
 
 #include "base/memory/scoped_refptr.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/mock_callback.h"
 #include "base/values.h"
 #include "components/keyed_service/core/service_access_type.h"
-#include "components/password_manager/core/browser/mock_password_store.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
 #include "components/password_manager/core/browser/password_reuse_detector.h"
 #include "components/prefs/pref_service.h"
@@ -43,7 +43,6 @@
 
 using ::testing::_;
 using password_manager::metrics_util::PasswordType;
-using password_manager::MockPasswordStore;
 using safe_browsing::LoginReputationClientRequest;
 using safe_browsing::LoginReputationClientResponse;
 using safe_browsing::PasswordProtectionTrigger;
@@ -82,7 +81,6 @@ constexpr struct {
      PasswordReuseLookup::REQUEST_FAILURE},
     {RequestOutcome::DISABLED_DUE_TO_USER_POPULATION,
      PasswordReuseLookup::REQUEST_FAILURE}};
-
 }  // namespace
 
 class FakeChromePasswordProtectionService
@@ -90,8 +88,13 @@ class FakeChromePasswordProtectionService
  public:
   explicit FakeChromePasswordProtectionService(
       SafeBrowsingService* sb_service,
-      ChromeBrowserState* browser_state)
+      ChromeBrowserState* browser_state,
-      : ChromePasswordProtectionService(sb_service, browser_state),
+      ChangePhishedCredentialsCallback add_phished_credentials,
+      ChangePhishedCredentialsCallback remove_phished_credentials)
+      : ChromePasswordProtectionService(sb_service,
+                                        browser_state,
+                                        add_phished_credentials,
+                                        remove_phished_credentials),
         is_incognito_(false),
         is_account_signed_in_(false),
         is_no_hosted_domain_found_(false) {}
@@ -126,7 +129,8 @@ class ChromePasswordProtectionServiceTest : public ChromeWebTest {
     safe_browsing_service_ = base::MakeRefCounted<FakeSafeBrowsingService>();
 
     service_ = std::make_unique<FakeChromePasswordProtectionService>(
-        safe_browsing_service_.get(), chrome_browser_state_.get());
+        safe_browsing_service_.get(), chrome_browser_state_.get(),
+        mock_add_callback_.Get(), mock_remove_callback_.Get());
 
     auto navigation_manager = std::make_unique<web::FakeNavigationManager>();
     fake_navigation_manager_ = navigation_manager.get();
@@ -151,13 +155,6 @@ class ChromePasswordProtectionServiceTest : public ChromeWebTest {
     fake_navigation_manager_->SetLastCommittedItem(item);
   }
 
-  MockPasswordStore* GetProfilePasswordStore() const {
-    return static_cast<MockPasswordStore*>(
-        IOSChromePasswordStoreFactory::GetForBrowserState(
-            chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS)
-            .get());
-  }
-
   syncer::FakeUserEventService* GetUserEventService() const {
     return static_cast<syncer::FakeUserEventService*>(
         IOSUserEventServiceFactory::GetForBrowserState(
@@ -217,6 +214,12 @@ class ChromePasswordProtectionServiceTest : public ChromeWebTest {
   std::unique_ptr<FakeChromePasswordProtectionService> service_;
   web::FakeWebState fake_web_state_;
   web::FakeNavigationManager* fake_navigation_manager_;
+  base::MockCallback<
+      ChromePasswordProtectionService::ChangePhishedCredentialsCallback>
+      mock_add_callback_;
+  base::MockCallback<
+      ChromePasswordProtectionService::ChangePhishedCredentialsCallback>
+      mock_remove_callback_;
   signin::IdentityTestEnvironment identity_test_env_;
 };
 
@@ -386,8 +389,8 @@ TEST_F(ChromePasswordProtectionServiceTest,
   std::vector<password_manager::MatchingReusedCredential> credentials = {
       {"http://example.test"}, {"http://2.example.com"}};
 
-  EXPECT_CALL(*GetProfilePasswordStore(), AddInsecureCredentialImpl(_))
+  EXPECT_CALL(mock_add_callback_, Run(_, credentials[0]));
-      .Times(2);
+  EXPECT_CALL(mock_add_callback_, Run(_, credentials[1]));
   service_->PersistPhishedSavedPasswordCredential(credentials);
 }
 
@@ -398,12 +401,8 @@ TEST_F(ChromePasswordProtectionServiceTest,
       {"http://example.test", u"username1"},
       {"http://2.example.test", u"username2"}};
 
-  EXPECT_CALL(*GetProfilePasswordStore(),
+  EXPECT_CALL(mock_remove_callback_, Run(_, credentials[0]));
-              RemoveInsecureCredentialsImpl(
+  EXPECT_CALL(mock_remove_callback_, Run(_, credentials[1]));
-                  _, _,
-                  password_manager::RemoveInsecureCredentialsReason::
-                      kMarkSiteAsLegitimate))
-      .Times(2);
   service_->RemovePhishedSavedPasswordCredential(credentials);
 }