[RPP] Create row limit and aggregate helper

One strategy I considered was to add a `maxRows` property to the table
component. The overlays for the "N others" row would be aggregated from
the table rows beyond `maxRows`. However there are a couple drawbacks to
this strategy:
- We would be creating lit templates for table rows that don't actually get rendered.
- It isn't possible to fill the second column of the "N others" row with relevant data (e.g. total network payload of the rows not shown)

The strategy taken in this CL resolves the above concerns but is less
DRY.

Bug: None
Change-Id: Id6e4dde29712f92c867d605ec6239b772feabcaa
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6355097
Commit-Queue: Adam Raine <[email protected]>
Reviewed-by: Connor Clark <[email protected]>

Author: Adam Raine

front_end/models/trace/insights/FontDisplay.ts

@@ -33,7 +33,7 @@ export const UIStrings = {
 const str_ = i18n.i18n.registerUIStrings('models/trace/insights/FontDisplay.ts', UIStrings);
 export const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
 
-interface RemoteFont {
+export interface RemoteFont {
   name?: string;
   request: Types.Events.SyntheticNetworkRequest;
   display: string;

front_end/models/trace/insights/UseCache.ts

@@ -182,9 +182,15 @@ export function cachingDisabled(
   return false;
 }
 
+export interface CacheableRequest {
+  request: Types.Events.SyntheticNetworkRequest;
+  ttl: number;
+  wastedBytes: number;
+}
+
 export function generateInsight(
     parsedTrace: Handlers.Types.ParsedTrace, context: InsightSetContext): UseCacheInsightModel {
-  const results = [];
+  const results: CacheableRequest[] = [];
   const allRequests = parsedTrace.NetworkRequests.byTime;
   let totalWastedBytes = 0;
   const wastedBytesByRequestId = new Map<string, number>();

front_end/panels/timeline/components/insights/BUILD.gn

@@ -80,6 +80,7 @@ ts_library("unittests") {
   sources = [
     "BaseInsightComponent.test.ts",
     "CLSCulprits.test.ts",
+    "Table.test.ts",
   ]
 
   deps = [

front_end/panels/timeline/components/insights/FontDisplay.ts

@@ -12,7 +12,7 @@ import type * as Overlays from '../../overlays/overlays.js';
 
 import {BaseInsightComponent} from './BaseInsightComponent.js';
 import {eventRef} from './EventRef.js';
-import type {TableData} from './Table.js';
+import {createLimitedRows, renderOthersLabel, type TableData, type TableDataRow} from './Table.js';
 
 const {UIStrings, i18nString} = Trace.Insights.Models.FontDisplay;
 
@@ -22,6 +22,25 @@ export class FontDisplay extends BaseInsightComponent<FontDisplayInsightModel> {
   static override readonly litTagName = Lit.StaticHtml.literal`devtools-performance-font-display`;
   override internalName = 'font-display';
 
+  mapToRow(font: Trace.Insights.Models.FontDisplay.RemoteFont): TableDataRow {
+    const overlay = this.#overlayForRequest.get(font.request);
+    return {
+      values: [
+        eventRef(font.request, {text: font.name}),
+        i18n.TimeUtilities.millisToString(font.wastedTime),
+      ],
+      overlays: overlay ? [overlay] : [],
+    };
+  }
+
+  createAggregatedTableRow(remaining: Trace.Insights.Models.FontDisplay.RemoteFont[]): TableDataRow {
+    return {
+      values: [renderOthersLabel(remaining.length), ''],
+      overlays: remaining.map(r => this.#overlayForRequest.get(r.request))
+                    .filter((o): o is Overlays.Overlays.TimelineOverlay => Boolean(o)),
+    };
+  }
+
   #overlayForRequest = new Map<Trace.Types.Events.SyntheticNetworkRequest, Overlays.Overlays.TimelineOverlay>();
 
   override createOverlays(): Overlays.Overlays.TimelineOverlay[] {
@@ -51,20 +70,16 @@ export class FontDisplay extends BaseInsightComponent<FontDisplayInsightModel> {
       return Lit.nothing;
     }
 
+    const rows = createLimitedRows(this.model.fonts, this);
+
     // clang-format off
     return html`
       <div class="insight-section">
         ${html`<devtools-performance-table
           .data=${{
             insight: this,
             headers: [i18nString(UIStrings.fontColumn), i18nString(UIStrings.wastedTimeColumn)],
-            rows: this.model.fonts.map(font => ({
-              values: [
-                eventRef(font.request, {text: font.name}),
-                i18n.TimeUtilities.millisToString(font.wastedTime),
-              ],
-              overlays: [this.#overlayForRequest.get(font.request)],
-            })),
+            rows,
           } as TableData}>
         </devtools-performance-table>`}
       </div>`;

front_end/panels/timeline/components/insights/ForcedReflow.ts

@@ -14,14 +14,29 @@ import * as Lit from '../../../../ui/lit/lit.js';
 import type * as Overlays from '../../overlays/overlays.js';
 
 import {BaseInsightComponent} from './BaseInsightComponent.js';
-import type {TableData} from './Table.js';
+import {createLimitedRows, renderOthersLabel, type TableData, type TableDataRow} from './Table.js';
 
 const {UIStrings, i18nString} = Trace.Insights.Models.ForcedReflow;
 
 const {html, nothing} = Lit;
 
 export class ForcedReflow extends BaseInsightComponent<ForcedReflowInsightModel> {
   static override readonly litTagName = Lit.StaticHtml.literal`devtools-performance-forced-reflow`;
+
+  mapToRow(data: Trace.Insights.Models.ForcedReflow.BottomUpCallStack): TableDataRow {
+    return {
+      values: [this.#linkifyUrl(data.bottomUpData)],
+      overlays: this.#createOverlayForEvents(data.relatedEvents),
+    };
+  }
+
+  createAggregatedTableRow(remaining: Trace.Insights.Models.ForcedReflow.BottomUpCallStack[]): TableDataRow {
+    return {
+      values: [renderOthersLabel(remaining.length)],
+      overlays: remaining.flatMap(r => this.#createOverlayForEvents(r.relatedEvents)),
+    };
+  }
+
   override internalName = 'forced-reflow';
 
   #linkifyUrl(callFrame: Trace.Types.Events.CallFrame|Protocol.Runtime.CallFrame|null): Lit.LitTemplate {
@@ -57,6 +72,9 @@ export class ForcedReflow extends BaseInsightComponent<ForcedReflowInsightModel>
     const bottomUpCallStackData = this.model.aggregatedBottomUpData;
     const time = (us: Trace.Types.Timing.Micro): string =>
         i18n.TimeUtilities.millisToString(Platform.Timing.microSecondsToMilliSeconds(us));
+
+    const rows = createLimitedRows(bottomUpCallStackData, this);
+
     // clang-format off
     return html`
       ${topLevelFunctionCallData ? html`
@@ -81,10 +99,7 @@ export class ForcedReflow extends BaseInsightComponent<ForcedReflowInsightModel>
           .data=${{
             insight: this,
             headers: [i18nString(UIStrings.relatedStackTrace)],
-            rows: bottomUpCallStackData.map(data => ({
-              values: [this.#linkifyUrl(data.bottomUpData)],
-              overlays: this.#createOverlayForEvents(data.relatedEvents),
-            })),
+            rows,
         } as TableData}>
         </devtools-performance-table>
       </div>`;

front_end/panels/timeline/components/insights/ImageDelivery.ts

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import '../../../../ui/components/icon_button/icon_button.js';
-import './Table.js';
 
 import type {ImageDeliveryInsightModel} from '../../../../models/trace/insights/ImageDelivery.js';
 import * as Trace from '../../../../models/trace/trace.js';
@@ -12,14 +11,12 @@ import type * as Overlays from '../../overlays/overlays.js';
 
 import {BaseInsightComponent} from './BaseInsightComponent.js';
 import {imageRef} from './EventRef.js';
-import type {TableDataRow} from './Table.js';
+import {createLimitedRows, renderOthersLabel, type TableDataRow} from './Table.js';
 
 const {UIStrings, i18nString} = Trace.Insights.Models.ImageDelivery;
 
 const {html} = Lit;
 
-const MAX_REQUESTS = 10;
-
 export class ImageDelivery extends BaseInsightComponent<ImageDeliveryInsightModel> {
   static override readonly litTagName = Lit.StaticHtml.literal`devtools-performance-image-delivery`;
   override internalName = 'image-delivery';
@@ -41,6 +38,20 @@ export class ImageDelivery extends BaseInsightComponent<ImageDeliveryInsightMode
     };
   }
 
+  mapToRow(image: Trace.Insights.Models.ImageDelivery.OptimizableImage): TableDataRow {
+    return {
+      values: [imageRef(image.request)],
+      overlays: [this.#createOverlayForRequest(image.request)],
+    };
+  }
+
+  createAggregatedTableRow(remaining: Trace.Insights.Models.ImageDelivery.OptimizableImage[]): TableDataRow {
+    return {
+      values: [renderOthersLabel(remaining.length)],
+      overlays: remaining.map(r => this.#createOverlayForRequest(r.request)),
+    };
+  }
+
   override getEstimatedSavingsBytes(): number|null {
     return this.model?.totalByteSavings ?? null;
   }
@@ -55,20 +66,7 @@ export class ImageDelivery extends BaseInsightComponent<ImageDeliveryInsightMode
     const topImages =
         optimizableImages.sort((a, b) => b.request.args.data.decodedBodyLength - a.request.args.data.decodedBodyLength);
 
-    const remaining = topImages.splice(MAX_REQUESTS);
-    const rows: TableDataRow[] = topImages.map(image => ({
-                                                 values: [imageRef(image.request)],
-                                                 overlays: [this.#createOverlayForRequest(image.request)],
-                                               }));
-
-    if (remaining.length > 0) {
-      const value =
-          remaining.length > 1 ? i18nString(UIStrings.others, {PH1: remaining.length}) : imageRef(remaining[0].request);
-      rows.push({
-        values: [value],
-        overlays: remaining.map(r => this.#createOverlayForRequest(r.request)),
-      });
-    }
+    const rows = createLimitedRows(topImages, this);
 
     if (!rows.length) {
       return html`<div class="insight-section">${i18nString(UIStrings.noOptimizableImages)}</div>`;

front_end/panels/timeline/components/insights/RenderBlocking.ts

@@ -2,24 +2,40 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import './Table.js';
-
 import * as i18n from '../../../../core/i18n/i18n.js';
-import * as Platform from '../../../../core/platform/platform.js';
 import type {RenderBlockingInsightModel} from '../../../../models/trace/insights/RenderBlocking.js';
 import * as Trace from '../../../../models/trace/trace.js';
 import * as Lit from '../../../../ui/lit/lit.js';
 import type * as Overlays from '../../overlays/overlays.js';
 
 import {BaseInsightComponent} from './BaseInsightComponent.js';
 import {eventRef} from './EventRef.js';
+import {createLimitedRows, renderOthersLabel, type TableDataRow} from './Table.js';
 
 const {UIStrings, i18nString} = Trace.Insights.Models.RenderBlocking;
 
 const {html} = Lit;
 
 export class RenderBlocking extends BaseInsightComponent<RenderBlockingInsightModel> {
   static override readonly litTagName = Lit.StaticHtml.literal`devtools-performance-render-blocking-requests`;
+
+  mapToRow(request: Trace.Types.Events.SyntheticNetworkRequest): TableDataRow {
+    return {
+      values: [
+        eventRef(request),
+        i18n.TimeUtilities.formatMicroSecondsTime(request.dur),
+      ],
+      overlays: [this.#createOverlayForRequest(request)],
+    };
+  }
+
+  createAggregatedTableRow(remaining: Trace.Types.Events.SyntheticNetworkRequest[]): TableDataRow {
+    return {
+      values: [renderOthersLabel(remaining.length), ''],
+      overlays: remaining.map(r => this.#createOverlayForRequest(r)),
+    };
+  }
+
   protected override hasAskAISupport = true;
   override internalName = 'render-blocking-requests';
 
@@ -48,27 +64,21 @@ export class RenderBlocking extends BaseInsightComponent<RenderBlockingInsightMo
       return Lit.nothing;
     }
 
-    const MAX_REQUESTS = 3;
-    const topRequests = this.model.renderBlockingRequests.slice(0, MAX_REQUESTS);
-
-    if (!topRequests.length) {
+    const requests = this.model.renderBlockingRequests;
+    if (!requests.length) {
       return html`<div class="insight-section">${i18nString(UIStrings.noRenderBlocking)}</div>`;
     }
 
+    const rows = createLimitedRows(requests, this);
+
     // clang-format off
     return html`
       <div class="insight-section">
         <devtools-performance-table
           .data=${{
             insight: this,
             headers: [i18nString(UIStrings.renderBlockingRequest), i18nString(UIStrings.duration)],
-            rows: topRequests.map(request => ({
-              values: [
-                eventRef(request),
-                i18n.TimeUtilities.millisToString(Platform.Timing.microSecondsToMilliSeconds(request.dur)),
-              ],
-              overlays: [this.#createOverlayForRequest(request)],
-            })),
+            rows,
           }}>
         </devtools-performance-table>
       </div>

front_end/panels/timeline/components/insights/Table.test.ts

@@ -0,0 +1,60 @@
+// Copyright 2025 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import * as Insights from './insights.js';
+import type {RowLimitAggregator} from './Table.js';
+
+const {createLimitedRows} = Insights.Table;
+
+const {assert} = chai;
+
+describe('rowLimitAggregate', () => {
+  const aggregator: RowLimitAggregator<number> = {
+    mapToRow: num => ({values: [num]}),
+    createAggregatedTableRow: others => ({values: [`${others.length} others`]}),
+  };
+
+  it('handles empty array', () => {
+    const rows = createLimitedRows([], aggregator);
+    assert.deepEqual(rows, []);
+  });
+
+  it('respects limit 0', () => {
+    const rows = createLimitedRows([1, 2], aggregator, 0);
+    assert.deepEqual(rows, []);
+  });
+
+  it('respects limit 1', () => {
+    const rows = createLimitedRows([1, 2, 3], aggregator, 1);
+    assert.deepEqual(rows, [
+      {values: ['3 others']},
+    ]);
+  });
+
+  it('correctly limits rows', () => {
+    const rows = createLimitedRows([1, 2, 3, 4, 5], aggregator, 3);
+    assert.deepEqual(rows, [
+      {values: [1]},
+      {values: [2]},
+      {values: ['3 others']},
+    ]);
+  });
+
+  it('does not aggregate if input shorter than the limit', () => {
+    const rows = createLimitedRows([1, 2], aggregator, 3);
+    assert.deepEqual(rows, [
+      {values: [1]},
+      {values: [2]},
+    ]);
+  });
+
+  it('does not aggregate if input is exactly at the limit', () => {
+    const rows = createLimitedRows([1, 2, 3], aggregator, 3);
+    assert.deepEqual(rows, [
+      {values: [1]},
+      {values: [2]},
+      {values: [3]},
+    ]);
+  });
+});