Add signal to request #39290
Merged
Add signal to request #39290
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Heads up! This PR modifies the following files:
|
|
🔨 Triggering try run (#17700801139) for Linux (WPT) |
TimvdLippe
force-pushed
the
add-request-signal
branch
from
41c264b to
b64491d
Compare
|
|
|
fwiw, https://doc.rust-lang.org/std/collections/struct.BTreeSet.html is documented as being an ordered set. |
|
Thanks for linking! Unfortunately that would require to implement |
TimvdLippe
force-pushed
the
add-request-signal
branch
from
b64491d to
2979270
Compare
servo-highfive
added
the
S-awaiting-review
|
🔨 Triggering try run (#17707314783) for Linux (WPT) |
|
|
TimvdLippe
force-pushed
the
add-request-signal
branch
from
2979270 to
bb252a4
Compare
TimvdLippe
commented
Sep 14, 2025
|
🔨 Triggering try run (#17707934070) for Linux (WPT) |
|
Test results for linux-wpt from try job (#17707934070): Flaky unexpected result (11)
Stable unexpected results that are known to be intermittent (28)
Stable unexpected results (5)
|
|
|
TimvdLippe
force-pushed
the
add-request-signal
branch
from
bb252a4 to
2c96af2
Compare
jdm
requested changes
Sep 14, 2025
servo-highfive
added
S-needs-code-changes
TimvdLippe
force-pushed
the
add-request-signal
branch
from
2c96af2 to
3113ba1
Compare
servo-highfive
removed
the
S-needs-code-changes
TimvdLippe
force-pushed
the
add-request-signal
branch
from
3113ba1 to
5564d53
Compare
servo-highfive
added
the
S-awaiting-review
|
🔨 Triggering try run (#17761634517) for Linux (WPT) |
|
|
jdm
added
S-needs-code-changes
Taym95
reviewed
Sep 16, 2025
| ) -> DomRoot<AbortSignal> { | ||
| // The static any(signals) method steps are to return the result | ||
| // of creating a dependent abort signal from signals using AbortSignal and the current realm. | ||
| Self::create_dependent_abort_signal(signals, global, can_gc) |
There was a problem hiding this comment.
Thanks for implementing Any. I was working on this in #36936, so it would be good to check to avoid duplicate work in the future.
There was a problem hiding this comment.
Ah I asked @jdm if you folks weren't working on it, as I indeed wanted to avoid duplicate work. He said that you weren't, but I wasn't aware that you were continuing on it in a different ticket.
Taym95
approved these changes
Sep 16, 2025
TimvdLippe
force-pushed
the
add-request-signal
branch
from
5564d53 to
19abf06
Compare
servo-highfive
added
S-awaiting-review
|
🔨 Triggering try run (#17774205386) for Linux (WPT) |
|
Test results for linux-wpt from try job (#17774205386): Flaky unexpected result (16)
Stable unexpected results that are known to be intermittent (31)
|
|
✨ Try run (#17774205386) succeeded. |
TimvdLippe
force-pushed
the
add-request-signal
branch
from
19abf06 to
89717b1
Compare
The signal taken from the requestinit is now passed into the request object with the relevant steps. I added all spec comments to this method, as I had trouble figuring out which steps I had to add. This required implementing the algorithm to create dependent signals, which is used in the `any()` method. So that's now implemented as well. All of that required the machinery to have dependent and source signals on an AbortSignal. It uses an FxHashSet as the spec requires it to be an ordered set. I don't think FxHashSet implements ordering, but at least it is a set. Part of servo#34866 Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
TimvdLippe
force-pushed
the
add-request-signal
branch
from
89717b1 to
3fd52b9
Compare
|
Cool, finally all green. Made a bit of a mess with the crown syntax |
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The signal taken from the requestinit is now passed into
the request object with the relevant steps. I added all
spec comments to this method, as I had trouble figuring
out which steps I had to add.
This required implementing the algorithm to create
dependent signals, which is used in the
any()method.So that's now implemented as well.
All of that required the machinery to have dependent and
source signals on an AbortSignal. It uses an IndexSet
as the spec requires it to be an ordered set.
Part of #34866