fix: prevent crash on Windows when closing child windows (#49901)

* guard against window destruction in min/max size checks

* use weakptr to prevent hit test crash on teardown

* revove web contents views during teardown

* fix test failure

* fix other tests
This commit is contained in:
Mitchell Cohen
2026-02-24 09:48:04 -05:00
committed by GitHub
parent fc3a0abf19
commit acd01e15e2
5 changed files with 27 additions and 13 deletions

View File

@@ -33,7 +33,7 @@ WebContentsView::WebContentsView(v8::Isolate* isolate,
gin_helper::Handle<WebContents> web_contents)
: View(web_contents->inspectable_web_contents()->GetView()),
web_contents_(isolate, web_contents.ToV8()),
api_web_contents_(web_contents.get()) {
api_web_contents_(web_contents->GetWeakPtr()) {
set_delete_view(false);
view()->SetProperty(
views::kFlexBehaviorKey,

View File

@@ -7,7 +7,7 @@
#include <optional>
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "content/public/browser/web_contents_observer.h"
#include "shell/browser/api/electron_api_view.h"
#include "shell/browser/draggable_region_provider.h"
@@ -63,7 +63,7 @@ class WebContentsView : public View,
// Keep a reference to v8 wrapper.
v8::Global<v8::Value> web_contents_;
raw_ptr<api::WebContents> api_web_contents_;
base::WeakPtr<api::WebContents> api_web_contents_;
};
} // namespace electron::api

View File

@@ -92,6 +92,13 @@ InspectableWebContentsView::InspectableWebContentsView(
}
InspectableWebContentsView::~InspectableWebContentsView() {
if (devtools_window_web_view_)
devtools_window_web_view_->SetWebContents(nullptr);
if (devtools_web_view_)
devtools_web_view_->SetWebContents(nullptr);
if (contents_web_view_)
contents_web_view_->SetWebContents(nullptr);
if (devtools_window_)
inspectable_web_contents()->SaveDevToolsBounds(
devtools_window_->GetWindowBoundsInScreen());

View File

@@ -275,12 +275,16 @@ bool WinFrameView::GetShouldPaintAsActive() {
}
gfx::Size WinFrameView::GetMinimumSize() const {
if (!window_)
return gfx::Size();
// Chromium expects minimum size to be in content dimensions on Windows
// because it adds the frame border automatically in OnGetMinMaxInfo.
return window_->GetContentMinimumSize();
}
gfx::Size WinFrameView::GetMaximumSize() const {
if (!window_)
return gfx::Size();
// Chromium expects minimum size to be in content dimensions on Windows
// because it adds the frame border automatically in OnGetMinMaxInfo.
gfx::Size size = window_->GetContentMaximumSize();

View File

@@ -59,10 +59,11 @@ describe('WebContentsView', () => {
const browserWindow = new BrowserWindow();
const webContentsView = new WebContentsView();
webContentsView.webContents.loadURL('about:blank');
webContentsView.webContents.destroy();
const wc = webContentsView.webContents;
wc.loadURL('about:blank');
wc.destroy();
const destroyed = once(webContentsView.webContents, 'destroyed');
const destroyed = once(wc, 'destroyed');
await destroyed;
expect(() => browserWindow.contentView.addChildView(webContentsView)).to.throw(
'Can\'t add a destroyed child view to a parent view'
@@ -90,13 +91,14 @@ describe('WebContentsView', () => {
const w = new BaseWindow({ show: false });
const v = new View();
const wcv = new WebContentsView();
const wc = wcv.webContents;
w.setContentView(v);
v.addChildView(wcv);
await wcv.webContents.loadURL('about:blank');
const destroyed = once(wcv.webContents, 'destroyed');
wcv.webContents.executeJavaScript('window.close()');
await wc.loadURL('about:blank');
const destroyed = once(wc, 'destroyed');
wc.executeJavaScript('window.close()');
await destroyed;
expect(wcv.webContents.isDestroyed()).to.be.true();
expect(wc.isDestroyed()).to.be.true();
v.removeChildView(wcv);
});
@@ -170,18 +172,19 @@ describe('WebContentsView', () => {
it('does not crash when closed via window.close()', async () => {
const bw = new BrowserWindow();
const wcv = new WebContentsView();
const wc = wcv.webContents;
await bw.loadURL('data:text/html,<h1>Main Window</h1>');
bw.contentView.addChildView(wcv);
const dto = new Promise<boolean>((resolve) => {
wcv.webContents.on('blur', () => {
const devToolsOpen = wcv.webContents.isDevToolsOpened();
wc.on('blur', () => {
const devToolsOpen = !wc.isDestroyed() && wc.isDevToolsOpened();
resolve(devToolsOpen);
});
});
wcv.webContents.loadURL('data:text/html,<script>window.close()</script>');
wc.loadURL('data:text/html,<script>window.close()</script>');
const open = await dto;
expect(open).to.be.false();