diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java index f3bcd73d..a0b381e7 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java @@ -404,11 +404,15 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { bindingCall.call(binding); } } else if ("close".equals(event)) { - isClosedOrClosing = true; - if (browser != null) { - browser.contexts.remove(this); - } - listeners.notify(EventType.CLOSE, this); + didClose(); } } + + void didClose() { + isClosedOrClosing = true; + if (browser != null) { + browser.contexts.remove(this); + } + listeners.notify(EventType.CLOSE, this); + } } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java index faac7f48..1644a416 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java @@ -38,7 +38,7 @@ import static com.microsoft.playwright.impl.Utils.convertViaJson; import static com.microsoft.playwright.impl.Utils.isSafeCloseError; class BrowserImpl extends ChannelOwner implements Browser { - final Set contexts = new HashSet<>(); + final Set contexts = new HashSet<>(); private final ListenerCollection listeners = new ListenerCollection<>(); public boolean isRemote; private boolean isConnected = true; @@ -65,7 +65,17 @@ class BrowserImpl extends ChannelOwner implements Browser { public void close() { withLogging("Browser.close", () -> closeImpl()); } + private void closeImpl() { + if (isRemote) { + try { + connection.close(); + } catch (IOException e) { + throw new PlaywrightException("Failed to close browser connection", e); + } + notifyRemoteClosed(); + return; + } try { sendMessage("close"); } catch (PlaywrightException e) { @@ -75,6 +85,17 @@ class BrowserImpl extends ChannelOwner implements Browser { } } + void notifyRemoteClosed() { + // Emulate all pages, contexts and the browser closing upon disconnect. + for (BrowserContextImpl context : new ArrayList<>(contexts)) { + for (PageImpl page : new ArrayList<>(context.pages)) { + page.didClose(); + } + context.didClose(); + } + didClose(); + } + @Override public List contexts() { return new ArrayList<>(contexts); @@ -185,8 +206,12 @@ class BrowserImpl extends ChannelOwner implements Browser { @Override void handleEvent(String event, JsonObject parameters) { if ("close".equals(event)) { - isConnected = false; - listeners.notify(EventType.DISCONNECTED, this); + didClose(); } } + + private void didClose() { + isConnected = false; + listeners.notify(EventType.DISCONNECTED, this); + } } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java index b32528ec..3efc302c 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java @@ -27,6 +27,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Path; import java.time.Duration; +import java.util.function.Consumer; import static com.microsoft.playwright.impl.Serialization.gson; @@ -60,19 +61,28 @@ class BrowserTypeImpl extends ChannelOwner implements BrowserType { if (options != null && options.timeout != null) { timeout = Duration.ofMillis(Math.round(options.timeout)); } - Connection connection = new Connection(new WebSocketTransport(new URI(wsEndpoint), timeout)); + WebSocketTransport transport = new WebSocketTransport(new URI(wsEndpoint), timeout); + Connection connection = new Connection(transport); RemoteBrowser remoteBrowser = (RemoteBrowser) connection.waitForObjectWithKnownName("remoteBrowser"); PlaywrightImpl playwright = this.connection.getExistingObject("Playwright"); SelectorsImpl selectors = remoteBrowser.selectors(); playwright.sharedSelectors.addChannel(selectors); BrowserImpl browser = remoteBrowser.browser(); browser.isRemote = true; + Consumer connectionCloseListener = new Consumer() { + @Override + public void accept(WebSocketTransport t) { + browser.notifyRemoteClosed(); + } + }; + transport.onClose(connectionCloseListener); browser.onDisconnected(b -> { playwright.sharedSelectors.removeChannel(selectors); + transport.offClose(connectionCloseListener); try { connection.close(); } catch (IOException e) { - e.printStackTrace(); + e.printStackTrace(System.err); } }); return browser; diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java index ef64f7b5..055ec0e5 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java @@ -225,12 +225,16 @@ public class PageImpl extends ChannelOwner implements Page { } else if ("crash".equals(event)) { listeners.notify(EventType.CRASH, this); } else if ("close".equals(event)) { - isClosed = true; - browserContext.pages.remove(this); - listeners.notify(EventType.CLOSE, this); + didClose(); } } + void didClose() { + isClosed = true; + browserContext.pages.remove(this); + listeners.notify(EventType.CLOSE, this); + } + private void willAddFileChooserListener() { if (!listeners.hasListeners(EventType.FILECHOOSER)) { updateFileChooserInterception(true); diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketTransport.java b/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketTransport.java index d85aafa3..0c35cf20 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketTransport.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketTransport.java @@ -26,6 +26,7 @@ import java.time.Duration; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; class WebSocketTransport implements Transport { @@ -33,6 +34,9 @@ class WebSocketTransport implements Transport { private final ClientConnection clientConnection; private boolean isClosed; private volatile Exception lastError; + ListenerCollection listeners = new ListenerCollection<>(); + + private enum EventType { CLOSE } private class ClientConnection extends WebSocketClient { ClientConnection(URI serverUri) { @@ -71,17 +75,13 @@ class WebSocketTransport implements Transport { @Override public void send(String message) { - if (clientConnection.isClosed()) { - throw new PlaywrightException("Playwright connection closed"); - } + checkIfClosed(); clientConnection.send(message); } @Override public String poll(Duration timeout) { - if (isClosed || clientConnection.isClosed()) { - throw new PlaywrightException("Playwright connection closed"); - } + checkIfClosed(); try { return incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS); } catch (InterruptedException e) { @@ -97,4 +97,23 @@ class WebSocketTransport implements Transport { isClosed = true; clientConnection.close(); } + + void onClose(Consumer handler) { + listeners.add(EventType.CLOSE, handler); + } + + void offClose(Consumer handler) { + listeners.remove(EventType.CLOSE, handler); + } + + private void checkIfClosed() { + if (isClosed) { + throw new PlaywrightException("Playwright connection closed"); + } + if (clientConnection.isClosed()) { + isClosed = true; + listeners.notify(EventType.CLOSE, this); + throw new PlaywrightException("Playwright connection closed"); + } + } } diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBase.java b/playwright/src/test/java/com/microsoft/playwright/TestBase.java index c505eba5..b4621ba2 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestBase.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestBase.java @@ -60,11 +60,13 @@ public class TestBase { return options; } - static void launchBrowser(BrowserType.LaunchOptions launchOptions) { + static void initBrowserType() { playwright = Playwright.create(); - browserType = Utils.getBrowserTypeFromEnv(playwright); + } + static void launchBrowser(BrowserType.LaunchOptions launchOptions) { + initBrowserType(); browser = browserType.launch(launchOptions); } diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java b/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java new file mode 100644 index 00000000..44c94cc5 --- /dev/null +++ b/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java @@ -0,0 +1,211 @@ +/* + * Copyright (c) Microsoft Corporation. + * + * Licensed 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 com.microsoft.playwright; + +import com.microsoft.playwright.impl.Driver; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.*; + +public class TestBrowserTypeConnect extends TestBase { + private static Process browserServer; + private static String wsEndpoint; + + private static class BrowserServer { + Process process; + String wsEndpoint; + + void kill() throws InterruptedException { + process.destroy(); + int exitCode = process.waitFor(); + assertEquals(0, exitCode); + } + } + + private static BrowserServer launchBrowserServer() { + try { + Path driver = Driver.ensureDriverInstalled(); + Path dir = driver.getParent(); + String node = dir.resolve(isWindows ? "node.exe" : "node").toString(); + String cliJs = dir.resolve("package/lib/cli/cli.js").toString(); + // We launch node process directly instead of using playwright.sh script as killing the script + // process will leave node process running and killing it would be more hassle. + ProcessBuilder pb = new ProcessBuilder(node, cliJs, "launch-server", browserType.name()); + pb.directory(dir.toFile()); + pb.redirectError(ProcessBuilder.Redirect.INHERIT); + BrowserServer result = new BrowserServer(); + result.process = pb.start(); + BufferedReader input = new BufferedReader(new InputStreamReader(result.process.getInputStream())); + result.wsEndpoint = input.readLine(); + if (!result.wsEndpoint.startsWith("ws://")) { + throw new RuntimeException("Invalid web socket address: " + result.wsEndpoint); + } + return result; + } catch (IOException e) { + throw new PlaywrightException("Failed to launch server", e); + } + } + + @BeforeAll + // Hide base class method to launch browser server and connect to it. + static void launchBrowser() { + initBrowserType(); + BrowserServer r = launchBrowserServer(); + wsEndpoint = r.wsEndpoint; + browserServer = r.process; + browser = browserType.connect(wsEndpoint); + // Do not actually connect to browser, the tests will do it manually. + } + + @AfterAll + static void closeBrowser() { + TestBase.closeBrowser(); + if (browserServer != null) { + browserServer.destroyForcibly(); + browserServer = null; + wsEndpoint = null; + } + } + + @Test + void shouldBeAbleToReconnectToABrowser() { + { + Browser browser = browserType.connect(wsEndpoint); + BrowserContext browserContext = browser.newContext(); + assertEquals(0, browserContext.pages().size()); + Page page = browserContext.newPage(); + assertEquals(121, page.evaluate("11 * 11")); + page.navigate(server.EMPTY_PAGE); + browser.close(); + } + { + Browser browser = browserType.connect(wsEndpoint); + BrowserContext browserContext = browser.newContext(); + Page page = browserContext.newPage(); + page.navigate(server.EMPTY_PAGE); + browser.close(); + } + } + + @Test + void shouldBeAbleToConnectTwoBrowsersAtTheSameTime() { + Browser browser1 = browserType.connect(wsEndpoint); + assertEquals(0, browser1.contexts().size()); + browser1.newContext(); + assertEquals(1, browser1.contexts().size()); + + Browser browser2 = browserType.connect(wsEndpoint); + assertEquals(0, browser2.contexts().size()); + browser2.newContext(); + assertEquals(1, browser2.contexts().size()); + assertEquals(1, browser1.contexts().size()); + + browser1.close(); + Page page2 = browser2.newPage(); + assertEquals(42, page2.evaluate("7 * 6")); // original browser should still work + + browser2.close(); + } + + @Test + void disconnectedEventShouldBeEmittedWhenBrowserIsClosedOrServerIsClosed() throws InterruptedException { + // Launch another server to not affect other tests. + BrowserServer remote = launchBrowserServer(); + + Browser browser1 = browserType.connect(remote.wsEndpoint); + Browser browser2 = browserType.connect(remote.wsEndpoint); + + int[] disconnected1 = {0}; + int[] disconnected2 = {0}; + browser1.onDisconnected(b -> ++disconnected1[0]); + browser2.onDisconnected(b -> ++disconnected2[0]); + + Page page2 = browser2.newPage(); + + browser1.close(); + assertEquals(1, disconnected1[0]); + assertEquals(0, disconnected2[0]); + + remote.kill(); + assertEquals(1, disconnected1[0]); + try { + // Tickle connection so that it gets a chance to dispatch disconnect event. + page2.title(); + fail("did not throw"); + } catch (PlaywrightException e) { + } + assertEquals(1, disconnected2[0]); + } + + @Test + void disconnectedEventShouldHaveBrowserAsArgument() { + Browser browser = browserType.connect(wsEndpoint); + Browser[] disconnected = {null}; + browser.onDisconnected(b -> disconnected[0] = b); + browser.close(); + assertEquals(browser, disconnected[0]); + } + + void shouldHandleExceptionsDuringConnect() { + // This is an implementation detail test + } + + @Test + void shouldSetTheBrowserConnectedState() { + Browser remote = browserType.connect(wsEndpoint); + assertTrue(remote.isConnected()); + remote.close(); + assertFalse(remote.isConnected()); + } + + @Test + void shouldThrowWhenUsedAfterIsConnectedReturnsFalse() throws InterruptedException { + // Launch another server to not affect other tests. + BrowserServer server = launchBrowserServer(); + Browser remote = browserType.connect(server.wsEndpoint); + Page page = remote.newPage(); + server.kill(); + try { + page.evaluate("1 + 1"); + } catch (PlaywrightException e) { + assertTrue(e.getMessage().contains("Playwright connection closed")); + } + assertFalse(remote.isConnected()); + } + + @Test + void shouldRejectNavigationWhenBrowserCloses() { + Browser remote = browserType.connect(wsEndpoint); + Page page = remote.newPage(); + + server.setRoute("/one-style.css", r -> {}); + page.onRequest(r -> remote.close()); + try { + page.navigate(server.PREFIX + "/one-style.html", new Page.NavigateOptions().withTimeout(60000)); + fail("did not throw"); + } catch (PlaywrightException e) { + assertTrue(e.getMessage().contains("Playwright connection closed")); + } + } + } diff --git a/scripts/CLI_VERSION b/scripts/CLI_VERSION index 373d6574..87e78a1b 100644 --- a/scripts/CLI_VERSION +++ b/scripts/CLI_VERSION @@ -1 +1 @@ -1.9.1-1614817648000 +1.9.1-1614885198000