fixed review comments and made the code way easier

Signed-off-by: René <snooz@posteo.de>

packages/playwright-core/src/client/tracing.ts

@@ -18,7 +18,6 @@ import type * as api from '../../types/types';
 import type * as channels from '@protocol/channels';
 import { Artifact } from './artifact';
 import { ChannelOwner } from './channelOwner';
-import { captureRawStack, filteredStackTrace } from '../utils';
 
 export class Tracing extends ChannelOwner<channels.TracingChannel> implements api.Tracing {
   private _includeSources = false;
@@ -53,10 +52,6 @@ export class Tracing extends ChannelOwner<channels.TracingChannel> implements ap
   }
 
   async group(name: string, options: { location?: { file: string, line?: number, column?: number } } = {}) {
-    if (!options.location) {
-      const filteredStack = filteredStackTrace(captureRawStack());
-      options.location = filteredStack[0];
-    }
     await this._channel.tracingGroup({ name, options });
   }
 

packages/playwright-core/src/server/dispatchers/tracingDispatcher.ts

@@ -15,6 +15,7 @@
  */
 
 import type * as channels from '@protocol/channels';
+import type { CallMetadata } from '@protocol/callMetadata';
 import type { Tracing } from '../trace/recorder/tracing';
 import { ArtifactDispatcher } from './artifactDispatcher';
 import { Dispatcher, existingDispatcher } from './dispatcher';
@@ -41,9 +42,9 @@ export class TracingDispatcher extends Dispatcher<Tracing, channels.TracingChann
     return await this._object.startChunk(params);
   }
 
-  async tracingGroup(params: channels.TracingTracingGroupParams): Promise<channels.TracingTracingGroupResult> {
+  async tracingGroup(params: channels.TracingTracingGroupParams, metadata: CallMetadata): Promise<channels.TracingTracingGroupResult> {
     const { name, options } = params;
-    await this._object.group(name, options);
+    await this._object.group(name, options, metadata);
   }
 
   async tracingGroupEnd(params: channels.TracingTracingGroupEndParams): Promise<channels.TracingTracingGroupEndResult> {

packages/playwright-core/src/server/trace/recorder/tracing.ts

@@ -62,7 +62,6 @@ type RecordingState = {
   recording: boolean;
   callIds: Set<string>;
   groupStack: string[];
-  groupId: number;
 };
 
 const kScreencastOptions = { width: 800, height: 600, quality: 90 };
@@ -151,7 +150,6 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
       recording: false,
       callIds: new Set(),
       groupStack: [],
-      groupId: 0,
     };
     this._fs.mkdir(this._state.resourcesDir);
     this._fs.writeFile(this._state.networkFile, '');
@@ -198,23 +196,27 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     return { traceName: this._state.traceName };
   }
 
-  async group(name: string, options: { location?: { file: string, line?: number, column?: number } } = {}): Promise<void> {
+  async group(name: string, options: { location?: { file: string, line?: number, column?: number } } = {}, metadata: CallMetadata): Promise<void> {
     if (!this._state)
       return;
-    const stackFrame: StackFrame = {
-      file: options.location?.file || '',
-      line: options.location?.line || 0,
-      column: options.location?.column || 0,
-    };
+    const stackFrames: StackFrame[] = [];
+    const { file, line, column } = options.location ?? metadata.location ?? {};
+    if (file) {
+      stackFrames.push({
+        file,
+        line: line ?? 0,
+        column: column ?? 0,
+      });
+    }
     const event: trace.BeforeActionTraceEvent = {
       type: 'before',
-      callId: `group-${this._state.groupId++}`,
-      startTime: monotonicTime(),
+      callId: metadata.id,
+      startTime: metadata.startTime,
       apiName: name,
       class: 'Tracing',
       method: 'group',
       params: { },
-      stack: [stackFrame],
+      stack: stackFrames,
     };
     if (this._state.groupStack.length)
       event.parentId = this._state.groupStack[this._state.groupStack.length - 1];
@@ -403,10 +405,12 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
 
   onBeforeCall(sdkObject: SdkObject, metadata: CallMetadata) {
     // IMPORTANT: no awaits before this._appendTraceEvent in this method.
-    const event = createBeforeActionTraceEvent(metadata);
+    const event = createBeforeActionTraceEvent(
+        metadata,
+        this._state?.groupStack.length ? this._state.groupStack[this._state.groupStack.length - 1] : undefined
+    );
     if (!event)
       return Promise.resolve();
-    this._applyOpenGroup(event);
     sdkObject.attribution.page?.temporarilyDisableTracingScreencastThrottling();
     event.beforeSnapshot = `before@${metadata.id}`;
     this._state?.callIds.add(metadata.id);
@@ -414,11 +418,6 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     return this._captureSnapshot(event.beforeSnapshot, sdkObject, metadata);
   }
 
-  private _applyOpenGroup(event: trace.BeforeActionTraceEvent) {
-    if (event.parentId === undefined && this._state?.groupStack.length)
-      event.parentId = this._state?.groupStack[this._state.groupStack.length - 1];
-  }
-
   onBeforeInputAction(sdkObject: SdkObject, metadata: CallMetadata) {
     if (!this._state?.callIds.has(metadata.id))
       return Promise.resolve();
@@ -626,10 +625,10 @@ export function shouldCaptureSnapshot(metadata: CallMetadata): boolean {
   return commandsWithTracingSnapshots.has(metadata.type + '.' + metadata.method);
 }
 
-function createBeforeActionTraceEvent(metadata: CallMetadata): trace.BeforeActionTraceEvent | null {
+function createBeforeActionTraceEvent(metadata: CallMetadata, parentId?: string): trace.BeforeActionTraceEvent | null {
   if (metadata.internal || metadata.method.startsWith('tracing'))
     return null;
-  return {
+  const event: trace.BeforeActionTraceEvent = {
     type: 'before',
     callId: metadata.id,
     startTime: metadata.startTime,
@@ -640,6 +639,9 @@ function createBeforeActionTraceEvent(metadata: CallMetadata): trace.BeforeActio
     stepId: metadata.stepId,
     pageId: metadata.pageId,
   };
+  if (parentId)
+    event.parentId = parentId;
+  return event;
 }
 
 function createInputActionTraceEvent(metadata: CallMetadata): trace.InputActionTraceEvent | null {

packages/playwright-core/src/utils/stackTrace.ts

@@ -47,25 +47,6 @@ export function captureRawStack(): RawStack {
   return stack.split('\n');
 }
 
-export function filterStackFile(file: string) {
-  if (!process.env.PWDEBUGIMPL && file.startsWith(CORE_DIR))
-    return false;
-  return true;
-}
-
-export function filteredStackTrace(rawStack: RawStack): StackFrame[] {
-  const frames: StackFrame[] = [];
-  for (const line of rawStack) {
-    const frame = parseStackTraceLine(line);
-    if (!frame || !frame.file)
-      continue;
-    if (!filterStackFile(frame.file))
-      continue;
-    frames.push(frame);
-  }
-  return frames;
-}
-
 export function captureLibraryStackTrace(): { frames: StackFrame[], apiName: string } {
   const stack = captureRawStack();
 

packages/playwright-core/types/types.d.ts

@@ -21059,8 +21059,8 @@ export interface Touchscreen {
  */
 export interface Tracing {
   /**
-   * Creates a new inline group in the trace log, causing any subsequent calls to be indented by an additional level,
-   * until [tracing.groupEnd()](https://playwright.dev/docs/api/class-tracing#tracing-group-end) is called.
+   * Creates a new inline group in the trace, causing any subsequent calls to belong to this group, until
+   * [tracing.groupEnd()](https://playwright.dev/docs/api/class-tracing#tracing-group-end) is called.
    * @param name Group name shown in the trace viewer.
    * @param options
    */
@@ -21084,7 +21084,7 @@ export interface Tracing {
   }): Promise<void>;
 
   /**
-   * Closes the last opened inline group in the trace log.
+   * Closes the last opened inline group in the trace.
    */
   groupEnd(): Promise<void>;