[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 (3) 📘 Rule violations (2) 📎 Requirement gaps (0)

Context used

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

1. Unrelated .idea/misc.xml change 📘 Rule violation ⚙ Maintainability ⭐ New

Description

The PR includes modifications to IntelliJ project metadata (.idea/misc.xml) that are unrelated to
the stated BiDi listener functionality. This adds review noise and can cause ongoing churn from
developer-local IDE state changes.

Code

.idea/misc.xml[R3-10]

+  <component name="ExternalStorageConfigurationManager" enabled="true" />
  <component name="JavaScriptSettings">
    <option name="languageLevel" value="ES6" />
  </component>
-  <component name="ProjectRootManager" version="2" languageLevel="JDK_11" default="false" project-jdk-name="11" project-jdk-type="JavaSDK">
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_11">
    <output url="file://$PROJECT_DIR$/build" />
  </component>
-</project>
+</project>

Evidence

The compliance checklist requires avoiding unrelated formatting/refactor churn; the added/modified
IDE configuration is not needed for implementing clearListener(s) behavior and increases diff
scope unnecessarily.

AGENTS.md
.idea/misc.xml[3-10]

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 includes changes to IntelliJ `.idea/misc.xml`, which is developer-local configuration and unrelated to the feature being implemented.

## Issue Context
Keeping IDE metadata out of PRs reduces churn and keeps diffs focused on product code.

## Fix Focus Areas
- .idea/misc.xml[3-10]

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

---

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

---

3. Context clear drops callbacks ✓ Resolved 🐞 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

---

View more (1)

4. clearListeners accepts empty set ✓ Resolved 📘 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

---

5. addListener(Set) now rejects empty 📘 Rule violation ⚙ Maintainability ⭐ New

Description

BiDi.addListener(Set<String>, ...) changed from only null-checking to rejecting empty sets, which
is a user-visible behavior change that can break existing callers who previously passed an empty
set. This risks an unintended compatibility break without a deprecation/transition path.

Code

java/src/org/openqa/selenium/bidi/BiDi.java[110]

+    Require.nonEmpty("List of browsing context ids", browsingContextIds);

Evidence

The checklist requires preserving public API/behavior compatibility. The diff shows the method now
throws for empty browsingContextIds, changing runtime behavior for previously accepted inputs.

AGENTS.md
java/src/org/openqa/selenium/bidi/BiDi.java[109-121]

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.addListener(Set<String> browsingContextIds, ...)` now enforces `Require.nonEmpty(...)` instead of `Require.nonNull(...)`, which can break existing consumers that passed an empty set.

## Issue Context
This is a public API method on `public class BiDi`, so tightening validation is an externally observable behavior change.

## Fix Focus Areas
- java/src/org/openqa/selenium/bidi/BiDi.java[109-121]

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

---

6. Stale contextListenerIds entries 🐞 Bug ⚙ Maintainability ⭐ New

Description

contextListenerIds is populated when adding context-specific listeners, but it is not cleaned up
when listeners are removed via removeListener(id) or cleared via
clearListener(event)/clearListeners(), and clearListener(Set, event) returns early when not
subscribed without removing stale entries. This leaves stale IDs/entries that can accumulate over
time and cause unnecessary work during later clears/removals.

Code

java/src/org/openqa/selenium/bidi/BiDi.java[R141-143]

+    if (!connection.isEventSubscribed(event)) {
+      return;
+    }

Evidence

The code shows contextListenerIds being populated on add, but no corresponding cleanup occurs in
the global clear/remove paths, and the early-return in context-clear skips any map cleanup.
Connection’s listener removal works by scanning/removing IDs from callback maps, so stale IDs don’t
crash but still represent stale state and extra work.

java/src/org/openqa/selenium/bidi/BiDi.java[39-40]
java/src/org/openqa/selenium/bidi/BiDi.java[119-121]
java/src/org/openqa/selenium/bidi/BiDi.java[124-133]
java/src/org/openqa/selenium/bidi/BiDi.java[141-143]
java/src/org/openqa/selenium/bidi/BiDi.java[161-167]
java/src/org/openqa/selenium/bidi/Connection.java[203-230]

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 new `contextListenerIds` map is written in `addListener(Set, ...)` but is not consistently cleaned up:
- `removeListener(long id)` does not remove `id` from any `contextListenerIds` lists.
- `clearListener(Event)` and `clearListeners()` do not clear related `contextListenerIds` entries.
- `clearListener(Set, Event)` returns early when `!connection.isEventSubscribed(event)` without cleaning `contextListenerIds`, leaving stale entries.

### Issue Context
`Connection.removeListener(id)` is safe even for non-existent IDs, but stale IDs/entries still consume memory and force extra scanning/cleanup work later.

### Fix Focus Areas
- java/src/org/openqa/selenium/bidi/BiDi.java[39-40]
- java/src/org/openqa/selenium/bidi/BiDi.java[119-121]
- java/src/org/openqa/selenium/bidi/BiDi.java[124-133]
- java/src/org/openqa/selenium/bidi/BiDi.java[141-143]
- java/src/org/openqa/selenium/bidi/BiDi.java[161-167]

### Suggested fix
1) Ensure `contextListenerIds` is cleaned when:
- `clearListener(Event)` succeeds (e.g., `contextListenerIds.remove(event)` after clearing).
- `clearListeners()` is called (e.g., `contextListenerIds.clear()`).
2) In `clearListener(Set, Event)`, before returning on `!connection.isEventSubscribed(event)`, remove any stale entry for that event from `contextListenerIds`.
3) Consider updating `removeListener(long id)` to also remove `id` from `contextListenerIds` (either by scanning lists or maintaining a reverse `id -> event` index).

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

---

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

---

Previous review results

Review updated until commit 5d07026

Results up to commit da1ed22

🐞 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. Context clear drops callbacks ✓ Resolved 🐞 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

---

3. clearListeners accepts empty set ✓ Resolved 📘 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

---

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.

[qodo-code-review]

Persistent review updated to latest commit 5d07026

[qodo-code-review]

1. Unrelated .idea/misc.xml change 📘 Rule violation ⚙ Maintainability

The PR includes modifications to IntelliJ project metadata (.idea/misc.xml) that are unrelated to
the stated BiDi listener functionality. This adds review noise and can cause ongoing churn from
developer-local IDE state changes.

Agent Prompt

## Issue description
The PR includes changes to IntelliJ `.idea/misc.xml`, which is developer-local configuration and unrelated to the feature being implemented.

## Issue Context
Keeping IDE metadata out of PRs reduces churn and keeps diffs focused on product code.

## Fix Focus Areas
- .idea/misc.xml[3-10]

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