-
Notifications
You must be signed in to change notification settings - Fork 31
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
[Spec] Network revocation patches for WebSocket and WebTransport APIs #206
base: master
Are you sure you want to change the base?
Conversation
LGTM % 1 comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
@@ -2123,6 +2135,16 @@ Issue: This will require a RFC to add a test-only function to the WPT web driver | |||
1. [=set/Append=] |nonce| to the user agent's [=network revocation nonce set=]. | |||
|
|||
1. [=fetch group/terminated|Terminate=] |settings|'s [=fetch/fetch group=]. | |||
|
|||
1. [=list/For each=] {{WebSocket}} object |webSocket| whose [=relevant settings object=] is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This algorithm is called in parallel so I don't really think it's valid for us to be inspecting "each" WebIDL WebSocket
object of a given settings object, and running algorithms on it that (a) HTML currently runs as a task on the Document's event loop, and (b) the WebSocket spec itself only runs as proper event-loop-bound networking tasks. So we should probably (a) figure out a better way to restructure this, and (b) add a step at the beginning of this algorithm to assert that this is running in parallel, to make it easier to catch this kind of stuff in the future.
This ensures that the following things happen for both
WebTransport
andWebSocket
objects:Preview | Diff