[Passwords] Wait for initialization in StartPasswordCheck

This change modifies StartPasswordCheck to wait for the initialization
of the PasswordCheckDelegate, which requires waiting for the initial
response from the PasswordStore. In order to support this async API, the
method now takes a callback, rather than synchronously returning a
value.

Bug: 1062985
Change-Id: I878bade15d06d1a564836566898a3d748fe82590
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2110230
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751976}

chrome/browser/extensions/api/passwords_private/password_check_delegate.cc

@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <map>
 #include <memory>
+#include <utility>
 
 #include "base/containers/flat_set.h"
 #include "base/memory/ref_counted.h"
@@ -48,17 +49,18 @@
 namespace extensions {
 
 using autofill::PasswordForm;
-using password_manager::BulkLeakCheckService;
 using password_manager::CanonicalizedCredential;
 using password_manager::CanonicalizeUsername;
 using password_manager::CompromiseType;
 using password_manager::CredentialWithPassword;
 using password_manager::LeakCheckCredential;
 using ui::TimeFormat;
+
 using CompromisedCredentialsView =
     password_manager::CompromisedCredentialsProvider::CredentialsView;
 using SavedPasswordsView =
     password_manager::SavedPasswordsPresenter::SavedPasswordsView;
+using State = password_manager::BulkLeakCheckService::State;
 
 // Key used to attach UserData to a LeakCheckCredential.
 constexpr char kPasswordCheckDataKey[] = "password-check-data-key";
@@ -143,23 +145,23 @@ api::passwords_private::CompromiseType ConvertCompromiseType(
 }
 
 api::passwords_private::PasswordCheckState ConvertPasswordCheckState(
-    BulkLeakCheckService::State state) {
+    State state) {
   switch (state) {
-    case BulkLeakCheckService::State::kIdle:
+    case State::kIdle:
       return api::passwords_private::PASSWORD_CHECK_STATE_IDLE;
-    case BulkLeakCheckService::State::kRunning:
+    case State::kRunning:
       return api::passwords_private::PASSWORD_CHECK_STATE_RUNNING;
-    case BulkLeakCheckService::State::kCanceled:
+    case State::kCanceled:
       return api::passwords_private::PASSWORD_CHECK_STATE_CANCELED;
-    case BulkLeakCheckService::State::kSignedOut:
+    case State::kSignedOut:
       return api::passwords_private::PASSWORD_CHECK_STATE_SIGNED_OUT;
-    case BulkLeakCheckService::State::kNetworkError:
+    case State::kNetworkError:
       return api::passwords_private::PASSWORD_CHECK_STATE_OFFLINE;
-    case BulkLeakCheckService::State::kQuotaLimit:
+    case State::kQuotaLimit:
       return api::passwords_private::PASSWORD_CHECK_STATE_QUOTA_LIMIT;
-    case BulkLeakCheckService::State::kTokenRequestFailure:
-    case BulkLeakCheckService::State::kHashingFailure:
-    case BulkLeakCheckService::State::kServiceError:
+    case State::kTokenRequestFailure:
+    case State::kHashingFailure:
+    case State::kServiceError:
       return api::passwords_private::PASSWORD_CHECK_STATE_OTHER_ERROR;
   }
 
@@ -377,10 +379,20 @@ bool PasswordCheckDelegate::RemoveCompromisedCredential(
   return !saved_passwords.empty();
 }
 
-bool PasswordCheckDelegate::StartPasswordCheck() {
+void PasswordCheckDelegate::StartPasswordCheck(
+    StartPasswordCheckCallback callback) {
+  // If the delegate isn't initialized yet, enqueue the callback and return
+  // early.
+  if (!is_initialized_) {
+    start_check_callbacks_.push_back(std::move(callback));
+    return;
+  }
+
+  // Also return early if the check is already running.
   if (bulk_leak_check_service_adapter_.GetBulkLeakCheckState() ==
-      BulkLeakCheckService::State::kRunning) {
-    return false;
+      State::kRunning) {
+    std::move(callback).Run(State::kRunning);
+    return;
   }
 
   auto progress = base::MakeRefCounted<PasswordCheckProgress>();
@@ -392,10 +404,17 @@ bool PasswordCheckDelegate::StartPasswordCheck() {
   is_check_running_ = bulk_leak_check_service_adapter_.StartBulkLeakCheck(
       kPasswordCheckDataKey, &data);
   DCHECK(is_check_running_);
-  return is_check_running_;
+  std::move(callback).Run(
+      bulk_leak_check_service_adapter_.GetBulkLeakCheckState());
 }
 
 void PasswordCheckDelegate::StopPasswordCheck() {
+  if (!is_initialized_) {
+    for (auto&& callback : std::exchange(start_check_callbacks_, {}))
+      std::move(callback).Run(State::kIdle);
+    return;
+  }
+
   bulk_leak_check_service_adapter_.StopBulkLeakCheck();
 }
 
@@ -412,13 +431,12 @@ PasswordCheckDelegate::GetPasswordCheckStatus() const {
         FormatElapsedTime(base::Time::FromDoubleT(last_check_completed)));
   }
 
-  BulkLeakCheckService::State state =
-      bulk_leak_check_service_adapter_.GetBulkLeakCheckState();
+  State state = bulk_leak_check_service_adapter_.GetBulkLeakCheckState();
   SavedPasswordsView saved_passwords =
       saved_passwords_presenter_.GetSavedPasswords();
 
   // Handle the currently running case first, only then consider errors.
-  if (state == BulkLeakCheckService::State::kRunning) {
+  if (state == State::kRunning) {
     result.state = api::passwords_private::PASSWORD_CHECK_STATE_RUNNING;
 
     if (password_check_progress_) {
@@ -444,6 +462,14 @@ PasswordCheckDelegate::GetPasswordCheckStatus() const {
 }
 
 void PasswordCheckDelegate::OnSavedPasswordsChanged(SavedPasswordsView) {
+  // Getting the first notification about a change in saved passwords implies
+  // that the delegate is initialized, and start check callbacks can be invoked,
+  // if any.
+  if (!std::exchange(is_initialized_, true)) {
+    for (auto&& callback : std::exchange(start_check_callbacks_, {}))
+      StartPasswordCheck(std::move(callback));
+  }
+
   // A change in the saved passwords might result in leaving or entering the
   // NO_PASSWORDS state, thus we need to trigger a notification.
   NotifyPasswordCheckStatusChanged();
@@ -459,11 +485,10 @@ void PasswordCheckDelegate::OnCompromisedCredentialsChanged(
   }
 }
 
-void PasswordCheckDelegate::OnStateChanged(BulkLeakCheckService::State state) {
-  if (is_check_running_ && state == BulkLeakCheckService::State::kIdle) {
+void PasswordCheckDelegate::OnStateChanged(State state) {
+  if (state == State::kIdle && std::exchange(is_check_running_, false)) {
     // When the service transitions from running into idle it has finished a
     // check.
-    is_check_running_ = false;
     profile_->GetPrefs()->SetDouble(
         password_manager::prefs::kLastTimePasswordCheckCompleted,
         base::Time::Now().ToDoubleT());
@@ -516,7 +541,7 @@ void PasswordCheckDelegate::OnCredentialDone(
   // While the check is still running trigger an update of the check status,
   // considering that the progress has changed.
   if (bulk_leak_check_service_adapter_.GetBulkLeakCheckState() ==
-      BulkLeakCheckService::State::kRunning) {
+      State::kRunning) {
     NotifyPasswordCheckStatusChanged();
   }
 }

chrome/browser/extensions/api/passwords_private/password_check_delegate.h

@@ -5,9 +5,12 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_PASSWORDS_PRIVATE_PASSWORD_CHECK_DELEGATE_H_
 #define CHROME_BROWSER_EXTENSIONS_API_PASSWORDS_PRIVATE_PASSWORD_CHECK_DELEGATE_H_
 
+#include "base/bind_helpers.h"
+#include "base/callback.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
+#include "chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_utils.h"
 #include "chrome/common/extensions/api/passwords_private.h"
 #include "components/password_manager/core/browser/bulk_leak_check_service.h"
@@ -38,6 +41,9 @@ class PasswordCheckDelegate
       public password_manager::CompromisedCredentialsProvider::Observer,
       public password_manager::BulkLeakCheckService::Observer {
  public:
+  using StartPasswordCheckCallback =
+      PasswordsPrivateDelegate::StartPasswordCheckCallback;
+
   using CredentialPasswordsMap =
       std::map<password_manager::CredentialWithPassword,
                std::vector<autofill::PasswordForm>,
@@ -72,9 +78,10 @@ class PasswordCheckDelegate
   bool RemoveCompromisedCredential(
       const api::passwords_private::CompromisedCredential& credential);
 
-  // Starts a check for compromised passwords. Returns true if a new check was
-  // started.
-  bool StartPasswordCheck();
+  // Requests to start a check for compromised passwords. Invokes |callback|
+  // once a check is running or the request was stopped via StopPasswordCheck().
+  void StartPasswordCheck(
+      StartPasswordCheckCallback callback = base::DoNothing());
   // Stops checking for compromised passwords.
   void StopPasswordCheck();
 
@@ -134,6 +141,15 @@ class PasswordCheckDelegate
   password_manager::BulkLeakCheckServiceAdapter
       bulk_leak_check_service_adapter_;
 
+  // Boolean that remembers whether the delegate is initialized. This is done
+  // when the delegate obtains the list of saved passwords for the first time.
+  bool is_initialized_ = false;
+
+  // List of callbacks that were passed to StartPasswordCheck() prior to the
+  // delegate being initialized. These will be run when either initialization
+  // finishes, or StopPasswordCheck() gets invoked before hand.
+  std::vector<StartPasswordCheckCallback> start_check_callbacks_;
+
   // Remembers the progress of the ongoing check. Null if no check is currently
   // running.
   base::WeakPtr<PasswordCheckProgress> password_check_progress_;

chrome/browser/extensions/api/passwords_private/password_check_delegate_unittest.cc

@@ -19,6 +19,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
+#include "base/test/mock_callback.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/time.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h"
@@ -84,9 +85,13 @@ using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::IsNull;
+using ::testing::Mock;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+using MockStartPasswordCheckCallback =
+    base::MockCallback<PasswordCheckDelegate::StartPasswordCheckCallback>;
+
 PasswordsPrivateEventRouter* CreateAndUsePasswordsPrivateEventRouter(
     Profile* profile) {
   return static_cast<PasswordsPrivateEventRouter*>(
@@ -853,8 +858,9 @@ TEST_F(PasswordCheckDelegateTest, LastTimePasswordCheckCompletedIsSet) {
 // in resetting the kLastTimePasswordCheckCompleted pref to the current time.
 TEST_F(PasswordCheckDelegateTest, LastTimePasswordCheckCompletedReset) {
   delegate().StartPasswordCheck();
-  service()->set_state_and_notify(BulkLeakCheckService::State::kIdle);
+  RunUntilIdle();
 
+  service()->set_state_and_notify(BulkLeakCheckService::State::kIdle);
   PasswordCheckStatus status = delegate().GetPasswordCheckStatus();
   EXPECT_THAT(status.elapsed_time_since_last_check,
               Pointee(std::string("Just now")));
@@ -909,4 +915,42 @@ TEST_F(PasswordCheckDelegateTest, OnCredentialDoneUpdatesProgress) {
   EXPECT_EQ(0, *status->remaining_in_queue);
 }
 
+// Tests that StopPasswordCheck() invokes pending callbacks before
+// initialization finishes.
+TEST_F(PasswordCheckDelegateTest,
+       StopPasswordCheckRespondsCancelsBeforeInitialization) {
+  MockStartPasswordCheckCallback callback1;
+  MockStartPasswordCheckCallback callback2;
+  delegate().StartPasswordCheck(callback1.Get());
+  delegate().StartPasswordCheck(callback2.Get());
+
+  EXPECT_CALL(callback1, Run(BulkLeakCheckService::State::kIdle));
+  EXPECT_CALL(callback2, Run(BulkLeakCheckService::State::kIdle));
+  delegate().StopPasswordCheck();
+
+  Mock::VerifyAndClearExpectations(&callback1);
+  Mock::VerifyAndClearExpectations(&callback2);
+  RunUntilIdle();
+}
+
+// Tests that pending callbacks get invoked once initialization finishes.
+TEST_F(PasswordCheckDelegateTest,
+       StartPasswordCheckRunsCallbacksAfterInitialization) {
+  identity_test_env().MakeAccountAvailable(kTestEmail);
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
+  RunUntilIdle();
+
+  MockStartPasswordCheckCallback callback1;
+  MockStartPasswordCheckCallback callback2;
+  EXPECT_CALL(callback1, Run(BulkLeakCheckService::State::kRunning));
+  EXPECT_CALL(callback2, Run(BulkLeakCheckService::State::kRunning));
+
+  // Use a local delegate instead of |delegate()| so that the Password Store can
+  // be set-up prior to constructing the object.
+  PasswordCheckDelegate delegate(&profile());
+  delegate.StartPasswordCheck(callback1.Get());
+  delegate.StartPasswordCheck(callback2.Get());
+  RunUntilIdle();
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/passwords_private/passwords_private_api.cc

@@ -324,10 +324,20 @@ PasswordsPrivateStartPasswordCheckFunction::
     ~PasswordsPrivateStartPasswordCheckFunction() = default;
 
 ResponseAction PasswordsPrivateStartPasswordCheckFunction::Run() {
-  if (!GetDelegate(browser_context())->StartPasswordCheck()) {
-    return RespondNow(Error("Starting password check failed."));
-  }
-  return RespondNow(NoArguments());
+  GetDelegate(browser_context())
+      ->StartPasswordCheck(base::BindOnce(
+          &PasswordsPrivateStartPasswordCheckFunction::OnStarted, this));
+
+  // OnStarted() might respond before we reach this point.
+  return did_respond() ? AlreadyResponded() : RespondLater();
+}
+
+void PasswordsPrivateStartPasswordCheckFunction::OnStarted(
+    password_manager::BulkLeakCheckService::State state) {
+  const bool is_running =
+      state == password_manager::BulkLeakCheckService::State::kRunning;
+  Respond(is_running ? NoArguments()
+                     : Error("Starting password check failed."));
 }
 
 // PasswordsPrivateStopPasswordCheckFunction:

chrome/browser/extensions/api/passwords_private/passwords_private_api.h

@@ -10,6 +10,7 @@
 #include "base/optional.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h"
+#include "components/password_manager/core/browser/bulk_leak_check_service.h"
 #include "extensions/browser/extension_function.h"
 
 namespace extensions {
@@ -274,6 +275,9 @@ class PasswordsPrivateStartPasswordCheckFunction : public ExtensionFunction {
 
   // ExtensionFunction overrides.
   ResponseAction Run() override;
+
+ private:
+  void OnStarted(password_manager::BulkLeakCheckService::State state);
 };
 
 class PasswordsPrivateStopPasswordCheckFunction : public ExtensionFunction {

chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc

@@ -88,8 +88,9 @@ class PasswordsPrivateApiTest : public ExtensionApiTest {
     return s_test_delegate_->StopPasswordCheckTriggered();
   }
 
-  void set_start_password_check_result(bool result) {
-    s_test_delegate_->SetStartPasswordCheckReturnSuccess(result);
+  void set_start_password_check_state(
+      password_manager::BulkLeakCheckService::State state) {
+    s_test_delegate_->SetStartPasswordCheckState(state);
   }
 
   bool IsOptedInForAccountStorage() {
@@ -232,14 +233,16 @@ IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, StartPasswordCheck) {
-  set_start_password_check_result(true);
+  set_start_password_check_state(
+      password_manager::BulkLeakCheckService::State::kRunning);
   EXPECT_FALSE(start_password_check_triggered());
   EXPECT_TRUE(RunPasswordsSubtest("startPasswordCheck")) << message_;
   EXPECT_TRUE(start_password_check_triggered());
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, StartPasswordCheckFailed) {
-  set_start_password_check_result(false);
+  set_start_password_check_state(
+      password_manager::BulkLeakCheckService::State::kIdle);
   EXPECT_FALSE(start_password_check_triggered());
   EXPECT_TRUE(RunPasswordsSubtest("startPasswordCheckFailed")) << message_;
   EXPECT_TRUE(start_password_check_triggered());

chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h

@@ -19,6 +19,7 @@
 #include "chrome/browser/ui/passwords/settings/password_ui_view.h"
 #include "chrome/common/extensions/api/passwords_private.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/password_manager/core/browser/bulk_leak_check_service.h"
 #include "components/password_manager/core/browser/ui/export_progress_status.h"
 #include "extensions/browser/extension_function.h"
 
@@ -36,6 +37,9 @@ class PasswordsPrivateDelegate : public KeyedService {
   using PlaintextPasswordCallback =
       base::OnceCallback<void(base::Optional<base::string16>)>;
 
+  using StartPasswordCheckCallback =
+      base::OnceCallback<void(password_manager::BulkLeakCheckService::State)>;
+
   using PlaintextCompromisedPasswordCallback = base::OnceCallback<void(
       base::Optional<api::passwords_private::CompromisedCredential>)>;
 
@@ -143,9 +147,9 @@ class PasswordsPrivateDelegate : public KeyedService {
   virtual bool RemoveCompromisedCredential(
       const api::passwords_private::CompromisedCredential& credential) = 0;
 
-  // Starts a check for compromised passwords. Returns true if a new check was
-  // started.
-  virtual bool StartPasswordCheck() = 0;
+  // Requests to start a check for compromised passwords. Invokes |callback|
+  // once a check is running or the request was stopped via StopPasswordCheck().
+  virtual void StartPasswordCheck(StartPasswordCheckCallback callback) = 0;
   // Stops a check for compromised passwords.
   virtual void StopPasswordCheck() = 0;
 

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc

@@ -501,8 +501,9 @@ bool PasswordsPrivateDelegateImpl::RemoveCompromisedCredential(
   return password_check_delegate_.RemoveCompromisedCredential(credential);
 }
 
-bool PasswordsPrivateDelegateImpl::StartPasswordCheck() {
-  return password_check_delegate_.StartPasswordCheck();
+void PasswordsPrivateDelegateImpl::StartPasswordCheck(
+    StartPasswordCheckCallback callback) {
+  password_check_delegate_.StartPasswordCheck(std::move(callback));
 }
 
 void PasswordsPrivateDelegateImpl::StopPasswordCheck() {

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h

@@ -82,7 +82,7 @@ class PasswordsPrivateDelegateImpl : public PasswordsPrivateDelegate,
       base::StringPiece new_password) override;
   bool RemoveCompromisedCredential(
       const api::passwords_private::CompromisedCredential& credential) override;
-  bool StartPasswordCheck() override;
+  void StartPasswordCheck(StartPasswordCheckCallback callback) override;
   void StopPasswordCheck() override;
   api::passwords_private::PasswordCheckStatus GetPasswordCheckStatus() override;
 

chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.cc

@@ -215,9 +215,10 @@ bool TestPasswordsPrivateDelegate::RemoveCompromisedCredential(
                        }) != 0;
 }
 
-bool TestPasswordsPrivateDelegate::StartPasswordCheck() {
+void TestPasswordsPrivateDelegate::StartPasswordCheck(
+    StartPasswordCheckCallback callback) {
   start_password_check_triggered_ = true;
-  return start_password_check_return_success_;
+  std::move(callback).Run(start_password_check_state_);
 }
 
 void TestPasswordsPrivateDelegate::StopPasswordCheck() {

chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.h

@@ -58,7 +58,7 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate {
   // delegate knows of a compromised credential with the same id.
   bool RemoveCompromisedCredential(
       const api::passwords_private::CompromisedCredential& credential) override;
-  bool StartPasswordCheck() override;
+  void StartPasswordCheck(StartPasswordCheckCallback callback) override;
   void StopPasswordCheck() override;
   api::passwords_private::PasswordCheckStatus GetPasswordCheckStatus() override;
 
@@ -78,8 +78,9 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate {
   bool StopPasswordCheckTriggered() const {
     return stop_password_check_triggered_;
   }
-  void SetStartPasswordCheckReturnSuccess(bool value) {
-    start_password_check_return_success_ = value;
+  void SetStartPasswordCheckState(
+      password_manager::BulkLeakCheckService::State state) {
+    start_password_check_state_ = state;
   }
 
  private:
@@ -114,7 +115,8 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate {
   // Flags for detecting whether password check operations have been invoked.
   bool start_password_check_triggered_ = false;
   bool stop_password_check_triggered_ = false;
-  bool start_password_check_return_success_ = true;
+  password_manager::BulkLeakCheckService::State start_password_check_state_ =
+      password_manager::BulkLeakCheckService::State::kRunning;
 };
 }  // namespace extensions
 

chrome/browser/ui/webui/settings/safety_check_handler.cc

@@ -144,6 +144,7 @@ void SafetyCheckHandler::HandleGetParentRanDisplayString(
 }
 
 void SafetyCheckHandler::CheckUpdates() {
+  // Usage of base::Unretained(this) is safe, because we own `version_updater_`.
   version_updater_->CheckForUpdate(
       base::Bind(&SafetyCheckHandler::OnUpdateCheckResult,
                  base::Unretained(this)),
@@ -174,13 +175,8 @@ void SafetyCheckHandler::CheckPasswords() {
   // browser should not crash.
   observed_leak_check_.RemoveAll();
   observed_leak_check_.Add(leak_service_);
-  passwords_delegate_->StartPasswordCheck();
-  // In the case of no passwords, there is no state transition and no callback.
-  // Because of that, it is necessary to check the state synchronously.
-  if (leak_service_->state() !=
-      password_manager::BulkLeakCheckService::State::kRunning) {
-    OnStateChanged(leak_service_->state());
-  }
+  passwords_delegate_->StartPasswordCheck(base::BindOnce(
+      &SafetyCheckHandler::OnStateChanged, weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SafetyCheckHandler::CheckExtensions() {
@@ -533,6 +529,8 @@ void SafetyCheckHandler::OnJavascriptDisallowed() {
 }
 
 void SafetyCheckHandler::RegisterMessages() {
+  // Usage of base::Unretained(this) is safe, because web_ui() owns `this` and
+  // won't release ownership until destruction.
   web_ui()->RegisterMessageCallback(
       kPerformSafetyCheck,
       base::BindRepeating(&SafetyCheckHandler::HandlePerformSafetyCheck,

chrome/browser/ui/webui/settings/safety_check_handler.h

@@ -173,6 +173,7 @@ class SafetyCheckHandler
   ScopedObserver<password_manager::BulkLeakCheckService,
                  password_manager::BulkLeakCheckService::Observer>
       observed_leak_check_{this};
+  base::WeakPtrFactory<SafetyCheckHandler> weak_ptr_factory_{this};
 };
 
 #endif  // CHROME_BROWSER_UI_WEBUI_SETTINGS_SAFETY_CHECK_HANDLER_H_

chrome/browser/ui/webui/settings/safety_check_handler_unittest.cc

@@ -98,14 +98,6 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate {
     state_ = state;
   }
 
-  // When |running_state_on_start_| is set to |true|, this class simulates the
-  // behavior of the real password check by synchronously settings the state to
-  // |kRunning| once |StartPasswordCheck()| is invoked. Since that is the case
-  // for the majority of non-error password check flows, the default is |true|.
-  // In some test cases with no transitions to the |kRunning| state, such as not
-  // having any passwords, it is necessary to disable this functionality.
-  void SetRunningStateOnStart(bool value) { running_state_on_start_ = value; }
-
   std::vector<extensions::api::passwords_private::CompromisedCredential>
   GetCompromisedCredentials() override {
     std::vector<extensions::api::passwords_private::CompromisedCredential>
@@ -123,22 +115,11 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate {
     return status;
   }
 
-  bool StartPasswordCheck() override {
-    bool ret_val =
-        extensions::TestPasswordsPrivateDelegate::StartPasswordCheck();
-    if (running_state_on_start_) {
-      leak_service_->set_state_and_notify(
-          password_manager::BulkLeakCheckService::State::kRunning);
-    }
-    return ret_val;
-  }
-
  private:
   password_manager::BulkLeakCheckService* leak_service_ = nullptr;
   int compromised_password_count_ = 0;
   extensions::api::passwords_private::PasswordCheckState state_ =
       extensions::api::passwords_private::PASSWORD_CHECK_STATE_IDLE;
-  bool running_state_on_start_ = true;
 };
 
 class TestSafetyCheckExtensionService : public TestExtensionService {
@@ -654,9 +635,6 @@ TEST_F(SafetyCheckHandlerTest, CheckPasswords_Error) {
 }
 
 TEST_F(SafetyCheckHandlerTest, CheckPasswords_RunningOneCompromised) {
-  test_passwords_delegate_.SetPasswordCheckState(
-      extensions::api::passwords_private::PASSWORD_CHECK_STATE_RUNNING);
-  test_passwords_delegate_.SetStartPasswordCheckReturnSuccess(false);
   test_passwords_delegate_.SetNumCompromisedCredentials(1);
   safety_check_->PerformSafetyCheck();
   EXPECT_TRUE(test_passwords_delegate_.StartPasswordCheckTriggered());
@@ -674,7 +652,8 @@ TEST_F(SafetyCheckHandlerTest, CheckPasswords_RunningOneCompromised) {
 }
 
 TEST_F(SafetyCheckHandlerTest, CheckPasswords_NoPasswords) {
-  test_passwords_delegate_.SetRunningStateOnStart(false);
+  test_passwords_delegate_.SetStartPasswordCheckState(
+      password_manager::BulkLeakCheckService::State::kIdle);
   safety_check_->PerformSafetyCheck();
   const base::DictionaryValue* event =
       GetSafetyCheckStatusChangedWithDataIfExists(