Conversation
|
🔨 Triggering try run (#18657178016) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18657178016): Flaky unexpected result (22)
Stable unexpected results that are known to be intermittent (21)
Stable unexpected results (7)
|
|
|
d96c362 to
edc86e6
Compare
|
🔨 Triggering try run (#18657930235) for Linux (WPT) |
edc86e6 to
3f90d4d
Compare
|
Test results for linux-wpt from try job (#18657930235): Flaky unexpected result (27)
Stable unexpected results that are known to be intermittent (20)
|
|
✨ Try run (#18657930235) succeeded. |
|
🔨 Triggering try run (#18660970382) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18660970382): Flaky unexpected result (15)
Stable unexpected results that are known to be intermittent (24)
|
|
✨ Try run (#18660970382) succeeded. |
gterzian
left a comment
There was a problem hiding this comment.
LGTM with some small changes
| // Step 3.1. Queue a global task on the timer task source given global to signal abort given signal and a new "TimeoutError" DOMException. | ||
| // For the duration of this timeout, if signal has any event listeners registered for its abort event, | ||
| // there must be a strong reference from global to signal. | ||
| let signal_keepalive: Trusted<AbortSignal> = Trusted::new(&signal); |
There was a problem hiding this comment.
I think we probably need to do something at the level of the global, and also tracking event listeners, but this can be done later as part of the garbage collection stuff.
components/script/dom/abortsignal.rs
Outdated
| ms_i64, | ||
| move |global, _can_gc| { | ||
| let task_source = global.task_manager().timer_task_source().to_sendable(); | ||
| let signal_trusted = signal_keepalive.clone(); |
components/script/dom/abortsignal.rs
Outdated
| /// <https://dom.spec.whatwg.org/#dom-abortsignal-timeout> | ||
| fn Timeout(global: &GlobalScope, milliseconds: u64) -> DomRoot<AbortSignal> { | ||
| // Step 1. Let signal be a new AbortSignal object. | ||
| let signal = AbortSignal::new_with_proto(global, None, CanGc::note()); |
There was a problem hiding this comment.
I think we should add the can_gc as an argument using the settings, and pass it down all to the way to inside the task.
components/script/dom/abortsignal.rs
Outdated
| // Step 2. Let global be signal’s relevant global object. | ||
| // We already have `global`. | ||
|
|
||
| // Step 3. Run steps after a timeout given global, "AbortSignal-timeout", milliseconds, and the following step: |
There was a problem hiding this comment.
I think this can move to just above global.run_steps_after_a_timeout
components/script/dom/abortsignal.rs
Outdated
|
|
||
| // Step 3. Run steps after a timeout given global, "AbortSignal-timeout", milliseconds, and the following step: | ||
| // | ||
| // Step 3.1. Queue a global task on the timer task source given global to signal abort given signal and a new "TimeoutError" DOMException. |
There was a problem hiding this comment.
This I would move to above task_source.queue.
3f90d4d to
67d8d14
Compare
67d8d14 to
b20ef65
Compare
Signed-off-by: Taym Haddadi <[email protected]>
b20ef65 to
c140493
Compare
|
🔨 Triggering try run (#18711472292) for Linux (WPT) |
|
Test results for linux-wpt from try job (#18711472292): Flaky unexpected result (20)
Stable unexpected results that are known to be intermittent (22)
|
|
✨ Try run (#18711472292) succeeded. |
Implement abortsignal timeout
Testing: should pass abortsignal timeout wpt test.
Part of #36936
on top of #39994