Repository navigation
fix(interaction-harness): WebKit busy proofs date frames on the controls - #10772
Conversation
WebKit stamps the frame after a delivery gap about 60 ms late, carrying one frame of motion. A busy proof on the stamps then reads a stall the control does not show. A WebKit busy proof with a control now dates each frame from the controls' positions. A frame with no reading keeps its stamp's distance from the nearest frame whose stamp agrees with its reading. The dated frames keep the cast's order, each at its own time, and the proof reports its judged part on the cast's stamps. A proof that judges every frame near the control's turns opts in to the control ahead. Chromium casts and casts with no control keep their stamps. Each cast frame records its engine for this.
WebKit's compositor can draw two frames 6 to 9 ms apart. A curve then shows no new step between them, and a busy proof read a held frame. A WebKit busy proof now keeps one dated frame per display interval, the earlier one, read to one px of the controls' travel. A frame dated from a neighbour always stays, so no held frame is left out. A WebKit cast rarely shows two frames one interval apart, so the proof judges no 30 Hz cadence there. Its summary says so in a note, with the count of such pairs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Switch · Storybook | 103.9 kB (0) · 9.9 kB CSS (0) | 1,734 (0) | ✓ · ✓ · ✓ |
Package size: no published package changed; no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 29 rows, core 36 rows, theme-butter 1 row, theme-y2k 1 row; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: the WebKit frame dating is sound and its new cases fail on the base and pass here, but one shared-path fix is needed in this pull request.
- The Switch track-colour proof still dates WebKit frames with its own
controlPageTime,atDisplayRateanddisplayIntervalPairs(packages/core/src/Switch/__tests__/Switch.interaction.browser.spec.ts, lines 2368-2444), so the harness now dates that proof a second time by a rule that already differs; please move it ontopooledBusyProofhere by passing the raw cast frames withaheadandcontrolStartsin eachBusyRun, deleting the local copy and its cadence annotation, and tagging P10's fixture framesengine: 'webkit'.
Non-blocking: the judged wording noted inline still applies, and no case fails when the ceiling pass in controlPageTimes (busy.ts, lines 446-455) is removed. After the fix, rebase onto main, where busy.ts now conflicts after #10765. CI has not run on this head yet.
[Automated review]
cixzhang
added
the
ci:run
|
/approve-spec 3008e89 |
Resolves the one conflict with meta#10765 in pooledBusyProof's options: the Omit list keeps both additions.
The Switch track-colour busy proof dated WebKit frames itself, with its own reading, display-rate and cadence helpers. With the harness now dating every WebKit busy proof with a control, two rules ran on it. It now passes both controls and their starts to pooledBusyProof, which dates and keeps its frames. The harness exports pageTimesOnControls, the readings its dating starts from, for checks that need a frame's own time: the colour's landing and the thumb's curve. The replay of two late-stamped WebKit casts tags its frames as WebKit's: dated by the harness it passes, while their stamps and their readings alone fail.
|
/approve-spec f219e23 |
There was a problem hiding this comment.
Requesting changes: the WebKit dating and the Switch track-colour proof's move onto the harness look right, but one copy of the controls' page-time reading remains outside the new shared helper.
- Miss:
packages/core/src/motion/__tests__/animator.interaction.browser.spec.ts(lines 198-216 and 907-922) still computes each frame's page time from the two controls inline, which is exactly what the newpageTimesOnControlsreturns, so please call it at both sites and delete the inline readers so that a later fix to the harness's reading also reaches the animator proof.
Non-blocking: no case fails when the order rule in controlPageTimes (busy.ts, lines 482-495) is removed, ceiling and floor passes alike; a case whose verdict or judged selection changes when two dated frames swap or share a time would pin it. The animator's 1 s block proof could also pass its second control as ahead (line 173). CI has not run on this head yet.
[Automated review]
| let ceiling = Number.POSITIVE_INFINITY; | ||
| for (const stamp of [...order].reverse()) { | ||
| const own = read.get(stamp); | ||
| if (own == null) { | ||
| times.set(stamp, Math.min(times.get(stamp) ?? stamp, ceiling)); | ||
| } else { | ||
| ceiling = own; | ||
| } | ||
| } | ||
| let floor = Number.NEGATIVE_INFINITY; | ||
| for (const stamp of order) { | ||
| floor = Math.max(times.get(stamp) ?? stamp, floor + DATED_GAP_MS); | ||
| times.set(stamp, floor); | ||
| } |
There was a problem hiding this comment.
Non-blocking: removing both passes (this ceiling pass and the floor pass after it) leaves every case in busy.webkitclock.test.ts and the Switch P10 replay green. The measured-cast and held-frame tests return identical summaries without them, although their casts have 3 and 1 out-of-order times before these passes. A case whose verdict changes when two dated frames swap or share a time would pin the rule.
There was a problem hiding this comment.
Added in 941311f: a late-burst case where frame 27, with no reading, would date after frame 28. Removing the floor or the ceiling pass alone turns it red; removing both leaves it green. With no clamp there is no tie. The judge sorts both series by the same times, so the component and its control swap together.
The animator's spring and retarget proofs read each frame's page time from the two controls inline: the reading pageTimesOnControls returns. They now call it, so a fix to the harness's reading reaches them too. A unit case pins each of the order rule's two passes, not their removal together. In a late burst, a held frame with no reading moves to just before the next frame with one, and still reads as a stall.
|
/approve-spec 941311f |
There was a problem hiding this comment.
Approved pending CI: the animator's spring and retarget proofs now read their frame times through pageTimesOnControls with identical results, so the last shared-path fix is in, and the WebKit dating and the Switch move still look right. CI has not run on this head yet. Every job skipped because the pull request is a draft, and CI runs once it is marked ready for review.
Non-blocking:
- The new late-burst case pins each order-rule pass on its own, but removing both passes in
controlPageTimes(busy.ts, lines 482-495) still leaves every unit case green, so "the dated frames keep the cast's order" stays untested as a whole; a case where unordered dating changes a verdict, or a narrower README sentence, would settle it. - The animator's 1 s block proof (
animator.interaction.browser.spec.ts, line 174) could pass its second control asahead, so that a late WebKit frame near the first control's turn cannot read as a stall.
[Automated review]
Maintainer impact
WebKit busy-main-thread proofs read stalls their control does not show, and fail intermittently in the WebKit CI job. WebKit's screencast stamps the frame after a delivery gap about 60 ms late, carrying one frame of motion. Its compositor can also draw two frames 6 to 9 ms apart, with no new step between them. #10758 worked around both inside the Switch track-colour proof; this moves the fix into the harness, that proof included.
Intended invariant
A busy proof judges each presented frame at the time the compositor drew it (README: busy-main-thread proofs). The busy control's position reads that time; other casts are judged as before.
Change
startBusyControl(page, {ahead: true})). Without it, a late frame there keeps its lateness.judgedstays on the cast's stamps, so specs select judged frames by them.pageTimesOnControlsexports the readings the dating starts from. In Chromium that proof reads stamps again, as before test(core): the Switch's colour busy proof dates frames on the controls #10758.pageTimesOnControls, with the same readings.Evidence
main. Among them: a late frame after a gap, or at a turn.Scope
Testing
In WebKit under CI's container profile (4 CPUs) unless noted:
mainb01b7ee. The 22 WebKit busy-proof spec files ran order-balanced (main, change, change,main) at 4 workers.mainfailed 15 and 17 tests, and the dating 11 and 12. Every difference is a load failure (input pacing, frame starvation, a page-timed read or a timeout), in both directions. None is a misread position.main09b4ff9, the 20 control-busy spec files ran with the two commits interleaved, at 2 workers. The first commit failed 4 tests, both commits 3. Each is pacing, starvation, a page-timed read or one known load-fragile Lightbox proof.mainfa30cff, the Switch track-colour proof ran with eight busy loops in the container, interleaved withmain. It passed 10 of 10 in WebKit and in Chromium, asmaindid. Without the loops: WebKit 10 of 10 on both, and Chromium on a 2-core pin 10 of 10 on both.main: 10 of 10 on both.pageTimesOnControls, in Chromium: 3 of 3 on both.main's), the harness and core typechecks and Prettier pass.