feat: implement phase2 tier1 lifecycle close robustness and lighter defaults
- commit
- c809edbab316d88378e9e2e66cf80215b4cc8025
- author
- brice · · 2026-02-07 05:11 UTC
- parent
- 4bb2583
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +632 -150
M docs/config/index.md +15 -6
| @@ -432,12 +432,21 @@ Dumber automatically detects your GPU vendor (AMD/Intel/NVIDIA) and sets optimal | |||
| 432 | 432 | [media] | |
| 433 | 433 | hardware_decoding = "auto" # HW preferred, SW fallback | |
| 434 | 434 | prefer_av1 = false # Let site choose codec | |
| 435 | - | show_diagnostics = false # Enable for debugging | |
| 435 | + | show_diagnostics = false # Keep off for daily use | |
| 436 | 436 | force_vsync = false # Let compositor handle VSync | |
| 437 | 437 | gl_rendering_mode = "auto" # GStreamer picks best GL API | |
| 438 | 438 | gstreamer_debug_level = 0 # Increase to 3-5 for debugging | |
| 439 | 439 | ``` | |
| 440 | 440 | ||
| 441 | + | **Diagnostic mode toggle path:** | |
| 442 | + | ```toml | |
| 443 | + | # ~/.config/dumber/config.toml | |
| 444 | + | [media] | |
| 445 | + | show_diagnostics = true | |
| 446 | + | gstreamer_debug_level = 3 | |
| 447 | + | ``` | |
| 448 | + | Set `show_diagnostics = false` and `gstreamer_debug_level = 0` to return to normal mode. | |
| 449 | + | ||
| 441 | 450 | **Diagnostics CLI:** | |
| 442 | 451 | ```bash | |
| 443 | 452 | # Check GStreamer plugins and VA-API status | |
| @@ -523,13 +532,13 @@ Performance profiles provide preset configurations for WebKitGTK tuning. These s | |||
| 523 | 532 | ||
| 524 | 533 | | Key | Type | Default | Valid Values | Description | | |
| 525 | 534 | |-----|------|---------|--------------|-------------| | |
| 526 | - | | `performance.profile` | string | `"default"` | `default`, `lite`, `max`, `custom` | Performance profile selection | | |
| 535 | + | | `performance.profile` | string | `"lite"` | `default`, `lite`, `max`, `custom` | Performance profile selection | | |
| 527 | 536 | ||
| 528 | 537 | ### Profiles | |
| 529 | 538 | ||
| 530 | 539 | | Profile | Description | Use Case | | |
| 531 | 540 | |---------|-------------|----------| | |
| 532 | - | | `default` | No tuning, uses WebKit defaults | Normal browsing, most users | | |
| 541 | + | | `default` | No tuning, uses WebKit defaults | Compatibility baseline, troubleshooting | | |
| 533 | 542 | | `lite` | Reduced resource usage | Low-RAM systems (< 4GB), battery saving | | |
| 534 | 543 | | `max` | Maximum responsiveness | Heavy pages (GitHub PRs, complex SPAs), high-end systems | | |
| 535 | 544 | | `custom` | Manual control over all settings | Advanced users who want fine-grained control | | |
| @@ -549,10 +558,10 @@ Performance profiles provide preset configurations for WebKitGTK tuning. These s | |||
| 549 | 558 | **Example:** | |
| 550 | 559 | ```toml | |
| 551 | 560 | [performance] | |
| 552 | - | profile = "default" # Most users - no tuning needed | |
| 561 | + | profile = "lite" # Daily-use default | |
| 553 | 562 | ||
| 554 | - | # For low-RAM systems: | |
| 555 | - | # profile = "lite" | |
| 563 | + | # For WebKit baseline behavior: | |
| 564 | + | # profile = "default" | |
| 556 | 565 | ||
| 557 | 566 | # For heavy pages (GitHub PRs, complex web apps): | |
| 558 | 567 | # profile = "max" | |
| @@ -432,12 +432,21 @@ Dumber automatically detects your GPU vendor (AMD/Intel/NVIDIA) and sets optimal | |||||
| 432 | [media] | 432 | [media] | ||
| 433 | hardware_decoding = "auto" # HW preferred, SW fallback | 433 | hardware_decoding = "auto" # HW preferred, SW fallback | ||
| 434 | prefer_av1 = false # Let site choose codec | 434 | prefer_av1 = false # Let site choose codec | ||
| 435 | - | show_diagnostics = false # Enable for debugging | 435 | + | show_diagnostics = false # Keep off for daily use |
| 436 | force_vsync = false # Let compositor handle VSync | 436 | force_vsync = false # Let compositor handle VSync | ||
| 437 | gl_rendering_mode = "auto" # GStreamer picks best GL API | 437 | gl_rendering_mode = "auto" # GStreamer picks best GL API | ||
| 438 | gstreamer_debug_level = 0 # Increase to 3-5 for debugging | 438 | gstreamer_debug_level = 0 # Increase to 3-5 for debugging | ||
| 439 | ``` | 439 | ``` | ||
| 440 | 440 | ||||
| 441 | + | **Diagnostic mode toggle path:** | |||
| 442 | + | ```toml | |||
| 443 | + | # ~/.config/dumber/config.toml | |||
| 444 | + | [media] | |||
| 445 | + | show_diagnostics = true | |||
| 446 | + | gstreamer_debug_level = 3 | |||
| 447 | + | ``` | |||
| 448 | + | Set `show_diagnostics = false` and `gstreamer_debug_level = 0` to return to normal mode. | |||
| 449 | + | ||||
| 441 | **Diagnostics CLI:** | 450 | **Diagnostics CLI:** | ||
| 442 | ```bash | 451 | ```bash | ||
| 443 | # Check GStreamer plugins and VA-API status | 452 | # Check GStreamer plugins and VA-API status | ||
| @@ -523,13 +532,13 @@ Performance profiles provide preset configurations for WebKitGTK tuning. These s | |||||
| 523 | 532 | ||||
| 524 | | Key | Type | Default | Valid Values | Description | | 533 | | Key | Type | Default | Valid Values | Description | | ||
| 525 | |-----|------|---------|--------------|-------------| | 534 | |-----|------|---------|--------------|-------------| | ||
| 526 | - | | `performance.profile` | string | `"default"` | `default`, `lite`, `max`, `custom` | Performance profile selection | | 535 | + | | `performance.profile` | string | `"lite"` | `default`, `lite`, `max`, `custom` | Performance profile selection | |
| 527 | 536 | ||||
| 528 | ### Profiles | 537 | ### Profiles | ||
| 529 | 538 | ||||
| 530 | | Profile | Description | Use Case | | 539 | | Profile | Description | Use Case | | ||
| 531 | |---------|-------------|----------| | 540 | |---------|-------------|----------| | ||
| 532 | - | | `default` | No tuning, uses WebKit defaults | Normal browsing, most users | | 541 | + | | `default` | No tuning, uses WebKit defaults | Compatibility baseline, troubleshooting | |
| 533 | | `lite` | Reduced resource usage | Low-RAM systems (< 4GB), battery saving | | 542 | | `lite` | Reduced resource usage | Low-RAM systems (< 4GB), battery saving | | ||
| 534 | | `max` | Maximum responsiveness | Heavy pages (GitHub PRs, complex SPAs), high-end systems | | 543 | | `max` | Maximum responsiveness | Heavy pages (GitHub PRs, complex SPAs), high-end systems | | ||
| 535 | | `custom` | Manual control over all settings | Advanced users who want fine-grained control | | 544 | | `custom` | Manual control over all settings | Advanced users who want fine-grained control | | ||
| @@ -549,10 +558,10 @@ Performance profiles provide preset configurations for WebKitGTK tuning. These s | |||||
| 549 | **Example:** | 558 | **Example:** | ||
| 550 | ```toml | 559 | ```toml | ||
| 551 | [performance] | 560 | [performance] | ||
| 552 | - | profile = "default" # Most users - no tuning needed | 561 | + | profile = "lite" # Daily-use default |
| 553 | 562 | ||||
| 554 | - | # For low-RAM systems: | 563 | + | # For WebKit baseline behavior: |
| 555 | - | # profile = "lite" | 564 | + | # profile = "default" |
| 556 | 565 | ||||
| 557 | # For heavy pages (GitHub PRs, complex web apps): | 566 | # For heavy pages (GitHub PRs, complex web apps): | ||
| 558 | # profile = "max" | 567 | # profile = "max" | ||
M docs/reference/configuration.md +1 -1
| @@ -104,7 +104,7 @@ | |||
| 104 | 104 | | `update.enable_on_startup` | bool | `true` | | | |
| 105 | 105 | | `update.auto_download` | bool | `false` | | | |
| 106 | 106 | | `update.notify_on_new_settings` | bool | `true` | | | |
| 107 | - | | `performance.profile` | string | `default` | `default`, `lite`, `max`, `custom` | | |
| 107 | + | | `performance.profile` | string | `lite` | `default`, `lite`, `max`, `custom` | | |
| 108 | 108 | | `performance.skia_cpu_painting_threads` | int | `0` | | | |
| 109 | 109 | | `performance.skia_gpu_painting_threads` | int | `-1` | | | |
| 110 | 110 | | `performance.skia_enable_cpu_rendering` | bool | `false` | | | |
| @@ -104,7 +104,7 @@ | |||||
| 104 | | `update.enable_on_startup` | bool | `true` | | | 104 | | `update.enable_on_startup` | bool | `true` | | | ||
| 105 | | `update.auto_download` | bool | `false` | | | 105 | | `update.auto_download` | bool | `false` | | | ||
| 106 | | `update.notify_on_new_settings` | bool | `true` | | | 106 | | `update.notify_on_new_settings` | bool | `true` | | | ||
| 107 | - | | `performance.profile` | string | `default` | `default`, `lite`, `max`, `custom` | | 107 | + | | `performance.profile` | string | `lite` | `default`, `lite`, `max`, `custom` | |
| 108 | | `performance.skia_cpu_painting_threads` | int | `0` | | | 108 | | `performance.skia_cpu_painting_threads` | int | `0` | | | ||
| 109 | | `performance.skia_gpu_painting_threads` | int | `-1` | | | 109 | | `performance.skia_gpu_painting_threads` | int | `-1` | | | ||
| 110 | | `performance.skia_enable_cpu_rendering` | bool | `false` | | | 110 | | `performance.skia_enable_cpu_rendering` | bool | `false` | | | ||
M docs/reference/troubleshooting.md +15 -2
| @@ -29,16 +29,29 @@ dumber doctor --media # GStreamer/VA-API only | |||
| 29 | 29 | ||
| 30 | 30 | **Solution:** | |
| 31 | 31 | 1. Run `dumber doctor --media` | |
| 32 | - | 2. Check hardware decoding: | |
| 32 | + | 2. Enable diagnostic mode for startup media warnings: | |
| 33 | + | ```toml | |
| 34 | + | # ~/.config/dumber/config.toml | |
| 35 | + | [media] | |
| 36 | + | show_diagnostics = true | |
| 37 | + | gstreamer_debug_level = 3 | |
| 38 | + | ``` | |
| 39 | + | 3. Check hardware decoding: | |
| 33 | 40 | ```toml | |
| 34 | 41 | # ~/.config/dumber/config.toml | |
| 35 | 42 | [media] | |
| 36 | 43 | hardware_decoding = "auto" # Try "disable" if issues persist | |
| 37 | 44 | ``` | |
| 38 | - | 3. Install VA-API drivers: | |
| 45 | + | 4. Install VA-API drivers: | |
| 39 | 46 | - AMD: `libva-mesa-driver` | |
| 40 | 47 | - Intel: `intel-media-driver` | |
| 41 | 48 | - NVIDIA: `libva-nvidia-driver` | |
| 49 | + | 5. After diagnosing, disable diagnostic mode for quieter daily runs: | |
| 50 | + | ```toml | |
| 51 | + | [media] | |
| 52 | + | show_diagnostics = false | |
| 53 | + | gstreamer_debug_level = 0 | |
| 54 | + | ``` | |
| 42 | 55 | ||
| 43 | 56 | ### Screen flickering on Wayland | |
| 44 | 57 | ||
| @@ -29,16 +29,29 @@ dumber doctor --media # GStreamer/VA-API only | |||||
| 29 | 29 | ||||
| 30 | **Solution:** | 30 | **Solution:** | ||
| 31 | 1. Run `dumber doctor --media` | 31 | 1. Run `dumber doctor --media` | ||
| 32 | - | 2. Check hardware decoding: | 32 | + | 2. Enable diagnostic mode for startup media warnings: |
| 33 | + | ```toml | |||
| 34 | + | # ~/.config/dumber/config.toml | |||
| 35 | + | [media] | |||
| 36 | + | show_diagnostics = true | |||
| 37 | + | gstreamer_debug_level = 3 | |||
| 38 | + | ``` | |||
| 39 | + | 3. Check hardware decoding: | |||
| 33 | ```toml | 40 | ```toml | ||
| 34 | # ~/.config/dumber/config.toml | 41 | # ~/.config/dumber/config.toml | ||
| 35 | [media] | 42 | [media] | ||
| 36 | hardware_decoding = "auto" # Try "disable" if issues persist | 43 | hardware_decoding = "auto" # Try "disable" if issues persist | ||
| 37 | ``` | 44 | ``` | ||
| 38 | - | 3. Install VA-API drivers: | 45 | + | 4. Install VA-API drivers: |
| 39 | - AMD: `libva-mesa-driver` | 46 | - AMD: `libva-mesa-driver` | ||
| 40 | - Intel: `intel-media-driver` | 47 | - Intel: `intel-media-driver` | ||
| 41 | - NVIDIA: `libva-nvidia-driver` | 48 | - NVIDIA: `libva-nvidia-driver` | ||
| 49 | + | 5. After diagnosing, disable diagnostic mode for quieter daily runs: | |||
| 50 | + | ```toml | |||
| 51 | + | [media] | |||
| 52 | + | show_diagnostics = false | |||
| 53 | + | gstreamer_debug_level = 0 | |||
| 54 | + | ``` | |||
| 42 | 55 | ||||
| 43 | ### Screen flickering on Wayland | 56 | ### Screen flickering on Wayland | ||
| 44 | 57 | ||||
M internal/infrastructure/config/defaults.go +1 -1
| @@ -293,7 +293,7 @@ func DefaultConfig() *Config { | |||
| 293 | 293 | Prefix: "", | |
| 294 | 294 | }, | |
| 295 | 295 | Performance: PerformanceConfig{ | |
| 296 | - | Profile: ProfileDefault, // Use WebKit defaults, no tuning | |
| 296 | + | Profile: ProfileLite, // Daily-use default: lighter profile | |
| 297 | 297 | ZoomCacheSize: defaultZoomCacheSize, | |
| 298 | 298 | WebViewPoolPrewarmCount: defaultWebViewPoolPrewarmCount, | |
| 299 | 299 | // Skia threading - balanced defaults (unset = use WebKit defaults) | |
| @@ -293,7 +293,7 @@ func DefaultConfig() *Config { | |||||
| 293 | Prefix: "", | 293 | Prefix: "", | ||
| 294 | }, | 294 | }, | ||
| 295 | Performance: PerformanceConfig{ | 295 | Performance: PerformanceConfig{ | ||
| 296 | - | Profile: ProfileDefault, // Use WebKit defaults, no tuning | 296 | + | Profile: ProfileLite, // Daily-use default: lighter profile |
| 297 | ZoomCacheSize: defaultZoomCacheSize, | 297 | ZoomCacheSize: defaultZoomCacheSize, | ||
| 298 | WebViewPoolPrewarmCount: defaultWebViewPoolPrewarmCount, | 298 | WebViewPoolPrewarmCount: defaultWebViewPoolPrewarmCount, | ||
| 299 | // Skia threading - balanced defaults (unset = use WebKit defaults) | 299 | // Skia threading - balanced defaults (unset = use WebKit defaults) | ||
M internal/infrastructure/config/defaults_test.go +2 -0
| @@ -10,6 +10,8 @@ func TestDefaultConfig_RuntimeLoggingProfile(t *testing.T) { | |||
| 10 | 10 | cfg := DefaultConfig() | |
| 11 | 11 | assert.Equal(t, "info", cfg.Logging.Level) | |
| 12 | 12 | assert.False(t, cfg.Logging.CaptureGTKLogs) | |
| 13 | + | assert.Equal(t, ProfileLite, cfg.Performance.Profile) | |
| 13 | 14 | assert.Equal(t, CookiePolicyNoThirdParty, cfg.Privacy.CookiePolicy) | |
| 14 | 15 | assert.True(t, cfg.Privacy.ITPEnabled) | |
| 16 | + | assert.False(t, cfg.Media.ShowDiagnosticsOnStartup) | |
| 15 | 17 | } | |
| @@ -10,6 +10,8 @@ func TestDefaultConfig_RuntimeLoggingProfile(t *testing.T) { | |||||
| 10 | cfg := DefaultConfig() | 10 | cfg := DefaultConfig() | ||
| 11 | assert.Equal(t, "info", cfg.Logging.Level) | 11 | assert.Equal(t, "info", cfg.Logging.Level) | ||
| 12 | assert.False(t, cfg.Logging.CaptureGTKLogs) | 12 | assert.False(t, cfg.Logging.CaptureGTKLogs) | ||
| 13 | + | assert.Equal(t, ProfileLite, cfg.Performance.Profile) | |||
| 13 | assert.Equal(t, CookiePolicyNoThirdParty, cfg.Privacy.CookiePolicy) | 14 | assert.Equal(t, CookiePolicyNoThirdParty, cfg.Privacy.CookiePolicy) | ||
| 14 | assert.True(t, cfg.Privacy.ITPEnabled) | 15 | assert.True(t, cfg.Privacy.ITPEnabled) | ||
| 16 | + | assert.False(t, cfg.Media.ShowDiagnosticsOnStartup) | |||
| 15 | } | 17 | } | ||
M internal/infrastructure/webkit/pool.go +156 -27
| @@ -3,6 +3,8 @@ package webkit | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | + | "os" | |
| 7 | + | "strings" | |
| 6 | 8 | "sync" | |
| 7 | 9 | "sync/atomic" | |
| 8 | 10 | "time" | |
| @@ -31,18 +33,32 @@ type PoolConfig struct { | |||
| 31 | 33 | PrewarmCount int | |
| 32 | 34 | // IdleTimeout is how long idle views stay in the pool before being destroyed. | |
| 33 | 35 | IdleTimeout time.Duration | |
| 36 | + | // ReusePolicy controls if/when released WebViews are returned to pool. | |
| 37 | + | // Values: off, safe, aggressive. | |
| 38 | + | ReusePolicy string | |
| 39 | + | // TerminatePolicy controls whether Destroy() should terminate web process. | |
| 40 | + | // Values: auto, always, never. | |
| 41 | + | TerminatePolicy string | |
| 34 | 42 | } | |
| 35 | 43 | ||
| 36 | 44 | // DefaultPoolConfig returns sensible defaults for the pool. | |
| 37 | 45 | func DefaultPoolConfig() PoolConfig { | |
| 38 | 46 | return PoolConfig{ | |
| 39 | - | MinSize: 2, | |
| 40 | - | MaxSize: 8, | |
| 41 | - | PrewarmCount: 4, // Pre-create 4 WebViews for faster initial tab creation | |
| 42 | - | IdleTimeout: 5 * time.Minute, | |
| 47 | + | MinSize: 2, | |
| 48 | + | MaxSize: 8, | |
| 49 | + | PrewarmCount: 4, // Pre-create 4 WebViews for faster initial tab creation | |
| 50 | + | IdleTimeout: 5 * time.Minute, | |
| 51 | + | ReusePolicy: reusePolicyOff, | |
| 52 | + | TerminatePolicy: terminatePolicyAuto, | |
| 43 | 53 | } | |
| 44 | 54 | } | |
| 45 | 55 | ||
| 56 | + | const ( | |
| 57 | + | reusePolicyOff = "off" | |
| 58 | + | reusePolicySafe = "safe" | |
| 59 | + | reusePolicyAggressive = "aggressive" | |
| 60 | + | ) | |
| 61 | + | ||
| 46 | 62 | // WebViewPool manages a pool of pre-created WebViews for fast tab creation. | |
| 47 | 63 | type WebViewPool struct { | |
| 48 | 64 | wkCtx *WebKitContext | |
| @@ -59,6 +75,19 @@ type WebViewPool struct { | |||
| 59 | 75 | ||
| 60 | 76 | closed atomic.Bool | |
| 61 | 77 | wg sync.WaitGroup | |
| 78 | + | ||
| 79 | + | reusePolicy string | |
| 80 | + | terminatePolicy string | |
| 81 | + | ||
| 82 | + | acquireCount atomic.Uint64 | |
| 83 | + | acquirePoolHits atomic.Uint64 | |
| 84 | + | acquirePoolMisses atomic.Uint64 | |
| 85 | + | releaseCount atomic.Uint64 | |
| 86 | + | releaseReused atomic.Uint64 | |
| 87 | + | releaseDestroyed atomic.Uint64 | |
| 88 | + | releaseDropped atomic.Uint64 | |
| 89 | + | releaseTotalNanos atomic.Int64 | |
| 90 | + | acquireTotalNanos atomic.Int64 | |
| 62 | 91 | } | |
| 63 | 92 | ||
| 64 | 93 | // NewWebViewPool creates a new WebView pool. | |
| @@ -79,21 +108,28 @@ func NewWebViewPool( | |||
| 79 | 108 | if cfg.MinSize > cfg.MaxSize { | |
| 80 | 109 | cfg.MinSize = cfg.MaxSize | |
| 81 | 110 | } | |
| 111 | + | cfg.ReusePolicy = resolveReusePolicy(cfg.ReusePolicy) | |
| 112 | + | cfg.TerminatePolicy = resolveTerminatePolicy(cfg.TerminatePolicy) | |
| 82 | 113 | if ctx == nil { | |
| 83 | 114 | ctx = context.Background() | |
| 84 | 115 | } | |
| 85 | 116 | ||
| 86 | 117 | p := &WebViewPool{ | |
| 87 | - | wkCtx: wkCtx, | |
| 88 | - | settings: settings, | |
| 89 | - | config: cfg, | |
| 90 | - | pool: make(chan *WebView, cfg.MaxSize), | |
| 91 | - | injector: injector, | |
| 92 | - | router: router, | |
| 118 | + | wkCtx: wkCtx, | |
| 119 | + | settings: settings, | |
| 120 | + | config: cfg, | |
| 121 | + | pool: make(chan *WebView, cfg.MaxSize), | |
| 122 | + | injector: injector, | |
| 123 | + | router: router, | |
| 124 | + | reusePolicy: cfg.ReusePolicy, | |
| 125 | + | terminatePolicy: cfg.TerminatePolicy, | |
| 93 | 126 | } | |
| 94 | 127 | ||
| 95 | 128 | log := logging.FromContext(ctx) | |
| 96 | - | log.Debug().Msg("creating webview pool") | |
| 129 | + | log.Debug(). | |
| 130 | + | Str("reuse_policy", p.reusePolicy). | |
| 131 | + | Str("terminate_policy", p.terminatePolicy). | |
| 132 | + | Msg("creating webview pool") | |
| 97 | 133 | ||
| 98 | 134 | return p | |
| 99 | 135 | } | |
| @@ -113,6 +149,7 @@ func (p *WebViewPool) SetBackgroundColor(r, g, b, a float32) { | |||
| 113 | 149 | // Acquire gets a WebView from the pool or creates a new one. | |
| 114 | 150 | func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |
| 115 | 151 | log := logging.FromContext(ctx) | |
| 152 | + | start := time.Now() | |
| 116 | 153 | ||
| 117 | 154 | if p.closed.Load() { | |
| 118 | 155 | return nil, ErrPoolClosed | |
| @@ -135,7 +172,15 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 135 | 172 | if p.filterApplier != nil { | |
| 136 | 173 | p.filterApplier.ApplyTo(ctx, wv.ucm) | |
| 137 | 174 | } | |
| 138 | - | log.Debug().Uint64("id", uint64(wv.ID())).Msg("acquired webview from pool") | |
| 175 | + | p.acquireCount.Add(1) | |
| 176 | + | p.acquirePoolHits.Add(1) | |
| 177 | + | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |
| 178 | + | log.Debug(). | |
| 179 | + | Uint64("id", uint64(wv.ID())). | |
| 180 | + | Dur("latency", time.Since(start)). | |
| 181 | + | Int("pool_size", len(p.pool)). | |
| 182 | + | Str("source", "pool"). | |
| 183 | + | Msg("acquired webview") | |
| 139 | 184 | return wv, nil | |
| 140 | 185 | } | |
| 141 | 186 | // Fall through to create new if the pooled one was destroyed | |
| @@ -145,7 +190,20 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 145 | 190 | ||
| 146 | 191 | // Pool empty or pooled view was destroyed, create new | |
| 147 | 192 | log.Debug().Msg("pool empty, creating new webview") | |
| 148 | - | return p.createWebView(ctx) | |
| 193 | + | wv, err := p.createWebView(ctx) | |
| 194 | + | p.acquireCount.Add(1) | |
| 195 | + | p.acquirePoolMisses.Add(1) | |
| 196 | + | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |
| 197 | + | if err != nil { | |
| 198 | + | return nil, err | |
| 199 | + | } | |
| 200 | + | log.Debug(). | |
| 201 | + | Uint64("id", uint64(wv.ID())). | |
| 202 | + | Dur("latency", time.Since(start)). | |
| 203 | + | Int("pool_size", len(p.pool)). | |
| 204 | + | Str("source", "new"). | |
| 205 | + | Msg("acquired webview") | |
| 206 | + | return wv, nil | |
| 149 | 207 | } | |
| 150 | 208 | ||
| 151 | 209 | func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | |
| @@ -174,29 +232,52 @@ func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | |||
| 174 | 232 | return wv, nil | |
| 175 | 233 | } | |
| 176 | 234 | ||
| 177 | - | // Release returns a WebView to the pool. | |
| 178 | - | // For safety, we always destroy and never pool WebViews that have been used. | |
| 179 | - | // This prevents crashes from GLib signals firing after the widget tree is modified. | |
| 180 | - | // The pool is only used for prewarmed WebViews that haven't been attached to UI yet. | |
| 235 | + | // Release returns a WebView to the pool when configured and safe. | |
| 236 | + | // Default behavior remains conservative (`reuse_policy=off`) and destroys on release. | |
| 181 | 237 | func (p *WebViewPool) Release(ctx context.Context, wv *WebView) { | |
| 182 | 238 | log := logging.FromContext(ctx) | |
| 239 | + | start := time.Now() | |
| 183 | 240 | ||
| 184 | 241 | if wv == nil || wv.IsDestroyed() || p.closed.Load() { | |
| 185 | 242 | if wv != nil && !wv.IsDestroyed() { | |
| 186 | - | wv.Destroy() | |
| 243 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |
| 187 | 244 | } | |
| 245 | + | p.releaseDropped.Add(1) | |
| 188 | 246 | return | |
| 189 | 247 | } | |
| 190 | 248 | ||
| 191 | - | // CRITICAL: Always destroy used WebViews instead of pooling them. | |
| 192 | - | // GLib signals are connected with closures that can fire asynchronously | |
| 193 | - | // even after we clear Go callbacks. Loading about:blank triggers signals | |
| 194 | - | // that can crash if the widget tree has been modified. | |
| 195 | - | // | |
| 196 | - | // The pool is designed for prewarmed WebViews only, not for recycling | |
| 197 | - | // WebViews that have been attached to the UI. | |
| 198 | - | log.Debug().Uint64("id", uint64(wv.ID())).Msg("destroying webview (not pooling used views)") | |
| 199 | - | wv.Destroy() | |
| 249 | + | action := "destroyed" | |
| 250 | + | if p.shouldReuseOnRelease(wv) { | |
| 251 | + | wv.ResetForPoolReuse() | |
| 252 | + | if p.tryPut(ctx, wv) { | |
| 253 | + | action = "reused" | |
| 254 | + | p.releaseReused.Add(1) | |
| 255 | + | } else { | |
| 256 | + | action = "destroyed_after_reuse_failed" | |
| 257 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |
| 258 | + | p.releaseDestroyed.Add(1) | |
| 259 | + | } | |
| 260 | + | } else { | |
| 261 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |
| 262 | + | p.releaseDestroyed.Add(1) | |
| 263 | + | } | |
| 264 | + | p.releaseCount.Add(1) | |
| 265 | + | elapsed := time.Since(start) | |
| 266 | + | p.releaseTotalNanos.Add(elapsed.Nanoseconds()) | |
| 267 | + | log.Debug(). | |
| 268 | + | Uint64("id", uint64(wv.ID())). | |
| 269 | + | Str("action", action). | |
| 270 | + | Str("reuse_policy", p.reusePolicy). | |
| 271 | + | Str("terminate_policy", p.terminatePolicy). | |
| 272 | + | Dur("latency", elapsed). | |
| 273 | + | Int("pool_size", len(p.pool)). | |
| 274 | + | Uint64("acquire_total", p.acquireCount.Load()). | |
| 275 | + | Uint64("acquire_hits", p.acquirePoolHits.Load()). | |
| 276 | + | Uint64("acquire_misses", p.acquirePoolMisses.Load()). | |
| 277 | + | Uint64("release_total", p.releaseCount.Load()). | |
| 278 | + | Uint64("release_reused", p.releaseReused.Load()). | |
| 279 | + | Uint64("release_destroyed", p.releaseDestroyed.Load()). | |
| 280 | + | Msg("released webview") | |
| 200 | 281 | } | |
| 201 | 282 | ||
| 202 | 283 | // PrewarmFirst creates exactly one WebView synchronously. | |
| @@ -426,3 +507,51 @@ func (p *WebViewPool) Close(ctx context.Context) { | |||
| 426 | 507 | ||
| 427 | 508 | log.Debug().Msg("webview pool closed") | |
| 428 | 509 | } | |
| 510 | + | ||
| 511 | + | func (p *WebViewPool) shouldReuseOnRelease(wv *WebView) bool { | |
| 512 | + | if p.reusePolicy == reusePolicyOff || wv == nil || wv.IsDestroyed() || wv.IsRelated() { | |
| 513 | + | return false | |
| 514 | + | } | |
| 515 | + | if len(p.pool) >= p.config.MaxSize { | |
| 516 | + | return false | |
| 517 | + | } | |
| 518 | + | switch p.reusePolicy { | |
| 519 | + | case reusePolicySafe: | |
| 520 | + | return !wv.HasNavigationActivity() | |
| 521 | + | case reusePolicyAggressive: | |
| 522 | + | return true | |
| 523 | + | default: | |
| 524 | + | return false | |
| 525 | + | } | |
| 526 | + | } | |
| 527 | + | ||
| 528 | + | func (p *WebViewPool) tryPut(ctx context.Context, wv *WebView) bool { | |
| 529 | + | log := logging.FromContext(ctx) | |
| 530 | + | select { | |
| 531 | + | case p.pool <- wv: | |
| 532 | + | log.Debug().Uint64("id", uint64(wv.ID())).Int("pool_size", len(p.pool)).Msg("returned webview to pool") | |
| 533 | + | return true | |
| 534 | + | default: | |
| 535 | + | return false | |
| 536 | + | } | |
| 537 | + | } | |
| 538 | + | ||
| 539 | + | func resolveReusePolicy(raw string) string { | |
| 540 | + | switch strings.ToLower(strings.TrimSpace(defaultIfEmpty(raw, os.Getenv("DUMBER_WEBVIEW_REUSE_POLICY")))) { | |
| 541 | + | case reusePolicySafe: | |
| 542 | + | return reusePolicySafe | |
| 543 | + | case reusePolicyAggressive: | |
| 544 | + | return reusePolicyAggressive | |
| 545 | + | case reusePolicyOff, "": | |
| 546 | + | return reusePolicyOff | |
| 547 | + | default: | |
| 548 | + | return reusePolicyOff | |
| 549 | + | } | |
| 550 | + | } | |
| 551 | + | ||
| 552 | + | func defaultIfEmpty(value, fallback string) string { | |
| 553 | + | if strings.TrimSpace(value) != "" { | |
| 554 | + | return value | |
| 555 | + | } | |
| 556 | + | return fallback | |
| 557 | + | } | |
| @@ -3,6 +3,8 @@ package webkit | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | + | "os" | |||
| 7 | + | "strings" | |||
| 6 | "sync" | 8 | "sync" | ||
| 7 | "sync/atomic" | 9 | "sync/atomic" | ||
| 8 | "time" | 10 | "time" | ||
| @@ -31,18 +33,32 @@ type PoolConfig struct { | |||||
| 31 | PrewarmCount int | 33 | PrewarmCount int | ||
| 32 | // IdleTimeout is how long idle views stay in the pool before being destroyed. | 34 | // IdleTimeout is how long idle views stay in the pool before being destroyed. | ||
| 33 | IdleTimeout time.Duration | 35 | IdleTimeout time.Duration | ||
| 36 | + | // ReusePolicy controls if/when released WebViews are returned to pool. | |||
| 37 | + | // Values: off, safe, aggressive. | |||
| 38 | + | ReusePolicy string | |||
| 39 | + | // TerminatePolicy controls whether Destroy() should terminate web process. | |||
| 40 | + | // Values: auto, always, never. | |||
| 41 | + | TerminatePolicy string | |||
| 34 | } | 42 | } | ||
| 35 | 43 | ||||
| 36 | // DefaultPoolConfig returns sensible defaults for the pool. | 44 | // DefaultPoolConfig returns sensible defaults for the pool. | ||
| 37 | func DefaultPoolConfig() PoolConfig { | 45 | func DefaultPoolConfig() PoolConfig { | ||
| 38 | return PoolConfig{ | 46 | return PoolConfig{ | ||
| 39 | - | MinSize: 2, | 47 | + | MinSize: 2, |
| 40 | - | MaxSize: 8, | 48 | + | MaxSize: 8, |
| 41 | - | PrewarmCount: 4, // Pre-create 4 WebViews for faster initial tab creation | 49 | + | PrewarmCount: 4, // Pre-create 4 WebViews for faster initial tab creation |
| 42 | - | IdleTimeout: 5 * time.Minute, | 50 | + | IdleTimeout: 5 * time.Minute, |
| 51 | + | ReusePolicy: reusePolicyOff, | |||
| 52 | + | TerminatePolicy: terminatePolicyAuto, | |||
| 43 | } | 53 | } | ||
| 44 | } | 54 | } | ||
| 45 | 55 | ||||
| 56 | + | const ( | |||
| 57 | + | reusePolicyOff = "off" | |||
| 58 | + | reusePolicySafe = "safe" | |||
| 59 | + | reusePolicyAggressive = "aggressive" | |||
| 60 | + | ) | |||
| 61 | + | ||||
| 46 | // WebViewPool manages a pool of pre-created WebViews for fast tab creation. | 62 | // WebViewPool manages a pool of pre-created WebViews for fast tab creation. | ||
| 47 | type WebViewPool struct { | 63 | type WebViewPool struct { | ||
| 48 | wkCtx *WebKitContext | 64 | wkCtx *WebKitContext | ||
| @@ -59,6 +75,19 @@ type WebViewPool struct { | |||||
| 59 | 75 | ||||
| 60 | closed atomic.Bool | 76 | closed atomic.Bool | ||
| 61 | wg sync.WaitGroup | 77 | wg sync.WaitGroup | ||
| 78 | + | ||||
| 79 | + | reusePolicy string | |||
| 80 | + | terminatePolicy string | |||
| 81 | + | ||||
| 82 | + | acquireCount atomic.Uint64 | |||
| 83 | + | acquirePoolHits atomic.Uint64 | |||
| 84 | + | acquirePoolMisses atomic.Uint64 | |||
| 85 | + | releaseCount atomic.Uint64 | |||
| 86 | + | releaseReused atomic.Uint64 | |||
| 87 | + | releaseDestroyed atomic.Uint64 | |||
| 88 | + | releaseDropped atomic.Uint64 | |||
| 89 | + | releaseTotalNanos atomic.Int64 | |||
| 90 | + | acquireTotalNanos atomic.Int64 | |||
| 62 | } | 91 | } | ||
| 63 | 92 | ||||
| 64 | // NewWebViewPool creates a new WebView pool. | 93 | // NewWebViewPool creates a new WebView pool. | ||
| @@ -79,21 +108,28 @@ func NewWebViewPool( | |||||
| 79 | if cfg.MinSize > cfg.MaxSize { | 108 | if cfg.MinSize > cfg.MaxSize { | ||
| 80 | cfg.MinSize = cfg.MaxSize | 109 | cfg.MinSize = cfg.MaxSize | ||
| 81 | } | 110 | } | ||
| 111 | + | cfg.ReusePolicy = resolveReusePolicy(cfg.ReusePolicy) | |||
| 112 | + | cfg.TerminatePolicy = resolveTerminatePolicy(cfg.TerminatePolicy) | |||
| 82 | if ctx == nil { | 113 | if ctx == nil { | ||
| 83 | ctx = context.Background() | 114 | ctx = context.Background() | ||
| 84 | } | 115 | } | ||
| 85 | 116 | ||||
| 86 | p := &WebViewPool{ | 117 | p := &WebViewPool{ | ||
| 87 | - | wkCtx: wkCtx, | 118 | + | wkCtx: wkCtx, |
| 88 | - | settings: settings, | 119 | + | settings: settings, |
| 89 | - | config: cfg, | 120 | + | config: cfg, |
| 90 | - | pool: make(chan *WebView, cfg.MaxSize), | 121 | + | pool: make(chan *WebView, cfg.MaxSize), |
| 91 | - | injector: injector, | 122 | + | injector: injector, |
| 92 | - | router: router, | 123 | + | router: router, |
| 124 | + | reusePolicy: cfg.ReusePolicy, | |||
| 125 | + | terminatePolicy: cfg.TerminatePolicy, | |||
| 93 | } | 126 | } | ||
| 94 | 127 | ||||
| 95 | log := logging.FromContext(ctx) | 128 | log := logging.FromContext(ctx) | ||
| 96 | - | log.Debug().Msg("creating webview pool") | 129 | + | log.Debug(). |
| 130 | + | Str("reuse_policy", p.reusePolicy). | |||
| 131 | + | Str("terminate_policy", p.terminatePolicy). | |||
| 132 | + | Msg("creating webview pool") | |||
| 97 | 133 | ||||
| 98 | return p | 134 | return p | ||
| 99 | } | 135 | } | ||
| @@ -113,6 +149,7 @@ func (p *WebViewPool) SetBackgroundColor(r, g, b, a float32) { | |||||
| 113 | // Acquire gets a WebView from the pool or creates a new one. | 149 | // Acquire gets a WebView from the pool or creates a new one. | ||
| 114 | func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | 150 | func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | ||
| 115 | log := logging.FromContext(ctx) | 151 | log := logging.FromContext(ctx) | ||
| 152 | + | start := time.Now() | |||
| 116 | 153 | ||||
| 117 | if p.closed.Load() { | 154 | if p.closed.Load() { | ||
| 118 | return nil, ErrPoolClosed | 155 | return nil, ErrPoolClosed | ||
| @@ -135,7 +172,15 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 135 | if p.filterApplier != nil { | 172 | if p.filterApplier != nil { | ||
| 136 | p.filterApplier.ApplyTo(ctx, wv.ucm) | 173 | p.filterApplier.ApplyTo(ctx, wv.ucm) | ||
| 137 | } | 174 | } | ||
| 138 | - | log.Debug().Uint64("id", uint64(wv.ID())).Msg("acquired webview from pool") | 175 | + | p.acquireCount.Add(1) |
| 176 | + | p.acquirePoolHits.Add(1) | |||
| 177 | + | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |||
| 178 | + | log.Debug(). | |||
| 179 | + | Uint64("id", uint64(wv.ID())). | |||
| 180 | + | Dur("latency", time.Since(start)). | |||
| 181 | + | Int("pool_size", len(p.pool)). | |||
| 182 | + | Str("source", "pool"). | |||
| 183 | + | Msg("acquired webview") | |||
| 139 | return wv, nil | 184 | return wv, nil | ||
| 140 | } | 185 | } | ||
| 141 | // Fall through to create new if the pooled one was destroyed | 186 | // Fall through to create new if the pooled one was destroyed | ||
| @@ -145,7 +190,20 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 145 | 190 | ||||
| 146 | // Pool empty or pooled view was destroyed, create new | 191 | // Pool empty or pooled view was destroyed, create new | ||
| 147 | log.Debug().Msg("pool empty, creating new webview") | 192 | log.Debug().Msg("pool empty, creating new webview") | ||
| 148 | - | return p.createWebView(ctx) | 193 | + | wv, err := p.createWebView(ctx) |
| 194 | + | p.acquireCount.Add(1) | |||
| 195 | + | p.acquirePoolMisses.Add(1) | |||
| 196 | + | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |||
| 197 | + | if err != nil { | |||
| 198 | + | return nil, err | |||
| 199 | + | } | |||
| 200 | + | log.Debug(). | |||
| 201 | + | Uint64("id", uint64(wv.ID())). | |||
| 202 | + | Dur("latency", time.Since(start)). | |||
| 203 | + | Int("pool_size", len(p.pool)). | |||
| 204 | + | Str("source", "new"). | |||
| 205 | + | Msg("acquired webview") | |||
| 206 | + | return wv, nil | |||
| 149 | } | 207 | } | ||
| 150 | 208 | ||||
| 151 | func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | 209 | func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | ||
| @@ -174,29 +232,52 @@ func (p *WebViewPool) createWebView(ctx context.Context) (*WebView, error) { | |||||
| 174 | return wv, nil | 232 | return wv, nil | ||
| 175 | } | 233 | } | ||
| 176 | 234 | ||||
| 177 | - | // Release returns a WebView to the pool. | 235 | + | // Release returns a WebView to the pool when configured and safe. |
| 178 | - | // For safety, we always destroy and never pool WebViews that have been used. | 236 | + | // Default behavior remains conservative (`reuse_policy=off`) and destroys on release. |
| 179 | - | // This prevents crashes from GLib signals firing after the widget tree is modified. | |||
| 180 | - | // The pool is only used for prewarmed WebViews that haven't been attached to UI yet. | |||
| 181 | func (p *WebViewPool) Release(ctx context.Context, wv *WebView) { | 237 | func (p *WebViewPool) Release(ctx context.Context, wv *WebView) { | ||
| 182 | log := logging.FromContext(ctx) | 238 | log := logging.FromContext(ctx) | ||
| 239 | + | start := time.Now() | |||
| 183 | 240 | ||||
| 184 | if wv == nil || wv.IsDestroyed() || p.closed.Load() { | 241 | if wv == nil || wv.IsDestroyed() || p.closed.Load() { | ||
| 185 | if wv != nil && !wv.IsDestroyed() { | 242 | if wv != nil && !wv.IsDestroyed() { | ||
| 186 | - | wv.Destroy() | 243 | + | wv.DestroyWithPolicy(p.terminatePolicy) |
| 187 | } | 244 | } | ||
| 245 | + | p.releaseDropped.Add(1) | |||
| 188 | return | 246 | return | ||
| 189 | } | 247 | } | ||
| 190 | 248 | ||||
| 191 | - | // CRITICAL: Always destroy used WebViews instead of pooling them. | 249 | + | action := "destroyed" |
| 192 | - | // GLib signals are connected with closures that can fire asynchronously | 250 | + | if p.shouldReuseOnRelease(wv) { |
| 193 | - | // even after we clear Go callbacks. Loading about:blank triggers signals | 251 | + | wv.ResetForPoolReuse() |
| 194 | - | // that can crash if the widget tree has been modified. | 252 | + | if p.tryPut(ctx, wv) { |
| 195 | - | // | 253 | + | action = "reused" |
| 196 | - | // The pool is designed for prewarmed WebViews only, not for recycling | 254 | + | p.releaseReused.Add(1) |
| 197 | - | // WebViews that have been attached to the UI. | 255 | + | } else { |
| 198 | - | log.Debug().Uint64("id", uint64(wv.ID())).Msg("destroying webview (not pooling used views)") | 256 | + | action = "destroyed_after_reuse_failed" |
| 199 | - | wv.Destroy() | 257 | + | wv.DestroyWithPolicy(p.terminatePolicy) |
| 258 | + | p.releaseDestroyed.Add(1) | |||
| 259 | + | } | |||
| 260 | + | } else { | |||
| 261 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |||
| 262 | + | p.releaseDestroyed.Add(1) | |||
| 263 | + | } | |||
| 264 | + | p.releaseCount.Add(1) | |||
| 265 | + | elapsed := time.Since(start) | |||
| 266 | + | p.releaseTotalNanos.Add(elapsed.Nanoseconds()) | |||
| 267 | + | log.Debug(). | |||
| 268 | + | Uint64("id", uint64(wv.ID())). | |||
| 269 | + | Str("action", action). | |||
| 270 | + | Str("reuse_policy", p.reusePolicy). | |||
| 271 | + | Str("terminate_policy", p.terminatePolicy). | |||
| 272 | + | Dur("latency", elapsed). | |||
| 273 | + | Int("pool_size", len(p.pool)). | |||
| 274 | + | Uint64("acquire_total", p.acquireCount.Load()). | |||
| 275 | + | Uint64("acquire_hits", p.acquirePoolHits.Load()). | |||
| 276 | + | Uint64("acquire_misses", p.acquirePoolMisses.Load()). | |||
| 277 | + | Uint64("release_total", p.releaseCount.Load()). | |||
| 278 | + | Uint64("release_reused", p.releaseReused.Load()). | |||
| 279 | + | Uint64("release_destroyed", p.releaseDestroyed.Load()). | |||
| 280 | + | Msg("released webview") | |||
| 200 | } | 281 | } | ||
| 201 | 282 | ||||
| 202 | // PrewarmFirst creates exactly one WebView synchronously. | 283 | // PrewarmFirst creates exactly one WebView synchronously. | ||
| @@ -426,3 +507,51 @@ func (p *WebViewPool) Close(ctx context.Context) { | |||||
| 426 | 507 | ||||
| 427 | log.Debug().Msg("webview pool closed") | 508 | log.Debug().Msg("webview pool closed") | ||
| 428 | } | 509 | } | ||
| 510 | + | ||||
| 511 | + | func (p *WebViewPool) shouldReuseOnRelease(wv *WebView) bool { | |||
| 512 | + | if p.reusePolicy == reusePolicyOff || wv == nil || wv.IsDestroyed() || wv.IsRelated() { | |||
| 513 | + | return false | |||
| 514 | + | } | |||
| 515 | + | if len(p.pool) >= p.config.MaxSize { | |||
| 516 | + | return false | |||
| 517 | + | } | |||
| 518 | + | switch p.reusePolicy { | |||
| 519 | + | case reusePolicySafe: | |||
| 520 | + | return !wv.HasNavigationActivity() | |||
| 521 | + | case reusePolicyAggressive: | |||
| 522 | + | return true | |||
| 523 | + | default: | |||
| 524 | + | return false | |||
| 525 | + | } | |||
| 526 | + | } | |||
| 527 | + | ||||
| 528 | + | func (p *WebViewPool) tryPut(ctx context.Context, wv *WebView) bool { | |||
| 529 | + | log := logging.FromContext(ctx) | |||
| 530 | + | select { | |||
| 531 | + | case p.pool <- wv: | |||
| 532 | + | log.Debug().Uint64("id", uint64(wv.ID())).Int("pool_size", len(p.pool)).Msg("returned webview to pool") | |||
| 533 | + | return true | |||
| 534 | + | default: | |||
| 535 | + | return false | |||
| 536 | + | } | |||
| 537 | + | } | |||
| 538 | + | ||||
| 539 | + | func resolveReusePolicy(raw string) string { | |||
| 540 | + | switch strings.ToLower(strings.TrimSpace(defaultIfEmpty(raw, os.Getenv("DUMBER_WEBVIEW_REUSE_POLICY")))) { | |||
| 541 | + | case reusePolicySafe: | |||
| 542 | + | return reusePolicySafe | |||
| 543 | + | case reusePolicyAggressive: | |||
| 544 | + | return reusePolicyAggressive | |||
| 545 | + | case reusePolicyOff, "": | |||
| 546 | + | return reusePolicyOff | |||
| 547 | + | default: | |||
| 548 | + | return reusePolicyOff | |||
| 549 | + | } | |||
| 550 | + | } | |||
| 551 | + | ||||
| 552 | + | func defaultIfEmpty(value, fallback string) string { | |||
| 553 | + | if strings.TrimSpace(value) != "" { | |||
| 554 | + | return value | |||
| 555 | + | } | |||
| 556 | + | return fallback | |||
| 557 | + | } | |||
A internal/infrastructure/webkit/pool_policy_test.go +18 -0
| @@ -0,0 +1,18 @@ | |||
| 1 | + | package webkit | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/assert" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestResolveReusePolicy(t *testing.T) { | |
| 10 | + | t.Setenv("DUMBER_WEBVIEW_REUSE_POLICY", "") | |
| 11 | + | assert.Equal(t, reusePolicyOff, resolveReusePolicy("")) | |
| 12 | + | assert.Equal(t, reusePolicySafe, resolveReusePolicy("safe")) | |
| 13 | + | assert.Equal(t, reusePolicyAggressive, resolveReusePolicy("aggressive")) | |
| 14 | + | assert.Equal(t, reusePolicyOff, resolveReusePolicy("unknown")) | |
| 15 | + | ||
| 16 | + | t.Setenv("DUMBER_WEBVIEW_REUSE_POLICY", "safe") | |
| 17 | + | assert.Equal(t, reusePolicySafe, resolveReusePolicy("")) | |
| 18 | + | } | |
| @@ -0,0 +1,18 @@ | |||||
| 1 | + | package webkit | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/assert" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestResolveReusePolicy(t *testing.T) { | |||
| 10 | + | t.Setenv("DUMBER_WEBVIEW_REUSE_POLICY", "") | |||
| 11 | + | assert.Equal(t, reusePolicyOff, resolveReusePolicy("")) | |||
| 12 | + | assert.Equal(t, reusePolicySafe, resolveReusePolicy("safe")) | |||
| 13 | + | assert.Equal(t, reusePolicyAggressive, resolveReusePolicy("aggressive")) | |||
| 14 | + | assert.Equal(t, reusePolicyOff, resolveReusePolicy("unknown")) | |||
| 15 | + | ||||
| 16 | + | t.Setenv("DUMBER_WEBVIEW_REUSE_POLICY", "safe") | |||
| 17 | + | assert.Equal(t, reusePolicySafe, resolveReusePolicy("")) | |||
| 18 | + | } | |||
M internal/infrastructure/webkit/webview.go +111 -9
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "net/url" | |
| 7 | + | "os" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "sync" | |
| 9 | 10 | "sync/atomic" | |
| @@ -157,6 +158,7 @@ type WebView struct { | |||
| 157 | 158 | mu sync.RWMutex | |
| 158 | 159 | ||
| 159 | 160 | frontendAttached atomic.Bool | |
| 161 | + | navigationActive atomic.Bool | |
| 160 | 162 | ||
| 161 | 163 | // asyncCallbacks keeps references to async JS callbacks to prevent GC | |
| 162 | 164 | asyncCallbacks []interface{} | |
| @@ -169,6 +171,12 @@ type WebView struct { | |||
| 169 | 171 | backForwardListSignalID uint32 | |
| 170 | 172 | } | |
| 171 | 173 | ||
| 174 | + | const ( | |
| 175 | + | terminatePolicyAuto = "auto" | |
| 176 | + | terminatePolicyAlways = "always" | |
| 177 | + | terminatePolicyNever = "never" | |
| 178 | + | ) | |
| 179 | + | ||
| 172 | 180 | type findControllerAdapter struct { | |
| 173 | 181 | fc *webkit.FindController | |
| 174 | 182 | } | |
| @@ -406,6 +414,7 @@ func (wv *WebView) connectLoadChangedSignal() { | |||
| 406 | 414 | ||
| 407 | 415 | switch event { | |
| 408 | 416 | case webkit.LoadStartedValue: | |
| 417 | + | wv.navigationActive.Store(true) | |
| 409 | 418 | wv.isLoading = true | |
| 410 | 419 | wv.logger.Debug().Str("uri", uri).Msg("load started") | |
| 411 | 420 | case webkit.LoadRedirectedValue: | |
| @@ -941,6 +950,7 @@ func (wv *WebView) LoadURI(ctx context.Context, uri string) error { | |||
| 941 | 950 | if wv.destroyed.Load() { | |
| 942 | 951 | return fmt.Errorf("webview %d is destroyed", wv.id) | |
| 943 | 952 | } | |
| 953 | + | wv.navigationActive.Store(true) | |
| 944 | 954 | wv.inner.LoadUri(uri) | |
| 945 | 955 | logging.FromContext(ctx).Debug().Str("uri", uri).Msg("loading URI") | |
| 946 | 956 | return nil | |
| @@ -951,6 +961,7 @@ func (wv *WebView) LoadHTML(ctx context.Context, content, baseURI string) error | |||
| 951 | 961 | if wv.destroyed.Load() { | |
| 952 | 962 | return fmt.Errorf("webview %d is destroyed", wv.id) | |
| 953 | 963 | } | |
| 964 | + | wv.navigationActive.Store(true) | |
| 954 | 965 | var baseURIPtr *string | |
| 955 | 966 | if baseURI != "" { | |
| 956 | 967 | baseURIPtr = &baseURI | |
| @@ -1249,6 +1260,16 @@ func (wv *WebView) IsDestroyed() bool { | |||
| 1249 | 1260 | return wv.destroyed.Load() | |
| 1250 | 1261 | } | |
| 1251 | 1262 | ||
| 1263 | + | // IsRelated returns true when this WebView shares process/session with a parent popup. | |
| 1264 | + | func (wv *WebView) IsRelated() bool { | |
| 1265 | + | return wv.isRelated | |
| 1266 | + | } | |
| 1267 | + | ||
| 1268 | + | // HasNavigationActivity reports whether this WebView has been used for content navigation. | |
| 1269 | + | func (wv *WebView) HasNavigationActivity() bool { | |
| 1270 | + | return wv.navigationActive.Load() | |
| 1271 | + | } | |
| 1272 | + | ||
| 1252 | 1273 | // Close triggers the close callback as if window.close() was called. | |
| 1253 | 1274 | // This is used for programmatic popup closure (e.g., OAuth auto-close). | |
| 1254 | 1275 | func (wv *WebView) Close() { | |
| @@ -1288,9 +1309,16 @@ func (wv *WebView) DisconnectSignals() { | |||
| 1288 | 1309 | // This must be called when a WebView is no longer needed to free GPU resources, | |
| 1289 | 1310 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | |
| 1290 | 1311 | func (wv *WebView) Destroy() { | |
| 1312 | + | wv.DestroyWithPolicy(resolveTerminatePolicy("")) | |
| 1313 | + | } | |
| 1314 | + | ||
| 1315 | + | // DestroyWithPolicy cleans up the WebView resources with explicit process policy. | |
| 1316 | + | // Valid policies: auto, always, never. | |
| 1317 | + | func (wv *WebView) DestroyWithPolicy(policy string) { | |
| 1291 | 1318 | if wv.destroyed.Swap(true) { | |
| 1292 | 1319 | return // Already destroyed | |
| 1293 | 1320 | } | |
| 1321 | + | policy = resolveTerminatePolicy(policy) | |
| 1294 | 1322 | ||
| 1295 | 1323 | // 1. Disconnect all signal handlers first to prevent callbacks during cleanup | |
| 1296 | 1324 | wv.DisconnectSignals() | |
| @@ -1322,11 +1350,9 @@ func (wv *WebView) Destroy() { | |||
| 1322 | 1350 | wv.inner.Unparent() | |
| 1323 | 1351 | } | |
| 1324 | 1352 | ||
| 1325 | - | // 5. Terminate the web process to free GPU resources (VA-API, DMA-BUF, GL contexts) | |
| 1326 | - | // This is critical to prevent zombie processes that hold video decoder resources. | |
| 1327 | - | // IMPORTANT: Skip for related views (popups) - they share the web process with their | |
| 1328 | - | // parent. Terminating the shared process would kill the parent WebView too! | |
| 1329 | - | if !wv.isRelated && wv.inner != nil { | |
| 1353 | + | // 5. Optionally terminate web process (skip for related popup views sharing parent process). | |
| 1354 | + | terminate := shouldTerminateWebProcess(policy, wv.isRelated, wv.navigationActive.Load()) | |
| 1355 | + | if terminate && wv.inner != nil { | |
| 1330 | 1356 | wv.inner.TerminateWebProcess() | |
| 1331 | 1357 | } | |
| 1332 | 1358 | ||
| @@ -1338,10 +1364,86 @@ func (wv *WebView) Destroy() { | |||
| 1338 | 1364 | wv.ucm = nil | |
| 1339 | 1365 | wv.findController = nil | |
| 1340 | 1366 | ||
| 1341 | - | if wv.isRelated { | |
| 1342 | - | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("related webview destroyed (process shared with parent)") | |
| 1343 | - | } else { | |
| 1344 | - | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("webview destroyed and web process terminated") | |
| 1367 | + | wv.logger.Debug(). | |
| 1368 | + | Uint64("id", uint64(wv.id)). | |
| 1369 | + | Bool("related", wv.isRelated). | |
| 1370 | + | Bool("navigation_active", wv.navigationActive.Load()). | |
| 1371 | + | Bool("terminated_process", terminate). | |
| 1372 | + | Str("terminate_policy", policy). | |
| 1373 | + | Msg("webview destroyed") | |
| 1374 | + | } | |
| 1375 | + | ||
| 1376 | + | // ResetForPoolReuse sanitizes callbacks/state so the WebView can be safely reused. | |
| 1377 | + | func (wv *WebView) ResetForPoolReuse() { | |
| 1378 | + | if wv == nil || wv.destroyed.Load() { | |
| 1379 | + | return | |
| 1380 | + | } | |
| 1381 | + | ||
| 1382 | + | wv.OnLoadChanged = nil | |
| 1383 | + | wv.OnTitleChanged = nil | |
| 1384 | + | wv.OnURIChanged = nil | |
| 1385 | + | wv.OnProgressChanged = nil | |
| 1386 | + | wv.OnFaviconChanged = nil | |
| 1387 | + | wv.OnClose = nil | |
| 1388 | + | wv.OnCreate = nil | |
| 1389 | + | wv.OnReadyToShow = nil | |
| 1390 | + | wv.OnLinkMiddleClick = nil | |
| 1391 | + | wv.OnEnterFullscreen = nil | |
| 1392 | + | wv.OnLeaveFullscreen = nil | |
| 1393 | + | wv.OnAudioStateChanged = nil | |
| 1394 | + | wv.OnLinkHover = nil | |
| 1395 | + | wv.OnWebProcessTerminated = nil | |
| 1396 | + | ||
| 1397 | + | wv.mu.Lock() | |
| 1398 | + | wv.uri = "" | |
| 1399 | + | wv.title = "" | |
| 1400 | + | wv.progress = 0 | |
| 1401 | + | wv.canGoBack = false | |
| 1402 | + | wv.canGoFwd = false | |
| 1403 | + | wv.isLoading = false | |
| 1404 | + | wv.asyncCallbacks = nil | |
| 1405 | + | wv.mu.Unlock() | |
| 1406 | + | ||
| 1407 | + | wv.isFullscreen.Store(false) | |
| 1408 | + | wv.isPlayingAudio.Store(false) | |
| 1409 | + | wv.navigationActive.Store(false) | |
| 1410 | + | ||
| 1411 | + | if wv.inner != nil { | |
| 1412 | + | wv.inner.StopLoading() | |
| 1413 | + | wv.inner.SetVisible(false) | |
| 1414 | + | } | |
| 1415 | + | } | |
| 1416 | + | ||
| 1417 | + | func resolveTerminatePolicy(raw string) string { | |
| 1418 | + | value := strings.ToLower(strings.TrimSpace(raw)) | |
| 1419 | + | if value == "" { | |
| 1420 | + | value = strings.ToLower(strings.TrimSpace(os.Getenv("DUMBER_WEBVIEW_TERMINATE_POLICY"))) | |
| 1421 | + | } | |
| 1422 | + | switch value { | |
| 1423 | + | case terminatePolicyAlways: | |
| 1424 | + | return terminatePolicyAlways | |
| 1425 | + | case terminatePolicyNever: | |
| 1426 | + | return terminatePolicyNever | |
| 1427 | + | case terminatePolicyAuto, "": | |
| 1428 | + | return terminatePolicyAuto | |
| 1429 | + | default: | |
| 1430 | + | return terminatePolicyAuto | |
| 1431 | + | } | |
| 1432 | + | } | |
| 1433 | + | ||
| 1434 | + | func shouldTerminateWebProcess(policy string, isRelated, navigationActive bool) bool { | |
| 1435 | + | if isRelated { | |
| 1436 | + | return false | |
| 1437 | + | } | |
| 1438 | + | switch resolveTerminatePolicy(policy) { | |
| 1439 | + | case terminatePolicyNever: | |
| 1440 | + | return false | |
| 1441 | + | case terminatePolicyAlways: | |
| 1442 | + | return true | |
| 1443 | + | case terminatePolicyAuto: | |
| 1444 | + | return navigationActive | |
| 1445 | + | default: | |
| 1446 | + | return navigationActive | |
| 1345 | 1447 | } | |
| 1346 | 1448 | } | |
| 1347 | 1449 | ||
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "net/url" | 6 | "net/url" | ||
| 7 | + | "os" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "sync" | 9 | "sync" | ||
| 9 | "sync/atomic" | 10 | "sync/atomic" | ||
| @@ -157,6 +158,7 @@ type WebView struct { | |||||
| 157 | mu sync.RWMutex | 158 | mu sync.RWMutex | ||
| 158 | 159 | ||||
| 159 | frontendAttached atomic.Bool | 160 | frontendAttached atomic.Bool | ||
| 161 | + | navigationActive atomic.Bool | |||
| 160 | 162 | ||||
| 161 | // asyncCallbacks keeps references to async JS callbacks to prevent GC | 163 | // asyncCallbacks keeps references to async JS callbacks to prevent GC | ||
| 162 | asyncCallbacks []interface{} | 164 | asyncCallbacks []interface{} | ||
| @@ -169,6 +171,12 @@ type WebView struct { | |||||
| 169 | backForwardListSignalID uint32 | 171 | backForwardListSignalID uint32 | ||
| 170 | } | 172 | } | ||
| 171 | 173 | ||||
| 174 | + | const ( | |||
| 175 | + | terminatePolicyAuto = "auto" | |||
| 176 | + | terminatePolicyAlways = "always" | |||
| 177 | + | terminatePolicyNever = "never" | |||
| 178 | + | ) | |||
| 179 | + | ||||
| 172 | type findControllerAdapter struct { | 180 | type findControllerAdapter struct { | ||
| 173 | fc *webkit.FindController | 181 | fc *webkit.FindController | ||
| 174 | } | 182 | } | ||
| @@ -406,6 +414,7 @@ func (wv *WebView) connectLoadChangedSignal() { | |||||
| 406 | 414 | ||||
| 407 | switch event { | 415 | switch event { | ||
| 408 | case webkit.LoadStartedValue: | 416 | case webkit.LoadStartedValue: | ||
| 417 | + | wv.navigationActive.Store(true) | |||
| 409 | wv.isLoading = true | 418 | wv.isLoading = true | ||
| 410 | wv.logger.Debug().Str("uri", uri).Msg("load started") | 419 | wv.logger.Debug().Str("uri", uri).Msg("load started") | ||
| 411 | case webkit.LoadRedirectedValue: | 420 | case webkit.LoadRedirectedValue: | ||
| @@ -941,6 +950,7 @@ func (wv *WebView) LoadURI(ctx context.Context, uri string) error { | |||||
| 941 | if wv.destroyed.Load() { | 950 | if wv.destroyed.Load() { | ||
| 942 | return fmt.Errorf("webview %d is destroyed", wv.id) | 951 | return fmt.Errorf("webview %d is destroyed", wv.id) | ||
| 943 | } | 952 | } | ||
| 953 | + | wv.navigationActive.Store(true) | |||
| 944 | wv.inner.LoadUri(uri) | 954 | wv.inner.LoadUri(uri) | ||
| 945 | logging.FromContext(ctx).Debug().Str("uri", uri).Msg("loading URI") | 955 | logging.FromContext(ctx).Debug().Str("uri", uri).Msg("loading URI") | ||
| 946 | return nil | 956 | return nil | ||
| @@ -951,6 +961,7 @@ func (wv *WebView) LoadHTML(ctx context.Context, content, baseURI string) error | |||||
| 951 | if wv.destroyed.Load() { | 961 | if wv.destroyed.Load() { | ||
| 952 | return fmt.Errorf("webview %d is destroyed", wv.id) | 962 | return fmt.Errorf("webview %d is destroyed", wv.id) | ||
| 953 | } | 963 | } | ||
| 964 | + | wv.navigationActive.Store(true) | |||
| 954 | var baseURIPtr *string | 965 | var baseURIPtr *string | ||
| 955 | if baseURI != "" { | 966 | if baseURI != "" { | ||
| 956 | baseURIPtr = &baseURI | 967 | baseURIPtr = &baseURI | ||
| @@ -1249,6 +1260,16 @@ func (wv *WebView) IsDestroyed() bool { | |||||
| 1249 | return wv.destroyed.Load() | 1260 | return wv.destroyed.Load() | ||
| 1250 | } | 1261 | } | ||
| 1251 | 1262 | ||||
| 1263 | + | // IsRelated returns true when this WebView shares process/session with a parent popup. | |||
| 1264 | + | func (wv *WebView) IsRelated() bool { | |||
| 1265 | + | return wv.isRelated | |||
| 1266 | + | } | |||
| 1267 | + | ||||
| 1268 | + | // HasNavigationActivity reports whether this WebView has been used for content navigation. | |||
| 1269 | + | func (wv *WebView) HasNavigationActivity() bool { | |||
| 1270 | + | return wv.navigationActive.Load() | |||
| 1271 | + | } | |||
| 1272 | + | ||||
| 1252 | // Close triggers the close callback as if window.close() was called. | 1273 | // Close triggers the close callback as if window.close() was called. | ||
| 1253 | // This is used for programmatic popup closure (e.g., OAuth auto-close). | 1274 | // This is used for programmatic popup closure (e.g., OAuth auto-close). | ||
| 1254 | func (wv *WebView) Close() { | 1275 | func (wv *WebView) Close() { | ||
| @@ -1288,9 +1309,16 @@ func (wv *WebView) DisconnectSignals() { | |||||
| 1288 | // This must be called when a WebView is no longer needed to free GPU resources, | 1309 | // This must be called when a WebView is no longer needed to free GPU resources, | ||
| 1289 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | 1310 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | ||
| 1290 | func (wv *WebView) Destroy() { | 1311 | func (wv *WebView) Destroy() { | ||
| 1312 | + | wv.DestroyWithPolicy(resolveTerminatePolicy("")) | |||
| 1313 | + | } | |||
| 1314 | + | ||||
| 1315 | + | // DestroyWithPolicy cleans up the WebView resources with explicit process policy. | |||
| 1316 | + | // Valid policies: auto, always, never. | |||
| 1317 | + | func (wv *WebView) DestroyWithPolicy(policy string) { | |||
| 1291 | if wv.destroyed.Swap(true) { | 1318 | if wv.destroyed.Swap(true) { | ||
| 1292 | return // Already destroyed | 1319 | return // Already destroyed | ||
| 1293 | } | 1320 | } | ||
| 1321 | + | policy = resolveTerminatePolicy(policy) | |||
| 1294 | 1322 | ||||
| 1295 | // 1. Disconnect all signal handlers first to prevent callbacks during cleanup | 1323 | // 1. Disconnect all signal handlers first to prevent callbacks during cleanup | ||
| 1296 | wv.DisconnectSignals() | 1324 | wv.DisconnectSignals() | ||
| @@ -1322,11 +1350,9 @@ func (wv *WebView) Destroy() { | |||||
| 1322 | wv.inner.Unparent() | 1350 | wv.inner.Unparent() | ||
| 1323 | } | 1351 | } | ||
| 1324 | 1352 | ||||
| 1325 | - | // 5. Terminate the web process to free GPU resources (VA-API, DMA-BUF, GL contexts) | 1353 | + | // 5. Optionally terminate web process (skip for related popup views sharing parent process). |
| 1326 | - | // This is critical to prevent zombie processes that hold video decoder resources. | 1354 | + | terminate := shouldTerminateWebProcess(policy, wv.isRelated, wv.navigationActive.Load()) |
| 1327 | - | // IMPORTANT: Skip for related views (popups) - they share the web process with their | 1355 | + | if terminate && wv.inner != nil { |
| 1328 | - | // parent. Terminating the shared process would kill the parent WebView too! | |||
| 1329 | - | if !wv.isRelated && wv.inner != nil { | |||
| 1330 | wv.inner.TerminateWebProcess() | 1356 | wv.inner.TerminateWebProcess() | ||
| 1331 | } | 1357 | } | ||
| 1332 | 1358 | ||||
| @@ -1338,10 +1364,86 @@ func (wv *WebView) Destroy() { | |||||
| 1338 | wv.ucm = nil | 1364 | wv.ucm = nil | ||
| 1339 | wv.findController = nil | 1365 | wv.findController = nil | ||
| 1340 | 1366 | ||||
| 1341 | - | if wv.isRelated { | 1367 | + | wv.logger.Debug(). |
| 1342 | - | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("related webview destroyed (process shared with parent)") | 1368 | + | Uint64("id", uint64(wv.id)). |
| 1343 | - | } else { | 1369 | + | Bool("related", wv.isRelated). |
| 1344 | - | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("webview destroyed and web process terminated") | 1370 | + | Bool("navigation_active", wv.navigationActive.Load()). |
| 1371 | + | Bool("terminated_process", terminate). | |||
| 1372 | + | Str("terminate_policy", policy). | |||
| 1373 | + | Msg("webview destroyed") | |||
| 1374 | + | } | |||
| 1375 | + | ||||
| 1376 | + | // ResetForPoolReuse sanitizes callbacks/state so the WebView can be safely reused. | |||
| 1377 | + | func (wv *WebView) ResetForPoolReuse() { | |||
| 1378 | + | if wv == nil || wv.destroyed.Load() { | |||
| 1379 | + | return | |||
| 1380 | + | } | |||
| 1381 | + | ||||
| 1382 | + | wv.OnLoadChanged = nil | |||
| 1383 | + | wv.OnTitleChanged = nil | |||
| 1384 | + | wv.OnURIChanged = nil | |||
| 1385 | + | wv.OnProgressChanged = nil | |||
| 1386 | + | wv.OnFaviconChanged = nil | |||
| 1387 | + | wv.OnClose = nil | |||
| 1388 | + | wv.OnCreate = nil | |||
| 1389 | + | wv.OnReadyToShow = nil | |||
| 1390 | + | wv.OnLinkMiddleClick = nil | |||
| 1391 | + | wv.OnEnterFullscreen = nil | |||
| 1392 | + | wv.OnLeaveFullscreen = nil | |||
| 1393 | + | wv.OnAudioStateChanged = nil | |||
| 1394 | + | wv.OnLinkHover = nil | |||
| 1395 | + | wv.OnWebProcessTerminated = nil | |||
| 1396 | + | ||||
| 1397 | + | wv.mu.Lock() | |||
| 1398 | + | wv.uri = "" | |||
| 1399 | + | wv.title = "" | |||
| 1400 | + | wv.progress = 0 | |||
| 1401 | + | wv.canGoBack = false | |||
| 1402 | + | wv.canGoFwd = false | |||
| 1403 | + | wv.isLoading = false | |||
| 1404 | + | wv.asyncCallbacks = nil | |||
| 1405 | + | wv.mu.Unlock() | |||
| 1406 | + | ||||
| 1407 | + | wv.isFullscreen.Store(false) | |||
| 1408 | + | wv.isPlayingAudio.Store(false) | |||
| 1409 | + | wv.navigationActive.Store(false) | |||
| 1410 | + | ||||
| 1411 | + | if wv.inner != nil { | |||
| 1412 | + | wv.inner.StopLoading() | |||
| 1413 | + | wv.inner.SetVisible(false) | |||
| 1414 | + | } | |||
| 1415 | + | } | |||
| 1416 | + | ||||
| 1417 | + | func resolveTerminatePolicy(raw string) string { | |||
| 1418 | + | value := strings.ToLower(strings.TrimSpace(raw)) | |||
| 1419 | + | if value == "" { | |||
| 1420 | + | value = strings.ToLower(strings.TrimSpace(os.Getenv("DUMBER_WEBVIEW_TERMINATE_POLICY"))) | |||
| 1421 | + | } | |||
| 1422 | + | switch value { | |||
| 1423 | + | case terminatePolicyAlways: | |||
| 1424 | + | return terminatePolicyAlways | |||
| 1425 | + | case terminatePolicyNever: | |||
| 1426 | + | return terminatePolicyNever | |||
| 1427 | + | case terminatePolicyAuto, "": | |||
| 1428 | + | return terminatePolicyAuto | |||
| 1429 | + | default: | |||
| 1430 | + | return terminatePolicyAuto | |||
| 1431 | + | } | |||
| 1432 | + | } | |||
| 1433 | + | ||||
| 1434 | + | func shouldTerminateWebProcess(policy string, isRelated, navigationActive bool) bool { | |||
| 1435 | + | if isRelated { | |||
| 1436 | + | return false | |||
| 1437 | + | } | |||
| 1438 | + | switch resolveTerminatePolicy(policy) { | |||
| 1439 | + | case terminatePolicyNever: | |||
| 1440 | + | return false | |||
| 1441 | + | case terminatePolicyAlways: | |||
| 1442 | + | return true | |||
| 1443 | + | case terminatePolicyAuto: | |||
| 1444 | + | return navigationActive | |||
| 1445 | + | default: | |||
| 1446 | + | return navigationActive | |||
| 1345 | } | 1447 | } | ||
| 1346 | } | 1448 | } | ||
| 1347 | 1449 | ||||
M internal/infrastructure/webkit/webview_termination_test.go +19 -0
| @@ -13,3 +13,22 @@ func TestWebProcessTerminationReasonString(t *testing.T) { | |||
| 13 | 13 | assert.Equal(t, "terminated_by_api", webProcessTerminationReasonString(webkitlib.WebProcessTerminatedByApiValue)) | |
| 14 | 14 | assert.Equal(t, "unknown", webProcessTerminationReasonString(webkitlib.WebProcessTerminationReason(99))) | |
| 15 | 15 | } | |
| 16 | + | ||
| 17 | + | func TestResolveTerminatePolicy(t *testing.T) { | |
| 18 | + | t.Setenv("DUMBER_WEBVIEW_TERMINATE_POLICY", "") | |
| 19 | + | assert.Equal(t, terminatePolicyAuto, resolveTerminatePolicy("")) | |
| 20 | + | assert.Equal(t, terminatePolicyAlways, resolveTerminatePolicy("always")) | |
| 21 | + | assert.Equal(t, terminatePolicyNever, resolveTerminatePolicy("never")) | |
| 22 | + | assert.Equal(t, terminatePolicyAuto, resolveTerminatePolicy("invalid")) | |
| 23 | + | ||
| 24 | + | t.Setenv("DUMBER_WEBVIEW_TERMINATE_POLICY", "always") | |
| 25 | + | assert.Equal(t, terminatePolicyAlways, resolveTerminatePolicy("")) | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func TestShouldTerminateWebProcess(t *testing.T) { | |
| 29 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAlways, true, true)) | |
| 30 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyNever, false, true)) | |
| 31 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAlways, false, false)) | |
| 32 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAuto, false, true)) | |
| 33 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAuto, false, false)) | |
| 34 | + | } | |
| @@ -13,3 +13,22 @@ func TestWebProcessTerminationReasonString(t *testing.T) { | |||||
| 13 | assert.Equal(t, "terminated_by_api", webProcessTerminationReasonString(webkitlib.WebProcessTerminatedByApiValue)) | 13 | assert.Equal(t, "terminated_by_api", webProcessTerminationReasonString(webkitlib.WebProcessTerminatedByApiValue)) | ||
| 14 | assert.Equal(t, "unknown", webProcessTerminationReasonString(webkitlib.WebProcessTerminationReason(99))) | 14 | assert.Equal(t, "unknown", webProcessTerminationReasonString(webkitlib.WebProcessTerminationReason(99))) | ||
| 15 | } | 15 | } | ||
| 16 | + | ||||
| 17 | + | func TestResolveTerminatePolicy(t *testing.T) { | |||
| 18 | + | t.Setenv("DUMBER_WEBVIEW_TERMINATE_POLICY", "") | |||
| 19 | + | assert.Equal(t, terminatePolicyAuto, resolveTerminatePolicy("")) | |||
| 20 | + | assert.Equal(t, terminatePolicyAlways, resolveTerminatePolicy("always")) | |||
| 21 | + | assert.Equal(t, terminatePolicyNever, resolveTerminatePolicy("never")) | |||
| 22 | + | assert.Equal(t, terminatePolicyAuto, resolveTerminatePolicy("invalid")) | |||
| 23 | + | ||||
| 24 | + | t.Setenv("DUMBER_WEBVIEW_TERMINATE_POLICY", "always") | |||
| 25 | + | assert.Equal(t, terminatePolicyAlways, resolveTerminatePolicy("")) | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | func TestShouldTerminateWebProcess(t *testing.T) { | |||
| 29 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAlways, true, true)) | |||
| 30 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyNever, false, true)) | |||
| 31 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAlways, false, false)) | |||
| 32 | + | assert.True(t, shouldTerminateWebProcess(terminatePolicyAuto, false, true)) | |||
| 33 | + | assert.False(t, shouldTerminateWebProcess(terminatePolicyAuto, false, false)) | |||
| 34 | + | } | |||
M internal/ui/coordinator/workspace.go +156 -104
| @@ -64,6 +64,16 @@ type stackPaneContext struct { | |||
| 64 | 64 | originalTitle string | |
| 65 | 65 | } | |
| 66 | 66 | ||
| 67 | + | type incrementalCloseContext struct { | |
| 68 | + | parentNode *entity.PaneNode | |
| 69 | + | siblingNode *entity.PaneNode | |
| 70 | + | grandparentNode *entity.PaneNode | |
| 71 | + | parentWidget layout.Widget | |
| 72 | + | siblingIsStartChild bool | |
| 73 | + | parentIsStartInGrand bool | |
| 74 | + | precheckReason string | |
| 75 | + | } | |
| 76 | + | ||
| 67 | 77 | // NewWorkspaceCoordinator creates a new WorkspaceCoordinator. | |
| 68 | 78 | func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig) *WorkspaceCoordinator { | |
| 69 | 79 | log := logging.FromContext(ctx) | |
| @@ -717,52 +727,8 @@ func (c *WorkspaceCoordinator) ClosePane(ctx context.Context) error { | |||
| 717 | 727 | return nil | |
| 718 | 728 | } | |
| 719 | 729 | ||
| 720 | - | // BEFORE domain changes: capture widget info for incremental close | |
| 721 | - | var parentNode *entity.PaneNode | |
| 722 | - | var siblingNode *entity.PaneNode | |
| 723 | - | var grandparentNode *entity.PaneNode | |
| 724 | - | var parentWidget layout.Widget | |
| 725 | - | var siblingIsStartChild bool // true if sibling is the start (left/top) child in parent | |
| 726 | - | var parentIsStartInGrand bool // true if parent is the start (left/top) child in grandparent | |
| 727 | - | ||
| 728 | - | if wsView != nil { | |
| 729 | - | tr := wsView.TreeRenderer() | |
| 730 | - | if tr != nil && activePane.Parent != nil { | |
| 731 | - | parentNode = activePane.Parent | |
| 732 | - | ||
| 733 | - | // Find sibling (the other child of parent) | |
| 734 | - | // If closing pane is start child, sibling is end child (and vice versa) | |
| 735 | - | if parentNode.Left() == activePane { | |
| 736 | - | siblingNode = parentNode.Right() | |
| 737 | - | siblingIsStartChild = false // sibling is end/right child | |
| 738 | - | } else { | |
| 739 | - | siblingNode = parentNode.Left() | |
| 740 | - | siblingIsStartChild = true // sibling is start/left child | |
| 741 | - | } | |
| 742 | - | ||
| 743 | - | grandparentNode = parentNode.Parent | |
| 744 | - | parentWidget = tr.Lookup(parentNode.ID) | |
| 745 | - | ||
| 746 | - | // Determine parent's position in grandparent (if grandparent exists) | |
| 747 | - | if grandparentNode != nil { | |
| 748 | - | parentIsStartInGrand = grandparentNode.Left() == parentNode | |
| 749 | - | } | |
| 750 | - | ||
| 751 | - | log.Debug(). | |
| 752 | - | Str("parent_id", parentNode.ID). | |
| 753 | - | Str("sibling_id", func() string { | |
| 754 | - | if siblingNode != nil { | |
| 755 | - | return siblingNode.ID | |
| 756 | - | } | |
| 757 | - | return nilString | |
| 758 | - | }()). | |
| 759 | - | Bool("sibling_is_start", siblingIsStartChild). | |
| 760 | - | Bool("parent_is_start_in_grand", parentIsStartInGrand). | |
| 761 | - | Bool("has_grandparent", grandparentNode != nil). | |
| 762 | - | Bool("has_parent_widget", parentWidget != nil). | |
| 763 | - | Msg("captured close context") | |
| 764 | - | } | |
| 765 | - | } | |
| 730 | + | // BEFORE domain changes: capture incremental close context. | |
| 731 | + | closeCtx := c.captureIncrementalCloseContext(wsView, activePane) | |
| 766 | 732 | ||
| 767 | 733 | // Now do domain changes | |
| 768 | 734 | _, err := c.panesUC.Close(ctx, ws, activePane) | |
| @@ -776,12 +742,7 @@ func (c *WorkspaceCoordinator) ClosePane(ctx context.Context) error { | |||
| 776 | 742 | wsView, | |
| 777 | 743 | ws, | |
| 778 | 744 | closingPaneID, | |
| 779 | - | parentNode, | |
| 780 | - | siblingNode, | |
| 781 | - | grandparentNode, | |
| 782 | - | parentWidget, | |
| 783 | - | siblingIsStartChild, | |
| 784 | - | parentIsStartInGrand, | |
| 745 | + | closeCtx, | |
| 785 | 746 | ) | |
| 786 | 747 | ||
| 787 | 748 | // Notify state change for session snapshots | |
| @@ -824,35 +785,8 @@ func (c *WorkspaceCoordinator) ClosePaneByID(ctx context.Context, paneID entity. | |||
| 824 | 785 | return nil | |
| 825 | 786 | } | |
| 826 | 787 | ||
| 827 | - | // BEFORE domain changes: capture widget info for incremental close | |
| 828 | - | var parentNode *entity.PaneNode | |
| 829 | - | var siblingNode *entity.PaneNode | |
| 830 | - | var grandparentNode *entity.PaneNode | |
| 831 | - | var parentWidget layout.Widget | |
| 832 | - | var siblingIsStartChild bool | |
| 833 | - | var parentIsStartInGrand bool | |
| 834 | - | ||
| 835 | - | if wsView != nil { | |
| 836 | - | tr := wsView.TreeRenderer() | |
| 837 | - | if tr != nil && paneNode.Parent != nil { | |
| 838 | - | parentNode = paneNode.Parent | |
| 839 | - | ||
| 840 | - | if parentNode.Left() == paneNode { | |
| 841 | - | siblingNode = parentNode.Right() | |
| 842 | - | siblingIsStartChild = false | |
| 843 | - | } else { | |
| 844 | - | siblingNode = parentNode.Left() | |
| 845 | - | siblingIsStartChild = true | |
| 846 | - | } | |
| 847 | - | ||
| 848 | - | grandparentNode = parentNode.Parent | |
| 849 | - | parentWidget = tr.Lookup(parentNode.ID) | |
| 850 | - | ||
| 851 | - | if grandparentNode != nil { | |
| 852 | - | parentIsStartInGrand = grandparentNode.Left() == parentNode | |
| 853 | - | } | |
| 854 | - | } | |
| 855 | - | } | |
| 788 | + | // BEFORE domain changes: capture incremental close context. | |
| 789 | + | closeCtx := c.captureIncrementalCloseContext(wsView, paneNode) | |
| 856 | 790 | ||
| 857 | 791 | // Now do domain changes | |
| 858 | 792 | _, err := c.panesUC.Close(ctx, ws, paneNode) | |
| @@ -866,12 +800,7 @@ func (c *WorkspaceCoordinator) ClosePaneByID(ctx context.Context, paneID entity. | |||
| 866 | 800 | wsView, | |
| 867 | 801 | ws, | |
| 868 | 802 | paneID, | |
| 869 | - | parentNode, | |
| 870 | - | siblingNode, | |
| 871 | - | grandparentNode, | |
| 872 | - | parentWidget, | |
| 873 | - | siblingIsStartChild, | |
| 874 | - | parentIsStartInGrand, | |
| 803 | + | closeCtx, | |
| 875 | 804 | ) | |
| 876 | 805 | ||
| 877 | 806 | // Notify state change for session snapshots | |
| @@ -894,6 +823,16 @@ func (c *WorkspaceCoordinator) doIncrementalClose( | |||
| 894 | 823 | parentIsStartInGrand bool, // true if parent is start/left child in grandparent | |
| 895 | 824 | ) error { | |
| 896 | 825 | log := logging.FromContext(ctx) | |
| 826 | + | if parentNode == nil { | |
| 827 | + | return fmt.Errorf("parent node missing") | |
| 828 | + | } | |
| 829 | + | if siblingNode == nil { | |
| 830 | + | return fmt.Errorf("sibling node missing") | |
| 831 | + | } | |
| 832 | + | if parentWidget == nil { | |
| 833 | + | return fmt.Errorf("parent widget missing") | |
| 834 | + | } | |
| 835 | + | ||
| 897 | 836 | tr := wsView.TreeRenderer() | |
| 898 | 837 | if tr == nil { | |
| 899 | 838 | return fmt.Errorf("tree renderer not available") | |
| @@ -1015,12 +954,7 @@ func (c *WorkspaceCoordinator) finalizePaneClose( | |||
| 1015 | 954 | wsView *component.WorkspaceView, | |
| 1016 | 955 | ws *entity.Workspace, | |
| 1017 | 956 | paneID entity.PaneID, | |
| 1018 | - | parentNode *entity.PaneNode, | |
| 1019 | - | siblingNode *entity.PaneNode, | |
| 1020 | - | grandparentNode *entity.PaneNode, | |
| 1021 | - | parentWidget layout.Widget, | |
| 1022 | - | siblingIsStartChild bool, | |
| 1023 | - | parentIsStartInGrand bool, | |
| 957 | + | closeCtx incrementalCloseContext, | |
| 1024 | 958 | ) { | |
| 1025 | 959 | if wsView == nil { | |
| 1026 | 960 | return | |
| @@ -1028,29 +962,40 @@ func (c *WorkspaceCoordinator) finalizePaneClose( | |||
| 1028 | 962 | ||
| 1029 | 963 | log := logging.FromContext(ctx) | |
| 1030 | 964 | ||
| 1031 | - | if parentNode != nil { | |
| 965 | + | if closeCtx.parentNode != nil { | |
| 1032 | 966 | var closeErr error | |
| 1033 | 967 | ||
| 1034 | - | if parentNode.IsStacked { | |
| 1035 | - | closeErr = c.doIncrementalStackClose(ctx, wsView, paneID, parentNode) | |
| 1036 | - | } else if siblingNode != nil && parentWidget != nil { | |
| 968 | + | if closeCtx.parentNode.IsStacked { | |
| 969 | + | closeErr = c.doIncrementalStackClose(ctx, wsView, paneID, closeCtx.parentNode) | |
| 970 | + | } else if closeCtx.precheckReason != "" { | |
| 971 | + | closeErr = fmt.Errorf("incremental close precheck failed: %s", closeCtx.precheckReason) | |
| 972 | + | } else if closeCtx.siblingNode != nil && closeCtx.parentWidget != nil { | |
| 1037 | 973 | closeErr = c.doIncrementalClose( | |
| 1038 | 974 | ctx, | |
| 1039 | 975 | wsView, | |
| 1040 | 976 | paneID, | |
| 1041 | - | siblingNode, | |
| 1042 | - | parentNode, | |
| 1043 | - | grandparentNode, | |
| 1044 | - | parentWidget, | |
| 1045 | - | siblingIsStartChild, | |
| 1046 | - | parentIsStartInGrand, | |
| 977 | + | closeCtx.siblingNode, | |
| 978 | + | closeCtx.parentNode, | |
| 979 | + | closeCtx.grandparentNode, | |
| 980 | + | closeCtx.parentWidget, | |
| 981 | + | closeCtx.siblingIsStartChild, | |
| 982 | + | closeCtx.parentIsStartInGrand, | |
| 1047 | 983 | ) | |
| 1048 | 984 | } else { | |
| 1049 | 985 | closeErr = fmt.Errorf("missing context for incremental close") | |
| 1050 | 986 | } | |
| 1051 | 987 | ||
| 1052 | 988 | if closeErr != nil { | |
| 1053 | - | log.Warn().Err(closeErr).Msg("incremental close failed, falling back to rebuild") | |
| 989 | + | fallbackLog := log.Warn().Err(closeErr). | |
| 990 | + | Str("pane_id", string(paneID)). | |
| 991 | + | Str("parent_id", paneNodeID(closeCtx.parentNode)). | |
| 992 | + | Str("sibling_id", paneNodeID(closeCtx.siblingNode)). | |
| 993 | + | Str("grandparent_id", paneNodeID(closeCtx.grandparentNode)). | |
| 994 | + | Bool("has_parent_widget", closeCtx.parentWidget != nil) | |
| 995 | + | if closeCtx.precheckReason != "" { | |
| 996 | + | fallbackLog = fallbackLog.Str("precheck_reason", closeCtx.precheckReason) | |
| 997 | + | } | |
| 998 | + | fallbackLog.Msg("incremental close failed, falling back to rebuild") | |
| 1054 | 999 | if err := wsView.Rebuild(ctx); err != nil { | |
| 1055 | 1000 | log.Error().Err(err).Msg("failed to rebuild workspace view") | |
| 1056 | 1001 | } | |
| @@ -1073,6 +1018,113 @@ func (c *WorkspaceCoordinator) finalizePaneClose( | |||
| 1073 | 1018 | wsView.FocusPane(ws.ActivePaneID) | |
| 1074 | 1019 | } | |
| 1075 | 1020 | ||
| 1021 | + | func (c *WorkspaceCoordinator) captureIncrementalCloseContext( | |
| 1022 | + | wsView *component.WorkspaceView, | |
| 1023 | + | closingPane *entity.PaneNode, | |
| 1024 | + | ) incrementalCloseContext { | |
| 1025 | + | closeCtx, err := deriveIncrementalCloseTreeContext(closingPane) | |
| 1026 | + | if err != nil { | |
| 1027 | + | closeCtx.precheckReason = err.Error() | |
| 1028 | + | return closeCtx | |
| 1029 | + | } | |
| 1030 | + | ||
| 1031 | + | if closeCtx.parentNode == nil || closeCtx.parentNode.IsStacked { | |
| 1032 | + | return closeCtx | |
| 1033 | + | } | |
| 1034 | + | ||
| 1035 | + | if wsView == nil { | |
| 1036 | + | closeCtx.precheckReason = "workspace view unavailable" | |
| 1037 | + | return closeCtx | |
| 1038 | + | } | |
| 1039 | + | ||
| 1040 | + | tr := wsView.TreeRenderer() | |
| 1041 | + | if tr == nil { | |
| 1042 | + | closeCtx.precheckReason = "tree renderer unavailable" | |
| 1043 | + | return closeCtx | |
| 1044 | + | } | |
| 1045 | + | ||
| 1046 | + | closeCtx.parentWidget = tr.Lookup(closeCtx.parentNode.ID) | |
| 1047 | + | if closeCtx.parentWidget == nil { | |
| 1048 | + | closeCtx.precheckReason = "parent widget missing" | |
| 1049 | + | } | |
| 1050 | + | ||
| 1051 | + | return closeCtx | |
| 1052 | + | } | |
| 1053 | + | ||
| 1054 | + | func deriveIncrementalCloseTreeContext(closingPane *entity.PaneNode) (incrementalCloseContext, error) { | |
| 1055 | + | var closeCtx incrementalCloseContext | |
| 1056 | + | if closingPane == nil { | |
| 1057 | + | return closeCtx, errors.New("closing pane missing") | |
| 1058 | + | } | |
| 1059 | + | ||
| 1060 | + | parentNode := closingPane.Parent | |
| 1061 | + | if parentNode == nil { | |
| 1062 | + | return closeCtx, errors.New("closing pane has no parent") | |
| 1063 | + | } | |
| 1064 | + | ||
| 1065 | + | closeCtx.parentNode = parentNode | |
| 1066 | + | closeCtx.grandparentNode = parentNode.Parent | |
| 1067 | + | ||
| 1068 | + | if parentNode.IsStacked { | |
| 1069 | + | return closeCtx, nil | |
| 1070 | + | } | |
| 1071 | + | ||
| 1072 | + | if !parentNode.IsSplit() { | |
| 1073 | + | return closeCtx, fmt.Errorf("parent node is not split: %s", parentNode.ID) | |
| 1074 | + | } | |
| 1075 | + | ||
| 1076 | + | if len(parentNode.Children) != 2 { | |
| 1077 | + | return closeCtx, fmt.Errorf("split parent has invalid child count: %d", len(parentNode.Children)) | |
| 1078 | + | } | |
| 1079 | + | ||
| 1080 | + | leftChild := parentNode.Left() | |
| 1081 | + | rightChild := parentNode.Right() | |
| 1082 | + | if leftChild == closingPane && rightChild == nil { | |
| 1083 | + | return closeCtx, errors.New("sibling missing") | |
| 1084 | + | } | |
| 1085 | + | if rightChild == closingPane && leftChild == nil { | |
| 1086 | + | return closeCtx, errors.New("sibling missing") | |
| 1087 | + | } | |
| 1088 | + | if leftChild == nil || rightChild == nil { | |
| 1089 | + | return closeCtx, errors.New("split parent has nil child") | |
| 1090 | + | } | |
| 1091 | + | ||
| 1092 | + | switch { | |
| 1093 | + | case leftChild == closingPane: | |
| 1094 | + | closeCtx.siblingNode = rightChild | |
| 1095 | + | closeCtx.siblingIsStartChild = false | |
| 1096 | + | case rightChild == closingPane: | |
| 1097 | + | closeCtx.siblingNode = leftChild | |
| 1098 | + | closeCtx.siblingIsStartChild = true | |
| 1099 | + | default: | |
| 1100 | + | return closeCtx, fmt.Errorf("closing pane not found under parent: %s", parentNode.ID) | |
| 1101 | + | } | |
| 1102 | + | ||
| 1103 | + | if closeCtx.siblingNode == nil { | |
| 1104 | + | return closeCtx, errors.New("sibling missing") | |
| 1105 | + | } | |
| 1106 | + | ||
| 1107 | + | if closeCtx.grandparentNode != nil { | |
| 1108 | + | switch { | |
| 1109 | + | case closeCtx.grandparentNode.Left() == parentNode: | |
| 1110 | + | closeCtx.parentIsStartInGrand = true | |
| 1111 | + | case closeCtx.grandparentNode.Right() == parentNode: | |
| 1112 | + | closeCtx.parentIsStartInGrand = false | |
| 1113 | + | default: | |
| 1114 | + | return closeCtx, fmt.Errorf("parent not found under grandparent: %s", closeCtx.grandparentNode.ID) | |
| 1115 | + | } | |
| 1116 | + | } | |
| 1117 | + | ||
| 1118 | + | return closeCtx, nil | |
| 1119 | + | } | |
| 1120 | + | ||
| 1121 | + | func paneNodeID(node *entity.PaneNode) string { | |
| 1122 | + | if node == nil { | |
| 1123 | + | return nilString | |
| 1124 | + | } | |
| 1125 | + | return node.ID | |
| 1126 | + | } | |
| 1127 | + | ||
| 1076 | 1128 | // doIncrementalStackClose removes a pane from a stack without rebuilding the entire tree. | |
| 1077 | 1129 | func (c *WorkspaceCoordinator) doIncrementalStackClose( | |
| 1078 | 1130 | ctx context.Context, | |
| @@ -64,6 +64,16 @@ type stackPaneContext struct { | |||||
| 64 | originalTitle string | 64 | originalTitle string | ||
| 65 | } | 65 | } | ||
| 66 | 66 | ||||
| 67 | + | type incrementalCloseContext struct { | |||
| 68 | + | parentNode *entity.PaneNode | |||
| 69 | + | siblingNode *entity.PaneNode | |||
| 70 | + | grandparentNode *entity.PaneNode | |||
| 71 | + | parentWidget layout.Widget | |||
| 72 | + | siblingIsStartChild bool | |||
| 73 | + | parentIsStartInGrand bool | |||
| 74 | + | precheckReason string | |||
| 75 | + | } | |||
| 76 | + | ||||
| 67 | // NewWorkspaceCoordinator creates a new WorkspaceCoordinator. | 77 | // NewWorkspaceCoordinator creates a new WorkspaceCoordinator. | ||
| 68 | func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig) *WorkspaceCoordinator { | 78 | func NewWorkspaceCoordinator(ctx context.Context, cfg WorkspaceCoordinatorConfig) *WorkspaceCoordinator { | ||
| 69 | log := logging.FromContext(ctx) | 79 | log := logging.FromContext(ctx) | ||
| @@ -717,52 +727,8 @@ func (c *WorkspaceCoordinator) ClosePane(ctx context.Context) error { | |||||
| 717 | return nil | 727 | return nil | ||
| 718 | } | 728 | } | ||
| 719 | 729 | ||||
| 720 | - | // BEFORE domain changes: capture widget info for incremental close | 730 | + | // BEFORE domain changes: capture incremental close context. |
| 721 | - | var parentNode *entity.PaneNode | 731 | + | closeCtx := c.captureIncrementalCloseContext(wsView, activePane) |
| 722 | - | var siblingNode *entity.PaneNode | |||
| 723 | - | var grandparentNode *entity.PaneNode | |||
| 724 | - | var parentWidget layout.Widget | |||
| 725 | - | var siblingIsStartChild bool // true if sibling is the start (left/top) child in parent | |||
| 726 | - | var parentIsStartInGrand bool // true if parent is the start (left/top) child in grandparent | |||
| 727 | - | ||||
| 728 | - | if wsView != nil { | |||
| 729 | - | tr := wsView.TreeRenderer() | |||
| 730 | - | if tr != nil && activePane.Parent != nil { | |||
| 731 | - | parentNode = activePane.Parent | |||
| 732 | - | ||||
| 733 | - | // Find sibling (the other child of parent) | |||
| 734 | - | // If closing pane is start child, sibling is end child (and vice versa) | |||
| 735 | - | if parentNode.Left() == activePane { | |||
| 736 | - | siblingNode = parentNode.Right() | |||
| 737 | - | siblingIsStartChild = false // sibling is end/right child | |||
| 738 | - | } else { | |||
| 739 | - | siblingNode = parentNode.Left() | |||
| 740 | - | siblingIsStartChild = true // sibling is start/left child | |||
| 741 | - | } | |||
| 742 | - | ||||
| 743 | - | grandparentNode = parentNode.Parent | |||
| 744 | - | parentWidget = tr.Lookup(parentNode.ID) | |||
| 745 | - | ||||
| 746 | - | // Determine parent's position in grandparent (if grandparent exists) | |||
| 747 | - | if grandparentNode != nil { | |||
| 748 | - | parentIsStartInGrand = grandparentNode.Left() == parentNode | |||
| 749 | - | } | |||
| 750 | - | ||||
| 751 | - | log.Debug(). | |||
| 752 | - | Str("parent_id", parentNode.ID). | |||
| 753 | - | Str("sibling_id", func() string { | |||
| 754 | - | if siblingNode != nil { | |||
| 755 | - | return siblingNode.ID | |||
| 756 | - | } | |||
| 757 | - | return nilString | |||
| 758 | - | }()). | |||
| 759 | - | Bool("sibling_is_start", siblingIsStartChild). | |||
| 760 | - | Bool("parent_is_start_in_grand", parentIsStartInGrand). | |||
| 761 | - | Bool("has_grandparent", grandparentNode != nil). | |||
| 762 | - | Bool("has_parent_widget", parentWidget != nil). | |||
| 763 | - | Msg("captured close context") | |||
| 764 | - | } | |||
| 765 | - | } | |||
| 766 | 732 | ||||
| 767 | // Now do domain changes | 733 | // Now do domain changes | ||
| 768 | _, err := c.panesUC.Close(ctx, ws, activePane) | 734 | _, err := c.panesUC.Close(ctx, ws, activePane) | ||
| @@ -776,12 +742,7 @@ func (c *WorkspaceCoordinator) ClosePane(ctx context.Context) error { | |||||
| 776 | wsView, | 742 | wsView, | ||
| 777 | ws, | 743 | ws, | ||
| 778 | closingPaneID, | 744 | closingPaneID, | ||
| 779 | - | parentNode, | 745 | + | closeCtx, |
| 780 | - | siblingNode, | |||
| 781 | - | grandparentNode, | |||
| 782 | - | parentWidget, | |||
| 783 | - | siblingIsStartChild, | |||
| 784 | - | parentIsStartInGrand, | |||
| 785 | ) | 746 | ) | ||
| 786 | 747 | ||||
| 787 | // Notify state change for session snapshots | 748 | // Notify state change for session snapshots | ||
| @@ -824,35 +785,8 @@ func (c *WorkspaceCoordinator) ClosePaneByID(ctx context.Context, paneID entity. | |||||
| 824 | return nil | 785 | return nil | ||
| 825 | } | 786 | } | ||
| 826 | 787 | ||||
| 827 | - | // BEFORE domain changes: capture widget info for incremental close | 788 | + | // BEFORE domain changes: capture incremental close context. |
| 828 | - | var parentNode *entity.PaneNode | 789 | + | closeCtx := c.captureIncrementalCloseContext(wsView, paneNode) |
| 829 | - | var siblingNode *entity.PaneNode | |||
| 830 | - | var grandparentNode *entity.PaneNode | |||
| 831 | - | var parentWidget layout.Widget | |||
| 832 | - | var siblingIsStartChild bool | |||
| 833 | - | var parentIsStartInGrand bool | |||
| 834 | - | ||||
| 835 | - | if wsView != nil { | |||
| 836 | - | tr := wsView.TreeRenderer() | |||
| 837 | - | if tr != nil && paneNode.Parent != nil { | |||
| 838 | - | parentNode = paneNode.Parent | |||
| 839 | - | ||||
| 840 | - | if parentNode.Left() == paneNode { | |||
| 841 | - | siblingNode = parentNode.Right() | |||
| 842 | - | siblingIsStartChild = false | |||
| 843 | - | } else { | |||
| 844 | - | siblingNode = parentNode.Left() | |||
| 845 | - | siblingIsStartChild = true | |||
| 846 | - | } | |||
| 847 | - | ||||
| 848 | - | grandparentNode = parentNode.Parent | |||
| 849 | - | parentWidget = tr.Lookup(parentNode.ID) | |||
| 850 | - | ||||
| 851 | - | if grandparentNode != nil { | |||
| 852 | - | parentIsStartInGrand = grandparentNode.Left() == parentNode | |||
| 853 | - | } | |||
| 854 | - | } | |||
| 855 | - | } | |||
| 856 | 790 | ||||
| 857 | // Now do domain changes | 791 | // Now do domain changes | ||
| 858 | _, err := c.panesUC.Close(ctx, ws, paneNode) | 792 | _, err := c.panesUC.Close(ctx, ws, paneNode) | ||
| @@ -866,12 +800,7 @@ func (c *WorkspaceCoordinator) ClosePaneByID(ctx context.Context, paneID entity. | |||||
| 866 | wsView, | 800 | wsView, | ||
| 867 | ws, | 801 | ws, | ||
| 868 | paneID, | 802 | paneID, | ||
| 869 | - | parentNode, | 803 | + | closeCtx, |
| 870 | - | siblingNode, | |||
| 871 | - | grandparentNode, | |||
| 872 | - | parentWidget, | |||
| 873 | - | siblingIsStartChild, | |||
| 874 | - | parentIsStartInGrand, | |||
| 875 | ) | 804 | ) | ||
| 876 | 805 | ||||
| 877 | // Notify state change for session snapshots | 806 | // Notify state change for session snapshots | ||
| @@ -894,6 +823,16 @@ func (c *WorkspaceCoordinator) doIncrementalClose( | |||||
| 894 | parentIsStartInGrand bool, // true if parent is start/left child in grandparent | 823 | parentIsStartInGrand bool, // true if parent is start/left child in grandparent | ||
| 895 | ) error { | 824 | ) error { | ||
| 896 | log := logging.FromContext(ctx) | 825 | log := logging.FromContext(ctx) | ||
| 826 | + | if parentNode == nil { | |||
| 827 | + | return fmt.Errorf("parent node missing") | |||
| 828 | + | } | |||
| 829 | + | if siblingNode == nil { | |||
| 830 | + | return fmt.Errorf("sibling node missing") | |||
| 831 | + | } | |||
| 832 | + | if parentWidget == nil { | |||
| 833 | + | return fmt.Errorf("parent widget missing") | |||
| 834 | + | } | |||
| 835 | + | ||||
| 897 | tr := wsView.TreeRenderer() | 836 | tr := wsView.TreeRenderer() | ||
| 898 | if tr == nil { | 837 | if tr == nil { | ||
| 899 | return fmt.Errorf("tree renderer not available") | 838 | return fmt.Errorf("tree renderer not available") | ||
| @@ -1015,12 +954,7 @@ func (c *WorkspaceCoordinator) finalizePaneClose( | |||||
| 1015 | wsView *component.WorkspaceView, | 954 | wsView *component.WorkspaceView, | ||
| 1016 | ws *entity.Workspace, | 955 | ws *entity.Workspace, | ||
| 1017 | paneID entity.PaneID, | 956 | paneID entity.PaneID, | ||
| 1018 | - | parentNode *entity.PaneNode, | 957 | + | closeCtx incrementalCloseContext, |
| 1019 | - | siblingNode *entity.PaneNode, | |||
| 1020 | - | grandparentNode *entity.PaneNode, | |||
| 1021 | - | parentWidget layout.Widget, | |||
| 1022 | - | siblingIsStartChild bool, | |||
| 1023 | - | parentIsStartInGrand bool, | |||
| 1024 | ) { | 958 | ) { | ||
| 1025 | if wsView == nil { | 959 | if wsView == nil { | ||
| 1026 | return | 960 | return | ||
| @@ -1028,29 +962,40 @@ func (c *WorkspaceCoordinator) finalizePaneClose( | |||||
| 1028 | 962 | ||||
| 1029 | log := logging.FromContext(ctx) | 963 | log := logging.FromContext(ctx) | ||
| 1030 | 964 | ||||
| 1031 | - | if parentNode != nil { | 965 | + | if closeCtx.parentNode != nil { |
| 1032 | var closeErr error | 966 | var closeErr error | ||
| 1033 | 967 | ||||
| 1034 | - | if parentNode.IsStacked { | 968 | + | if closeCtx.parentNode.IsStacked { |
| 1035 | - | closeErr = c.doIncrementalStackClose(ctx, wsView, paneID, parentNode) | 969 | + | closeErr = c.doIncrementalStackClose(ctx, wsView, paneID, closeCtx.parentNode) |
| 1036 | - | } else if siblingNode != nil && parentWidget != nil { | 970 | + | } else if closeCtx.precheckReason != "" { |
| 971 | + | closeErr = fmt.Errorf("incremental close precheck failed: %s", closeCtx.precheckReason) | |||
| 972 | + | } else if closeCtx.siblingNode != nil && closeCtx.parentWidget != nil { | |||
| 1037 | closeErr = c.doIncrementalClose( | 973 | closeErr = c.doIncrementalClose( | ||
| 1038 | ctx, | 974 | ctx, | ||
| 1039 | wsView, | 975 | wsView, | ||
| 1040 | paneID, | 976 | paneID, | ||
| 1041 | - | siblingNode, | 977 | + | closeCtx.siblingNode, |
| 1042 | - | parentNode, | 978 | + | closeCtx.parentNode, |
| 1043 | - | grandparentNode, | 979 | + | closeCtx.grandparentNode, |
| 1044 | - | parentWidget, | 980 | + | closeCtx.parentWidget, |
| 1045 | - | siblingIsStartChild, | 981 | + | closeCtx.siblingIsStartChild, |
| 1046 | - | parentIsStartInGrand, | 982 | + | closeCtx.parentIsStartInGrand, |
| 1047 | ) | 983 | ) | ||
| 1048 | } else { | 984 | } else { | ||
| 1049 | closeErr = fmt.Errorf("missing context for incremental close") | 985 | closeErr = fmt.Errorf("missing context for incremental close") | ||
| 1050 | } | 986 | } | ||
| 1051 | 987 | ||||
| 1052 | if closeErr != nil { | 988 | if closeErr != nil { | ||
| 1053 | - | log.Warn().Err(closeErr).Msg("incremental close failed, falling back to rebuild") | 989 | + | fallbackLog := log.Warn().Err(closeErr). |
| 990 | + | Str("pane_id", string(paneID)). | |||
| 991 | + | Str("parent_id", paneNodeID(closeCtx.parentNode)). | |||
| 992 | + | Str("sibling_id", paneNodeID(closeCtx.siblingNode)). | |||
| 993 | + | Str("grandparent_id", paneNodeID(closeCtx.grandparentNode)). | |||
| 994 | + | Bool("has_parent_widget", closeCtx.parentWidget != nil) | |||
| 995 | + | if closeCtx.precheckReason != "" { | |||
| 996 | + | fallbackLog = fallbackLog.Str("precheck_reason", closeCtx.precheckReason) | |||
| 997 | + | } | |||
| 998 | + | fallbackLog.Msg("incremental close failed, falling back to rebuild") | |||
| 1054 | if err := wsView.Rebuild(ctx); err != nil { | 999 | if err := wsView.Rebuild(ctx); err != nil { | ||
| 1055 | log.Error().Err(err).Msg("failed to rebuild workspace view") | 1000 | log.Error().Err(err).Msg("failed to rebuild workspace view") | ||
| 1056 | } | 1001 | } | ||
| @@ -1073,6 +1018,113 @@ func (c *WorkspaceCoordinator) finalizePaneClose( | |||||
| 1073 | wsView.FocusPane(ws.ActivePaneID) | 1018 | wsView.FocusPane(ws.ActivePaneID) | ||
| 1074 | } | 1019 | } | ||
| 1075 | 1020 | ||||
| 1021 | + | func (c *WorkspaceCoordinator) captureIncrementalCloseContext( | |||
| 1022 | + | wsView *component.WorkspaceView, | |||
| 1023 | + | closingPane *entity.PaneNode, | |||
| 1024 | + | ) incrementalCloseContext { | |||
| 1025 | + | closeCtx, err := deriveIncrementalCloseTreeContext(closingPane) | |||
| 1026 | + | if err != nil { | |||
| 1027 | + | closeCtx.precheckReason = err.Error() | |||
| 1028 | + | return closeCtx | |||
| 1029 | + | } | |||
| 1030 | + | ||||
| 1031 | + | if closeCtx.parentNode == nil || closeCtx.parentNode.IsStacked { | |||
| 1032 | + | return closeCtx | |||
| 1033 | + | } | |||
| 1034 | + | ||||
| 1035 | + | if wsView == nil { | |||
| 1036 | + | closeCtx.precheckReason = "workspace view unavailable" | |||
| 1037 | + | return closeCtx | |||
| 1038 | + | } | |||
| 1039 | + | ||||
| 1040 | + | tr := wsView.TreeRenderer() | |||
| 1041 | + | if tr == nil { | |||
| 1042 | + | closeCtx.precheckReason = "tree renderer unavailable" | |||
| 1043 | + | return closeCtx | |||
| 1044 | + | } | |||
| 1045 | + | ||||
| 1046 | + | closeCtx.parentWidget = tr.Lookup(closeCtx.parentNode.ID) | |||
| 1047 | + | if closeCtx.parentWidget == nil { | |||
| 1048 | + | closeCtx.precheckReason = "parent widget missing" | |||
| 1049 | + | } | |||
| 1050 | + | ||||
| 1051 | + | return closeCtx | |||
| 1052 | + | } | |||
| 1053 | + | ||||
| 1054 | + | func deriveIncrementalCloseTreeContext(closingPane *entity.PaneNode) (incrementalCloseContext, error) { | |||
| 1055 | + | var closeCtx incrementalCloseContext | |||
| 1056 | + | if closingPane == nil { | |||
| 1057 | + | return closeCtx, errors.New("closing pane missing") | |||
| 1058 | + | } | |||
| 1059 | + | ||||
| 1060 | + | parentNode := closingPane.Parent | |||
| 1061 | + | if parentNode == nil { | |||
| 1062 | + | return closeCtx, errors.New("closing pane has no parent") | |||
| 1063 | + | } | |||
| 1064 | + | ||||
| 1065 | + | closeCtx.parentNode = parentNode | |||
| 1066 | + | closeCtx.grandparentNode = parentNode.Parent | |||
| 1067 | + | ||||
| 1068 | + | if parentNode.IsStacked { | |||
| 1069 | + | return closeCtx, nil | |||
| 1070 | + | } | |||
| 1071 | + | ||||
| 1072 | + | if !parentNode.IsSplit() { | |||
| 1073 | + | return closeCtx, fmt.Errorf("parent node is not split: %s", parentNode.ID) | |||
| 1074 | + | } | |||
| 1075 | + | ||||
| 1076 | + | if len(parentNode.Children) != 2 { | |||
| 1077 | + | return closeCtx, fmt.Errorf("split parent has invalid child count: %d", len(parentNode.Children)) | |||
| 1078 | + | } | |||
| 1079 | + | ||||
| 1080 | + | leftChild := parentNode.Left() | |||
| 1081 | + | rightChild := parentNode.Right() | |||
| 1082 | + | if leftChild == closingPane && rightChild == nil { | |||
| 1083 | + | return closeCtx, errors.New("sibling missing") | |||
| 1084 | + | } | |||
| 1085 | + | if rightChild == closingPane && leftChild == nil { | |||
| 1086 | + | return closeCtx, errors.New("sibling missing") | |||
| 1087 | + | } | |||
| 1088 | + | if leftChild == nil || rightChild == nil { | |||
| 1089 | + | return closeCtx, errors.New("split parent has nil child") | |||
| 1090 | + | } | |||
| 1091 | + | ||||
| 1092 | + | switch { | |||
| 1093 | + | case leftChild == closingPane: | |||
| 1094 | + | closeCtx.siblingNode = rightChild | |||
| 1095 | + | closeCtx.siblingIsStartChild = false | |||
| 1096 | + | case rightChild == closingPane: | |||
| 1097 | + | closeCtx.siblingNode = leftChild | |||
| 1098 | + | closeCtx.siblingIsStartChild = true | |||
| 1099 | + | default: | |||
| 1100 | + | return closeCtx, fmt.Errorf("closing pane not found under parent: %s", parentNode.ID) | |||
| 1101 | + | } | |||
| 1102 | + | ||||
| 1103 | + | if closeCtx.siblingNode == nil { | |||
| 1104 | + | return closeCtx, errors.New("sibling missing") | |||
| 1105 | + | } | |||
| 1106 | + | ||||
| 1107 | + | if closeCtx.grandparentNode != nil { | |||
| 1108 | + | switch { | |||
| 1109 | + | case closeCtx.grandparentNode.Left() == parentNode: | |||
| 1110 | + | closeCtx.parentIsStartInGrand = true | |||
| 1111 | + | case closeCtx.grandparentNode.Right() == parentNode: | |||
| 1112 | + | closeCtx.parentIsStartInGrand = false | |||
| 1113 | + | default: | |||
| 1114 | + | return closeCtx, fmt.Errorf("parent not found under grandparent: %s", closeCtx.grandparentNode.ID) | |||
| 1115 | + | } | |||
| 1116 | + | } | |||
| 1117 | + | ||||
| 1118 | + | return closeCtx, nil | |||
| 1119 | + | } | |||
| 1120 | + | ||||
| 1121 | + | func paneNodeID(node *entity.PaneNode) string { | |||
| 1122 | + | if node == nil { | |||
| 1123 | + | return nilString | |||
| 1124 | + | } | |||
| 1125 | + | return node.ID | |||
| 1126 | + | } | |||
| 1127 | + | ||||
| 1076 | // doIncrementalStackClose removes a pane from a stack without rebuilding the entire tree. | 1128 | // doIncrementalStackClose removes a pane from a stack without rebuilding the entire tree. | ||
| 1077 | func (c *WorkspaceCoordinator) doIncrementalStackClose( | 1129 | func (c *WorkspaceCoordinator) doIncrementalStackClose( | ||
| 1078 | ctx context.Context, | 1130 | ctx context.Context, | ||
A internal/ui/coordinator/workspace_close_test.go +138 -0
| @@ -0,0 +1,138 @@ | |||
| 1 | + | package coordinator | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "strings" | |
| 5 | + | "sync" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func testLeafNode(id string) *entity.PaneNode { | |
| 12 | + | return &entity.PaneNode{ | |
| 13 | + | ID: id, | |
| 14 | + | Pane: entity.NewPane(entity.PaneID(id)), | |
| 15 | + | } | |
| 16 | + | } | |
| 17 | + | ||
| 18 | + | func testSplitNode(id string, left, right *entity.PaneNode) *entity.PaneNode { | |
| 19 | + | node := &entity.PaneNode{ | |
| 20 | + | ID: id, | |
| 21 | + | SplitDir: entity.SplitHorizontal, | |
| 22 | + | Children: []*entity.PaneNode{left, right}, | |
| 23 | + | } | |
| 24 | + | if left != nil { | |
| 25 | + | left.Parent = node | |
| 26 | + | } | |
| 27 | + | if right != nil { | |
| 28 | + | right.Parent = node | |
| 29 | + | } | |
| 30 | + | return node | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | func TestDeriveIncrementalCloseTreeContext_ValidSplit(t *testing.T) { | |
| 34 | + | closing := testLeafNode("closing") | |
| 35 | + | sibling := testLeafNode("sibling") | |
| 36 | + | parent := testSplitNode("parent", closing, sibling) | |
| 37 | + | other := testLeafNode("other") | |
| 38 | + | grand := testSplitNode("grand", parent, other) | |
| 39 | + | ||
| 40 | + | ctx, err := deriveIncrementalCloseTreeContext(closing) | |
| 41 | + | if err != nil { | |
| 42 | + | t.Fatalf("unexpected error: %v", err) | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | if ctx.parentNode != parent { | |
| 46 | + | t.Fatalf("parent mismatch") | |
| 47 | + | } | |
| 48 | + | if ctx.siblingNode != sibling { | |
| 49 | + | t.Fatalf("sibling mismatch") | |
| 50 | + | } | |
| 51 | + | if ctx.grandparentNode != grand { | |
| 52 | + | t.Fatalf("grandparent mismatch") | |
| 53 | + | } | |
| 54 | + | if ctx.siblingIsStartChild { | |
| 55 | + | t.Fatalf("sibling should be end child") | |
| 56 | + | } | |
| 57 | + | if !ctx.parentIsStartInGrand { | |
| 58 | + | t.Fatalf("parent should be start child in grandparent") | |
| 59 | + | } | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | func TestDeriveIncrementalCloseTreeContext_MissingSibling(t *testing.T) { | |
| 63 | + | closing := testLeafNode("closing") | |
| 64 | + | parent := testSplitNode("parent", closing, nil) | |
| 65 | + | ||
| 66 | + | _, err := deriveIncrementalCloseTreeContext(closing) | |
| 67 | + | if err == nil { | |
| 68 | + | t.Fatalf("expected invariant error") | |
| 69 | + | } | |
| 70 | + | if !strings.Contains(err.Error(), "sibling") && !strings.Contains(err.Error(), "nil child") { | |
| 71 | + | t.Fatalf("expected sibling/nil-child error, got: %v", err) | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | if got := paneNodeID(parent); got != "parent" { | |
| 75 | + | t.Fatalf("unexpected parent id helper output: %s", got) | |
| 76 | + | } | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | func TestDeriveIncrementalCloseTreeContext_ParentDoesNotContainClosingPane(t *testing.T) { | |
| 80 | + | closing := testLeafNode("closing") | |
| 81 | + | left := testLeafNode("left") | |
| 82 | + | right := testLeafNode("right") | |
| 83 | + | parent := testSplitNode("parent", left, right) | |
| 84 | + | closing.Parent = parent | |
| 85 | + | ||
| 86 | + | _, err := deriveIncrementalCloseTreeContext(closing) | |
| 87 | + | if err == nil { | |
| 88 | + | t.Fatalf("expected invariant error") | |
| 89 | + | } | |
| 90 | + | if !strings.Contains(err.Error(), "not found under parent") { | |
| 91 | + | t.Fatalf("unexpected error: %v", err) | |
| 92 | + | } | |
| 93 | + | } | |
| 94 | + | ||
| 95 | + | func TestCaptureIncrementalCloseContext_MissingSiblingSetsPrecheckReason(t *testing.T) { | |
| 96 | + | closing := testLeafNode("closing") | |
| 97 | + | testSplitNode("parent", closing, nil) | |
| 98 | + | ||
| 99 | + | coord := &WorkspaceCoordinator{} | |
| 100 | + | ctx := coord.captureIncrementalCloseContext(nil, closing) | |
| 101 | + | if ctx.precheckReason == "" { | |
| 102 | + | t.Fatalf("expected precheck reason") | |
| 103 | + | } | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | func TestDeriveIncrementalCloseTreeContext_ConcurrentPaneAndTabCloseSnapshots(t *testing.T) { | |
| 107 | + | paneCloseNode := testLeafNode("pane-close") | |
| 108 | + | paneCloseSibling := testLeafNode("pane-close-sibling") | |
| 109 | + | testSplitNode("pane-parent", paneCloseNode, paneCloseSibling) | |
| 110 | + | ||
| 111 | + | tabCloseNode := testLeafNode("tab-close") | |
| 112 | + | tabCloseSibling := testLeafNode("tab-close-sibling") | |
| 113 | + | testSplitNode("tab-parent", tabCloseSibling, tabCloseNode) | |
| 114 | + | ||
| 115 | + | run := func(node *entity.PaneNode, wg *sync.WaitGroup, errCh chan<- error) { | |
| 116 | + | defer wg.Done() | |
| 117 | + | for i := 0; i < 200; i++ { | |
| 118 | + | if _, err := deriveIncrementalCloseTreeContext(node); err != nil { | |
| 119 | + | errCh <- err | |
| 120 | + | return | |
| 121 | + | } | |
| 122 | + | } | |
| 123 | + | } | |
| 124 | + | ||
| 125 | + | errCh := make(chan error, 2) | |
| 126 | + | var wg sync.WaitGroup | |
| 127 | + | wg.Add(2) | |
| 128 | + | go run(paneCloseNode, &wg, errCh) | |
| 129 | + | go run(tabCloseNode, &wg, errCh) | |
| 130 | + | wg.Wait() | |
| 131 | + | close(errCh) | |
| 132 | + | ||
| 133 | + | for err := range errCh { | |
| 134 | + | if err != nil { | |
| 135 | + | t.Fatalf("unexpected concurrent derive error: %v", err) | |
| 136 | + | } | |
| 137 | + | } | |
| 138 | + | } | |
| @@ -0,0 +1,138 @@ | |||||
| 1 | + | package coordinator | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "strings" | |||
| 5 | + | "sync" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func testLeafNode(id string) *entity.PaneNode { | |||
| 12 | + | return &entity.PaneNode{ | |||
| 13 | + | ID: id, | |||
| 14 | + | Pane: entity.NewPane(entity.PaneID(id)), | |||
| 15 | + | } | |||
| 16 | + | } | |||
| 17 | + | ||||
| 18 | + | func testSplitNode(id string, left, right *entity.PaneNode) *entity.PaneNode { | |||
| 19 | + | node := &entity.PaneNode{ | |||
| 20 | + | ID: id, | |||
| 21 | + | SplitDir: entity.SplitHorizontal, | |||
| 22 | + | Children: []*entity.PaneNode{left, right}, | |||
| 23 | + | } | |||
| 24 | + | if left != nil { | |||
| 25 | + | left.Parent = node | |||
| 26 | + | } | |||
| 27 | + | if right != nil { | |||
| 28 | + | right.Parent = node | |||
| 29 | + | } | |||
| 30 | + | return node | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | func TestDeriveIncrementalCloseTreeContext_ValidSplit(t *testing.T) { | |||
| 34 | + | closing := testLeafNode("closing") | |||
| 35 | + | sibling := testLeafNode("sibling") | |||
| 36 | + | parent := testSplitNode("parent", closing, sibling) | |||
| 37 | + | other := testLeafNode("other") | |||
| 38 | + | grand := testSplitNode("grand", parent, other) | |||
| 39 | + | ||||
| 40 | + | ctx, err := deriveIncrementalCloseTreeContext(closing) | |||
| 41 | + | if err != nil { | |||
| 42 | + | t.Fatalf("unexpected error: %v", err) | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | if ctx.parentNode != parent { | |||
| 46 | + | t.Fatalf("parent mismatch") | |||
| 47 | + | } | |||
| 48 | + | if ctx.siblingNode != sibling { | |||
| 49 | + | t.Fatalf("sibling mismatch") | |||
| 50 | + | } | |||
| 51 | + | if ctx.grandparentNode != grand { | |||
| 52 | + | t.Fatalf("grandparent mismatch") | |||
| 53 | + | } | |||
| 54 | + | if ctx.siblingIsStartChild { | |||
| 55 | + | t.Fatalf("sibling should be end child") | |||
| 56 | + | } | |||
| 57 | + | if !ctx.parentIsStartInGrand { | |||
| 58 | + | t.Fatalf("parent should be start child in grandparent") | |||
| 59 | + | } | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | func TestDeriveIncrementalCloseTreeContext_MissingSibling(t *testing.T) { | |||
| 63 | + | closing := testLeafNode("closing") | |||
| 64 | + | parent := testSplitNode("parent", closing, nil) | |||
| 65 | + | ||||
| 66 | + | _, err := deriveIncrementalCloseTreeContext(closing) | |||
| 67 | + | if err == nil { | |||
| 68 | + | t.Fatalf("expected invariant error") | |||
| 69 | + | } | |||
| 70 | + | if !strings.Contains(err.Error(), "sibling") && !strings.Contains(err.Error(), "nil child") { | |||
| 71 | + | t.Fatalf("expected sibling/nil-child error, got: %v", err) | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | if got := paneNodeID(parent); got != "parent" { | |||
| 75 | + | t.Fatalf("unexpected parent id helper output: %s", got) | |||
| 76 | + | } | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | func TestDeriveIncrementalCloseTreeContext_ParentDoesNotContainClosingPane(t *testing.T) { | |||
| 80 | + | closing := testLeafNode("closing") | |||
| 81 | + | left := testLeafNode("left") | |||
| 82 | + | right := testLeafNode("right") | |||
| 83 | + | parent := testSplitNode("parent", left, right) | |||
| 84 | + | closing.Parent = parent | |||
| 85 | + | ||||
| 86 | + | _, err := deriveIncrementalCloseTreeContext(closing) | |||
| 87 | + | if err == nil { | |||
| 88 | + | t.Fatalf("expected invariant error") | |||
| 89 | + | } | |||
| 90 | + | if !strings.Contains(err.Error(), "not found under parent") { | |||
| 91 | + | t.Fatalf("unexpected error: %v", err) | |||
| 92 | + | } | |||
| 93 | + | } | |||
| 94 | + | ||||
| 95 | + | func TestCaptureIncrementalCloseContext_MissingSiblingSetsPrecheckReason(t *testing.T) { | |||
| 96 | + | closing := testLeafNode("closing") | |||
| 97 | + | testSplitNode("parent", closing, nil) | |||
| 98 | + | ||||
| 99 | + | coord := &WorkspaceCoordinator{} | |||
| 100 | + | ctx := coord.captureIncrementalCloseContext(nil, closing) | |||
| 101 | + | if ctx.precheckReason == "" { | |||
| 102 | + | t.Fatalf("expected precheck reason") | |||
| 103 | + | } | |||
| 104 | + | } | |||
| 105 | + | ||||
| 106 | + | func TestDeriveIncrementalCloseTreeContext_ConcurrentPaneAndTabCloseSnapshots(t *testing.T) { | |||
| 107 | + | paneCloseNode := testLeafNode("pane-close") | |||
| 108 | + | paneCloseSibling := testLeafNode("pane-close-sibling") | |||
| 109 | + | testSplitNode("pane-parent", paneCloseNode, paneCloseSibling) | |||
| 110 | + | ||||
| 111 | + | tabCloseNode := testLeafNode("tab-close") | |||
| 112 | + | tabCloseSibling := testLeafNode("tab-close-sibling") | |||
| 113 | + | testSplitNode("tab-parent", tabCloseSibling, tabCloseNode) | |||
| 114 | + | ||||
| 115 | + | run := func(node *entity.PaneNode, wg *sync.WaitGroup, errCh chan<- error) { | |||
| 116 | + | defer wg.Done() | |||
| 117 | + | for i := 0; i < 200; i++ { | |||
| 118 | + | if _, err := deriveIncrementalCloseTreeContext(node); err != nil { | |||
| 119 | + | errCh <- err | |||
| 120 | + | return | |||
| 121 | + | } | |||
| 122 | + | } | |||
| 123 | + | } | |||
| 124 | + | ||||
| 125 | + | errCh := make(chan error, 2) | |||
| 126 | + | var wg sync.WaitGroup | |||
| 127 | + | wg.Add(2) | |||
| 128 | + | go run(paneCloseNode, &wg, errCh) | |||
| 129 | + | go run(tabCloseNode, &wg, errCh) | |||
| 130 | + | wg.Wait() | |||
| 131 | + | close(errCh) | |||
| 132 | + | ||||
| 133 | + | for err := range errCh { | |||
| 134 | + | if err != nil { | |||
| 135 | + | t.Fatalf("unexpected concurrent derive error: %v", err) | |||
| 136 | + | } | |||
| 137 | + | } | |||
| 138 | + | } | |||