CVE-2026-102316
Overview
Background
- `PictureInPictureWindowManager` (PiP manager)
- the singleton that tracks and controls the active document/video picture-in-picture window and its associated objects.
- `ExitPictureInPicture`
- a method that synchronously closes any open PiP window and destroys objects tied to it, including per-tab
TabFeatures. - `ExitPictureInPictureSoon`
- a static helper that posts a task to exit PiP later, after the current call returns, avoiding re-entrant teardown.
- `OnOcclusionStateChanged`
- dialog-delegate observer callbacks (in shortcut/web-app dialogs) invoked when a PiP window occludes a dialog, which force the PiP window closed to prevent spoofing.
Root Cause Analysis
Several dialog occlusion observers (CreateDesktopShortcutDelegate::OnOcclusionStateChanged, WebAppModalDialogDelegate’s occlusion handler, and the handler in web_app_update_review_dialog.cc) responded to a PiP window occluding a dialog by calling ExitPictureInPicture() synchronously from within the observer/callback. That synchronous exit tore down the PiP window and destroyed the TabFeatures associated with it while the dialog-show / visibility-change flow that triggered the callback was still executing, so code that continued after the callback (attempting to show the dialog on that window) touched freed objects, producing an ASan-reported use-after-free.
The fix converts these call sites to the newly-promoted static ExitPictureInPictureSoon(), which posts the exit as an asynchronous task so teardown happens after the current stack unwinds rather than re-entrantly. The header change also documents ExitPictureInPicture as unsafe to call from callbacks/observers and directs such callers to the async variant.
ExitPictureInPictureSoon so it runs after the notification returns.Attack Path
- Open a PiP window over a dialog A page/app opens a document or video picture-in-picture window positioned so it occludes a security-sensitive dialog (shortcut-creation or web-app modal).
- Trigger the occlusion observer
The occlusion state change fires
OnOcclusionStateChanged(occluded=true), whose anti-spoofing logic callsExitPictureInPicture()synchronously. - Re-entrant teardown frees TabFeatures
The synchronous exit destroys the PiP window and its associated
TabFeatureswhile the dialog show / visibility-change flow that invoked the observer is still on the stack. (inferred) - Use-after-free on freed objects Continued execution of the dialog-showing path dereferences the now-freed objects, the ASan crash the commit fixes.
Impact Assessment
(inferred). The regression tests confirm only that exit is now asynchronous and does not crash when no window exists.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/picture_in_picture/document_picture_in_picture_mixin_test_base.cc |
modified | |
ifchrome/browser/ui/views/shortcuts/create_desktop_shortcut_delegate.cc |
modified | |
ifchrome/browser/ui/views/web_apps/web_app_modal_dialog_delegate.cc |
modified | |
ifchrome/browser/ui/views/web_apps/web_app_update_review_dialog.cc |
modified | |
TEST_Fchrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc |
modified |
Files Changed
chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.ccchrome/browser/ui/views/web_apps/web_app_modal_dialog_delegate.ccchrome/browser/picture_in_picture/document_picture_in_picture_mixin_test_base.ccchrome/browser/ui/views/shortcuts/create_desktop_shortcut_delegate.ccchrome/browser/picture_in_picture/picture_in_picture_window_manager.hchrome/browser/ui/views/web_apps/web_app_update_review_dialog.cc
Audit Directions
- Synchronous destruction from observersLook for observer/callback bodies that synchronously destroy the object graph (windows,
TabFeatures, controllers) that the notifying flow is still walking. - Re-entrant lifecycle calls during show/visibilityAudit dialog and widget show / occlusion / visibility-change paths for calls that free the very objects being shown.
- Sync vs. async teardown APIsWhere a class exposes both immediate and “Soon”/posted-task variants, verify callbacks and observers use the deferred one.
Patch
From afb9b3f8eaf646f34743aa0f474988fc6970ff3f Mon Sep 17 00:00:00 2001 From: Alison Gale <[email protected]> Date: Thu, 17 Sep 2026 17:08:45 -0700 Subject: [PATCH] [Dialog] Fix crash with pip and dialog manager Synchronously tearing down the pip window destroyed the tab features associated with it which caused an asan crash when trying to show a dialog on that window. Switch to exposing the async version to prevent this. Bug: 560251736,560238698,560503095,562174487,467460499 Change-Id: Ie34406e317267d819472d90aec96914b4c5a38f5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8408228 Commit-Queue: Alison Gale <[email protected]> Reviewed-by: Benjamin Keen <[email protected]> Cr-Commit-Position: refs/heads/main@{#1700295} --- diff --git a/chrome/browser/picture_in_picture/document_picture_in_picture_mixin_test_base.cc b/chrome/browser/picture_in_picture/document_picture_in_picture_mixin_test_base.cc index f42ec3f..d62f385 100644 --- a/chrome/browser/picture_in_picture/document_picture_in_picture_mixin_test_base.cc +++ b/chrome/browser/picture_in_picture/document_picture_in_picture_mixin_test_base.cc @@ -9,6 +9,7 @@ #include "base/files/file_path.h" #include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" +#include "base/test/run_until.h" #include "base/time/time.h" #include "chrome/browser/ui/browser_window/public/browser_window_interface.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -78,17 +79,8 @@ } bool DocumentPictureInPictureMixinTestBase::AwaitPipWindowClosedSuccessfully() { - auto* render_widget_host_view = GetRenderWidgetHostView(); - if (!render_widget_host_view) { - return true; - } - ui_test_utils::CheckWaiter( - base::BindRepeating(&content::RenderWidgetHostView::IsShowing, - base::Unretained(render_widget_host_view)), - false, base::Seconds(30)) - .Wait(); - - return (GetRenderWidgetHostView() == nullptr); + return base::test::RunUntil( + [this]() { return GetRenderWidgetHostView() == nullptr; }); } content::RenderWidgetHostView* diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_manager.h b/chrome/browser/picture_in_picture/picture_in_picture_window_manager.h index 444c5b4..063a8f7 100644 --- a/chrome/browser/picture_in_picture/picture_in_picture_window_manager.h +++ b/chrome/browser/picture_in_picture/picture_in_picture_window_manager.h @@ -117,8 +117,22 @@ // Closes any existing picture-in-picture windows (video or document pip). // Returns true if a picture-in-picture window was closed, and false if there // were no picture-in-picture windows to close. + // + // NOTE: This synchronously closes the window and destroys associated objects + // (such as TabFeatures). Calling this synchronously from callbacks or + // observers (e.g. during widget show or visibility changes) can cause + // re-entrancy issues and use-after-free crashes. Use + // `ExitPictureInPictureSoon()` unless you know that synchronous closure will + // not be a problem. bool ExitPictureInPicture(); + // Exits picture in picture soon, but not before this call returns. If + // picture in picture closes between now and then, that's okay. Intended as a + // helper for callbacks and observers to avoid re-entrant calls during pip + // set-up or in observer notifications (e.g. when an occluded dialog is being + // shown). + static void ExitPictureInPictureSoon(); + // Called to notify that the initiator web contents should be focused. void FocusInitiator(); @@ -352,10 +366,6 @@ // Called when the document PiP parent web contents is being destroyed. void DocumentWebContentsDestroyed(); - // Exits picture in picture soon, but not before this call returns. If - // picture in picture closes between now and then, that's okay. Intended as a - // helper class for callbacks, to avoid re-entrant calls during pip set-up. - static void ExitPictureInPictureSoon(); #if !BUILDFLAG(IS_ANDROID) // Creates the `occlusion_tracker_` if it does not already exist and should diff --git a/chrome/browser/ui/views/shortcuts/create_desktop_shortcut_delegate.cc b/chrome/browser/ui/views/shortcuts/create_desktop_shortcut_delegate.cc index 1601d983..8cfd2e6 100644 --- a/chrome/browser/ui/views/shortcuts/create_desktop_shortcut_delegate.cc +++ b/chrome/browser/ui/views/shortcuts/create_desktop_shortcut_delegate.cc @@ -103,10 +103,8 @@ } void CreateDesktopShortcutDelegate::OnOcclusionStateChanged(bool occluded) { - // If a picture-in-picture window is occluding the dialog, force it to close - // to prevent spoofing. if (occluded) { - PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture(); + PictureInPictureWindowManager::ExitPictureInPictureSoon(); } } diff --git a/chrome/browser/ui/views/web_apps/web_app_modal_dialog_delegate.cc b/chrome/browser/ui/views/web_apps/web_app_modal_dialog_delegate.cc index f46ab3f7..2d563af 100644 --- a/chrome/browser/ui/views/web_apps/web_app_modal_dialog_delegate.cc +++ b/chrome/browser/ui/views/web_apps/web_app_modal_dialog_delegate.cc @@ -55,7 +55,7 @@ // If a picture-in-picture window is occluding the dialog, force it to close // to prevent spoofing. if (occluded) { - PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture(); + PictureInPictureWindowManager::ExitPictureInPictureSoon(); } } diff --git a/chrome/browser/ui/views/web_apps/web_app_update_review_dialog.cc b/chrome/browser/ui/views/web_apps/web_app_update_review_dialog.cc index 21ad4a98..0e2efda 100644 --- a/chrome/browser/ui/views/web_apps/web_app_update_review_dialog.cc +++ b/chrome/browser/ui/views/web_apps/web_app_update_review_dialog.cc @@ -202,7 +202,7 @@ // If a picture-in-picture window is occluding the dialog, force it to close // to prevent spoofing. if (occluded) { - PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture(); + PictureInPictureWindowManager::ExitPictureInPictureSoon(); } } diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc b/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc index d1c3c50..7a9bb2d 100644 --- a/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc +++ b/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc @@ -176,6 +176,41 @@ PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture()); } +TEST_F(PictureInPictureWindowManagerTest, + ExitPictureInPictureSoonClosesWindowAsynchronously) { + PictureInPictureWindowManager* picture_in_picture_window_manager = + PictureInPictureWindowManager::GetInstance(); + MockPictureInPictureWindowController controller; + picture_in_picture_window_manager->EnterPictureInPictureWithController( + &controller); + EXPECT_TRUE(picture_in_picture_window_manager->IsInPictureInPicture()); + + // Window should not be closed synchronously. + EXPECT_CALL(controller, Close(/*should_pause_video=*/false)).Times(0); + PictureInPictureWindowManager::ExitPictureInPictureSoon(); + EXPECT_TRUE(picture_in_picture_window_manager->IsInPictureInPicture()); + + // Window should close asynchronously once the posted task runs. + base::RunLoop run_loop; + EXPECT_CALL(controller, Close(/*should_pause_video=*/false)) + .WillOnce([&run_loop]() { run_loop.Quit(); }); + run_loop.Run(); + EXPECT_FALSE(picture_in_picture_window_manager->IsInPictureInPicture()); +} + +TEST_F(PictureInPictureWindowManagerTest, + ExitPictureInPictureSoonDoesNotCrashWhenThereIsNoWindow) { + EXPECT_FALSE( + PictureInPictureWindowManager::GetInstance()->IsInPictureInPicture()); + PictureInPictureWindowManager::ExitPictureInPictureSoon(); + base::RunLoop run_loop; + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, run_loop.QuitClosure()); + run_loop.Run(); + EXPECT_FALSE( + PictureInPictureWindowManager::GetInstance()->IsInPictureInPicture()); +} + TEST_F(PictureInPictureWindowManagerTest, IsInPictureInPicture) { PictureInPictureWindowManager* picture_in_picture_window_manager = PictureInPictureWindowManager::GetInstance();
Regression Test / PoC
diff --git a/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc b/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc
index d1c3c50..7a9bb2d 100644
--- a/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc
+++ b/chrome/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc
@@ -176,6 +176,41 @@
PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture());
}
+TEST_F(PictureInPictureWindowManagerTest,
+ ExitPictureInPictureSoonClosesWindowAsynchronously) {
+ PictureInPictureWindowManager* picture_in_picture_window_manager =
+ PictureInPictureWindowManager::GetInstance();
+ MockPictureInPictureWindowController controller;
+ picture_in_picture_window_manager->EnterPictureInPictureWithController(
+ &controller);
+ EXPECT_TRUE(picture_in_picture_window_manager->IsInPictureInPicture());
+
+ // Window should not be closed synchronously.
+ EXPECT_CALL(controller, Close(/*should_pause_video=*/false)).Times(0);
+ PictureInPictureWindowManager::ExitPictureInPictureSoon();
+ EXPECT_TRUE(picture_in_picture_window_manager->IsInPictureInPicture());
+
+ // Window should close asynchronously once the posted task runs.
+ base::RunLoop run_loop;
+ EXPECT_CALL(controller, Close(/*should_pause_video=*/false))
+ .WillOnce([&run_loop]() { run_loop.Quit(); });
+ run_loop.Run();
+ EXPECT_FALSE(picture_in_picture_window_manager->IsInPictureInPicture());
+}
+
+TEST_F(PictureInPictureWindowManagerTest,
+ ExitPictureInPictureSoonDoesNotCrashWhenThereIsNoWindow) {
+ EXPECT_FALSE(
+ PictureInPictureWindowManager::GetInstance()->IsInPictureInPicture());
+ PictureInPictureWindowManager::ExitPictureInPictureSoon();
+ base::RunLoop run_loop;
+ base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
+ FROM_HERE, run_loop.QuitClosure());
+ run_loop.Run();
+ EXPECT_FALSE(
+ PictureInPictureWindowManager::GetInstance()->IsInPictureInPicture());
+}
+
TEST_F(PictureInPictureWindowManagerTest, IsInPictureInPicture) {
PictureInPictureWindowManager* picture_in_picture_window_manager =
PictureInPictureWindowManager::GetInstance();