feat(driver): launch driver per playwright instance (#75)

playwright/src/main/java/com/microsoft/playwright/Playwright.java

@@ -1,12 +1,12 @@
 /*
  * Copyright (c) Microsoft Corporation.
- * <p>
+ *
  * 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
- * <p>
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * <p>
+ *
  * 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.
@@ -18,10 +18,9 @@ package com.microsoft.playwright;
 
 import com.microsoft.playwright.impl.PlaywrightImpl;
 
-import java.io.IOException;
 import java.util.Map;
 
-public interface Playwright {
+public interface Playwright extends AutoCloseable {
   static Playwright create() {
     return PlaywrightImpl.create();
   }
@@ -33,4 +32,7 @@ public interface Playwright {
   Map<String, DeviceDescriptor> devices();
 
   Selectors selectors();
+
+  @Override
+  void close() throws Exception;
 }

playwright/src/main/java/com/microsoft/playwright/example/Main.java

@@ -21,7 +21,7 @@ import java.io.File;
 import java.nio.file.Paths;
 
 public class Main {
-  public static void main(String[] args) {
+  public static void main(String[] args) throws Exception {
     Playwright playwright = Playwright.create();
     Browser browser = playwright.chromium().launch();
     BrowserContext context = browser.newContext(
@@ -31,5 +31,6 @@ public class Main {
     page.click("text=check feature status");
     page.screenshot(new Page.ScreenshotOptions().withPath(Paths.get("s.png")));
     browser.close();
+    playwright.close();
   }
 }

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

@@ -29,28 +29,32 @@ import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.*;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 public class PlaywrightImpl extends ChannelOwner implements Playwright {
   private static Path driverTempDir;
+  private Process driverProcess;
 
   public static PlaywrightImpl create() {
     try {
-      Path driver = ensureDriverExtracted();
+      Path driver = ensureDriverInstalled();
       ProcessBuilder pb = new ProcessBuilder(driver.toString(), "run-driver");
       pb.redirectError(ProcessBuilder.Redirect.INHERIT);
 //      pb.environment().put("DEBUG", "pw:pro*");
       Process p = pb.start();
       Connection connection = new Connection(p.getInputStream(), p.getOutputStream());
-      return (PlaywrightImpl) connection.waitForObjectWithKnownName("Playwright");
-    } catch (IOException e) {
+      PlaywrightImpl result = (PlaywrightImpl) connection.waitForObjectWithKnownName("Playwright");
+      result.driverProcess = p;
+      return result;
+    } catch (IOException | InterruptedException | URISyntaxException e) {
       throw new PlaywrightException("Failed to launch driver", e);
     }
   }
 
-  private static Path ensureDriverExtracted() {
+  private static synchronized Path ensureDriverInstalled() throws IOException, InterruptedException, URISyntaxException {
     if (driverTempDir == null) {
-      try {
       driverTempDir = Files.createTempDirectory("playwright-java-");
       driverTempDir.toFile().deleteOnExit();
       ClassLoader classloader = Thread.currentThread().getContextClassLoader();
@@ -59,14 +63,20 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright {
         try {
           extractResource(filePath, driverTempDir);
         } catch (IOException e) {
-            throw new RuntimeException("Failed to extract driver from " + path, e);
+          throw new PlaywrightException("Failed to extract driver from " + path, e);
         }
       });
-      } catch (IOException | URISyntaxException e) {
-        throw new PlaywrightException("Failed to launch driver", e);
+
+      Path driver = driverTempDir.resolve("playwright-cli");
+      ProcessBuilder pb = new ProcessBuilder(driver.toString(), "install");
+      pb.redirectError(ProcessBuilder.Redirect.INHERIT);
+      pb.redirectOutput(ProcessBuilder.Redirect.INHERIT);
+      Process p = pb.start();
+      boolean result = p.waitFor(10, TimeUnit.MINUTES);
+      if (!result) {
+        System.err.println("Timed out waiting for browsers to install");
       }
     }
-    // TODO: remove dir on exit
     return driverTempDir.resolve("playwright-cli");
   }
 
@@ -75,7 +85,7 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright {
     Files.copy(from, path);
     path.toFile().setExecutable(true);
     path.toFile().deleteOnExit();
-    System.out.println("extracting: " + from.toString() + " to " + path.toString());
+//    System.out.println("extracting: " + from.toString() + " to " + path.toString());
     return path;
   }
 
@@ -85,7 +95,7 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright {
   private final Selectors selectors;
   private final Map<String, DeviceDescriptor> devices = new HashMap<>();
 
-  public PlaywrightImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) {
+  PlaywrightImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) {
     super(parent, type, guid, initializer);
     chromium = parent.connection.getExistingObject(initializer.getAsJsonObject("chromium").get("guid").getAsString());
     firefox = parent.connection.getExistingObject(initializer.getAsJsonObject("firefox").get("guid").getAsString());
@@ -126,11 +136,13 @@ public class PlaywrightImpl extends ChannelOwner implements Playwright {
     return selectors;
   }
 
-  public void close() {
-    try {
+  @Override
+  public void close() throws Exception {
     connection.close();
-    } catch (IOException e) {
-      throw new PlaywrightException("Failed to close", e);
+    // playwright-cli will exit when its stdin is closed, we wait for that.
+    boolean didClose = driverProcess.waitFor(30, TimeUnit.SECONDS);
+    if (!didClose) {
+      System.err.println("WARNING: Timed out while waiting for driver process to exit");
     }
   }
 }

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

@@ -69,16 +69,19 @@ public class Transport {
       return;
     }
     isClosed = true;
-    readerThread.interrupt();
-    readerThread.in.close();
-    writerThread.interrupt();
+    // We interrupt only the outgoing pipe and keep reader thread running as
+    // otherwise child process may block on writing to its stdout and never
+    // exit (observed on Windows).
+    readerThread.isClosing = true;
     writerThread.out.close();
+    writerThread.interrupt();
   }
 }
 
 class ReaderThread extends Thread {
-  final DataInputStream in;
+  private final DataInputStream in;
   private final BlockingQueue<String> queue;
+  volatile boolean isClosing;
 
   private static int readIntLE(DataInputStream in) throws IOException {
     int ch1 = in.read();
@@ -103,8 +106,9 @@ class ReaderThread extends Thread {
       try {
         queue.put(readMessage());
       } catch (IOException e) {
-        if (!isInterrupted())
+        if (!isInterrupted() && !isClosing) {
           e.printStackTrace();
+        }
         break;
       } catch (InterruptedException e) {
         break;

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

@@ -69,6 +69,12 @@ public class TestBase {
     httpsServer = null;
   }
 
+  @AfterAll
+  static void closePlaywright() throws Exception {
+    playwright.close();
+    playwright = null;
+  }
+
   @BeforeEach
   void createContextAndPage() {
     server.reset();