Skip to content

Commit

Permalink
core(trace): use tracing-started event for frame tree info (#13913)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored and connorjclark committed May 9, 2022
1 parent 67100c4 commit 308d15e
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 25 deletions.
38 changes: 29 additions & 9 deletions lighthouse-core/lib/tracehouse/trace-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,39 +612,59 @@ class TraceProcessor {
// Find the inspected frame
const mainFrameIds = this.findMainFrameIds(keyEvents);

const frames = keyEvents
/** @type {Map<string, {id: string, url: string, parent?: string}>} */
const framesById = new Map();

// Begin collection of frame tree information with TracingStartedInBrowser,
// which should be present even without navigations.
const tracingStartedFrames = keyEvents
.find(e => e.name === 'TracingStartedInBrowser')?.args?.data?.frames;
if (tracingStartedFrames) {
for (const frame of tracingStartedFrames) {
framesById.set(frame.frame, {
id: frame.frame,
url: frame.url,
parent: frame.parent,
});
}
}

// Update known frames if FrameCommittedInBrowser events come in, typically
// with updated `url`, as well as pid, etc. Some traces (like timespans) may
// not have any committed frames.
keyEvents
.filter(/** @return {evt is FrameCommittedEvent} */ evt => {
return Boolean(
evt.name === 'FrameCommittedInBrowser' &&
evt.args.data?.frame &&
evt.args.data.url
);
})
.map(evt => {
return {
}).forEach(evt => {
framesById.set(evt.args.data.frame, {
id: evt.args.data.frame,
url: evt.args.data.url,
parent: evt.args.data.parent,
};
});
});
const frames = [...framesById.values()];
const frameIdToRootFrameId = this.resolveRootFrames(frames);

// Filter to just events matching the main frame ID, just to make sure.
const frameEvents = keyEvents.filter(e => e.args.frame === mainFrameIds.frameId);

// Filter to just events matching the main frame ID or any child frame IDs.
// In practice, there should always be FrameCommittedInBrowser events to define the frame tree.
// Unfortunately, many test traces do not include FrameCommittedInBrowser events due to minification.
// This ensures there is always a minimal frame tree and events so those tests don't fail.
let frameTreeEvents = [];
if (frameIdToRootFrameId.has(mainFrameIds.frameId)) {
frameTreeEvents = keyEvents.filter(e => {
return e.args.frame && frameIdToRootFrameId.get(e.args.frame) === mainFrameIds.frameId;
});
} else {
// In practice, there should always be TracingStartedInBrowser/FrameCommittedInBrowser events to
// define the frame tree. Unfortunately, many test traces do not that frame info due to minification.
// This ensures there is always a minimal frame tree and events so those tests don't fail.
log.warn(
'trace-of-tab',
'frameTreeEvents may be incomplete, make sure the trace has FrameCommittedInBrowser events'
'frameTreeEvents may be incomplete, make sure the trace has frame events'
);
frameIdToRootFrameId.set(mainFrameIds.frameId, mainFrameIds.frameId);
frameTreeEvents = frameEvents;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ describe('Metrics: CLS', () => {
const cat = 'loading,rail,devtools.timeline';
trace.traceEvents.push(
/* eslint-disable max-len */
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: mainFrame, url: 'https://example.com'}}},
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: childFrame, parent: mainFrame, url: 'https://frame.com'}}},
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: otherMainFrame, url: 'https://example.com'}}},
{name: 'LayoutShift', cat, args: {frame: mainFrame, data: {had_recent_input: false, score: 1, weighted_score_delta: 1, is_main_frame: true}}},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7997,7 +7997,7 @@
"description": "Consider reducing the time spent parsing, compiling, and executing JS. You may find delivering smaller JS payloads helps with this. [Learn more](https://web.dev/bootup-time/).",
"score": 1,
"scoreDisplayMode": "numeric",
"numericValue": 397.00499999999965,
"numericValue": 383.5219999999999,
"numericUnit": "millisecond",
"displayValue": "0.4 s",
"details": {
Expand Down Expand Up @@ -8029,26 +8029,26 @@
],
"items": [
{
"url": "Unattributable",
"total": 321.11799999999914,
"scripting": 24.528999999999996,
"url": "https://www.mikescerealshack.co/",
"total": 455.9500000000001,
"scripting": 193.52499999999992,
"scriptParseCompile": 0
},
{
"url": "https://www.mikescerealshack.co/_next/static/chunks/framework.9d524150d48315f49e80.js",
"total": 283.2019999999998,
"scripting": 186.7769999999998,
"total": 192.43299999999994,
"scripting": 176.82499999999996,
"scriptParseCompile": 0
},
{
"url": "https://www.mikescerealshack.co/_next/static/chunks/1aeab0175d4c4d823d7a78205bceb5dd9cd36d32.a629f28ec97ae6e480bf.js",
"total": 190.22199999999992,
"scripting": 185.6989999999999,
"url": "Unattributable",
"total": 135.34700000000004,
"scripting": 13.171999999999992,
"scriptParseCompile": 0
}
],
"summary": {
"wastedMs": 397.00499999999965
"wastedMs": 383.5219999999999
}
}
},
Expand Down Expand Up @@ -8705,7 +8705,7 @@
],
"items": [
{
"url": "https://www.mikescerealshack.co/_next/static/chunks/1aeab0175d4c4d823d7a78205bceb5dd9cd36d32.a629f28ec97ae6e480bf.js",
"url": "https://www.mikescerealshack.co/",
"duration": 194.901,
"startTime": 2256.773
}
Expand Down Expand Up @@ -10503,7 +10503,7 @@
},
{
"values": {
"timeInMs": 397.00499999999965
"timeInMs": 383.5219999999999
},
"path": "audits[bootup-time].displayValue"
}
Expand Down
34 changes: 31 additions & 3 deletions lighthouse-core/test/lib/tracehouse/trace-processor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ describe('TraceProcessor', () => {
const cat = 'loading,rail,devtools.timeline';
testTrace.traceEvents.push(
/* eslint-disable max-len */
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: mainFrame, url: 'https://example.com'}}},
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: childFrame, parent: mainFrame, url: 'https://frame.com'}}},
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: otherMainFrame, url: 'https://example.com'}}},
{name: 'Event1', cat, args: {frame: mainFrame}},
Expand Down Expand Up @@ -251,6 +250,37 @@ describe('TraceProcessor', () => {
'Event1',
]);
});

it('frameTreeEvents included even if no FrameCommittedInBrowser events', () => {
const testTrace = createTestTrace({timeOrigin: 0, traceEnd: 2000});
testTrace.traceEvents = testTrace.traceEvents
.filter(e => e.name !== 'FrameCommittedInBrowser');

const mainFrame = testTrace.traceEvents[0].args.frame;
const childFrame = 'CHILDFRAME';
const otherMainFrame = 'ANOTHERTAB';
const cat = 'loading,rail,devtools.timeline';

testTrace.traceEvents.find(e => e.name === 'TracingStartedInBrowser').args.data.frames.push(
{frame: childFrame, parent: mainFrame, url: 'https://frame.com'},
{frame: otherMainFrame, url: 'https://example.com'}
);

testTrace.traceEvents.push(
{name: 'Event1', cat, args: {frame: mainFrame}},
{name: 'Event2', cat, args: {frame: childFrame}},
{name: 'Event3', cat, args: {frame: otherMainFrame}}
);
const trace = TraceProcessor.processTrace(testTrace);
expect(trace.frameTreeEvents.map(e => e.name)).toEqual([
'navigationStart',
'domContentLoadedEventEnd',
'firstContentfulPaint',
'firstMeaningfulPaint',
'Event1',
'Event2',
]);
});
});

describe('getMainThreadTopLevelEvents', () => {
Expand Down Expand Up @@ -639,7 +669,6 @@ Object {

testTrace.traceEvents.push(
/* eslint-disable max-len */
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: mainFrame, url: 'https://example.com'}}, ts: 900, duration: 10},
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: childFrame, parent: mainFrame, url: 'https://frame.com'}}, ts: 910, duration: 10},
{name: 'firstContentfulPaint', cat, args: {frame: childFrame}, ts: 1000, duration: 10},
{name: 'firstContentfulPaint', cat, args: {frame: mainFrame}, ts: 1100, duration: 10}
Expand All @@ -658,7 +687,6 @@ Object {
const cat = 'loading,rail,devtools.timeline';
testTrace.traceEvents.push(
/* eslint-disable max-len */
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: mainFrame, url: 'https://example.com'}}, ts: 900, duration: 10},
{name: 'FrameCommittedInBrowser', cat, args: {data: {frame: childFrame, parent: mainFrame, url: 'https://frame.com'}}, ts: 910, duration: 10},
{name: 'largestContentfulPaint::Candidate', cat, args: {data: {size: 300}, frame: mainFrame}, ts: 1000, duration: 10},
{name: 'largestContentfulPaint::Candidate', cat, args: {data: {size: 100}, frame: childFrame}, ts: 1100, duration: 10},
Expand Down
1 change: 1 addition & 0 deletions types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,7 @@ export interface TraceEvent {
documentLoaderURL?: string;
frames?: {
frame: string;
url: string;
parent?: string;
processId?: number;
}[];
Expand Down

0 comments on commit 308d15e

Please sign in to comment.