Skip to content

test(SnapPaging): edge-move touches land on the scene's clock - #11068

Merged
cixzhang merged 1 commit into
meta:mainfrom
vjeux:snap-paging-edge-move-catch
Nov 10, 2026
Merged

cixzhang merged 1 commit into
meta:mainfrom
vjeux:snap-paging-edge-move-catch

Conversation

@vjeux

@vjeux vjeux commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Maintainer impact

On a starved runner, three SnapPaging interaction cases failed with raw assertions or a plain freeze timeout. Two are touch cases during an edge button's move, with a 150 ms and a 40 ms hold. The third checks that the scroll offset holds through a swipe. A raw failure reads as a product red, though such a run proves nothing about the product.

Intended invariant

On a runner with CI's CPU limits, the cases prove what they proved. A run whose input or frames the runner held back fails by name, as the harness's LiftPlacementError, FrameStarvationError and touch pacing contracts state.

Change

  • The edge-move touch cases sent their tap after the thaw. A loaded runner's round trips could deliver it after the move ended. The row then rested on the move's target, and the case failed raw. The touch-down is now held to 100 ms into the move on the scene's clock (holdInputUntil). One that reaches the page later fails as LiftPlacementError.
  • The press-in-settle freeze now logs the row on every frame it checks. A row that crossed its freeze window between two frames more than the starvation gap apart fails as FrameStarvationError, through requireFrames. Any other timeout fails as before.
  • The swipe case judges the release the page reads against the advance speed, as the file's swipe helper does. A release read on the other side fails by name.

Evidence

  • Failure before the change, on main's specs: 32 raw failures in 60 WebKit runs under load (Testing).
  • Success after the change: no raw failure in 60 WebKit runs under the same load. On CI's profile, every case passes in both engines.
  • Product/runtime behavior verified unchanged: no product file changes.
  • CI impact: none; no workflow, action or CI script changes.

Scope

Components: SnapPaging

  • No intended public API, product behavior, visual direction, or policy change.
  • Product changes discovered during the work were removed or split.
  • Public text and artifacts contain no internal Meta context.

Testing

  • The load: WebKit ran in its Playwright container, limited to 2 CPUs on one pinned pair. Two busy loops shared that pair, with 4 workers.
  • All runs used main's build at d130dff, with main's specs or this head's (e1ed32e). The runs predate one comment edit. Each case ran 20 times with each.
  • The swipe case: main's specs 8 raw failures (a resting offset), 9 passes and 3 named. This head's: 18 passes and 2 TouchPageStallError.
  • The 150 ms touch during an edge button's move: main's specs 13 raw failures and 7 passes. Each raw failure is a freeze timeout or a row on the move's target. This head's: 14 LiftPlacementError and 6 FrameStarvationError.
  • The 40 ms touch: main's specs 11 raw failures and 9 passes. This head's: 17 LiftPlacementError and 3 FrameStarvationError.
  • Under this load, the touch reached the page 36 to 602 ms after its time. So no edge-move touch run could pass, and each says so by name.
  • On CI's profile at 1 worker: the swipe case and every press-in-settle case pass 3 of 3 in each engine.
  • Strict ESLint reports 0, and Prettier is clean on the changed file. Core tsc reports 0 errors. No comment sentence on the changed lines runs over 25 words.

During an edge button's move, the touch cases sent their tap after the
thaw. A loaded runner's round trips could deliver it after the move
ended, so the row rested on the move's target and the case failed raw.
The touch-down is now held to 100 ms into the move on the scene's
clock. One that reaches the page later fails as LiftPlacementError.

The press-in-settle freeze logs the row on every frame it checks. A row
that crossed its freeze window between two frames more than the
starvation gap apart fails as FrameStarvationError.

The swipe case judges the release the page reads against the advance
speed, as the file's swipe helper does.
@vercel

vercel Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 10, 2026 6:24pm UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 10, 2026
@astracat-bot

astracat-bot Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 10, 2026, 6:41 PM UTC

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · e1ed32e

Storybook · Sandbox · built from this PR's head

Package size: no published package changed; no new dependencies.

API changes: none in this pull request

Committed API report behind this build: cli 36 rows, core 36 rows; run pnpm api:report.

Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI has not run on this head yet, because the pull request is a draft from a fork. Next step: a maintainer adds the ci:run label and the pull request is marked ready for review, which starts CI.

The change is test-only. The edge-move touch cases now land at a fixed point of the move on the scene's clock, and a late touch or starved frames now fail as the harness's named errors (LiftPlacementError, FrameStarvationError, touch pacing errors) instead of as raw assertions.

  • Non-blocking: logShown keeps sampling layout on every frame, for up to 2,000 frames, after the freeze it serves has landed. Stopping it once the freeze lands would keep that work out of the frames this change protects.

[Automated review]

el.getBoundingClientRect().left -
items[0].getBoundingClientRect().left,
});
if (log.length < 2000) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: this loop re-arms on every frame until it holds 2,000 entries, but only freezeWhenHeldIn's catch path reads the log. After freezePageWhen resolves, the loop keeps doing two getBoundingClientRect reads per frame through the edge press, the thaw, the move and the touch: main-thread work in the frames this change protects on a starved runner. You could stop it once the freeze lands, for example with if (log.length < 2000 && !freeze?.frozen) (the install exposes frozen), or with a flag set after freezePageWhen resolves.

@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 10, 2026
@vjeux
vjeux marked this pull request as ready for review November 10, 2026 20:12
@cixzhang
cixzhang merged commit 2569989 into meta:main Nov 10, 2026
127 checks passed
@vjeux
vjeux deleted the snap-paging-edge-move-catch branch November 10, 2026 20:51

This branch was successfully deployed

1 active deployment
Preview — e1ed32ec Deployed Nov 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants