[java][BiDi] add clearListners via browsingContextIds for inspectors

[Delta456]

🔗 Related Issues

💥 What does this PR do?

Add clearListners via browsingContextIds for Inspectors for Java BiDi

Followup of #17130 specifically #17130 (comment)

🔧 Implementation Notes

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): GitHub Copilot
  • What was generated: The tests
  • 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]

Review Summary by Qodo

Add clearListener support for browsing context IDs in BiDi inspectors

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add clearListener method to BiDi supporting browsing context IDs
• Implement clearListener and clearListeners methods in inspector modules
• Add comprehensive tests for clearing listeners in BrowsingContext, Log, and Speculation inspectors
• Enable selective event listener cleanup for specific browsing contexts

File Changes

1. java/src/org/openqa/selenium/bidi/BiDi.java ✨ Enhancement +15/-0

Add clearListener with browsing context IDs support

• Added overloaded clearListener method accepting browsingContextIds parameter
• Sends session.unsubscribe command with contexts field to browser
• Maintains null checks and event subscription validation

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

---

2. java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java ✨ Enhancement +22/-0

Add clearListener methods for browsing contexts

• Added clearListener(String browsingContextId) method for single context
• Added clearListeners(Set<String> browsingContextIds) method for multiple contexts
• Clears all navigation and context events for specified browsing contexts
• Added import for List utility class

java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java

---

3. java/src/org/openqa/selenium/bidi/module/LogInspector.java ✨ Enhancement +10/-0

Add clearListener methods for log events

• Added clearListener(String browsingContextId) method for single context
• Added clearListeners(Set<String> browsingContextIds) method for multiple contexts
• Clears log entry added events for specified browsing contexts

java/src/org/openqa/selenium/bidi/module/LogInspector.java

---

View more (4)

4. java/src/org/openqa/selenium/bidi/module/SpeculationInspector.java ✨ Enhancement +10/-0

Add clearListener methods for speculation events

• Added clearListener(String browsingContextId) method for single context
• Added clearListeners(Set<String> browsingContextIds) method for multiple contexts
• Clears prefetch status updated events for specified browsing contexts

java/src/org/openqa/selenium/bidi/module/SpeculationInspector.java

---

5. java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java 🧪 Tests +69/-0

Add tests for BrowsingContextInspector clearListener functionality

• Added test canClearListenersForBrowsingContext verifying single context listener clearing
• Added test canClearListenersForMultipleBrowsingContexts verifying multiple contexts clearing
• Tests verify listeners can be re-subscribed after clearing
• Added imports for ArrayList, HashSet, Set, and CountDownLatch

java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java

---

6. java/test/org/openqa/selenium/bidi/log/LogInspectorTest.java 🧪 Tests +70/-0

Add tests for LogInspector clearListener functionality

• Added test canClearListenersForBrowsingContext verifying single context listener clearing
• Added test canClearListenersForMultipleBrowsingContexts verifying multiple contexts clearing
• Tests verify console log events can be received after re-subscribing
• Added imports for ArrayList, List, and Set

java/test/org/openqa/selenium/bidi/log/LogInspectorTest.java

---

7. java/test/org/openqa/selenium/bidi/speculation/SpeculationInspectorTest.java 🧪 Tests +112/-0

Add tests for SpeculationInspector clearListener functionality

• Added test canClearListenersForBrowsingContext verifying single context listener clearing
• Added test canClearListenersForMultipleBrowsingContexts verifying multiple contexts clearing
• Tests verify prefetch status events can be received after re-subscribing
• Added imports for HashSet and Set

java/test/org/openqa/selenium/bidi/speculation/SpeculationInspectorTest.java

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. LogInspector close no-ops 🐞 Bug ☼ Reliability

Description

LogInspector.close() creates a new Event instance (Log.entryAdded()) when clearing, so
BiDi.clearListener won’t find the originally-subscribed Event key and will skip
unsubscribe/cleanup.

Code

java/src/org/openqa/selenium/bidi/module/LogInspector.java[R165-167]

  @Override
  public void close() {
    this.bidi.clearListener(Log.entryAdded());

Evidence

LogInspector subscribes using the instance field logEntryAddedEvent = Log.entryAdded() and
passes that same object into bidi.addListener(...). But Connection stores callbacks in a
Map<Event<?>, ...> keyed by Event object identity, and BiDi.clearListener gates on
connection.isEventSubscribed(event) which uses containsKey(event). Since Log.entryAdded()
returns a new Event object each call and Event does not override equals/hashCode, close()
passes a different key and cleanup is skipped.

java/src/org/openqa/selenium/bidi/module/LogInspector.java[41-66]
java/src/org/openqa/selenium/bidi/module/LogInspector.java[147-153]
java/src/org/openqa/selenium/bidi/module/LogInspector.java[165-168]
java/src/org/openqa/selenium/bidi/Connection.java[177-201]
java/src/org/openqa/selenium/bidi/Connection.java[222-229]
java/src/org/openqa/selenium/bidi/Event.java[24-46]
java/src/org/openqa/selenium/bidi/log/Log.java[35-60]

Agent prompt

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

### Issue description
`LogInspector.close()` calls `bidi.clearListener(Log.entryAdded())`, but this creates a new `Event` instance that does not match the one used for subscription, so listener cleanup can silently no-op.

### Issue Context
`Connection` keys callbacks by `Event` object identity and `Event` has no `equals/hashCode` override.

### Fix Focus Areas
- java/src/org/openqa/selenium/bidi/module/LogInspector.java[41-68]
- java/src/org/openqa/selenium/bidi/module/LogInspector.java[147-168]

### Fix
- Change `close()` to clear using the same instance used for subscription, e.g. `this.bidi.clearListener(this.logEntryAddedEvent)` (or use the new context-aware clear if appropriate).
- Consider adding a regression test ensuring `close()` actually removes callbacks/unsubscribes (optional but recommended).

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

---

2. clearListeners accepts empty set ☑ 📘 Rule violation ≡ Correctness

Description

New clearListener(Set<String>, ...) APIs only validate non-null input, allowing an empty
browsingContextIds set to reach session.unsubscribe as contexts: [], which can cause protocol
errors or unclear no-op behavior. This fails to validate protocol-derived inputs early with a clear
exception or defined behavior.

Code

java/src/org/openqa/selenium/bidi/BiDi.java[R130-142]

+  public <X> void clearListener(Set<String> browsingContextIds, Event<X> event) {
+    Require.nonNull("List of browsing context ids", browsingContextIds);
+    Require.nonNull("Event to listen for", event);
+
+    // The browser throws an error if we try to unsubscribe an event that was not subscribed in the
+    // first place
+    if (connection.isEventSubscribed(event)) {
+      send(
+          new Command<>(
+              "session.unsubscribe",
+              Map.of("contexts", browsingContextIds, "events", List.of(event.getMethod()))));
+      connection.clearListener(event);
+    }

Evidence

The checklist requires validating external/protocol-derived inputs (including empty collections)
near the boundary with clear exceptions. The new overloads forward browsingContextIds directly
into the BiDi session.unsubscribe command without guarding against an empty set, and the new
Inspector helpers expose this behavior publicly.

java/src/org/openqa/selenium/bidi/BiDi.java[130-142]
java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java[244-263]
java/src/org/openqa/selenium/bidi/module/LogInspector.java[155-163]
java/src/org/openqa/selenium/bidi/module/SpeculationInspector.java[72-80]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
New `clearListener(Set<String>, Event<X>)` and Inspector `clearListeners(Set<String>)` methods only check for non-null sets. Passing an empty set can send `session.unsubscribe` with `contexts: []`, which may error at runtime or create ambiguous behavior.

## Issue Context
Other inspector subscription paths treat `browsingContextIds.isEmpty()` specially (subscribe without contexts). `clearListeners` should similarly define behavior for empty sets (either treat as global clear, or fail fast with a clear `IllegalArgumentException`).

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/BiDi.java[130-142]
- java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java[244-263]
- java/src/org/openqa/selenium/bidi/module/LogInspector.java[155-163]
- java/src/org/openqa/selenium/bidi/module/SpeculationInspector.java[72-80]

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

---

3. Context clear drops callbacks ☑ 🐞 Bug ≡ Correctness

Description

BiDi.clearListener(Set<String>, Event) unsubscribes for specific contexts but then removes the
entire local callback registry for that Event, causing unrelated listeners (including other
contexts/inspectors) to stop receiving events while the remote may remain subscribed for other
contexts.

Code

java/src/org/openqa/selenium/bidi/BiDi.java[R130-142]

+  public <X> void clearListener(Set<String> browsingContextIds, Event<X> event) {
+    Require.nonNull("List of browsing context ids", browsingContextIds);
+    Require.nonNull("Event to listen for", event);
+
+    // The browser throws an error if we try to unsubscribe an event that was not subscribed in the
+    // first place
+    if (connection.isEventSubscribed(event)) {
+      send(
+          new Command<>(
+              "session.unsubscribe",
+              Map.of("contexts", browsingContextIds, "events", List.of(event.getMethod()))));
+      connection.clearListener(event);
+    }

Evidence

The new method always calls connection.clearListener(event) after sending a context-scoped
unsubscribe. In Connection, callbacks are stored only by Event object (no browsing-context
dimension), and clearListener removes the whole entry for that Event, so clearing for one
context necessarily clears callbacks for all contexts associated with that Event key. This is
especially risky for inspectors using shared/static Event instances (e.g., in
BrowsingContextInspector), where multiple inspector instances may share the same Event key.

java/src/org/openqa/selenium/bidi/BiDi.java[119-143]
java/src/org/openqa/selenium/bidi/Connection.java[177-230]
java/src/org/openqa/selenium/bidi/Connection.java[193-201]
java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java[84-126]

Agent prompt

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

### Issue description
`BiDi.clearListener(Set<String>, Event)` performs a context-scoped `session.unsubscribe` but then clears all local callbacks for that `Event`, which is not context-scoped and can break other active listeners.

### Issue Context
`Connection` currently keys callbacks only by `Event` object; it does not track which callbacks belong to which browsing context(s). A context-aware clear must not delete callbacks indiscriminately.

### Fix Focus Areas
- java/src/org/openqa/selenium/bidi/BiDi.java[130-142]
- java/src/org/openqa/selenium/bidi/Connection.java[177-230]
- java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java[244-263]

### What to change (one viable approach)
- Introduce context-aware callback tracking (e.g., store callbacks keyed by `(eventMethod, contextId)` or similar) and filter dispatch accordingly.
- Only remove the callbacks associated with the contexts being cleared.
- Only send `session.unsubscribe` for a context when there are no remaining callbacks needing that `(event, context)` subscription.

### Minimal-risk interim option (if full context tracking is too large)
- Do **not** call `connection.clearListener(event)` from the context-scoped clear method; instead provide/implement a context-aware removal path so clearing one context doesn’t delete callbacks for all contexts.

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

---

4. Racy ArrayList in tests 🐞 Bug ☼ Reliability

Description

New tests append events into ArrayList from BiDi callback threads while the test thread
reads/asserts, which is a data race and can cause flaky test failures.

Code

java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java[R361-368]

+    try (BrowsingContextInspector inspector = new BrowsingContextInspector(driver)) {
+      List<NavigationInfo> receivedEvents = new ArrayList<>();
+      CountDownLatch latch = new CountDownLatch(1);
+      inspector.onNavigationStarted(
+          info -> {
+            receivedEvents.add(info);
+            latch.countDown();
+          });

Evidence

BiDi events are dispatched on a separate executor thread (Connection.Listener.onText uses
EXECUTOR.execute(...)). The new tests mutate ArrayList from the event callback and then read it
from the main test thread after await, which is unsynchronized and not thread-safe.

java/src/org/openqa/selenium/bidi/Connection.java[263-275]
java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java[361-375]
java/test/org/openqa/selenium/bidi/log/LogInspectorTest.java[472-488]
java/test/org/openqa/selenium/bidi/speculation/SpeculationInspectorTest.java[232-254]

Agent prompt

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

### Issue description
Several new tests collect events into `ArrayList` inside BiDi callbacks, but callbacks run on a different thread than the assertions; this can cause racy/flaky behavior.

### Issue Context
`Connection.Listener.onText` dispatches events via `EXECUTOR.execute(...)`, so event consumers run asynchronously.

### Fix Focus Areas
- java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java[361-375]
- java/test/org/openqa/selenium/bidi/log/LogInspectorTest.java[472-488]
- java/test/org/openqa/selenium/bidi/speculation/SpeculationInspectorTest.java[232-266]
- java/src/org/openqa/selenium/bidi/Connection.java[263-275]

### Fix
- Replace `new ArrayList<>()` with a thread-safe structure (e.g., `CopyOnWriteArrayList`, `Collections.synchronizedList(new ArrayList<>())`, or `ConcurrentLinkedQueue`).
- Alternatively, avoid shared mutable collections and use `CompletableFuture`, `AtomicReference`, or a `BlockingQueue` to transfer a single event to the test thread.

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

---

[asolntsev]

@Delta456 Seems that AI comments are reasonable, I would start from them.

[Delta456]

@Delta456 Seems that AI comments are reasonable, I would start from them.

Done.