fix: marker cleanup, pool/retry correctness, stacked title sync, and race fixes
- Session markers: writeShutdownMarker removes startup marker after embedding its timestamp; sweepPairedMarkers cleans old trios in background cleanup - Retry delay: clamp base+jitter to retryMaxDelay - Pool: use DestroyWithPolicy uniformly; move acquire metrics after error check - Crash page: remove redundant sanitize call in buildCrashPageHTML - Loading skeleton: replace plain string with atomic.Value for skeletonVersion - Stacked titles: sync on LoadCommitted and on title bar pane switch
- commit
- 3eb245fb5f8d9504befe9bddf066427067bb1d75
- author
- brice · · 2026-02-07 11:32 UTC
- parent
- daee41a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
10 files · +149 -14
M CHANGELOG.md +7 -0
| @@ -23,6 +23,13 @@ All notable changes to this project will be documented in this file. | |||
| 23 | 23 | ||
| 24 | 24 | ### Fixed | |
| 25 | 25 | ||
| 26 | + | - **Stacked pane title desync on navigation**: Titles in the stacked title bar now sync immediately on `LoadCommitted` and when switching panes, preventing stale titles after background navigation. | |
| 27 | + | - **Session marker file accumulation**: `writeShutdownMarker` now removes the matching startup marker (embedding its timestamp for classification), and a new `sweepPairedMarkers` sweep in background cleanup removes old marker trios past the configured age. | |
| 28 | + | - **Retry delay exceeding max**: `retryDelayForAttempt` now clamps the final delay (base + jitter) to `retryMaxDelay` so backoff never overshoots the hard cap. | |
| 29 | + | - **Inconsistent WebView termination in pool**: `PrewarmFirst`, `PrewarmAsync`, `prewarmSync`, and `RefreshScripts` now call `DestroyWithPolicy(terminatePolicy)` instead of bare `Destroy()`, matching `Release` behavior. | |
| 30 | + | - **Pool metrics counted on failed creation**: `acquireCount`, `acquirePoolMisses`, and `acquireTotalNanos` are now incremented only after `createWebView` succeeds. | |
| 31 | + | - **Redundant crash page URI sanitization**: Removed double `sanitizeCrashPageOriginalURI` call inside `buildCrashPageHTML`; the caller already sanitizes. | |
| 32 | + | - **Racy skeleton version global**: Replaced plain `string` with `atomic.Value` so concurrent reads/writes to `skeletonVersion` are safe. | |
| 26 | 33 | - **Inline ghost suggestion stability**: Reworked ghostwriter flow to keep suggestions selected (not auto-committed), with stronger token guards and stale-callback protection. | |
| 27 | 34 | - **GTK thread-safety in omnibox**: Removed unsafe off-main-thread widget reads in async flows. | |
| 28 | 35 | - **Review follow-ups (Copilot/CodeRabbit)**: Fixed custom-scheme GET handling for empty WebKit methods, synchronized pool close/send paths, aligned WebKit signal ID storage types, and marshalled OAuth parent refresh reloads back to the GTK main loop. | |
| @@ -23,6 +23,13 @@ All notable changes to this project will be documented in this file. | |||||
| 23 | 23 | ||||
| 24 | ### Fixed | 24 | ### Fixed | ||
| 25 | 25 | ||||
| 26 | + | - **Stacked pane title desync on navigation**: Titles in the stacked title bar now sync immediately on `LoadCommitted` and when switching panes, preventing stale titles after background navigation. | |||
| 27 | + | - **Session marker file accumulation**: `writeShutdownMarker` now removes the matching startup marker (embedding its timestamp for classification), and a new `sweepPairedMarkers` sweep in background cleanup removes old marker trios past the configured age. | |||
| 28 | + | - **Retry delay exceeding max**: `retryDelayForAttempt` now clamps the final delay (base + jitter) to `retryMaxDelay` so backoff never overshoots the hard cap. | |||
| 29 | + | - **Inconsistent WebView termination in pool**: `PrewarmFirst`, `PrewarmAsync`, `prewarmSync`, and `RefreshScripts` now call `DestroyWithPolicy(terminatePolicy)` instead of bare `Destroy()`, matching `Release` behavior. | |||
| 30 | + | - **Pool metrics counted on failed creation**: `acquireCount`, `acquirePoolMisses`, and `acquireTotalNanos` are now incremented only after `createWebView` succeeds. | |||
| 31 | + | - **Redundant crash page URI sanitization**: Removed double `sanitizeCrashPageOriginalURI` call inside `buildCrashPageHTML`; the caller already sanitizes. | |||
| 32 | + | - **Racy skeleton version global**: Replaced plain `string` with `atomic.Value` so concurrent reads/writes to `skeletonVersion` are safe. | |||
| 26 | - **Inline ghost suggestion stability**: Reworked ghostwriter flow to keep suggestions selected (not auto-committed), with stronger token guards and stale-callback protection. | 33 | - **Inline ghost suggestion stability**: Reworked ghostwriter flow to keep suggestions selected (not auto-committed), with stronger token guards and stale-callback protection. | ||
| 27 | - **GTK thread-safety in omnibox**: Removed unsafe off-main-thread widget reads in async flows. | 34 | - **GTK thread-safety in omnibox**: Removed unsafe off-main-thread widget reads in async flows. | ||
| 28 | - **Review follow-ups (Copilot/CodeRabbit)**: Fixed custom-scheme GET handling for empty WebKit methods, synchronized pool close/send paths, aligned WebKit signal ID storage types, and marshalled OAuth parent refresh reloads back to the GTK main loop. | 35 | - **Review follow-ups (Copilot/CodeRabbit)**: Fixed custom-scheme GET handling for empty WebKit methods, synchronized pool close/send paths, aligned WebKit signal ID storage types, and marshalled OAuth parent refresh reloads back to the GTK main loop. | ||
M internal/bootstrap/session.go +76 -1
| @@ -325,10 +325,25 @@ func writeShutdownMarker(lockDir, sessionID string, endedAt time.Time) error { | |||
| 325 | 325 | if err := os.MkdirAll(lockDir, lockDirPerm); err != nil { | |
| 326 | 326 | return err | |
| 327 | 327 | } | |
| 328 | - | content := []byte(endedAt.Format(time.RFC3339Nano) + "\n") | |
| 328 | + | ||
| 329 | + | // Preserve the startup time inside the shutdown marker so | |
| 330 | + | // ClassifySessionExitFromMarkers can still read it after the | |
| 331 | + | // startup marker file is removed. | |
| 332 | + | var startupLine string | |
| 333 | + | startupPath := startupMarkerPath(lockDir, sessionID) | |
| 334 | + | if raw, readErr := os.ReadFile(startupPath); readErr == nil { | |
| 335 | + | if t := strings.TrimSpace(string(raw)); t != "" { | |
| 336 | + | startupLine = "started_at=" + t + "\n" | |
| 337 | + | } | |
| 338 | + | } | |
| 339 | + | ||
| 340 | + | content := []byte(endedAt.Format(time.RFC3339Nano) + "\n" + startupLine) | |
| 329 | 341 | if err := os.WriteFile(shutdownMarkerPath(lockDir, sessionID), content, markerFilePerm); err != nil { | |
| 330 | 342 | return err | |
| 331 | 343 | } | |
| 344 | + | // Clean up the matching startup marker now that shutdown is recorded. | |
| 345 | + | // Errors are non-fatal: the marker will be swept later if removal fails. | |
| 346 | + | _ = os.Remove(startupPath) | |
| 332 | 347 | return nil | |
| 333 | 348 | } | |
| 334 | 349 | ||
| @@ -470,5 +485,65 @@ func runSessionCleanupAsync( | |||
| 470 | 485 | log.Info().Int64("deleted", cleanupOutput.TotalDeleted).Msg("background: cleaned up old sessions") | |
| 471 | 486 | } | |
| 472 | 487 | } | |
| 488 | + | ||
| 489 | + | // Sweep paired startup+shutdown markers that are no longer needed. | |
| 490 | + | if lockDir != "" { | |
| 491 | + | sweepPairedMarkers(lockDir, cfg.Session.MaxExitedSessionAgeDays, log) | |
| 492 | + | } | |
| 473 | 493 | }() | |
| 474 | 494 | } | |
| 495 | + | ||
| 496 | + | // sweepPairedMarkers removes startup, shutdown, and abrupt marker files for | |
| 497 | + | // sessions that have both a startup and shutdown marker older than maxAgeDays. | |
| 498 | + | // Removal errors are non-fatal (logged and skipped). | |
| 499 | + | func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | |
| 500 | + | if lockDir == "" { | |
| 501 | + | return | |
| 502 | + | } | |
| 503 | + | if maxAgeDays <= 0 { | |
| 504 | + | maxAgeDays = 7 | |
| 505 | + | } | |
| 506 | + | cutoff := time.Now().Add(-time.Duration(maxAgeDays) * 24 * time.Hour) | |
| 507 | + | ||
| 508 | + | shutdownMarkers, err := filepath.Glob(filepath.Join(lockDir, "session_*.shutdown.marker")) | |
| 509 | + | if err != nil { | |
| 510 | + | if log != nil { | |
| 511 | + | log.Warn().Err(err).Msg("sweepPairedMarkers: glob failed") | |
| 512 | + | } | |
| 513 | + | return | |
| 514 | + | } | |
| 515 | + | ||
| 516 | + | swept := 0 | |
| 517 | + | for _, shutdownPath := range shutdownMarkers { | |
| 518 | + | base := filepath.Base(shutdownPath) | |
| 519 | + | if !strings.HasPrefix(base, "session_") || !strings.HasSuffix(base, ".shutdown.marker") { | |
| 520 | + | continue | |
| 521 | + | } | |
| 522 | + | sessionID := strings.TrimSuffix(strings.TrimPrefix(base, "session_"), ".shutdown.marker") | |
| 523 | + | if sessionID == "" { | |
| 524 | + | continue | |
| 525 | + | } | |
| 526 | + | ||
| 527 | + | info, err := os.Stat(shutdownPath) | |
| 528 | + | if err != nil || info.ModTime().After(cutoff) { | |
| 529 | + | continue | |
| 530 | + | } | |
| 531 | + | ||
| 532 | + | startupPath := startupMarkerPath(lockDir, sessionID) | |
| 533 | + | abruptPath := abruptMarkerPath(lockDir, sessionID) | |
| 534 | + | ||
| 535 | + | // Remove the trio: shutdown, startup (if still present), abrupt (if present). | |
| 536 | + | for _, p := range []string{shutdownPath, startupPath, abruptPath} { | |
| 537 | + | if rmErr := os.Remove(p); rmErr != nil && !os.IsNotExist(rmErr) { | |
| 538 | + | if log != nil { | |
| 539 | + | log.Warn().Err(rmErr).Str("path", p).Msg("sweepPairedMarkers: remove failed") | |
| 540 | + | } | |
| 541 | + | } | |
| 542 | + | } | |
| 543 | + | swept++ | |
| 544 | + | } | |
| 545 | + | ||
| 546 | + | if swept > 0 && log != nil { | |
| 547 | + | log.Info().Int("swept", swept).Msg("sweepPairedMarkers: cleaned up old marker files") | |
| 548 | + | } | |
| 549 | + | } | |
| @@ -325,10 +325,25 @@ func writeShutdownMarker(lockDir, sessionID string, endedAt time.Time) error { | |||||
| 325 | if err := os.MkdirAll(lockDir, lockDirPerm); err != nil { | 325 | if err := os.MkdirAll(lockDir, lockDirPerm); err != nil { | ||
| 326 | return err | 326 | return err | ||
| 327 | } | 327 | } | ||
| 328 | - | content := []byte(endedAt.Format(time.RFC3339Nano) + "\n") | 328 | + | |
| 329 | + | // Preserve the startup time inside the shutdown marker so | |||
| 330 | + | // ClassifySessionExitFromMarkers can still read it after the | |||
| 331 | + | // startup marker file is removed. | |||
| 332 | + | var startupLine string | |||
| 333 | + | startupPath := startupMarkerPath(lockDir, sessionID) | |||
| 334 | + | if raw, readErr := os.ReadFile(startupPath); readErr == nil { | |||
| 335 | + | if t := strings.TrimSpace(string(raw)); t != "" { | |||
| 336 | + | startupLine = "started_at=" + t + "\n" | |||
| 337 | + | } | |||
| 338 | + | } | |||
| 339 | + | ||||
| 340 | + | content := []byte(endedAt.Format(time.RFC3339Nano) + "\n" + startupLine) | |||
| 329 | if err := os.WriteFile(shutdownMarkerPath(lockDir, sessionID), content, markerFilePerm); err != nil { | 341 | if err := os.WriteFile(shutdownMarkerPath(lockDir, sessionID), content, markerFilePerm); err != nil { | ||
| 330 | return err | 342 | return err | ||
| 331 | } | 343 | } | ||
| 344 | + | // Clean up the matching startup marker now that shutdown is recorded. | |||
| 345 | + | // Errors are non-fatal: the marker will be swept later if removal fails. | |||
| 346 | + | _ = os.Remove(startupPath) | |||
| 332 | return nil | 347 | return nil | ||
| 333 | } | 348 | } | ||
| 334 | 349 | ||||
| @@ -470,5 +485,65 @@ func runSessionCleanupAsync( | |||||
| 470 | log.Info().Int64("deleted", cleanupOutput.TotalDeleted).Msg("background: cleaned up old sessions") | 485 | log.Info().Int64("deleted", cleanupOutput.TotalDeleted).Msg("background: cleaned up old sessions") | ||
| 471 | } | 486 | } | ||
| 472 | } | 487 | } | ||
| 488 | + | ||||
| 489 | + | // Sweep paired startup+shutdown markers that are no longer needed. | |||
| 490 | + | if lockDir != "" { | |||
| 491 | + | sweepPairedMarkers(lockDir, cfg.Session.MaxExitedSessionAgeDays, log) | |||
| 492 | + | } | |||
| 473 | }() | 493 | }() | ||
| 474 | } | 494 | } | ||
| 495 | + | ||||
| 496 | + | // sweepPairedMarkers removes startup, shutdown, and abrupt marker files for | |||
| 497 | + | // sessions that have both a startup and shutdown marker older than maxAgeDays. | |||
| 498 | + | // Removal errors are non-fatal (logged and skipped). | |||
| 499 | + | func sweepPairedMarkers(lockDir string, maxAgeDays int, log *zerolog.Logger) { | |||
| 500 | + | if lockDir == "" { | |||
| 501 | + | return | |||
| 502 | + | } | |||
| 503 | + | if maxAgeDays <= 0 { | |||
| 504 | + | maxAgeDays = 7 | |||
| 505 | + | } | |||
| 506 | + | cutoff := time.Now().Add(-time.Duration(maxAgeDays) * 24 * time.Hour) | |||
| 507 | + | ||||
| 508 | + | shutdownMarkers, err := filepath.Glob(filepath.Join(lockDir, "session_*.shutdown.marker")) | |||
| 509 | + | if err != nil { | |||
| 510 | + | if log != nil { | |||
| 511 | + | log.Warn().Err(err).Msg("sweepPairedMarkers: glob failed") | |||
| 512 | + | } | |||
| 513 | + | return | |||
| 514 | + | } | |||
| 515 | + | ||||
| 516 | + | swept := 0 | |||
| 517 | + | for _, shutdownPath := range shutdownMarkers { | |||
| 518 | + | base := filepath.Base(shutdownPath) | |||
| 519 | + | if !strings.HasPrefix(base, "session_") || !strings.HasSuffix(base, ".shutdown.marker") { | |||
| 520 | + | continue | |||
| 521 | + | } | |||
| 522 | + | sessionID := strings.TrimSuffix(strings.TrimPrefix(base, "session_"), ".shutdown.marker") | |||
| 523 | + | if sessionID == "" { | |||
| 524 | + | continue | |||
| 525 | + | } | |||
| 526 | + | ||||
| 527 | + | info, err := os.Stat(shutdownPath) | |||
| 528 | + | if err != nil || info.ModTime().After(cutoff) { | |||
| 529 | + | continue | |||
| 530 | + | } | |||
| 531 | + | ||||
| 532 | + | startupPath := startupMarkerPath(lockDir, sessionID) | |||
| 533 | + | abruptPath := abruptMarkerPath(lockDir, sessionID) | |||
| 534 | + | ||||
| 535 | + | // Remove the trio: shutdown, startup (if still present), abrupt (if present). | |||
| 536 | + | for _, p := range []string{shutdownPath, startupPath, abruptPath} { | |||
| 537 | + | if rmErr := os.Remove(p); rmErr != nil && !os.IsNotExist(rmErr) { | |||
| 538 | + | if log != nil { | |||
| 539 | + | log.Warn().Err(rmErr).Str("path", p).Msg("sweepPairedMarkers: remove failed") | |||
| 540 | + | } | |||
| 541 | + | } | |||
| 542 | + | } | |||
| 543 | + | swept++ | |||
| 544 | + | } | |||
| 545 | + | ||||
| 546 | + | if swept > 0 && log != nil { | |||
| 547 | + | log.Info().Int("swept", swept).Msg("sweepPairedMarkers: cleaned up old marker files") | |||
| 548 | + | } | |||
| 549 | + | } | |||
M internal/bootstrap/session_exit_classification.go +9 -1
| @@ -53,6 +53,11 @@ func ClassifySessionExitFromMarkers(lockDir, sessionID string) (SessionExitClass | |||
| 53 | 53 | return classification, err | |
| 54 | 54 | } | |
| 55 | 55 | ||
| 56 | + | // If the startup marker was already cleaned up (writeShutdownMarker removes | |
| 57 | + | // it), fall back to the started_at line embedded in the shutdown marker. | |
| 58 | + | if startupAt == nil && shutdownExists { | |
| 59 | + | startupAt, _, _, _ = readMarkerTime(shutdownMarkerPath(lockDir, sessionID), "started_at=") | |
| 60 | + | } | |
| 56 | 61 | classification.StartupAt = startupAt | |
| 57 | 62 | classification.ShutdownAt = shutdownAt | |
| 58 | 63 | classification.AbruptDetectedAt = abruptAt | |
| @@ -180,7 +185,10 @@ func readMarkerTime(path, keyPrefix string) (*time.Time, bool, time.Time, error) | |||
| 180 | 185 | return nil, true, stat.ModTime(), nil | |
| 181 | 186 | } | |
| 182 | 187 | ||
| 183 | - | parsed, parseErr := time.Parse(time.RFC3339Nano, raw) | |
| 188 | + | // When no key prefix is given, parse the first line as the timestamp. | |
| 189 | + | // Multi-line markers may contain additional metadata (e.g. started_at=). | |
| 190 | + | firstLine, _, _ := strings.Cut(raw, "\n") | |
| 191 | + | parsed, parseErr := time.Parse(time.RFC3339Nano, firstLine) | |
| 184 | 192 | if parseErr != nil { | |
| 185 | 193 | return nil, true, stat.ModTime(), nil | |
| 186 | 194 | } | |
| @@ -53,6 +53,11 @@ func ClassifySessionExitFromMarkers(lockDir, sessionID string) (SessionExitClass | |||||
| 53 | return classification, err | 53 | return classification, err | ||
| 54 | } | 54 | } | ||
| 55 | 55 | ||||
| 56 | + | // If the startup marker was already cleaned up (writeShutdownMarker removes | |||
| 57 | + | // it), fall back to the started_at line embedded in the shutdown marker. | |||
| 58 | + | if startupAt == nil && shutdownExists { | |||
| 59 | + | startupAt, _, _, _ = readMarkerTime(shutdownMarkerPath(lockDir, sessionID), "started_at=") | |||
| 60 | + | } | |||
| 56 | classification.StartupAt = startupAt | 61 | classification.StartupAt = startupAt | ||
| 57 | classification.ShutdownAt = shutdownAt | 62 | classification.ShutdownAt = shutdownAt | ||
| 58 | classification.AbruptDetectedAt = abruptAt | 63 | classification.AbruptDetectedAt = abruptAt | ||
| @@ -180,7 +185,10 @@ func readMarkerTime(path, keyPrefix string) (*time.Time, bool, time.Time, error) | |||||
| 180 | return nil, true, stat.ModTime(), nil | 185 | return nil, true, stat.ModTime(), nil | ||
| 181 | } | 186 | } | ||
| 182 | 187 | ||||
| 183 | - | parsed, parseErr := time.Parse(time.RFC3339Nano, raw) | 188 | + | // When no key prefix is given, parse the first line as the timestamp. |
| 189 | + | // Multi-line markers may contain additional metadata (e.g. started_at=). | |||
| 190 | + | firstLine, _, _ := strings.Cut(raw, "\n") | |||
| 191 | + | parsed, parseErr := time.Parse(time.RFC3339Nano, firstLine) | |||
| 184 | if parseErr != nil { | 192 | if parseErr != nil { | ||
| 185 | return nil, true, stat.ModTime(), nil | 193 | return nil, true, stat.ModTime(), nil | ||
| 186 | } | 194 | } | ||
M internal/infrastructure/updater/github.go +4 -0
| @@ -346,6 +346,10 @@ func retryDelayForAttempt(attempt int, randInt63 func(n int64) int64) time.Durat | |||
| 346 | 346 | delay += time.Duration(randInt63(int64(retryJitterMax))) | |
| 347 | 347 | } | |
| 348 | 348 | ||
| 349 | + | if delay > retryMaxDelay { | |
| 350 | + | delay = retryMaxDelay | |
| 351 | + | } | |
| 352 | + | ||
| 349 | 353 | return delay | |
| 350 | 354 | } | |
| 351 | 355 | ||
| @@ -346,6 +346,10 @@ func retryDelayForAttempt(attempt int, randInt63 func(n int64) int64) time.Durat | |||||
| 346 | delay += time.Duration(randInt63(int64(retryJitterMax))) | 346 | delay += time.Duration(randInt63(int64(retryJitterMax))) | ||
| 347 | } | 347 | } | ||
| 348 | 348 | ||||
| 349 | + | if delay > retryMaxDelay { | |||
| 350 | + | delay = retryMaxDelay | |||
| 351 | + | } | |||
| 352 | + | ||||
| 349 | return delay | 353 | return delay | ||
| 350 | } | 354 | } | ||
| 351 | 355 | ||||
M internal/infrastructure/webkit/pool.go +7 -7
| @@ -190,12 +190,12 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 190 | 190 | // Pool empty or pooled view was destroyed, create new | |
| 191 | 191 | log.Debug().Msg("pool empty, creating new webview") | |
| 192 | 192 | wv, err := p.createWebView(ctx) | |
| 193 | - | p.acquireCount.Add(1) | |
| 194 | - | p.acquirePoolMisses.Add(1) | |
| 195 | - | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |
| 196 | 193 | if err != nil { | |
| 197 | 194 | return nil, err | |
| 198 | 195 | } | |
| 196 | + | p.acquireCount.Add(1) | |
| 197 | + | p.acquirePoolMisses.Add(1) | |
| 198 | + | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |
| 199 | 199 | ||
| 200 | 200 | // Re-check after creation: if pool closed during createWebView, destroy | |
| 201 | 201 | // the new WebView and return an error to avoid leaking resources. | |
| @@ -310,7 +310,7 @@ func (p *WebViewPool) PrewarmFirst(ctx context.Context) error { | |||
| 310 | 310 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed first webview for cold start") | |
| 311 | 311 | return nil | |
| 312 | 312 | } | |
| 313 | - | wv.Destroy() // Pool somehow full (shouldn't happen) | |
| 313 | + | wv.DestroyWithPolicy(p.terminatePolicy) // Pool somehow full (shouldn't happen) | |
| 314 | 314 | return nil | |
| 315 | 315 | } | |
| 316 | 316 | ||
| @@ -384,7 +384,7 @@ func (p *WebViewPool) PrewarmAsync(ctx context.Context, count int) { | |||
| 384 | 384 | if p.tryPut(ctx, wv) { | |
| 385 | 385 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed webview added to pool") | |
| 386 | 386 | } else { | |
| 387 | - | wv.Destroy() | |
| 387 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |
| 388 | 388 | } | |
| 389 | 389 | } | |
| 390 | 390 | ||
| @@ -426,7 +426,7 @@ func (p *WebViewPool) prewarmSync(ctx context.Context, count int) { | |||
| 426 | 426 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed webview added to pool") | |
| 427 | 427 | } else { | |
| 428 | 428 | // Pool full, destroy | |
| 429 | - | wv.Destroy() | |
| 429 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |
| 430 | 430 | } | |
| 431 | 431 | ||
| 432 | 432 | // Small delay between creations to avoid overwhelming the system | |
| @@ -475,7 +475,7 @@ refreshLoop: | |||
| 475 | 475 | if p.tryPut(ctx, wv) { | |
| 476 | 476 | refreshed++ | |
| 477 | 477 | } else { | |
| 478 | - | wv.Destroy() | |
| 478 | + | wv.DestroyWithPolicy(p.terminatePolicy) | |
| 479 | 479 | } | |
| 480 | 480 | default: | |
| 481 | 481 | // Pool was modified during iteration, stop | |
| @@ -190,12 +190,12 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 190 | // Pool empty or pooled view was destroyed, create new | 190 | // Pool empty or pooled view was destroyed, create new | ||
| 191 | log.Debug().Msg("pool empty, creating new webview") | 191 | log.Debug().Msg("pool empty, creating new webview") | ||
| 192 | wv, err := p.createWebView(ctx) | 192 | wv, err := p.createWebView(ctx) | ||
| 193 | - | p.acquireCount.Add(1) | |||
| 194 | - | p.acquirePoolMisses.Add(1) | |||
| 195 | - | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |||
| 196 | if err != nil { | 193 | if err != nil { | ||
| 197 | return nil, err | 194 | return nil, err | ||
| 198 | } | 195 | } | ||
| 196 | + | p.acquireCount.Add(1) | |||
| 197 | + | p.acquirePoolMisses.Add(1) | |||
| 198 | + | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |||
| 199 | 199 | ||||
| 200 | // Re-check after creation: if pool closed during createWebView, destroy | 200 | // Re-check after creation: if pool closed during createWebView, destroy | ||
| 201 | // the new WebView and return an error to avoid leaking resources. | 201 | // the new WebView and return an error to avoid leaking resources. | ||
| @@ -310,7 +310,7 @@ func (p *WebViewPool) PrewarmFirst(ctx context.Context) error { | |||||
| 310 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed first webview for cold start") | 310 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed first webview for cold start") | ||
| 311 | return nil | 311 | return nil | ||
| 312 | } | 312 | } | ||
| 313 | - | wv.Destroy() // Pool somehow full (shouldn't happen) | 313 | + | wv.DestroyWithPolicy(p.terminatePolicy) // Pool somehow full (shouldn't happen) |
| 314 | return nil | 314 | return nil | ||
| 315 | } | 315 | } | ||
| 316 | 316 | ||||
| @@ -384,7 +384,7 @@ func (p *WebViewPool) PrewarmAsync(ctx context.Context, count int) { | |||||
| 384 | if p.tryPut(ctx, wv) { | 384 | if p.tryPut(ctx, wv) { | ||
| 385 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed webview added to pool") | 385 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed webview added to pool") | ||
| 386 | } else { | 386 | } else { | ||
| 387 | - | wv.Destroy() | 387 | + | wv.DestroyWithPolicy(p.terminatePolicy) |
| 388 | } | 388 | } | ||
| 389 | } | 389 | } | ||
| 390 | 390 | ||||
| @@ -426,7 +426,7 @@ func (p *WebViewPool) prewarmSync(ctx context.Context, count int) { | |||||
| 426 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed webview added to pool") | 426 | log.Debug().Uint64("id", uint64(wv.ID())).Msg("prewarmed webview added to pool") | ||
| 427 | } else { | 427 | } else { | ||
| 428 | // Pool full, destroy | 428 | // Pool full, destroy | ||
| 429 | - | wv.Destroy() | 429 | + | wv.DestroyWithPolicy(p.terminatePolicy) |
| 430 | } | 430 | } | ||
| 431 | 431 | ||||
| 432 | // Small delay between creations to avoid overwhelming the system | 432 | // Small delay between creations to avoid overwhelming the system | ||
| @@ -475,7 +475,7 @@ refreshLoop: | |||||
| 475 | if p.tryPut(ctx, wv) { | 475 | if p.tryPut(ctx, wv) { | ||
| 476 | refreshed++ | 476 | refreshed++ | ||
| 477 | } else { | 477 | } else { | ||
| 478 | - | wv.Destroy() | 478 | + | wv.DestroyWithPolicy(p.terminatePolicy) |
| 479 | } | 479 | } | ||
| 480 | default: | 480 | default: | ||
| 481 | // Pool was modified during iteration, stop | 481 | // Pool was modified during iteration, stop | ||
M internal/infrastructure/webkit/scheme_handler.go +1 -1
| @@ -234,7 +234,7 @@ func sanitizeCrashPageOriginalURI(originalURI string) string { | |||
| 234 | 234 | ||
| 235 | 235 | func buildCrashPageHTML(originalURI string) string { | |
| 236 | 236 | escapedOriginal := html.EscapeString(originalURI) | |
| 237 | - | escapedReloadTarget := html.EscapeString(sanitizeCrashPageOriginalURI(originalURI)) | |
| 237 | + | escapedReloadTarget := html.EscapeString(originalURI) | |
| 238 | 238 | return fmt.Sprintf(`<!DOCTYPE html> | |
| 239 | 239 | <html lang="en"> | |
| 240 | 240 | <head> | |
| @@ -234,7 +234,7 @@ func sanitizeCrashPageOriginalURI(originalURI string) string { | |||||
| 234 | 234 | ||||
| 235 | func buildCrashPageHTML(originalURI string) string { | 235 | func buildCrashPageHTML(originalURI string) string { | ||
| 236 | escapedOriginal := html.EscapeString(originalURI) | 236 | escapedOriginal := html.EscapeString(originalURI) | ||
| 237 | - | escapedReloadTarget := html.EscapeString(sanitizeCrashPageOriginalURI(originalURI)) | 237 | + | escapedReloadTarget := html.EscapeString(originalURI) |
| 238 | return fmt.Sprintf(`<!DOCTYPE html> | 238 | return fmt.Sprintf(`<!DOCTYPE html> | ||
| 239 | <html lang="en"> | 239 | <html lang="en"> | ||
| 240 | <head> | 240 | <head> | ||
M internal/infrastructure/webkit/scheme_handler_test.go +5 -1
| @@ -26,7 +26,11 @@ func TestBuildCrashPageHTMLFallbackReloadWithoutTarget(t *testing.T) { | |||
| 26 | 26 | } | |
| 27 | 27 | ||
| 28 | 28 | func TestBuildCrashPageHTMLRejectsUnsafeReloadScheme(t *testing.T) { | |
| 29 | - | body := buildCrashPageHTML("javascript:alert(1)") | |
| 29 | + | // buildCrashPageHTML receives already-sanitized input from the caller. | |
| 30 | + | // sanitizeCrashPageOriginalURI strips unsafe schemes to "". | |
| 31 | + | sanitized := sanitizeCrashPageOriginalURI("javascript:alert(1)") | |
| 32 | + | assert.Empty(t, sanitized) | |
| 33 | + | body := buildCrashPageHTML(sanitized) | |
| 30 | 34 | assert.Contains(t, body, `data-target=""`) | |
| 31 | 35 | } | |
| 32 | 36 | ||
| @@ -26,7 +26,11 @@ func TestBuildCrashPageHTMLFallbackReloadWithoutTarget(t *testing.T) { | |||||
| 26 | } | 26 | } | ||
| 27 | 27 | ||||
| 28 | func TestBuildCrashPageHTMLRejectsUnsafeReloadScheme(t *testing.T) { | 28 | func TestBuildCrashPageHTMLRejectsUnsafeReloadScheme(t *testing.T) { | ||
| 29 | - | body := buildCrashPageHTML("javascript:alert(1)") | 29 | + | // buildCrashPageHTML receives already-sanitized input from the caller. |
| 30 | + | // sanitizeCrashPageOriginalURI strips unsafe schemes to "". | |||
| 31 | + | sanitized := sanitizeCrashPageOriginalURI("javascript:alert(1)") | |||
| 32 | + | assert.Empty(t, sanitized) | |||
| 33 | + | body := buildCrashPageHTML(sanitized) | |||
| 30 | assert.Contains(t, body, `data-target=""`) | 34 | assert.Contains(t, body, `data-target=""`) | ||
| 31 | } | 35 | } | ||
| 32 | 36 | ||||
M internal/ui/component/loading_skeleton.go +5 -3
| @@ -1,6 +1,8 @@ | |||
| 1 | 1 | package component | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "sync/atomic" | |
| 5 | + | ||
| 4 | 6 | "github.com/bnema/dumber/internal/ui/layout" | |
| 5 | 7 | "github.com/jwijenbergh/puregotk/v4/gtk" | |
| 6 | 8 | ) | |
| @@ -23,11 +25,11 @@ const ( | |||
| 23 | 25 | ) | |
| 24 | 26 | ||
| 25 | 27 | // skeletonVersion is set via SetSkeletonVersion during startup. | |
| 26 | - | var skeletonVersion string | |
| 28 | + | var skeletonVersion atomic.Value | |
| 27 | 29 | ||
| 28 | 30 | // SetSkeletonVersion sets the version string shown on loading skeletons. | |
| 29 | 31 | func SetSkeletonVersion(v string) { | |
| 30 | - | skeletonVersion = v | |
| 32 | + | skeletonVersion.Store(v) | |
| 31 | 33 | } | |
| 32 | 34 | ||
| 33 | 35 | func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | |
| @@ -72,7 +74,7 @@ func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | |||
| 72 | 74 | spinner.AddCssClass("loading-skeleton-spinner") | |
| 73 | 75 | ||
| 74 | 76 | // Version label below spinner | |
| 75 | - | versionText := skeletonVersion | |
| 77 | + | versionText, _ := skeletonVersion.Load().(string) | |
| 76 | 78 | if versionText == "" { | |
| 77 | 79 | versionText = "dev" | |
| 78 | 80 | } | |
| @@ -1,6 +1,8 @@ | |||||
| 1 | package component | 1 | package component | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "sync/atomic" | |||
| 5 | + | ||||
| 4 | "github.com/bnema/dumber/internal/ui/layout" | 6 | "github.com/bnema/dumber/internal/ui/layout" | ||
| 5 | "github.com/jwijenbergh/puregotk/v4/gtk" | 7 | "github.com/jwijenbergh/puregotk/v4/gtk" | ||
| 6 | ) | 8 | ) | ||
| @@ -23,11 +25,11 @@ const ( | |||||
| 23 | ) | 25 | ) | ||
| 24 | 26 | ||||
| 25 | // skeletonVersion is set via SetSkeletonVersion during startup. | 27 | // skeletonVersion is set via SetSkeletonVersion during startup. | ||
| 26 | - | var skeletonVersion string | 28 | + | var skeletonVersion atomic.Value |
| 27 | 29 | ||||
| 28 | // SetSkeletonVersion sets the version string shown on loading skeletons. | 30 | // SetSkeletonVersion sets the version string shown on loading skeletons. | ||
| 29 | func SetSkeletonVersion(v string) { | 31 | func SetSkeletonVersion(v string) { | ||
| 30 | - | skeletonVersion = v | 32 | + | skeletonVersion.Store(v) |
| 31 | } | 33 | } | ||
| 32 | 34 | ||||
| 33 | func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | 35 | func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | ||
| @@ -72,7 +74,7 @@ func NewLoadingSkeleton(factory layout.WidgetFactory) *LoadingSkeleton { | |||||
| 72 | spinner.AddCssClass("loading-skeleton-spinner") | 74 | spinner.AddCssClass("loading-skeleton-spinner") | ||
| 73 | 75 | ||||
| 74 | // Version label below spinner | 76 | // Version label below spinner | ||
| 75 | - | versionText := skeletonVersion | 77 | + | versionText, _ := skeletonVersion.Load().(string) |
| 76 | if versionText == "" { | 78 | if versionText == "" { | ||
| 77 | versionText = "dev" | 79 | versionText = "dev" | ||
| 78 | } | 80 | } | ||
M internal/ui/coordinator/content.go +23 -0
| @@ -880,6 +880,22 @@ func (c *ContentCoordinator) updateStackedPaneTitle( | |||
| 880 | 880 | } | |
| 881 | 881 | } | |
| 882 | 882 | ||
| 883 | + | // syncStackedTitle updates the stacked title bar for a pane if it's in a stack. | |
| 884 | + | // Called from onLoadCommitted to keep titles in sync during navigation. | |
| 885 | + | func (c *ContentCoordinator) syncStackedTitle(ctx context.Context, paneID entity.PaneID, title string) { | |
| 886 | + | _, wsView := c.getActiveWS() | |
| 887 | + | if wsView == nil { | |
| 888 | + | return | |
| 889 | + | } | |
| 890 | + | tr := wsView.TreeRenderer() | |
| 891 | + | if tr == nil { | |
| 892 | + | return | |
| 893 | + | } | |
| 894 | + | if sv := tr.GetStackedViewForPane(string(paneID)); sv != nil { | |
| 895 | + | c.updateStackedPaneTitle(ctx, sv, paneID, title) | |
| 896 | + | } | |
| 897 | + | } | |
| 898 | + | ||
| 883 | 899 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | |
| 884 | 900 | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | |
| 885 | 901 | log := logging.FromContext(ctx) | |
| @@ -1052,6 +1068,13 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||
| 1052 | 1068 | // Update domain model with current URI for session snapshots | |
| 1053 | 1069 | c.updatePaneURI(paneID, uri) | |
| 1054 | 1070 | ||
| 1071 | + | // Sync StackedView title bar with the WebView's current title. | |
| 1072 | + | // This keeps the stacked title bar up-to-date immediately on navigation, | |
| 1073 | + | // before the asynchronous notify::title signal fires. | |
| 1074 | + | if title := wv.Title(); title != "" { | |
| 1075 | + | c.syncStackedTitle(ctx, paneID, title) | |
| 1076 | + | } | |
| 1077 | + | ||
| 1055 | 1078 | // Record history - URI is guaranteed to be correct at LoadCommitted | |
| 1056 | 1079 | if c.onHistoryRecord != nil { | |
| 1057 | 1080 | c.onHistoryRecord(ctx, paneID, uri) | |
| @@ -880,6 +880,22 @@ func (c *ContentCoordinator) updateStackedPaneTitle( | |||||
| 880 | } | 880 | } | ||
| 881 | } | 881 | } | ||
| 882 | 882 | ||||
| 883 | + | // syncStackedTitle updates the stacked title bar for a pane if it's in a stack. | |||
| 884 | + | // Called from onLoadCommitted to keep titles in sync during navigation. | |||
| 885 | + | func (c *ContentCoordinator) syncStackedTitle(ctx context.Context, paneID entity.PaneID, title string) { | |||
| 886 | + | _, wsView := c.getActiveWS() | |||
| 887 | + | if wsView == nil { | |||
| 888 | + | return | |||
| 889 | + | } | |||
| 890 | + | tr := wsView.TreeRenderer() | |||
| 891 | + | if tr == nil { | |||
| 892 | + | return | |||
| 893 | + | } | |||
| 894 | + | if sv := tr.GetStackedViewForPane(string(paneID)); sv != nil { | |||
| 895 | + | c.updateStackedPaneTitle(ctx, sv, paneID, title) | |||
| 896 | + | } | |||
| 897 | + | } | |||
| 898 | + | ||||
| 883 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | 899 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | ||
| 884 | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | 900 | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | ||
| 885 | log := logging.FromContext(ctx) | 901 | log := logging.FromContext(ctx) | ||
| @@ -1052,6 +1068,13 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||||
| 1052 | // Update domain model with current URI for session snapshots | 1068 | // Update domain model with current URI for session snapshots | ||
| 1053 | c.updatePaneURI(paneID, uri) | 1069 | c.updatePaneURI(paneID, uri) | ||
| 1054 | 1070 | ||||
| 1071 | + | // Sync StackedView title bar with the WebView's current title. | |||
| 1072 | + | // This keeps the stacked title bar up-to-date immediately on navigation, | |||
| 1073 | + | // before the asynchronous notify::title signal fires. | |||
| 1074 | + | if title := wv.Title(); title != "" { | |||
| 1075 | + | c.syncStackedTitle(ctx, paneID, title) | |||
| 1076 | + | } | |||
| 1077 | + | ||||
| 1055 | // Record history - URI is guaranteed to be correct at LoadCommitted | 1078 | // Record history - URI is guaranteed to be correct at LoadCommitted | ||
| 1056 | if c.onHistoryRecord != nil { | 1079 | if c.onHistoryRecord != nil { | ||
| 1057 | c.onHistoryRecord(ctx, paneID, uri) | 1080 | c.onHistoryRecord(ctx, paneID, uri) | ||
M internal/ui/coordinator/workspace.go +12 -0
| @@ -1623,6 +1623,18 @@ func (c *WorkspaceCoordinator) onTitleBarClick(ctx context.Context, stackNode *e | |||
| 1623 | 1623 | } | |
| 1624 | 1624 | clickedPaneID := clickedChild.Pane.ID | |
| 1625 | 1625 | ||
| 1626 | + | // Sync the incoming pane's title bar with its current WebView title | |
| 1627 | + | // to avoid showing a stale title after navigation occurred while hidden. | |
| 1628 | + | incomingTitle := c.contentCoord.GetTitle(clickedPaneID) | |
| 1629 | + | if incomingTitle == "" { | |
| 1630 | + | incomingTitle = clickedChild.Pane.Title | |
| 1631 | + | } | |
| 1632 | + | if incomingTitle != "" { | |
| 1633 | + | if err := sv.UpdateTitle(clickedIndex, incomingTitle); err != nil { | |
| 1634 | + | log.Warn().Err(err).Msg("failed to update incoming pane title") | |
| 1635 | + | } | |
| 1636 | + | } | |
| 1637 | + | ||
| 1626 | 1638 | // Update StackedView active index | |
| 1627 | 1639 | if err := sv.SetActive(ctx, clickedIndex); err != nil { | |
| 1628 | 1640 | log.Warn().Err(err).Int("index", clickedIndex).Msg("failed to set active pane in stack") | |
| @@ -1623,6 +1623,18 @@ func (c *WorkspaceCoordinator) onTitleBarClick(ctx context.Context, stackNode *e | |||||
| 1623 | } | 1623 | } | ||
| 1624 | clickedPaneID := clickedChild.Pane.ID | 1624 | clickedPaneID := clickedChild.Pane.ID | ||
| 1625 | 1625 | ||||
| 1626 | + | // Sync the incoming pane's title bar with its current WebView title | |||
| 1627 | + | // to avoid showing a stale title after navigation occurred while hidden. | |||
| 1628 | + | incomingTitle := c.contentCoord.GetTitle(clickedPaneID) | |||
| 1629 | + | if incomingTitle == "" { | |||
| 1630 | + | incomingTitle = clickedChild.Pane.Title | |||
| 1631 | + | } | |||
| 1632 | + | if incomingTitle != "" { | |||
| 1633 | + | if err := sv.UpdateTitle(clickedIndex, incomingTitle); err != nil { | |||
| 1634 | + | log.Warn().Err(err).Msg("failed to update incoming pane title") | |||
| 1635 | + | } | |||
| 1636 | + | } | |||
| 1637 | + | ||||
| 1626 | // Update StackedView active index | 1638 | // Update StackedView active index | ||
| 1627 | if err := sv.SetActive(ctx, clickedIndex); err != nil { | 1639 | if err := sv.SetActive(ctx, clickedIndex); err != nil { | ||
| 1628 | log.Warn().Err(err).Int("index", clickedIndex).Msg("failed to set active pane in stack") | 1640 | log.Warn().Err(err).Int("index", clickedIndex).Msg("failed to set active pane in stack") | ||