Merge branch 'main' into agent

src/browser/ScriptManager.zig

@@ -284,8 +284,9 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e
         return;
     }
 
-    if (script.status == 0) {
-        // an error (that we already logged)
+    if (script.status < 200 or script.status > 299) {
+        log.info(.http, "script load error", .{ .status = script.status });
+        script.executeCallback(comptime .wrap("error"));
         script.deinit();
         return;
     }

src/browser/ScriptManagerBase.zig

@@ -687,7 +687,7 @@ pub const Script = struct {
                 const entry = manager.imported_modules.getPtr(self.url).?;
                 entry.state = .err;
             },
-            .frame => {},
+            .frame => self.executeCallback(comptime .wrap("error")),
         }
         self.deinit();
         manager.evaluate();
@@ -792,7 +792,7 @@ pub const Script = struct {
         self.executeCallback(comptime .wrap("error"));
     }
 
-    fn executeCallback(self: *const Script, typ: String) void {
+    pub fn executeCallback(self: *const Script, typ: String) void {
         const fe = self.extra.frame;
         const frame = fe.frame;
         const Event = @import("webapi/Event.zig");

src/browser/tests/page/meta.html

@@ -44,3 +44,19 @@
 
 <!-- Leave it, it used to crash -->
 <script src='empty.js?x=["violated-directive=worker-src","TEST COMPLETE"]'></script>
+
+<!--
+  A blocking <script src> whose fetch returns a non-2xx status must:
+    - NOT execute the response body as JavaScript
+    - Fire an `error` event on the element (and NOT `load`)
+  /404.js returns valid JS (`window.__404_body_executed = true;`) with a
+  404 status, so the regression where we eval'd 4xx bodies would flip
+  __404_body_executed to true.
+-->
+<script>window.__bad404 = { error: false, load: false };</script>
+<script src="404.js" onerror="window.__bad404.error = true" onload="window.__bad404.load = true"></script>
+<script>
+  testing.expectEqual(true, window.__bad404.error);
+  testing.expectEqual(false, window.__bad404.load);
+  testing.expectEqual('undefined', typeof window.__404_body_executed);
+</script>

src/cdp/CDP.zig

@@ -44,7 +44,6 @@ pub const URL_BASE = "chrome://newtab/";
 
 const IS_DEBUG = @import("builtin").mode == .Debug;
 
-const TargetIdGen = Incrementing(u32, "TID");
 const SessionIdGen = Incrementing(u32, "SID");
 const BrowserContextIdGen = Incrementing(u32, "BID");
 
@@ -62,7 +61,6 @@ browser: Browser,
 // when true, any target creation must be attached.
 target_auto_attach: bool = false,
 
-target_id_gen: TargetIdGen = .{},
 session_id_gen: SessionIdGen = .{},
 browser_context_id_gen: BrowserContextIdGen = .{},
 

src/cdp/domains/page.zig

@@ -624,7 +624,8 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N
         }, .{ .session_id = session_id });
     }
 
-    const frame = bc.session.currentFrame() orelse return error.FrameNotLoaded;
+    const root_frame = bc.session.currentFrame() orelse return error.FrameNotLoaded;
+    const is_root_frame = event.frame_id == root_frame._frame_id;
 
     // When we actually recreated the context we should have the inspector send
     // this event, see: resetContextGroup. Sending this event will tell the
@@ -635,10 +636,17 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N
     // frames (iframes), clearing all contexts would destroy the main frame's
     // context, causing Puppeteer's frame.evaluate()/frame.content() to hang
     // forever.
-    if (event.frame_id == frame._frame_id) {
+    if (is_root_frame) {
         try cdp.sendEvent("Runtime.executionContextsCleared", null, .{ .session_id = session_id });
     }
 
+    // Look up the actual navigated frame. For main frame navigations this is
+    // the root frame; for iframes it is the child frame. Using the correct
+    // frame's JS context for inspector.contextCreated prevents re-registering
+    // the root context under a new id (which silently invalidates the
+    // previous id on the V8 side).
+    const frame = bc.session.findFrameByFrameId(event.frame_id) orelse return error.FrameNotFound;
+
     // frameNavigated event
     try cdp.sendEvent("Page.frameNavigated", .{
         .type = "Navigation",
@@ -663,25 +671,32 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N
             "",
             frame.origin orelse "",
             aux_data,
-            true,
+            is_root_frame,
         );
     }
-    for (bc.isolated_worlds.items) |isolated_world| {
-        const aux_json = try std.fmt.allocPrint(arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\",\"loaderId\":\"{s}\"}}", .{ frame_id, loader_id });
+    // Isolated worlds are session-wide (single V8 context shared across
+    // navigations). Only re-register them for main frame navigations;
+    // re-registering during child frame (iframe) navigations would
+    // re-register the same V8 context under a new inspector id, silently
+    // invalidating the id the main frame is using.
+    if (is_root_frame) {
+        for (bc.isolated_worlds.items) |isolated_world| {
+            const aux_json = try std.fmt.allocPrint(arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\",\"loaderId\":\"{s}\"}}", .{ frame_id, loader_id });
 
-        // Calling contextCreated will assign a new Id to the context and send the contextCreated event
+            // Calling contextCreated will assign a new Id to the context and send the contextCreated event
 
-        var ls: js.Local.Scope = undefined;
-        (isolated_world.context orelse continue).localScope(&ls);
-        defer ls.deinit();
+            var ls: js.Local.Scope = undefined;
+            (isolated_world.context orelse continue).localScope(&ls);
+            defer ls.deinit();
 
-        bc.inspector_session.inspector.contextCreated(
-            &ls.local,
-            isolated_world.name,
-            "://",
-            aux_json,
-            false,
-        );
+            bc.inspector_session.inspector.contextCreated(
+                &ls.local,
+                isolated_world.name,
+                "://",
+                aux_json,
+                false,
+            );
+        }
     }
 
     // Evaluate scripts registered via Page.addScriptToEvaluateOnNewDocument.
@@ -705,18 +720,6 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N
         }
     }
 
-    // frameNavigated event
-    try cdp.sendEvent("Page.frameNavigated", .{
-        .type = "Navigation",
-        .frame = CDPFrame{
-            .id = frame_id,
-            .url = event.url,
-            .loaderId = loader_id,
-            .securityOrigin = bc.security_origin,
-            .secureContextType = bc.secure_context_type,
-        },
-    }, .{ .session_id = session_id });
-
     // The DOM.documentUpdated event must be send after the frameNavigated one.
     // chromedp client expects to receive the events is this order.
     // see https://github.com/chromedp/chromedp/issues/1558

src/testing.zig

@@ -650,6 +650,18 @@ fn testHTTPHandler(req: *std.http.Server.Request) !void {
         });
     }
 
+    if (std.mem.eql(u8, path, "/404.js")) {
+        // Valid JS body served with a 404 status. Used to assert that
+        // ScriptManager does NOT execute the body of a failed script
+        // fetch — if it did, window.__404_body_executed would be set.
+        return req.respond("window.__404_body_executed = true;", .{
+            .status = .not_found,
+            .extra_headers = &.{
+                .{ .name = "Content-Type", .value = "application/javascript" },
+            },
+        });
+    }
+
     if (std.mem.eql(u8, path, "/xhr/500")) {
         return req.respond("Internal Server Error", .{
             .status = .internal_server_error,