Skip to content

Commit 7cbc660

Browse files
Adriana IxbaDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Bring back DCL and L markers
https://screenshot.googleplex.com/6zv3Yrvgtz3Sf2f Bug:384980553 Change-Id: If078e81d37a047f2375df3dc87714ee87531ae4a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6107674 Commit-Queue: Adriana Ixba <[email protected]> Reviewed-by: Adam Raine <[email protected]>
1 parent 378550e commit 7cbc660

File tree

4 files changed

+47
-20
lines changed

4 files changed

+47
-20
lines changed

front_end/panels/timeline/TimelineFlameChartView.ts

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,20 @@ const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
5757
* This defines the order these markers will be rendered if they are at the
5858
* same timestamp. The smaller number will be shown first - e.g. so if NavigationStart, MarkFCP,
5959
* MarkLCPCandidate have the same timestamp, visually we
60-
* will render [Nav][FCP][LCP] everytime.
60+
* will render [Nav][FCP][DCL][LCP] everytime.
6161
*/
6262
export const SORT_ORDER_PAGE_LOAD_MARKERS: Readonly<Record<string, number>> = {
6363
[Trace.Types.Events.Name.NAVIGATION_START]: 0,
64-
[Trace.Types.Events.Name.MARK_FCP]: 1,
65-
[Trace.Types.Events.Name.MARK_LCP_CANDIDATE]: 2,
64+
[Trace.Types.Events.Name.MARK_LOAD]: 1,
65+
[Trace.Types.Events.Name.MARK_FCP]: 2,
66+
[Trace.Types.Events.Name.MARK_DOM_CONTENT]: 3,
67+
[Trace.Types.Events.Name.MARK_LCP_CANDIDATE]: 4,
6668
};
6769

70+
// Threshold to match up overlay markers that are off by a tiny amount so they aren't rendered
71+
// on top of each other.
72+
const TIMESTAMP_THRESHOLD_MS = Trace.Types.Timing.MicroSeconds(10);
73+
6874
export class TimelineFlameChartView extends
6975
Common.ObjectWrapper.eventMixin<TimelineTreeView.EventTypes, typeof UI.Widget.VBox>(UI.Widget.VBox)
7076
implements PerfUI.FlameChart.FlameChartDelegate, UI.SearchableView.Searchable {
@@ -466,33 +472,42 @@ export class TimelineFlameChartView extends
466472
// Clear out any markers.
467473
this.bulkRemoveOverlays(this.#markers);
468474
const markerEvents = parsedTrace.PageLoadMetrics.allMarkerEvents;
469-
// Set markers for Navigations, LCP, FCP.
475+
// Set markers for Navigations, LCP, FCP, DCL, L.
470476
const markers = markerEvents.filter(
471477
event => event.name === Trace.Types.Events.Name.NAVIGATION_START ||
472478
event.name === Trace.Types.Events.Name.MARK_LCP_CANDIDATE ||
473-
event.name === Trace.Types.Events.Name.MARK_FCP);
479+
event.name === Trace.Types.Events.Name.MARK_FCP ||
480+
event.name === Trace.Types.Events.Name.MARK_DOM_CONTENT ||
481+
event.name === Trace.Types.Events.Name.MARK_LOAD);
474482

475483
this.#sortMarkersForPreferredVisualOrder(markers);
476-
const markerOverlays: Overlays.Overlays.TimingsMarker[] = [];
477-
markers.forEach((marker, i) => {
484+
const overlayByTs = new Map<Trace.Types.Timing.MicroSeconds, Overlays.Overlays.TimingsMarker>();
485+
markers.forEach(marker => {
478486
const adjustedTimestamp = Trace.Helpers.Timing.timeStampForEventAdjustedByClosestNavigation(
479487
marker,
480488
parsedTrace.Meta.traceBounds,
481489
parsedTrace.Meta.navigationsByNavigationId,
482490
parsedTrace.Meta.navigationsByFrameId,
483491
);
484492
// If any of the markers overlap in timing, lets put them on the same marker.
485-
if (i > 0 && marker.ts === markerOverlays[markerOverlays.length - 1].entries[0].ts) {
486-
markerOverlays[markerOverlays.length - 1].entries.push(marker);
487-
return;
493+
let matchingOverlay = false;
494+
for (const [ts, overlay] of overlayByTs.entries()) {
495+
if (Math.abs(marker.ts - ts) <= TIMESTAMP_THRESHOLD_MS) {
496+
overlay.entries.push(marker);
497+
matchingOverlay = true;
498+
break;
499+
}
500+
}
501+
if (!matchingOverlay) {
502+
const overlay = {
503+
type: 'TIMINGS_MARKER',
504+
entries: [marker],
505+
adjustedTimestamp,
506+
} as Overlays.Overlays.TimingsMarker;
507+
overlayByTs.set(marker.ts, overlay);
488508
}
489-
const overlay = {
490-
type: 'TIMINGS_MARKER',
491-
entries: [marker],
492-
adjustedTimestamp,
493-
} as Overlays.Overlays.TimingsMarker;
494-
markerOverlays.push(overlay);
495509
});
510+
const markerOverlays: Overlays.Overlays.TimingsMarker[] = [...overlayByTs.values()];
496511
this.#markers = markerOverlays;
497512
if (this.#markers.length === 0) {
498513
return;

front_end/panels/timeline/TimelineUIUtils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2403,11 +2403,11 @@ export class TimelineUIUtils {
24032403
tall = true;
24042404
break;
24052405
case Trace.Types.Events.Name.MARK_DOM_CONTENT:
2406-
color = '#0867CB';
2406+
color = 'var(--color-text-disabled)';
24072407
tall = true;
24082408
break;
24092409
case Trace.Types.Events.Name.MARK_LOAD:
2410-
color = '#B31412';
2410+
color = 'var(--color-text-disabled)';
24112411
tall = true;
24122412
break;
24132413
case Trace.Types.Events.Name.MARK_FIRST_PAINT:

front_end/panels/timeline/utils/EntryStyles.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,5 +1131,13 @@ export function markerDetailsForEvent(event: Trace.Types.Events.Event): {
11311131
color = 'var(--color-text-primary)';
11321132
title = Trace.Handlers.ModelHandlers.PageLoadMetrics.MetricName.NAV;
11331133
}
1134+
if (Trace.Types.Events.isMarkDOMContent(event)) {
1135+
color = 'var(--color-text-disabled)';
1136+
title = Trace.Handlers.ModelHandlers.PageLoadMetrics.MetricName.DCL;
1137+
}
1138+
if (Trace.Types.Events.isMarkLoad(event)) {
1139+
color = 'var(--color-text-disabled)';
1140+
title = Trace.Handlers.ModelHandlers.PageLoadMetrics.MetricName.L;
1141+
}
11341142
return {color, title};
11351143
}

test/interactions/panels/performance/timeline/revealing-insights_test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,22 @@ describe('Performance panel insights', () => {
3232
return await header.evaluate(elem => elem.getAttribute('aria-expanded') === 'true');
3333
});
3434

35-
// Clicking the insight should create an overlay. For the LCP request insight there are 5:
35+
// Clicking the insight should create an overlay. For the LCP request insight there are 7:
3636
// 1. Candy stripe.
3737
// 2. Red outline on the event.
3838
// 3. "LCP loaded 33ms after..." timespan.
3939
// 4. Navigation Timings Marker
4040
// 5. FCP Timings Marker & LCP Timings Marker
41+
// 7. DCL Timings Marker
42+
// 8. L Timings Marker
4143
const overlays = await $$('.overlay-item', flameChart);
42-
assert.lengthOf(overlays, 5);
44+
assert.lengthOf(overlays, 7);
4345
const jsLogContexts = await Promise.all(overlays.map(async overlay => {
4446
return await overlay.evaluate(elem => elem.getAttribute('jslog') ?? '');
4547
}));
4648
assert.deepEqual(jsLogContexts, [
49+
'Item; context: timeline.overlays.timings-marker',
50+
'Item; context: timeline.overlays.timings-marker',
4751
'Item; context: timeline.overlays.timings-marker',
4852
'Item; context: timeline.overlays.timings-marker',
4953
'Item; context: timeline.overlays.entry-outline-error',

0 commit comments

Comments
 (0)