[dotnet] [bidi] Keep events deserialization at transport layer

[nvborisenko]

message -> deserialize -> queue

So we process incoming messages as fast as possible, letting transport layer to reuse memory (upcoming PRs).

💥 What does this PR do?

This pull request refactors how BiDi events are processed and dispatched, focusing on improving type safety and simplifying event handling. The main changes involve replacing manual byte array slicing and deserialization with direct use of typed EventArgs objects, and consolidating event registration logic for clarity and maintainability.

🔄 Types of changes

• Cleanup (formatting, renaming)
• New feature (non-breaking change which adds functionality and tests!)

[qodo-code-review]

Review Summary by Qodo

Move BiDi event deserialization to transport layer for faster processing

✨ Enhancement

Walkthroughs

Description

• Move BiDi event deserialization from background thread to transport layer
• Deserialize events immediately upon receipt with typed EventArgs objects
• Simplify PendingEvent structure by removing JSON byte data
• Add TryGetJsonTypeInfo method for event type lookup and validation

File Changes

1. dotnet/src/webdriver/BiDi/Broker.cs ✨ Enhancement +19/-9

Deserialize BiDi events at transport layer

• Replace byte index tracking (paramsStartIndex, paramsEndIndex) with Utf8JsonReader snapshot
• Deserialize event params directly in ProcessReceivedMessage using JsonSerializer.Deserialize
• Validate event type mapping before deserialization with early exit on unknown events
• Set BiDi property on deserialized EventArgs before enqueueing

dotnet/src/webdriver/BiDi/Broker.cs

---

2. dotnet/src/webdriver/BiDi/EventDispatcher.cs ✨ Enhancement +21/-23

Simplify event dispatching with pre-deserialized args

• Rename _events to _eventRegistrations for clarity
• Change EnqueueEvent signature to accept EventArgs instead of raw JSON bytes
• Remove deserialization logic from ProcessEventsAwaiterAsync background thread
• Add public TryGetJsonTypeInfo method to retrieve type info for event validation
• Simplify PendingEvent record to contain only method name and EventArgs

dotnet/src/webdriver/BiDi/EventDispatcher.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📎 Requirement gaps (0)

1. No tests for event deserialization 📘 Rule violation ✓ Correctness

Description

Event handling behavior was materially changed by deserializing params into typed EventArgs at
the transport layer and changing the dispatcher queue to carry EventArgs. No corresponding tests
were added/updated in this PR diff to validate the new behavior and prevent regressions.

Code

dotnet/src/webdriver/BiDi/Broker.cs[R222-237]

+                if (!_eventDispatcher.TryGetJsonTypeInfo(method, out var jsonTypeInfo))
+                {
+                    if (_logger.IsEnabled(LogEventLevel.Warn))
+                    {
+                        _logger.Warn($"Received BiDi event with method '{method}', but no event type mapping was found. Event will be ignored. Message content: {System.Text.Encoding.UTF8.GetString(data)}");
+                    }
+
+                    break;
+                }
+
+                var eventArgs = JsonSerializer.Deserialize(ref paramsReader, jsonTypeInfo) as EventArgs
+                    ?? throw new BiDiException("Remote end returned null event args in the 'params' property.");
+
+                eventArgs.BiDi = _bidi;
+
+                _eventDispatcher.EnqueueEvent(method, eventArgs);

Evidence

PR Compliance ID 4 requires behavior changes to be accompanied by tests when reasonably possible.
The PR description and code changes show a non-trivial refactor of event deserialization/dispatch,
but the provided PR diff contains only production code changes and no test updates.

AGENTS.md
dotnet/src/webdriver/BiDi/Broker.cs[222-237]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[72-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
BiDi event processing was refactored to deserialize `params` into typed `EventArgs` earlier and queue typed objects, but there are no accompanying tests validating this new behavior.

## Issue Context
Prefer small/unit tests that do not require a real browser. Key scenarios include: successful deserialization and handler invocation, missing/unknown type mappings (event ignored), and null/invalid `params` payload handling.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Broker.cs[119-237]
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[51-126]
- dotnet/test/common/Selenium.WebDriver.Common.Tests.csproj[1-39]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unvalidated params reader 🐞 Bug ✓ Correctness

Description

Broker.ProcessReceivedMessage deserializes events using paramsReader without verifying the
incoming message contained a params property, so paramsReader may remain default and cause
deserialization exceptions on the transport thread. This can turn a single malformed/partial event
into repeated error logs and dropped messages.

Code

dotnet/src/webdriver/BiDi/Broker.cs[R232-233]

+                var eventArgs = JsonSerializer.Deserialize(ref paramsReader, jsonTypeInfo) as EventArgs
+                    ?? throw new BiDiException("Remote end returned null event args in the 'params' property.");

Evidence

paramsReader starts as default and is only assigned when a "params" property is encountered,
but the event path always attempts to deserialize from it once an event type mapping exists.

dotnet/src/webdriver/BiDi/Broker.cs[125-166]
dotnet/src/webdriver/BiDi/Broker.cs[219-237]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Broker.ProcessReceivedMessage` can call `JsonSerializer.Deserialize(ref paramsReader, ...)` when `paramsReader` was never initialized because the message had no `params` property. This leads to runtime exceptions originating from `System.Text.Json` rather than a clear BiDi protocol error.

### Issue Context
`paramsReader` is initialized to `default` and only set when a `"params"` property is parsed. The `TypeEvent` switch case currently does not validate that `params` was present before deserializing.

### Fix Focus Areas
- dotnet/src/webdriver/BiDi/Broker.cs[125-166]
- dotnet/src/webdriver/BiDi/Broker.cs[219-237]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Warn logs full message bytes 📘 Rule violation ⛨ Security

Description

New warning logging includes full incoming message content via Encoding.UTF8.GetString(data),
which can leak sensitive data and produce noisy/large logs. The compliance checklist requires
avoiding sensitive/nullable messages in logs and preferring stable identifiers.

Code

dotnet/src/webdriver/BiDi/Broker.cs[R224-227]

+                    if (_logger.IsEnabled(LogEventLevel.Warn))
+                    {
+                        _logger.Warn($"Received BiDi event with method '{method}', but no event type mapping was found. Event will be ignored. Message content: {System.Text.Encoding.UTF8.GetString(data)}");
+                    }

Evidence

PR Compliance ID 13 requires resilient logging that avoids potentially sensitive message content.
The new warning log for unknown events prints the entire raw message payload (GetString(data)),
rather than logging stable identifiers (e.g., method name only, size, ids) or a sanitized/truncated
payload.

dotnet/src/webdriver/BiDi/Broker.cs[224-227]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new warning log for unknown BiDi events logs the full incoming message body (`Encoding.UTF8.GetString(data)`), which may leak sensitive data and create very large/noisy logs.

## Issue Context
Prefer logging stable identifiers (e.g., `method`, payload size, maybe a correlation id if present) and avoid emitting full raw payloads at `Warn`.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Broker.cs[222-227]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. // snapshot lacks rationale 📘 Rule violation ⛯ Reliability

Description

New comments like // snapshot restate what the assignment does without explaining why the snapshot
is needed. This reduces maintainability and violates the guidance to document intent/rationale
instead of obvious mechanics.

Code

dotnet/src/webdriver/BiDi/Broker.cs[R162-166]

            else if (reader.ValueTextEquals("params"u8))
            {
                reader.Read();
-                paramsStartIndex = (int)reader.TokenStartIndex;
-                isParams = true;
+                paramsReader = reader; // snapshot
            }

Evidence

PR Compliance ID 8 requires comments to explain the "why". The added // snapshot comments do not
capture intent (e.g., that Utf8JsonReader is advanced later and the snapshot preserves the
position for later deserialization).

AGENTS.md
dotnet/src/webdriver/BiDi/Broker.cs[159-166]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The added `// snapshot` comments do not explain intent/rationale and mainly restate the code.

## Issue Context
Comments should capture the reason/constraint (e.g., reader advancement) rather than narrating an assignment.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Broker.cs[157-166]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[Copilot]

Pull request overview

This PR refactors BiDi event deserialization in the .NET binding, moving it from the background event-processing thread to the transport/message-receiving layer in Broker. This simplifies EventDispatcher by having it work with already-deserialized EventArgs objects instead of raw byte arrays.

Changes:

• Deserialization of event params moved from EventDispatcher.ProcessEventsAwaiterAsync to Broker.ProcessReceivedMessage, using a snapshot Utf8JsonReader instead of byte-slicing.
• EventDispatcher exposes TryGetJsonTypeInfo so the broker can obtain the correct type info for deserialization.
• PendingEvent simplified from carrying raw bytes + type info to carrying a typed EventArgs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
dotnet/src/webdriver/BiDi/EventDispatcher.cs	Simplified EnqueueEvent to accept EventArgs, added TryGetJsonTypeInfo, renamed dictionary field
dotnet/src/webdriver/BiDi/Broker.cs	Moved deserialization logic here using Utf8JsonReader snapshot, replaced byte-slicing approach