Merge pull request #583 from lightpanda-io/share-state-and-global-with-the-isolated

Share underlying DOM global Window with the isolated World

Author: karlseguin

src/browser/browser.zig

@@ -272,7 +272,7 @@ pub const Page = struct {
                 .cookie_jar = &session.cookie_jar,
                 .http_client = browser.http_client,
             },
-            .scope = try session.executor.startScope(&self.window, &self.state, self),
+            .scope = try session.executor.startScope(&self.window, &self.state, self, true),
         };
 
         // load polyfills

src/cdp/cdp.zig

@@ -25,6 +25,7 @@ const Env = @import("../browser/env.zig").Env;
 const asUint = @import("../str/parser.zig").asUint;
 const Browser = @import("../browser/browser.zig").Browser;
 const Session = @import("../browser/browser.zig").Session;
+const Page = @import("../browser/browser.zig").Page;
 const Inspector = @import("../browser/env.zig").Env.Inspector;
 const Incrementing = @import("../id.zig").Incrementing;
 const Notification = @import("../notification.zig").Notification;
@@ -359,7 +360,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
             self.node_search_list.reset();
         }
 
-        pub fn createIsolatedWorld(self: *Self) !void {
+        pub fn createIsolatedWorld(self: *Self, page: *Page) !void {
             if (self.isolated_world != null) {
                 return error.CurrentlyOnly1IsolatedWorldSupported;
             }
@@ -369,15 +370,18 @@ pub fn BrowserContext(comptime CDP_T: type) type {
 
             self.isolated_world = .{
                 .name = "",
-                .global = .{},
                 .scope = undefined,
                 .executor = executor,
-                .grant_universal_access = false,
+                .grant_universal_access = true,
             };
             var world = &self.isolated_world.?;
 
-            // TODO: can we do something better than passing `undefined` for the state?
-            world.scope = try world.executor.startScope(&world.global, undefined, {});
+            // The isolate world must share at least some of the state with the related page, specifically the DocumentHTML
+            // (assuming grantUniveralAccess will be set to True!).
+            // 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 untill a new page is created.
+            // Currently we have only 1 page/frame and thus also only 1 state in the isolate world.
+            world.scope = try world.executor.startScope(&page.window, &page.state, {}, false);
         }
 
         pub fn nodeWriter(self: *Self, node: *const Node, opts: Node.Writer.Opts) Node.Writer {
@@ -499,7 +503,6 @@ const IsolatedWorld = struct {
     scope: *Env.Scope,
     executor: Env.Executor,
     grant_universal_access: bool,
-    global: @import("../browser/html/window.zig").Window,
 
     pub fn deinit(self: *IsolatedWorld) void {
         self.executor.deinit();

src/cdp/domains/target.zig

@@ -122,17 +122,10 @@ fn createTarget(cmd: anytype) !void {
 
     const target_id = cmd.cdp.target_id_gen.next();
 
-    try bc.createIsolatedWorld();
     bc.target_id = target_id;
 
-    const page = try bc.session.createPage();
-
-    // The isolate world must share at least some of the state with the related page, specifically the DocumentHTML
-    // (assuming grantUniveralAccess will be set to True!).
-    // 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 untill a new page is created.
-    // Currently we have only 1 page/frame and thus also only 1 state in the isolate world.
-    bc.isolated_world.?.scope.state = &page.state;
+    var page = try bc.session.createPage();
+    try bc.createIsolatedWorld(page);
 
     {
         const aux_data = try std.fmt.allocPrint(cmd.arena, "{{\"isDefault\":true,\"type\":\"default\",\"frameId\":\"{s}\"}}", .{target_id});

src/runtime/js.zig

@@ -328,7 +328,7 @@ pub fn Env(comptime S: type, comptime types: anytype) 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: *Executor, global: anytype, state: State, module_loader: anytype) !*Scope {
+            pub fn startScope(self: *Executor, global: anytype, state: State, module_loader: anytype, enter: bool) !*Scope {
                 std.debug.assert(self.scope == null);
 
                 const ModuleLoader = switch (@typeInfo(@TypeOf(module_loader))) {
@@ -345,61 +345,76 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
                 const isolate = env.isolate;
                 const Global = @TypeOf(global.*);
 
-                var handle_scope: v8.HandleScope = undefined;
-                v8.HandleScope.init(&handle_scope, isolate);
-                errdefer handle_scope.deinit();
+                var context: v8.Context = blk: {
+                    var handle_scope: v8.HandleScope = undefined;
+                    v8.HandleScope.init(&handle_scope, isolate);
+                    defer handle_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 = @field(types, s.name);
-                    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 = @field(types, s.name);
+                        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 = v8.Context.init(isolate, global_template, null);
-                context.enter();
-                errdefer context.exit();
+                    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 = @field(types, s.name);
+
+                        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);
+                            }
 
-                // 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 = @field(types, s.name);
+                            const proto_index = @field(TYPE_LOOKUP, proto_name).index;
+                            const proto_obj = templates[proto_index].getFunction(context).toObject();
 
-                    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 self_obj = templates[i].getFunction(context).toObject();
+                            _ = self_obj.setPrototype(context, proto_obj);
                         }
-
-                        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);
                     }
+                    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);
                 }
+                errdefer if (enter) handle_scope.?.deinit();
 
                 self.scope = Scope{
                     .state = state,
@@ -453,8 +468,9 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
         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,
+            handle_scope: ?v8.HandleScope,
 
             // references the Env.template array
             templates: []v8.FunctionTemplate,
@@ -506,8 +522,12 @@ pub fn Env(comptime S: type, comptime types: anytype) type {
                 for (self.callbacks.items) |*cb| {
                     cb.deinit();
                 }
-                self.context.exit();
-                self.handle_scope.deinit();
+                if (self.handle_scope) |*scope| {
+                    scope.deinit();
+                    self.context.exit();
+                }
+                var presistent_context = v8.Persistent(v8.Context).recoverCast(self.context);
+                presistent_context.deinit();
             }
 
             fn trackCallback(self: *Scope, pf: PersistentFunction) !void {

src/runtime/testing.zig

@@ -49,6 +49,7 @@ pub fn Runner(comptime State: type, comptime Global: type, comptime types: anyty
                 if (Global == void) &default_global else global,
                 state,
                 {},
+                true,
             );
             return self;
         }

src/testing.zig

@@ -437,7 +437,7 @@ pub const JsRunner = struct {
         self.executor = try self.env.newExecutor();
         errdefer self.executor.deinit();
 
-        self.scope = try self.executor.startScope(&self.window, &self.state, {});
+        self.scope = try self.executor.startScope(&self.window, &self.state, {}, true);
         return self;
     }