[dotnet] [bidi] Use shared json context for core modules

[nvborisenko]

User description

💥 What does this PR do?

There is no reason to create new json context per module.

💡 Additional Considerations

Using one big shared json context is better for us:

• Performance efficiency because of caching
• Resolves cross-referencing issue

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Enhancement

---

Description

• Consolidate module-specific JSON serializer contexts into single shared context

• Improves performance through JSON serialization caching

• Resolves cross-referencing issues between modules

• Simplifies module initialization by removing per-module context creation

---

Diagram Walkthrough

flowchart LR
  A["Module-Specific<br/>JsonSerializerContext"] -->|Consolidate| B["BiDiJsonSerializerContext"]
  B -->|Shared| C["All Modules"]
  C -->|Performance<br/>Benefit| D["Caching &<br/>Efficiency"]
  C -->|Resolves| E["Cross-Module<br/>References"]

Loading

File Walkthrough

	Relevant files
Enhancement	4 files BiDi.cs Initialize shared JSON context at BiDi level                          +6/-1      Module.cs Update Module base class to use shared context                      +12/-5    BiDiJsonSerializerContext.cs Create unified JSON serializer context for all modules      +199/-0  PermissionsModule.cs Update Initialize method signature for shared context        +4/-2
Cleanup	10 files BrowserModule.cs Remove module-specific context, use shared context              +8/-31    BrowsingContextModule.cs Remove module-specific context, use shared context              +40/-85  EmulationModule.cs Remove module-specific context, use shared context              +9/-34    InputModule.cs Remove module-specific context, use shared context              +3/-24    LogModule.cs Remove module-specific context, use shared context              +2/-49    NetworkModule.cs Remove module-specific context, use shared context              +24/-65  ScriptModule.cs Remove module-specific context, use shared context              +12/-79  SessionModule.cs Remove module-specific context, use shared context              +5/-26    StorageModule.cs Remove module-specific context, use shared context              +3/-20    WebExtensionModule.cs Remove module-specific context, use shared context              +2/-17

---

[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
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No auditing added: The changes introduce a shared JSON serializer context and refactor module calls but do not add or modify any logging of critical actions, so audit trail coverage is unchanged and cannot be verified from this diff alone. Referred Code public sealed class BiDi : IAsyncDisposable { private readonly BiDiJsonSerializerContext _jsonContext; private readonly ConcurrentDictionary<Type, Module> _modules = new(); private BiDi(string url) { var uri = new Uri(url); Broker = new Broker(this, uri); _jsonContext = new BiDiJsonSerializerContext(GetJsonOptions()); } internal Session.SessionModule SessionModule => AsModule<Session.SessionModule>(); public BrowsingContext.BrowsingContextModule BrowsingContext => AsModule<BrowsingContext.BrowsingContextModule>(); public Browser.BrowserModule Browser => AsModule<Browser.BrowserModule>(); ... (clipped 44 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Error handling unchanged: The refactor switches to a shared JsonSerializerContext without adding new error handling or null/edge-case checks, and robustness of Broker.ExecuteCommandAsync remains outside the shown changes. Referred Code protected Broker Broker { get; private set; } = null!; internal BiDiJsonSerializerContext JsonContext { get; private set; } = null!; protected virtual void Initialize(JsonSerializerContext jsonSerializerContext) { JsonContext = (BiDiJsonSerializerContext)jsonSerializerContext; } internal static TModule Create<TModule>(BiDi bidi, Broker broker, BiDiJsonSerializerContext jsonSerializerContext) where TModule : Module, new() { TModule module = new() { BiDi = bidi, Broker = broker, JsonContext = jsonSerializerContext }; module.Initialize(jsonSerializerContext); ... (clipped 3 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: No user errors here: The changes only adjust serializer context usage and do not introduce user-facing error messages, so secure error handling cannot be assessed from this diff alone. Referred Code // </copyright> using System; using System.Threading.Tasks; namespace OpenQA.Selenium.BiDi.Log; public sealed class LogModule : Module { public async Task<Subscription> OnEntryAddedAsync(Func<LogEntry, Task> handler, SubscriptionOptions? options = null) { return await Broker.SubscribeAsync("log.entryAdded", handler, options, JsonContext.LogEntry).ConfigureAwait(false); } public async Task<Subscription> OnEntryAddedAsync(Action<LogEntry> handler, SubscriptionOptions? options = null) { return await Broker.SubscribeAsync("log.entryAdded", handler, options, JsonContext.LogEntry).ConfigureAwait(false); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Validation unchanged: The new code paths defer to Broker.ExecuteCommandAsync with shared JsonContext and do not add input validation or sanitation; security of external inputs cannot be determined from this refactor alone. Referred Code { var @params = new AddInterceptParameters(phases, options?.Contexts, options?.UrlPatterns); var result = await Broker.ExecuteCommandAsync(new AddInterceptCommand(@params), options, JsonContext.AddInterceptCommand, JsonContext.AddInterceptResult).ConfigureAwait(false); return result.Intercept; } public async Task<RemoveDataCollectorResult> RemoveDataCollectorAsync(Collector collector, RemoveDataCollectorOptions? options = null) { var @params = new RemoveDataCollectorParameters(collector); return await Broker.ExecuteCommandAsync(new RemoveDataCollectorCommand(@params), options, JsonContext.RemoveDataCollectorCommand, JsonContext.RemoveDataCollectorResult).ConfigureAwait(false); } public async Task<RemoveInterceptResult> RemoveInterceptAsync(Intercept intercept, RemoveInterceptOptions? options = null) { var @params = new RemoveInterceptParameters(intercept); return await Broker.ExecuteCommandAsync(new RemoveInterceptCommand(@params), options, JsonContext.RemoveInterceptCommand, JsonContext.RemoveInterceptResult).ConfigureAwait(false); } ... (clipped 43 lines) Learn more about managing compliance generic rules or creating your own custom rules
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
High-level	Integrate extension module into shared context The PermissionsModule is not using the shared BiDiJsonSerializerContext and instead creates its own, which contradicts the PR's goal. It should be integrated into the shared context. Examples: dotnet/src/webdriver/BiDi/Permissions/PermissionsModule.cs [27-48] public class PermissionsModule : Module { private PermissionsJsonSerializerContext _jsonContext = null!; public async Task<SetPermissionResult> SetPermissionAsync(PermissionDescriptor desriptor, PermissionState state, string origin, SetPermissionOptions? options = null) { var @params = new SetPermissionCommandParameters(desriptor, state, origin, options?.EmbeddedOrigin, options?.UserContext); return await Broker.ExecuteCommandAsync(new SetPermissionCommand(@params), options, _jsonContext.SetPermissionCommand, _jsonContext.SetPermissionResult).ConfigureAwait(false); } ... (clipped 12 lines) Solution Walkthrough: Before: // dotnet/src/webdriver/BiDi/Permissions/PermissionsModule.cs public class PermissionsModule : Module { private PermissionsJsonSerializerContext _jsonContext = null!; public async Task<SetPermissionResult> SetPermissionAsync(...) { // ... return await Broker.ExecuteCommandAsync(..., _jsonContext.SetPermissionCommand, ...); } protected override void Initialize(JsonSerializerContext jsonSerializerContext) { var jsonOptions = new JsonSerializerOptions(jsonSerializerContext.Options); _jsonContext = new PermissionsJsonSerializerContext(jsonOptions); } } [JsonSerializable(typeof(SetPermissionCommand))] internal partial class PermissionsJsonSerializerContext : JsonSerializerContext; After: // dotnet/src/webdriver/BiDi/Permissions/PermissionsModule.cs public class PermissionsModule : Module { public async Task<SetPermissionResult> SetPermissionAsync(...) { // ... return await Broker.ExecuteCommandAsync(..., JsonContext.SetPermissionCommand, ...); } // The Initialize method override and local context are removed. } // dotnet/src/webdriver/BiDi/Json/BiDiJsonSerializerContext.cs // ... (other serializable types) [JsonSerializable(typeof(Permissions.SetPermissionCommand))] [JsonSerializable(typeof(Permissions.SetPermissionResult))] internal partial class BiDiJsonSerializerContext : JsonSerializerContext; Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that PermissionsModule was not migrated to the shared BiDiJsonSerializerContext, which undermines the PR's primary goal of consolidating all JSON contexts for improved performance and maintainability.	High
Possible issue	Call base method in override In the Initialize method of PermissionsModule, add a call to the base class implementation base.Initialize(jsonSerializerContext) to ensure the JsonContext property is correctly initialized. dotnet/src/webdriver/BiDi/Permissions/PermissionsModule.cs [38-43] protected override void Initialize(JsonSerializerContext jsonSerializerContext) { + base.Initialize(jsonSerializerContext); var jsonOptions = new JsonSerializerOptions(jsonSerializerContext.Options); _jsonContext = new PermissionsJsonSerializerContext(jsonOptions); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the overridden Initialize method in PermissionsModule does not call the base implementation, which could lead to future NullReferenceException if the base JsonContext property is used.	Low
Learned best practice	Protect resource initialization with cleanup Guard initialization of external resources by using try/finally and dispose/close them in finally to avoid leaks if an exception occurs during construction or later initialization. dotnet/src/webdriver/BiDi/BiDi.cs [37-44] private BiDi(string url) { var uri = new Uri(url); - - Broker = new Broker(this, uri); - - _jsonContext = new BiDiJsonSerializerContext(GetJsonOptions()); + Broker? broker = null; + try + { + broker = new Broker(this, uri); + Broker = broker; + _jsonContext = new BiDiJsonSerializerContext(GetJsonOptions()); + } + finally + { + if (_jsonContext == null && broker is IAsyncDisposable ad) + { + ad.DisposeAsync().AsTask().GetAwaiter().GetResult(); + } + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Pattern 1: Wrap creation of external contexts/resources in try/finally and ensure cleanup in finally to prevent leaks on exceptions.	Low
Update

[nvborisenko]

Root types (with nested types) defined are not trimmable by design. Using small contexts per modules are more friendly with AOT trimming.