[dotnet] [bidi] Cache modules in the root BiDi

[nvborisenko]

User description

Unify allocation of modules when needed only.

🔧 Implementation Notes

Just cache. For both internal modules and external.

💡 Additional Considerations

I believe that checking cached object in ConcurrentDictionary is faster than allocating new object. Didn't verify it.

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Enhancement

---

Description

• Replace eager module initialization with lazy-loaded cached properties

• Remove redundant module assignments from constructor

• Leverage AsModule caching for on-demand module instantiation

• Simplify BiDi class initialization logic

---

Diagram Walkthrough

flowchart LR
  A["BiDi Constructor"] -->|Before| B["Eager Module Initialization"]
  B --> C["Store in Properties"]
  A -->|After| D["Lazy-Loaded Properties"]
  D --> E["AsModule Caching"]
  E --> F["On-Demand Instantiation"]

Loading

File Walkthrough

	Relevant files
Enhancement	BiDi.cs Convert modules to lazy-loaded cached properties                  dotnet/src/webdriver/BiDi/BiDi.cs Removed eager module initialization from constructor Converted module properties from auto-properties to expression-bodied properties Each property now calls AsModule() on access, leveraging internal caching Reduces memory footprint by deferring module creation until first use +10/-21

---

[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 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: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities 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: Missing logging: The new lazy module accessors and constructor changes introduce no audit logging for critical actions, but the diff does not show whether such logging is handled elsewhere. Referred Code internal Session.SessionModule SessionModule => AsModule<Session.SessionModule>(); public BrowsingContext.BrowsingContextModule BrowsingContext => AsModule<BrowsingContext.BrowsingContextModule>(); public Browser.BrowserModule Browser => AsModule<Browser.BrowserModule>(); public Network.NetworkModule Network => AsModule<Network.NetworkModule>(); internal Input.InputModule InputModule => AsModule<Input.InputModule>(); public Script.ScriptModule Script => AsModule<Script.ScriptModule>(); public Log.LogModule Log => AsModule<Log.LogModule>(); public Storage.StorageModule Storage => AsModule<Storage.StorageModule>(); public WebExtension.WebExtensionModule WebExtension => AsModule<WebExtension.WebExtensionModule>(); public Emulation.EmulationModule Emulation => AsModule<Emulation.EmulationModule>(); public Task<Session.StatusResult> StatusAsync() 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: Unhandled failures: Newly added lazy properties call AsModule<> without visible error handling or null/edge-case safeguards, but handling may exist within AsModule which is not shown. Referred Code internal Session.SessionModule SessionModule => AsModule<Session.SessionModule>(); public BrowsingContext.BrowsingContextModule BrowsingContext => AsModule<BrowsingContext.BrowsingContextModule>(); public Browser.BrowserModule Browser => AsModule<Browser.BrowserModule>(); public Network.NetworkModule Network => AsModule<Network.NetworkModule>(); internal Input.InputModule InputModule => AsModule<Input.InputModule>(); public Script.ScriptModule Script => AsModule<Script.ScriptModule>(); public Log.LogModule Log => AsModule<Log.LogModule>(); public Storage.StorageModule Storage => AsModule<Storage.StorageModule>(); public WebExtension.WebExtensionModule WebExtension => AsModule<WebExtension.WebExtensionModule>(); public Emulation.EmulationModule Emulation => AsModule<Emulation.EmulationModule>(); public Task<Session.StatusResult> StatusAsync() 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 ✨

No code suggestions found for the PR.

[Copilot]

Pull request overview

This PR refactors the BiDi class to use lazy-loaded module properties instead of eager initialization in the constructor. The change aims to defer module creation until first use by leveraging the existing AsModule caching mechanism.

Key Changes:

• Removed eager module initialization from the constructor (10 modules previously initialized upfront)
• Converted module properties from auto-properties to expression-bodied properties that call AsModule<T>()
• Reduced initial memory allocation by deferring module creation until accessed

Comments suppressed due to low confidence (1)

dotnet/src/webdriver/BiDi/BiDi.cs:88

• The GetOrAdd call uses the wrong overload, causing GetJsonOptions() and Module.Create<T>() to be evaluated on every property access, even when the module is already cached. This defeats the purpose of the lazy-loading optimization. Use the factory overload instead: return (T)_modules.GetOrAdd(typeof(T), _ => Module.Create<T>(this, Broker, GetJsonOptions()));

        return (T)_modules.GetOrAdd(typeof(T), Module.Create<T>(this, Broker, GetJsonOptions()));

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[nvborisenko]

• The GetOrAdd call uses the wrong overload, causing GetJsonOptions() and Module.Create<T>() to be evaluated on every property access, even when the module is already cached. This defeats the purpose of the lazy-loading optimization. Use the factory overload instead: return (T)_modules.GetOrAdd(typeof(T), _ => Module.Create<T>(this, Broker, GetJsonOptions()));

        return (T)_modules.GetOrAdd(typeof(T), Module.Create<T>(this, Broker, GetJsonOptions()));

Nice catch!

[nvborisenko]

Move forward, in any case we can revert it back.