From 692ebf6c7f7e823892ded7e0e597ef5951dcf1c5 Mon Sep 17 00:00:00 2001
From: Adem Derinel <[email protected]>
Date: Fri, 18 Sep 2026 05:17:39 -0700
Subject: [PATCH] [Passwords] Delete Credential Manager dialogs async in ResetDialog

Previously, CredentialManagerDialogControllerImpl::ResetDialog()
synchronously destroyed account_chooser_dialog_ and autosignin_dialog_.
Because AccountChooserDialogView owns its views::Widget, synchronous
destruction invokes ~Widget() -> WillClose() ->
BlockWebContentsInteraction(false) -> web_contents->Focus().

In a Document Picture-in-Picture window, focusing the WebContents can
deactivate chooser bubbles, triggering PiP exit and synchronous
WebContents destruction (and therefore ManagePasswordsUIController
destruction) while ManagePasswordsUIController or
CredentialManagerDialogControllerImpl is still executing on the stack,
leading to a heap use-after-free (UAF).

This CL updates ResetDialog() to delete the dialogs asynchronously via
DeleteSoon(), matching the existing behavior in OnCloseDialog(). This
prevents synchronous widget closing and recursive WebContents
destruction during dialog resetting.

TAG=agy
CONV=51cb8a63-900d-47ea-8b42-5208ffb2fe9c

Fixed: 562243925
Change-Id: I70f3b6a66c500a61ade7660832a828166598e39b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8419710
Reviewed-by: Rafał Godlewski <[email protected]>
Commit-Queue: Adem Derinel <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1700661}
---
diff --git a/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl.cc b/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl.cc
index 449ae99c..d0761ab 100644
--- a/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl.cc
+++ b/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl.cc
@@ -191,14 +191,7 @@
       profile_->GetPrefs());
   password_manager::metrics_util::LogAutoSigninPromoUserAction(
       password_manager::metrics_util::AUTO_SIGNIN_OK_GOT_IT);
-  if (autosignin_dialog_) {
-    autosignin_dialog_->ControllerGone();
-    // Delete the dialog asynchronously because we are currently in a views
-    // callback. Synchronous destruction would cause a use-after-free crash
-    // when the call stack unwinds back to the views framework.
-    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
-        FROM_HERE, std::move(autosignin_dialog_));
-  }
+  ResetDialog();
   delegate_->OnDialogHidden();
 }
 
@@ -209,14 +202,7 @@
       profile_->GetPrefs());
   password_manager::metrics_util::LogAutoSigninPromoUserAction(
       password_manager::metrics_util::AUTO_SIGNIN_TURN_OFF);
-  if (autosignin_dialog_) {
-    autosignin_dialog_->ControllerGone();
-    // Delete the dialog asynchronously because we are currently in a views
-    // callback. Synchronous destruction would cause a use-after-free crash
-    // when the call stack unwinds back to the views framework.
-    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
-        FROM_HERE, std::move(autosignin_dialog_));
-  }
+  ResetDialog();
   delegate_->OnDialogHidden();
 }
 
@@ -239,13 +225,18 @@
 }
 
 void CredentialManagerDialogControllerImpl::ResetDialog() {
+  // Delete the dialogs asynchronously to prevent synchronous destruction of the
+  // underlying views::Widget, which would unblock and focus the WebContents,
+  // potentially causing re-entrancy and synchronous WebContents destruction.
   if (account_chooser_dialog_) {
     account_chooser_dialog_->ControllerGone();
-    account_chooser_dialog_.reset();
+    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
+        FROM_HERE, std::move(account_chooser_dialog_));
   }
   if (autosignin_dialog_) {
     autosignin_dialog_->ControllerGone();
-    autosignin_dialog_.reset();
+    base::SingleThreadTaskRunner::GetCurrentDefault()->DeleteSoon(
+        FROM_HERE, std::move(autosignin_dialog_));
   }
 }
 
diff --git a/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc b/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc
index fe8ee78b..197fa66 100644
--- a/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc
+++ b/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc
@@ -39,13 +39,25 @@
                            public AutoSigninFirstRunPrompt {
  public:
   MockPasswordPrompt() = default;
+  ~MockPasswordPrompt() override {
+    if (on_destroy_) {
+      std::move(on_destroy_).Run();
+    }
+  }
 
   MockPasswordPrompt(const MockPasswordPrompt&) = delete;
   MockPasswordPrompt& operator=(const MockPasswordPrompt&) = delete;
 
+  void set_on_destroy(base::OnceClosure on_destroy) {
+    on_destroy_ = std::move(on_destroy);
+  }
+
   MOCK_METHOD(void, ShowAccountChooser, (), (override));
   MOCK_METHOD(void, ShowAutoSigninPrompt, (), (override));
   MOCK_METHOD(void, ControllerGone, (), (override));
+
+ private:
+  base::OnceClosure on_destroy_;
 };
 
 password_manager::PasswordForm GetLocalForm() {
@@ -398,4 +410,78 @@
 }
 #endif
 
+TEST_F(CredentialManagerDialogControllerTest,
+       ResetDialogDeletesDialogsAsynchronously) {
+  auto prompt = std::make_unique<StrictMock<MockPasswordPrompt>>();
+  auto* prompt_ptr = prompt.get();
+  password_manager::PasswordForm local_form = GetLocalForm();
+  std::vector<std::unique_ptr<password_manager::PasswordForm>> locals;
+  locals.push_back(
+      std::make_unique<password_manager::PasswordForm>(local_form));
+
+  base::RunLoop run_loop;
+  bool prompt_destroyed = false;
+  prompt_ptr->set_on_destroy(base::BindOnce(
+      [](bool* destroyed, base::OnceClosure quit_closure) {
+        *destroyed = true;
+        std::move(quit_closure).Run();
+      },
+      &prompt_destroyed, run_loop.QuitClosure()));
+
+  EXPECT_CALL(*prompt_ptr, ShowAccountChooser());
+  controller().ShowAccountChooser(std::move(prompt), std::move(locals));
+
+  // Close the dialog via choosing credential.
+  EXPECT_CALL(*prompt_ptr, ControllerGone());
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS)
+  EXPECT_CALL(feature_manager(), IsBiometricAuthenticationBeforeFillingEnabled)
+      .WillOnce(testing::Return(false));
+#endif
+  EXPECT_CALL(ui_controller_mock(),
+              ChooseCredential(
+                  local_form,
+                  password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD));
+  controller().OnChooseCredentials(
+      local_form, password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD);
+
+  // The prompt must NOT be destroyed synchronously inside OnChooseCredentials /
+  // ResetDialog.
+  EXPECT_FALSE(prompt_destroyed);
+
+  // The prompt is destroyed asynchronously once pending tasks run.
+  run_loop.Run();
+  EXPECT_TRUE(prompt_destroyed);
+}
+
+TEST_F(CredentialManagerDialogControllerTest,
+       ResetDialogDeletesAutosigninPromptAsynchronously) {
+  auto prompt = std::make_unique<StrictMock<MockPasswordPrompt>>();
+  auto* prompt_ptr = prompt.get();
+
+  base::RunLoop run_loop;
+  bool prompt_destroyed = false;
+  prompt_ptr->set_on_destroy(base::BindOnce(
+      [](bool* destroyed, base::OnceClosure quit_closure) {
+        *destroyed = true;
+        std::move(quit_closure).Run();
+      },
+      &prompt_destroyed, run_loop.QuitClosure()));
+
+  EXPECT_CALL(*prompt_ptr, ShowAutoSigninPrompt());
+  controller().ShowAutosigninPrompt(std::move(prompt));
+
+  // Close the dialog via OnAutoSigninOK (which invokes ResetDialog).
+  EXPECT_CALL(*prompt_ptr, ControllerGone());
+  EXPECT_CALL(ui_controller_mock(), OnDialogHidden());
+  controller().OnAutoSigninOK();
+
+  // The prompt must NOT be destroyed synchronously inside OnAutoSigninOK /
+  // ResetDialog.
+  EXPECT_FALSE(prompt_destroyed);
+
+  // The prompt is destroyed asynchronously once pending tasks run.
+  run_loop.Run();
+  EXPECT_TRUE(prompt_destroyed);
+}
+
 }  // namespace

diff --git a/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc b/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc
index fe8ee78b..197fa66 100644
--- a/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc
+++ b/chrome/browser/ui/passwords/credential_manager_dialog_controller_impl_unittest.cc
@@ -39,13 +39,25 @@
                            public AutoSigninFirstRunPrompt {
  public:
   MockPasswordPrompt() = default;
+  ~MockPasswordPrompt() override {
+    if (on_destroy_) {
+      std::move(on_destroy_).Run();
+    }
+  }
 
   MockPasswordPrompt(const MockPasswordPrompt&) = delete;
   MockPasswordPrompt& operator=(const MockPasswordPrompt&) = delete;
 
+  void set_on_destroy(base::OnceClosure on_destroy) {
+    on_destroy_ = std::move(on_destroy);
+  }
+
   MOCK_METHOD(void, ShowAccountChooser, (), (override));
   MOCK_METHOD(void, ShowAutoSigninPrompt, (), (override));
   MOCK_METHOD(void, ControllerGone, (), (override));
+
+ private:
+  base::OnceClosure on_destroy_;
 };
 
 password_manager::PasswordForm GetLocalForm() {
@@ -398,4 +410,78 @@
 }
 #endif
 
+TEST_F(CredentialManagerDialogControllerTest,
+       ResetDialogDeletesDialogsAsynchronously) {
+  auto prompt = std::make_unique<StrictMock<MockPasswordPrompt>>();
+  auto* prompt_ptr = prompt.get();
+  password_manager::PasswordForm local_form = GetLocalForm();
+  std::vector<std::unique_ptr<password_manager::PasswordForm>> locals;
+  locals.push_back(
+      std::make_unique<password_manager::PasswordForm>(local_form));
+
+  base::RunLoop run_loop;
+  bool prompt_destroyed = false;
+  prompt_ptr->set_on_destroy(base::BindOnce(
+      [](bool* destroyed, base::OnceClosure quit_closure) {
+        *destroyed = true;
+        std::move(quit_closure).Run();
+      },
+      &prompt_destroyed, run_loop.QuitClosure()));
+
+  EXPECT_CALL(*prompt_ptr, ShowAccountChooser());
+  controller().ShowAccountChooser(std::move(prompt), std::move(locals));
+
+  // Close the dialog via choosing credential.
+  EXPECT_CALL(*prompt_ptr, ControllerGone());
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS)
+  EXPECT_CALL(feature_manager(), IsBiometricAuthenticationBeforeFillingEnabled)
+      .WillOnce(testing::Return(false));
+#endif
+  EXPECT_CALL(ui_controller_mock(),
+              ChooseCredential(
+                  local_form,
+                  password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD));
+  controller().OnChooseCredentials(
+      local_form, password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD);
+
+  // The prompt must NOT be destroyed synchronously inside OnChooseCredentials /
+  // ResetDialog.
+  EXPECT_FALSE(prompt_destroyed);
+
+  // The prompt is destroyed asynchronously once pending tasks run.
+  run_loop.Run();
+  EXPECT_TRUE(prompt_destroyed);
+}
+
+TEST_F(CredentialManagerDialogControllerTest,
+       ResetDialogDeletesAutosigninPromptAsynchronously) {
+  auto prompt = std::make_unique<StrictMock<MockPasswordPrompt>>();
+  auto* prompt_ptr = prompt.get();
+
+  base::RunLoop run_loop;
+  bool prompt_destroyed = false;
+  prompt_ptr->set_on_destroy(base::BindOnce(
+      [](bool* destroyed, base::OnceClosure quit_closure) {
+        *destroyed = true;
+        std::move(quit_closure).Run();
+      },
+      &prompt_destroyed, run_loop.QuitClosure()));
+
+  EXPECT_CALL(*prompt_ptr, ShowAutoSigninPrompt());
+  controller().ShowAutosigninPrompt(std::move(prompt));
+
+  // Close the dialog via OnAutoSigninOK (which invokes ResetDialog).
+  EXPECT_CALL(*prompt_ptr, ControllerGone());
+  EXPECT_CALL(ui_controller_mock(), OnDialogHidden());
+  controller().OnAutoSigninOK();
+
+  // The prompt must NOT be destroyed synchronously inside OnAutoSigninOK /
+  // ResetDialog.
+  EXPECT_FALSE(prompt_destroyed);
+
+  // The prompt is destroyed asynchronously once pending tasks run.
+  run_loop.Run();
+  EXPECT_TRUE(prompt_destroyed);
+}
+
 }  // namespace