public/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-03-22 04:34:44 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

Simplify navigation logic.

Browse Source 
Must of the complexity in the previous commit had to do with the fact that
about:blank is processed synchronously, meaning that we could process a
scheduled navigation -> page.navigate -> scheduled navigation:

```
let iframe = document.createElement('iframe');
iframe.addEventListner('load', () => {
  iframe.src = "about:blank";
});
```

This is an infinite loop which is going to be a problem no mater what, but there
are different degrees of problems this can cause, e.g. looping forever vs use-
after-free or other undefined behavior.

The new approach does 2 passes through scheduled navigations, first processing
"asynchronous" navigation (anything not "about:blank"), then processing
synchronous navigation ("about:blank"). The main advantage is that if the
synchronous navigation causes more synchronous navigation, it won't be
processed until the next tick. PLUS, we can detect about:blank that loads
about:blank and stop it (which might not be to spec, but seems right to do
nonetheless). This 2-pass approach removes the need for a couple of checks and
makes everything else simpler.
This commit is contained in:
Karl Seguin
2026-03-05 17:06:23 +08:00
parent 9c7ecf221e
commit 768c3a533b
2 changed files with 103 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/browser/Page.zig
View File

@@ -579,16 +579,17 @@ pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOp
} }
fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void { fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void {
const resolved_url = blk: { const resolved_url, const is_about_blank = blk: {
if (std.mem.eql(u8, request_url, "about:blank")) { if (std.mem.eql(u8, request_url, "about:blank")) {
break :blk "about:blank"; // navigate will handle this special case break :blk .{ "about:blank", true }; // navigate will handle this special case
} }
break :blk try URL.resolve( const u = try URL.resolve(
arena, arena,
self.base(), self.base(),
request_url, request_url,
.{ .always_dupe = true, .encode = true }, .{ .always_dupe = true, .encode = true },
); );
break :blk .{ u, false };
}; };
const session = self._session; const session = self._session;
@@ -629,11 +630,11 @@ fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []con
.arena = arena, .arena = arena,
.url = resolved_url, .url = resolved_url,
.priority = priority, .priority = priority,
.iframe = self.iframe, .is_about_blank = is_about_blank,
}; };
self._queued_navigation = qn; self._queued_navigation = qn;
return session.scheduleNavigation(qn); return session.scheduleNavigation(self);
} }
// A script can have multiple competing navigation events, say it starts off // A script can have multiple competing navigation events, say it starts off
@@ -3054,7 +3055,7 @@ pub const QueuedNavigation = struct {
url: [:0]const u8, url: [:0]const u8,
opts: NavigateOpts, opts: NavigateOpts,
priority: NavigationPriority, priority: NavigationPriority,
iframe: ?*Element.Html.IFrame, is_about_blank: bool,
}; };
pub fn triggerMouseClick(self: *Page, x: f64, y: f64) !void { pub fn triggerMouseClick(self: *Page, x: f64, y: f64) !void {

160
src/browser/Session.zig
View File

@@ -44,13 +44,11 @@ const Session = @This();
browser: *Browser, browser: *Browser,
notification: *Notification, notification: *Notification,
queued_navigation: std.ArrayList(*QueuedNavigation), queued_navigation: std.ArrayList(*Page),
// It's possible (but unlikely) that a queued navigation happens when we're // Temporary buffer for about:blank navigations during processing.
// processessing queued navigations (thank you WPT). This causes a lot of issues // We process async navigations first (safe from re-entrance), then sync
// including possibly invalidating `queued_navigation` and endless loops. // about:blank navigations (which may add to queued_navigation).
// We use a double queue to avoid this. queued_queued_navigation: std.ArrayList(*Page),
processing_queued_navigation: bool,
queued_queued_navigation: std.ArrayList(*QueuedNavigation),
// Used to create our Inspector and in the BrowserContext. // Used to create our Inspector and in the BrowserContext.
arena: Allocator, arena: Allocator,
@@ -81,7 +79,6 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi
.browser = browser, .browser = browser,
.queued_navigation = .{}, .queued_navigation = .{},
.queued_queued_navigation = .{}, .queued_queued_navigation = .{},
.processing_queued_navigation = false,
.notification = notification, .notification = notification,
.cookie_jar = storage.Cookie.Jar.init(allocator), .cookie_jar = storage.Cookie.Jar.init(allocator),
}; };
@@ -352,28 +349,22 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult {
} }
} }
pub fn scheduleNavigation(self: *Session, qn: *QueuedNavigation) !void { pub fn scheduleNavigation(self: *Session, page: *Page) !void {
const iframe = qn.iframe; const list = &self.queued_navigation;
const list = if (self.processing_queued_navigation) &self.queued_queued_navigation else &self.queued_navigation;
for (list.items, 0..) |existing, i| { // Check if page is already queued
if (existing.iframe == iframe) { for (list.items) |existing| {
self.browser.arena_pool.release(existing.arena); if (existing == page) {
list.items[i] = qn; // Already queued
return; return;
} }
} else {
return list.append(self.arena, qn);
} }
return list.append(self.arena, page);
} }
fn processQueuedNavigation(self: *Session) !void { fn processQueuedNavigation(self: *Session) !void {
const navigations = &self.queued_navigation; const navigations = &self.queued_navigation;
defer {
navigations.clearRetainingCapacity();
const copy = navigations.*;
self.queued_navigation = self.queued_queued_navigation;
self.queued_queued_navigation = copy;
}
if (self.page.?._queued_navigation != null) { if (self.page.?._queued_navigation != null) {
// This is both an optimization and a simplification of sorts. If the // This is both an optimization and a simplification of sorts. If the
@@ -381,55 +372,96 @@ fn processQueuedNavigation(self: *Session) !void {
// navigation. Also, the navigation for the root page and for a frame // navigation. Also, the navigation for the root page and for a frame
// is different enough that have two distinct code blocks is, imo, // is different enough that have two distinct code blocks is, imo,
// better. Yes, there will be duplication. // better. Yes, there will be duplication.
navigations.clearRetainingCapacity();
return self.processRootQueuedNavigation(); return self.processRootQueuedNavigation();
} }
self.processing_queued_navigation = true;
defer self.processing_queued_navigation = false; const about_blank_queue = &self.queued_queued_navigation;
defer about_blank_queue.clearRetainingCapacity();
// First pass: process async navigations (non-about:blank)
// These cannot cause re-entrant navigation scheduling
for (navigations.items) |page| {
const qn = page._queued_navigation.?;
if (qn.is_about_blank) {
// Defer about:blank to second pass
try about_blank_queue.append(self.arena, page);
continue;
}
try self.processFrameNavigation(page, qn);
}
// Clear the queue after first pass
navigations.clearRetainingCapacity();
// Second pass: process synchronous navigations (about:blank)
// These may trigger new navigations which go into queued_navigation
for (about_blank_queue.items) |page| {
const qn = page._queued_navigation.?;
try self.processFrameNavigation(page, qn);
}
// Safety: Remove any about:blank navigations that were queued during the
// second pass to prevent infinite loops
var i: usize = 0;
while (i < navigations.items.len) {
const page = navigations.items[i];
if (page._queued_navigation) |qn| {
if (qn.is_about_blank) {
log.warn(.page, "recursive about blank", .{});
_ = navigations.swapRemove(i);
continue;
}
}
i += 1;
}
}
fn processFrameNavigation(self: *Session, current_page: *Page, qn: *QueuedNavigation) !void {
lp.assert(current_page.parent != null, "root queued navigation", .{});
const browser = self.browser; const browser = self.browser;
for (navigations.items) |qn| { const iframe = current_page.iframe.?;
const iframe = qn.iframe.?; const parent = current_page.parent.?;
const current_page = iframe._content_window.?._page; // Get the CURRENT page from iframe
lp.assert(current_page.parent != null, "root queued navigation", .{});
current_page._queued_navigation = null; current_page._queued_navigation = null;
defer browser.arena_pool.release(qn.arena); defer browser.arena_pool.release(qn.arena);
const parent = current_page.parent.?; errdefer iframe._content_window = null;
errdefer iframe._content_window = null;
if (current_page._parent_notified) { if (current_page._parent_notified) {
// we already notified the parent that we had loaded // we already notified the parent that we had loaded
parent._pending_loads += 1; parent._pending_loads += 1;
} }
const frame_id = current_page._frame_id; const frame_id = current_page._frame_id;
defer current_page.deinit(true); defer current_page.deinit(true);
const new_page = try parent.arena.create(Page); const new_page = try parent.arena.create(Page);
try Page.init(new_page, frame_id, self, parent); try Page.init(new_page, frame_id, self, parent);
errdefer new_page.deinit(true); errdefer new_page.deinit(true);
new_page.iframe = iframe; new_page.iframe = iframe;
iframe._content_window = new_page.window; iframe._content_window = new_page.window;
new_page.navigate(qn.url, qn.opts) catch |err| { new_page.navigate(qn.url, qn.opts) catch |err| {
log.err(.browser, "queued frame navigation error", .{ .err = err }); log.err(.browser, "queued frame navigation error", .{ .err = err });
return err; return err;
}; };
for (parent.frames.items, 0..) |p, i| { for (parent.frames.items, 0..) |p, i| {
// Page.frames may or may not be sorted (depending on the // Page.frames may or may not be sorted (depending on the
// Page.frames_sorted flag). Putting this new page at the same // Page.frames_sorted flag). Putting this new page at the same
// position as the one it's replacing is the simplest, safest and // position as the one it's replacing is the simplest, safest and
// probably most efficient option. // probably most efficient option.
if (p == current_page) { if (p == current_page) {
parent.frames.items[i] = new_page; parent.frames.items[i] = new_page;
break; break;
}
} else {
lp.assert(false, "Existing frame not found", .{ .len = parent.frames.items.len });
} }
} else {
lp.assert(false, "Existing frame not found", .{ .len = parent.frames.items.len });
} }
} }