[ForcedReflow] Highlight all relevant forced reflow events

- Only include events from the main frame and in the context window
- Layout events can have stack traces but we weren't detecting them.
  This can be helpful for LH uses cases where JS sampling is disabled.
- Forced reflow events that still lack any JS stack information are
  now highlighted and listed under a "Unattributed" item.
- Bottum-up attribution is determined using common helper functions
  - `Extras.StackTraceForEvent.get`
  - `Helpers.Trace.getZeroIndexedStackTraceForEvent`
- `relatedEvents` is all of the forced reflow events. No longer the
  top level function calls.

Bug: 401538867
Change-Id: I9d80e2593be0f242fbf1c1a442d2d6ce658b7466
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6341691
Reviewed-by: Andres Olivares <[email protected]>
Commit-Queue: Adam Raine <[email protected]>

Author: Adam Raine

front_end/models/trace/extras/StackTraceForEvent.ts

@@ -37,6 +37,12 @@ export function get(event: Types.Events.Event, parsedTrace: Handlers.Types.Parse
     result = getForExtensionEntry(event, parsedTrace);
   } else if (Types.Events.isUserTiming(event)) {
     result = getForUserTiming(event, parsedTrace);
+  } else if (Types.Events.isLayout(event) || Types.Events.isUpdateLayoutTree(event)) {
+    const node = parsedTrace.Renderer.entryToNode.get(event);
+    const parent = node?.parent?.entry;
+    if (parent) {
+      result = get(parent, parsedTrace);
+    }
   }
   if (result) {
     cacheForTrace.set(event, result);

front_end/models/trace/helpers/Trace.ts

@@ -39,6 +39,9 @@ export function stackTraceInEvent(event: Types.Events.Event): Types.Events.CallF
   if (Types.Events.isUpdateLayoutTree(event)) {
     return event.args.beginData?.stackTrace || null;
   }
+  if (Types.Events.isLayout(event)) {
+    return event.args.beginData.stackTrace ?? null;
+  }
   if (Types.Events.isFunctionCall(event)) {
     const data = event.args.data;
     if (!data) {
@@ -442,6 +445,7 @@ export function getZeroIndexedStackTraceForEvent(event: Types.Events.Event): Typ
       case Types.Events.Name.SCHEDULE_STYLE_RECALCULATION:
       case Types.Events.Name.INVALIDATE_LAYOUT:
       case Types.Events.Name.FUNCTION_CALL:
+      case Types.Events.Name.LAYOUT:
       case Types.Events.Name.UPDATE_LAYOUT_TREE: {
         return makeZeroBasedCallFrame(callFrame);
       }

front_end/models/trace/insights/ForcedReflow.test.ts

@@ -28,8 +28,8 @@ describeWithEnvironment('ForcedReflow', function() {
     assert.strictEqual(insight.topLevelFunctionCallData?.totalReflowTime, 26052);
 
     const callStack = insight.aggregatedBottomUpData[1];
-    assert.strictEqual(callStack.bottomUpData.columnNumber, 137906);
-    assert.strictEqual(callStack.bottomUpData.lineNumber, 6);
+    assert.strictEqual(callStack.bottomUpData!.columnNumber, 197203);
+    assert.strictEqual(callStack.bottomUpData!.lineNumber, 32);
     assert.lengthOf(callStack.relatedEvents, 2);
   });
 });

front_end/models/trace/insights/ForcedReflow.ts

@@ -3,15 +3,14 @@
 // found in the LICENSE file.
 
 import * as i18n from '../../../core/i18n/i18n.js';
+import * as Platform from '../../../core/platform/platform.js';
 import type * as Protocol from '../../../generated/protocol.js';
+import * as Extras from '../extras/extras.js';
 import type * as Handlers from '../handlers/handlers.js';
-import type {Warning} from '../handlers/WarningsHandler.js';
 import * as Helpers from '../helpers/helpers.js';
 import * as Types from '../types/types.js';
 
 import {
-  type BottomUpCallStack,
-  type ForcedReflowAggregatedData,
   InsightCategory,
   InsightKeys,
   type InsightModel,
@@ -41,6 +40,10 @@ export const UIStrings = {
    * @description Text to describe the total reflow time
    */
   totalReflowTime: 'Total reflow time',
+  /**
+   * @description Text to describe CPU processor tasks that could not be attributed to any specific source code.
+   */
+  unattributed: 'Unattributed',
 } as const;
 
 const str_ = i18n.i18n.registerUIStrings('models/trace/insights/ForcedReflow.ts', UIStrings);
@@ -51,49 +54,39 @@ export type ForcedReflowInsightModel = InsightModel<typeof UIStrings, {
   aggregatedBottomUpData: BottomUpCallStack[],
 }>;
 
-function aggregateForcedReflow(
-    data: Map<Warning, Types.Events.Event[]>,
-    entryToNodeMap: Map<Types.Events.Event, Helpers.TreeHelpers.TraceEntryNode>):
-    [ForcedReflowAggregatedData|undefined, BottomUpCallStack[]] {
+export interface BottomUpCallStack {
+  /**
+   * `null` indicates that this data is for unattributed force reflows.
+   */
+  bottomUpData: Types.Events.CallFrame|Protocol.Runtime.CallFrame|null;
+  totalTime: number;
+  relatedEvents: Types.Events.Event[];
+}
+
+export interface ForcedReflowAggregatedData {
+  topLevelFunctionCall: Types.Events.CallFrame|Protocol.Runtime.CallFrame;
+  totalReflowTime: number;
+  topLevelFunctionCallEvents: Types.Events.Event[];
+}
+
+function getCallFrameId(callFrame: Types.Events.CallFrame|Protocol.Runtime.CallFrame): string {
+  return callFrame.scriptId + ':' + callFrame.lineNumber + ':' + callFrame.columnNumber;
+}
+
+function getLargestTopLevelFunctionData(
+    forcedReflowEvents: Types.Events.Event[], traceParsedData: Handlers.Types.ParsedTrace): ForcedReflowAggregatedData|
+    undefined {
+  const entryToNodeMap = traceParsedData.Renderer.entryToNode;
   const dataByTopLevelFunction = new Map<string, ForcedReflowAggregatedData>();
-  const bottomUpDataMap = new Map<string, BottomUpCallStack>();
-  const forcedReflowEvents = data.get('FORCED_REFLOW');
-  if (!forcedReflowEvents || forcedReflowEvents.length === 0) {
-    return [undefined, []];
+  if (forcedReflowEvents.length === 0) {
+    return;
   }
 
-  forcedReflowEvents.forEach(e => {
+  for (const event of forcedReflowEvents) {
     // Gather the stack traces by searching in the tree
-    const traceNode = entryToNodeMap.get(e);
-
+    const traceNode = entryToNodeMap.get(event);
     if (!traceNode) {
-      return;
-    }
-    // Compute call stack fully
-    const bottomUpData: Array<Types.Events.CallFrame|Protocol.Runtime.CallFrame> = [];
-    let currentNode = traceNode;
-    let previousNode;
-    const childStack: Protocol.Runtime.CallFrame[] = [];
-
-    // Some profileCalls maybe constructed as its children in hierarchy tree
-    while (currentNode.children.length > 0) {
-      const childNode = currentNode.children[0];
-      if (!previousNode) {
-        previousNode = childNode;
-      }
-      const eventData = childNode.entry;
-      if (Types.Events.isProfileCall(eventData)) {
-        childStack.push(eventData.callFrame);
-      }
-      currentNode = childNode;
-    }
-
-    // In order to avoid too much information, we only contain 2 levels bottomUp data,
-    while (childStack.length > 0 && bottomUpData.length < 2) {
-      const traceData = childStack.pop();
-      if (traceData) {
-        bottomUpData.push(traceData);
-      }
+      continue;
     }
 
     let node = traceNode.parent;
@@ -102,88 +95,43 @@ function aggregateForcedReflow(
     while (node) {
       const eventData = node.entry;
       if (Types.Events.isProfileCall(eventData)) {
-        if (bottomUpData.length < 2) {
-          bottomUpData.push(eventData.callFrame);
-        }
+        topLevelFunctionCall = eventData.callFrame;
+        topLevelFunctionCallEvent = eventData;
       } else {
         // We have finished searching bottom up data
         if (Types.Events.isFunctionCall(eventData) && eventData.args.data &&
             Types.Events.objectIsCallFrame(eventData.args.data)) {
           topLevelFunctionCall = eventData.args.data;
           topLevelFunctionCallEvent = eventData;
-          if (bottomUpData.length === 0) {
-            bottomUpData.push(topLevelFunctionCall);
-          }
-        } else {
-          // Sometimes the top level task can be other JSInvocation event
-          // then we use the top level profile call as topLevelFunctionCall's data
-          const previousData = previousNode?.entry;
-          if (previousData && Types.Events.isProfileCall(previousData)) {
-            topLevelFunctionCall = previousData.callFrame;
-            topLevelFunctionCallEvent = previousNode?.entry;
-          }
         }
         break;
       }
-      previousNode = node;
       node = node.parent;
     }
 
-    if (!topLevelFunctionCall || !topLevelFunctionCallEvent || bottomUpData.length === 0) {
-      return;
-    }
-    const bottomUpDataId =
-        bottomUpData[0].scriptId + ':' + bottomUpData[0].lineNumber + ':' + bottomUpData[0].columnNumber + ':';
-
-    const data = bottomUpDataMap.get(bottomUpDataId) ?? {
-      bottomUpData: bottomUpData[0],
-      totalTime: 0,
-      relatedEvents: [],
-    };
-    data.totalTime += (e.dur ?? 0);
-    data.relatedEvents.push(e);
-    bottomUpDataMap.set(bottomUpDataId, data);
-
-    const aggregatedDataId =
-        topLevelFunctionCall.scriptId + ':' + topLevelFunctionCall.lineNumber + ':' + topLevelFunctionCall.columnNumber;
-    if (!dataByTopLevelFunction.has(aggregatedDataId)) {
-      dataByTopLevelFunction.set(aggregatedDataId, {
-        topLevelFunctionCall,
-        totalReflowTime: 0,
-        bottomUpData: new Set<string>(),
-        topLevelFunctionCallEvents: [],
-      });
+    if (!topLevelFunctionCall || !topLevelFunctionCallEvent) {
+      continue;
     }
-    const aggregatedData = dataByTopLevelFunction.get(aggregatedDataId);
-    if (aggregatedData) {
-      aggregatedData.totalReflowTime += (e.dur ?? 0);
-      aggregatedData.bottomUpData.add(bottomUpDataId);
-      aggregatedData.topLevelFunctionCallEvents.push(topLevelFunctionCallEvent);
-    }
-  });
 
-  let topTimeConsumingDataId = '';
-  let maxTime = 0;
-  dataByTopLevelFunction.forEach((value, key) => {
-    if (value.totalReflowTime > maxTime) {
-      maxTime = value.totalReflowTime;
-      topTimeConsumingDataId = key;
+    const aggregatedDataId = getCallFrameId(topLevelFunctionCall);
+    const aggregatedData =
+        Platform.MapUtilities.getWithDefault(dataByTopLevelFunction, aggregatedDataId, () => ({
+                                                                                         topLevelFunctionCall,
+                                                                                         totalReflowTime: 0,
+                                                                                         topLevelFunctionCallEvents: [],
+                                                                                       }));
+    aggregatedData.totalReflowTime += (event.dur ?? 0);
+    aggregatedData.topLevelFunctionCallEvents.push(topLevelFunctionCallEvent);
+  }
+
+  let topTimeConsumingData: ForcedReflowAggregatedData|undefined = undefined;
+  dataByTopLevelFunction.forEach(data => {
+    if (!topTimeConsumingData || data.totalReflowTime > topTimeConsumingData.totalReflowTime) {
+      topTimeConsumingData = data;
     }
   });
 
-  const aggregatedBottomUpData: BottomUpCallStack[] = [];
-  const topLevelFunctionCallData = dataByTopLevelFunction.get(topTimeConsumingDataId);
-  const dataSet = dataByTopLevelFunction.get(topTimeConsumingDataId)?.bottomUpData;
-  if (dataSet) {
-    dataSet.forEach(value => {
-      const callStackData = bottomUpDataMap.get(value);
-      if (callStackData && callStackData.totalTime > Helpers.Timing.milliToMicro(Types.Timing.Milli(1))) {
-        aggregatedBottomUpData.push(callStackData);
-      }
-    });
-  }
-
-  return [topLevelFunctionCallData, aggregatedBottomUpData];
+  return topTimeConsumingData;
 }
 
 function finalize(partialModel: PartialInsightModel<ForcedReflowInsightModel>): ForcedReflowInsightModel {
@@ -193,32 +141,50 @@ function finalize(partialModel: PartialInsightModel<ForcedReflowInsightModel>):
     title: i18nString(UIStrings.title),
     description: i18nString(UIStrings.description),
     category: InsightCategory.ALL,
-    state: partialModel.topLevelFunctionCallData !== undefined && partialModel.aggregatedBottomUpData.length !== 0 ?
-        'fail' :
-        'pass',
+    state: partialModel.aggregatedBottomUpData.length !== 0 ? 'fail' : 'pass',
     ...partialModel,
   };
 }
 
+function getBottomCallFrameForEvent(event: Types.Events.Event, traceParsedData: Handlers.Types.ParsedTrace):
+    Types.Events.CallFrame|Protocol.Runtime.CallFrame|null {
+  const profileStackTrace = Extras.StackTraceForEvent.get(event, traceParsedData);
+  const eventStackTrace = Helpers.Trace.getZeroIndexedStackTraceForEvent(event);
+
+  return profileStackTrace?.callFrames[0] ?? eventStackTrace?.[0] ?? null;
+}
+
 export function generateInsight(
-    traceParsedData: Handlers.Types.ParsedTrace, _context: InsightSetContext): ForcedReflowInsightModel {
-  const warningsData = traceParsedData.Warnings;
-  const entryToNodeMap = traceParsedData.Renderer.entryToNode;
+    traceParsedData: Handlers.Types.ParsedTrace, context: InsightSetContext): ForcedReflowInsightModel {
+  const isWithinContext = (event: Types.Events.Event): boolean => {
+    const frameId = Helpers.Trace.frameIDForEvent(event);
+    if (frameId !== context.frameId) {
+      return false;
+    }
 
-  if (!warningsData) {
-    throw new Error('no warnings data');
-  }
+    return Helpers.Timing.eventIsInBounds(event, context.bounds);
+  };
 
-  if (!entryToNodeMap) {
-    throw new Error('no renderer data');
+  const bottomUpDataMap = new Map<string, BottomUpCallStack>();
+  const events = traceParsedData.Warnings.perWarning.get('FORCED_REFLOW')?.filter(isWithinContext) ?? [];
+  for (const event of events) {
+    const bottomCallFrame = getBottomCallFrameForEvent(event, traceParsedData);
+    const bottomCallId = bottomCallFrame ? getCallFrameId(bottomCallFrame) : 'UNATTRIBUTED';
+    const bottomUpData =
+        Platform.MapUtilities.getWithDefault(bottomUpDataMap, bottomCallId, () => ({
+                                                                              bottomUpData: bottomCallFrame,
+                                                                              totalTime: 0,
+                                                                              relatedEvents: [],
+                                                                            }));
+    bottomUpData.totalTime += event.dur ?? 0;
+    bottomUpData.relatedEvents.push(event);
   }
 
-  const [topLevelFunctionCallData, aggregatedBottomUpData] =
-      aggregateForcedReflow(warningsData.perWarning, entryToNodeMap);
+  const topLevelFunctionCallData = getLargestTopLevelFunctionData(events, traceParsedData);
 
   return finalize({
-    relatedEvents: topLevelFunctionCallData?.topLevelFunctionCallEvents,
+    relatedEvents: events,
     topLevelFunctionCallData,
-    aggregatedBottomUpData,
+    aggregatedBottomUpData: [...bottomUpDataMap.values()],
   });
 }

front_end/models/trace/insights/types.ts

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import type * as Common from '../../../core/common/common.js';
-import type * as Protocol from '../../../generated/protocol.js';
 import type * as Lantern from '../lantern/lantern.js';
 import type * as Types from '../types/types.js';
 
@@ -34,19 +33,6 @@ export interface LanternContext {
   metrics: Record<string, Lantern.Metrics.MetricResult>;
 }
 
-export interface ForcedReflowAggregatedData {
-  topLevelFunctionCall: Types.Events.CallFrame|Protocol.Runtime.CallFrame;
-  totalReflowTime: number;
-  bottomUpData: Set<string>;
-  topLevelFunctionCallEvents: Types.Events.Event[];
-}
-
-export interface BottomUpCallStack {
-  bottomUpData: Types.Events.CallFrame|Protocol.Runtime.CallFrame;
-  totalTime: number;
-  relatedEvents: Types.Events.Event[];
-}
-
 export type InsightModelsType = typeof Models;
 
 export enum InsightWarning {

front_end/models/trace/types/TraceEvents.ts

@@ -1908,6 +1908,7 @@ export interface Layout extends Complete {
   args: Args&{
     beginData: {
       sampleTraceId?: number, frame: string, dirtyObjects: number, partialLayout: boolean, totalObjects: number,
+      stackTrace?: CallFrame[],
     },
     // endData is not reliably populated.
     // Why? TBD. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame_view.cc;l=847-851;drc=8b6aaad8027390ce6b32d82d57328e93f34bb8e5

front_end/panels/timeline/TimelineUIUtils.test.ts

@@ -652,7 +652,7 @@ describeWithMockConnection('TimelineUIUtils', function() {
           // The "Recalculation forced" Stack trace
           title: undefined,
           value:
-              'testFuncs.changeAttributeAndDisplay @ chromedevtools.github.io/performance-stories/style-invalidations/app.js:47:40',
+              'testFuncs.changeAttributeAndDisplay @ chromedevtools.github.io/performance-stories/style-invalidations/app.js:44:47\n(anonymous) @ chromedevtools.github.io/performance-stories/style-invalidations/app.js:64:36',
         },
         {
           title: 'Initiated by',