[java][bidi]: add DownloadInfo class and test for onDownloadWillBegin

[navin772]

User description

🔗 Related Issues

💥 What does this PR do?

Adds a DownloadInfo class for download related info. As per the spec we have a suggestedFilename param which was earlier not present in the selenium java bindings.

This PR also adds a test for the onDownloadWillBegin event.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement

---

Description

• Add DownloadInfo class extending NavigationInfo with suggestedFilename field

• Update onDownloadWillBegin event to use DownloadInfo instead of NavigationInfo

• Add test for onDownloadWillBegin event functionality

• Make NavigationInfo constructor protected for inheritance

---

Diagram Walkthrough

flowchart LR
  A["NavigationInfo"] --> B["DownloadInfo extends NavigationInfo"]
  B --> C["BrowsingContextInspector"]
  C --> D["onDownloadWillBegin event"]
  D --> E["Test validation"]

Loading

File Walkthrough

	Relevant files
Enhancement	DownloadInfo.java New DownloadInfo class with suggestedFilename                        java/src/org/openqa/selenium/bidi/browsingcontext/DownloadInfo.java Create new DownloadInfo class extending NavigationInfo Add suggestedFilename field with getter method Implement JSON deserialization with fromJson method +80/-0    NavigationInfo.java Make NavigationInfo constructor protected                                java/src/org/openqa/selenium/bidi/browsingcontext/NavigationInfo.java Change constructor visibility from private to protected Enable inheritance for DownloadInfo class +1/-1      BrowsingContextInspector.java Update onDownloadWillBegin to use DownloadInfo                      java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java Add DownloadInfo import and mapper function Update onDownloadWillBegin method to use DownloadInfo type Add download event listener cleanup in close method +19/-2
Tests	BrowsingContextInspectorTest.java Add onDownloadWillBegin event test                                              java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java Add test for onDownloadWillBegin event Validate download info properties including suggested filename Mark test as not implemented for Firefox browser +23/-0

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1234 - Partially compliant Compliant requirements: None Non-compliant requirements: Fix regression for JS-in-href click behavior in Firefox. Add tests validating alert/JS execution on click. Requires further human verification: None 5678 - Partially compliant Compliant requirements: None Non-compliant requirements: Resolve ConnectFailure across multiple ChromeDriver instantiations. Add tests or documentation to verify multiple instance stability. Requires further human verification: None

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Handling Validate that all fields parsed from JSON, especially 'timestamp' and IDs, are present and non-null. Currently defaults (e.g., 0 for long) are used without validation, which could mask malformed events. public static DownloadInfo fromJson(JsonInput input) { String browsingContextId = null; String navigationId = null; long timestamp = 0; String url = null; String suggestedFilename = null; input.beginObject(); while (input.hasNext()) { switch (input.nextName()) { case "context": browsingContextId = input.read(String.class); break; case "navigation": navigationId = input.read(String.class); break; case "timestamp": timestamp = input.read(Long.class); break; case "url": url = input.read(String.class); break; case "suggestedFilename": suggestedFilename = input.read(String.class); break; default: input.skipValue(); break; } } input.endObject(); return new DownloadInfo(browsingContextId, navigationId, timestamp, url, suggestedFilename); } Listener Cleanup Ensure no listeners remain when specific context-scoped listeners are added; verify that 'downloadWillBeginEvent' respects context filters like other navigation events and is always cleared in close(). public void onBrowsingContextLoaded(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.load", consumer); } public void onDownloadWillBegin(Consumer<DownloadInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(downloadWillBeginEvent, consumer); } else { this.bidi.addListener(browsingContextIds, downloadWillBeginEvent, consumer); } } public void onNavigationAborted(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationAborted", consumer); } public void onNavigationFailed(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationFailed", consumer); } public void onNavigationCommitted(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationCommitted", consumer); } public void onUserPromptClosed(Consumer<UserPromptClosed> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(userPromptClosed, consumer); } else { this.bidi.addListener(browsingContextIds, userPromptClosed, consumer); } } public void onUserPromptOpened(Consumer<UserPromptOpened> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(userPromptOpened, consumer); } else { this.bidi.addListener(browsingContextIds, userPromptOpened, consumer); } } public void onHistoryUpdated(Consumer<HistoryUpdated> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(historyUpdated, consumer); } else { this.bidi.addListener(browsingContextIds, historyUpdated, consumer); } } private void addNavigationEventListener(String name, Consumer<NavigationInfo> consumer) { Event<NavigationInfo> navigationEvent = new Event<>(name, navigationInfoMapper); navigationEventSet.add(navigationEvent); if (browsingContextIds.isEmpty()) { this.bidi.addListener(navigationEvent, consumer); } else { this.bidi.addListener(browsingContextIds, navigationEvent, consumer); } } @Override public void close() { this.bidi.clearListener(browsingContextCreated); this.bidi.clearListener(browsingContextDestroyed); this.bidi.clearListener(userPromptOpened); this.bidi.clearListener(userPromptClosed); this.bidi.clearListener(historyUpdated); this.bidi.clearListener(downloadWillBeginEvent); navigationEventSet.forEach(this.bidi::clearListener);

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Preserve API compatibility Changing onDownloadWillBegin from Consumer to Consumer risks breaking existing callers that pass a typed Consumer (generics are invariant). Add an overload that accepts Consumer and delegates to the DownloadInfo version (e.g., di -> consumer.accept(di)), and deprecate it to guide migration. This keeps the new functionality while maintaining source compatibility for current users. Examples: java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java [166-172] public void onDownloadWillBegin(Consumer<DownloadInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(downloadWillBeginEvent, consumer); } else { this.bidi.addListener(browsingContextIds, downloadWillBeginEvent, consumer); } } Solution Walkthrough: Before: // In BrowsingContextInspector.java public class BrowsingContextInspector implements AutoCloseable { // ... private final Event<DownloadInfo> downloadWillBeginEvent = new Event<>("browsingContext.downloadWillBegin", downloadWillBeginMapper); // ... // This is a breaking change from the previous API public void onDownloadWillBegin(Consumer<DownloadInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(downloadWillBeginEvent, consumer); } else { this.bidi.addListener(browsingContextIds, downloadWillBeginEvent, consumer); } } // ... } After: // In BrowsingContextInspector.java public class BrowsingContextInspector implements AutoCloseable { // ... private final Event<DownloadInfo> downloadWillBeginEvent = new Event<>("browsingContext.downloadWillBegin", downloadWillBeginMapper); // ... // New method with DownloadInfo public void onDownloadWillBegin(Consumer<DownloadInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(downloadWillBeginEvent, consumer); } else { this.bidi.addListener(browsingContextIds, downloadWillBeginEvent, consumer); } } // Deprecated overload for backward compatibility @Deprecated public void onDownloadWillBegin(Consumer<NavigationInfo> consumer) { onDownloadWillBegin(downloadInfo -> consumer.accept(downloadInfo)); } // ... } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a breaking API change in onDownloadWillBegin and proposes a standard, non-breaking solution using method overloading and deprecation, which is crucial for library maintenance.	High
Possible issue	Make timestamp parsing robust Read the timestamp as a generic number to avoid ClassCastException when the backend sends a floating-point value. Also guard against explicit nulls to prevent NullPointerException during unboxing. java/src/org/openqa/selenium/bidi/browsingcontext/DownloadInfo.java [54-56] case "timestamp": - timestamp = input.read(Long.class); + Number ts = input.read(Number.class); + timestamp = ts == null ? 0L : ts.longValue(); break; Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that reading a JSON number directly into a Long can cause a ClassCastException if it has a decimal part or a NullPointerException if the value is null. Changing the code to read a Number and then convert it to a long with a null check makes the parsing more robust.	Medium
Learned best practice	Add parameter null check Guard against a null input to prevent a NullPointerException when reading JSON. java/src/org/openqa/selenium/bidi/browsingcontext/DownloadInfo.java [36-41] public static DownloadInfo fromJson(JsonInput input) { + java.util.Objects.requireNonNull(input, "JsonInput must not be null"); String browsingContextId = null; String navigationId = null; long timestamp = 0; String url = null; String suggestedFilename = null; Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add null checks for parameters to prevent NullPointerExceptions.	Low
Update

[pujagani]

Thank you! LGTM @navin772

[navin772]

Failing test is unrelated.