Skip to content

Fast verified headful /configure resize without Chromium restart#263

Open
rgarcia wants to merge 1 commit into
mainfrom
autoresearch/viewport-configure-resize/01-fast-headful-configure-resize
Open

Fast verified headful /configure resize without Chromium restart#263
rgarcia wants to merge 1 commit into
mainfrom
autoresearch/viewport-configure-resize/01-fast-headful-configure-resize

Conversation

@rgarcia
Copy link
Copy Markdown
Contributor

@rgarcia rgarcia commented May 29, 2026

Optimize headful display resize for POST /configure while preserving correctness: target the real Xorg dummy RandR output, avoid unnecessary Chromium restarts, verify the X/Chromium window has reached the requested dimensions, and keep CDP responsive after resize. The final Neko-enabled path relies on Neko/Mutter resizing the already-maximized Chromium window, verifies the maximized X window state with xdotool/xprop, and performs a real Browser.getVersion CDP round-trip before returning.

Experiments: #1, #3, #5, #11, #12, #16, #17, #19, #20, #21, #26, #29, #31, #36, #37, #38
Metric: configure_ms_median 8591ms → 31ms (-99.6% overall). Neko-enabled valid baseline 46ms → 31ms (-32.6%).

Checklist

  • A link to a related issue in our repository
  • A description of the changes proposed in the pull request.
  • @mentions of the person or team responsible for reviewing proposed changes.

Note

Medium Risk
Touches live-view display, X11/RandR, and browser session paths; behavior change is intentional but regressions could affect resolution, maximized window state, or CDP readiness after configure.

Overview
Headful POST /configure and PATCH display now resize the real Xorg framebuffer and Chromium window without restarting the browser, while still honoring restart_chromium: true by verifying readiness instead of killing the process.

The headful image adds x11-utils, a 1365x768 Xorg modeline, and API logic that drives DUMMY0 via xrandr (with gtf fallback for odd widths), optionally resizes Chromium with xdotool, and on the Neko path checks a maximized window (xprop/xdotool) plus a CDP Browser.getVersion round-trip. CDP Browser.setWindowBounds support was added for OS-level window sizing. Display-only configure uses a fast path through PatchDisplay when Chromium does not need a stop/start cycle. A custom_viewport_config_test harness reproduces the control-plane multipart configure flow under Docker.

Reviewed by Cursor Bugbot for commit 785a321. Bugbot is set up for automated code reviews on this repo. Configure here.

Optimize headful display resize for POST /configure while preserving correctness: target the real Xorg dummy RandR output, avoid unnecessary Chromium restarts, verify the X/Chromium window has reached the requested dimensions, and keep CDP responsive after resize. The final Neko-enabled path relies on Neko/Mutter resizing the already-maximized Chromium window, verifies the maximized X window state with xdotool/xprop, and performs a real Browser.getVersion CDP round-trip before returning.

Experiments: #1, #3, #5, #11, #12, #16, #17, #19, #20, #21, #26, #29, #31, #36, #37, #38
Metric: configure_ms_median 8591ms → 31ms (-99.6% overall). Neko-enabled valid baseline 46ms → 31ms (-32.6%).
@firetiger-agent
Copy link
Copy Markdown

Firetiger deploy monitoring skipped

This PR didn't match the auto-monitor filter configured on your GitHub connection:

PRs in the kernel, infra, hypeman, and hypeship repos. kernel is a ~mono repo with many logical services underneath, ensure to focus on the implicated service for the PR

Reason: PR appears to be about display/window resizing in a browser automation context, but the repository name is not mentioned and cannot be verified against the allowed repos (kernel, infra, hypeman, hypeship).

To monitor this PR anyway, reply with @firetiger monitor this.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Unused function setMaximizedWindowViaCDP is dead code
    • Removed the unused setMaximizedWindowViaCDP helper from display.go since it had no callers.
  • ✅ Fixed: omitempty drops explicit zero position in CDP bounds
    • Changed the resize Browser.setWindowBounds payload to an explicit map so left: 0 and top: 0 are always sent.

Create PR

Or push these changes by commenting:

@cursor push 66c2aadc5d
Preview (66c2aadc5d)
diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go
--- a/server/cmd/api/api/display.go
+++ b/server/cmd/api/api/display.go
@@ -493,34 +493,6 @@
 	return nil
 }
 
-func (s *ApiService) setMaximizedWindowViaCDP(ctx context.Context, width, height int) error {
-	log := logger.FromContext(ctx)
-
-	upstreamURL := s.upstreamMgr.Current()
-	if upstreamURL == "" {
-		return fmt.Errorf("devtools upstream not available")
-	}
-
-	cdpCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
-	defer cancel()
-
-	client, err := cdpclient.Dial(cdpCtx, upstreamURL)
-	if err != nil {
-		return fmt.Errorf("failed to connect to devtools: %w", err)
-	}
-	defer client.Close()
-
-	if err := client.SetFirstPageWindowBoundsAndMaximize(cdpCtx, width, height); err != nil {
-		return fmt.Errorf("CDP setWindowBounds: %w", err)
-	}
-	if err := s.verifyMaximizedChromiumWindow(ctx, width, height); err != nil {
-		return err
-	}
-
-	log.Info("chromium window resized and maximized via CDP", "width", width, "height", height)
-	return nil
-}
-
 func (s *ApiService) verifyCurrentCDP(ctx context.Context) error {
 	upstreamURL := s.upstreamMgr.Current()
 	if upstreamURL == "" {

diff --git a/server/lib/cdpclient/cdpclient.go b/server/lib/cdpclient/cdpclient.go
--- a/server/lib/cdpclient/cdpclient.go
+++ b/server/lib/cdpclient/cdpclient.go
@@ -193,11 +193,11 @@
 	}
 	if _, err := c.send(ctx, "Browser.setWindowBounds", map[string]any{
 		"windowId": window.WindowID,
-		"bounds": browserWindowBounds{
-			Left:   0,
-			Top:    0,
-			Width:  width,
-			Height: height,
+		"bounds": map[string]any{
+			"left":   0,
+			"top":    0,
+			"width":  width,
+			"height": height,
 		},
 	}, ""); err != nil {
 		return fmt.Errorf("Browser.setWindowBounds size: %w", err)

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 785a321. Configure here.


log.Info("chromium window resized and maximized via CDP", "width", width, "height", height)
return nil
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unused function setMaximizedWindowViaCDP is dead code

Low Severity

The newly added setMaximizedWindowViaCDP method is never called anywhere in the codebase. It's defined but has zero callers, making it dead code that adds maintenance burden without providing value.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 785a321. Configure here.

Width int `json:"width,omitempty"`
Height int `json:"height,omitempty"`
WindowState string `json:"windowState,omitempty"`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

omitempty drops explicit zero position in CDP bounds

Low Severity

The browserWindowBounds struct uses omitempty on Left and Top int fields. In SetFirstPageWindowBoundsAndMaximize, the code explicitly sets Left: 0, Top: 0 intending to move the window to the origin, but omitempty silently drops zero-valued ints from the JSON payload. The CDP call will only resize the window without repositioning it.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 785a321. Configure here.

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.

1 participant