SeleniumHQ / SeleniumHQ/selenium
[🐛 Bug]: [java][bidi] `BiDi.clearListener(Set<String> browsingContextIds, Event<X> event)` no longer scopes to browsing context, causing stale events to leak to new subscribers
- Dominant language
- Java
- Stars
- 34.5k
- Forks
- 8.7k
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 92
Description
### Description
`BiDi.clearListener(Set browsingContextIds, Event event)` used to unsubscribe only the browser-side subscription(s) scoped to the given browsing context(s), via a `contextListenerIds` map. Since #17759 ("[java][bidi] Track events subscriptions using subscription ids"), the method ignores the `browsingContextIds` argument entirely and just delegates to a blanket clear:
```java
// java/src/org/openqa/selenium/bidi/BiDi.java
public void clearListener(Set browsingContextIds, Event event) {
Require.nonEmpty("List of browsing context ids", browsingContextIds);
// This is not the correct implementation, everything needs to be tired to subscription id.
// This is currently used by hand-written BiDi code, soon to be replaced by generated BiDi code.
// After that this method will be removed.
// This is just to avoid changes to hand-written classes meanwhile.
this.clearListener(event);
}
```
`clearListener(event)` does a full `session.unsubscribe` for the event type and wipes **every** registered handler for it, then callers immediately resubscribe. Because `Connection.handleEventResponse` dispatches an incoming event to *every* consumer registered for that event method (it never filters by which specific browsing-context/subscription the event actually belongs to), a late/in-flight event tied to the *old* subscription can land on the *newly (re)subscribed* listener during the brief unsubscribe→resubscribe window.
This surfaced as a consistent (non-flaky, 2/2 reproductions) CI failure on `SpeculationInspectorTest`:
```
org.opentest4j.AssertionFailedError:
expected: ".../common/square.png"
but was: ".../common/dummy.xml"
at org.openqa.selenium.bidi.speculation.SpeculationInspectorTest.canClearListenersForBrowsingContext(SpeculationInspectorTest.java:280)
at org.openqa.selenium.bidi.speculation.SpeculationInspectorTest.canClearListenersForMultipleBrowsingContexts(SpeculationInspectorTest.java:337)
```
Both tests: subscribe → prefetch `dummy.xml` → clear listener for the browsing context → resubscribe → prefetch `square.png` → assert the first new event's URL is `square.png`. The first event received by the new subscription is instead the stale `dummy.xml` status update from before the clear, because the clear no longer isolates by context and the dispatch has no context/subscription filtering to fall back on.
Seen on the `release-preparation-selenium-4.46.0` branch, run: https://github.com/SeleniumHQ/selenium/actions/runs/29127293306/job/86475511308 (PR #17763 — unrelated to the failure; the release-prep PR doesn't touch this code at all, the regression was already on `trunk` via #17759, merged the day before).
### How to reproduce
```
bazel test //java/test/org/openqa/selenium/bidi/speculation:SpeculationInspectorTest-chrome-beta --runs_per_test=3
```
(or any Chrome variant) — `canClearListenersForBrowsingContext` / `canClearListenersForMultipleBrowsingContexts` fail deterministically enough to show up in CI.
### Suggested direction
Restore context-scoped unsubscribe (like the pre-#17759 `contextListenerIds` approach), or — since the comment says this is meant to be temporary until "generated BiDi code" replaces the hand-written classes — make sure `Connection`'s event dispatch filters by the subscription/context that produced the event rather than broadcasting to every handler registered for that event method.
### Environment
Trunk, as of commit 9f6ccde23171bd076a05d2c43e8274c227e19ccf (#17759).
Contributor guide
Assessment
This issue has not been assessed yet.