When calling webdriver.close() for the last tab, quit() the webdriver

[asolntsev]

User description

Before:

RemoteWebDriver driver = new RemoteWebDriver();
driver.close(); // result: driver still has SessionID
driver.close(); // result: NoSuchSessionException: invalid session id

Now:

RemoteWebDriver driver = new RemoteWebDriver();
driver.close(); // result: driver.getSessionId() == null
driver.close(); // result: no-op (same behaviour as in `quit()` method)

🔧 Thoughts

I am not yet sure this is the right change. :)
Let it be in a "Draft" status by now.

🔄 Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Bug fix, Enhancement

---

Description

• Auto-quit webdriver when closing last tab instead of leaving session open

• Replace hardcoded sleep with dynamic wait for session closure

• Reduce SessionHandlingTest execution time from 28 to 10 seconds

• Handle null window handles and improve session cleanup logic

---

Diagram Walkthrough

flowchart LR
  A["close() called"] --> B{"Last window?"}
  B -->|Yes| C["quit() invoked"]
  B -->|No| D["Return normally"]
  C --> E["Session ID nullified"]
  D --> E
  F["Test: sleepTight"] --> G["waitUntilBrowserFullyClosed"]
  G --> H["Poll until sessionId == null"]

Loading

File Walkthrough

	Relevant files
Bug fix	RemoteWebDriver.java Auto-quit on last window close and improve session handling java/src/org/openqa/selenium/remote/RemoteWebDriver.java Added null check at start of close() method to return early if session already closed Changed logic to call quit() when closing last window instead of just closing BiDi session Replaced ArrayList cast with generic List type for window handles Improved null safety by checking for null or empty window handles before quit decision +9/-7
Tests	SessionHandlingTest.java Replace hardcoded sleeps with dynamic session closure waits java/test/org/openqa/selenium/SessionHandlingTest.java Replaced all sleepTight() calls with waitUntilBrowserFullyClosed() dynamic wait method Added setUp() method to verify session ID is not null before each test Implemented waitUntilBrowserFullyClosed() helper that polls until session ID becomes null Simplified shouldContinueAfterSleep() test to use assertion helpers instead of long sleep Removed hardcoded 3-second and 10-second sleep calls throughout test class +23/-14

---

[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: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities 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: No action logs: The new logic that auto-quits the browser when closing the last window performs a critical action without adding any audit logging in the changed code. Referred Code public void close() { // no-op if session id is null. We're only going to make ourselves unhappy if (sessionId == null) { return; } if (this instanceof HasDevTools) { // This is a brute force approach to "solving" the problem of a hanging // CDP connection. Take a look at // https://github.com/aslushnikov/getting-started-with-cdp#session-hierarchy // to get up to speed, but essentially if the page session of the // first browser window is closed, the next CDP command will hang // indefinitely. To prevent that from happening, we close the current // connection. The next CDP command _should_ make us reconnect try { ((HasDevTools) this).maybeGetDevTools().ifPresent(DevTools::disconnectSession); } catch (ConnectionFailedException unableToEstablishWebsocketConnection) { LOG.log( SEVERE, "Failed to disconnect DevTools session", unableToEstablishWebsocketConnection); } ... (clipped 11 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: Edge cases unhandled: The cast of response value to List<String> lacks defensive handling for unexpected types or malformed responses and silently proceeds to quit without logging when windowHandles is null or empty. Referred Code Response response = execute(DriverCommand.CLOSE); Object value = response.getValue(); List<String> windowHandles = (List<String>) value; if (windowHandles == null || windowHandles.isEmpty()) { // Time to quit the browser if it was the last opened window. quit(); } } Learn more about managing compliance generic rules or creating your own custom rules

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	This breaking change alters core API This PR introduces a breaking change by making WebDriver.close() on the last window behave like quit(), which terminates the session. This alters the established API contract where close() only affects the window, potentially breaking existing user code. Examples: java/src/org/openqa/selenium/remote/RemoteWebDriver.java [442-445] if (windowHandles == null || windowHandles.isEmpty()) { // Time to quit the browser if it was the last opened window. quit(); } Solution Walkthrough: Before: // In RemoteWebDriver.java public void close() { // ... Response response = execute(DriverCommand.CLOSE); List<String> windowHandles = (ArrayList<String>) response.getValue(); if (windowHandles.isEmpty() && this instanceof HasBiDi) { // Close BiDi session, but WebDriver session remains. ((HasBiDi) this).maybeGetBiDi().ifPresent(BiDi::close); } // The WebDriver session is not terminated. } After: // In RemoteWebDriver.java public void close() { if (sessionId == null) { return; } // ... Response response = execute(DriverCommand.CLOSE); List<String> windowHandles = (List<String>) response.getValue(); if (windowHandles == null || windowHandles.isEmpty()) { // Quit the browser if it was the last opened window. quit(); // This terminates the session. } } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant, intentional breaking change in the core WebDriver.close() API, which now calls quit() when the last window is closed, altering its fundamental contract.	High
Possible issue	Ensure driver quits on close exception Wrap the close() command execution in a try-catch block and call quit() in the catch block to ensure the driver session is properly terminated even if closing the window fails. java/src/org/openqa/selenium/remote/RemoteWebDriver.java [438-445] -Response response = execute(DriverCommand.CLOSE); -Object value = response.getValue(); -List<String> windowHandles = (List<String>) value; +try { + Response response = execute(DriverCommand.CLOSE); + Object value = response.getValue(); + List<String> windowHandles = (List<String>) value; -if (windowHandles == null || windowHandles.isEmpty()) { - // Time to quit the browser if it was the last opened window. + if (windowHandles == null || windowHandles.isEmpty()) { + // Time to quit the browser if it was the last opened window. + quit(); + } +} catch (WebDriverException e) { + // This can happen if the window is unexpectedly closed. In this case, we should just quit. quit(); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that an exception during execute(DriverCommand.CLOSE) would bypass the new quit() logic, and proposes a robust try-catch block to ensure the session is terminated, improving error handling.	Medium
Learned best practice	Add timeout to wait Add a bounded timeout to the wait to prevent tests from hanging indefinitely if the session is never nulled. java/test/org/openqa/selenium/SessionHandlingTest.java [80-82] private void waitUntilBrowserFullyClosed() { - wait.until($ -> getSessionId() == null); + wait.withTimeout(Duration.ofSeconds(10)).until($ -> getSessionId() == null); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Always guard sleeps/waits and polling with sensible timeouts to avoid indefinite hangs; compute timeouts clearly once (Pattern 5).	Low
More