-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
LibWeb: Implement Resource Timing #3704
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
Conversation
I've not had a thorough look through the code yet, but here are some WPT results for the Before:
After:
I'm not sure why fewer subtests get executed now, I suspect http://wpt.live/resource-timing/response-status-code.html , may be timing out. |
612ab2b
to
733fde2
Compare
Your pull request has conflicts that need to be resolved before it can be reviewed and merged. Make sure to rebase your branch on top of the latest |
This timing info will be used to create a PerformanceResourceTiming entry.
The User Timing, Resource Timing and Navigation Timing specifications have not been updated to account for the queue method to also append to the underlying performance buffer and it's method of checking it's full. This separates the functionality into a different function, with a flag to indicate whether to use the custom full buffer logic or not.
Marking a resource timing entry requires calling non-const methods on the global object to append to the performance buffer.
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.
Couple comments, and I saw that there's a few AD-HOC and "Spec-Issue" comments that should probably be associated with an actual spec issue. Doesn't much matter to me if that actual link is set up in a follow-up though.
HighResolutionTime::DOMHighResTimeStamp convert_fetch_timestamp(HighResolutionTime::DOMHighResTimeStamp time_stamp, JS::Object const& global) | ||
{ | ||
// 1. If ts is zero, return zero. | ||
if (time_stamp == 0.0) |
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.
what if it's -0.0 ? do we have an is_zero helper for doubles?
Fetch spec issues for the issues mentioned in the PR description have been filed: |
I'mma just merge this. if that 0.0 thing is an issue I'm sure we'll find out eventually. |
See individual commits for details.
There are two fetch spec issues to file: