[dotnet] [bidi] Deserialize message fast instead of defer it

[nvborisenko]

User description

Review after #16402

💥 What does this PR do?

As soon as we received remote message just deserialize it. It also improves memory consumption because of unnecessary
JsonElement object.

Before:

| Method                         | Mean       | Error     | StdDev    | Gen0    | Gen1   | Allocated  |
|------------------------------- |-----------:|----------:|----------:|--------:|-------:|-----------:|
| SeleniumLocateNodesAsync       |   2.770 ms | 0.0507 ms | 0.0335 ms | 19.5313 | 3.9063 |  131.12 KB |
| SeleniumCaptureScreenshotAsync | 118.490 ms | 3.2250 ms | 2.1331 ms |       - |      - | 1837.29 KB |

After:

| Method                         | Mean       | Error     | StdDev    | Gen0    | Allocated  |
|------------------------------- |-----------:|----------:|----------:|--------:|-----------:|
| SeleniumLocateNodesAsync       |   2.699 ms | 0.0435 ms | 0.0288 ms | 11.7188 |   87.83 KB |
| SeleniumCaptureScreenshotAsync | 116.053 ms | 2.1708 ms | 1.4358 ms |       - | 1173.63 KB |

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement

---

Description

• Optimize BiDi message deserialization for better performance

• Replace deferred JsonElement with immediate type-specific deserialization

• Reduce memory allocation by ~33% in node location operations

• Improve screenshot capture performance by ~2ms

---

Diagram Walkthrough

flowchart LR
  A["Received Message"] --> B["Immediate Deserialization"]
  B --> C["Type-specific JsonTypeInfo"]
  C --> D["Reduced Memory Usage"]
  B --> E["Faster Processing"]

Loading

File Walkthrough

Relevant files

Enhancement

Broker.cs Core broker deserialization optimization                                  dotnet/src/webdriver/BiDi/Communication/Broker.cs Replace generic JsonSerializerContext with specific JsonTypeInfo for events Modify CommandInfo to use typed TaskCompletionSource instead of JsonElement Update message processing to deserialize immediately using specific type info Remove unnecessary JsonElement intermediate storage +16/-16  BrowsingContextModule.cs BrowsingContext event subscription updates                              dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs Update all event subscription calls to use specific JsonTypeInfo Replace generic JsonContext with type-specific context properties Apply changes to navigation, download, context, and user prompt events +28/-28  NetworkModule.cs Network event subscription updates                                              dotnet/src/webdriver/BiDi/Network/NetworkModule.cs Update network event subscriptions to use specific JsonTypeInfo Apply changes to request, response, fetch error, and auth events +10/-10  ScriptModule.cs Script event subscription updates                                                dotnet/src/webdriver/BiDi/Script/ScriptModule.cs Update script event subscriptions to use specific JsonTypeInfo Apply changes to message and realm events +6/-6      LogModule.cs Log event subscription updates                                                      dotnet/src/webdriver/BiDi/Log/LogModule.cs Update log entry event subscriptions to use specific JsonTypeInfo +2/-2

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Refactor CommandInfo for better type safety Refactor CommandInfo into an abstract base class and a generic derived class. This will allow storing various command types in the _pendingCommands dictionary while maintaining type safety. dotnet/src/webdriver/BiDi/Communication/Broker.cs [348-356] -class CommandInfo<TResult>(long id, TaskCompletionSource<TResult> taskCompletionSource, JsonTypeInfo jsonResultTypeInfo) - where TResult : EmptyResult +abstract class CommandInfo(long id, JsonTypeInfo jsonResultTypeInfo) { public long Id { get; } = id; - public TaskCompletionSource<TResult> TaskCompletionSource { get; } = taskCompletionSource; + public JsonTypeInfo JsonResultTypeInfo { get; } = jsonResultTypeInfo; - public JsonTypeInfo JsonResultTypeInfo { get; } = jsonResultTypeInfo; -}; + public abstract void SetResult(object result); +} +class CommandInfo<TResult> : CommandInfo + where TResult : EmptyResult +{ + public TaskCompletionSource<TResult> TaskCompletionSource { get; } + + public CommandInfo(long id, TaskCompletionSource<TResult> taskCompletionSource, JsonTypeInfo jsonResultTypeInfo) + : base(id, jsonResultTypeInfo) + { + TaskCompletionSource = taskCompletionSource; + } + + public override void SetResult(object result) + { + TaskCompletionSource.SetResult((TResult)result); + } +} + Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion provides a complete and robust solution to a significant type-safety flaw in the PR by refactoring CommandInfo into a base and generic derived class, which is an excellent design pattern for this scenario.	High
	✅ Use a non-generic base class Suggestion Impact: The commit replaced ConcurrentDictionary> with ConcurrentDictionary and updated CommandInfo from generic to non-generic, aligning with the suggestion. code diff: - private readonly ConcurrentDictionary<long, CommandInfo<EmptyResult>> _pendingCommands = new(); - private readonly BlockingCollection<MessageEvent> _pendingEvents = []; + private readonly ConcurrentDictionary<long, CommandInfo> _pendingCommands = new(); + private readonly BlockingCollection<(string Method, EventArgs Params)> _pendingEvents = []; private readonly Dictionary<string, JsonTypeInfo> _eventTypesMap = []; private readonly ConcurrentDictionary<string, List<EventHandler>> _eventHandlers = new(); @@ -147,7 +147,7 @@ var timeout = options?.Timeout ?? TimeSpan.FromSeconds(30); using var cts = new CancellationTokenSource(timeout); cts.Token.Register(() => tcs.TrySetCanceled(cts.Token)); - var commandInfo = new CommandInfo<EmptyResult>(command.Id, tcs, jsonResultTypeInfo); + var commandInfo = new CommandInfo(command.Id, tcs, jsonResultTypeInfo); _pendingCommands[command.Id] = commandInfo; Modify _pendingCommands to use a non-generic base CommandInfo class. This will allow storing different command result types in the dictionary, improving type safety. dotnet/src/webdriver/BiDi/Communication/Broker.cs [40] -private readonly ConcurrentDictionary<long, CommandInfo<EmptyResult>> _pendingCommands = new(); +private readonly ConcurrentDictionary<long, CommandInfo> _pendingCommands = new(); [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a type safety issue where _pendingCommands is constrained to CommandInfo<EmptyResult>, which forces an unsafe cast later. Proposing a non-generic base class is a valid and robust design pattern to solve this problem.	Medium
Learned best practice	Add null/empty guards for inputs Guard against null eventName and jsonTypeInfo before using them to avoid ArgumentNullException later in dictionary access and deserialization. dotnet/src/webdriver/BiDi/Communication/Broker.cs [159-162] public async Task<Subscription> SubscribeAsync<TEventArgs>(string eventName, Action<TEventArgs> action, SubscriptionOptions? options, JsonTypeInfo jsonTypeInfo) where TEventArgs : EventArgs { + if (string.IsNullOrWhiteSpace(eventName)) throw new ArgumentException("eventName is required.", nameof(eventName)); + if (jsonTypeInfo is null) throw new ArgumentNullException(nameof(jsonTypeInfo)); _eventTypesMap[eventName] = jsonTypeInfo; Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate inputs and states early to prevent null dereferences and logic errors.	Low
Update

[nvborisenko]

Finally, no Gen2 and Gen1 allocations.

[RenderMichael]

Two tiny comments, in general I really like this change! One more leap towards great performance.