blocking=render should override fetch priority

We recently introduced the concept of render-blocking. Since marking an element as render-blocking indicates that the resource is important, it does not make sense if we fetch the resource with a low fetch priority.

I'm going to put up a PR about this. The basic idea is that if an element has both fetchpriority=low and blocking=render attrbutes, then we'll ignore fetchpriority=low and fetch the resource with auto priority.

Would it make sense to apply this more generally? e.g. if we have a render blocking script with fetchpriority=low, implementations are likely to also ignore the hint. Maybe we can add that logic to the spec as well.

I don't know that it needs to be called out specifically, at least in the priority-hints spec. The hinted fetch priority is already only taken as one of many signals that is considered when setting the actual priority. Specifically, it's step 6 of the fetch integration where the internalpriority takes the priority hint as input.

What we probably want is just for the fetch integration to also take in render-blocking as a signal when setting internal priority and leave it at that.

@domenic

(This issue seems to be a better place for discussions so I'll also discuss here)

Thanks for the comments!

What we probably want is just for the fetch integration to also take in render-blocking as a signal when setting internal priority and leave it at that.

This also makes more sense to me than overriding the priority hint. Thanks for the idea!

So should we also add a flag to Request to indicate render-blocking behavior?

(Btw we already have such a flag in Blink's implementation)

@domenic

I think adding a read-only attribute to fetch Request makes sense and makes the spec cleaner as far as how it integrates with fetch. That also opens up the possibility to extend it to being set on fetches if we want it to be.

I think what we have here is just a staging problem.

Right now Fetch takes no input from anyone on how to determine what it currently calls priority.

After both blocking=render is integrated, and Priority Hints is integrated, then we will rename that to internalpriority, and we will use both the fetchpriority="" attribute state and the blocking="" attribute state to determine internalpriority. That will all happen in the sections of the HTML spec that perform fetches for the link, style, and script elements. (The iframe element will only take the fetchpriority="" attribute state into account.)

But right now blocking=render is integrated into HTML, and fetchpriority="" is not. We thus have several options for how to integrate:

Update HTML to compute fetch priority from blocking="" for link, style, and script. Update Priority Hints to be rebased on top of that new version of HTML, so that it takes into account both blocking="" and fetchpriority="" when computing internalpriority.
Leave HTML as-is for now. Update Priority Hints to compute internalpriority based on both blocking="" and fetchpriority="".

I was advising (2) based on the fact that fetch priority right now is very vague and it didn't make much sense to just make it a bit more rigorous, when we knew a big change was coming down the pipeline with Priority Hints. It would also be less work for everyone involved: we just integrate #70 and then later merge Priority Hints into HTML, instead of editing HTML, rebasing Priority Hints to include the equivalent of those HTML edits, and then later merging Priority Hints into HTML.

But, I admit doing (1) is a bit more proper. After all, blocking=render is an existing part of the HTML spec, and the fact that its influence on fetch priority is not yet specced is a gap that we should perhaps fix ASAP. Let me know what you prefer.

So should we also add a flag to Request to indicate render-blocking behavior?

I don't think this is necessary, if the only thing it influences is request priority. For example it isn't something that we'd let people set with fetch(), IMO.

Step #15 of the Fetching process model of fetch already has the setting of the priority. Priority-hints renames it to internalpriority to eliminate the name collision but the aggregation of data sources for setting the internal priority is already part of fetch.

The priority-hints updates won't really help because you'd still need to plumb blocking=render through to fetch for the existing supported element types, just updating the existing priority part of the fetch spec to take it as input. priority-hints can then rename it (and adopt the updated language to include blocking) as it is merged in.

As far as not updating fetch, I don't think there is a way to plumb the blocking=render state into fetch without adding it to Request in some form and updating the processing model on the HTML side to pass the state info to fetch (pretty much mirroring what priority-hints had done).

The way to plumb blocking=render into fetch without updating fetch is the same way you plumb fetchpriority="" into fetch without updating fetch. You just modify the priority you pass in. This does not require adding a new field.

Anyway, if you prefer (1), we can do that instead of (2). It'll just be more work (including for the priority hints spec).

fetchpriority added a new priority field to fetch and renamed the existing one (and updated the HTML processing models to set the new priority-hints priority attribute). The existing priority field is an opaque vendor-specific placeholder that doesn't take input from any of the HTML spec directly.

Right, but now it will.

BTW I realized a concrete difference here between what you're proposing and what is in #70 is that in a service worker, in your version, it will be possible to observe request.priority === "low" for a blocking=render resource. That doesn't seem desirable to me, but perhaps it is to you and to web developers, I'm not sure.

I don't see a way to plumb blocking=render to fetch requests without updating the fetch spec, either. Did I miss anything?

If we need to update the fetch spec, the most straightforward thing (to me) to do is to standarize the RenderBlockingBehavior flag that we already have, and add it to fetch requests.

it will be possible to observe request.priority === "low" for a blocking=render resource

Maybe rename priority to priorityHint? Since it's not the final priority but just a developer-provided hint.

Anyway, this is already orthogonal to render-blocking.

FWIW, it was named priority after much bikeshedding because it is the developer-requested priority, either through the fetchPriority HTML attribute or as a proprty of the Request on the fetch() API call directly (and it is developer-exposed for setting).

+1 to standardize the blocking property/flag values and adding it to fetch as a read-only attribute (unless you want to allow for it to be set on fetch() requests by developers as well).

@annevk

Hmm, well, I still feel that having both blocking="" and fetchpriority="" contribute to the priority/internalpriority is the best approach. But if you both feel the opposite way, then we should get @annevk's opinion as Fetch editor to make the final call.

Okay, so we have these concepts:

Developer-controlled "fetch priority" (enum)
Developer-controlled "blocking" (boolean)
Implementation-controlled "actual priority" (any)

And it sounds like if "blocking" is used, then "fetch priority" ends up ignored. ("Actual priority" would take both into account as well as other information. I'm not sure it makes sense for callers of fetch to directly set "actual priority" as currently designed. It doesn't really have a meaningful type.)

So the question is whether developers should get to learn (in service workers) whether "fetch priority" is ignored. Several alternatives were proposed:

Make it clear "fetch priority" is a hint through naming.
Also expose "blocking" on Request objects and require developers to look at both "fetch priority" and "blocking".
Make "blocking" change the computed value of "fetch priority".

On the face of it 3 seems rather attractive as developers would only have to care about a single field. It also more accurately reflects how an implementation would look at it. However, as "blocking" is still relevant for implementations, perhaps 2 ought to be done as well at some point? I.e., have

Make "blocking" change the computed value of "fetch priority", but also consider exposing "blocking" on Request objects as it does have an impact on things.

I don't think 1 is great as it doesn't make it clear when it will be actively ignored and only doing 2 seems like giving developers complexity we could have resolved.

Hope that helps.

Then it sounds like we have two separate issues:

Whether to add a developer-controled flag ("blocking", "renderblocking", etc, TBD) to Request objects, and make it affect the "actual priority". This is part of the FETCH spec
Whether to change the computed value of "fetch priority" when element is render-blocking

I think we have all agreed on 1? If so, I'll start working on a FETCH spec PR, and we can leave 2 open here.

Hmm, I guess there is some precedent for that (window), but largely that's not something we've been doing. It might be okay though. Same comment about follow-up issue applies.

I also just noticed that if you forward render-blocking fetches in a service worker the render-blocking state is lost. That's probably a bug? (Step 12 of https://fetch.spec.whatwg.org/#dom-request doesn't forward the state.)

At this point, now that we seem to be treating all three fields as orthogonal, I think it's most straightforward to expose priority as a public API on both RequestInit and Request. Unlike blocking, it doesn't seem particularly complicated.

I'd rather not expose priority on Request without blocking. Given that in most cases where blocking=true it'll end up ignored, I think that would lead to confusion in a service worker. (I agree that exposing blocking fully may be tricky though.)

@pmeenan

I feel bad I made @pmeenan write a bunch of web platform tests for exposing priority in a service worker then. (And also testing, via service worker, that fetchpriority="" set things in a good way.) But I guess the conclusion is reasonable...

So the plan is:

Expose priority in RequestInit (and also via fetchpriority="" HTML attribute)
Do not expose either priority or blocking on Request for now
Treat all three of priority, blocking, and "internal priority" as separate inputs the UA uses to make prioritization decisions? (Not entirely sure on this.)
Make sure service workers properly pass through all three of them from their input argument by modifying step 12 of https://fetch.spec.whatwg.org/#dom-request appropriately.

Is that correct?

As long as we don't expose priority and blocking, passing along "internal priority" would suffice as it's the sum of those inputs plus others. And we are doing that (although it's still called priority) so I suspect there's no problem for blocking at the moment and I was wrong above. And "internal priority" is what UAs can use then for scheduling and transport layer settings.

Otherwise that seems correct, yeah.

@annevk

Much happier to write tests that weren't needed than to expose something to the web that we end up regretting so I'm happy to make the changes to the tests and PR.

internal priority is the result of the prioritization decisions as @annevk mentions so it's not necessarily needed to cascade priority and render-blocking in step 12 of https://fetch.spec.whatwg.org/#dom-request but it kind of feels like they would be left in an undefined/default state if they weren't explicitly copied over as well.

I'll go ahead and make the suggested changes to the PR and include the copies in step 12 for now but I can also remove them if we think they should explicitly not be copied. I'll also remove the web platform tests for priority being exposed on Request.

Removed the service worker priority-hints tests

Bug 1801953 [wpt PR 37090] - Removed the service worker priority-hint…

Removed the service worker priority-hints tests

Bug 1801953 [wpt PR 37090] - Removed the service worker priority-hint…

I think this can be closed given where we ended up.

Navigation Menu

Repository navigation

`blocking=render` should override fetch priority #69

11 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

blocking=render should override fetch priority #69

Description

Activity

yoavweiss commented on Apr 15, 2022

pmeenan commented on Apr 15, 2022

xiaochengh commented on Apr 15, 2022

pmeenan commented on Apr 15, 2022

domenic commented on Apr 18, 2022

pmeenan commented on Apr 18, 2022

domenic commented on Apr 18, 2022

pmeenan commented on Apr 18, 2022

domenic commented on Apr 18, 2022

domenic commented on Apr 18, 2022

xiaochengh commented on Apr 20, 2022

pmeenan commented on Apr 21, 2022

domenic commented on Apr 21, 2022

annevk commented on Apr 22, 2022

xiaochengh commented on Apr 28, 2022

11 remaining items

annevk commented on Nov 21, 2022

domenic commented on Nov 22, 2022

annevk commented on Nov 22, 2022

domenic commented on Nov 22, 2022

annevk commented on Nov 22, 2022

pmeenan commented on Nov 22, 2022

annevk commented on Dec 15, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`blocking=render` should override fetch priority #69