Add tests to verify preload caching behavior#31539
Conversation
|
Submitted implementation bugs: |
yoavweiss
left a comment
There was a problem hiding this comment.
Looks good! A couple of questions
| <script src="/preload/resources/preload_helper.js"></script> | ||
| <script> | ||
|
|
||
| const crossOriginHost = 'https://{{host}}:{{ports[https][0]}}' |
There was a problem hiding this comment.
We typically use get-host-info.sub.js for those things, as it encapsulates the "sub" parts and is generally more readable.
There was a problem hiding this comment.
Yea, originally it needed to be as a sub.html, I'll revise
| for (const resourceConfigName of configNames) { | ||
| for (const preloadConfigName of Object.keys(configs)) { | ||
| if (resourceConfigName === 'same-origin' && preloadConfigName !== 'same-origin') | ||
| continue; |
There was a problem hiding this comment.
Nit: || those 2 conditions? (instead of having those separate if statements)
There was a problem hiding this comment.
Also, we're skipping those cases because in those cases we'd get a different URL?
There was a problem hiding this comment.
Because if it's the same origin the CORS attributes are ignored anyway. Will try to clarify with a comment
| }, `Loading ${type} (${resourceConfig}) with link (${preloadConfig}) should ${expected} the preloaded response`); | ||
| } | ||
|
|
||
| for (const resourceTypeName in resourceTypes) { |
There was a problem hiding this comment.
No, it's a key iteration
There was a problem hiding this comment.
Fixed to use Object.entries instead
| } | ||
|
|
||
| const configs = { | ||
| 'same-origin': {crossOrigin: false, attributes: {}}, |
There was a problem hiding this comment.
Can you add a comment saying what each of the config values does?
| <script src="/common/get-host-info.sub.js"></script> | ||
| <script> | ||
|
|
||
| const {REMOTE_HOST, HTTPS_PORT} = get_host_info(); |
There was a problem hiding this comment.
Nit: you can use HTTPS_REMOTE_ORIGIN instead of those two
|
|
||
| } | ||
|
|
||
| for (const resourceTypeName in resourceTypes) { |
Check that preloaded resources are reused/ignored based on the parameters available to a preload link (href, crossorigin, as), and do not rely on other parameters (e.g. whether a script is a module). See whatwg/html#7260 and whatwg/fetch#1342
Before any particular fetch steps are performed, see if there is a matching request already in the preload store and consume it. This is called from the main fetch to avoid race conditions. Depends on whatwg/html#7260, and together they fix #590. Tests: web-platform-tests/wpt#31539.
Before any particular fetch steps are performed, see if there is a matching request already in the preload store and consume it. This is called from the main fetch to avoid race conditions. Depends on whatwg/html#7260, and together they fix whatwg#590. Tests: web-platform-tests/wpt#31539.
Check that preloaded resources are reused/ignored based on the
parameters available to a preload link (href, crossorigin, as),
and do not rely on other parameters (e.g. whether a script is a module, or an image is decoded successfully).
See whatwg/html#7260
and whatwg/fetch#1342