From 2d3ce7f943c9e0fd8b67736f36e22657b3027839 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 3 Sep 2026 13:48:03 -0400 Subject: [PATCH] views: Preserve overlay contents during Chrome window teardown ChromeBrowserWidget closes owned widgets before the parent WindowClosing callback can detach overlay contents. Closing an overlay widget with CloseNow therefore deletes its contents View and invalidates the CefView retained by the client. Extract CefWindowView::CloseOverlayViews and call it from both WindowClosing and ChromeBrowserWidget::CloseOwnedWidgets. This detaches overlay contents and restores CEF ownership before owned widgets are destroyed. Direct overlay widget closure retains its destruction behavior, which Alloy overlay browsers need to complete browser shutdown. Add an explicit IsValid assertion to the overlay destruction tests. To test: ceftests --gtest_filter=ChromeChildWindowTest.*:OwnedWidgetShutdownTest.*:ViewsBrowserViewTest.* --- .../chrome/views/chrome_browser_widget.cc | 6 ++++++ libcef/browser/views/overlay_view_host.cc | 10 ++++------ libcef/browser/views/window_view.cc | 16 ++++++++++------ libcef/browser/views/window_view.h | 4 ++++ tests/ceftests/views/window_unittest.cc | 1 + 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/libcef/browser/chrome/views/chrome_browser_widget.cc b/libcef/browser/chrome/views/chrome_browser_widget.cc index cbefc6330..52676a4d4 100644 --- a/libcef/browser/chrome/views/chrome_browser_widget.cc +++ b/libcef/browser/chrome/views/chrome_browser_widget.cc @@ -219,6 +219,12 @@ void ChromeBrowserWidget::CloseOwnedWidgets() { return; } + // Preserve overlay contents before CloseNow bypasses the normal overlay + // cleanup in CefWindowView::WindowClosing. + if (window_view_) { + window_view_->CloseOverlayViews(); + } + views::Widget::ForEachOwnedWidget(GetNativeView(), [this](views::Widget* widget) { if (widget != this) { diff --git a/libcef/browser/views/overlay_view_host.cc b/libcef/browser/views/overlay_view_host.cc index a78ee0671..d2ad9eecf 100644 --- a/libcef/browser/views/overlay_view_host.cc +++ b/libcef/browser/views/overlay_view_host.cc @@ -350,12 +350,10 @@ gfx::Rect CefOverlayViewHost::ComputeBounds() const { void CefOverlayViewHost::Cleanup() { // This method may be called multiple times. For example, explicitly after the // client calls CefOverlayController::Destroy or implicitly when the host - // Widget is being closed or destroyed. In most implicit cases - // CefWindowView::WindowClosing will call this before the host Widget is - // destroyed, allowing the client to optionally reuse the child View. However, - // if CefWindowView::WindowClosing is not called, DeleteDelegate will call - // this after the host Widget and all associated Widgets/Views have been - // destroyed. In the DeleteDelegate case |widget_| will return nullptr. + // Widget is being closed or destroyed. CefWindowView::CloseOverlayViews calls + // this before the host Widget is destroyed, allowing the client to reuse the + // child View. If the overlay Widget is closed directly instead, its contents + // are destroyed and WidgetIsZombie calls this after resetting |widget_|. if (view_ && widget_) { // Remove the child View immediately. It may be reused by the client. auto view = view_util::GetFor(view_, /*find_known_parent=*/false); diff --git a/libcef/browser/views/window_view.cc b/libcef/browser/views/window_view.cc index 47ff9bf78..9c030f0db 100644 --- a/libcef/browser/views/window_view.cc +++ b/libcef/browser/views/window_view.cc @@ -763,12 +763,7 @@ bool CefWindowView::CanMaximize() const { void CefWindowView::WindowClosing() { // Close any overlays now, before the Widget is destroyed. - // Use a copy of the array because the original may be modified while - // iterating. - std::vector> overlay_hosts = overlay_hosts_; - for (auto& overlay_host : overlay_hosts) { - overlay_host->Close(); - } + CloseOverlayViews(); #if BUILDFLAG(IS_LINUX) #if BUILDFLAG(SUPPORTS_OZONE_X11) @@ -961,6 +956,15 @@ void CefWindowView::RemoveOverlayView(CefOverlayViewHost* host, overlay_hosts_.erase(it); } +void CefWindowView::CloseOverlayViews() { + // Use a copy of the array because the original may be modified while + // iterating. + std::vector> overlay_hosts = overlay_hosts_; + for (auto& overlay_host : overlay_hosts) { + overlay_host->Close(); + } +} + void CefWindowView::MoveOverlaysIfNecessary() { if (overlay_hosts_.empty()) { return; diff --git a/libcef/browser/views/window_view.h b/libcef/browser/views/window_view.h index 68dc821be..458ddcd48 100644 --- a/libcef/browser/views/window_view.h +++ b/libcef/browser/views/window_view.h @@ -108,6 +108,10 @@ class CefWindowView : public CefPanelView, // Called from CefOverlayViewHost::Cleanup(). void RemoveOverlayView(CefOverlayViewHost* host, views::View* host_view); + // Close overlays before owned Widgets are destroyed, preserving their + // contents Views for reuse by the client. + void CloseOverlayViews(); + // Set/get the draggable regions. void SetDraggableRegions(const std::vector& regions); SkRegion* draggable_region() const { return draggable_region_.get(); } diff --git a/tests/ceftests/views/window_unittest.cc b/tests/ceftests/views/window_unittest.cc index 6b20bcc6e..97b7769b2 100644 --- a/tests/ceftests/views/window_unittest.cc +++ b/tests/ceftests/views/window_unittest.cc @@ -739,6 +739,7 @@ class OverlayTestWindowDelegate : public TestWindowDelegate { // View is still visible but no longer drawn (because it no longer belongs // to the controller). + EXPECT_TRUE(view_->IsValid()); EXPECT_EQ(nullptr, view_->GetWindow()); EXPECT_TRUE(view_->IsVisible()); EXPECT_FALSE(view_->IsDrawn());