Skip to content

Take scope out of scope #709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Take scope out of scope #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/browser/page.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand All @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/browser/session.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/cdp/cdp.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, {});
}
};

Expand Down
126 changes: 59 additions & 67 deletions src/runtime/js.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand All @@ -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
Expand All @@ -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 = .{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 7 additions & 1 deletion src/runtime/testing.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}

Expand Down