From 11014145ce37a8d44b6083fcf2c5f71453f018c7 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Fri, 27 Sep 2024 21:18:30 -0700
Subject: [PATCH] chore: update trace event on action merge (#32860)

---
 .../playwright-core/src/server/recorder.ts    |  2 +-
 .../src/server/recorder/contextRecorder.ts    |  6 +--
 .../src/server/recorder/recorderApp.ts        |  4 +-
 .../src/server/recorder/recorderCollection.ts | 10 ++--
 .../src/server/recorder/recorderFrontend.ts   |  2 +-
 .../server/recorder/recorderInTraceViewer.ts  |  4 +-
 .../src/server/recorder/recorderUtils.ts      |  5 +-
 packages/recorder/src/actions.ts              |  3 +-
 packages/trace-viewer/src/ui/inspectorTab.tsx |  7 ++-
 .../src/ui/recorder/backendContext.tsx        | 22 +++-----
 .../src/ui/recorder/recorderView.tsx          | 50 +++++++++++++------
 packages/trace-viewer/src/ui/workbench.tsx    |  1 +
 12 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts
index 79efd4a46ed3f..4aab712b9bd64 100644
--- a/packages/playwright-core/src/server/recorder.ts
+++ b/packages/playwright-core/src/server/recorder.ts
@@ -139,7 +139,7 @@ export class Recorder implements InstrumentationListener, IRecorder {
     });
     this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], actions: actions.ActionInContext[] }) => {
       this._recorderSources = data.sources;
-      recorderApp.setActions(data.actions);
+      recorderApp.setActions(data.actions, data.sources);
       this._pushAllSources();
     });
 
diff --git a/packages/playwright-core/src/server/recorder/contextRecorder.ts b/packages/playwright-core/src/server/recorder/contextRecorder.ts
index 04bdee3735490..933a036233cc5 100644
--- a/packages/playwright-core/src/server/recorder/contextRecorder.ts
+++ b/packages/playwright-core/src/server/recorder/contextRecorder.ts
@@ -172,7 +172,7 @@ export class ContextRecorder extends EventEmitter {
           name: 'closePage',
           signals: [],
         },
-        timestamp: monotonicTime()
+        startTime: monotonicTime()
       });
       this._pageAliases.delete(page);
     });
@@ -195,7 +195,7 @@ export class ContextRecorder extends EventEmitter {
           url: page.mainFrame().url(),
           signals: [],
         },
-        timestamp: monotonicTime()
+        startTime: monotonicTime()
       });
     }
   }
@@ -232,7 +232,7 @@ export class ContextRecorder extends EventEmitter {
       frame: frameDescription,
       action,
       description: undefined,
-      timestamp: monotonicTime()
+      startTime: monotonicTime()
     };
     await this._delegate.rewriteActionInContext?.(this._pageAliases, actionInContext);
     return actionInContext;
diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts
index fa7327c4809fa..30149f9816ca2 100644
--- a/packages/playwright-core/src/server/recorder/recorderApp.ts
+++ b/packages/playwright-core/src/server/recorder/recorderApp.ts
@@ -38,7 +38,7 @@ export class EmptyRecorderApp extends EventEmitter implements IRecorderApp {
   async setSelector(selector: string, userGesture?: boolean): Promise<void> {}
   async updateCallLogs(callLogs: CallLog[]): Promise<void> {}
   async setSources(sources: Source[]): Promise<void> {}
-  async setActions(actions: actions.ActionInContext[]): Promise<void> {}
+  async setActions(actions: actions.ActionInContext[], sources: Source[]): Promise<void> {}
 }
 
 export class RecorderApp extends EventEmitter implements IRecorderApp {
@@ -155,7 +155,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
     }
   }
 
-  async setActions(actions: actions.ActionInContext[]): Promise<void> {
+  async setActions(actions: actions.ActionInContext[], sources: Source[]): Promise<void> {
   }
 
   async setSelector(selector: string, userGesture?: boolean): Promise<void> {
diff --git a/packages/playwright-core/src/server/recorder/recorderCollection.ts b/packages/playwright-core/src/server/recorder/recorderCollection.ts
index c0fe19441a41e..d61f727fa8688 100644
--- a/packages/playwright-core/src/server/recorder/recorderCollection.ts
+++ b/packages/playwright-core/src/server/recorder/recorderCollection.ts
@@ -75,9 +75,12 @@ export class RecorderCollection extends EventEmitter {
     this._fireChange();
     const error = await callback?.(callMetadata).catch((e: Error) => e);
     callMetadata.endTime = monotonicTime();
+    actionInContext.endTime = callMetadata.endTime;
     callMetadata.error = error ? serializeError(error) : undefined;
     // Do not wait for onAfterCall so that performAction returned immediately after the action.
-    mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata).catch(() => {});
+    mainFrame.instrumentation.onAfterCall(mainFrame, callMetadata).then(() => {
+      this._fireChange();
+    }).catch(() => {});
   }
 
   signal(pageAlias: string, frame: Frame, signal: Signal) {
@@ -94,7 +97,7 @@ export class RecorderCollection extends EventEmitter {
         generateGoto = true;
       else if (lastAction.action.name !== 'click' && lastAction.action.name !== 'press')
         generateGoto = true;
-      else if (timestamp - lastAction.timestamp > signalThreshold)
+      else if (timestamp - lastAction.startTime > signalThreshold)
         generateGoto = true;
 
       if (generateGoto) {
@@ -108,7 +111,8 @@ export class RecorderCollection extends EventEmitter {
             url: frame.url(),
             signals: [],
           },
-          timestamp
+          startTime: timestamp,
+          endTime: timestamp,
         });
       }
       return;
diff --git a/packages/playwright-core/src/server/recorder/recorderFrontend.ts b/packages/playwright-core/src/server/recorder/recorderFrontend.ts
index 4c456d24ab0e1..97df1d3ceb6c4 100644
--- a/packages/playwright-core/src/server/recorder/recorderFrontend.ts
+++ b/packages/playwright-core/src/server/recorder/recorderFrontend.ts
@@ -32,7 +32,7 @@ export interface IRecorderApp extends EventEmitter {
   setSelector(selector: string, userGesture?: boolean): Promise<void>;
   updateCallLogs(callLogs: CallLog[]): Promise<void>;
   setSources(sources: Source[]): Promise<void>;
-  setActions(actions: actions.ActionInContext[]): Promise<void>;
+  setActions(actions: actions.ActionInContext[], sources: Source[]): Promise<void>;
 }
 
 export type IRecorderAppFactory = (recorder: IRecorder) => Promise<IRecorderApp>;
diff --git a/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts b/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts
index 32dcb8954be18..ab67fe562cf7f 100644
--- a/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts
+++ b/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts
@@ -86,8 +86,8 @@ export class RecorderInTraceViewer extends EventEmitter implements IRecorderApp
     }
   }
 
-  async setActions(actions: actions.ActionInContext[]): Promise<void> {
-    this._transport.deliverEvent('setActions', { actions });
+  async setActions(actions: actions.ActionInContext[], sources: Source[]): Promise<void> {
+    this._transport.deliverEvent('setActions', { actions, sources });
   }
 }
 
diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts
index fafeaee434cbc..c40a6ac2014bb 100644
--- a/packages/playwright-core/src/server/recorder/recorderUtils.ts
+++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts
@@ -76,12 +76,11 @@ export function callMetadataForAction(pageAliases: Map<Page, string>, actionInCo
 
   const callMetadata: CallMetadata = {
     id: `call@${createGuid()}`,
-    stepId: `recorder@${createGuid()}`,
     apiName: 'page.' + method,
     objectId: mainFrame.guid,
     pageId: mainFrame._page.guid,
     frameId: mainFrame.guid,
-    startTime: actionInContext.timestamp,
+    startTime: actionInContext.startTime,
     endTime: 0,
     type: 'Frame',
     method,
@@ -102,7 +101,9 @@ export function collapseActions(actions: actions.ActionInContext[]): actions.Act
       result.push(action);
       continue;
     }
+    const startTime = result[result.length - 1].startTime;
     result[result.length - 1] = action;
+    result[result.length - 1].startTime = startTime;
   }
   return result;
 }
diff --git a/packages/recorder/src/actions.ts b/packages/recorder/src/actions.ts
index c416684b2a15f..a17e0c172bad3 100644
--- a/packages/recorder/src/actions.ts
+++ b/packages/recorder/src/actions.ts
@@ -153,5 +153,6 @@ export type ActionInContext = {
   frame: FrameDescription;
   description?: string;
   action: Action;
-  timestamp: number;
+  startTime: number;
+  endTime?: number;
 };
diff --git a/packages/trace-viewer/src/ui/inspectorTab.tsx b/packages/trace-viewer/src/ui/inspectorTab.tsx
index ba37cf1bd1d84..c608cde21b9be 100644
--- a/packages/trace-viewer/src/ui/inspectorTab.tsx
+++ b/packages/trace-viewer/src/ui/inspectorTab.tsx
@@ -17,18 +17,17 @@
 import { CodeMirrorWrapper } from '@web/components/codeMirrorWrapper';
 import type { Language } from '@web/components/codeMirrorWrapper';
 import { ToolbarButton } from '@web/components/toolbarButton';
-import { copy, useSetting } from '@web/uiUtils';
+import { copy } from '@web/uiUtils';
 import * as React from 'react';
 import './sourceTab.css';
 
 export const InspectorTab: React.FunctionComponent<{
+  showScreenshot: boolean,
   sdkLanguage: Language,
   setIsInspecting: (isInspecting: boolean) => void,
   highlightedLocator: string,
   setHighlightedLocator: (locator: string) => void,
-}> = ({ sdkLanguage, setIsInspecting, highlightedLocator, setHighlightedLocator }) => {
-  const [showScreenshot] = useSetting('screenshot-instead-of-snapshot', false);
-
+}> = ({ showScreenshot, sdkLanguage, setIsInspecting, highlightedLocator, setHighlightedLocator }) => {
   return <div className='vbox' style={{ backgroundColor: 'var(--vscode-sideBar-background)' }}>
     <div style={{ margin: '10px 0px 10px 10px', color: 'var(--vscode-editorCodeLens-foreground)', flex: 'none' }}>Locator</div>
     <div style={{ margin: '0 10px 10px', flex: 'auto' }}>
diff --git a/packages/trace-viewer/src/ui/recorder/backendContext.tsx b/packages/trace-viewer/src/ui/recorder/backendContext.tsx
index 29c8a9b19cc2f..312281001e7a3 100644
--- a/packages/trace-viewer/src/ui/recorder/backendContext.tsx
+++ b/packages/trace-viewer/src/ui/recorder/backendContext.tsx
@@ -25,9 +25,8 @@ export const BackendProvider: React.FunctionComponent<React.PropsWithChildren<{
 }>> = ({ guid, children }) => {
   const [connection, setConnection] = React.useState<Connection | undefined>(undefined);
   const [mode, setMode] = React.useState<Mode>('none');
-  const [actions, setActions] = React.useState<actionTypes.ActionInContext[]>([]);
-  const [sources, setSources] = React.useState<Source[]>([]);
-  const callbacks = React.useRef({ setMode, setActions, setSources });
+  const [actions, setActions] = React.useState<{ actions: actionTypes.ActionInContext[], sources: Source[] }>({ actions: [], sources: [] });
+  const callbacks = React.useRef({ setMode, setActions });
 
   React.useEffect(() => {
     const wsURL = new URL(`../${guid}`, window.location.toString());
@@ -40,8 +39,8 @@ export const BackendProvider: React.FunctionComponent<React.PropsWithChildren<{
   }, [guid]);
 
   const backend = React.useMemo(() => {
-    return connection ? { mode, actions, sources, connection } : undefined;
-  }, [actions, mode, sources, connection]);
+    return connection ? { mode, actions: actions.actions, sources: actions.sources, connection } : undefined;
+  }, [actions, mode, connection]);
 
   return <BackendContext.Provider value={backend}>
     {children}
@@ -56,8 +55,7 @@ export type Backend = {
 
 type ConnectionCallbacks = {
   setMode: (mode: Mode) => void;
-  setActions: (actions: actionTypes.ActionInContext[]) => void;
-  setSources: (sources: Source[]) => void;
+  setActions: (data: { actions: actionTypes.ActionInContext[], sources: Source[] }) => void;
 };
 
 class Connection {
@@ -111,14 +109,10 @@ class Connection {
       const { mode } = params as { mode: Mode };
       this._options.setMode(mode);
     }
-    if (method === 'setSources') {
-      const { sources } = params as { sources: Source[] };
-      this._options.setSources(sources);
-      (window as any).playwrightSourcesEchoForTest = sources;
-    }
     if (method === 'setActions') {
-      const { actions } = params as { actions: actionTypes.ActionInContext[] };
-      this._options.setActions(actions.filter(a => a.action.name !== 'openPage' && a.action.name !== 'closePage'));
+      const { actions, sources } = params as { actions: actionTypes.ActionInContext[], sources: Source[] };
+      this._options.setActions({ actions: actions.filter(a => a.action.name !== 'openPage' && a.action.name !== 'closePage'), sources });
+      (window as any).playwrightSourcesEchoForTest = sources;
     }
   }
 }
diff --git a/packages/trace-viewer/src/ui/recorder/recorderView.tsx b/packages/trace-viewer/src/ui/recorder/recorderView.tsx
index 67dae2c23929a..ed36efbbb6121 100644
--- a/packages/trace-viewer/src/ui/recorder/recorderView.tsx
+++ b/packages/trace-viewer/src/ui/recorder/recorderView.tsx
@@ -54,17 +54,25 @@ export const Workbench: React.FunctionComponent = () => {
   const backend = React.useContext(BackendContext);
   const model = React.useContext(ModelContext);
   const [fileId, setFileId] = React.useState<string | undefined>();
-  const [selectedCallTime, setSelectedCallTime] = React.useState<number | undefined>(undefined);
+  const [selectedStartTime, setSelectedStartTime] = React.useState<number | undefined>(undefined);
   const [isInspecting, setIsInspecting] = React.useState(false);
-  const [highlightedLocator, setHighlightedLocator] = React.useState<string>('');
+  const [highlightedLocatorInProperties, setHighlightedLocatorInProperties] = React.useState<string>('');
+  const [highlightedLocatorInTrace, setHighlightedLocatorInTrace] = React.useState<string>('');
+  const [traceCallId, setTraceCallId] = React.useState<string | undefined>();
 
   const setSelectedAction = React.useCallback((action: actionTypes.ActionInContext | undefined) => {
-    setSelectedCallTime(action?.timestamp);
+    setSelectedStartTime(action?.startTime);
   }, []);
 
   const selectedAction = React.useMemo(() => {
-    return backend?.actions.find(a => a.timestamp === selectedCallTime);
-  }, [backend?.actions, selectedCallTime]);
+    return backend?.actions.find(a => a.startTime === selectedStartTime);
+  }, [backend?.actions, selectedStartTime]);
+
+  React.useEffect(() => {
+    const callId = model?.actions.find(a => a.endTime && a.endTime === selectedAction?.endTime)?.callId;
+    if (callId)
+      setTraceCallId(callId);
+  }, [model, selectedAction]);
 
   const source = React.useMemo(() => backend?.sources.find(s => s.id === fileId) || backend?.sources[0], [backend?.sources, fileId]);
   const sourceLocation = React.useMemo(() => {
@@ -95,6 +103,17 @@ export const Workbench: React.FunctionComponent = () => {
     return { boundaries };
   }, [model]);
 
+  const locatorPickedInTrace = React.useCallback((locator: string) => {
+    setHighlightedLocatorInProperties(locator);
+    setHighlightedLocatorInTrace('');
+    setIsInspecting(false);
+  }, []);
+
+  const locatorTypedInProperties = React.useCallback((locator: string) => {
+    setHighlightedLocatorInTrace(locator);
+    setHighlightedLocatorInProperties(locator);
+  }, []);
+
   const actionList = <ActionListView
     sdkLanguage={sdkLanguage}
     actions={backend?.actions || []}
@@ -129,25 +148,23 @@ export const Workbench: React.FunctionComponent = () => {
     <SourceChooser fileId={fileId} sources={backend?.sources || []} setFileId={fileId => {
       setFileId(fileId);
     }} />
-    <ToolbarButton icon='clear-all' title='Clear' onClick={() => {
-    }}></ToolbarButton>
     <ToolbarButton icon='color-mode' title='Toggle color mode' toggled={false} onClick={() => toggleTheme()}></ToolbarButton>
   </Toolbar>;
 
   const sidebarTabbedPane = <TabbedPane tabs={[actionsTab]} />;
   const traceView = <TraceView
     sdkLanguage={sdkLanguage}
-    callTime={selectedCallTime || 0}
+    callId={traceCallId}
     isInspecting={isInspecting}
     setIsInspecting={setIsInspecting}
-    highlightedLocator={highlightedLocator}
-    setHighlightedLocator={setHighlightedLocator} />;
+    highlightedLocator={highlightedLocatorInTrace}
+    setHighlightedLocator={locatorPickedInTrace} />;
   const propertiesView = <PropertiesView
     sdkLanguage={sdkLanguage}
     boundaries={boundaries}
     setIsInspecting={setIsInspecting}
-    highlightedLocator={highlightedLocator}
-    setHighlightedLocator={setHighlightedLocator}
+    highlightedLocator={highlightedLocatorInProperties}
+    setHighlightedLocator={locatorTypedInProperties}
     sourceLocation={sourceLocation} />;
 
   return <div className='vbox workbench'>
@@ -196,6 +213,7 @@ const PropertiesView: React.FunctionComponent<{
     id: 'inspector',
     title: 'Locator',
     render: () => <InspectorTab
+      showScreenshot={false}
       sdkLanguage={sdkLanguage}
       setIsInspecting={setIsInspecting}
       highlightedLocator={highlightedLocator}
@@ -240,14 +258,14 @@ const PropertiesView: React.FunctionComponent<{
 
 const TraceView: React.FunctionComponent<{
   sdkLanguage: Language,
-  callTime: number;
+  callId: string | undefined,
   isInspecting: boolean;
   setIsInspecting: (value: boolean) => void;
   highlightedLocator: string;
   setHighlightedLocator: (locator: string) => void;
 }> = ({
   sdkLanguage,
-  callTime,
+  callId,
   isInspecting,
   setIsInspecting,
   highlightedLocator,
@@ -255,8 +273,8 @@ const TraceView: React.FunctionComponent<{
 }) => {
   const model = React.useContext(ModelContext);
   const action = React.useMemo(() => {
-    return model?.actions.find(a => a.startTime === callTime);
-  }, [model, callTime]);
+    return model?.actions.find(a => a.callId === callId);
+  }, [model, callId]);
 
   const snapshot = React.useMemo(() => {
     const snapshot = collectSnapshots(action);
diff --git a/packages/trace-viewer/src/ui/workbench.tsx b/packages/trace-viewer/src/ui/workbench.tsx
index 14f74bec2f776..5a5d285d11351 100644
--- a/packages/trace-viewer/src/ui/workbench.tsx
+++ b/packages/trace-viewer/src/ui/workbench.tsx
@@ -165,6 +165,7 @@ export const Workbench: React.FunctionComponent<{
     id: 'inspector',
     title: 'Locator',
     render: () => <InspectorTab
+      showScreenshot={showScreenshot}
       sdkLanguage={sdkLanguage}
       setIsInspecting={setIsInspecting}
       highlightedLocator={highlightedLocator}