Page.scheduleNavigation for location changes

src/Server.zig

@@ -173,6 +173,7 @@ fn readLoop(self: *Server, socket: posix.socket_t, timeout_ms: u32) !void {
                 }
                 ms_remaining -= @intCast(elapsed);
             },
+            .navigate => unreachable, // must have been handled by the session
         }
     }
 }

src/browser/Page.zig

@@ -136,6 +136,10 @@ _parse_state: ParseState,
 _notified_network_idle: IdleNotification = .init,
 _notified_network_almost_idle: IdleNotification = .init,
 
+// A navigation event that happens from a script gets scheduled to run on the
+// next tick.
+_queued_navigation: ?QueuedNavigation = null,
+
 // The URL of the current page
 url: [:0]const u8,
 
@@ -233,6 +237,7 @@ fn reset(self: *Page, comptime initializing: bool) !void {
 
     self._parse_state = .pre;
     self._load_state = .parsing;
+    self._queued_navigation = null;
     self._attribute_lookup = .empty;
     self._attribute_named_node_map_lookup = .empty;
     self._event_manager = EventManager.init(self);
@@ -304,11 +309,11 @@ pub fn isSameOrigin(self: *const Page, url: [:0]const u8) !bool {
     return std.mem.startsWith(u8, url, current_origin);
 }
 
-pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts, kind: NavigationKind) !void {
+pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !void {
     const session = self._session;
 
     const resolved_url = try URL.resolve(
-        session.transfer_arena,
+        self.arena,
         self.url,
         request_url,
         .{ .always_dupe = true },
@@ -316,19 +321,13 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts, kind
 
     // setting opts.force = true will force a page load.
     // otherwise, we will need to ensure this is a true (not document) navigation.
-    if (!opts.force) {
-        // If we are navigating within the same document, just change URL.
-        if (URL.eqlDocument(self.url, resolved_url)) {
-            // update page url
-            self.url = resolved_url;
-
-            // update location
-            self.window._location = try Location.init(self.url, self);
-            self.document._location = self.window._location;
-
-            try session.navigation.updateEntries(resolved_url, kind, self, true);
-            return;
-        }
+    if (!opts.force and URL.eqlDocument(self.url, resolved_url)) {
+        // update page url
+        self.url = resolved_url;
+        self.window._location = try Location.init(self.url, self);
+        self.document._location = self.window._location;
+        try session.navigation.updateEntries(resolved_url, opts.kind, self, true);
+        return;
     }
 
     if (self._parse_state != .pre) {
@@ -406,7 +405,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts, kind
         .timestamp = timestamp(.monotonic),
     });
 
-    session.navigation._current_navigation_kind = kind;
+    session.navigation._current_navigation_kind = opts.kind;
 
     http_client.request(.{
         .ctx = self,
@@ -426,6 +425,79 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts, kind
     };
 }
 
+// We cannot navigate immediately as navigating will delete the DOM tree,
+// which holds this event's node.
+// As such we schedule the function to be called as soon as possible.
+// The page.arena is safe to use here, but the transfer_arena exists
+// 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;
+    }
+
+    const session = self._session;
+    const URLRaw = @import("URL.zig");
+
+    const resolved_url = try URL.resolve(
+        session.transfer_arena,
+        self.url,
+        request_url,
+        .{ .always_dupe = true },
+    );
+
+    if (!opts.force and URLRaw.eqlDocument(self.url, resolved_url)) {
+        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);
+    }
+
+    log.info(.browser, "schedule navigation", .{
+        .url = resolved_url,
+        .reason = opts.reason,
+        .target = resolved_url,
+    });
+
+    self._session.browser.http_client.abort();
+
+    self._queued_navigation = .{
+        .opts = opts,
+        .url = resolved_url,
+        .priority = priority,
+    };
+}
+
+// A script can have multiple competing navigation events, say it starts off
+// by doing top.location = 'x' and then does a form submission.
+// You might think that we just stop at the first one, but that doesn't seem
+// to be what browsers do, and it isn't particularly well supported by v8 (i.e.
+// halting execution mid-script).
+// From what I can tell, there are 3 "levels" of priority, in order:
+// 1 - form submission
+// 2 - JavaScript apis (e.g. top.location)
+// 3 - anchor clicks
+// Within, each category, it's last-one-wins.
+fn canScheduleNavigation(self: *Page, priority: NavigationPriority) bool {
+    const existing = self._queued_navigation orelse return true;
+
+    if (existing.priority == priority) {
+        // same reason, than this latest one wins
+        return true;
+    }
+
+    return switch (existing.priority) {
+        .anchor => true, // everything is higher priority than an anchor
+        .form => false, // nothing is higher priority than a form
+        .script => priority == .form, // a form is higher priority than a script
+    };
+}
+
 pub fn documentIsLoaded(self: *Page) void {
     if (self._load_state != .parsing) {
         // Ideally, documentIsLoaded would only be called once, but if a
@@ -707,8 +779,6 @@ fn _wait(self: *Page, wait_ms: u32) !Session.WaitResult {
                     // haven't started navigating, I guess.
                     return .done;
                 }
-                self.js.runMicrotasks();
-
                 // Either we have active http connections, or we're in CDP
                 // mode with an extra socket. Either way, we're waiting
                 // for http traffic
@@ -724,6 +794,10 @@ fn _wait(self: *Page, wait_ms: u32) !Session.WaitResult {
                 }
             },
             .html, .complete => {
+                if (self._queued_navigation != null) {
+                    return .navigate;
+                }
+
                 // The HTML page was parsed. We now either have JS scripts to
                 // download, or scheduled tasks to execute, or both.
 
@@ -895,7 +969,16 @@ pub fn tick(self: *Page) void {
     self.js.runMicrotasks();
 }
 
+pub fn isGoingAway(self: *const Page) bool {
+    return self._queued_navigation != null;
+}
+
 pub fn scriptAddedCallback(self: *Page, comptime from_parser: bool, script: *Element.Html.Script) !void {
+    if (self.isGoingAway()) {
+        // if we're planning on navigating to another page, don't run this script
+        return;
+    }
+
     self._script_manager.addFromElement(from_parser, script, "parsing") catch |err| {
         log.err(.page, "page.scriptAddedCallback", .{
             .err = err,
@@ -2309,6 +2392,7 @@ pub const NavigateOpts = struct {
     body: ?[]const u8 = null,
     header: ?[:0]const u8 = null,
     force: bool = false,
+    kind: NavigationKind = .{ .push = null },
 };
 
 pub const NavigatedOpts = struct {
@@ -2317,6 +2401,18 @@ pub const NavigatedOpts = struct {
     method: Http.Method = .GET,
 };
 
+const NavigationPriority = enum {
+    form,
+    script,
+    anchor,
+};
+
+const QueuedNavigation = struct {
+    url: [:0]const u8,
+    opts: NavigateOpts,
+    priority: NavigationPriority,
+};
+
 const RequestCookieOpts = struct {
     is_http: bool = true,
     is_navigation: bool = false,

src/browser/ScriptManager.zig

@@ -734,6 +734,11 @@ pub const Script = struct {
         // never evaluated, source is passed back to v8 when asked for it.
         std.debug.assert(self.mode != .import);
 
+        if (page.isGoingAway()) {
+            // don't evaluate scripts for a dying page.
+            return;
+        }
+
         const script_element = self.script_element.?;
 
         const previous_script = page.document._current_script;

src/browser/Session.zig

@@ -62,12 +62,6 @@ navigation: Navigation,
 
 page: ?*Page = null,
 
-// If the current page want to navigate to a new page
-//  (form submit, link click, top.location = xxx)
-// the details are stored here so that, on the next call to session.wait
-// we can destroy the current page and start a new one.
-queued_navigation: ?QueuedNavigation,
-
 pub fn init(self: *Session, browser: *Browser) !void {
     var executor = try browser.env.newExecutionWorld();
     errdefer executor.deinit();
@@ -79,7 +73,6 @@ pub fn init(self: *Session, browser: *Browser) !void {
         .browser = browser,
         .executor = executor,
         .storage_shed = .{},
-        .queued_navigation = null,
         .arena = session_allocator,
         .cookie_jar = storage.Cookie.Jar.init(allocator),
         .navigation = Navigation.init(session_allocator),
@@ -145,48 +138,29 @@ pub const WaitResult = enum {
     done,
     no_page,
     extra_socket,
+    navigate,
 };
 
 pub fn wait(self: *Session, wait_ms: u32) WaitResult {
-    _ = self.processQueuedNavigation() catch {
-        // There was an error processing the queue navigation. This already
-        // logged the error, just return.
-        return .done;
-    };
-
-    if (self.page) |page| {
-        return page.wait(wait_ms);
-    }
-    return .no_page;
-}
-
-pub fn fetchWait(self: *Session, wait_ms: u32) void {
     while (true) {
-        const page = self.page orelse return;
-        _ = page.wait(wait_ms);
-        const navigated = self.processQueuedNavigation() catch {
-            // There was an error processing the queue navigation. This already
-            // logged the error, just return.
-            return;
-        };
-
-        if (navigated == false) {
-            return;
+        const page = self.page orelse return .no_page;
+        switch (page.wait(wait_ms)) {
+            .navigate => self.processScheduledNavigation() catch return .done,
+            else => |result| return result,
         }
+        // if we've successfull navigated, we'll give the new page another
+        // page.wait(wait_ms)
     }
 }
 
-fn processQueuedNavigation(self: *Session) !bool {
-    const qn = self.queued_navigation orelse return false;
+fn processScheduledNavigation(self: *Session) !void {
+    const qn = self.page.?._queued_navigation.?;
+    defer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 8 * 1024 });
+
     // 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();
-
-    // Page.navigateFromWebAPI terminatedExecution. If we don't resume
-    // it before doing a shutdown we'll get an error.
-    self.executor.resumeExecution();
     self.removePage();
-    self.queued_navigation = null;
 
     const page = self.createPage() catch |err| {
         log.err(.browser, "queued navigation page error", .{
@@ -196,19 +170,8 @@ fn processQueuedNavigation(self: *Session) !bool {
         return err;
     };
 
-    page.navigate(
-        qn.url,
-        qn.opts,
-        self.navigation._current_navigation_kind orelse .{ .push = null },
-    ) catch |err| {
+    page.navigate(qn.url, qn.opts) catch |err| {
         log.err(.browser, "queued navigation error", .{ .err = err, .url = qn.url });
         return err;
     };
-
-    return true;
 }
-
-const QueuedNavigation = struct {
-    url: [:0]const u8,
-    opts: NavigateOpts,
-};

src/browser/webapi/HTMLDocument.zig

@@ -138,6 +138,10 @@ pub fn getLocation(self: *const HTMLDocument) ?*@import("Location.zig") {
     return self._proto._location;
 }
 
+pub fn setLocation(_: *const HTMLDocument, url: [:0]const u8, page: *Page) !void {
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .script);
+}
+
 pub fn getAll(self: *HTMLDocument, page: *Page) !*collections.HTMLAllCollection {
     return page._factory.create(collections.HTMLAllCollection.init(self.asNode(), page));
 }
@@ -206,7 +210,7 @@ pub const JsApi = struct {
     pub const applets = bridge.accessor(HTMLDocument.getApplets, null, .{});
     pub const plugins = bridge.accessor(HTMLDocument.getEmbeds, null, .{});
     pub const currentScript = bridge.accessor(HTMLDocument.getCurrentScript, null, .{});
-    pub const location = bridge.accessor(HTMLDocument.getLocation, null, .{ .cache = "location" });
+    pub const location = bridge.accessor(HTMLDocument.getLocation, HTMLDocument.setLocation, .{ .cache = "location" });
     pub const all = bridge.accessor(HTMLDocument.getAll, null, .{});
     pub const cookie = bridge.accessor(HTMLDocument.getCookie, HTMLDocument.setCookie, .{});
     pub const doctype = bridge.accessor(HTMLDocument.getDocType, null, .{});

src/browser/webapi/Location.zig

@@ -77,11 +77,25 @@ pub fn setHash(_: *const Location, hash: []const u8, page: *Page) !void {
         } else if (hash[0] == '#')
             break :blk hash
         else
-            break :blk try std.fmt.allocPrint(page.arena, "#{s}", .{hash});
+            break :blk try std.fmt.allocPrint(page.call_arena, "#{s}", .{hash});
     };
 
-    const duped_hash = try page.arena.dupeZ(u8, normalized_hash);
-    return page.navigate(duped_hash, .{ .reason = .script }, .{ .replace = null });
+    return page.scheduleNavigation(normalized_hash, .{
+        .reason = .script,
+        .kind = .{ .replace = null },
+    }, .script);
+}
+
+pub fn assign(_: *const Location, url: [:0]const u8, page: *Page) !void {
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .script);
+}
+
+pub fn replace(_: *const Location, url: [:0]const u8, page: *Page) !void {
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .replace = null } }, .script);
+}
+
+pub fn reload(_: *const Location, page: *Page) !void {
+    return page.scheduleNavigation(page.url, .{ .reason = .script, .kind = .reload }, .script);
 }
 
 pub fn toString(self: *const Location, page: *const Page) ![:0]const u8 {
@@ -98,7 +112,11 @@ pub const JsApi = struct {
     };
 
     pub const toString = bridge.function(Location.toString, .{});
-    pub const href = bridge.accessor(Location.toString, null, .{});
+    pub const href = bridge.accessor(Location.toString, setHref, .{});
+    fn setHref(self: *const Location, url: [:0]const u8, page: *Page) !void {
+        return self.assign(url, page);
+    }
+
     pub const search = bridge.accessor(Location.getSearch, null, .{});
     pub const hash = bridge.accessor(Location.getHash, Location.setHash, .{});
     pub const pathname = bridge.accessor(Location.getPathname, null, .{});
@@ -107,4 +125,7 @@ pub const JsApi = struct {
     pub const port = bridge.accessor(Location.getPort, null, .{});
     pub const origin = bridge.accessor(Location.getOrigin, null, .{});
     pub const protocol = bridge.accessor(Location.getProtocol, null, .{});
+    pub const assign = bridge.function(Location.assign, .{});
+    pub const replace = bridge.function(Location.replace, .{});
+    pub const reload = bridge.function(Location.reload, .{});
 };

src/browser/webapi/Window.zig

@@ -115,6 +115,10 @@ pub fn getLocation(self: *const Window) *Location {
     return self._location;
 }
 
+pub fn setLocation(_: *const Window, url: [:0]const u8, page: *Page) !void {
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .script);
+}
+
 pub fn getHistory(_: *Window, page: *Page) *History {
     return &page._session.history;
 }
@@ -530,7 +534,7 @@ pub const JsApi = struct {
     pub const localStorage = bridge.accessor(Window.getLocalStorage, null, .{ .cache = "localStorage" });
     pub const sessionStorage = bridge.accessor(Window.getSessionStorage, null, .{ .cache = "sessionStorage" });
     pub const document = bridge.accessor(Window.getDocument, null, .{ .cache = "document" });
-    pub const location = bridge.accessor(Window.getLocation, null, .{ .cache = "location" });
+    pub const location = bridge.accessor(Window.getLocation, Window.setLocation, .{ .cache = "location" });
     pub const history = bridge.accessor(Window.getHistory, null, .{});
     pub const navigation = bridge.accessor(Window.getNavigation, null, .{});
     pub const crypto = bridge.accessor(Window.getCrypto, null, .{ .cache = "crypto" });

src/browser/webapi/navigation/Navigation.zig

@@ -289,7 +289,7 @@ pub fn navigateInner(
 
                 _ = try self.pushEntry(url, .{ .source = .navigation, .value = state }, page, true);
             } else {
-                try page.navigate(url, .{ .reason = .navigation }, kind);
+                try page.navigate(url, .{ .reason = .navigation, .kind = kind });
             }
         },
         .replace => |state| {
@@ -302,7 +302,7 @@ pub fn navigateInner(
 
                 _ = try self.replaceEntry(url, .{ .source = .navigation, .value = state }, page, true);
             } else {
-                try page.navigate(url, .{ .reason = .navigation }, kind);
+                try page.navigate(url, .{ .reason = .navigation, .kind = kind });
             }
         },
         .traverse => |index| {
@@ -315,11 +315,11 @@ pub fn navigateInner(
                 // todo: Fire navigate event
                 finished.resolve("navigation traverse", {});
             } else {
-                try page.navigate(url, .{ .reason = .navigation }, kind);
+                try page.navigate(url, .{ .reason = .navigation, .kind = kind });
             }
         },
         .reload => {
-            try page.navigate(url, .{ .reason = .navigation }, kind);
+            try page.navigate(url, .{ .reason = .navigation, .kind = kind });
         },
     }
 

src/cdp/cdp.zig

@@ -115,8 +115,6 @@ pub fn CDPT(comptime TypeProvider: type) type {
         // A bit hacky right now. The main server loop doesn't unblock for
         // scheduled task. So we run this directly in order to process any
         // timeouts (or http events) which are ready to be processed.
-
-        pub fn hasPage() bool {}
         pub fn pageWait(self: *Self, ms: u32) Session.WaitResult {
             const session = &(self.browser.session orelse return .no_page);
             return session.wait(ms);

src/cdp/domains/page.zig

@@ -221,7 +221,8 @@ fn navigate(cmd: anytype) !void {
     try page.navigate(params.url, .{
         .reason = .address_bar,
         .cdp_id = cmd.input.id,
-    }, .{ .push = null });
+        .kind = .{ .push = null },
+    });
 }
 
 pub fn pageNavigate(arena: Allocator, bc: anytype, event: *const Notification.PageNavigate) !void {

src/cdp/domains/target.zig

@@ -209,8 +209,7 @@ fn createTarget(cmd: anytype) !void {
     if (!std.mem.eql(u8, "about:blank", params.url)) {
         try page.navigate(
             params.url,
-            .{ .reason = .address_bar },
-            .{ .push = null },
+            .{ .reason = .address_bar, .kind = .{ .push = null } },
         );
     }
 

src/cdp/testing.zig

@@ -130,8 +130,8 @@ const TestContext = struct {
                 .{url},
                 0,
             );
-            try page.navigate(full_url, .{}, .{ .push = null });
-            bc.session.fetchWait(2000);
+            try page.navigate(full_url, .{});
+            _ = bc.session.wait(2000);
         }
         return bc;
     }

src/lightpanda.zig

@@ -44,7 +44,7 @@ pub fn fetch(app: *App, url: [:0]const u8, opts: FetchOpts) !void {
 
     // // Comment this out to get a profile of the JS code in v8/profile.json.
     // // You can open this in Chrome's profiler.
-    // // I've seen it generate invalid JSON, but I'm not sure why. It only
+    // // I've seen it generate invalid JSON, but I'm not sure why. It
     // // happens rarely, and I manually fix the file.
     // page.js.startCpuProfiler();
     // defer {
@@ -60,8 +60,11 @@ pub fn fetch(app: *App, url: [:0]const u8, opts: FetchOpts) !void {
     //     }
     // }
 
-    _ = try page.navigate(url, .{}, .{ .push = null });
-    _ = session.fetchWait(opts.wait_ms);
+    _ = try page.navigate(url, .{
+        .reason = .address_bar,
+        .kind = .{ .push = null },
+    });
+    _ = session.wait(opts.wait_ms);
 
     const writer = opts.writer orelse return;
     try dump.root(page.window._document, opts.dump, writer, page);

src/main_legacy_test.zig

@@ -86,7 +86,7 @@ pub fn run(allocator: Allocator, file: []const u8, session: *lp.Session) !void {
     defer try_catch.deinit();
 
     try page.navigate(url, .{}, .{ .push = null });
-    session.fetchWait(2000);
+    session.wait(2000);
 
     page._session.browser.runMicrotasks();
     page._session.browser.runMessageLoop();

src/testing.zig

@@ -403,8 +403,8 @@ fn runWebApiTest(test_file: [:0]const u8) !void {
     try_catch.init(js_context);
     defer try_catch.deinit();
 
-    try page.navigate(url, .{}, .{ .push = null });
-    test_session.fetchWait(2000);
+    try page.navigate(url, .{});
+    _ = test_session.wait(2000);
 
     page._session.browser.runMicrotasks();
 
@@ -427,8 +427,8 @@ pub fn pageTest(comptime test_file: []const u8) !*Page {
         0,
     );
 
-    try page.navigate(url, .{}, .{ .push = null });
-    test_session.fetchWait(2000);
+    try page.navigate(url, .{});
+    _ = test_session.wait(2000);
     return page;
 }