-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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(network-requests): add frame and preload debug data #14161
Conversation
(min, record) => Math.min(min, record.startTime), | ||
Infinity | ||
); | ||
const records = await NetworkRecords.request(devtoolsLog, context); |
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.
switching to await
causes a lot more noise here (sorry, much easier to review with ?w=1
), but adding MainResource
required this to be either async, a promise chain, or Promise.all
, so it was much easier to bite the bullet and drag this code out of 2018.
const TCPMs = record.lrStatistics?.TCPMs; | ||
const requestMs = record.lrStatistics?.requestMs; | ||
const responseMs = record.lrStatistics?.responseMs; | ||
// Default these to undefined so omitted from JSON in the negative case. |
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.
we really don't need a ton of isLinkPreload: false
in all our LHRs, so seemed better to map false
to undefined
so the property is just dropped when the LHR is JSON.stringify()
ed.
const experimentalFromMainFrame = mainFrameId ? | ||
((record.frameId === mainFrameId) || undefined) : | ||
undefined; |
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.
really need a reverse ??
, returning undefined
if the value on the left is undefined
:) Alternatives welcome
Similar to #14155, this PR fills in some missing pieces of debugData to make it easier to analyze LCP issues in HTTP Archive data.
Adds two new optional properties to the
network-requests
hidden audit:isLinkPreload
experimentalFromMainFrame
(navigations only)The second one is just if the request came from the main frame or not, but it turns out there's not really any way to do that in a
timespan
right now. We have network request frame information, but we don't have information on the main frame, and it's not even "the main frame" singular, since the frame will change if the timespan contains navigations :)Since HTTP Archive only includes navigation runs, I decided to defer the general navigation/timespan solution to the already-in-discussion request source/target idea in #14157, and call it
experimentalFromMainFrame
so it's slightly more clear that the property might not be around in a few releases. Happy to bikeshed on the name, implementation, etc.