Scripts now properly block rendering

Re-enabled CDP tests

Fixed more tests

Author: karlseguin

src/browser/EventManager.zig

@@ -130,16 +130,6 @@ pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *E
 }
 
 fn dispatchNode(self: *EventManager, target: *Node, event: *Event) !void {
-    if (event._bubbles == false) {
-        event._event_phase = .at_target;
-        const target_et = target.asEventTarget();
-        if (self.lookup.getPtr(@intFromPtr(target_et))) |list| {
-            try self.dispatchPhase(list, target_et, event, null);
-        }
-        event._event_phase = .none;
-        return;
-    }
-
     var path_len: usize = 0;
     var path_buffer: [128]*EventTarget = undefined;
 
@@ -150,7 +140,8 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event) !void {
         path_len += 1;
     }
 
-    // Even though the window isn't part of the DOM, events bubble to it
+    // Even though the window isn't part of the DOM, events always propagate
+    // through it in the capture phase
     if (path_len < path_buffer.len) {
         path_buffer[path_len] = self.page.window.asEventTarget();
         path_len += 1;
@@ -159,6 +150,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event) !void {
     const path = path_buffer[0..path_len];
 
     // Phase 1: Capturing phase (root → target, excluding target)
+    // This happens for all events, regardless of bubbling
     event._event_phase = .capturing_phase;
     var i: usize = path_len;
     while (i > 1) {
@@ -173,6 +165,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event) !void {
         }
     }
 
+    // Phase 2: At target
     event._event_phase = .at_target;
     const target_et = target.asEventTarget();
     if (self.lookup.getPtr(@intFromPtr(target_et))) |list| {
@@ -183,12 +176,16 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event) !void {
         }
     }
 
-    event._event_phase = .bubbling_phase;
-    for (path[1..]) |current_target| {
-        if (self.lookup.getPtr(@intFromPtr(current_target))) |list| {
-            try self.dispatchPhase(list, current_target, event, false);
-            if (event._stop_propagation) {
-                break;
+    // Phase 3: Bubbling phase (target → root, excluding target)
+    // This only happens if the event bubbles
+    if (event._bubbles) {
+        event._event_phase = .bubbling_phase;
+        for (path[1..]) |current_target| {
+            if (self.lookup.getPtr(@intFromPtr(current_target))) |list| {
+                try self.dispatchPhase(list, current_target, event, false);
+                if (event._stop_propagation) {
+                    break;
+                }
             }
         }
     }

src/browser/ScriptManager.zig

@@ -17,6 +17,7 @@
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 const std = @import("std");
+const builtin = @import("builtin");
 
 const js = @import("js/js.zig");
 const log = @import("../log.zig");
@@ -31,11 +32,13 @@ const Element = @import("webapi/Element.zig");
 const Allocator = std.mem.Allocator;
 const ArrayListUnmanaged = std.ArrayListUnmanaged;
 
+const IS_DEBUG = builtin.mode == .Debug;
+
 const ScriptManager = @This();
 
 page: *Page,
 
-// used to prevent recursive evalutaion
+// used to prevent recursive evaluation
 is_evaluating: bool,
 
 // Only once this is true can deferred scripts be run
@@ -45,9 +48,6 @@ static_scripts_done: bool,
 // on shutdown/abort, we need to cleanup any pending ones.
 asyncs: OrderList,
 
-// Normal scripts (non-deferred & non-async). These must be executed in order
-scripts: OrderList,
-
 // List of deferred scripts. These must be executed in order, but only once
 // dom_loaded == true,
 deferreds: OrderList,
@@ -85,7 +85,6 @@ pub fn init(page: *Page) ScriptManager {
     return .{
         .page = page,
         .asyncs = .{},
-        .scripts = .{},
         .deferreds = .{},
         .importmap = .empty,
         .sync_modules = .empty,
@@ -130,7 +129,6 @@ pub fn reset(self: *ScriptManager) void {
     self.importmap = .empty;
 
     self.clearList(&self.asyncs);
-    self.clearList(&self.scripts);
     self.clearList(&self.deferreds);
     self.static_scripts_done = false;
 }
@@ -212,57 +210,78 @@ pub fn add(self: *ScriptManager, script_element: *Element.Html.Script, comptime
         .is_async = if (remote_url == null) false else element.getAttributeSafe("async") != null,
     };
 
-    if (source == .@"inline" and self.scripts.first == null) {
-        // inline script with no pending scripts, execute it immediately.
-        // (if there is a pending script, then we cannot execute this immediately
-        // as it needs to be executed in order)
+    if (source == .@"inline") {
+        // inline script gets executed immediately
         return script.eval(page);
     }
 
-    const pending_script = try self.script_pool.create();
-    errdefer self.script_pool.destroy(pending_script);
-    pending_script.* = .{
-        .script = script,
-        .complete = false,
-        .manager = self,
-        .node = .{},
-    };
+    const pending_script = blk: {
+        // Done in a block this way so that, if something fails in this block
+        // it's cleaned up with these errdefers
+        // BUT, if we need to load/execute the script immediately, cleanup/lifetimes
+        // become the responsibility of the outer block.
+        const pending_script = try self.script_pool.create();
+        errdefer self.script_pool.destroy(pending_script);
+
+        pending_script.* = .{
+            .script = script,
+            .complete = false,
+            .manager = self,
+            .node = .{},
+        };
+        errdefer pending_script.deinit();
 
-    if (source == .@"inline") {
-        // if we're here, it means that we have pending scripts (i.e. self.scripts
-        // is not empty). Because the script is inline, it's complete/ready, but
-        // we need to process them in order
-        pending_script.complete = true;
-        self.scripts.append(&pending_script.node);
-        return;
-    } else {
-        log.debug(.http, "script queue", .{
-            .ctx = ctx,
+        if (comptime IS_DEBUG) {
+            log.debug(.http, "script queue", .{
+                .ctx = ctx,
+                .url = remote_url.?,
+                .stack = page.js.stackTrace() catch "???",
+            });
+        }
+
+        var headers = try self.client.newHeaders();
+        try page.requestCookie(.{}).headersForRequest(page.arena, remote_url.?, &headers);
+
+        try self.client.request(.{
             .url = remote_url.?,
-            .stack = page.js.stackTrace() catch "???",
+            .ctx = pending_script,
+            .method = .GET,
+            .headers = headers,
+            .resource_type = .script,
+            .cookie_jar = &page._session.cookie_jar,
+            .start_callback = if (log.enabled(.http, .debug)) startCallback else null,
+            .header_callback = headerCallback,
+            .data_callback = dataCallback,
+            .done_callback = doneCallback,
+            .error_callback = errorCallback,
         });
-    }
 
-    pending_script.getList().append(&pending_script.node);
+        if (script.is_defer) {
+            // non-blocking loading, track the list this belongs to, and return
+            pending_script.list = &self.deferreds;
+            return;
+        }
 
-    errdefer pending_script.deinit();
+        if (script.is_async) {
+            // non-blocking loading, track the list this belongs to, and return
+            pending_script.list = &self.asyncs;
+            return;
+        }
 
-    var headers = try self.client.newHeaders();
-    try page.requestCookie(.{}).headersForRequest(page.arena, remote_url.?, &headers);
+        break :blk pending_script;
+    };
 
-    try self.client.request(.{
-        .url = remote_url.?,
-        .ctx = pending_script,
-        .method = .GET,
-        .headers = headers,
-        .resource_type = .script,
-        .cookie_jar = &page._session.cookie_jar,
-        .start_callback = if (log.enabled(.http, .debug)) startCallback else null,
-        .header_callback = headerCallback,
-        .data_callback = dataCallback,
-        .done_callback = doneCallback,
-        .error_callback = errorCallback,
-    });
+    defer pending_script.deinit();
+
+    // this is <script src="..."></script>, it needs to block the caller
+    // until it's evaluated
+    var client = self.client;
+    while (true) {
+        if (pending_script.complete) {
+            return pending_script.script.eval(page);
+        }
+        _ = try client.tick(200);
+    }
 }
 
 // Resolve a module specifier to an valid URL.
@@ -394,6 +413,7 @@ pub fn getAsyncModule(self: *ScriptManager, url: [:0]const u8, cb: AsyncModule.C
         .error_callback = AsyncModule.errorCallback,
     });
 }
+
 pub fn pageIsLoaded(self: *ScriptManager) void {
     std.debug.assert(self.static_scripts_done == false);
     self.static_scripts_done = true;
@@ -415,15 +435,6 @@ fn evaluate(self: *ScriptManager) void {
     self.is_evaluating = true;
     defer self.is_evaluating = false;
 
-    while (self.scripts.first) |n| {
-        var pending_script: *PendingScript = @fieldParentPtr("node", n);
-        if (pending_script.complete == false) {
-            return;
-        }
-        defer pending_script.deinit();
-        pending_script.script.eval(page);
-    }
-
     if (self.static_scripts_done == false) {
         // We can only execute deferred scripts if
         // 1 - all the normal scripts are done
@@ -460,7 +471,6 @@ fn evaluate(self: *ScriptManager) void {
 pub fn isDone(self: *const ScriptManager) bool {
     return self.asyncs.first == null and // there are no more async scripts
         self.static_scripts_done and // and we've finished parsing the HTML to queue all <scripts>
-        self.scripts.first == null and // and there are no more <script src=> to wait for
         self.deferreds.first == null; // and there are no more <script defer src=> to wait for
 }
 
@@ -536,12 +546,13 @@ fn parseImportmap(self: *ScriptManager, script: *const Script) !void {
 // A script which is pending execution.
 // It could be pending because:
 //   (a) we're still downloading its content or
-//   (b) this is a non-async script that has to be executed in order
+//   (b) it's a deferred script which has to be executed in order
 pub const PendingScript = struct {
     script: Script,
     complete: bool,
     node: OrderList.Node,
     manager: *ScriptManager,
+    list: ?*std.DoublyLinkedList = null,
 
     fn deinit(self: *PendingScript) void {
         const script = &self.script;
@@ -550,14 +561,11 @@ pub const PendingScript = struct {
         if (script.source == .remote) {
             manager.buffer_pool.release(script.source.remote);
         }
-        self.getList().remove(&self.node);
-    }
 
-    fn remove(self: *PendingScript) void {
-        if (self.node) |*node| {
-            self.getList().remove(node);
-            self.node = null;
+        if (self.list) |list| {
+            list.remove(&self.node);
         }
+        manager.script_pool.destroy(self);
     }
 
     fn startCallback(self: *PendingScript, transfer: *Http.Transfer) !void {
@@ -614,6 +622,7 @@ pub const PendingScript = struct {
             manager.evaluate();
             return;
         }
+
         // async script can be evaluated immediately
         self.script.eval(manager.page);
         self.deinit();
@@ -634,23 +643,6 @@ pub const PendingScript = struct {
 
         manager.evaluate();
     }
-
-    fn getList(self: *const PendingScript) *OrderList {
-        // When a script has both the async and defer flag set, it should be
-        // treated as async. Async is newer, so some websites use both so that
-        // if async isn't known, it'll fallback to defer.
-
-        const script = &self.script;
-        if (script.is_async) {
-            return &self.manager.asyncs;
-        }
-
-        if (script.is_defer) {
-            return &self.manager.deferreds;
-        }
-
-        return &self.manager.scripts;
-    }
 };
 
 const Script = struct {

src/browser/js/Context.zig

@@ -571,16 +571,16 @@ pub fn mapZigInstanceToJs(self: *Context, js_obj_: ?v8.Object, value: anytype) !
             return self.mapZigInstanceToJs(js_obj_, heap);
         },
         .pointer => |ptr| {
-            const gop = try self.identity_map.getOrPut(arena, @intFromPtr(value));
+            const resolved = resolveValue(value);
+
+            const gop = try self.identity_map.getOrPut(arena, @intFromPtr(resolved.ptr));
             if (gop.found_existing) {
                 // we've seen this instance before, return the same
                 // PersistentObject.
                 return gop.value_ptr.*;
             }
 
             const isolate = self.isolate;
-            const resolved = resolveValue(value);
-
             // Sometimes we're creating a new v8.Object, like when
             // we're returning a value from a function. In those cases
             // we have to get the object template, and we can get an object
@@ -1874,11 +1874,6 @@ fn zigJsonToJs(isolate: v8.Isolate, v8_context: v8.Context, value: std.json.Valu
     }
 }
 
-pub fn getGlobalThis(self: *Context) js.This {
-    const js_global = self.v8_context.getGlobal();
-    return .{ .obj = .{ .js_obj = js_global, .context = self } };
-}
-
 // == Misc ==
 // An interface for types that want to have their jsDeinit function to be
 // called when the call context ends

src/browser/js/Env.zig

@@ -241,7 +241,11 @@ pub fn attachClass(comptime JsApi: type, isolate: v8.Isolate, template: v8.Funct
             bridge.Function => {
                 const function_template = v8.FunctionTemplate.initCallback(isolate, value.func);
                 const js_name: v8.Name = v8.String.initUtf8(isolate, name).toName();
-                template_proto.set(js_name, function_template, v8.PropertyAttribute.None);
+                if (value.static) {
+                    template.set(js_name, function_template, v8.PropertyAttribute.None);
+                } else {
+                    template_proto.set(js_name, function_template, v8.PropertyAttribute.None);
+                }
             },
             bridge.Indexed => {
                 const configuration = v8.IndexedPropertyHandlerConfiguration{