Skip chromium restart on headful display resize

[hiroTamada]

Summary

Headful PATCH /display previously restarted chromium after the
xrandr/Neko screen resize so chromium could re-apply --start-maximized
against the new X root. The restart costs ~9s and wipes browser-side
state (Emulation.* overrides, devtools sessions).

The restart turns out not to be load-bearing. Mutter reflows a window in
windowState=maximized (or fullscreen) onto the new root whenever the
X root resizes via xrandr/Neko. The server just has to make sure the
window is in that state, then verify the X root reached the requested
size before returning.

This PR replaces the restart on the Xorg branch with a single
Browser.setWindowBounds{windowState:"maximized"} CDP call plus an
xrandr poll for the X root. The restart_chromium request field stays
in the API schema for compatibility but is now a no-op on the Xorg
path. Existing callers that hardcode restart_chromium=true
(kraft.go, chromium_configure.go in kernel/kernel) silently get the
faster path.

The trap we avoid

The explicit-bounds form of setWindowBounds — windowState:"normal"
with {left, top, width, height} — is intentionally not used. Once
a window is in normal state, mutter stops auto-tracking RANDR events,
which silently breaks subsequent resizes.

Why poll the X root, not the chromium window

The post-condition is verified by polling getCurrentResolution
(xrandr's Screen 0: ... current WxH), not the chromium window's own
CDP bounds. The X root is the value the server actually controls;
polling it is also panel-robust — if mutter ever gains a taskbar/dock,
a maximized chromium window would be smaller than the root by the
panel's reserved space, and a window-bounds poll would silently time
out.

Changes

• server/lib/cdpclient/cdpclient.go — SetWindowBoundsMaximized(ctx).
  Idempotent: early-returns on maximized or fullscreen so kiosk
  windows aren't demoted. Plus a firstPageTargetID helper to
  deduplicate the Target.getTargets → page-target boilerplate that was
  spread across three methods, and a GetWindowBounds(ctx) reader
  reused by SetWindowBoundsMaximized.
• server/cmd/api/api/display.go — Xorg branch: always
  setWindowMaximizedViaCDP + waitForXRootSize after a successful
  resize. The legacy restart branch is gone. A small withCDPClient
  helper hides the dial/timeout/close scaffolding shared by the two
  per-call CDP helpers.
• server/cmd/api/api/chromium_configure.go — drop the now-unused
  restartChrome argument from the setResolutionXorgViaXrandr call.
• server/e2e/e2e_display_resize_window_test.go — new test exercising
  four scenarios: headless CDP fast path, headful --start-maximized
  • neko, headful --kiosk + fullscreen, and headful non-Neko Xorg
    (xrandr --output DUMMY0 --mode WxH_RR.00). Each subtest asserts
    X root convergence, renderer screen.* and outer{Width,Height}
    convergence, WindowState preservation, and a stable windowId
    (proving no chromium restart).

The Xvfb-with-recordings branch in display.go is orthogonal (headless
chromium has no OS window) and still honours restart_chromium=true.

Numbers

subtest	wall-clock before	after	windowId stable
headful_start_maximized	~35s	17s	yes
headful_kiosk	~34s	17s	yes
headful_xorg_no_neko	n/a	17s	yes

Two resizes per scenario; ~17s saved per PATCH /display call against
the kernel/kernel callers that all hardcode restart_chromium=true
today.

Test plan

• cd server && go vet ./...
• cd server && go test ./lib/cdpclient/... ./cmd/api/... — unit
  tests for cdpclient and the api package
• New TestDisplayResizeChromiumWindow — 4 subtests, all pass
  against an overlay image carrying this branch's kernel-images-api
• Existing TestDisplayResolutionChange still passes against the
  modified server
• Manual smoke against a metro VM to confirm the same behaviour on
  the unikernel runtime as in the docker e2e

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes core display/WM integration (xrandr output, CDP window bounds, stricter 500s) and leaves chromium_configure’s stop-cycle Xorg resize without the new CDP/X-root post-steps.

Overview
Headful PATCH /display no longer restarts Chromium after an Xorg resize. After Neko or xrandr applies the new mode, the server re-asserts a maximized/fullscreen OS window via Browser.setWindowBounds (CDP) and treats success only when xrandr reports the X root at the requested size—failures on either step return 500 instead of 200 with a mismatched display. restart_chromium remains in the API but is a no-op on this Xorg path; the Xvfb/recording path can still restart when requested.

xrandr on the dummy driver now targets DUMMY0 (or KERNEL_IMAGES_XRANDR_OUTPUT) with a named WxH_RR.00 modeline instead of --output default, which could exit 0 without resizing. cdpclient adds SetWindowBoundsMaximized / GetWindowBounds and shared firstPageTargetID; display.go adds withCDPClient, setWindowMaximizedViaCDP, and waitForXRootSize. New e2e TestDisplayResizeChromiumWindow covers headless, Neko maximized, kiosk fullscreen, and non-Neko Xorg.

^{Reviewed by Cursor Bugbot for commit 7f4d47d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[firetiger-agent]

Monitoring Plan: Display Resize Skips Chromium Restart, Uses CDP Instead

What this PR does: Headful browser display resizes are now ~9 seconds faster and no longer wipe browser-side state (Emulation.* overrides, open devtools sessions). Instead of restarting Chromium after an xrandr resize, the server re-asserts the window's maximized state via a single CDP call — Mutter then automatically reflows the window to fill the new screen.

Intended effect:

• CDP maximize re-assert success log: baseline 0 (new log message); confirmed if "re-asserted maximized window state via CDP" INFO appears post-deploy on headful resize calls
• Chromium restart errors on resize: baseline 0 occurrences/7d; confirmed if "failed to restart chromium after resolution change" ERROR remains at 0 post-deploy on the headful path

Risks:

• CDP maximize WARN burst — "CDP maximize re-assert failed after Xorg resolution change (non-fatal)" WARN log; alert if any occurrences post-deploy (baseline: 0 — this message didn't exist pre-deploy)
• Window not filling screen — if Mutter's reflow invariant fails or the CDP call silently errors, users see a clipped viewport; surface via the CDP maximize WARN log or customer reports
• Kiosk mode disruption — SetWindowBoundsMaximized guards against demoting a fullscreen window; alert if kiosk sessions transition from fullscreen to maximized post-resize

Status updates will be posted automatically on this PR as monitoring progresses.

View monitor

[rgarcia]

test comment is backwards. chromiumWindowBounds (e2e_display_resize_window_test.go:114-117) describes the maximized/normal bounds cases as the inverse of cdpclient.go:292-295, which is the correct one: maximized/fullscreen report the live size (matching the root), normal reports saved-restore. waitForWindowSize relies on the live-size reading, so flip the test comment to match.

Medium — poll the X-root, not the window's self-report. waitForWindowSize (display.go:445-470) waits for an exact b.Width==width match on the window's own bounds; that only matches because maximized == full root in these panel-less mutter images. If mutter ever gains a panel/struts, maximized < root → permanent 3s timeout → 500 on every resize. Poll the X-root size instead — it's the value you actually set, and the test already reads it via xrandr.

Medium — two simplifications.

• SetWindowBoundsMaximized (cdpclient.go:256-290) re-implements the getWindowForTarget read that GetWindowBounds (:306-333) already does; it only needs {WindowID, WindowState}, which that returns — call it and drop ~15 lines + a duplicate struct.
• setWindowMaximizedViaCDP and setViewportViaCDP (display.go:412-439, :475-498) share identical dial scaffolding — extract a withCDPClient helper.

Medium — PR body out of sync. It claims five subtests (incl. headful_kiosk_no_restart, headful_force_maximized_no_restart, and the "trap we avoid" normal→maximized one); the file has four after 5b968a6 (headless_default, headful_start_maximized, headful_kiosk, headful_xorg_no_neko). Update the body + Numbers table — the normal→maximized path it claims to test isn't actually covered.

[hiroTamada]

@rgarcia in #262 (comment)

Thanks — all four valid, addressed in 2f81c56:

1. Test comment flipped. chromiumWindowBounds now matches cdpclient.WindowBounds: maximized/fullscreen → live size, normal → saved-restore.

2. Poll the X-root. Dropped waitForWindowSize (CDP Browser.getWindowForTarget polling), added waitForXRootSize (polls s.getCurrentResolution → xrandr Screen 0: current WxH). setWindowMaximizedViaCDP lost its width/height args — it does state only. PatchDisplay now does CDP-maximize then X-root-wait as two separate fatal steps. Panel-robust: if mutter ever gains a taskbar, the maximized window would be smaller than the root by the panel's reserved space, but the root itself stays the value we set.

3a. SetWindowBoundsMaximized now calls GetWindowBounds. Dropped the duplicate Browser.getWindowForTarget block + struct (~14 lines).

3b. Extracted withCDPClient(ctx, fn) for the dial+timeout+defer-close. setWindowMaximizedViaCDP and setViewportViaCDP are each ~3 lines now.

4. PR body updated to reflect the four-subtest reality. Dropped the stale claims about headful_kiosk_no_restart, headful_force_maximized_no_restart, and the "trap we avoid" normal→maximized test that the dropped subtest was the only thing exercising. The trap remains avoided in the code (we never send explicit normal bounds), just no longer claimed as e2e-covered.

Numbers updated in a separate comment.

[hiroTamada]

Benchmark: PATCH /display before vs after

Same docker host, same image base (onkernel/chromium-headful:11e1b04), same production-equivalent CHROMIUM_FLAGS + ENABLE_WEBRTC=true + NEKO_ADMIN_PASSWORD=admin. 8 iterations alternating 1920×1080 ↔ 1280×720.

	min	median	mean	max
BEFORE (restart_chromium=true against unmodified :11e1b04)	8701 ms	8981 ms	8942 ms	9062 ms
AFTER (default, omitted restart_chromium, this branch)	58 ms	76 ms	70 ms	83 ms

~118× faster at the median. ~8.9s saved per PATCH /display.

The AFTER number includes the full server post-condition: CDP setWindowBounds{windowState:"maximized"} and the xrandr X-root poll. The poll dominates — Neko's ScreenConfigurationChange is essentially synchronous, so the polling loop almost always matches on the first read; convergence sits well under the 3s deadline.

kernel/kernel#2264 drops restart_chromium=true from both control-plane callers, so the existing kraft.go and chromium_configure.go paths get this latency improvement transparently once both PRs land.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2f81c56. Configure here.}

[cursor]

X root poll uses unreliable grep '*' after Neko resize

Medium Severity

waitForXRootSize calls s.getCurrentResolution(), which internally runs xrandr | grep -E '\\*' to find the active mode. However, the new test helper getXRootResolution in the same PR explicitly avoids the *-grep approach, with a comment explaining that "Xorg-with-Neko surfaces a synthetic active mode that xrandr does not mark with an asterisk." If this is accurate, waitForXRootSize will always time out on the Neko path after a resize because the new mode won't have a * marker, causing PATCH /display to return a 500 even though the resize succeeded.

Additional Locations (1)

• server/e2e/e2e_display_resize_window_test.go#L75-L112

 

^{Reviewed by Cursor Bugbot for commit 2f81c56. Configure here.}

[hiroTamada]

checking on this. I did not see this issue when I was testing

[hiroTamada]

Right — the bot was working off a wrong claim in my test-helper doc, not a real bug. Verified in a running container on this branch + Neko enabled:

BEFORE PATCH:  xrandr | grep '*' | awk '{print $1}' → 1920x1080_25.00
PATCH 2560x1440 → 200 OK
AFTER PATCH:   xrandr | grep '*' | awk '{print $1}' → 2560x1440_60

Neko's active mode IS marked with * after a resize, so the server's getCurrentResolutionFromXrandr works fine. The benchmark above (~76ms median over 8 iterations) is the empirical confirmation that the polling loop converges quickly.

Fixed the stale doc claim in 0bad77e — no server code change needed.

[hiroTamada]

Good catch — fixed in 7f4d47d. Now calls getCurrentResolutionFromXrandr directly, so the post-condition is bound to the X root regardless of whether anyone ever sets a viewportOverride on the Xorg path. Doc comment updated to call out why the higher-level wrapper is the wrong abstraction here.

[hiroTamada]

@cursor[bot]: `X root poll uses unreliable `grep '*'` after Neko resize`

The doc comment that claim was based on is stale — verified empirically in the running container (kernel-images-headful:test-option2, which carries this branch + Neko):

BEFORE PATCH:  xrandr | grep '*' | awk '{print $1}' → 1920x1080_25.00
PATCH 2560x1440 → 200 OK
AFTER PATCH:   xrandr | grep '*' | awk '{print $1}' → 2560x1440_60

The active mode IS marked with * after a Neko resize. getCurrentResolutionFromXrandr works fine on the Neko path; the benchmark above (~76ms median) is the empirical confirmation.

The wrong claim was in getXRootResolution's doc comment, fixed in 0bad77e. No server code change needed.

[rgarcia]

LGTM — all four findings addressed cleanly (test comment flipped, X-root poll, withCDPClient / GetWindowBounds simplifications, PR body). Build + vet green. One non-blocking nit:

nit — have waitForXRootSize read the root directly. It polls getCurrentResolution (display.go:468), which returns viewportOverride when one is set, not the X root. Safe today only because the override is set on a single path — the headless fast path (display.go:186) — that an Xorg container never reaches. That's a non-local invariant sitting far from the poll site: if a viewport override is ever set on the Xorg path, this "panel-robust" check silently validates against the CDP viewport instead of the root. Calling getCurrentResolutionFromXrandr (display.go:619) directly makes the post-condition read the root unconditionally.

[rgarcia]

Re-reviewed at 7f4d47d — the X-root-reader nit is resolved (waitForXRootSize now calls getCurrentResolutionFromXrandr directly, decoupled from viewportOverride) and the Screen 0: current vs per-output grep '*' discrepancy is documented. Build + vet green. LGTM.