[dotnet] [bidi] Support getting of client windows in browser module

[nvborisenko]

User description

Motivation and Context

Fixes #14533

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, Tests

---

Description

• Added support for the browser.getClientWindows BiDi command.

• Implemented new converters and serializers for ClientWindow and GetClientWindowsResult.

• Introduced new classes for ClientWindow, ClientWindowInfo, and GetClientWindowsCommand.

• Added a test case for GetClientWindowsAsync with browser-specific exclusions.

---

Changes walkthrough 📝

	Relevant files
Enhancement	8 files Broker.cs Added converters for `ClientWindow` and `GetClientWindowsResult`. +2/-0      BiDiJsonSerializerContext.cs Registered new types for GetClientWindowsCommand and related results. +3/-0      BrowserClientWindowConverter.cs Added JSON converter for `ClientWindow`.                                  +42/-0    GetClientWindowsResultConverter.cs Added JSON converter for `GetClientWindowsResult`.              +44/-0    BrowserModule.cs Added `GetClientWindowsAsync` method to `BrowserModule`.  +5/-0      ClientWindow.cs Introduced `ClientWindow` class to represent client windows. +32/-0    ClientWindowInfo.cs Added `ClientWindowInfo` class and `ClientWindowState` enum. +34/-0    GetClientWindowsCommand.cs Implemented `GetClientWindowsCommand` and related result handling. +49/-0
Tests	1 files BrowserTest.cs Added test for `GetClientWindowsAsync` with browser exclusions. +12/-0

---

Need help?

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

Check out the documentation for more information.

[nvborisenko]

Still in draft, seems Chrome/Edge doesn't implement it yet. We can ignore these browsers in tests, or wait until will be supported by all browsers. Hard topic.

[RenderMichael]

@nvborisenko Just to clarify the implementation state of BiDi, is this only available on Firefox?

[nvborisenko]

https://wpt.fyi/results/webdriver/tests/bidi?label=experimental&label=master&aligned

[RenderMichael]

It seems like a lot of work and a lot of potential misses if we explicitly [Ignore] this test on Chrome/Edge. Maybe we can have some logic in BiDiTestFixture.BiDiSetUp to check if a feature exists on the current browser, and Assert.Ignore if it does not? We could have some attribute on the test like [BiDiCommand("browser.getClientWindows")] for this.

[nvborisenko]

Ignoring tests per specific browser is already supported. The question is whether we should release something when some browser doesn't support it yet.

[RenderMichael]

Thank you, I understand better the concern.

I think it is better for Selenium to avoid being the missing link in BiDi adoption. That is especially so for this command, since Firefox already supports it.

[nvborisenko]

Yes, in this particular case we can release. It is additional feature without any breaking change.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 14533 - Fully compliant Compliant requirements: Implement support for browser.getClientWindows BiDi command Command should return client windows information Support async operation (GetClientWindowsAsync)

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Unimplemented Method The Write method throws NotImplementedException. Consider implementing it or providing a reason why it's not needed. public override void Write(Utf8JsonWriter writer, GetClientWindowsResult value, JsonSerializerOptions options) { throw new NotImplementedException(); }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null check for safety Add null check before calling GetString() to prevent potential NullReferenceException if the JSON value is null dotnet/src/webdriver/BiDi/Communication/Json/Converters/BrowserClientWindowConverter.cs [33-35] +if (reader.TokenType == JsonTokenType.Null) + return null; var id = reader.GetString(); return new ClientWindow(id!); Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion adds important null handling that prevents potential runtime exceptions when processing JSON data, which is a critical safety improvement for the JSON converter.	Medium
	✅ Validate JSON property existence Suggestion Impact: The commit implements a null check for clientWindows property using JsonNode's null-conditional operator, achieving similar error prevention as suggested but with different syntax code diff: - var doc = JsonDocument.ParseValue(ref reader); - var clientWindows = doc.RootElement.GetProperty("clientWindows").Deserialize<IReadOnlyList<ClientWindowInfo>>(options); + var jsonNode = JsonNode.Parse(ref reader); + + var clientWindows = jsonNode?["clientWindows"].Deserialize<IReadOnlyList<ClientWindowInfo>>(options); Add null check for the 'clientWindows' property existence in JSON to prevent potential JsonException dotnet/src/webdriver/BiDi/Communication/Json/Converters/Enumerable/GetClientWindowsResultConverter.cs [35] -var clientWindows = doc.RootElement.GetProperty("clientWindows").Deserialize<IReadOnlyList<ClientWindowInfo>>(options); +if (!doc.RootElement.TryGetProperty("clientWindows", out JsonElement clientWindowsElement)) + throw new JsonException("Missing required 'clientWindows' property"); +var clientWindows = clientWindowsElement.Deserialize<IReadOnlyList<ClientWindowInfo>>(options); Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion adds crucial error handling to validate JSON structure before deserialization, preventing potential runtime exceptions and providing clearer error messages.	Medium

[nvborisenko]

CI failed not related to this PR.