[RPP] Include unsized images to CLS culprits

https://screenshot.googleplex.com/54UR4P4ZKj8NZMm

Bug:373488475
Change-Id: I9b70d65518fea2ad2dc01ef173adef8e66ed7a0d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5933343
Commit-Queue: Adriana Ixba <[email protected]>
Reviewed-by: Paul Irish <[email protected]>

Author: Adriana Ixba

front_end/models/trace/handlers/LayoutShiftsHandler.ts

@@ -46,11 +46,13 @@ interface LayoutShifts {
   // We use these to calculate root causes for a given LayoutShift
   // TODO(crbug/41484172): should be readonly
   prePaintEvents: Types.Events.PrePaint[];
+  paintImageEvents: Types.Events.PaintImage[];
   layoutInvalidationEvents: readonly Types.Events.LayoutInvalidationTracking[];
   scheduleStyleInvalidationEvents: readonly Types.Events.ScheduleStyleInvalidationTracking[];
   styleRecalcInvalidationEvents: readonly Types.Events.StyleRecalcInvalidationTracking[];
   renderFrameImplCreateChildFrameEvents: readonly Types.Events.RenderFrameImplCreateChildFrame[];
   domLoadingEvents: readonly Types.Events.DomLoading[];
+  layoutImageUnsizedEvents: readonly Types.Events.LayoutImageUnsized[];
   beginRemoteFontLoadEvents: readonly Types.Events.BeginRemoteFontLoad[];
   scoreRecords: readonly ScoreRecord[];
   // TODO(crbug/41484172): should be readonly
@@ -81,6 +83,7 @@ const scheduleStyleInvalidationEvents: Types.Events.ScheduleStyleInvalidationTra
 const styleRecalcInvalidationEvents: Types.Events.StyleRecalcInvalidationTracking[] = [];
 const renderFrameImplCreateChildFrameEvents: Types.Events.RenderFrameImplCreateChildFrame[] = [];
 const domLoadingEvents: Types.Events.DomLoading[] = [];
+const layoutImageUnsizedEvents: Types.Events.LayoutImageUnsized[] = [];
 const beginRemoteFontLoadEvents: Types.Events.BeginRemoteFontLoad[] = [];
 
 const backendNodeIds = new Set<Protocol.DOM.BackendNodeId>();
@@ -91,6 +94,8 @@ const backendNodeIds = new Set<Protocol.DOM.BackendNodeId>();
 // node of such shift.
 const prePaintEvents: Types.Events.PrePaint[] = [];
 
+const paintImageEvents: Types.Events.PaintImage[] = [];
+
 let sessionMaxScore = 0;
 
 let clsWindowID = -1;
@@ -125,7 +130,9 @@ export function reset(): void {
   scheduleStyleInvalidationEvents.length = 0;
   styleRecalcInvalidationEvents.length = 0;
   prePaintEvents.length = 0;
+  paintImageEvents.length = 0;
   renderFrameImplCreateChildFrameEvents.length = 0;
+  layoutImageUnsizedEvents.length = 0;
   domLoadingEvents.length = 0;
   beginRemoteFontLoadEvents.length = 0;
   backendNodeIds.clear();
@@ -165,9 +172,15 @@ export function handleEvent(event: Types.Events.Event): void {
   if (Types.Events.isDomLoading(event)) {
     domLoadingEvents.push(event);
   }
+  if (Types.Events.isLayoutImageUnsized(event)) {
+    layoutImageUnsizedEvents.push(event);
+  }
   if (Types.Events.isBeginRemoteFontLoad(event)) {
     beginRemoteFontLoadEvents.push(event);
   }
+  if (Types.Events.isPaintImage(event)) {
+    paintImageEvents.push(event);
+  }
 }
 
 function traceWindowFromTime(time: Types.Timing.MicroSeconds): Types.Timing.TraceWindowMicroSeconds {
@@ -251,7 +264,9 @@ export async function finalize(): Promise<void> {
   layoutInvalidationEvents.sort((a, b) => a.ts - b.ts);
   renderFrameImplCreateChildFrameEvents.sort((a, b) => a.ts - b.ts);
   domLoadingEvents.sort((a, b) => a.ts - b.ts);
+  layoutImageUnsizedEvents.sort((a, b) => a.ts - b.ts);
   beginRemoteFontLoadEvents.sort((a, b) => a.ts - b.ts);
+  paintImageEvents.sort((a, b) => a.ts - b.ts);
 
   // Each function transforms the data used by the next, as such the invoke order
   // is important.
@@ -508,11 +523,13 @@ export function data(): LayoutShifts {
     styleRecalcInvalidationEvents: [],
     renderFrameImplCreateChildFrameEvents,
     domLoadingEvents,
+    layoutImageUnsizedEvents,
     beginRemoteFontLoadEvents,
     scoreRecords,
     // TODO(crbug/41484172): change the type so no need to clone
     backendNodeIds: [...backendNodeIds],
     clustersByNavigationId: new Map(clustersByNavigationId),
+    paintImageEvents,
   };
 }
 

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

@@ -189,6 +189,28 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
         assert.isOk(shift3);
         assert.isEmpty(shift3[1].fontRequests);
       });
+
+      it('handles potential unsized images root cause correctly', async function() {
+        const {data, insights} = await processTrace(this, 'unsized-images.json.gz');
+        const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
+        const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
+        const {shifts} = insight;
+        assert.exists(shifts);
+        assert.strictEqual(shifts.size, 2);
+
+        const unsizedImages = data.LayoutShifts.layoutImageUnsizedEvents;
+        assert.strictEqual(unsizedImages.length, 2);
+
+        const layoutShiftEvents = Array.from(shifts.entries());
+        const shift1 = layoutShiftEvents.at(0);
+        assert.isOk(shift1);
+        // Root cause should match the nodeId of the unsized images events.
+        assert.strictEqual(shift1[1].unsizedImages[0], unsizedImages[0].args.data.nodeId);
+
+        const shift2 = layoutShiftEvents.at(1);
+        assert.isOk(shift2);
+        assert.strictEqual(shift2[1].unsizedImages[0], unsizedImages[1].args.data.nodeId);
+      });
     });
   });
   describe('clusters', function() {

front_end/models/trace/insights/CumulativeLayoutShift.ts

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import * as Platform from '../../../core/platform/platform.js';
+import type * as Protocol from '../../../generated/protocol.js';
 import * as Helpers from '../helpers/helpers.js';
 import * as Types from '../types/types.js';
 
@@ -146,17 +147,23 @@ const ACTIONABLE_FAILURE_REASONS = [
 
 // 500ms window.
 // Use this window to consider events and requests that may have caused a layout shift.
-const INVALIDATION_WINDOW = Helpers.Timing.secondsToMicroseconds(Types.Timing.Seconds(0.5));
+const ROOT_CAUSE_WINDOW = Helpers.Timing.secondsToMicroseconds(Types.Timing.Seconds(0.5));
 
 export interface LayoutShiftRootCausesData {
   iframeIds: string[];
   fontRequests: Types.Events.SyntheticNetworkRequest[];
   nonCompositedAnimations: NoncompositedAnimationFailure[];
+  unsizedImages: Protocol.DOM.BackendNodeId[];
 }
 
-function isInInvalidationWindow(event: Types.Events.Event, targetEvent: Types.Events.Event): boolean {
+/**
+ * Returns if an event happens within the root cause window, before the target event.
+ *          ROOT_CAUSE_WINDOW               v target event
+ *        |------------------------|=======================
+ */
+function isInRootCauseWindow(event: Types.Events.Event, targetEvent: Types.Events.Event): boolean {
   const eventEnd = event.dur ? event.ts + event.dur : event.ts;
-  return eventEnd < targetEvent.ts && eventEnd >= targetEvent.ts - INVALIDATION_WINDOW;
+  return eventEnd < targetEvent.ts && eventEnd >= targetEvent.ts - ROOT_CAUSE_WINDOW;
 }
 
 export function getNonCompositedFailure(animationEvent: Types.Events.SyntheticAnimationPair):
@@ -205,14 +212,14 @@ function getNonCompositedFailureRootCauses(
     }
     allAnimationFailures.push(...failures);
 
-    const nextPrePaint = getNextPrePaintEvent(prePaintEvents, animation);
+    const nextPrePaint = getNextEvent(prePaintEvents, animation) as Types.Events.PrePaint | null;
     // If no following prePaint, this is not a root cause.
     if (!nextPrePaint) {
       continue;
     }
 
-    // If the animation event is outside the INVALIDATION_WINDOW, it could not be a root cause.
-    if (!isInInvalidationWindow(animation, nextPrePaint)) {
+    // If the animation event is outside the ROOT_CAUSE_WINDOW, it could not be a root cause.
+    if (!isInRootCauseWindow(animation, nextPrePaint)) {
       continue;
     }
 
@@ -269,19 +276,18 @@ function getShiftsByPrePaintEvents(
 }
 
 /**
- * This gets the first prePaint event that follows the provided event and returns it.
+ * Given a source event list, this returns the first event of that list that directly follows the target event.
  */
-function getNextPrePaintEvent(
-    prePaintEvents: Types.Events.PrePaint[], targetEvent: Types.Events.Event): Types.Events.PrePaint|undefined {
-  // Get the first PrePaint event that happened after the targetEvent.
-  const nextPrePaintIndex = Platform.ArrayUtilities.nearestIndexFromBeginning(
-      prePaintEvents, prePaint => prePaint.ts > targetEvent.ts + (targetEvent.dur || 0));
+function getNextEvent(sourceEvents: Types.Events.Event[], targetEvent: Types.Events.Event): Types.Events.Event|
+    undefined {
+  const index = Platform.ArrayUtilities.nearestIndexFromBeginning(
+      sourceEvents, source => source.ts > targetEvent.ts + (targetEvent.dur || 0));
   // No PrePaint event registered after this event
-  if (nextPrePaintIndex === null) {
+  if (index === null) {
     return undefined;
   }
 
-  return prePaintEvents[nextPrePaintIndex];
+  return sourceEvents[index];
 }
 
 /**
@@ -294,7 +300,7 @@ function getIframeRootCauses(
     rootCausesByShift: Map<Types.Events.LayoutShift, LayoutShiftRootCausesData>,
     domLoadingEvents: readonly Types.Events.DomLoading[]): Map<Types.Events.LayoutShift, LayoutShiftRootCausesData> {
   for (const iframeEvent of iframeCreatedEvents) {
-    const nextPrePaint = getNextPrePaintEvent(prePaintEvents, iframeEvent);
+    const nextPrePaint = getNextEvent(prePaintEvents, iframeEvent) as Types.Events.PrePaint | null;
     // If no following prePaint, this is not a root cause.
     if (!nextPrePaint) {
       continue;
@@ -324,10 +330,43 @@ function getIframeRootCauses(
   return rootCausesByShift;
 }
 
+/**
+ * An unsized image is considered a root cause if its PaintImage can be correlated to a
+ * layout shift. We can correlate PaintImages with unsized images by their matching nodeIds.
+ *                           X      <- layout shift
+ *              |----------------|
+ *                    ^ PrePaint event   |-----|
+ *                                          ^ PaintImage
+ */
+function getUnsizedImageRootCauses(
+    unsizedImageEvents: readonly Types.Events.LayoutImageUnsized[], paintImageEvents: Types.Events.PaintImage[],
+    shiftsByPrePaint: Map<Types.Events.PrePaint, Types.Events.LayoutShift[]>,
+    rootCausesByShift: Map<Types.Events.LayoutShift, LayoutShiftRootCausesData>):
+    Map<Types.Events.LayoutShift, LayoutShiftRootCausesData> {
+  shiftsByPrePaint.forEach((shifts, prePaint) => {
+    const paintImage = getNextEvent(paintImageEvents, prePaint) as Types.Events.PaintImage | null;
+    // The unsized image corresponds to this PaintImage.
+    const matchingNode =
+        unsizedImageEvents.find(unsizedImage => unsizedImage.args.data.nodeId === paintImage?.args.data.nodeId);
+    if (!matchingNode) {
+      return;
+    }
+    // The unsized image is a potential root cause of all the shifts of this prePaint.
+    for (const shift of shifts) {
+      const rootCausesForShift = rootCausesByShift.get(shift);
+      if (!rootCausesForShift) {
+        throw new Error('Unaccounted shift');
+      }
+      rootCausesForShift.unsizedImages.push(matchingNode.args.data.nodeId);
+    }
+  });
+  return rootCausesByShift;
+}
+
 /**
  * A font request is considered a root cause if the request occurs before a prePaint event
  * and within this prePaint event a layout shift(s) occurs. Additionally, this font request should
- * happen within the INVALIDATION_WINDOW of the prePaint event.
+ * happen within the ROOT_CAUSE_WINDOW of the prePaint event.
  */
 function getFontRootCauses(
     networkRequests: Types.Events.SyntheticNetworkRequest[], prePaintEvents: Types.Events.PrePaint[],
@@ -338,13 +377,13 @@ function getFontRootCauses(
       networkRequests.filter(req => req.args.data.resourceType === 'Font' && req.args.data.mimeType.startsWith('font'));
 
   for (const req of fontRequests) {
-    const nextPrePaint = getNextPrePaintEvent(prePaintEvents, req);
+    const nextPrePaint = getNextEvent(prePaintEvents, req) as Types.Events.PrePaint | null;
     if (!nextPrePaint) {
       continue;
     }
 
-    // If the req is outside the INVALIDATION_WINDOW, it could not be a root cause.
-    if (!isInInvalidationWindow(req, nextPrePaint)) {
+    // If the req is outside the ROOT_CAUSE_WINDOW, it could not be a root cause.
+    if (!isInRootCauseWindow(req, nextPrePaint)) {
       continue;
     }
 
@@ -374,25 +413,28 @@ export function generateInsight(parsedTrace: RequiredData<typeof deps>, context:
   const iframeEvents = parsedTrace.LayoutShifts.renderFrameImplCreateChildFrameEvents.filter(isWithinContext);
   const networkRequests = parsedTrace.NetworkRequests.byTime.filter(isWithinContext);
   const domLoadingEvents = parsedTrace.LayoutShifts.domLoadingEvents.filter(isWithinContext);
+  const unsizedImageEvents = parsedTrace.LayoutShifts.layoutImageUnsizedEvents.filter(isWithinContext);
 
   const clusterKey = context.navigation ? context.navigationId : Types.Events.NO_NAVIGATION;
   const clusters = parsedTrace.LayoutShifts.clustersByNavigationId.get(clusterKey) ?? [];
   const clustersByScore = clusters.toSorted((a, b) => b.clusterCumulativeScore - a.clusterCumulativeScore);
   const worstCluster = clustersByScore.at(0);
   const layoutShifts = clusters.flatMap(cluster => cluster.events);
   const prePaintEvents = parsedTrace.LayoutShifts.prePaintEvents.filter(isWithinContext);
+  const paintImageEvents = parsedTrace.LayoutShifts.paintImageEvents.filter(isWithinContext);
 
   // Get root causes.
   const rootCausesByShift = new Map<Types.Events.SyntheticLayoutShift, LayoutShiftRootCausesData>();
   const shiftsByPrePaint = getShiftsByPrePaintEvents(layoutShifts, prePaintEvents);
 
   for (const shift of layoutShifts) {
-    rootCausesByShift.set(shift, {iframeIds: [], fontRequests: [], nonCompositedAnimations: []});
+    rootCausesByShift.set(shift, {iframeIds: [], fontRequests: [], nonCompositedAnimations: [], unsizedImages: []});
   }
 
   // Populate root causes for rootCausesByShift.
   getIframeRootCauses(iframeEvents, prePaintEvents, shiftsByPrePaint, rootCausesByShift, domLoadingEvents);
   getFontRootCauses(networkRequests, prePaintEvents, shiftsByPrePaint, rootCausesByShift);
+  getUnsizedImageRootCauses(unsizedImageEvents, paintImageEvents, shiftsByPrePaint, rootCausesByShift);
   const animationFailures =
       getNonCompositedFailureRootCauses(compositeAnimationEvents, prePaintEvents, shiftsByPrePaint, rootCausesByShift);
 

front_end/models/trace/types/TraceEvents.ts

@@ -1193,10 +1193,25 @@ export interface RenderFrameImplCreateChildFrame extends Event {
     frame_token: string,
   };
 }
+
 export function isRenderFrameImplCreateChildFrame(event: Event): event is RenderFrameImplCreateChildFrame {
   return event.name === Name.RENDER_FRAME_IMPL_CREATE_CHILD_FRAME;
 }
 
+export interface LayoutImageUnsized extends Event {
+  name: Name.LAYOUT_IMAGE_UNSIZED;
+  args: Args&{
+    data: {
+      nodeId: Protocol.DOM.BackendNodeId,
+      frameId: string,
+    },
+  };
+}
+
+export function isLayoutImageUnsized(event: Event): event is LayoutImageUnsized {
+  return event.name === Name.LAYOUT_IMAGE_UNSIZED;
+}
+
 export interface PrePaint extends Complete {
   name: 'PrePaint';
 }
@@ -2758,6 +2773,7 @@ export const enum Name {
   HANDLE_POST_MESSAGE = 'HandlePostMessage',
 
   RENDER_FRAME_IMPL_CREATE_CHILD_FRAME = 'RenderFrameImpl::createChildFrame',
+  LAYOUT_IMAGE_UNSIZED = 'LayoutImageUnsized',
 
   DOM_LOADING = 'domLoading',
   BEGIN_REMOTE_FONT_LOAD = 'BeginRemoteFontLoad',

front_end/panels/timeline/components/LayoutShiftDetails.ts

@@ -71,6 +71,10 @@ const UIStrings = {
    * @description Text referring to the total cumulative score of a layout shift cluster.
    */
   total: 'Total',
+  /**
+   * @description Text for a culprit type of Unsized image.
+   */
+  unsizedImage: 'Unsized image',
 };
 
 const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/LayoutShiftDetails.ts', UIStrings);
@@ -184,12 +188,26 @@ export class LayoutShiftDetails extends HTMLElement {
     // clang-format on
   }
 
+  #renderUnsizedImage(node: Protocol.DOM.BackendNodeId): LitHtml.TemplateResult|null {
+    // clang-format off
+    const el = html`
+      <devtools-performance-node-link
+        .data=${{
+          backendNodeId: node,
+        } as Insights.NodeLink.NodeLinkData}>
+      </devtools-performance-node-link>`;
+    return html`
+    <span class="culprit"><span class="culprit-type">${i18nString(UIStrings.unsizedImage)}: </span><span class="culprit-value">${el}</span></span>`;
+    // clang-format on
+  }
+
   #renderRootCauseValues(rootCauses: Trace.Insights.InsightRunners.CumulativeLayoutShift.LayoutShiftRootCausesData|
                          undefined): LitHtml.TemplateResult|null {
     return html`
       ${rootCauses?.fontRequests.map(fontReq => this.#renderFontRequest(fontReq))}
       ${rootCauses?.iframeIds.map(iframe => this.#renderIframe(iframe))}
       ${rootCauses?.nonCompositedAnimations.map(failure => this.#renderAnimation(failure))}
+      ${rootCauses?.unsizedImages.map(image => this.#renderUnsizedImage(image))}
     `;
   }
 
@@ -217,7 +235,8 @@ export class LayoutShiftDetails extends HTMLElement {
     }
     const hasCulprits = Boolean(
         rootCauses &&
-        (rootCauses.fontRequests.length || rootCauses.iframeIds.length || rootCauses.nonCompositedAnimations.length));
+        (rootCauses.fontRequests.length || rootCauses.iframeIds.length || rootCauses.nonCompositedAnimations.length ||
+         rootCauses.unsizedImages.length));
 
     // clang-format off
     return html`
@@ -280,7 +299,8 @@ export class LayoutShiftDetails extends HTMLElement {
     const rootCauses = clsInsight.shifts.get(layoutShift);
     const elementsShifted = layoutShift.args.data?.impacted_nodes ?? [];
     const hasCulprits = rootCauses &&
-        (rootCauses.fontRequests.length || rootCauses.iframeIds.length || rootCauses.nonCompositedAnimations.length);
+        (rootCauses.fontRequests.length || rootCauses.iframeIds.length || rootCauses.nonCompositedAnimations.length ||
+         rootCauses.unsizedImages.length);
     const hasShiftedElements = elementsShifted?.length;
 
     const parentCluster = clsInsight.clusters.find(cluster => {