[dotnet] [bidi] JsonSerializerContext instance per module by nvborisenko · Pull Request #16649

[dotnet] [bidi] JsonSerializerContext instance per module by nvborisenko · Pull Request #16649 · SeleniumHQ/selenium

User description

Preparation for separate JsonSerializerContext per each Module.

💥 What does this PR do?

Hide public DefaultJsonOptions() in BiDi class
Each module has its own one-time setup context avoiding casting
Don't share JsonOptions across modules, meaning each module may modify options (like adding custom converters)

🔄 Types of changes

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

PR Type

Enhancement

Description

Refactor JSON context initialization to per-module pattern
Hide GetJsonOptions() as internal method in BiDi class
Replace CreateJsonContext() with Initialize() void method
Simplify module creation by removing JsonOptions parameter passing
Each module now manages its own JsonSerializerContext instance

Diagram Walkthrough

flowchart LR
  BiDi["BiDi class<br/>GetJsonOptions internal"]
  Module["Module base class<br/>Initialize abstract method"]
  Modules["All modules<br/>BrowserModule, NetworkModule, etc"]
  BiDi -- "calls Initialize" --> Module
  Module -- "implements Initialize" --> Modules
  Modules -- "creates _jsonContext" --> Modules

File Walkthrough

Relevant files

Refactoring

15 files

BiDi.cs `Hide GetJsonOptions and simplify module initialization`	+41/-47
Broker.cs `Remove JsonSerializerOptions parameter from constructor`	+1/-1
Module.cs `Replace CreateJsonContext with Initialize void method`	+3/-6
BrowserModule.cs `Implement Initialize method with private _jsonContext field`	+12/-12
BrowsingContextModule.cs `Implement Initialize method with private _jsonContext field`	+44/-44
EmulationModule.cs `Implement Initialize method with private _jsonContext field`	+13/-13
InputModule.cs `Implement Initialize method with private _jsonContext field`	+7/-7
LogModule.cs `Implement Initialize method with private _jsonContext field`	+6/-6
NetworkModule.cs `Implement Initialize method with private _jsonContext field`	+28/-28
PermissionsBiDiExtensions.cs `Simplify module creation call without JsonOptions`	+1/-1
PermissionsModule.cs `Implement Initialize method with private _jsonContext field`	+4/-4
ScriptModule.cs `Implement Initialize method with private _jsonContext field`	+16/-16
SessionModule.cs `Implement Initialize method with private _jsonContext field`	+9/-9
StorageModule.cs `Implement Initialize method with private _jsonContext field`	+7/-7
WebExtensionModule.cs `Implement Initialize method with private _jsonContext field`	+6/-6

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 Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. 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: Event logging: New commands and subscriptions are added but there is no explicit logging of critical actions or outcomes within the added code segments, which may rely on external logging outside this diff. Referred Code public async Task<ProvideResponseResult> ProvideResponseAsync(Request request, ProvideResponseOptions? options = null) { var @params = new ProvideResponseParameters(request, options?.Body, options?.Cookies, options?.Headers, options?.ReasonPhrase, options?.StatusCode); return await Broker.ExecuteCommandAsync(new ProvideResponseCommand(@params), options, _jsonContext.ProvideResponseCommand, _jsonContext.ProvideResponseResult).ConfigureAwait(false); } public async Task<ContinueWithAuthResult> ContinueWithAuthAsync(Request request, AuthCredentials credentials, ContinueWithAuthCredentialsOptions? options = null) { return await Broker.ExecuteCommandAsync(new ContinueWithAuthCommand(new ContinueWithAuthCredentials(request, credentials)), options, _jsonContext.ContinueWithAuthCommand, _jsonContext.ContinueWithAuthResult).ConfigureAwait(false); } public async Task<ContinueWithAuthResult> ContinueWithAuthAsync(Request request, ContinueWithAuthDefaultCredentialsOptions? options = null) { return await Broker.ExecuteCommandAsync(new ContinueWithAuthCommand(new ContinueWithAuthDefaultCredentials(request)), options, _jsonContext.ContinueWithAuthCommand, _jsonContext.ContinueWithAuthResult).ConfigureAwait(false); } public async Task<ContinueWithAuthResult> ContinueWithAuthAsync(Request request, ContinueWithAuthCancelCredentialsOptions? options = null) { return await Broker.ExecuteCommandAsync(new ContinueWithAuthCommand(new ContinueWithAuthCancelCredentials(request)), options, _jsonContext.ContinueWithAuthCommand, _jsonContext.ContinueWithAuthResult).ConfigureAwait(false); } ... (clipped 50 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: Limited validation: The extension method validates null and throws ArgumentNullException but added module methods rely on Broker calls without visible error handling in this diff, potentially delegating to shared infrastructure. Referred Code public static PermissionsModule AsPermissions(this BiDi bidi) { if (bidi is null) { throw new ArgumentNullException(nameof(bidi)); } return Module.Create<PermissionsModule>(bidi); } 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: Input validation: The new per-module Initialize pathway creates JsonSerializerOptions per module without explicit validation or immutability checks in this diff, which may be acceptable if upstream Broker and converters enforce validation. Referred Code protected abstract void Initialize(JsonSerializerOptions options); public static TModule Create<TModule>(BiDi bidi) where TModule : Module, new() { TModule module = new() { Broker = bidi.Broker, }; module.Initialize(bidi.GetJsonOptions()); return module; } 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

PR Code Suggestions ✨

Explore these optional code suggestions:

Category Suggestion Impact

General

Category	Suggestion	Impact
General	Cache JsonSerializerOptions to improve performance Cache the `JsonSerializerOptions` instance created in `GetJsonOptions()` to avoid repeated allocations and improve performance during module initialization. dotnet/src/webdriver/BiDi/BiDi.cs [96-121] +private JsonSerializerOptions? _jsonOptions; + internal JsonSerializerOptions GetJsonOptions() { - return new JsonSerializerOptions + if (_jsonOptions is null) { - PropertyNameCaseInsensitive = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, + _jsonOptions = new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, - // BiDi returns special numbers such as "NaN" as strings - // Additionally, -0 is returned as a string "-0" - NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals \| JsonNumberHandling.AllowReadingFromString, - Converters = - { - new BrowsingContextConverter(this), - new BrowserUserContextConverter(this), - new CollectorConverter(this), - new InterceptConverter(this), - new HandleConverter(this), - new InternalIdConverter(this), - new PreloadScriptConverter(this), - new RealmConverter(this), - new DateTimeOffsetConverter(), - new WebExtensionConverter(this), - } - }; + // BiDi returns special numbers such as "NaN" as strings + // Additionally, -0 is returned as a string "-0" + NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals \| JsonNumberHandling.AllowReadingFromString, + Converters = + { + new BrowsingContextConverter(this), + new BrowserUserContextConverter(this), + new CollectorConverter(this), + new InterceptConverter(this), + new HandleConverter(this), + new InternalIdConverter(this), + new PreloadScriptConverter(this), + new RealmConverter(this), + new DateTimeOffsetConverter(), + new WebExtensionConverter(this), + } + }; + } + + return _jsonOptions; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that `GetJsonOptions()` is called for each module's initialization, and caching the `JsonSerializerOptions` is a valid performance optimization to reduce object allocations.	Low
Update

Cache JsonSerializerOptions to improve performance

Cache the JsonSerializerOptions instance created in GetJsonOptions() to avoid
repeated allocations and improve performance during module initialization.

dotnet/src/webdriver/BiDi/BiDi.cs [96-121]

+private JsonSerializerOptions? _jsonOptions;
+
 internal JsonSerializerOptions GetJsonOptions()
 {
-    return new JsonSerializerOptions
+    if (_jsonOptions is null)
     {
-        PropertyNameCaseInsensitive = true,
-        PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
-        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
+        _jsonOptions = new JsonSerializerOptions
+        {
+            PropertyNameCaseInsensitive = true,
+            PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
+            DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
 
-        // BiDi returns special numbers such as "NaN" as strings
-        // Additionally, -0 is returned as a string "-0"
-        NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals | JsonNumberHandling.AllowReadingFromString,
-        Converters =
-        {
-            new BrowsingContextConverter(this),
-            new BrowserUserContextConverter(this),
-            new CollectorConverter(this),
-            new InterceptConverter(this),
-            new HandleConverter(this),
-            new InternalIdConverter(this),
-            new PreloadScriptConverter(this),
-            new RealmConverter(this),
-            new DateTimeOffsetConverter(),
-            new WebExtensionConverter(this),
-        }
-    };
+            // BiDi returns special numbers such as "NaN" as strings
+            // Additionally, -0 is returned as a string "-0"
+            NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals | JsonNumberHandling.AllowReadingFromString,
+            Converters =
+            {
+                new BrowsingContextConverter(this),
+                new BrowserUserContextConverter(this),
+                new CollectorConverter(this),
+                new InterceptConverter(this),
+                new HandleConverter(this),
+                new InternalIdConverter(this),
+                new PreloadScriptConverter(this),
+                new RealmConverter(this),
+                new DateTimeOffsetConverter(),
+                new WebExtensionConverter(this),
+            }
+        };
+    }
+
+    return _jsonOptions;
 }

Apply / Chat

Suggestion importance[1-10]: 6

Why: The suggestion correctly identifies that GetJsonOptions() is called for each module's initialization, and caching the JsonSerializerOptions is a valid performance optimization to reduce object allocations.

Low

Update

nvborisenko changed the title ~~[dotnet] [bidi] JsonContext instance per module~~ [dotnet] [bidi] JsonSerializerContext instance per module

Nov 29, 2025

Perfect! Now we can think about how to split big BiDiJsonSerializerContext into smaller chunks per module, given that modules are cross-referenced.

CC @RenderMichael