chore: switch to connect implementation in driver (#615)

playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java

@@ -71,7 +71,6 @@ class BrowserImpl extends ChannelOwner implements Browser {
       } catch (IOException e) {
         throw new PlaywrightException("Failed to close browser connection", e);
       }
-      notifyRemoteClosed();
       return;
     }
     try {

playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java

@@ -16,6 +16,7 @@
 
 package com.microsoft.playwright.impl;
 
+import com.google.gson.Gson;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.microsoft.playwright.Browser;
@@ -23,12 +24,7 @@ import com.microsoft.playwright.BrowserType;
 import com.microsoft.playwright.PlaywrightException;
 
 import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.file.Path;
-import java.time.Duration;
-import java.util.Collections;
-import java.util.Map;
 import java.util.function.Consumer;
 
 import static com.microsoft.playwright.impl.Serialization.gson;
@@ -58,51 +54,40 @@ class BrowserTypeImpl extends ChannelOwner implements BrowserType {
   }
 
   private Browser connectImpl(String wsEndpoint, ConnectOptions options) {
-    try {
-      Duration timeout = Duration.ofDays(1);
-      Map<String, String> headers = Collections.emptyMap();
-      Duration slowMo = null;
-      if (options != null) {
-        if (options.timeout != null) {
-          timeout = Duration.ofMillis(Math.round(options.timeout));
-        }
-        if (options.headers != null) {
-          headers = options.headers;
-        }
-        if (options.slowMo != null) {
-          slowMo = Duration.ofMillis(options.slowMo.intValue());
-        }
-      }
-      WebSocketTransport transport = new WebSocketTransport(new URI(wsEndpoint), headers, timeout, slowMo);
-      Connection connection = new Connection(transport);
-      PlaywrightImpl playwright = connection.initializePlaywright();
-      if (!playwright.initializer.has("preLaunchedBrowser")) {
-        try {
-          connection.close();
-        } catch (IOException e) {
-          e.printStackTrace(System.err);
-        }
-        throw new PlaywrightException("Malformed endpoint. Did you use launchServer method?");
-      }
-      playwright.initSharedSelectors(this.connection.getExistingObject("Playwright"));
-      BrowserImpl browser = connection.getExistingObject(playwright.initializer.getAsJsonObject("preLaunchedBrowser").get("guid").getAsString());
-      browser.isRemote = true;
-      browser.isConnectedOverWebSocket = true;
-      Consumer<WebSocketTransport> connectionCloseListener = t -> browser.notifyRemoteClosed();
-      transport.onClose(connectionCloseListener);
-      browser.onDisconnected(b -> {
-        playwright.unregisterSelectors();
-        transport.offClose(connectionCloseListener);
-        try {
-          connection.close();
-        } catch (IOException e) {
-          e.printStackTrace(System.err);
-        }
-      });
-      return browser;
-    } catch (URISyntaxException e) {
-      throw new PlaywrightException("Failed to connect", e);
+    if (options == null) {
+      options = new ConnectOptions();
     }
+    // We don't use gson() here as the headers map should be serialized to a json object.
+    JsonObject params = new Gson().toJsonTree(options).getAsJsonObject();
+    params.addProperty("wsEndpoint", wsEndpoint);
+    JsonObject json = sendMessage("connect", params).getAsJsonObject();
+    JsonPipe pipe = connection.getExistingObject(json.getAsJsonObject("pipe").get("guid").getAsString());
+    Connection connection = new Connection(pipe);
+    PlaywrightImpl playwright = connection.initializePlaywright();
+    if (!playwright.initializer.has("preLaunchedBrowser")) {
+      try {
+        connection.close();
+      } catch (IOException e) {
+        e.printStackTrace(System.err);
+      }
+      throw new PlaywrightException("Malformed endpoint. Did you use launchServer method?");
+    }
+    playwright.initSharedSelectors(this.connection.getExistingObject("Playwright"));
+    BrowserImpl browser = connection.getExistingObject(playwright.initializer.getAsJsonObject("preLaunchedBrowser").get("guid").getAsString());
+    browser.isRemote = true;
+    browser.isConnectedOverWebSocket = true;
+    Consumer<JsonPipe> connectionCloseListener = t -> browser.notifyRemoteClosed();
+    pipe.onClose(connectionCloseListener);
+    browser.onDisconnected(b -> {
+      playwright.unregisterSelectors();
+      pipe.offClose(connectionCloseListener);
+      try {
+        connection.close();
+      } catch (IOException e) {
+        e.printStackTrace(System.err);
+      }
+    });
+    return browser;
   }
 
   @Override

playwright/src/main/java/com/microsoft/playwright/impl/Connection.java

@@ -85,6 +85,9 @@ public class Connection {
   }
 
   Connection(Transport transport) {
+    if (isLogging) {
+      transport = new TransportLogger(transport);
+    }
     this.transport = transport;
     root = new Root(this);
     String srcRoot = System.getenv("PLAYWRIGHT_JAVA_SRC");
@@ -172,11 +175,7 @@ public class Connection {
       metadata.addProperty("apiName", apiName);
     }
     message.add("metadata", metadata);
-    String messageString = gson().toJson(message);
-    if (isLogging) {
-      logWithTimestamp("SEND ► " + messageString);
-    }
-    transport.send(messageString);
+    transport.send(message);
     return result;
   }
 
@@ -200,16 +199,13 @@ public class Connection {
   }
 
   void processOneMessage() {
-    String messageString = transport.poll(Duration.ofMillis(10));
-    if (messageString == null) {
+    JsonObject message = transport.poll(Duration.ofMillis(10));
+    if (message == null) {
       return;
     }
-    if (isLogging) {
-      logWithTimestamp("◀ RECV " + messageString);
-    }
     Gson gson = gson();
-    Message message = gson.fromJson(messageString, Message.class);
-    dispatch(message);
+    Message messageObj = gson.fromJson(message, Message.class);
+    dispatch(messageObj);
   }
 
   private void dispatch(Message message) {
@@ -315,6 +311,9 @@ public class Connection {
       case "JSHandle":
         result = new JSHandleImpl(parent, type, guid, initializer);
         break;
+      case "JsonPipe":
+        result = new JsonPipe(parent, type, guid, initializer);
+        break;
       case "Page":
         result = new PageImpl(parent, type, guid, initializer);
         break;

playwright/src/main/java/com/microsoft/playwright/impl/JsonPipe.java

@@ -0,0 +1,109 @@
+/*
+ * 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.impl;
+
+import com.google.gson.JsonObject;
+import com.microsoft.playwright.PlaywrightException;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.LinkedList;
+import java.util.Queue;
+import java.util.function.Consumer;
+
+import static com.microsoft.playwright.impl.Serialization.gson;
+
+class JsonPipe extends ChannelOwner implements Transport {
+  private final Queue<JsonObject> incoming = new LinkedList<>();
+  private ListenerCollection<EventType> listeners = new ListenerCollection<>();
+  private enum EventType { CLOSE }
+  private boolean isClosed;
+
+  JsonPipe(ChannelOwner parent, String type, String guid, JsonObject initializer) {
+    super(parent, type, guid, initializer);
+  }
+
+  @Override
+  public void send(JsonObject message) {
+    checkIfClosed();
+    JsonObject params = new JsonObject();
+    params.add("message", message);
+    sendMessage("send", params);
+  }
+
+  @Override
+  public JsonObject poll(Duration timeout) {
+    Instant start = Instant.now();
+    return runUntil(() -> {}, new Waitable<JsonObject>() {
+      JsonObject message;
+      @Override
+      public boolean isDone() {
+        if (!incoming.isEmpty()) {
+          message = incoming.remove();
+          return true;
+        }
+        checkIfClosed();
+        if (Duration.between(start, Instant.now()).compareTo(timeout) > 0) {
+          return true;
+        }
+        return false;
+      }
+
+      @Override
+      public JsonObject get() {
+        return message;
+      }
+
+      @Override
+      public void dispose() {
+      }
+    });
+  }
+
+  @Override
+  public void close() throws IOException {
+    if (!isClosed) {
+      sendMessage("close");
+    }
+  }
+
+  void onClose(Consumer<JsonPipe> handler) {
+    listeners.add(EventType.CLOSE, handler);
+  }
+
+  void offClose(Consumer<JsonPipe> handler) {
+    listeners.remove(EventType.CLOSE, handler);
+  }
+
+
+  @Override
+  protected void handleEvent(String event, JsonObject params) {
+    if ("message".equals(event)) {
+      incoming.add(params.get("message").getAsJsonObject());
+    } else if ("closed".equals(event)) {
+      isClosed = true;
+      listeners.notify(EventType.CLOSE, this);
+    }
+  }
+
+  private void checkIfClosed() {
+    if (isClosed) {
+      throw new PlaywrightException("Browser has been closed");
+    }
+  }
+}

playwright/src/main/java/com/microsoft/playwright/impl/PipeTransport.java

@@ -15,6 +15,7 @@
  */
 package com.microsoft.playwright.impl;
 
+import com.google.gson.JsonObject;
 import com.microsoft.playwright.PlaywrightException;
 
 import java.io.*;
@@ -24,8 +25,10 @@ import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.TimeUnit;
 
+import static com.microsoft.playwright.impl.Serialization.gson;
+
 public class PipeTransport implements Transport {
-  private final BlockingQueue<String> incoming = new ArrayBlockingQueue<>(1000);
+  private final BlockingQueue<JsonObject> incoming = new ArrayBlockingQueue<>(1000);
   private final BlockingQueue<String> outgoing= new ArrayBlockingQueue<>(1000);
 
   private final ReaderThread readerThread;
@@ -42,24 +45,27 @@ public class PipeTransport implements Transport {
   }
 
   @Override
-  public void send(String message) {
+  public void send(JsonObject message) {
     if (isClosed) {
       throw new PlaywrightException("Playwright connection closed");
     }
     try {
-      outgoing.put(message);
+      // We could serialize the message on the IO thread but there is no guarantee
+      // that the message object won't be modified on this thread after it's added
+      // to the queue.
+      outgoing.put(gson().toJson(message));
     } catch (InterruptedException e) {
       throw new PlaywrightException("Failed to send message", e);
     }
   }
 
   @Override
-  public String poll(Duration timeout) {
+  public JsonObject poll(Duration timeout) {
     if (isClosed) {
       throw new PlaywrightException("Playwright connection closed");
     }
     try {
-      String message = incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS);
+      JsonObject message = incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS);
       if (message == null && readerThread.exception != null) {
         try {
           close();
@@ -91,7 +97,7 @@ public class PipeTransport implements Transport {
 
 class ReaderThread extends Thread {
   private final DataInputStream in;
-  private final BlockingQueue<String> queue;
+  private final BlockingQueue<JsonObject> queue;
   volatile boolean isClosing;
   volatile Exception exception;
 
@@ -107,7 +113,7 @@ class ReaderThread extends Thread {
     }
   }
 
-  ReaderThread(DataInputStream in, BlockingQueue<String> queue) {
+  ReaderThread(DataInputStream in, BlockingQueue<JsonObject> queue) {
     this.in = in;
     this.queue = queue;
   }
@@ -116,7 +122,8 @@ class ReaderThread extends Thread {
   public void run() {
     while (!isInterrupted()) {
       try {
-        queue.put(readMessage());
+        JsonObject message = gson().fromJson(readMessage(), JsonObject.class);
+        queue.put(message);
       } catch (IOException e) {
         if (!isInterrupted() && !isClosing) {
           exception = e;

playwright/src/main/java/com/microsoft/playwright/impl/Serialization.java

@@ -33,29 +33,26 @@ import java.nio.file.Path;
 import java.util.*;
 
 class Serialization {
-  private static Gson gson;
+  private static Gson gson = new GsonBuilder()
+    .registerTypeAdapter(SameSiteAttribute.class, new SameSiteAdapter().nullSafe())
+    .registerTypeAdapter(BrowserChannel.class, new ToLowerCaseAndDashSerializer<BrowserChannel>())
+    .registerTypeAdapter(ColorScheme.class, new ToLowerCaseAndDashSerializer<ColorScheme>())
+    .registerTypeAdapter(Media.class, new ToLowerCaseSerializer<Media>())
+    .registerTypeAdapter(ForcedColors.class, new ToLowerCaseSerializer<ForcedColors>())
+    .registerTypeAdapter(ReducedMotion.class, new ToLowerCaseAndDashSerializer<ReducedMotion>())
+    .registerTypeAdapter(ScreenshotType.class, new ToLowerCaseSerializer<ScreenshotType>())
+    .registerTypeAdapter(MouseButton.class, new ToLowerCaseSerializer<MouseButton>())
+    .registerTypeAdapter(LoadState.class, new ToLowerCaseSerializer<LoadState>())
+    .registerTypeAdapter(WaitUntilState.class, new ToLowerCaseSerializer<WaitUntilState>())
+    .registerTypeAdapter(WaitForSelectorState.class, new ToLowerCaseSerializer<WaitForSelectorState>())
+    .registerTypeAdapter((new TypeToken<List<KeyboardModifier>>(){}).getType(), new KeyboardModifiersSerializer())
+    .registerTypeAdapter(Optional.class, new OptionalSerializer())
+    .registerTypeHierarchyAdapter(JSHandleImpl.class, new HandleSerializer())
+    .registerTypeAdapter((new TypeToken<Map<String, String>>(){}).getType(), new StringMapSerializer())
+    .registerTypeAdapter((new TypeToken<Map<String, Object>>(){}).getType(), new FirefoxUserPrefsSerializer())
+    .registerTypeHierarchyAdapter(Path.class, new PathSerializer()).create();;
 
   static Gson gson() {
-    if (gson == null) {
-      gson = new GsonBuilder()
-        .registerTypeAdapter(SameSiteAttribute.class, new SameSiteAdapter().nullSafe())
-        .registerTypeAdapter(BrowserChannel.class, new ToLowerCaseAndDashSerializer<BrowserChannel>())
-        .registerTypeAdapter(ColorScheme.class, new ToLowerCaseAndDashSerializer<ColorScheme>())
-        .registerTypeAdapter(Media.class, new ToLowerCaseSerializer<Media>())
-        .registerTypeAdapter(ForcedColors.class, new ToLowerCaseSerializer<ForcedColors>())
-        .registerTypeAdapter(ReducedMotion.class, new ToLowerCaseAndDashSerializer<ReducedMotion>())
-        .registerTypeAdapter(ScreenshotType.class, new ToLowerCaseSerializer<ScreenshotType>())
-        .registerTypeAdapter(MouseButton.class, new ToLowerCaseSerializer<MouseButton>())
-        .registerTypeAdapter(LoadState.class, new ToLowerCaseSerializer<LoadState>())
-        .registerTypeAdapter(WaitUntilState.class, new ToLowerCaseSerializer<WaitUntilState>())
-        .registerTypeAdapter(WaitForSelectorState.class, new ToLowerCaseSerializer<WaitForSelectorState>())
-        .registerTypeAdapter((new TypeToken<List<KeyboardModifier>>(){}).getType(), new KeyboardModifiersSerializer())
-        .registerTypeAdapter(Optional.class, new OptionalSerializer())
-        .registerTypeHierarchyAdapter(JSHandleImpl.class, new HandleSerializer())
-        .registerTypeAdapter((new TypeToken<Map<String, String>>(){}).getType(), new StringMapSerializer())
-        .registerTypeAdapter((new TypeToken<Map<String, Object>>(){}).getType(), new FirefoxUserPrefsSerializer())
-        .registerTypeHierarchyAdapter(Path.class, new PathSerializer()).create();
-    }
     return gson;
   }
 

playwright/src/main/java/com/microsoft/playwright/impl/Transport.java

@@ -16,11 +16,13 @@
 
 package com.microsoft.playwright.impl;
 
+import com.google.gson.JsonObject;
+
 import java.io.IOException;
 import java.time.Duration;
 
 public interface Transport {
-  void send(String message);
-  String poll(Duration timeout);
+  void send(JsonObject message);
+  JsonObject poll(Duration timeout);
   void close() throws IOException;
 }

playwright/src/main/java/com/microsoft/playwright/impl/TransportLogger.java

@@ -0,0 +1,39 @@
+package com.microsoft.playwright.impl;
+
+import com.google.gson.JsonObject;
+
+import java.io.IOException;
+import java.time.Duration;
+
+import static com.microsoft.playwright.impl.LoggingSupport.logWithTimestamp;
+import static com.microsoft.playwright.impl.Serialization.gson;
+
+class TransportLogger implements Transport {
+  private final Transport transport;
+
+  TransportLogger(Transport transport) {
+    this.transport = transport;
+  }
+
+  @Override
+  public void send(JsonObject message) {
+    String messageString = gson().toJson(message);
+    logWithTimestamp("SEND ► " + messageString);
+    transport.send(message);
+  }
+
+  @Override
+  public JsonObject poll(Duration timeout) {
+    JsonObject message = transport.poll(timeout);
+    if (message != null) {
+      String messageString = gson().toJson(message);
+      logWithTimestamp("◀ RECV " + messageString);
+    }
+    return message;
+  }
+
+  @Override
+  public void close() throws IOException {
+    transport.close();
+  }
+}

playwright/src/main/java/com/microsoft/playwright/impl/WebSocketTransport.java

@@ -1,129 +0,0 @@
-/*
- * 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.impl;
-
-import com.microsoft.playwright.PlaywrightException;
-import org.java_websocket.client.WebSocketClient;
-import org.java_websocket.handshake.ServerHandshake;
-
-import java.io.IOException;
-import java.net.URI;
-import java.time.Duration;
-import java.util.Map;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.TimeUnit;
-import java.util.function.Consumer;
-
-
-class WebSocketTransport implements Transport {
-  private final BlockingQueue<String> incoming = new LinkedBlockingQueue<>();
-  private final ClientConnection clientConnection;
-  private final Duration slowMo;
-  private boolean isClosed;
-  private volatile Exception lastError;
-  ListenerCollection<EventType> listeners = new ListenerCollection<>();
-
-  private enum EventType { CLOSE }
-
-  private class ClientConnection extends WebSocketClient {
-    ClientConnection(URI serverUri) {
-      super(serverUri);
-    }
-
-    @Override
-    public void onOpen(ServerHandshake handshakedata) {
-    }
-
-    @Override
-    public void onMessage(String message) {
-      incoming.add(message);
-    }
-
-    @Override
-    public void onClose(int code, String reason, boolean remote) {
-    }
-
-    @Override
-    public void onError(Exception ex) {
-      lastError = ex;
-    }
-  }
-
-  WebSocketTransport(URI uri, Map<String, String> headers, Duration timeout, Duration slowMo) {
-    clientConnection = new ClientConnection(uri);
-    for (Map.Entry<String, String> entry : headers.entrySet()) {
-      clientConnection.addHeader(entry.getKey(), entry.getValue());
-    }
-    try {
-      if (!clientConnection.connectBlocking(timeout.toMillis(), TimeUnit.MILLISECONDS)) {
-        throw new PlaywrightException("Failed to connect", lastError);
-      }
-    } catch (InterruptedException e) {
-      throw new PlaywrightException("Failed to connect", e);
-    }
-    this.slowMo = slowMo;
-  }
-
-  @Override
-  public void send(String message) {
-    checkIfClosed();
-    clientConnection.send(message);
-  }
-
-  @Override
-  public String poll(Duration timeout) {
-    checkIfClosed();
-    try {
-      String message = incoming.poll(timeout.toMillis(), TimeUnit.MILLISECONDS);
-      if (slowMo != null && message != null) {
-        Thread.sleep(slowMo.toMillis());
-      }
-      return message;
-    } catch (InterruptedException e) {
-      throw new PlaywrightException("Failed to read message", e);
-    }
-  }
-
-  @Override
-  public void close() throws IOException {
-    if (isClosed) {
-      return;
-    }
-    isClosed = true;
-    clientConnection.close();
-  }
-
-  void onClose(Consumer<WebSocketTransport> handler) {
-    listeners.add(EventType.CLOSE, handler);
-  }
-
-  void offClose(Consumer<WebSocketTransport> 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");
-    }
-  }
-}

playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java

@@ -226,11 +226,21 @@ public class TestBrowserTypeConnect extends TestBase {
     BrowserServer server = launchBrowserServer(browserType);
     Browser remote = browserType.connect(server.wsEndpoint);
     Page page = remote.newPage();
+    boolean[] disconnected = {false};
+    remote.onDisconnected(b -> disconnected[0] = true);
     server.kill();
+    while (!disconnected[0]) {
+      try {
+        page.waitForTimeout(10);
+      } catch (PlaywrightException e) {
+        assertTrue(e.getMessage().contains("Browser has been closed"));
+      }
+    }
+    assertFalse(remote.isConnected());
     try {
       page.evaluate("1 + 1");
     } catch (PlaywrightException e) {
-      assertTrue(e.getMessage().contains("Playwright connection closed"));
+      assertTrue(e.getMessage().contains("Browser has been closed"), e.getMessage());
     }
     assertFalse(remote.isConnected());
   }
@@ -248,7 +258,7 @@ public class TestBrowserTypeConnect extends TestBase {
       try {
         page.waitForTimeout(10);
       } catch (PlaywrightException e) {
-        assertTrue(e.getMessage().contains("Playwright connection closed"));
+        assertTrue(e.getMessage().contains("Browser has been closed"));
       }
     }
     assertFalse(browser.isConnected());
@@ -256,7 +266,7 @@ public class TestBrowserTypeConnect extends TestBase {
       page.waitForNavigation(() -> {});
       fail("did not throw");
     } catch (PlaywrightException e) {
-      assertTrue(e.getMessage().contains("Playwright connection closed"));
+      assertTrue(e.getMessage().contains("Browser has been closed"));
     }
   }
 
@@ -271,7 +281,7 @@ public class TestBrowserTypeConnect extends TestBase {
       page.navigate(server.PREFIX + "/one-style.html", new Page.NavigateOptions().setTimeout(60000));
       fail("did not throw");
     } catch (PlaywrightException e) {
-      assertTrue(e.getMessage().contains("Playwright connection closed"));
+      assertTrue(e.getMessage().contains("Browser has been closed"));
     }
   }
 

pom.xml

@@ -76,6 +76,7 @@
         <groupId>org.java-websocket</groupId>
         <artifactId>Java-WebSocket</artifactId>
         <version>${websocket.version}</version>
+        <scope>test</scope>
       </dependency>
     </dependencies>
   </dependencyManagement>