[dotnet] [bidi] Revise event args#17304
Merged
nvborisenko merged 28 commits intoSeleniumHQ:trunkfrom Apr 5, 2026
Merged
Conversation
15 tasks
Contributor
Review Summary by Qodo[dotnet] [BiDi] Refactor event subscription with type-safe event arguments and factory pattern
WalkthroughsDescriptionComprehensive refactoring of the BiDi event subscription and dispatching mechanism to improve type safety, extensibility, and maintainability: * **Event Subscription Architecture**: Refactored Broker class to use a new EventMetadata record pattern, enabling flexible and type-safe event argument creation through factory functions instead of direct type references. * **Type-Safe Event Arguments**: Introduced specific, non-serializable public event argument types for all BiDi events (e.g., ContextCreatedEventArgs, NavigationStartedEventArgs, FragmentNavigatedEventArgs, EntryAddedEventArgs, RealmCreatedEventArgs, FileDialogOpenedEventArgs), replacing generic base types and preventing future breaking changes. * **Polymorphic Deserialization**: Implemented custom JSON converters (LogEntryConverter, DownloadEndParamsConverter, RealmInfoConverter, RemoteValueConverter, EvaluateResultConverter) for proper handling of polymorphic event parameter types during deserialization. * **Constructor-Based Dependency Injection**: Updated all event argument types to accept IBiDi parameter through constructor injection, simplifying the event args base class and removing lazy initialization logic. * **Simplified EventDispatcher**: Removed subscription logic and session provider dependency from EventDispatcher, delegating to Broker for metadata management and using `Func<EventArgs, ValueTask>` delegates for handler management. * **Factory Method Pattern**: Added factory methods in all module classes (BrowsingContextModule, NetworkModule, ScriptModule, LogModule, InputModule, SpeculationModule) to construct typed event arguments from deserialized parameters. * **Consistent Naming**: Renamed event argument types to match their event names (e.g., LogEntryEventArgs → EntryAddedEventArgs, FileDialogEventArgs → FileDialogOpenedEventArgs, RealmInfoEventArgs → RealmCreatedEventArgs). * **Test Updates**: Updated all BiDi tests to use new event argument types and verify correct event handling with the refactored architecture. File Changes1. dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs
|
Contributor
Code Review by Qodo
1. IBrowsingContextModule handler types changed
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the .NET BiDi event subscription/dispatch pipeline to construct non-serializable public *EventArgs types from spec-serializable parameter models, improving type safety and reducing future breaking changes.
Changes:
- Reworks
Broker/EventDispatchersubscription handling to use event metadata + factories for event-args creation. - Splits event payloads into internal
*Parameters/*Infomodels and public*EventArgs(carryingIBiDi) across modules. - Updates BiDi tests and module interfaces to use the new strongly-typed event args.
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/test/webdriver/BiDi/Script/ScriptEventsTests.cs | Updates Script event tests for RealmCreatedEventArgs types. |
| dotnet/test/webdriver/BiDi/Script/ScriptCommandsTests.cs | Updates log event test wiring to EntryAddedEventArgs. |
| dotnet/test/webdriver/BiDi/Log/LogTests.cs | Updates Log module tests to new EntryAddedEventArgs hierarchy. |
| dotnet/test/webdriver/BiDi/Input/InputEventsTests.cs | Updates Input event test to FileDialogOpenedEventArgs. |
| dotnet/src/webdriver/BiDi/Subscription.cs | Subscription now tracks broker + event name + handler delegate. |
| dotnet/src/webdriver/BiDi/Speculation/SpeculationModule.cs | Subscribes using params model + event-args factory. |
| dotnet/src/webdriver/BiDi/Speculation/PrefetchStatusUpdatedEventArgs.cs | Splits event args vs serializable parameters model. |
| dotnet/src/webdriver/BiDi/Script/ScriptModule.cs | Updates Script subscriptions and adds event-args factories. |
| dotnet/src/webdriver/BiDi/Script/RemoteValue.cs | Moves polymorphic RemoteValue converter into Script namespace/file. |
| dotnet/src/webdriver/BiDi/Script/RealmInfoEventArgs.cs | Replaces RealmInfoEventArgs with RealmCreatedEventArgs hierarchy. |
| dotnet/src/webdriver/BiDi/Script/RealmInfo.cs | Adds internal RealmInfoConverter for polymorphic realm shapes. |
| dotnet/src/webdriver/BiDi/Script/RealmDestroyedEventArgs.cs | Adds IBiDi to event args; adds parameters record. |
| dotnet/src/webdriver/BiDi/Script/MessageEventArgs.cs | Adds IBiDi to event args; adds parameters record. |
| dotnet/src/webdriver/BiDi/Script/IScriptModule.cs | Updates public interface to new event-args types. |
| dotnet/src/webdriver/BiDi/Script/EvaluateCommand.cs | Adds in-file EvaluateResultConverter; reshapes parameters record. |
| dotnet/src/webdriver/BiDi/Network/ResponseStartedEventArgs.cs | Splits public args from internal params; adds IBiDi and UserContext. |
| dotnet/src/webdriver/BiDi/Network/ResponseCompletedEventArgs.cs | Splits public args from internal params; adds IBiDi and UserContext. |
| dotnet/src/webdriver/BiDi/Network/NetworkModule.cs | Switches network subscriptions to params + factories. |
| dotnet/src/webdriver/BiDi/Network/FetchErrorEventArgs.cs | Splits public args from internal params; adds IBiDi and UserContext. |
| dotnet/src/webdriver/BiDi/Network/BeforeRequestSentEventArgs.cs | Splits public args from internal params; adds IBiDi and UserContext. |
| dotnet/src/webdriver/BiDi/Network/BaseParameters.cs | Replaces BaseParametersEventArgs with internal BaseParameters record. |
| dotnet/src/webdriver/BiDi/Network/AuthRequiredEventArgs.cs | Splits public args from internal params; adds IBiDi and UserContext. |
| dotnet/src/webdriver/BiDi/Module.cs | Updates SubscribeAsync helpers to accept params + factory. |
| dotnet/src/webdriver/BiDi/Log/LogModule.cs | Switches log subscription to deserialize LogEntry + factory to event args. |
| dotnet/src/webdriver/BiDi/Log/LogEntryEventArgs.cs | Removes old serializable event-args model. |
| dotnet/src/webdriver/BiDi/Log/ILogModule.cs | Updates interface to EntryAddedEventArgs. |
| dotnet/src/webdriver/BiDi/Log/EntryAddedEventArgs.cs | Introduces new public EntryAddedEventArgs + internal LogEntry models + converter. |
| dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RemoteValueConverter.cs | Removes old converter location (moved into Script). |
| dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoEventArgsConverter.cs | Removes old converter for now-removed RealmInfoEventArgs. |
| dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/RealmInfoConverter.cs | Removes old converter location (moved into Script). |
| dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/LogEntryEventArgsConverter.cs | Removes old converter for removed LogEntryEventArgs. |
| dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/EvaluateResultConverter.cs | Removes old converter location (moved into Script). |
| dotnet/src/webdriver/BiDi/Json/Converters/Polymorphic/DownloadEndEventArgsConverter.cs | Removes old converter (replaced with params converter in BrowsingContext). |
| dotnet/src/webdriver/BiDi/Input/InputModule.cs | Switches input subscription to params + factory and new event args. |
| dotnet/src/webdriver/BiDi/Input/IInputModule.cs | Updates interface to FileDialogOpenedEventArgs. |
| dotnet/src/webdriver/BiDi/Input/FileDialogOpenedEventArgs.cs | Adds new public event args + internal FileDialogInfo params model. |
| dotnet/src/webdriver/BiDi/EventHandler.cs | Removes old EventHandler abstraction (replaced with delegates). |
| dotnet/src/webdriver/BiDi/EventDispatcher.cs | Reworks dispatcher to store delegate handlers per event name. |
| dotnet/src/webdriver/BiDi/EventArgs.cs | Changes base EventArgs to carry IBiDi via constructor. |
| dotnet/src/webdriver/BiDi/BrowsingContext/UserPromptType.cs | Fixes file contents/namespace; defines UserPromptType enum. |
| dotnet/src/webdriver/BiDi/BrowsingContext/UserPromptOpenedEventArgs.cs | Adds IBiDi + internal parameters record. |
| dotnet/src/webdriver/BiDi/BrowsingContext/UserPromptClosedEventArgs.cs | Adds IBiDi + internal parameters record. |
| dotnet/src/webdriver/BiDi/BrowsingContext/NavigationStartedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/NavigationInfo.cs | Formats navigation payload model. |
| dotnet/src/webdriver/BiDi/BrowsingContext/NavigationFailedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/NavigationEventArgs.cs | Makes navigation base args carry IBiDi. |
| dotnet/src/webdriver/BiDi/BrowsingContext/NavigationCommittedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/NavigationAbortedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/LoadEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/Info.cs | Renames browsing context info model to Info. |
| dotnet/src/webdriver/BiDi/BrowsingContext/IBrowsingContextModule.cs | Updates interface event signatures to typed event args. |
| dotnet/src/webdriver/BiDi/BrowsingContext/IBrowsingContextLogModule.cs | Updates interface to EntryAddedEventArgs. |
| dotnet/src/webdriver/BiDi/BrowsingContext/IBrowsingContextInputModule.cs | Updates interface to FileDialogOpenedEventArgs. |
| dotnet/src/webdriver/BiDi/BrowsingContext/HistoryUpdatedEventArgs.cs | Adds IBiDi; introduces separate parameters record. |
| dotnet/src/webdriver/BiDi/BrowsingContext/GetTreeCommand.cs | Updates GetTree result model to use Info. |
| dotnet/src/webdriver/BiDi/BrowsingContext/FragmentNavigatedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/DownloadWillBeginEventArgs.cs | Adds IBiDi; introduces internal params model. |
| dotnet/src/webdriver/BiDi/BrowsingContext/DownloadEndEventArgs.cs | Splits public args from internal params + params converter. |
| dotnet/src/webdriver/BiDi/BrowsingContext/DomContentLoadedEventArgs.cs | Replaces old args with typed navigation event args. |
| dotnet/src/webdriver/BiDi/BrowsingContext/ContextDestroyedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/ContextCreatedEventArgs.cs | Adds dedicated typed event args class. |
| dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextModule.cs | Updates all event subscriptions to params + factories + typed args. |
| dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextLogModule.cs | Updates context-scoped log event handling to new args type. |
| dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextInputModule.cs | Updates context-scoped input event handling to new args type. |
| dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs | Updates context instance event helpers to typed event args. |
| dotnet/src/webdriver/BiDi/Broker.cs | Adds event metadata + args factories; moves unsubscribe into broker. |
| dotnet/src/webdriver/BiDi/BiDi.cs | Updates broker construction to new signature. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 67 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
dotnet/src/webdriver/BiDi/BrowsingContext/Info.cs:29
- Renaming the public
BrowsingContextInfotype to a very generic public type nameInfois a source-breaking change for consumers and also reduces API clarity/discoverability (e.g.,BrowsingContext.Infois hard to search for and can collide with otherInfotypes). If the intent is to keep the wire/serialization shape internal, consider keepingBrowsingContextInfoas the public type (or providing a backwards-compatible shim/obsolete alias) and using a separate internal parameters type for serialization.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 67 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
dotnet/src/webdriver/BiDi/BrowsingContext/Info.cs:29
Infois a very generic public type name for a user-facing API (it’s now used inGetTreeResultand context-created/destroyed event args). Consider using a more specific name (e.g.,BrowsingContextInfoorBrowsingContextTreeInfo) to avoid ambiguity and improve discoverability.
nvborisenko
changed the title
Member
Author
|
CI fails: This is because tests are not isolated. We have item in backlog how to improve and fix all flaky tests. Merging this one. |
AutomatedTester
pushed a commit
that referenced
this pull request
Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New pattern detected
Transforms to:
ContextCreatedEventArgs- non-serializable top-level public type.Infoserializable, might beinternal- doesn't matter.Why
Normalizes all event args, name is the same as its declaration. Still safe at compilation level, any change in high/low level shows affected areas.
🔗 Related Issues
Contributes to #16095
💥 What does this PR do?
This pull request refactors the event subscription and dispatching mechanism in the BiDi (Bidirectional) WebDriver implementation, focusing on improving type safety, extensibility, and maintainability. The main changes include updating event handler signatures to use more specific event argument types, overhauling the
Brokerclass's subscription methods, and introducing a new event metadata management approach.Event Subscription and Dispatching Improvements:
The
Brokerclass now manages event metadata using a newEventMetadatarecord, enabling more flexible and type-safe event argument creation. Subscription methods have been refactored to support generic event argument and parameter types, and to simplify handler registration and unregistration. [1] [2] [3] [4] [5]The
EventDispatcheris now instantiated without a session provider, reflecting the new event handling structure. [1] [2]Event Handler Type Safety:
All event handler methods in
BrowsingContext.cshave been updated to use specific event argument types (e.g.,NavigationStartedEventArgs,FragmentNavigatedEventArgs,DomContentLoadedEventArgs, etc.) instead of the genericNavigationEventArgs. This improves type safety and clarity for consumers of these APIs. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]The corresponding private handler methods in
BrowsingContext.cshave also been updated to accept and process the new specific event argument types. [1] [2]These changes collectively make the event subscription API more robust, easier to use, and less error-prone by leveraging strong typing and modern C# features.
🔧 Implementation Notes
This is good new abstraction for .NET world. Moreover in future we can easily introduce
[JsonAdditionalData] IReadOnly.I should mention:
Broker(performance downgrade, but it is ok). We will think about it in details later.🔄 Types of changes