Skip to content

Commit 425fab0

Browse files
committed
avoid establishing CDP connection in maybeGet*() method
1. method `getBiDi`/`getDevTools` should establish a connection, BUT 1. method `maybeGet*()` returns connection only if the connection is already established, but should NOT establish a new connection. It's because method `maybeGet*()` is used from `WebDriver.close()` and `WebDriver.quite()`. At this moment, we don't want a new connection, we only want to close the existing connection.
1 parent f36cdfa commit 425fab0

File tree

2 files changed

+24
-9
lines changed

2 files changed

+24
-9
lines changed

java/src/org/openqa/selenium/bidi/BiDiProvider.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.net.URISyntaxException;
2323
import java.util.Optional;
2424
import java.util.function.Predicate;
25+
2526
import org.openqa.selenium.Capabilities;
2627
import org.openqa.selenium.remote.AugmenterProvider;
2728
import org.openqa.selenium.remote.ExecuteMethod;
@@ -45,22 +46,28 @@ public Class<HasBiDi> getDescribedInterface() {
4546
@Override
4647
public HasBiDi getImplementation(Capabilities caps, ExecuteMethod executeMethod) {
4748
return new HasBiDi() {
48-
private volatile Optional<BiDi> biDi;
49+
private volatile BiDi biDi;
50+
private final Object lock = new Object();
4951

5052
@Override
5153
public Optional<BiDi> maybeGetBiDi() {
54+
return Optional.ofNullable(biDi);
55+
}
56+
57+
@Override
58+
public BiDi getBiDi() {
5259
if (biDi == null) {
53-
synchronized (this) {
60+
synchronized (lock) {
5461
if (biDi == null) {
5562
URI wsUri =
56-
getBiDiUrl(caps).orElseThrow(() -> new BiDiException("BiDi not supported"));
63+
getBiDiUrl(caps).orElseThrow(() -> new BiDiException("BiDi not supported"));
5764

5865
HttpClient.Factory clientFactory = HttpClient.Factory.createDefault();
5966
ClientConfig wsConfig = ClientConfig.defaultConfig().baseUri(wsUri);
6067
HttpClient wsClient = clientFactory.createClient(wsConfig);
6168
Connection connection = new Connection(wsClient, wsUri.toString());
6269

63-
biDi = Optional.of(new BiDi(connection));
70+
biDi = new BiDi(connection);
6471
}
6572
}
6673
}

java/src/org/openqa/selenium/devtools/DevToolsProvider.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.net.URI;
2222
import java.util.Optional;
2323
import java.util.function.Predicate;
24+
2425
import org.openqa.selenium.Capabilities;
2526
import org.openqa.selenium.devtools.noop.NoOpCdpInfo;
2627
import org.openqa.selenium.remote.AugmenterProvider;
@@ -43,21 +44,28 @@ public Class<HasDevTools> getDescribedInterface() {
4344
@Override
4445
public HasDevTools getImplementation(Capabilities caps, ExecuteMethod executeMethod) {
4546
return new HasDevTools() {
46-
private volatile Optional<DevTools> devTools;
47+
private volatile DevTools devTools;
48+
private final Object lock = new Object();
4749

4850
@Override
4951
public Optional<DevTools> maybeGetDevTools() {
52+
return Optional.ofNullable(devTools);
53+
}
54+
55+
@Override
56+
public DevTools getDevTools() {
5057
if (devTools == null) {
51-
synchronized (this) {
58+
synchronized (lock) {
5259
if (devTools == null) {
5360
Object cdpVersion = caps.getCapability("se:cdpVersion");
5461
String version =
55-
cdpVersion instanceof String ? (String) cdpVersion : caps.getBrowserVersion();
62+
cdpVersion instanceof String ? (String) cdpVersion : caps.getBrowserVersion();
5663

5764
CdpInfo info = new CdpVersionFinder().match(version).orElseGet(NoOpCdpInfo::new);
5865
this.devTools =
59-
SeleniumCdpConnection.create(caps)
60-
.map(conn -> new DevTools(info::getDomains, conn));
66+
SeleniumCdpConnection.create(caps)
67+
.map(conn -> new DevTools(info::getDomains, conn))
68+
.orElseThrow(() -> new DevToolsException("Unable to create DevTools connection"));;
6169
}
6270
}
6371
}

0 commit comments

Comments
 (0)