diff --git a/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc b/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc index 30059c9f1b939..e8d41eb767f81 100644 --- a/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc +++ b/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc @@ -4,15 +4,19 @@ #include "chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog.h" +#include "base/functional/callback.h" #include "base/scoped_observation.h" #include "base/test/scoped_feature_list.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" #include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" #include "content/public/common/content_features.h" #include "content/public/test/browser_test.h" #include "testing/gtest/include/gtest/gtest-spi.h" +#include "third_party/blink/public/mojom/frame/fullscreen.mojom.h" #include "ui/base/mojom/dialog_button.mojom.h" #include "ui/events/test/test_event.h" #include "ui/views/bubble/bubble_dialog_delegate_view.h" @@ -67,6 +71,37 @@ class WidgetVisibilityObserver : public views::WidgetObserver { this}; }; +// Models the documented hazard at WebContentsImpl::ExitFullscreenMode ("This +// may spin the message loop and destroy this object"): when the fullscreen +// toggle observer fires synchronously inside ExitFullscreen(), it tears down +// the owning object — exactly what ~DigitalIdentityRequestImpl → +// ~DigitalIdentityProviderDesktop → dialog_.reset() does when the hosting +// RenderFrameHost is destroyed during the nested run loop. +class DestroyOnFullscreenToggleObserver : public content::WebContentsObserver { + public: + explicit DestroyOnFullscreenToggleObserver(content::WebContents* web_contents) + : content::WebContentsObserver(web_contents) {} + + void Arm(base::OnceClosure destroy_callback) { + destroy_callback_ = std::move(destroy_callback); + } + + bool fired() const { return fired_; } + + // content::WebContentsObserver: + void DidToggleFullscreenModeForTab(bool entered_fullscreen, + bool will_cause_resize) override { + if (destroy_callback_) { + fired_ = true; + std::move(destroy_callback_).Run(); + } + } + + private: + bool fired_ = false; + base::OnceClosure destroy_callback_; +}; + } // anonymous namespace class DigitalIdentityMultiStepDialogBrowserTest : public InProcessBrowserTest { @@ -233,3 +268,64 @@ IN_PROC_BROWSER_TEST_F(DigitalIdentityMultiStepDialogBrowserTest, ui::mojom::DialogButton::kOk)); } } + +// Regression test: DigitalIdentityMultiStepDialog::TryShow writes to +// `this->dialog_` after constrained_window::ShowWebModalDialogViews() returns, +// but ShowWebModalDialogViews → BlockWebContentsInteraction(true) → +// SetWebContentsBlocked(true) → web_contents->ExitFullscreen(true) can +// synchronously run observers that (per the in-tree comment at +// WebContentsImpl::ExitFullscreenMode) may destroy the hosting WebContents / +// RenderFrameHost and therefore the owning DigitalIdentityProviderDesktop — +// which frees `this`. The Widget itself survives (Widget::Close() is async), so +// CHECK(weak_widget) inside ShowWebModalDialogViews passes and control returns +// to TryShow with a dangling `this`. +// +// This test models that destruction by resetting the owning unique_ptr from +// the synchronous DidToggleFullscreenModeForTab observer notification (which +// fires inside ExitFullscreen). On an ASan build this fails with +// heap-use-after-free in DigitalIdentityMultiStepDialog::TryShow. +IN_PROC_BROWSER_TEST_F(DigitalIdentityMultiStepDialogBrowserTest, + TryShowUseAfterFreeWhenOwnerDestroyedDuringShow) { + content::WebContents* web_contents = GetActiveWebContents(); + + // Enter HTML (content) fullscreen so that BlockWebContentsInteraction(true) + // → BrowserWindowModalDialogDelegate::SetWebContentsBlocked(true) takes the + // `web_contents->ExitFullscreen(true)` branch. + { + ui_test_utils::FullscreenWaiter waiter(browser(), {.tab_fullscreen = true}); + static_cast(browser()) + ->EnterFullscreenModeForTab(web_contents->GetPrimaryMainFrame(), {}); + waiter.Wait(); + } + + // Mirrors DigitalIdentityProviderDesktop::dialog_ ownership. + auto dialog = std::make_unique( + web_contents->GetWeakPtr()); + DigitalIdentityMultiStepDialog* dialog_raw = dialog.get(); + + // When ExitFullscreen fires its synchronous observers inside + // ShowWebModalDialogViews, free `dialog` — modelling + // ~DigitalIdentityRequestImpl → provider_.reset() → + // ~DigitalIdentityProviderDesktop → dialog_.reset(). + DestroyOnFullscreenToggleObserver observer(web_contents); + observer.Arm(base::BindOnce( + [](std::unique_ptr* owned) { + owned->reset(); + }, + &dialog)); + + // TryShow → ShowWebModalDialogViews → ShowDialogWithManager → + // BlockWebContentsInteraction(true) → SetWebContentsBlocked(true) → + // ExitFullscreen(true) → DidToggleFullscreenModeForTab → dialog.reset() → + // ~DigitalIdentityMultiStepDialog (this->dialog_ is still empty so the dtor + // body is a no-op and the Widget survives) → return to TryShow which then + // move-assigns into freed `this->dialog_` and reads it back. + dialog_raw->TryShow(std::make_optional(), base::DoNothing(), + ButtonParams(), base::DoNothing(), u"Title", u"Body", + /*custom_body_field=*/nullptr, + /*show_progress_bar=*/false); + + // The observer must have fired synchronously inside TryShow; otherwise the + // re-entrancy precondition was not exercised. + EXPECT_TRUE(observer.fired()); +}