Skip to content

[java][BiDi] implement speculation module#17130

Open
Delta456 wants to merge 10 commits intoSeleniumHQ:trunkfrom
Delta456:java_bidi_speculation
Open

[java][BiDi] implement speculation module#17130
Delta456 wants to merge 10 commits intoSeleniumHQ:trunkfrom
Delta456:java_bidi_speculation

Conversation

@Delta456
Copy link
Member

💥 What does this PR do?

This PR implements an external BiDi nav speculation for Java binding and adds speculation.PrefetchStatusUpdated.

The speculation module contains commands for managing the remote end behavior for prefetches, prerenders, and speculation rules.

🔧 Implementation Notes

Took tests from https://wpt.fyi/results/webdriver/tests/bidi/external/speculation?label=master&label=stable&aligned

💡 Additional Considerations

🔄 Types of changes

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

@Delta456 Delta456 requested a review from asolntsev February 23, 2026 08:20
@qodo-code-review
Copy link
Contributor

PR Type

Enhancement


Description

  • Implements BiDi speculation module for Java bindings

  • Adds PrefetchStatusUpdated event listener with status tracking

  • Provides SpeculationInspector for managing prefetch and prerender behavior

  • Includes comprehensive test coverage for prefetch status events


File Walkthrough

Relevant files
Enhancement
4 files
SpeculationInspector.java
New SpeculationInspector module for BiDi events                   
+76/-0   
PrefetchStatusUpdatedParameters.java
Event parameters for prefetch status updates                         
+54/-0   
PreloadingStatus.java
Enum for preloading status values                                               
+45/-0   
Speculation.java
Speculation module event factory class                                     
+28/-0   
Documentation
1 files
package-info.java
Package documentation with null safety annotations             
+21/-0   
Tests
1 files
SpeculationInspectorTest.java
Comprehensive tests for speculation inspector functionality
+282/-0 
Configuration changes
3 files
BUILD.bazel
Add speculation module dependency to build                             
+1/-0     
BUILD.bazel
Build configuration for speculation module                             
+24/-0   
BUILD.bazel
Build configuration for speculation tests                               
+31/-0   

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Feb 23, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Feb 23, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: 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: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null checks: fromJson blindly casts and consumes params.get(...) values without checking for missing
keys/nulls, risking NullPointerException/ClassCastException and poor diagnostics for
malformed BiDi event payloads.

Referred Code
public static PrefetchStatusUpdatedParameters fromJson(Map<String, Object> params) {
  String context = (String) params.get("context");
  String url = (String) params.get("url");
  String statusStr = (String) params.get("status");
  PreloadingStatus status = PreloadingStatus.fromString(statusStr);

  return new PrefetchStatusUpdatedParameters(context, url, status);

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:
Unvalidated external input: BiDi event data in fromJson is treated as trusted input (no validation of required
fields/types such as context, url, and status), which can lead to runtime failures when
the remote end returns unexpected or malformed payloads.

Referred Code
public static PrefetchStatusUpdatedParameters fromJson(Map<String, Object> params) {
  String context = (String) params.get("context");
  String url = (String) params.get("url");
  String statusStr = (String) params.get("status");
  PreloadingStatus status = PreloadingStatus.fromString(statusStr);

  return new PrefetchStatusUpdatedParameters(context, url, status);

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
Copy link
Contributor

qodo-code-review bot commented Feb 23, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Implement commands for managing speculation rules

The PR should be extended to include commands for managing speculation rules,
such as speculation.setSpeculativeRules. This would eliminate the need for users
to inject JavaScript to control speculation behavior.

Examples:

java/test/org/openqa/selenium/bidi/speculation/SpeculationInspectorTest.java [62-83]
  void addSpeculationRulesAndLink(String rules, String href, String linkText, String linkId) {
    String expression =
        String.format(
            "const script = document.createElement('script');"
                + "script.type = 'speculationrules';"
                + "script.textContent = `%s`;"
                + "document.head.appendChild(script);"
                + "const link = document.createElement('a');"
                + "link.href = '%s';"
                + "link.textContent = '%s';"

 ... (clipped 12 lines)

Solution Walkthrough:

Before:

// In SpeculationInspectorTest.java
void addSpeculationRulesAndLink(String rules, String href, ...) {
    String expression =
        String.format(
            "const script = document.createElement('script');"
                + "script.type = 'speculationrules';"
                + "script.textContent = `%s`;"
                + "document.head.appendChild(script); ...",
            rules, href, ...);

    script.callFunctionInBrowsingContext(
        driver.getWindowHandle(),
        expression,
        ...);
}

After:

// In a new/updated class like `Speculation`
public class Speculation {
  // ... existing event methods

  public Command<Void> setSpeculativeRules(Set<String> contexts, List<Rule> rules) {
    // ... implementation to send 'speculation.setSpeculativeRules' command
  }
}

// Test usage would be cleaner, without JS injection
@Test
void canListenToPrefetchStatusUpdated() {
  List<Rule> rules = ...;
  speculation.setSpeculativeRules(Set.of(driver.getWindowHandle()), rules);
  // ...
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a major functional gap, as the PR introduces the speculation module but omits the commands needed to control it, making the feature incomplete and reliant on JavaScript workarounds.

High
Possible issue
Avoid unintended side effects during cleanup

Modify SpeculationInspector to track subscription IDs of created listeners. In
the close() method, remove listeners individually by their ID instead of
clearing all listeners for the event, preventing unintended side effects.

java/src/org/openqa/selenium/bidi/module/SpeculationInspector.java [33-76]

+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+...
+
 public class SpeculationInspector implements AutoCloseable {
   private final Event<PrefetchStatusUpdatedParameters> prefetchStatusUpdatedEvent;
   private final Set<String> browsingContextIds;
+  private final List<Long> subscriptionIds = new CopyOnWriteArrayList<>();
 
   private final BiDi bidi;
 
   public SpeculationInspector(WebDriver driver) {
 ...
   public long onPrefetchStatusUpdated(Consumer<PrefetchStatusUpdatedParameters> consumer) {
+    long subscriptionId;
     if (browsingContextIds.isEmpty()) {
-      return this.bidi.addListener(this.prefetchStatusUpdatedEvent, consumer);
+      subscriptionId = this.bidi.addListener(this.prefetchStatusUpdatedEvent, consumer);
     } else {
-      return this.bidi.addListener(browsingContextIds, this.prefetchStatusUpdatedEvent, consumer);
+      subscriptionId =
+          this.bidi.addListener(browsingContextIds, this.prefetchStatusUpdatedEvent, consumer);
     }
+    this.subscriptionIds.add(subscriptionId);
+    return subscriptionId;
   }
 
   public void removeListener(long subscriptionId) {
+    this.subscriptionIds.remove(subscriptionId);
     this.bidi.removeListener(subscriptionId);
   }
 
   @Override
   public void close() {
-    this.bidi.clearListener(Speculation.prefetchStatusUpdated());
+    this.subscriptionIds.forEach(this.bidi::removeListener);
   }
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies that bidi.clearListener() can cause unintended side effects by removing all listeners for an event, not just those from the current instance. The proposed solution of tracking and removing individual subscription IDs makes the SpeculationInspector class more robust and prevents it from interfering with other components.

Medium
General
Improve test reliability with single listener

Refactor the canListenToPrefetchStatusUpdatedWithNavigationAndSuccess test to
use a single event listener with multiple CompletableFutures. This avoids
potential race conditions and improves test reliability by removing the need for
two separate listeners.

java/test/org/openqa/selenium/bidi/speculation/SpeculationInspectorTest.java [128-185]

 @Test
 @NeedsFreshDriver
 @NotYetImplemented(FIREFOX)
 @NotYetImplemented(SAFARI)
 void canListenToPrefetchStatusUpdatedWithNavigationAndSuccess()
     throws ExecutionException, InterruptedException, TimeoutException {
-  CountDownLatch latch = new CountDownLatch(2);
-  List<PrefetchStatusUpdatedParameters> events = new ArrayList<>();
+  CompletableFuture<PrefetchStatusUpdatedParameters> pendingFuture = new CompletableFuture<>();
+  CompletableFuture<PrefetchStatusUpdatedParameters> readyFuture = new CompletableFuture<>();
+  CompletableFuture<PrefetchStatusUpdatedParameters> successFuture = new CompletableFuture<>();
 
   speculationInspector.onPrefetchStatusUpdated(
       event -> {
-        events.add(event);
-        latch.countDown();
+        if (event.getStatus() == PreloadingStatus.PENDING) {
+          pendingFuture.complete(event);
+        } else if (event.getStatus() == PreloadingStatus.READY) {
+          readyFuture.complete(event);
+        } else if (event.getStatus() == PreloadingStatus.SUCCESS) {
+          successFuture.complete(event);
+        }
       });
 
   String testUrl = appServer.whereIs("/common/blank.html");
   driver.get(testUrl);
 
   String prefetchTarget = appServer.whereIs("/common/dummy.xml");
   String speculationRules =
       String.format(
           "{\"prefetch\": [{\"where\": {\"href_matches\": \"%s\"}, \"eagerness\":"
               + " \"immediate\"}]}",
           prefetchTarget);
 
   addSpeculationRulesAndLink(speculationRules, prefetchTarget, "Test Link", "prefetch-page");
 
   // Wait for pending and ready events
-  latch.await(5, TimeUnit.SECONDS);
-
-  assertThat(events).hasSizeGreaterThanOrEqualTo(2);
-  assertThat(events.get(0).getStatus()).isEqualTo(PreloadingStatus.PENDING);
-  assertThat(events.get(1).getStatus()).isEqualTo(PreloadingStatus.READY);
-
-  // Set up for success event
-  CompletableFuture<PrefetchStatusUpdatedParameters> successFuture = new CompletableFuture<>();
-  speculationInspector.onPrefetchStatusUpdated(
-      event -> {
-        if (event.getStatus() == PreloadingStatus.SUCCESS) {
-          successFuture.complete(event);
-        }
-      });
+  pendingFuture.get(5, TimeUnit.SECONDS);
+  readyFuture.get(5, TimeUnit.SECONDS);
 
   // Navigate to the prefetched page by clicking the link
   script.callFunctionInBrowsingContext(
       driver.getWindowHandle(),
       "const link = document.getElementById('prefetch-page'); if (link) { link.click(); }",
       false,
       Optional.empty(),
       Optional.empty(),
       Optional.empty());
 
   // Wait for success event
   PrefetchStatusUpdatedParameters successEvent = successFuture.get(5, TimeUnit.SECONDS);
   assertThat(successEvent.getUrl()).isEqualTo(prefetchTarget);
   assertThat(successEvent.getStatus()).isEqualTo(PreloadingStatus.SUCCESS);
   assertThat(successEvent.getContext()).isEqualTo(driver.getWindowHandle());
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that using two separate listeners in the test is not ideal and could lead to race conditions. Refactoring to a single listener that manages multiple CompletableFutures is a much cleaner and more reliable pattern for testing asynchronous events, improving the test's robustness.

Medium
Learned
best practice
Validate parsed event payload fields

Add explicit null/type checks (and actionable error messages) for required
fields like context, url, and status to prevent ClassCastException/NPE when
parsing BiDi payloads.

java/src/org/openqa/selenium/bidi/speculation/PrefetchStatusUpdatedParameters.java [34-41]

 public static PrefetchStatusUpdatedParameters fromJson(Map<String, Object> params) {
-  String context = (String) params.get("context");
-  String url = (String) params.get("url");
-  String statusStr = (String) params.get("status");
+  Object contextObj = params.get("context");
+  Object urlObj = params.get("url");
+  Object statusObj = params.get("status");
+
+  if (!(contextObj instanceof String context) || context.isBlank()) {
+    throw new IllegalArgumentException("Missing/invalid 'context' in prefetchStatusUpdated");
+  }
+  if (!(urlObj instanceof String url) || url.isBlank()) {
+    throw new IllegalArgumentException("Missing/invalid 'url' in prefetchStatusUpdated");
+  }
+  if (!(statusObj instanceof String statusStr) || statusStr.isBlank()) {
+    throw new IllegalArgumentException("Missing/invalid 'status' in prefetchStatusUpdated");
+  }
+
   PreloadingStatus status = PreloadingStatus.fromString(statusStr);
-
   return new PrefetchStatusUpdatedParameters(context, url, status);
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate and sanitize untrusted input (e.g., JSON/maps) before use; fail fast with clear errors instead of relying on unsafe casts or allowing nulls to flow.

Low
  • Update

@Delta456 Delta456 force-pushed the java_bidi_speculation branch from a6e1b73 to 3970ac3 Compare February 23, 2026 09:25
Copy link
Contributor

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asolntsev asolntsev added this to the 4.42.0 milestone Feb 24, 2026
@asolntsev asolntsev added the P-enhancement PR with a new feature label Feb 24, 2026
@Delta456 Delta456 requested a review from asolntsev March 2, 2026 09:35
@Delta456 Delta456 force-pushed the java_bidi_speculation branch from fdce405 to ba8368e Compare March 2, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings P-enhancement PR with a new feature Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants