diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 5c2720df..a7a40e84 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -323,9 +323,9 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void } } -pub fn deinit(self: *Page) void { +pub fn deinit(self: *Page, abort_http: bool) void { for (self.frames.items) |frame| { - frame.deinit(); + frame.deinit(abort_http); } if (comptime IS_DEBUG) { @@ -346,10 +346,16 @@ pub fn deinit(self: *Page) void { session.browser.env.destroyContext(self.js); self._script_manager.shutdown = true; + if (self.parent == null) { - // only the root frame needs to abort this. It's more efficient this way session.browser.http_client.abort(); + } else if (abort_http) { + // a small optimization, it's faster to abort _everything_ on the root + // page, so we prefer that. But if it's just the frame that's going + // away (a frame navigation) then we'll abort the frame-related requests + session.browser.http_client.abortFrame(self._frame_id); } + self._script_manager.deinit(); if (comptime IS_DEBUG) { @@ -465,6 +471,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi // if the url is about:blank, we load an empty HTML document in the // page and dispatch the events. if (std.mem.eql(u8, "about:blank", request_url)) { + self.url = "about:blank"; // Assume we parsed the document. // It's important to force a reset during the following navigation. self._parse_state = .complete; @@ -578,31 +585,50 @@ 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 { - const resolved_url = try URL.resolve( - arena, - self.base(), - request_url, - .{ .always_dupe = true, .encode = true }, - ); + const resolved_url, const is_about_blank = blk: { + if (std.mem.eql(u8, request_url, "about:blank")) { + break :blk .{ "about:blank", true }; // navigate will handle this special case + } + const u = try URL.resolve( + arena, + self.base(), + request_url, + .{ .always_dupe = true, .encode = true }, + ); + break :blk .{ u, false }; + }; const session = self._session; if (!opts.force and URL.eqlDocument(self.url, resolved_url)) { - self.arena_pool.release(arena); - self.url = try self.arena.dupeZ(u8, resolved_url); self.window._location = try Location.init(self.url, self); self.document._location = self.window._location; - return session.navigation.updateEntries(self.url, opts.kind, self, true); + if (self.parent == null) { + try session.navigation.updateEntries(self.url, opts.kind, self, true); + } + // doin't defer this, the caller, the caller is responsible for freeing + // it on error + self.arena_pool.release(arena); + return; } log.info(.browser, "schedule navigation", .{ .url = resolved_url, .reason = opts.reason, - .target = resolved_url, .type = self._type, }); - session.browser.http_client.abort(); + // This is a micro-optimization. Terminate any inflight request as early + // as we can. This will be more propery shutdown when we process the + // scheduled navigation. + if (self.parent == null) { + session.browser.http_client.abort(); + } else { + // This doesn't terminate any inflight requests for nested frames, but + // again, this is just an optimization. We'll correctly shut down all + // nested inflight requests when we process the navigation. + session.browser.http_client.abortFrame(self._frame_id); + } const qn = try arena.create(QueuedNavigation); qn.* = .{ @@ -610,12 +636,11 @@ fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []con .arena = arena, .url = resolved_url, .priority = priority, + .is_about_blank = is_about_blank, }; - if (self._queued_navigation) |existing| { - self.arena_pool.release(existing.arena); - } self._queued_navigation = qn; + return session.scheduleNavigation(self); } // A script can have multiple competing navigation events, say it starts off @@ -629,6 +654,12 @@ fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []con // 3 - anchor clicks // Within, each category, it's last-one-wins. fn canScheduleNavigation(self: *Page, priority: NavigationPriority) bool { + if (self.parent) |parent| { + if (parent.isGoingAway()) { + return false; + } + } + const existing = self._queued_navigation orelse return true; if (existing.priority == priority) { @@ -637,7 +668,8 @@ fn canScheduleNavigation(self: *Page, priority: NavigationPriority) bool { } return switch (existing.priority) { - .anchor => true, // everything is higher priority than an anchor + .iframe => true, // everything is higher priority than iframe.src = "x" + .anchor => priority != .iframe, // an anchor is only higher priority than an iframe .form => false, // nothing is higher priority than a form .script => priority == .form, // a form is higher priority than a script }; @@ -764,6 +796,8 @@ fn _documentIsComplete(self: *Page) !void { } fn notifyParentLoadComplete(self: *Page) void { + const parent = self.parent orelse return; + if (self._parent_notified == true) { if (comptime IS_DEBUG) { std.debug.assert(false); @@ -773,9 +807,7 @@ fn notifyParentLoadComplete(self: *Page) void { } self._parent_notified = true; - if (self.parent) |p| { - p.iframeCompletedLoading(self.iframe.?); - } + parent.iframeCompletedLoading(self.iframe.?); } fn pageHeaderDoneCallback(transfer: *Http.Transfer) !bool { @@ -955,7 +987,11 @@ fn pageErrorCallback(ctx: *anyopaque, err: anyerror) void { } pub fn isGoingAway(self: *const Page) bool { - return self._queued_navigation != null; + if (self._queued_navigation != null) { + return true; + } + const parent = self.parent orelse return false; + return parent.isGoingAway(); } pub fn scriptAddedCallback(self: *Page, comptime from_parser: bool, script: *Element.Html.Script) !void { @@ -988,30 +1024,39 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void { return; } + if (iframe._content_window) |cw| { + // This frame is being re-navigated. We need to do this through a + // scheduleNavigation phase. We can't navigate immediately here, for + // the same reason that a "root" page can't immediately navigate: + // we could be in the middle of a JS callback or something else that + // doesn't exit the page to just suddenly go away. + return cw._page.scheduleNavigation(src, .{ + .reason = .script, + .kind = .{ .push = null }, + }, .iframe); + } + iframe._executed = true; const session = self._session; - // A frame can be re-navigated by setting the src. - const existing_window = iframe._content_window; - const page_frame = try self.arena.create(Page); - const frame_id = blk: { - if (existing_window) |w| { - const existing_frame_id = w._page._frame_id; - session.browser.http_client.abortFrame(existing_frame_id); - break :blk existing_frame_id; - } - break :blk session.nextFrameId(); - }; + const frame_id = session.nextFrameId(); try Page.init(page_frame, frame_id, session, self); - errdefer page_frame.deinit(); + errdefer page_frame.deinit(true); self._pending_loads += 1; page_frame.iframe = iframe; iframe._content_window = page_frame.window; errdefer iframe._content_window = null; + // on first load, dispatch frame_created evnet + self._session.notification.dispatch(.page_frame_created, &.{ + .frame_id = frame_id, + .parent_id = self._frame_id, + .timestamp = timestamp(.monotonic), + }); + const url = blk: { if (std.mem.eql(u8, src, "about:blank")) { break :blk "about:blank"; // navigate will handle this special case @@ -1024,42 +1069,14 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void { ); }; - if (existing_window == null) { - // on first load, dispatch frame_created evnet - self._session.notification.dispatch(.page_frame_created, &.{ - .frame_id = frame_id, - .parent_id = self._frame_id, - .timestamp = timestamp(.monotonic), - }); - } - page_frame.navigate(url, .{ .reason = .initialFrameNavigation }) catch |err| { log.warn(.page, "iframe navigate failure", .{ .url = url, .err = err }); self._pending_loads -= 1; iframe._content_window = null; - page_frame.deinit(); + page_frame.deinit(true); return error.IFrameLoadError; }; - if (existing_window) |w| { - const existing_page = w._page; - if (existing_page._parent_notified == false) { - self._pending_loads -= 1; - } - - for (self.frames.items, 0..) |p, i| { - if (p == existing_page) { - self.frames.items[i] = page_frame; - break; - } - } else { - lp.assert(false, "Existing frame not found", .{ .len = self.frames.items.len }); - } - - existing_page.deinit(); - return; - } - // window[N] is based on document order. For now we'll just append the frame // at the end of our list and set frames_sorted == false. window.getFrame // will check this flag to decide if it needs to sort the frames or not. @@ -3036,6 +3053,7 @@ const NavigationPriority = enum { form, script, anchor, + iframe, }; pub const QueuedNavigation = struct { @@ -3043,6 +3061,7 @@ pub const QueuedNavigation = struct { url: [:0]const u8, opts: NavigateOpts, priority: NavigationPriority, + is_about_blank: bool, }; pub fn triggerMouseClick(self: *Page, x: f64, y: f64) !void { diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 66bb0cd0..7bcfba3f 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -30,6 +30,7 @@ const History = @import("webapi/History.zig"); const Page = @import("Page.zig"); const Browser = @import("Browser.zig"); const Notification = @import("../Notification.zig"); +const QueuedNavigation = Page.QueuedNavigation; const Allocator = std.mem.Allocator; const IS_DEBUG = builtin.mode == .Debug; @@ -43,6 +44,12 @@ const Session = @This(); browser: *Browser, notification: *Notification, +queued_navigation: std.ArrayList(*Page), +// Temporary buffer for about:blank navigations during processing. +// We process async navigations first (safe from re-entrance), then sync +// about:blank navigations (which may add to queued_navigation). +queued_queued_navigation: std.ArrayList(*Page), + // Used to create our Inspector and in the BrowserContext. arena: Allocator, @@ -70,6 +77,8 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi .navigation = .{ ._proto = undefined }, .storage_shed = .{}, .browser = browser, + .queued_navigation = .{}, + .queued_queued_navigation = .{}, .notification = notification, .cookie_jar = storage.Cookie.Jar.init(allocator), }; @@ -79,9 +88,9 @@ pub fn deinit(self: *Session) void { if (self.page != null) { self.removePage(); } - const browser = self.browser; - self.cookie_jar.deinit(); + + const browser = self.browser; self.storage_shed.deinit(browser.app.allocator); browser.arena_pool.release(self.arena); } @@ -113,7 +122,7 @@ pub fn removePage(self: *Session) void { self.notification.dispatch(.page_remove, .{}); lp.assert(self.page != null, "Session.removePage - page is null", .{}); - self.page.?.deinit(); + self.page.?.deinit(false); self.page = null; self.navigation.onRemovePage(); @@ -133,7 +142,7 @@ pub fn replacePage(self: *Session) !*Page { var current = self.page.?; const frame_id = current._frame_id; const parent = current.parent; - current.deinit(); + current.deinit(false); self.browser.env.memoryPressureNotification(.moderate); @@ -174,10 +183,11 @@ pub fn wait(self: *Session, wait_ms: u32) WaitResult { switch (wait_result) { .done => { - if (page._queued_navigation == null) { + if (self.queued_navigation.items.len == 0) { return .done; } - page = self.processScheduledNavigation(page) catch return .done; + self.processQueuedNavigation() catch return .done; + page = &self.page.?; // might have changed }, else => |result| return result, } @@ -229,7 +239,7 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { } }, .html, .complete => { - if (page._queued_navigation != null) { + if (self.queued_navigation.items.len != 0) { return .done; } @@ -345,42 +355,147 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { } } -fn processScheduledNavigation(self: *Session, current_page: *Page) !*Page { - const browser = self.browser; +pub fn scheduleNavigation(self: *Session, page: *Page) !void { + const list = &self.queued_navigation; + + // Check if page is already queued + for (list.items) |existing| { + if (existing == page) { + // Already queued + return; + } + } + + return list.append(self.arena, page); +} + +fn processQueuedNavigation(self: *Session) !void { + const navigations = &self.queued_navigation; + + if (self.page.?._queued_navigation != null) { + // This is both an optimization and a simplification of sorts. If the + // root page is navigating, then we don't need to process any other + // navigation. Also, the navigation for the root page and for a frame + // is different enough that have two distinct code blocks is, imo, + // better. Yes, there will be duplication. + navigations.clearRetainingCapacity(); + return self.processRootQueuedNavigation(); + } + + 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 iframe = current_page.iframe.?; + const parent = current_page.parent.?; - const qn = current_page._queued_navigation.?; - // take ownership of the page's queued navigation current_page._queued_navigation = null; defer browser.arena_pool.release(qn.arena); - const frame_id, const parent = blk: { - const page = &self.page.?; - const frame_id = page._frame_id; - const parent = page.parent; + errdefer iframe._content_window = null; - browser.http_client.abort(); - self.removePage(); + if (current_page._parent_notified) { + // we already notified the parent that we had loaded + parent._pending_loads += 1; + } - break :blk .{ frame_id, parent }; - }; + const frame_id = current_page._frame_id; + defer current_page.deinit(true); - self.page = @as(Page, undefined); - const page = &self.page.?; - try Page.init(page, frame_id, self, parent); + const new_page = try parent.arena.create(Page); + try Page.init(new_page, frame_id, self, parent); + errdefer new_page.deinit(true); - // Creates a new NavigationEventTarget for this page. - try self.navigation.onNewPage(page); + new_page.iframe = iframe; + iframe._content_window = new_page.window; - // start JS env - // Inform CDP the main page has been created such that additional context for other Worlds can be created as well - self.notification.dispatch(.page_created, page); - - page.navigate(qn.url, qn.opts) catch |err| { - log.err(.browser, "queued navigation error", .{ .err = err, .url = qn.url }); + new_page.navigate(qn.url, qn.opts) catch |err| { + log.err(.browser, "queued frame navigation error", .{ .err = err }); return err; }; - return page; + for (parent.frames.items, 0..) |p, i| { + // Page.frames may or may not be sorted (depending on the + // Page.frames_sorted flag). Putting this new page at the same + // position as the one it's replacing is the simplest, safest and + // probably most efficient option. + if (p == current_page) { + parent.frames.items[i] = new_page; + break; + } + } else { + lp.assert(false, "Existing frame not found", .{ .len = parent.frames.items.len }); + } +} + +fn processRootQueuedNavigation(self: *Session) !void { + const current_page = &self.page.?; + const frame_id = current_page._frame_id; + + // create a copy before the page is cleared + const qn = current_page._queued_navigation.?; + current_page._queued_navigation = null; + defer self.browser.arena_pool.release(qn.arena); + + self.removePage(); + self.page = @as(Page, undefined); + const new_page = &self.page.?; + try Page.init(new_page, frame_id, self, null); + + // Creates a new NavigationEventTarget for this page. + try self.navigation.onNewPage(new_page); + + // start JS env + // Inform CDP the main page has been created such that additional context for other Worlds can be created as well + self.notification.dispatch(.page_created, new_page); + + new_page.navigate(qn.url, qn.opts) catch |err| { + log.err(.browser, "queued navigation error", .{ .err = err }); + return err; + }; } pub fn nextFrameId(self: *Session) u32 { diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index e9bccb61..01243d35 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -160,8 +160,8 @@ fn _tryCallWithThis(self: *const Function, comptime T: type, this: anytype, args try_catch.rethrow(); return error.TryCatchRethrow; } - caught.* = try_catch.caughtOrError(local.call_arena, error.JSExecCallback); - return error.JSExecCallback; + caught.* = try_catch.caughtOrError(local.call_arena, error.JsException); + return error.JsException; }; if (@typeInfo(T) == .void) { diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 04a65bc7..6a68b332 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -137,7 +137,7 @@ pub fn compileAndRun(self: *const Local, src: []const u8, name: ?[]const u8) !js ) orelse return error.CompilationError; // Run the script - const result = v8.v8__Script__Run(v8_script, self.handle) orelse return error.ExecutionError; + const result = v8.v8__Script__Run(v8_script, self.handle) orelse return error.JsException; return .{ .local = self, .handle = result }; } diff --git a/src/browser/tests/frames/frames.html b/src/browser/tests/frames/frames.html index f1519323..31e53be8 100644 --- a/src/browser/tests/frames/frames.html +++ b/src/browser/tests/frames/frames.html @@ -62,6 +62,7 @@ { let f3_load_event = false; let f3 = document.createElement('iframe'); + f3.id = 'f3'; f3.addEventListener('load', () => { f3_load_event = true; }); @@ -75,9 +76,10 @@ } - + + + + + diff --git a/src/browser/tests/frames/support/after_link.html b/src/browser/tests/frames/support/after_link.html new file mode 100644 index 00000000..0cd5fc99 --- /dev/null +++ b/src/browser/tests/frames/support/after_link.html @@ -0,0 +1,2 @@ + +It was clicked! diff --git a/src/browser/tests/frames/support/with_link.html b/src/browser/tests/frames/support/with_link.html new file mode 100644 index 00000000..2fd9ca77 --- /dev/null +++ b/src/browser/tests/frames/support/with_link.html @@ -0,0 +1,2 @@ + +a link diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index e93cc1f7..15541491 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -724,6 +724,9 @@ const CloneError = error{ TooManyContexts, LinkLoadError, StyleLoadError, + TypeError, + CompilationError, + JsException, }; pub fn cloneNode(self: *Node, deep_: ?bool, page: *Page) CloneError!*Node { const deep = deep_ orelse false;