[dotnet] Replace Lazy<T> with ??= in BrowsingContext

[RenderMichael]

User description

Sample program:

using OpenQA.Selenium.BiDi;
using OpenQA.Selenium.Chrome;
using System;

using var driver = new ChromeDriver(new ChromeOptions
{
    UseWebSocketUrl = true
});

await driver.Navigate().GoToUrlAsync("https://www.google.com");

var bidi = await driver.AsBiDiAsync();
var context = (await bidi.BrowsingContext.GetTreeAsync()).Contexts[0].Context!;

var one = await context.Script.EvaluateAsync("1", awaitPromise: false);

Console.WriteLine(one);

Console.ReadKey();

Size difference: 11.3MB -> 11.0MB

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Enhancement

---

Description

• Replace Lazy<T> with nullable fields and ??= operator

• Simplify lazy initialization pattern in BrowsingContext

• Reduce binary size by ~300KB through pattern optimization

• Improve code readability and maintainability

---

Diagram Walkthrough

flowchart LR
  A["Lazy&lt;T&gt; fields<br/>with Value access"] -- "Refactor to" --> B["Nullable fields<br/>with ??= operator"]
  B -- "Benefits" --> C["Smaller binary<br/>Simpler code<br/>Same behavior"]

Loading

File Walkthrough

	Relevant files
Enhancement	BrowsingContext.cs Refactor lazy initialization to use ??= operator                  dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs Replaced five Lazy field declarations with nullable field declarations Removed lazy initialization from constructor Updated property getters to use ??= operator for lazy initialization Maintains same lazy-loading behavior with cleaner syntax +10/-16

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Thread safety regression Description: Replacing thread-safe Lazy with non-thread-safe null-coalescing initialization (??=) can cause race conditions if multiple threads access properties like Log, Network, or Script concurrently, potentially creating multiple instances and observing partially constructed states. BrowsingContext.cs [46-58] Referred Code public BrowsingContextLogModule Log => _logModule ??= new BrowsingContextLogModule(this, BiDi.Log); [JsonIgnore] public BrowsingContextNetworkModule Network => _networkModule ??= new BrowsingContextNetworkModule(this, BiDi.Network); [JsonIgnore] public BrowsingContextScriptModule Script => _scriptModule ??= new BrowsingContextScriptModule(this, BiDi.Script); [JsonIgnore] public BrowsingContextStorageModule Storage => _storageModule ??= new BrowsingContextStorageModule(this, BiDi.Storage); [JsonIgnore] public BrowsingContextInputModule Input => _inputModule ??= new BrowsingContextInputModule(this, BiDi.InputModule);
Ticket Compliance
🟡	🎫 #1234 🔴 Investigate and resolve regression where clicking a link with JavaScript in its href no longer triggers in Selenium 2.48.x on Firefox 42 (works in 2.47.1). Provide a fix or workaround ensuring click() triggers href JavaScript as before. Verify behavior specifically on Firefox 42 (32-bit on 64-bit machine). Include tests or validation demonstrating the alert is triggered again.
🟡	🎫 #5678 🔴 Diagnose and resolve intermittent "Error: ConnectFailure (Connection refused)" when instantiating multiple ChromeDriver instances on Ubuntu 16.04.4 with Selenium 3.9.0 and Chrome 65/ChromeDriver 2.35. Ensure subsequent ChromeDriver instantiations connect reliably after the first one. Provide reproduction steps, guidance, or code changes to eliminate the connection failures. Validate fix on the specified environment.
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: No audit logs: The added lazy-initialized properties and constructor changes do not add or reference any audit logging for critical actions, but it is unclear whether such logging is handled elsewhere in the system. Referred Code public BrowsingContextLogModule Log => _logModule ??= new BrowsingContextLogModule(this, BiDi.Log); [JsonIgnore] public BrowsingContextNetworkModule Network => _networkModule ??= new BrowsingContextNetworkModule(this, BiDi.Network); [JsonIgnore] public BrowsingContextScriptModule Script => _scriptModule ??= new BrowsingContextScriptModule(this, BiDi.Script); [JsonIgnore] public BrowsingContextStorageModule Storage => _storageModule ??= new BrowsingContextStorageModule(this, BiDi.Storage); [JsonIgnore] public BrowsingContextInputModule Input => _inputModule ??= new BrowsingContextInputModule(this, BiDi.InputModule); 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: Missing null checks: The new lazy initialization via ??= on module properties lacks explicit null checks or error handling if BiDi or its submodules are null, which may lead to runtime exceptions without contextual handling. Referred Code public BrowsingContextLogModule Log => _logModule ??= new BrowsingContextLogModule(this, BiDi.Log); [JsonIgnore] public BrowsingContextNetworkModule Network => _networkModule ??= new BrowsingContextNetworkModule(this, BiDi.Network); [JsonIgnore] public BrowsingContextScriptModule Script => _scriptModule ??= new BrowsingContextScriptModule(this, BiDi.Script); [JsonIgnore] public BrowsingContextStorageModule Storage => _storageModule ??= new BrowsingContextStorageModule(this, BiDi.Storage); [JsonIgnore] public BrowsingContextInputModule Input => _inputModule ??= new BrowsingContextInputModule(this, BiDi.InputModule); 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 unclear: The refactor changes initialization only and does not show validation or authorization around module access, leaving uncertainty whether external inputs handled by these modules are validated elsewhere. Referred Code public BrowsingContextLogModule Log => _logModule ??= new BrowsingContextLogModule(this, BiDi.Log); [JsonIgnore] public BrowsingContextNetworkModule Network => _networkModule ??= new BrowsingContextNetworkModule(this, BiDi.Network); [JsonIgnore] public BrowsingContextScriptModule Script => _scriptModule ??= new BrowsingContextScriptModule(this, BiDi.Script); [JsonIgnore] public BrowsingContextStorageModule Storage => _storageModule ??= new BrowsingContextStorageModule(this, BiDi.Storage); [JsonIgnore] public BrowsingContextInputModule Input => _inputModule ??= new BrowsingContextInputModule(this, BiDi.InputModule); 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
Possible issue	✅ Fix race condition with thread-safe initialization Suggestion Impact: The commit replaced the ??= initializations for the module properties with Interlocked.CompareExchange to ensure thread-safe initialization and added a using for System.Threading. It did not mark the fields as volatile. code diff: using System; using System.Text.Json.Serialization; +using System.Threading; using System.Threading.Tasks; namespace OpenQA.Selenium.BiDi.BrowsingContext; @@ -43,19 +44,19 @@ public BiDi BiDi { get; } [JsonIgnore] - public BrowsingContextLogModule Log => _logModule ??= new BrowsingContextLogModule(this, BiDi.Log); - - [JsonIgnore] - public BrowsingContextNetworkModule Network => _networkModule ??= new BrowsingContextNetworkModule(this, BiDi.Network); - - [JsonIgnore] - public BrowsingContextScriptModule Script => _scriptModule ??= new BrowsingContextScriptModule(this, BiDi.Script); - - [JsonIgnore] - public BrowsingContextStorageModule Storage => _storageModule ??= new BrowsingContextStorageModule(this, BiDi.Storage); - - [JsonIgnore] - public BrowsingContextInputModule Input => _inputModule ??= new BrowsingContextInputModule(this, BiDi.InputModule); + public BrowsingContextLogModule Log => _logModule ?? Interlocked.CompareExchange(ref _logModule, new BrowsingContextLogModule(this, BiDi.Log), null) ?? _logModule; + + [JsonIgnore] + public BrowsingContextNetworkModule Network => _networkModule ?? Interlocked.CompareExchange(ref _networkModule, new BrowsingContextNetworkModule(this, BiDi.Network), null) ?? _networkModule; + + [JsonIgnore] + public BrowsingContextScriptModule Script => _scriptModule ?? Interlocked.CompareExchange(ref _scriptModule, new BrowsingContextScriptModule(this, BiDi.Script), null) ?? _scriptModule; + + [JsonIgnore] + public BrowsingContextStorageModule Storage => _storageModule ?? Interlocked.CompareExchange(ref _storageModule, new BrowsingContextStorageModule(this, BiDi.Storage), null) ?? _storageModule; + + [JsonIgnore] + public BrowsingContextInputModule Input => _inputModule ?? Interlocked.CompareExchange(ref _inputModule, new BrowsingContextInputModule(this, BiDi.InputModule), null) ?? _inputModule; The lazy initialization using ??= is not thread-safe and can cause a race condition. To fix this, use Interlocked.CompareExchange for a lock-free, thread-safe initialization and mark the backing fields as volatile. dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs [34-58] -private BrowsingContextLogModule? _logModule; -private BrowsingContextNetworkModule? _networkModule; -private BrowsingContextScriptModule? _scriptModule; -private BrowsingContextStorageModule? _storageModule; -private BrowsingContextInputModule? _inputModule; +private volatile BrowsingContextLogModule? _logModule; +private volatile BrowsingContextNetworkModule? _networkModule; +private volatile BrowsingContextScriptModule? _scriptModule; +private volatile BrowsingContextStorageModule? _storageModule; +private volatile BrowsingContextInputModule? _inputModule; internal string Id { get; } [JsonIgnore] public BiDi BiDi { get; } [JsonIgnore] -public BrowsingContextLogModule Log => _logModule ??= new BrowsingContextLogModule(this, BiDi.Log); +public BrowsingContextLogModule Log => _logModule ?? Interlocked.CompareExchange(ref _logModule, new BrowsingContextLogModule(this, BiDi.Log), null) ?? _logModule!; [JsonIgnore] -public BrowsingContextNetworkModule Network => _networkModule ??= new BrowsingContextNetworkModule(this, BiDi.Network); +public BrowsingContextNetworkModule Network => _networkModule ?? Interlocked.CompareExchange(ref _networkModule, new BrowsingContextNetworkModule(this, BiDi.Network), null) ?? _networkModule!; [JsonIgnore] -public BrowsingContextScriptModule Script => _scriptModule ??= new BrowsingContextScriptModule(this, BiDi.Script); +public BrowsingContextScriptModule Script => _scriptModule ?? Interlocked.CompareExchange(ref _scriptModule, new BrowsingContextScriptModule(this, BiDi.Script), null) ?? _scriptModule!; [JsonIgnore] -public BrowsingContextStorageModule Storage => _storageModule ??= new BrowsingContextStorageModule(this, BiDi.Storage); +public BrowsingContextStorageModule Storage => _storageModule ?? Interlocked.CompareExchange(ref _storageModule, new BrowsingContextStorageModule(this, BiDi.Storage), null) ?? _storageModule!; [JsonIgnore] -public BrowsingContextInputModule Input => _inputModule ??= new BrowsingContextInputModule(this, BiDi.InputModule); +public BrowsingContextInputModule Input => _inputModule ?? Interlocked.CompareExchange(ref _inputModule, new BrowsingContextInputModule(this, BiDi.InputModule), null) ?? _inputModule!; [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that the refactoring from Lazy<T> to ??= introduces a race condition, as ??= is not an atomic operation. This is a critical regression in thread safety, and the proposed fix using Interlocked.CompareExchange is a correct and robust pattern for thread-safe lazy initialization.	High
Update

[nvborisenko]

Nice! Would also be good if we can make it thread-safe (here or later).

[nvborisenko]

Hm, it doesn't look the same:

    <PublishAot>true</PublishAot>
    <PublishTrimmed>true</PublishTrimmed>

await bidi.StatusAsync()

I may miss something.

[nvborisenko]

I realized you demonstrated the diff, alright. I also added thread-safety.

[RenderMichael]

@nvborisenko Did you use my sample code? What does your program look like?

(I’m using .NET 10)