Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 30 additions & 3 deletions server/cmd/api/api/browser_location.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import (

const (
browserLocationApplyTimeout = 5 * time.Second
// browserLocationStartupTimeout bounds how long after its DevTools endpoint
// comes up Chromium may take to notice /etc/localtime being replaced. Until
// then it ignores the replacement, and one restoring a slow saved tab can
// take longer than browserLocationApplyTimeout.
browserLocationStartupTimeout = 10 * time.Second
// browserLocationRoundTimeout bounds how long one attempt waits for
// Chromium's acknowledgements before it sends the location again.
browserLocationRoundTimeout = 1500 * time.Millisecond
Expand Down Expand Up @@ -143,7 +148,7 @@ func (s *ApiService) startBrowserLocationReconcileLocked(bundle oapi.BrowserLoca

func (s *ApiService) reconcileBrowserLocation(ctx context.Context, bundle oapi.BrowserLocationBundle) {
started := time.Now()
deadline := started.Add(browserLocationApplyTimeout)
deadline := browserLocationDeadline(started, s.chromiumSince())
for {
if ctx.Err() != nil || !s.browserLocationCurrent(bundle) {
return
Expand Down Expand Up @@ -183,8 +188,9 @@ func (s *ApiService) reconcileBrowserLocation(ctx context.Context, bundle oapi.B
s.browserLocation.lastError = err.Error()
s.browserLocation.failures.Add(1)
}
components := s.browserLocation.components
s.browserLocationMu.Unlock()
logger.FromContext(ctx).Error("browser location did not converge", "error", err, "generation", bundle.Generation)
logger.FromContext(ctx).Error("browser location did not converge", "error", err, "generation", bundle.Generation, "components", components)
return
}
s.browserLocation.retries.Add(1)
Expand All @@ -196,6 +202,27 @@ func (s *ApiService) reconcileBrowserLocation(ctx context.Context, bundle oapi.B
}
}

// browserLocationDeadline is when an attempt that started at started stops
// retrying: browserLocationApplyTimeout after it started, or
// browserLocationStartupTimeout after Chromium's DevTools endpoint came up at
// chromiumSince, whichever is later.
func browserLocationDeadline(started, chromiumSince time.Time) time.Time {
deadline := started.Add(browserLocationApplyTimeout)
if startup := chromiumSince.Add(browserLocationStartupTimeout); startup.After(deadline) {
return startup
}
return deadline
}

// chromiumSince is when the running Chromium's DevTools endpoint came up, or
// the zero time if it is unknown.
func (s *ApiService) chromiumSince() time.Time {
if s.upstreamMgr == nil {
return time.Time{}
}
return s.upstreamMgr.Since()
}

// watchPendingRenderers follows the renderers that were pending when bundle
// applied until they acknowledge it. If the browser then reports a component
// that didn't converge, the location is no longer applied and is sent again.
Expand Down Expand Up @@ -597,7 +624,7 @@ func browserLocationError(message string) *string {
// waitBrowserLocationApplied waits until the running browser confirms bundle,
// or reports why it didn't.
func (s *ApiService) waitBrowserLocationApplied(ctx context.Context, bundle oapi.BrowserLocationBundle) error {
ctx, cancel := context.WithTimeout(ctx, browserLocationApplyTimeout+time.Second)
ctx, cancel := context.WithDeadline(ctx, browserLocationDeadline(time.Now(), s.chromiumSince()).Add(time.Second))
defer cancel()
ticker := time.NewTicker(25 * time.Millisecond)
defer ticker.Stop()
Expand Down
36 changes: 36 additions & 0 deletions server/cmd/api/api/browser_location_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,42 @@ func TestSetBrowserLocationAllowsPendingRenderers(t *testing.T) {
assert.Equal(t, 1, status.Components.RenderersPending)
}

// Chromium ignores /etc/localtime being replaced until it has finished
// starting, which restoring a slow saved tab delays past
// browserLocationApplyTimeout. A location sent to a Chromium that just started
// keeps replacing it until Chromium observes it.
func TestSetBrowserLocationWaitsForStartingChromiumToObserveTimezone(t *testing.T) {
service := newBrowserLocationStateService(t)
observesFrom := time.Now().Add(browserLocationApplyTimeout + 500*time.Millisecond)
var observed atomic.Bool
previous := setBrowserLocaltime
setBrowserLocaltime = func(string) error {
if time.Now().After(observesFrom) {
observed.Store(true)
}
return nil
}
t.Cleanup(func() { setBrowserLocaltime = previous })
fakeLocationChromium(t, service, func() map[string]any {
return map[string]any{"renderersConverged": true, "renderersPending": 0, "timezoneConverged": observed.Load()}
})

resp := service.applyBrowserLocation(context.Background(), testLocationBundle(1))

require.IsType(t, oapi.SetBrowserLocation200JSONResponse{}, resp, "error: %s", snapshotError(service))
assert.True(t, observed.Load())
}

func TestBrowserLocationDeadline(t *testing.T) {
started := time.Now()

assert.Equal(t, started.Add(browserLocationApplyTimeout), browserLocationDeadline(started, time.Time{}), "an unknown Chromium gets the apply timeout")
settled := started.Add(-time.Minute)
assert.Equal(t, started.Add(browserLocationApplyTimeout), browserLocationDeadline(started, settled), "a Chromium past startup gets the apply timeout")
starting := started.Add(-time.Second)
assert.Equal(t, starting.Add(browserLocationStartupTimeout), browserLocationDeadline(started, starting), "a starting Chromium gets until it is past startup")
}

func TestBrowserLocationConfirmed(t *testing.T) {
converged := oapi.BrowserLocationComponents{Browser: true, Timezone: true, Renderers: true, NetworkContexts: true}
pending := converged
Expand Down
14 changes: 14 additions & 0 deletions server/lib/devtoolsproxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ type UpstreamManager struct {
logger *slog.Logger

currentURL atomic.Value // string
// currentSince is when currentURL last changed, in wall-clock Unix
// nanoseconds.
currentSince atomic.Int64

startOnce sync.Once
stopOnce sync.Once
Expand Down Expand Up @@ -154,10 +157,21 @@ func (u *UpstreamManager) Current() string {
return val
}

// Since returns when the current upstream URL was discovered, or the zero
// time if none has been.
func (u *UpstreamManager) Since() time.Time {
since := u.currentSince.Load()
if since == 0 {
return time.Time{}
}
return time.Unix(0, since)
}

func (u *UpstreamManager) setCurrent(url string) {
prev := u.Current()
if url != "" && url != prev {
u.logger.Info("devtools upstream updated", slog.String("url", url))
u.currentSince.Store(time.Now().UnixNano())
u.currentURL.Store(url)
// Broadcast update to subscribers without blocking. If a subscriber's
// channel buffer (size 1) is full, replace the buffered value with the
Expand Down
Loading