From 1dc811d0c9bf359361c30da95ab56c8b39c55310 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:56:11 +0000 Subject: [PATCH 1/5] Wait for a page target before resizing the display DevTools can accept connections shortly before Chromium opens its first tab, at startup and after a restart. firstPageTargetID looked up the page target once and failed immediately, so PATCH /display and the display step of /chromium/configure returned 500 "no page target found" in that window. Poll Target.getTargets for up to 5s before giving up, still honoring the caller's context. --- server/lib/cdpclient/cdpclient.go | 35 ++++++++++++++++-- server/lib/cdpclient/cdpclient_test.go | 49 +++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/server/lib/cdpclient/cdpclient.go b/server/lib/cdpclient/cdpclient.go index e2b2a974..8a00db54 100644 --- a/server/lib/cdpclient/cdpclient.go +++ b/server/lib/cdpclient/cdpclient.go @@ -592,10 +592,39 @@ func relatedHosts(a, b string) bool { return a == b || strings.HasSuffix(a, "."+b) || strings.HasSuffix(b, "."+a) } +// pageTargetWaitTimeout bounds how long firstPageTargetID waits for a page +// target. DevTools can accept connections shortly before Chromium opens its +// first tab, both at startup and after a restart. +const ( + pageTargetWaitTimeout = 5 * time.Second + pageTargetPollInterval = 100 * time.Millisecond +) + // firstPageTargetID returns the targetId of the first page target reported -// by Target.getTargets. Callers that need to operate on the user-facing -// browser window (Emulation, Browser.* window bounds) use this to find it. +// by Target.getTargets, polling for up to pageTargetWaitTimeout if none +// exists yet. Callers that need to operate on the user-facing browser window +// (Emulation, Browser.* window bounds) use this to find it. func (c *Client) firstPageTargetID(ctx context.Context) (string, error) { + deadline := time.Now().Add(pageTargetWaitTimeout) + for { + targetID, err := c.findPageTargetID(ctx) + if err != nil || targetID != "" { + return targetID, err + } + if time.Now().After(deadline) { + return "", fmt.Errorf("no page target found") + } + select { + case <-ctx.Done(): + return "", fmt.Errorf("no page target found: %w", ctx.Err()) + case <-time.After(pageTargetPollInterval): + } + } +} + +// findPageTargetID returns the targetId of the first page target, or "" if +// there is none. +func (c *Client) findPageTargetID(ctx context.Context) (string, error) { targetsResult, err := c.Send(ctx, "Target.getTargets", nil, "") if err != nil { return "", fmt.Errorf("Target.getTargets: %w", err) @@ -614,7 +643,7 @@ func (c *Client) firstPageTargetID(ctx context.Context) (string, error) { return t.TargetID, nil } } - return "", fmt.Errorf("no page target found") + return "", nil } // SetWindowBoundsMaximized puts the OS window backing the first page target diff --git a/server/lib/cdpclient/cdpclient_test.go b/server/lib/cdpclient/cdpclient_test.go index a5b1db6e..b9b7b733 100644 --- a/server/lib/cdpclient/cdpclient_test.go +++ b/server/lib/cdpclient/cdpclient_test.go @@ -19,6 +19,7 @@ import ( // SetDeviceMetricsOverride and GetBrowserVersion. type fakeCDP struct { getTargetsCalled bool + getTargetsCalls int attachCalled bool setMetricsCalled bool setMetricsWidth int @@ -29,6 +30,9 @@ type fakeCDP struct { failGetTargets bool failSetMetrics bool returnNoPageTargets bool + // noPageTargetsFor makes the first N Target.getTargets calls return no + // page target, as Chromium does before it opens its first tab. + noPageTargetsFor int getVersionCalled bool failGetVersion bool productResponse string @@ -71,11 +75,12 @@ func (f *fakeCDP) handler(w http.ResponseWriter, r *http.Request) { switch req.Method { case "Target.getTargets": f.getTargetsCalled = true + f.getTargetsCalls++ if f.failGetTargets { cdpErr = &Error{Code: -1, Message: "mock error"} } else { targets := []map[string]string{} - if !f.returnNoPageTargets { + if !f.returnNoPageTargets && f.getTargetsCalls > f.noPageTargetsFor { targets = append(targets, map[string]string{ "targetId": f.pageTargetID, "type": "page", @@ -206,9 +211,51 @@ func TestSetDeviceMetricsOverride(t *testing.T) { require.NoError(t, err) defer client.Close() + start := time.Now() err = client.SetDeviceMetricsOverride(ctx, 1920, 1080) require.Error(t, err) assert.Contains(t, err.Error(), "no page target found") + assert.GreaterOrEqual(t, time.Since(start), pageTargetWaitTimeout) + assert.False(t, f.attachCalled) + }) + + t.Run("waits for first page target", func(t *testing.T) { + f := &fakeCDP{ + pageTargetID: "target-123", + sessionID: "session-abc", + noPageTargetsFor: 3, + } + url := startFakeCDP(t, f) + + ctx := context.Background() + client, err := Dial(ctx, url) + require.NoError(t, err) + defer client.Close() + + err = client.SetDeviceMetricsOverride(ctx, 1920, 1080) + require.NoError(t, err) + + assert.Equal(t, 4, f.getTargetsCalls) + assert.True(t, f.setMetricsCalled) + }) + + t.Run("no page target respects context", func(t *testing.T) { + f := &fakeCDP{ + returnNoPageTargets: true, + } + url := startFakeCDP(t, f) + + client, err := Dial(context.Background(), url) + require.NoError(t, err) + defer client.Close() + + ctx, cancel := context.WithCancel(context.Background()) + time.AfterFunc(250*time.Millisecond, cancel) + + start := time.Now() + err = client.SetDeviceMetricsOverride(ctx, 1920, 1080) + require.Error(t, err) + assert.Less(t, time.Since(start), pageTargetWaitTimeout) }) t.Run("getTargets failure", func(t *testing.T) { From cfb1fdd8bc6fa0a09490f7e1e15eeddf8f3e704b Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:08:53 +0000 Subject: [PATCH 2/5] Shorten page target wait in tests and assert cancellation error --- server/lib/cdpclient/cdpclient.go | 10 +++++----- server/lib/cdpclient/cdpclient_test.go | 4 ++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/server/lib/cdpclient/cdpclient.go b/server/lib/cdpclient/cdpclient.go index 8a00db54..eb8faa9a 100644 --- a/server/lib/cdpclient/cdpclient.go +++ b/server/lib/cdpclient/cdpclient.go @@ -594,11 +594,11 @@ func relatedHosts(a, b string) bool { // pageTargetWaitTimeout bounds how long firstPageTargetID waits for a page // target. DevTools can accept connections shortly before Chromium opens its -// first tab, both at startup and after a restart. -const ( - pageTargetWaitTimeout = 5 * time.Second - pageTargetPollInterval = 100 * time.Millisecond -) +// first tab, both at startup and after a restart. It is a var so tests can +// shorten it. +var pageTargetWaitTimeout = 5 * time.Second + +const pageTargetPollInterval = 100 * time.Millisecond // firstPageTargetID returns the targetId of the first page target reported // by Target.getTargets, polling for up to pageTargetWaitTimeout if none diff --git a/server/lib/cdpclient/cdpclient_test.go b/server/lib/cdpclient/cdpclient_test.go index b9b7b733..77a45fc1 100644 --- a/server/lib/cdpclient/cdpclient_test.go +++ b/server/lib/cdpclient/cdpclient_test.go @@ -201,6 +201,9 @@ func TestSetDeviceMetricsOverride(t *testing.T) { }) t.Run("no page target", func(t *testing.T) { + defer func(d time.Duration) { pageTargetWaitTimeout = d }(pageTargetWaitTimeout) + pageTargetWaitTimeout = 300 * time.Millisecond + f := &fakeCDP{ returnNoPageTargets: true, } @@ -255,6 +258,7 @@ func TestSetDeviceMetricsOverride(t *testing.T) { start := time.Now() err = client.SetDeviceMetricsOverride(ctx, 1920, 1080) require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) assert.Less(t, time.Since(start), pageTargetWaitTimeout) }) From 58a8cad2da2bdd85e5a9434d2e93afc4345972e7 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:18:28 +0000 Subject: [PATCH 3/5] Clamp final page target poll to the deadline and tighten wait tests --- server/lib/cdpclient/cdpclient.go | 13 +++++++------ server/lib/cdpclient/cdpclient_test.go | 11 ++++++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/server/lib/cdpclient/cdpclient.go b/server/lib/cdpclient/cdpclient.go index eb8faa9a..407b2d7a 100644 --- a/server/lib/cdpclient/cdpclient.go +++ b/server/lib/cdpclient/cdpclient.go @@ -594,11 +594,12 @@ func relatedHosts(a, b string) bool { // pageTargetWaitTimeout bounds how long firstPageTargetID waits for a page // target. DevTools can accept connections shortly before Chromium opens its -// first tab, both at startup and after a restart. It is a var so tests can -// shorten it. -var pageTargetWaitTimeout = 5 * time.Second - -const pageTargetPollInterval = 100 * time.Millisecond +// first tab, both at startup and after a restart. These are vars so tests can +// adjust them. +var ( + pageTargetWaitTimeout = 5 * time.Second + pageTargetPollInterval = 100 * time.Millisecond +) // firstPageTargetID returns the targetId of the first page target reported // by Target.getTargets, polling for up to pageTargetWaitTimeout if none @@ -617,7 +618,7 @@ func (c *Client) firstPageTargetID(ctx context.Context) (string, error) { select { case <-ctx.Done(): return "", fmt.Errorf("no page target found: %w", ctx.Err()) - case <-time.After(pageTargetPollInterval): + case <-time.After(min(pageTargetPollInterval, time.Until(deadline))): } } } diff --git a/server/lib/cdpclient/cdpclient_test.go b/server/lib/cdpclient/cdpclient_test.go index 77a45fc1..abf643c4 100644 --- a/server/lib/cdpclient/cdpclient_test.go +++ b/server/lib/cdpclient/cdpclient_test.go @@ -209,7 +209,10 @@ func TestSetDeviceMetricsOverride(t *testing.T) { } url := startFakeCDP(t, f) - ctx := context.Background() + // The context outlives the wait timeout so the test fails instead of + // hanging if the wait is not bounded. + ctx, cancel := context.WithTimeout(context.Background(), pageTargetWaitTimeout+time.Second) + defer cancel() client, err := Dial(ctx, url) require.NoError(t, err) defer client.Close() @@ -218,6 +221,7 @@ func TestSetDeviceMetricsOverride(t *testing.T) { err = client.SetDeviceMetricsOverride(ctx, 1920, 1080) require.Error(t, err) assert.Contains(t, err.Error(), "no page target found") + assert.NotErrorIs(t, err, context.DeadlineExceeded) assert.GreaterOrEqual(t, time.Since(start), pageTargetWaitTimeout) assert.False(t, f.attachCalled) }) @@ -243,6 +247,11 @@ func TestSetDeviceMetricsOverride(t *testing.T) { }) t.Run("no page target respects context", func(t *testing.T) { + // A poll interval longer than the wait timeout means only the + // ctx.Done() case can end the wait before the deadline. + defer func(d time.Duration) { pageTargetPollInterval = d }(pageTargetPollInterval) + pageTargetPollInterval = time.Minute + f := &fakeCDP{ returnNoPageTargets: true, } From a0ea77498e47c6e8b6c12ad122de31f3956c7576 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:27:38 +0000 Subject: [PATCH 4/5] Cover the page target deadline clamp in tests --- server/lib/cdpclient/cdpclient_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/lib/cdpclient/cdpclient_test.go b/server/lib/cdpclient/cdpclient_test.go index abf643c4..aba43658 100644 --- a/server/lib/cdpclient/cdpclient_test.go +++ b/server/lib/cdpclient/cdpclient_test.go @@ -203,6 +203,10 @@ func TestSetDeviceMetricsOverride(t *testing.T) { t.Run("no page target", func(t *testing.T) { defer func(d time.Duration) { pageTargetWaitTimeout = d }(pageTargetWaitTimeout) pageTargetWaitTimeout = 300 * time.Millisecond + // A poll interval longer than the wait timeout means the wait only + // ends on time if the last sleep is clamped to the deadline. + defer func(d time.Duration) { pageTargetPollInterval = d }(pageTargetPollInterval) + pageTargetPollInterval = time.Minute f := &fakeCDP{ returnNoPageTargets: true, From dee70d12ebaf077d305e61a9473622d08939478e Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 5 Oct 2026 19:29:28 +0000 Subject: [PATCH 5/5] Trim page target wait comments and drop redundant getTargetsCalled --- server/lib/cdpclient/cdpclient.go | 6 ++---- server/lib/cdpclient/cdpclient_test.go | 17 ++++------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/server/lib/cdpclient/cdpclient.go b/server/lib/cdpclient/cdpclient.go index 407b2d7a..3c53e8c0 100644 --- a/server/lib/cdpclient/cdpclient.go +++ b/server/lib/cdpclient/cdpclient.go @@ -592,10 +592,8 @@ func relatedHosts(a, b string) bool { return a == b || strings.HasSuffix(a, "."+b) || strings.HasSuffix(b, "."+a) } -// pageTargetWaitTimeout bounds how long firstPageTargetID waits for a page -// target. DevTools can accept connections shortly before Chromium opens its -// first tab, both at startup and after a restart. These are vars so tests can -// adjust them. +// DevTools can accept connections shortly before Chromium opens its first tab, +// so firstPageTargetID polls for a page target. Vars so tests can adjust them. var ( pageTargetWaitTimeout = 5 * time.Second pageTargetPollInterval = 100 * time.Millisecond diff --git a/server/lib/cdpclient/cdpclient_test.go b/server/lib/cdpclient/cdpclient_test.go index aba43658..01408b64 100644 --- a/server/lib/cdpclient/cdpclient_test.go +++ b/server/lib/cdpclient/cdpclient_test.go @@ -18,7 +18,6 @@ import ( // fakeCDP is a minimal CDP server that responds to the commands used by // SetDeviceMetricsOverride and GetBrowserVersion. type fakeCDP struct { - getTargetsCalled bool getTargetsCalls int attachCalled bool setMetricsCalled bool @@ -30,8 +29,6 @@ type fakeCDP struct { failGetTargets bool failSetMetrics bool returnNoPageTargets bool - // noPageTargetsFor makes the first N Target.getTargets calls return no - // page target, as Chromium does before it opens its first tab. noPageTargetsFor int getVersionCalled bool failGetVersion bool @@ -74,7 +71,6 @@ func (f *fakeCDP) handler(w http.ResponseWriter, r *http.Request) { switch req.Method { case "Target.getTargets": - f.getTargetsCalled = true f.getTargetsCalls++ if f.failGetTargets { cdpErr = &Error{Code: -1, Message: "mock error"} @@ -192,7 +188,7 @@ func TestSetDeviceMetricsOverride(t *testing.T) { err = client.SetDeviceMetricsOverride(ctx, 1920, 1080) require.NoError(t, err) - assert.True(t, f.getTargetsCalled) + assert.Equal(t, 1, f.getTargetsCalls) assert.True(t, f.attachCalled) assert.True(t, f.setMetricsCalled) assert.True(t, f.detachCalled) @@ -203,18 +199,15 @@ func TestSetDeviceMetricsOverride(t *testing.T) { t.Run("no page target", func(t *testing.T) { defer func(d time.Duration) { pageTargetWaitTimeout = d }(pageTargetWaitTimeout) pageTargetWaitTimeout = 300 * time.Millisecond - // A poll interval longer than the wait timeout means the wait only - // ends on time if the last sleep is clamped to the deadline. defer func(d time.Duration) { pageTargetPollInterval = d }(pageTargetPollInterval) - pageTargetPollInterval = time.Minute + pageTargetPollInterval = time.Minute // only the deadline clamp ends the wait in time f := &fakeCDP{ returnNoPageTargets: true, } url := startFakeCDP(t, f) - // The context outlives the wait timeout so the test fails instead of - // hanging if the wait is not bounded. + // Fail instead of hanging if the wait is unbounded. ctx, cancel := context.WithTimeout(context.Background(), pageTargetWaitTimeout+time.Second) defer cancel() client, err := Dial(ctx, url) @@ -251,10 +244,8 @@ func TestSetDeviceMetricsOverride(t *testing.T) { }) t.Run("no page target respects context", func(t *testing.T) { - // A poll interval longer than the wait timeout means only the - // ctx.Done() case can end the wait before the deadline. defer func(d time.Duration) { pageTargetPollInterval = d }(pageTargetPollInterval) - pageTargetPollInterval = time.Minute + pageTargetPollInterval = time.Minute // only ctx.Done() can end the wait early f := &fakeCDP{ returnNoPageTargets: true,