Skip to content

[dotnet] [bidi] Dedicated RealmInfoEventArgs type for OnRealmCreated event - #17240

Merged
nvborisenko merged 2 commits into
SeleniumHQ:trunkfrom
nvborisenko:bidi-realminfo-eventargs
Mar 19, 2026
Merged

[dotnet] [bidi] Dedicated RealmInfoEventArgs type for OnRealmCreated event#17240
nvborisenko merged 2 commits into
SeleniumHQ:trunkfrom
nvborisenko:bidi-realminfo-eventargs

Conversation

@nvborisenko

Copy link
Copy Markdown
Member

Now all event args types are really *EventArgs.

💥 What does this PR do?

Refactors the handling of the "realm created" event in the BiDi Script module to support polymorphic event arguments and improve type safety. The main change is the introduction of a new RealmInfoEventArgs hierarchy, replacing the previous RealmInfo type for event subscriptions and serialization. This enables more precise event argument types for different realm kinds and lays groundwork for future extensibility.

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • New feature (non-breaking change which adds functionality and tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Mar 19, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add RealmInfoEventArgs type hierarchy for OnRealmCreated event

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Introduces dedicated RealmInfoEventArgs hierarchy for event subscriptions
• Separates event argument types from data model types
• Implements polymorphic JSON deserialization for realm types
• Improves type safety and consistency across event handlers

Grey Divider

File Changes

1. dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoEventArgsConverter.cs ✨ Enhancement +49/-0

Polymorphic converter for realm event args

• New polymorphic JSON converter for RealmInfoEventArgs deserialization
• Handles 8 realm type discriminators (window, dedicated-worker, shared-worker, service-worker,
 worker, paint-worklet, audio-worklet, worklet)
• Routes JSON to appropriate concrete event args type based on "type" field

dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoEventArgsConverter.cs


2. dotnet/src/webdriver/BiDi/Script/RealmInfoEventArgs.cs ✨ Enhancement +52/-0

New event args types for realm creation

• New abstract base record RealmInfoEventArgs inheriting from EventArgs
• Eight sealed record implementations for different realm types
• Each sealed type includes realm-specific properties (Context, UserContext, Sandbox, Owners)
• Decorated with JsonConverter attribute for polymorphic deserialization

dotnet/src/webdriver/BiDi/Script/RealmInfoEventArgs.cs


3. dotnet/src/webdriver/BiDi/Script/RealmInfo.cs ✨ Enhancement +1/-1

Remove EventArgs from RealmInfo base

• Removed EventArgs inheritance from abstract RealmInfo record
• RealmInfo now serves only as data model, not event argument type
• Maintains existing sealed record implementations unchanged

dotnet/src/webdriver/BiDi/Script/RealmInfo.cs


View more (3)
4. dotnet/src/webdriver/BiDi/Script/IScriptModule.cs ✨ Enhancement +2/-2

Update interface signatures for event args

• Updated OnRealmCreatedAsync method signatures to use RealmInfoEventArgs instead of RealmInfo
• Both async and sync handler overloads updated
• Maintains backward compatibility at interface level

dotnet/src/webdriver/BiDi/Script/IScriptModule.cs


5. dotnet/src/webdriver/BiDi/Script/ScriptModule.cs ✨ Enhancement +15/-4

Update implementation and serialization context

• Updated OnRealmCreatedAsync implementations to use RealmInfoEventArgs
• Changed JSON context reference from RealmInfo to RealmInfoEventArgs
• Added [JsonSerializable] attributes for base and all 8 concrete event args types
• Grouped polymorphic type registrations in region comment

dotnet/src/webdriver/BiDi/Script/ScriptModule.cs


6. dotnet/test/common/BiDi/Script/ScriptEventsTests.cs 🧪 Tests +2/-2

Update test for new event args type

• Updated test to use RealmInfoEventArgs instead of RealmInfo
• Changed assertion to check for WindowRealmInfoEventArgs type
• Maintains test logic and coverage

dotnet/test/common/BiDi/Script/ScriptEventsTests.cs


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. OnRealmCreatedAsync handler type changed 📘 Rule violation ⚙ Maintainability
Description
The public IScriptModule.OnRealmCreatedAsync overloads changed handler parameter type from
RealmInfo to RealmInfoEventArgs, breaking existing consumers at compile time. The old public
overloads were removed without any deprecation path guiding users to the replacement.
Code

dotnet/src/webdriver/BiDi/Script/IScriptModule.cs[R35-36]

+    Task<Subscription> OnRealmCreatedAsync(Func<RealmInfoEventArgs, Task> handler, SubscriptionOptions? options = null, CancellationToken cancellationToken = default);
+    Task<Subscription> OnRealmCreatedAsync(Action<RealmInfoEventArgs> handler, SubscriptionOptions? options = null, CancellationToken cancellationToken = default);
Evidence
PR Compliance ID 1 requires public API compatibility, and PR Compliance ID 2 requires deprecating
before removal; the diff shows the OnRealmCreatedAsync overloads now accept RealmInfoEventArgs
instead of RealmInfo, and the previous RealmInfo-typed overloads are deleted with no deprecation
markers in the interface.

AGENTS.md
AGENTS.md
dotnet/src/webdriver/BiDi/Script/IScriptModule.cs[35-36]

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

## Issue description
`IScriptModule.OnRealmCreatedAsync` changed from `RealmInfo` handlers to `RealmInfoEventArgs` handlers, which is a breaking public API change. The previous overloads were removed without a deprecation period.

## Issue Context
To maintain upgradeability, keep the existing public overloads and mark them `[Obsolete]` with a message pointing to the new `RealmInfoEventArgs` overloads, or provide a forwarding shim that adapts `RealmInfoEventArgs` to the old shape.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Script/IScriptModule.cs[35-36]
- dotnet/src/webdriver/BiDi/Script/ScriptModule.cs[97-105]

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


2. RealmInfo no longer EventArgs 📘 Rule violation ⚙ Maintainability
Description
RealmInfo was changed to no longer derive from EventArgs, which is a breaking change to a public
type's inheritance contract. Any consumer treating RealmInfo as an EventArgs (or using APIs
constrained to EventArgs) will break.
Code

dotnet/src/webdriver/BiDi/Script/RealmInfo.cs[36]

+public abstract record RealmInfo(Realm Realm, string Origin);
Evidence
PR Compliance ID 1 disallows breaking changes to public interfaces/contracts; the diff shows
RealmInfo removed its : EventArgs base type, changing its public inheritance and compatibility
characteristics.

AGENTS.md
dotnet/src/webdriver/BiDi/Script/RealmInfo.cs[36-36]

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

## Issue description
`RealmInfo` no longer inherits from `EventArgs`, which breaks consumers relying on the previous inheritance contract.

## Issue Context
If `RealmInfo` must no longer be used as an event args type internally, consider keeping `RealmInfo` as-is for backward compatibility (including `: EventArgs`) and introducing the new `RealmInfoEventArgs` hierarchy for the event. Alternatively, introduce a deprecation plan before changing/removing the inheritance contract.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/Script/RealmInfo.cs[36-36]
- dotnet/src/webdriver/BiDi/Script/RealmInfoEventArgs.cs[36-52]

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



Remediation recommended

3. Unknown realm type drops event 🐞 Bug ⛯ Reliability
Description
RealmInfoEventArgsConverter returns null for unrecognized realm "type", which makes event
deserialization fail and the realmCreated event get dropped. This breaks OnRealmCreatedAsync
subscriptions if the remote end sends any new/unsupported realm kind.
Code

dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoEventArgsConverter.cs[R31-42]

+        return reader.GetDiscriminator("type") switch
+        {
+            "window" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<WindowRealmInfoEventArgs>()),
+            "dedicated-worker" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<DedicatedWorkerRealmInfoEventArgs>()),
+            "shared-worker" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<SharedWorkerRealmInfoEventArgs>()),
+            "service-worker" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<ServiceWorkerRealmInfoEventArgs>()),
+            "worker" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<WorkerRealmInfoEventArgs>()),
+            "paint-worklet" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<PaintWorkletRealmInfoEventArgs>()),
+            "audio-worklet" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<AudioWorkletRealmInfoEventArgs>()),
+            "worklet" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<WorkletRealmInfoEventArgs>()),
+            _ => null,
+        };
Evidence
The new converter explicitly returns null for the default discriminator case. In the event receive
path, the Broker requires non-null event args; a null result causes an exception and the event is
not enqueued/dispatch to handlers.

dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoEventArgsConverter.cs[29-42]
dotnet/src/webdriver/BiDi/Broker.cs[235-254]
dotnet/src/webdriver/BiDi/Broker.cs[305-315]

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

### Issue description
`RealmInfoEventArgsConverter.Read` returns `null` when the incoming JSON has an unknown/unsupported realm discriminator value. The event pipeline treats a `null` deserialization result as a hard failure and drops the event.

### Issue Context
This is a polymorphic event-args converter for `script.realmCreated`. Returning `null` leads to an exception in the Broker (`Remote end returned null event args...`) and the event never reaches subscribed handlers.

### Fix Focus Areas
- file/path[start_line-end_line]
- dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoEventArgsConverter.cs[29-42]

### Suggested fix
1. Capture the discriminator into a local variable (`var type = reader.GetDiscriminator(&quot;type&quot;);`).
2. Replace the default arm (`_ =&gt; null`) with a thrown `JsonException` that includes the discriminator value (e.g., `throw new JsonException($&quot;Unsupported realm type discriminator &#x27;{type}&#x27;.&quot;);`).

*(Optional follow-up for forward compatibility)*: introduce an `UnknownRealmInfoEventArgs` concrete type and return that instead of throwing, so handlers can still observe the event even when new realm kinds appear.

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


Grey Divider

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

Grey Divider

Qodo Logo

Comment thread dotnet/src/webdriver/BiDi/Script/IScriptModule.cs
Comment thread dotnet/src/webdriver/BiDi/Script/RealmInfo.cs
@nvborisenko
nvborisenko merged commit 4420503 into SeleniumHQ:trunk Mar 19, 2026
21 checks passed
@nvborisenko
nvborisenko deleted the bidi-realminfo-eventargs branch March 19, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants