Allow range headers to pass through a service worker #10348
Conversation
wpt-pr-bot
requested review from
annevk,
asutherland,
ayg,
beidson,
gsnedders,
jdm,
jgraham,
mfalken,
mkruisselbrink,
mnot,
wanderview,
youennf and
yutakahirano
resources/testharness.js
Outdated
| this_obj.doneResolve = resolve; | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
@jgraham @gsnedders I've added this so developers can await fetch_tests_from_worker(worker). This means:
- Promise tests can run in a predictable order.
- Service worker tests can complete before the next test unregisters it.
There was a problem hiding this comment.
@jgraham @gsnedders nudge I don't suppose one of you could take a look at this bit?
There was a problem hiding this comment.
These changes seem okay, but can you extract them into a separate PR and update the docs in docs/_writing-tests/testharness-api.md?
There was a problem hiding this comment.
@jakearchibald I mostly defer to others for testharness.js changes. Like, uh, @Ms2ger. :)
There was a problem hiding this comment.
@Ms2ger shall do. Thanks for taking a look.
There was a problem hiding this comment.
Extracted: #10978
jakearchibald
force-pushed
the
range-requests
branch
from
0e4a090 to
4c581fe
Compare
jakearchibald
changed the title
resources/testharness.js
Outdated
| @@ -1790,6 +1796,8 @@ policies and contribution forms [3]. | |||
| this.running = false; | |||
| this.remote = null; | |||
| this.message_target = null; | |||
| if (this.doneResolve) this.doneResolve(); | |||
There was a problem hiding this comment.
Wrap in braces and add line breaks.
resources/testharness.js
Outdated
| @@ -1741,6 +1741,12 @@ policies and contribution forms [3]. | |||
| } | |||
| }; | |||
|
|
|||
| if (self.Promise) { | |||
| this.done = new Promise(function(resolve) { | |||
There was a problem hiding this comment.
Is there a reason to create this property, or is it just because that's the easiest way to pass the promise to the caller?
There was a problem hiding this comment.
Yeah, this is purely so it can be returned in https://github.com/w3c/web-platform-tests/pull/10348/files#diff-d9d64f7c061650556b18794cc9719152R2159.
resources/testharness.js
Outdated
| this_obj.doneResolve = resolve; | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
These changes seem okay, but can you extract them into a separate PR and update the docs in docs/_writing-tests/testharness-api.md?
jakearchibald
force-pushed
the
range-requests
branch
from
4c581fe to
ff231de
Compare
| * | ||
| * @param {string} url | ||
| * @returns {HTMLIFrameElement} | ||
| */ |
There was a problem hiding this comment.
This change was just to improve autocomplete in VSCode.
There was a problem hiding this comment.
range/general.any.js and range/partial-script.window.js seem fine, but the service worker script is rather non-deterministic when I run it. It first finds 8 tests and then on refresh there's 4 (all saying Error: wait_for_state must be passed a ServiceWorker).
|
It might be nice for |
Sounds like a bug. Will investigate & fix. |
This is part of #144. The aim is to allow APIs to use the range header for no-cors requests, and allow them to pass through a service worker, but disallow modification of these requests, and disallow developers creating their own no-cors ranged requests. Tests: web-platform-tests/wpt#10348.
This is part of #144. The aim is to allow APIs to use the range header for no-cors requests, and allow them to pass through a service worker, but disallow modification of these requests, and disallow developers creating their own no-cors ranged requests. Tests: web-platform-tests/wpt#10348.
|
@annevk test is now stable in Firefox, and I've split up the general tests. |
This is part of #144. The aim is to allow APIs to use the Range header for "no-cors" requests, and allow them to pass through a service worker, but disallow modification of these requests, and disallow developers creating their own "no-cors" ranged requests. Tests: web-platform-tests/wpt#10348.
For whatwg/fetch#560.