[SHv2] Add functionality to run password check

This CL adds functions to the password status check module of
SafetyHubv2 that allows running the password check asynchronously.
In a follow-up CL this will be triggered in regular intervals to run
the check in the background.

Change-Id: I62d1533e4859d8155d46af54b3d8c33fb5540106
Bug: 1443466
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4661332
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Eduard Hez <ehez@chromium.org>
Reviewed-by: Side YILMAZ <sideyilmaz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1185229}

chrome/browser/ui/safety_hub/password_status_check_service.cc

@@ -7,7 +7,6 @@
 #include "base/json/values_util.h"
 #include "base/rand_util.h"
 #include "base/time/time.h"
-#include "chrome/browser/extensions/api/passwords_private/password_check_delegate.h"
 #include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/affiliation_service_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
@@ -55,13 +54,17 @@ bool ShouldFindNewCheckTime(Profile* profile) {
 PasswordStatusCheckService::PasswordStatusCheckService(Profile* profile)
     : profile_(profile) {
   StartRepeatedUpdates();
+  UpdateInsecureCredentialCountAsync();
 }
 
 PasswordStatusCheckService::~PasswordStatusCheckService() = default;
 
 void PasswordStatusCheckService::Shutdown() {
   saved_passwords_presenter_observation_.Reset();
+  bulk_leak_check_observation_.Reset();
+  password_check_delegate_.reset();
   saved_passwords_presenter_.reset();
+  credential_id_generator_.reset();
 }
 
 void PasswordStatusCheckService::StartRepeatedUpdates() {
@@ -88,29 +91,109 @@ void PasswordStatusCheckService::StartRepeatedUpdates() {
 }
 
 void PasswordStatusCheckService::UpdateInsecureCredentialCountAsync() {
-  saved_passwords_presenter_observation_.Reset();
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (is_update_credential_count_pending_) {
+    return;
+  }
+
+  is_update_credential_count_pending_ = true;
+
+  InitializePasswordCheckInfrastructure();
+
+  CHECK(saved_passwords_presenter_);
+  if (!saved_passwords_presenter_observation_.IsObserving()) {
+    saved_passwords_presenter_observation_.Observe(
+        saved_passwords_presenter_.get());
+  }
+}
+
+void PasswordStatusCheckService::RunPasswordCheckAsync() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (is_password_check_running_) {
+    return;
+  }
+
+  is_password_check_running_ = true;
+
+  InitializePasswordCheckInfrastructure();
+
+  CHECK(password_check_delegate_);
+  if (!bulk_leak_check_observation_.IsObserving()) {
+    bulk_leak_check_observation_.Observe(
+        BulkLeakCheckServiceFactory::GetForProfile(profile_));
+  }
+
+  password_check_delegate_->StartPasswordCheck();
+}
+
+void PasswordStatusCheckService::OnSavedPasswordsChanged(
+    const password_manager::PasswordStoreChangeList& changes) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  CHECK(IsInfrastructureReady());
 
+  is_update_credential_count_pending_ = false;
+  UpdateInsecureCredentialCount();
+  MaybeResetInfrastructureAsync();
+}
+
+void PasswordStatusCheckService::OnStateChanged(
+    password_manager::BulkLeakCheckService::State state) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  CHECK(IsInfrastructureReady());
+
+  // TODO(crbug.com/1443466): Currently this logic only differentiates between
+  // running and not running and treats any non-running state as a successful
+  // run. Depending on the state some additional action may be warranted, such
+  // as changing re-run period on network error. Additionally, when connecting
+  // to the UI we'll likely need to keep the exit state for display.
+  switch (state) {
+    case password_manager::BulkLeakCheckServiceInterface::State::kRunning:
+      return;
+    case password_manager::BulkLeakCheckService::State::kServiceError:
+    case password_manager::BulkLeakCheckServiceInterface::State::kIdle:
+    case password_manager::BulkLeakCheckServiceInterface::State::kCanceled:
+    case password_manager::BulkLeakCheckServiceInterface::State::kSignedOut:
+    case password_manager::BulkLeakCheckServiceInterface::State::
+        kTokenRequestFailure:
+    case password_manager::BulkLeakCheckServiceInterface::State::
+        kHashingFailure:
+    case password_manager::BulkLeakCheckServiceInterface::State::kNetworkError:
+    case password_manager::BulkLeakCheckServiceInterface::State::kQuotaLimit:
+      is_password_check_running_ = false;
+      MaybeResetInfrastructureAsync();
+  }
+}
+
+void PasswordStatusCheckService::OnCredentialDone(
+    const password_manager::LeakCheckCredential& credential,
+    password_manager::IsLeaked is_leaked) {}
+
+void PasswordStatusCheckService::InitializePasswordCheckInfrastructure() {
+  if (IsInfrastructureReady()) {
+    return;
+  }
+
+  credential_id_generator_ = std::make_unique<extensions::IdGenerator>();
   saved_passwords_presenter_ =
       std::make_unique<password_manager::SavedPasswordsPresenter>(
           AffiliationServiceFactory::GetForProfile(profile_),
           PasswordStoreFactory::GetForProfile(
               profile_, ServiceAccessType::IMPLICIT_ACCESS),
           AccountPasswordStoreFactory::GetForProfile(
               profile_, ServiceAccessType::IMPLICIT_ACCESS));
-  saved_passwords_presenter_observation_.Observe(
-      saved_passwords_presenter_.get());
   saved_passwords_presenter_->Init();
-}
-
-void PasswordStatusCheckService::OnSavedPasswordsChanged(
-    const password_manager::PasswordStoreChangeList& changes) {
-  extensions::IdGenerator credential_id_generator;
-  auto password_check_delegate =
+  password_check_delegate_ =
       std::make_unique<extensions::PasswordCheckDelegate>(
-          profile_, saved_passwords_presenter_.get(), &credential_id_generator);
+          profile_, saved_passwords_presenter_.get(),
+          credential_id_generator_.get());
+}
 
-  std::vector<password_manager::CredentialUIEntry> insecure_credentials =
-      password_check_delegate->GetInsecureCredentialsManager()
+void PasswordStatusCheckService::UpdateInsecureCredentialCount() {
+  CHECK(IsInfrastructureReady());
+  auto insecure_credentials =
+      password_check_delegate_->GetInsecureCredentialsManager()
           ->GetInsecureCredentialEntries();
 
   compromised_credential_count_ = 0;
@@ -129,17 +212,38 @@ void PasswordStatusCheckService::OnSavedPasswordsChanged(
       reused_credential_count_++;
     }
   }
+}
 
-  password_check_delegate.reset();
-  saved_passwords_presenter_observation_.Reset();
-  saved_passwords_presenter_.reset();
-
-  if (on_passwords_changed_finished_callback_for_test_) {
-    on_passwords_changed_finished_callback_for_test_.Run();
+void PasswordStatusCheckService::MaybeResetInfrastructureAsync() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!is_update_credential_count_pending_ && !is_password_check_running_) {
+    saved_passwords_presenter_observation_.Reset();
+    bulk_leak_check_observation_.Reset();
+
+    // The reset is done as a task rather than directly because when observers
+    // are notified that e.g. the password check is done, it may be too early to
+    // reset the infrastructure immediately. Synchronous operations may still be
+    // ongoing in `SavedPasswordsPresenter`.
+    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
+        FROM_HERE, std::move(password_check_delegate_));
+    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
+        FROM_HERE, std::move(saved_passwords_presenter_));
+    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
+        FROM_HERE, std::move(credential_id_generator_));
   }
 }
 
-void PasswordStatusCheckService::RunPasswordCheck() {
-  // TODO(crbug.com/1443466)
-  NOTIMPLEMENTED();
+bool PasswordStatusCheckService::IsInfrastructureReady() const {
+  if (saved_passwords_presenter_ || password_check_delegate_ ||
+      credential_id_generator_) {
+    // `saved_passwords_presenter_`, `password_check_delegate_`, and
+    // `credential_id_generator_` should always be initialized at the same time.
+    CHECK(credential_id_generator_);
+    CHECK(saved_passwords_presenter_);
+    CHECK(password_check_delegate_);
+    return true;
+  }
+
+  return false;
 }

chrome/browser/ui/safety_hub/password_status_check_service.h

@@ -6,13 +6,16 @@
 #define CHROME_BROWSER_UI_SAFETY_HUB_PASSWORD_STATUS_CHECK_SERVICE_H_
 
 #include "base/scoped_observation.h"
+#include "chrome/browser/extensions/api/passwords_private/password_check_delegate.h"
+#include "chrome/browser/password_manager/bulk_leak_check_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
 
 class PasswordStatusCheckService
     : public KeyedService,
-      public password_manager::SavedPasswordsPresenter::Observer {
+      public password_manager::SavedPasswordsPresenter::Observer,
+      public password_manager::BulkLeakCheckServiceInterface::Observer {
  public:
   explicit PasswordStatusCheckService(Profile* profile);
 
@@ -26,66 +29,120 @@ class PasswordStatusCheckService
   void Shutdown() override;
 
   // Getters for different issues with credentials.
-  size_t compromised_credential_count() {
+  size_t compromised_credential_count() const {
     return compromised_credential_count_;
   }
-  size_t weak_credential_count() { return weak_credential_count_; }
-  size_t reused_credential_count() { return reused_credential_count_; }
+  size_t weak_credential_count() const { return weak_credential_count_; }
+  size_t reused_credential_count() const { return reused_credential_count_; }
+
+  bool is_update_credential_count_pending() const {
+    return is_update_credential_count_pending_;
+  }
+
+  bool is_password_check_running() const { return is_password_check_running_; }
 
   // Register a delayed task running the password check.
   void StartRepeatedUpdates();
 
-  // Triggers an update to cached credential issues. Will start initialization
-  // of `saved_passwords_presenter_` and observes `OnSavedPasswordsChanged`.
+  // Bring cached credential issues up to date with data from Password Manager.
   void UpdateInsecureCredentialCountAsync();
 
+  // Triggers Password Manager's password check to discover new credential
+  // issues.
+  //
+  // TODO(crbug.com/1443466) Make private once there is a way for the password
+  // check to be publicly triggered.
+  void RunPasswordCheckAsync();
+
+  // Testing functions.
+  bool IsObservingSavedPasswordsPresenterForTesting() const {
+    return saved_passwords_presenter_observation_.IsObserving();
+  }
+
+  bool IsObservingBulkLeakCheckForTesting() const {
+    return bulk_leak_check_observation_.IsObserving();
+  }
+
+  // Public getters for testing.
   password_manager::SavedPasswordsPresenter*
   GetSavedPasswordsPresenterForTesting() {
     return saved_passwords_presenter_.get();
   }
 
-  bool IsObservingSavedPasswordsPresenterForTesting() {
-    return saved_passwords_presenter_observation_.IsObserving();
-  }
-
-  void SetTestingCallback(base::RepeatingClosure callback) {
-    on_passwords_changed_finished_callback_for_test_ = std::move(callback);
+  extensions::PasswordCheckDelegate* GetPasswordCheckDelegateForTesting() {
+    return password_check_delegate_.get();
   }
 
  private:
   // SavedPasswordsPresenter::Observer implementation.
-  // Brings cached values for insecure credential counts up to date with
-  // `saved_passwords_presenter_`. Getting notified about this indicates that
-  // the presenter is initialized. When update is complete
-  // `saved_passwords_presenter_` is reset to save memory.
+  // Getting notified about this indicates that the presenter is initialized
+  // and ready to be queried for credential issues.
   void OnSavedPasswordsChanged(
       const password_manager::PasswordStoreChangeList& changes) override;
 
-  // This function is called at regular intervals and triggers the password
-  // check, which will retrieve and store credential issues. As a result,
-  // reasonably up-to-date information is made available for SafetyHub.
-  void RunPasswordCheck();
+  // BulkLeakCheckService::Observer implementation.
+  // This is observed to get notified of the progress of the password check.
+  void OnStateChanged(
+      password_manager::BulkLeakCheckService::State state) override;
+  void OnCredentialDone(const password_manager::LeakCheckCredential& credential,
+                        password_manager::IsLeaked is_leaked) override;
+
+  // Initializes |saved_passwords_presenter_| and |password_check_delegate_|.
+  void InitializePasswordCheckInfrastructure();
+
+  // Brings cached values for insecure credential counts up to date with
+  // |saved_passwords_presenter_|.
+  void UpdateInsecureCredentialCount();
+
+  // Posts a task to delete `password_check_delegate_` and
+  // `saved_passwords_presenter_` if async operations have concluded to keep
+  // memory footprint low.
+  void MaybeResetInfrastructureAsync();
+
+  // Verifies that both `password_check_delegate_` and
+  // `saved_passwords_presenter_` are initialized.
+  bool IsInfrastructureReady() const;
 
   raw_ptr<Profile> profile_;
 
-  // Required to obtain the list of saved passwords and run the password check.
-  // Because it is memory-intensive, only initialized when needed.
+  // Required for `password_check_delegate_`. Because it is memory intensive,
+  // only initialized when needed.
+  std::unique_ptr<extensions::IdGenerator> credential_id_generator_;
+
+  // Required to obtain the list of saved passwords. Also is required for
+  // construction of `PasswordCheckDelegate`. Because it is memory intensive,
+  // only initialized when needed.
   std::unique_ptr<password_manager::SavedPasswordsPresenter>
       saved_passwords_presenter_;
 
-  // A scoped observer for `saved_passwords_presenter_`.
+  // Required to run the password check. Because it is memory intensive, only
+  // initialized when needed.
+  std::unique_ptr<extensions::PasswordCheckDelegate> password_check_delegate_;
+
+  // A scoped observer for `saved_passwords_presenter_`. This is used for
+  // detecting when `saved_passwords_presenter_` is initialized through
+  // `OnSavedPasswordsChanged`.
   base::ScopedObservation<password_manager::SavedPasswordsPresenter,
                           password_manager::SavedPasswordsPresenter::Observer>
       saved_passwords_presenter_observation_{this};
 
+  // A scoped observer for `BulkLeakCheckService` which is used by
+  // `PasswordCheckDelegate`. This is used for detecting when password check is
+  // complete through `OnStateChanged`.
+  base::ScopedObservation<
+      password_manager::BulkLeakCheckServiceInterface,
+      password_manager::BulkLeakCheckServiceInterface::Observer>
+      bulk_leak_check_observation_{this};
+
   // Cached results of the password check.
   size_t compromised_credential_count_ = 0;
   size_t weak_credential_count_ = 0;
   size_t reused_credential_count_ = 0;
 
-  // If bound, will be invoked at the end of the scope of
-  // `OnSavedPasswordsChanged()`.
-  base::RepeatingClosure on_passwords_changed_finished_callback_for_test_;
+  // Flags to indicate which async operations are currently ongoing. Memory
+  // intensive objects will be reset after all have finished.
+  bool is_update_credential_count_pending_ = false;
+  bool is_password_check_running_ = false;
 };
 
 #endif  // CHROME_BROWSER_UI_SAFETY_HUB_PASSWORD_STATUS_CHECK_SERVICE_H_

chrome/browser/ui/safety_hub/password_status_check_service_factory.cc

@@ -39,6 +39,7 @@ PasswordStatusCheckServiceFactory::PasswordStatusCheckServiceFactory()
               .Build()) {
   DependsOn(AccountPasswordStoreFactory::GetInstance());
   DependsOn(AffiliationServiceFactory::GetInstance());
+  DependsOn(BulkLeakCheckServiceFactory::GetInstance());
   DependsOn(PasswordStoreFactory::GetInstance());
 }