Skip to content

Fix spurious CLS report of 0 after bfcache restore - #786

Merged
tunetheweb merged 1 commit into
GoogleChrome:mainfrom
suhailopensource:fix-cls-bfcache-double-report
Aug 24, 2026
Merged

Fix spurious CLS report of 0 after bfcache restore#786
tunetheweb merged 1 commit into
GoogleChrome:mainfrom
suhailopensource:fix-cls-bfcache-double-report

Conversation

@suhailopensource

Copy link
Copy Markdown
Contributor

Fixes #785.

Restores the zero-arg wrapper inside doubleRAF() so its callback is always invoked with no arguments:

-  requestAnimationFrame(() => requestAnimationFrame(cb));
+  requestAnimationFrame(() => requestAnimationFrame(() => cb()));

Per @Mimori256's suggestion in #785 I've taken the doubleRAF() hardening as the primary fix rather than reverting the onCLS() call site, since it re-establishes the invariant for every call site. src/onCLS.ts is unchanged.

Tests

Added e2e coverage for both reportAllChanges values, as @tunetheweb requested:

  • does not report a spurious zero value after bfcache restore (reportAllChanges === false) — the regression test. Fails on main (through both retries), passes with the fix.
  • reports the initial zero value after bfcache restore (reportAllChanges === true) — guards against over-correcting, so the zero-value report is still emitted when reportAllChanges is true. Passes with and without the fix.

Drive-by

Also adds a missing await beaconCountIs(1) to reports zero if no layout shifts occurred on first visibility hidden (reportAllChanges === false). That test reads the beacon file without waiting for the beacon to arrive, unlike its reportAllChanges === true sibling which has exactly that wait — an unrelated pre-existing race (those lines date to 2020/2022) that I hit locally. Happy to split it out if you'd prefer.

Verification

npm run test:e2e -- --browsers=chrome: 5/5 spec files, 153 passing, 4 skipped. Build, lint, format:check and unit tests all green. Reverting only the doubleRAF change leaves the new reportAllChanges === false test failing and everything else green.

Restore the zero-arg wrapper inside doubleRAF() so its callback is always
invoked without arguments. Without it the rAF DOMHighResTimeStamp was passed
to bindReporter's forceReport parameter, forcing a CLS report of 0 on every
bfcache restore even when reportAllChanges was false.

Adds e2e coverage for both reportAllChanges values, and a missing
beaconCountIs() wait in an unrelated pre-existing racy test.

Fixes GoogleChrome#785
@tunetheweb
tunetheweb merged commit 3d62f3d into GoogleChrome:main Aug 24, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onCLS() reports a spurious CLS of 0 after every bfcache restore

2 participants