Skip to content

test(interaction-harness): a late hand-off reads the caught start - #10888

Merged
cixzhang merged 3 commits into
meta:mainfrom
vjeux:bank/busy-late-handoff-default
Nov 10, 2026
Merged

cixzhang merged 3 commits into
meta:mainfrom
vjeux:bank/busy-late-handoff-default

Conversation

@vjeux

@vjeux vjeux commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Maintainer impact

Under load, Chromium presents a just-started compositor animation late: in SliderTouch's coast, 25–111 ms after its start (median 57 ms). Held frames inside the judged part read as misses the control does not show. On main 738919b the proof failed: "moved on 6 of the 7 frames the control moved".

Intended invariant

A busy proof judges every frame after the animation's first presented change. Only a hold that starts before the block, at the value the animation starts from, is a late hand-off. A main-thread animation still fails.

Change

setBeforeBlock names that value. Without it, a window that caught an animation (blockMainThread's during) takes the value on the cast's last frame before that start. An explicit setBeforeBlock still wins. No threshold, floor, minJudgedFrames or other late-start rule changes.

Nothing is counted twice. The late-start rule runs before any frame is judged. A prediction (#10812) judges only from the first presented change. A Chromium cast with a control is dated first (#10887). Without the controls' starts, its dated frames keep the cast's lag, so a release shown within it gets no excuse. A held frame dated before the judged part is not judged.

Evidence

  • Failure before the change, on main eee8f1d: 161 recorded Chromium casts (SliderTouch under load), judged both ways on the same frames. One start-up hold at the release position turns fail to pass. Another already passes, dated before the judged part.
  • Excuse boundary, on main 738919b with the value passed explicitly: 80 casts held into the judged part all fail plain; 79 pass excused. The 80th fails on a slow-tail miss ("moved on 7 of the 8 frames the control moved"). A hold 3 px away, and a freeze after the first move, fail all 80 both ways.
  • Success after the change, at 7b57978 with 1 worker, across the 19 specs that block with during:
    • Every proof call, judged both ways, keeps its verdict: 65 in Chromium at CI's profile, 64 under load and 42 in WebKit.
    • The frozen and main-thread mutants still fail at CI's profile: 10 in Chromium, 8 in WebKit (2 Chromium-only).
  • Pins in busy.control.test.ts: the default equals the explicit value; a mismatched value, a main-thread mover and a moved-to hold still fail. The positive pins fail on the base.
  • Product/runtime behavior verified unchanged: the diff touches only the harness and its README. Units pass (293); tsc is clean.
  • CI impact: none.

Scope

  • 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

pnpm exec vitest run internal/interaction-harness/src
CI=1 pnpm exec playwright test --config playwright.interaction.config.ts --project=chromium <the 19 specs> -g 'main.thread|blocked|busy' --workers=1

WebKit runs the same specs through internal/interaction-harness/scripts/webkit-container.mjs with CI=1 --workers=1.

@vercel

vercel Bot commented Nov 9, 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 1:02am UTC

Request Review

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

astracat-bot Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 10, 2026, 1:08 AM UTC

@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 7b57978

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 29 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. The pull request is a draft, so marking it ready for review starts CI; it already carries ci:run.

The new default is narrow and well pinned: both positive cases fail against the base harness and pass here, an explicit setBeforeBlock still wins, and a main-thread mover still fails.

Optional follow-up: busy.ts now reads the value before the caught animation's start in two places with slightly different rules (the progress replay's anchor and this default), and one shared helper would keep them aligned.

[Automated review]

@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 9, 2026
A busy proof excuses a hold that starts before the block and ends at the
first presented change inside it, when the hold sits at the value the
page set before the block (`setBeforeBlock`). Without that value, a
window that caught an animation now takes the value on the cast's last
frame before that animation's start. Chromium on a loaded runner also
presents a just-started compositor animation late: a slider's coast
first showed 25-111 ms after its start, with a catch-up on that frame.
An explicit `setBeforeBlock` still wins. A hold at another value, a
freeze after the first change and a main-thread mover still fail.
@vjeux
vjeux force-pushed the bank/busy-late-handoff-default branch from 7b0bf5a to 2a3a5e6 Compare November 9, 2026 22:29

@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. The pull request is a draft, so marking it ready for review starts CI; it already carries ci:run.

No blocking findings. The new default stays narrow: an explicit setBeforeBlock still wins, a hold at any other value or a frame without the element gets no excuse, and every frame after the first change is still judged.

[Automated review]

@cixzhang

cixzhang commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

/approve-spec f0d7462

@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. The pull request is a draft, so marking it ready for review starts CI; it already carries ci:run.

No blocking findings. The merge from main leaves the patch unchanged, and the new default stays narrow: an explicit setBeforeBlock still wins, and a hold at any other value or a frame without the element gets no excuse.

[Automated review]

@cixzhang

Copy link
Copy Markdown
Contributor

/approve-spec 7b57978

@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. The pull request is a draft, so marking it ready for review starts CI; it already carries ci:run.

No blocking findings. The merge from main leaves the patch unchanged, and the new default stays narrow: an explicit setBeforeBlock still wins, and a hold at any other value gets no excuse.

  • Small wording fix for the description: "the default reads dated frames, on the start's clock" holds only when a proof dates its cast from the controls' starts (a replay with an ahead control, or controlStarts); SliderTouch's coast proof does neither, so its frames keep the cast's stamp lag and the default reads a frame at least that lag before the start, withholding the excuse when the release shows within that lag.

[Automated review]

@vjeux
vjeux marked this pull request as ready for review November 10, 2026 01:44
@cixzhang
cixzhang merged commit 8e8bf91 into meta:main Nov 10, 2026
126 checks passed
@vjeux
vjeux deleted the bank/busy-late-handoff-default branch November 10, 2026 02:15

This branch was successfully deployed

1 active deployment
Preview — 7b579789 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