From 4644e55883db619fa67a83c6d42a44f6b1a102e1 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Jun 2025 14:16:36 +0800 Subject: [PATCH] Do not reset transfer_arena if page navigation results in delayed navigation We normally expect a navigation event to happen at some point after the page loads, like a puppeteer script clicking on a link. But, it's also possible for the main navigation event to result in a delayed navigation. For example, an html page with this JS: Would result in a delayed navigation being called from the main navigate function. In these cases, we cannot clear the transfer_arena when navigate is completed, as its memory is needed by the new "sub" delayed navigation. --- src/browser/page.zig | 4 ++++ src/browser/session.zig | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/browser/page.zig b/src/browser/page.zig index ae313eaa..ebb90528 100644 --- a/src/browser/page.zig +++ b/src/browser/page.zig @@ -92,6 +92,9 @@ pub const Page = struct { // current_script could by fetch module to resolve module's url to fetch. current_script: ?*const Script = null, + // indicates intention to navigate to another page on the next loop execution. + delayed_navigation: bool = false, + pub fn init(self: *Page, arena: Allocator, session: *Session) !void { const browser = session.browser; self.* = .{ @@ -580,6 +583,7 @@ pub const Page = struct { // The page.arena is safe to use here, but the transfer_arena exists // specifically for this type of lifetime. pub fn navigateFromWebAPI(self: *Page, url: []const u8, opts: NavigateOpts) !void { + self.delayed_navigation = true; const arena = self.session.transfer_arena; const navi = try arena.create(DelayedNavigation); navi.* = .{ diff --git a/src/browser/session.zig b/src/browser/session.zig index d71bb27e..532a3907 100644 --- a/src/browser/session.zig +++ b/src/browser/session.zig @@ -128,7 +128,14 @@ pub const Session = struct { // it isn't null! std.debug.assert(self.page != null); - defer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); + defer if (self.page) |*p| { + if (!p.delayed_navigation) { + // If, while loading the page, we intend to navigate to another + // page, then we need to keep the transfer_arena around, as this + // sub-navigation is probably using it. + _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); + } + }; // it's safe to use the transfer arena here, because the page will // eventually clone the URL using its own page_arena (after it gets