Implement run steps after a timeout by Taym95 · Pull Request #39994 · servo/servo

Taym95 · 2025-10-19T21:57:27Z

Implement run steps after a timeout, spec:
https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#run-steps-after-a-timeout

Testing: should pass existing wpt test
Fixes: #36934

github-actions · 2025-10-19T22:08:04Z

🔨 Triggering try run (#18636760458) for Linux (WPT)

github-actions · 2025-10-19T22:33:41Z

Test results for linux-wpt from try job (#18636760458):

Flaky unexpected result (18)

OK /FileAPI/url/url-with-fetch.any.worker.html (#21517)
- FAIL [expected PASS] subtest: Revoke blob URL after calling fetch, fetch should succeed
```
promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
```
OK /IndexedDB/idbfactory_open.any.html
- FAIL [expected PASS] subtest: Calling open() with version argument 1.5 should not throw.
```
assert_equals: version expected 1 but got 9007199254740991
```
OK /_webgl/conformance/textures/misc/texture-upload-size.html (#21770)
- PASS [expected FAIL] subtest: WebGL test #45
- PASS [expected FAIL] subtest: WebGL test #47
- PASS [expected FAIL] subtest: WebGL test #49
- PASS [expected FAIL] subtest: WebGL test #51
- FAIL [expected PASS] subtest: WebGL test #53
```
assert_true: Texture was smaller than the expected size 2x2 expected true got false
```
- FAIL [expected PASS] subtest: WebGL test #55
```
assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false
```
- FAIL [expected PASS] subtest: WebGL test #57
```
assert_true: Texture was smaller than the expected size 2x2 expected true got false
```
- FAIL [expected PASS] subtest: WebGL test #59
```
assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false
```
- PASS [expected FAIL] subtest: WebGL test #61
- PASS [expected FAIL] subtest: WebGL test #63
- And 18 more unexpected results...
OK /content-security-policy/frame-ancestors/frame-ancestors-path-ignored.window.html (#36468)
- PASS [expected FAIL] subtest: A 'frame-ancestors' CSP directive with a URL that includes a path should be ignored.
TIMEOUT [expected OK] /credential-management/credentialscontainer-frame-basics.https.html (#39430)
- TIMEOUT [expected FAIL] subtest: navigator.credentials should be undefined in documents generated from data: URLs.
```
Test timed out
```
TIMEOUT [expected OK] /fetch/api/redirect/redirect-keepalive.https.any.html (#32153)
- TIMEOUT [expected PASS] subtest: [keepalive][iframe][load] mixed content redirect; setting up
```
Test timed out
```
OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-site destination
ERROR /fetch/metadata/generated/serviceworker.https.sub.html (#36247)
- FAIL [expected PASS] subtest: sec-fetch-site - Same origin, no options - registration
```
promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
```
TIMEOUT [expected CRASH] /html/anonymous-iframe/indexeddb.tentative.https.window.html (#39254)
OK /html/browsers/browsing-the-web/navigating-across-documents/006.html (#21382)
- PASS [expected FAIL] subtest: Link with onclick form submit and href navigation
OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-nosrc.html (#34819)
- PASS [expected FAIL] subtest: link click
OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
- FAIL [expected PASS] subtest: application/x-www-form-urlencoded: Basic test (normal form)
```
assert_equals: expected "basic=test" but got ""
```
OK [expected CRASH] /html/semantics/forms/the-fieldset-element/disabled-003.html (#31730, #39631)
FAIL [expected PASS] /png/apng/fcTL-dispose-previous-final.html
OK /preload/link-header-preload-delay-onload.html (#39622)
- FAIL [expected PASS] subtest: Makes sure that Link headers preload resources and block window.onload after resource discovery
```
assert_true: expected true got false
```
OK /preload/prefetch-document.html (#37210)
- FAIL [expected PASS] subtest: different-site document prefetch with 'as=document' should not be consumed
```
assert_equals: expected 2 but got 1
```
OK /wasm/webapi/abort.any.worker.html
- FAIL [expected PASS] subtest: instantiateStreaming() asynchronously racing with abort should succeed or reject with AbortError
```
assert_equals: expected "AbortError" but got "CompileError"
```
OK [expected ERROR] /webxr/render_state_update.https.html (#27535)

Stable unexpected results that are known to be intermittent (19)

OK /IndexedDB/idbobjectstore_getAll.any.html (#39276)
- PASS [expected FAIL] subtest: Get all values with transaction.commit()
OK /IndexedDB/idbobjectstore_getAll.any.worker.html (#39400)
- PASS [expected FAIL] subtest: Get all values with transaction.commit()

OK /IndexedDB/key-conversion-exceptions.any.html (#39305)

FAIL [expected PASS] subtest: IDBCursor continue() method with throwing/invalid keys

assert_throws_exactly: key conversion with throwing getter should rethrow function "() =&gt; {
      receiver[method](key);
    }" threw object "TypeError: receiver[method] is not a function" but we expected it to throw object "getter: throwing from getter"

OK /IndexedDB/key-conversion-exceptions.any.worker.html (#39284)

FAIL [expected PASS] subtest: IDBCursor continue() method with throwing/invalid keys

assert_throws_exactly: key conversion with throwing getter should rethrow function "() =&gt; {
      receiver[method](key);
    }" threw object "TypeError: receiver[method] is not a function" but we expected it to throw object "getter: throwing from getter"

FAIL [expected PASS] /_mozilla/mozilla/sslfail.html (#10760)
TIMEOUT [expected OK] /_mozilla/mozilla/window_resize_event.html (#36741)
- TIMEOUT [expected PASS] subtest: Popup onresize event fires after resizeTo
```
Test timed out
```
OK /css/css-cascade/layer-font-face-override.html (#35935)
- FAIL [expected PASS] subtest: @font-face override update with appended sheet 1
```
assert_equals: expected "80px" but got "38.3166666666667px"
```
- FAIL [expected PASS] subtest: @font-face override update with appended sheet 2
```
assert_equals: expected "80px" but got "38.3166666666667px"
```
OK /custom-elements/form-associated/ElementInternals-setFormValue.html (#29174)
- PASS [expected FAIL] subtest: Single value - name is missing
TIMEOUT [expected FAIL] /dom/xslt/large-cdata.html (#38029)
OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
- FAIL [expected PASS] subtest: sec-fetch-user
```
promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
```
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Same site
OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
- PASS [expected FAIL] subtest: Check execution order on load handler
- PASS [expected FAIL] subtest: Check execution order from nested timeout
OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/autofocus-dialog.html (#29087)
TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_other_frame_popup.sub.html (#39702)
- TIMEOUT [expected FAIL] subtest: Sandboxed iframe can not navigate other frame's popup
```
Test timed out
```
OK /navigation-timing/test-navigation-type-reload.html (#33334)
- FAIL [expected PASS] subtest: Reload domComplete > Original domComplete
```
assert_true: Reload domComplete &gt; Original domComplete expected true got false
```
- FAIL [expected PASS] subtest: Reload domContentLoadedEventEnd > Original domContentLoadedEventEnd
```
assert_true: Reload domContentLoadedEventEnd &gt; Original domContentLoadedEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload domContentLoadedEventStart > Original domContentLoadedEventStart
```
assert_true: Reload domContentLoadedEventStart &gt; Original domContentLoadedEventStart expected true got false
```
- FAIL [expected PASS] subtest: Reload fetchStart > Original fetchStart
```
assert_true: Reload fetchStart &gt; Original fetchStart expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventEnd > Original loadEventEnd
```
assert_true: Reload loadEventEnd &gt; Original loadEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventStart > Original loadEventStart
```
assert_true: Reload loadEventStart &gt; Original loadEventStart expected true got false
```

OK /preload/preload-error.sub.html (#37177)

PASS [expected FAIL] subtest: success (fetch): main

FAIL [expected PASS] subtest: 404 (fetch): main

assert_greater_than: http://web-platform.test:8000/preload/resources/dummy.xml?pipe=status%28404%29&amp;label=fetch should be loaded expected a number greater than 0 but got 0

PASS [expected FAIL] subtest: CORS (fetch): main

OK /trusted-types/trusted-types-navigation.html?01-05 (#38975)
- PASS [expected FAIL] subtest: Navigate a window via anchor with javascript:-urls in enforcing mode.
- PASS [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls in enforcing mode.
OK /trusted-types/trusted-types-navigation.html?21-25 (#38997)
- PASS [expected FAIL] subtest: Navigate a window via form-submission with javascript:-urls in enforcing mode.
OK /trusted-types/trusted-types-navigation.html?26-30 (#38807)
- PASS [expected FAIL] subtest: Navigate a frame via form-submission with javascript:-urls in enforcing mode.
TIMEOUT [expected OK] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.html (#29053)
- TIMEOUT [expected PASS] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe
```
Test timed out
```

github-actions · 2025-10-19T22:33:48Z

✨ Try run (#18636760458) succeeded.

TimvdLippe

Some nits at a glance. The timer implementation looks fine, but it is difficult to judge for me as I am not familiar with that code yet

Since there are no WPT changes, can we add the AbortSignal.timeout implementation as well? I expect that to be minimal and then we also have test coverage for this new code.

TimvdLippe · 2025-10-20T04:56:24Z

components/script/timers.rs

            suspended_since: Cell::new(None),
            suspension_offset: Cell::new(Duration::ZERO),
            expected_event_id: Cell::new(TimerEventId(0)),
+            runsteps_active: DomRefCell::new(FxHashMap::default()),


Nit: Default::default() for the whole expression (same on next line)

Good point.

TimvdLippe · 2025-10-20T04:57:33Z

components/script/timers.rs

    }

+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    /// Step 2. Let startTime be the current high resolution time given global.


Nit: move spec comment in the method

Good point.

TimvdLippe · 2025-10-20T05:00:56Z

components/script/timers.rs

+type OrderingIdentifier = DOMString;
+
+// Per-ordering queue entry: (deadline_ms, sequence, handle)
+type OrderingEntry = (u64, u64, OneshotTimerHandle);


Nit: make this a struct with named fields, so it is easier to distinguish the two u64

TimvdLippe · 2025-10-20T05:04:57Z

components/script/dom/globalscope.rs

    }
+
+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    pub(crate) fn run_steps_after_a_timeout<F>(


I don't see any usages of this method. Why isn't it marked as unused and does Clippy fail on it?

you can check #40032

Taym95 · 2025-10-20T07:41:12Z

Some nits at a glance. The timer implementation looks fine, but it is difficult to judge for me as I am not familiar with that code yet

Since there are no WPT changes, can we add the AbortSignal.timeout implementation as well? I expect that to be minimal and then we also have test coverage for this new code.

Yeah @gterzian has context and he can review, you don't need to review PR if you don't have full context(we have something about this in PR review guideline), this is incremental work AbortSignal.timeout will be next.

gterzian

LGTM as a first step with some comments.

gterzian · 2025-10-21T08:35:54Z

components/script/timers.rs

+    runsteps_queues: DomRefCell<OrderingQueues>,
+
+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    /// Step 1. Let timerKey be a new unique internal value.


I think just <html.spec.whatwg.org/multipage/#timers:unique-internal-value-5> is better

gterzian · 2025-10-21T08:39:09Z

components/script/timers.rs

    expected_event_id: Cell<TimerEventId>,
+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    /// Step 3. Set global's map of active timers[timerKey] to startTime plus milliseconds.
+    runsteps_active: DomRefCell<RunStepsActiveMap>,


https://html.spec.whatwg.org/multipage/#map-of-active-timers

I think the name should be like in the spec, and we should add a todo that this should also be used for the other timers as per html.spec.whatwg.org/multipage/#map-of-settimeout-and-setinterval-ids.

gterzian · 2025-10-21T08:40:03Z

components/script/timers.rs

+type OrderingIdentifier = DOMString;
+
+// Per-ordering queue entry: (deadline_ms, sequence, handle)
+type OrderingEntry = (u64, u64, OneshotTimerHandle);


gterzian · 2025-10-21T08:41:32Z

components/script/timers.rs

+type CompletionStep = Box<dyn FnOnce(&GlobalScope, CanGc) + 'static>;
+
+// OrderingIdentifier per spec ("orderingIdentifier")
+type OrderingIdentifier = DOMString;


here it's worth adding a link to the algo.

gterzian · 2025-10-21T08:42:42Z

components/script/timers.rs

    RefreshRedirectDue(RefreshRedirectDue),
+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    RunStepsAfterTimeout {
+        // Step 1. timerKey


I think this should be /// also

gterzian · 2025-10-21T08:43:28Z

components/script/timers.rs

            suspended_since: Cell::new(None),
            suspension_offset: Cell::new(Duration::ZERO),
            expected_event_id: Cell::new(TimerEventId(0)),
+            runsteps_active: DomRefCell::new(FxHashMap::default()),


Good point.

gterzian · 2025-10-21T08:44:03Z

components/script/timers.rs

    }

+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    /// Step 2. Let startTime be the current high resolution time given global.


Good point.

gterzian · 2025-10-21T08:52:28Z

components/script/dom/globalscope.rs

    }
+
+    /// <https://html.spec.whatwg.org/multipage/#run-steps-after-a-timeout>
+    pub(crate) fn run_steps_after_a_timeout<F>(


This should end-up being used in the other timer mechanism as well, so we should add a todo to integrate as per https://html.spec.whatwg.org/multipage/#timers:run-steps-after-a-timeout

gterzian · 2025-10-21T08:55:43Z

components/script/timers.rs

-            let callback = timer.callback;
-            callback.invoke(global, &self.js_timers, can_gc);
+            match &timer.callback {
+                OneshotTimerCallback::RunStepsAfterTimeout { ordering_id, .. } => {


So this should end-up turned inside out, where the "normal" timers are just using the "run steps after timeout" mechanism and an ordering id of "setTimeout/setInterval". Let's file an issue for that.

The current implementation means that you could actually just use SetTimeOut, but the way the spec works is rather that this is the general mechanism, and SetTimeOut is a specific use of it.

gterzian · 2025-10-21T08:57:50Z

components/script/timers.rs

+                    let is_head = head_handle_opt.is_none_or(|head| head == timer.handle);
+
+                    if !is_head {
+                        let rein = OneshotTimer {


Not sure about this re queuing, but this would go away if we revisit timers as I mentioned in my other comments.

Signed-off-by: Taym Haddadi <[email protected]>

Implement abortsignal timeout Testing: should pass abortsignal timeout wpt test. Part of #36936 on top of #39994

Taym95 requested a review from gterzian as a code owner October 19, 2025 21:57

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 19, 2025

Taym95 force-pushed the Implement-run-steps-after-a-timeout branch from e0afe9c to da86cbf Compare October 19, 2025 22:05

Taym95 added the T-linux-wpt Do a try run of the WPT label Oct 19, 2025

github-actions bot removed the T-linux-wpt Do a try run of the WPT label Oct 19, 2025

TimvdLippe reviewed Oct 20, 2025

View reviewed changes

TimvdLippe mentioned this pull request Oct 20, 2025

Make components/script codeowners more granular #40026

Open

Taym95 force-pushed the Implement-run-steps-after-a-timeout branch from da86cbf to 5658982 Compare October 20, 2025 16:08

Taym95 mentioned this pull request Oct 20, 2025

Implement abortsignal timeout #40032

Merged

Taym95 force-pushed the Implement-run-steps-after-a-timeout branch from 5658982 to 48a0045 Compare October 20, 2025 16:11

Taym95 mentioned this pull request Oct 20, 2025

Implement advanced features of AbortSignal #36936

Closed

4 tasks

gterzian approved these changes Oct 21, 2025

View reviewed changes

servo-highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 21, 2025

gterzian reviewed Oct 21, 2025

View reviewed changes

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. and removed S-needs-rebase There are merge conflict errors. labels Oct 21, 2025

Implement run steps after a timeout

7c02abf

Signed-off-by: Taym Haddadi <[email protected]>

Taym95 force-pushed the Implement-run-steps-after-a-timeout branch from 826eb29 to 6259646 Compare October 21, 2025 20:16

servo-highfive removed the S-needs-rebase There are merge conflict errors. label Oct 21, 2025

Taym95 enabled auto-merge October 21, 2025 20:16

Taym95 disabled auto-merge October 21, 2025 20:19

Taym95 mentioned this pull request Oct 21, 2025

Unify timers to use “Run Steps After a Timeout” with ordering IDs (setTimeout / setInterval..) #40060

Open

Apply reviewer feedback (naming, docs, small refactors)

4530705

Signed-off-by: Taym Haddadi <[email protected]>

Taym95 force-pushed the Implement-run-steps-after-a-timeout branch from 6259646 to 4530705 Compare October 21, 2025 20:23

Taym95 enabled auto-merge October 21, 2025 20:23

Taym95 added this pull request to the merge queue Oct 21, 2025

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 21, 2025

Merged via the queue into servo:main with commit 405e1ec Oct 21, 2025
30 checks passed

Taym95 deleted the Implement-run-steps-after-a-timeout branch October 21, 2025 21:33

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 21, 2025

github-merge-queue bot pushed a commit that referenced this pull request Oct 22, 2025

Implement abortsignal timeout (#40032)

6357fc8

Implement abortsignal timeout Testing: should pass abortsignal timeout wpt test. Part of #36936 on top of #39994

Uh oh!

Conversation

Taym95 commented Oct 19, 2025

Uh oh!

github-actions bot commented Oct 19, 2025

Uh oh!

github-actions bot commented Oct 19, 2025

Uh oh!

github-actions bot commented Oct 19, 2025

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Taym95 commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gterzian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gterzian Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gterzian Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Taym95 commented Oct 20, 2025 •

edited

Loading

gterzian Oct 21, 2025 •

edited

Loading

gterzian Oct 21, 2025 •

edited

Loading