perf(engine): skip the screenshot on frames the page reports unchanged in beginframe capture - #7027

Request changes at 26a3f1fc: a change that lands in the post-screenshot settle frame is never captured, and later frames reuse the stale buffer.

Blocker. The settle BeginFrame (screenshotService.ts, screenshotTicks + interval / 4) runs a full main frame and updates the display. Anything committed in that frame is drawn there and nowhere else: rAF callbacks queued during the screenshot frame, or a late video frame / image decode / font swap. The next probe then reports hasDamage: false relative to the settle frame and returns lastFrameCache, which predates the change. The stale frame then repeats for the whole still stretch, until something else damages. On main the next frame's screenshot picks the change up, so at worst one frame is late.

Repro (headless-shell 152, same flags as beginFrameReuse.browser.test.ts). After the page settles, queue rAF → green, rAF → blue, rAF → yellow, then call beginFrameCapture three times:

• A: hasDamage: true. The buffer pixel is blue, but the DOM is already yellow.
• B, C: hasDamage: false, and each returns A's blue buffer byte for byte.
• After forgetBeginFrameCapture, a fresh capture is yellow.

So the test name's "never a stale one" doesn't hold. The three fixtures came out identical only because nothing in them changes after a screenshot frame.

I tried the obvious fix: a settle with noDisplayUpdates: true closes the hole, but then the probe always reports damage and nothing is reused (your test fails at held.hasDamage). The fix needs a different signal, e.g. a page-side change counter (MutationObserver plus media/rAF activity) checked between the screenshot and the probe, or gating reuse to pages that can't change outside the seek. Please also add a test like the repro above, one that fails at this head.

Non-blocking:

• Measured: the PR's browser test passes here.
• frameCapture.ts also sends BeginFrames for warmup and commit (init only, cache empty) and in drawelement mode. Neither reaches the cache today, because the first-capture forgetBeginFrameCapture covers page reuse. That's worth a line in the comment.

Approve at df8cc999. This resolves the blocker I raised on 26a3f1fc.

• Removing the settle frame and taking the post-probe screenshot with noDisplayUpdates closes the gap. A change made in the screenshot frame isn't displayed yet, so the next probe draws it, reports damage and captures it. Nothing gets drawn without being captured.
• Measured with my earlier repro, extended to a four-step rAF chain and then a synchronous change. Every change appears in the output within one frame, the held frames afterwards match a fresh capture, and nothing stays stale.
• The new test fails with screenshotService.ts from 26a3f1fc (1 failed, 1 passed) and passes at this head (3 runs, 2/2).

Non-blocking:

• Every capture after the first now runs two main frames (probe + screenshot) instead of one. Content driven by rAF frame count therefore advances twice per output frame, and intermediate states can be skipped (the blue step of the chain never appears). Seek-driven compositions are unaffected, and raw-rAF compositions are already sent to screenshot capture. Worth a line in the comment.
• The body still says the video fixture "shows no slowdown from the extra settle frame". There's no settle frame any more.

Request changes at 91fda7e4: the new install step fails in CI, so the Flake gate is red at this head.

Blocker. The setup-test-workspace step id: shell exits 127 in the Flake gate (job 125464920196):

npx --yes @puppeteer/browsers install chrome-headless-shell@stable --path "$dir"
sh: 1: browsers: not found

npx inside the workspace can't resolve the package's browsers bin, so the shell is never installed and the job fails before flake-gate.mjs runs. The PR therefore doesn't yet do what its title says. Something like npx --yes --package=@puppeteer/browsers@<version> browsers install … should fix it, but please confirm on a CI run before re-pinning.

Non-blocking:

• Pin the shell version (or reuse the one the engine pins) instead of @stable. Otherwise the gate can change underneath you when Chrome ships.
• If find comes back empty, the output path is "". ?? doesn't fall through on an empty string, so HF_BEGINFRAME_SHELL="" silently skips the suite in the one job meant to run it. Fail the step when the binary isn't found.
• The engine source hasn't changed since df8cc999, which I approved. The test-side basename filter is fine.

Request changes at 874e0471: the Flake gate is still red, with the same browsers: not found / exit 127 in the id: shell step (job 125476447776). The browser test never runs.

Cause, reproduced locally. The command fails only when run from the repo root:

# in the hyperframes checkout
$ npx --yes --package=@puppeteer/[email protected] browsers --version
sh: 1: browsers: not found
# in /tmp
$ npx --yes --package=@puppeteer/[email protected] browsers --version
3.2.2

Inside the bun workspace, npx resolves against the workspace's own dependencies and never puts the browsers bin on PATH. Run it from outside the checkout, for example:

dir="$RUNNER_TEMP/headless-shell"
(cd "$RUNNER_TEMP" && npx --yes --package=@puppeteer/[email protected] browsers install chrome-headless-shell@stable --path "$dir")
shell=$(find "$dir" -type f -name chrome-headless-shell | head -n 1)
test -n "$shell"
echo "path=$shell" >> "$GITHUB_OUTPUT"

Please show a green Flake gate before the next re-pin. The test -n also covers my earlier note that an empty path silently skips the suite. Pinning @puppeteer/browsers is good; the shell itself is still @stable, which is non-blocking.

Approve at cbffe241. The CI blocker is resolved.

• Switching to bunx works. In the Flake gate (job 125482640805) it installed [email protected] and passed that path as HF_BEGINFRAME_SHELL. The gate then ran beginFrameReuse.browser.test.ts 10 times under CPU contention: 10/10 passed, taking about 40 s, so the suite actually ran rather than being skipped. The Flake gate is green.
• The engine source matches df8cc999, which I approved after checking the stale-frame fix. Since then only CI setup and the test's shell selection have changed.

Non-blocking (carried over): the shell is still @stable, and an empty find result would silently skip the suite. A test -n guard would make that fail loudly.