Merge pull request #1458 from lightpanda-io/tweaks

A few small tweaks

src/browser/Page.zig

@@ -207,23 +207,23 @@ scheduler: Scheduler,
 _req_id: ?usize = 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) {
         log.debug(.page, "page.init", .{});
     }
 
-    const page = try session.browser.allocator.create(Page);
-    page._session = session;
+    const browser = session.browser;
+    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) {
-        page._arena_pool_leak_track = .empty;
+        self._arena_pool_leak_track = .empty;
     }
 
-    try page.reset(true);
-    return page;
+    try self.reset(true);
 }
 
 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 {
@@ -290,7 +288,9 @@ fn reset(self: *Page, comptime initializing: bool) !void {
         if (comptime IS_DEBUG) {
             var it = self._arena_pool_leak_track.valueIterator();
             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;
         }
@@ -564,12 +564,6 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi
 // specifically for this type of lifetime.
 pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void {
     if (self.canScheduleNavigation(priority) == false) {
-        if (comptime IS_DEBUG) {
-            log.debug(.browser, "ignored navigation", .{
-                .target = request_url,
-                .reason = opts.reason,
-            });
-        }
         return;
     }
 
@@ -3353,6 +3347,11 @@ pub fn submitForm(self: *Page, submitter_: ?*Element, form_: ?*Element.Html.Form
             return;
         }
     }
+
+    if (self.canScheduleNavigation(.form) == false) {
+        return;
+    }
+
     const form_element = form.asElement();
 
     const FormData = @import("webapi/net/FormData.zig");

src/browser/Session.zig

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

src/browser/js/Inspector.zig

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