fix: resolve follow-up coderabbit findings
- commit
- 96f2b517b4cfdc8145424bde18c04ec75f76b1e0
- author
- brice · · 2026-02-07 09:16 UTC
- parent
- c1cf8b7
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +74 -30
M CHANGELOG.md +4 -0
| @@ -5,11 +5,13 @@ All notable changes to this project will be documented in this file. | |||
| 5 | 5 | ## [Unreleased] | |
| 6 | 6 | ||
| 7 | 7 | ### Added | |
| 8 | + | ||
| 8 | 9 | - **Session exit classification runbook and diagnostics docs**: Added dedicated operational documentation for abrupt/clean exit classification and troubleshooting paths. | |
| 9 | 10 | - **Session/bootstrap resilience coverage**: Added tests and supporting logic for startup markers, WebKit stack bootstrap, and exit classification handling. | |
| 10 | 11 | - **Omnibox async guard tests**: Added focused tests for ghost text async/state safety and normalization behavior. | |
| 11 | 12 | ||
| 12 | 13 | ### Changed | |
| 14 | + | ||
| 13 | 15 | - **WebKit/puregotk compatibility refresh**: Updated bindings and call sites for `github.com/bnema/puregotk-webkit v0.0.10` and latest puregotk replacement, including signal ID/type handling updates. | |
| 14 | 16 | - **Dependencies updated**: Bumped key runtime/developer dependencies, including `go-sqlite3`, `bubbles`, and related transitive modules. | |
| 15 | 17 | - **Omnibox behavior tuning**: Refined ghost suggestion ranking/selection behavior and dynamic result sizing tied to pane height. | |
| @@ -18,6 +20,7 @@ All notable changes to this project will be documented in this file. | |||
| 18 | 20 | - **Config normalization hardening**: Normalization now coerces invalid profile/appearance values to safe defaults. | |
| 19 | 21 | ||
| 20 | 22 | ### Fixed | |
| 23 | + | ||
| 21 | 24 | - **Inline ghost suggestion stability**: Reworked ghostwriter flow to keep suggestions selected (not auto-committed), with stronger token guards and stale-callback protection. | |
| 22 | 25 | - **GTK thread-safety in omnibox**: Removed unsafe off-main-thread widget reads in async flows. | |
| 23 | 26 | - **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. | |
| @@ -32,6 +35,7 @@ All notable changes to this project will be documented in this file. | |||
| 32 | 35 | - **Filtering/snapshot/session reliability**: Fixed race/error-handling paths in filtering updates, snapshot persistence, and session startup marker processing. | |
| 33 | 36 | ||
| 34 | 37 | ### Security | |
| 38 | + | ||
| 35 | 39 | - **External scheme launch guard**: Restricted external URL scheme launching to user-gesture initiated navigations. | |
| 36 | 40 | ||
| 37 | 41 | ## [0.26.1] - 2026-01-25 | |
| @@ -5,11 +5,13 @@ All notable changes to this project will be documented in this file. | |||||
| 5 | ## [Unreleased] | 5 | ## [Unreleased] | ||
| 6 | 6 | ||||
| 7 | ### Added | 7 | ### Added | ||
| 8 | + | ||||
| 8 | - **Session exit classification runbook and diagnostics docs**: Added dedicated operational documentation for abrupt/clean exit classification and troubleshooting paths. | 9 | - **Session exit classification runbook and diagnostics docs**: Added dedicated operational documentation for abrupt/clean exit classification and troubleshooting paths. | ||
| 9 | - **Session/bootstrap resilience coverage**: Added tests and supporting logic for startup markers, WebKit stack bootstrap, and exit classification handling. | 10 | - **Session/bootstrap resilience coverage**: Added tests and supporting logic for startup markers, WebKit stack bootstrap, and exit classification handling. | ||
| 10 | - **Omnibox async guard tests**: Added focused tests for ghost text async/state safety and normalization behavior. | 11 | - **Omnibox async guard tests**: Added focused tests for ghost text async/state safety and normalization behavior. | ||
| 11 | 12 | ||||
| 12 | ### Changed | 13 | ### Changed | ||
| 14 | + | ||||
| 13 | - **WebKit/puregotk compatibility refresh**: Updated bindings and call sites for `github.com/bnema/puregotk-webkit v0.0.10` and latest puregotk replacement, including signal ID/type handling updates. | 15 | - **WebKit/puregotk compatibility refresh**: Updated bindings and call sites for `github.com/bnema/puregotk-webkit v0.0.10` and latest puregotk replacement, including signal ID/type handling updates. | ||
| 14 | - **Dependencies updated**: Bumped key runtime/developer dependencies, including `go-sqlite3`, `bubbles`, and related transitive modules. | 16 | - **Dependencies updated**: Bumped key runtime/developer dependencies, including `go-sqlite3`, `bubbles`, and related transitive modules. | ||
| 15 | - **Omnibox behavior tuning**: Refined ghost suggestion ranking/selection behavior and dynamic result sizing tied to pane height. | 17 | - **Omnibox behavior tuning**: Refined ghost suggestion ranking/selection behavior and dynamic result sizing tied to pane height. | ||
| @@ -18,6 +20,7 @@ All notable changes to this project will be documented in this file. | |||||
| 18 | - **Config normalization hardening**: Normalization now coerces invalid profile/appearance values to safe defaults. | 20 | - **Config normalization hardening**: Normalization now coerces invalid profile/appearance values to safe defaults. | ||
| 19 | 21 | ||||
| 20 | ### Fixed | 22 | ### Fixed | ||
| 23 | + | ||||
| 21 | - **Inline ghost suggestion stability**: Reworked ghostwriter flow to keep suggestions selected (not auto-committed), with stronger token guards and stale-callback protection. | 24 | - **Inline ghost suggestion stability**: Reworked ghostwriter flow to keep suggestions selected (not auto-committed), with stronger token guards and stale-callback protection. | ||
| 22 | - **GTK thread-safety in omnibox**: Removed unsafe off-main-thread widget reads in async flows. | 25 | - **GTK thread-safety in omnibox**: Removed unsafe off-main-thread widget reads in async flows. | ||
| 23 | - **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. | 26 | - **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. | ||
| @@ -32,6 +35,7 @@ All notable changes to this project will be documented in this file. | |||||
| 32 | - **Filtering/snapshot/session reliability**: Fixed race/error-handling paths in filtering updates, snapshot persistence, and session startup marker processing. | 35 | - **Filtering/snapshot/session reliability**: Fixed race/error-handling paths in filtering updates, snapshot persistence, and session startup marker processing. | ||
| 33 | 36 | ||||
| 34 | ### Security | 37 | ### Security | ||
| 38 | + | ||||
| 35 | - **External scheme launch guard**: Restricted external URL scheme launching to user-gesture initiated navigations. | 39 | - **External scheme launch guard**: Restricted external URL scheme launching to user-gesture initiated navigations. | ||
| 36 | 40 | ||||
| 37 | ## [0.26.1] - 2026-01-25 | 41 | ## [0.26.1] - 2026-01-25 | ||
M internal/bootstrap/session.go +16 -0
| @@ -352,6 +352,22 @@ func markAbruptExits(lockDir string, detectedAt time.Time, logger *zerolog.Logge | |||
| 352 | 352 | if sessionID == "" { | |
| 353 | 353 | continue | |
| 354 | 354 | } | |
| 355 | + | ||
| 356 | + | lockPath := sessionLockPath(lockDir, entity.SessionID(sessionID)) | |
| 357 | + | if _, err := os.Stat(lockPath); err == nil { | |
| 358 | + | // Session still actively running; do not mark as abrupt. | |
| 359 | + | continue | |
| 360 | + | } else if !os.IsNotExist(err) { | |
| 361 | + | if logger != nil { | |
| 362 | + | logger.Warn(). | |
| 363 | + | Err(err). | |
| 364 | + | Str("session_id", sessionID). | |
| 365 | + | Str("path", lockPath). | |
| 366 | + | Msg("markAbruptExits: stat failed") | |
| 367 | + | } | |
| 368 | + | continue | |
| 369 | + | } | |
| 370 | + | ||
| 355 | 371 | shutdownPath := shutdownMarkerPath(lockDir, sessionID) | |
| 356 | 372 | if _, err := os.Stat(shutdownPath); err == nil { | |
| 357 | 373 | continue | |
| @@ -352,6 +352,22 @@ func markAbruptExits(lockDir string, detectedAt time.Time, logger *zerolog.Logge | |||||
| 352 | if sessionID == "" { | 352 | if sessionID == "" { | ||
| 353 | continue | 353 | continue | ||
| 354 | } | 354 | } | ||
| 355 | + | ||||
| 356 | + | lockPath := sessionLockPath(lockDir, entity.SessionID(sessionID)) | |||
| 357 | + | if _, err := os.Stat(lockPath); err == nil { | |||
| 358 | + | // Session still actively running; do not mark as abrupt. | |||
| 359 | + | continue | |||
| 360 | + | } else if !os.IsNotExist(err) { | |||
| 361 | + | if logger != nil { | |||
| 362 | + | logger.Warn(). | |||
| 363 | + | Err(err). | |||
| 364 | + | Str("session_id", sessionID). | |||
| 365 | + | Str("path", lockPath). | |||
| 366 | + | Msg("markAbruptExits: stat failed") | |||
| 367 | + | } | |||
| 368 | + | continue | |||
| 369 | + | } | |||
| 370 | + | ||||
| 355 | shutdownPath := shutdownMarkerPath(lockDir, sessionID) | 371 | shutdownPath := shutdownMarkerPath(lockDir, sessionID) | ||
| 356 | if _, err := os.Stat(shutdownPath); err == nil { | 372 | if _, err := os.Stat(shutdownPath); err == nil { | ||
| 357 | continue | 373 | continue | ||
M internal/infrastructure/filtering/downloader.go +0 -12
| @@ -216,18 +216,6 @@ func (*Downloader) renameTempFile(tmpPath, localPath string) error { | |||
| 216 | 216 | return fmt.Errorf("failed to create target dir: %w", err) | |
| 217 | 217 | } | |
| 218 | 218 | ||
| 219 | - | parentInfo, err := os.Stat(parentDir) | |
| 220 | - | if err != nil { | |
| 221 | - | return fmt.Errorf("failed to stat target dir: %w", err) | |
| 222 | - | } | |
| 223 | - | if !parentInfo.IsDir() { | |
| 224 | - | return fmt.Errorf("target parent path is not a directory: %s", parentDir) | |
| 225 | - | } | |
| 226 | - | ||
| 227 | - | if _, err := os.Stat(tmpPath); err != nil { | |
| 228 | - | return fmt.Errorf("temp file missing before rename: %w", err) | |
| 229 | - | } | |
| 230 | - | ||
| 231 | 219 | if err := os.Rename(tmpPath, localPath); err != nil { | |
| 232 | 220 | return fmt.Errorf("failed to rename temp file: %w", err) | |
| 233 | 221 | } | |
| @@ -216,18 +216,6 @@ func (*Downloader) renameTempFile(tmpPath, localPath string) error { | |||||
| 216 | return fmt.Errorf("failed to create target dir: %w", err) | 216 | return fmt.Errorf("failed to create target dir: %w", err) | ||
| 217 | } | 217 | } | ||
| 218 | 218 | ||||
| 219 | - | parentInfo, err := os.Stat(parentDir) | |||
| 220 | - | if err != nil { | |||
| 221 | - | return fmt.Errorf("failed to stat target dir: %w", err) | |||
| 222 | - | } | |||
| 223 | - | if !parentInfo.IsDir() { | |||
| 224 | - | return fmt.Errorf("target parent path is not a directory: %s", parentDir) | |||
| 225 | - | } | |||
| 226 | - | ||||
| 227 | - | if _, err := os.Stat(tmpPath); err != nil { | |||
| 228 | - | return fmt.Errorf("temp file missing before rename: %w", err) | |||
| 229 | - | } | |||
| 230 | - | ||||
| 231 | if err := os.Rename(tmpPath, localPath); err != nil { | 219 | if err := os.Rename(tmpPath, localPath); err != nil { | ||
| 232 | return fmt.Errorf("failed to rename temp file: %w", err) | 220 | return fmt.Errorf("failed to rename temp file: %w", err) | ||
| 233 | } | 221 | } | ||
M internal/infrastructure/webkit/webview.go +1 -1
| @@ -1345,7 +1345,7 @@ func (wv *WebView) DisconnectSignals() { | |||
| 1345 | 1345 | // This must be called when a WebView is no longer needed to free GPU resources, | |
| 1346 | 1346 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | |
| 1347 | 1347 | func (wv *WebView) Destroy() { | |
| 1348 | - | wv.DestroyWithPolicy(resolveTerminatePolicy("")) | |
| 1348 | + | wv.DestroyWithPolicy("") | |
| 1349 | 1349 | } | |
| 1350 | 1350 | ||
| 1351 | 1351 | // DestroyWithPolicy cleans up the WebView resources with explicit process policy. | |
| @@ -1345,7 +1345,7 @@ func (wv *WebView) DisconnectSignals() { | |||||
| 1345 | // This must be called when a WebView is no longer needed to free GPU resources, | 1345 | // This must be called when a WebView is no longer needed to free GPU resources, | ||
| 1346 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | 1346 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | ||
| 1347 | func (wv *WebView) Destroy() { | 1347 | func (wv *WebView) Destroy() { | ||
| 1348 | - | wv.DestroyWithPolicy(resolveTerminatePolicy("")) | 1348 | + | wv.DestroyWithPolicy("") |
| 1349 | } | 1349 | } | ||
| 1350 | 1350 | ||||
| 1351 | // DestroyWithPolicy cleans up the WebView resources with explicit process policy. | 1351 | // DestroyWithPolicy cleans up the WebView resources with explicit process policy. | ||
M internal/ui/coordinator/content.go +53 -17
| @@ -93,6 +93,7 @@ type ContentCoordinator struct { | |||
| 93 | 93 | injector *webkit.ContentInjector | |
| 94 | 94 | ||
| 95 | 95 | webViews map[entity.PaneID]*webkit.WebView | |
| 96 | + | webViewsMu sync.RWMutex | |
| 96 | 97 | paneTitles map[entity.PaneID]string | |
| 97 | 98 | titleMu sync.RWMutex | |
| 98 | 99 | ||
| @@ -261,7 +262,7 @@ func (c *ContentCoordinator) SetOnFirstLoadStarted(fn func()) { | |||
| 261 | 262 | func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (*webkit.WebView, error) { | |
| 262 | 263 | log := logging.FromContext(ctx) | |
| 263 | 264 | ||
| 264 | - | if wv, ok := c.webViews[paneID]; ok && wv != nil && !wv.IsDestroyed() { | |
| 265 | + | if wv := c.getWebViewLocked(paneID); wv != nil && !wv.IsDestroyed() { | |
| 265 | 266 | return wv, nil | |
| 266 | 267 | } | |
| 267 | 268 | ||
| @@ -270,7 +271,7 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||
| 270 | 271 | } | |
| 271 | 272 | ||
| 272 | 273 | // Mark tab_created on first webview (first tab) | |
| 273 | - | if len(c.webViews) == 0 { | |
| 274 | + | if c.webViewCount() == 0 { | |
| 274 | 275 | logging.Trace().Mark("tab_created") | |
| 275 | 276 | } | |
| 276 | 277 | ||
| @@ -280,7 +281,7 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||
| 280 | 281 | } | |
| 281 | 282 | logging.Trace().Mark("webview_acquired") | |
| 282 | 283 | ||
| 283 | - | c.webViews[paneID] = wv | |
| 284 | + | c.setWebViewLocked(paneID, wv) | |
| 284 | 285 | c.setupWebViewCallbacks(ctx, paneID, wv) | |
| 285 | 286 | ||
| 286 | 287 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | |
| @@ -291,11 +292,10 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||
| 291 | 292 | func (c *ContentCoordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | |
| 292 | 293 | log := logging.FromContext(ctx) | |
| 293 | 294 | ||
| 294 | - | wv, ok := c.webViews[paneID] | |
| 295 | - | if !ok || wv == nil { | |
| 295 | + | wv := c.deleteWebViewLocked(paneID) | |
| 296 | + | if wv == nil { | |
| 296 | 297 | return | |
| 297 | 298 | } | |
| 298 | - | delete(c.webViews, paneID) | |
| 299 | 299 | c.clearPendingAppearance(paneID) | |
| 300 | 300 | ||
| 301 | 301 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | |
| @@ -422,20 +422,56 @@ func (c *ContentCoordinator) ActiveWebView(ctx context.Context) *webkit.WebView | |||
| 422 | 422 | return nil | |
| 423 | 423 | } | |
| 424 | 424 | ||
| 425 | - | return c.webViews[pane.Pane.ID] | |
| 425 | + | return c.getWebViewLocked(pane.Pane.ID) | |
| 426 | 426 | } | |
| 427 | 427 | ||
| 428 | 428 | // GetWebView returns the WebView for a specific pane. | |
| 429 | 429 | func (c *ContentCoordinator) GetWebView(paneID entity.PaneID) *webkit.WebView { | |
| 430 | - | return c.webViews[paneID] | |
| 430 | + | return c.getWebViewLocked(paneID) | |
| 431 | 431 | } | |
| 432 | 432 | ||
| 433 | 433 | // RegisterPopupWebView registers a popup WebView that was created externally. | |
| 434 | 434 | // This is used when popup tabs are created and the WebView needs to be tracked. | |
| 435 | 435 | func (c *ContentCoordinator) RegisterPopupWebView(paneID entity.PaneID, wv *webkit.WebView) { | |
| 436 | 436 | if wv != nil && paneID != "" { | |
| 437 | - | c.webViews[paneID] = wv | |
| 437 | + | c.setWebViewLocked(paneID, wv) | |
| 438 | + | } | |
| 439 | + | } | |
| 440 | + | ||
| 441 | + | func (c *ContentCoordinator) getWebViewLocked(paneID entity.PaneID) *webkit.WebView { | |
| 442 | + | c.webViewsMu.RLock() | |
| 443 | + | defer c.webViewsMu.RUnlock() | |
| 444 | + | return c.webViews[paneID] | |
| 445 | + | } | |
| 446 | + | ||
| 447 | + | func (c *ContentCoordinator) setWebViewLocked(paneID entity.PaneID, wv *webkit.WebView) { | |
| 448 | + | c.webViewsMu.Lock() | |
| 449 | + | c.webViews[paneID] = wv | |
| 450 | + | c.webViewsMu.Unlock() | |
| 451 | + | } | |
| 452 | + | ||
| 453 | + | func (c *ContentCoordinator) deleteWebViewLocked(paneID entity.PaneID) *webkit.WebView { | |
| 454 | + | c.webViewsMu.Lock() | |
| 455 | + | defer c.webViewsMu.Unlock() | |
| 456 | + | wv := c.webViews[paneID] | |
| 457 | + | delete(c.webViews, paneID) | |
| 458 | + | return wv | |
| 459 | + | } | |
| 460 | + | ||
| 461 | + | func (c *ContentCoordinator) webViewCount() int { | |
| 462 | + | c.webViewsMu.RLock() | |
| 463 | + | defer c.webViewsMu.RUnlock() | |
| 464 | + | return len(c.webViews) | |
| 465 | + | } | |
| 466 | + | ||
| 467 | + | func (c *ContentCoordinator) snapshotWebViews() map[entity.PaneID]*webkit.WebView { | |
| 468 | + | c.webViewsMu.RLock() | |
| 469 | + | defer c.webViewsMu.RUnlock() | |
| 470 | + | snapshot := make(map[entity.PaneID]*webkit.WebView, len(c.webViews)) | |
| 471 | + | for paneID, wv := range c.webViews { | |
| 472 | + | snapshot[paneID] = wv | |
| 438 | 473 | } | |
| 474 | + | return snapshot | |
| 439 | 475 | } | |
| 440 | 476 | ||
| 441 | 477 | // ApplyFiltersToAll applies content filters to all active webviews. | |
| @@ -443,7 +479,7 @@ func (c *ContentCoordinator) RegisterPopupWebView(paneID entity.PaneID, wv *webk | |||
| 443 | 479 | func (c *ContentCoordinator) ApplyFiltersToAll(ctx context.Context, applier webkit.FilterApplier) { | |
| 444 | 480 | log := logging.FromContext(ctx) | |
| 445 | 481 | ||
| 446 | - | for paneID, wv := range c.webViews { | |
| 482 | + | for paneID, wv := range c.snapshotWebViews() { | |
| 447 | 483 | if wv != nil && !wv.IsDestroyed() { | |
| 448 | 484 | applier.ApplyTo(ctx, wv.UserContentManager()) | |
| 449 | 485 | log.Debug().Str("pane_id", string(paneID)).Msg("applied filters to existing webview") | |
| @@ -458,7 +494,7 @@ func (c *ContentCoordinator) ApplySettingsToAll(ctx context.Context, sm *webkit. | |||
| 458 | 494 | return | |
| 459 | 495 | } | |
| 460 | 496 | ||
| 461 | - | for paneID, wv := range c.webViews { | |
| 497 | + | for paneID, wv := range c.snapshotWebViews() { | |
| 462 | 498 | if wv == nil || wv.IsDestroyed() { | |
| 463 | 499 | continue | |
| 464 | 500 | } | |
| @@ -802,7 +838,7 @@ func (c *ContentCoordinator) onTitleChanged(ctx context.Context, paneID entity.P | |||
| 802 | 838 | ||
| 803 | 839 | // Notify history persistence (get URL from WebView) | |
| 804 | 840 | if c.onTitleUpdated != nil { | |
| 805 | - | if wv := c.webViews[paneID]; wv != nil { | |
| 841 | + | if wv := c.getWebViewLocked(paneID); wv != nil { | |
| 806 | 842 | currentURI := wv.URI() | |
| 807 | 843 | if currentURI != "" && title != "" { | |
| 808 | 844 | c.onTitleUpdated(ctx, paneID, currentURI, title) | |
| @@ -849,7 +885,7 @@ func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity | |||
| 849 | 885 | log := logging.FromContext(ctx) | |
| 850 | 886 | ||
| 851 | 887 | // Get current URI to extract domain for caching | |
| 852 | - | wv := c.webViews[paneID] | |
| 888 | + | wv := c.getWebViewLocked(paneID) | |
| 853 | 889 | if wv == nil { | |
| 854 | 890 | return | |
| 855 | 891 | } | |
| @@ -1159,7 +1195,7 @@ func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity. | |||
| 1159 | 1195 | return | |
| 1160 | 1196 | } | |
| 1161 | 1197 | ||
| 1162 | - | wv := c.webViews[paneID] | |
| 1198 | + | wv := c.getWebViewLocked(paneID) | |
| 1163 | 1199 | if wv == nil || wv.IsDestroyed() { | |
| 1164 | 1200 | return | |
| 1165 | 1201 | } | |
| @@ -1351,7 +1387,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1351 | 1387 | } | |
| 1352 | 1388 | ||
| 1353 | 1389 | // Register WebView in our map (after successful insertion) | |
| 1354 | - | c.webViews[paneID] = popupWV | |
| 1390 | + | c.setWebViewLocked(paneID, popupWV) | |
| 1355 | 1391 | ||
| 1356 | 1392 | // Setup standard callbacks (after successful insertion to avoid leak) | |
| 1357 | 1393 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | |
| @@ -1767,7 +1803,7 @@ func (c *ContentCoordinator) refreshPaneAfterOAuth( | |||
| 1767 | 1803 | popupID port.WebViewID, | |
| 1768 | 1804 | ) { | |
| 1769 | 1805 | log := logging.FromContext(ctx) | |
| 1770 | - | wv := c.webViews[parentPaneID] | |
| 1806 | + | wv := c.getWebViewLocked(parentPaneID) | |
| 1771 | 1807 | if wv == nil || wv.IsDestroyed() { | |
| 1772 | 1808 | log.Debug(). | |
| 1773 | 1809 | Str("parent_pane_id", string(parentPaneID)). | |
| @@ -1834,7 +1870,7 @@ func (c *ContentCoordinator) handleLinkMiddleClick(ctx context.Context, parentPa | |||
| 1834 | 1870 | newPane.URI = uri | |
| 1835 | 1871 | ||
| 1836 | 1872 | // Register WebView | |
| 1837 | - | c.webViews[paneID] = newWV | |
| 1873 | + | c.setWebViewLocked(paneID, newWV) | |
| 1838 | 1874 | ||
| 1839 | 1875 | // Setup callbacks | |
| 1840 | 1876 | c.setupWebViewCallbacks(ctx, paneID, newWV) | |
| @@ -93,6 +93,7 @@ type ContentCoordinator struct { | |||||
| 93 | injector *webkit.ContentInjector | 93 | injector *webkit.ContentInjector | ||
| 94 | 94 | ||||
| 95 | webViews map[entity.PaneID]*webkit.WebView | 95 | webViews map[entity.PaneID]*webkit.WebView | ||
| 96 | + | webViewsMu sync.RWMutex | |||
| 96 | paneTitles map[entity.PaneID]string | 97 | paneTitles map[entity.PaneID]string | ||
| 97 | titleMu sync.RWMutex | 98 | titleMu sync.RWMutex | ||
| 98 | 99 | ||||
| @@ -261,7 +262,7 @@ func (c *ContentCoordinator) SetOnFirstLoadStarted(fn func()) { | |||||
| 261 | func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (*webkit.WebView, error) { | 262 | func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (*webkit.WebView, error) { | ||
| 262 | log := logging.FromContext(ctx) | 263 | log := logging.FromContext(ctx) | ||
| 263 | 264 | ||||
| 264 | - | if wv, ok := c.webViews[paneID]; ok && wv != nil && !wv.IsDestroyed() { | 265 | + | if wv := c.getWebViewLocked(paneID); wv != nil && !wv.IsDestroyed() { |
| 265 | return wv, nil | 266 | return wv, nil | ||
| 266 | } | 267 | } | ||
| 267 | 268 | ||||
| @@ -270,7 +271,7 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||||
| 270 | } | 271 | } | ||
| 271 | 272 | ||||
| 272 | // Mark tab_created on first webview (first tab) | 273 | // Mark tab_created on first webview (first tab) | ||
| 273 | - | if len(c.webViews) == 0 { | 274 | + | if c.webViewCount() == 0 { |
| 274 | logging.Trace().Mark("tab_created") | 275 | logging.Trace().Mark("tab_created") | ||
| 275 | } | 276 | } | ||
| 276 | 277 | ||||
| @@ -280,7 +281,7 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||||
| 280 | } | 281 | } | ||
| 281 | logging.Trace().Mark("webview_acquired") | 282 | logging.Trace().Mark("webview_acquired") | ||
| 282 | 283 | ||||
| 283 | - | c.webViews[paneID] = wv | 284 | + | c.setWebViewLocked(paneID, wv) |
| 284 | c.setupWebViewCallbacks(ctx, paneID, wv) | 285 | c.setupWebViewCallbacks(ctx, paneID, wv) | ||
| 285 | 286 | ||||
| 286 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | 287 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | ||
| @@ -291,11 +292,10 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||||
| 291 | func (c *ContentCoordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | 292 | func (c *ContentCoordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | ||
| 292 | log := logging.FromContext(ctx) | 293 | log := logging.FromContext(ctx) | ||
| 293 | 294 | ||||
| 294 | - | wv, ok := c.webViews[paneID] | 295 | + | wv := c.deleteWebViewLocked(paneID) |
| 295 | - | if !ok || wv == nil { | 296 | + | if wv == nil { |
| 296 | return | 297 | return | ||
| 297 | } | 298 | } | ||
| 298 | - | delete(c.webViews, paneID) | |||
| 299 | c.clearPendingAppearance(paneID) | 299 | c.clearPendingAppearance(paneID) | ||
| 300 | 300 | ||||
| 301 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | 301 | // CRITICAL: If this webview was inhibiting idle (fullscreen or audio playing), | ||
| @@ -422,20 +422,56 @@ func (c *ContentCoordinator) ActiveWebView(ctx context.Context) *webkit.WebView | |||||
| 422 | return nil | 422 | return nil | ||
| 423 | } | 423 | } | ||
| 424 | 424 | ||||
| 425 | - | return c.webViews[pane.Pane.ID] | 425 | + | return c.getWebViewLocked(pane.Pane.ID) |
| 426 | } | 426 | } | ||
| 427 | 427 | ||||
| 428 | // GetWebView returns the WebView for a specific pane. | 428 | // GetWebView returns the WebView for a specific pane. | ||
| 429 | func (c *ContentCoordinator) GetWebView(paneID entity.PaneID) *webkit.WebView { | 429 | func (c *ContentCoordinator) GetWebView(paneID entity.PaneID) *webkit.WebView { | ||
| 430 | - | return c.webViews[paneID] | 430 | + | return c.getWebViewLocked(paneID) |
| 431 | } | 431 | } | ||
| 432 | 432 | ||||
| 433 | // RegisterPopupWebView registers a popup WebView that was created externally. | 433 | // RegisterPopupWebView registers a popup WebView that was created externally. | ||
| 434 | // This is used when popup tabs are created and the WebView needs to be tracked. | 434 | // This is used when popup tabs are created and the WebView needs to be tracked. | ||
| 435 | func (c *ContentCoordinator) RegisterPopupWebView(paneID entity.PaneID, wv *webkit.WebView) { | 435 | func (c *ContentCoordinator) RegisterPopupWebView(paneID entity.PaneID, wv *webkit.WebView) { | ||
| 436 | if wv != nil && paneID != "" { | 436 | if wv != nil && paneID != "" { | ||
| 437 | - | c.webViews[paneID] = wv | 437 | + | c.setWebViewLocked(paneID, wv) |
| 438 | + | } | |||
| 439 | + | } | |||
| 440 | + | ||||
| 441 | + | func (c *ContentCoordinator) getWebViewLocked(paneID entity.PaneID) *webkit.WebView { | |||
| 442 | + | c.webViewsMu.RLock() | |||
| 443 | + | defer c.webViewsMu.RUnlock() | |||
| 444 | + | return c.webViews[paneID] | |||
| 445 | + | } | |||
| 446 | + | ||||
| 447 | + | func (c *ContentCoordinator) setWebViewLocked(paneID entity.PaneID, wv *webkit.WebView) { | |||
| 448 | + | c.webViewsMu.Lock() | |||
| 449 | + | c.webViews[paneID] = wv | |||
| 450 | + | c.webViewsMu.Unlock() | |||
| 451 | + | } | |||
| 452 | + | ||||
| 453 | + | func (c *ContentCoordinator) deleteWebViewLocked(paneID entity.PaneID) *webkit.WebView { | |||
| 454 | + | c.webViewsMu.Lock() | |||
| 455 | + | defer c.webViewsMu.Unlock() | |||
| 456 | + | wv := c.webViews[paneID] | |||
| 457 | + | delete(c.webViews, paneID) | |||
| 458 | + | return wv | |||
| 459 | + | } | |||
| 460 | + | ||||
| 461 | + | func (c *ContentCoordinator) webViewCount() int { | |||
| 462 | + | c.webViewsMu.RLock() | |||
| 463 | + | defer c.webViewsMu.RUnlock() | |||
| 464 | + | return len(c.webViews) | |||
| 465 | + | } | |||
| 466 | + | ||||
| 467 | + | func (c *ContentCoordinator) snapshotWebViews() map[entity.PaneID]*webkit.WebView { | |||
| 468 | + | c.webViewsMu.RLock() | |||
| 469 | + | defer c.webViewsMu.RUnlock() | |||
| 470 | + | snapshot := make(map[entity.PaneID]*webkit.WebView, len(c.webViews)) | |||
| 471 | + | for paneID, wv := range c.webViews { | |||
| 472 | + | snapshot[paneID] = wv | |||
| 438 | } | 473 | } | ||
| 474 | + | return snapshot | |||
| 439 | } | 475 | } | ||
| 440 | 476 | ||||
| 441 | // ApplyFiltersToAll applies content filters to all active webviews. | 477 | // ApplyFiltersToAll applies content filters to all active webviews. | ||
| @@ -443,7 +479,7 @@ func (c *ContentCoordinator) RegisterPopupWebView(paneID entity.PaneID, wv *webk | |||||
| 443 | func (c *ContentCoordinator) ApplyFiltersToAll(ctx context.Context, applier webkit.FilterApplier) { | 479 | func (c *ContentCoordinator) ApplyFiltersToAll(ctx context.Context, applier webkit.FilterApplier) { | ||
| 444 | log := logging.FromContext(ctx) | 480 | log := logging.FromContext(ctx) | ||
| 445 | 481 | ||||
| 446 | - | for paneID, wv := range c.webViews { | 482 | + | for paneID, wv := range c.snapshotWebViews() { |
| 447 | if wv != nil && !wv.IsDestroyed() { | 483 | if wv != nil && !wv.IsDestroyed() { | ||
| 448 | applier.ApplyTo(ctx, wv.UserContentManager()) | 484 | applier.ApplyTo(ctx, wv.UserContentManager()) | ||
| 449 | log.Debug().Str("pane_id", string(paneID)).Msg("applied filters to existing webview") | 485 | log.Debug().Str("pane_id", string(paneID)).Msg("applied filters to existing webview") | ||
| @@ -458,7 +494,7 @@ func (c *ContentCoordinator) ApplySettingsToAll(ctx context.Context, sm *webkit. | |||||
| 458 | return | 494 | return | ||
| 459 | } | 495 | } | ||
| 460 | 496 | ||||
| 461 | - | for paneID, wv := range c.webViews { | 497 | + | for paneID, wv := range c.snapshotWebViews() { |
| 462 | if wv == nil || wv.IsDestroyed() { | 498 | if wv == nil || wv.IsDestroyed() { | ||
| 463 | continue | 499 | continue | ||
| 464 | } | 500 | } | ||
| @@ -802,7 +838,7 @@ func (c *ContentCoordinator) onTitleChanged(ctx context.Context, paneID entity.P | |||||
| 802 | 838 | ||||
| 803 | // Notify history persistence (get URL from WebView) | 839 | // Notify history persistence (get URL from WebView) | ||
| 804 | if c.onTitleUpdated != nil { | 840 | if c.onTitleUpdated != nil { | ||
| 805 | - | if wv := c.webViews[paneID]; wv != nil { | 841 | + | if wv := c.getWebViewLocked(paneID); wv != nil { |
| 806 | currentURI := wv.URI() | 842 | currentURI := wv.URI() | ||
| 807 | if currentURI != "" && title != "" { | 843 | if currentURI != "" && title != "" { | ||
| 808 | c.onTitleUpdated(ctx, paneID, currentURI, title) | 844 | c.onTitleUpdated(ctx, paneID, currentURI, title) | ||
| @@ -849,7 +885,7 @@ func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity | |||||
| 849 | log := logging.FromContext(ctx) | 885 | log := logging.FromContext(ctx) | ||
| 850 | 886 | ||||
| 851 | // Get current URI to extract domain for caching | 887 | // Get current URI to extract domain for caching | ||
| 852 | - | wv := c.webViews[paneID] | 888 | + | wv := c.getWebViewLocked(paneID) |
| 853 | if wv == nil { | 889 | if wv == nil { | ||
| 854 | return | 890 | return | ||
| 855 | } | 891 | } | ||
| @@ -1159,7 +1195,7 @@ func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity. | |||||
| 1159 | return | 1195 | return | ||
| 1160 | } | 1196 | } | ||
| 1161 | 1197 | ||||
| 1162 | - | wv := c.webViews[paneID] | 1198 | + | wv := c.getWebViewLocked(paneID) |
| 1163 | if wv == nil || wv.IsDestroyed() { | 1199 | if wv == nil || wv.IsDestroyed() { | ||
| 1164 | return | 1200 | return | ||
| 1165 | } | 1201 | } | ||
| @@ -1351,7 +1387,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1351 | } | 1387 | } | ||
| 1352 | 1388 | ||||
| 1353 | // Register WebView in our map (after successful insertion) | 1389 | // Register WebView in our map (after successful insertion) | ||
| 1354 | - | c.webViews[paneID] = popupWV | 1390 | + | c.setWebViewLocked(paneID, popupWV) |
| 1355 | 1391 | ||||
| 1356 | // Setup standard callbacks (after successful insertion to avoid leak) | 1392 | // Setup standard callbacks (after successful insertion to avoid leak) | ||
| 1357 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | 1393 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | ||
| @@ -1767,7 +1803,7 @@ func (c *ContentCoordinator) refreshPaneAfterOAuth( | |||||
| 1767 | popupID port.WebViewID, | 1803 | popupID port.WebViewID, | ||
| 1768 | ) { | 1804 | ) { | ||
| 1769 | log := logging.FromContext(ctx) | 1805 | log := logging.FromContext(ctx) | ||
| 1770 | - | wv := c.webViews[parentPaneID] | 1806 | + | wv := c.getWebViewLocked(parentPaneID) |
| 1771 | if wv == nil || wv.IsDestroyed() { | 1807 | if wv == nil || wv.IsDestroyed() { | ||
| 1772 | log.Debug(). | 1808 | log.Debug(). | ||
| 1773 | Str("parent_pane_id", string(parentPaneID)). | 1809 | Str("parent_pane_id", string(parentPaneID)). | ||
| @@ -1834,7 +1870,7 @@ func (c *ContentCoordinator) handleLinkMiddleClick(ctx context.Context, parentPa | |||||
| 1834 | newPane.URI = uri | 1870 | newPane.URI = uri | ||
| 1835 | 1871 | ||||
| 1836 | // Register WebView | 1872 | // Register WebView | ||
| 1837 | - | c.webViews[paneID] = newWV | 1873 | + | c.setWebViewLocked(paneID, newWV) |
| 1838 | 1874 | ||||
| 1839 | // Setup callbacks | 1875 | // Setup callbacks | ||
| 1840 | c.setupWebViewCallbacks(ctx, paneID, newWV) | 1876 | c.setupWebViewCallbacks(ctx, paneID, newWV) | ||