diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 3a8ab9a93..b547fa995 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -288,11 +288,24 @@ pub const HttpHeader = struct { value: []const u8, }; -pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void { +pub const InitOpts = struct { + parent: ?*Frame = null, + + // When a frame/popup re-navigates, we should preserve the same window. + // There are a couple reasons for this. First, iframe.contentWindow should + // maintain the same identity (which we don't correctly do now, but this + // is a first step). Secondly, a reference to the window can be acquired + // prior to navigation, and then used after. So it should remain valid. + reuse_window: ?*Window = null, +}; + +pub fn init(self: *Frame, frame_id: u32, page: *Page, opts: InitOpts) !void { if (comptime IS_DEBUG) { log.debug(.frame, "frame.init", .{}); } + const parent = opts.parent; + const session = page.session; const call_arena = try session.getArena(.medium, "call_arena"); errdefer session.releaseArena(call_arena); @@ -341,7 +354,7 @@ pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void { }); } - self.window = try factory.eventTarget(Window{ + const window_template = Window{ ._frame = self, ._proto = undefined, ._document = self.document, @@ -350,7 +363,16 @@ pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void { ._screen = screen, ._visual_viewport = visual_viewport, ._cross_origin_wrapper = undefined, - }); + }; + + if (opts.reuse_window) |w| { + const proto = w._proto; + w.* = window_template; + w._proto = proto; + self.window = w; + } else { + self.window = try factory.eventTarget(window_template); + } self.window._cross_origin_wrapper = .{ .window = self.window }; self._style_manager = try StyleManager.init(self); @@ -1463,7 +1485,7 @@ pub fn iframeAddedCallback(self: *Frame, iframe: *IFrame) !void { const new_frame = try self.arena.create(Frame); const frame_id = session.nextFrameId(); - try Frame.init(new_frame, frame_id, self._page, self); + try Frame.init(new_frame, frame_id, self._page, .{ .parent = self }); errdefer new_frame.deinit(); self._pending_loads += 1; @@ -1587,7 +1609,7 @@ pub fn openPopup(self: *Frame, opts: OpenPopupOpts) !*Frame { errdefer page.frame_arena.destroy(popup); const frame_id = session.nextFrameId(); - try Frame.init(popup, frame_id, page, null); + try Frame.init(popup, frame_id, page, .{}); errdefer popup.deinit(); popup.window._opener = opts.opener; diff --git a/src/browser/Page.zig b/src/browser/Page.zig index a6fa6749c..bfbf8937e 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -126,7 +126,7 @@ pub fn init(self: *Page, session: *Session, frame_id: u32) !void { }; self.queued_navigation = &self.queued_navigation_1; - try Frame.init(&self.frame, frame_id, self, null); + try Frame.init(&self.frame, frame_id, self, .{}); } // Tear down the Page and its root Frame. Equivalent to the old diff --git a/src/browser/Session.zig b/src/browser/Session.zig index c355bee53..1070b044d 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -530,6 +530,7 @@ fn _processFrameNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation) } const frame_id = frame._frame_id; + const reuse_window = frame.window; const page = self.currentPage().?; frame.deinit(); frame.* = undefined; @@ -546,7 +547,7 @@ fn _processFrameNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation) } } - try Frame.init(frame, frame_id, page, parent); + try Frame.init(frame, frame_id, page, .{ .parent = parent, .reuse_window = reuse_window }); errdefer { if (parent_notified) { parent._pending_loads -= 1; @@ -563,14 +564,16 @@ fn _processFrameNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation) }; } -// Re-navigates a popup Frame in place. The Frame pointer stays stable -// (scripts in the opener may hold a cached Window ref — though the Window -// object inside is replaced, matching how iframes behave on navigation). +// Re-navigates a popup Frame in place. Both the Frame pointer and its Window +// stay stable across the re-init, so a cached `window.open()` return value keeps +// a valid `.location` instead of dangling against the freed one. fn processPopupNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation) !void { // Preserve popup identity fields. _name lives in the Page arena and // survives Frame.deinit; _opener is just a pointer. - const saved_name = frame.window._name; - const saved_opener = frame.window._opener; + + const reuse_window = frame.window; + const saved_name = reuse_window._name; + const saved_opener = reuse_window._opener; const frame_id = frame._frame_id; const page = self.currentPage().?; @@ -587,7 +590,7 @@ fn processPopupNavigation(self: *Session, frame: *Frame, qn: *QueuedNavigation) } } - try Frame.init(frame, frame_id, page, null); + try Frame.init(frame, frame_id, page, .{ .reuse_window = reuse_window }); errdefer frame.deinit(); frame.window._name = saved_name; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index ea121e64d..34a47da5f 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -221,6 +221,20 @@ pub fn deinit(self: *Context) void { // have a dangling pointer to our freed Context struct. v8.v8__Context__SetAlignedPointerInEmbedderData(entered.handle, 1, null); + // Evict the window's entry from the identity map. The window pointer is the + // identity-map key (see Env.createContext), and an in-place navigation + // reuses the same Window address — a lingering entry would make the next + // context's getOrPut see `found_existing` and skip registering its global. + switch (self.global) { + .frame => |frame| { + if (self.identity.identity_map.fetchRemove(@intFromPtr(frame.window))) |kv| { + var global = kv.value; + v8.v8__Global__Reset(&global); + } + }, + .worker => {}, + } + v8.v8__Global__Reset(&self.handle); env.isolate.notifyContextDisposed(); // There can be other tasks associated with this context that we need to diff --git a/src/browser/tests/frames/support/win_a.html b/src/browser/tests/frames/support/win_a.html new file mode 100644 index 000000000..d12e6fcbc --- /dev/null +++ b/src/browser/tests/frames/support/win_a.html @@ -0,0 +1,2 @@ + +win_a diff --git a/src/browser/tests/frames/support/win_b.html b/src/browser/tests/frames/support/win_b.html new file mode 100644 index 000000000..f2e935951 --- /dev/null +++ b/src/browser/tests/frames/support/win_b.html @@ -0,0 +1,2 @@ + +win_b diff --git a/src/browser/tests/frames/window_reuse.html b/src/browser/tests/frames/window_reuse.html new file mode 100644 index 000000000..0128fac4c --- /dev/null +++ b/src/browser/tests/frames/window_reuse.html @@ -0,0 +1,52 @@ + + + + + + + + + + +