From 1a05da9e55ad00fc5c17356119366823e056347c Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 29 Jan 2026 11:22:01 +0800 Subject: [PATCH] Remove js.ExecutionWorld The ExecutionWorld doesn't do anything meaningful. It doesn't map to, or abstract any, v8 concepts. It creates a js.Context, destroys the context and points to the context. Those all all things the Env can do (and it isn't like the Env is over-burdened as-is). Plus the benefit of going through the Env is that we can track/collect all known Contexts for an isolate in 1 place (the Env), which can facilitate things like context creation/deletion notifications. --- src/browser/Browser.zig | 2 +- src/browser/Page.zig | 15 ++-- src/browser/Scheduler.zig | 1 - src/browser/Session.zig | 12 +-- src/browser/js/Context.zig | 3 + src/browser/js/Env.zig | 113 +++++++++++++++++++++---- src/browser/js/ExecutionWorld.zig | 136 ------------------------------ src/browser/js/js.zig | 1 - src/cdp/cdp.zig | 32 +++---- src/cdp/domains/dom.zig | 2 +- src/cdp/domains/page.zig | 7 +- 11 files changed, 132 insertions(+), 192 deletions(-) delete mode 100644 src/browser/js/ExecutionWorld.zig diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index 1d514a12..625305db 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -53,7 +53,7 @@ notification: *Notification, pub fn init(app: *App) !Browser { const allocator = app.allocator; - var env = try js.Env.init(allocator, &app.platform, &app.snapshot); + var env = try js.Env.init(app); errdefer env.deinit(); const notification = try Notification.init(allocator, app.notification); diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 4c6b6ea8..907c0b96 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -244,7 +244,7 @@ pub fn deinit(self: *Page) void { } const session = self._session; - session.executor.removeContext(); + session.browser.env.destroyContext(self.js); self._script_manager.shutdown = true; session.browser.http_client.abort(); @@ -263,8 +263,10 @@ pub fn deinit(self: *Page) void { } fn reset(self: *Page, comptime initializing: bool) !void { + const browser = self._session.browser; + if (comptime initializing == false) { - self._session.executor.removeContext(); + browser.env.destroyContext(self.js); // removing a context can trigger finalizers, so we can only check for // a leak after the above. @@ -276,15 +278,14 @@ fn reset(self: *Page, comptime initializing: bool) !void { self._arena_pool_leak_track.clearRetainingCapacity(); } - // We force a garbage collection between page navigations to keep v8 // memory usage as low as possible. - self._session.browser.env.memoryPressureNotification(.moderate); + browser.env.memoryPressureNotification(.moderate); self._script_manager.shutdown = true; - self._session.browser.http_client.abort(); + browser.http_client.abort(); self._script_manager.deinit(); - _ = self._session.browser.page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); + _ = browser.page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); } self._factory = Factory.init(self); @@ -320,7 +321,7 @@ fn reset(self: *Page, comptime initializing: bool) !void { self._script_manager = ScriptManager.init(self); errdefer self._script_manager.deinit(); - self.js = try self._session.executor.createContext(self, true); + self.js = try browser.env.createContext(self, true); errdefer self.js.deinit(); self._element_styles = .{}; diff --git a/src/browser/Scheduler.zig b/src/browser/Scheduler.zig index 41a69484..6d2c2075 100644 --- a/src/browser/Scheduler.zig +++ b/src/browser/Scheduler.zig @@ -19,7 +19,6 @@ const std = @import("std"); const builtin = @import("builtin"); -const js = @import("js/js.zig"); const log = @import("../log.zig"); const milliTimestamp = @import("../datetime.zig").milliTimestamp; diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 340d6b61..4211e32c 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -53,7 +53,6 @@ arena: Allocator, // page and start another. transfer_arena: Allocator, -executor: js.ExecutionWorld, cookie_jar: storage.Cookie.Jar, storage_shed: storage.Shed, @@ -63,20 +62,16 @@ navigation: Navigation, page: ?*Page = null, pub fn init(self: *Session, browser: *Browser) !void { - var executor = try browser.env.newExecutionWorld(); - errdefer executor.deinit(); - const allocator = browser.app.allocator; const session_allocator = browser.session_arena.allocator(); self.* = .{ - .browser = browser, - .executor = executor, + .history = .{}, + .navigation = .{}, .storage_shed = .{}, + .browser = browser, .arena = session_allocator, .cookie_jar = storage.Cookie.Jar.init(allocator), - .navigation = .{}, - .history = .{}, .transfer_arena = browser.transfer_arena.allocator(), }; } @@ -87,7 +82,6 @@ pub fn deinit(self: *Session) void { } self.cookie_jar.deinit(); self.storage_shed.deinit(self.browser.app.allocator); - self.executor.deinit(); } // NOTE: the caller is not the owner of the returned value, diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 648f6de3..98bc0e9b 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -21,6 +21,7 @@ const lp = @import("lightpanda"); const log = @import("../../log.zig"); const js = @import("js.zig"); +const Env = @import("Env.zig"); const bridge = @import("bridge.zig"); const TaggedOpaque = @import("TaggedOpaque.zig"); @@ -38,6 +39,7 @@ const IS_DEBUG = @import("builtin").mode == .Debug; const Context = @This(); id: usize, +env: *Env, page: *Page, isolate: js.Isolate, @@ -207,6 +209,7 @@ pub fn deinit(self: *Context) void { v8.v8__Context__Exit(ls.local.handle); } v8.v8__Global__Reset(&self.handle); + self.env.app.arena_pool.release(self.arena); } pub fn weakRef(self: *Context, obj: anytype) void { diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 69f7cd20..b181048c 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -20,6 +20,7 @@ const std = @import("std"); const js = @import("js.zig"); const v8 = js.v8; +const App = @import("../../App.zig"); const log = @import("../../log.zig"); const bridge = @import("bridge.zig"); @@ -28,13 +29,13 @@ const Isolate = @import("Isolate.zig"); const Platform = @import("Platform.zig"); const Snapshot = @import("Snapshot.zig"); const Inspector = @import("Inspector.zig"); -const ExecutionWorld = @import("ExecutionWorld.zig"); +const Page = @import("../Page.zig"); const Window = @import("../webapi/Window.zig"); const JsApis = bridge.JsApis; const Allocator = std.mem.Allocator; -const ArenaAllocator = std.heap.ArenaAllocator; +const IS_DEBUG = @import("builtin").mode == .Debug; // The Env maps to a V8 isolate, which represents a isolated sandbox for // executing JavaScript. The Env is where we'll define our V8 <-> Zig bindings, @@ -44,13 +45,15 @@ const ArenaAllocator = std.heap.ArenaAllocator; // The `types` parameter is a tuple of Zig structures we want to bind to V8. const Env = @This(); -allocator: Allocator, +app: *App, platform: *const Platform, // the global isolate isolate: js.Isolate, +contexts: std.ArrayList(*js.Context), + // just kept around because we need to free it on deinit isolate_params: *v8.CreateParams, @@ -65,7 +68,10 @@ templates: []*const v8.FunctionTemplate, // Global template created once per isolate and reused across all contexts global_template: v8.Eternal, -pub fn init(allocator: Allocator, platform: *const Platform, snapshot: *Snapshot) !Env { +pub fn init(app: *App) !Env { + const allocator = app.allocator; + const snapshot = &app.snapshot; + var params = try allocator.create(v8.CreateParams); errdefer allocator.destroy(params); v8.v8__Isolate__CreateParams__CONSTRUCT(params); @@ -140,10 +146,11 @@ pub fn init(allocator: Allocator, platform: *const Platform, snapshot: *Snapshot } return .{ + .app = app, .context_id = 0, + .contexts = .empty, .isolate = isolate, - .platform = platform, - .allocator = allocator, + .platform = &app.platform, .templates = templates, .isolate_params = params, .eternal_function_templates = eternal_function_templates, @@ -152,13 +159,94 @@ pub fn init(allocator: Allocator, platform: *const Platform, snapshot: *Snapshot } pub fn deinit(self: *Env) void { - self.allocator.free(self.templates); - self.allocator.free(self.eternal_function_templates); + if (comptime IS_DEBUG) { + std.debug.assert(self.contexts.items.len == 0); + } + for (self.contexts.items) |ctx| { + ctx.deinit(); + } + + const allocator = self.app.allocator; + self.contexts.deinit(allocator); + + allocator.free(self.templates); + allocator.free(self.eternal_function_templates); self.isolate.exit(); self.isolate.deinit(); v8.v8__ArrayBuffer__Allocator__DELETE(self.isolate_params.array_buffer_allocator.?); - self.allocator.destroy(self.isolate_params); + allocator.destroy(self.isolate_params); +} + +pub fn createContext(self: *Env, page: *Page, enter: bool) !*Context { + const context_arena = try self.app.arena_pool.acquire(); + errdefer self.app.arena_pool.release(context_arena); + + const isolate = self.isolate; + var hs: js.HandleScope = undefined; + hs.init(isolate); + defer hs.deinit(); + + // Get the global template that was created once per isolate + const global_template: *const v8.ObjectTemplate = @ptrCast(@alignCast(v8.v8__Eternal__Get(&self.global_template, isolate.handle).?)); + const v8_context = v8.v8__Context__New(isolate.handle, global_template, null).?; + + // Create the v8::Context and wrap it in a v8::Global + var context_global: v8.Global = undefined; + v8.v8__Global__New(isolate.handle, v8_context, &context_global); + + // our window wrapped in a v8::Global + const global_obj = v8.v8__Context__Global(v8_context).?; + var global_global: v8.Global = undefined; + v8.v8__Global__New(isolate.handle, global_obj, &global_global); + + if (enter) { + v8.v8__Context__Enter(v8_context); + } + errdefer if (enter) { + v8.v8__Context__Exit(v8_context); + }; + + const context_id = self.context_id; + self.context_id = context_id + 1; + + const context = try context_arena.create(Context); + context.* = .{ + .env = self, + .page = page, + .id = context_id, + .entered = enter, + .isolate = isolate, + .arena = context_arena, + .handle = context_global, + .templates = self.templates, + .script_manager = &page._script_manager, + .call_arena = page.call_arena, + }; + try context.identity_map.putNoClobber(context_arena, @intFromPtr(page.window), global_global); + + // Store a pointer to our context inside the v8 context so that, given + // a v8 context, we can get our context out + const data = isolate.initBigInt(@intFromPtr(context)); + v8.v8__Context__SetEmbedderData(v8_context, 1, @ptrCast(data.handle)); + + try self.contexts.append(self.app.allocator, context); + return context; +} + +pub fn destroyContext(self: *Env, context: *Context) void { + for (self.contexts.items, 0..) |ctx, i| { + if (ctx == context) { + _ = self.contexts.swapRemove(i); + break; + } + } else { + if (comptime IS_DEBUG) { + @panic("Tried to remove unknown context"); + } + } + context.deinit(); + self.isolate.notifyContextDisposed(); } pub fn newInspector(self: *Env, arena: Allocator, ctx: anytype) !*Inspector { @@ -182,13 +270,6 @@ pub fn pumpMessageLoop(self: *const Env) bool { pub fn runIdleTasks(self: *const Env) void { v8.v8__Platform__RunIdleTasks(self.platform.handle, self.isolate.handle, 1); } -pub fn newExecutionWorld(self: *Env) !ExecutionWorld { - return .{ - .env = self, - .context = null, - .context_arena = ArenaAllocator.init(self.allocator), - }; -} // V8 doesn't immediately free memory associated with // a Context, it's managed by the garbage collector. We use the diff --git a/src/browser/js/ExecutionWorld.zig b/src/browser/js/ExecutionWorld.zig deleted file mode 100644 index e468b167..00000000 --- a/src/browser/js/ExecutionWorld.zig +++ /dev/null @@ -1,136 +0,0 @@ -// Copyright (C) 2023-2025 Lightpanda (Selecy SAS) -// -// Francis Bouvier -// Pierre Tachoire -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as -// published by the Free Software Foundation, either version 3 of the -// License, or (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -const std = @import("std"); -const lp = @import("lightpanda"); - -const log = @import("../../log.zig"); -const Page = @import("../Page.zig"); - -const js = @import("js.zig"); -const v8 = js.v8; - -const Env = @import("Env.zig"); -const bridge = @import("bridge.zig"); -const Context = @import("Context.zig"); - -const ArenaAllocator = std.heap.ArenaAllocator; -const IS_DEBUG = @import("builtin").mode == .Debug; - -const CONTEXT_ARENA_RETAIN = 1024 * 64; - -// ExecutionWorld closely models a JS World. -// https://chromium.googlesource.com/chromium/src/+/master/third_party/blink/renderer/bindings/core/v8/V8BindingDesign.md#World -// https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/scripting/ExecutionWorld -const ExecutionWorld = @This(); - -env: *Env, - -// Arena whose lifetime is for a single page load. Where -// the call_arena lives for a single function call, the context_arena -// lives for the lifetime of the entire page. The allocator will be -// owned by the Context, but the arena itself is owned by the ExecutionWorld -// so that we can re-use it from context to context. -context_arena: ArenaAllocator, - -// Currently a context maps to a Browser's Page. Here though, it's only a -// mechanism to organization page-specific memory. The ExecutionWorld -// does all the work, but having all page-specific data structures -// grouped together helps keep things clean. -context: ?Context = null, - -// no init, must be initialized via env.newExecutionWorld() - -pub fn deinit(self: *ExecutionWorld) void { - if (self.context != null) { - self.removeContext(); - } - self.context_arena.deinit(); -} - -// Only the top Context in the Main ExecutionWorld should hold a handle_scope. -// A js.HandleScope is like an arena. Once created, any "Local" that -// v8 creates will be released (or at least, releasable by the v8 GC) -// when the handle_scope is freed. -// We also maintain our own "context_arena" which allows us to have -// all page related memory easily managed. -pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool) !*Context { - lp.assert(self.context == null, "ExecptionWorld.createContext has context", .{}); - - const env = self.env; - const isolate = env.isolate; - const arena = self.context_arena.allocator(); - - var hs: js.HandleScope = undefined; - hs.init(isolate); - defer hs.deinit(); - - // Get the global template that was created once per isolate - const global_template: *const v8.ObjectTemplate = @ptrCast(@alignCast(v8.v8__Eternal__Get(&env.global_template, isolate.handle).?)); - const v8_context = v8.v8__Context__New(isolate.handle, global_template, null).?; - - // Create the v8::Context and wrap it in a v8::Global - var context_global: v8.Global = undefined; - v8.v8__Global__New(isolate.handle, v8_context, &context_global); - - // our window wrapped in a v8::Global - const global_obj = v8.v8__Context__Global(v8_context).?; - var global_global: v8.Global = undefined; - v8.v8__Global__New(isolate.handle, global_obj, &global_global); - - if (enter) { - v8.v8__Context__Enter(v8_context); - } - errdefer if (enter) { - v8.v8__Context__Exit(v8_context); - }; - - const context_id = env.context_id; - env.context_id = context_id + 1; - - self.context = Context{ - .page = page, - .id = context_id, - .entered = enter, - .isolate = isolate, - .handle = context_global, - .templates = env.templates, - .script_manager = &page._script_manager, - .call_arena = page.call_arena, - .arena = arena, - }; - - var context = &self.context.?; - try context.identity_map.putNoClobber(arena, @intFromPtr(page.window), global_global); - - // Store a pointer to our context inside the v8 context so that, given - // a v8 context, we can get our context out - const data = isolate.initBigInt(@intFromPtr(&self.context.?)); - v8.v8__Context__SetEmbedderData(v8_context, 1, @ptrCast(data.handle)); - - return &self.context.?; -} - -pub fn removeContext(self: *ExecutionWorld) void { - var context = &(self.context orelse return); - context.deinit(); - self.context = null; - - self.env.isolate.notifyContextDisposed(); - _ = self.context_arena.reset(.{ .retain_with_limit = CONTEXT_ARENA_RETAIN }); -} diff --git a/src/browser/js/js.zig b/src/browser/js/js.zig index ee3c8009..4927f0f4 100644 --- a/src/browser/js/js.zig +++ b/src/browser/js/js.zig @@ -24,7 +24,6 @@ const string = @import("../../string.zig"); pub const Env = @import("Env.zig"); pub const bridge = @import("bridge.zig"); -pub const ExecutionWorld = @import("ExecutionWorld.zig"); pub const Caller = @import("Caller.zig"); pub const Context = @import("Context.zig"); pub const Local = @import("Local.zig"); diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 9942decf..48825573 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -467,15 +467,13 @@ pub fn BrowserContext(comptime CDP_T: type) type { } pub fn createIsolatedWorld(self: *Self, world_name: []const u8, grant_universal_access: bool) !*IsolatedWorld { - var executor = try self.cdp.browser.env.newExecutionWorld(); - errdefer executor.deinit(); - const owned_name = try self.arena.dupe(u8, world_name); const world = try self.isolated_worlds.addOne(self.arena); world.* = .{ + .context = null, .name = owned_name, - .executor = executor, + .env = &self.cdp.browser.env, .grant_universal_access = grant_universal_access, }; @@ -746,15 +744,20 @@ pub fn BrowserContext(comptime CDP_T: type) type { /// An object id is unique across all contexts, different object ids can refer to the same Node in different contexts. const IsolatedWorld = struct { name: []const u8, - executor: js.ExecutionWorld, + env: *js.Env, + context: ?*js.Context = null, grant_universal_access: bool, pub fn deinit(self: *IsolatedWorld) void { - self.executor.deinit(); + if (self.context) |ctx| { + self.env.destroyContext(ctx); + self.context = null; + } } pub fn removeContext(self: *IsolatedWorld) !void { - if (self.executor.context == null) return error.NoIsolatedContextToRemove; - self.executor.removeContext(); + const ctx = self.context orelse return error.NoIsolatedContextToRemove; + self.env.destroyContext(ctx); + self.context = null; } // The isolate world must share at least some of the state with the related page, specifically the DocumentHTML @@ -762,19 +765,16 @@ const IsolatedWorld = struct { // We just created the world and the page. The page's state lives in the session, but is update on navigation. // This also means this pointer becomes invalid after removePage until a new page is created. // Currently we have only 1 page/frame and thus also only 1 state in the isolate world. - pub fn createContext(self: *IsolatedWorld, page: *Page) !void { - // if (self.executor.context != null) return error.Only1IsolatedContextSupported; - if (self.executor.context != null) { + pub fn createContext(self: *IsolatedWorld, page: *Page) !*js.Context { + if (self.context == null) { + self.context = try self.env.createContext(page, false); + } else { log.warn(.cdp, "not implemented", .{ .feature = "createContext: Not implemented second isolated context creation", .info = "reuse existing context", }); - return; } - _ = try self.executor.createContext( - page, - false, - ); + return self.context.?; } }; diff --git a/src/cdp/domains/dom.zig b/src/cdp/domains/dom.zig index d3270b1b..8bb3ac5e 100644 --- a/src/cdp/domains/dom.zig +++ b/src/cdp/domains/dom.zig @@ -288,7 +288,7 @@ fn resolveNode(cmd: anytype) !void { ls.?.deinit(); ls = null; - const ctx = &(isolated_world.executor.context orelse return error.ContextNotFound); + const ctx = (isolated_world.context orelse return error.ContextNotFound); ls = undefined; ctx.localScope(&ls.?); if (ls.?.local.debugContextId() == context_id) { diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index de368821..09f5741a 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -190,8 +190,7 @@ fn createIsolatedWorld(cmd: anytype) !void { const world = try bc.createIsolatedWorld(params.worldName, params.grantUniveralAccess); const page = bc.session.currentPage() orelse return error.PageNotLoaded; - try world.createContext(page); - const js_context = &world.executor.context.?; + const js_context = try world.createContext(page); // Create the auxdata json for the contextCreated event // Calling contextCreated will assign a Id to the context and send the contextCreated event @@ -292,7 +291,7 @@ pub fn pageRemove(bc: anytype) !void { pub fn pageCreated(bc: anytype, page: *Page) !void { for (bc.isolated_worlds.items) |*isolated_world| { - try isolated_world.createContext(page); + _ = try isolated_world.createContext(page); } } @@ -375,7 +374,7 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P // Calling contextCreated will assign a new Id to the context and send the contextCreated event var ls: js.Local.Scope = undefined; - (isolated_world.executor.context orelse continue).localScope(&ls); + (isolated_world.context orelse continue).localScope(&ls); defer ls.deinit(); bc.inspector.contextCreated(