Ignacy Łątka 4c645f1619 fix(chromium): correct the hidden-window guard's docs and pin its exemptions (#611) ...

Follow-ups from a post-merge review of #605. Each one is a comment
thread on that PR.

No behaviour changes to what the guard does — the three emitted
`failure_stage` strings and the `CHROMIUM_WINDOW_HIDDEN` code are
byte-for-byte identical, so telemetry stays joinable.

## Changes

**Derive `failure_stage` from `action`.** It was a second free-form
`string` parameter sitting next to `action`, and all three call sites
passed a strictly redundant pair — `("tap",
"chromium_tap_window_hidden")` and so on. Two adjacent same-typed params
meant transposing them or mistyping the stage compiled cleanly and read
fine in review. That carries extra weight here because #605 deliberately
froze `chromium_scroll_window_hidden` to keep pre- and post-migration
rows joinable. `action` is now `"tap" | "drag" | "scroll"`, so a typo is
a compile error, and the docblock spells out all three literals so they
stay greppable in `src`.

**Pin the keyboard exemption.** Key events skip hit-testing and stay
fast on a hidden window, so `keyboard` is deliberately unguarded. That
was correct but lived only in a comment: adding the guard to `keyboard`
would have made it refuse input that demonstrably works, and the suite
would have stayed green. Now covered by
`keyboard-chromium-unguarded.test.ts`.

**Pin the `button` exemption too.** The same file now asserts that
`assertSupported` rejects a Chromium device for `button`. The claim that
`button` can never reach the guard rested entirely on a capability
literal no test read — adding `chromium: { app: true }` to it reddened
nothing.

**Correct the same comment's claim about `button` — three times.** The
first pass replaced the wrong rationale (that `button` skips the guard
because key events bypass hit-testing) with the real one: its capability
omits chromium. The replacement was itself wrong about where Chromium's
button surface lives. `sendButton` is reached only from
`handleWsCommand`, i.e. the chromium-server **WebSocket** channel — the
HTTP router exposes screenshot / clipboard / fps / navigate / reload /
history / viewport / stream and no button at all. It also over-claimed:
`sendButton` emulates `Back` alone as an Alt+Left chord and throws for
every other button, so "chromium hardware buttons exist" contradicted
`button/index.ts` ("Chromium apps have no hardware buttons"). The
wording now agrees with both neighbours. A third pass dropped the last
false note in it: the lockout test said the gate stops `button` "before
it can reach the guard", but `button` has no chromium branch and never
imports the guard — there is no guard on its path to reach.

**Give the keyboard measurement its missing condition.**
`Input.dispatchKeyEvent` at 1–14 ms against 5002–5005 ms per mouse move,
and "focus emulation keeps input fast even for a genuinely minimized
window", describe the same scenario with opposite outcomes — those
numbers can only come from a window where focus emulation was not in
effect. The docblock says so, and names focus emulation alone:
`primePageSession` applies it to every session at connect, so the probe
reads "hidden" only where the runtime refused it.

The anti-throttling launch flags are not part of that condition and do
not narrow the guard's reach. A window carrying all three, minimized
without emulation, still reads "hidden" and still costs ~5 s per
`Input.dispatchMouseEvent` — Electron 42.5.0 5001–5005 ms and Chrome 152
5002–5010 ms, 3 trials each — so an argent-spawned app is no less
exposed than an externally launched one. `boot-electron.ts`'s
`ANTI_THROTTLING_ARGS` docblock, where that claim originated, is
corrected to match, along with its test.

**Two mechanisms that were stated unconditionally.** `Page.bringToFront`
un-minimizes on Chrome 152 (`windowState` minimized → normal,
`visibilityState` hidden → visible, mouse dispatch 5002–5006 ms → 16–25
ms, 3/3) and not on Electron 42.5.0 (returns ok, stays `"hidden"`, stays
5001–5005 ms, 3/3), so `flow-run`'s comment calls the outcome
runtime-dependent rather than denying it. And the capability gate that
keeps `button` off a Chromium device is not universal: `assertSupported`
runs at the HTTP edge and in run-sequence's pre-flight, but
`Registry.invokeTool` skips it, so a flow step dies resolving
`services()` with `SimulatorServer.factory does not support platform
"chromium"` rather than an `UnsupportedOperationError`.

**Replace the try-boundary rationale.** It said the property access had
to stay inside the `try` because fakes "may lack `cdp`" — but for such
an api `chromium.cdp` yields `undefined` without throwing; the TypeError
comes from reading `.send`, which is inside the `try` wherever
`chromium.cdp` is read. Verified: hoisting `const client =
chromium.cdp;` above the `try` leaves every relevant test green. The
placement would only matter if `chromium` itself were nullish, which no
registry path produces.

**Cover the no-`cdp` probe path directly.** The existing test claimed to
cover "api objects with no `cdp` at all" but used a rejecting `cdp.send`
— a different failure mode. The real shape was exercised only
incidentally, by bare fakes in `chromium-drag.test.ts` and
`tools/gesture-tap.test.ts`.

**Cover the other half of "a failed or empty read proves nothing".**
Only the throw paths were pinned. Flipping the comparison to `value !==
"visible"` left the entire suite green, so a probe that resolves without
a usable value — `Runtime.evaluate` against a torn-down document — would
have started refusing tap/drag/scroll on a fully visible window.

**Pin the action word in the user-facing message.** Hardcoding `Cannot
tap:` for all three tools also stayed green, so a scroll refusal reading
"Cannot tap" could have shipped unnoticed.

**Update `flow-run`'s comment.** It still named gesture-scroll as the
only tool that fails fast on a hidden window; tap and drag have since
#605. It also over-promised "fail fast": on the flow path
`frontChromiumPage` has just primed focus emulation, so the three tools
proceed. They carry the guard; whether it fires depends on the
mitigations.

## Verification

- Full gate on the branch: `tsc --build`, `typecheck:tests`, `eslint
--max-warnings 0`, `prettier --check`, and the whole tool-server suite
sharded — **360 files / 4504 passed, 1 skipped, 0 failed**.
- Every empirical claim in the comments was re-measured on a live CDP
testbed rather than carried over: an Electron 42.5.0 app and Chrome 152,
each launched with and without the three `ANTI_THROTTLING_ARGS` plus
`--remote-debugging-port`, minimized, probed for
`document.visibilityState` and per-event `Input.dispatchMouseEvent` /
`Input.dispatchKeyEvent` latency before any focus emulation, then after
`Page.bringToFront`, then after `Emulation.setFocusEmulationEnabled`.
Runs were sequential, one browser at a time, 3 trials per condition.
- The `button`-on-chromium flow path was run, not just read:
`registry.invokeTool("button", { udid: "chromium-cdp-9222", button:
"home" })` with the simulator-server blueprint registered throws
`ToolExecutionError … SimulatorServer.factory does not support platform
"chromium"`.
- The measurements behind the keyboard exemption were taken on a real
minimized Electron window (v42.5.0): `Input.dispatchKeyEvent` 1–14 ms,
against 5002–5005 ms for each of ten consecutive
`Input.dispatchMouseEvent` moves, with `Runtime.evaluate` staying at 1–3
ms.
- Mutation-tested every pin rather than assuming it bites. Each of these
fails the suite: adding the guard to `keyboard/chromium`; adding
`chromium` to `button`'s capability; making the probe rethrow instead of
swallowing; drifting the derived stage template; collapsing the stage to
one action for all three tools; flipping the comparison to `value !==
"visible"`; and hardcoding the action word in the message.


- No docs update needed: comments only, no user-facing behaviour or
tool-surface change.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Documentation**
- Clarified how window minimization, focus emulation, and
anti-throttling behavior affect Chromium interactions.

- **Bug Fixes**
- Standardized hidden-window handling and error reporting for tap, drag,
and scroll gestures.
  - Preserved keyboard input on hidden Chromium windows.

- **Tests**
- Expanded coverage for hidden-window gestures, keyboard input, missing
browser capabilities, and varied visibility responses.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

2026-09-04 14:43:54 +02:00

17 KiB

Raw Permalink Blame History

View Raw