mirror/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-06-11 01:25:53 -04:00
Code Issues Packages Projects Releases Wiki Activity

Defer window.close()

Browse Source 
Cannot Frame.deinit in window.close() as that's happening inside JS runtime.
Instead, defer on Page.deinit. This is MUCH later than necessary, but I'd like
to address the timing separately. This commit, as-is, prevents real crashes.
This commit is contained in:
Karl Seguin
2026-04-30 18:04:56 +08:00
parent e42acc5335
commit f921869fb6
4 changed files with 55 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/browser/Page.zig
View File

@@ -96,6 +96,12 @@ frame: Frame,
// to the original page like this.
popups: std.ArrayList(*Frame) = .empty,
// Popups that have called window.close() but whose teardown is deferred to
// Page.deinit. We can't deinit synchronously from window.close() because
// that's invoked from JS still running on top of the Frame's V8 context (or
// from a script eval whose parser still holds the Frame).
queued_close: std.ArrayList(*Frame) = .empty,
// Initialize a Page and its root Frame.
pub fn init(self: *Page, session: *Session, frame_id: u32) !void {
const frame_arena = try session.arena_pool.acquire(.large, "Page.frame_arena");
@@ -115,6 +121,11 @@ pub fn init(self: *Page, session: *Session, frame_id: u32) !void {
// Tear down the Page and its root Frame. Equivalent to the old
// Session.removePage + Session.resetFrameResources.
pub fn deinit(self: *Page, abort_http: bool) void {
for (self.queued_close.items) |popup| {
popup.deinit(abort_http);
}
self.queued_close = .empty;
for (self.popups.items) |popup| {
popup.deinit(abort_http);
}

24
src/browser/tests/window/open.html
View File

@@ -107,3 +107,27 @@
testing.expectEqual(null, window.opener);
}
</script>
<script id=popup_self_close type=module>
{
// Popup navigates to a real URL (not about:blank, which takes a synchronous
// shortcut) and calls window.close() from inside its own running script.
// Regression: synchronous frame teardown here used to destroy the popup's
// V8 context mid-eval, panicking on the deferred load-event dispatch.
// Receiving the postMessage proves the popup's script ran to completion
// (including the close call) without crashing.
const state = await testing.async();
window.addEventListener('message', (e) => {
if (e.data && e.data.from === 'popup_self_close') {
state.resolve(e.data);
}
}, { once: true });
const w = window.open(testing.BASE_URL + 'window/support/popup_self_close.html');
testing.expectTrue(w != null);
await state.done((data) => {
testing.expectEqual('popup_self_close', data.from);
});
}
</script>

12
src/browser/tests/window/support/popup_self_close.html Normal file
View File

@@ -0,0 +1,12 @@
<!DOCTYPE html>
<script>
// Post a message back to the opener, then close ourselves from inside the
// popup's own running script. This exercises the deferred-close path:
// close() is invoked while the popup's V8 context is still entered and the
// parser is still on the stack. A synchronous teardown here would destroy
// the context and crash the eval's deferred load-event dispatch.
if (window.opener) {
window.opener.postMessage({ from: 'popup_self_close' }, '*');
}
window.close();
</script>

9
src/browser/webapi/Window.zig
View File

@@ -568,7 +568,14 @@ pub fn close(self: *Window) void {
}
}
frame.deinit(true);
// We can't tear the Frame down here — close() is invoked from JS still
// running on top of this Frame's V8 context, often deep inside a script
// eval whose parser is still holding the Frame. Destroying the context
// now leaves dangling pointers in the unwinding script eval (load event
// dispatch, runMacrotasks, etc.). Defer to Page.deinit instead.
page.queued_close.append(page.frame_arena, frame) catch |err| {
log.err(.frame, "queue popup close", .{ .err = err });
};
}
pub fn postMessage(self: *Window, message: js.Value.Temp, target_origin: ?[]const u8, frame: *Frame) !void {