[dotnet] [bidi] Get tree command returns GetTreeResult object

[nvborisenko]

User description

💥 What does this PR do?

Be more closer to low-level specification.

🔧 Implementation Notes

User is still able to use previous code, like

var tree = await bidi.BrowsingContext.GetTreeAsync();
var theFirst = tree[0];
// or
var theFirst = tree.First();

And additionally:

var tree = await bidi.BrowsingContext.GetTreeAsync();
var allContext = tree.Contexts;

In general we reveal nested Enumerable property of Enumerable Result to support more properties in future.

💡 Additional Considerations

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement

---

Description

• Refactor BiDi result objects to expose nested properties

• Make GetTreeResult implement IReadOnlyList<BrowsingContextInfo>

• Add Contexts property to GetTreeResult for better API access

• Update multiple result classes to use public properties instead of private fields

---

Changes diagram

flowchart LR
  A["Private Fields"] --> B["Public Properties"]
  B --> C["IReadOnlyList Implementation"]
  C --> D["Enhanced API Access"]

Loading

---

Changes walkthrough 📝

	Relevant files
Enhancement	8 files GetTreeCommand.cs Refactor GetTreeResult to implement IReadOnlyList                +18/-1    BrowsingContextModule.cs Update GetTreeAsync return type                                                    +2/-5      BrowsingContext.cs Update GetTreeAsync method signature                                          +1/-2      GetTreeResultConverter.cs Add JSON converter for GetTreeResult                                          +43/-0    GetClientWindowsCommand.cs Expose ClientWindows as public property                                    +9/-7      GetUserContextsCommand.cs Expose UserContexts as public property                                      +7/-7      LocateNodesCommand.cs Expose Nodes as public property                                                    +7/-7      GetCookiesCommand.cs Expose Cookies as public property                                                +7/-7
Configuration changes	1 files Broker.cs Register GetTreeResultConverter                                                    +1/-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-code-review]

PR Reviewer Guide 🔍

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 Inconsistent Pattern The Nodes field is declared as readonly instead of a property like other similar classes in this PR. This breaks consistency with the established pattern of using properties. public readonly IReadOnlyList<Script.NodeRemoteValue> Nodes; Missing Import The System.Collections import is added but IEnumerable cast suggests it might be unnecessary. Verify if this import is actually needed or if explicit casting can be avoided. using System.Collections; using System.Collections.Generic; Incomplete Implementation The Write method throws NotImplementedException which could cause runtime errors if serialization is attempted. Consider implementing or documenting why serialization is not supported. public override void Write(Utf8JsonWriter writer, GetTreeResult value, JsonSerializerOptions options) { throw new NotImplementedException(); }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ Convert field to property Suggestion Impact: The suggestion was directly implemented - the public readonly field was converted to a property with a getter as suggested code diff: - public readonly IReadOnlyList<Script.NodeRemoteValue> Nodes; + public IReadOnlyList<Script.NodeRemoteValue> Nodes { get; } The Nodes field should be a property with a getter to maintain consistency with other similar classes in the codebase. This ensures proper encapsulation and follows C# conventions. dotnet/src/webdriver/BiDi/BrowsingContext/LocateNodesCommand.cs [47] -public readonly IReadOnlyList<Script.NodeRemoteValue> Nodes; +public IReadOnlyList<Script.NodeRemoteValue> Nodes { get; } [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that Nodes is a public field, while other similar classes in the PR use properties, and changing it improves consistency.	Low
Learned best practice	Add null validation before usage The code uses null-forgiving operator (!) without proper validation, which can lead to runtime exceptions. Add proper null checking before creating the GetTreeResult instance to ensure the deserialized contexts is not null. dotnet/src/webdriver/BiDi/Communication/Json/Converters/Enumerable/GetTreeResultConverter.cs [31-37] public override GetTreeResult Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { using var doc = JsonDocument.ParseValue(ref reader); var contexts = doc.RootElement.GetProperty("contexts").Deserialize(options.GetTypeInfo<IReadOnlyList<BrowsingContextInfo>>()); - return new GetTreeResult(contexts!); + ArgumentNullException.ThrowIfNull(contexts); + return new GetTreeResult(contexts); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add null checks and validation for parameters, properties, and return values before using them to prevent NullReferenceException and other runtime errors. This includes checking dictionary keys exist before accessing them, validating required JSON properties, and ensuring objects are not null before method calls.	Low
Update

[nvborisenko]

Thanks Nick