[java][bidi] Allow passing subscription parameters for events

[pujagani]

🔗 Related Issues

💥 What does this PR do?

It add a method that is extensible in future to allow passing subscription parameters for events related to BiDi.

🔧 Implementation Notes

Using the generated SubscriptionParameters is not ideal and would create circular dependency.

This change is precursor to the CDDL generator work.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s):
  • What was generated:
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

[qodo-code-review]

PR Summary by Qodo

Add scoped BiDi event subscriptions via SubscriptionScope

✨ Enhancement 🕐 10-20 Minutes

AI Description

• Introduce a SubscriptionScope transport type to carry context/user-context subscription scoping.
• Add new subscribe/listener overloads to pass scoping parameters into session.subscribe.
• Keep subscription parameter construction outside generated types to avoid generator coupling.

Diagram

graph TD
  module["Module subclasses"] --> handle["Handle"] --> bidi["BiDi"] --> conn["Connection"] --> remote{{"BiDi remote end"}}
  module --> scope["SubscriptionScope"] --> bidi

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Accept raw Map subscription params

• ➕ Maximum flexibility for future BiDi subscription options without new types
• ➕ Avoids any additional API type surface
• ➖ Exposes protocol-specific keys/values to callers and reduces type safety
• ➖ Harder to validate and document supported combinations

2. Use generated SubscriptionParameters types

• ➕ Strong typing aligned with spec/CDDL output
• ➕ Centralized validation/serialization
• ➖ Creates tight coupling (and potential circular dependencies) with generated code
• ➖ Blocks progress until generator output and packaging are finalized

3. Make SubscriptionScope immutable (builder or value type)

• ➕ Safer to share across threads and easier to reason about
• ➕ Enables simple equality/testing and future validation hooks
• ➖ More boilerplate now; may slow iteration while the generator work is in flight

Recommendation: The current approach (a small, transport-layer SubscriptionScope passed through new overloads) is a good precursor to CDDL generation: it keeps subscription-shaping capability without binding the API to generated parameter classes. If this becomes widely used, consider evolving SubscriptionScope toward an immutable value type and adding light validation/documentation of supported key combinations.

Files changed (4) +78 / -0

Enhancement (4) +78 / -0

BiDi.javaAdd scoped addListener overload that merges SubscriptionScope into subscribe params +13/-0

Add scoped addListener overload that merges SubscriptionScope into subscribe params

• Introduces a package-private addListener overload that accepts a SubscriptionScope, merges its map into the session.subscribe parameters, and subscribes to the event. Uses a mutable params map to combine scope keys with the required "events" field.

java/src/org/openqa/selenium/bidi/BiDi.java

Handle.javaExpose scoped subscribe overload for package-internal module usage +4/-0

Expose scoped subscribe overload for package-internal module usage

• Adds a new subscribe overload that forwards event/handler/scope to BiDi, enabling modules to request scoped subscriptions without accessing BiDi directly.

java/src/org/openqa/selenium/bidi/Handle.java

Module.javaAdd protected subscribe overload accepting SubscriptionScope +5/-0

Add protected subscribe overload accepting SubscriptionScope

• Extends the base Module API with a protected subscribe method that takes a SubscriptionScope, allowing generated modules to pass scoping information through the Handle.

java/src/org/openqa/selenium/bidi/Module.java

SubscriptionScope.javaIntroduce SubscriptionScope to model contexts and userContexts scoping +56/-0

Introduce SubscriptionScope to model contexts and userContexts scoping

• Adds a new @Beta transport-layer type with fluent setters for browsing contexts and user contexts. Provides toMap() that emits only non-empty parameters to be merged into session.subscribe.

java/src/org/openqa/selenium/bidi/SubscriptionScope.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 2.48 doesn't trigger javascript in link's href on click() 🎫 Instance ChroneDriver Error: ConnectFailure

✅ Compliance rules (platform): 18 rules

1. SubscriptionScope.contexts() missing Javadoc ✓ Resolved 📘 Rule violation ✧ Quality

Description

The new public API methods contexts(...) and userContexts(...) in SubscriptionScope lack
Javadoc, which makes the new API harder to understand and violates the requirement for complete
public method documentation (including tags).

Code

java/src/org/openqa/selenium/bidi/SubscriptionScope.java[R36-44]

+  public SubscriptionScope contexts(Set<String> contexts) {
+    this.contexts = Require.nonNull("Browsing context ids", contexts);
+    return this;
+  }
+
+  public SubscriptionScope userContexts(Set<String> userContexts) {
+    this.userContexts = Require.nonNull("User context ids", userContexts);
+    return this;
+  }

Evidence

Rule 330200/330201 requires Javadoc for all public API methods and complete tags. In
SubscriptionScope, the public methods contexts(Set<String> contexts) and
userContexts(Set<String> userContexts) have no preceding Javadoc blocks.

Rule 330200: Require Javadoc for all public API types and methods
Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/bidi/SubscriptionScope.java[31-44]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubscriptionScope` introduces new public methods but they have no Javadoc. This violates the project requirement to document public API methods and include complete tags.

## Issue Context
`SubscriptionScope` is `public` and annotated `@Beta`, so it is part of the user-visible Java API surface and should be documented.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/SubscriptionScope.java[36-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Scoped subscribe has no tests 📘 Rule violation ▣ Testability

Description

New subscription-scoping functionality was added (via SubscriptionScope and the new scoped
addListener/subscribe overloads), but no corresponding tests were added or updated to exercise
the new behavior.

Code

java/src/org/openqa/selenium/bidi/BiDi.java[R120-130]

+  <X> String addListener(Event<X> event, Consumer<X> handler, SubscriptionScope scope) {
+    Require.nonNull("Event to listen for", event);
+    Require.nonNull("Handler to call", handler);
+    Require.nonNull("Subscription scope", scope);
+
+    Map<String, Object> params = new HashMap<>(scope.toMap());
+    params.put("events", List.of(event.getMethod()));
+    String subscriptionId = subscribe(params);
+    connection.addListener(subscriptionId, event, handler);
+    return subscriptionId;
+  }

Evidence

Rule 389273 requires tests for new functionality. The PR adds a new scoped listener/subscription
path in BiDi.addListener(..., SubscriptionScope) and introduces SubscriptionScope map-building
logic, but there are no accompanying test changes covering these new code paths.

Rule 389273: Require tests for all new functionality and bug fixes
java/src/org/openqa/selenium/bidi/BiDi.java[120-130]
java/src/org/openqa/selenium/bidi/SubscriptionScope.java[31-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds new functionality for subscription scoping but does not add test coverage to ensure scoped subscriptions are correctly translated into `session.subscribe` parameters.

## Issue Context
This change adds a new API surface (`SubscriptionScope`) and a new subscription path that builds a params map from scope + events.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/BiDi.java[120-130]
- java/src/org/openqa/selenium/bidi/SubscriptionScope.java[46-55]
- java/test/org/openqa/selenium/bidi/BiDiTest.java[37-67]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Scope retains mutable sets ✓ Resolved 🐞 Bug ☼ Reliability

Description

SubscriptionScope stores caller-provided Set instances and places them directly into the
session.subscribe params map. If those sets are mutated concurrently with JSON serialization,
JsonOutput’s Collection iteration may throw ConcurrentModificationException (or serialize
inconsistent data), causing subscription setup to fail.

Code

java/src/org/openqa/selenium/bidi/SubscriptionScope.java[R33-53]

+  private Set<String> contexts = Set.of();
+  private Set<String> userContexts = Set.of();
+
+  public SubscriptionScope contexts(Set<String> contexts) {
+    this.contexts = Require.nonNull("Browsing context ids", contexts);
+    return this;
+  }
+
+  public SubscriptionScope userContexts(Set<String> userContexts) {
+    this.userContexts = Require.nonNull("User context ids", userContexts);
+    return this;
+  }
+
+  Map<String, Object> toMap() {
+    Map<String, Object> params = new HashMap<>();
+    if (!contexts.isEmpty()) {
+      params.put("contexts", contexts);
+    }
+    if (!userContexts.isEmpty()) {
+      params.put("userContexts", userContexts);
+    }

Evidence

SubscriptionScope forwards the stored sets into the params map; Connection.send serializes the
params directly, and JsonOutput handles Collection values by streaming over them, which is
susceptible to concurrent modification of non-thread-safe sets.

java/src/org/openqa/selenium/bidi/SubscriptionScope.java[33-53]
java/src/org/openqa/selenium/bidi/Connection.java[140-151]
java/src/org/openqa/selenium/json/JsonOutput.java[189-203]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SubscriptionScope` keeps direct references to the caller’s `Set<String>` instances (`contexts`, `userContexts`) and returns them via `toMap()`. Those sets are later serialized by `Connection.send()` using `JsonOutput`, which iterates `Collection` values; if a caller mutates a non-thread-safe set concurrently, serialization can throw or produce inconsistent output.

### Issue Context
This is primarily an API-ownership / thread-safety hardening issue: callers can pass mutable sets (e.g., `HashSet`) and reuse them across threads while subscribing.

### Fix Focus Areas
- java/src/org/openqa/selenium/bidi/SubscriptionScope.java[33-54]

### Suggested fix
- Snapshot inputs in the setters, e.g. `this.contexts = Set.copyOf(contexts);` and `this.userContexts = Set.copyOf(userContexts);`.
- (Optional) If you want to preserve insertion order for JSON output, consider copying to `List.copyOf(...)` in `toMap()` instead of returning a `Set`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. BiDi scope cross-binding mismatch ✓ Resolved 📘 Rule violation ≡ Correctness

Description

The new Java API adds userContexts scoping support, but other bindings appear to differ (some
support only contexts), and the change is not documented as an intentional cross-binding
divergence.

Code

java/src/org/openqa/selenium/bidi/SubscriptionScope.java[R26-44]

+/**
+ * Where a subscription applies: globally, or scoped to browsing contexts and/or user contexts. Part
+ * of the transport layer, not generated — the remote end decides which combinations are valid.
+ */
+@Beta
+public final class SubscriptionScope {
+
+  private Set<String> contexts = Set.of();
+  private Set<String> userContexts = Set.of();
+
+  public SubscriptionScope contexts(Set<String> contexts) {
+    this.contexts = Require.nonNull("Browsing context ids", contexts);
+    return this;
+  }
+
+  public SubscriptionScope userContexts(Set<String> userContexts) {
+    this.userContexts = Require.nonNull("User context ids", userContexts);
+    return this;
+  }

Evidence

Rule 389265 requires comparing cross-language bindings when changing user-visible behavior. Java now
exposes scoping via SubscriptionScope including userContexts, while the JavaScript and .NET
bindings’ subscribe APIs shown here only expose context scoping; Ruby exposes both, indicating
potential cross-binding inconsistency that should be explicitly checked and documented.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
java/src/org/openqa/selenium/bidi/SubscriptionScope.java[31-53]
javascript/selenium-webdriver/bidi/index.js[231-269]
dotnet/src/webdriver/BiDi/Session/SessionModule.cs[47-52]
rb/lib/selenium/webdriver/bidi/protocol/session.rb[226-229]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR changes user-visible BiDi subscription scoping behavior in the Java binding, but there is no nearby documentation explaining how this aligns (or intentionally diverges) from other language bindings.

## Issue Context
Other bindings implement `session.subscribe` with varying support for `contexts` and `userContexts`. The Java binding now exposes both via `SubscriptionScope`.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/SubscriptionScope.java[26-44]
- java/src/org/openqa/selenium/bidi/BiDi.java[120-130]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0c9782a