Skip to content

Commit 4644e55

Browse files
committed
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: <script>top.location = '/';</script> 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.
1 parent 747a8ad commit 4644e55

File tree

2 files changed

+12
-1
lines changed

2 files changed

+12
-1
lines changed

src/browser/page.zig

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ pub const Page = struct {
9292
// current_script could by fetch module to resolve module's url to fetch.
9393
current_script: ?*const Script = null,
9494

95+
// indicates intention to navigate to another page on the next loop execution.
96+
delayed_navigation: bool = false,
97+
9598
pub fn init(self: *Page, arena: Allocator, session: *Session) !void {
9699
const browser = session.browser;
97100
self.* = .{
@@ -580,6 +583,7 @@ pub const Page = struct {
580583
// The page.arena is safe to use here, but the transfer_arena exists
581584
// specifically for this type of lifetime.
582585
pub fn navigateFromWebAPI(self: *Page, url: []const u8, opts: NavigateOpts) !void {
586+
self.delayed_navigation = true;
583587
const arena = self.session.transfer_arena;
584588
const navi = try arena.create(DelayedNavigation);
585589
navi.* = .{

src/browser/session.zig

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,14 @@ pub const Session = struct {
128128
// it isn't null!
129129
std.debug.assert(self.page != null);
130130

131-
defer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 });
131+
defer if (self.page) |*p| {
132+
if (!p.delayed_navigation) {
133+
// If, while loading the page, we intend to navigate to another
134+
// page, then we need to keep the transfer_arena around, as this
135+
// sub-navigation is probably using it.
136+
_ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 });
137+
}
138+
};
132139

133140
// it's safe to use the transfer arena here, because the page will
134141
// eventually clone the URL using its own page_arena (after it gets

0 commit comments

Comments
 (0)