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

Add Render Blocking Status to PerformanceResourceTiming #327

Merged
merged 5 commits into from
Aug 31, 2022

Conversation

abinpaul1
Copy link
Contributor

@abinpaul1 abinpaul1 commented Jun 2, 2022

@yoavweiss
Copy link
Contributor

Thanks @abinpaul1!!

@caribouW3 - What do we need to appease the IPR bots here? Should Abin join the WG as an invited expert?

@caribouW3
Copy link
Member

Are we in situation 2. b described here: https://github.com/w3c/Guide/blob/master/process/non-participant-commitment.md
I think that if it's a one-time contribution, a non-participant commitment is enough
(Contributor needs a W3C public account, link it with the GH account, then it's done by submitting a licensing commitment form).

@abinpaul1 abinpaul1 marked this pull request as ready for review June 10, 2022 05:57
@yoavweiss
Copy link
Contributor

We discussed this on the WG call yesterday.

@bdekoz @sefeng211 @achristensen07 - I'd love y'all's thoughts on this

@yoavweiss
Copy link
Contributor

Left a relevant comment on the Fetch PR that would impact this PR as well/

@noamr
Copy link
Contributor

noamr commented Jun 18, 2022

Any WPTs for this?

@noamr
Copy link
Contributor

noamr commented Jun 18, 2022

Great idea, though I believe this would be confusing with preloads until we resolve #303. If you preload a stylesheet and then load it as render-blocking, it would currently appear as not render-blocking because currently we don't expose preload-consuming requests in resource timing.

@abinpaul1
Copy link
Contributor Author

abinpaul1 commented Jun 18, 2022

Any WPTs for this?

It's being added over at https://chromium-review.googlesource.com/c/chromium/src/+/3709521
Have added a few cases but its still a WIP

@noamr
Copy link
Contributor

noamr commented Jun 18, 2022

It's probably failing because the Fetch PR is not merged yet. I want that merged first and to get some understanding about #327 (comment) before proceeding

@yoavweiss
Copy link
Contributor

I agree that this would be confusing initially, but don't know if we want to block this work on solving preloads.
@noamr - Would an intermediate state where preload RT enrties are marked as non-blocking be too confusing in your opinion? I believe this is what Chrome is currently doing with traces.

@noamr
Copy link
Contributor

noamr commented Jun 20, 2022

I agree that this would be confusing initially, but don't know if we want to block this work on solving preloads.

@noamr - Would an intermediate state where preload RT enrties are marked as non-blocking be too confusing in your opinion? I believe this is what Chrome is currently doing with traces.

Sounds ok, just wanted to get that conversation started (this PR is blocked on the fetch PR)

domenic pushed a commit to whatwg/fetch that referenced this pull request Jul 29, 2022
This supports the addition of a render blocking status field to PerformanceResourceTiming. Further details are available at w3c/resource-timing#327.
@abinpaul1
Copy link
Contributor Author

Changed the type of render blocking status from string to enum as per TAG suggestion w3ctag/design-reviews#753 (comment).

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@noamr noamr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants