From 753391b7e26d113544f302eb64e7b79adcf350ba Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Mon, 9 Mar 2026 08:47:43 +0800
Subject: [PATCH] Add origins safety cleanup when destroying the context for
 the root page

---
 src/browser/Page.zig   |  5 +++--
 src/browser/js/Env.zig | 32 ++++++++++++++++++++++++++++++--
 src/cdp/cdp.zig        |  2 +-
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/browser/Page.zig b/src/browser/Page.zig
index 9b05c39b..dbb93792 100644
--- a/src/browser/Page.zig
+++ b/src/browser/Page.zig
@@ -345,11 +345,12 @@ pub fn deinit(self: *Page, abort_http: bool) void {
     }
 
     const session = self._session;
-    session.browser.env.destroyContext(self.js);
+    const is_root = self.parent == null;
+    session.browser.env.destroyContext(self.js, is_root);
 
     self._script_manager.shutdown = true;
 
-    if (self.parent == null) {
+    if (is_root) {
         session.browser.http_client.abort();
     } else if (abort_http) {
         // a small optimization, it's faster to abort _everything_ on the root
diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig
index 9bacc088..d64ef649 100644
--- a/src/browser/js/Env.zig
+++ b/src/browser/js/Env.zig
@@ -73,7 +73,17 @@ isolate_params: *v8.CreateParams,
 
 context_id: usize,
 
-// Maps origin -> shared Origin contains, for v8 values shared across same-origin Contexts
+// Maps origin -> shared Origin contains, for v8 values shared across
+// same-origin Contexts. There's a mismatch here between our JS model and our
+// Browser model. Origins only live as long as the root page of a session exists.
+// It would be wrong/dangerous to re-use an Origin across root page navigations.
+// But we have no mechanism to capture that lifetime in js. We used to have a
+// js.BrowserContext which mapped to a Session (oops, I took it out), but even
+// that wouldn't match correctly, because 1 session can have have muliple non-
+// concurrent pages. We deal with this in destroyContext by checking if we're
+// destroying the root context and, if so, making sure origins is empty. But, if
+// we ever add multiple Sessions to a Browser or mulitple Pages to a Session,
+// this map will have to live in a new, better scoped, container.
 origins: std.StringHashMapUnmanaged(*Origin) = .empty,
 
 // Global handles that need to be freed on deinit
@@ -348,7 +358,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context {
     return context;
 }
 
-pub fn destroyContext(self: *Env, context: *Context) void {
+pub fn destroyContext(self: *Env, context: *Context, is_root: bool) void {
     for (self.contexts[0..self.context_count], 0..) |ctx, i| {
         if (ctx == context) {
             // Swap with last element and decrement count
@@ -371,6 +381,24 @@ pub fn destroyContext(self: *Env, context: *Context) void {
     }
 
     context.deinit();
+
+    if (is_root) {
+        // When the root is destroyed, the all of our contexts should be gone
+        // and with them, all of our origins. Keep origins around longer than
+        // intended would cause issues, so we're going to be defensive here and
+        // clean things up.
+        if (comptime IS_DEBUG) {
+            std.debug.assert(self.context_count == 0);
+            std.debug.assert(self.origins.count() == 0);
+        }
+
+        const app = self.app;
+        var it = self.origins.valueIterator();
+        while (it.next()) |value| {
+            value.*.deinit(app);
+        }
+        self.origins.clearRetainingCapacity();
+    }
 }
 
 pub fn getOrCreateOrigin(self: *Env, key_: ?[]const u8) !*Origin {
diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig
index 78e5ab50..40057b8b 100644
--- a/src/cdp/cdp.zig
+++ b/src/cdp/cdp.zig
@@ -759,7 +759,7 @@ const IsolatedWorld = struct {
 
     pub fn removeContext(self: *IsolatedWorld) !void {
         const ctx = self.context orelse return error.NoIsolatedContextToRemove;
-        self.browser.env.destroyContext(ctx);
+        self.browser.env.destroyContext(ctx, false);
         self.context = null;
     }