-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
fix: normalize all times to navStart, remove traceviewer model #2347
Changes from all commits
e791557
8c2e789
15d9424
2fb8850
25f157c
ea619df
f32cccf
78036fc
ef2eccd
184537b
1a0aacb
9bfcdee
5f1a15f
65de1eb
ba571ab
f62b2a4
d4c4605
df3c12e
08797fc
2bb290b
71682ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ class TraceOfTab extends ComputedArtifact { | |
|
||
/** | ||
* @param {{traceEvents: !Array}} trace | ||
* @return {!{processEvents: !Array<TraceEvent>, startedInPageEvt: TraceEvent, navigationStartEvt: TraceEvent, firstContentfulPaintEvt: TraceEvent, firstMeaningfulPaintEvt: TraceEvent}} | ||
* @return {!TraceOfTabArtifact} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
*/ | ||
compute_(trace) { | ||
// Parse the trace for our key events and sort them by timestamp. | ||
|
@@ -86,6 +86,9 @@ class TraceOfTab extends ComputedArtifact { | |
.filter(e => e.pid === startedInPageEvt.pid) | ||
.sort((event0, event1) => event0.ts - event1.ts); | ||
|
||
const mainThreadEvents = processEvents | ||
.filter(e => e.tid === startedInPageEvt.tid); | ||
|
||
const traceEnd = trace.traceEvents.reduce((max, evt) => { | ||
return max.ts > evt.ts ? max : evt; | ||
}); | ||
|
@@ -95,7 +98,7 @@ class TraceOfTab extends ComputedArtifact { | |
firstPaint, | ||
firstContentfulPaint, | ||
firstMeaningfulPaint, | ||
traceEnd, | ||
traceEnd: {ts: traceEnd.ts + (traceEnd.dur || 0)}, | ||
onLoad, | ||
domContentLoaded, | ||
}; | ||
|
@@ -112,7 +115,8 @@ class TraceOfTab extends ComputedArtifact { | |
timings, | ||
timestamps, | ||
processEvents, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
startedInPageEvt: startedInPageEvt, | ||
mainThreadEvents, | ||
startedInPageEvt, | ||
navigationStartEvt: navigationStart, | ||
firstPaintEvt: firstPaint, | ||
firstContentfulPaintEvt: firstContentfulPaint, | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like only
data.tabTrace
is being used above, notdata.trace
, so can maybe dropdata
in favor of justtabTrace
? (also need to update the jsdocs still referring tomodel
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we can just kill the audit entirely :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha, good point. I'm fine with leaving as is and deleting it next :)