Allow upgrade of renderBlockingStatus in Resource Timing for preloaded resources. [chromium/src : main]
Michal Mocny (Gerrit)
Michal Mocny voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michal Mocny
| Code-Review | +1 |
1 comment
Related details
- Barry Pollard
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 1 comment![]( "Open in Gerrit")
PendingResourceTimingInfoMap::iterator it =https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/resource.cc;l=1254?q=IsLoadEventBlockingResourceType&ss=chromium doesn't correspond to a render blocking resource necessarily (e.g. an image preload would turn here to a render blocking one). I think we'd need to find a different way to map the resource to its render blocking status.
Related details
- Barry Pollard
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 1 comment![]( "Open in Gerrit")
<link rel=preload as=style href="resources/empty_style.css?link-style-preload-used-immediately">Can you also add an image preload to make sure it's not render blocking?
Barry Pollard (Gerrit)
Barry Pollard added 2 comments![]( "Open in Gerrit")
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/resource.cc;l=1254?q=IsLoadEventBlockingResourceType&ss=chromium doesn't correspond to a render blocking resource necessarily (e.g. an image preload would turn here to a render blocking one). I think we'd need to find a different way to map the resource to its render blocking status.
Oh good spot. I misread that as render-blocking, rather than load-blocking.
Fixed, and I also renamed the function to make it more obvious it's now not just lonad-blocking.
<link rel=preload as=style href="resources/empty_style.css?link-style-preload-used-immediately">Can you also add an image preload to make sure it's not render blocking?
Done.
Also added a couple of script checks (one blocking, one defer and so non=blocking)
Related details
- Michal Mocny
- Noam Rosenthal
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 3 comments![]( "Open in Gerrit")
// If new resource is render-blocking then mark it as such in PendingNit: s/Pending/pending/
// IsLoadEventBlockingResourceType check as that doesn't include scriptsNit: '.' at the end of sentences
if (params.GetResourceRequest().GetRenderBlockingBehavior() ==Should this simply assign the value from the resourceRequest to the RT entry?https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/render_blocking_behavior.h;l=11?q=RenderBlockingBehavior&ss=chromium%2Fchromium%2Fsrc has other values (e.g. potentially blocking) that might be worthwhile passing on to the preload
Related details
- Barry Pollard
- Michal Mocny
- Noam Rosenthal
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Barry Pollard (Gerrit)
Barry Pollard added 3 comments![]( "Open in Gerrit")
// If new resource is render-blocking then mark it as such in PendingBarry PollardNit: s/Pending/pending/
Done
// IsLoadEventBlockingResourceType check as that doesn't include scriptsNit: '.' at the end of sentences
Done
if (params.GetResourceRequest().GetRenderBlockingBehavior() ==Should this simply assign the value from the resourceRequest to the RT entry?https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/render_blocking_behavior.h;l=11?q=RenderBlockingBehavior&ss=chromium%2Fchromium%2Fsrc has other values (e.g. potentially blocking) that might be worthwhile passing on to the preload
We could pass this on, though note at present only kBlocking is passed on to resource timing:
But I guess that might change in the future?
The downside of passing everything is that we would run this iterator and `render_blocking_behavior` update on more status' — though probably not that many more since it's linked to link preload upgrades anyway.
WDYT?
Related details
- Michal Mocny
- Noam Rosenthal
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) added 1 comment![]( "Open in Gerrit")
if (params.GetResourceRequest().GetRenderBlockingBehavior() ==Barry PollardShould this simply assign the value from the resourceRequest to the RT entry?https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/render_blocking_behavior.h;l=11?q=RenderBlockingBehavior&ss=chromium%2Fchromium%2Fsrc has other values (e.g. potentially blocking) that might be worthwhile passing on to the preload
We could pass this on, though note at present only kBlocking is passed on to resource timing:
But I guess that might change in the future?
The downside of passing everything is that we would run this iterator and `render_blocking_behavior` update on more status' — though probably not that many more since it's linked to link preload upgrades anyway.
WDYT?
You're right that currently that's functionally identical. If in the future we'd expand the RT exposed blocking status, this risks causing a bug with that. At the same time, I'm not aware of such plans..
Your call!
Related details
- Barry Pollard
- Michal Mocny
- Noam Rosenthal
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Barry Pollard (Gerrit)
Barry Pollard added 1 comment![]( "Open in Gerrit")
if (params.GetResourceRequest().GetRenderBlockingBehavior() ==Barry PollardShould this simply assign the value from the resourceRequest to the RT entry?https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/loader/fetch/render_blocking_behavior.h;l=11?q=RenderBlockingBehavior&ss=chromium%2Fchromium%2Fsrc has other values (e.g. potentially blocking) that might be worthwhile passing on to the preload
Yoav Weiss (@Shopify)We could pass this on, though note at present only kBlocking is passed on to resource timing:
But I guess that might change in the future?
The downside of passing everything is that we would run this iterator and `render_blocking_behavior` update on more status' — though probably not that many more since it's linked to link preload upgrades anyway.
WDYT?
You're right that currently that's functionally identical. If in the future we'd expand the RT exposed blocking status, this risks causing a bug with that. At the same time, I'm not aware of such plans..
Your call!
I'm going to leave it as is. Consider this scenario:
<link rel=preload as=style href=styles.css>
<link rel=stylesheet href=styles.css>
<link rel=preload as=style href=styles.css>
We don't want the third one to reset the render-blocking status back to non-blocking. So we'd need some concept of ordering to always set the "most blocking" behaviour. And I don't think it's worth introducing this logic now when, as you say, there are no plans to add this to Resource Timing.
I did however add comments to warn about this, and the above test.
PTAL.
Related details
- Michal Mocny
- Noam Rosenthal
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) voted and added 1 comment![]( "Open in Gerrit")
Votes added by Yoav Weiss (@Shopify)
| Code-Review | +1 |
LGTM
Please send a PSA for this web-exposed change, and follow up with spec discussion for actually defining this behavior.
Related details
- Barry Pollard
- Michal Mocny
- Noam Rosenthal
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Noam Rosenthal (Gerrit)
Noam Rosenthal voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Barry Pollard
- Michal Mocny
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Barry Pollard (Gerrit)
Barry Pollard voted Commit-Queue+2![]( "Open in Gerrit")
- Michal Mocny
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Allow upgrade of renderBlockingStatus in Resource Timing for preloaded resources.
Note this only happens if the Resource Timing entry has not been emitted yet and makes the Resource Timing entry reflect Chrome's internal understanding of whether this is render blocking or not. Previously this was just stuck at the initial status.
Spec discussion: https://github.com/w3c/resource-timing/issues/420
- M third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
- M third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
- M third_party/blink/web_tests/external/wpt/resource-timing/render-blocking-status-link.html
Code-Review: +1 by Noam Rosenthal, +1 by Yoav Weiss (@Shopify)
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/55871
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |