Repository navigation
test(usePager): judge the report two frames into the settle, not 20 ms - #10778
Conversation
The callbacks test checked that a release's report found the settle more than 20 ms past the browser's start. The pager reports two frames after the frame that the settle started in. Chromium dates that start anywhere inside its frame, up to about 15 ms in under load. So the report can find the settle 18-20 ms in, and the check failed. It now counts the page's frames from the browser's start to the report, and asks for two. A report one frame early still fails it.
|
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 ·
|
cixzhang
added
the
ci:run
|
/approve-spec 5db6ef4 |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: the claim that the new check catches an early report holds only in Chromium, so the description and commit message need to say so (or the WebKit count needs a fix).
- The description says the new check "still fails when the report comes early" and the commit message says "A report one frame early still fails it", but in WebKit the settle's start frame is stamped after the animation's
startTime, so the count atusePager.interaction.browser.spec.ts:181-182includes that frame and a pager that reports one frame early still gets:moving(it passes 10 of 10 runs in WebKit and fails 10 of 10 in Chromium); either say in the description, the commit message and one line of the fixture comment that the early-report proof is Chromium's (the unit tests atusePager.test.ts:588and:605cover WebKit's frame order), or make the WebKit count skip the start's own frame.
The Chromium fix itself holds up: counting frames from the browser's start matches the pager's documented two-frame timing, and the whole spec file passes in both engines. The WebKit gap is not new, since the old 20 ms bar passed the same early report.
[Automated review]
The report check counted frames from the browser's own start. WebKit stamps the frame that starts an animation at or after the start, so the check counted that frame too, and a report one frame early passed. In Chromium, a preempted main thread runs the frame that commits the settle late, with a stale stamp. Chromium then dates its start a frame later, so the report came one frame after it, and the check failed. The check now counts from the frame that the pager counts from: in WebKit, the frame that its start falls in, and in Chromium, the first frame whose callbacks run after the settle is made.
|
/approve-spec 0210354 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet because the pull request is a draft, and it runs once the pull request is marked ready for review.
The earlier WebKit gap is fixed: the check no longer counts the settle's own start frame in WebKit, so a pager that reports one frame early now fails the callbacks case in both engines, while the shipped two-frame timing passes in both. The whole spec file also passes in Chromium and WebKit.
[Automated review]
Maintainer impact
One test in
usePager.interaction.browser.spec.tssometimes fails in Chromium on a loaded runner. Its title starts "callbacks: the new page once the settle is moving". The report arrives asindex:2:drag, without:moving. On main fa30cff, with 4 workers on 2 pinned cores, it failed 6 of 420 runs.Intended invariant
createPager's documented timing does not change:onIndexChangereports a release's page "once its settle runs on the compositor, two frames after the frame it started in."Change
The fixture marked a report
:movingwhen a running animation hadcurrentTime > 20ms. Chromium dates the start anywhere inside its frame, up to about 15 ms in. The pager reports two frames after the start's frame, so the report can find the settle only 18–20 ms in.The fixture now counts frames instead of milliseconds, from the frame the pager counts from. The browser must also have told the settle's start. In Chromium, the pager counts from the frame that commits the settle (
animator.ts: "the call starts at the frame that commits it"). That is the first frame whose callbacks run after the settle is made. A preempted main thread can run that frame late with a stale stamp, and Chromium then dates its own start a frame later. WebKit stamps its start frame at or after the start, so the count starts there.This is a cause fix, not a loosening: the old bar measured the wrong quantity, and a report one frame early fails in both engines.
Evidence
currentTime18.0–19.7 ms, under the 20 ms bar.usePager.tschanged to report one frame early (afterFramesPastStart(1, …)). The check fails it 10 of 10 in each engine.Scope
Testing
WebKit runs the same filter through
internal/interaction-harness/scripts/webkit-container.mjswithCI=1 --repeat-each=20 --workers=1.