A few small tweaks

1 - Embed Page into Session, avoids having to allocate/deallocate the page
2 - Fix false-positive arena pool leak detection
3 - On form submit, pre-check if navigation will allowed before building the
    contents
4 - Make sure QueuedNavigation structure isn't use after page is removed
This commit is contained in:
Karl Seguin
2026-02-03 13:26:48 +08:00
parent d46a9d6286
commit 5467b8dd0d
3 changed files with 43 additions and 35 deletions

View File

@@ -207,23 +207,23 @@ scheduler: Scheduler,
_req_id: ?usize = null, _req_id: ?usize = null,
_navigated_options: ?NavigatedOpts = null, _navigated_options: ?NavigatedOpts = null,
pub fn init(arena: Allocator, call_arena: Allocator, session: *Session) !*Page { pub fn init(self: *Page, session: *Session) !void {
if (comptime IS_DEBUG) { if (comptime IS_DEBUG) {
log.debug(.page, "page.init", .{}); log.debug(.page, "page.init", .{});
} }
const page = try session.browser.allocator.create(Page); const browser = session.browser;
page._session = session; self._session = session;
self.arena_pool = browser.arena_pool;
self.arena = browser.page_arena.allocator();
self.call_arena = browser.call_arena.allocator();
page.arena = arena;
page.call_arena = call_arena;
page.arena_pool = session.browser.arena_pool;
if (comptime IS_DEBUG) { if (comptime IS_DEBUG) {
page._arena_pool_leak_track = .empty; self._arena_pool_leak_track = .empty;
} }
try page.reset(true); try self.reset(true);
return page;
} }
pub fn deinit(self: *Page) void { pub fn deinit(self: *Page) void {
@@ -265,8 +265,6 @@ pub fn deinit(self: *Page) void {
} }
} }
} }
session.browser.allocator.destroy(self);
} }
fn reset(self: *Page, comptime initializing: bool) !void { fn reset(self: *Page, comptime initializing: bool) !void {
@@ -290,7 +288,9 @@ fn reset(self: *Page, comptime initializing: bool) !void {
if (comptime IS_DEBUG) { if (comptime IS_DEBUG) {
var it = self._arena_pool_leak_track.valueIterator(); var it = self._arena_pool_leak_track.valueIterator();
while (it.next()) |value_ptr| { while (it.next()) |value_ptr| {
log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.* }); if (value_ptr.count > 0) {
log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.owner });
}
} }
self._arena_pool_leak_track = .empty; self._arena_pool_leak_track = .empty;
} }
@@ -564,12 +564,6 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi
// specifically for this type of lifetime. // specifically for this type of lifetime.
pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void { pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void {
if (self.canScheduleNavigation(priority) == false) { if (self.canScheduleNavigation(priority) == false) {
if (comptime IS_DEBUG) {
log.debug(.browser, "ignored navigation", .{
.target = request_url,
.reason = opts.reason,
});
}
return; return;
} }
@@ -3353,6 +3347,11 @@ pub fn submitForm(self: *Page, submitter_: ?*Element, form_: ?*Element.Html.Form
return; return;
} }
} }
if (self.canScheduleNavigation(.form) == false) {
return;
}
const form_element = form.asElement(); const form_element = form.asElement();
const FormData = @import("webapi/net/FormData.zig"); const FormData = @import("webapi/net/FormData.zig");

View File

@@ -59,13 +59,14 @@ storage_shed: storage.Shed,
history: History, history: History,
navigation: Navigation, navigation: Navigation,
page: ?*Page = null, page: ?Page,
pub fn init(self: *Session, browser: *Browser) !void { pub fn init(self: *Session, browser: *Browser) !void {
const allocator = browser.app.allocator; const allocator = browser.app.allocator;
const session_allocator = browser.session_arena.allocator(); const session_allocator = browser.session_arena.allocator();
self.* = .{ self.* = .{
.page = null,
.history = .{}, .history = .{},
.navigation = .{}, .navigation = .{},
.storage_shed = .{}, .storage_shed = .{},
@@ -89,11 +90,11 @@ pub fn deinit(self: *Session) void {
pub fn createPage(self: *Session) !*Page { pub fn createPage(self: *Session) !*Page {
lp.assert(self.page == null, "Session.createPage - page not null", .{}); lp.assert(self.page == null, "Session.createPage - page not null", .{});
const page_arena = &self.browser.page_arena; _ = self.browser.page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 });
_ = page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 });
self.page = try Page.init(page_arena.allocator(), self.browser.call_arena.allocator(), self); self.page = @as(Page, undefined);
const page = self.page.?; const page = &self.page.?;
try Page.init(page, self);
// Creates a new NavigationEventTarget for this page. // Creates a new NavigationEventTarget for this page.
try self.navigation.onNewPage(page); try self.navigation.onNewPage(page);
@@ -124,7 +125,7 @@ pub fn removePage(self: *Session) void {
} }
pub fn currentPage(self: *Session) ?*Page { pub fn currentPage(self: *Session) ?*Page {
return self.page orelse return null; return &(self.page orelse return null);
} }
pub const WaitResult = enum { pub const WaitResult = enum {
@@ -136,7 +137,7 @@ pub const WaitResult = enum {
pub fn wait(self: *Session, wait_ms: u32) WaitResult { pub fn wait(self: *Session, wait_ms: u32) WaitResult {
while (true) { while (true) {
const page = self.page orelse return .no_page; const page = &(self.page orelse return .no_page);
switch (page.wait(wait_ms)) { switch (page.wait(wait_ms)) {
.navigate => self.processScheduledNavigation() catch return .done, .navigate => self.processScheduledNavigation() catch return .done,
else => |result| return result, else => |result| return result,
@@ -147,24 +148,32 @@ pub fn wait(self: *Session, wait_ms: u32) WaitResult {
} }
fn processScheduledNavigation(self: *Session) !void { fn processScheduledNavigation(self: *Session) !void {
const qn = self.page.?._queued_navigation.?; errdefer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 8 * 1024 });
defer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 8 * 1024 }); const url, const opts = blk: {
const qn = self.page.?._queued_navigation.?;
// qn might not be safe to use after self.removePage is called, hence
// this block;
const url = qn.url;
const opts = qn.opts;
// This was already aborted on the page, but it would be pretty // This was already aborted on the page, but it would be pretty
// bad if old requests went to the new page, so let's make double sure // bad if old requests went to the new page, so let's make double sure
self.browser.http_client.abort(); self.browser.http_client.abort();
self.removePage(); self.removePage();
break :blk .{ url, opts };
};
const page = self.createPage() catch |err| { const page = self.createPage() catch |err| {
log.err(.browser, "queued navigation page error", .{ log.err(.browser, "queued navigation page error", .{
.err = err, .err = err,
.url = qn.url, .url = url,
}); });
return err; return err;
}; };
page.navigate(qn.url, qn.opts) catch |err| { page.navigate(url, opts) catch |err| {
log.err(.browser, "queued navigation error", .{ .err = err, .url = qn.url }); log.err(.browser, "queued navigation error", .{ .err = err, .url = url });
return err; return err;
}; };
} }

View File

@@ -85,7 +85,7 @@ pub fn startSession(self: *Inspector, ctx: anytype) *Session {
std.debug.assert(self.session == null); std.debug.assert(self.session == null);
} }
self.session = undefined; self.session = @as(Session, undefined);
Session.init(&self.session.?, self, ctx); Session.init(&self.session.?, self, ctx);
return &self.session.?; return &self.session.?;
} }