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

---

Description

• Added support for SetClientWindowState in the Browser module.

  • Introduced new command classes and parameters for handling window states.
  • Added methods for setting named and rectangular window states.

• Enhanced ClientWindow to include BiDi reference and state-setting methods.

• Updated JSON serialization to include SetClientWindowStateCommand.

• Added unit tests for SetClientWindowState functionality.

---

Changes walkthrough 📝

	Relevant files
Enhancement	6 files Broker.cs Updated `BrowserClientWindowConverter` to include BiDi reference +2/-2      BiDiJsonSerializerContext.cs Added JSON serialization for `SetClientWindowStateCommand` +1/-0      BrowserClientWindowConverter.cs Modified `BrowserClientWindowConverter` to use BiDi reference +8/-1      BrowserModule.cs Added methods for setting client window states                      +14/-1    ClientWindow.cs Enhanced `ClientWindow` with state-setting methods              +16/-1    SetClientWindowStateCommand.cs Introduced `SetClientWindowStateCommand` and related classes +66/-0
Tests	1 files BrowserTest.cs Added unit tests for `SetClientWindowState` functionality +22/-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 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Property Initialization The properties in SetClientWindowRectStateCommandParameters are initialized from Options which could be null, but there's no null check before accessing its properties. Consider using the null-conditional operator or adding a null check. 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 The tests for SetClientWindowNamedState and SetClientWindowRectState only verify that the result is not null, but don't validate the actual state changes. The TODO comments indicate these assertions need to be improved. // TODO: Make assertion meaningfull Assert.That(clientWindowInfo, Is.Not.Null); } [Test] 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);

[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.