Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions internal/interaction-harness/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,11 @@ before the block until its first presented change inside it: WebKit presents
a just-started compositor animation late (a sheet's release held on screen for
130 to 300 ms while its timeline ran), and its cast reaches Node about 0.7 s
late on a loaded runner, too late to wait on before blocking. Only a hold that
starts before the block, on two frames at least, is excused, and every frame
after its first change is judged, so a freeze later in the block still fails.
starts before the block, on two frames at least, at the position the page last
set before the block is excused. Pass that position as `setBeforeBlock`, such
as the drag's last position at the release. An animation frozen at the block's
start holds a position of its own, and is judged as a stall. Every frame after
its first change is judged, so a freeze later in the block still fails.
Any other element already moving is judged from the block's start. WebKit's
casts are 2 px quantized: their JPEG frames hold the
marker's color in 2 by 2 pixel chroma blocks (4:2:0, which the frame header
Expand Down
120 changes: 118 additions & 2 deletions internal/interaction-harness/src/busy.control.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,17 +456,105 @@ describe('a hold that starts before the block', () => {
});
};
const control = asControl(moving());
// The position the page set last, at the release: where the sheet holds.
const released = sheet()
.filter(f => f.t < holdFrom)
.at(-1)!.box!.y;

it('is judged from its first presented change inside the block', () => {
it('is judged from its first presented change inside the block when it holds the position the page set', () => {
const proof = busyWindowProof(sheet(), BLOCK, {
axis: 'y',
control,
setBeforeBlock: released,
});
expect(proof.verdict.failures, proof.summary).toEqual([]);
expect(proof.onScreenFromMs).toBeGreaterThanOrEqual(100);
});

it('is not excused without the position the page set', () => {
const proof = busyWindowProof(sheet(), BLOCK, {axis: 'y', control});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});

it('is not excused when it holds a position the page did not set', () => {
const proof = busyWindowProof(sheet(), BLOCK, {
axis: 'y',
control,
setBeforeBlock: released + 6,
});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});
Comment on lines +480 to +488

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: this case runs on frames with no step, so it takes the 1 px fallback. Every real cast of the default marker carries a step and takes the castStep branch in busy.ts, where no case refuses a hold at another position: replacing castStep(tracked, axis) with Infinity keeps every unit passing. A stepped copy of this case pins it:

const grid = sheet().map(f => ({...f, step: {x: 2, y: 2}}));
const proof = busyWindowProof(grid, BLOCK, {axis: 'y', control, setBeforeBlock: released + 6});
expect(proof.verdict.pass, proof.summary).toBe(false);


it('is excused on a 2 px cast that reads the held position up to a step off', () => {
// WebKit's cast reads positions on a 2 px grid: a hold at a half pixel
// reads up to 1.5 px off.
const grid = sheet().map(f => ({...f, step: {x: 2, y: 2}}));
const proof = busyWindowProof(grid, BLOCK, {
axis: 'y',
control,
setBeforeBlock: released + 1.5,
});
expect(proof.verdict.failures, proof.summary).toEqual([]);
expect(proof.onScreenFromMs).toBeGreaterThanOrEqual(100);
});

it('matches the held position within a pixel on a cast that carries no step', () => {
const proof = busyWindowProof(sheet(), BLOCK, {
axis: 'y',
control,
setBeforeBlock: released + 1.5,
});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});

it('is not excused when the position it holds shows on one frame only', () => {
// Moving up to the last frame before the block, then not found on any
// frame until 150 ms in, then moving on. The position the page set
// shows on one frame only, which is no hold. So the window is judged
// from the judged part's start.
let y = 0;
const frames = moving().map(f => {
y += 3;
return (
f.t >= BLOCK.from && f.t < BLOCK.from + 150
? {...f, box: null}
: {...f, box: {...f.box!, y}}
) as TrackedFrame;
});
const last = frames.filter(f => f.t < BLOCK.from).at(-1)!.box!.y;
const proof = busyWindowProof(frames, BLOCK, {
axis: 'y',
control,
setBeforeBlock: last,
});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});

it('fails an animation that was moving, freezes from just before the block for 50 ms, then moves on', () => {
// Moving every frame up to the block, frozen from the frame before it
// until 50 ms in, then moving on every frame. This is a compositor
// animation stalled at the block's start, at none of the page's positions.
let y = 0;
const frames = moving().map(f => {
if (f.t < BLOCK.from - 5 || f.t >= BLOCK.from + 50) {
y += 3;
}
return {...f, box: {...f.box!, y}} as TrackedFrame;
});
const proof = busyWindowProof(frames, BLOCK, {axis: 'y', control});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});

it('still fails when the element changes once inside the block and then freezes', () => {
const proof = busyWindowProof(sheet({freezeAfterFirst: true}), BLOCK, {
axis: 'y',
control,
setBeforeBlock: released,
});
expect(proof.verdict.pass, proof.summary).toBe(false);
});
Expand All @@ -479,7 +567,35 @@ describe('a hold that starts before the block', () => {
}
return {...f, box: {...f.box!, y}} as TrackedFrame;
});
const proof = busyWindowProof(frames, BLOCK, {axis: 'y', control});
// Even at the last position before the judged part, as if the page had
// set it.
const proof = busyWindowProof(frames, BLOCK, {
axis: 'y',
control,
setBeforeBlock: frames.filter(f => f.t < BLOCK.from + 20).at(-1)!.box!.y,
});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});

it('is not excused when its first held frame is the one at the block start', () => {
// Moving up to the frame presented as the block starts, then held on it
// and the frames after until 250 ms in. The hold has two frames before
// the judged part, at the position the page set, but starts inside the
// block.
let y = 0;
const frames = moving().map(f => {
if (f.t <= BLOCK.from || f.t >= BLOCK.from + 250) {
y += 3;
}
return {...f, box: {...f.box!, y}} as TrackedFrame;
});
const held = frames.find(f => f.t >= BLOCK.from)!.box!.y;
const proof = busyWindowProof(frames, BLOCK, {
axis: 'y',
control,
setBeforeBlock: held,
});
expect(proof.verdict.pass, proof.summary).toBe(false);
expect(proof.onScreenFromMs).toBeNull();
});
Expand Down
29 changes: 26 additions & 3 deletions internal/interaction-harness/src/busy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,8 +618,10 @@ function motionStepPx(values: readonly number[]): number {
* at the first presented change inside it: WebKit presents a just-started
* compositor animation late (a sheet's release held on screen for 130 to
* 300 ms while its timeline ran, and the screencast reaches Node about 0.7 s
* late on a loaded runner, too late to wait on before blocking). Every frame
* after that first change is judged, so a freeze later in the block fails.
* late on a loaded runner, too late to wait on before blocking). Only a hold
* at `setBeforeBlock`, the value the page itself last set before the block,
* is excused. Every frame after that first change is judged, so a freeze
* later in the block fails.
* The judged part still ends with the block and the `minJudgedFrames` floor
* applies to it, so a start the block itself delays fails; any other
* animation that was already moving is judged from the block's start, and a
Expand Down Expand Up @@ -684,6 +686,7 @@ export function busyWindowProof(
settledWithin,
settledWithinSteps,
settledAt,
setBeforeBlock,
maxMedianFrameMs = Infinity,
maxStallFrames = 3,
minMovedFraction = 0.9,
Expand Down Expand Up @@ -735,6 +738,17 @@ export function busyWindowProof(
* position until the main thread commits the end state.
*/
settledAt?: number;
/**
* The tracked value the page itself last set before the block, such as a
* drag's last position at its release. It is in the tracked box's
* coordinates: where the box sits at rest, plus the offset the page wrote.
* A hold that starts before the block and ends at its first presented
* change can be a late hand-off. It is one only when it holds this value,
* where the animation starts. Without it, or when the hold shows another
* value, the hold is judged as a stall. An animation can freeze at the
* block's start.
*/
setBeforeBlock?: number;
/**
* When the compositor presented each frame, epoch ms on the page clock
* (`startCompositorTrace`; Chromium). Empty or absent, the screencast is
Expand Down Expand Up @@ -877,11 +891,20 @@ export function busyWindowProof(
v.t < (inWindow[0] ?? -Infinity) &&
Math.abs(v.value - lead[lead.length - 1]) <= 0.25,
).length;
// A late hand-off holds the page's last position. An animation that froze
// at the block's start holds a position of its own. Within one cast step:
// WebKit's cast reads positions on a 2 px grid, so a held position can read
// up to a step off. A cast with no step is read within a pixel.
const matchPx = tracked.some(f => f.step != null)
? castStep(tracked, axis)
: 1;
const heldFromBeforeBlock =
setBeforeBlock != null &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With this gate, is not excused when it starts inside the block in busy.control.test.ts (which passes no setBeforeBlock) can no longer reach the start rule and the two-frame rule below. Removing both rules leaves every harness unit test passing. Passing frames.filter(f => f.t < BLOCK.from + 20).at(-1)!.box!.y as setBeforeBlock in that test restores the coverage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: that case now passes the last position before the judged part as setBeforeBlock, so it reaches both rules. At 0d8298e, deleting the start rule fails is not excused when its first held frame is the one at the block start, and deleting the two-frame rule fails is not excused when the position it holds shows on one frame only.

lead.length > 0 &&
holdStart > 0 &&
prior[holdStart].t < window.from &&
heldFrames >= 2;
heldFrames >= 2 &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Deleting this line alone leaves every harness unit passing. judges a gap at the edge of the judged part from the screencast alone in busy.test.ts pinned it on the base, but it passes no setBeforeBlock, so since line 640 it never reaches this rule. Passing setBeforeBlock: frames.filter(f => f.t < BLOCK.from).at(-1)!.box!.y in that case restores the coverage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: is not excused when the position it holds shows on one frame only passes the position the page set, so it reaches this rule. At 0d8298e, deleting the rule fails that case.

Math.abs(lead[lead.length - 1] - setBeforeBlock) <= matchPx;
const onScreenFrom =
prior.length > 0 &&
(!movedBefore || heldFromBeforeBlock) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -929,10 +929,12 @@ test.describe('frame evidence', () => {
const control = await startBusyControl(page);
// On the compositor: a move, a 300 ms hold (as WebKit holds a sheet's
// just-started release on screen), then a move; or one jump inside the
// block and nothing after it.
// block and nothing after it. `heldAt` is the hold's offset, the position
// the page set last.
const run = async (
keyframes: Keyframe[],
options: KeyframeAnimationOptions,
heldAt?: number,
) => {
const cast = await startScreencast(page);
await page.waitForTimeout(150);
Expand All @@ -950,19 +952,27 @@ test.describe('frame evidence', () => {
});
await page.waitForTimeout(150);
const frames = await cast.stop();
return busyWindowProof(await trackMarker(page, frames), window, {
const tracked = await trackMarker(page, frames);
// Where the box sits untransformed: the run starts there.
const top = Math.min(
...tracked.flatMap(f => (f.box == null ? [] : [f.box.y])),
);
return busyWindowProof(tracked, window, {
axis: 'y',
control: await trackMarker(page, frames, {rgb: CONTROL_RGB}),
...(heldAt == null ? {} : {setBeforeBlock: top + heldAt}),
});
};
// Held at a half pixel: WebKit's 2 px cast reads it up to a step off.
const held = await run(
[
{transform: 'translateY(0px)', offset: 0},
{transform: 'translateY(100px)', offset: 0.3},
{transform: 'translateY(100px)', offset: 0.45},
{transform: 'translateY(100.5px)', offset: 0.3},
{transform: 'translateY(100.5px)', offset: 0.45},
{transform: 'translateY(560px)', offset: 1},
],
{duration: 2000, easing: 'linear'},
100.5,
);
expect(held.verdict.failures, held.summary).toEqual([]);
const jumpThenFreeze = await run(
Expand Down
Loading