RPP: Fix 3P table scrolling, plus Summary tab polish

- Scrolling was mostly buggy due to a hardcoded row height in DataGrid (20), whereas our rows were not. It's dynamic now and doesn't force a layout thrash either.
- All of TimelineTreeView styles have been in timelinePanel.css, but due to shadow DOM, the 3P table wasn't using them. This lead to some unpredictability. Now, 3PTTV extends the component _and_ its styles.
  - This included moving 3P table styles from timelineSummary.css to timelinePanel.css. That said, I've adjusted 60% of those styles, too.
- Reduced DOM complexity of the summary: fewer containers, removed use of slots. Also removed a div within .entity-badge
- A few renames to help distinguish Details (the pane with tabs), Summary (the first tab which is often used to show the _details_ of a trace event), the Category Summary (the Scripting/Rendering/etc numbers), and Range Summary (a container holding the Category Summary and 3P table, side by side)
- Pixel-perfect vertical rhythm, matching mocks: https://screenshot.googleplex.com/C2Pkr5anVgU3F5o (That was fun :)
- Changed how layout of the bottom-up button works, to get more predictable layout behavior.

Change-Id: Ia92a4a78e17813e1ce9c64d313e2a978f9aaf7d5
Bug: 388458798
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6221821
Reviewed-by: Jack Franklin <[email protected]>
Reviewed-by: Adriana Ixba <[email protected]>
Commit-Queue: Jack Franklin <[email protected]>

Author: paulirish

front_end/panels/timeline/ThirdPartyTreeView.test.ts

@@ -50,7 +50,7 @@ describeWithEnvironment('TimelineTreeView', function() {
           firstNode, firstNode.totalTime, firstNode.selfTime, firstNode.totalTime, treeView);
       const extEntity = extensionNode?.createCell('site');
 
-      let gotBadgeName = extEntity.querySelector<HTMLTableRowElement>('.entity-badge-name')?.textContent || '';
+      let gotBadgeName = extEntity.querySelector<HTMLTableRowElement>('.entity-badge')?.textContent || '';
       assert.strictEqual(gotBadgeName, 'Extension');
 
       // Node with first party
@@ -61,7 +61,7 @@ describeWithEnvironment('TimelineTreeView', function() {
           secondNode, secondNode.totalTime, secondNode.selfTime, secondNode.totalTime, treeView);
       const firstPartyEntity = firstPartyNode?.createCell('site');
 
-      gotBadgeName = firstPartyEntity.querySelector<HTMLTableRowElement>('.entity-badge-name')?.textContent || '';
+      gotBadgeName = firstPartyEntity.querySelector<HTMLTableRowElement>('.entity-badge')?.textContent || '';
       assert.strictEqual(gotBadgeName, '1st party');
     });
   });

front_end/panels/timeline/ThirdPartyTreeView.ts

@@ -49,6 +49,7 @@ export class ThirdPartyTreeViewWidget extends TimelineTreeView.TimelineTreeView
     this.element.setAttribute('jslog', `${VisualLogging.pane('third-party-tree').track({hover: true})}`);
     this.init();
     this.dataGrid.markColumnAsSortedBy('self', DataGrid.DataGrid.Order.Descending);
+    this.dataGrid.setResizeMethod(DataGrid.DataGrid.ResizeMethod.NEAREST);
     /**
      * By default data grids always expand when arrowing.
      * For 3P table, we don't use this feature.
@@ -130,21 +131,23 @@ export class ThirdPartyTreeViewWidget extends TimelineTreeView.TimelineTreeView
         {
           id: 'site',
           title: i18nString(UIStrings.firstOrThirdPartyName),
-          width: '100px',
-          fixedWidth: true,
+          // It's important that this width is the `.widget.vbox.timeline-tree-view` max-width (550)
+          // minus the two fixed sizes below. (550-100-105) == 345
+          width: '345px',
+          // And with this column not-fixed-width and resizingMethod NEAREST, the name-column will appropriately flex.
           sortable: true,
         },
         {
           id: 'transfer-size',
           title: i18nString(UIStrings.transferSize),
-          width: '80px',
+          width: '100px',  // Mostly so there's room for the header plus sorting triangle
           fixedWidth: true,
           sortable: true,
         },
         {
           id: 'self',
           title: i18nString(UIStrings.selfTime),
-          width: '80px',
+          width: '105px',  // Mostly to fit large self-time plus devtools-button
           fixedWidth: true,
           sortable: true,
         });
@@ -257,7 +260,7 @@ export class ThirdPartyTreeViewWidget extends TimelineTreeView.TimelineTreeView
   }
 }
 
-export class ThirdPartyTreeView extends UI.Widget.WidgetElement<UI.Widget.Widget> {
+export class ThirdPartyTreeElement extends UI.Widget.WidgetElement<UI.Widget.Widget> {
   #treeView?: ThirdPartyTreeViewWidget;
 
   set treeView(treeView: ThirdPartyTreeViewWidget) {
@@ -279,10 +282,10 @@ export class ThirdPartyTreeView extends UI.Widget.WidgetElement<UI.Widget.Widget
   }
 }
 
-customElements.define('devtools-performance-third-party-tree-view', ThirdPartyTreeView);
+customElements.define('devtools-performance-third-party-tree-view', ThirdPartyTreeElement);
 
 declare global {
   interface HTMLElementTagNameMap {
-    'devtools-performance-third-party-tree-view': ThirdPartyTreeView;
+    'devtools-performance-third-party-tree-view': ThirdPartyTreeElement;
   }
 }

front_end/panels/timeline/TimelineDetailsView.test.ts

@@ -26,7 +26,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
 
   it('displays the details of a network request event correctly', async function() {
     const {parsedTrace, insights} = await TraceLoader.traceEngine(this, 'lcp-web-font.json.gz');
-    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
+    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
 
     const networkRequests = parsedTrace.NetworkRequests.byTime;
     const cssRequest = networkRequests.find(request => {
@@ -53,7 +53,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
 
   it('displays the details for a frame correctly', async function() {
     const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
-    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
+    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
     await detailsView.setModel({
       parsedTrace,
       selectedEvents: null,
@@ -79,7 +79,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
 
   it('renders the layout shift component for a single layout shift', async function() {
     const {parsedTrace} = await TraceLoader.traceEngine(this, 'shift-attribution.json.gz');
-    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
+    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
     await detailsView.setModel({
       parsedTrace,
       selectedEvents: null,
@@ -101,7 +101,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
 
   it('renders the layout shift component for a selected cluster', async function() {
     const {parsedTrace} = await TraceLoader.traceEngine(this, 'shift-attribution.json.gz');
-    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
+    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
     await detailsView.setModel({
       parsedTrace,
       selectedEvents: null,
@@ -123,7 +123,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
 
   it('updates the range details when the user has a range selected', async function() {
     const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
-    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
+    const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
     await detailsView.setModel({
       parsedTrace,
       // We have to set selected events for the range selection UI to be drawn

front_end/panels/timeline/TimelineDetailsView.ts

@@ -63,7 +63,7 @@ const UIStrings = {
 };
 const str_ = i18n.i18n.registerUIStrings('panels/timeline/TimelineDetailsView.ts', UIStrings);
 const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
-export class TimelineDetailsView extends
+export class TimelineDetailsPane extends
     Common.ObjectWrapper.eventMixin<TimelineTreeView.EventTypes, typeof UI.Widget.VBox>(UI.Widget.VBox) {
   private readonly detailsLinkifier: Components.Linkifier.Linkifier;
   private tabbedPane: UI.TabbedPane.TabbedPane;
@@ -260,7 +260,7 @@ export class TimelineDetailsView extends
     await this.setSelection(null);
   }
 
-  private setContent(node: Node): void {
+  private setSummaryContent(node: Node): void {
     const allTabs = this.tabbedPane.otherTabs(Tab.Details);
     for (let i = 0; i < allTabs.length; ++i) {
       if (!this.rangeDetailViews.has(allTabs[i])) {
@@ -269,7 +269,7 @@ export class TimelineDetailsView extends
     }
     this.defaultDetailsContentWidget.detach();
     this.defaultDetailsContentWidget = this.#createContentWidget();
-    this.defaultDetailsContentWidget.contentElement.appendChild(node);
+    this.defaultDetailsContentWidget.contentElement.append(node);
     if (this.#relatedInsightChips) {
       this.defaultDetailsContentWidget.contentElement.appendChild(this.#relatedInsightChips);
     }
@@ -318,7 +318,7 @@ export class TimelineDetailsView extends
    */
   private scheduleUpdateContentsFromWindow(forceImmediateUpdate: boolean = false): void {
     if (!this.#parsedTrace) {
-      this.setContent(UI.Fragment.html`<div/>`);
+      this.setSummaryContent(UI.Fragment.html`<div/>`);
       return;
     }
     if (forceImmediateUpdate) {
@@ -361,7 +361,8 @@ export class TimelineDetailsView extends
 
   #setSelectionForTimelineFrame(frame: Trace.Types.Events.LegacyTimelineFrame): void {
     const matchedFilmStripFrame = this.#getFilmStripFrame(frame);
-    this.setContent(TimelineUIUtils.generateDetailsContentForFrame(frame, this.#filmStrip, matchedFilmStripFrame));
+    this.setSummaryContent(
+        TimelineUIUtils.generateDetailsContentForFrame(frame, this.#filmStrip, matchedFilmStripFrame));
     const target = SDK.TargetManager.TargetManager.instance().rootTarget();
     if (frame.layerTree && target) {
       const layerTreeForFrame = new TimelineModel.TracingLayerTree.TracingFrameLayerTree(target, frame.layerTree);
@@ -384,7 +385,7 @@ export class TimelineDetailsView extends
       this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
     }
 
-    this.setContent(this.#networkRequestDetails);
+    this.setSummaryContent(this.#networkRequestDetails);
   }
 
   async #setSelectionForTraceEvent(event: Trace.Types.Events.Event): Promise<void> {
@@ -402,7 +403,7 @@ export class TimelineDetailsView extends
     if (Trace.Types.Events.isSyntheticLayoutShift(event) || Trace.Types.Events.isSyntheticLayoutShiftCluster(event)) {
       const isFreshRecording = Boolean(this.#parsedTrace && Tracker.instance().recordingIsFresh(this.#parsedTrace));
       this.#layoutShiftDetails.setData(event, this.#traceInsightsSets, this.#parsedTrace, isFreshRecording);
-      this.setContent(this.#layoutShiftDetails);
+      this.setSummaryContent(this.#layoutShiftDetails);
       return;
     }
 
@@ -497,7 +498,7 @@ export class TimelineDetailsView extends
   }
 
   private appendDetailsTabsForTraceEventAndShowDetails(event: Trace.Types.Events.Event, content: Node): void {
-    this.setContent(content);
+    this.setSummaryContent(content);
     if (Trace.Types.Events.isPaint(event) || Trace.Types.Events.isRasterTask(event)) {
       this.showEventInPaintProfiler(event);
     }
@@ -536,9 +537,10 @@ export class TimelineDetailsView extends
     const aggregatedStats = TimelineUIUtils.statsForTimeRange(this.#selectedEvents, startTime, endTime);
     const startOffset = startTime - minBoundsMilli;
     const endOffset = endTime - minBoundsMilli;
-    const summaryDetails = TimelineUIUtils.generateSummaryDetails(
+    const summaryDetailElem = TimelineUIUtils.generateSummaryDetails(
         aggregatedStats, startOffset, endOffset, this.#selectedEvents, this.#thirdPartyTree);
-    this.setContent(summaryDetails);
+
+    this.setSummaryContent(summaryDetailElem);
 
     // Find all recalculate style events data from range
     const isSelectorStatsEnabled =

front_end/panels/timeline/TimelineFlameChartView.ts

@@ -23,7 +23,7 @@ import {ModificationsManager} from './ModificationsManager.js';
 import * as OverlayComponents from './overlays/components/components.js';
 import * as Overlays from './overlays/overlays.js';
 import {targetForEvent} from './TargetForEvent.js';
-import {TimelineDetailsView} from './TimelineDetailsView.js';
+import {TimelineDetailsPane} from './TimelineDetailsView.js';
 import {TimelineRegExp} from './TimelineFilters.js';
 import {
   Events as TimelineFlameChartDataProviderEvents,
@@ -107,7 +107,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
   private brickGame?: PerfUI.BrickBreaker.BrickBreaker;
   private readonly countersView: CountersGraph;
   private readonly detailsSplitWidget: UI.SplitWidget.SplitWidget;
-  private readonly detailsView: TimelineDetailsView;
+  private readonly detailsView: TimelineDetailsPane;
   private readonly onMainAddEntryLabelAnnotation: (event: Common.EventTarget.EventTargetEvent<{
     entryIndex: number,
     withLinkCreationButton: boolean,
@@ -346,7 +346,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
     // Create top level properties splitter.
     this.detailsSplitWidget = new UI.SplitWidget.SplitWidget(false, true, 'timeline-panel-details-split-view-state');
     this.detailsSplitWidget.element.classList.add('timeline-details-split');
-    this.detailsView = new TimelineDetailsView(delegate);
+    this.detailsView = new TimelineDetailsPane(delegate);
     this.detailsSplitWidget.installResizer(this.detailsView.headerElement());
     this.detailsSplitWidget.setMainWidget(this.chartSplitWidget);
     this.detailsSplitWidget.setSidebarWidget(this.detailsView);

front_end/panels/timeline/TimelineTreeView.ts

@@ -735,8 +735,8 @@ export class GridNode extends DataGrid.SortableDataGrid.SortableDataGridNode<Gri
 
         if (badgeText) {
           const badge = container.createChild('div', 'entity-badge');
+          badge.textContent = badgeText;
           UI.ARIAUtils.setLabel(badge, badgeText);
-          badge.createChild('div', 'entity-badge-name').textContent = badgeText;
         }
       }
     } else if (event) {
@@ -817,23 +817,19 @@ export class GridNode extends DataGrid.SortableDataGrid.SortableDataGridNode<Gri
           i18nString(UIStrings.percentPlaceholder, {PH1: (value / this.grandTotalTime * 100).toFixed(1)});
     }
     if (maxTime) {
-      textDiv.classList.add('background-percent-bar');
+      textDiv.classList.add('background-bar-text');
       cell.createChild('div', 'background-bar-container').createChild('div', 'background-bar').style.width =
           (value * 100 / maxTime).toFixed(1) + '%';
     }
     // Generate button on hover for 3P self time cell.
     if (showBottomUpButton) {
-      this.generateBottomUpButton(cell);
+      this.generateBottomUpButton(textDiv);
     }
     return cell;
   }
 
   // Generates bottom up tree hover button and appends it to the provided cell element.
-  private generateBottomUpButton(cell: HTMLElement): void {
-    const buttonContainer = document.createElement('div');
-    buttonContainer.className = 'button-container';
-    cell.classList.add('hover-bottom-up-button');
-
+  private generateBottomUpButton(textDiv: HTMLElement): void {
     const button = new Buttons.Button.Button();
     button.data = {
       variant: Buttons.Button.Variant.ICON,
@@ -843,11 +839,10 @@ export class GridNode extends DataGrid.SortableDataGrid.SortableDataGridNode<Gri
     };
     UI.ARIAUtils.setLabel(button, i18nString(UIStrings.viewBottomUp));
     button.addEventListener('click', () => this.#bottomUpButtonClicked());
-    buttonContainer.appendChild(button);
     UI.Tooltip.Tooltip.install(button, i18nString(UIStrings.bottomUp));
 
     // Append the button to the last column
-    cell.appendChild(buttonContainer);
+    textDiv.appendChild(button);
   }
 
   #bottomUpButtonClicked(): void {

front_end/panels/timeline/TimelineUIUtils.ts

@@ -2187,17 +2187,17 @@ export class TimelineUIUtils {
       aggregatedStats: Record<string, number>, rangeStart: number, rangeEnd: number,
       selectedEvents: Trace.Types.Events.Event[],
       thirdPartyTree: ThirdPartyTreeView.ThirdPartyTreeViewWidget): Element {
+    const element = document.createElement('div');
+    element.classList.add('timeline-details-range-summary', 'hbox');
+
+    // First, the category bar chart.
     let total = 0;
+    let categories: TimelineComponents.TimelineSummary.CategoryData[] = [];
     // Calculate total of all categories.
     for (const categoryName in aggregatedStats) {
       total += aggregatedStats[categoryName];
     }
 
-    const element = document.createElement('div');
-    element.classList.add('timeline-details-view-summary');
-
-    let categories: TimelineComponents.TimelineSummary.CategoryData[] = [];
-
     // Get stats values from categories.
     for (const categoryName in Utils.EntryStyles.getCategoryStyles()) {
       const category = Utils.EntryStyles.getCategoryStyles()[categoryName as keyof Utils.EntryStyles.CategoryPalette];
@@ -2217,36 +2217,24 @@ export class TimelineUIUtils {
     categories = categories.sort((a, b) => b.value - a.value);
     const start = Trace.Types.Timing.Milli(rangeStart);
     const end = Trace.Types.Timing.Milli(rangeEnd);
-    const summaryTable = new TimelineComponents.TimelineSummary.TimelineSummary();
-    summaryTable.data = {
+    const categorySummaryTable = new TimelineComponents.TimelineSummary.CategorySummary();
+    categorySummaryTable.data = {
       rangeStart: start,
       rangeEnd: end,
       total,
       categories,
       selectedEvents,
     };
-    const summaryTableContainer = element.createChild('div');
-    summaryTableContainer.appendChild(summaryTable);
+    element.append(categorySummaryTable);
 
-    if (!Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_THIRD_PARTY_DEPENDENCIES)) {
-      return element;
+    if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_THIRD_PARTY_DEPENDENCIES)) {
+      // Add the 3p datagrid
+      const treeView = new ThirdPartyTreeView.ThirdPartyTreeElement();
+      treeView.treeView = thirdPartyTree;
+      UI.ARIAUtils.setLabel(treeView, i18nString(UIStrings.thirdPartyTable));
+      element.append(treeView);
     }
 
-    const treeView = new ThirdPartyTreeView.ThirdPartyTreeView();
-    treeView.treeView = thirdPartyTree;
-    const treeSlot = document.createElement('slot');
-
-    const thirdPartyDiv = document.createElement('div');
-    thirdPartyDiv.className = 'third-party-table';
-    UI.ARIAUtils.setLabel(thirdPartyDiv, i18nString(UIStrings.thirdPartyTable));
-
-    treeSlot.name = 'third-party-table';
-    treeSlot.append(treeView);
-
-    thirdPartyDiv.appendChild(treeSlot);
-    if (summaryTable.shadowRoot) {
-      summaryTable.shadowRoot?.appendChild(thirdPartyDiv);
-    }
     return element;
   }
 

front_end/panels/timeline/components/TimelineSummary.test.ts

@@ -16,7 +16,7 @@ describeWithMockConnection('TimelineSummary', () => {
       {title: 'Rendering', value: 0, color: 'black'},
     ];
 
-    const summary = new TimelineComponents.TimelineSummary.TimelineSummary();
+    const summary = new TimelineComponents.TimelineSummary.CategorySummary();
     summary.data = {
       rangeStart: 0,
       rangeEnd: 110,
@@ -45,7 +45,7 @@ describeWithMockConnection('TimelineSummary', () => {
   it('no categories should just render Total', async function() {
     const categories: TimelineComponents.TimelineSummary.CategoryData[] = [];
 
-    const summary = new TimelineComponents.TimelineSummary.TimelineSummary();
+    const summary = new TimelineComponents.TimelineSummary.CategorySummary();
     summary.data = {
       rangeStart: 0,
       rangeEnd: 110,