Skip to content

Separate live and restart configure paths #342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rgarcia merged 3 commits into from
Aug 21, 2026
+186 −104
Merged

Separate live and restart configure paths #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b871a41
Separate Chromium configure execution paths
rgarcia Aug 19, 2026
4e940fc
Preserve configure path ordering
rgarcia Aug 19, 2026
26a037a
Simplify configure path completion
rgarcia Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 126 additions & 88 deletions server/cmd/api/api/chromium_configure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (st *chromiumConfigureState) cleanup() {
}

// ChromiumConfigure batched Chromium/session configuration plus optional navigation.
func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.ChromiumConfigureRequestObject) (resp oapi.ChromiumConfigureResponseObject, err error) {
func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.ChromiumConfigureRequestObject) (oapi.ChromiumConfigureResponseObject, error) {
start := time.Now()

if request.Body == nil {
Expand Down Expand Up @@ -80,7 +80,68 @@ func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.Chromiu
s.chromiumConfigMu.Lock()
defer s.chromiumConfigMu.Unlock()

needsStop := chromiumNeedsStopCycle(st)
var configureResp oapi.ChromiumConfigureResponseObject
switch chromiumConfigureModeFor(st) {
case chromiumConfigureModeLive:
configureResp = s.chromiumConfigureLive(ctx, st, spec)
case chromiumConfigureModeRestart:
configureResp = s.chromiumConfigureRestart(ctx, st, spec)
default:
return cfg500Configure("unhandled configure mode"), nil
}
if configureResp != nil {
return configureResp, nil
}

logger.FromContext(ctx).Info("chromium configure finished", "elapsed", time.Since(start).String())
return oapi.ChromiumConfigure200JSONResponse{Ok: true}, nil
}

type chromiumConfigureMode uint8

const (
chromiumConfigureModeLive chromiumConfigureMode = iota
chromiumConfigureModeRestart
)

func chromiumConfigureModeFor(st *chromiumConfigureState) chromiumConfigureMode {
if st.hasProfile ||
len(st.extItems) > 0 ||
policiesContentNonEmpty(st.chromePoliciesJSON) ||
flagsContentNonEmpty(st.chromiumFlagsJSON) {
return chromiumConfigureModeRestart
}
return chromiumConfigureModeLive
}

func (s *ApiService) chromiumConfigureLive(ctx context.Context, st *chromiumConfigureState, spec startURLParsed) oapi.ChromiumConfigureResponseObject {
if st.displayJSON != nil && strings.TrimSpace(*st.displayJSON) != "" {
displayPlan, displayResp := chromiumPrepareDisplay(ctx, s, st.displayJSON)
if displayResp != nil {
return displayResp
}
if displayPlan != nil {
if resp := chromiumRunPatchDisplay(ctx, s, displayPlan.body); resp != nil {
return resp
}
}
}

chromiumConfigureNavigate(ctx, s, spec)
return nil
}

func chromiumConfigureNavigate(ctx context.Context, s *ApiService, spec startURLParsed) {
if !spec.needsNav {
return
}
if err := chromiumDoNavigate(ctx, s, spec); err != nil {
logger.FromContext(ctx).Warn("start_url dispatch failed", "error", err)
}
}

func (s *ApiService) chromiumConfigureRestart(ctx context.Context, st *chromiumConfigureState, spec startURLParsed) (resp oapi.ChromiumConfigureResponseObject) {
var stoppedRecordings []stoppedRecordingInfo
chromiumStopped := false
restartAfterStop := func() error {
if !chromiumStopped {
Expand All @@ -93,109 +154,93 @@ func (s *ApiService) ChromiumConfigure(ctx context.Context, request oapi.Chromiu
return nil
}
defer func() {
// Error paths restart recordings before Chromium recovery. chromiumStopped
// also covers a panic before the explicit restart below.
if (resp != nil || chromiumStopped) && len(stoppedRecordings) > 0 {
go s.startNewRecordingSegments(context.WithoutCancel(ctx), stoppedRecordings)
stoppedRecordings = nil
}
if restartErr := restartAfterStop(); restartErr != nil {
if resp != nil {
logger.FromContext(ctx).Error("failed to restart chromium after configure error", "error", restartErr)
return
}
resp = cfg500ConfigureStep(chromiumConfigureStepStart, restartErr.Error())
err = nil
}
}()

if needsStop {
logger.FromContext(ctx).Info("chromium configure (stop/start path)")
if err := s.stopChromium(ctx); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepStop, err.Error()), nil
}
chromiumStopped = true
logger.FromContext(ctx).Info("chromium configure (stop/start path)")
if err := s.stopChromium(ctx); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepStop, err.Error())
}
chromiumStopped = true

policyOverrides, err := chromiumValidatePolicies(st.chromePoliciesJSON)
if err != nil {
return cfgResponseFromStepError(chromiumConfigureStepPolicies, err), nil
}
if err := chromiumApplyPolicies(ctx, s, policyOverrides); err != nil {
return cfgResponseFromStepError(chromiumConfigureStepPolicies, err), nil
}
policyOverrides, err := chromiumValidatePolicies(st.chromePoliciesJSON)
if err != nil {
return cfgResponseFromStepError(chromiumConfigureStepPolicies, err)
}
if err := chromiumApplyPolicies(ctx, s, policyOverrides); err != nil {
return cfgResponseFromStepError(chromiumConfigureStepPolicies, err)
}

if reqMsgs, ierr := chromiumApplyExtensions(ctx, s, st.extItems); reqMsgs != "" {
return cfg400(fmt.Sprintf("%s: %s", chromiumConfigureStepExtensions, reqMsgs)), nil
} else if ierr != nil {
return cfg500ConfigureStep(chromiumConfigureStepExtensions, ierr.Error()), nil
}
if reqMsgs, ierr := chromiumApplyExtensions(ctx, s, st.extItems); reqMsgs != "" {
return cfg400(fmt.Sprintf("%s: %s", chromiumConfigureStepExtensions, reqMsgs))
} else if ierr != nil {
return cfg500ConfigureStep(chromiumConfigureStepExtensions, ierr.Error())
}

if st.displayJSON != nil && strings.TrimSpace(*st.displayJSON) != "" {
displayPlan, displayResp := chromiumPrepareDisplay(ctx, s, st.displayJSON)
if displayResp != nil {
return displayResp, nil
if st.displayJSON != nil && strings.TrimSpace(*st.displayJSON) != "" {
displayPlan, displayResp := chromiumPrepareDisplay(ctx, s, st.displayJSON)
if displayResp != nil {
return displayResp
}
if displayPlan != nil {
stopped, stopErr := s.stopActiveRecordings(ctx)
if stopErr != nil {
return cfg500ConfigureStep(chromiumConfigureStepDisplay, fmt.Sprintf("failed to stop recordings: %v", stopErr))
}
if displayPlan != nil {
stopped, stopErr := s.stopActiveRecordings(ctx)
if stopErr != nil {
return cfg500ConfigureStep(chromiumConfigureStepDisplay, fmt.Sprintf("failed to stop recordings: %v", stopErr)), nil
}
if len(stopped) > 0 {
defer func() {
go s.startNewRecordingSegments(context.WithoutCancel(ctx), stopped)
}()
}
if rr := chromiumDisplayApplyWhileStopped(ctx, s, displayPlan); rr != nil {
return rr, nil
}
stoppedRecordings = stopped
if rr := chromiumDisplayApplyWhileStopped(ctx, s, displayPlan); rr != nil {
return rr
}
}
}

flagsPlan, err := chromiumValidateFlags(st.chromiumFlagsJSON)
if err != nil {
return cfgResponseFromStepError(chromiumConfigureStepFlags, err), nil
}
if err := chromiumMergeFlags(ctx, s, flagsPlan); err != nil {
return cfgResponseFromStepError(chromiumConfigureStepFlags, err), nil
}
flagsPlan, err := chromiumValidateFlags(st.chromiumFlagsJSON)
if err != nil {
return cfgResponseFromStepError(chromiumConfigureStepFlags, err)
}
if err := chromiumMergeFlags(ctx, s, flagsPlan); err != nil {
return cfgResponseFromStepError(chromiumConfigureStepFlags, err)
}

if st.hasProfile {
preparedProfile, cleanupProfile, err := chromiumPrepareProfileArchive(st.profileTemp, st.stripComponents)
if cleanupProfile != nil {
defer cleanupProfile()
}
if err != nil {
return cfg500ConfigureStep(chromiumConfigureStepProfile, err.Error()), nil
}
if spec.needsNav {
if err := stripProfileSessionRestore(preparedProfile); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepProfile, err.Error()), nil
}
}
if err := chromiumInstallPreparedProfile(preparedProfile); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepProfile, err.Error()), nil
}
if st.hasProfile {
preparedProfile, cleanupProfile, err := chromiumPrepareProfileArchive(st.profileTemp, st.stripComponents)
if cleanupProfile != nil {
defer cleanupProfile()
}

if err := restartAfterStop(); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepStart, err.Error()), nil
if err != nil {
return cfg500ConfigureStep(chromiumConfigureStepProfile, err.Error())
}
} else {
if st.displayJSON != nil && strings.TrimSpace(*st.displayJSON) != "" {
displayPlan, displayResp := chromiumPrepareDisplay(ctx, s, st.displayJSON)
if displayResp != nil {
return displayResp, nil
}
if displayPlan != nil {
if rr := chromiumRunPatchDisplay(ctx, s, displayPlan.body); rr != nil {
return rr, nil
}
if spec.needsNav {
if err := stripProfileSessionRestore(preparedProfile); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepProfile, err.Error())
}
}
}

if spec.needsNav {
if err := chromiumDoNavigate(ctx, s, spec); err != nil {
logger.FromContext(ctx).Warn("start_url dispatch failed", "error", err)
if err := chromiumInstallPreparedProfile(preparedProfile); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepProfile, err.Error())
}
}

logger.FromContext(ctx).Info("chromium configure finished", "elapsed", time.Since(start).String())
return oapi.ChromiumConfigure200JSONResponse{Ok: true}, nil
if err := restartAfterStop(); err != nil {
return cfg500ConfigureStep(chromiumConfigureStepStart, err.Error())
}
chromiumConfigureNavigate(ctx, s, spec)
if len(stoppedRecordings) > 0 {
go s.startNewRecordingSegments(context.WithoutCancel(ctx), stoppedRecordings)
stoppedRecordings = nil
}
return nil
}

type startURLParsed struct {
Expand Down Expand Up @@ -300,13 +345,6 @@ type chromiumDisplayPlan struct {
refreshRate int
}

func chromiumNeedsStopCycle(st *chromiumConfigureState) bool {
return st.hasProfile ||
len(st.extItems) > 0 ||
policiesContentNonEmpty(st.chromePoliciesJSON) ||
flagsContentNonEmpty(st.chromiumFlagsJSON)
}

func policiesContentNonEmpty(s *string) bool {
if !policiesNonEmpty(s) {
return false
Expand Down
59 changes: 43 additions & 16 deletions server/cmd/api/api/chromium_configure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,57 @@ func TestPoliciesContentNonEmpty(t *testing.T) {
require.True(t, policiesContentNonEmpty(&real))
}

func TestChromiumConfigureActionableFlags(t *testing.T) {
emptyFlags := `{"flags":[]}`
realFlags := `{"flags":["--kiosk"]}`
func TestChromiumConfigureModeFor(t *testing.T) {
stringPtr := func(value string) *string { return &value }

st := &chromiumConfigureState{chromiumFlagsJSON: &emptyFlags}
require.Equal(t, 0, cfgActionables(st))
require.False(t, chromiumNeedsStopCycle(st))
tests := []struct {
name string
state chromiumConfigureState
want chromiumConfigureMode
}{
{name: "no restart fields", want: chromiumConfigureModeLive},
{name: "display only", state: chromiumConfigureState{displayJSON: stringPtr(`{"width":1280}`)}, want: chromiumConfigureModeLive},
{name: "start URL only", state: chromiumConfigureState{startURLRaw: stringPtr("https://example.com")}, want: chromiumConfigureModeLive},
{name: "empty policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{}`)}, want: chromiumConfigureModeLive},
{name: "nonempty policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{"QuicAllowed":false}`)}, want: chromiumConfigureModeRestart},
{name: "invalid policies", state: chromiumConfigureState{chromePoliciesJSON: stringPtr(`{bad-json`)}, want: chromiumConfigureModeRestart},
{name: "empty flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{"flags":[]}`)}, want: chromiumConfigureModeLive},
{name: "nonempty flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{"flags":["--kiosk"]}`)}, want: chromiumConfigureModeRestart},
{name: "invalid flags", state: chromiumConfigureState{chromiumFlagsJSON: stringPtr(`{bad-json`)}, want: chromiumConfigureModeRestart},
{name: "profile", state: chromiumConfigureState{hasProfile: true}, want: chromiumConfigureModeRestart},
{name: "extensions", state: chromiumConfigureState{extItems: []extensionZipItem{{name: "test"}}}, want: chromiumConfigureModeRestart},
{name: "display and extension", state: chromiumConfigureState{displayJSON: stringPtr(`{"width":1280}`), extItems: []extensionZipItem{{name: "test"}}}, want: chromiumConfigureModeRestart},
}

st = &chromiumConfigureState{chromiumFlagsJSON: &realFlags}
require.Equal(t, 1, cfgActionables(st))
require.True(t, chromiumNeedsStopCycle(st))
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.want, chromiumConfigureModeFor(&tt.state))
})
}
}

func TestChromiumConfigureActionablePolicies(t *testing.T) {
func TestChromiumConfigureActionables(t *testing.T) {
emptyFlags := `{"flags":[]}`
realFlags := `{"flags":["--kiosk"]}`
emptyPolicies := `{}`
realPolicies := `{"QuicAllowed":false}`

st := &chromiumConfigureState{chromePoliciesJSON: &emptyPolicies}
require.Equal(t, 0, cfgActionables(st))
require.False(t, chromiumNeedsStopCycle(st))
tests := []struct {
name string
state chromiumConfigureState
want int
}{
{name: "empty flags", state: chromiumConfigureState{chromiumFlagsJSON: &emptyFlags}},
{name: "nonempty flags", state: chromiumConfigureState{chromiumFlagsJSON: &realFlags}, want: 1},
{name: "empty policies", state: chromiumConfigureState{chromePoliciesJSON: &emptyPolicies}},
{name: "nonempty policies", state: chromiumConfigureState{chromePoliciesJSON: &realPolicies}, want: 1},
}

st = &chromiumConfigureState{chromePoliciesJSON: &realPolicies}
require.Equal(t, 1, cfgActionables(st))
require.True(t, chromiumNeedsStopCycle(st))
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.want, cfgActionables(&tt.state))
})
}
}

func TestChromiumStartURLSpec(t *testing.T) {
Expand Down
17 changes: 17 additions & 0 deletions server/e2e/e2e_chromium_configure_powerset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestChromiumConfigureMultipartPowerset(t *testing.T) {

matrix := []int{
matDisplay,
matStartURL,
matPolicy | matKioskFlags,
matExtension,
matDisplay | matPolicy | matKioskFlags | matExtension | matStartURL,
Expand Down Expand Up @@ -73,6 +74,9 @@ func TestChromiumConfigureMultipartPowerset(t *testing.T) {
defer func() { _ = c.Stop(context.WithoutCancel(ctx)) }()

require.NoError(t, c.WaitReady(ctx))
require.NoError(t, c.WaitDevTools(ctx))
browserWebSocketBefore, err := fetchBrowserWebSocketURL(ctx, c)
require.NoError(t, err)

var body bytes.Buffer
w := multipart.NewWriter(&body)
Expand All @@ -89,10 +93,23 @@ func TestChromiumConfigureMultipartPowerset(t *testing.T) {
"bits=%02x unexpected status=%s body=%s", bits, rsp.Status(), string(rsp.Body))
require.NotNil(t, rsp.JSON200, "want ok JSON")
require.True(t, rsp.JSON200.Ok)

browserWebSocketAfter, err := fetchBrowserWebSocketURL(ctx, c)
require.NoError(t, err)
if chromiumConfigurePowersetRestarts(bits) {
require.NotEqual(t, browserWebSocketBefore, browserWebSocketAfter, "restart path must replace the browser WebSocket identity")
} else {
require.Equal(t, browserWebSocketBefore, browserWebSocketAfter, "live path must preserve the browser WebSocket identity")
}
})
}
}

func chromiumConfigurePowersetRestarts(bits int) bool {
// This matrix runs against headless Xvfb, where display-only configure stays live.
return bits&(matPolicy|matKioskFlags|matExtension) != 0
}

func chromiumConfigurePowersetLabel(bits int) string {
var p []string
if bits&matDisplay != 0 {
Expand Down
Loading