[dotnet] Source-generate CDP command and response serialization

[RenderMichael]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Adds a common JSON serializer context for CDP commands and responses.

Motivation and Context

Contributes to #14480

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement

---

Description

• Introduced a JSON serialization context for CDP commands and responses.

• Updated DevToolsSession to use the new serialization context.

• Added source generation for serialization in command templates.

• Improved type safety and serialization handling for CDP commands.

---

Changes walkthrough 📝

Relevant files

Enhancement

DevToolsSession.cs Update DevToolsSession to use serialization context            dotnet/src/webdriver/DevTools/DevToolsSession.cs Integrated CdpSerializationContext for JSON serialization. Enhanced SendCommand method to use type-specific serialization. Improved deserialization with command response type mapping. +4/-2      CdpJsonSerializationContext.cs Add CdpSerializationContext for JSON source generation      dotnet/src/webdriver/DevTools/Json/CdpJsonSerializationContext.cs Added a new partial class CdpSerializationContext. Configured JSON source generation for CDP commands and responses. +8/-0      command.hbs Update command templates for JSON serialization                    third_party/dotnet/devtools/src/generator/Templates/command.hbs Added JSON serialization attributes for command settings and responses. Integrated CdpSerializationContext into command templates. +7/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The new serialization code does not appear to handle JSON serialization exceptions that could occur during command serialization or response deserialization var result = await SendCommand(command.CommandName, sessionId, JsonSerializer.SerializeToNode(command, CdpSerializationContext.Default.GetTypeInfo(typeof(TCommand))), cancellationToken, millisecondsTimeout, throwExceptionIfResponseNotReceived).ConfigureAwait(false); if (result == null) { return null; } if (!this.domains.VersionSpecificDomains.ResponseTypeMap.TryGetCommandResponseType(command, out Type commandResponseType)) { throw new InvalidOperationException($"Type {typeof(TCommand)} does not correspond to a known command response type."); } var commandResponseSerialization = CdpSerializationContext.Default.GetTypeInfo(commandResponseType); return (ICommandResponse<TCommand>)result.Value.Deserialize(commandResponseType, commandResponseSerialization); Type Safety The JsonSerializable attribute is using interface types (ICommand and ICommandResponse<>) which may not properly handle all concrete implementations during source generation [JsonSerializable(typeof(ICommand))] [JsonSerializable(typeof(ICommandResponse<>))] [JsonSourceGenerationOptions(Converters = [typeof(StringConverter)])]

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	✅ Add null check for serialization Suggestion Impact: The commit addresses the same concern but implements a different solution by using JsonSerializerOptions instead of directly checking for null serialization info code diff: + private static readonly JsonSerializerOptions s_devToolsSerializerOptions = new() + { + TypeInfoResolver = CdpSerializationContext.Default, + }; + /// <summary> /// Initializes a new instance of the DevToolsSession class, using the specified WebSocket endpoint. /// </summary> @@ -190,7 +195,7 @@ throw new ArgumentNullException(nameof(command)); } - var result = await SendCommand(command.CommandName, sessionId, JsonSerializer.SerializeToNode(command, CdpSerializationContext.Default.GetTypeInfo(typeof(TCommand))), cancellationToken, millisecondsTimeout, throwExceptionIfResponseNotReceived).ConfigureAwait(false); + var result = await SendCommand(command.CommandName, sessionId, JsonSerializer.SerializeToNode(command, s_devToolsSerializerOptions), cancellationToken, millisecondsTimeout, throwExceptionIfResponseNotReceived).ConfigureAwait(false); if (result == null) { @@ -202,8 +207,7 @@ throw new InvalidOperationException($"Type {typeof(TCommand)} does not correspond to a known command response type."); } - var commandResponseSerialization = CdpSerializationContext.Default.GetTypeInfo(commandResponseType); - return (ICommandResponse<TCommand>)result.Value.Deserialize(commandResponseType, commandResponseSerialization); + return (ICommandResponse<TCommand>)result.Value.Deserialize(commandResponseType, s_devToolsSerializerOptions); Add null check for commandResponseSerialization to prevent potential NullReferenceException during deserialization dotnet/src/webdriver/DevTools/DevToolsSession.cs [205-206] var commandResponseSerialization = CdpSerializationContext.Default.GetTypeInfo(commandResponseType); +if (commandResponseSerialization == null) + throw new InvalidOperationException($"Failed to get serialization info for type {commandResponseType}"); return (ICommandResponse<TCommand>)result.Value.Deserialize(commandResponseType, commandResponseSerialization); Apply this suggestion Suggestion importance[1-10]: 8 Why: This is a critical safety check that prevents potential runtime crashes. The null check for commandResponseSerialization is important as GetTypeInfo could potentially return null, leading to a NullReferenceException.	8

[nvborisenko]

dotnet/src/webdriver/DevTools/DevToolsSession.cs(73,91): error CS1002: ; expected

[RenderMichael]

That's my fault, I'm editing this as text because the IDE can't handle it. Syntax should be fixed.

[nvborisenko]

We have 2 options:

• Having only one single CdpSerializationContext for types from all cdp namespaces (v130, v131, v132) (and we know how to resolve collisions)
• Or separate json context per cdp version

I am not deep dived yet. Please suggest which option would be more friendly for us.

[RenderMichael]

I separated the JSON serialization contexts into protocol versions, then I separated each context into request and response. This was still not enough. I will experiment by adding the context to each domain.

[RenderMichael]

It looks to be working now that each ...Adapter has their own context. To accomplish this, I had to move the logic serialization/deserialization logic into each adapter.

This relies on the "JsonNode request, JsonElement response" command overload to be AOT-safe. That means this PR relies on this one: #15159

[RenderMichael]

Above generated code looks like this:

[RenderMichael]

Due to a bug in System.Text.Json I had to import protocolVersion (eg. V130) into each class name dotnet/runtime#72671

Generated code looks like this: