fix(tracing): title the waits in the trace like the Node.js client (#2023)

Author: yury-s

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

@@ -342,7 +342,7 @@ private <T> T waitForEventWithTimeout(EventType eventType, Runnable code, Predic
 
   @Override
   public Page waitForPage(WaitForPageOptions options, Runnable code) {
-    return withWaitLogging("BrowserContext.close", logger -> waitForPageImpl(options, code));
+    return withWaitLogging("BrowserContext.waitForPage", "page", logger -> waitForPageImpl(options, code));
   }
 
   private Page waitForPageImpl(WaitForPageOptions options, Runnable code) {
@@ -706,7 +706,7 @@ public void waitForCondition(BooleanSupplier predicate, WaitForConditionOptions
 
   @Override
   public ConsoleMessage waitForConsoleMessage(WaitForConsoleMessageOptions options, Runnable code) {
-    return withWaitLogging("BrowserContext.waitForConsoleMessage", logger -> waitForConsoleMessageImpl(options, code));
+    return withWaitLogging("BrowserContext.waitForConsoleMessage", "console", logger -> waitForConsoleMessageImpl(options, code));
   }
 
   private ConsoleMessage waitForConsoleMessageImpl(WaitForConsoleMessageOptions options, Runnable code) {

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

@@ -81,7 +81,16 @@ void adopt(ChannelOwner child) {
   }
 
   <T> T withWaitLogging(String apiName, Function<Logger, T> code) {
-    return new WaitForEventLogger<>(this, apiName, code).get();
+    return new WaitForEventLogger<>(this, apiName, null, null, code).get();
+  }
+
+  /** A wait for the event, titled {@code Wait for event "<event>"} in the trace like the Node.js client's waitForEvent. */
+  <T> T withWaitLogging(String apiName, String event, Function<Logger, T> code) {
+    return new WaitForEventLogger<>(this, apiName, event, "Wait for event \"" + event + "\"", code).get();
+  }
+
+  <T> T withWaitLoggingTitled(String apiName, String title, Function<Logger, T> code) {
+    return new WaitForEventLogger<>(this, apiName, null, title, code).get();
   }
 
 

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

@@ -131,6 +131,10 @@ void setIsTracing(boolean tracing) {
     }
   }
 
+  String title() {
+    return title;
+  }
+
   String setTitle(String newTitle) {
     String previous = title;
     titleReported = false;

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

@@ -961,7 +961,7 @@ public Response get() {
 
   @Override
   public Response waitForNavigation(WaitForNavigationOptions options, Runnable code) {
-    return withWaitLogging("Frame.waitForNavigation", logger -> waitForNavigationImpl(logger, code, options, null));
+    return withWaitLoggingTitled("Frame.waitForNavigation", "Wait for navigation", logger -> waitForNavigationImpl(logger, code, options, null));
   }
 
   Response waitForNavigationImpl(Logger logger, Runnable code, WaitForNavigationOptions options) {

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

@@ -469,7 +469,7 @@ public ClockImpl clock() {
 
   @Override
   public Page waitForClose(WaitForCloseOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForClose", logger -> waitForCloseImpl(options, code));
+    return withWaitLogging("Page.waitForClose", "close", logger -> waitForCloseImpl(options, code));
   }
 
   private Page waitForCloseImpl(WaitForCloseOptions options, Runnable code) {
@@ -481,7 +481,7 @@ private Page waitForCloseImpl(WaitForCloseOptions options, Runnable code) {
 
   @Override
   public ConsoleMessage waitForConsoleMessage(WaitForConsoleMessageOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForConsoleMessage", logger -> waitForConsoleMessageImpl(options, code));
+    return withWaitLogging("Page.waitForConsoleMessage", "console", logger -> waitForConsoleMessageImpl(options, code));
   }
 
   private ConsoleMessage waitForConsoleMessageImpl(WaitForConsoleMessageOptions options, Runnable code) {
@@ -493,7 +493,7 @@ private ConsoleMessage waitForConsoleMessageImpl(WaitForConsoleMessageOptions op
 
   @Override
   public Download waitForDownload(WaitForDownloadOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForDownload", logger -> waitForDownloadImpl(options, code));
+    return withWaitLogging("Page.waitForDownload", "download", logger -> waitForDownloadImpl(options, code));
   }
 
   private Download waitForDownloadImpl(WaitForDownloadOptions options, Runnable code) {
@@ -505,7 +505,7 @@ private Download waitForDownloadImpl(WaitForDownloadOptions options, Runnable co
 
   @Override
   public FileChooser waitForFileChooser(WaitForFileChooserOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForFileChooser", logger -> waitForFileChooserImpl(options, code));
+    return withWaitLogging("Page.waitForFileChooser", "filechooser", logger -> waitForFileChooserImpl(options, code));
   }
 
   private FileChooser waitForFileChooserImpl(WaitForFileChooserOptions options, Runnable code) {
@@ -518,7 +518,7 @@ private FileChooser waitForFileChooserImpl(WaitForFileChooserOptions options, Ru
 
   @Override
   public Page waitForPopup(WaitForPopupOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForPopup", logger -> waitForPopupImpl(options, code));
+    return withWaitLogging("Page.waitForPopup", "popup", logger -> waitForPopupImpl(options, code));
   }
 
   private Page waitForPopupImpl(WaitForPopupOptions options, Runnable code) {
@@ -530,7 +530,7 @@ private Page waitForPopupImpl(WaitForPopupOptions options, Runnable code) {
 
   @Override
   public WebSocket waitForWebSocket(WaitForWebSocketOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForWebSocket", logger -> waitForWebSocketImpl(options, code));
+    return withWaitLogging("Page.waitForWebSocket", "websocket", logger -> waitForWebSocketImpl(options, code));
   }
 
   private WebSocket waitForWebSocketImpl(WaitForWebSocketOptions options, Runnable code) {
@@ -542,7 +542,7 @@ private WebSocket waitForWebSocketImpl(WaitForWebSocketOptions options, Runnable
 
   @Override
   public Worker waitForWorker(WaitForWorkerOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForWorker", logger -> waitForWorkerImpl(options, code));
+    return withWaitLogging("Page.waitForWorker", "worker", logger -> waitForWorkerImpl(options, code));
   }
 
   private Worker waitForWorkerImpl(WaitForWorkerOptions options, Runnable code) {
@@ -1477,7 +1477,7 @@ public void waitForLoadState(LoadState state, WaitForLoadStateOptions options) {
 
   @Override
   public Response waitForNavigation(WaitForNavigationOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForNavigation", logger -> waitForNavigationImpl(logger, code, options));
+    return withWaitLoggingTitled("Page.waitForNavigation", "Wait for navigation", logger -> waitForNavigationImpl(logger, code, options));
   }
 
   private Response waitForNavigationImpl(Logger logger, Runnable code, WaitForNavigationOptions options) {
@@ -1554,7 +1554,7 @@ public Request waitForRequest(Predicate<Request> predicate, WaitForRequestOption
   }
 
   private Request waitForRequest(UrlMatcher urlMatcher, Predicate<Request> predicate, WaitForRequestOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForRequest", logger -> {
+    return withWaitLogging("Page.waitForRequest", "request", logger -> {
       logger.log("waiting for request " + ((urlMatcher == null) ? "matching predicate" : urlMatcher.toString()));
       Predicate<Request> requestPredicate = predicate;
       if (requestPredicate == null) {
@@ -1573,7 +1573,7 @@ private Request waitForRequestImpl(Predicate<Request> predicate, WaitForRequestO
 
   @Override
   public Request waitForRequestFinished(WaitForRequestFinishedOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForRequestFinished", logger -> waitForRequestFinishedImpl(options, code));
+    return withWaitLogging("Page.waitForRequestFinished", "requestfinished", logger -> waitForRequestFinishedImpl(options, code));
   }
 
   private Request waitForRequestFinishedImpl(WaitForRequestFinishedOptions options, Runnable code) {
@@ -1599,7 +1599,7 @@ public Response waitForResponse(Predicate<Response> predicate, WaitForResponseOp
   }
 
   private Response waitForResponse(UrlMatcher urlMatcher, Predicate<Response> predicate, WaitForResponseOptions options, Runnable code) {
-    return withWaitLogging("Page.waitForResponse", logger -> {
+    return withWaitLogging("Page.waitForResponse", "response", logger -> {
       logger.log("waiting for response " + ((urlMatcher == null) ? "matching predicate" : urlMatcher.toString()));
       Predicate<Response> responsePredicate = predicate;
       if (responsePredicate == null) {

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

@@ -28,11 +28,15 @@ public class WaitForEventLogger<T> implements Supplier<T>, Logger {
   private final ChannelOwner channel;
   private final String waitId;
   private final String apiName;
+  private final String event;
+  private final String title;
 
-  WaitForEventLogger(ChannelOwner channelOwner, String apiName, Function<Logger, T> supplier) {
+  WaitForEventLogger(ChannelOwner channelOwner, String apiName, String event, String title, Function<Logger, T> supplier) {
     this.supplier = supplier;
     this.channel = channelOwner;
     this.apiName = apiName;
+    this.event = event;
+    this.title = title;
     this.waitId = createGuid();
   }
 
@@ -41,8 +45,8 @@ public T get() {
     {
       JsonObject info = new JsonObject();
       info.addProperty("phase", "before");
-      info.addProperty("event", "");
-      sendWaitInfo(info);
+      info.addProperty("event", event == null ? "" : event);
+      sendWaitInfo(info, title);
     }
     JsonObject info = new JsonObject();
     info.addProperty("phase", "after");
@@ -66,11 +70,27 @@ public void log(String message) {
   }
 
   private void sendWaitInfo(JsonObject info) {
+    sendWaitInfo(info, null);
+  }
+
+  // The title goes with the first message of the wait, as in the Node.js client, where it is the
+  // title of the api zone; an outer call that has a title of its own keeps it. It is set for that
+  // message only, so that the calls made from the callback of the wait are not hidden as internal.
+  private void sendWaitInfo(JsonObject info, String title) {
     info.addProperty("waitId", waitId);
+    Connection connection = channel.connection;
+    boolean setTitle = title != null && connection.title() == null;
+    if (setTitle) {
+      connection.setTitle(title);
+    }
     try {
       channel.sendMessageNoReply("__waitInfo__", info);
     } catch (RuntimeException e) {
       // Fire-and-forget: never throw to the caller.
+    } finally {
+      if (setTitle) {
+        connection.setTitle(null);
+      }
     }
   }
 }

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

@@ -85,7 +85,7 @@ public void offSocketError(Consumer<String> handler) {
 
   @Override
   public WebSocketFrame waitForFrameReceived(WaitForFrameReceivedOptions options, Runnable code) {
-    return withWaitLogging("WebSocket.waitForFrameReceived", logger -> waitForFrameReceivedImpl(options, code));
+    return withWaitLogging("WebSocket.waitForFrameReceived", "framereceived", logger -> waitForFrameReceivedImpl(options, code));
   }
 
   private WebSocketFrame waitForFrameReceivedImpl(WaitForFrameReceivedOptions options, Runnable code) {
@@ -97,7 +97,7 @@ private WebSocketFrame waitForFrameReceivedImpl(WaitForFrameReceivedOptions opti
 
   @Override
   public WebSocketFrame waitForFrameSent(WaitForFrameSentOptions options, Runnable code) {
-    return withWaitLogging("WebSocket.waitForFrameSent", logger -> waitForFrameSentImpl(options, code));
+    return withWaitLogging("WebSocket.waitForFrameSent", "framesent", logger -> waitForFrameSentImpl(options, code));
   }
 
   private WebSocketFrame waitForFrameSentImpl(WaitForFrameSentOptions options, Runnable code) {

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

@@ -73,12 +73,12 @@ private <T> T waitForEventWithTimeout(EventType eventType, Runnable code, Predic
 
   @Override
   public Worker waitForClose(WaitForCloseOptions options, Runnable code) {
-    return withWaitLogging("Worker.waitForClose", logger -> waitForCloseImpl(options, code));
+    return withWaitLogging("Worker.waitForClose", "close", logger -> waitForCloseImpl(options, code));
   }
 
   @Override
   public ConsoleMessage waitForConsoleMessage(WaitForConsoleMessageOptions options, Runnable code) {
-    return withWaitLogging("Worker.waitForConsoleMessage", logger -> waitForConsoleMessageImpl(options, code));
+    return withWaitLogging("Worker.waitForConsoleMessage", "console", logger -> waitForConsoleMessageImpl(options, code));
   }
 
   private ConsoleMessage waitForConsoleMessageImpl(WaitForConsoleMessageOptions options, Runnable code) {

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

@@ -375,6 +375,28 @@ public void shouldShowWaitForLoadState(Page page, BrowserContext context, Browse
     });
   }
 
+  @PlaywrightTest
+  public void shouldShowWaitForEventAndNavigation(Page page, BrowserContext context, BrowserType browserType, Server server, @TempDir Path tempDir) throws Exception {
+    context.tracing().start(new Tracing.StartOptions());
+
+    page.navigate(server.EMPTY_PAGE);
+    page.waitForConsoleMessage(() -> page.evaluate("() => console.log('hello')"));
+    page.waitForNavigation(() -> page.evaluate("() => window.location.href = '/one-style.html'"));
+
+    Path traceFile = tempDir.resolve("trace.zip");
+    context.tracing().stop(new Tracing.StopOptions().setPath(traceFile));
+
+    TraceViewerPage.showTraceViewer(browserType, traceFile, traceViewer -> {
+      assertThat(traceViewer.actionTitles()).hasText(new Pattern[] {
+        Pattern.compile("Navigate.*/empty.html"),
+        Pattern.compile("Wait for event \"console\""),
+        Pattern.compile("Evaluate"),
+        Pattern.compile("Wait for navigation"),
+        Pattern.compile("Evaluate"),
+      });
+    });
+  }
+
   @PlaywrightTest
   public void shouldRecordHarWithStartHarStopHar(Page page, BrowserContext context, Server server, @TempDir Path tempDir) throws Exception {
     Path harPath = tempDir.resolve("tracing.har");