feat(trace viewer): improve source tab (#5038)

- Show stack trace and allow to select one.
- Fix an issue when the page is closed before action end and we lack an id.
- Fix timeline time labels.

src/cli/traceViewer/traceModel.ts

@@ -66,7 +66,6 @@ export type VideoMetaInfo = {
 export function readTraceFile(events: trace.TraceEvent[], traceModel: TraceModel, filePath: string) {
   const contextEntries = new Map<string, ContextEntry>();
   const pageEntries = new Map<string, PageEntry>();
-
   for (const event of events) {
     switch (event.type) {
       case 'context-created': {

src/cli/traceViewer/web/ui/sourceTab.css

@@ -17,10 +17,56 @@
 .source-tab {
   flex: auto;
   position: relative;
-  overflow: auto;
+  overflow: hidden;
   background: #fdfcfc;
   font-family: var(--monospace-font);
   white-space: nowrap;
+  display: flex;
+  flex-direction: row;
+}
+
+.source-content {
+  flex: 1 1 600px;
+  overflow: auto;
+}
+
+.source-stack {
+  flex: 1 1 120px;
+  display: flex;
+  flex-direction: column;
+  align-items: stretch;
+  overflow-y: auto;
+}
+
+.source-stack-frame {
+  flex: 0 0 20px;
+  font-size: smaller;
+  display: flex;
+  flex-direction: row;
+  align-items: center;
+  cursor: pointer;
+}
+
+.source-stack-frame.selected,
+.source-stack-frame:hover {
+  background: var(--inactive-focus-ring);
+}
+
+.source-stack-frame-function {
+  flex: 1 1 100px;
+  overflow: hidden;
+  text-overflow: ellipsis;
+}
+
+.source-stack-frame-location {
+  flex: 1 1 100px;
+  overflow: hidden;
+  text-overflow: ellipsis;
+  text-align: end;
+}
+
+.source-stack-frame-line {
+  flex: none;
 }
 
 .source-line-number {

src/cli/traceViewer/web/ui/sourceTab.tsx

@@ -21,27 +21,82 @@ import './sourceTab.css';
 import '../../../../third_party/highlightjs/highlightjs/tomorrow.css';
 import * as highlightjs from '../../../../third_party/highlightjs/highlightjs';
 
+type StackInfo = string | {
+  frames: {
+    filePath: string,
+    fileName: string,
+    lineNumber: number,
+    functionName: string,
+  }[];
+  fileContent: Map<string, string>;
+};
+
 export const SourceTab: React.FunctionComponent<{
   actionEntry: ActionEntry | undefined,
 }> = ({ actionEntry }) => {
-  const location = React.useMemo<{ fileName?: string, lineNumber?: number, value?: string }>(() => {
+  const [lastAction, setLastAction] = React.useState<ActionEntry | undefined>();
+  const [selectedFrame, setSelectedFrame] = React.useState<number>(0);
+  const [needReveal, setNeedReveal] = React.useState<boolean>(false);
+
+  if (lastAction !== actionEntry) {
+    setLastAction(actionEntry);
+    setSelectedFrame(0);
+    setNeedReveal(true);
+  }
+
+  const stackInfo = React.useMemo<StackInfo>(() => {
     if (!actionEntry)
-      return { value: '' };
+      return '';
     const { action } = actionEntry;
-    const frames = action.stack!.split('\n').slice(1);
+    if (!action.stack)
-    const frame = frames.filter(frame => !frame.includes('playwright/lib/') && !frame.includes('playwright/src/'))[0];
+      return '';
-    if (!frame)
+    let frames = action.stack.split('\n').slice(1);
-      return { value: action.stack! };
+    frames = frames.filter(frame => !frame.includes('playwright/lib/') && !frame.includes('playwright/src/'));
-    const match = frame.match(/at [^(]+\(([^:]+):(\d+):\d+\)/) || frame.match(/at ([^:^(]+):(\d+):\d+/);
+    const info: StackInfo = {
-    if (!match)
+      frames: [],
-      return { value: action.stack! };
+      fileContent: new Map(),
-    const fileName = match[1];
+    };
-    const lineNumber = parseInt(match[2], 10);
+    for (const frame of frames) {
-    return { fileName, lineNumber };
+      let filePath: string;
+      let lineNumber: number;
+      let functionName: string;
+      const match1 = frame.match(/at ([^(]+)\(([^:]+):(\d+):\d+\)/);
+      const match2 = frame.match(/at ([^:^(]+):(\d+):\d+/);
+      if (match1) {
+        functionName = match1[1];
+        filePath = match1[2];
+        lineNumber = parseInt(match1[3], 10);
+      } else if (match2) {
+        functionName = '';
+        filePath = match2[1];
+        lineNumber = parseInt(match2[2], 10);
+      } else {
+        continue;
+      }
+      const pathSep = navigator.platform.includes('Win') ? '\\' : '/';
+      const fileName = filePath.substring(filePath.lastIndexOf(pathSep) + 1);
+      info.frames.push({
+        filePath,
+        fileName,
+        lineNumber,
+        functionName: functionName || '(anonymous)',
+      });
+    }
+    if (!info.frames.length)
+      return action.stack;
+    return info;
   }, [actionEntry]);
 
   const content = useAsyncMemo<string[]>(async () => {
-    const value = location.fileName ? await window.readFile(location.fileName) : location.value;
+    let value: string;
+    if (typeof stackInfo === 'string') {
+      value = stackInfo;
+    } else {
+      const filePath = stackInfo.frames[selectedFrame].filePath;
+      if (!stackInfo.fileContent.has(filePath))
+        stackInfo.fileContent.set(filePath, await window.readFile(filePath).catch(e => `<Unable to read "${filePath}">`));
+      value = stackInfo.fileContent.get(filePath)!;
+    }
     const result = [];
     let continuation: any;
     for (const line of (value || '').split('\n')) {
@@ -50,26 +105,53 @@ export const SourceTab: React.FunctionComponent<{
       result.push(highlighted.value);
     }
     return result;
-  }, [location.fileName, location.value], []);
+  }, [stackInfo, selectedFrame], []);
+
+  const targetLine = typeof stackInfo === 'string' ? -1 : stackInfo.frames[selectedFrame].lineNumber;
 
   const targetLineRef = React.createRef<HTMLDivElement>();
   React.useLayoutEffect(() => {
-    if (targetLineRef.current)
+    if (needReveal && targetLineRef.current) {
       targetLineRef.current.scrollIntoView({ block: 'center', inline: 'nearest' });
-  }, [content, location.lineNumber, targetLineRef]);
+      setNeedReveal(false);
+    }
+  }, [needReveal, targetLineRef]);
 
-  return <div className='source-tab'>{
+  return <div className='source-tab'>
-    content.map((markup, index) => {
+    <div className='source-content'>{
-      const isTargetLine = (index + 1) === location.lineNumber;
+      content.map((markup, index) => {
-      return <div
+        const isTargetLine = (index + 1) === targetLine;
-        key={index}
+        return <div
-        className={isTargetLine ? 'source-line-highlight' : ''}
+          key={index}
-        ref={isTargetLine ? targetLineRef : null}
+          className={isTargetLine ? 'source-line-highlight' : ''}
-      >
+          ref={isTargetLine ? targetLineRef : null}
-        <div className='source-line-number'>{index + 1}</div>
+        >
-        <div className='source-code' dangerouslySetInnerHTML={{ __html: markup }}></div>
+          <div className='source-line-number'>{index + 1}</div>
-      </div>;
+          <div className='source-code' dangerouslySetInnerHTML={{ __html: markup }}></div>
-    })
+        </div>;
-  }
+      })
+    }</div>
+    {typeof stackInfo !== 'string' && <div className='source-stack'>{
+      stackInfo.frames.map((frame, index) => {
+        return <div
+          key={index}
+          className={'source-stack-frame' + (selectedFrame === index ? ' selected' : '')}
+          onClick={() => {
+            setSelectedFrame(index);
+            setNeedReveal(true);
+          }}
+        >
+          <span className='source-stack-frame-function'>
+            {frame.functionName}
+          </span>
+          <span className='source-stack-frame-location'>
+            {frame.fileName}
+          </span>
+          <span className='source-stack-frame-line'>
+            {':' + frame.lineNumber}
+          </span>
+        </div>;
+      })
+    }</div>}
   </div>;
 };

src/cli/traceViewer/web/ui/timeline.tsx

@@ -248,12 +248,12 @@ function msToString(ms: number): string {
 
   const minutes = seconds / 60;
   if (minutes < 60)
-    return minutes.toFixed(1) + 's';
+    return minutes.toFixed(1) + 'm';
 
   const hours = minutes / 60;
   if (hours < 24)
     return hours.toFixed(1) + 'h';
 
   const days = hours / 24;
-  return days.toFixed(1) + 'h';
+  return days.toFixed(1) + 'd';
 }

src/trace/tracer.ts

@@ -69,6 +69,8 @@ class Tracer implements ContextListener {
   }
 }
 
+const pageIdSymbol = Symbol('pageId');
+
 class ContextTracer implements SnapshotterDelegate, ActionListener {
   private _context: BrowserContext;
   private _contextId: string;
@@ -78,7 +80,6 @@ class ContextTracer implements SnapshotterDelegate, ActionListener {
   private _snapshotter: Snapshotter;
   private _eventListeners: RegisteredListener[];
   private _disposed = false;
-  private _pageToId = new Map<Page, string>();
   private _traceFile: string;
 
   constructor(context: BrowserContext, traceStorageDir: string, traceFile: string) {
@@ -125,7 +126,7 @@ class ContextTracer implements SnapshotterDelegate, ActionListener {
   }
 
   pageId(page: Page): string {
-    return this._pageToId.get(page)!;
+    return (page as any)[pageIdSymbol];
   }
 
   async onAfterAction(result: ProgressResult, metadata: ActionMetadata): Promise<void> {
@@ -135,7 +136,7 @@ class ContextTracer implements SnapshotterDelegate, ActionListener {
         timestamp: monotonicTime(),
         type: 'action',
         contextId: this._contextId,
-        pageId: this._pageToId.get(metadata.page),
+        pageId: this.pageId(metadata.page),
         action: metadata.type,
         selector: typeof metadata.target === 'string' ? metadata.target : undefined,
         value: metadata.value,
@@ -153,7 +154,7 @@ class ContextTracer implements SnapshotterDelegate, ActionListener {
 
   private _onPage(page: Page) {
     const pageId = 'page@' + createGuid();
-    this._pageToId.set(page, pageId);
+    (page as any)[pageIdSymbol] = pageId;
 
     const event: trace.PageCreatedTraceEvent = {
       timestamp: monotonicTime(),
@@ -230,7 +231,6 @@ class ContextTracer implements SnapshotterDelegate, ActionListener {
     });
 
     page.once(Page.Events.Close, () => {
-      this._pageToId.delete(page);
       if (this._disposed)
         return;
       const event: trace.PageDestroyedTraceEvent = {
@@ -263,7 +263,6 @@ class ContextTracer implements SnapshotterDelegate, ActionListener {
     this._disposed = true;
     this._context._actionListeners.delete(this);
     helper.removeEventListeners(this._eventListeners);
-    this._pageToId.clear();
     this._snapshotter.dispose();
     const event: trace.ContextDestroyedTraceEvent = {
       timestamp: monotonicTime(),