Merge pull request #3595 from cdebled/cdp-reply-session-id

cdp: echo the sessionId in replies to browser-level commands

src/server/cdp/CDP.zig

@@ -1397,14 +1397,12 @@ pub const Command = struct {
         return self.browser_context.?;
     }
 
-    const SendResultOpts = struct {
-        include_session_id: bool = true,
-    };
-    pub fn sendResult(self: *Command, result: anytype, opts: SendResultOpts) !void {
+    const SendResultOpts = struct {};
+    pub fn sendResult(self: *Command, result: anytype, _: SendResultOpts) !void {
         return self.sender.sendJSON(.{
             .id = self.input.id,
             .result = if (comptime @typeInfo(@TypeOf(result)) == .null) struct {}{} else result,
-            .sessionId = if (opts.include_session_id) self.input.session_id else null,
+            .sessionId = self.input.session_id,
         });
     }
 
@@ -1413,14 +1411,12 @@ pub const Command = struct {
         return self.cdp.sendEvent(method, p, opts);
     }
 
-    const SendErrorOpts = struct {
-        include_session_id: bool = true,
-    };
-    pub fn sendError(self: *Command, code: i32, message: []const u8, opts: SendErrorOpts) !void {
+    const SendErrorOpts = struct {};
+    pub fn sendError(self: *Command, code: i32, message: []const u8, _: SendErrorOpts) !void {
         return self.sender.sendJSON(.{
             .id = self.input.id,
             .@"error" = .{ .code = code, .message = message },
-            .sessionId = if (opts.include_session_id) self.input.session_id else null,
+            .sessionId = self.input.session_id,
         });
     }
 

src/server/cdp/domains/browser.zig

@@ -74,7 +74,7 @@ fn getVersion(cmd: *CDP.Command) !void {
         .revision = REVISION,
         .userAgent = CDP_USER_AGENT,
         .jsVersion = JS_VERSION,
-    }, .{ .include_session_id = false });
+    }, .{});
 }
 
 // https://chromedevtools.github.io/devtools-protocol/tot/Browser/#method-setDownloadBehavior
@@ -195,7 +195,7 @@ fn grantPermissions(cmd: *CDP.Command) !void {
         try browser.setPermission(name, .granted);
     }
 
-    return cmd.sendResult(null, .{ .include_session_id = false });
+    return cmd.sendResult(null, .{});
 }
 
 // Set a single permission to an explicit state ("granted", "denied" or
@@ -219,14 +219,14 @@ fn setPermission(cmd: *CDP.Command) !void {
         return error.InvalidPermissionSetting;
     };
     try cmd.cdp.browser.setPermission(params.permission.name, state);
-    return cmd.sendResult(null, .{ .include_session_id = false });
+    return cmd.sendResult(null, .{});
 }
 
 // Clear all granted permissions; navigator.permissions.query() falls back to
 // the default "prompt".
 fn resetPermissions(cmd: *CDP.Command) !void {
     cmd.cdp.browser.clearPermissions();
-    return cmd.sendResult(null, .{ .include_session_id = false });
+    return cmd.sendResult(null, .{});
 }
 
 const testing = @import("../testing.zig");
@@ -249,6 +249,16 @@ test "cdp.browser: getVersion" {
     }, .{ .id = 32, .index = 0, .session_id = null });
 }
 
+// Clients route replies by (sessionId, id): a reply must echo the sessionId of its command.
+test "cdp.browser: replies echo the sessionId" {
+    var ctx = try testing.context();
+    defer ctx.deinit();
+
+    _ = try ctx.loadBrowserContext(.{ .session_id = "SID-X" });
+    try ctx.processMessage(.{ .id = 1, .method = "Browser.getVersion", .sessionId = "SID-X" });
+    try ctx.expectSentResult(.{ .product = PRODUCT }, .{ .id = 1, .session_id = "SID-X" });
+}
+
 test "cdp.browser: getWindowForTarget" {
     var ctx = try testing.context();
     defer ctx.deinit();

src/server/cdp/domains/target.zig

@@ -68,13 +68,13 @@ fn getTargets(cmd: *CDP.Command) !void {
     const bc = cmd.browser_context orelse {
         return cmd.sendResult(.{
             .targetInfos = [_]TargetInfo{},
-        }, .{ .include_session_id = false });
+        }, .{});
     };
 
     const target_id = &(bc.target_id orelse {
         return cmd.sendResult(.{
             .targetInfos = [_]TargetInfo{},
-        }, .{ .include_session_id = false });
+        }, .{});
     });
 
     return cmd.sendResult(.{
@@ -86,7 +86,7 @@ fn getTargets(cmd: *CDP.Command) !void {
             .attached = true,
             .canAccessOpener = false,
         }},
-    }, .{ .include_session_id = false });
+    }, .{});
 }
 
 fn getBrowserContexts(cmd: *CDP.Command) !void {
@@ -99,7 +99,7 @@ fn getBrowserContexts(cmd: *CDP.Command) !void {
 
     return cmd.sendResult(.{
         .browserContextIds = browser_context_ids,
-    }, .{ .include_session_id = false });
+    }, .{});
 }
 
 fn createBrowserContext(cmd: *CDP.Command) !void {
@@ -316,7 +316,7 @@ fn closeTarget(cmd: *CDP.Command) !void {
     // can't be null if we have a target_id
     lp.assert(bc.session.hasPage(), "CDP.target.closeTarget null frame", .{});
 
-    try cmd.sendResult(.{ .success = true }, .{ .include_session_id = false });
+    try cmd.sendResult(.{ .success = true }, .{});
 
     for (bc.attached_sessions.items) |session| {
         bc.fetchDisableForSession(session.id);
@@ -382,7 +382,7 @@ fn getTargetInfo(cmd: *CDP.Command) !void {
                 .attached = true,
                 .canAccessOpener = false,
             },
-        }, .{ .include_session_id = false });
+        }, .{});
     }
 
     return cmd.sendResult(.{
@@ -394,7 +394,7 @@ fn getTargetInfo(cmd: *CDP.Command) !void {
             .attached = true,
             .canAccessOpener = false,
         },
-    }, .{ .include_session_id = false });
+    }, .{});
 }
 
 fn sendMessageToTarget(cmd: *CDP.Command) !void {