Add target-aware(ish) navigation

All inner navigations have an originator and a target. Consider this:

```js
aframe.contentDocument.querySelector('#link').click();
```

The originator is the context in which this JavaScript is called, the target is
`aframe. Importantly, relative URLs are resolved based on the originator. This
commit adds that.

This is only a first step, there are other aspect to this relationship that
isn't addressed yet, like differences in behavior if the originator and target
are on different origins, and specific target targetting via the things like
the "target" attribute. What this commit does though is address the normal /
common case.

It builds on top of https://github.com/lightpanda-io/browser/pull/1720

src/browser/Page.zig

@@ -69,6 +69,7 @@ const ArenaPool = App.ArenaPool;
 const timestamp = @import("../datetime.zig").timestamp;
 const milliTimestamp = @import("../datetime.zig").milliTimestamp;
 
+const IFrame = Element.Html.IFrame;
 const WebApiURL = @import("webapi/URL.zig");
 const GlobalEventHandlersLookup = @import("webapi/global_event_handlers.zig").Lookup;
 
@@ -223,7 +224,7 @@ _arena_pool_leak_track: (if (IS_DEBUG) std.AutoHashMapUnmanaged(usize, struct {
 parent: ?*Page,
 window: *Window,
 document: *Document,
-iframe: ?*Element.Html.IFrame = null,
+iframe: ?*IFrame = null,
 frames: std.ArrayList(*Page) = .{},
 frames_sorted: bool = true,
 
@@ -566,62 +567,76 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi
     };
 }
 
-// We cannot navigate immediately as navigating will delete the DOM tree,
+// Navigation can happen in many places, such as executing a <script> tag or
-// which holds this event's node.
+// a JavaScript callback, a CDP command, etc...It's rarely safe to do immediately
-// As such we schedule the function to be called as soon as possible.
+// as the caller almost certainly does'nt expect the page to go away during the
-pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void {
+// call. So, we schedule the navigation for the next tick.
-    if (self.canScheduleNavigation(priority) == false) {
+pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOpts, nt: Navigation) !void {
+    if (self.canScheduleNavigation(std.meta.activeTag(nt)) == false) {
         return;
     }
     const arena = try self.arena_pool.acquire();
     errdefer self.arena_pool.release(arena);
-    return self.scheduleNavigationWithArena(arena, request_url, opts, priority);
+    return self.scheduleNavigationWithArena(arena, request_url, opts, nt);
 }
 
-fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []const u8, opts: NavigateOpts, priority: NavigationPriority) !void {
+// Don't name the first parameter "self", because the target of this navigation
+// might change inside the function. So the code should be explicit about the
+// page that it's acting on.
+fn scheduleNavigationWithArena(originator: *Page, arena: Allocator, request_url: []const u8, opts: NavigateOpts, nt: Navigation) !void {
     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
+            // navigate will handle this special case
+            break :blk .{ "about:blank", true };
         }
         const u = try URL.resolve(
             arena,
-            self.base(),
+            originator.base(),
             request_url,
             .{ .always_dupe = true, .encode = true },
         );
         break :blk .{ u, false };
     };
 
-    const session = self._session;
+    const target = switch (nt) {
-    if (!opts.force and URL.eqlDocument(self.url, resolved_url)) {
+        .script => |p| p orelse originator,
-        self.url = try self.arena.dupeZ(u8, resolved_url);
+        .iframe => |iframe| iframe._window.?._page, // only an frame with existing content (i.e. a window) can be navigated
-        self.window._location = try Location.init(self.url, self);
+        .anchor, .form => |node| blk: {
-        self.document._location = self.window._location;
+            const doc = node.ownerDocument(originator) orelse break :blk originator;
-        if (self.parent == null) {
+            break :blk doc._page orelse originator;
-            try session.navigation.updateEntries(self.url, opts.kind, self, true);
+        },
+    };
+
+    const session = target._session;
+    if (!opts.force and URL.eqlDocument(target.url, resolved_url)) {
+        target.url = try target.arena.dupeZ(u8, resolved_url);
+        target.window._location = try Location.init(target.url, target);
+        target.document._location = target.window._location;
+        if (target.parent == null) {
+            try session.navigation.updateEntries(target.url, opts.kind, target, true);
         }
         // doin't defer this, the caller, the caller is responsible for freeing
         // it on error
-        self.arena_pool.release(arena);
+        target.arena_pool.release(arena);
         return;
     }
 
     log.info(.browser, "schedule navigation", .{
         .url = resolved_url,
         .reason = opts.reason,
-        .type = self._type,
+        .type = target._type,
     });
 
     // 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) {
+    if (target.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);
+        session.browser.http_client.abortFrame(target._frame_id);
     }
 
     const qn = try arena.create(QueuedNavigation);
@@ -629,12 +644,12 @@ fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []con
         .opts = opts,
         .arena = arena,
         .url = resolved_url,
-        .priority = priority,
         .is_about_blank = is_about_blank,
+        .navigation_type = std.meta.activeTag(nt),
     };
 
-    self._queued_navigation = qn;
+    target._queued_navigation = qn;
-    return session.scheduleNavigation(self);
+    return session.scheduleNavigation(target);
 }
 
 // A script can have multiple competing navigation events, say it starts off
@@ -642,30 +657,31 @@ fn scheduleNavigationWithArena(self: *Page, arena: Allocator, request_url: []con
 // 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:
+// From what I can tell, there are 4 "levels" of priority, in order:
 // 1 - form submission
 // 2 - JavaScript apis (e.g. top.location)
 // 3 - anchor clicks
+// 4 - iframe.src =
 // Within, each category, it's last-one-wins.
-fn canScheduleNavigation(self: *Page, priority: NavigationPriority) bool {
+fn canScheduleNavigation(self: *Page, new_target_type: NavigationType) bool {
     if (self.parent) |parent| {
         if (parent.isGoingAway()) {
             return false;
         }
     }
 
-    const existing = self._queued_navigation orelse return true;
+    const existing_target_type = (self._queued_navigation orelse return true).navigation_type;
 
-    if (existing.priority == priority) {
+    if (existing_target_type == new_target_type) {
         // same reason, than this latest one wins
         return true;
     }
 
-    return switch (existing.priority) {
+    return switch (existing_target_type) {
         .iframe => true, // everything is higher priority than iframe.src = "x"
-        .anchor => priority != .iframe, // an anchor is only higher priority than an iframe
+        .anchor => new_target_type != .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
+        .script => new_target_type == .form, // a form is higher priority than a script
     };
 }
 
@@ -697,7 +713,7 @@ pub fn scriptsCompletedLoading(self: *Page) void {
     self.pendingLoadCompleted();
 }
 
-pub fn iframeCompletedLoading(self: *Page, iframe: *Element.Html.IFrame) void {
+pub fn iframeCompletedLoading(self: *Page, iframe: *IFrame) void {
     blk: {
         var ls: JS.Local.Scope = undefined;
         self.js.localScope(&ls);
@@ -1004,7 +1020,7 @@ pub fn scriptAddedCallback(self: *Page, comptime from_parser: bool, script: *Ele
     };
 }
 
-pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
+pub fn iframeAddedCallback(self: *Page, iframe: *IFrame) !void {
     if (self.isGoingAway()) {
         // if we're planning on navigating to another page, don't load this iframe
         return;
@@ -1018,16 +1034,16 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
         return;
     }
 
-    if (iframe._content_window) |cw| {
+    if (iframe._window != null) {
         // 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, .{
+        return self.scheduleNavigation(src, .{
             .reason = .script,
             .kind = .{ .push = null },
-        }, .iframe);
+        }, .{ .iframe = iframe });
     }
 
     iframe._executed = true;
@@ -1041,8 +1057,8 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
 
     self._pending_loads += 1;
     page_frame.iframe = iframe;
-    iframe._content_window = page_frame.window;
+    iframe._window = page_frame.window;
-    errdefer iframe._content_window = null;
+    errdefer iframe._window = null;
 
     // on first load, dispatch frame_created evnet
     self._session.notification.dispatch(.page_frame_created, &.{
@@ -1066,7 +1082,7 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void {
     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;
+        iframe._window = null;
         page_frame.deinit(true);
         return error.IFrameLoadError;
     };
@@ -1986,7 +2002,7 @@ pub fn createElementNS(self: *Page, namespace: Element.Namespace, name: []const
                         .{ ._proto = undefined },
                     ),
                     asUint("iframe") => return self.createHtmlElementT(
-                        Element.Html.IFrame,
+                        IFrame,
                         namespace,
                         attribute_iterator,
                         .{ ._proto = undefined },
@@ -2897,7 +2913,7 @@ fn nodeIsReady(self: *Page, comptime from_parser: bool, node: *Node) !void {
             log.err(.page, "page.nodeIsReady", .{ .err = err, .element = "script", .type = self._type, .url = self.url });
             return err;
         };
-    } else if (node.is(Element.Html.IFrame)) |iframe| {
+    } else if (node.is(IFrame)) |iframe| {
         if ((comptime from_parser == false) and iframe._src.len == 0) {
             // iframe was added via JavaScript, but without a src
             return;
@@ -3043,19 +3059,26 @@ pub const NavigatedOpts = struct {
     method: Http.Method = .GET,
 };
 
-const NavigationPriority = enum {
+const NavigationType = enum {
     form,
     script,
     anchor,
     iframe,
 };
 
+const Navigation = union(NavigationType) {
+    form: *Node,
+    script: ?*Page,
+    anchor: *Node,
+    iframe: *IFrame,
+};
+
 pub const QueuedNavigation = struct {
     arena: Allocator,
     url: [:0]const u8,
     opts: NavigateOpts,
-    priority: NavigationPriority,
     is_about_blank: bool,
+    navigation_type: NavigationType,
 };
 
 pub fn triggerMouseClick(self: *Page, x: f64, y: f64) !void {
@@ -3108,11 +3131,17 @@ pub fn handleClick(self: *Page, target: *Node) !void {
                 return;
             }
 
+            // TODO: We need to support targets properly, but this is the most
+            // common case: a click on an anchor navigates the page/frame that
+            // anchor is in.
+
+            // ownerDocument only returns null when `target` is a document, which
+            // it is NOT in this case. Even for a detched node, it'll return self.document
             try element.focus(self);
             try self.scheduleNavigation(href, .{
                 .reason = .script,
                 .kind = .{ .push = null },
-            }, .anchor);
+            }, .{ .anchor = target });
         },
         .input => |input| {
             try element.focus(self);
@@ -3243,7 +3272,7 @@ pub fn submitForm(self: *Page, submitter_: ?*Element, form_: ?*Element.Html.Form
     } else {
         action = try URL.concatQueryString(arena, action, buf.written());
     }
-    return self.scheduleNavigationWithArena(arena, action, opts, .form);
+    return self.scheduleNavigationWithArena(arena, action, opts, .{ .form = form_element.asNode() });
 }
 
 // insertText is a shortcut to insert text into the active element.

src/browser/Session.zig

@@ -419,50 +419,37 @@ fn processQueuedNavigation(self: *Session) !void {
     }
 }
 
-fn processFrameNavigation(self: *Session, current_page: *Page, qn: *QueuedNavigation) !void {
+fn processFrameNavigation(self: *Session, page: *Page, qn: *QueuedNavigation) !void {
-    lp.assert(current_page.parent != null, "root queued navigation", .{});
+    lp.assert(page.parent != null, "root queued navigation", .{});
 
     const browser = self.browser;
-    const iframe = current_page.iframe.?;
+    const iframe = page.iframe.?;
-    const parent = current_page.parent.?;
+    const parent = page.parent.?;
 
-    current_page._queued_navigation = null;
+    page._queued_navigation = null;
     defer browser.arena_pool.release(qn.arena);
 
-    errdefer iframe._content_window = null;
+    errdefer iframe._window = null;
 
-    if (current_page._parent_notified) {
+    if (page._parent_notified) {
         // we already notified the parent that we had loaded
         parent._pending_loads += 1;
     }
 
-    const frame_id = current_page._frame_id;
+    const frame_id = page._frame_id;
-    defer current_page.deinit(true);
+    page.deinit(true);
+    page.* = undefined;
 
-    const new_page = try parent.arena.create(Page);
+    try Page.init(page, frame_id, self, parent);
-    try Page.init(new_page, frame_id, self, parent);
+    errdefer page.deinit(true);
-    errdefer new_page.deinit(true);
 
-    new_page.iframe = iframe;
+    page.iframe = iframe;
-    iframe._content_window = new_page.window;
+    iframe._window = page.window;
 
-    new_page.navigate(qn.url, qn.opts) catch |err| {
+    page.navigate(qn.url, qn.opts) catch |err| {
         log.err(.browser, "queued frame navigation error", .{ .err = err });
         return err;
     };
-
-    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 {

src/browser/js/Context.zig

@@ -153,7 +153,7 @@ pub fn fromIsolate(isolate: js.Isolate) *Context {
 }
 
 pub fn deinit(self: *Context) void {
-    if (comptime IS_DEBUG) {
+    if (comptime IS_DEBUG and @import("builtin").is_test == false) {
         var it = self.unknown_properties.iterator();
         while (it.next()) |kv| {
             log.debug(.unknown_prop, "unknown property", .{

src/browser/tests/frames/frames.html

@@ -13,6 +13,7 @@
 <script id="basic">
    // reload it
   $('#f2').src = 'support/sub2.html';
+  testing.expectEqual(true, true);
 
   testing.eventually(() => {
     testing.expectEqual(undefined, window[10]);
@@ -103,29 +104,29 @@
   }
 </script>
 
-<!--
-Need correct _target support
 <script id=link_click>
-  {
+  testing.async(async (restore) => {
+    await new Promise((resolve) => {
+      let count = 0;
       let f6 = document.createElement('iframe');
       f6.id = 'f6';
       f6.addEventListener('load', () => {
+        if (++count == 2) {
+          resolve();
+          return;
+        }
         f6.contentDocument.querySelector('#link').click();
-    }, {once: true});
+      });
       f6.src = "support/with_link.html";
       document.documentElement.appendChild(f6);
-
-    testing.eventually(() => {
-      console.warn(f6.contentDocument);
-      testing.expectEqual("<html><head></head><body></body></html>", f6.contentDocument.documentElement.outerHTML);
     });
-  }
+    restore();
+    testing.expectEqual("<html><head></head><body>It was clicked!\n</body></html>", f6.contentDocument.documentElement.outerHTML);
+  });
 </script>
- -->
 
 <script id=count>
   testing.eventually(() => {
-    testing.expectEqual(5, window.length);
+    testing.expectEqual(6, window.length);
   });
 </script>
-

src/browser/tests/frames/support/with_link.html

@@ -1,2 +1,2 @@
 <!DOCTYPE html>
-<a href="after_link.html" id=link>a link</a>
+<a href="support/after_link.html" id=link>a link</a>

src/browser/webapi/HTMLDocument.zig

@@ -180,8 +180,8 @@ 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 {
+pub fn setLocation(self: *HTMLDocument, url: [:0]const u8, page: *Page) !void {
-    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .script);
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .{ .script = self._proto._page });
 }
 
 pub fn getAll(self: *HTMLDocument, page: *Page) !*collections.HTMLAllCollection {

src/browser/webapi/Location.zig

@@ -83,19 +83,19 @@ pub fn setHash(_: *const Location, hash: []const u8, page: *Page) !void {
     return page.scheduleNavigation(normalized_hash, .{
         .reason = .script,
         .kind = .{ .replace = null },
-    }, .script);
+    }, .{ .script = page });
 }
 
 pub fn assign(_: *const Location, url: [:0]const u8, page: *Page) !void {
-    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .script);
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .{ .script = page });
 }
 
 pub fn replace(_: *const Location, url: [:0]const u8, page: *Page) !void {
-    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .replace = null } }, .script);
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .replace = null } }, .{ .script = page });
 }
 
 pub fn reload(_: *const Location, page: *Page) !void {
-    return page.scheduleNavigation(page.url, .{ .reason = .script, .kind = .reload }, .script);
+    return page.scheduleNavigation(page.url, .{ .reason = .script, .kind = .reload }, .{ .script = page });
 }
 
 pub fn toString(self: *const Location, page: *const Page) ![:0]const u8 {

src/browser/webapi/Window.zig

@@ -160,8 +160,8 @@ pub fn getSelection(self: *const Window) *Selection {
     return &self._document._selection;
 }
 
-pub fn setLocation(_: *const Window, url: [:0]const u8, page: *Page) !void {
+pub fn setLocation(self: *Window, url: [:0]const u8, page: *Page) !void {
-    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .script);
+    return page.scheduleNavigation(url, .{ .reason = .script, .kind = .{ .push = null } }, .{ .script = self._page });
 }
 
 pub fn getHistory(_: *Window, page: *Page) *History {

src/browser/webapi/element/html/IFrame.zig

@@ -30,7 +30,7 @@ const IFrame = @This();
 _proto: *HtmlElement,
 _src: []const u8 = "",
 _executed: bool = false,
-_content_window: ?*Window = null,
+_window: ?*Window = null,
 
 pub fn asElement(self: *IFrame) *Element {
     return self._proto._proto;
@@ -40,11 +40,11 @@ pub fn asNode(self: *IFrame) *Node {
 }
 
 pub fn getContentWindow(self: *const IFrame) ?*Window {
-    return self._content_window;
+    return self._window;
 }
 
 pub fn getContentDocument(self: *const IFrame) ?*Document {
-    const window = self._content_window orelse return null;
+    const window = self._window orelse return null;
     return window._document;
 }
 

src/browser/webapi/navigation/Navigation.zig

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