Skip to content

Commit d795820

Browse files
joerg1985diemol
joerg1985
and
diemol
authored
[java] Create less HttpClient instances while creating a CDP connection (#12216)
Create less HttpClient instances while creating a CDP connection Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
1 parent c49a24d commit d795820

5 files changed

Lines changed: 110 additions & 37 deletions

File tree

‎java/src/org/openqa/selenium/chromium/ChromiumDriver.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Optional;
2929
import java.util.function.Predicate;
3030
import java.util.function.Supplier;
31+
import java.util.logging.Level;
3132
import java.util.logging.Logger;
3233
import org.openqa.selenium.BuildInfo;
3334
import org.openqa.selenium.Capabilities;
@@ -132,19 +133,25 @@ protected ChromiumDriver(
132133

133134
this.biDi = createBiDi(biDiUri);
134135

135-
Optional<URI> cdpUri =
136-
CdpEndpointFinder.getReportedUri(capabilityKey, originalCapabilities)
137-
.flatMap(uri -> CdpEndpointFinder.getCdpEndPoint(factory, uri));
136+
Optional<URI> reportedUri = CdpEndpointFinder.getReportedUri(capabilityKey, originalCapabilities);
137+
Optional<HttpClient> client = reportedUri.map(uri -> CdpEndpointFinder.getHttpClient(factory, uri));
138+
Optional<URI> cdpUri;
138139

139140
try {
140-
connection =
141-
cdpUri.map(
142-
uri ->
143-
new Connection(
144-
factory.createClient(ClientConfig.defaultConfig().baseUri(uri)),
145-
uri.toString()));
141+
try {
142+
cdpUri = client.flatMap(httpClient -> CdpEndpointFinder.getCdpEndPoint(httpClient));
143+
} catch (Exception e) {
144+
try {
145+
client.ifPresent(HttpClient::close);
146+
} catch (Exception ex) {
147+
e.addSuppressed(ex);
148+
}
149+
throw e;
150+
}
151+
connection = cdpUri.map(uri -> new Connection(client.get(), uri.toString()));
146152
} catch (ConnectionFailedException e) {
147-
LOG.warning("Unable to establish websocket connection to " + cdpUri.get());
153+
cdpUri = Optional.empty();
154+
LOG.log(Level.WARNING, "Unable to establish websocket connection to " + reportedUri.get(), e);
148155
connection = Optional.empty();
149156
}
150157

‎java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,20 @@ public class CdpEndpointFinder {
4141
private static final Logger LOG = Logger.getLogger(CdpEndpointFinder.class.getName());
4242
private static final Json JSON = new Json();
4343

44-
public static Optional<URI> getCdpEndPoint(HttpClient.Factory clientFactory, URI reportedUri) {
44+
public static HttpClient getHttpClient(HttpClient.Factory clientFactory, URI reportedUri) {
4545
Require.nonNull("HTTP client factory", clientFactory);
4646
Require.nonNull("DevTools URI", reportedUri);
4747

4848
ClientConfig config = ClientConfig.defaultConfig().baseUri(reportedUri);
4949

50+
return clientFactory.createClient(config);
51+
}
52+
53+
public static Optional<URI> getCdpEndPoint(HttpClient client) {
54+
Require.nonNull("HTTP client", client);
55+
5056
HttpResponse res;
51-
try (HttpClient client = clientFactory.createClient(config)) {
57+
try {
5258
res = client.execute(new HttpRequest(GET, "/json/version"));
5359
} catch (UncheckedIOException e) {
5460
LOG.warning("Unable to connect to determine websocket url: " + e.getMessage());

‎java/src/org/openqa/selenium/devtools/SeleniumCdpConnection.java‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,18 @@
2020
import java.net.URI;
2121
import java.net.URISyntaxException;
2222
import java.util.Optional;
23+
import java.util.logging.Level;
24+
import java.util.logging.Logger;
2325
import org.openqa.selenium.Capabilities;
2426
import org.openqa.selenium.HasCapabilities;
2527
import org.openqa.selenium.WebDriver;
2628
import org.openqa.selenium.internal.Require;
27-
import org.openqa.selenium.remote.http.ClientConfig;
2829
import org.openqa.selenium.remote.http.HttpClient;
2930

3031
public class SeleniumCdpConnection extends Connection {
3132

33+
private static final Logger LOG = Logger.getLogger(SeleniumCdpConnection.class.getName());
34+
3235
private SeleniumCdpConnection(HttpClient client, String url) {
3336
super(client, url);
3437
}
@@ -51,28 +54,48 @@ public static Optional<Connection> create(
5154
Require.nonNull("HTTP client factory", clientFactory);
5255
Require.nonNull("Capabilities", capabilities);
5356

54-
return getCdpUri(clientFactory, capabilities)
55-
.map(
56-
uri ->
57-
new SeleniumCdpConnection(
58-
clientFactory.createClient(ClientConfig.defaultConfig().baseUri(uri)),
59-
uri.toString()));
60-
}
6157

62-
public static Optional<URI> getCdpUri(
63-
HttpClient.Factory clientFactory, Capabilities capabilities) {
64-
Object cdp = capabilities.getCapability("se:cdp");
58+
Optional<URI> cdpUri = Optional
59+
.ofNullable(capabilities.getCapability("se:cdp"))
60+
.flatMap((uri) -> {
61+
if (uri instanceof String) {
62+
try {
63+
return Optional.of(new URI((String) uri));
64+
} catch (URISyntaxException e) {
65+
return Optional.empty();
66+
}
67+
}
68+
return Optional.empty();
69+
});
70+
71+
Optional<HttpClient> client;
72+
73+
if (cdpUri.isPresent()) {
74+
client = Optional.of(CdpEndpointFinder.getHttpClient(clientFactory, cdpUri.get()));
75+
} else {
76+
Optional<URI> reportedUri = CdpEndpointFinder.getReportedUri(capabilities);
77+
client = reportedUri.map(uri -> CdpEndpointFinder.getHttpClient(clientFactory, uri));
6578

66-
if (cdp instanceof String) {
6779
try {
68-
return Optional.of(new URI((String) cdp));
69-
} catch (URISyntaxException e) {
70-
return Optional.empty();
80+
cdpUri = client.flatMap(httpClient -> CdpEndpointFinder.getCdpEndPoint(httpClient));
81+
} catch (Exception e) {
82+
try {
83+
client.ifPresent(HttpClient::close);
84+
} catch (Exception ex) {
85+
e.addSuppressed(ex);
86+
}
87+
throw e;
7188
}
72-
}
7389

74-
Optional<URI> reportedUri = CdpEndpointFinder.getReportedUri(capabilities);
90+
if (!cdpUri.isPresent()) {
91+
try {
92+
client.ifPresent(HttpClient::close);
93+
} catch (Exception e) {
94+
LOG.log(Level.FINE, "failed to close the http client used to check the reported CDP endpoint: " + reportedUri.get(), e);
95+
}
96+
}
97+
}
7598

76-
return reportedUri.flatMap(uri -> CdpEndpointFinder.getCdpEndPoint(clientFactory, uri));
99+
return cdpUri.map(uri -> new SeleniumCdpConnection(client.get(), uri.toString()));
77100
}
78101
}

‎java/src/org/openqa/selenium/firefox/FirefoxDriver.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.nio.file.Path;
2626
import java.util.Map;
2727
import java.util.Optional;
28+
import java.util.logging.Level;
2829
import java.util.logging.Logger;
2930
import org.openqa.selenium.Beta;
3031
import org.openqa.selenium.Capabilities;
@@ -156,10 +157,28 @@ private FirefoxDriver(
156157
context = new AddHasContext().getImplementation(getCapabilities(), getExecuteMethod());
157158

158159
Capabilities capabilities = super.getCapabilities();
159-
HttpClient.Factory clientFactory = HttpClient.Factory.createDefault();
160-
Optional<URI> cdpUri =
161-
CdpEndpointFinder.getReportedUri("moz:debuggerAddress", capabilities)
162-
.flatMap(reported -> CdpEndpointFinder.getCdpEndPoint(clientFactory, reported));
160+
HttpClient.Factory factory = HttpClient.Factory.createDefault();
161+
162+
Optional<URI> reportedUri = CdpEndpointFinder.getReportedUri("moz:debuggerAddress", capabilities);
163+
Optional<HttpClient> client = reportedUri.map(uri -> CdpEndpointFinder.getHttpClient(factory, uri));
164+
Optional<URI> cdpUri;
165+
166+
try {
167+
cdpUri = client.flatMap(httpClient -> CdpEndpointFinder.getCdpEndPoint(httpClient));
168+
} catch (Exception e) {
169+
try {
170+
client.ifPresent(HttpClient::close);
171+
} catch (Exception ex) {
172+
e.addSuppressed(ex);
173+
}
174+
throw e;
175+
}
176+
177+
try {
178+
client.ifPresent(HttpClient::close);
179+
} catch (Exception e) {
180+
LOG.log(Level.FINE, "failed to close the http client used to check the reported CDP endpoint: " + reportedUri.get(), e);
181+
}
163182

164183
Optional<String> webSocketUrl =
165184
Optional.ofNullable((String) capabilities.getCapability("webSocketUrl"));

‎java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,30 @@ private Optional<Consumer<Message>> findCdpEndpoint(
117117
Consumer<Message> downstream, Capabilities caps) {
118118
// Using strings here to avoid Node depending upon specific drivers.
119119
for (String cdpEndpointCap : CDP_ENDPOINT_CAPS) {
120-
Optional<URI> cdpUri =
121-
CdpEndpointFinder.getReportedUri(cdpEndpointCap, caps)
122-
.flatMap(reported -> CdpEndpointFinder.getCdpEndPoint(clientFactory, reported));
120+
Optional<URI> reportedUri = CdpEndpointFinder.getReportedUri(cdpEndpointCap, caps);
121+
Optional<HttpClient> client = reportedUri.map(uri -> CdpEndpointFinder.getHttpClient(clientFactory, uri));
122+
Optional<URI> cdpUri;
123+
124+
try {
125+
cdpUri = client.flatMap(httpClient -> CdpEndpointFinder.getCdpEndPoint(httpClient));
126+
} catch (Exception e) {
127+
try {
128+
client.ifPresent(HttpClient::close);
129+
} catch (Exception ex) {
130+
e.addSuppressed(ex);
131+
}
132+
throw e;
133+
}
134+
123135
if (cdpUri.isPresent()) {
124136
LOG.log(getDebugLogLevel(), String.format("Endpoint found in %s", cdpEndpointCap));
125137
return cdpUri.map(cdp -> createWsEndPoint(cdp, downstream));
138+
} else {
139+
try {
140+
client.ifPresent(HttpClient::close);
141+
} catch (Exception e) {
142+
LOG.log(Level.FINE, "failed to close the http client used to check the reported CDP endpoint: " + reportedUri.get(), e);
143+
}
126144
}
127145
}
128146
return Optional.empty();

0 commit comments

Comments
 (0)