[dotnet] Annotate nullability on JavaScriptEngine and related types

[RenderMichael]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Annotate nullability on JavaScriptEngine and all related types (some types are modernized without nullability annotations - the refactorings are crucial to nullability on JavaScriptEngine, but the specific properties are not relevant to this PR and require further investigation.

Motivation and Context

Contributes to #14640

Also bugfix.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Bug fix

---

Description

• Annotated nullability for JavaScriptEngine and related types.

• Refactored constructors and properties for immutability and clarity.

• Added null checks and exception handling for improved robustness.

• Fixed JavaScriptEngine.ScriptCallbackBindings to include new bindings.

---

Changes walkthrough 📝

Relevant files

Enhancement

DomMutatedEventArgs.cs Refactor `DomMutatedEventArgs` for immutability                    dotnet/src/webdriver/DomMutatedEventArgs.cs Refactored AttributeData property to be immutable. Added constructor for initializing DomMutatedEventArgs. +5/-6      DomMutationData.cs Refactor `DomMutationData` properties for immutability      dotnet/src/webdriver/DomMutationData.cs Refactored properties to be auto-implemented and immutable. Added JsonIgnore attribute for Element property. Updated ToString method to use new property structure. +6/-26    IJavaScriptEngine.cs Add nullability annotations to `IJavaScriptEngine`              dotnet/src/webdriver/IJavaScriptEngine.cs Enabled nullable reference types. Annotated events and methods with nullability. Added exception handling for null arguments. +12/-7    InitializationScript.cs Refactor `InitializationScript` for immutability                  dotnet/src/webdriver/InitializationScript.cs Added constructor to enforce immutability of properties. Enabled nullable reference types. +12/-3    JavaScriptCallbackExecutedEventArgs.cs Refactor `JavaScriptCallbackExecutedEventArgs` for clarity dotnet/src/webdriver/JavaScriptCallbackExecutedEventArgs.cs Added constructor for initializing properties. Removed backing fields for properties. +14/-6    JavaScriptConsoleApiCalledEventArgs.cs Refactor `JavaScriptConsoleApiCalledEventArgs` for clarity dotnet/src/webdriver/JavaScriptConsoleApiCalledEventArgs.cs Added constructor for initializing properties. Removed backing fields for properties. +16/-7    JavaScriptEngine.cs Refactor `JavaScriptEngine` for nullability and robustness dotnet/src/webdriver/JavaScriptEngine.cs Enabled nullable reference types. Refactored fields to be readonly where applicable. Added null checks and exception handling for robustness. Fixed ScriptCallbackBindings to include new bindings. Updated event invocations to use null-conditional operators. +69/-57  JavaScriptExceptionThrownEventArgs.cs Refactor `JavaScriptExceptionThrownEventArgs` for clarity dotnet/src/webdriver/JavaScriptExceptionThrownEventArgs.cs Added constructor for initializing Message property. Removed backing field for Message. +9/-2

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit bea599b)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Check The OnScriptBindingCalled method assumes JsonSerializer.Deserialize<DomMutationData> will not return null, but this should be validated with a null check before accessing properties. DomMutationData valueChangeData = JsonSerializer.Deserialize<DomMutationData>(e.Payload) ?? throw new JsonException("DomMutationData returned null"); var locator = By.CssSelector($"*[data-__webdriver_id='{valueChangeData.TargetId}']"); Event Handler The OnConsoleApiCalled method still checks for null on JavaScriptConsoleApiCalled event despite using the null-conditional operator elsewhere. This should be consistent with other event handler patterns. if (this.JavaScriptConsoleApiCalled != null) { for (int i = 0; i < e.Arguments.Count; i++)

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to bea599b
Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add null check for collection The OnConsoleApiCalled method should check if e.Arguments is null before iterating through it to avoid potential null reference exceptions. dotnet/src/webdriver/JavaScriptEngine.cs [452-460] -for (int i = 0; i < e.Arguments.Count; i++) +if (e.Arguments?.Count > 0) { - this.JavaScriptConsoleApiCalled(this, new JavaScriptConsoleApiCalledEventArgs - ( - messageContent: e.Arguments[i].Value, - messageTimeStamp: e.Timestamp, - messageType: e.Type - )); + for (int i = 0; i < e.Arguments.Count; i++) + { + this.JavaScriptConsoleApiCalled(this, new JavaScriptConsoleApiCalledEventArgs + ( + messageContent: e.Arguments[i].Value, + messageTimeStamp: e.Timestamp, + messageType: e.Type + )); + } } Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding a null check for e.Arguments is important for preventing potential NullReferenceException, improving the robustness of the code in handling edge cases.	7
	Validate element exists before assignment The OnScriptBindingCalled method should verify that driver.FindElements() returns a non-null element before accessing it. dotnet/src/webdriver/JavaScriptEngine.cs [429-430] var locator = By.CssSelector($"*[data-__webdriver_id='{valueChangeData.TargetId}']"); -valueChangeData.Element = driver.FindElements(locator).FirstOrDefault(); +var element = driver.FindElements(locator).FirstOrDefault(); +if (element != null) +{ + valueChangeData.Element = element; +} Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion improves code safety by explicitly checking for null before assigning the element, preventing potential issues with null references in downstream code.	6
Learned best practice	Add parameter validation to prevent null reference exceptions by checking required parameters at method entry points Add null validation check for the 'driver' parameter in the constructor to prevent potential NullReferenceException when accessing driver properties later. The validation should throw ArgumentNullException with a clear message. dotnet/src/webdriver/JavaScriptEngine.cs [58-64] public JavaScriptEngine(IWebDriver driver) { + this.driver = driver ?? throw new ArgumentNullException(nameof(driver)); + // Use of Lazy<T> means this exception won't be thrown until the user first // attempts to access the DevTools session, probably on the first call to // StartEventMonitoring() or in adding scripts to the instance. - this.driver = driver; this.session = new Lazy<DevToolsSession>(() => Apply this suggestion	6

---

Previous suggestions

Suggestions up to commit 8defe2d

Category	Suggestion	Score
Possible issue	Add null check before event Add null check for the valueChangeData.Element before invoking DomMutated event to prevent potential null reference exceptions when element is not found. dotnet/src/webdriver/JavaScriptEngine.cs [429-432] var locator = By.CssSelector($"*[data-__webdriver_id='{valueChangeData.TargetId}']"); valueChangeData.Element = driver.FindElements(locator).FirstOrDefault(); -this.DomMutated?.Invoke(this, new DomMutatedEventArgs(valueChangeData)); +if (valueChangeData.Element != null) +{ + this.DomMutated?.Invoke(this, new DomMutatedEventArgs(valueChangeData)); +} Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential null reference exception that could occur in production. Adding a null check before invoking the event handler is a critical safety measure.	8
	Handle null console arguments Add null check for e.Arguments[i].Value to prevent potential null reference exceptions when console arguments are null. dotnet/src/webdriver/JavaScriptEngine.cs [454-459] +var value = e.Arguments[i].Value ?? string.Empty; this.JavaScriptConsoleApiCalled(this, new JavaScriptConsoleApiCalledEventArgs ( - messageContent: e.Arguments[i].Value, + messageContent: value, messageTimeStamp: e.Timestamp, messageType: e.Type )); Suggestion importance[1-10]: 7 Why: The suggestion prevents potential null reference exceptions when handling console arguments, which is important for robust error handling and system stability.	7

[RenderMichael]

Test failures are unrelated to this PR

//java/test/org/openqa/selenium/mobile:NetworkConnectionTest
//java/test/org/openqa/selenium/remote:RemoteWebDriverScreenshotTest
//java/test/org/openqa/selenium/remote:RemoteWebDriverScreenshotTest-firefox-beta
//rb/spec/integration/selenium/webdriver:network-firefox-beta-bidi
//rb/spec/integration/selenium/webdriver:network-firefox-bidi