Skip chromium restart on headful display resize#262
Open
hiroTamada wants to merge 4 commits into
Open
Conversation
Headful PATCH /display previously restarted chromium to re-apply
--start-maximized against the new X root. The restart costs ~9s and
also wipes browser-side state (Emulation.* overrides, devtools
sessions). It turns out the restart isn't 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.
This replaces the restart on the Xorg branch of PatchDisplay with a
single Browser.setWindowBounds{windowState:"maximized"} call. The
helper is idempotent: it early-returns when the window is already
maximized or fullscreen, so kiosk windows stay in fullscreen. Callers
that still want the legacy restart can pass restart_chromium=true.
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 would
silently break subsequent resizes.
New e2e test e2e_display_resize_window_test.go covers headless fast
path, --start-maximized + neko, kiosk + fullscreen, and a
force-maximized-via-CDP scenario. Each subtest asserts:
- X root reaches the requested size (xrandr Screen 0: current WxH)
- renderer screen.* and outer{Width,Height} converge
- CDP windowState is preserved across the resize
- CDP windowId is stable (proves no chromium restart)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Follow-up to the prior commit: remove the restart_chromium=true escape hatch from the Xorg branch entirely. The CDP re-assert-maximized path is strictly better (faster, preserves browser state, stable windowId) and the e2e tests exercise it exclusively. Existing callers in kernel/kernel (kraft.go, chromium_configure.go) hardcode restart_chromium=true today and now transparently get the faster behaviour. The restart_chromium field stays in the request schema for API compatibility. On the Xorg branch it is now a no-op. The Xvfb-with- recordings branch (orthogonal — headless chromium has no OS window) still honours the flag. Also drop the unused restartChrome parameter from setResolutionXorgViaXrandr and its one caller in chromium_configure.go. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
After the legacy restart_chromium path was removed, two of the five
TestDisplayResizeChromiumWindow scenarios became strictly redundant:
- headful_kiosk_no_restart: passes restart_chromium=false explicitly,
but the field is a no-op on the Xorg branch — functionally
identical to headful_kiosk.
- headful_force_maximized_no_restart: forces windowState=maximized
via CDP at baseline, then resizes. Was useful while investigating
whether mutter's "maximized window reflows on RANDR" invariant
holds; headful_start_maximized now covers the same hot path with
production-equivalent CHROMIUM_FLAGS.
Both also flaked in CI when running later in a heavy e2e suite —
neko/mutter occasionally fails to apply its 1920x1080 desktop.screen
startup mode under load, leaving the X root at the dummy DDX's
3840x2160 default and breaking the resize assertions. Running them
locally or earlier in the suite passes consistently, so it's an
environmental flake, not a code regression.
Also remove the now-unused helpers setChromiumWindowStateCDP,
waitForWindowState, baselineOrForcedState, and the
forceMaximizeAtBaseline / restartChromium fields on resizeScenario.
The remaining three subtests cover every production path: headless
CDP fast path, headful --start-maximized + neko, headful kiosk +
fullscreen.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Monitoring Plan: Display Resize Skips Chromium Restart, Uses CDP InsteadWhat 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:
Risks:
Status updates will be posted automatically on this PR as monitoring progresses. |
Three follow-ups addressing review feedback: 1. xrandr targets DUMMY0 (or KERNEL_IMAGES_XRANDR_OUTPUT env override), not "default". The headful Xorg dummy driver does not expose a "default" output, so `xrandr --output default --mode ...` exits 0 without applying anything. This affects only the non-Neko Xorg path (production runs with Neko enabled, which doesn't touch xrandr), but left the path silently broken. 2. CDP maximize re-assert failure is now fatal. Previously logged a warning and returned 200; if the window happened to be in normal state and the CDP call failed, the server returned success with the browser window stuck at the old size. Now the resize fails closed with a 500 so the caller sees the mismatch. 3. TestDisplayResizeChromiumWindow now asserts windowID stability and windowState preservation across both resizes. Previously logged but never asserted; a silent chromium restart regression would have slipped through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
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 9f62371. Configure here.
| log.Info("using specific modeline", "output", output, "mode", modeName) | ||
| } else { | ||
| xrandrCmd = fmt.Sprintf("xrandr -s %dx%d", width, height) | ||
| xrandrCmd = fmt.Sprintf("xrandr --output %s --size %dx%d", output, width, height) |
There was a problem hiding this comment.
Invalid xrandr --size used as per-output option
High Severity
The else branch (when refreshRate <= 0) constructs xrandr --output DUMMY0 --size WxH, but --size is a legacy RandR 1.0/1.1 global screen option (long form of -s), not a per-output option. Per-output resolution changes require --mode, not --size. Combining --output with --size is invalid and will either silently no-op or error. The refreshRate > 0 branch correctly uses --mode. The test comment at line 460 of e2e_display_resize_window_test.go still references the old working form (xrandr -s WxH), confirming the intent was a global size set, but the code was incorrectly rewritten to use --output with --size.
Reviewed by Cursor Bugbot for commit 9f62371. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Headful
PATCH /displaypreviously restarted chromium after thexrandr/Neko screen resize so chromium could re-apply
--start-maximizedagainst 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(orfullscreen) onto the new root whenever theX root resizes via xrandr/Neko. The server just has to make sure the
window is in that state.
This PR replaces the restart on the Xorg branch with a single
Browser.setWindowBounds{windowState:"maximized"}call via CDP. Therestart_chromiumrequest field stays in the API schema forcompatibility but is now a no-op on the Xorg path. Existing callers
that hardcode
restart_chromium=true(kraft.go, chromium_configure.goin 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. Oncea window is in normal state, mutter stops auto-tracking RANDR events,
which silently breaks subsequent resizes. The new e2e test
headful_force_maximized_no_restartexercises this indirectly by goingnormal → maximized via the helper and confirming reflow still works on
both up- and down-resize.
Changes
server/lib/cdpclient/cdpclient.go—SetWindowBoundsMaximized(ctx).Idempotent: early-returns on
maximizedorfullscreenso kioskwindows aren't demoted.
server/cmd/api/api/display.go— Xorg branch: always callsetWindowMaximizedViaCDPafter a successful resize. The legacyrestart branch is gone.
restartChromeis no longer threaded intosetResolutionXorgViaXrandr.server/cmd/api/api/chromium_configure.go— drop the now-unusedfifth argument from the
setResolutionXorgViaXrandrcall.server/e2e/e2e_display_resize_window_test.go— new test exercisingfive scenarios: headless fast path, headful
--start-maximized+neko, headful kiosk, kiosk with explicit
restart_chromium=false,and force-maximized-via-CDP starting from
normalstate. Eachsubtest asserts X root, renderer
screen.*andouter{Width,Height}convergence,
WindowStatepreservation, and a stablewindowId(proving no chromium restart).
The Xvfb-with-recordings branch in
display.gois orthogonal (headlesschromium has no OS window) and still honours
restart_chromium=true.Numbers
headful_start_maximizedheadful_kioskheadful_kiosk_no_restartheadful_force_maximized_no_restartTwo resizes per scenario; ~17s saved per PATCH
/displaycall againstthe kernel/kernel callers that all hardcode
restart_chromium=truetoday.
Test plan
cd server && go vet ./...cd server && go test ./lib/cdpclient/... ./cmd/api/...— unittests for cdpclient and the api package
TestDisplayResizeChromiumWindow— 5 subtests, all passagainst an overlay image carrying this branch's
kernel-images-apiTestDisplayResolutionChangestill passes against themodified server
the unikernel runtime as in the docker e2e
🤖 Generated with Claude Code
Note
Medium Risk
Changes production headful display resize semantics and makes CDP failures block success; mitigated by xrandr correctness fix and extensive e2e tests, but behavior differs from the prior restart-based path.
Overview
Headful
PATCH /displayno longer restarts Chromium after an Xorg screen resize (Neko or xrandr). The Xorg path now re-assertswindowState=maximizedvia CDP (Browser.setWindowBounds), relying on Mutter to reflow maximized/fullscreen windows to the new X root.restart_chromiumremains in the API but is ignored on this branch; CDP re-assert failure returns 500 so callers are not left with a resized root and a stuck window.setResolutionXorgViaXrandris fixed to targetDUMMY0(orKERNEL_IMAGES_XRANDR_OUTPUT) with--output/--sizeinstead of commands that could exit 0 without changing resolution.cdpclient.SetWindowBoundsMaximizedis added (idempotent; does not demote fullscreen). New e2eTestDisplayResizeChromiumWindowchecks X root, renderer size, stablewindowId, and preserved window state across resizes for headless and headful scenarios.Reviewed by Cursor Bugbot for commit 9f62371. Bugbot is set up for automated code reviews on this repo. Configure here.