Repository navigation
test(interaction-harness): two page-freeze cases wait for the frame - #9873
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft from a fork, so CI starts once it is marked ready for review and a maintainer adds the ci:run label.
Both page-freeze cases now wait for the engine's own signal (finished, ready) instead of a fixed timeout, and they keep the claims they proved before. With the page's first frame held back, the old forms fail and the new ones pass.
- Non-blocking: in Chromium the finish check at line 4814 now passes by about 1 ms whenever the finishing frame lands exactly on start + 200 ms, because
frozenAtis read as soon asfinishedresolves. Awaiting one more frame inside the same evaluate (await new Promise(requestAnimationFrame);) would restore a frame of margin.
[Automated review]
| [{transform: 'translateX(0px)'}, {transform: 'translateX(100px)'}], | ||
| {duration: 200}, | ||
| ); | ||
| ).finished; |
There was a problem hiding this comment.
Non-blocking: frozenAt (line 4806) is now read as soon as finished resolves. The recorder dates the finish at start + 200 ms, and in Chromium that is often exactly the finishing frame's time. So the check at line 4814 (finishedAt < frozenAt, with frozenAt a whole millisecond) passes by about 1 to 2 ms there. One more frame before returning keeps the ordering this change wants and adds a frame of margin whatever the load:
await el.animate(/* … */).finished;
await new Promise(requestAnimationFrame);
cixzhang
added
the
ci:run
Two page-freeze self-test cases assumed that a fixed wait covered the frame that starts a run. A busy machine can draw no frame for longer than the wait, and both cases failed once that way in a full Chromium run. The run that finishes before the freeze had not started when the freeze landed, so it was dated after the freeze (487 ms). The ten-cycle case read the run's start time before any frame, got null, and so measured a drift of the run's whole start (1224 ms). The first case now waits for the run's own end before it freezes, and the second reads the start time once the run is ready.
5212d64 to
8ed6edc
Compare
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. Its admission step found no room under the CI ceiling, so the required checks report failure without having run; a maintainer re-runs all jobs, or a later push starts a new run.
Both page-freeze cases now wait for the engine's own signal (finished, ready) instead of a fixed timeout, and they keep the claims they proved before. With the page's first frame held back, the old forms fail and the new ones pass.
- Non-blocking: the finish check at line 4821 passes by about 1 ms in roughly half of Chromium runs, because
frozenAtis read as soon asfinishedresolves; awaiting one more frame inside the same evaluate (await new Promise(requestAnimationFrame);) would restore a frame of margin.
[Automated review]
Maintainer impact
Two page-freeze self-test cases failed once in a full Chromium interaction run on a loaded machine (1 worker, load average 69 to 86). "A run that finished before a freeze…" dated the run 487 ms after the freeze. "…ten freeze and thaw cycles" measured a drift of 1,224 ms. Side-by-side reruns passed, so the red read as a one-off, but it is a real dependency on the machine's load.
#9701 still carries an older copy of this section; its rebase takes
main's.Intended invariant
Each page-freeze case proves the same claim as before, whatever the delay before the page's next frame.
Change
finishedbefore it readsfrozenAtand freezes, instead of waiting 400 ms.startTimeoncerun.readyresolves, instead of after 100 ms. Read before any frame, a pending run's start time is null, whichNumber()reads as 0. The drift then measured the run's whole start.Evidence
mainstops the page drawing frames after each run starts. A busy loop on the harness's real clock holds the main thread for 500 ms in the first case and 300 ms in the second. Both cases fail 3 of 3, the same way as the loaded run: the run dated 350 ms or more after the freeze, and a drift of about 350 ms. With each case's fixed wait at 0 instead (3138a9a), the first case fails 3 of 3 and the second 1 of 3. A CPU quota alone (1 CPU beside 10 busy loops) did not delay frames enough to reproduce it:mainpassed 20 of 20.Scope
Testing
Rebased onto
mainbd69bed. The file's import oftestnow comes frommain's@astryxdesign/a11y-spec/playwright, and this change's own lines are the same.At 8ed6edc:
CI=true) over the whole repository: no errors;pnpm review:pre: no findings.At 5212d64, on
main1800b47:--cpus=4,CI=1, 1 worker): 95 passed, 13 Chromium-only skipped;pnpm review:pre: no findings.