diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index d943db7a6624d..fa6349631e5e1 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -31,7 +31,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; -import java.util.ArrayList; import java.util.Base64; import java.util.Collection; import java.util.Collections; @@ -414,6 +413,11 @@ public String getPageSource() { @Override 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 @@ -433,13 +437,11 @@ public void close() { Response response = execute(DriverCommand.CLOSE); Object value = response.getValue(); - List windowHandles = (ArrayList) value; + List windowHandles = (List) value; - if (windowHandles.isEmpty() && this instanceof HasBiDi) { - // If no top-level browsing contexts are open after calling close, it indicates that the - // WebDriver session is closed. - // If the WebDriver session is closed, the BiDi session also needs to be closed. - ((HasBiDi) this).maybeGetBiDi().ifPresent(BiDi::close); + if (windowHandles == null || windowHandles.isEmpty()) { + // Time to quit the browser if it was the last opened window. + quit(); } } diff --git a/java/test/org/openqa/selenium/SessionHandlingTest.java b/java/test/org/openqa/selenium/SessionHandlingTest.java index ba261c061418f..36344921b2876 100644 --- a/java/test/org/openqa/selenium/SessionHandlingTest.java +++ b/java/test/org/openqa/selenium/SessionHandlingTest.java @@ -17,22 +17,30 @@ package org.openqa.selenium; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.*; import static org.openqa.selenium.testing.drivers.Browser.FIREFOX; import static org.openqa.selenium.testing.drivers.Browser.SAFARI; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.openqa.selenium.remote.RemoteWebDriver; +import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.testing.JupiterTestBase; import org.openqa.selenium.testing.NoDriverAfterTest; import org.openqa.selenium.testing.NotYetImplemented; class SessionHandlingTest extends JupiterTestBase { + @BeforeEach + void setUp() { + assertThat(getSessionId()).isNotNull(); + } + @NoDriverAfterTest @Test void callingQuitMoreThanOnceOnASessionIsANoOp() { driver.quit(); - sleepTight(3000); + waitUntilBrowserFullyClosed(); driver.quit(); } @@ -42,7 +50,7 @@ void callingQuitMoreThanOnceOnASessionIsANoOp() { @NotYetImplemented(SAFARI) public void callingQuitAfterClosingTheLastWindowIsANoOp() { driver.close(); - sleepTight(3000); + waitUntilBrowserFullyClosed(); driver.quit(); } @@ -50,7 +58,7 @@ public void callingQuitAfterClosingTheLastWindowIsANoOp() { @Test void callingAnyOperationAfterClosingTheLastWindowShouldThrowAnException() { driver.close(); - sleepTight(3000); + waitUntilBrowserFullyClosed(); assertThatExceptionOfType(NoSuchSessionException.class).isThrownBy(driver::getCurrentUrl); } @@ -58,21 +66,22 @@ void callingAnyOperationAfterClosingTheLastWindowShouldThrowAnException() { @Test void callingAnyOperationAfterQuitShouldThrowAnException() { driver.quit(); - sleepTight(3000); + waitUntilBrowserFullyClosed(); assertThatExceptionOfType(NoSuchSessionException.class).isThrownBy(driver::getCurrentUrl); } @Test - void shouldContinueAfterSleep() { - sleepTight(10000); - driver.getWindowHandle(); // should not throw + void shouldContinueAfterSleep() throws InterruptedException { + assertThatCode(() -> driver.getWindowHandle()).doesNotThrowAnyException(); + Thread.sleep(100); + assertThatCode(() -> driver.getWindowHandle()).doesNotThrowAnyException(); + } + + private void waitUntilBrowserFullyClosed() { + wait.until($ -> getSessionId() == null); } - private void sleepTight(long duration) { - try { - Thread.sleep(duration); - } catch (InterruptedException e) { - e.printStackTrace(); - } + private SessionId getSessionId() { + return ((RemoteWebDriver) driver).getSessionId(); } }