Merge pull request #126 from bnema/fix/stability-improvements
Stability improvements and GTK logging config
20 files · +851 -180
M CHANGELOG.md +7 -0
| @@ -2,6 +2,13 @@ | |||
| 2 | 2 | ||
| 3 | 3 | All notable changes to this project will be documented in this file. | |
| 4 | 4 | ||
| 5 | + | ## [Unreleased] | |
| 6 | + | ||
| 7 | + | ### Fixed | |
| 8 | + | - **Session snapshot FK constraint**: Fixed SQLite foreign key constraint violation when snapshot service tried to save before session was persisted to database. Added ready flag and callback notification pattern. | |
| 9 | + | - **Stacked pane index out of bounds**: Fixed crash when domain model and UI got out of sync during stacked pane operations. Added bounds checking and proper rollback when UI updates fail. | |
| 10 | + | - **Noisy gesture navigation logs**: Mouse button back/forward navigation errors (expected when no history) now log at debug level instead of error level. | |
| 11 | + | ||
| 5 | 12 | ## [0.25.0] - 2026-01-08 | |
| 6 | 13 | ||
| 7 | 14 | ### Added | |
| @@ -2,6 +2,13 @@ | |||||
| 2 | 2 | ||||
| 3 | All notable changes to this project will be documented in this file. | 3 | All notable changes to this project will be documented in this file. | ||
| 4 | 4 | ||||
| 5 | + | ## [Unreleased] | |||
| 6 | + | ||||
| 7 | + | ### Fixed | |||
| 8 | + | - **Session snapshot FK constraint**: Fixed SQLite foreign key constraint violation when snapshot service tried to save before session was persisted to database. Added ready flag and callback notification pattern. | |||
| 9 | + | - **Stacked pane index out of bounds**: Fixed crash when domain model and UI got out of sync during stacked pane operations. Added bounds checking and proper rollback when UI updates fail. | |||
| 10 | + | - **Noisy gesture navigation logs**: Mouse button back/forward navigation errors (expected when no history) now log at debug level instead of error level. | |||
| 11 | + | ||||
| 5 | ## [0.25.0] - 2026-01-08 | 12 | ## [0.25.0] - 2026-01-08 | ||
| 6 | 13 | ||||
| 7 | ### Added | 14 | ### Added | ||
M cmd/dumber/main.go +96 -32
| @@ -29,6 +29,7 @@ import ( | |||
| 29 | 29 | "github.com/bnema/dumber/internal/logging" | |
| 30 | 30 | "github.com/bnema/dumber/internal/ui" | |
| 31 | 31 | "github.com/bnema/dumber/internal/ui/theme" | |
| 32 | + | "github.com/rs/zerolog" | |
| 32 | 33 | ) | |
| 33 | 34 | ||
| 34 | 35 | // Build-time variables (set via ldflags). | |
| @@ -75,17 +76,12 @@ func runGUI() int { | |||
| 75 | 76 | cfg := initConfig() | |
| 76 | 77 | timer.Mark("config") | |
| 77 | 78 | ||
| 78 | - | ctx := initStartupContext(cfg) | |
| 79 | + | ctx := initStartupContextWithTrace(cfg) | |
| 79 | 80 | timer.Mark("logger") | |
| 80 | 81 | bootstrapLog := logging.FromContext(ctx) | |
| 81 | 82 | ||
| 82 | - | // Parallel phase: directories and theme setup | |
| 83 | - | initResult, err := bootstrap.RunParallelInit(bootstrap.ParallelInitInput{ | |
| 84 | - | Ctx: ctx, | |
| 85 | - | Config: cfg, | |
| 86 | - | }) | |
| 83 | + | initResult, err := runParallelInitPhase(ctx, cfg) | |
| 87 | 84 | if err != nil { | |
| 88 | - | handleParallelInitError(ctx, err) | |
| 89 | 85 | return 1 | |
| 90 | 86 | } | |
| 91 | 87 | timer.MarkDuration("parallel_phase", initResult.Duration) | |
| @@ -101,44 +97,27 @@ func runGUI() int { | |||
| 101 | 97 | } | |
| 102 | 98 | timer.Mark("db_webkit_parallel") | |
| 103 | 99 | ||
| 104 | - | browserSession, sessionCtx, err := bootstrap.StartBrowserSession(ctx, cfg, repos.session, !needsEagerDB) | |
| 105 | - | if err != nil { | |
| 106 | - | bootstrapLog.Fatal().Err(err).Msg("failed to start session") | |
| 107 | - | } | |
| 108 | - | if browserSession.LogCleanup != nil { | |
| 109 | - | defer browserSession.LogCleanup() | |
| 110 | - | } | |
| 111 | - | defer func() { _ = browserSession.End(sessionCtx) }() | |
| 100 | + | ctx, browserSession, sessionCleanup := initBrowserSession(ctx, cfg, repos, bootstrapLog) | |
| 101 | + | defer sessionCleanup() | |
| 112 | 102 | timer.Mark("session") | |
| 113 | 103 | ||
| 114 | - | ctx = sessionCtx | |
| 115 | 104 | log := logging.FromContext(ctx) | |
| 105 | + | logging.Trace().UpdateLogger(log) | |
| 106 | + | ||
| 116 | 107 | if stack.MessageRouter != nil { | |
| 117 | 108 | stack.MessageRouter.SetBaseContext(ctx) | |
| 118 | 109 | } | |
| 119 | 110 | ||
| 120 | - | // Repositories and use cases | |
| 121 | 111 | useCases := createUseCases(repos, cfg) | |
| 122 | 112 | if needsEagerDB { | |
| 123 | 113 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | |
| 124 | 114 | } | |
| 125 | 115 | ||
| 126 | 116 | idleInhibitor := idle.NewPortalInhibitor(ctx) | |
| 127 | - | defer func() { | |
| 128 | - | if idleInhibitor != nil { | |
| 129 | - | _ = idleInhibitor.Close() | |
| 130 | - | } | |
| 131 | - | }() | |
| 117 | + | defer closeIdleInhibitor(idleInhibitor) | |
| 132 | 118 | timer.Mark("use_cases") | |
| 133 | 119 | ||
| 134 | - | // Build UI | |
| 135 | - | uiDeps := buildUIDependencies( | |
| 136 | - | ctx, cfg, initResult.ThemeManager, | |
| 137 | - | initResult.ColorResolver, initResult.AdwaitaDetector, | |
| 138 | - | &stack, repos, useCases, idleInhibitor, browserSession.Session.ID, | |
| 139 | - | ) | |
| 140 | - | configureDeferredInit(uiDeps, cfg, browserSession) | |
| 141 | - | app, err := ui.New(uiDeps) | |
| 120 | + | app, err := buildAndConfigureApp(ctx, cfg, initResult, &stack, repos, useCases, idleInhibitor, browserSession) | |
| 142 | 121 | if err != nil { | |
| 143 | 122 | log.Error().Err(err).Msg("failed to create application") | |
| 144 | 123 | return 1 | |
| @@ -146,12 +125,83 @@ func runGUI() int { | |||
| 146 | 125 | timer.Mark("ui_deps") | |
| 147 | 126 | timer.Log(ctx) | |
| 148 | 127 | ||
| 149 | - | // Signal handling | |
| 150 | 128 | setupSignalHandler(ctx, app) | |
| 151 | 129 | ||
| 152 | 130 | return app.Run(ctx, os.Args) | |
| 153 | 131 | } | |
| 154 | 132 | ||
| 133 | + | func initStartupContextWithTrace(cfg *config.Config) context.Context { | |
| 134 | + | logging.InitStartupTrace(cfg.Logging.Level) | |
| 135 | + | logging.Trace().Mark("config_loaded") | |
| 136 | + | ||
| 137 | + | ctx := initStartupContext(cfg) | |
| 138 | + | bootstrapLog := logging.FromContext(ctx) | |
| 139 | + | ||
| 140 | + | logging.Trace().SetLogger(bootstrapLog) | |
| 141 | + | logging.Trace().Mark("logger_init") | |
| 142 | + | ||
| 143 | + | return ctx | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | func runParallelInitPhase(ctx context.Context, cfg *config.Config) (*bootstrap.ParallelInitResult, error) { | |
| 147 | + | logging.Trace().Mark("parallel_start") | |
| 148 | + | initResult, err := bootstrap.RunParallelInit(bootstrap.ParallelInitInput{ | |
| 149 | + | Ctx: ctx, | |
| 150 | + | Config: cfg, | |
| 151 | + | }) | |
| 152 | + | if err != nil { | |
| 153 | + | handleParallelInitError(ctx, err) | |
| 154 | + | return nil, err | |
| 155 | + | } | |
| 156 | + | logging.Trace().Mark("parallel_done") | |
| 157 | + | return initResult, nil | |
| 158 | + | } | |
| 159 | + | ||
| 160 | + | func initBrowserSession( | |
| 161 | + | ctx context.Context, | |
| 162 | + | cfg *config.Config, | |
| 163 | + | repos *repositories, | |
| 164 | + | bootstrapLog *zerolog.Logger, | |
| 165 | + | ) (context.Context, *bootstrap.BrowserSession, func()) { | |
| 166 | + | deferPersist := restoreSessionID == "" && !cfg.Session.AutoRestore | |
| 167 | + | browserSession, sessionCtx, err := bootstrap.StartBrowserSession(ctx, cfg, repos.session, deferPersist) | |
| 168 | + | if err != nil { | |
| 169 | + | bootstrapLog.Fatal().Err(err).Msg("failed to start session") | |
| 170 | + | } | |
| 171 | + | cleanup := func() { | |
| 172 | + | if browserSession.LogCleanup != nil { | |
| 173 | + | browserSession.LogCleanup() | |
| 174 | + | } | |
| 175 | + | _ = browserSession.End(sessionCtx) | |
| 176 | + | } | |
| 177 | + | return sessionCtx, browserSession, cleanup | |
| 178 | + | } | |
| 179 | + | ||
| 180 | + | func closeIdleInhibitor(inhibitor port.IdleInhibitor) { | |
| 181 | + | if inhibitor != nil { | |
| 182 | + | _ = inhibitor.Close() | |
| 183 | + | } | |
| 184 | + | } | |
| 185 | + | ||
| 186 | + | func buildAndConfigureApp( | |
| 187 | + | ctx context.Context, | |
| 188 | + | cfg *config.Config, | |
| 189 | + | initResult *bootstrap.ParallelInitResult, | |
| 190 | + | stack *bootstrap.WebKitStack, | |
| 191 | + | repos *repositories, | |
| 192 | + | useCases *useCases, | |
| 193 | + | idleInhibitor port.IdleInhibitor, | |
| 194 | + | browserSession *bootstrap.BrowserSession, | |
| 195 | + | ) (*ui.App, error) { | |
| 196 | + | uiDeps := buildUIDependencies( | |
| 197 | + | ctx, cfg, initResult.ThemeManager, | |
| 198 | + | initResult.ColorResolver, initResult.AdwaitaDetector, | |
| 199 | + | stack, repos, useCases, idleInhibitor, browserSession.Session.ID, | |
| 200 | + | ) | |
| 201 | + | configureDeferredInit(uiDeps, cfg, browserSession) | |
| 202 | + | return ui.New(uiDeps) | |
| 203 | + | } | |
| 204 | + | ||
| 155 | 205 | func initStartupContext(cfg *config.Config) context.Context { | |
| 156 | 206 | deps.ApplyPrefixEnv(cfg.Runtime.Prefix) | |
| 157 | 207 | bootstrapLogger := logging.NewFromConfigValuesWithTimeFormat( | |
| @@ -217,6 +267,11 @@ func configureDeferredInit( | |||
| 217 | 267 | if uiDeps == nil { | |
| 218 | 268 | return | |
| 219 | 269 | } | |
| 270 | + | ||
| 271 | + | // If session was already persisted (not deferred), mark snapshot service ready immediately | |
| 272 | + | // via callback that will be set by the app after initialization | |
| 273 | + | sessionAlreadyPersisted := session == nil || session.Persist == nil | |
| 274 | + | ||
| 220 | 275 | uiDeps.OnFirstWebViewShown = func(cbCtx context.Context) { | |
| 221 | 276 | logger := logging.FromContext(cbCtx) | |
| 222 | 277 | bgCtx := logging.WithContext(context.Background(), *logger) | |
| @@ -227,10 +282,19 @@ func configureDeferredInit( | |||
| 227 | 282 | }) | |
| 228 | 283 | logDeferredInitResults(bgCtx, result) | |
| 229 | 284 | }() | |
| 230 | - | if session != nil && session.Persist != nil { | |
| 285 | + | ||
| 286 | + | if sessionAlreadyPersisted { | |
| 287 | + | // Session was persisted eagerly, notify immediately | |
| 288 | + | if uiDeps.OnSessionPersisted != nil { | |
| 289 | + | uiDeps.OnSessionPersisted() | |
| 290 | + | } | |
| 291 | + | } else if session.Persist != nil { | |
| 292 | + | // Session persist is deferred, notify after it completes | |
| 231 | 293 | go func() { | |
| 232 | 294 | if persistErr := session.Persist(bgCtx); persistErr != nil { | |
| 233 | 295 | logger.Error().Err(persistErr).Msg("deferred session persistence failed") | |
| 296 | + | } else if uiDeps.OnSessionPersisted != nil { | |
| 297 | + | uiDeps.OnSessionPersisted() | |
| 234 | 298 | } | |
| 235 | 299 | }() | |
| 236 | 300 | } | |
| @@ -29,6 +29,7 @@ import ( | |||||
| 29 | "github.com/bnema/dumber/internal/logging" | 29 | "github.com/bnema/dumber/internal/logging" | ||
| 30 | "github.com/bnema/dumber/internal/ui" | 30 | "github.com/bnema/dumber/internal/ui" | ||
| 31 | "github.com/bnema/dumber/internal/ui/theme" | 31 | "github.com/bnema/dumber/internal/ui/theme" | ||
| 32 | + | "github.com/rs/zerolog" | |||
| 32 | ) | 33 | ) | ||
| 33 | 34 | ||||
| 34 | // Build-time variables (set via ldflags). | 35 | // Build-time variables (set via ldflags). | ||
| @@ -75,17 +76,12 @@ func runGUI() int { | |||||
| 75 | cfg := initConfig() | 76 | cfg := initConfig() | ||
| 76 | timer.Mark("config") | 77 | timer.Mark("config") | ||
| 77 | 78 | ||||
| 78 | - | ctx := initStartupContext(cfg) | 79 | + | ctx := initStartupContextWithTrace(cfg) |
| 79 | timer.Mark("logger") | 80 | timer.Mark("logger") | ||
| 80 | bootstrapLog := logging.FromContext(ctx) | 81 | bootstrapLog := logging.FromContext(ctx) | ||
| 81 | 82 | ||||
| 82 | - | // Parallel phase: directories and theme setup | 83 | + | initResult, err := runParallelInitPhase(ctx, cfg) |
| 83 | - | initResult, err := bootstrap.RunParallelInit(bootstrap.ParallelInitInput{ | |||
| 84 | - | Ctx: ctx, | |||
| 85 | - | Config: cfg, | |||
| 86 | - | }) | |||
| 87 | if err != nil { | 84 | if err != nil { | ||
| 88 | - | handleParallelInitError(ctx, err) | |||
| 89 | return 1 | 85 | return 1 | ||
| 90 | } | 86 | } | ||
| 91 | timer.MarkDuration("parallel_phase", initResult.Duration) | 87 | timer.MarkDuration("parallel_phase", initResult.Duration) | ||
| @@ -101,44 +97,27 @@ func runGUI() int { | |||||
| 101 | } | 97 | } | ||
| 102 | timer.Mark("db_webkit_parallel") | 98 | timer.Mark("db_webkit_parallel") | ||
| 103 | 99 | ||||
| 104 | - | browserSession, sessionCtx, err := bootstrap.StartBrowserSession(ctx, cfg, repos.session, !needsEagerDB) | 100 | + | ctx, browserSession, sessionCleanup := initBrowserSession(ctx, cfg, repos, bootstrapLog) |
| 105 | - | if err != nil { | 101 | + | defer sessionCleanup() |
| 106 | - | bootstrapLog.Fatal().Err(err).Msg("failed to start session") | |||
| 107 | - | } | |||
| 108 | - | if browserSession.LogCleanup != nil { | |||
| 109 | - | defer browserSession.LogCleanup() | |||
| 110 | - | } | |||
| 111 | - | defer func() { _ = browserSession.End(sessionCtx) }() | |||
| 112 | timer.Mark("session") | 102 | timer.Mark("session") | ||
| 113 | 103 | ||||
| 114 | - | ctx = sessionCtx | |||
| 115 | log := logging.FromContext(ctx) | 104 | log := logging.FromContext(ctx) | ||
| 105 | + | logging.Trace().UpdateLogger(log) | |||
| 106 | + | ||||
| 116 | if stack.MessageRouter != nil { | 107 | if stack.MessageRouter != nil { | ||
| 117 | stack.MessageRouter.SetBaseContext(ctx) | 108 | stack.MessageRouter.SetBaseContext(ctx) | ||
| 118 | } | 109 | } | ||
| 119 | 110 | ||||
| 120 | - | // Repositories and use cases | |||
| 121 | useCases := createUseCases(repos, cfg) | 111 | useCases := createUseCases(repos, cfg) | ||
| 122 | if needsEagerDB { | 112 | if needsEagerDB { | ||
| 123 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | 113 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | ||
| 124 | } | 114 | } | ||
| 125 | 115 | ||||
| 126 | idleInhibitor := idle.NewPortalInhibitor(ctx) | 116 | idleInhibitor := idle.NewPortalInhibitor(ctx) | ||
| 127 | - | defer func() { | 117 | + | defer closeIdleInhibitor(idleInhibitor) |
| 128 | - | if idleInhibitor != nil { | |||
| 129 | - | _ = idleInhibitor.Close() | |||
| 130 | - | } | |||
| 131 | - | }() | |||
| 132 | timer.Mark("use_cases") | 118 | timer.Mark("use_cases") | ||
| 133 | 119 | ||||
| 134 | - | // Build UI | 120 | + | app, err := buildAndConfigureApp(ctx, cfg, initResult, &stack, repos, useCases, idleInhibitor, browserSession) |
| 135 | - | uiDeps := buildUIDependencies( | |||
| 136 | - | ctx, cfg, initResult.ThemeManager, | |||
| 137 | - | initResult.ColorResolver, initResult.AdwaitaDetector, | |||
| 138 | - | &stack, repos, useCases, idleInhibitor, browserSession.Session.ID, | |||
| 139 | - | ) | |||
| 140 | - | configureDeferredInit(uiDeps, cfg, browserSession) | |||
| 141 | - | app, err := ui.New(uiDeps) | |||
| 142 | if err != nil { | 121 | if err != nil { | ||
| 143 | log.Error().Err(err).Msg("failed to create application") | 122 | log.Error().Err(err).Msg("failed to create application") | ||
| 144 | return 1 | 123 | return 1 | ||
| @@ -146,12 +125,83 @@ func runGUI() int { | |||||
| 146 | timer.Mark("ui_deps") | 125 | timer.Mark("ui_deps") | ||
| 147 | timer.Log(ctx) | 126 | timer.Log(ctx) | ||
| 148 | 127 | ||||
| 149 | - | // Signal handling | |||
| 150 | setupSignalHandler(ctx, app) | 128 | setupSignalHandler(ctx, app) | ||
| 151 | 129 | ||||
| 152 | return app.Run(ctx, os.Args) | 130 | return app.Run(ctx, os.Args) | ||
| 153 | } | 131 | } | ||
| 154 | 132 | ||||
| 133 | + | func initStartupContextWithTrace(cfg *config.Config) context.Context { | |||
| 134 | + | logging.InitStartupTrace(cfg.Logging.Level) | |||
| 135 | + | logging.Trace().Mark("config_loaded") | |||
| 136 | + | ||||
| 137 | + | ctx := initStartupContext(cfg) | |||
| 138 | + | bootstrapLog := logging.FromContext(ctx) | |||
| 139 | + | ||||
| 140 | + | logging.Trace().SetLogger(bootstrapLog) | |||
| 141 | + | logging.Trace().Mark("logger_init") | |||
| 142 | + | ||||
| 143 | + | return ctx | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | func runParallelInitPhase(ctx context.Context, cfg *config.Config) (*bootstrap.ParallelInitResult, error) { | |||
| 147 | + | logging.Trace().Mark("parallel_start") | |||
| 148 | + | initResult, err := bootstrap.RunParallelInit(bootstrap.ParallelInitInput{ | |||
| 149 | + | Ctx: ctx, | |||
| 150 | + | Config: cfg, | |||
| 151 | + | }) | |||
| 152 | + | if err != nil { | |||
| 153 | + | handleParallelInitError(ctx, err) | |||
| 154 | + | return nil, err | |||
| 155 | + | } | |||
| 156 | + | logging.Trace().Mark("parallel_done") | |||
| 157 | + | return initResult, nil | |||
| 158 | + | } | |||
| 159 | + | ||||
| 160 | + | func initBrowserSession( | |||
| 161 | + | ctx context.Context, | |||
| 162 | + | cfg *config.Config, | |||
| 163 | + | repos *repositories, | |||
| 164 | + | bootstrapLog *zerolog.Logger, | |||
| 165 | + | ) (context.Context, *bootstrap.BrowserSession, func()) { | |||
| 166 | + | deferPersist := restoreSessionID == "" && !cfg.Session.AutoRestore | |||
| 167 | + | browserSession, sessionCtx, err := bootstrap.StartBrowserSession(ctx, cfg, repos.session, deferPersist) | |||
| 168 | + | if err != nil { | |||
| 169 | + | bootstrapLog.Fatal().Err(err).Msg("failed to start session") | |||
| 170 | + | } | |||
| 171 | + | cleanup := func() { | |||
| 172 | + | if browserSession.LogCleanup != nil { | |||
| 173 | + | browserSession.LogCleanup() | |||
| 174 | + | } | |||
| 175 | + | _ = browserSession.End(sessionCtx) | |||
| 176 | + | } | |||
| 177 | + | return sessionCtx, browserSession, cleanup | |||
| 178 | + | } | |||
| 179 | + | ||||
| 180 | + | func closeIdleInhibitor(inhibitor port.IdleInhibitor) { | |||
| 181 | + | if inhibitor != nil { | |||
| 182 | + | _ = inhibitor.Close() | |||
| 183 | + | } | |||
| 184 | + | } | |||
| 185 | + | ||||
| 186 | + | func buildAndConfigureApp( | |||
| 187 | + | ctx context.Context, | |||
| 188 | + | cfg *config.Config, | |||
| 189 | + | initResult *bootstrap.ParallelInitResult, | |||
| 190 | + | stack *bootstrap.WebKitStack, | |||
| 191 | + | repos *repositories, | |||
| 192 | + | useCases *useCases, | |||
| 193 | + | idleInhibitor port.IdleInhibitor, | |||
| 194 | + | browserSession *bootstrap.BrowserSession, | |||
| 195 | + | ) (*ui.App, error) { | |||
| 196 | + | uiDeps := buildUIDependencies( | |||
| 197 | + | ctx, cfg, initResult.ThemeManager, | |||
| 198 | + | initResult.ColorResolver, initResult.AdwaitaDetector, | |||
| 199 | + | stack, repos, useCases, idleInhibitor, browserSession.Session.ID, | |||
| 200 | + | ) | |||
| 201 | + | configureDeferredInit(uiDeps, cfg, browserSession) | |||
| 202 | + | return ui.New(uiDeps) | |||
| 203 | + | } | |||
| 204 | + | ||||
| 155 | func initStartupContext(cfg *config.Config) context.Context { | 205 | func initStartupContext(cfg *config.Config) context.Context { | ||
| 156 | deps.ApplyPrefixEnv(cfg.Runtime.Prefix) | 206 | deps.ApplyPrefixEnv(cfg.Runtime.Prefix) | ||
| 157 | bootstrapLogger := logging.NewFromConfigValuesWithTimeFormat( | 207 | bootstrapLogger := logging.NewFromConfigValuesWithTimeFormat( | ||
| @@ -217,6 +267,11 @@ func configureDeferredInit( | |||||
| 217 | if uiDeps == nil { | 267 | if uiDeps == nil { | ||
| 218 | return | 268 | return | ||
| 219 | } | 269 | } | ||
| 270 | + | ||||
| 271 | + | // If session was already persisted (not deferred), mark snapshot service ready immediately | |||
| 272 | + | // via callback that will be set by the app after initialization | |||
| 273 | + | sessionAlreadyPersisted := session == nil || session.Persist == nil | |||
| 274 | + | ||||
| 220 | uiDeps.OnFirstWebViewShown = func(cbCtx context.Context) { | 275 | uiDeps.OnFirstWebViewShown = func(cbCtx context.Context) { | ||
| 221 | logger := logging.FromContext(cbCtx) | 276 | logger := logging.FromContext(cbCtx) | ||
| 222 | bgCtx := logging.WithContext(context.Background(), *logger) | 277 | bgCtx := logging.WithContext(context.Background(), *logger) | ||
| @@ -227,10 +282,19 @@ func configureDeferredInit( | |||||
| 227 | }) | 282 | }) | ||
| 228 | logDeferredInitResults(bgCtx, result) | 283 | logDeferredInitResults(bgCtx, result) | ||
| 229 | }() | 284 | }() | ||
| 230 | - | if session != nil && session.Persist != nil { | 285 | + | |
| 286 | + | if sessionAlreadyPersisted { | |||
| 287 | + | // Session was persisted eagerly, notify immediately | |||
| 288 | + | if uiDeps.OnSessionPersisted != nil { | |||
| 289 | + | uiDeps.OnSessionPersisted() | |||
| 290 | + | } | |||
| 291 | + | } else if session.Persist != nil { | |||
| 292 | + | // Session persist is deferred, notify after it completes | |||
| 231 | go func() { | 293 | go func() { | ||
| 232 | if persistErr := session.Persist(bgCtx); persistErr != nil { | 294 | if persistErr := session.Persist(bgCtx); persistErr != nil { | ||
| 233 | logger.Error().Err(persistErr).Msg("deferred session persistence failed") | 295 | logger.Error().Err(persistErr).Msg("deferred session persistence failed") | ||
| 296 | + | } else if uiDeps.OnSessionPersisted != nil { | |||
| 297 | + | uiDeps.OnSessionPersisted() | |||
| 234 | } | 298 | } | ||
| 235 | }() | 299 | }() | ||
| 236 | } | 300 | } | ||
M internal/application/usecase/navigate.go +5 -2
| @@ -17,6 +17,9 @@ const ( | |||
| 17 | 17 | // historyQueueSize is the buffer size for the async history queue. | |
| 18 | 18 | // If the queue is full, new records are dropped with a warning. | |
| 19 | 19 | historyQueueSize = 100 | |
| 20 | + | ||
| 21 | + | // logURLMaxLen is the max length for URLs in log messages. | |
| 22 | + | logURLMaxLen = 60 | |
| 20 | 23 | ) | |
| 21 | 24 | ||
| 22 | 25 | // historyRecord holds data for async history recording. | |
| @@ -137,7 +140,7 @@ func (uc *NavigateUseCase) RecordHistory(ctx context.Context, url string) { | |||
| 137 | 140 | // Non-blocking send to async queue | |
| 138 | 141 | select { | |
| 139 | 142 | case uc.historyQueue <- historyRecord{url: url, timestamp: now}: | |
| 140 | - | log.Debug().Str("url", url).Msg("history record queued") | |
| 143 | + | log.Debug().Str("url", logging.TruncateURL(url, logURLMaxLen)).Msg("history record queued") | |
| 141 | 144 | default: | |
| 142 | 145 | // Queue full - log warning but don't block navigation | |
| 143 | 146 | log.Warn().Str("url", url).Msg("history queue full, dropping record") | |
| @@ -205,7 +208,7 @@ func (uc *NavigateUseCase) UpdateHistoryTitle(ctx context.Context, url, title st | |||
| 205 | 208 | ||
| 206 | 209 | // Normalize URL to match storage (e.g., github.com/ -> github.com) | |
| 207 | 210 | url = normalizeURLForHistory(url) | |
| 208 | - | log.Debug().Str("url", url).Str("title", title).Msg("updating history title") | |
| 211 | + | log.Debug().Str("url", logging.TruncateURL(url, logURLMaxLen)).Str("title", title).Msg("updating history title") | |
| 209 | 212 | ||
| 210 | 213 | entry, err := uc.historyRepo.FindByURL(ctx, url) | |
| 211 | 214 | if err != nil { | |
| @@ -17,6 +17,9 @@ const ( | |||||
| 17 | // historyQueueSize is the buffer size for the async history queue. | 17 | // historyQueueSize is the buffer size for the async history queue. | ||
| 18 | // If the queue is full, new records are dropped with a warning. | 18 | // If the queue is full, new records are dropped with a warning. | ||
| 19 | historyQueueSize = 100 | 19 | historyQueueSize = 100 | ||
| 20 | + | ||||
| 21 | + | // logURLMaxLen is the max length for URLs in log messages. | |||
| 22 | + | logURLMaxLen = 60 | |||
| 20 | ) | 23 | ) | ||
| 21 | 24 | ||||
| 22 | // historyRecord holds data for async history recording. | 25 | // historyRecord holds data for async history recording. | ||
| @@ -137,7 +140,7 @@ func (uc *NavigateUseCase) RecordHistory(ctx context.Context, url string) { | |||||
| 137 | // Non-blocking send to async queue | 140 | // Non-blocking send to async queue | ||
| 138 | select { | 141 | select { | ||
| 139 | case uc.historyQueue <- historyRecord{url: url, timestamp: now}: | 142 | case uc.historyQueue <- historyRecord{url: url, timestamp: now}: | ||
| 140 | - | log.Debug().Str("url", url).Msg("history record queued") | 143 | + | log.Debug().Str("url", logging.TruncateURL(url, logURLMaxLen)).Msg("history record queued") |
| 141 | default: | 144 | default: | ||
| 142 | // Queue full - log warning but don't block navigation | 145 | // Queue full - log warning but don't block navigation | ||
| 143 | log.Warn().Str("url", url).Msg("history queue full, dropping record") | 146 | log.Warn().Str("url", url).Msg("history queue full, dropping record") | ||
| @@ -205,7 +208,7 @@ func (uc *NavigateUseCase) UpdateHistoryTitle(ctx context.Context, url, title st | |||||
| 205 | 208 | ||||
| 206 | // Normalize URL to match storage (e.g., github.com/ -> github.com) | 209 | // Normalize URL to match storage (e.g., github.com/ -> github.com) | ||
| 207 | url = normalizeURLForHistory(url) | 210 | url = normalizeURLForHistory(url) | ||
| 208 | - | log.Debug().Str("url", url).Str("title", title).Msg("updating history title") | 211 | + | log.Debug().Str("url", logging.TruncateURL(url, logURLMaxLen)).Str("title", title).Msg("updating history title") |
| 209 | 212 | ||||
| 210 | entry, err := uc.historyRepo.FindByURL(ctx, url) | 213 | entry, err := uc.historyRepo.FindByURL(ctx, url) | ||
| 211 | if err != nil { | 214 | if err != nil { | ||
M internal/bootstrap/webkit_stack.go +84 -39
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "github.com/bnema/dumber/internal/infrastructure/env" | |
| 12 | 12 | "github.com/bnema/dumber/internal/infrastructure/filtering" | |
| 13 | 13 | "github.com/bnema/dumber/internal/infrastructure/webkit" | |
| 14 | + | "github.com/bnema/dumber/internal/logging" | |
| 14 | 15 | "github.com/bnema/dumber/internal/ui/theme" | |
| 15 | 16 | "github.com/rs/zerolog" | |
| 16 | 17 | ) | |
| @@ -38,14 +39,44 @@ type WebKitStack struct { | |||
| 38 | 39 | func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |
| 39 | 40 | ctx := input.Ctx | |
| 40 | 41 | cfg := input.Config | |
| 41 | - | dataDir := input.DataDir | |
| 42 | - | cacheDir := input.CacheDir | |
| 43 | - | themeManager := input.ThemeManager | |
| 44 | - | colorResolver := input.ColorResolver | |
| 45 | 42 | logger := input.Logger | |
| 46 | - | // Detect hardware for performance profile scaling | |
| 43 | + | ||
| 44 | + | perfSettings := surveyHardwareAndResolveProfile(ctx, cfg, logger) | |
| 45 | + | ||
| 46 | + | configureRenderingEnvironment(ctx, cfg, &perfSettings, logger) | |
| 47 | + | logging.Trace().Mark("render_env") | |
| 48 | + | ||
| 49 | + | wkOpts := buildWebKitContextOptions(input.DataDir, input.CacheDir, &perfSettings) | |
| 50 | + | wkCtx, err := webkit.NewWebKitContextWithOptions(ctx, wkOpts) | |
| 51 | + | if err != nil { | |
| 52 | + | logger.Fatal().Err(err).Msg("failed to initialize WebKit context") | |
| 53 | + | } | |
| 54 | + | logging.Trace().Mark("webkit_context") | |
| 55 | + | ||
| 56 | + | filterManager := initFilterManager(ctx, cfg, input.DataDir, logger) | |
| 57 | + | ||
| 58 | + | schemeHandler := webkit.NewDumbSchemeHandler(ctx) | |
| 59 | + | schemeHandler.SetAssets(assets.WebUIAssets) | |
| 60 | + | schemeHandler.RegisterWithContext(wkCtx) | |
| 61 | + | ||
| 62 | + | settings, injector, messageRouter := initWebKitComponents(ctx, cfg, input.ThemeManager, input.ColorResolver, logger) | |
| 63 | + | ||
| 64 | + | pool := initWebViewPool(ctx, wkCtx, settings, injector, messageRouter, input.ThemeManager, filterManager, &perfSettings, logger) | |
| 65 | + | ||
| 66 | + | return WebKitStack{ | |
| 67 | + | Context: wkCtx, | |
| 68 | + | Settings: settings, | |
| 69 | + | Injector: injector, | |
| 70 | + | MessageRouter: messageRouter, | |
| 71 | + | Pool: pool, | |
| 72 | + | FilterManager: filterManager, | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | func surveyHardwareAndResolveProfile(ctx context.Context, cfg *config.Config, logger zerolog.Logger) config.ResolvedPerformanceSettings { | |
| 47 | 77 | hwSurveyor := env.NewHardwareSurveyor() | |
| 48 | 78 | hwInfo := hwSurveyor.Survey(ctx) | |
| 79 | + | logging.Trace().Mark("hardware_survey") | |
| 49 | 80 | logger.Info(). | |
| 50 | 81 | Int("cpu_cores", hwInfo.CPUCores). | |
| 51 | 82 | Int("cpu_threads", hwInfo.CPUThreads). | |
| @@ -54,8 +85,8 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 54 | 85 | Uint64("vram_mb", hwInfo.VRAM/(1024*1024)). | |
| 55 | 86 | Msg("hardware survey completed") | |
| 56 | 87 | ||
| 57 | - | // Resolve performance profile to get actual settings | |
| 58 | 88 | perfSettings := config.ResolvePerformanceProfile(&cfg.Performance, &hwInfo) | |
| 89 | + | logging.Trace().Mark("performance_profile") | |
| 59 | 90 | logger.Info(). | |
| 60 | 91 | Str("profile", string(cfg.Performance.Profile)). | |
| 61 | 92 | Int("skia_cpu_threads", perfSettings.SkiaCPUPaintingThreads). | |
| @@ -63,17 +94,16 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 63 | 94 | Int("webview_pool_prewarm", perfSettings.WebViewPoolPrewarmCount). | |
| 64 | 95 | Msg("resolved performance profile") | |
| 65 | 96 | ||
| 66 | - | // Configure rendering environment (must happen before WebKit/GTK init) | |
| 67 | - | configureRenderingEnvironment(ctx, cfg, &perfSettings, logger) | |
| 97 | + | return perfSettings | |
| 98 | + | } | |
| 68 | 99 | ||
| 69 | - | // Build WebKitContext options with memory pressure settings | |
| 100 | + | func buildWebKitContextOptions(dataDir, cacheDir string, perfSettings *config.ResolvedPerformanceSettings) port.WebKitContextOptions { | |
| 70 | 101 | wkOpts := port.WebKitContextOptions{ | |
| 71 | 102 | DataDir: dataDir, | |
| 72 | 103 | CacheDir: cacheDir, | |
| 73 | 104 | } | |
| 74 | 105 | ||
| 75 | - | // Configure web process memory pressure if any setting is configured | |
| 76 | - | if hasWebProcessMemoryConfig(&perfSettings) { | |
| 106 | + | if hasWebProcessMemoryConfig(perfSettings) { | |
| 77 | 107 | wkOpts.WebProcessMemory = &port.MemoryPressureConfig{ | |
| 78 | 108 | MemoryLimitMB: perfSettings.WebProcessMemoryLimitMB, | |
| 79 | 109 | PollIntervalSec: perfSettings.WebProcessMemoryPollIntervalSec, | |
| @@ -82,8 +112,7 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 82 | 112 | } | |
| 83 | 113 | } | |
| 84 | 114 | ||
| 85 | - | // Configure network process memory pressure if any setting is configured | |
| 86 | - | if hasNetworkProcessMemoryConfig(&perfSettings) { | |
| 115 | + | if hasNetworkProcessMemoryConfig(perfSettings) { | |
| 87 | 116 | wkOpts.NetworkProcessMemory = &port.MemoryPressureConfig{ | |
| 88 | 117 | MemoryLimitMB: perfSettings.NetworkProcessMemoryLimitMB, | |
| 89 | 118 | PollIntervalSec: perfSettings.NetworkProcessMemoryPollIntervalSec, | |
| @@ -92,11 +121,10 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 92 | 121 | } | |
| 93 | 122 | } | |
| 94 | 123 | ||
| 95 | - | wkCtx, err := webkit.NewWebKitContextWithOptions(ctx, wkOpts) | |
| 96 | - | if err != nil { | |
| 97 | - | logger.Fatal().Err(err).Msg("failed to initialize WebKit context") | |
| 98 | - | } | |
| 124 | + | return wkOpts | |
| 125 | + | } | |
| 99 | 126 | ||
| 127 | + | func initFilterManager(ctx context.Context, cfg *config.Config, dataDir string, logger zerolog.Logger) *filtering.Manager { | |
| 100 | 128 | filterStoreDir := filepath.Join(dataDir, "filters", "store") | |
| 101 | 129 | filterJSONDir := filepath.Join(dataDir, "filters", "json") | |
| 102 | 130 | filterManager, err := filtering.NewManager(filtering.ManagerConfig{ | |
| @@ -107,20 +135,25 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 107 | 135 | }) | |
| 108 | 136 | if err != nil { | |
| 109 | 137 | logger.Warn().Err(err).Msg("failed to create filter manager, continuing without content filtering") | |
| 110 | - | } else { | |
| 111 | - | if err := filterManager.Initialize(ctx); err != nil { | |
| 112 | - | logger.Warn().Err(err).Msg("failed to initialize filters, will load async") | |
| 113 | - | } | |
| 138 | + | return nil | |
| 114 | 139 | } | |
| 140 | + | if err := filterManager.Initialize(ctx); err != nil { | |
| 141 | + | logger.Warn().Err(err).Msg("failed to initialize filters, will load async") | |
| 142 | + | } | |
| 143 | + | logging.Trace().Mark("filter_manager") | |
| 144 | + | return filterManager | |
| 145 | + | } | |
| 115 | 146 | ||
| 116 | - | schemeHandler := webkit.NewDumbSchemeHandler(ctx) | |
| 117 | - | schemeHandler.SetAssets(assets.WebUIAssets) | |
| 118 | - | schemeHandler.RegisterWithContext(wkCtx) | |
| 119 | - | ||
| 147 | + | func initWebKitComponents( | |
| 148 | + | ctx context.Context, | |
| 149 | + | cfg *config.Config, | |
| 150 | + | themeManager *theme.Manager, | |
| 151 | + | colorResolver port.ColorSchemeResolver, | |
| 152 | + | logger zerolog.Logger, | |
| 153 | + | ) (*webkit.SettingsManager, *webkit.ContentInjector, *webkit.MessageRouter) { | |
| 120 | 154 | settings := webkit.NewSettingsManager(ctx, cfg) | |
| 121 | 155 | injector := webkit.NewContentInjector(colorResolver) | |
| 122 | 156 | ||
| 123 | - | // Set up auto-copy on selection config getter | |
| 124 | 157 | injector.SetAutoCopyConfigGetter(func() bool { | |
| 125 | 158 | return config.Get().Clipboard.AutoCopyOnSelection | |
| 126 | 159 | }) | |
| @@ -132,13 +165,28 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 132 | 165 | } | |
| 133 | 166 | ||
| 134 | 167 | messageRouter := webkit.NewMessageRouter(ctx) | |
| 168 | + | logging.Trace().Mark("settings_manager") | |
| 169 | + | ||
| 170 | + | return settings, injector, messageRouter | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | func initWebViewPool( | |
| 174 | + | ctx context.Context, | |
| 175 | + | wkCtx *webkit.WebKitContext, | |
| 176 | + | settings *webkit.SettingsManager, | |
| 177 | + | injector *webkit.ContentInjector, | |
| 178 | + | messageRouter *webkit.MessageRouter, | |
| 179 | + | themeManager *theme.Manager, | |
| 180 | + | filterManager *filtering.Manager, | |
| 181 | + | perfSettings *config.ResolvedPerformanceSettings, | |
| 182 | + | logger zerolog.Logger, | |
| 183 | + | ) *webkit.WebViewPool { | |
| 135 | 184 | poolCfg := webkit.DefaultPoolConfig() | |
| 136 | - | // Override prewarm count from resolved profile settings | |
| 137 | 185 | if perfSettings.WebViewPoolPrewarmCount > 0 { | |
| 138 | 186 | poolCfg.PrewarmCount = perfSettings.WebViewPoolPrewarmCount | |
| 139 | 187 | } | |
| 140 | 188 | pool := webkit.NewWebViewPool(ctx, wkCtx, settings, poolCfg, injector, messageRouter) | |
| 141 | - | // Ensure prewarmed WebViews pick up the theme background color. | |
| 189 | + | ||
| 142 | 190 | bgR, bgG, bgB, bgA := themeManager.GetBackgroundRGBA() | |
| 143 | 191 | pool.SetBackgroundColor(bgR, bgG, bgB, bgA) | |
| 144 | 192 | ||
| @@ -146,21 +194,12 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||
| 146 | 194 | pool.SetFilterApplier(filterManager) | |
| 147 | 195 | } | |
| 148 | 196 | ||
| 149 | - | // Pre-create ONE WebView synchronously for instant first navigation. | |
| 150 | - | // This is the heaviest operation but happens before window is shown, | |
| 151 | - | // so users perceive it as part of the normal "loading" phase. | |
| 152 | 197 | if err := pool.PrewarmFirst(ctx); err != nil { | |
| 153 | 198 | logger.Warn().Err(err).Msg("failed to prewarm first webview, first tab may be slower") | |
| 154 | 199 | } | |
| 200 | + | logging.Trace().Mark("pool_prewarm_first") | |
| 155 | 201 | ||
| 156 | - | return WebKitStack{ | |
| 157 | - | Context: wkCtx, | |
| 158 | - | Settings: settings, | |
| 159 | - | Injector: injector, | |
| 160 | - | MessageRouter: messageRouter, | |
| 161 | - | Pool: pool, | |
| 162 | - | FilterManager: filterManager, | |
| 163 | - | } | |
| 202 | + | return pool | |
| 164 | 203 | } | |
| 165 | 204 | ||
| 166 | 205 | // hasWebProcessMemoryConfig returns true if any web process memory setting is configured. | |
| @@ -187,6 +226,12 @@ func configureRenderingEnvironment( | |||
| 187 | 226 | perfSettings *config.ResolvedPerformanceSettings, | |
| 188 | 227 | logger zerolog.Logger, | |
| 189 | 228 | ) { | |
| 229 | + | // Install GLib log handler FIRST if configured (must be before any GTK/GLib calls) | |
| 230 | + | if cfg.Logging.CaptureGTKLogs { | |
| 231 | + | enableDebug := cfg.Logging.Level == "debug" || cfg.Logging.Level == "trace" | |
| 232 | + | logging.InstallGLibLogHandler(ctx, logger, enableDebug) | |
| 233 | + | } | |
| 234 | + | ||
| 190 | 235 | renderEnv := env.NewManager() | |
| 191 | 236 | gpuVendor := renderEnv.DetectGPUVendor(ctx) | |
| 192 | 237 | renderSettings := port.RenderingEnvSettings{ | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "github.com/bnema/dumber/internal/infrastructure/env" | 11 | "github.com/bnema/dumber/internal/infrastructure/env" | ||
| 12 | "github.com/bnema/dumber/internal/infrastructure/filtering" | 12 | "github.com/bnema/dumber/internal/infrastructure/filtering" | ||
| 13 | "github.com/bnema/dumber/internal/infrastructure/webkit" | 13 | "github.com/bnema/dumber/internal/infrastructure/webkit" | ||
| 14 | + | "github.com/bnema/dumber/internal/logging" | |||
| 14 | "github.com/bnema/dumber/internal/ui/theme" | 15 | "github.com/bnema/dumber/internal/ui/theme" | ||
| 15 | "github.com/rs/zerolog" | 16 | "github.com/rs/zerolog" | ||
| 16 | ) | 17 | ) | ||
| @@ -38,14 +39,44 @@ type WebKitStack struct { | |||||
| 38 | func BuildWebKitStack(input WebKitStackInput) WebKitStack { | 39 | func BuildWebKitStack(input WebKitStackInput) WebKitStack { | ||
| 39 | ctx := input.Ctx | 40 | ctx := input.Ctx | ||
| 40 | cfg := input.Config | 41 | cfg := input.Config | ||
| 41 | - | dataDir := input.DataDir | |||
| 42 | - | cacheDir := input.CacheDir | |||
| 43 | - | themeManager := input.ThemeManager | |||
| 44 | - | colorResolver := input.ColorResolver | |||
| 45 | logger := input.Logger | 42 | logger := input.Logger | ||
| 46 | - | // Detect hardware for performance profile scaling | 43 | + | |
| 44 | + | perfSettings := surveyHardwareAndResolveProfile(ctx, cfg, logger) | |||
| 45 | + | ||||
| 46 | + | configureRenderingEnvironment(ctx, cfg, &perfSettings, logger) | |||
| 47 | + | logging.Trace().Mark("render_env") | |||
| 48 | + | ||||
| 49 | + | wkOpts := buildWebKitContextOptions(input.DataDir, input.CacheDir, &perfSettings) | |||
| 50 | + | wkCtx, err := webkit.NewWebKitContextWithOptions(ctx, wkOpts) | |||
| 51 | + | if err != nil { | |||
| 52 | + | logger.Fatal().Err(err).Msg("failed to initialize WebKit context") | |||
| 53 | + | } | |||
| 54 | + | logging.Trace().Mark("webkit_context") | |||
| 55 | + | ||||
| 56 | + | filterManager := initFilterManager(ctx, cfg, input.DataDir, logger) | |||
| 57 | + | ||||
| 58 | + | schemeHandler := webkit.NewDumbSchemeHandler(ctx) | |||
| 59 | + | schemeHandler.SetAssets(assets.WebUIAssets) | |||
| 60 | + | schemeHandler.RegisterWithContext(wkCtx) | |||
| 61 | + | ||||
| 62 | + | settings, injector, messageRouter := initWebKitComponents(ctx, cfg, input.ThemeManager, input.ColorResolver, logger) | |||
| 63 | + | ||||
| 64 | + | pool := initWebViewPool(ctx, wkCtx, settings, injector, messageRouter, input.ThemeManager, filterManager, &perfSettings, logger) | |||
| 65 | + | ||||
| 66 | + | return WebKitStack{ | |||
| 67 | + | Context: wkCtx, | |||
| 68 | + | Settings: settings, | |||
| 69 | + | Injector: injector, | |||
| 70 | + | MessageRouter: messageRouter, | |||
| 71 | + | Pool: pool, | |||
| 72 | + | FilterManager: filterManager, | |||
| 73 | + | } | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | func surveyHardwareAndResolveProfile(ctx context.Context, cfg *config.Config, logger zerolog.Logger) config.ResolvedPerformanceSettings { | |||
| 47 | hwSurveyor := env.NewHardwareSurveyor() | 77 | hwSurveyor := env.NewHardwareSurveyor() | ||
| 48 | hwInfo := hwSurveyor.Survey(ctx) | 78 | hwInfo := hwSurveyor.Survey(ctx) | ||
| 79 | + | logging.Trace().Mark("hardware_survey") | |||
| 49 | logger.Info(). | 80 | logger.Info(). | ||
| 50 | Int("cpu_cores", hwInfo.CPUCores). | 81 | Int("cpu_cores", hwInfo.CPUCores). | ||
| 51 | Int("cpu_threads", hwInfo.CPUThreads). | 82 | Int("cpu_threads", hwInfo.CPUThreads). | ||
| @@ -54,8 +85,8 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 54 | Uint64("vram_mb", hwInfo.VRAM/(1024*1024)). | 85 | Uint64("vram_mb", hwInfo.VRAM/(1024*1024)). | ||
| 55 | Msg("hardware survey completed") | 86 | Msg("hardware survey completed") | ||
| 56 | 87 | ||||
| 57 | - | // Resolve performance profile to get actual settings | |||
| 58 | perfSettings := config.ResolvePerformanceProfile(&cfg.Performance, &hwInfo) | 88 | perfSettings := config.ResolvePerformanceProfile(&cfg.Performance, &hwInfo) | ||
| 89 | + | logging.Trace().Mark("performance_profile") | |||
| 59 | logger.Info(). | 90 | logger.Info(). | ||
| 60 | Str("profile", string(cfg.Performance.Profile)). | 91 | Str("profile", string(cfg.Performance.Profile)). | ||
| 61 | Int("skia_cpu_threads", perfSettings.SkiaCPUPaintingThreads). | 92 | Int("skia_cpu_threads", perfSettings.SkiaCPUPaintingThreads). | ||
| @@ -63,17 +94,16 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 63 | Int("webview_pool_prewarm", perfSettings.WebViewPoolPrewarmCount). | 94 | Int("webview_pool_prewarm", perfSettings.WebViewPoolPrewarmCount). | ||
| 64 | Msg("resolved performance profile") | 95 | Msg("resolved performance profile") | ||
| 65 | 96 | ||||
| 66 | - | // Configure rendering environment (must happen before WebKit/GTK init) | 97 | + | return perfSettings |
| 67 | - | configureRenderingEnvironment(ctx, cfg, &perfSettings, logger) | 98 | + | } |
| 68 | 99 | ||||
| 69 | - | // Build WebKitContext options with memory pressure settings | 100 | + | func buildWebKitContextOptions(dataDir, cacheDir string, perfSettings *config.ResolvedPerformanceSettings) port.WebKitContextOptions { |
| 70 | wkOpts := port.WebKitContextOptions{ | 101 | wkOpts := port.WebKitContextOptions{ | ||
| 71 | DataDir: dataDir, | 102 | DataDir: dataDir, | ||
| 72 | CacheDir: cacheDir, | 103 | CacheDir: cacheDir, | ||
| 73 | } | 104 | } | ||
| 74 | 105 | ||||
| 75 | - | // Configure web process memory pressure if any setting is configured | 106 | + | if hasWebProcessMemoryConfig(perfSettings) { |
| 76 | - | if hasWebProcessMemoryConfig(&perfSettings) { | |||
| 77 | wkOpts.WebProcessMemory = &port.MemoryPressureConfig{ | 107 | wkOpts.WebProcessMemory = &port.MemoryPressureConfig{ | ||
| 78 | MemoryLimitMB: perfSettings.WebProcessMemoryLimitMB, | 108 | MemoryLimitMB: perfSettings.WebProcessMemoryLimitMB, | ||
| 79 | PollIntervalSec: perfSettings.WebProcessMemoryPollIntervalSec, | 109 | PollIntervalSec: perfSettings.WebProcessMemoryPollIntervalSec, | ||
| @@ -82,8 +112,7 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 82 | } | 112 | } | ||
| 83 | } | 113 | } | ||
| 84 | 114 | ||||
| 85 | - | // Configure network process memory pressure if any setting is configured | 115 | + | if hasNetworkProcessMemoryConfig(perfSettings) { |
| 86 | - | if hasNetworkProcessMemoryConfig(&perfSettings) { | |||
| 87 | wkOpts.NetworkProcessMemory = &port.MemoryPressureConfig{ | 116 | wkOpts.NetworkProcessMemory = &port.MemoryPressureConfig{ | ||
| 88 | MemoryLimitMB: perfSettings.NetworkProcessMemoryLimitMB, | 117 | MemoryLimitMB: perfSettings.NetworkProcessMemoryLimitMB, | ||
| 89 | PollIntervalSec: perfSettings.NetworkProcessMemoryPollIntervalSec, | 118 | PollIntervalSec: perfSettings.NetworkProcessMemoryPollIntervalSec, | ||
| @@ -92,11 +121,10 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 92 | } | 121 | } | ||
| 93 | } | 122 | } | ||
| 94 | 123 | ||||
| 95 | - | wkCtx, err := webkit.NewWebKitContextWithOptions(ctx, wkOpts) | 124 | + | return wkOpts |
| 96 | - | if err != nil { | 125 | + | } |
| 97 | - | logger.Fatal().Err(err).Msg("failed to initialize WebKit context") | |||
| 98 | - | } | |||
| 99 | 126 | ||||
| 127 | + | func initFilterManager(ctx context.Context, cfg *config.Config, dataDir string, logger zerolog.Logger) *filtering.Manager { | |||
| 100 | filterStoreDir := filepath.Join(dataDir, "filters", "store") | 128 | filterStoreDir := filepath.Join(dataDir, "filters", "store") | ||
| 101 | filterJSONDir := filepath.Join(dataDir, "filters", "json") | 129 | filterJSONDir := filepath.Join(dataDir, "filters", "json") | ||
| 102 | filterManager, err := filtering.NewManager(filtering.ManagerConfig{ | 130 | filterManager, err := filtering.NewManager(filtering.ManagerConfig{ | ||
| @@ -107,20 +135,25 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 107 | }) | 135 | }) | ||
| 108 | if err != nil { | 136 | if err != nil { | ||
| 109 | logger.Warn().Err(err).Msg("failed to create filter manager, continuing without content filtering") | 137 | logger.Warn().Err(err).Msg("failed to create filter manager, continuing without content filtering") | ||
| 110 | - | } else { | 138 | + | return nil |
| 111 | - | if err := filterManager.Initialize(ctx); err != nil { | |||
| 112 | - | logger.Warn().Err(err).Msg("failed to initialize filters, will load async") | |||
| 113 | - | } | |||
| 114 | } | 139 | } | ||
| 140 | + | if err := filterManager.Initialize(ctx); err != nil { | |||
| 141 | + | logger.Warn().Err(err).Msg("failed to initialize filters, will load async") | |||
| 142 | + | } | |||
| 143 | + | logging.Trace().Mark("filter_manager") | |||
| 144 | + | return filterManager | |||
| 145 | + | } | |||
| 115 | 146 | ||||
| 116 | - | schemeHandler := webkit.NewDumbSchemeHandler(ctx) | 147 | + | func initWebKitComponents( |
| 117 | - | schemeHandler.SetAssets(assets.WebUIAssets) | 148 | + | ctx context.Context, |
| 118 | - | schemeHandler.RegisterWithContext(wkCtx) | 149 | + | cfg *config.Config, |
| 119 | - | 150 | + | themeManager *theme.Manager, | |
| 151 | + | colorResolver port.ColorSchemeResolver, | |||
| 152 | + | logger zerolog.Logger, | |||
| 153 | + | ) (*webkit.SettingsManager, *webkit.ContentInjector, *webkit.MessageRouter) { | |||
| 120 | settings := webkit.NewSettingsManager(ctx, cfg) | 154 | settings := webkit.NewSettingsManager(ctx, cfg) | ||
| 121 | injector := webkit.NewContentInjector(colorResolver) | 155 | injector := webkit.NewContentInjector(colorResolver) | ||
| 122 | 156 | ||||
| 123 | - | // Set up auto-copy on selection config getter | |||
| 124 | injector.SetAutoCopyConfigGetter(func() bool { | 157 | injector.SetAutoCopyConfigGetter(func() bool { | ||
| 125 | return config.Get().Clipboard.AutoCopyOnSelection | 158 | return config.Get().Clipboard.AutoCopyOnSelection | ||
| 126 | }) | 159 | }) | ||
| @@ -132,13 +165,28 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 132 | } | 165 | } | ||
| 133 | 166 | ||||
| 134 | messageRouter := webkit.NewMessageRouter(ctx) | 167 | messageRouter := webkit.NewMessageRouter(ctx) | ||
| 168 | + | logging.Trace().Mark("settings_manager") | |||
| 169 | + | ||||
| 170 | + | return settings, injector, messageRouter | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | func initWebViewPool( | |||
| 174 | + | ctx context.Context, | |||
| 175 | + | wkCtx *webkit.WebKitContext, | |||
| 176 | + | settings *webkit.SettingsManager, | |||
| 177 | + | injector *webkit.ContentInjector, | |||
| 178 | + | messageRouter *webkit.MessageRouter, | |||
| 179 | + | themeManager *theme.Manager, | |||
| 180 | + | filterManager *filtering.Manager, | |||
| 181 | + | perfSettings *config.ResolvedPerformanceSettings, | |||
| 182 | + | logger zerolog.Logger, | |||
| 183 | + | ) *webkit.WebViewPool { | |||
| 135 | poolCfg := webkit.DefaultPoolConfig() | 184 | poolCfg := webkit.DefaultPoolConfig() | ||
| 136 | - | // Override prewarm count from resolved profile settings | |||
| 137 | if perfSettings.WebViewPoolPrewarmCount > 0 { | 185 | if perfSettings.WebViewPoolPrewarmCount > 0 { | ||
| 138 | poolCfg.PrewarmCount = perfSettings.WebViewPoolPrewarmCount | 186 | poolCfg.PrewarmCount = perfSettings.WebViewPoolPrewarmCount | ||
| 139 | } | 187 | } | ||
| 140 | pool := webkit.NewWebViewPool(ctx, wkCtx, settings, poolCfg, injector, messageRouter) | 188 | pool := webkit.NewWebViewPool(ctx, wkCtx, settings, poolCfg, injector, messageRouter) | ||
| 141 | - | // Ensure prewarmed WebViews pick up the theme background color. | 189 | + | |
| 142 | bgR, bgG, bgB, bgA := themeManager.GetBackgroundRGBA() | 190 | bgR, bgG, bgB, bgA := themeManager.GetBackgroundRGBA() | ||
| 143 | pool.SetBackgroundColor(bgR, bgG, bgB, bgA) | 191 | pool.SetBackgroundColor(bgR, bgG, bgB, bgA) | ||
| 144 | 192 | ||||
| @@ -146,21 +194,12 @@ func BuildWebKitStack(input WebKitStackInput) WebKitStack { | |||||
| 146 | pool.SetFilterApplier(filterManager) | 194 | pool.SetFilterApplier(filterManager) | ||
| 147 | } | 195 | } | ||
| 148 | 196 | ||||
| 149 | - | // Pre-create ONE WebView synchronously for instant first navigation. | |||
| 150 | - | // This is the heaviest operation but happens before window is shown, | |||
| 151 | - | // so users perceive it as part of the normal "loading" phase. | |||
| 152 | if err := pool.PrewarmFirst(ctx); err != nil { | 197 | if err := pool.PrewarmFirst(ctx); err != nil { | ||
| 153 | logger.Warn().Err(err).Msg("failed to prewarm first webview, first tab may be slower") | 198 | logger.Warn().Err(err).Msg("failed to prewarm first webview, first tab may be slower") | ||
| 154 | } | 199 | } | ||
| 200 | + | logging.Trace().Mark("pool_prewarm_first") | |||
| 155 | 201 | ||||
| 156 | - | return WebKitStack{ | 202 | + | return pool |
| 157 | - | Context: wkCtx, | |||
| 158 | - | Settings: settings, | |||
| 159 | - | Injector: injector, | |||
| 160 | - | MessageRouter: messageRouter, | |||
| 161 | - | Pool: pool, | |||
| 162 | - | FilterManager: filterManager, | |||
| 163 | - | } | |||
| 164 | } | 203 | } | ||
| 165 | 204 | ||||
| 166 | // hasWebProcessMemoryConfig returns true if any web process memory setting is configured. | 205 | // hasWebProcessMemoryConfig returns true if any web process memory setting is configured. | ||
| @@ -187,6 +226,12 @@ func configureRenderingEnvironment( | |||||
| 187 | perfSettings *config.ResolvedPerformanceSettings, | 226 | perfSettings *config.ResolvedPerformanceSettings, | ||
| 188 | logger zerolog.Logger, | 227 | logger zerolog.Logger, | ||
| 189 | ) { | 228 | ) { | ||
| 229 | + | // Install GLib log handler FIRST if configured (must be before any GTK/GLib calls) | |||
| 230 | + | if cfg.Logging.CaptureGTKLogs { | |||
| 231 | + | enableDebug := cfg.Logging.Level == "debug" || cfg.Logging.Level == "trace" | |||
| 232 | + | logging.InstallGLibLogHandler(ctx, logger, enableDebug) | |||
| 233 | + | } | |||
| 234 | + | ||||
| 190 | renderEnv := env.NewManager() | 235 | renderEnv := env.NewManager() | ||
| 191 | gpuVendor := renderEnv.DetectGPUVendor(ctx) | 236 | gpuVendor := renderEnv.DetectGPUVendor(ctx) | ||
| 192 | renderSettings := port.RenderingEnvSettings{ | 237 | renderSettings := port.RenderingEnvSettings{ | ||
M internal/infrastructure/config/defaults.go +1 -0
| @@ -110,6 +110,7 @@ func DefaultConfig() *Config { | |||
| 110 | 110 | LogDir: getDefaultLogDir(), | |
| 111 | 111 | EnableFileLog: true, | |
| 112 | 112 | CaptureConsole: false, // Disabled by default | |
| 113 | + | CaptureGTKLogs: false, // Disabled by default | |
| 113 | 114 | }, | |
| 114 | 115 | Appearance: AppearanceConfig{ | |
| 115 | 116 | SansFont: "Fira Sans", | |
| @@ -110,6 +110,7 @@ func DefaultConfig() *Config { | |||||
| 110 | LogDir: getDefaultLogDir(), | 110 | LogDir: getDefaultLogDir(), | ||
| 111 | EnableFileLog: true, | 111 | EnableFileLog: true, | ||
| 112 | CaptureConsole: false, // Disabled by default | 112 | CaptureConsole: false, // Disabled by default | ||
| 113 | + | CaptureGTKLogs: false, // Disabled by default | |||
| 113 | }, | 114 | }, | ||
| 114 | Appearance: AppearanceConfig{ | 115 | Appearance: AppearanceConfig{ | ||
| 115 | SansFont: "Fira Sans", | 116 | SansFont: "Fira Sans", | ||
M internal/infrastructure/config/loader.go +1 -0
| @@ -397,6 +397,7 @@ func (m *Manager) setLoggingDefaults(defaults *Config) { | |||
| 397 | 397 | m.viper.SetDefault("logging.log_dir", defaults.Logging.LogDir) | |
| 398 | 398 | m.viper.SetDefault("logging.enable_file_log", defaults.Logging.EnableFileLog) | |
| 399 | 399 | m.viper.SetDefault("logging.capture_console", defaults.Logging.CaptureConsole) | |
| 400 | + | m.viper.SetDefault("logging.capture_gtk_logs", defaults.Logging.CaptureGTKLogs) | |
| 400 | 401 | } | |
| 401 | 402 | ||
| 402 | 403 | func (m *Manager) setDebugDefaults(defaults *Config) { | |
| @@ -397,6 +397,7 @@ func (m *Manager) setLoggingDefaults(defaults *Config) { | |||||
| 397 | m.viper.SetDefault("logging.log_dir", defaults.Logging.LogDir) | 397 | m.viper.SetDefault("logging.log_dir", defaults.Logging.LogDir) | ||
| 398 | m.viper.SetDefault("logging.enable_file_log", defaults.Logging.EnableFileLog) | 398 | m.viper.SetDefault("logging.enable_file_log", defaults.Logging.EnableFileLog) | ||
| 399 | m.viper.SetDefault("logging.capture_console", defaults.Logging.CaptureConsole) | 399 | m.viper.SetDefault("logging.capture_console", defaults.Logging.CaptureConsole) | ||
| 400 | + | m.viper.SetDefault("logging.capture_gtk_logs", defaults.Logging.CaptureGTKLogs) | |||
| 400 | } | 401 | } | ||
| 401 | 402 | ||||
| 402 | func (m *Manager) setDebugDefaults(defaults *Config) { | 403 | func (m *Manager) setDebugDefaults(defaults *Config) { | ||
M internal/infrastructure/config/schema.go +3 -0
| @@ -298,6 +298,9 @@ type LoggingConfig struct { | |||
| 298 | 298 | ||
| 299 | 299 | // Capture browser console to logs | |
| 300 | 300 | CaptureConsole bool `mapstructure:"capture_console" yaml:"capture_console" toml:"capture_console" json:"capture_console"` | |
| 301 | + | ||
| 302 | + | // Capture GTK4/WebKitGTK6 messages to session logs | |
| 303 | + | CaptureGTKLogs bool `mapstructure:"capture_gtk_logs" yaml:"capture_gtk_logs" toml:"capture_gtk_logs" json:"capture_gtk_logs"` | |
| 301 | 304 | } | |
| 302 | 305 | ||
| 303 | 306 | // AppearanceConfig holds UI/rendering preferences. | |
| @@ -298,6 +298,9 @@ type LoggingConfig struct { | |||||
| 298 | 298 | ||||
| 299 | // Capture browser console to logs | 299 | // Capture browser console to logs | ||
| 300 | CaptureConsole bool `mapstructure:"capture_console" yaml:"capture_console" toml:"capture_console" json:"capture_console"` | 300 | CaptureConsole bool `mapstructure:"capture_console" yaml:"capture_console" toml:"capture_console" json:"capture_console"` | ||
| 301 | + | ||||
| 302 | + | // Capture GTK4/WebKitGTK6 messages to session logs | |||
| 303 | + | CaptureGTKLogs bool `mapstructure:"capture_gtk_logs" yaml:"capture_gtk_logs" toml:"capture_gtk_logs" json:"capture_gtk_logs"` | |||
| 301 | } | 304 | } | ||
| 302 | 305 | ||||
| 303 | // AppearanceConfig holds UI/rendering preferences. | 306 | // AppearanceConfig holds UI/rendering preferences. | ||
M internal/infrastructure/persistence/sqlite/history_repo.go +3 -1
| @@ -14,6 +14,8 @@ import ( | |||
| 14 | 14 | "github.com/bnema/dumber/internal/logging" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | + | const logURLMaxLen = 60 | |
| 18 | + | ||
| 17 | 19 | type historyRepo struct { | |
| 18 | 20 | queries *sqlc.Queries | |
| 19 | 21 | } | |
| @@ -25,7 +27,7 @@ func NewHistoryRepository(db *sql.DB) repository.HistoryRepository { | |||
| 25 | 27 | ||
| 26 | 28 | func (r *historyRepo) Save(ctx context.Context, entry *entity.HistoryEntry) error { | |
| 27 | 29 | log := logging.FromContext(ctx) | |
| 28 | - | log.Debug().Str("url", entry.URL).Msg("saving history entry") | |
| 30 | + | log.Debug().Str("url", logging.TruncateURL(entry.URL, logURLMaxLen)).Msg("saving history entry") | |
| 29 | 31 | ||
| 30 | 32 | return r.queries.UpsertHistory(ctx, sqlc.UpsertHistoryParams{ | |
| 31 | 33 | Url: entry.URL, | |
| @@ -14,6 +14,8 @@ import ( | |||||
| 14 | "github.com/bnema/dumber/internal/logging" | 14 | "github.com/bnema/dumber/internal/logging" | ||
| 15 | ) | 15 | ) | ||
| 16 | 16 | ||||
| 17 | + | const logURLMaxLen = 60 | |||
| 18 | + | ||||
| 17 | type historyRepo struct { | 19 | type historyRepo struct { | ||
| 18 | queries *sqlc.Queries | 20 | queries *sqlc.Queries | ||
| 19 | } | 21 | } | ||
| @@ -25,7 +27,7 @@ func NewHistoryRepository(db *sql.DB) repository.HistoryRepository { | |||||
| 25 | 27 | ||||
| 26 | func (r *historyRepo) Save(ctx context.Context, entry *entity.HistoryEntry) error { | 28 | func (r *historyRepo) Save(ctx context.Context, entry *entity.HistoryEntry) error { | ||
| 27 | log := logging.FromContext(ctx) | 29 | log := logging.FromContext(ctx) | ||
| 28 | - | log.Debug().Str("url", entry.URL).Msg("saving history entry") | 30 | + | log.Debug().Str("url", logging.TruncateURL(entry.URL, logURLMaxLen)).Msg("saving history entry") |
| 29 | 31 | ||||
| 30 | return r.queries.UpsertHistory(ctx, sqlc.UpsertHistoryParams{ | 32 | return r.queries.UpsertHistory(ctx, sqlc.UpsertHistoryParams{ | ||
| 31 | Url: entry.URL, | 33 | Url: entry.URL, | ||
M internal/infrastructure/snapshot/service.go +17 -0
| @@ -19,6 +19,7 @@ type Service struct { | |||
| 19 | 19 | mu sync.Mutex | |
| 20 | 20 | timer *time.Timer | |
| 21 | 21 | dirty bool | |
| 22 | + | ready bool // true when session is persisted to DB and snapshots can be saved | |
| 22 | 23 | ctx context.Context | |
| 23 | 24 | cancel context.CancelFunc | |
| 24 | 25 | } | |
| @@ -48,6 +49,15 @@ func (s *Service) Start(ctx context.Context) { | |||
| 48 | 49 | logging.FromContext(ctx).Debug().Dur("interval", s.interval).Msg("snapshot service started") | |
| 49 | 50 | } | |
| 50 | 51 | ||
| 52 | + | // SetReady marks the service as ready to save snapshots. | |
| 53 | + | // Call this after the session has been persisted to the database | |
| 54 | + | // to avoid FK constraint violations. | |
| 55 | + | func (s *Service) SetReady() { | |
| 56 | + | s.mu.Lock() | |
| 57 | + | defer s.mu.Unlock() | |
| 58 | + | s.ready = true | |
| 59 | + | } | |
| 60 | + | ||
| 51 | 61 | // Stop stops the service and saves final state. | |
| 52 | 62 | func (s *Service) Stop(ctx context.Context) error { | |
| 53 | 63 | s.mu.Lock() | |
| @@ -111,6 +121,13 @@ func (s *Service) SaveNow(ctx context.Context) error { | |||
| 111 | 121 | ||
| 112 | 122 | func (s *Service) saveSnapshot(ctx context.Context) error { | |
| 113 | 123 | s.mu.Lock() | |
| 124 | + | ready := s.ready | |
| 125 | + | if !ready { | |
| 126 | + | // Don't clear dirty if not ready - keep pending snapshot for later | |
| 127 | + | s.mu.Unlock() | |
| 128 | + | return nil | |
| 129 | + | } | |
| 130 | + | // Only clear dirty when we're actually going to save | |
| 114 | 131 | s.dirty = false | |
| 115 | 132 | s.mu.Unlock() | |
| 116 | 133 | ||
| @@ -19,6 +19,7 @@ type Service struct { | |||||
| 19 | mu sync.Mutex | 19 | mu sync.Mutex | ||
| 20 | timer *time.Timer | 20 | timer *time.Timer | ||
| 21 | dirty bool | 21 | dirty bool | ||
| 22 | + | ready bool // true when session is persisted to DB and snapshots can be saved | |||
| 22 | ctx context.Context | 23 | ctx context.Context | ||
| 23 | cancel context.CancelFunc | 24 | cancel context.CancelFunc | ||
| 24 | } | 25 | } | ||
| @@ -48,6 +49,15 @@ func (s *Service) Start(ctx context.Context) { | |||||
| 48 | logging.FromContext(ctx).Debug().Dur("interval", s.interval).Msg("snapshot service started") | 49 | logging.FromContext(ctx).Debug().Dur("interval", s.interval).Msg("snapshot service started") | ||
| 49 | } | 50 | } | ||
| 50 | 51 | ||||
| 52 | + | // SetReady marks the service as ready to save snapshots. | |||
| 53 | + | // Call this after the session has been persisted to the database | |||
| 54 | + | // to avoid FK constraint violations. | |||
| 55 | + | func (s *Service) SetReady() { | |||
| 56 | + | s.mu.Lock() | |||
| 57 | + | defer s.mu.Unlock() | |||
| 58 | + | s.ready = true | |||
| 59 | + | } | |||
| 60 | + | ||||
| 51 | // Stop stops the service and saves final state. | 61 | // Stop stops the service and saves final state. | ||
| 52 | func (s *Service) Stop(ctx context.Context) error { | 62 | func (s *Service) Stop(ctx context.Context) error { | ||
| 53 | s.mu.Lock() | 63 | s.mu.Lock() | ||
| @@ -111,6 +121,13 @@ func (s *Service) SaveNow(ctx context.Context) error { | |||||
| 111 | 121 | ||||
| 112 | func (s *Service) saveSnapshot(ctx context.Context) error { | 122 | func (s *Service) saveSnapshot(ctx context.Context) error { | ||
| 113 | s.mu.Lock() | 123 | s.mu.Lock() | ||
| 124 | + | ready := s.ready | |||
| 125 | + | if !ready { | |||
| 126 | + | // Don't clear dirty if not ready - keep pending snapshot for later | |||
| 127 | + | s.mu.Unlock() | |||
| 128 | + | return nil | |||
| 129 | + | } | |||
| 130 | + | // Only clear dirty when we're actually going to save | |||
| 114 | s.dirty = false | 131 | s.dirty = false | ||
| 115 | s.mu.Unlock() | 132 | s.mu.Unlock() | ||
| 116 | 133 | ||||
M internal/infrastructure/webkit/pool.go +2 -1
| @@ -312,7 +312,8 @@ func (p *WebViewPool) PrewarmAsync(ctx context.Context, count int) { | |||
| 312 | 312 | schedule() | |
| 313 | 313 | return false | |
| 314 | 314 | }) | |
| 315 | - | glib.IdleAdd(&cb, 0) | |
| 315 | + | // Use LOW priority so initial navigation and other high-priority work runs first | |
| 316 | + | glib.IdleAddFull(glib.PRIORITY_LOW, &cb, 0, nil) | |
| 316 | 317 | } | |
| 317 | 318 | ||
| 318 | 319 | schedule() | |
| @@ -312,7 +312,8 @@ func (p *WebViewPool) PrewarmAsync(ctx context.Context, count int) { | |||||
| 312 | schedule() | 312 | schedule() | ||
| 313 | return false | 313 | return false | ||
| 314 | }) | 314 | }) | ||
| 315 | - | glib.IdleAdd(&cb, 0) | 315 | + | // Use LOW priority so initial navigation and other high-priority work runs first |
| 316 | + | glib.IdleAddFull(glib.PRIORITY_LOW, &cb, 0, nil) | |||
| 316 | } | 317 | } | ||
| 317 | 318 | ||||
| 318 | schedule() | 319 | schedule() | ||
M internal/infrastructure/webkit/webview.go +25 -0
| @@ -327,11 +327,20 @@ func NewWebViewWithRelated(ctx context.Context, parent *WebView, settings *Setti | |||
| 327 | 327 | return nil, fmt.Errorf("parent webview %d is destroyed", parent.id) | |
| 328 | 328 | } | |
| 329 | 329 | ||
| 330 | + | log.Debug(). | |
| 331 | + | Uint64("parent_ptr", uint64(parent.inner.GoPointer())). | |
| 332 | + | Msg("creating related webview with parent pointer") | |
| 333 | + | ||
| 330 | 334 | inner := webkit.NewWebViewWithRelatedView(parent.inner) | |
| 331 | 335 | if inner == nil { | |
| 332 | 336 | return nil, fmt.Errorf("failed to create related webkit webview") | |
| 333 | 337 | } | |
| 334 | 338 | ||
| 339 | + | log.Debug(). | |
| 340 | + | Uint64("new_ptr", uint64(inner.GoPointer())). | |
| 341 | + | Uint64("new_widget_ptr", uint64(inner.Widget.GoPointer())). | |
| 342 | + | Msg("related webview created, checking pointers") | |
| 343 | + | ||
| 335 | 344 | wv := &WebView{ | |
| 336 | 345 | inner: inner, | |
| 337 | 346 | isRelated: true, // Shares web process with parent - must not terminate process on destroy | |
| @@ -441,11 +450,18 @@ func (wv *WebView) connectCreateSignal() { | |||
| 441 | 450 | ParentID: wv.id, | |
| 442 | 451 | } | |
| 443 | 452 | ||
| 453 | + | wv.logger.Debug().Msg("create signal: invoking OnCreate handler") | |
| 444 | 454 | newWV := wv.OnCreate(popupReq) | |
| 455 | + | wv.logger.Debug().Bool("nil", newWV == nil).Msg("create signal: OnCreate returned") | |
| 445 | 456 | if newWV == nil { | |
| 446 | 457 | return gtk.Widget{} // Block popup | |
| 447 | 458 | } | |
| 448 | 459 | ||
| 460 | + | wv.logger.Debug(). | |
| 461 | + | Uint64("parent_id", uint64(wv.id)). | |
| 462 | + | Uint64("popup_id", uint64(newWV.id)). | |
| 463 | + | Msg("create signal: returning webview widget to WebKit") | |
| 464 | + | ||
| 449 | 465 | return newWV.inner.Widget | |
| 450 | 466 | } | |
| 451 | 467 | sigID := wv.inner.ConnectCreate(&createCb) | |
| @@ -889,6 +905,15 @@ func (wv *WebView) SetBackgroundColor(r, g, b, a float32) { | |||
| 889 | 905 | wv.inner.SetBackgroundColor(rgba) | |
| 890 | 906 | } | |
| 891 | 907 | ||
| 908 | + | // Show makes the WebView widget visible. | |
| 909 | + | // This should be called after the WebView is ready to be displayed. | |
| 910 | + | func (wv *WebView) Show() { | |
| 911 | + | if wv.destroyed.Load() { | |
| 912 | + | return | |
| 913 | + | } | |
| 914 | + | wv.inner.SetVisible(true) | |
| 915 | + | } | |
| 916 | + | ||
| 892 | 917 | // State returns the current WebView state as a snapshot. | |
| 893 | 918 | func (wv *WebView) State() port.WebViewState { | |
| 894 | 919 | return port.WebViewState{ | |
| @@ -327,11 +327,20 @@ func NewWebViewWithRelated(ctx context.Context, parent *WebView, settings *Setti | |||||
| 327 | return nil, fmt.Errorf("parent webview %d is destroyed", parent.id) | 327 | return nil, fmt.Errorf("parent webview %d is destroyed", parent.id) | ||
| 328 | } | 328 | } | ||
| 329 | 329 | ||||
| 330 | + | log.Debug(). | |||
| 331 | + | Uint64("parent_ptr", uint64(parent.inner.GoPointer())). | |||
| 332 | + | Msg("creating related webview with parent pointer") | |||
| 333 | + | ||||
| 330 | inner := webkit.NewWebViewWithRelatedView(parent.inner) | 334 | inner := webkit.NewWebViewWithRelatedView(parent.inner) | ||
| 331 | if inner == nil { | 335 | if inner == nil { | ||
| 332 | return nil, fmt.Errorf("failed to create related webkit webview") | 336 | return nil, fmt.Errorf("failed to create related webkit webview") | ||
| 333 | } | 337 | } | ||
| 334 | 338 | ||||
| 339 | + | log.Debug(). | |||
| 340 | + | Uint64("new_ptr", uint64(inner.GoPointer())). | |||
| 341 | + | Uint64("new_widget_ptr", uint64(inner.Widget.GoPointer())). | |||
| 342 | + | Msg("related webview created, checking pointers") | |||
| 343 | + | ||||
| 335 | wv := &WebView{ | 344 | wv := &WebView{ | ||
| 336 | inner: inner, | 345 | inner: inner, | ||
| 337 | isRelated: true, // Shares web process with parent - must not terminate process on destroy | 346 | isRelated: true, // Shares web process with parent - must not terminate process on destroy | ||
| @@ -441,11 +450,18 @@ func (wv *WebView) connectCreateSignal() { | |||||
| 441 | ParentID: wv.id, | 450 | ParentID: wv.id, | ||
| 442 | } | 451 | } | ||
| 443 | 452 | ||||
| 453 | + | wv.logger.Debug().Msg("create signal: invoking OnCreate handler") | |||
| 444 | newWV := wv.OnCreate(popupReq) | 454 | newWV := wv.OnCreate(popupReq) | ||
| 455 | + | wv.logger.Debug().Bool("nil", newWV == nil).Msg("create signal: OnCreate returned") | |||
| 445 | if newWV == nil { | 456 | if newWV == nil { | ||
| 446 | return gtk.Widget{} // Block popup | 457 | return gtk.Widget{} // Block popup | ||
| 447 | } | 458 | } | ||
| 448 | 459 | ||||
| 460 | + | wv.logger.Debug(). | |||
| 461 | + | Uint64("parent_id", uint64(wv.id)). | |||
| 462 | + | Uint64("popup_id", uint64(newWV.id)). | |||
| 463 | + | Msg("create signal: returning webview widget to WebKit") | |||
| 464 | + | ||||
| 449 | return newWV.inner.Widget | 465 | return newWV.inner.Widget | ||
| 450 | } | 466 | } | ||
| 451 | sigID := wv.inner.ConnectCreate(&createCb) | 467 | sigID := wv.inner.ConnectCreate(&createCb) | ||
| @@ -889,6 +905,15 @@ func (wv *WebView) SetBackgroundColor(r, g, b, a float32) { | |||||
| 889 | wv.inner.SetBackgroundColor(rgba) | 905 | wv.inner.SetBackgroundColor(rgba) | ||
| 890 | } | 906 | } | ||
| 891 | 907 | ||||
| 908 | + | // Show makes the WebView widget visible. | |||
| 909 | + | // This should be called after the WebView is ready to be displayed. | |||
| 910 | + | func (wv *WebView) Show() { | |||
| 911 | + | if wv.destroyed.Load() { | |||
| 912 | + | return | |||
| 913 | + | } | |||
| 914 | + | wv.inner.SetVisible(true) | |||
| 915 | + | } | |||
| 916 | + | ||||
| 892 | // State returns the current WebView state as a snapshot. | 917 | // State returns the current WebView state as a snapshot. | ||
| 893 | func (wv *WebView) State() port.WebViewState { | 918 | func (wv *WebView) State() port.WebViewState { | ||
| 894 | return port.WebViewState{ | 919 | return port.WebViewState{ | ||
A internal/logging/glib.go +70 -0
| @@ -0,0 +1,70 @@ | |||
| 1 | + | package logging | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "sync" | |
| 6 | + | ||
| 7 | + | "github.com/jwijenbergh/puregotk/v4/glib" | |
| 8 | + | "github.com/rs/zerolog" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | // glibLogger holds the logger for the GLib handler. | |
| 12 | + | // We need this because the GLib callback doesn't support passing Go pointers directly. | |
| 13 | + | var ( | |
| 14 | + | glibLogger zerolog.Logger | |
| 15 | + | glibLoggerOnce sync.Once | |
| 16 | + | ) | |
| 17 | + | ||
| 18 | + | // InstallGLibLogHandler installs a custom GLib log handler that routes | |
| 19 | + | // GTK4/WebKitGTK6/GLib messages to the provided zerolog logger. | |
| 20 | + | // This must be called before GTK/WebKit initialization. | |
| 21 | + | // The handler captures messages from all GLib-based libraries (GTK, GDK, WebKit, etc.) | |
| 22 | + | // If enableDebug is true, GLib debug messages will also be captured (useful when log level is debug/trace). | |
| 23 | + | func InstallGLibLogHandler(ctx context.Context, logger zerolog.Logger, enableDebug bool) { | |
| 24 | + | log := FromContext(ctx) | |
| 25 | + | ||
| 26 | + | glibLoggerOnce.Do(func() { | |
| 27 | + | glibLogger = logger | |
| 28 | + | ||
| 29 | + | // Only enable GLib debug messages if requested (matches app log level) | |
| 30 | + | if enableDebug { | |
| 31 | + | glib.LogSetDebugEnabled(true) | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | // Install our custom handler for all log messages | |
| 35 | + | handler := glib.LogFunc(glibLogHandler) | |
| 36 | + | glib.LogSetDefaultHandler(&handler, 0) | |
| 37 | + | ||
| 38 | + | log.Info().Bool("debug_enabled", enableDebug).Msg("GLib log handler installed") | |
| 39 | + | }) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | // glibLogHandler is the callback invoked by GLib for all log messages. | |
| 43 | + | func glibLogHandler(domain string, level glib.LogLevelFlags, message string, _ uintptr) { | |
| 44 | + | // Map GLib log levels to zerolog levels | |
| 45 | + | var event *zerolog.Event | |
| 46 | + | ||
| 47 | + | switch { | |
| 48 | + | case level&glib.GLogLevelErrorValue != 0: | |
| 49 | + | event = glibLogger.Error() | |
| 50 | + | case level&glib.GLogLevelCriticalValue != 0: | |
| 51 | + | event = glibLogger.Error() | |
| 52 | + | case level&glib.GLogLevelWarningValue != 0: | |
| 53 | + | event = glibLogger.Warn() | |
| 54 | + | case level&glib.GLogLevelMessageValue != 0: | |
| 55 | + | event = glibLogger.Info() | |
| 56 | + | case level&glib.GLogLevelInfoValue != 0: | |
| 57 | + | event = glibLogger.Info() | |
| 58 | + | case level&glib.GLogLevelDebugValue != 0: | |
| 59 | + | event = glibLogger.Debug() | |
| 60 | + | default: | |
| 61 | + | event = glibLogger.Debug() | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | // Add domain if present | |
| 65 | + | if domain != "" { | |
| 66 | + | event = event.Str("glib_domain", domain) | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | event.Msg(message) | |
| 70 | + | } | |
| @@ -0,0 +1,70 @@ | |||||
| 1 | + | package logging | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "sync" | |||
| 6 | + | ||||
| 7 | + | "github.com/jwijenbergh/puregotk/v4/glib" | |||
| 8 | + | "github.com/rs/zerolog" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | // glibLogger holds the logger for the GLib handler. | |||
| 12 | + | // We need this because the GLib callback doesn't support passing Go pointers directly. | |||
| 13 | + | var ( | |||
| 14 | + | glibLogger zerolog.Logger | |||
| 15 | + | glibLoggerOnce sync.Once | |||
| 16 | + | ) | |||
| 17 | + | ||||
| 18 | + | // InstallGLibLogHandler installs a custom GLib log handler that routes | |||
| 19 | + | // GTK4/WebKitGTK6/GLib messages to the provided zerolog logger. | |||
| 20 | + | // This must be called before GTK/WebKit initialization. | |||
| 21 | + | // The handler captures messages from all GLib-based libraries (GTK, GDK, WebKit, etc.) | |||
| 22 | + | // If enableDebug is true, GLib debug messages will also be captured (useful when log level is debug/trace). | |||
| 23 | + | func InstallGLibLogHandler(ctx context.Context, logger zerolog.Logger, enableDebug bool) { | |||
| 24 | + | log := FromContext(ctx) | |||
| 25 | + | ||||
| 26 | + | glibLoggerOnce.Do(func() { | |||
| 27 | + | glibLogger = logger | |||
| 28 | + | ||||
| 29 | + | // Only enable GLib debug messages if requested (matches app log level) | |||
| 30 | + | if enableDebug { | |||
| 31 | + | glib.LogSetDebugEnabled(true) | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | // Install our custom handler for all log messages | |||
| 35 | + | handler := glib.LogFunc(glibLogHandler) | |||
| 36 | + | glib.LogSetDefaultHandler(&handler, 0) | |||
| 37 | + | ||||
| 38 | + | log.Info().Bool("debug_enabled", enableDebug).Msg("GLib log handler installed") | |||
| 39 | + | }) | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | // glibLogHandler is the callback invoked by GLib for all log messages. | |||
| 43 | + | func glibLogHandler(domain string, level glib.LogLevelFlags, message string, _ uintptr) { | |||
| 44 | + | // Map GLib log levels to zerolog levels | |||
| 45 | + | var event *zerolog.Event | |||
| 46 | + | ||||
| 47 | + | switch { | |||
| 48 | + | case level&glib.GLogLevelErrorValue != 0: | |||
| 49 | + | event = glibLogger.Error() | |||
| 50 | + | case level&glib.GLogLevelCriticalValue != 0: | |||
| 51 | + | event = glibLogger.Error() | |||
| 52 | + | case level&glib.GLogLevelWarningValue != 0: | |||
| 53 | + | event = glibLogger.Warn() | |||
| 54 | + | case level&glib.GLogLevelMessageValue != 0: | |||
| 55 | + | event = glibLogger.Info() | |||
| 56 | + | case level&glib.GLogLevelInfoValue != 0: | |||
| 57 | + | event = glibLogger.Info() | |||
| 58 | + | case level&glib.GLogLevelDebugValue != 0: | |||
| 59 | + | event = glibLogger.Debug() | |||
| 60 | + | default: | |||
| 61 | + | event = glibLogger.Debug() | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | // Add domain if present | |||
| 65 | + | if domain != "" { | |||
| 66 | + | event = event.Str("glib_domain", domain) | |||
| 67 | + | } | |||
| 68 | + | ||||
| 69 | + | event.Msg(message) | |||
| 70 | + | } | |||
M internal/logging/logger.go +8 -0
| @@ -189,3 +189,11 @@ func ParseLevel(level string) zerolog.Level { | |||
| 189 | 189 | return zerolog.InfoLevel | |
| 190 | 190 | } | |
| 191 | 191 | } | |
| 192 | + | ||
| 193 | + | // TruncateURL truncates a URL to maxLen characters for logging. | |
| 194 | + | func TruncateURL(url string, maxLen int) string { | |
| 195 | + | if len(url) <= maxLen { | |
| 196 | + | return url | |
| 197 | + | } | |
| 198 | + | return url[:maxLen] + "..." | |
| 199 | + | } | |
| @@ -189,3 +189,11 @@ func ParseLevel(level string) zerolog.Level { | |||||
| 189 | return zerolog.InfoLevel | 189 | return zerolog.InfoLevel | ||
| 190 | } | 190 | } | ||
| 191 | } | 191 | } | ||
| 192 | + | ||||
| 193 | + | // TruncateURL truncates a URL to maxLen characters for logging. | |||
| 194 | + | func TruncateURL(url string, maxLen int) string { | |||
| 195 | + | if len(url) <= maxLen { | |||
| 196 | + | return url | |||
| 197 | + | } | |||
| 198 | + | return url[:maxLen] + "..." | |||
| 199 | + | } | |||
A internal/logging/startup_trace.go +220 -0
| @@ -0,0 +1,220 @@ | |||
| 1 | + | package logging | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "fmt" | |
| 5 | + | "strings" | |
| 6 | + | "sync" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/rs/zerolog" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | // StartupTrace tracks cold start milestones from process launch to first paint. | |
| 13 | + | // Thread-safe for use across goroutines. | |
| 14 | + | // Enabled only when log level is debug or trace. | |
| 15 | + | type StartupTrace struct { | |
| 16 | + | mu sync.Mutex | |
| 17 | + | t0 time.Time | |
| 18 | + | milestones []Milestone | |
| 19 | + | enabled bool | |
| 20 | + | logger *zerolog.Logger | |
| 21 | + | buffered []Milestone // milestones recorded before logger is set | |
| 22 | + | finished bool | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | // Milestone represents a timing checkpoint during startup. | |
| 26 | + | type Milestone struct { | |
| 27 | + | Name string | |
| 28 | + | Elapsed time.Duration // time since t0 | |
| 29 | + | Delta time.Duration // time since previous milestone | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | var ( | |
| 33 | + | globalTrace *StartupTrace | |
| 34 | + | globalTraceMu sync.Mutex | |
| 35 | + | globalTraceOnce sync.Once | |
| 36 | + | ) | |
| 37 | + | ||
| 38 | + | // InitStartupTrace initializes the global startup trace. | |
| 39 | + | // Call this as early as possible in main() to capture T0. | |
| 40 | + | // The trace is enabled only if the given log level is debug or trace. | |
| 41 | + | func InitStartupTrace(logLevel string) { | |
| 42 | + | globalTraceOnce.Do(func() { | |
| 43 | + | enabled := logLevel == "debug" || logLevel == "trace" | |
| 44 | + | globalTraceMu.Lock() | |
| 45 | + | globalTrace = &StartupTrace{ | |
| 46 | + | t0: time.Now(), | |
| 47 | + | milestones: make([]Milestone, 0, 32), | |
| 48 | + | enabled: enabled, | |
| 49 | + | buffered: make([]Milestone, 0, 8), | |
| 50 | + | } | |
| 51 | + | globalTraceMu.Unlock() | |
| 52 | + | ||
| 53 | + | if enabled { | |
| 54 | + | // Record process_start as first milestone | |
| 55 | + | globalTrace.Mark("process_start") | |
| 56 | + | } | |
| 57 | + | }) | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | // Trace returns the global startup trace instance. | |
| 61 | + | // Returns a no-op trace if not initialized or disabled. | |
| 62 | + | func Trace() *StartupTrace { | |
| 63 | + | globalTraceMu.Lock() | |
| 64 | + | defer globalTraceMu.Unlock() | |
| 65 | + | if globalTrace == nil { | |
| 66 | + | // Return a disabled no-op trace | |
| 67 | + | return &StartupTrace{enabled: false} | |
| 68 | + | } | |
| 69 | + | return globalTrace | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | // SetLogger sets the logger for emitting milestone logs. | |
| 73 | + | // Flushes any buffered milestones that were recorded before the logger was set. | |
| 74 | + | func (st *StartupTrace) SetLogger(logger *zerolog.Logger) { | |
| 75 | + | if st == nil || !st.enabled { | |
| 76 | + | return | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | st.mu.Lock() | |
| 80 | + | defer st.mu.Unlock() | |
| 81 | + | ||
| 82 | + | st.logger = logger | |
| 83 | + | ||
| 84 | + | // Flush buffered milestones | |
| 85 | + | for _, m := range st.buffered { | |
| 86 | + | st.emitMilestone(m) | |
| 87 | + | } | |
| 88 | + | st.buffered = nil | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | // UpdateLogger updates the logger and re-emits all milestones to the new logger. | |
| 92 | + | // Call this after the session file logger is available to capture milestones in the log file. | |
| 93 | + | func (st *StartupTrace) UpdateLogger(logger *zerolog.Logger) { | |
| 94 | + | if st == nil || !st.enabled { | |
| 95 | + | return | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | st.mu.Lock() | |
| 99 | + | defer st.mu.Unlock() | |
| 100 | + | ||
| 101 | + | st.logger = logger | |
| 102 | + | ||
| 103 | + | // Re-emit all milestones to the new logger | |
| 104 | + | for _, m := range st.milestones { | |
| 105 | + | st.emitMilestone(m) | |
| 106 | + | } | |
| 107 | + | } | |
| 108 | + | ||
| 109 | + | // Mark records a milestone with the given name. | |
| 110 | + | // Emits a debug log line showing elapsed time and delta from previous milestone. | |
| 111 | + | func (st *StartupTrace) Mark(name string) { | |
| 112 | + | if st == nil || !st.enabled { | |
| 113 | + | return | |
| 114 | + | } | |
| 115 | + | ||
| 116 | + | st.mu.Lock() | |
| 117 | + | defer st.mu.Unlock() | |
| 118 | + | ||
| 119 | + | if st.finished { | |
| 120 | + | return | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | now := time.Now() | |
| 124 | + | elapsed := now.Sub(st.t0) | |
| 125 | + | ||
| 126 | + | var delta time.Duration | |
| 127 | + | if len(st.milestones) > 0 { | |
| 128 | + | delta = elapsed - st.milestones[len(st.milestones)-1].Elapsed | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | m := Milestone{ | |
| 132 | + | Name: name, | |
| 133 | + | Elapsed: elapsed, | |
| 134 | + | Delta: delta, | |
| 135 | + | } | |
| 136 | + | st.milestones = append(st.milestones, m) | |
| 137 | + | ||
| 138 | + | if st.logger != nil { | |
| 139 | + | st.emitMilestone(m) | |
| 140 | + | } else { | |
| 141 | + | // Buffer until logger is set | |
| 142 | + | st.buffered = append(st.buffered, m) | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | // emitMilestone logs a single milestone. Caller must hold mutex. | |
| 147 | + | func (st *StartupTrace) emitMilestone(m Milestone) { | |
| 148 | + | if st.logger == nil { | |
| 149 | + | return | |
| 150 | + | } | |
| 151 | + | ||
| 152 | + | elapsedMs := m.Elapsed.Milliseconds() | |
| 153 | + | ||
| 154 | + | event := st.logger.Debug(). | |
| 155 | + | Str("milestone", m.Name). | |
| 156 | + | Int64("t_ms", elapsedMs) | |
| 157 | + | ||
| 158 | + | if m.Delta > 0 { | |
| 159 | + | event = event.Int64("delta_ms", m.Delta.Milliseconds()) | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | // Format: startup_trace: milestone_name (T+123ms, +45ms) | |
| 163 | + | if m.Delta > 0 { | |
| 164 | + | event.Msgf("startup_trace: %s (T+%dms, +%dms)", m.Name, elapsedMs, m.Delta.Milliseconds()) | |
| 165 | + | } else { | |
| 166 | + | event.Msgf("startup_trace: %s (T+%dms)", m.Name, elapsedMs) | |
| 167 | + | } | |
| 168 | + | } | |
| 169 | + | ||
| 170 | + | // Finish marks the startup trace as complete and emits a summary. | |
| 171 | + | // Called on first_paint milestone. | |
| 172 | + | func (st *StartupTrace) Finish() { | |
| 173 | + | if st == nil || !st.enabled { | |
| 174 | + | return | |
| 175 | + | } | |
| 176 | + | ||
| 177 | + | st.mu.Lock() | |
| 178 | + | defer st.mu.Unlock() | |
| 179 | + | ||
| 180 | + | if st.finished { | |
| 181 | + | return | |
| 182 | + | } | |
| 183 | + | st.finished = true | |
| 184 | + | ||
| 185 | + | if st.logger == nil { | |
| 186 | + | return | |
| 187 | + | } | |
| 188 | + | ||
| 189 | + | // Build summary string | |
| 190 | + | total := time.Since(st.t0) | |
| 191 | + | var parts []string | |
| 192 | + | for _, m := range st.milestones { | |
| 193 | + | parts = append(parts, fmt.Sprintf("%s:%d", m.Name, m.Elapsed.Milliseconds())) | |
| 194 | + | } | |
| 195 | + | summary := strings.Join(parts, ",") | |
| 196 | + | ||
| 197 | + | st.logger.Info(). | |
| 198 | + | Int64("total_ms", total.Milliseconds()). | |
| 199 | + | Str("milestones", summary). | |
| 200 | + | Msg("startup_trace: cold start complete") | |
| 201 | + | } | |
| 202 | + | ||
| 203 | + | // Enabled returns whether the trace is active. | |
| 204 | + | func (st *StartupTrace) Enabled() bool { | |
| 205 | + | if st == nil { | |
| 206 | + | return false | |
| 207 | + | } | |
| 208 | + | return st.enabled | |
| 209 | + | } | |
| 210 | + | ||
| 211 | + | // TotalElapsed returns the total time since T0. | |
| 212 | + | func (st *StartupTrace) TotalElapsed() time.Duration { | |
| 213 | + | if st == nil || !st.enabled { | |
| 214 | + | return 0 | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | st.mu.Lock() | |
| 218 | + | defer st.mu.Unlock() | |
| 219 | + | return time.Since(st.t0) | |
| 220 | + | } | |
| @@ -0,0 +1,220 @@ | |||||
| 1 | + | package logging | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "fmt" | |||
| 5 | + | "strings" | |||
| 6 | + | "sync" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/rs/zerolog" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | // StartupTrace tracks cold start milestones from process launch to first paint. | |||
| 13 | + | // Thread-safe for use across goroutines. | |||
| 14 | + | // Enabled only when log level is debug or trace. | |||
| 15 | + | type StartupTrace struct { | |||
| 16 | + | mu sync.Mutex | |||
| 17 | + | t0 time.Time | |||
| 18 | + | milestones []Milestone | |||
| 19 | + | enabled bool | |||
| 20 | + | logger *zerolog.Logger | |||
| 21 | + | buffered []Milestone // milestones recorded before logger is set | |||
| 22 | + | finished bool | |||
| 23 | + | } | |||
| 24 | + | ||||
| 25 | + | // Milestone represents a timing checkpoint during startup. | |||
| 26 | + | type Milestone struct { | |||
| 27 | + | Name string | |||
| 28 | + | Elapsed time.Duration // time since t0 | |||
| 29 | + | Delta time.Duration // time since previous milestone | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | var ( | |||
| 33 | + | globalTrace *StartupTrace | |||
| 34 | + | globalTraceMu sync.Mutex | |||
| 35 | + | globalTraceOnce sync.Once | |||
| 36 | + | ) | |||
| 37 | + | ||||
| 38 | + | // InitStartupTrace initializes the global startup trace. | |||
| 39 | + | // Call this as early as possible in main() to capture T0. | |||
| 40 | + | // The trace is enabled only if the given log level is debug or trace. | |||
| 41 | + | func InitStartupTrace(logLevel string) { | |||
| 42 | + | globalTraceOnce.Do(func() { | |||
| 43 | + | enabled := logLevel == "debug" || logLevel == "trace" | |||
| 44 | + | globalTraceMu.Lock() | |||
| 45 | + | globalTrace = &StartupTrace{ | |||
| 46 | + | t0: time.Now(), | |||
| 47 | + | milestones: make([]Milestone, 0, 32), | |||
| 48 | + | enabled: enabled, | |||
| 49 | + | buffered: make([]Milestone, 0, 8), | |||
| 50 | + | } | |||
| 51 | + | globalTraceMu.Unlock() | |||
| 52 | + | ||||
| 53 | + | if enabled { | |||
| 54 | + | // Record process_start as first milestone | |||
| 55 | + | globalTrace.Mark("process_start") | |||
| 56 | + | } | |||
| 57 | + | }) | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | // Trace returns the global startup trace instance. | |||
| 61 | + | // Returns a no-op trace if not initialized or disabled. | |||
| 62 | + | func Trace() *StartupTrace { | |||
| 63 | + | globalTraceMu.Lock() | |||
| 64 | + | defer globalTraceMu.Unlock() | |||
| 65 | + | if globalTrace == nil { | |||
| 66 | + | // Return a disabled no-op trace | |||
| 67 | + | return &StartupTrace{enabled: false} | |||
| 68 | + | } | |||
| 69 | + | return globalTrace | |||
| 70 | + | } | |||
| 71 | + | ||||
| 72 | + | // SetLogger sets the logger for emitting milestone logs. | |||
| 73 | + | // Flushes any buffered milestones that were recorded before the logger was set. | |||
| 74 | + | func (st *StartupTrace) SetLogger(logger *zerolog.Logger) { | |||
| 75 | + | if st == nil || !st.enabled { | |||
| 76 | + | return | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | st.mu.Lock() | |||
| 80 | + | defer st.mu.Unlock() | |||
| 81 | + | ||||
| 82 | + | st.logger = logger | |||
| 83 | + | ||||
| 84 | + | // Flush buffered milestones | |||
| 85 | + | for _, m := range st.buffered { | |||
| 86 | + | st.emitMilestone(m) | |||
| 87 | + | } | |||
| 88 | + | st.buffered = nil | |||
| 89 | + | } | |||
| 90 | + | ||||
| 91 | + | // UpdateLogger updates the logger and re-emits all milestones to the new logger. | |||
| 92 | + | // Call this after the session file logger is available to capture milestones in the log file. | |||
| 93 | + | func (st *StartupTrace) UpdateLogger(logger *zerolog.Logger) { | |||
| 94 | + | if st == nil || !st.enabled { | |||
| 95 | + | return | |||
| 96 | + | } | |||
| 97 | + | ||||
| 98 | + | st.mu.Lock() | |||
| 99 | + | defer st.mu.Unlock() | |||
| 100 | + | ||||
| 101 | + | st.logger = logger | |||
| 102 | + | ||||
| 103 | + | // Re-emit all milestones to the new logger | |||
| 104 | + | for _, m := range st.milestones { | |||
| 105 | + | st.emitMilestone(m) | |||
| 106 | + | } | |||
| 107 | + | } | |||
| 108 | + | ||||
| 109 | + | // Mark records a milestone with the given name. | |||
| 110 | + | // Emits a debug log line showing elapsed time and delta from previous milestone. | |||
| 111 | + | func (st *StartupTrace) Mark(name string) { | |||
| 112 | + | if st == nil || !st.enabled { | |||
| 113 | + | return | |||
| 114 | + | } | |||
| 115 | + | ||||
| 116 | + | st.mu.Lock() | |||
| 117 | + | defer st.mu.Unlock() | |||
| 118 | + | ||||
| 119 | + | if st.finished { | |||
| 120 | + | return | |||
| 121 | + | } | |||
| 122 | + | ||||
| 123 | + | now := time.Now() | |||
| 124 | + | elapsed := now.Sub(st.t0) | |||
| 125 | + | ||||
| 126 | + | var delta time.Duration | |||
| 127 | + | if len(st.milestones) > 0 { | |||
| 128 | + | delta = elapsed - st.milestones[len(st.milestones)-1].Elapsed | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | m := Milestone{ | |||
| 132 | + | Name: name, | |||
| 133 | + | Elapsed: elapsed, | |||
| 134 | + | Delta: delta, | |||
| 135 | + | } | |||
| 136 | + | st.milestones = append(st.milestones, m) | |||
| 137 | + | ||||
| 138 | + | if st.logger != nil { | |||
| 139 | + | st.emitMilestone(m) | |||
| 140 | + | } else { | |||
| 141 | + | // Buffer until logger is set | |||
| 142 | + | st.buffered = append(st.buffered, m) | |||
| 143 | + | } | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | // emitMilestone logs a single milestone. Caller must hold mutex. | |||
| 147 | + | func (st *StartupTrace) emitMilestone(m Milestone) { | |||
| 148 | + | if st.logger == nil { | |||
| 149 | + | return | |||
| 150 | + | } | |||
| 151 | + | ||||
| 152 | + | elapsedMs := m.Elapsed.Milliseconds() | |||
| 153 | + | ||||
| 154 | + | event := st.logger.Debug(). | |||
| 155 | + | Str("milestone", m.Name). | |||
| 156 | + | Int64("t_ms", elapsedMs) | |||
| 157 | + | ||||
| 158 | + | if m.Delta > 0 { | |||
| 159 | + | event = event.Int64("delta_ms", m.Delta.Milliseconds()) | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | // Format: startup_trace: milestone_name (T+123ms, +45ms) | |||
| 163 | + | if m.Delta > 0 { | |||
| 164 | + | event.Msgf("startup_trace: %s (T+%dms, +%dms)", m.Name, elapsedMs, m.Delta.Milliseconds()) | |||
| 165 | + | } else { | |||
| 166 | + | event.Msgf("startup_trace: %s (T+%dms)", m.Name, elapsedMs) | |||
| 167 | + | } | |||
| 168 | + | } | |||
| 169 | + | ||||
| 170 | + | // Finish marks the startup trace as complete and emits a summary. | |||
| 171 | + | // Called on first_paint milestone. | |||
| 172 | + | func (st *StartupTrace) Finish() { | |||
| 173 | + | if st == nil || !st.enabled { | |||
| 174 | + | return | |||
| 175 | + | } | |||
| 176 | + | ||||
| 177 | + | st.mu.Lock() | |||
| 178 | + | defer st.mu.Unlock() | |||
| 179 | + | ||||
| 180 | + | if st.finished { | |||
| 181 | + | return | |||
| 182 | + | } | |||
| 183 | + | st.finished = true | |||
| 184 | + | ||||
| 185 | + | if st.logger == nil { | |||
| 186 | + | return | |||
| 187 | + | } | |||
| 188 | + | ||||
| 189 | + | // Build summary string | |||
| 190 | + | total := time.Since(st.t0) | |||
| 191 | + | var parts []string | |||
| 192 | + | for _, m := range st.milestones { | |||
| 193 | + | parts = append(parts, fmt.Sprintf("%s:%d", m.Name, m.Elapsed.Milliseconds())) | |||
| 194 | + | } | |||
| 195 | + | summary := strings.Join(parts, ",") | |||
| 196 | + | ||||
| 197 | + | st.logger.Info(). | |||
| 198 | + | Int64("total_ms", total.Milliseconds()). | |||
| 199 | + | Str("milestones", summary). | |||
| 200 | + | Msg("startup_trace: cold start complete") | |||
| 201 | + | } | |||
| 202 | + | ||||
| 203 | + | // Enabled returns whether the trace is active. | |||
| 204 | + | func (st *StartupTrace) Enabled() bool { | |||
| 205 | + | if st == nil { | |||
| 206 | + | return false | |||
| 207 | + | } | |||
| 208 | + | return st.enabled | |||
| 209 | + | } | |||
| 210 | + | ||||
| 211 | + | // TotalElapsed returns the total time since T0. | |||
| 212 | + | func (st *StartupTrace) TotalElapsed() time.Duration { | |||
| 213 | + | if st == nil || !st.enabled { | |||
| 214 | + | return 0 | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | st.mu.Lock() | |||
| 218 | + | defer st.mu.Unlock() | |||
| 219 | + | return time.Since(st.t0) | |||
| 220 | + | } | |||
M internal/ui/app.go +52 -14
| @@ -115,6 +115,10 @@ type App struct { | |||
| 115 | 115 | insertAccentUC *usecase.InsertAccentUseCase | |
| 116 | 116 | accentFocusProvider *textinput.FocusProvider | |
| 117 | 117 | ||
| 118 | + | // Deferred initialization - runs after first load_started to avoid blocking initial navigation | |
| 119 | + | deferredInitOnce sync.Once | |
| 120 | + | deferredInitFn func() | |
| 121 | + | ||
| 118 | 122 | // lifecycle | |
| 119 | 123 | cancel context.CancelCauseFunc | |
| 120 | 124 | } | |
| @@ -182,6 +186,7 @@ func (a *App) Run(ctx context.Context, args []string) int { | |||
| 182 | 186 | // Initialize libadwaita once (required before using StyleManager). | |
| 183 | 187 | // This also initializes GTK implicitly. | |
| 184 | 188 | adw.Init() | |
| 189 | + | logging.Trace().Mark("gtk_init") | |
| 185 | 190 | ||
| 186 | 191 | // Mark adwaita detector as available now that adw.Init() is complete. | |
| 187 | 192 | // This enables the highest-priority color scheme detector. | |
| @@ -203,6 +208,7 @@ func (a *App) Run(ctx context.Context, args []string) int { | |||
| 203 | 208 | return 1 | |
| 204 | 209 | } | |
| 205 | 210 | defer a.gtkApp.Unref() | |
| 211 | + | logging.Trace().Mark("gtk_app_created") | |
| 206 | 212 | ||
| 207 | 213 | // Connect activate signal | |
| 208 | 214 | activateCb := func(_ gio.Application) { | |
| @@ -236,6 +242,7 @@ func (a *App) onActivate(ctx context.Context) { | |||
| 236 | 242 | log.Error().Err(err).Msg("failed to create main window") | |
| 237 | 243 | return | |
| 238 | 244 | } | |
| 245 | + | logging.Trace().Mark("window_created") | |
| 239 | 246 | ||
| 240 | 247 | a.initLayoutInfrastructure() | |
| 241 | 248 | a.initAppToasterOverlay() | |
| @@ -244,6 +251,7 @@ func (a *App) onActivate(ctx context.Context) { | |||
| 244 | 251 | a.initDownloadHandler(ctx) | |
| 245 | 252 | ||
| 246 | 253 | a.initCoordinators(ctx) | |
| 254 | + | logging.Trace().Mark("coordinators_init") | |
| 247 | 255 | a.initKeyboardHandler(ctx) | |
| 248 | 256 | a.initOmniboxConfig(ctx) | |
| 249 | 257 | a.initFindBarConfig() | |
| @@ -252,8 +260,6 @@ func (a *App) onActivate(ctx context.Context) { | |||
| 252 | 260 | a.initSnapshotService(ctx) | |
| 253 | 261 | a.initUpdateCoordinator(ctx) | |
| 254 | 262 | a.createInitialTab(ctx) | |
| 255 | - | // Prewarm the remaining WebViews after the initial tab starts loading. | |
| 256 | - | a.prewarmWebViewPoolAsync(ctx) | |
| 257 | 263 | a.finalizeActivation(ctx) | |
| 258 | 264 | } | |
| 259 | 265 | ||
| @@ -1016,6 +1022,11 @@ func (a *App) initSnapshotService(ctx context.Context) { | |||
| 1016 | 1022 | a.snapshotService = snapshot.NewService(a.deps.SnapshotUC, a, intervalMs) | |
| 1017 | 1023 | a.snapshotService.Start(ctx) | |
| 1018 | 1024 | ||
| 1025 | + | // Set up callback for main.go to notify when session is persisted | |
| 1026 | + | a.deps.OnSessionPersisted = func() { | |
| 1027 | + | a.snapshotService.SetReady() | |
| 1028 | + | } | |
| 1029 | + | ||
| 1019 | 1030 | log.Debug().Int("interval_ms", intervalMs).Msg("snapshot service started") | |
| 1020 | 1031 | } | |
| 1021 | 1032 | ||
| @@ -1149,26 +1160,48 @@ func (a *App) restoreSession(ctx context.Context, sessionID entity.SessionID) er | |||
| 1149 | 1160 | func (a *App) finalizeActivation(ctx context.Context) { | |
| 1150 | 1161 | log := logging.FromContext(ctx) | |
| 1151 | 1162 | ||
| 1152 | - | // Show the window. | |
| 1163 | + | // Show the window | |
| 1153 | 1164 | if a.mainWindow != nil { | |
| 1154 | 1165 | a.mainWindow.Show() | |
| 1155 | 1166 | } | |
| 1156 | 1167 | log.Info().Msg("main window displayed") | |
| 1157 | 1168 | ||
| 1158 | - | // Start watching config for appearance changes. | |
| 1159 | - | a.initConfigWatcher(ctx) | |
| 1169 | + | // Defer non-critical initialization until after first navigation starts. | |
| 1170 | + | // This keeps pool prewarm, config watcher, and filter loading from | |
| 1171 | + | // competing with the initial page load. | |
| 1172 | + | a.runAfterFirstLoadStarted(func() { | |
| 1173 | + | a.prewarmWebViewPoolAsync(ctx) | |
| 1174 | + | a.initConfigWatcher(ctx) | |
| 1175 | + | a.initFilteringAsync(ctx) | |
| 1176 | + | a.checkConfigMigration(ctx) | |
| 1177 | + | }) | |
| 1178 | + | } | |
| 1160 | 1179 | ||
| 1161 | - | // Ensure the current appearance config is applied once at startup. | |
| 1162 | - | // This avoids the WebUI showing default tokens until the first config change event. | |
| 1163 | - | if a.deps != nil { | |
| 1164 | - | a.applyAppearanceConfig(ctx, a.deps.Config) | |
| 1165 | - | } | |
| 1180 | + | // runAfterFirstLoadStarted schedules work to run after the first navigation starts. | |
| 1181 | + | // This keeps the GTK main loop free to process the initial load_uri() quickly, | |
| 1182 | + | // reducing the gap between webview_attached and load_started. | |
| 1183 | + | func (a *App) runAfterFirstLoadStarted(fn func()) { | |
| 1184 | + | a.deferredInitFn = fn | |
| 1185 | + | } | |
| 1166 | 1186 | ||
| 1167 | - | // Start async filter loading after window is visible. | |
| 1168 | - | a.initFilteringAsync(ctx) | |
| 1187 | + | // triggerDeferredInit is called from ContentCoordinator on first load_started. | |
| 1188 | + | // It runs deferred initialization at LOW priority so navigation continues unblocked. | |
| 1189 | + | func (a *App) triggerDeferredInit(ctx context.Context) { | |
| 1190 | + | a.deferredInitOnce.Do(func() { | |
| 1191 | + | if a.deferredInitFn == nil { | |
| 1192 | + | return | |
| 1193 | + | } | |
| 1194 | + | log := logging.FromContext(ctx) | |
| 1195 | + | log.Debug().Msg("triggering deferred initialization") | |
| 1169 | 1196 | ||
| 1170 | - | // Check for config migration after window is visible. | |
| 1171 | - | a.checkConfigMigration(ctx) | |
| 1197 | + | fn := a.deferredInitFn | |
| 1198 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 1199 | + | fn() | |
| 1200 | + | return false | |
| 1201 | + | }) | |
| 1202 | + | // Use LOW priority so navigation processing continues first | |
| 1203 | + | glib.IdleAddFull(glib.PRIORITY_LOW, &cb, 0, nil) | |
| 1204 | + | }) | |
| 1172 | 1205 | } | |
| 1173 | 1206 | ||
| 1174 | 1207 | // onShutdown is called when the GTK application is shutting down. | |
| @@ -1242,6 +1275,11 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 1242 | 1275 | a.contentCoord.SetIdleInhibitor(a.deps.IdleInhibitor) | |
| 1243 | 1276 | } | |
| 1244 | 1277 | ||
| 1278 | + | // Wire deferred init trigger - runs after first navigation starts | |
| 1279 | + | a.contentCoord.SetOnFirstLoadStarted(func() { | |
| 1280 | + | a.triggerDeferredInit(ctx) | |
| 1281 | + | }) | |
| 1282 | + | ||
| 1245 | 1283 | // 2. Tab Coordinator | |
| 1246 | 1284 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | |
| 1247 | 1285 | TabsUC: a.tabsUC, | |
| @@ -115,6 +115,10 @@ type App struct { | |||||
| 115 | insertAccentUC *usecase.InsertAccentUseCase | 115 | insertAccentUC *usecase.InsertAccentUseCase | ||
| 116 | accentFocusProvider *textinput.FocusProvider | 116 | accentFocusProvider *textinput.FocusProvider | ||
| 117 | 117 | ||||
| 118 | + | // Deferred initialization - runs after first load_started to avoid blocking initial navigation | |||
| 119 | + | deferredInitOnce sync.Once | |||
| 120 | + | deferredInitFn func() | |||
| 121 | + | ||||
| 118 | // lifecycle | 122 | // lifecycle | ||
| 119 | cancel context.CancelCauseFunc | 123 | cancel context.CancelCauseFunc | ||
| 120 | } | 124 | } | ||
| @@ -182,6 +186,7 @@ func (a *App) Run(ctx context.Context, args []string) int { | |||||
| 182 | // Initialize libadwaita once (required before using StyleManager). | 186 | // Initialize libadwaita once (required before using StyleManager). | ||
| 183 | // This also initializes GTK implicitly. | 187 | // This also initializes GTK implicitly. | ||
| 184 | adw.Init() | 188 | adw.Init() | ||
| 189 | + | logging.Trace().Mark("gtk_init") | |||
| 185 | 190 | ||||
| 186 | // Mark adwaita detector as available now that adw.Init() is complete. | 191 | // Mark adwaita detector as available now that adw.Init() is complete. | ||
| 187 | // This enables the highest-priority color scheme detector. | 192 | // This enables the highest-priority color scheme detector. | ||
| @@ -203,6 +208,7 @@ func (a *App) Run(ctx context.Context, args []string) int { | |||||
| 203 | return 1 | 208 | return 1 | ||
| 204 | } | 209 | } | ||
| 205 | defer a.gtkApp.Unref() | 210 | defer a.gtkApp.Unref() | ||
| 211 | + | logging.Trace().Mark("gtk_app_created") | |||
| 206 | 212 | ||||
| 207 | // Connect activate signal | 213 | // Connect activate signal | ||
| 208 | activateCb := func(_ gio.Application) { | 214 | activateCb := func(_ gio.Application) { | ||
| @@ -236,6 +242,7 @@ func (a *App) onActivate(ctx context.Context) { | |||||
| 236 | log.Error().Err(err).Msg("failed to create main window") | 242 | log.Error().Err(err).Msg("failed to create main window") | ||
| 237 | return | 243 | return | ||
| 238 | } | 244 | } | ||
| 245 | + | logging.Trace().Mark("window_created") | |||
| 239 | 246 | ||||
| 240 | a.initLayoutInfrastructure() | 247 | a.initLayoutInfrastructure() | ||
| 241 | a.initAppToasterOverlay() | 248 | a.initAppToasterOverlay() | ||
| @@ -244,6 +251,7 @@ func (a *App) onActivate(ctx context.Context) { | |||||
| 244 | a.initDownloadHandler(ctx) | 251 | a.initDownloadHandler(ctx) | ||
| 245 | 252 | ||||
| 246 | a.initCoordinators(ctx) | 253 | a.initCoordinators(ctx) | ||
| 254 | + | logging.Trace().Mark("coordinators_init") | |||
| 247 | a.initKeyboardHandler(ctx) | 255 | a.initKeyboardHandler(ctx) | ||
| 248 | a.initOmniboxConfig(ctx) | 256 | a.initOmniboxConfig(ctx) | ||
| 249 | a.initFindBarConfig() | 257 | a.initFindBarConfig() | ||
| @@ -252,8 +260,6 @@ func (a *App) onActivate(ctx context.Context) { | |||||
| 252 | a.initSnapshotService(ctx) | 260 | a.initSnapshotService(ctx) | ||
| 253 | a.initUpdateCoordinator(ctx) | 261 | a.initUpdateCoordinator(ctx) | ||
| 254 | a.createInitialTab(ctx) | 262 | a.createInitialTab(ctx) | ||
| 255 | - | // Prewarm the remaining WebViews after the initial tab starts loading. | |||
| 256 | - | a.prewarmWebViewPoolAsync(ctx) | |||
| 257 | a.finalizeActivation(ctx) | 263 | a.finalizeActivation(ctx) | ||
| 258 | } | 264 | } | ||
| 259 | 265 | ||||
| @@ -1016,6 +1022,11 @@ func (a *App) initSnapshotService(ctx context.Context) { | |||||
| 1016 | a.snapshotService = snapshot.NewService(a.deps.SnapshotUC, a, intervalMs) | 1022 | a.snapshotService = snapshot.NewService(a.deps.SnapshotUC, a, intervalMs) | ||
| 1017 | a.snapshotService.Start(ctx) | 1023 | a.snapshotService.Start(ctx) | ||
| 1018 | 1024 | ||||
| 1025 | + | // Set up callback for main.go to notify when session is persisted | |||
| 1026 | + | a.deps.OnSessionPersisted = func() { | |||
| 1027 | + | a.snapshotService.SetReady() | |||
| 1028 | + | } | |||
| 1029 | + | ||||
| 1019 | log.Debug().Int("interval_ms", intervalMs).Msg("snapshot service started") | 1030 | log.Debug().Int("interval_ms", intervalMs).Msg("snapshot service started") | ||
| 1020 | } | 1031 | } | ||
| 1021 | 1032 | ||||
| @@ -1149,26 +1160,48 @@ func (a *App) restoreSession(ctx context.Context, sessionID entity.SessionID) er | |||||
| 1149 | func (a *App) finalizeActivation(ctx context.Context) { | 1160 | func (a *App) finalizeActivation(ctx context.Context) { | ||
| 1150 | log := logging.FromContext(ctx) | 1161 | log := logging.FromContext(ctx) | ||
| 1151 | 1162 | ||||
| 1152 | - | // Show the window. | 1163 | + | // Show the window |
| 1153 | if a.mainWindow != nil { | 1164 | if a.mainWindow != nil { | ||
| 1154 | a.mainWindow.Show() | 1165 | a.mainWindow.Show() | ||
| 1155 | } | 1166 | } | ||
| 1156 | log.Info().Msg("main window displayed") | 1167 | log.Info().Msg("main window displayed") | ||
| 1157 | 1168 | ||||
| 1158 | - | // Start watching config for appearance changes. | 1169 | + | // Defer non-critical initialization until after first navigation starts. |
| 1159 | - | a.initConfigWatcher(ctx) | 1170 | + | // This keeps pool prewarm, config watcher, and filter loading from |
| 1171 | + | // competing with the initial page load. | |||
| 1172 | + | a.runAfterFirstLoadStarted(func() { | |||
| 1173 | + | a.prewarmWebViewPoolAsync(ctx) | |||
| 1174 | + | a.initConfigWatcher(ctx) | |||
| 1175 | + | a.initFilteringAsync(ctx) | |||
| 1176 | + | a.checkConfigMigration(ctx) | |||
| 1177 | + | }) | |||
| 1178 | + | } | |||
| 1160 | 1179 | ||||
| 1161 | - | // Ensure the current appearance config is applied once at startup. | 1180 | + | // runAfterFirstLoadStarted schedules work to run after the first navigation starts. |
| 1162 | - | // This avoids the WebUI showing default tokens until the first config change event. | 1181 | + | // This keeps the GTK main loop free to process the initial load_uri() quickly, |
| 1163 | - | if a.deps != nil { | 1182 | + | // reducing the gap between webview_attached and load_started. |
| 1164 | - | a.applyAppearanceConfig(ctx, a.deps.Config) | 1183 | + | func (a *App) runAfterFirstLoadStarted(fn func()) { |
| 1165 | - | } | 1184 | + | a.deferredInitFn = fn |
| 1185 | + | } | |||
| 1166 | 1186 | ||||
| 1167 | - | // Start async filter loading after window is visible. | 1187 | + | // triggerDeferredInit is called from ContentCoordinator on first load_started. |
| 1168 | - | a.initFilteringAsync(ctx) | 1188 | + | // It runs deferred initialization at LOW priority so navigation continues unblocked. |
| 1189 | + | func (a *App) triggerDeferredInit(ctx context.Context) { | |||
| 1190 | + | a.deferredInitOnce.Do(func() { | |||
| 1191 | + | if a.deferredInitFn == nil { | |||
| 1192 | + | return | |||
| 1193 | + | } | |||
| 1194 | + | log := logging.FromContext(ctx) | |||
| 1195 | + | log.Debug().Msg("triggering deferred initialization") | |||
| 1169 | 1196 | ||||
| 1170 | - | // Check for config migration after window is visible. | 1197 | + | fn := a.deferredInitFn |
| 1171 | - | a.checkConfigMigration(ctx) | 1198 | + | cb := glib.SourceFunc(func(_ uintptr) bool { |
| 1199 | + | fn() | |||
| 1200 | + | return false | |||
| 1201 | + | }) | |||
| 1202 | + | // Use LOW priority so navigation processing continues first | |||
| 1203 | + | glib.IdleAddFull(glib.PRIORITY_LOW, &cb, 0, nil) | |||
| 1204 | + | }) | |||
| 1172 | } | 1205 | } | ||
| 1173 | 1206 | ||||
| 1174 | // onShutdown is called when the GTK application is shutting down. | 1207 | // onShutdown is called when the GTK application is shutting down. | ||
| @@ -1242,6 +1275,11 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 1242 | a.contentCoord.SetIdleInhibitor(a.deps.IdleInhibitor) | 1275 | a.contentCoord.SetIdleInhibitor(a.deps.IdleInhibitor) | ||
| 1243 | } | 1276 | } | ||
| 1244 | 1277 | ||||
| 1278 | + | // Wire deferred init trigger - runs after first navigation starts | |||
| 1279 | + | a.contentCoord.SetOnFirstLoadStarted(func() { | |||
| 1280 | + | a.triggerDeferredInit(ctx) | |||
| 1281 | + | }) | |||
| 1282 | + | ||||
| 1245 | // 2. Tab Coordinator | 1283 | // 2. Tab Coordinator | ||
| 1246 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | 1284 | a.tabCoord = coordinator.NewTabCoordinator(ctx, coordinator.TabCoordinatorConfig{ | ||
| 1247 | TabsUC: a.tabsUC, | 1285 | TabsUC: a.tabsUC, | ||
M internal/ui/coordinator/content.go +185 -79
| @@ -21,7 +21,10 @@ import ( | |||
| 21 | 21 | "github.com/jwijenbergh/puregotk/v4/gdk" | |
| 22 | 22 | ) | |
| 23 | 23 | ||
| 24 | - | const aboutBlankURI = "about:blank" | |
| 24 | + | const ( | |
| 25 | + | aboutBlankURI = "about:blank" | |
| 26 | + | logURLMaxLen = 80 | |
| 27 | + | ) | |
| 25 | 28 | ||
| 26 | 29 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | |
| 27 | 30 | type ContentCoordinator struct { | |
| @@ -95,6 +98,10 @@ type ContentCoordinator struct { | |||
| 95 | 98 | ||
| 96 | 99 | // Callback when WebView gains focus (for accent picker text input targeting) | |
| 97 | 100 | onWebViewFocused func(paneID entity.PaneID, wv *webkit.WebView) | |
| 101 | + | ||
| 102 | + | // Callback for first load_started event (triggers deferred initialization) | |
| 103 | + | onFirstLoadStarted func() | |
| 104 | + | loadStartedOnce sync.Once | |
| 98 | 105 | } | |
| 99 | 106 | ||
| 100 | 107 | type pendingThemeUpdate struct { | |
| @@ -173,6 +180,13 @@ func (c *ContentCoordinator) SetOnWebViewFocused(fn func(paneID entity.PaneID, w | |||
| 173 | 180 | c.onWebViewFocused = fn | |
| 174 | 181 | } | |
| 175 | 182 | ||
| 183 | + | // SetOnFirstLoadStarted sets the callback for when the first navigation starts. | |
| 184 | + | // This is used to trigger deferred initialization after the initial load_uri() | |
| 185 | + | // has been processed by the GTK main loop. | |
| 186 | + | func (c *ContentCoordinator) SetOnFirstLoadStarted(fn func()) { | |
| 187 | + | c.onFirstLoadStarted = fn | |
| 188 | + | } | |
| 189 | + | ||
| 176 | 190 | // EnsureWebView acquires or reuses a WebView for the given pane. | |
| 177 | 191 | func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (*webkit.WebView, error) { | |
| 178 | 192 | log := logging.FromContext(ctx) | |
| @@ -185,10 +199,16 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||
| 185 | 199 | return nil, fmt.Errorf("webview pool not configured") | |
| 186 | 200 | } | |
| 187 | 201 | ||
| 202 | + | // Mark tab_created on first webview (first tab) | |
| 203 | + | if len(c.webViews) == 0 { | |
| 204 | + | logging.Trace().Mark("tab_created") | |
| 205 | + | } | |
| 206 | + | ||
| 188 | 207 | wv, err := c.pool.Acquire(ctx) | |
| 189 | 208 | if err != nil { | |
| 190 | 209 | return nil, err | |
| 191 | 210 | } | |
| 211 | + | logging.Trace().Mark("webview_acquired") | |
| 192 | 212 | ||
| 193 | 213 | c.webViews[paneID] = wv | |
| 194 | 214 | ||
| @@ -331,6 +351,7 @@ func (c *ContentCoordinator) AttachToWorkspace(ctx context.Context, ws *entity.W | |||
| 331 | 351 | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | |
| 332 | 352 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | |
| 333 | 353 | } | |
| 354 | + | logging.Trace().Mark("webview_attached") | |
| 334 | 355 | } | |
| 335 | 356 | } | |
| 336 | 357 | ||
| @@ -908,6 +929,7 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||
| 908 | 929 | // Also shows the WebView widget (it's hidden during creation to avoid white flash). | |
| 909 | 930 | func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 910 | 931 | log := logging.FromContext(ctx) | |
| 932 | + | logging.Trace().Mark("load_committed") | |
| 911 | 933 | ||
| 912 | 934 | url := wv.URI() | |
| 913 | 935 | if url == "" { | |
| @@ -1020,6 +1042,16 @@ func (c *ContentCoordinator) updatePaneURI(paneID entity.PaneID, url string) { | |||
| 1020 | 1042 | ||
| 1021 | 1043 | // onLoadStarted shows the progress bar when page loading begins. | |
| 1022 | 1044 | func (c *ContentCoordinator) onLoadStarted(paneID entity.PaneID) { | |
| 1045 | + | logging.Trace().Mark("load_started") | |
| 1046 | + | ||
| 1047 | + | // Trigger deferred initialization on first load_started. | |
| 1048 | + | // This ensures non-critical init runs after initial navigation starts. | |
| 1049 | + | c.loadStartedOnce.Do(func() { | |
| 1050 | + | if c.onFirstLoadStarted != nil { | |
| 1051 | + | c.onFirstLoadStarted() | |
| 1052 | + | } | |
| 1053 | + | }) | |
| 1054 | + | ||
| 1023 | 1055 | _, wsView := c.getActiveWS() | |
| 1024 | 1056 | if wsView == nil { | |
| 1025 | 1057 | return | |
| @@ -1106,6 +1138,11 @@ func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity. | |||
| 1106 | 1138 | Str("reason", reason). | |
| 1107 | 1139 | Msg("webview revealed") | |
| 1108 | 1140 | } | |
| 1141 | + | ||
| 1142 | + | // Mark first_paint and finish startup trace | |
| 1143 | + | logging.Trace().Mark("first_paint") | |
| 1144 | + | logging.Trace().Finish() | |
| 1145 | + | ||
| 1109 | 1146 | if c.onWebViewShown != nil { | |
| 1110 | 1147 | c.onWebViewShown(paneID) | |
| 1111 | 1148 | } | |
| @@ -1170,8 +1207,34 @@ func (c *ContentCoordinator) SetupPopupHandling(ctx context.Context, paneID enti | |||
| 1170 | 1207 | log.Debug().Str("pane_id", string(paneID)).Msg("popup handling configured for webview") | |
| 1171 | 1208 | } | |
| 1172 | 1209 | ||
| 1210 | + | // createPopupPane creates a new pane entity for a popup window. | |
| 1211 | + | func (c *ContentCoordinator) createPopupPane( | |
| 1212 | + | popupID port.WebViewID, | |
| 1213 | + | parentPaneID entity.PaneID, | |
| 1214 | + | targetURI string, | |
| 1215 | + | ) (entity.PaneID, *entity.Pane) { | |
| 1216 | + | var paneID entity.PaneID | |
| 1217 | + | if c.generateID != nil { | |
| 1218 | + | paneID = entity.PaneID(c.generateID()) | |
| 1219 | + | } else { | |
| 1220 | + | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |
| 1221 | + | } | |
| 1222 | + | ||
| 1223 | + | popupPane := entity.NewPane(paneID) | |
| 1224 | + | popupPane.WindowType = entity.WindowPopup | |
| 1225 | + | popupPane.IsRelated = true | |
| 1226 | + | popupPane.ParentPaneID = &parentPaneID | |
| 1227 | + | popupPane.URI = targetURI | |
| 1228 | + | ||
| 1229 | + | return paneID, popupPane | |
| 1230 | + | } | |
| 1231 | + | ||
| 1173 | 1232 | // handlePopupCreate handles the WebKit "create" signal for popup windows. | |
| 1174 | 1233 | // Returns a new related WebView if popup is allowed, nil to block. | |
| 1234 | + | // | |
| 1235 | + | // IMPORTANT: The WebView MUST be added to a GtkWindow hierarchy BEFORE this | |
| 1236 | + | // signal handler returns. Otherwise WebKit won't establish window.opener. | |
| 1237 | + | // The WebView stays hidden until ready-to-show signal is received. | |
| 1175 | 1238 | func (c *ContentCoordinator) handlePopupCreate( | |
| 1176 | 1239 | ctx context.Context, | |
| 1177 | 1240 | parentPaneID entity.PaneID, | |
| @@ -1199,7 +1262,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1199 | 1262 | return nil | |
| 1200 | 1263 | } | |
| 1201 | 1264 | ||
| 1202 | - | // Create related WebView for session sharing | |
| 1265 | + | // Create related WebView for session sharing (created hidden) | |
| 1203 | 1266 | parentID := parentWV.ID() | |
| 1204 | 1267 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | |
| 1205 | 1268 | if err != nil { | |
| @@ -1209,9 +1272,63 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1209 | 1272 | ||
| 1210 | 1273 | // Detect popup type | |
| 1211 | 1274 | popupType := DetectPopupType(req.FrameName) | |
| 1212 | - | ||
| 1213 | - | // Store pending popup for ready-to-show handling | |
| 1214 | 1275 | popupID := popupWV.ID() | |
| 1276 | + | ||
| 1277 | + | // Determine behavior from config | |
| 1278 | + | behavior := GetBehavior(popupType, c.popupConfig) | |
| 1279 | + | placement := "right" | |
| 1280 | + | if c.popupConfig != nil { | |
| 1281 | + | placement = c.popupConfig.Placement | |
| 1282 | + | } | |
| 1283 | + | ||
| 1284 | + | // Create popup pane entity | |
| 1285 | + | paneID, popupPane := c.createPopupPane(popupID, parentPaneID, req.TargetURI) | |
| 1286 | + | ||
| 1287 | + | // Check OAuth configuration | |
| 1288 | + | hasConfig := c.popupConfig != nil | |
| 1289 | + | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |
| 1290 | + | isOAuth := IsOAuthURL(req.TargetURI) | |
| 1291 | + | log.Debug(). | |
| 1292 | + | Bool("has_config", hasConfig). | |
| 1293 | + | Bool("oauth_enabled", oauthEnabled). | |
| 1294 | + | Bool("is_oauth", isOAuth). | |
| 1295 | + | Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |
| 1296 | + | Msg("popup OAuth check") | |
| 1297 | + | ||
| 1298 | + | // Insert into workspace IMMEDIATELY (WebView stays hidden) | |
| 1299 | + | // This is required for WebKit to establish window.opener relationship | |
| 1300 | + | if c.onInsertPopup != nil { | |
| 1301 | + | popupInput := InsertPopupInput{ | |
| 1302 | + | ParentPaneID: parentPaneID, | |
| 1303 | + | PopupPane: popupPane, | |
| 1304 | + | WebView: popupWV, | |
| 1305 | + | Behavior: behavior, | |
| 1306 | + | Placement: placement, | |
| 1307 | + | PopupType: popupType, | |
| 1308 | + | TargetURI: req.TargetURI, | |
| 1309 | + | } | |
| 1310 | + | ||
| 1311 | + | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |
| 1312 | + | log.Error().Err(err).Msg("failed to insert popup into workspace") | |
| 1313 | + | popupWV.Destroy() | |
| 1314 | + | return nil | |
| 1315 | + | } | |
| 1316 | + | } | |
| 1317 | + | ||
| 1318 | + | // Register WebView in our map (after successful insertion) | |
| 1319 | + | c.webViews[paneID] = popupWV | |
| 1320 | + | ||
| 1321 | + | // Setup standard callbacks (after successful insertion to avoid leak) | |
| 1322 | + | c.setupWebViewCallbacks(ctx, paneID, popupWV) | |
| 1323 | + | ||
| 1324 | + | // Setup OAuth auto-close if configured | |
| 1325 | + | if hasConfig && oauthEnabled && isOAuth { | |
| 1326 | + | popupPane.AutoClose = true | |
| 1327 | + | c.setupOAuthAutoClose(ctx, paneID, popupWV) | |
| 1328 | + | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |
| 1329 | + | } | |
| 1330 | + | ||
| 1331 | + | // Store pending popup for ready-to-show handling (just visibility now) | |
| 1215 | 1332 | pending := &PendingPopup{ | |
| 1216 | 1333 | WebView: popupWV, | |
| 1217 | 1334 | ParentPaneID: parentPaneID, | |
| @@ -1227,7 +1344,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1227 | 1344 | c.pendingPopups[popupID] = pending | |
| 1228 | 1345 | c.popupMu.Unlock() | |
| 1229 | 1346 | ||
| 1230 | - | // Wire ready-to-show and close signals | |
| 1347 | + | // Wire ready-to-show (now just for visibility) and close signals | |
| 1231 | 1348 | popupWV.OnReadyToShow = func() { | |
| 1232 | 1349 | c.handlePopupReadyToShow(ctx, popupID) | |
| 1233 | 1350 | } | |
| @@ -1237,15 +1354,17 @@ func (c *ContentCoordinator) handlePopupCreate( | |||
| 1237 | 1354 | ||
| 1238 | 1355 | log.Info(). | |
| 1239 | 1356 | Uint64("popup_id", uint64(popupID)). | |
| 1357 | + | Str("pane_id", string(paneID)). | |
| 1240 | 1358 | Str("popup_type", popupType.String()). | |
| 1241 | - | Str("target_uri", req.TargetURI). | |
| 1242 | - | Msg("popup created, awaiting ready-to-show") | |
| 1359 | + | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |
| 1360 | + | Msg("popup inserted (hidden), awaiting ready-to-show for visibility") | |
| 1243 | 1361 | ||
| 1244 | 1362 | return popupWV | |
| 1245 | 1363 | } | |
| 1246 | 1364 | ||
| 1247 | 1365 | // handlePopupReadyToShow handles the WebKit "ready-to-show" signal. | |
| 1248 | - | // This is called when the popup is configured and ready to be displayed. | |
| 1366 | + | // The popup WebView was already inserted into the workspace (hidden) during | |
| 1367 | + | // the create signal. This handler just makes it visible. | |
| 1249 | 1368 | func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID port.WebViewID) { | |
| 1250 | 1369 | log := logging.FromContext(ctx) | |
| 1251 | 1370 | ||
| @@ -1265,67 +1384,17 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID | |||
| 1265 | 1384 | log.Debug(). | |
| 1266 | 1385 | Uint64("popup_id", uint64(popupID)). | |
| 1267 | 1386 | Str("popup_type", pending.PopupType.String()). | |
| 1268 | - | Msg("popup ready to show") | |
| 1269 | - | ||
| 1270 | - | // Determine behavior from config | |
| 1271 | - | behavior := GetBehavior(pending.PopupType, c.popupConfig) | |
| 1272 | - | placement := "right" | |
| 1273 | - | if c.popupConfig != nil { | |
| 1274 | - | placement = c.popupConfig.Placement | |
| 1275 | - | } | |
| 1276 | - | ||
| 1277 | - | // Create popup pane entity | |
| 1278 | - | var paneID entity.PaneID | |
| 1279 | - | if c.generateID != nil { | |
| 1280 | - | paneID = entity.PaneID(c.generateID()) | |
| 1281 | - | } else { | |
| 1282 | - | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |
| 1283 | - | } | |
| 1387 | + | Msg("popup ready to show - making visible") | |
| 1284 | 1388 | ||
| 1285 | - | popupPane := entity.NewPane(paneID) | |
| 1286 | - | popupPane.WindowType = entity.WindowPopup | |
| 1287 | - | popupPane.IsRelated = true | |
| 1288 | - | popupPane.ParentPaneID = &pending.ParentPaneID | |
| 1289 | - | popupPane.URI = pending.TargetURI | |
| 1290 | - | ||
| 1291 | - | // Enable OAuth auto-close if configured | |
| 1292 | - | if c.popupConfig != nil && c.popupConfig.OAuthAutoClose && IsOAuthURL(pending.TargetURI) { | |
| 1293 | - | popupPane.AutoClose = true | |
| 1294 | - | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |
| 1295 | - | } | |
| 1296 | - | ||
| 1297 | - | // Register WebView in our map | |
| 1298 | - | c.webViews[paneID] = pending.WebView | |
| 1299 | - | ||
| 1300 | - | // Setup standard callbacks | |
| 1301 | - | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | |
| 1302 | - | ||
| 1303 | - | // Request insertion into workspace | |
| 1304 | - | if c.onInsertPopup != nil { | |
| 1305 | - | popupInput := InsertPopupInput{ | |
| 1306 | - | ParentPaneID: pending.ParentPaneID, | |
| 1307 | - | PopupPane: popupPane, | |
| 1308 | - | WebView: pending.WebView, | |
| 1309 | - | Behavior: behavior, | |
| 1310 | - | Placement: placement, | |
| 1311 | - | PopupType: pending.PopupType, | |
| 1312 | - | TargetURI: pending.TargetURI, | |
| 1313 | - | } | |
| 1314 | - | ||
| 1315 | - | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |
| 1316 | - | log.Error().Err(err).Msg("failed to insert popup into workspace") | |
| 1317 | - | // Clean up on failure | |
| 1318 | - | delete(c.webViews, paneID) | |
| 1319 | - | pending.WebView.Destroy() | |
| 1320 | - | return | |
| 1321 | - | } | |
| 1389 | + | // Make the WebView visible now that it's ready | |
| 1390 | + | if pending.WebView != nil { | |
| 1391 | + | pending.WebView.Show() | |
| 1322 | 1392 | } | |
| 1323 | 1393 | ||
| 1324 | 1394 | log.Info(). | |
| 1325 | - | Str("pane_id", string(paneID)). | |
| 1326 | - | Str("behavior", string(behavior)). | |
| 1327 | - | Str("placement", placement). | |
| 1328 | - | Msg("popup inserted into workspace") | |
| 1395 | + | Uint64("popup_id", uint64(popupID)). | |
| 1396 | + | Str("target_uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)). | |
| 1397 | + | Msg("popup now visible") | |
| 1329 | 1398 | } | |
| 1330 | 1399 | ||
| 1331 | 1400 | // handlePopupClose handles the WebKit "close" signal for popup windows. | |
| @@ -1426,9 +1495,44 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1426 | 1495 | } | |
| 1427 | 1496 | ||
| 1428 | 1497 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | |
| 1498 | + | // It uses URL pattern detection for standard OAuth callbacks (code=, access_token=, etc.) | |
| 1499 | + | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | |
| 1500 | + | // window.close() which triggers WebKit's close signal. | |
| 1501 | + | // A long safety timeout (30s) catches popups that get stuck. | |
| 1429 | 1502 | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1430 | 1503 | log := logging.FromContext(ctx) | |
| 1431 | 1504 | ||
| 1505 | + | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | |
| 1506 | + | var safetyTimer *time.Timer | |
| 1507 | + | var safetyTimerMu sync.Mutex | |
| 1508 | + | const oauthSafetyTimeout = 30 * time.Second | |
| 1509 | + | ||
| 1510 | + | startSafetyTimer := func() { | |
| 1511 | + | safetyTimerMu.Lock() | |
| 1512 | + | defer safetyTimerMu.Unlock() | |
| 1513 | + | if safetyTimer != nil { | |
| 1514 | + | safetyTimer.Stop() | |
| 1515 | + | } | |
| 1516 | + | safetyTimer = time.AfterFunc(oauthSafetyTimeout, func() { | |
| 1517 | + | if wv != nil && !wv.IsDestroyed() { | |
| 1518 | + | log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup") | |
| 1519 | + | wv.Close() | |
| 1520 | + | } | |
| 1521 | + | }) | |
| 1522 | + | } | |
| 1523 | + | ||
| 1524 | + | cancelSafetyTimer := func() { | |
| 1525 | + | safetyTimerMu.Lock() | |
| 1526 | + | defer safetyTimerMu.Unlock() | |
| 1527 | + | if safetyTimer != nil { | |
| 1528 | + | safetyTimer.Stop() | |
| 1529 | + | safetyTimer = nil | |
| 1530 | + | } | |
| 1531 | + | } | |
| 1532 | + | ||
| 1533 | + | // Start safety timer immediately | |
| 1534 | + | startSafetyTimer() | |
| 1535 | + | ||
| 1432 | 1536 | // Wrap the existing OnURIChanged to also check for OAuth callbacks | |
| 1433 | 1537 | originalURIChanged := wv.OnURIChanged | |
| 1434 | 1538 | wv.OnURIChanged = func(uri string) { | |
| @@ -1437,39 +1541,32 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||
| 1437 | 1541 | originalURIChanged(uri) | |
| 1438 | 1542 | } | |
| 1439 | 1543 | ||
| 1440 | - | // Check for OAuth callback | |
| 1544 | + | // Check for OAuth callback (URL-based detection) | |
| 1441 | 1545 | if ShouldAutoClose(uri) { | |
| 1442 | - | log.Info(). | |
| 1443 | - | Str("pane_id", string(paneID)). | |
| 1444 | - | Str("uri", uri). | |
| 1445 | - | Msg("OAuth callback detected, auto-closing popup") | |
| 1446 | - | ||
| 1447 | - | // Trigger close after a short delay to let the page process | |
| 1546 | + | cancelSafetyTimer() | |
| 1547 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback detected, closing") | |
| 1448 | 1548 | go func() { | |
| 1449 | 1549 | time.Sleep(500 * time.Millisecond) | |
| 1450 | 1550 | if wv != nil && !wv.IsDestroyed() { | |
| 1451 | - | // This will trigger handlePopupClose | |
| 1452 | 1551 | wv.Close() | |
| 1453 | 1552 | } | |
| 1454 | 1553 | }() | |
| 1455 | 1554 | } | |
| 1456 | 1555 | } | |
| 1457 | 1556 | ||
| 1458 | - | // Also check on load committed for immediate redirects | |
| 1557 | + | // Monitor load events for URL-based detection | |
| 1459 | 1558 | originalLoadChanged := wv.OnLoadChanged | |
| 1460 | 1559 | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |
| 1461 | 1560 | if originalLoadChanged != nil { | |
| 1462 | 1561 | originalLoadChanged(event) | |
| 1463 | 1562 | } | |
| 1464 | 1563 | ||
| 1564 | + | // Check URL on commit for immediate detection | |
| 1465 | 1565 | if event == webkit.LoadCommitted { | |
| 1466 | 1566 | uri := wv.URI() | |
| 1467 | 1567 | if ShouldAutoClose(uri) { | |
| 1468 | - | log.Info(). | |
| 1469 | - | Str("pane_id", string(paneID)). | |
| 1470 | - | Str("uri", uri). | |
| 1471 | - | Msg("OAuth callback detected on load, auto-closing popup") | |
| 1472 | - | ||
| 1568 | + | cancelSafetyTimer() | |
| 1569 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback on load, closing") | |
| 1473 | 1570 | go func() { | |
| 1474 | 1571 | time.Sleep(500 * time.Millisecond) | |
| 1475 | 1572 | if wv != nil && !wv.IsDestroyed() { | |
| @@ -1479,6 +1576,15 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||
| 1479 | 1576 | } | |
| 1480 | 1577 | } | |
| 1481 | 1578 | } | |
| 1579 | + | ||
| 1580 | + | // Wrap OnClose to cancel safety timer when popup closes normally | |
| 1581 | + | originalOnClose := wv.OnClose | |
| 1582 | + | wv.OnClose = func() { | |
| 1583 | + | cancelSafetyTimer() | |
| 1584 | + | if originalOnClose != nil { | |
| 1585 | + | originalOnClose() | |
| 1586 | + | } | |
| 1587 | + | } | |
| 1482 | 1588 | } | |
| 1483 | 1589 | ||
| 1484 | 1590 | // handleLinkMiddleClick handles middle-click / Ctrl+click on links. | |
| @@ -21,7 +21,10 @@ import ( | |||||
| 21 | "github.com/jwijenbergh/puregotk/v4/gdk" | 21 | "github.com/jwijenbergh/puregotk/v4/gdk" | ||
| 22 | ) | 22 | ) | ||
| 23 | 23 | ||||
| 24 | - | const aboutBlankURI = "about:blank" | 24 | + | const ( |
| 25 | + | aboutBlankURI = "about:blank" | |||
| 26 | + | logURLMaxLen = 80 | |||
| 27 | + | ) | |||
| 25 | 28 | ||||
| 26 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | 29 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | ||
| 27 | type ContentCoordinator struct { | 30 | type ContentCoordinator struct { | ||
| @@ -95,6 +98,10 @@ type ContentCoordinator struct { | |||||
| 95 | 98 | ||||
| 96 | // Callback when WebView gains focus (for accent picker text input targeting) | 99 | // Callback when WebView gains focus (for accent picker text input targeting) | ||
| 97 | onWebViewFocused func(paneID entity.PaneID, wv *webkit.WebView) | 100 | onWebViewFocused func(paneID entity.PaneID, wv *webkit.WebView) | ||
| 101 | + | ||||
| 102 | + | // Callback for first load_started event (triggers deferred initialization) | |||
| 103 | + | onFirstLoadStarted func() | |||
| 104 | + | loadStartedOnce sync.Once | |||
| 98 | } | 105 | } | ||
| 99 | 106 | ||||
| 100 | type pendingThemeUpdate struct { | 107 | type pendingThemeUpdate struct { | ||
| @@ -173,6 +180,13 @@ func (c *ContentCoordinator) SetOnWebViewFocused(fn func(paneID entity.PaneID, w | |||||
| 173 | c.onWebViewFocused = fn | 180 | c.onWebViewFocused = fn | ||
| 174 | } | 181 | } | ||
| 175 | 182 | ||||
| 183 | + | // SetOnFirstLoadStarted sets the callback for when the first navigation starts. | |||
| 184 | + | // This is used to trigger deferred initialization after the initial load_uri() | |||
| 185 | + | // has been processed by the GTK main loop. | |||
| 186 | + | func (c *ContentCoordinator) SetOnFirstLoadStarted(fn func()) { | |||
| 187 | + | c.onFirstLoadStarted = fn | |||
| 188 | + | } | |||
| 189 | + | ||||
| 176 | // EnsureWebView acquires or reuses a WebView for the given pane. | 190 | // EnsureWebView acquires or reuses a WebView for the given pane. | ||
| 177 | func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (*webkit.WebView, error) { | 191 | func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (*webkit.WebView, error) { | ||
| 178 | log := logging.FromContext(ctx) | 192 | log := logging.FromContext(ctx) | ||
| @@ -185,10 +199,16 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||||
| 185 | return nil, fmt.Errorf("webview pool not configured") | 199 | return nil, fmt.Errorf("webview pool not configured") | ||
| 186 | } | 200 | } | ||
| 187 | 201 | ||||
| 202 | + | // Mark tab_created on first webview (first tab) | |||
| 203 | + | if len(c.webViews) == 0 { | |||
| 204 | + | logging.Trace().Mark("tab_created") | |||
| 205 | + | } | |||
| 206 | + | ||||
| 188 | wv, err := c.pool.Acquire(ctx) | 207 | wv, err := c.pool.Acquire(ctx) | ||
| 189 | if err != nil { | 208 | if err != nil { | ||
| 190 | return nil, err | 209 | return nil, err | ||
| 191 | } | 210 | } | ||
| 211 | + | logging.Trace().Mark("webview_acquired") | |||
| 192 | 212 | ||||
| 193 | c.webViews[paneID] = wv | 213 | c.webViews[paneID] = wv | ||
| 194 | 214 | ||||
| @@ -331,6 +351,7 @@ func (c *ContentCoordinator) AttachToWorkspace(ctx context.Context, ws *entity.W | |||||
| 331 | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | 351 | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | ||
| 332 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | 352 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | ||
| 333 | } | 353 | } | ||
| 354 | + | logging.Trace().Mark("webview_attached") | |||
| 334 | } | 355 | } | ||
| 335 | } | 356 | } | ||
| 336 | 357 | ||||
| @@ -908,6 +929,7 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||||
| 908 | // Also shows the WebView widget (it's hidden during creation to avoid white flash). | 929 | // Also shows the WebView widget (it's hidden during creation to avoid white flash). | ||
| 909 | func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 930 | func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | ||
| 910 | log := logging.FromContext(ctx) | 931 | log := logging.FromContext(ctx) | ||
| 932 | + | logging.Trace().Mark("load_committed") | |||
| 911 | 933 | ||||
| 912 | url := wv.URI() | 934 | url := wv.URI() | ||
| 913 | if url == "" { | 935 | if url == "" { | ||
| @@ -1020,6 +1042,16 @@ func (c *ContentCoordinator) updatePaneURI(paneID entity.PaneID, url string) { | |||||
| 1020 | 1042 | ||||
| 1021 | // onLoadStarted shows the progress bar when page loading begins. | 1043 | // onLoadStarted shows the progress bar when page loading begins. | ||
| 1022 | func (c *ContentCoordinator) onLoadStarted(paneID entity.PaneID) { | 1044 | func (c *ContentCoordinator) onLoadStarted(paneID entity.PaneID) { | ||
| 1045 | + | logging.Trace().Mark("load_started") | |||
| 1046 | + | ||||
| 1047 | + | // Trigger deferred initialization on first load_started. | |||
| 1048 | + | // This ensures non-critical init runs after initial navigation starts. | |||
| 1049 | + | c.loadStartedOnce.Do(func() { | |||
| 1050 | + | if c.onFirstLoadStarted != nil { | |||
| 1051 | + | c.onFirstLoadStarted() | |||
| 1052 | + | } | |||
| 1053 | + | }) | |||
| 1054 | + | ||||
| 1023 | _, wsView := c.getActiveWS() | 1055 | _, wsView := c.getActiveWS() | ||
| 1024 | if wsView == nil { | 1056 | if wsView == nil { | ||
| 1025 | return | 1057 | return | ||
| @@ -1106,6 +1138,11 @@ func (c *ContentCoordinator) revealIfPending(ctx context.Context, paneID entity. | |||||
| 1106 | Str("reason", reason). | 1138 | Str("reason", reason). | ||
| 1107 | Msg("webview revealed") | 1139 | Msg("webview revealed") | ||
| 1108 | } | 1140 | } | ||
| 1141 | + | ||||
| 1142 | + | // Mark first_paint and finish startup trace | |||
| 1143 | + | logging.Trace().Mark("first_paint") | |||
| 1144 | + | logging.Trace().Finish() | |||
| 1145 | + | ||||
| 1109 | if c.onWebViewShown != nil { | 1146 | if c.onWebViewShown != nil { | ||
| 1110 | c.onWebViewShown(paneID) | 1147 | c.onWebViewShown(paneID) | ||
| 1111 | } | 1148 | } | ||
| @@ -1170,8 +1207,34 @@ func (c *ContentCoordinator) SetupPopupHandling(ctx context.Context, paneID enti | |||||
| 1170 | log.Debug().Str("pane_id", string(paneID)).Msg("popup handling configured for webview") | 1207 | log.Debug().Str("pane_id", string(paneID)).Msg("popup handling configured for webview") | ||
| 1171 | } | 1208 | } | ||
| 1172 | 1209 | ||||
| 1210 | + | // createPopupPane creates a new pane entity for a popup window. | |||
| 1211 | + | func (c *ContentCoordinator) createPopupPane( | |||
| 1212 | + | popupID port.WebViewID, | |||
| 1213 | + | parentPaneID entity.PaneID, | |||
| 1214 | + | targetURI string, | |||
| 1215 | + | ) (entity.PaneID, *entity.Pane) { | |||
| 1216 | + | var paneID entity.PaneID | |||
| 1217 | + | if c.generateID != nil { | |||
| 1218 | + | paneID = entity.PaneID(c.generateID()) | |||
| 1219 | + | } else { | |||
| 1220 | + | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |||
| 1221 | + | } | |||
| 1222 | + | ||||
| 1223 | + | popupPane := entity.NewPane(paneID) | |||
| 1224 | + | popupPane.WindowType = entity.WindowPopup | |||
| 1225 | + | popupPane.IsRelated = true | |||
| 1226 | + | popupPane.ParentPaneID = &parentPaneID | |||
| 1227 | + | popupPane.URI = targetURI | |||
| 1228 | + | ||||
| 1229 | + | return paneID, popupPane | |||
| 1230 | + | } | |||
| 1231 | + | ||||
| 1173 | // handlePopupCreate handles the WebKit "create" signal for popup windows. | 1232 | // handlePopupCreate handles the WebKit "create" signal for popup windows. | ||
| 1174 | // Returns a new related WebView if popup is allowed, nil to block. | 1233 | // Returns a new related WebView if popup is allowed, nil to block. | ||
| 1234 | + | // | |||
| 1235 | + | // IMPORTANT: The WebView MUST be added to a GtkWindow hierarchy BEFORE this | |||
| 1236 | + | // signal handler returns. Otherwise WebKit won't establish window.opener. | |||
| 1237 | + | // The WebView stays hidden until ready-to-show signal is received. | |||
| 1175 | func (c *ContentCoordinator) handlePopupCreate( | 1238 | func (c *ContentCoordinator) handlePopupCreate( | ||
| 1176 | ctx context.Context, | 1239 | ctx context.Context, | ||
| 1177 | parentPaneID entity.PaneID, | 1240 | parentPaneID entity.PaneID, | ||
| @@ -1199,7 +1262,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1199 | return nil | 1262 | return nil | ||
| 1200 | } | 1263 | } | ||
| 1201 | 1264 | ||||
| 1202 | - | // Create related WebView for session sharing | 1265 | + | // Create related WebView for session sharing (created hidden) |
| 1203 | parentID := parentWV.ID() | 1266 | parentID := parentWV.ID() | ||
| 1204 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | 1267 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | ||
| 1205 | if err != nil { | 1268 | if err != nil { | ||
| @@ -1209,9 +1272,63 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1209 | 1272 | ||||
| 1210 | // Detect popup type | 1273 | // Detect popup type | ||
| 1211 | popupType := DetectPopupType(req.FrameName) | 1274 | popupType := DetectPopupType(req.FrameName) | ||
| 1212 | - | ||||
| 1213 | - | // Store pending popup for ready-to-show handling | |||
| 1214 | popupID := popupWV.ID() | 1275 | popupID := popupWV.ID() | ||
| 1276 | + | ||||
| 1277 | + | // Determine behavior from config | |||
| 1278 | + | behavior := GetBehavior(popupType, c.popupConfig) | |||
| 1279 | + | placement := "right" | |||
| 1280 | + | if c.popupConfig != nil { | |||
| 1281 | + | placement = c.popupConfig.Placement | |||
| 1282 | + | } | |||
| 1283 | + | ||||
| 1284 | + | // Create popup pane entity | |||
| 1285 | + | paneID, popupPane := c.createPopupPane(popupID, parentPaneID, req.TargetURI) | |||
| 1286 | + | ||||
| 1287 | + | // Check OAuth configuration | |||
| 1288 | + | hasConfig := c.popupConfig != nil | |||
| 1289 | + | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |||
| 1290 | + | isOAuth := IsOAuthURL(req.TargetURI) | |||
| 1291 | + | log.Debug(). | |||
| 1292 | + | Bool("has_config", hasConfig). | |||
| 1293 | + | Bool("oauth_enabled", oauthEnabled). | |||
| 1294 | + | Bool("is_oauth", isOAuth). | |||
| 1295 | + | Str("uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |||
| 1296 | + | Msg("popup OAuth check") | |||
| 1297 | + | ||||
| 1298 | + | // Insert into workspace IMMEDIATELY (WebView stays hidden) | |||
| 1299 | + | // This is required for WebKit to establish window.opener relationship | |||
| 1300 | + | if c.onInsertPopup != nil { | |||
| 1301 | + | popupInput := InsertPopupInput{ | |||
| 1302 | + | ParentPaneID: parentPaneID, | |||
| 1303 | + | PopupPane: popupPane, | |||
| 1304 | + | WebView: popupWV, | |||
| 1305 | + | Behavior: behavior, | |||
| 1306 | + | Placement: placement, | |||
| 1307 | + | PopupType: popupType, | |||
| 1308 | + | TargetURI: req.TargetURI, | |||
| 1309 | + | } | |||
| 1310 | + | ||||
| 1311 | + | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |||
| 1312 | + | log.Error().Err(err).Msg("failed to insert popup into workspace") | |||
| 1313 | + | popupWV.Destroy() | |||
| 1314 | + | return nil | |||
| 1315 | + | } | |||
| 1316 | + | } | |||
| 1317 | + | ||||
| 1318 | + | // Register WebView in our map (after successful insertion) | |||
| 1319 | + | c.webViews[paneID] = popupWV | |||
| 1320 | + | ||||
| 1321 | + | // Setup standard callbacks (after successful insertion to avoid leak) | |||
| 1322 | + | c.setupWebViewCallbacks(ctx, paneID, popupWV) | |||
| 1323 | + | ||||
| 1324 | + | // Setup OAuth auto-close if configured | |||
| 1325 | + | if hasConfig && oauthEnabled && isOAuth { | |||
| 1326 | + | popupPane.AutoClose = true | |||
| 1327 | + | c.setupOAuthAutoClose(ctx, paneID, popupWV) | |||
| 1328 | + | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |||
| 1329 | + | } | |||
| 1330 | + | ||||
| 1331 | + | // Store pending popup for ready-to-show handling (just visibility now) | |||
| 1215 | pending := &PendingPopup{ | 1332 | pending := &PendingPopup{ | ||
| 1216 | WebView: popupWV, | 1333 | WebView: popupWV, | ||
| 1217 | ParentPaneID: parentPaneID, | 1334 | ParentPaneID: parentPaneID, | ||
| @@ -1227,7 +1344,7 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1227 | c.pendingPopups[popupID] = pending | 1344 | c.pendingPopups[popupID] = pending | ||
| 1228 | c.popupMu.Unlock() | 1345 | c.popupMu.Unlock() | ||
| 1229 | 1346 | ||||
| 1230 | - | // Wire ready-to-show and close signals | 1347 | + | // Wire ready-to-show (now just for visibility) and close signals |
| 1231 | popupWV.OnReadyToShow = func() { | 1348 | popupWV.OnReadyToShow = func() { | ||
| 1232 | c.handlePopupReadyToShow(ctx, popupID) | 1349 | c.handlePopupReadyToShow(ctx, popupID) | ||
| 1233 | } | 1350 | } | ||
| @@ -1237,15 +1354,17 @@ func (c *ContentCoordinator) handlePopupCreate( | |||||
| 1237 | 1354 | ||||
| 1238 | log.Info(). | 1355 | log.Info(). | ||
| 1239 | Uint64("popup_id", uint64(popupID)). | 1356 | Uint64("popup_id", uint64(popupID)). | ||
| 1357 | + | Str("pane_id", string(paneID)). | |||
| 1240 | Str("popup_type", popupType.String()). | 1358 | Str("popup_type", popupType.String()). | ||
| 1241 | - | Str("target_uri", req.TargetURI). | 1359 | + | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). |
| 1242 | - | Msg("popup created, awaiting ready-to-show") | 1360 | + | Msg("popup inserted (hidden), awaiting ready-to-show for visibility") |
| 1243 | 1361 | ||||
| 1244 | return popupWV | 1362 | return popupWV | ||
| 1245 | } | 1363 | } | ||
| 1246 | 1364 | ||||
| 1247 | // handlePopupReadyToShow handles the WebKit "ready-to-show" signal. | 1365 | // handlePopupReadyToShow handles the WebKit "ready-to-show" signal. | ||
| 1248 | - | // This is called when the popup is configured and ready to be displayed. | 1366 | + | // The popup WebView was already inserted into the workspace (hidden) during |
| 1367 | + | // the create signal. This handler just makes it visible. | |||
| 1249 | func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID port.WebViewID) { | 1368 | func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID port.WebViewID) { | ||
| 1250 | log := logging.FromContext(ctx) | 1369 | log := logging.FromContext(ctx) | ||
| 1251 | 1370 | ||||
| @@ -1265,67 +1384,17 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID | |||||
| 1265 | log.Debug(). | 1384 | log.Debug(). | ||
| 1266 | Uint64("popup_id", uint64(popupID)). | 1385 | Uint64("popup_id", uint64(popupID)). | ||
| 1267 | Str("popup_type", pending.PopupType.String()). | 1386 | Str("popup_type", pending.PopupType.String()). | ||
| 1268 | - | Msg("popup ready to show") | 1387 | + | Msg("popup ready to show - making visible") |
| 1269 | - | ||||
| 1270 | - | // Determine behavior from config | |||
| 1271 | - | behavior := GetBehavior(pending.PopupType, c.popupConfig) | |||
| 1272 | - | placement := "right" | |||
| 1273 | - | if c.popupConfig != nil { | |||
| 1274 | - | placement = c.popupConfig.Placement | |||
| 1275 | - | } | |||
| 1276 | - | ||||
| 1277 | - | // Create popup pane entity | |||
| 1278 | - | var paneID entity.PaneID | |||
| 1279 | - | if c.generateID != nil { | |||
| 1280 | - | paneID = entity.PaneID(c.generateID()) | |||
| 1281 | - | } else { | |||
| 1282 | - | paneID = entity.PaneID(fmt.Sprintf("popup_%d", popupID)) | |||
| 1283 | - | } | |||
| 1284 | 1388 | ||||
| 1285 | - | popupPane := entity.NewPane(paneID) | 1389 | + | // Make the WebView visible now that it's ready |
| 1286 | - | popupPane.WindowType = entity.WindowPopup | 1390 | + | if pending.WebView != nil { |
| 1287 | - | popupPane.IsRelated = true | 1391 | + | pending.WebView.Show() |
| 1288 | - | popupPane.ParentPaneID = &pending.ParentPaneID | |||
| 1289 | - | popupPane.URI = pending.TargetURI | |||
| 1290 | - | ||||
| 1291 | - | // Enable OAuth auto-close if configured | |||
| 1292 | - | if c.popupConfig != nil && c.popupConfig.OAuthAutoClose && IsOAuthURL(pending.TargetURI) { | |||
| 1293 | - | popupPane.AutoClose = true | |||
| 1294 | - | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |||
| 1295 | - | } | |||
| 1296 | - | ||||
| 1297 | - | // Register WebView in our map | |||
| 1298 | - | c.webViews[paneID] = pending.WebView | |||
| 1299 | - | ||||
| 1300 | - | // Setup standard callbacks | |||
| 1301 | - | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | |||
| 1302 | - | ||||
| 1303 | - | // Request insertion into workspace | |||
| 1304 | - | if c.onInsertPopup != nil { | |||
| 1305 | - | popupInput := InsertPopupInput{ | |||
| 1306 | - | ParentPaneID: pending.ParentPaneID, | |||
| 1307 | - | PopupPane: popupPane, | |||
| 1308 | - | WebView: pending.WebView, | |||
| 1309 | - | Behavior: behavior, | |||
| 1310 | - | Placement: placement, | |||
| 1311 | - | PopupType: pending.PopupType, | |||
| 1312 | - | TargetURI: pending.TargetURI, | |||
| 1313 | - | } | |||
| 1314 | - | ||||
| 1315 | - | if err := c.onInsertPopup(ctx, popupInput); err != nil { | |||
| 1316 | - | log.Error().Err(err).Msg("failed to insert popup into workspace") | |||
| 1317 | - | // Clean up on failure | |||
| 1318 | - | delete(c.webViews, paneID) | |||
| 1319 | - | pending.WebView.Destroy() | |||
| 1320 | - | return | |||
| 1321 | - | } | |||
| 1322 | } | 1392 | } | ||
| 1323 | 1393 | ||||
| 1324 | log.Info(). | 1394 | log.Info(). | ||
| 1325 | - | Str("pane_id", string(paneID)). | 1395 | + | Uint64("popup_id", uint64(popupID)). |
| 1326 | - | Str("behavior", string(behavior)). | 1396 | + | Str("target_uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)). |
| 1327 | - | Str("placement", placement). | 1397 | + | Msg("popup now visible") |
| 1328 | - | Msg("popup inserted into workspace") | |||
| 1329 | } | 1398 | } | ||
| 1330 | 1399 | ||||
| 1331 | // handlePopupClose handles the WebKit "close" signal for popup windows. | 1400 | // handlePopupClose handles the WebKit "close" signal for popup windows. | ||
| @@ -1426,9 +1495,44 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1426 | } | 1495 | } | ||
| 1427 | 1496 | ||||
| 1428 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | 1497 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | ||
| 1498 | + | // It uses URL pattern detection for standard OAuth callbacks (code=, access_token=, etc.) | |||
| 1499 | + | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | |||
| 1500 | + | // window.close() which triggers WebKit's close signal. | |||
| 1501 | + | // A long safety timeout (30s) catches popups that get stuck. | |||
| 1429 | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 1502 | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | ||
| 1430 | log := logging.FromContext(ctx) | 1503 | log := logging.FromContext(ctx) | ||
| 1431 | 1504 | ||||
| 1505 | + | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | |||
| 1506 | + | var safetyTimer *time.Timer | |||
| 1507 | + | var safetyTimerMu sync.Mutex | |||
| 1508 | + | const oauthSafetyTimeout = 30 * time.Second | |||
| 1509 | + | ||||
| 1510 | + | startSafetyTimer := func() { | |||
| 1511 | + | safetyTimerMu.Lock() | |||
| 1512 | + | defer safetyTimerMu.Unlock() | |||
| 1513 | + | if safetyTimer != nil { | |||
| 1514 | + | safetyTimer.Stop() | |||
| 1515 | + | } | |||
| 1516 | + | safetyTimer = time.AfterFunc(oauthSafetyTimeout, func() { | |||
| 1517 | + | if wv != nil && !wv.IsDestroyed() { | |||
| 1518 | + | log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup") | |||
| 1519 | + | wv.Close() | |||
| 1520 | + | } | |||
| 1521 | + | }) | |||
| 1522 | + | } | |||
| 1523 | + | ||||
| 1524 | + | cancelSafetyTimer := func() { | |||
| 1525 | + | safetyTimerMu.Lock() | |||
| 1526 | + | defer safetyTimerMu.Unlock() | |||
| 1527 | + | if safetyTimer != nil { | |||
| 1528 | + | safetyTimer.Stop() | |||
| 1529 | + | safetyTimer = nil | |||
| 1530 | + | } | |||
| 1531 | + | } | |||
| 1532 | + | ||||
| 1533 | + | // Start safety timer immediately | |||
| 1534 | + | startSafetyTimer() | |||
| 1535 | + | ||||
| 1432 | // Wrap the existing OnURIChanged to also check for OAuth callbacks | 1536 | // Wrap the existing OnURIChanged to also check for OAuth callbacks | ||
| 1433 | originalURIChanged := wv.OnURIChanged | 1537 | originalURIChanged := wv.OnURIChanged | ||
| 1434 | wv.OnURIChanged = func(uri string) { | 1538 | wv.OnURIChanged = func(uri string) { | ||
| @@ -1437,39 +1541,32 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||||
| 1437 | originalURIChanged(uri) | 1541 | originalURIChanged(uri) | ||
| 1438 | } | 1542 | } | ||
| 1439 | 1543 | ||||
| 1440 | - | // Check for OAuth callback | 1544 | + | // Check for OAuth callback (URL-based detection) |
| 1441 | if ShouldAutoClose(uri) { | 1545 | if ShouldAutoClose(uri) { | ||
| 1442 | - | log.Info(). | 1546 | + | cancelSafetyTimer() |
| 1443 | - | Str("pane_id", string(paneID)). | 1547 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback detected, closing") |
| 1444 | - | Str("uri", uri). | |||
| 1445 | - | Msg("OAuth callback detected, auto-closing popup") | |||
| 1446 | - | ||||
| 1447 | - | // Trigger close after a short delay to let the page process | |||
| 1448 | go func() { | 1548 | go func() { | ||
| 1449 | time.Sleep(500 * time.Millisecond) | 1549 | time.Sleep(500 * time.Millisecond) | ||
| 1450 | if wv != nil && !wv.IsDestroyed() { | 1550 | if wv != nil && !wv.IsDestroyed() { | ||
| 1451 | - | // This will trigger handlePopupClose | |||
| 1452 | wv.Close() | 1551 | wv.Close() | ||
| 1453 | } | 1552 | } | ||
| 1454 | }() | 1553 | }() | ||
| 1455 | } | 1554 | } | ||
| 1456 | } | 1555 | } | ||
| 1457 | 1556 | ||||
| 1458 | - | // Also check on load committed for immediate redirects | 1557 | + | // Monitor load events for URL-based detection |
| 1459 | originalLoadChanged := wv.OnLoadChanged | 1558 | originalLoadChanged := wv.OnLoadChanged | ||
| 1460 | wv.OnLoadChanged = func(event webkit.LoadEvent) { | 1559 | wv.OnLoadChanged = func(event webkit.LoadEvent) { | ||
| 1461 | if originalLoadChanged != nil { | 1560 | if originalLoadChanged != nil { | ||
| 1462 | originalLoadChanged(event) | 1561 | originalLoadChanged(event) | ||
| 1463 | } | 1562 | } | ||
| 1464 | 1563 | ||||
| 1564 | + | // Check URL on commit for immediate detection | |||
| 1465 | if event == webkit.LoadCommitted { | 1565 | if event == webkit.LoadCommitted { | ||
| 1466 | uri := wv.URI() | 1566 | uri := wv.URI() | ||
| 1467 | if ShouldAutoClose(uri) { | 1567 | if ShouldAutoClose(uri) { | ||
| 1468 | - | log.Info(). | 1568 | + | cancelSafetyTimer() |
| 1469 | - | Str("pane_id", string(paneID)). | 1569 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback on load, closing") |
| 1470 | - | Str("uri", uri). | |||
| 1471 | - | Msg("OAuth callback detected on load, auto-closing popup") | |||
| 1472 | - | ||||
| 1473 | go func() { | 1570 | go func() { | ||
| 1474 | time.Sleep(500 * time.Millisecond) | 1571 | time.Sleep(500 * time.Millisecond) | ||
| 1475 | if wv != nil && !wv.IsDestroyed() { | 1572 | if wv != nil && !wv.IsDestroyed() { | ||
| @@ -1479,6 +1576,15 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||||
| 1479 | } | 1576 | } | ||
| 1480 | } | 1577 | } | ||
| 1481 | } | 1578 | } | ||
| 1579 | + | ||||
| 1580 | + | // Wrap OnClose to cancel safety timer when popup closes normally | |||
| 1581 | + | originalOnClose := wv.OnClose | |||
| 1582 | + | wv.OnClose = func() { | |||
| 1583 | + | cancelSafetyTimer() | |||
| 1584 | + | if originalOnClose != nil { | |||
| 1585 | + | originalOnClose() | |||
| 1586 | + | } | |||
| 1587 | + | } | |||
| 1482 | } | 1588 | } | ||
| 1483 | 1589 | ||||
| 1484 | // handleLinkMiddleClick handles middle-click / Ctrl+click on links. | 1590 | // handleLinkMiddleClick handles middle-click / Ctrl+click on links. | ||
M internal/ui/coordinator/oauth.go +0 -1
| @@ -101,7 +101,6 @@ func IsOAuthCallback(url string) bool { | |||
| 101 | 101 | // - Errors: error=, error_description= | |
| 102 | 102 | // - Various OAuth providers (Google, GitHub, Auth0, etc.) | |
| 103 | 103 | func ShouldAutoClose(url string) bool { | |
| 104 | - | // Only auto-close on callback URLs that have OAuth response params | |
| 105 | 104 | return IsOAuthCallback(url) | |
| 106 | 105 | } | |
| 107 | 106 | ||
| @@ -101,7 +101,6 @@ func IsOAuthCallback(url string) bool { | |||||
| 101 | // - Errors: error=, error_description= | 101 | // - Errors: error=, error_description= | ||
| 102 | // - Various OAuth providers (Google, GitHub, Auth0, etc.) | 102 | // - Various OAuth providers (Google, GitHub, Auth0, etc.) | ||
| 103 | func ShouldAutoClose(url string) bool { | 103 | func ShouldAutoClose(url string) bool { | ||
| 104 | - | // Only auto-close on callback URLs that have OAuth response params | |||
| 105 | return IsOAuthCallback(url) | 104 | return IsOAuthCallback(url) | ||
| 106 | } | 105 | } | ||
| 107 | 106 | ||||
M internal/ui/coordinator/workspace.go +57 -6
| @@ -1135,11 +1135,24 @@ func (c *WorkspaceCoordinator) syncStackedPaneTitles(ctx context.Context, stacke | |||
| 1135 | 1135 | return | |
| 1136 | 1136 | } | |
| 1137 | 1137 | ||
| 1138 | + | // Get UI pane count to avoid index out of bounds when domain and UI are out of sync | |
| 1139 | + | uiPaneCount := stackedView.Count() | |
| 1140 | + | ||
| 1138 | 1141 | for i, child := range stackNode.Children { | |
| 1139 | 1142 | if child.Pane == nil { | |
| 1140 | 1143 | continue | |
| 1141 | 1144 | } | |
| 1142 | 1145 | ||
| 1146 | + | // Skip if index exceeds UI pane count (domain/UI desync) | |
| 1147 | + | if i >= uiPaneCount { | |
| 1148 | + | log.Debug(). | |
| 1149 | + | Int("index", i). | |
| 1150 | + | Int("ui_count", uiPaneCount). | |
| 1151 | + | Str("pane_id", string(child.Pane.ID)). | |
| 1152 | + | Msg("skipping title sync: index exceeds UI pane count") | |
| 1153 | + | continue | |
| 1154 | + | } | |
| 1155 | + | ||
| 1143 | 1156 | title := c.contentCoord.GetTitle(child.Pane.ID) | |
| 1144 | 1157 | if title == "" { | |
| 1145 | 1158 | title = child.Pane.Title | |
| @@ -1798,11 +1811,13 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins | |||
| 1798 | 1811 | return fmt.Errorf("parent pane not found: %s", input.ParentPaneID) | |
| 1799 | 1812 | } | |
| 1800 | 1813 | ||
| 1801 | - | // Resolve or create stack node | |
| 1802 | - | stackNode := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID) | |
| 1814 | + | // Resolve or create stack node (track conversion for potential rollback) | |
| 1815 | + | stackNode, conversionInfo := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID) | |
| 1803 | 1816 | ||
| 1804 | 1817 | // Add popup pane to stack using use case | |
| 1805 | 1818 | if _, err := c.panesUC.AddToStack(ctx, ws, stackNode, input.PopupPane); err != nil { | |
| 1819 | + | // Rollback stack conversion if we created a new stack | |
| 1820 | + | conversionInfo.revert() | |
| 1806 | 1821 | return fmt.Errorf("failed to add popup to stack: %w", err) | |
| 1807 | 1822 | } | |
| 1808 | 1823 | ||
| @@ -1812,6 +1827,17 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins | |||
| 1812 | 1827 | } | |
| 1813 | 1828 | ||
| 1814 | 1829 | if err := c.attachPopupPaneView(ctx, input, wsView, stackNode, log); err != nil { | |
| 1830 | + | // UI update failed - rollback domain changes to maintain consistency | |
| 1831 | + | log.Warn().Err(err).Msg("rolling back domain changes due to UI failure") | |
| 1832 | + | ||
| 1833 | + | // Remove popup pane from stack | |
| 1834 | + | if removeErr := c.panesUC.RemoveFromStack(ctx, stackNode, input.PopupPane.ID); removeErr != nil { | |
| 1835 | + | log.Error().Err(removeErr).Msg("failed to rollback popup pane addition") | |
| 1836 | + | } | |
| 1837 | + | ||
| 1838 | + | // Revert stack conversion if we created a new stack | |
| 1839 | + | conversionInfo.revert() | |
| 1840 | + | ||
| 1815 | 1841 | return err | |
| 1816 | 1842 | } | |
| 1817 | 1843 | ||
| @@ -1823,14 +1849,35 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins | |||
| 1823 | 1849 | return nil | |
| 1824 | 1850 | } | |
| 1825 | 1851 | ||
| 1852 | + | // stackConversionInfo holds state needed to revert a leaf-to-stack conversion. | |
| 1853 | + | type stackConversionInfo struct { | |
| 1854 | + | wasConverted bool // true if resolveOrCreateStackNode converted a leaf to a stack | |
| 1855 | + | originalPane *entity.Pane // the pane that was in the node before conversion | |
| 1856 | + | node *entity.PaneNode // the node that was converted | |
| 1857 | + | } | |
| 1858 | + | ||
| 1859 | + | // revert undoes a leaf-to-stack conversion. | |
| 1860 | + | func (info *stackConversionInfo) revert() { | |
| 1861 | + | if !info.wasConverted || info.node == nil { | |
| 1862 | + | return | |
| 1863 | + | } | |
| 1864 | + | ||
| 1865 | + | // Restore node to leaf state | |
| 1866 | + | info.node.Pane = info.originalPane | |
| 1867 | + | info.node.IsStacked = false | |
| 1868 | + | info.node.ActiveStackIndex = 0 | |
| 1869 | + | info.node.Children = nil | |
| 1870 | + | } | |
| 1871 | + | ||
| 1826 | 1872 | // resolveOrCreateStackNode finds an existing stack or creates one for popup insertion. | |
| 1827 | 1873 | // Note: For popups, we can't use CreateStack use case since it creates a new pane, | |
| 1828 | 1874 | // but we already have the popup pane. This conversion is a special case. | |
| 1875 | + | // Returns the stack node and conversion info for potential rollback. | |
| 1829 | 1876 | func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |
| 1830 | 1877 | ctx context.Context, | |
| 1831 | 1878 | parentNode *entity.PaneNode, | |
| 1832 | 1879 | parentPaneID entity.PaneID, | |
| 1833 | - | ) *entity.PaneNode { | |
| 1880 | + | ) (*entity.PaneNode, stackConversionInfo) { | |
| 1834 | 1881 | log := logging.FromContext(ctx) | |
| 1835 | 1882 | ||
| 1836 | 1883 | // Check if parent is already in a stack | |
| @@ -1838,7 +1885,7 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |||
| 1838 | 1885 | log.Debug(). | |
| 1839 | 1886 | Str("stack_id", parentNode.Parent.ID). | |
| 1840 | 1887 | Msg("using existing parent stack for popup") | |
| 1841 | - | return parentNode.Parent | |
| 1888 | + | return parentNode.Parent, stackConversionInfo{} | |
| 1842 | 1889 | } | |
| 1843 | 1890 | ||
| 1844 | 1891 | // Check if parent is already a stack container | |
| @@ -1846,7 +1893,7 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |||
| 1846 | 1893 | log.Debug(). | |
| 1847 | 1894 | Str("stack_id", parentNode.ID). | |
| 1848 | 1895 | Msg("parent is already a stack container") | |
| 1849 | - | return parentNode | |
| 1896 | + | return parentNode, stackConversionInfo{} | |
| 1850 | 1897 | } | |
| 1851 | 1898 | ||
| 1852 | 1899 | // Need to convert leaf to stack for popup insertion. | |
| @@ -1880,7 +1927,11 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |||
| 1880 | 1927 | Str("original_pane", string(originalPane.ID)). | |
| 1881 | 1928 | Msg("converted leaf to stacked node for popup") | |
| 1882 | 1929 | ||
| 1883 | - | return parentNode | |
| 1930 | + | return parentNode, stackConversionInfo{ | |
| 1931 | + | wasConverted: true, | |
| 1932 | + | originalPane: originalPane, | |
| 1933 | + | node: parentNode, | |
| 1934 | + | } | |
| 1884 | 1935 | } | |
| 1885 | 1936 | ||
| 1886 | 1937 | func (c *WorkspaceCoordinator) attachPopupPaneView( | |
| @@ -1135,11 +1135,24 @@ func (c *WorkspaceCoordinator) syncStackedPaneTitles(ctx context.Context, stacke | |||||
| 1135 | return | 1135 | return | ||
| 1136 | } | 1136 | } | ||
| 1137 | 1137 | ||||
| 1138 | + | // Get UI pane count to avoid index out of bounds when domain and UI are out of sync | |||
| 1139 | + | uiPaneCount := stackedView.Count() | |||
| 1140 | + | ||||
| 1138 | for i, child := range stackNode.Children { | 1141 | for i, child := range stackNode.Children { | ||
| 1139 | if child.Pane == nil { | 1142 | if child.Pane == nil { | ||
| 1140 | continue | 1143 | continue | ||
| 1141 | } | 1144 | } | ||
| 1142 | 1145 | ||||
| 1146 | + | // Skip if index exceeds UI pane count (domain/UI desync) | |||
| 1147 | + | if i >= uiPaneCount { | |||
| 1148 | + | log.Debug(). | |||
| 1149 | + | Int("index", i). | |||
| 1150 | + | Int("ui_count", uiPaneCount). | |||
| 1151 | + | Str("pane_id", string(child.Pane.ID)). | |||
| 1152 | + | Msg("skipping title sync: index exceeds UI pane count") | |||
| 1153 | + | continue | |||
| 1154 | + | } | |||
| 1155 | + | ||||
| 1143 | title := c.contentCoord.GetTitle(child.Pane.ID) | 1156 | title := c.contentCoord.GetTitle(child.Pane.ID) | ||
| 1144 | if title == "" { | 1157 | if title == "" { | ||
| 1145 | title = child.Pane.Title | 1158 | title = child.Pane.Title | ||
| @@ -1798,11 +1811,13 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins | |||||
| 1798 | return fmt.Errorf("parent pane not found: %s", input.ParentPaneID) | 1811 | return fmt.Errorf("parent pane not found: %s", input.ParentPaneID) | ||
| 1799 | } | 1812 | } | ||
| 1800 | 1813 | ||||
| 1801 | - | // Resolve or create stack node | 1814 | + | // Resolve or create stack node (track conversion for potential rollback) |
| 1802 | - | stackNode := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID) | 1815 | + | stackNode, conversionInfo := c.resolveOrCreateStackNode(ctx, parentNode, input.ParentPaneID) |
| 1803 | 1816 | ||||
| 1804 | // Add popup pane to stack using use case | 1817 | // Add popup pane to stack using use case | ||
| 1805 | if _, err := c.panesUC.AddToStack(ctx, ws, stackNode, input.PopupPane); err != nil { | 1818 | if _, err := c.panesUC.AddToStack(ctx, ws, stackNode, input.PopupPane); err != nil { | ||
| 1819 | + | // Rollback stack conversion if we created a new stack | |||
| 1820 | + | conversionInfo.revert() | |||
| 1806 | return fmt.Errorf("failed to add popup to stack: %w", err) | 1821 | return fmt.Errorf("failed to add popup to stack: %w", err) | ||
| 1807 | } | 1822 | } | ||
| 1808 | 1823 | ||||
| @@ -1812,6 +1827,17 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins | |||||
| 1812 | } | 1827 | } | ||
| 1813 | 1828 | ||||
| 1814 | if err := c.attachPopupPaneView(ctx, input, wsView, stackNode, log); err != nil { | 1829 | if err := c.attachPopupPaneView(ctx, input, wsView, stackNode, log); err != nil { | ||
| 1830 | + | // UI update failed - rollback domain changes to maintain consistency | |||
| 1831 | + | log.Warn().Err(err).Msg("rolling back domain changes due to UI failure") | |||
| 1832 | + | ||||
| 1833 | + | // Remove popup pane from stack | |||
| 1834 | + | if removeErr := c.panesUC.RemoveFromStack(ctx, stackNode, input.PopupPane.ID); removeErr != nil { | |||
| 1835 | + | log.Error().Err(removeErr).Msg("failed to rollback popup pane addition") | |||
| 1836 | + | } | |||
| 1837 | + | ||||
| 1838 | + | // Revert stack conversion if we created a new stack | |||
| 1839 | + | conversionInfo.revert() | |||
| 1840 | + | ||||
| 1815 | return err | 1841 | return err | ||
| 1816 | } | 1842 | } | ||
| 1817 | 1843 | ||||
| @@ -1823,14 +1849,35 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input Ins | |||||
| 1823 | return nil | 1849 | return nil | ||
| 1824 | } | 1850 | } | ||
| 1825 | 1851 | ||||
| 1852 | + | // stackConversionInfo holds state needed to revert a leaf-to-stack conversion. | |||
| 1853 | + | type stackConversionInfo struct { | |||
| 1854 | + | wasConverted bool // true if resolveOrCreateStackNode converted a leaf to a stack | |||
| 1855 | + | originalPane *entity.Pane // the pane that was in the node before conversion | |||
| 1856 | + | node *entity.PaneNode // the node that was converted | |||
| 1857 | + | } | |||
| 1858 | + | ||||
| 1859 | + | // revert undoes a leaf-to-stack conversion. | |||
| 1860 | + | func (info *stackConversionInfo) revert() { | |||
| 1861 | + | if !info.wasConverted || info.node == nil { | |||
| 1862 | + | return | |||
| 1863 | + | } | |||
| 1864 | + | ||||
| 1865 | + | // Restore node to leaf state | |||
| 1866 | + | info.node.Pane = info.originalPane | |||
| 1867 | + | info.node.IsStacked = false | |||
| 1868 | + | info.node.ActiveStackIndex = 0 | |||
| 1869 | + | info.node.Children = nil | |||
| 1870 | + | } | |||
| 1871 | + | ||||
| 1826 | // resolveOrCreateStackNode finds an existing stack or creates one for popup insertion. | 1872 | // resolveOrCreateStackNode finds an existing stack or creates one for popup insertion. | ||
| 1827 | // Note: For popups, we can't use CreateStack use case since it creates a new pane, | 1873 | // Note: For popups, we can't use CreateStack use case since it creates a new pane, | ||
| 1828 | // but we already have the popup pane. This conversion is a special case. | 1874 | // but we already have the popup pane. This conversion is a special case. | ||
| 1875 | + | // Returns the stack node and conversion info for potential rollback. | |||
| 1829 | func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | 1876 | func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | ||
| 1830 | ctx context.Context, | 1877 | ctx context.Context, | ||
| 1831 | parentNode *entity.PaneNode, | 1878 | parentNode *entity.PaneNode, | ||
| 1832 | parentPaneID entity.PaneID, | 1879 | parentPaneID entity.PaneID, | ||
| 1833 | - | ) *entity.PaneNode { | 1880 | + | ) (*entity.PaneNode, stackConversionInfo) { |
| 1834 | log := logging.FromContext(ctx) | 1881 | log := logging.FromContext(ctx) | ||
| 1835 | 1882 | ||||
| 1836 | // Check if parent is already in a stack | 1883 | // Check if parent is already in a stack | ||
| @@ -1838,7 +1885,7 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |||||
| 1838 | log.Debug(). | 1885 | log.Debug(). | ||
| 1839 | Str("stack_id", parentNode.Parent.ID). | 1886 | Str("stack_id", parentNode.Parent.ID). | ||
| 1840 | Msg("using existing parent stack for popup") | 1887 | Msg("using existing parent stack for popup") | ||
| 1841 | - | return parentNode.Parent | 1888 | + | return parentNode.Parent, stackConversionInfo{} |
| 1842 | } | 1889 | } | ||
| 1843 | 1890 | ||||
| 1844 | // Check if parent is already a stack container | 1891 | // Check if parent is already a stack container | ||
| @@ -1846,7 +1893,7 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |||||
| 1846 | log.Debug(). | 1893 | log.Debug(). | ||
| 1847 | Str("stack_id", parentNode.ID). | 1894 | Str("stack_id", parentNode.ID). | ||
| 1848 | Msg("parent is already a stack container") | 1895 | Msg("parent is already a stack container") | ||
| 1849 | - | return parentNode | 1896 | + | return parentNode, stackConversionInfo{} |
| 1850 | } | 1897 | } | ||
| 1851 | 1898 | ||||
| 1852 | // Need to convert leaf to stack for popup insertion. | 1899 | // Need to convert leaf to stack for popup insertion. | ||
| @@ -1880,7 +1927,11 @@ func (c *WorkspaceCoordinator) resolveOrCreateStackNode( | |||||
| 1880 | Str("original_pane", string(originalPane.ID)). | 1927 | Str("original_pane", string(originalPane.ID)). | ||
| 1881 | Msg("converted leaf to stacked node for popup") | 1928 | Msg("converted leaf to stacked node for popup") | ||
| 1882 | 1929 | ||||
| 1883 | - | return parentNode | 1930 | + | return parentNode, stackConversionInfo{ |
| 1931 | + | wasConverted: true, | |||
| 1932 | + | originalPane: originalPane, | |||
| 1933 | + | node: parentNode, | |||
| 1934 | + | } | |||
| 1884 | } | 1935 | } | ||
| 1885 | 1936 | ||||
| 1886 | func (c *WorkspaceCoordinator) attachPopupPaneView( | 1937 | func (c *WorkspaceCoordinator) attachPopupPaneView( | ||
M internal/ui/deps.go +1 -0
| @@ -25,6 +25,7 @@ type Dependencies struct { | |||
| 25 | 25 | InitialURL string // URL to open on startup (optional) | |
| 26 | 26 | RestoreSessionID string // Session ID to restore on startup (optional) | |
| 27 | 27 | OnFirstWebViewShown func(context.Context) | |
| 28 | + | OnSessionPersisted func() // Called by main after session is persisted to DB | |
| 28 | 29 | ||
| 29 | 30 | // Theme and color scheme management | |
| 30 | 31 | Theme *theme.Manager | |
| @@ -25,6 +25,7 @@ type Dependencies struct { | |||||
| 25 | InitialURL string // URL to open on startup (optional) | 25 | InitialURL string // URL to open on startup (optional) | ||
| 26 | RestoreSessionID string // Session ID to restore on startup (optional) | 26 | RestoreSessionID string // Session ID to restore on startup (optional) | ||
| 27 | OnFirstWebViewShown func(context.Context) | 27 | OnFirstWebViewShown func(context.Context) | ||
| 28 | + | OnSessionPersisted func() // Called by main after session is persisted to DB | |||
| 28 | 29 | ||||
| 29 | // Theme and color scheme management | 30 | // Theme and color scheme management | ||
| 30 | Theme *theme.Manager | 31 | Theme *theme.Manager | ||
M internal/ui/input/gesture.go +14 -5
| @@ -107,11 +107,20 @@ func (h *GestureHandler) handlePressed(nPress int) { | |||
| 107 | 107 | if handler != nil { | |
| 108 | 108 | if err := handler(h.ctx, action); err != nil { | |
| 109 | 109 | log := logging.FromContext(h.ctx) | |
| 110 | - | log.Error(). | |
| 111 | - | Err(err). | |
| 112 | - | Str("action", string(action)). | |
| 113 | - | Uint("button", button). | |
| 114 | - | Msg("gesture action handler error") | |
| 110 | + | // Navigation errors are expected (e.g., no back/forward history) | |
| 111 | + | if action == ActionGoBack || action == ActionGoForward { | |
| 112 | + | log.Debug(). | |
| 113 | + | Err(err). | |
| 114 | + | Str("action", string(action)). | |
| 115 | + | Uint("button", button). | |
| 116 | + | Msg("gesture navigation not available") | |
| 117 | + | } else { | |
| 118 | + | log.Error(). | |
| 119 | + | Err(err). | |
| 120 | + | Str("action", string(action)). | |
| 121 | + | Uint("button", button). | |
| 122 | + | Msg("gesture action handler error") | |
| 123 | + | } | |
| 115 | 124 | } | |
| 116 | 125 | } | |
| 117 | 126 | } | |
| @@ -107,11 +107,20 @@ func (h *GestureHandler) handlePressed(nPress int) { | |||||
| 107 | if handler != nil { | 107 | if handler != nil { | ||
| 108 | if err := handler(h.ctx, action); err != nil { | 108 | if err := handler(h.ctx, action); err != nil { | ||
| 109 | log := logging.FromContext(h.ctx) | 109 | log := logging.FromContext(h.ctx) | ||
| 110 | - | log.Error(). | 110 | + | // Navigation errors are expected (e.g., no back/forward history) |
| 111 | - | Err(err). | 111 | + | if action == ActionGoBack || action == ActionGoForward { |
| 112 | - | Str("action", string(action)). | 112 | + | log.Debug(). |
| 113 | - | Uint("button", button). | 113 | + | Err(err). |
| 114 | - | Msg("gesture action handler error") | 114 | + | Str("action", string(action)). |
| 115 | + | Uint("button", button). | |||
| 116 | + | Msg("gesture navigation not available") | |||
| 117 | + | } else { | |||
| 118 | + | log.Error(). | |||
| 119 | + | Err(err). | |||
| 120 | + | Str("action", string(action)). | |||
| 121 | + | Uint("button", button). | |||
| 122 | + | Msg("gesture action handler error") | |||
| 123 | + | } | |||
| 115 | } | 124 | } | ||
| 116 | } | 125 | } | ||
| 117 | } | 126 | } | ||