From 3c338021719b2d638ebe24a4796f28af04e0ac2a Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Fri, 24 Feb 2023 11:46:40 +0530 Subject: [PATCH 1/2] [java][cdp] Allow reusing DevTools instance upon calling close() --- .../openqa/selenium/devtools/Connection.java | 18 ++++++- .../openqa/selenium/devtools/DevTools.java | 4 ++ .../selenium/devtools/DevToolsReuseTest.java | 49 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 java/test/org/openqa/selenium/devtools/DevToolsReuseTest.java diff --git a/java/src/org/openqa/selenium/devtools/Connection.java b/java/src/org/openqa/selenium/devtools/Connection.java index bd67046897803..5f4a1e1156829 100644 --- a/java/src/org/openqa/selenium/devtools/Connection.java +++ b/java/src/org/openqa/selenium/devtools/Connection.java @@ -42,6 +42,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; @@ -65,17 +66,29 @@ public class Connection implements Closeable { return thread; }); private static final AtomicLong NEXT_ID = new AtomicLong(1L); - private final WebSocket socket; + private WebSocket socket; private final Map>> methodCallbacks = new ConcurrentHashMap<>(); private final ReadWriteLock callbacksLock = new ReentrantReadWriteLock(true); private final Multimap, Consumer> eventCallbacks = HashMultimap.create(); private final HttpClient client; + private final String url; + private final AtomicBoolean isClosed; public Connection(HttpClient client, String url) { Require.nonNull("HTTP client", client); Require.nonNull("URL to connect to", url); + this.url = url; this.client = client; - socket = this.client.openSocket(new HttpRequest(GET, url), new Listener()); + this.socket = this.client.openSocket(new HttpRequest(GET, url), new Listener()); + this.isClosed = new AtomicBoolean(); + } + + boolean isClosed() { + return isClosed.get(); + } + + void reopen() { + this.socket = this.client.openSocket(new HttpRequest(GET, url), new Listener()); } private static class NamedConsumer implements Consumer { @@ -190,6 +203,7 @@ public void clearListeners() { public void close() { socket.close(); client.close(); + this.isClosed.set(true); } private class Listener implements WebSocket.Listener { diff --git a/java/src/org/openqa/selenium/devtools/DevTools.java b/java/src/org/openqa/selenium/devtools/DevTools.java index aee43a871c2fd..8284c576aa9bc 100644 --- a/java/src/org/openqa/selenium/devtools/DevTools.java +++ b/java/src/org/openqa/selenium/devtools/DevTools.java @@ -73,6 +73,7 @@ cdpSession, getDomains().target().detachFromTarget(Optional.of(id), Optional.emp // Exceptions should not prevent closing the connection and the web driver log.warning("Exception while detaching from target: " + e.getMessage()); } + cdpSession = null; } } @@ -118,6 +119,9 @@ public void createSession() { * @param windowHandle result of {@link WebDriver#getWindowHandle()}, optional. */ public void createSession(String windowHandle) { + if (connection.isClosed()) { + connection.reopen(); + } TargetID targetId = findTarget(windowHandle); // Starts the session diff --git a/java/test/org/openqa/selenium/devtools/DevToolsReuseTest.java b/java/test/org/openqa/selenium/devtools/DevToolsReuseTest.java new file mode 100644 index 0000000000000..64d0ae341dbc5 --- /dev/null +++ b/java/test/org/openqa/selenium/devtools/DevToolsReuseTest.java @@ -0,0 +1,49 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.openqa.selenium.devtools; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.remote.Augmenter; + +class DevToolsReuseTest extends DevToolsTestBase { + + @Test + public void shouldBeAbleToCloseDevToolsAndCreateNewInstance() { + WebDriver driver = new Augmenter().augment(this.driver); + + DevTools devTools = ((HasDevTools) driver).getDevTools(); + devTools.createSession(); + addConsoleLogListener(devTools); + + devTools.close(); + + devTools = ((HasDevTools) driver).getDevTools(); + devTools.createSession(); + addConsoleLogListener(devTools); + + devTools.close(); + } + + private static void addConsoleLogListener(DevTools devTools) { + devTools.getDomains().events().addConsoleListener( + consoleEvent -> assertThat(consoleEvent.getMessages()).contains("Hello, world!")); + } +} From d3b50457452c8f6080f90b6977ddab55a5489116 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Mon, 27 Feb 2023 15:18:06 +0530 Subject: [PATCH 2/2] [java][cdp] Avoid unnecessary null assignment --- java/src/org/openqa/selenium/devtools/DevTools.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java/src/org/openqa/selenium/devtools/DevTools.java b/java/src/org/openqa/selenium/devtools/DevTools.java index 8284c576aa9bc..a25dbe04bde7d 100644 --- a/java/src/org/openqa/selenium/devtools/DevTools.java +++ b/java/src/org/openqa/selenium/devtools/DevTools.java @@ -73,7 +73,6 @@ cdpSession, getDomains().target().detachFromTarget(Optional.of(id), Optional.emp // Exceptions should not prevent closing the connection and the web driver log.warning("Exception while detaching from target: " + e.getMessage()); } - cdpSession = null; } }