fix(core): decouple viewport sync and honor config overrides
- commit
- 5fef01b3190ef8f61c93d76f921f277356ee99e6
- author
- brice · · 2026-05-21 06:40 UTC
- parent
- 92b4503
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +395 -31
M internal/infrastructure/cef/webview.go +29 -8
| @@ -191,11 +191,12 @@ type WebView struct { | |||
| 191 | 191 | loadDiagLastLoadStateAt time.Time | |
| 192 | 192 | ||
| 193 | 193 | // Atomic state. | |
| 194 | - | destroyed atomic.Bool | |
| 195 | - | fullscreen atomic.Bool | |
| 196 | - | generation atomic.Uint64 | |
| 197 | - | audioPlaying atomic.Bool | |
| 198 | - | zoomFactor atomic.Value // float64, initialized to 1.0 | |
| 194 | + | destroyed atomic.Bool | |
| 195 | + | fullscreen atomic.Bool | |
| 196 | + | generation atomic.Uint64 | |
| 197 | + | audioPlaying atomic.Bool | |
| 198 | + | zoomFactor atomic.Value // float64, initialized to 1.0 | |
| 199 | + | lastAppliedOSRBackingScaleBits atomic.Uint64 | |
| 199 | 200 | ||
| 200 | 201 | // Browser creation defaults copied from the factory so native popup shells | |
| 201 | 202 | // can apply the same settings in OnBeforePopup. | |
| @@ -463,7 +464,7 @@ func pageZoomFromCEFAndBackingLevel(level, backingScale float64) float64 { | |||
| 463 | 464 | return factorFromCEFZoom(level) * normalizeScale(backingScale) | |
| 464 | 465 | } | |
| 465 | 466 | ||
| 466 | - | func (wv *WebView) applyCEFZoomLevel(host purecef.BrowserHost, factor, cefLevel float64) { | |
| 467 | + | func (wv *WebView) applyCEFZoomLevel(host purecef.BrowserHost, factor, cefLevel, backingScale float64) { | |
| 467 | 468 | host.SetZoomLevel(cefLevel) | |
| 468 | 469 | // Force CEF to produce a new frame at the new zoom level. In OSR mode, | |
| 469 | 470 | // SetZoomLevel changes the Blink layout zoom but doesn't guarantee a | |
| @@ -472,6 +473,7 @@ func (wv *WebView) applyCEFZoomLevel(host purecef.BrowserHost, factor, cefLevel | |||
| 472 | 473 | // SynchronizeVisualProperties cycle, which makes the renderer produce | |
| 473 | 474 | // a new compositor frame at the new zoom level. | |
| 474 | 475 | host.NotifyScreenInfoChanged() | |
| 476 | + | wv.recordAppliedOSRBackingScale(backingScale) | |
| 475 | 477 | // Zoom is applied asynchronously in the renderer process. Request a couple | |
| 476 | 478 | // of follow-up refreshes on the CEF UI thread so OSR captures the updated | |
| 477 | 479 | // compositor frame after the zoom IPC has been processed. | |
| @@ -511,7 +513,7 @@ func (wv *WebView) SetZoomLevel(_ context.Context, factor float64) error { | |||
| 511 | 513 | Float64("cef_level", cefLevel). | |
| 512 | 514 | Float64("osr_backing_scale", backingScale). | |
| 513 | 515 | Msg("cef: SetZoomLevel") | |
| 514 | - | wv.applyCEFZoomLevel(host, factor, cefLevel) | |
| 516 | + | wv.applyCEFZoomLevel(host, factor, cefLevel, backingScale) | |
| 515 | 517 | wv.zoomFactor.Store(factor) | |
| 516 | 518 | return nil | |
| 517 | 519 | } | |
| @@ -1415,6 +1417,25 @@ func (wv *WebView) osrBackingScaleFactor() float64 { | |||
| 1415 | 1417 | return normalizeScale(wv.viewBridge.OSRBackingScaleFactor()) | |
| 1416 | 1418 | } | |
| 1417 | 1419 | ||
| 1420 | + | func (wv *WebView) recordAppliedOSRBackingScale(scale float64) { | |
| 1421 | + | if wv == nil { | |
| 1422 | + | return | |
| 1423 | + | } | |
| 1424 | + | wv.lastAppliedOSRBackingScaleBits.Store(math.Float64bits(normalizeScale(scale))) | |
| 1425 | + | } | |
| 1426 | + | ||
| 1427 | + | func (wv *WebView) shouldReapplyZoomForBackingScale(scale float64) bool { | |
| 1428 | + | if wv == nil { | |
| 1429 | + | return false | |
| 1430 | + | } | |
| 1431 | + | normalized := normalizeScale(scale) | |
| 1432 | + | bits := wv.lastAppliedOSRBackingScaleBits.Load() | |
| 1433 | + | if bits == 0 { | |
| 1434 | + | return true | |
| 1435 | + | } | |
| 1436 | + | return math.Float64frombits(bits) != normalized | |
| 1437 | + | } | |
| 1438 | + | ||
| 1418 | 1439 | func (wv *WebView) handleMiddleClickFromBridge() bool { | |
| 1419 | 1440 | if wv == nil { | |
| 1420 | 1441 | return false | |
| @@ -1701,7 +1722,7 @@ func (wv *WebView) reapplyCurrentZoomForBackingScale(reason string) { | |||
| 1701 | 1722 | factor := wv.GetZoomLevel() | |
| 1702 | 1723 | backingScale := wv.osrBackingScaleFactor() | |
| 1703 | 1724 | cefLevel := cefZoomFromPageAndBackingFactor(factor, backingScale) | |
| 1704 | - | wv.applyCEFZoomLevel(host, factor, cefLevel) | |
| 1725 | + | wv.applyCEFZoomLevel(host, factor, cefLevel, backingScale) | |
| 1705 | 1726 | logging.FromContext(wv.ctx).Debug(). | |
| 1706 | 1727 | Str("reason", reason). | |
| 1707 | 1728 | Float64("factor", factor). | |
| @@ -191,11 +191,12 @@ type WebView struct { | |||||
| 191 | loadDiagLastLoadStateAt time.Time | 191 | loadDiagLastLoadStateAt time.Time | ||
| 192 | 192 | ||||
| 193 | // Atomic state. | 193 | // Atomic state. | ||
| 194 | - | destroyed atomic.Bool | 194 | + | destroyed atomic.Bool |
| 195 | - | fullscreen atomic.Bool | 195 | + | fullscreen atomic.Bool |
| 196 | - | generation atomic.Uint64 | 196 | + | generation atomic.Uint64 |
| 197 | - | audioPlaying atomic.Bool | 197 | + | audioPlaying atomic.Bool |
| 198 | - | zoomFactor atomic.Value // float64, initialized to 1.0 | 198 | + | zoomFactor atomic.Value // float64, initialized to 1.0 |
| 199 | + | lastAppliedOSRBackingScaleBits atomic.Uint64 | |||
| 199 | 200 | ||||
| 200 | // Browser creation defaults copied from the factory so native popup shells | 201 | // Browser creation defaults copied from the factory so native popup shells | ||
| 201 | // can apply the same settings in OnBeforePopup. | 202 | // can apply the same settings in OnBeforePopup. | ||
| @@ -463,7 +464,7 @@ func pageZoomFromCEFAndBackingLevel(level, backingScale float64) float64 { | |||||
| 463 | return factorFromCEFZoom(level) * normalizeScale(backingScale) | 464 | return factorFromCEFZoom(level) * normalizeScale(backingScale) | ||
| 464 | } | 465 | } | ||
| 465 | 466 | ||||
| 466 | - | func (wv *WebView) applyCEFZoomLevel(host purecef.BrowserHost, factor, cefLevel float64) { | 467 | + | func (wv *WebView) applyCEFZoomLevel(host purecef.BrowserHost, factor, cefLevel, backingScale float64) { |
| 467 | host.SetZoomLevel(cefLevel) | 468 | host.SetZoomLevel(cefLevel) | ||
| 468 | // Force CEF to produce a new frame at the new zoom level. In OSR mode, | 469 | // Force CEF to produce a new frame at the new zoom level. In OSR mode, | ||
| 469 | // SetZoomLevel changes the Blink layout zoom but doesn't guarantee a | 470 | // SetZoomLevel changes the Blink layout zoom but doesn't guarantee a | ||
| @@ -472,6 +473,7 @@ func (wv *WebView) applyCEFZoomLevel(host purecef.BrowserHost, factor, cefLevel | |||||
| 472 | // SynchronizeVisualProperties cycle, which makes the renderer produce | 473 | // SynchronizeVisualProperties cycle, which makes the renderer produce | ||
| 473 | // a new compositor frame at the new zoom level. | 474 | // a new compositor frame at the new zoom level. | ||
| 474 | host.NotifyScreenInfoChanged() | 475 | host.NotifyScreenInfoChanged() | ||
| 476 | + | wv.recordAppliedOSRBackingScale(backingScale) | |||
| 475 | // Zoom is applied asynchronously in the renderer process. Request a couple | 477 | // Zoom is applied asynchronously in the renderer process. Request a couple | ||
| 476 | // of follow-up refreshes on the CEF UI thread so OSR captures the updated | 478 | // of follow-up refreshes on the CEF UI thread so OSR captures the updated | ||
| 477 | // compositor frame after the zoom IPC has been processed. | 479 | // compositor frame after the zoom IPC has been processed. | ||
| @@ -511,7 +513,7 @@ func (wv *WebView) SetZoomLevel(_ context.Context, factor float64) error { | |||||
| 511 | Float64("cef_level", cefLevel). | 513 | Float64("cef_level", cefLevel). | ||
| 512 | Float64("osr_backing_scale", backingScale). | 514 | Float64("osr_backing_scale", backingScale). | ||
| 513 | Msg("cef: SetZoomLevel") | 515 | Msg("cef: SetZoomLevel") | ||
| 514 | - | wv.applyCEFZoomLevel(host, factor, cefLevel) | 516 | + | wv.applyCEFZoomLevel(host, factor, cefLevel, backingScale) |
| 515 | wv.zoomFactor.Store(factor) | 517 | wv.zoomFactor.Store(factor) | ||
| 516 | return nil | 518 | return nil | ||
| 517 | } | 519 | } | ||
| @@ -1415,6 +1417,25 @@ func (wv *WebView) osrBackingScaleFactor() float64 { | |||||
| 1415 | return normalizeScale(wv.viewBridge.OSRBackingScaleFactor()) | 1417 | return normalizeScale(wv.viewBridge.OSRBackingScaleFactor()) | ||
| 1416 | } | 1418 | } | ||
| 1417 | 1419 | ||||
| 1420 | + | func (wv *WebView) recordAppliedOSRBackingScale(scale float64) { | |||
| 1421 | + | if wv == nil { | |||
| 1422 | + | return | |||
| 1423 | + | } | |||
| 1424 | + | wv.lastAppliedOSRBackingScaleBits.Store(math.Float64bits(normalizeScale(scale))) | |||
| 1425 | + | } | |||
| 1426 | + | ||||
| 1427 | + | func (wv *WebView) shouldReapplyZoomForBackingScale(scale float64) bool { | |||
| 1428 | + | if wv == nil { | |||
| 1429 | + | return false | |||
| 1430 | + | } | |||
| 1431 | + | normalized := normalizeScale(scale) | |||
| 1432 | + | bits := wv.lastAppliedOSRBackingScaleBits.Load() | |||
| 1433 | + | if bits == 0 { | |||
| 1434 | + | return true | |||
| 1435 | + | } | |||
| 1436 | + | return math.Float64frombits(bits) != normalized | |||
| 1437 | + | } | |||
| 1438 | + | ||||
| 1418 | func (wv *WebView) handleMiddleClickFromBridge() bool { | 1439 | func (wv *WebView) handleMiddleClickFromBridge() bool { | ||
| 1419 | if wv == nil { | 1440 | if wv == nil { | ||
| 1420 | return false | 1441 | return false | ||
| @@ -1701,7 +1722,7 @@ func (wv *WebView) reapplyCurrentZoomForBackingScale(reason string) { | |||||
| 1701 | factor := wv.GetZoomLevel() | 1722 | factor := wv.GetZoomLevel() | ||
| 1702 | backingScale := wv.osrBackingScaleFactor() | 1723 | backingScale := wv.osrBackingScaleFactor() | ||
| 1703 | cefLevel := cefZoomFromPageAndBackingFactor(factor, backingScale) | 1724 | cefLevel := cefZoomFromPageAndBackingFactor(factor, backingScale) | ||
| 1704 | - | wv.applyCEFZoomLevel(host, factor, cefLevel) | 1725 | + | wv.applyCEFZoomLevel(host, factor, cefLevel, backingScale) |
| 1705 | logging.FromContext(wv.ctx).Debug(). | 1726 | logging.FromContext(wv.ctx).Debug(). | ||
| 1706 | Str("reason", reason). | 1727 | Str("reason", reason). | ||
| 1707 | Float64("factor", factor). | 1728 | Float64("factor", factor). | ||
M internal/infrastructure/cef/webview_handlers.go +2 -0
| @@ -617,6 +617,7 @@ func (h *handlerSet) attachAfterCreatedBrowser( | |||
| 617 | 617 | ||
| 618 | 618 | h.wv.browser = browser | |
| 619 | 619 | h.wv.host = host | |
| 620 | + | h.wv.lastAppliedOSRBackingScaleBits.Store(0) | |
| 620 | 621 | h.wv.pendingCreate = nil | |
| 621 | 622 | bridge := h.wv.viewBridge | |
| 622 | 623 | h.wv.inputAttached = bridge == nil | |
| @@ -745,6 +746,7 @@ func (h *handlerSet) OnBeforeClose(browser purecef.Browser) { | |||
| 745 | 746 | h.wv.mu.Lock() | |
| 746 | 747 | h.wv.browser = nil | |
| 747 | 748 | h.wv.host = nil | |
| 749 | + | h.wv.lastAppliedOSRBackingScaleBits.Store(0) | |
| 748 | 750 | h.wv.inputAttached = false | |
| 749 | 751 | if h.wv.findCtrl != nil { | |
| 750 | 752 | h.wv.findCtrl.setHost(nil) | |
| @@ -617,6 +617,7 @@ func (h *handlerSet) attachAfterCreatedBrowser( | |||||
| 617 | 617 | ||||
| 618 | h.wv.browser = browser | 618 | h.wv.browser = browser | ||
| 619 | h.wv.host = host | 619 | h.wv.host = host | ||
| 620 | + | h.wv.lastAppliedOSRBackingScaleBits.Store(0) | |||
| 620 | h.wv.pendingCreate = nil | 621 | h.wv.pendingCreate = nil | ||
| 621 | bridge := h.wv.viewBridge | 622 | bridge := h.wv.viewBridge | ||
| 622 | h.wv.inputAttached = bridge == nil | 623 | h.wv.inputAttached = bridge == nil | ||
| @@ -745,6 +746,7 @@ func (h *handlerSet) OnBeforeClose(browser purecef.Browser) { | |||||
| 745 | h.wv.mu.Lock() | 746 | h.wv.mu.Lock() | ||
| 746 | h.wv.browser = nil | 747 | h.wv.browser = nil | ||
| 747 | h.wv.host = nil | 748 | h.wv.host = nil | ||
| 749 | + | h.wv.lastAppliedOSRBackingScaleBits.Store(0) | |||
| 748 | h.wv.inputAttached = false | 750 | h.wv.inputAttached = false | ||
| 749 | if h.wv.findCtrl != nil { | 751 | if h.wv.findCtrl != nil { | ||
| 750 | h.wv.findCtrl.setHost(nil) | 752 | h.wv.findCtrl.setHost(nil) | ||
M internal/infrastructure/cef/webview_viewport.go +30 -1
| @@ -126,6 +126,7 @@ func (wv *WebView) syncViewportNowOnGTK(ctx context.Context, reason string) bool | |||
| 126 | 126 | widget := wv.viewBridge.Widget() | |
| 127 | 127 | visible := widget != nil && widget.IsVisible() | |
| 128 | 128 | wv.notifyViewportSyncOnCEFUIThread(host, visible) | |
| 129 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, reason) | |
| 129 | 130 | logging.FromContext(ctx).Debug(). | |
| 130 | 131 | Uint64("webview_id", uint64(wv.id)). | |
| 131 | 132 | Bool("visible", visible). | |
| @@ -471,7 +472,35 @@ func (wv *WebView) notifyViewportSyncOnCEFUIThread(host viewportSyncBrowserHost, | |||
| 471 | 472 | return | |
| 472 | 473 | } | |
| 473 | 474 | notifyBrowserViewportSync(host, visible) | |
| 474 | - | wv.reapplyCurrentZoomForBackingScale("viewport-sync") | |
| 475 | + | })) | |
| 476 | + | if task == nil { | |
| 477 | + | return | |
| 478 | + | } | |
| 479 | + | cefPostDelayedTask(purecef.ThreadIDTidUi, task, 0) | |
| 480 | + | } | |
| 481 | + | ||
| 482 | + | func (wv *WebView) syncZoomForBackingScaleOnCEFUIThread(host purecef.BrowserHost, reason string) { | |
| 483 | + | if host == nil || wv == nil { | |
| 484 | + | return | |
| 485 | + | } | |
| 486 | + | if !wv.shouldReapplyZoomForBackingScale(wv.osrBackingScaleFactor()) { | |
| 487 | + | return | |
| 488 | + | } | |
| 489 | + | task := cefNewTask(cefTaskFunc(func() { | |
| 490 | + | if wv == nil || wv.destroyed.Load() { | |
| 491 | + | return | |
| 492 | + | } | |
| 493 | + | wv.mu.RLock() | |
| 494 | + | currentHost := wv.host | |
| 495 | + | wv.mu.RUnlock() | |
| 496 | + | if currentHost != host { | |
| 497 | + | return | |
| 498 | + | } | |
| 499 | + | backingScale := wv.osrBackingScaleFactor() | |
| 500 | + | if !wv.shouldReapplyZoomForBackingScale(backingScale) { | |
| 501 | + | return | |
| 502 | + | } | |
| 503 | + | wv.reapplyCurrentZoomForBackingScale(reason) | |
| 475 | 504 | })) | |
| 476 | 505 | if task == nil { | |
| 477 | 506 | return | |
| @@ -126,6 +126,7 @@ func (wv *WebView) syncViewportNowOnGTK(ctx context.Context, reason string) bool | |||||
| 126 | widget := wv.viewBridge.Widget() | 126 | widget := wv.viewBridge.Widget() | ||
| 127 | visible := widget != nil && widget.IsVisible() | 127 | visible := widget != nil && widget.IsVisible() | ||
| 128 | wv.notifyViewportSyncOnCEFUIThread(host, visible) | 128 | wv.notifyViewportSyncOnCEFUIThread(host, visible) | ||
| 129 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, reason) | |||
| 129 | logging.FromContext(ctx).Debug(). | 130 | logging.FromContext(ctx).Debug(). | ||
| 130 | Uint64("webview_id", uint64(wv.id)). | 131 | Uint64("webview_id", uint64(wv.id)). | ||
| 131 | Bool("visible", visible). | 132 | Bool("visible", visible). | ||
| @@ -471,7 +472,35 @@ func (wv *WebView) notifyViewportSyncOnCEFUIThread(host viewportSyncBrowserHost, | |||||
| 471 | return | 472 | return | ||
| 472 | } | 473 | } | ||
| 473 | notifyBrowserViewportSync(host, visible) | 474 | notifyBrowserViewportSync(host, visible) | ||
| 474 | - | wv.reapplyCurrentZoomForBackingScale("viewport-sync") | 475 | + | })) |
| 476 | + | if task == nil { | |||
| 477 | + | return | |||
| 478 | + | } | |||
| 479 | + | cefPostDelayedTask(purecef.ThreadIDTidUi, task, 0) | |||
| 480 | + | } | |||
| 481 | + | ||||
| 482 | + | func (wv *WebView) syncZoomForBackingScaleOnCEFUIThread(host purecef.BrowserHost, reason string) { | |||
| 483 | + | if host == nil || wv == nil { | |||
| 484 | + | return | |||
| 485 | + | } | |||
| 486 | + | if !wv.shouldReapplyZoomForBackingScale(wv.osrBackingScaleFactor()) { | |||
| 487 | + | return | |||
| 488 | + | } | |||
| 489 | + | task := cefNewTask(cefTaskFunc(func() { | |||
| 490 | + | if wv == nil || wv.destroyed.Load() { | |||
| 491 | + | return | |||
| 492 | + | } | |||
| 493 | + | wv.mu.RLock() | |||
| 494 | + | currentHost := wv.host | |||
| 495 | + | wv.mu.RUnlock() | |||
| 496 | + | if currentHost != host { | |||
| 497 | + | return | |||
| 498 | + | } | |||
| 499 | + | backingScale := wv.osrBackingScaleFactor() | |||
| 500 | + | if !wv.shouldReapplyZoomForBackingScale(backingScale) { | |||
| 501 | + | return | |||
| 502 | + | } | |||
| 503 | + | wv.reapplyCurrentZoomForBackingScale(reason) | |||
| 475 | })) | 504 | })) | ||
| 476 | if task == nil { | 505 | if task == nil { | ||
| 477 | return | 506 | return | ||
M internal/infrastructure/cef/webview_viewport_test.go +106 -1
| @@ -80,7 +80,7 @@ func TestNotifyBrowserViewportSync_HiddenSkipsWasHidden(t *testing.T) { | |||
| 80 | 80 | require.Equal(t, []string{"NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | |
| 81 | 81 | } | |
| 82 | 82 | ||
| 83 | - | func TestNotifyViewportSyncOnCEFUIThread_PostsCEFWork(t *testing.T) { | |
| 83 | + | func TestNotifyViewportSyncOnCEFUIThread_PostsViewportWorkOnly(t *testing.T) { | |
| 84 | 84 | oldNewTask := cefNewTask | |
| 85 | 85 | oldPostDelayedTask := cefPostDelayedTask | |
| 86 | 86 | defer func() { | |
| @@ -113,11 +113,116 @@ func TestNotifyViewportSyncOnCEFUIThread_PostsCEFWork(t *testing.T) { | |||
| 113 | 113 | "NotifyScreenInfoChanged", | |
| 114 | 114 | "WasResized", | |
| 115 | 115 | "Invalidate", | |
| 116 | + | }, host.calls) | |
| 117 | + | } | |
| 118 | + | ||
| 119 | + | func TestNotifyViewportSyncOnCEFUIThread_RemainsViewportOnlyAcrossRepeatedResizes(t *testing.T) { | |
| 120 | + | oldNewTask := cefNewTask | |
| 121 | + | oldPostDelayedTask := cefPostDelayedTask | |
| 122 | + | defer func() { | |
| 123 | + | cefNewTask = oldNewTask | |
| 124 | + | cefPostDelayedTask = oldPostDelayedTask | |
| 125 | + | }() | |
| 126 | + | ||
| 127 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 128 | + | ||
| 129 | + | var scheduled purecef.Task | |
| 130 | + | cefPostDelayedTask = func(_ purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |
| 131 | + | if delayMs == 0 && scheduled == nil { | |
| 132 | + | scheduled = task | |
| 133 | + | } | |
| 134 | + | return 1 | |
| 135 | + | } | |
| 136 | + | ||
| 137 | + | host := &viewportSyncOrderHost{} | |
| 138 | + | wv := &WebView{ctx: context.Background(), host: host} | |
| 139 | + | ||
| 140 | + | wv.notifyViewportSyncOnCEFUIThread(host, true) | |
| 141 | + | require.NotNil(t, scheduled) | |
| 142 | + | scheduled.Execute() | |
| 143 | + | ||
| 144 | + | host.calls = nil | |
| 145 | + | scheduled = nil | |
| 146 | + | ||
| 147 | + | wv.notifyViewportSyncOnCEFUIThread(host, true) | |
| 148 | + | require.NotNil(t, scheduled) | |
| 149 | + | scheduled.Execute() | |
| 150 | + | ||
| 151 | + | require.Equal(t, []string{ | |
| 152 | + | "WasHidden", | |
| 153 | + | "NotifyScreenInfoChanged", | |
| 154 | + | "WasResized", | |
| 155 | + | "Invalidate", | |
| 156 | + | }, host.calls) | |
| 157 | + | } | |
| 158 | + | ||
| 159 | + | func TestSyncZoomForBackingScaleOnCEFUIThread_PostsZoomWork(t *testing.T) { | |
| 160 | + | oldNewTask := cefNewTask | |
| 161 | + | oldPostDelayedTask := cefPostDelayedTask | |
| 162 | + | defer func() { | |
| 163 | + | cefNewTask = oldNewTask | |
| 164 | + | cefPostDelayedTask = oldPostDelayedTask | |
| 165 | + | }() | |
| 166 | + | ||
| 167 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 168 | + | ||
| 169 | + | var scheduled purecef.Task | |
| 170 | + | cefPostDelayedTask = func(threadID purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |
| 171 | + | require.Equal(t, purecef.ThreadIDTidUi, threadID) | |
| 172 | + | require.NotNil(t, task) | |
| 173 | + | if delayMs == 0 && scheduled == nil { | |
| 174 | + | scheduled = task | |
| 175 | + | } | |
| 176 | + | return 1 | |
| 177 | + | } | |
| 178 | + | ||
| 179 | + | host := &viewportSyncOrderHost{} | |
| 180 | + | wv := &WebView{ctx: context.Background(), host: host} | |
| 181 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, "gtk-size-observer") | |
| 182 | + | ||
| 183 | + | require.Empty(t, host.calls) | |
| 184 | + | require.NotNil(t, scheduled) | |
| 185 | + | ||
| 186 | + | scheduled.Execute() | |
| 187 | + | require.Equal(t, []string{ | |
| 116 | 188 | "SetZoomLevel", | |
| 117 | 189 | "NotifyScreenInfoChanged", | |
| 118 | 190 | }, host.calls) | |
| 119 | 191 | } | |
| 120 | 192 | ||
| 193 | + | func TestSyncZoomForBackingScaleOnCEFUIThread_SkipsRedundantReapplyWhenBackingScaleUnchanged(t *testing.T) { | |
| 194 | + | oldNewTask := cefNewTask | |
| 195 | + | oldPostDelayedTask := cefPostDelayedTask | |
| 196 | + | defer func() { | |
| 197 | + | cefNewTask = oldNewTask | |
| 198 | + | cefPostDelayedTask = oldPostDelayedTask | |
| 199 | + | }() | |
| 200 | + | ||
| 201 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 202 | + | ||
| 203 | + | var scheduled purecef.Task | |
| 204 | + | cefPostDelayedTask = func(_ purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |
| 205 | + | if delayMs == 0 && scheduled == nil { | |
| 206 | + | scheduled = task | |
| 207 | + | } | |
| 208 | + | return 1 | |
| 209 | + | } | |
| 210 | + | ||
| 211 | + | host := &viewportSyncOrderHost{} | |
| 212 | + | wv := &WebView{ctx: context.Background(), host: host} | |
| 213 | + | ||
| 214 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, "gtk-size-observer") | |
| 215 | + | require.NotNil(t, scheduled) | |
| 216 | + | scheduled.Execute() | |
| 217 | + | ||
| 218 | + | host.calls = nil | |
| 219 | + | scheduled = nil | |
| 220 | + | ||
| 221 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, "gtk-size-observer") | |
| 222 | + | require.Nil(t, scheduled) | |
| 223 | + | require.Empty(t, host.calls) | |
| 224 | + | } | |
| 225 | + | ||
| 121 | 226 | func TestNotifyViewportSyncOnCEFUIThread_SkipsStaleHost(t *testing.T) { | |
| 122 | 227 | oldNewTask := cefNewTask | |
| 123 | 228 | oldPostDelayedTask := cefPostDelayedTask | |
| @@ -80,7 +80,7 @@ func TestNotifyBrowserViewportSync_HiddenSkipsWasHidden(t *testing.T) { | |||||
| 80 | require.Equal(t, []string{"NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | 80 | require.Equal(t, []string{"NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | ||
| 81 | } | 81 | } | ||
| 82 | 82 | ||||
| 83 | - | func TestNotifyViewportSyncOnCEFUIThread_PostsCEFWork(t *testing.T) { | 83 | + | func TestNotifyViewportSyncOnCEFUIThread_PostsViewportWorkOnly(t *testing.T) { |
| 84 | oldNewTask := cefNewTask | 84 | oldNewTask := cefNewTask | ||
| 85 | oldPostDelayedTask := cefPostDelayedTask | 85 | oldPostDelayedTask := cefPostDelayedTask | ||
| 86 | defer func() { | 86 | defer func() { | ||
| @@ -113,11 +113,116 @@ func TestNotifyViewportSyncOnCEFUIThread_PostsCEFWork(t *testing.T) { | |||||
| 113 | "NotifyScreenInfoChanged", | 113 | "NotifyScreenInfoChanged", | ||
| 114 | "WasResized", | 114 | "WasResized", | ||
| 115 | "Invalidate", | 115 | "Invalidate", | ||
| 116 | + | }, host.calls) | |||
| 117 | + | } | |||
| 118 | + | ||||
| 119 | + | func TestNotifyViewportSyncOnCEFUIThread_RemainsViewportOnlyAcrossRepeatedResizes(t *testing.T) { | |||
| 120 | + | oldNewTask := cefNewTask | |||
| 121 | + | oldPostDelayedTask := cefPostDelayedTask | |||
| 122 | + | defer func() { | |||
| 123 | + | cefNewTask = oldNewTask | |||
| 124 | + | cefPostDelayedTask = oldPostDelayedTask | |||
| 125 | + | }() | |||
| 126 | + | ||||
| 127 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 128 | + | ||||
| 129 | + | var scheduled purecef.Task | |||
| 130 | + | cefPostDelayedTask = func(_ purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |||
| 131 | + | if delayMs == 0 && scheduled == nil { | |||
| 132 | + | scheduled = task | |||
| 133 | + | } | |||
| 134 | + | return 1 | |||
| 135 | + | } | |||
| 136 | + | ||||
| 137 | + | host := &viewportSyncOrderHost{} | |||
| 138 | + | wv := &WebView{ctx: context.Background(), host: host} | |||
| 139 | + | ||||
| 140 | + | wv.notifyViewportSyncOnCEFUIThread(host, true) | |||
| 141 | + | require.NotNil(t, scheduled) | |||
| 142 | + | scheduled.Execute() | |||
| 143 | + | ||||
| 144 | + | host.calls = nil | |||
| 145 | + | scheduled = nil | |||
| 146 | + | ||||
| 147 | + | wv.notifyViewportSyncOnCEFUIThread(host, true) | |||
| 148 | + | require.NotNil(t, scheduled) | |||
| 149 | + | scheduled.Execute() | |||
| 150 | + | ||||
| 151 | + | require.Equal(t, []string{ | |||
| 152 | + | "WasHidden", | |||
| 153 | + | "NotifyScreenInfoChanged", | |||
| 154 | + | "WasResized", | |||
| 155 | + | "Invalidate", | |||
| 156 | + | }, host.calls) | |||
| 157 | + | } | |||
| 158 | + | ||||
| 159 | + | func TestSyncZoomForBackingScaleOnCEFUIThread_PostsZoomWork(t *testing.T) { | |||
| 160 | + | oldNewTask := cefNewTask | |||
| 161 | + | oldPostDelayedTask := cefPostDelayedTask | |||
| 162 | + | defer func() { | |||
| 163 | + | cefNewTask = oldNewTask | |||
| 164 | + | cefPostDelayedTask = oldPostDelayedTask | |||
| 165 | + | }() | |||
| 166 | + | ||||
| 167 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 168 | + | ||||
| 169 | + | var scheduled purecef.Task | |||
| 170 | + | cefPostDelayedTask = func(threadID purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |||
| 171 | + | require.Equal(t, purecef.ThreadIDTidUi, threadID) | |||
| 172 | + | require.NotNil(t, task) | |||
| 173 | + | if delayMs == 0 && scheduled == nil { | |||
| 174 | + | scheduled = task | |||
| 175 | + | } | |||
| 176 | + | return 1 | |||
| 177 | + | } | |||
| 178 | + | ||||
| 179 | + | host := &viewportSyncOrderHost{} | |||
| 180 | + | wv := &WebView{ctx: context.Background(), host: host} | |||
| 181 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, "gtk-size-observer") | |||
| 182 | + | ||||
| 183 | + | require.Empty(t, host.calls) | |||
| 184 | + | require.NotNil(t, scheduled) | |||
| 185 | + | ||||
| 186 | + | scheduled.Execute() | |||
| 187 | + | require.Equal(t, []string{ | |||
| 116 | "SetZoomLevel", | 188 | "SetZoomLevel", | ||
| 117 | "NotifyScreenInfoChanged", | 189 | "NotifyScreenInfoChanged", | ||
| 118 | }, host.calls) | 190 | }, host.calls) | ||
| 119 | } | 191 | } | ||
| 120 | 192 | ||||
| 193 | + | func TestSyncZoomForBackingScaleOnCEFUIThread_SkipsRedundantReapplyWhenBackingScaleUnchanged(t *testing.T) { | |||
| 194 | + | oldNewTask := cefNewTask | |||
| 195 | + | oldPostDelayedTask := cefPostDelayedTask | |||
| 196 | + | defer func() { | |||
| 197 | + | cefNewTask = oldNewTask | |||
| 198 | + | cefPostDelayedTask = oldPostDelayedTask | |||
| 199 | + | }() | |||
| 200 | + | ||||
| 201 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 202 | + | ||||
| 203 | + | var scheduled purecef.Task | |||
| 204 | + | cefPostDelayedTask = func(_ purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |||
| 205 | + | if delayMs == 0 && scheduled == nil { | |||
| 206 | + | scheduled = task | |||
| 207 | + | } | |||
| 208 | + | return 1 | |||
| 209 | + | } | |||
| 210 | + | ||||
| 211 | + | host := &viewportSyncOrderHost{} | |||
| 212 | + | wv := &WebView{ctx: context.Background(), host: host} | |||
| 213 | + | ||||
| 214 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, "gtk-size-observer") | |||
| 215 | + | require.NotNil(t, scheduled) | |||
| 216 | + | scheduled.Execute() | |||
| 217 | + | ||||
| 218 | + | host.calls = nil | |||
| 219 | + | scheduled = nil | |||
| 220 | + | ||||
| 221 | + | wv.syncZoomForBackingScaleOnCEFUIThread(host, "gtk-size-observer") | |||
| 222 | + | require.Nil(t, scheduled) | |||
| 223 | + | require.Empty(t, host.calls) | |||
| 224 | + | } | |||
| 225 | + | ||||
| 121 | func TestNotifyViewportSyncOnCEFUIThread_SkipsStaleHost(t *testing.T) { | 226 | func TestNotifyViewportSyncOnCEFUIThread_SkipsStaleHost(t *testing.T) { | ||
| 122 | oldNewTask := cefNewTask | 227 | oldNewTask := cefNewTask | ||
| 123 | oldPostDelayedTask := cefPostDelayedTask | 228 | oldPostDelayedTask := cefPostDelayedTask | ||
M internal/infrastructure/config/loader.go +51 -8
| @@ -19,15 +19,26 @@ import ( | |||
| 19 | 19 | type Manager struct { | |
| 20 | 20 | config *Config | |
| 21 | 21 | viper *viper.Viper | |
| 22 | + | automaticEnv automaticEnvConfig | |
| 22 | 23 | mu sync.RWMutex | |
| 23 | 24 | callbacks []func(*Config) | |
| 24 | 25 | watching bool | |
| 25 | 26 | skipNextReload bool // Set by Save() to prevent fsnotify reload from overwriting in-memory config | |
| 26 | 27 | } | |
| 27 | 28 | ||
| 29 | + | type automaticEnvConfig struct { | |
| 30 | + | enabled bool | |
| 31 | + | prefix string | |
| 32 | + | keyReplacer *strings.Replacer | |
| 33 | + | } | |
| 34 | + | ||
| 28 | 35 | // NewManager creates a new configuration manager. | |
| 29 | 36 | func NewManager() (*Manager, error) { | |
| 30 | 37 | v := viper.New() | |
| 38 | + | m := &Manager{ | |
| 39 | + | viper: v, | |
| 40 | + | callbacks: make([]func(*Config), 0), | |
| 41 | + | } | |
| 31 | 42 | ||
| 32 | 43 | // Configure Viper for TOML as default format | |
| 33 | 44 | v.SetConfigName("config") // Name without extension | |
| @@ -42,9 +53,7 @@ func NewManager() (*Manager, error) { | |||
| 42 | 53 | v.AddConfigPath(".") // Current directory for development | |
| 43 | 54 | ||
| 44 | 55 | // Set up environment variable support | |
| 45 | - | v.SetEnvPrefix("DUMBER") | |
| 46 | - | v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | |
| 47 | - | v.AutomaticEnv() | |
| 56 | + | m.configureAutomaticEnv() | |
| 48 | 57 | ||
| 49 | 58 | // Note: Most environment variables are handled automatically via AutomaticEnv() | |
| 50 | 59 | // with the DUMBER_ prefix (e.g., DUMBER_DATABASE_PATH, DUMBER_LOGGING_LEVEL). | |
| @@ -69,10 +78,7 @@ func NewManager() (*Manager, error) { | |||
| 69 | 78 | return nil, err | |
| 70 | 79 | } | |
| 71 | 80 | ||
| 72 | - | return &Manager{ | |
| 73 | - | viper: v, | |
| 74 | - | callbacks: make([]func(*Config), 0), | |
| 75 | - | }, nil | |
| 81 | + | return m, nil | |
| 76 | 82 | } | |
| 77 | 83 | ||
| 78 | 84 | // Load loads the configuration from file and environment variables. | |
| @@ -153,7 +159,7 @@ func (m *Manager) transformLegacyConfig() { | |||
| 153 | 159 | rawConfig := m.viper.AllSettings() | |
| 154 | 160 | transformer := NewLegacyConfigTransformer() | |
| 155 | 161 | transformer.TransformLegacyActions(rawConfig) | |
| 156 | - | transformer.TransformLegacyEngineConfig(rawConfig) | |
| 162 | + | transformer.TransformLegacyEngineConfigWithExplicitAdaptive(rawConfig, m.hasExplicitAdaptiveWindowlessFrameRateSetting(), m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |
| 157 | 163 | ||
| 158 | 164 | // AllSettings includes defaults, so workspace.browsing_contexts may already be | |
| 159 | 165 | // present even when the user only configured legacy workspace.popups. Always | |
| @@ -171,6 +177,43 @@ func (m *Manager) transformLegacyConfig() { | |||
| 171 | 177 | } | |
| 172 | 178 | } | |
| 173 | 179 | ||
| 180 | + | func (m *Manager) configureAutomaticEnv() { | |
| 181 | + | if m == nil || m.viper == nil { | |
| 182 | + | return | |
| 183 | + | } | |
| 184 | + | replacer := strings.NewReplacer(".", "_") | |
| 185 | + | m.viper.SetEnvPrefix("DUMBER") | |
| 186 | + | m.viper.SetEnvKeyReplacer(replacer) | |
| 187 | + | m.viper.AutomaticEnv() | |
| 188 | + | m.automaticEnv = automaticEnvConfig{ | |
| 189 | + | enabled: true, | |
| 190 | + | prefix: "DUMBER", | |
| 191 | + | keyReplacer: replacer, | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | ||
| 195 | + | func (m *Manager) hasExplicitAdaptiveWindowlessFrameRateSetting() bool { | |
| 196 | + | if m == nil || m.viper == nil { | |
| 197 | + | return false | |
| 198 | + | } | |
| 199 | + | if m.viper.InConfig("engine.cef.adaptive_windowless_frame_rate") { | |
| 200 | + | return true | |
| 201 | + | } | |
| 202 | + | if !m.automaticEnv.enabled { | |
| 203 | + | return false | |
| 204 | + | } | |
| 205 | + | envKey := "engine.cef.adaptive_windowless_frame_rate" | |
| 206 | + | if m.automaticEnv.keyReplacer != nil { | |
| 207 | + | envKey = m.automaticEnv.keyReplacer.Replace(envKey) | |
| 208 | + | } | |
| 209 | + | envKey = strings.ToUpper(envKey) | |
| 210 | + | if prefix := strings.TrimSpace(m.automaticEnv.prefix); prefix != "" { | |
| 211 | + | envKey = prefix + "_" + envKey | |
| 212 | + | } | |
| 213 | + | value, ok := os.LookupEnv(envKey) | |
| 214 | + | return ok && value != "" | |
| 215 | + | } | |
| 216 | + | ||
| 174 | 217 | func (m *Manager) mergeLegacyPopupsIntoCanonicalBrowsingContexts(rawConfig map[string]any) { | |
| 175 | 218 | workspace, ok := rawConfig["workspace"].(map[string]any) | |
| 176 | 219 | if !ok { | |
| @@ -19,15 +19,26 @@ import ( | |||||
| 19 | type Manager struct { | 19 | type Manager struct { | ||
| 20 | config *Config | 20 | config *Config | ||
| 21 | viper *viper.Viper | 21 | viper *viper.Viper | ||
| 22 | + | automaticEnv automaticEnvConfig | |||
| 22 | mu sync.RWMutex | 23 | mu sync.RWMutex | ||
| 23 | callbacks []func(*Config) | 24 | callbacks []func(*Config) | ||
| 24 | watching bool | 25 | watching bool | ||
| 25 | skipNextReload bool // Set by Save() to prevent fsnotify reload from overwriting in-memory config | 26 | skipNextReload bool // Set by Save() to prevent fsnotify reload from overwriting in-memory config | ||
| 26 | } | 27 | } | ||
| 27 | 28 | ||||
| 29 | + | type automaticEnvConfig struct { | |||
| 30 | + | enabled bool | |||
| 31 | + | prefix string | |||
| 32 | + | keyReplacer *strings.Replacer | |||
| 33 | + | } | |||
| 34 | + | ||||
| 28 | // NewManager creates a new configuration manager. | 35 | // NewManager creates a new configuration manager. | ||
| 29 | func NewManager() (*Manager, error) { | 36 | func NewManager() (*Manager, error) { | ||
| 30 | v := viper.New() | 37 | v := viper.New() | ||
| 38 | + | m := &Manager{ | |||
| 39 | + | viper: v, | |||
| 40 | + | callbacks: make([]func(*Config), 0), | |||
| 41 | + | } | |||
| 31 | 42 | ||||
| 32 | // Configure Viper for TOML as default format | 43 | // Configure Viper for TOML as default format | ||
| 33 | v.SetConfigName("config") // Name without extension | 44 | v.SetConfigName("config") // Name without extension | ||
| @@ -42,9 +53,7 @@ func NewManager() (*Manager, error) { | |||||
| 42 | v.AddConfigPath(".") // Current directory for development | 53 | v.AddConfigPath(".") // Current directory for development | ||
| 43 | 54 | ||||
| 44 | // Set up environment variable support | 55 | // Set up environment variable support | ||
| 45 | - | v.SetEnvPrefix("DUMBER") | 56 | + | m.configureAutomaticEnv() |
| 46 | - | v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) | |||
| 47 | - | v.AutomaticEnv() | |||
| 48 | 57 | ||||
| 49 | // Note: Most environment variables are handled automatically via AutomaticEnv() | 58 | // Note: Most environment variables are handled automatically via AutomaticEnv() | ||
| 50 | // with the DUMBER_ prefix (e.g., DUMBER_DATABASE_PATH, DUMBER_LOGGING_LEVEL). | 59 | // with the DUMBER_ prefix (e.g., DUMBER_DATABASE_PATH, DUMBER_LOGGING_LEVEL). | ||
| @@ -69,10 +78,7 @@ func NewManager() (*Manager, error) { | |||||
| 69 | return nil, err | 78 | return nil, err | ||
| 70 | } | 79 | } | ||
| 71 | 80 | ||||
| 72 | - | return &Manager{ | 81 | + | return m, nil |
| 73 | - | viper: v, | |||
| 74 | - | callbacks: make([]func(*Config), 0), | |||
| 75 | - | }, nil | |||
| 76 | } | 82 | } | ||
| 77 | 83 | ||||
| 78 | // Load loads the configuration from file and environment variables. | 84 | // Load loads the configuration from file and environment variables. | ||
| @@ -153,7 +159,7 @@ func (m *Manager) transformLegacyConfig() { | |||||
| 153 | rawConfig := m.viper.AllSettings() | 159 | rawConfig := m.viper.AllSettings() | ||
| 154 | transformer := NewLegacyConfigTransformer() | 160 | transformer := NewLegacyConfigTransformer() | ||
| 155 | transformer.TransformLegacyActions(rawConfig) | 161 | transformer.TransformLegacyActions(rawConfig) | ||
| 156 | - | transformer.TransformLegacyEngineConfig(rawConfig) | 162 | + | transformer.TransformLegacyEngineConfigWithExplicitAdaptive(rawConfig, m.hasExplicitAdaptiveWindowlessFrameRateSetting(), m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) |
| 157 | 163 | ||||
| 158 | // AllSettings includes defaults, so workspace.browsing_contexts may already be | 164 | // AllSettings includes defaults, so workspace.browsing_contexts may already be | ||
| 159 | // present even when the user only configured legacy workspace.popups. Always | 165 | // present even when the user only configured legacy workspace.popups. Always | ||
| @@ -171,6 +177,43 @@ func (m *Manager) transformLegacyConfig() { | |||||
| 171 | } | 177 | } | ||
| 172 | } | 178 | } | ||
| 173 | 179 | ||||
| 180 | + | func (m *Manager) configureAutomaticEnv() { | |||
| 181 | + | if m == nil || m.viper == nil { | |||
| 182 | + | return | |||
| 183 | + | } | |||
| 184 | + | replacer := strings.NewReplacer(".", "_") | |||
| 185 | + | m.viper.SetEnvPrefix("DUMBER") | |||
| 186 | + | m.viper.SetEnvKeyReplacer(replacer) | |||
| 187 | + | m.viper.AutomaticEnv() | |||
| 188 | + | m.automaticEnv = automaticEnvConfig{ | |||
| 189 | + | enabled: true, | |||
| 190 | + | prefix: "DUMBER", | |||
| 191 | + | keyReplacer: replacer, | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | ||||
| 195 | + | func (m *Manager) hasExplicitAdaptiveWindowlessFrameRateSetting() bool { | |||
| 196 | + | if m == nil || m.viper == nil { | |||
| 197 | + | return false | |||
| 198 | + | } | |||
| 199 | + | if m.viper.InConfig("engine.cef.adaptive_windowless_frame_rate") { | |||
| 200 | + | return true | |||
| 201 | + | } | |||
| 202 | + | if !m.automaticEnv.enabled { | |||
| 203 | + | return false | |||
| 204 | + | } | |||
| 205 | + | envKey := "engine.cef.adaptive_windowless_frame_rate" | |||
| 206 | + | if m.automaticEnv.keyReplacer != nil { | |||
| 207 | + | envKey = m.automaticEnv.keyReplacer.Replace(envKey) | |||
| 208 | + | } | |||
| 209 | + | envKey = strings.ToUpper(envKey) | |||
| 210 | + | if prefix := strings.TrimSpace(m.automaticEnv.prefix); prefix != "" { | |||
| 211 | + | envKey = prefix + "_" + envKey | |||
| 212 | + | } | |||
| 213 | + | value, ok := os.LookupEnv(envKey) | |||
| 214 | + | return ok && value != "" | |||
| 215 | + | } | |||
| 216 | + | ||||
| 174 | func (m *Manager) mergeLegacyPopupsIntoCanonicalBrowsingContexts(rawConfig map[string]any) { | 217 | func (m *Manager) mergeLegacyPopupsIntoCanonicalBrowsingContexts(rawConfig map[string]any) { | ||
| 175 | workspace, ok := rawConfig["workspace"].(map[string]any) | 218 | workspace, ok := rawConfig["workspace"].(map[string]any) | ||
| 176 | if !ok { | 219 | if !ok { | ||
M internal/infrastructure/config/loader_legacy_test.go +77 -0
| @@ -1,6 +1,8 @@ | |||
| 1 | 1 | package config | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "os" | |
| 5 | + | "path/filepath" | |
| 4 | 6 | "testing" | |
| 5 | 7 | ||
| 6 | 8 | "github.com/spf13/viper" | |
| @@ -75,3 +77,78 @@ func TestCheckLegacyFormat_RuntimePrefix_ReturnsError(t *testing.T) { | |||
| 75 | 77 | require.Error(t, err) | |
| 76 | 78 | require.Contains(t, err.Error(), "dumber config migrate") | |
| 77 | 79 | } | |
| 80 | + | ||
| 81 | + | func TestTransformLegacyConfig_DefaultAdaptiveDoesNotBlockLegacyCEFMigration(t *testing.T) { | |
| 82 | + | dir := t.TempDir() | |
| 83 | + | configPath := filepath.Join(dir, "config.toml") | |
| 84 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |
| 85 | + | ||
| 86 | + | m := &Manager{viper: viper.New()} | |
| 87 | + | m.viper.SetConfigFile(configPath) | |
| 88 | + | m.viper.SetConfigType("toml") | |
| 89 | + | m.setDefaults() | |
| 90 | + | require.NoError(t, m.viper.ReadInConfig()) | |
| 91 | + | ||
| 92 | + | m.transformLegacyConfig() | |
| 93 | + | ||
| 94 | + | require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |
| 95 | + | require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |
| 96 | + | require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max")) | |
| 97 | + | } | |
| 98 | + | ||
| 99 | + | func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigration(t *testing.T) { | |
| 100 | + | dir := t.TempDir() | |
| 101 | + | configPath := filepath.Join(dir, "config.toml") | |
| 102 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |
| 103 | + | t.Setenv("DUMBER_ENGINE_CEF_ADAPTIVE_WINDOWLESS_FRAME_RATE", "false") | |
| 104 | + | ||
| 105 | + | m := &Manager{viper: viper.New()} | |
| 106 | + | m.viper.SetConfigFile(configPath) | |
| 107 | + | m.viper.SetConfigType("toml") | |
| 108 | + | m.configureAutomaticEnv() | |
| 109 | + | m.setDefaults() | |
| 110 | + | require.NoError(t, m.viper.ReadInConfig()) | |
| 111 | + | ||
| 112 | + | m.transformLegacyConfig() | |
| 113 | + | ||
| 114 | + | require.False(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |
| 115 | + | require.Equal(t, 60, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *testing.T) { | |
| 119 | + | dir := t.TempDir() | |
| 120 | + | configPath := filepath.Join(dir, "config.toml") | |
| 121 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |
| 122 | + | t.Setenv("DUMBER_ENGINE_CEF_ADAPTIVE_WINDOWLESS_FRAME_RATE", "false") | |
| 123 | + | ||
| 124 | + | m := &Manager{viper: viper.New()} | |
| 125 | + | m.viper.SetConfigFile(configPath) | |
| 126 | + | m.viper.SetConfigType("toml") | |
| 127 | + | m.setDefaults() | |
| 128 | + | require.NoError(t, m.viper.ReadInConfig()) | |
| 129 | + | ||
| 130 | + | m.transformLegacyConfig() | |
| 131 | + | ||
| 132 | + | require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |
| 133 | + | require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t *testing.T) { | |
| 137 | + | dir := t.TempDir() | |
| 138 | + | configPath := filepath.Join(dir, "config.toml") | |
| 139 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |
| 140 | + | t.Setenv("DUMBER_ENGINE_CEF_ADAPTIVE_WINDOWLESS_FRAME_RATE", "true") | |
| 141 | + | ||
| 142 | + | m := &Manager{viper: viper.New()} | |
| 143 | + | m.viper.SetConfigFile(configPath) | |
| 144 | + | m.viper.SetConfigType("toml") | |
| 145 | + | m.configureAutomaticEnv() | |
| 146 | + | m.setDefaults() | |
| 147 | + | require.NoError(t, m.viper.ReadInConfig()) | |
| 148 | + | ||
| 149 | + | m.transformLegacyConfig() | |
| 150 | + | ||
| 151 | + | require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |
| 152 | + | require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |
| 153 | + | require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max")) | |
| 154 | + | } | |
| @@ -1,6 +1,8 @@ | |||||
| 1 | package config | 1 | package config | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "os" | |||
| 5 | + | "path/filepath" | |||
| 4 | "testing" | 6 | "testing" | ||
| 5 | 7 | ||||
| 6 | "github.com/spf13/viper" | 8 | "github.com/spf13/viper" | ||
| @@ -75,3 +77,78 @@ func TestCheckLegacyFormat_RuntimePrefix_ReturnsError(t *testing.T) { | |||||
| 75 | require.Error(t, err) | 77 | require.Error(t, err) | ||
| 76 | require.Contains(t, err.Error(), "dumber config migrate") | 78 | require.Contains(t, err.Error(), "dumber config migrate") | ||
| 77 | } | 79 | } | ||
| 80 | + | ||||
| 81 | + | func TestTransformLegacyConfig_DefaultAdaptiveDoesNotBlockLegacyCEFMigration(t *testing.T) { | |||
| 82 | + | dir := t.TempDir() | |||
| 83 | + | configPath := filepath.Join(dir, "config.toml") | |||
| 84 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |||
| 85 | + | ||||
| 86 | + | m := &Manager{viper: viper.New()} | |||
| 87 | + | m.viper.SetConfigFile(configPath) | |||
| 88 | + | m.viper.SetConfigType("toml") | |||
| 89 | + | m.setDefaults() | |||
| 90 | + | require.NoError(t, m.viper.ReadInConfig()) | |||
| 91 | + | ||||
| 92 | + | m.transformLegacyConfig() | |||
| 93 | + | ||||
| 94 | + | require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |||
| 95 | + | require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |||
| 96 | + | require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max")) | |||
| 97 | + | } | |||
| 98 | + | ||||
| 99 | + | func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigration(t *testing.T) { | |||
| 100 | + | dir := t.TempDir() | |||
| 101 | + | configPath := filepath.Join(dir, "config.toml") | |||
| 102 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |||
| 103 | + | t.Setenv("DUMBER_ENGINE_CEF_ADAPTIVE_WINDOWLESS_FRAME_RATE", "false") | |||
| 104 | + | ||||
| 105 | + | m := &Manager{viper: viper.New()} | |||
| 106 | + | m.viper.SetConfigFile(configPath) | |||
| 107 | + | m.viper.SetConfigType("toml") | |||
| 108 | + | m.configureAutomaticEnv() | |||
| 109 | + | m.setDefaults() | |||
| 110 | + | require.NoError(t, m.viper.ReadInConfig()) | |||
| 111 | + | ||||
| 112 | + | m.transformLegacyConfig() | |||
| 113 | + | ||||
| 114 | + | require.False(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |||
| 115 | + | require.Equal(t, 60, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *testing.T) { | |||
| 119 | + | dir := t.TempDir() | |||
| 120 | + | configPath := filepath.Join(dir, "config.toml") | |||
| 121 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |||
| 122 | + | t.Setenv("DUMBER_ENGINE_CEF_ADAPTIVE_WINDOWLESS_FRAME_RATE", "false") | |||
| 123 | + | ||||
| 124 | + | m := &Manager{viper: viper.New()} | |||
| 125 | + | m.viper.SetConfigFile(configPath) | |||
| 126 | + | m.viper.SetConfigType("toml") | |||
| 127 | + | m.setDefaults() | |||
| 128 | + | require.NoError(t, m.viper.ReadInConfig()) | |||
| 129 | + | ||||
| 130 | + | m.transformLegacyConfig() | |||
| 131 | + | ||||
| 132 | + | require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |||
| 133 | + | require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t *testing.T) { | |||
| 137 | + | dir := t.TempDir() | |||
| 138 | + | configPath := filepath.Join(dir, "config.toml") | |||
| 139 | + | require.NoError(t, os.WriteFile(configPath, []byte("[engine.cef]\nwindowless_frame_rate = 60\n"), 0o600)) | |||
| 140 | + | t.Setenv("DUMBER_ENGINE_CEF_ADAPTIVE_WINDOWLESS_FRAME_RATE", "true") | |||
| 141 | + | ||||
| 142 | + | m := &Manager{viper: viper.New()} | |||
| 143 | + | m.viper.SetConfigFile(configPath) | |||
| 144 | + | m.viper.SetConfigType("toml") | |||
| 145 | + | m.configureAutomaticEnv() | |||
| 146 | + | m.setDefaults() | |||
| 147 | + | require.NoError(t, m.viper.ReadInConfig()) | |||
| 148 | + | ||||
| 149 | + | m.transformLegacyConfig() | |||
| 150 | + | ||||
| 151 | + | require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate")) | |||
| 152 | + | require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate")) | |||
| 153 | + | require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max")) | |||
| 154 | + | } | |||
M internal/infrastructure/config/transform_legacy.go +33 -4
| @@ -44,7 +44,16 @@ func (t *LegacyConfigTransformer) TransformLegacyActions(rawConfig map[string]an | |||
| 44 | 44 | } | |
| 45 | 45 | ||
| 46 | 46 | // TransformLegacyEngineConfig removes deprecated engine config keys. | |
| 47 | - | func (*LegacyConfigTransformer) TransformLegacyEngineConfig(rawConfig map[string]any) { | |
| 47 | + | func (t *LegacyConfigTransformer) TransformLegacyEngineConfig(rawConfig map[string]any) { | |
| 48 | + | adaptiveEnabled, hasAdaptive := nestedConfigBoolValue(rawConfig, "engine", "cef", "adaptive_windowless_frame_rate") | |
| 49 | + | t.transformLegacyEngineConfig(rawConfig, hasAdaptive, adaptiveEnabled) | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | func (t *LegacyConfigTransformer) TransformLegacyEngineConfigWithExplicitAdaptive(rawConfig map[string]any, hasAdaptive bool, adaptiveEnabled bool) { | |
| 53 | + | t.transformLegacyEngineConfig(rawConfig, hasAdaptive, adaptiveEnabled) | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | func (*LegacyConfigTransformer) transformLegacyEngineConfig(rawConfig map[string]any, hasAdaptive, adaptiveEnabled bool) { | |
| 48 | 57 | engine, ok := rawConfig["engine"].(map[string]any) | |
| 49 | 58 | if !ok { | |
| 50 | 59 | return | |
| @@ -56,11 +65,11 @@ func (*LegacyConfigTransformer) TransformLegacyEngineConfig(rawConfig map[string | |||
| 56 | 65 | } | |
| 57 | 66 | ||
| 58 | 67 | delete(cef, "enable_context_menu_handler") | |
| 59 | - | migrateLegacyCEFWindowlessFrameRateDefault(cef) | |
| 68 | + | migrateLegacyCEFWindowlessFrameRateDefault(cef, hasAdaptive, adaptiveEnabled) | |
| 60 | 69 | } | |
| 61 | 70 | ||
| 62 | - | func migrateLegacyCEFWindowlessFrameRateDefault(cef map[string]any) { | |
| 63 | - | if _, hasAdaptive := cef["adaptive_windowless_frame_rate"]; hasAdaptive { | |
| 71 | + | func migrateLegacyCEFWindowlessFrameRateDefault(cef map[string]any, hasAdaptive, adaptiveEnabled bool) { | |
| 72 | + | if hasAdaptive && !adaptiveEnabled { | |
| 64 | 73 | return | |
| 65 | 74 | } | |
| 66 | 75 | frameRate, ok := int64ConfigValue(cef["windowless_frame_rate"]) | |
| @@ -74,6 +83,26 @@ func migrateLegacyCEFWindowlessFrameRateDefault(cef map[string]any) { | |||
| 74 | 83 | } | |
| 75 | 84 | } | |
| 76 | 85 | ||
| 86 | + | func nestedConfigBoolValue(rawConfig map[string]any, path ...string) (bool, bool) { | |
| 87 | + | current := rawConfig | |
| 88 | + | for i, key := range path { | |
| 89 | + | value, ok := current[key] | |
| 90 | + | if !ok { | |
| 91 | + | return false, false | |
| 92 | + | } | |
| 93 | + | if i == len(path)-1 { | |
| 94 | + | parsed, ok := value.(bool) | |
| 95 | + | return parsed, ok | |
| 96 | + | } | |
| 97 | + | next, ok := value.(map[string]any) | |
| 98 | + | if !ok { | |
| 99 | + | return false, false | |
| 100 | + | } | |
| 101 | + | current = next | |
| 102 | + | } | |
| 103 | + | return false, false | |
| 104 | + | } | |
| 105 | + | ||
| 77 | 106 | func int64ConfigValue(value any) (int64, bool) { | |
| 78 | 107 | switch v := value.(type) { | |
| 79 | 108 | case int: | |
| @@ -44,7 +44,16 @@ func (t *LegacyConfigTransformer) TransformLegacyActions(rawConfig map[string]an | |||||
| 44 | } | 44 | } | ||
| 45 | 45 | ||||
| 46 | // TransformLegacyEngineConfig removes deprecated engine config keys. | 46 | // TransformLegacyEngineConfig removes deprecated engine config keys. | ||
| 47 | - | func (*LegacyConfigTransformer) TransformLegacyEngineConfig(rawConfig map[string]any) { | 47 | + | func (t *LegacyConfigTransformer) TransformLegacyEngineConfig(rawConfig map[string]any) { |
| 48 | + | adaptiveEnabled, hasAdaptive := nestedConfigBoolValue(rawConfig, "engine", "cef", "adaptive_windowless_frame_rate") | |||
| 49 | + | t.transformLegacyEngineConfig(rawConfig, hasAdaptive, adaptiveEnabled) | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | func (t *LegacyConfigTransformer) TransformLegacyEngineConfigWithExplicitAdaptive(rawConfig map[string]any, hasAdaptive bool, adaptiveEnabled bool) { | |||
| 53 | + | t.transformLegacyEngineConfig(rawConfig, hasAdaptive, adaptiveEnabled) | |||
| 54 | + | } | |||
| 55 | + | ||||
| 56 | + | func (*LegacyConfigTransformer) transformLegacyEngineConfig(rawConfig map[string]any, hasAdaptive, adaptiveEnabled bool) { | |||
| 48 | engine, ok := rawConfig["engine"].(map[string]any) | 57 | engine, ok := rawConfig["engine"].(map[string]any) | ||
| 49 | if !ok { | 58 | if !ok { | ||
| 50 | return | 59 | return | ||
| @@ -56,11 +65,11 @@ func (*LegacyConfigTransformer) TransformLegacyEngineConfig(rawConfig map[string | |||||
| 56 | } | 65 | } | ||
| 57 | 66 | ||||
| 58 | delete(cef, "enable_context_menu_handler") | 67 | delete(cef, "enable_context_menu_handler") | ||
| 59 | - | migrateLegacyCEFWindowlessFrameRateDefault(cef) | 68 | + | migrateLegacyCEFWindowlessFrameRateDefault(cef, hasAdaptive, adaptiveEnabled) |
| 60 | } | 69 | } | ||
| 61 | 70 | ||||
| 62 | - | func migrateLegacyCEFWindowlessFrameRateDefault(cef map[string]any) { | 71 | + | func migrateLegacyCEFWindowlessFrameRateDefault(cef map[string]any, hasAdaptive, adaptiveEnabled bool) { |
| 63 | - | if _, hasAdaptive := cef["adaptive_windowless_frame_rate"]; hasAdaptive { | 72 | + | if hasAdaptive && !adaptiveEnabled { |
| 64 | return | 73 | return | ||
| 65 | } | 74 | } | ||
| 66 | frameRate, ok := int64ConfigValue(cef["windowless_frame_rate"]) | 75 | frameRate, ok := int64ConfigValue(cef["windowless_frame_rate"]) | ||
| @@ -74,6 +83,26 @@ func migrateLegacyCEFWindowlessFrameRateDefault(cef map[string]any) { | |||||
| 74 | } | 83 | } | ||
| 75 | } | 84 | } | ||
| 76 | 85 | ||||
| 86 | + | func nestedConfigBoolValue(rawConfig map[string]any, path ...string) (bool, bool) { | |||
| 87 | + | current := rawConfig | |||
| 88 | + | for i, key := range path { | |||
| 89 | + | value, ok := current[key] | |||
| 90 | + | if !ok { | |||
| 91 | + | return false, false | |||
| 92 | + | } | |||
| 93 | + | if i == len(path)-1 { | |||
| 94 | + | parsed, ok := value.(bool) | |||
| 95 | + | return parsed, ok | |||
| 96 | + | } | |||
| 97 | + | next, ok := value.(map[string]any) | |||
| 98 | + | if !ok { | |||
| 99 | + | return false, false | |||
| 100 | + | } | |||
| 101 | + | current = next | |||
| 102 | + | } | |||
| 103 | + | return false, false | |||
| 104 | + | } | |||
| 105 | + | ||||
| 77 | func int64ConfigValue(value any) (int64, bool) { | 106 | func int64ConfigValue(value any) (int64, bool) { | ||
| 78 | switch v := value.(type) { | 107 | switch v := value.(type) { | ||
| 79 | case int: | 108 | case int: | ||
M internal/ui/app.go +10 -4
| @@ -2361,20 +2361,26 @@ func (a *App) switchBrowserWindowTabIndex(ctx context.Context, bw *browserWindow | |||
| 2361 | 2361 | logging.FromContext(ctx).Warn().Msg("switch tab index ignored: no browser window") | |
| 2362 | 2362 | return nil | |
| 2363 | 2363 | } | |
| 2364 | - | a.activateBrowserWindow(bw) | |
| 2365 | - | target := a.ensureTabTargetForBrowserWindow(bw) | |
| 2366 | 2364 | if index < 0 { | |
| 2367 | 2365 | logging.FromContext(ctx).Debug().Int("index", index).Msg("switch tab index ignored: invalid negative index") | |
| 2368 | 2366 | return nil | |
| 2369 | 2367 | } | |
| 2370 | - | if index < target.Tabs.Count() { | |
| 2368 | + | target := a.tabTargetForBrowserWindow(bw) | |
| 2369 | + | tabCount := 0 | |
| 2370 | + | if target.Tabs != nil { | |
| 2371 | + | tabCount = target.Tabs.Count() | |
| 2372 | + | } | |
| 2373 | + | if index < tabCount { | |
| 2374 | + | a.activateBrowserWindow(bw) | |
| 2371 | 2375 | return a.tabCoord.SwitchByIndex(ctx, target, index) | |
| 2372 | 2376 | } | |
| 2373 | 2377 | if a.deps == nil || a.deps.Config == nil || a.deps.Config.Workspace.NewPaneURL == "" { | |
| 2374 | 2378 | logging.FromContext(ctx).Warn().Msg("switch tab index ignored: new pane URL is not configured") | |
| 2375 | 2379 | return fmt.Errorf("newPaneURL is not configured") | |
| 2376 | 2380 | } | |
| 2377 | - | _, err := a.tabCoord.Create(ctx, target, urlutil.Normalize(a.deps.Config.Workspace.NewPaneURL)) | |
| 2381 | + | a.activateBrowserWindow(bw) | |
| 2382 | + | ensureTarget := a.ensureTabTargetForBrowserWindow(bw) | |
| 2383 | + | _, err := a.tabCoord.Create(ctx, ensureTarget, urlutil.Normalize(a.deps.Config.Workspace.NewPaneURL)) | |
| 2378 | 2384 | return err | |
| 2379 | 2385 | } | |
| 2380 | 2386 | ||
| @@ -2361,20 +2361,26 @@ func (a *App) switchBrowserWindowTabIndex(ctx context.Context, bw *browserWindow | |||||
| 2361 | logging.FromContext(ctx).Warn().Msg("switch tab index ignored: no browser window") | 2361 | logging.FromContext(ctx).Warn().Msg("switch tab index ignored: no browser window") | ||
| 2362 | return nil | 2362 | return nil | ||
| 2363 | } | 2363 | } | ||
| 2364 | - | a.activateBrowserWindow(bw) | |||
| 2365 | - | target := a.ensureTabTargetForBrowserWindow(bw) | |||
| 2366 | if index < 0 { | 2364 | if index < 0 { | ||
| 2367 | logging.FromContext(ctx).Debug().Int("index", index).Msg("switch tab index ignored: invalid negative index") | 2365 | logging.FromContext(ctx).Debug().Int("index", index).Msg("switch tab index ignored: invalid negative index") | ||
| 2368 | return nil | 2366 | return nil | ||
| 2369 | } | 2367 | } | ||
| 2370 | - | if index < target.Tabs.Count() { | 2368 | + | target := a.tabTargetForBrowserWindow(bw) |
| 2369 | + | tabCount := 0 | |||
| 2370 | + | if target.Tabs != nil { | |||
| 2371 | + | tabCount = target.Tabs.Count() | |||
| 2372 | + | } | |||
| 2373 | + | if index < tabCount { | |||
| 2374 | + | a.activateBrowserWindow(bw) | |||
| 2371 | return a.tabCoord.SwitchByIndex(ctx, target, index) | 2375 | return a.tabCoord.SwitchByIndex(ctx, target, index) | ||
| 2372 | } | 2376 | } | ||
| 2373 | if a.deps == nil || a.deps.Config == nil || a.deps.Config.Workspace.NewPaneURL == "" { | 2377 | if a.deps == nil || a.deps.Config == nil || a.deps.Config.Workspace.NewPaneURL == "" { | ||
| 2374 | logging.FromContext(ctx).Warn().Msg("switch tab index ignored: new pane URL is not configured") | 2378 | logging.FromContext(ctx).Warn().Msg("switch tab index ignored: new pane URL is not configured") | ||
| 2375 | return fmt.Errorf("newPaneURL is not configured") | 2379 | return fmt.Errorf("newPaneURL is not configured") | ||
| 2376 | } | 2380 | } | ||
| 2377 | - | _, err := a.tabCoord.Create(ctx, target, urlutil.Normalize(a.deps.Config.Workspace.NewPaneURL)) | 2381 | + | a.activateBrowserWindow(bw) |
| 2382 | + | ensureTarget := a.ensureTabTargetForBrowserWindow(bw) | |||
| 2383 | + | _, err := a.tabCoord.Create(ctx, ensureTarget, urlutil.Normalize(a.deps.Config.Workspace.NewPaneURL)) | |||
| 2378 | return err | 2384 | return err | ||
| 2379 | } | 2385 | } | ||
| 2380 | 2386 | ||||
M internal/ui/app_browser_launch_test.go +57 -5
| @@ -2357,9 +2357,9 @@ func TestApp_SwitchBrowserWindowTabIndexNegativeIndexIsIgnored(t *testing.T) { | |||
| 2357 | 2357 | deps: &Dependencies{ | |
| 2358 | 2358 | Config: &config.Config{Workspace: config.WorkspaceConfig{NewPaneURL: "about:blank"}}, | |
| 2359 | 2359 | }, | |
| 2360 | - | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 2360 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 2361 | 2361 | browserWindows: map[string]*browserWindow{second.id: second}, | |
| 2362 | - | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, | |
| 2362 | + | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, | |
| 2363 | 2363 | } | |
| 2364 | 2364 | app.initTabCoordinator(ctx) | |
| 2365 | 2365 | ||
| @@ -2375,6 +2375,33 @@ func TestApp_SwitchBrowserWindowTabIndexNegativeIndexIsIgnored(t *testing.T) { | |||
| 2375 | 2375 | } | |
| 2376 | 2376 | } | |
| 2377 | 2377 | ||
| 2378 | + | func TestApp_DispatchBrowserWindowActionSwitchTabIndexNegativeDoesNotMutateWindowState(t *testing.T) { | |
| 2379 | + | ctx := context.Background() | |
| 2380 | + | ||
| 2381 | + | second := &browserWindow{id: "window-2"} | |
| 2382 | + | ||
| 2383 | + | app := &App{ | |
| 2384 | + | deps: &Dependencies{ | |
| 2385 | + | Config: &config.Config{Workspace: config.WorkspaceConfig{NewPaneURL: "about:blank"}}, | |
| 2386 | + | }, | |
| 2387 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 2388 | + | browserWindows: map[string]*browserWindow{second.id: second}, | |
| 2389 | + | lastFocusedWindowID: "window-1", | |
| 2390 | + | } | |
| 2391 | + | app.initTabCoordinator(ctx) | |
| 2392 | + | ||
| 2393 | + | err := app.switchBrowserWindowTabIndex(ctx, second, -1) | |
| 2394 | + | if err != nil { | |
| 2395 | + | t.Fatalf("switchBrowserWindowTabIndex returned error: %v", err) | |
| 2396 | + | } | |
| 2397 | + | if got := app.lastFocusedWindowID; got != "window-1" { | |
| 2398 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", got, "window-1") | |
| 2399 | + | } | |
| 2400 | + | if second.tabs != nil { | |
| 2401 | + | t.Fatalf("second tabs should stay nil for invalid index") | |
| 2402 | + | } | |
| 2403 | + | } | |
| 2404 | + | ||
| 2378 | 2405 | func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab(t *testing.T) { | |
| 2379 | 2406 | ctx := context.Background() | |
| 2380 | 2407 | ||
| @@ -2385,10 +2412,10 @@ func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab | |||
| 2385 | 2412 | second := &browserWindow{id: "window-2", tabs: secondTabs} | |
| 2386 | 2413 | ||
| 2387 | 2414 | app := &App{ | |
| 2388 | - | deps: &Dependencies{Config: &config.Config{}}, | |
| 2389 | - | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 2415 | + | deps: &Dependencies{Config: &config.Config{}}, | |
| 2416 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 2390 | 2417 | browserWindows: map[string]*browserWindow{second.id: second}, | |
| 2391 | - | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, | |
| 2418 | + | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, | |
| 2392 | 2419 | } | |
| 2393 | 2420 | app.initTabCoordinator(ctx) | |
| 2394 | 2421 | ||
| @@ -2404,6 +2431,31 @@ func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab | |||
| 2404 | 2431 | } | |
| 2405 | 2432 | } | |
| 2406 | 2433 | ||
| 2434 | + | func TestApp_SwitchBrowserWindowTabIndexMissingURLDoesNotMutateWindowState(t *testing.T) { | |
| 2435 | + | ctx := context.Background() | |
| 2436 | + | ||
| 2437 | + | second := &browserWindow{id: "window-2"} | |
| 2438 | + | ||
| 2439 | + | app := &App{ | |
| 2440 | + | deps: &Dependencies{Config: &config.Config{}}, | |
| 2441 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 2442 | + | browserWindows: map[string]*browserWindow{second.id: second}, | |
| 2443 | + | lastFocusedWindowID: "window-1", | |
| 2444 | + | } | |
| 2445 | + | app.initTabCoordinator(ctx) | |
| 2446 | + | ||
| 2447 | + | err := app.switchBrowserWindowTabIndex(ctx, second, 3) | |
| 2448 | + | if err == nil { | |
| 2449 | + | t.Fatal("switchBrowserWindowTabIndex returned nil error, want config error") | |
| 2450 | + | } | |
| 2451 | + | if got := app.lastFocusedWindowID; got != "window-1" { | |
| 2452 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", got, "window-1") | |
| 2453 | + | } | |
| 2454 | + | if second.tabs != nil { | |
| 2455 | + | t.Fatalf("second tabs should stay nil when new pane URL is missing") | |
| 2456 | + | } | |
| 2457 | + | } | |
| 2458 | + | ||
| 2407 | 2459 | func TestApp_WorkspaceOmniboxNavigateUsesOwnerWindow(t *testing.T) { | |
| 2408 | 2460 | gtkApp := requireGTKDisplayApp(t) | |
| 2409 | 2461 | defer gtkApp.Unref() | |
| @@ -2357,9 +2357,9 @@ func TestApp_SwitchBrowserWindowTabIndexNegativeIndexIsIgnored(t *testing.T) { | |||||
| 2357 | deps: &Dependencies{ | 2357 | deps: &Dependencies{ | ||
| 2358 | Config: &config.Config{Workspace: config.WorkspaceConfig{NewPaneURL: "about:blank"}}, | 2358 | Config: &config.Config{Workspace: config.WorkspaceConfig{NewPaneURL: "about:blank"}}, | ||
| 2359 | }, | 2359 | }, | ||
| 2360 | - | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | 2360 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), |
| 2361 | browserWindows: map[string]*browserWindow{second.id: second}, | 2361 | browserWindows: map[string]*browserWindow{second.id: second}, | ||
| 2362 | - | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, | 2362 | + | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, |
| 2363 | } | 2363 | } | ||
| 2364 | app.initTabCoordinator(ctx) | 2364 | app.initTabCoordinator(ctx) | ||
| 2365 | 2365 | ||||
| @@ -2375,6 +2375,33 @@ func TestApp_SwitchBrowserWindowTabIndexNegativeIndexIsIgnored(t *testing.T) { | |||||
| 2375 | } | 2375 | } | ||
| 2376 | } | 2376 | } | ||
| 2377 | 2377 | ||||
| 2378 | + | func TestApp_DispatchBrowserWindowActionSwitchTabIndexNegativeDoesNotMutateWindowState(t *testing.T) { | |||
| 2379 | + | ctx := context.Background() | |||
| 2380 | + | ||||
| 2381 | + | second := &browserWindow{id: "window-2"} | |||
| 2382 | + | ||||
| 2383 | + | app := &App{ | |||
| 2384 | + | deps: &Dependencies{ | |||
| 2385 | + | Config: &config.Config{Workspace: config.WorkspaceConfig{NewPaneURL: "about:blank"}}, | |||
| 2386 | + | }, | |||
| 2387 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |||
| 2388 | + | browserWindows: map[string]*browserWindow{second.id: second}, | |||
| 2389 | + | lastFocusedWindowID: "window-1", | |||
| 2390 | + | } | |||
| 2391 | + | app.initTabCoordinator(ctx) | |||
| 2392 | + | ||||
| 2393 | + | err := app.switchBrowserWindowTabIndex(ctx, second, -1) | |||
| 2394 | + | if err != nil { | |||
| 2395 | + | t.Fatalf("switchBrowserWindowTabIndex returned error: %v", err) | |||
| 2396 | + | } | |||
| 2397 | + | if got := app.lastFocusedWindowID; got != "window-1" { | |||
| 2398 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", got, "window-1") | |||
| 2399 | + | } | |||
| 2400 | + | if second.tabs != nil { | |||
| 2401 | + | t.Fatalf("second tabs should stay nil for invalid index") | |||
| 2402 | + | } | |||
| 2403 | + | } | |||
| 2404 | + | ||||
| 2378 | func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab(t *testing.T) { | 2405 | func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab(t *testing.T) { | ||
| 2379 | ctx := context.Background() | 2406 | ctx := context.Background() | ||
| 2380 | 2407 | ||||
| @@ -2385,10 +2412,10 @@ func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab | |||||
| 2385 | second := &browserWindow{id: "window-2", tabs: secondTabs} | 2412 | second := &browserWindow{id: "window-2", tabs: secondTabs} | ||
| 2386 | 2413 | ||||
| 2387 | app := &App{ | 2414 | app := &App{ | ||
| 2388 | - | deps: &Dependencies{Config: &config.Config{}}, | 2415 | + | deps: &Dependencies{Config: &config.Config{}}, |
| 2389 | - | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | 2416 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), |
| 2390 | browserWindows: map[string]*browserWindow{second.id: second}, | 2417 | browserWindows: map[string]*browserWindow{second.id: second}, | ||
| 2391 | - | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, | 2418 | + | windowForTab: map[entity.TabID]*browserWindow{secondTab.ID: second}, |
| 2392 | } | 2419 | } | ||
| 2393 | app.initTabCoordinator(ctx) | 2420 | app.initTabCoordinator(ctx) | ||
| 2394 | 2421 | ||||
| @@ -2404,6 +2431,31 @@ func TestApp_DispatchBrowserWindowActionSwitchTabIndexMissingURLDoesNotCreateTab | |||||
| 2404 | } | 2431 | } | ||
| 2405 | } | 2432 | } | ||
| 2406 | 2433 | ||||
| 2434 | + | func TestApp_SwitchBrowserWindowTabIndexMissingURLDoesNotMutateWindowState(t *testing.T) { | |||
| 2435 | + | ctx := context.Background() | |||
| 2436 | + | ||||
| 2437 | + | second := &browserWindow{id: "window-2"} | |||
| 2438 | + | ||||
| 2439 | + | app := &App{ | |||
| 2440 | + | deps: &Dependencies{Config: &config.Config{}}, | |||
| 2441 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |||
| 2442 | + | browserWindows: map[string]*browserWindow{second.id: second}, | |||
| 2443 | + | lastFocusedWindowID: "window-1", | |||
| 2444 | + | } | |||
| 2445 | + | app.initTabCoordinator(ctx) | |||
| 2446 | + | ||||
| 2447 | + | err := app.switchBrowserWindowTabIndex(ctx, second, 3) | |||
| 2448 | + | if err == nil { | |||
| 2449 | + | t.Fatal("switchBrowserWindowTabIndex returned nil error, want config error") | |||
| 2450 | + | } | |||
| 2451 | + | if got := app.lastFocusedWindowID; got != "window-1" { | |||
| 2452 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", got, "window-1") | |||
| 2453 | + | } | |||
| 2454 | + | if second.tabs != nil { | |||
| 2455 | + | t.Fatalf("second tabs should stay nil when new pane URL is missing") | |||
| 2456 | + | } | |||
| 2457 | + | } | |||
| 2458 | + | ||||
| 2407 | func TestApp_WorkspaceOmniboxNavigateUsesOwnerWindow(t *testing.T) { | 2459 | func TestApp_WorkspaceOmniboxNavigateUsesOwnerWindow(t *testing.T) { | ||
| 2408 | gtkApp := requireGTKDisplayApp(t) | 2460 | gtkApp := requireGTKDisplayApp(t) | ||
| 2409 | defer gtkApp.Unref() | 2461 | defer gtkApp.Unref() | ||