#16645 switch DevTools connection between tabs/windows

[asolntsev]

User description

1. When createSessionIfThereIsNotOne(tab1) is called, CDP session is opened - and attached to the first tab.
2. When createSessionIfThereIsNotOne(tab2) is called, we detach connection from the first tab, and attach to the second tab (a new CDP session ID is created).

🔗 Related Issues

Fixes #16645

🔧 Implementation Notes

Current implementation holds only one CDP session at a time.
When user wants to switch to other tab, the previous CDP session gets closed.

An alternative solution would be holding map of CDP sessions for all existing tabs.
But ChatGPT suggested that it's worth it: every attachTarget and detachTarget is very fast (less than 1 ms), so holding multiple sessions would not give any benefit in speed.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Switch DevTools CDP session between tabs/windows dynamically

• Track current window handle to detect tab switches

• Disconnect from previous tab and attach to new tab

• Refactor session attachment logic into separate methods

---

Diagram Walkthrough

flowchart LR
  A["createSessionIfThereIsNotOne<br/>with windowHandle"] -->|"cdpSession == null"| B["createSession"]
  A -->|"windowHandle changed"| C["disconnectSession"]
  C --> D["attachToWindow"]
  B --> D
  D --> E["findTarget"]
  E --> F["attachToTarget"]
  F --> G["CDP Session Active<br/>on new tab"]

Loading

File Walkthrough

Relevant files

Bug fix

DevTools.java Implement dynamic DevTools tab switching with session management java/src/org/openqa/selenium/devtools/DevTools.java Added windowHandle field to track current tab/window connection Enhanced createSessionIfThereIsNotOne() to detect tab switches and reconnect Refactored session attachment logic into separate attachToWindow() and attachToTarget() methods Added @Nullable and @NonNull annotations for better null-safety Clear windowHandle when disconnecting session +20/-2

---

[qodo-code-review]

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: 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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logs: The new session switching logic (attach/detach between tabs) does not add any audit logging of critical actions like session attach/detach or window handle changes, making it hard to reconstruct events. Referred Code public void createSessionIfThereIsNotOne() { createSessionIfThereIsNotOne(null); } public void createSessionIfThereIsNotOne(@Nullable String windowHandle) { if (cdpSession == null) { createSession(windowHandle); } else if (!Objects.equals(this.windowHandle, windowHandle)) { disconnectSession(); attachToWindow(windowHandle); } } public void createSession() { createSession(null); } /** * Create CDP session on given window/tab (aka target). If windowHandle is null, then the first * "page" type will be selected. Pass the windowHandle if you have multiple windows/tabs opened to * connect to the expected window/tab. ... (clipped 15 lines) 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: Null window handle: New methods accept a nullable window handle but lack explicit handling or validation for null/empty cases when finding/attaching targets, which may cause ambiguous behavior during tab switches. Referred Code public void createSession(@Nullable String windowHandle) { if (connection.isClosed()) { connection.reopen(); } attachToWindow(windowHandle); } private void attachToWindow(String windowHandle) { TargetID targetId = findTarget(windowHandle); attachToTarget(targetId); this.windowHandle = windowHandle; } private void attachToTarget(TargetID targetId) { 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: Input validation: The new public API accepts a nullable window handle without validating its format or existence before attempting to attach, which could lead to unintended target selection. Referred Code public void createSession(@Nullable String windowHandle) { if (connection.isClosed()) { connection.reopen(); } attachToWindow(windowHandle); } private void attachToWindow(String windowHandle) { TargetID targetId = findTarget(windowHandle); attachToTarget(targetId); this.windowHandle = windowHandle; } 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]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consider managing multiple concurrent sessions The current implementation disconnects and reconnects the CDP session on each tab switch, which is inefficient. A more performant design would maintain a map of active sessions for each tab to avoid re-connection latency. Examples: java/src/org/openqa/selenium/devtools/DevTools.java [147-150] } else if (!Objects.equals(this.windowHandle, windowHandle)) { disconnectSession(); attachToWindow(windowHandle); } Solution Walkthrough: Before: class DevTools { private String windowHandle = null; private SessionID cdpSession = null; public void createSessionIfThereIsNotOne(String newWindowHandle) { if (cdpSession == null) { createSession(newWindowHandle); } else if (this.windowHandle has changed) { // Tear down the old session disconnectSession(); // Create a new session for the new window attachToWindow(newWindowHandle); } } // ... } After: class DevTools { private Map<String, SessionID> sessions = new HashMap<>(); private SessionID activeSession = null; public void createSessionIfThereIsNotOne(String windowHandle) { if (sessions.containsKey(windowHandle)) { // Simply switch to the existing session this.activeSession = sessions.get(windowHandle); } else { // Create a new session, add it to the map, and set it as active SessionID newSession = attachToWindow(windowHandle); sessions.put(windowHandle, newSession); this.activeSession = newSession; } } // ... } Suggestion importance[1-10]: 8 __ Why: This is a valid and significant architectural suggestion that addresses a performance limitation in the current design, and the PR author even acknowledged this alternative, making it highly relevant.	Medium
Possible issue	Prevent unintended session switching In createSessionIfThereIsNotOne, add a null check for windowHandle before attempting to switch sessions to prevent unintentionally disconnecting an active session when null is passed. java/src/org/openqa/selenium/devtools/DevTools.java [144-151] public void createSessionIfThereIsNotOne(@Nullable String windowHandle) { if (cdpSession == null) { createSession(windowHandle); - } else if (!Objects.equals(this.windowHandle, windowHandle)) { + } else if (windowHandle != null && !Objects.equals(this.windowHandle, windowHandle)) { disconnectSession(); attachToWindow(windowHandle); } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a logical flaw in the new session switching logic where passing null to createSessionIfThereIsNotOne would unintentionally disconnect an active session and reconnect to a potentially different window.	Low
Learned best practice	Fix session detachment cleanup Do not pass the nulled cdpSession to sendAndWait; use the preserved id and wrap cleanup updates in try-finally to ensure consistent state. java/src/org/openqa/selenium/devtools/DevTools.java [78-90] SessionID id = cdpSession; -cdpSession = null; -windowHandle = null; - try { connection.sendAndWait( - cdpSession, + null, getDomains().target().detachFromTarget(Optional.of(id), Optional.empty()), timeout); } catch (Exception e) { - // Exceptions should not prevent closing the connection and the web driver LOG.log(Level.WARNING, "Exception while detaching from target", e); +} finally { + cdpSession = null; + windowHandle = null; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Ensure resource cleanup and cancellation using try-finally; avoid using mutated state after nulling a handle.	Low
General	✅ Improve window handle matching logic Suggestion Impact: The commit updated the filter to use windowHandle.endsWith(id.toString()) instead of contains, implementing the suggested logic. code diff: - .filter(id -> windowHandle == null || windowHandle.contains(id.toString())) + .filter(id -> windowHandle == null || windowHandle.endsWith(id.toString())) In findTarget, change the filter logic from windowHandle.contains(id.toString()) to windowHandle.endsWith(id.toString()) for a more precise and robust matching of the window handle to the target ID. java/src/org/openqa/selenium/devtools/DevTools.java [214-228] private TargetID findTarget(String windowHandle) { // Figure out the targets. List<TargetInfo> infos = connection.sendAndWait(cdpSession, getDomains().target().getTargets(), timeout); // Grab the first "page" type, and glom on to that. // Find out which one might be the current one // (using given window handle like "CDwindow-24426957AC62D8BC83E58C184C38AF2D") return infos.stream() .filter(info -> "page".equals(info.getType())) .map(TargetInfo::getTargetId) - .filter(id -> windowHandle == null || windowHandle.contains(id.toString())) + .filter(id -> windowHandle == null || windowHandle.endsWith(id.toString())) .findAny() .orElseThrow(() -> new DevToolsException("Unable to find target id of a page")); } [Suggestion processed] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that using endsWith is more precise than contains for matching a windowHandle to a TargetID, making the implementation more robust against potential edge cases.	Low
Update