diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index 285ae8ff..36879515 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/base64" + "errors" "fmt" "log/slog" "os" @@ -594,19 +595,35 @@ func (s *ApiService) applyResolutionAndConverge(ctx context.Context, width, heig } // setViewportViaCDP resizes the browser viewport using the CDP -// Emulation.setDeviceMetricsOverride command. This is near-instant and does -// not require restarting Chromium or Xvfb. +// Browser.setContentsSize command. This is near-instant and does not require +// restarting Chromium or Xvfb. When Chromium refuses to resize the window (a +// fullscreen window, or a CDP error), it falls back to +// Emulation.setDeviceMetricsOverride, which Chromium 154+ clears as soon as +// this client detaches, so on those versions the viewport stays as it was. func (s *ApiService) setViewportViaCDP(ctx context.Context, width, height int) error { log := logger.FromContext(ctx) if err := s.withCDPClient(ctx, func(cdpCtx context.Context, client *cdpclient.Client) error { + err := client.SetContentsSize(cdpCtx, width, height) + if !contentsSizeRefused(err) { + return err + } + log.Warn("CDP setContentsSize refused, falling back to setDeviceMetricsOverride", "error", err) return client.SetDeviceMetricsOverride(cdpCtx, width, height) }); err != nil { - return fmt.Errorf("CDP setDeviceMetricsOverride: %w", err) + return fmt.Errorf("CDP viewport resize: %w", err) } log.Info("viewport resized via CDP", "width", width, "height", height) return nil } +// contentsSizeRefused reports whether SetContentsSize failed because Chromium +// declined to resize the window, as opposed to the page target or the +// connection being unavailable, where the fallback would fail the same way. +func contentsSizeRefused(err error) bool { + var cdpErr *cdpclient.Error + return errors.Is(err, cdpclient.ErrWindowFullscreen) || errors.As(err, &cdpErr) +} + // anyRecordingActive returns true if any registered recorder is currently recording. func (s *ApiService) anyRecordingActive(ctx context.Context) bool { for _, r := range s.recordManager.ListActiveRecorders(ctx) { diff --git a/server/cmd/api/api/display_test.go b/server/cmd/api/api/display_test.go index fd283b87..da1bfc9f 100644 --- a/server/cmd/api/api/display_test.go +++ b/server/cmd/api/api/display_test.go @@ -2,6 +2,8 @@ package api import ( "context" + "errors" + "fmt" "io" "log/slog" "os" @@ -9,6 +11,7 @@ import ( "testing" "time" + "github.com/kernel/kernel-images/server/lib/cdpclient" "github.com/kernel/kernel-images/server/lib/logger" oapi "github.com/kernel/kernel-images/server/lib/oapi" "github.com/kernel/kernel-images/server/lib/recorder" @@ -254,6 +257,26 @@ func TestStopAndStartNewSegment_RoundTrip(t *testing.T) { _ = newRec.Stop(ctx) } +func TestContentsSizeRefused(t *testing.T) { + cdpErr := &cdpclient.Error{Code: -32000, Message: "Restore window to normal state before setting content size"} + for _, tc := range []struct { + name string + err error + want bool + }{ + {name: "success", err: nil, want: false}, + {name: "fullscreen", err: cdpclient.ErrWindowFullscreen, want: true}, + {name: "cdp error", err: fmt.Errorf("Browser.setContentsSize: %w", cdpErr), want: true}, + {name: "no page target", err: errors.New("no page target found"), want: false}, + {name: "context", err: fmt.Errorf("read: %w", context.DeadlineExceeded), want: false}, + {name: "connection", err: fmt.Errorf("read: %w", cdpclient.ErrOutcomeUnknown), want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, contentsSizeRefused(tc.err)) + }) + } +} + func TestResolveDisplayParams(t *testing.T) { intPtr := func(v int) *int { return &v } rrPtr := func(v int) *oapi.PatchDisplayRequestRefreshRate { diff --git a/server/e2e/e2e_display_resize_window_test.go b/server/e2e/e2e_display_resize_window_test.go index 276e65bb..4d618f82 100644 --- a/server/e2e/e2e_display_resize_window_test.go +++ b/server/e2e/e2e_display_resize_window_test.go @@ -226,10 +226,10 @@ func waitForRendererViewport(t *testing.T, ctx context.Context, c *TestContainer } // headlessViewportPredicate asserts innerWidth/innerHeight match the request. -// In --headless=new the renderer uses Chrome's internal window -// (Emulation.setDeviceMetricsOverride drives inner*) — screen.* and outer* -// stay at Chrome's startup defaults and do not move on resize, so they are -// intentionally not asserted. +// In --headless=new the renderer uses Chrome's internal window, which the +// server sizes around the page (Browser.setContentsSize drives inner*) — +// screen.* stays at Chrome's startup default and outer* includes the window +// frame, so they are intentionally not asserted. func headlessViewportPredicate(v rendererViewport, wantW, wantH int) bool { return v.InnerWidth == wantW && v.InnerHeight == wantH } @@ -260,9 +260,10 @@ func abs(x int) int { // inject --kiosk into the default CHROMIUM_FLAGS). predicate decides when a // renderer viewport reading is acceptable for this scenario. type resizeScenario struct { - name string - image string - extraEnv map[string]string + name string + image string + extraEnv map[string]string + // predicate is nil when the renderer viewport is not asserted. predicate viewportPredicate skipReason string // assertXRoot toggles X-root convergence checks. @@ -275,6 +276,9 @@ type resizeScenario struct { // requireBaselineState asserts the window's CDP windowState at startup // equals this value before any resize. Empty = no assertion. requireBaselineState string + // resizedState is the windowState expected after each resize. Empty = + // unchanged from the baseline. + resizedState string } func (sc resizeScenario) upSize() (int, int) { @@ -308,6 +312,30 @@ func TestDisplayResizeChromiumWindow(t *testing.T) { predicate: headlessViewportPredicate, assertXRoot: true, // background Xvfb resize must converge }, + { + // Production headless launches maximized. Chromium refuses + // Browser.setContentsSize on a maximized window, so the server + // restores it to normal before resizing. + name: "headless_start_maximized", + image: headlessImage, + extraEnv: map[string]string{"CHROMIUM_FLAGS": "--start-maximized --disable-infobars --test-type"}, + predicate: headlessViewportPredicate, + assertXRoot: true, + requireBaselineState: "maximized", + resizedState: "normal", + }, + { + // A fullscreen window is not resized: leaving fullscreen would + // exit kiosk mode. The server falls back to the device metrics + // override, which Chromium 154+ clears when the server's CDP + // session detaches, so the viewport is not asserted. The request + // must still succeed and the window must stay fullscreen. + name: "headless_kiosk", + image: headlessImage, + extraEnv: map[string]string{"CHROMIUM_FLAGS": "--kiosk --disable-infobars --test-type"}, + assertXRoot: true, + requireBaselineState: "fullscreen", + }, { // Production-equivalent scenario: --start-maximized + neko + // the CHROMIUM_FLAGS_DEFAULT mirror from run-docker.sh:17. @@ -423,6 +451,10 @@ func TestDisplayResizeChromiumWindow(t *testing.T) { require.Equal(t, sc.requireBaselineState, baseCDP.WindowState, "baseline windowState mismatch: chromium did not come up in the expected state — production behaviour relies on this invariant") } + wantState := baseCDP.WindowState + if sc.resizedState != "" { + wantState = sc.resizedState + } // Resize up: must be a real delta from the baseline so the X root // actually changes. Headful starts at Neko's default 1920x1080; @@ -432,15 +464,17 @@ func TestDisplayResizeChromiumWindow(t *testing.T) { if sc.assertXRoot { waitForXRootResolution(t, ctx, c, upW, upH, 30*time.Second) } - waitForRendererViewport(t, ctx, c, upW, upH, sc.predicate, sc.name+":up", 60*time.Second) + if sc.predicate != nil { + waitForRendererViewport(t, ctx, c, upW, upH, sc.predicate, sc.name+":up", 60*time.Second) + } cdpAfterUp, err := getChromiumWindowBoundsCDP(ctx, c) require.NoError(t, err, "cdp window after up-resize") t.Logf("[%s] after-up cdp=%+v", sc.name, cdpAfterUp) require.Equal(t, baseCDP.WindowID, cdpAfterUp.WindowID, "windowID changed after up-resize — chromium was restarted (windowID is monotonic per-process; a new one signals a relaunch)") - require.Equal(t, baseCDP.WindowState, cdpAfterUp.WindowState, - "windowState changed after up-resize — the WM-tracking invariant the no-restart path relies on was broken") + require.Equal(t, wantState, cdpAfterUp.WindowState, + "unexpected windowState after up-resize — the WM-tracking invariant the no-restart path relies on was broken") // Resize back down to a smaller size, also a real delta. dnW, dnH := sc.downSize() @@ -448,15 +482,17 @@ func TestDisplayResizeChromiumWindow(t *testing.T) { if sc.assertXRoot { waitForXRootResolution(t, ctx, c, dnW, dnH, 30*time.Second) } - waitForRendererViewport(t, ctx, c, dnW, dnH, sc.predicate, sc.name+":down", 60*time.Second) + if sc.predicate != nil { + waitForRendererViewport(t, ctx, c, dnW, dnH, sc.predicate, sc.name+":down", 60*time.Second) + } cdpAfterDown, err := getChromiumWindowBoundsCDP(ctx, c) require.NoError(t, err, "cdp window after down-resize") t.Logf("[%s] after-down cdp=%+v", sc.name, cdpAfterDown) require.Equal(t, baseCDP.WindowID, cdpAfterDown.WindowID, "windowID changed after down-resize — chromium was restarted between resizes") - require.Equal(t, baseCDP.WindowState, cdpAfterDown.WindowState, - "windowState changed after down-resize") + require.Equal(t, wantState, cdpAfterDown.WindowState, + "unexpected windowState after down-resize") }) } } diff --git a/server/e2e/playwright/index.ts b/server/e2e/playwright/index.ts index b674a2b1..4412282f 100644 --- a/server/e2e/playwright/index.ts +++ b/server/e2e/playwright/index.ts @@ -326,7 +326,10 @@ class CDPClient { await this.page.goto('chrome://extensions'); console.log('[cdp] enabling developer mode'); - const devMode = this.page.getByRole('button', { name: 'Developer mode' }); + // Chromium 154 changed cr-toggle from role=button to role=switch. + const devMode = this.page + .getByRole('switch', { name: 'Developer mode' }) + .or(this.page.getByRole('button', { name: 'Developer mode' })); await devMode.click(); console.log('[cdp] checking for MV3 Service Worker Test extension'); diff --git a/server/lib/cdpclient/cdpclient.go b/server/lib/cdpclient/cdpclient.go index baeff807..21c09113 100644 --- a/server/lib/cdpclient/cdpclient.go +++ b/server/lib/cdpclient/cdpclient.go @@ -776,6 +776,45 @@ func (c *Client) GetWindowBounds(ctx context.Context) (WindowBounds, error) { }, nil } +// ErrWindowFullscreen is returned by SetContentsSize when the window is +// fullscreen. Leaving fullscreen would exit kiosk mode or a page's HTML5 +// fullscreen, so the window is left as it is. +var ErrWindowFullscreen = errors.New("window is fullscreen") + +// SetContentsSize resizes the OS window backing the first page target so its +// web contents are width x height, via Browser.setContentsSize. Chromium +// rejects that for a maximized or minimized window, so such a window is first +// restored to normal. The size belongs to the window and outlives this client, +// unlike Emulation.setDeviceMetricsOverride, which Chromium 154+ clears when +// the session that set it detaches. +func (c *Client) SetContentsSize(ctx context.Context, width, height int) error { + bounds, err := c.GetWindowBounds(ctx) + if err != nil { + return err + } + + switch bounds.WindowState { + case "fullscreen": + return ErrWindowFullscreen + case "maximized", "minimized": + if _, err := c.Send(ctx, "Browser.setWindowBounds", map[string]any{ + "windowId": bounds.WindowID, + "bounds": map[string]any{"windowState": "normal"}, + }, ""); err != nil { + return fmt.Errorf("Browser.setWindowBounds normal: %w", err) + } + } + + if _, err := c.Send(ctx, "Browser.setContentsSize", map[string]any{ + "windowId": bounds.WindowID, + "width": width, + "height": height, + }, ""); err != nil { + return fmt.Errorf("Browser.setContentsSize: %w", err) + } + return nil +} + // SetDeviceMetricsOverride sets the viewport dimensions on the first page // target found in the browser. It attaches to the target with a flattened // session, sends Emulation.setDeviceMetricsOverride, then detaches. diff --git a/server/lib/cdpclient/cdpclient_test.go b/server/lib/cdpclient/cdpclient_test.go index 542eaf11..00d6c4b7 100644 --- a/server/lib/cdpclient/cdpclient_test.go +++ b/server/lib/cdpclient/cdpclient_test.go @@ -49,6 +49,12 @@ type fakeCDP struct { createParams map[string]any closedTargetID string closeTargetMissing bool + windowID int + windowState string + browserCalls []string + setWindowBoundsArgs map[string]any + setContentsArgs map[string]any + failSetContents bool } func (f *fakeCDP) handler(w http.ResponseWriter, r *http.Request) { @@ -115,6 +121,24 @@ func (f *fakeCDP) handler(w http.ResponseWriter, r *http.Request) { f.setMetricsHeight = int(params["height"].(float64)) result = map[string]any{} } + case "Browser.getWindowForTarget": + f.browserCalls = append(f.browserCalls, req.Method) + result = map[string]any{ + "windowId": f.windowID, + "bounds": map[string]any{"width": 800, "height": 600, "windowState": f.windowState}, + } + case "Browser.setWindowBounds": + f.browserCalls = append(f.browserCalls, req.Method) + _ = json.Unmarshal(req.Params, &f.setWindowBoundsArgs) + result = map[string]any{} + case "Browser.setContentsSize": + f.browserCalls = append(f.browserCalls, req.Method) + if f.failSetContents { + cdpErr = &Error{Code: -32000, Message: "Restore window to normal state before setting content size"} + } else { + _ = json.Unmarshal(req.Params, &f.setContentsArgs) + result = map[string]any{} + } case "Target.detachFromTarget": f.detachCalled = true result = map[string]any{} @@ -332,6 +356,64 @@ func TestSetDeviceMetricsOverride(t *testing.T) { }) } +func TestSetContentsSize(t *testing.T) { + dial := func(t *testing.T, f *fakeCDP) *Client { + t.Helper() + client, err := Dial(context.Background(), startFakeCDP(t, f)) + require.NoError(t, err) + t.Cleanup(func() { client.Close() }) + return client + } + + t.Run("normal window", func(t *testing.T) { + f := &fakeCDP{pageTargetID: "target-123", windowID: 7, windowState: "normal"} + require.NoError(t, dial(t, f).SetContentsSize(context.Background(), 1920, 1080)) + + assert.Equal(t, []string{"Browser.getWindowForTarget", "Browser.setContentsSize"}, f.browserCalls) + assert.Equal(t, map[string]any{"windowId": 7.0, "width": 1920.0, "height": 1080.0}, f.setContentsArgs) + }) + + for _, state := range []string{"maximized", "minimized"} { + t.Run(state+" window is restored first", func(t *testing.T) { + f := &fakeCDP{pageTargetID: "target-123", windowID: 7, windowState: state} + require.NoError(t, dial(t, f).SetContentsSize(context.Background(), 1280, 720)) + + assert.Equal(t, []string{"Browser.getWindowForTarget", "Browser.setWindowBounds", "Browser.setContentsSize"}, f.browserCalls) + assert.Equal(t, map[string]any{"windowId": 7.0, "bounds": map[string]any{"windowState": "normal"}}, f.setWindowBoundsArgs) + assert.Equal(t, map[string]any{"windowId": 7.0, "width": 1280.0, "height": 720.0}, f.setContentsArgs) + }) + } + + t.Run("fullscreen window is left alone", func(t *testing.T) { + f := &fakeCDP{pageTargetID: "target-123", windowID: 7, windowState: "fullscreen"} + err := dial(t, f).SetContentsSize(context.Background(), 1280, 720) + + assert.ErrorIs(t, err, ErrWindowFullscreen) + assert.Equal(t, []string{"Browser.getWindowForTarget"}, f.browserCalls) + }) + + t.Run("setContentsSize failure", func(t *testing.T) { + f := &fakeCDP{pageTargetID: "target-123", windowID: 7, windowState: "normal", failSetContents: true} + err := dial(t, f).SetContentsSize(context.Background(), 1280, 720) + + var cdpErr *Error + require.ErrorAs(t, err, &cdpErr) + assert.Contains(t, err.Error(), "Browser.setContentsSize") + }) + + t.Run("no page target", func(t *testing.T) { + defer func(d time.Duration) { pageTargetWaitTimeout = d }(pageTargetWaitTimeout) + pageTargetWaitTimeout = 100 * time.Millisecond + + f := &fakeCDP{returnNoPageTargets: true} + err := dial(t, f).SetContentsSize(context.Background(), 1280, 720) + + require.Error(t, err) + assert.Contains(t, err.Error(), "no page target found") + assert.Empty(t, f.browserCalls) + }) +} + func TestDispatchStartURLAndWait(t *testing.T) { t.Run("waits for loaded destination", func(t *testing.T) { f := &fakeCDP{