Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: support traces with TracingStartedInBrowser event #5271

Merged
merged 5 commits into from
Jun 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 28 additions & 8 deletions lighthouse-core/gather/computed/trace-of-tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,39 @@ class TraceOfTab extends ComputedArtifact {
const keyEvents = trace.traceEvents
.filter(e => {
return e.cat.includes('blink.user_timing') ||
e.cat.includes('loading') ||
e.cat.includes('devtools.timeline') ||
e.name === 'TracingStartedInPage';
e.cat.includes('loading') ||
e.cat.includes('devtools.timeline') ||
e.cat === '__metadata';
})
// @ts-ignore - stableSort added to Array by WebInspector.
.stableSort((event0, event1) => event0.ts - event1.ts);

// The first TracingStartedInPage in the trace is definitely our renderer thread of interest
// Beware: the tracingStartedInPage event can appear slightly after a navigationStart
const startedInPageEvt = keyEvents.find(e => e.name === 'TracingStartedInPage');
// Find out the inspected page frame.
/** @type {LH.TraceEvent|undefined} */
let startedInPageEvt;
const startedInBrowserEvt = keyEvents.find(e => e.name === 'TracingStartedInBrowser');
if (startedInBrowserEvt && startedInBrowserEvt.args.data &&
startedInBrowserEvt.args.data.frames) {
const mainFrame = startedInBrowserEvt.args.data.frames.find(frame => !frame.parent);
const pid = mainFrame && mainFrame.processId;
const threadNameEvt = keyEvents.find(e => e.pid === pid && e.ph === 'M' &&
e.cat === '__metadata' && e.name === 'thread_name' && e.args.name === 'CrRendererMain');
startedInPageEvt = mainFrame && threadNameEvt ?
Object.assign({}, startedInBrowserEvt, {
pid, tid: threadNameEvt.tid, name: 'TracingStartedInPage',
args: {data: {page: mainFrame.frame}}}) :
undefined;
}
// Support legacy browser versions that do not emit TracingStartedInBrowser event.
if (!startedInPageEvt) {
// The first TracingStartedInPage in the trace is definitely our renderer thread of interest
// Beware: the tracingStartedInPage event can appear slightly after a navigationStart
startedInPageEvt = keyEvents.find(e => e.name === 'TracingStartedInPage');
}
if (!startedInPageEvt) throw new LHError(LHError.errors.NO_TRACING_STARTED);
// @ts-ignore - property chain exists for 'TracingStartedInPage' event.
const frameId = startedInPageEvt.args.data.page;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the casts are annoying, could also do something like

 /** @type {LH.TraceEvent|undefined} */
let possibleStartedInPageEvt;
...

above, and then here

const startedInPageEvt = possibleStartedInPageEvt;

because the compiler will know that possibleStartedInPageEvt is definitely defined at this point, the type for startedInPageEvt will be inferred correctly as LH.TraceEvent, and then it won't need any casts below this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, but I didn't want to introduce more code to workaround compiler issues. I'm fine with casts.


// Filter to just events matching the frame ID for sanity
const frameEvents = keyEvents.filter(e => e.args.frame === frameId);

Expand Down Expand Up @@ -106,12 +126,12 @@ class TraceOfTab extends ComputedArtifact {
// stable-sort events to keep them correctly nested.
/** @type Array<LH.TraceEvent> */
const processEvents = trace.traceEvents
.filter(e => e.pid === startedInPageEvt.pid)
.filter(e => e.pid === /** @type {LH.TraceEvent} */ (startedInPageEvt).pid)
// @ts-ignore - stableSort added to Array by WebInspector.
.stableSort((event0, event1) => event0.ts - event1.ts);

const mainThreadEvents = processEvents
.filter(e => e.tid === startedInPageEvt.tid);
.filter(e => e.tid === /** @type {LH.TraceEvent} */ (startedInPageEvt).tid);

const traceEnd = trace.traceEvents.reduce((max, evt) => {
return max.ts > evt.ts ? max : evt;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"tid": 1295,
"ts": 900000000000,
"ph": "X",
"cat": "toplevel",
"cat": "devtools.timeline",
"name": "TracingStartedInPage",
"args": {
"data": {
Expand Down
49 changes: 49 additions & 0 deletions lighthouse-core/test/gather/computed/trace-of-tab-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,55 @@ describe('Trace of Tab computed artifact:', () => {
assert.equal(trace.firstMeaningfulPaintEvt, undefined, 'bad fmp');
});

it('handles traces with TracingStartedInBrowser events', async () => {
const tracingStartedInBrowserTrace = {
'traceEvents': [{
'pid': 69850,
'tid': 69850,
'ts': 2193564729582,
'ph': 'I',
'cat': 'disabled-by-default-devtools.timeline',
'name': 'TracingStartedInBrowser',
'args': {'data': {
'frameTreeNodeId': 1,
'frames': [{
'frame': 'B192D1F3355A6F961EC8F0B01623C1FB',
'url': 'http://www.example.com/',
'name': '',
'processId': 69920,
}],
}},
'tts': 1085165,
's': 't',
}, {
'pid': 69920,
'tid': 1,
'ts': 2193564790059,
'ph': 'R',
'cat': 'blink.user_timing',
'name': 'navigationStart',
'args': {
'frame': 'B192D1F3355A6F961EC8F0B01623C1FB',
'data': {
'documentLoaderURL': 'http://www.example.com/',
'isLoadingMainFrame': true,
},
},
'tts': 141371,
}, {
'pid': 69920,
'tid': 1,
'ts': 0,
'ph': 'M',
'cat': '__metadata',
'name': 'thread_name',
'args': {'name': 'CrRendererMain'},
}]};
const trace = await traceOfTab.compute_(tracingStartedInBrowserTrace);
assert.equal(trace.startedInPageEvt.ts, 2193564729582);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want to assert the navStart is found and in the trace.mainThreadEvents since I assume that's what the thread_name addition was for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

assert.equal(trace.navigationStartEvt.ts, 2193564790059);
});

it('stably sorts events', async () => {
const traceJson = fs.readFileSync(__dirname +
'/../../fixtures/traces/tracingstarted-after-navstart.json', 'utf8');
Expand Down
6 changes: 6 additions & 0 deletions typings/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ declare global {
cat: string;
args: {
data?: {
frames?: {
frame: string;
parent?: string;
processId?: number;
}[];
page?: string;
readyState?: number;
requestId?: string;
Expand All @@ -148,6 +153,7 @@ declare global {
url?: string;
};
frame?: string;
name?: string;
};
pid: number;
tid: number;
Expand Down