specify nullability in package org.openqa.selenium.bidi.browser

[asolntsev]

💥 What does this PR do?

• add @NullMarked and @nullable annotations
• add missing unit-tests
• add missing toString() methods - they can be useful for logging & debugging

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Type

Enhancement, Tests

---

Description

• Add nullability annotations (@NullMarked, @nullable) to BiDi browser package

• Create new DownloadBehavior class with validation logic

• Refactor SetDownloadBehaviorParameters to use DownloadBehavior

• Add toString() methods to ClientWindowInfo, DownloadBehavior, SetDownloadBehaviorParameters

• Add comprehensive unit tests for browser package classes

• Add isNull() validation method to Require utility class

• Deprecate old constructors in SetDownloadBehaviorParameters and ClientWindow

---

File Walkthrough

	Relevant files
Enhancement	5 files package-info.java Add @NullMarked annotation to package                                        +21/-0    ClientWindowInfo.java Add toString() method for logging                                                +7/-0      DownloadBehavior.java Create new DownloadBehavior class with validation                +69/-0    SetDownloadBehaviorParameters.java Refactor to use DownloadBehavior, add nullability                +29/-20  Require.java Add isNull() validation method                                                      +7/-0
Documentation	1 files ClientWindow.java Deprecate ClientWindow class with documentation                    +4/-0
Tests	4 files ClientWindowInfoTest.java Add unit tests for ClientWindowInfo                                            +42/-0    ClientWindowStateTest.java Add unit tests for ClientWindowState enum                                +45/-0    DownloadBehaviorTest.java Add comprehensive tests for DownloadBehavior                          +62/-0    SetDownloadBehaviorParametersTest.java Add tests for SetDownloadBehaviorParameters                            +86/-0
Configuration changes	1 files BUILD.bazel Add jspecify dependency for nullability annotations            +1/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Information disclosure Description: toMap() and toString() serialize and may log the absolute destinationFolder path (potentially user-controlled or environment-sensitive), which can expose local filesystem layout/usernames in logs, telemetry, or remote BiDi payloads. DownloadBehavior.java [54-68] Referred Code Map<String, String> toMap() { return allowed ? Map.of( "type", "allowed", "destinationFolder", requireNonNull(destinationFolder).toAbsolutePath().toString()) : Map.of("type", "denied"); } @Override public String toString() { return String.format( "DownloadBehavior{allowed=%s, destinationFolder=%s}", allowed, destinationFolder); }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix logic in deprecated constructor Fix the logic in the deprecated SetDownloadBehaviorParameters constructor to correctly handle allowed and destinationFolder arguments, preventing potential exceptions. java/src/org/openqa/selenium/bidi/browser/SetDownloadBehaviorParameters.java [44-49] @Deprecated @SuppressWarnings("DeprecatedIsStillUsed") public SetDownloadBehaviorParameters( @Nullable Boolean allowed, @Nullable Path destinationFolder) { - this(allowed == null ? null : new DownloadBehavior(allowed, destinationFolder)); + if (allowed == null) { + this.map.put("downloadBehavior", null); + } else if (allowed) { + this.map.put("downloadBehavior", DownloadBehavior.allowed(destinationFolder).toMap()); + } else { + if (destinationFolder != null) { + throw new IllegalArgumentException( + "destinationFolder should not be provided when allowed is false"); + } + this.map.put("downloadBehavior", DownloadBehavior.denied().toMap()); + } } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug in the deprecated constructor where it fails to validate arguments according to the constraints of the DownloadBehavior class, leading to exceptions.	Medium
	Use safe string formatting In ClientWindowInfo.toString(), change the format specifiers for nullable Integer fields from %d to %s to prevent a NullPointerException if they are null. java/src/org/openqa/selenium/bidi/browser/ClientWindowInfo.java [87-92] @Override public String toString() { return String.format( - "ClientWindowInfo{clientWindow='%s', state=%s, width=%d, height=%d, x=%d, y=%d, active=%s}", + "ClientWindowInfo{clientWindow='%s', state=%s, width=%s, height=%s, x=%s, y=%s, active=%s}", clientWindow, state, width, height, x, y, active); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that using %d with a null Integer will cause a NullPointerException and proposes using %s as a safe and effective fix.	Medium
Learned best practice	Return defensive map copy Returning the internal map allows callers to mutate SetDownloadBehaviorParameters state unexpectedly. Return an unmodifiable copy (e.g., Map.copyOf(map)) to preserve encapsulation. java/src/org/openqa/selenium/bidi/browser/SetDownloadBehaviorParameters.java [64-66] public Map<String, @Nullable Object> toMap() { - return map; + return Map.copyOf(map); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Do not expose internal mutable state; return an unmodifiable defensive copy from accessors.	Low
Update