diff --git a/src/browser/page.zig b/src/browser/page.zig index 9f8138a93..0e665e1e9 100644 --- a/src/browser/page.zig +++ b/src/browser/page.zig @@ -85,6 +85,11 @@ pub const Page = struct { // execute any JavaScript scope: *Env.Scope, + // For a Page we only create one HandleScope and keep it alive for the duration of the page. + // If needed JS Locals' lifetimes can be reduced by layering on additional Scopes. + // Note: An isolate has only 1 scope stack shared by all main and isolated worlds. + handle_scope: Env.HandleScope, + // List of modules currently fetched/loaded. module_map: std.StringHashMapUnmanaged([]const u8), @@ -108,9 +113,18 @@ pub const Page = struct { .window_clicked_event_node = .{ .func = windowClicked }, .request_factory = browser.http_client.requestFactory(browser.notification), .scope = undefined, + .handle_scope = undefined, .module_map = .empty, }; - self.scope = try session.executor.startScope(&self.window, self, self, true); + + self.scope = try session.executor.startScope(&self.window, self, self); + + // Start a scope (stackframe) for JS Local variables. + Env.HandleScope.init(&self.handle_scope, browser.env.isolate); + errdefer self.handle_scope.deinit(); + + self.scope.enter(); + errdefer self.scope.exit(); // load polyfills try polyfill.load(self.arena, self.scope); diff --git a/src/browser/session.zig b/src/browser/session.zig index 28432498a..97ae61f96 100644 --- a/src/browser/session.zig +++ b/src/browser/session.zig @@ -109,7 +109,11 @@ pub const Session = struct { std.debug.assert(self.page != null); // Reset all existing callbacks. self.browser.app.loop.reset(); + + self.executor.scope.?.exit(); self.executor.endScope(); + + self.page.?.handle_scope.deinit(); self.page = null; // clear netsurf memory arena. diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 19ba3c719..91e2a5d0f 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -555,7 +555,7 @@ const IsolatedWorld = struct { // 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.scope != null) return error.Only1IsolatedContextSupported; - _ = try self.executor.startScope(&page.window, page, {}, false); + _ = try self.executor.startScope(&page.window, page, {}); } }; diff --git a/src/runtime/js.zig b/src/runtime/js.zig index ccd1ab70e..a7f66cc2d 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -321,7 +321,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // when the handle_scope is freed. // We also maintain our own "scope_arena" which allows us to have // all page related memory easily managed. - pub fn startScope(self: *ExecutionWorld, global: anytype, state: State, module_loader: anytype, enter: bool) !*Scope { + pub fn startScope(self: *ExecutionWorld, global: anytype, state: State, module_loader: anytype) !*Scope { std.debug.assert(self.scope == null); const ModuleLoader = switch (@typeInfo(@TypeOf(module_loader))) { @@ -338,76 +338,63 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { const isolate = env.isolate; const Global = @TypeOf(global.*); - var context: v8.Context = blk: { - var temp_scope: v8.HandleScope = undefined; - v8.HandleScope.init(&temp_scope, isolate); - defer temp_scope.deinit(); + // All locals created in this function are temporary + var temp_scope: v8.HandleScope = undefined; + v8.HandleScope.init(&temp_scope, isolate); + defer temp_scope.deinit(); - const js_global = v8.FunctionTemplate.initDefault(isolate); - attachClass(Global, isolate, js_global); + const js_global = v8.FunctionTemplate.initDefault(isolate); + attachClass(Global, isolate, js_global); - const global_template = js_global.getInstanceTemplate(); - global_template.setInternalFieldCount(1); + const global_template = js_global.getInstanceTemplate(); + global_template.setInternalFieldCount(1); - // All the FunctionTemplates that we created and setup in Env.init - // are now going to get associated with our global instance. - const templates = &self.env.templates; - inline for (Types, 0..) |s, i| { - const Struct = s.defaultValue().?; - const class_name = v8.String.initUtf8(isolate, comptime classNameForStruct(Struct)); - global_template.set(class_name.toName(), templates[i], v8.PropertyAttribute.None); - } + // All the FunctionTemplates that we created and setup in Env.init + // are now going to get associated with our global instance. + const templates = &self.env.templates; + inline for (Types, 0..) |s, i| { + const Struct = s.defaultValue().?; + const class_name = v8.String.initUtf8(isolate, comptime classNameForStruct(Struct)); + global_template.set(class_name.toName(), templates[i], v8.PropertyAttribute.None); + } - // The global object (Window) has already been hooked into the v8 - // engine when the Env was initialized - like every other type. - // But the V8 global is its own FunctionTemplate instance so even - // though it's also a Window, we need to set the prototype for this - // specific instance of the the Window. - if (@hasDecl(Global, "prototype")) { - const proto_type = Receiver(@typeInfo(Global.prototype).pointer.child); - const proto_name = @typeName(proto_type); - const proto_index = @field(TYPE_LOOKUP, proto_name).index; - js_global.inherit(templates[proto_index]); - } + // The global object (Window) has already been hooked into the v8 + // engine when the Env was initialized - like every other type. + // But the V8 global is its own FunctionTemplate instance so even + // though it's also a Window, we need to set the prototype for this + // specific instance of the the Window. + if (@hasDecl(Global, "prototype")) { + const proto_type = Receiver(@typeInfo(Global.prototype).pointer.child); + const proto_name = @typeName(proto_type); + const proto_index = @field(TYPE_LOOKUP, proto_name).index; + js_global.inherit(templates[proto_index]); + } - const context_local = v8.Context.init(isolate, global_template, null); - const context = v8.Persistent(v8.Context).init(isolate, context_local).castToContext(); - context.enter(); - errdefer if (enter) context.exit(); - defer if (!enter) context.exit(); - - // This shouldn't be necessary, but it is: - // https://groups.google.com/g/v8-users/c/qAQQBmbi--8 - // TODO: see if newer V8 engines have a way around this. - inline for (Types, 0..) |s, i| { - const Struct = s.defaultValue().?; - - if (@hasDecl(Struct, "prototype")) { - const proto_type = Receiver(@typeInfo(Struct.prototype).pointer.child); - const proto_name = @typeName(proto_type); - if (@hasField(TypeLookup, proto_name) == false) { - @compileError("Type '" ++ @typeName(Struct) ++ "' defines an unknown prototype: " ++ proto_name); - } + const context_local = v8.Context.init(isolate, global_template, null); + const context = v8.Persistent(v8.Context).init(isolate, context_local).castToContext(); + context.enter(); + defer context.exit(); - const proto_index = @field(TYPE_LOOKUP, proto_name).index; - const proto_obj = templates[proto_index].getFunction(context).toObject(); + // This shouldn't be necessary, but it is: + // https://groups.google.com/g/v8-users/c/qAQQBmbi--8 + // TODO: see if newer V8 engines have a way around this. + inline for (Types, 0..) |s, i| { + const Struct = s.defaultValue().?; - const self_obj = templates[i].getFunction(context).toObject(); - _ = self_obj.setPrototype(context, proto_obj); + if (@hasDecl(Struct, "prototype")) { + const proto_type = Receiver(@typeInfo(Struct.prototype).pointer.child); + const proto_name = @typeName(proto_type); + if (@hasField(TypeLookup, proto_name) == false) { + @compileError("Type '" ++ @typeName(Struct) ++ "' defines an unknown prototype: " ++ proto_name); } - } - break :blk context; - }; - // For a Page we only create one HandleScope, it is stored in the main World (enter==true). A page can have multple contexts, 1 for each World. - // The main Context/Scope that enters and holds the HandleScope should therefore always be created first. Following other worlds for this page - // like isolated Worlds, will thereby place their objects on the main page's HandleScope. Note: In the furure the number of context will multiply multiple frames support - var handle_scope: ?v8.HandleScope = null; - if (enter) { - handle_scope = @as(v8.HandleScope, undefined); - v8.HandleScope.init(&handle_scope.?, isolate); + const proto_index = @field(TYPE_LOOKUP, proto_name).index; + const proto_obj = templates[proto_index].getFunction(context).toObject(); + + const self_obj = templates[i].getFunction(context).toObject(); + _ = self_obj.setPrototype(context, proto_obj); + } } - errdefer if (enter) handle_scope.?.deinit(); { // If we want to overwrite the built-in console, we have to @@ -424,7 +411,6 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { .isolate = isolate, .context = context, .templates = &env.templates, - .handle_scope = handle_scope, .call_arena = self.call_arena.allocator(), .scope_arena = self.scope_arena.allocator(), .module_loader = .{ @@ -477,13 +463,14 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { const PersistentObject = v8.Persistent(v8.Object); const PersistentFunction = v8.Persistent(v8.Function); + pub const HandleScope = v8.HandleScope; + // Loosely maps to a Browser Page. pub const Scope = struct { state: State, isolate: v8.Isolate, // This context is a persistent object. The persistent needs to be recovered and reset. context: v8.Context, - handle_scope: ?v8.HandleScope, // references the Env.template array templates: []v8.FunctionTemplate, @@ -568,14 +555,19 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { for (self.callbacks.items) |*cb| { cb.deinit(); } - if (self.handle_scope) |*scope| { - scope.deinit(); - self.context.exit(); - } + var presistent_context = v8.Persistent(v8.Context).recoverCast(self.context); presistent_context.deinit(); } + pub fn enter(self: *Scope) void { + self.context.enter(); + } + + pub fn exit(self: *Scope) void { + self.context.exit(); + } + fn trackCallback(self: *Scope, pf: PersistentFunction) !void { return self.callbacks.append(self.scope_arena, pf); } diff --git a/src/runtime/testing.zig b/src/runtime/testing.zig index 8cfc5c441..1353527fa 100644 --- a/src/runtime/testing.zig +++ b/src/runtime/testing.zig @@ -30,6 +30,7 @@ pub fn Runner(comptime State: type, comptime Global: type, comptime types: anyty return struct { env: *Env, scope: *Env.Scope, + handle_scope: Env.HandleScope, executor: Env.ExecutionWorld, pub const Env = js.Env(State, struct { @@ -52,8 +53,13 @@ pub fn Runner(comptime State: type, comptime Global: type, comptime types: anyty if (Global == void) &default_global else global, state, {}, - true, ); + self.scope.enter(); + + // Start a scope (stackframe) for JS Local variables. + Env.HandleScope.init(&self.handle_scope, self.executor.env.isolate); + errdefer self.handle_scope.deinit(); + return self; }