[dotnet] [bidi] Support SetClientWindowState in Browser module

[nvborisenko]

User description

https://w3c.github.io/webdriver-bidi/#command-browser-setClientWindowState

browser.SetClientWindowState = (
  method: "browser.setClientWindowState",
  params: browser.SetClientWindowStateParameters
)

browser.SetClientWindowStateParameters = {
  clientWindow: browser.ClientWindow,
  (browser.ClientWindowNamedState // browser.ClientWindowRectState)
}

browser.ClientWindowNamedState = (
  state: "fullscreen" / "maximized" / "minimized"
)

browser.ClientWindowRectState = (
  state: "normal",
  ? width: js-uint,
  ? height: js-uint,
  ? x: js-int,
  ? y: js-int,
)

Motivation and Context

Improve bidi coverage.

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

---

Description

• Add SetClientWindowState command support for BiDi Browser module
• Implement named state (fullscreen/maximized/minimized) and rect state options
• Add public API methods for setting client window states
• Include test coverage for new functionality

---

Changes walkthrough 📝

	Relevant files
Enhancement	5 files BrowserModule.cs Add SetClientWindowState async methods                                      +14/-0    ClientWindow.cs Add BiDi reference and SetState methods                                    +16/-1    ClientWindowInfo.cs Inherit from EmptyResult base class                                            +4/-1      SetClientWindowStateCommand.cs Create new command with parameters and options                      +66/-0    BrowserClientWindowConverter.cs Accept BiDi parameter in constructor                                          +8/-1
Bug fix	1 files Broker.cs Pass BiDi instance to converter                                                    +2/-2
Configuration changes	1 files BiDiJsonSerializerContext.cs Register SetClientWindowStateCommand for serialization      +1/-0
Tests	1 files BrowserTest.cs Add tests for window state functionality                                  +24/-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.

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit 0335935)

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Property Initialization Properties in SetClientWindowRectStateCommandParameters are initialized from Options in the constructor but can be overwritten by public setters, potentially causing inconsistent state between constructor parameters and property values. public int? Width { get; set; } = Options?.Width; public int? Height { get; set; } = Options?.Height; public int? X { get; set; } = Options?.X; public int? Y { get; set; } = Options?.Y; Incomplete Tests Test methods have placeholder assertions that don't validate the actual functionality. The tests are also ignored, which means they won't run in CI/CD pipelines to catch regressions. // TODO: Make assertion meaningfull Assert.That(clientWindowInfo, Is.Not.Null); } [Test] [Ignore("Not yet implemented by all vendors")] public async Task CanSetClientWindowRectState() { var clientWindows = await bidi.Browser.GetClientWindowsAsync(); var clientWindowInfo = await clientWindows[0].ClientWindow.SetStateAsync(new() { Width = 500, Height = 300, X = 10, Y = 10 }); // TODO: Make assertion meaningfull Assert.That(clientWindowInfo, Is.Not.Null); Missing Null Check The Read method doesn't handle the case where reader.GetString() returns null, which could cause a NullReferenceException when creating the ClientWindow instance. var id = reader.GetString(); return new ClientWindow(_bidi, id!);

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use property getters The properties in SetClientWindowRectStateCommandParameters are initialized in the constructor but never updated if Options changes. Consider using property getters that directly access the Options object instead of initializing the properties once. dotnet/src/webdriver/BiDi/Modules/Browser/SetClientWindowStateCommand.cs [34-46] internal record SetClientWindowRectStateCommandParameters(ClientWindow ClientWindow, [property: JsonIgnore] SetClientWindowRectStateOptions? Options) : SetClientWindowStateCommandParameters(ClientWindow) { [JsonInclude] internal string State { get; } = "normal"; - public int? Width { get; set; } = Options?.Width; + public int? Width => Options?.Width; - public int? Height { get; set; } = Options?.Height; + public int? Height => Options?.Height; - public int? X { get; set; } = Options?.X; + public int? X => Options?.X; - public int? Y { get; set; } = Options?.Y; + public int? Y => Options?.Y; } Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue where the properties are initialized only once from Options in the constructor, but won't reflect any changes to Options after initialization. Using property getters ensures the values always reflect the current state of the Options object, improving correctness and maintainability.	Medium
Learned best practice	Add null validation for constructor parameters to prevent potential null reference exceptions The constructor accepts bidi and id parameters but doesn't validate them for null values. Since these parameters are required for the class to function properly, you should add null checks using ArgumentNullException.ThrowIfNull() to ensure they're not null before assigning them to properties. dotnet/src/webdriver/BiDi/Modules/Browser/ClientWindow.cs [26-30] internal ClientWindow(BiDi bidi, string id) { + ArgumentNullException.ThrowIfNull(bidi, nameof(bidi)); + ArgumentNullException.ThrowIfNull(id, nameof(id)); + BiDi = bidi; Id = id; } Apply this suggestion Suggestion importance[1-10]: 6	Low
Update

[nvborisenko]

I am not sure I have to add ClientWindowInfo type explicitly here, since it is already added by the code 3 lines below. @RenderMichael do you know? I cannot verify, the tests are ignored.

We can leave this PR opened until we verify how it works with real browser. At this moment I verified serialization only.

[nvborisenko]

In draft because of not supported by browsers yet.

[qodo-merge-pro]

PR Code Suggestions ✨

No code suggestions found for the PR.