perf(cef): GUI initialization and first-frame scheduling (#412)
* test(cef): isolate collector fixtures with fake Go/Git provenance * feat(cef): bind collector provenance to measured binary and probe runtime * feat(cef): add isolated startup harness with scenario fixtures * perf(cef): decode packaged PNG for loading logo * fix(cef): submit one navigation per intent * fix(cef): share safety merge and combine omnibox re-exec * fix(cef): resolve lint for startup lot * fix(cef): correlate replay browsers by identifier * fix(cef): split navigation claim to satisfy complexity gate * fix(cef): address Copilot review on startup lot * fix(cef): address second Copilot review on startup lot * fix(cef): address CodeRabbit review on startup lot Claim navigation against the attached browser instance so a swap between the identifier check and the claim retries instead of submitting to a stale frame. Merge every GODEBUG environ entry before normalization so duplicates collapse without dropping unrelated keys. Replace hashlib.file_digest with a 3.x-compatible streaming hash in all four checksum paths. Require exact redirect document counts, abort process-warm on failed warm-up, and bind the manifest to embedded VCS metadata from a stamped build. Fix the collector fake to emit the real go version -m format. * fix(cef): resubmit pending navigation when browser is replaced after claim * feat(cef): add bounded residency policy with generation guard * feat(cef): add startup-only residency timeout config * feat(cef): integrate residency hold and close paths * feat(cef): integrate residency lifecycle with quiescence * fix(cef): resolve lint for residency lifecycle * feat(cef): converge close paths and add reopen diagnostics * fix(cef): address senior review blockers on residency * fix(cef): address Copilot review on residency stack * feat(cef): construct hidden sidebars on first request Window creation no longer constructs the hidden history and favorites panels or queries their use cases. New ensureHistorySidebar/ensureFavoritesSidebar build each panel once on first open with a detached lifetime context, leaving mounting and visibility to the existing show path. Outer keyboard-action handlers ensure before testing availability so shortcuts keep working; hide paths construct nothing. Covers issues #396. * feat(cef): warm lazy database while the engine initializes Non-restore browser startup creates the single LazyDB provider before CEF initialization and warms it in an owned background task with a detached context, so slow database setup overlaps engine startup instead of stalling GTK on first access. Close waits for in-flight initialization to settle before releasing the connection and never triggers initialization itself. Covers issue #398. * perf(cef): scope deferred diagnostics and theme reapply Automatic startup no longer runs WebKit pkg-config and GStreamer media diagnostics under CEF; explicit doctor and media commands still invoke the checks directly. Theme application skips the CSS reload and provider re-add for an unchanged theme on the same display, tracking effective CSS identity and native display address without retaining display lifetimes. Covers issues #399 and #402. * fix(cef): address Go review on GUI presentation stack Track each Warmup background task synchronously so Close settles warmup before checking in-flight initialization, closing the goroutine-scheduling orphan window. Track the applied GTK font per display instead of globally so every display gets its GtkSettings configured. Prove deferred checks are not invoked under CEF with a probe hook, and document the mutable test seams as non-parallel. * fix(cef): address Copilot review on GUI presentation stack Serialize LazyDB admission with closure so no initialization can start once Close begins (late callers get a closed-provider error), abort admitted in-flight work via the owned init context, and make repeat Close idempotent. Drop constructor data loads so first Show issues the single query batch; reopening reloads per existing semantics. Cover all four paths with call-count and admission tests.
- commit
- 66de0ba62a2299e905818137fce654e4bf1e7b39
- author
- bnema · · 2026-09-08 10:54 UTC
- committer
- GitHub · · 2026-09-08 10:54 UTC
- parent
- dcda339
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
16 files · +924 -67
M cmd/dumber/main.go +17 -5
| @@ -667,6 +667,20 @@ func initStackAndRepos( | |||
| 667 | 667 | } | |
| 668 | 668 | ||
| 669 | 669 | log := logging.FromContext(ctx) | |
| 670 | + | ||
| 671 | + | // Non-restore browser startup uses the same lazy provider throughout: | |
| 672 | + | // create it before engine initialization and warm it in an owned | |
| 673 | + | // background task so slow database initialization overlaps CEF startup | |
| 674 | + | // instead of blocking the GTK main context on first access. The detached | |
| 675 | + | // warmup context preserves values without letting shutdown cancellation | |
| 676 | + | // poison the cached initialization result; Close waits for the warmup to | |
| 677 | + | // settle before releasing the connection. | |
| 678 | + | lazyDB, err := bootstrap.CreateLazyDatabase() | |
| 679 | + | if err != nil { | |
| 680 | + | return nil, nil, nil, err | |
| 681 | + | } | |
| 682 | + | lazyDB.Warmup(context.WithoutCancel(ctx)) | |
| 683 | + | ||
| 670 | 684 | engine, err := bootstrap.BuildEngine(bootstrap.EngineInput{ | |
| 671 | 685 | Ctx: ctx, | |
| 672 | 686 | Config: cfg, | |
| @@ -677,14 +691,12 @@ func initStackAndRepos( | |||
| 677 | 691 | Logger: *log, | |
| 678 | 692 | }) | |
| 679 | 693 | if err != nil { | |
| 694 | + | _ = lazyDB.Close() | |
| 680 | 695 | return nil, nil, nil, err | |
| 681 | 696 | } | |
| 682 | 697 | ||
| 683 | - | repos, dbCleanup, err := initStandaloneOmniboxRepos() | |
| 684 | - | if err != nil { | |
| 685 | - | return nil, nil, nil, err | |
| 686 | - | } | |
| 687 | - | return engine, repos, dbCleanup, nil | |
| 698 | + | dbCleanup := func() { _ = lazyDB.Close() } | |
| 699 | + | return engine, createLazyRepositories(lazyDB), dbCleanup, nil | |
| 688 | 700 | } | |
| 689 | 701 | ||
| 690 | 702 | func initStandaloneOmniboxRepos() (*repositories, func(), error) { | |
| @@ -667,6 +667,20 @@ func initStackAndRepos( | |||||
| 667 | } | 667 | } | ||
| 668 | 668 | ||||
| 669 | log := logging.FromContext(ctx) | 669 | log := logging.FromContext(ctx) | ||
| 670 | + | ||||
| 671 | + | // Non-restore browser startup uses the same lazy provider throughout: | |||
| 672 | + | // create it before engine initialization and warm it in an owned | |||
| 673 | + | // background task so slow database initialization overlaps CEF startup | |||
| 674 | + | // instead of blocking the GTK main context on first access. The detached | |||
| 675 | + | // warmup context preserves values without letting shutdown cancellation | |||
| 676 | + | // poison the cached initialization result; Close waits for the warmup to | |||
| 677 | + | // settle before releasing the connection. | |||
| 678 | + | lazyDB, err := bootstrap.CreateLazyDatabase() | |||
| 679 | + | if err != nil { | |||
| 680 | + | return nil, nil, nil, err | |||
| 681 | + | } | |||
| 682 | + | lazyDB.Warmup(context.WithoutCancel(ctx)) | |||
| 683 | + | ||||
| 670 | engine, err := bootstrap.BuildEngine(bootstrap.EngineInput{ | 684 | engine, err := bootstrap.BuildEngine(bootstrap.EngineInput{ | ||
| 671 | Ctx: ctx, | 685 | Ctx: ctx, | ||
| 672 | Config: cfg, | 686 | Config: cfg, | ||
| @@ -677,14 +691,12 @@ func initStackAndRepos( | |||||
| 677 | Logger: *log, | 691 | Logger: *log, | ||
| 678 | }) | 692 | }) | ||
| 679 | if err != nil { | 693 | if err != nil { | ||
| 694 | + | _ = lazyDB.Close() | |||
| 680 | return nil, nil, nil, err | 695 | return nil, nil, nil, err | ||
| 681 | } | 696 | } | ||
| 682 | 697 | ||||
| 683 | - | repos, dbCleanup, err := initStandaloneOmniboxRepos() | 698 | + | dbCleanup := func() { _ = lazyDB.Close() } |
| 684 | - | if err != nil { | 699 | + | return engine, createLazyRepositories(lazyDB), dbCleanup, nil |
| 685 | - | return nil, nil, nil, err | |||
| 686 | - | } | |||
| 687 | - | return engine, repos, dbCleanup, nil | |||
| 688 | } | 700 | } | ||
| 689 | 701 | ||||
| 690 | func initStandaloneOmniboxRepos() (*repositories, func(), error) { | 702 | func initStandaloneOmniboxRepos() (*repositories, func(), error) { | ||
A internal/bootstrap/deferred_init_test.go +55 -0
| @@ -0,0 +1,55 @@ | |||
| 1 | + | package bootstrap | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "sync" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 9 | + | "github.com/stretchr/testify/assert" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // TestRunDeferredInit_CEFExecutesNoWebKitDiagnostics verifies that automatic | |
| 14 | + | // CEF startup runs no unrelated WebKit pkg-config or GStreamer media | |
| 15 | + | // diagnostics. Explicit diagnostic commands invoke the Check* functions | |
| 16 | + | // directly and are unaffected. | |
| 17 | + | func TestRunDeferredInit_CEFExecutesNoWebKitDiagnostics(t *testing.T) { | |
| 18 | + | t.Setenv("DUMBER_ENGINE", config.EngineTypeCEF) | |
| 19 | + | cfg := config.DefaultConfig() | |
| 20 | + | cfg.Engine.Type = config.EngineTypeCEF | |
| 21 | + | ||
| 22 | + | // The probe fires inside each check goroutine, so a zero count proves | |
| 23 | + | // engine selection skipped the checks without invoking them. Not | |
| 24 | + | // parallel-safe with other probe users by design; restored via defer. | |
| 25 | + | var mu sync.Mutex | |
| 26 | + | var invoked []string | |
| 27 | + | deferredInitProbeForTest = func(kind string) { | |
| 28 | + | mu.Lock() | |
| 29 | + | defer mu.Unlock() | |
| 30 | + | invoked = append(invoked, kind) | |
| 31 | + | } | |
| 32 | + | defer func() { deferredInitProbeForTest = nil }() | |
| 33 | + | ||
| 34 | + | result := RunDeferredInit(DeferredInitInput{Ctx: context.Background(), Config: cfg}) | |
| 35 | + | ||
| 36 | + | assert.NoError(t, result.RuntimeErr, "CEF startup must not run WebKit runtime diagnostics") | |
| 37 | + | assert.NoError(t, result.MediaErr, "CEF startup must not run GStreamer media diagnostics") | |
| 38 | + | assert.Empty(t, invoked, "CEF startup must not invoke WebKit runtime/media diagnostics at all") | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // TestCheckFunctions_RemainAvailableForExplicitCommands verifies the | |
| 42 | + | // underlying checks stay callable for explicit diagnostic commands after | |
| 43 | + | // RunDeferredInit stops invoking them automatically for CEF. | |
| 44 | + | func TestCheckFunctions_RemainAvailableForExplicitCommands(t *testing.T) { | |
| 45 | + | t.Setenv("DUMBER_ENGINE", config.EngineTypeCEF) | |
| 46 | + | cfg := config.DefaultConfig() | |
| 47 | + | cfg.Engine.Type = config.EngineTypeCEF | |
| 48 | + | ||
| 49 | + | // The functions must exist and be invocable regardless of engine; their | |
| 50 | + | // results depend on host tooling, so only invocation (not success) is | |
| 51 | + | // asserted here. | |
| 52 | + | _ = CheckRuntimeRequirements(context.Background(), cfg) | |
| 53 | + | _ = CheckMediaRequirements(context.Background(), cfg) | |
| 54 | + | require.NotNil(t, cfg) | |
| 55 | + | } | |
| @@ -0,0 +1,55 @@ | |||||
| 1 | + | package bootstrap | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "sync" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |||
| 9 | + | "github.com/stretchr/testify/assert" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // TestRunDeferredInit_CEFExecutesNoWebKitDiagnostics verifies that automatic | |||
| 14 | + | // CEF startup runs no unrelated WebKit pkg-config or GStreamer media | |||
| 15 | + | // diagnostics. Explicit diagnostic commands invoke the Check* functions | |||
| 16 | + | // directly and are unaffected. | |||
| 17 | + | func TestRunDeferredInit_CEFExecutesNoWebKitDiagnostics(t *testing.T) { | |||
| 18 | + | t.Setenv("DUMBER_ENGINE", config.EngineTypeCEF) | |||
| 19 | + | cfg := config.DefaultConfig() | |||
| 20 | + | cfg.Engine.Type = config.EngineTypeCEF | |||
| 21 | + | ||||
| 22 | + | // The probe fires inside each check goroutine, so a zero count proves | |||
| 23 | + | // engine selection skipped the checks without invoking them. Not | |||
| 24 | + | // parallel-safe with other probe users by design; restored via defer. | |||
| 25 | + | var mu sync.Mutex | |||
| 26 | + | var invoked []string | |||
| 27 | + | deferredInitProbeForTest = func(kind string) { | |||
| 28 | + | mu.Lock() | |||
| 29 | + | defer mu.Unlock() | |||
| 30 | + | invoked = append(invoked, kind) | |||
| 31 | + | } | |||
| 32 | + | defer func() { deferredInitProbeForTest = nil }() | |||
| 33 | + | ||||
| 34 | + | result := RunDeferredInit(DeferredInitInput{Ctx: context.Background(), Config: cfg}) | |||
| 35 | + | ||||
| 36 | + | assert.NoError(t, result.RuntimeErr, "CEF startup must not run WebKit runtime diagnostics") | |||
| 37 | + | assert.NoError(t, result.MediaErr, "CEF startup must not run GStreamer media diagnostics") | |||
| 38 | + | assert.Empty(t, invoked, "CEF startup must not invoke WebKit runtime/media diagnostics at all") | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | // TestCheckFunctions_RemainAvailableForExplicitCommands verifies the | |||
| 42 | + | // underlying checks stay callable for explicit diagnostic commands after | |||
| 43 | + | // RunDeferredInit stops invoking them automatically for CEF. | |||
| 44 | + | func TestCheckFunctions_RemainAvailableForExplicitCommands(t *testing.T) { | |||
| 45 | + | t.Setenv("DUMBER_ENGINE", config.EngineTypeCEF) | |||
| 46 | + | cfg := config.DefaultConfig() | |||
| 47 | + | cfg.Engine.Type = config.EngineTypeCEF | |||
| 48 | + | ||||
| 49 | + | // The functions must exist and be invocable regardless of engine; their | |||
| 50 | + | // results depend on host tooling, so only invocation (not success) is | |||
| 51 | + | // asserted here. | |||
| 52 | + | _ = CheckRuntimeRequirements(context.Background(), cfg) | |||
| 53 | + | _ = CheckMediaRequirements(context.Background(), cfg) | |||
| 54 | + | require.NotNil(t, cfg) | |||
| 55 | + | } | |||
M internal/bootstrap/gui.go +22 -2
| @@ -196,25 +196,45 @@ func LogResolvedTheme(ctx context.Context, resolved entity.ResolvedTheme) { | |||
| 196 | 196 | event.Msg("theme resolved") | |
| 197 | 197 | } | |
| 198 | 198 | ||
| 199 | + | // deferredInitProbeForTest, when non-nil, observes each deferred check | |
| 200 | + | // before it runs. Tests use it to prove engine selection skips checks | |
| 201 | + | // without invoking them. It must remain nil in production. | |
| 202 | + | var deferredInitProbeForTest func(kind string) | |
| 203 | + | ||
| 199 | 204 | // RunDeferredInit runs deferred initialization checks off the critical path. | |
| 200 | - | // This includes runtime requirements and media checks. | |
| 205 | + | // Applicable checks are selected by engine: WebKit runs the pkg-config | |
| 206 | + | // runtime and GStreamer media diagnostics, while CEF skips both (they cover | |
| 207 | + | // the WebKit stack only) and reports no error. Explicit diagnostic commands | |
| 208 | + | // (doctor, media checks) invoke the Check* functions directly and are | |
| 209 | + | // unaffected by this selection. | |
| 201 | 210 | func RunDeferredInit(input DeferredInitInput) DeferredInitResult { | |
| 211 | + | start := time.Now() | |
| 212 | + | if input.Config != nil && input.Config.Engine.ResolveEngineType() == config.EngineTypeCEF { | |
| 213 | + | logging.FromContext(input.Ctx).Debug().Msg("skipping WebKit runtime/media diagnostics for CEF engine") | |
| 214 | + | return DeferredInitResult{Duration: time.Since(start)} | |
| 215 | + | } | |
| 216 | + | ||
| 202 | 217 | var ( | |
| 203 | 218 | runtimeErr error | |
| 204 | 219 | mediaErr error | |
| 205 | 220 | wg sync.WaitGroup | |
| 206 | 221 | ) | |
| 207 | 222 | ||
| 208 | - | start := time.Now() | |
| 209 | 223 | wg.Add(2) | |
| 210 | 224 | ||
| 211 | 225 | go func() { | |
| 212 | 226 | defer wg.Done() | |
| 227 | + | if deferredInitProbeForTest != nil { | |
| 228 | + | deferredInitProbeForTest("runtime") | |
| 229 | + | } | |
| 213 | 230 | runtimeErr = CheckRuntimeRequirements(input.Ctx, input.Config) | |
| 214 | 231 | }() | |
| 215 | 232 | ||
| 216 | 233 | go func() { | |
| 217 | 234 | defer wg.Done() | |
| 235 | + | if deferredInitProbeForTest != nil { | |
| 236 | + | deferredInitProbeForTest("media") | |
| 237 | + | } | |
| 218 | 238 | mediaErr = CheckMediaRequirements(input.Ctx, input.Config) | |
| 219 | 239 | }() | |
| 220 | 240 | ||
| @@ -196,25 +196,45 @@ func LogResolvedTheme(ctx context.Context, resolved entity.ResolvedTheme) { | |||||
| 196 | event.Msg("theme resolved") | 196 | event.Msg("theme resolved") | ||
| 197 | } | 197 | } | ||
| 198 | 198 | ||||
| 199 | + | // deferredInitProbeForTest, when non-nil, observes each deferred check | |||
| 200 | + | // before it runs. Tests use it to prove engine selection skips checks | |||
| 201 | + | // without invoking them. It must remain nil in production. | |||
| 202 | + | var deferredInitProbeForTest func(kind string) | |||
| 203 | + | ||||
| 199 | // RunDeferredInit runs deferred initialization checks off the critical path. | 204 | // RunDeferredInit runs deferred initialization checks off the critical path. | ||
| 200 | - | // This includes runtime requirements and media checks. | 205 | + | // Applicable checks are selected by engine: WebKit runs the pkg-config |
| 206 | + | // runtime and GStreamer media diagnostics, while CEF skips both (they cover | |||
| 207 | + | // the WebKit stack only) and reports no error. Explicit diagnostic commands | |||
| 208 | + | // (doctor, media checks) invoke the Check* functions directly and are | |||
| 209 | + | // unaffected by this selection. | |||
| 201 | func RunDeferredInit(input DeferredInitInput) DeferredInitResult { | 210 | func RunDeferredInit(input DeferredInitInput) DeferredInitResult { | ||
| 211 | + | start := time.Now() | |||
| 212 | + | if input.Config != nil && input.Config.Engine.ResolveEngineType() == config.EngineTypeCEF { | |||
| 213 | + | logging.FromContext(input.Ctx).Debug().Msg("skipping WebKit runtime/media diagnostics for CEF engine") | |||
| 214 | + | return DeferredInitResult{Duration: time.Since(start)} | |||
| 215 | + | } | |||
| 216 | + | ||||
| 202 | var ( | 217 | var ( | ||
| 203 | runtimeErr error | 218 | runtimeErr error | ||
| 204 | mediaErr error | 219 | mediaErr error | ||
| 205 | wg sync.WaitGroup | 220 | wg sync.WaitGroup | ||
| 206 | ) | 221 | ) | ||
| 207 | 222 | ||||
| 208 | - | start := time.Now() | |||
| 209 | wg.Add(2) | 223 | wg.Add(2) | ||
| 210 | 224 | ||||
| 211 | go func() { | 225 | go func() { | ||
| 212 | defer wg.Done() | 226 | defer wg.Done() | ||
| 227 | + | if deferredInitProbeForTest != nil { | |||
| 228 | + | deferredInitProbeForTest("runtime") | |||
| 229 | + | } | |||
| 213 | runtimeErr = CheckRuntimeRequirements(input.Ctx, input.Config) | 230 | runtimeErr = CheckRuntimeRequirements(input.Ctx, input.Config) | ||
| 214 | }() | 231 | }() | ||
| 215 | 232 | ||||
| 216 | go func() { | 233 | go func() { | ||
| 217 | defer wg.Done() | 234 | defer wg.Done() | ||
| 235 | + | if deferredInitProbeForTest != nil { | |||
| 236 | + | deferredInitProbeForTest("media") | |||
| 237 | + | } | |||
| 218 | mediaErr = CheckMediaRequirements(input.Ctx, input.Config) | 238 | mediaErr = CheckMediaRequirements(input.Ctx, input.Config) | ||
| 219 | }() | 239 | }() | ||
| 220 | 240 | ||||
M internal/infrastructure/persistence/sqlite/lazy_db.go +142 -15
| @@ -3,6 +3,7 @@ package sqlite | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "database/sql" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 7 | 8 | "sync" | |
| 8 | 9 | ||
| @@ -11,38 +12,112 @@ import ( | |||
| 11 | 12 | ) | |
| 12 | 13 | ||
| 13 | 14 | // LazyDB implements port.DatabaseProvider with lazy initialization. | |
| 14 | - | // The database connection is created on first access, deferring the ~300-400ms | |
| 15 | - | // WASM compilation and migration overhead until actually needed. | |
| 15 | + | // The database connection is created on first access, deferring the | |
| 16 | + | // connection and migration overhead until actually needed. | |
| 16 | 17 | // | |
| 17 | - | // Currently unused: the application uses eager initialization via OpenDatabase | |
| 18 | - | // in RunParallelDBWebKit. This implementation is kept for potential future use | |
| 19 | - | // in scenarios where deferring database initialization past first paint provides | |
| 20 | - | // additional latency benefits, or for CLI commands that may not need DB access. | |
| 18 | + | // Callers may warm the provider with Warmup while other startup work (for | |
| 19 | + | // example engine initialization) runs concurrently. Warmup registers its | |
| 20 | + | // background task synchronously, so a Close that observes Warmup's return | |
| 21 | + | // always waits for the task to settle instead of racing goroutine | |
| 22 | + | // scheduling. Initialization admission is serialized with closure: once | |
| 23 | + | // Close begins, no new initialization can start and late callers receive a | |
| 24 | + | // closed-provider error, so no connection can open with nobody left to | |
| 25 | + | // close it. An initialization admitted before Close began runs to | |
| 26 | + | // completion (or aborts early via the owned init context) and its | |
| 27 | + | // connection is then released. Ownership contract: Warmup must return | |
| 28 | + | // before Close is called; concurrent Warmup and Close from different | |
| 29 | + | // goroutines without external sequencing is not supported. | |
| 21 | 30 | type LazyDB struct { | |
| 22 | 31 | dbPath string | |
| 23 | 32 | db *sql.DB | |
| 24 | 33 | err error | |
| 25 | 34 | once sync.Once | |
| 26 | 35 | mu sync.RWMutex | |
| 36 | + | // started is closed when the first initialization begins; done is | |
| 37 | + | // closed when it completes. They let Close distinguish "never used" | |
| 38 | + | // from "initialization in flight" without triggering initialization. | |
| 39 | + | started chan struct{} | |
| 40 | + | done chan struct{} | |
| 41 | + | // warmTasks tracks one completion channel per Warmup call, registered | |
| 42 | + | // synchronously under mu. Close waits for every registered task, so a | |
| 43 | + | // Close that observes Warmup's return settles the background task even | |
| 44 | + | // when the once-initializer has not been scheduled yet. Each channel is | |
| 45 | + | // closed exactly once by its own goroutine after its DB call returns, | |
| 46 | + | // which happens after the once-initializer completes, so observing all | |
| 47 | + | // closures implies initialization has settled. | |
| 48 | + | warmTasks []chan struct{} | |
| 49 | + | // closed is set by Close under mu. The once-initializer checks it under | |
| 50 | + | // the same mutex before opening anything, which serializes admission | |
| 51 | + | // with closure: init admitted before Close runs to completion and is | |
| 52 | + | // then released; init arriving after is skipped with ErrLazyDBClosed. | |
| 53 | + | closed bool | |
| 54 | + | // dbClosed records that Close released the connection, making repeat | |
| 55 | + | // Close calls idempotent. | |
| 56 | + | dbClosed bool | |
| 57 | + | // initCtx carries the connection work; initCancel aborts an admitted | |
| 58 | + | // in-flight initialization when Close begins so shutdown (and the | |
| 59 | + | // engine-build error path) is not stuck behind a doomed migration. | |
| 60 | + | // Canceling a settled provider is a no-op for later callers because | |
| 61 | + | // closure already rejects them. | |
| 62 | + | initCtx context.Context | |
| 63 | + | initCancel context.CancelFunc | |
| 27 | 64 | } | |
| 28 | 65 | ||
| 29 | 66 | // Compile-time interface check. | |
| 30 | 67 | var _ port.DatabaseProvider = (*LazyDB)(nil) | |
| 31 | 68 | ||
| 69 | + | // ErrLazyDBClosed is returned to callers arriving after the provider was | |
| 70 | + | // closed, including initializations that lose the admission race. | |
| 71 | + | var ErrLazyDBClosed = errors.New("sqlite: database provider closed") | |
| 72 | + | ||
| 73 | + | // initGateForTest, when non-nil, runs inside the once-initializer after | |
| 74 | + | // admission is granted and before the connection is opened. Tests use it to | |
| 75 | + | // hold an admitted initialization in flight. It must remain nil in | |
| 76 | + | // production. | |
| 77 | + | var initGateForTest func() | |
| 78 | + | ||
| 32 | 79 | // NewLazyDB creates a new lazy database provider. | |
| 33 | - | // The actual connection is not established until DB() is called. | |
| 80 | + | // The actual connection is not established until DB() is called, or until a | |
| 81 | + | // Warmup task triggers initialization in the background. | |
| 34 | 82 | func NewLazyDB(dbPath string) *LazyDB { | |
| 35 | - | return &LazyDB{dbPath: dbPath} | |
| 83 | + | initCtx, initCancel := context.WithCancel(context.Background()) | |
| 84 | + | return &LazyDB{ | |
| 85 | + | dbPath: dbPath, | |
| 86 | + | started: make(chan struct{}), | |
| 87 | + | done: make(chan struct{}), | |
| 88 | + | initCtx: initCtx, | |
| 89 | + | initCancel: initCancel, | |
| 90 | + | } | |
| 36 | 91 | } | |
| 37 | 92 | ||
| 38 | 93 | // DB returns the database connection, initializing it if necessary. | |
| 39 | - | // This method is thread-safe and will only initialize once. | |
| 94 | + | // This method is thread-safe and will only initialize once. Calls arriving | |
| 95 | + | // after Close receive ErrLazyDBClosed without starting initialization. | |
| 40 | 96 | func (l *LazyDB) DB(ctx context.Context) (*sql.DB, error) { | |
| 41 | 97 | l.once.Do(func() { | |
| 98 | + | close(l.started) | |
| 99 | + | defer close(l.done) | |
| 100 | + | l.mu.Lock() | |
| 101 | + | admitted := !l.closed | |
| 102 | + | l.mu.Unlock() | |
| 103 | + | if !admitted { | |
| 104 | + | l.mu.Lock() | |
| 105 | + | l.err = ErrLazyDBClosed | |
| 106 | + | l.mu.Unlock() | |
| 107 | + | logging.FromContext(ctx).Debug().Msg("lazy database initialization skipped: provider closed") | |
| 108 | + | return | |
| 109 | + | } | |
| 110 | + | if initGateForTest != nil { | |
| 111 | + | initGateForTest() | |
| 112 | + | } | |
| 42 | 113 | log := logging.FromContext(ctx) | |
| 43 | 114 | log.Debug().Str("path", l.dbPath).Msg("lazy database initialization starting") | |
| 44 | 115 | ||
| 45 | - | db, err := NewConnection(ctx, l.dbPath) | |
| 116 | + | // The owned init context carries the connection work so Close can | |
| 117 | + | // abort a doomed initialization; caller values are preserved for | |
| 118 | + | // logging above. Shutdown cancellation can no longer poison the | |
| 119 | + | // cache because closure rejects later callers. | |
| 120 | + | db, err := NewConnection(l.initCtx, l.dbPath) | |
| 46 | 121 | ||
| 47 | 122 | // Acquire lock before assigning to ensure IsInitialized() sees | |
| 48 | 123 | // a consistent state when reading l.db under RLock. | |
| @@ -59,24 +134,76 @@ func (l *LazyDB) DB(ctx context.Context) (*sql.DB, error) { | |||
| 59 | 134 | }) | |
| 60 | 135 | ||
| 61 | 136 | l.mu.RLock() | |
| 62 | - | db, err := l.db, l.err | |
| 137 | + | db, err, closed := l.db, l.err, l.closed | |
| 63 | 138 | l.mu.RUnlock() | |
| 64 | 139 | ||
| 65 | 140 | if err != nil { | |
| 141 | + | if errors.Is(err, ErrLazyDBClosed) { | |
| 142 | + | return nil, ErrLazyDBClosed | |
| 143 | + | } | |
| 66 | 144 | return nil, fmt.Errorf("database initialization failed: %w", err) | |
| 67 | 145 | } | |
| 146 | + | if closed { | |
| 147 | + | return nil, ErrLazyDBClosed | |
| 148 | + | } | |
| 68 | 149 | return db, nil | |
| 69 | 150 | } | |
| 70 | 151 | ||
| 71 | - | // Close closes the database connection if it was initialized. | |
| 152 | + | // Warmup triggers database initialization in an owned background task and | |
| 153 | + | // returns immediately. Initialization errors are cached and reported to | |
| 154 | + | // later DB callers; pass a detached context (for example | |
| 155 | + | // context.WithoutCancel) so shutdown cancellation cannot poison the cached | |
| 156 | + | // result. The task is registered synchronously, so Close called after | |
| 157 | + | // Warmup returns settles it (see the ownership contract on LazyDB). | |
| 158 | + | func (l *LazyDB) Warmup(ctx context.Context) { | |
| 159 | + | task := make(chan struct{}) | |
| 160 | + | l.mu.Lock() | |
| 161 | + | l.warmTasks = append(l.warmTasks, task) | |
| 162 | + | l.mu.Unlock() | |
| 163 | + | go func() { | |
| 164 | + | defer close(task) | |
| 165 | + | _, _ = l.DB(ctx) | |
| 166 | + | }() | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | // Close seals the provider against new initialization, aborts admitted | |
| 170 | + | // in-flight connection work via the owned init context, settles every | |
| 171 | + | // registered Warmup task and awaited initialization, then releases the | |
| 172 | + | // connection. No connection can open after Close begins: late initializers | |
| 173 | + | // are skipped with ErrLazyDBClosed and admitted ones are awaited and | |
| 174 | + | // closed. Close never triggers initialization itself, and repeat calls are | |
| 175 | + | // idempotent. | |
| 72 | 176 | func (l *LazyDB) Close() error { | |
| 177 | + | // Abort doomed connection work first so shutdown and the engine-build | |
| 178 | + | // error path are not stuck behind a hung open or migration. Safe to | |
| 179 | + | // call repeatedly and on settled providers. | |
| 180 | + | l.initCancel() | |
| 181 | + | ||
| 182 | + | l.mu.Lock() | |
| 183 | + | l.closed = true | |
| 184 | + | tasks := make([]chan struct{}, len(l.warmTasks)) | |
| 185 | + | copy(tasks, l.warmTasks) | |
| 186 | + | l.mu.Unlock() | |
| 187 | + | for _, task := range tasks { | |
| 188 | + | <-task | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | select { | |
| 192 | + | case <-l.started: | |
| 193 | + | <-l.done | |
| 194 | + | default: | |
| 195 | + | return nil | |
| 196 | + | } | |
| 197 | + | ||
| 73 | 198 | l.mu.Lock() | |
| 74 | 199 | defer l.mu.Unlock() | |
| 75 | 200 | ||
| 76 | - | if l.db != nil { | |
| 77 | - | return l.db.Close() | |
| 201 | + | if l.dbClosed || l.db == nil { | |
| 202 | + | return nil | |
| 78 | 203 | } | |
| 79 | - | return nil | |
| 204 | + | err := l.db.Close() | |
| 205 | + | l.dbClosed = true | |
| 206 | + | return err | |
| 80 | 207 | } | |
| 81 | 208 | ||
| 82 | 209 | // IsInitialized returns true if the database has been initialized. | |
| @@ -3,6 +3,7 @@ package sqlite | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "database/sql" | 5 | "database/sql" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 7 | "sync" | 8 | "sync" | ||
| 8 | 9 | ||||
| @@ -11,38 +12,112 @@ import ( | |||||
| 11 | ) | 12 | ) | ||
| 12 | 13 | ||||
| 13 | // LazyDB implements port.DatabaseProvider with lazy initialization. | 14 | // LazyDB implements port.DatabaseProvider with lazy initialization. | ||
| 14 | - | // The database connection is created on first access, deferring the ~300-400ms | 15 | + | // The database connection is created on first access, deferring the |
| 15 | - | // WASM compilation and migration overhead until actually needed. | 16 | + | // connection and migration overhead until actually needed. |
| 16 | // | 17 | // | ||
| 17 | - | // Currently unused: the application uses eager initialization via OpenDatabase | 18 | + | // Callers may warm the provider with Warmup while other startup work (for |
| 18 | - | // in RunParallelDBWebKit. This implementation is kept for potential future use | 19 | + | // example engine initialization) runs concurrently. Warmup registers its |
| 19 | - | // in scenarios where deferring database initialization past first paint provides | 20 | + | // background task synchronously, so a Close that observes Warmup's return |
| 20 | - | // additional latency benefits, or for CLI commands that may not need DB access. | 21 | + | // always waits for the task to settle instead of racing goroutine |
| 22 | + | // scheduling. Initialization admission is serialized with closure: once | |||
| 23 | + | // Close begins, no new initialization can start and late callers receive a | |||
| 24 | + | // closed-provider error, so no connection can open with nobody left to | |||
| 25 | + | // close it. An initialization admitted before Close began runs to | |||
| 26 | + | // completion (or aborts early via the owned init context) and its | |||
| 27 | + | // connection is then released. Ownership contract: Warmup must return | |||
| 28 | + | // before Close is called; concurrent Warmup and Close from different | |||
| 29 | + | // goroutines without external sequencing is not supported. | |||
| 21 | type LazyDB struct { | 30 | type LazyDB struct { | ||
| 22 | dbPath string | 31 | dbPath string | ||
| 23 | db *sql.DB | 32 | db *sql.DB | ||
| 24 | err error | 33 | err error | ||
| 25 | once sync.Once | 34 | once sync.Once | ||
| 26 | mu sync.RWMutex | 35 | mu sync.RWMutex | ||
| 36 | + | // started is closed when the first initialization begins; done is | |||
| 37 | + | // closed when it completes. They let Close distinguish "never used" | |||
| 38 | + | // from "initialization in flight" without triggering initialization. | |||
| 39 | + | started chan struct{} | |||
| 40 | + | done chan struct{} | |||
| 41 | + | // warmTasks tracks one completion channel per Warmup call, registered | |||
| 42 | + | // synchronously under mu. Close waits for every registered task, so a | |||
| 43 | + | // Close that observes Warmup's return settles the background task even | |||
| 44 | + | // when the once-initializer has not been scheduled yet. Each channel is | |||
| 45 | + | // closed exactly once by its own goroutine after its DB call returns, | |||
| 46 | + | // which happens after the once-initializer completes, so observing all | |||
| 47 | + | // closures implies initialization has settled. | |||
| 48 | + | warmTasks []chan struct{} | |||
| 49 | + | // closed is set by Close under mu. The once-initializer checks it under | |||
| 50 | + | // the same mutex before opening anything, which serializes admission | |||
| 51 | + | // with closure: init admitted before Close runs to completion and is | |||
| 52 | + | // then released; init arriving after is skipped with ErrLazyDBClosed. | |||
| 53 | + | closed bool | |||
| 54 | + | // dbClosed records that Close released the connection, making repeat | |||
| 55 | + | // Close calls idempotent. | |||
| 56 | + | dbClosed bool | |||
| 57 | + | // initCtx carries the connection work; initCancel aborts an admitted | |||
| 58 | + | // in-flight initialization when Close begins so shutdown (and the | |||
| 59 | + | // engine-build error path) is not stuck behind a doomed migration. | |||
| 60 | + | // Canceling a settled provider is a no-op for later callers because | |||
| 61 | + | // closure already rejects them. | |||
| 62 | + | initCtx context.Context | |||
| 63 | + | initCancel context.CancelFunc | |||
| 27 | } | 64 | } | ||
| 28 | 65 | ||||
| 29 | // Compile-time interface check. | 66 | // Compile-time interface check. | ||
| 30 | var _ port.DatabaseProvider = (*LazyDB)(nil) | 67 | var _ port.DatabaseProvider = (*LazyDB)(nil) | ||
| 31 | 68 | ||||
| 69 | + | // ErrLazyDBClosed is returned to callers arriving after the provider was | |||
| 70 | + | // closed, including initializations that lose the admission race. | |||
| 71 | + | var ErrLazyDBClosed = errors.New("sqlite: database provider closed") | |||
| 72 | + | ||||
| 73 | + | // initGateForTest, when non-nil, runs inside the once-initializer after | |||
| 74 | + | // admission is granted and before the connection is opened. Tests use it to | |||
| 75 | + | // hold an admitted initialization in flight. It must remain nil in | |||
| 76 | + | // production. | |||
| 77 | + | var initGateForTest func() | |||
| 78 | + | ||||
| 32 | // NewLazyDB creates a new lazy database provider. | 79 | // NewLazyDB creates a new lazy database provider. | ||
| 33 | - | // The actual connection is not established until DB() is called. | 80 | + | // The actual connection is not established until DB() is called, or until a |
| 81 | + | // Warmup task triggers initialization in the background. | |||
| 34 | func NewLazyDB(dbPath string) *LazyDB { | 82 | func NewLazyDB(dbPath string) *LazyDB { | ||
| 35 | - | return &LazyDB{dbPath: dbPath} | 83 | + | initCtx, initCancel := context.WithCancel(context.Background()) |
| 84 | + | return &LazyDB{ | |||
| 85 | + | dbPath: dbPath, | |||
| 86 | + | started: make(chan struct{}), | |||
| 87 | + | done: make(chan struct{}), | |||
| 88 | + | initCtx: initCtx, | |||
| 89 | + | initCancel: initCancel, | |||
| 90 | + | } | |||
| 36 | } | 91 | } | ||
| 37 | 92 | ||||
| 38 | // DB returns the database connection, initializing it if necessary. | 93 | // DB returns the database connection, initializing it if necessary. | ||
| 39 | - | // This method is thread-safe and will only initialize once. | 94 | + | // This method is thread-safe and will only initialize once. Calls arriving |
| 95 | + | // after Close receive ErrLazyDBClosed without starting initialization. | |||
| 40 | func (l *LazyDB) DB(ctx context.Context) (*sql.DB, error) { | 96 | func (l *LazyDB) DB(ctx context.Context) (*sql.DB, error) { | ||
| 41 | l.once.Do(func() { | 97 | l.once.Do(func() { | ||
| 98 | + | close(l.started) | |||
| 99 | + | defer close(l.done) | |||
| 100 | + | l.mu.Lock() | |||
| 101 | + | admitted := !l.closed | |||
| 102 | + | l.mu.Unlock() | |||
| 103 | + | if !admitted { | |||
| 104 | + | l.mu.Lock() | |||
| 105 | + | l.err = ErrLazyDBClosed | |||
| 106 | + | l.mu.Unlock() | |||
| 107 | + | logging.FromContext(ctx).Debug().Msg("lazy database initialization skipped: provider closed") | |||
| 108 | + | return | |||
| 109 | + | } | |||
| 110 | + | if initGateForTest != nil { | |||
| 111 | + | initGateForTest() | |||
| 112 | + | } | |||
| 42 | log := logging.FromContext(ctx) | 113 | log := logging.FromContext(ctx) | ||
| 43 | log.Debug().Str("path", l.dbPath).Msg("lazy database initialization starting") | 114 | log.Debug().Str("path", l.dbPath).Msg("lazy database initialization starting") | ||
| 44 | 115 | ||||
| 45 | - | db, err := NewConnection(ctx, l.dbPath) | 116 | + | // The owned init context carries the connection work so Close can |
| 117 | + | // abort a doomed initialization; caller values are preserved for | |||
| 118 | + | // logging above. Shutdown cancellation can no longer poison the | |||
| 119 | + | // cache because closure rejects later callers. | |||
| 120 | + | db, err := NewConnection(l.initCtx, l.dbPath) | |||
| 46 | 121 | ||||
| 47 | // Acquire lock before assigning to ensure IsInitialized() sees | 122 | // Acquire lock before assigning to ensure IsInitialized() sees | ||
| 48 | // a consistent state when reading l.db under RLock. | 123 | // a consistent state when reading l.db under RLock. | ||
| @@ -59,24 +134,76 @@ func (l *LazyDB) DB(ctx context.Context) (*sql.DB, error) { | |||||
| 59 | }) | 134 | }) | ||
| 60 | 135 | ||||
| 61 | l.mu.RLock() | 136 | l.mu.RLock() | ||
| 62 | - | db, err := l.db, l.err | 137 | + | db, err, closed := l.db, l.err, l.closed |
| 63 | l.mu.RUnlock() | 138 | l.mu.RUnlock() | ||
| 64 | 139 | ||||
| 65 | if err != nil { | 140 | if err != nil { | ||
| 141 | + | if errors.Is(err, ErrLazyDBClosed) { | |||
| 142 | + | return nil, ErrLazyDBClosed | |||
| 143 | + | } | |||
| 66 | return nil, fmt.Errorf("database initialization failed: %w", err) | 144 | return nil, fmt.Errorf("database initialization failed: %w", err) | ||
| 67 | } | 145 | } | ||
| 146 | + | if closed { | |||
| 147 | + | return nil, ErrLazyDBClosed | |||
| 148 | + | } | |||
| 68 | return db, nil | 149 | return db, nil | ||
| 69 | } | 150 | } | ||
| 70 | 151 | ||||
| 71 | - | // Close closes the database connection if it was initialized. | 152 | + | // Warmup triggers database initialization in an owned background task and |
| 153 | + | // returns immediately. Initialization errors are cached and reported to | |||
| 154 | + | // later DB callers; pass a detached context (for example | |||
| 155 | + | // context.WithoutCancel) so shutdown cancellation cannot poison the cached | |||
| 156 | + | // result. The task is registered synchronously, so Close called after | |||
| 157 | + | // Warmup returns settles it (see the ownership contract on LazyDB). | |||
| 158 | + | func (l *LazyDB) Warmup(ctx context.Context) { | |||
| 159 | + | task := make(chan struct{}) | |||
| 160 | + | l.mu.Lock() | |||
| 161 | + | l.warmTasks = append(l.warmTasks, task) | |||
| 162 | + | l.mu.Unlock() | |||
| 163 | + | go func() { | |||
| 164 | + | defer close(task) | |||
| 165 | + | _, _ = l.DB(ctx) | |||
| 166 | + | }() | |||
| 167 | + | } | |||
| 168 | + | ||||
| 169 | + | // Close seals the provider against new initialization, aborts admitted | |||
| 170 | + | // in-flight connection work via the owned init context, settles every | |||
| 171 | + | // registered Warmup task and awaited initialization, then releases the | |||
| 172 | + | // connection. No connection can open after Close begins: late initializers | |||
| 173 | + | // are skipped with ErrLazyDBClosed and admitted ones are awaited and | |||
| 174 | + | // closed. Close never triggers initialization itself, and repeat calls are | |||
| 175 | + | // idempotent. | |||
| 72 | func (l *LazyDB) Close() error { | 176 | func (l *LazyDB) Close() error { | ||
| 177 | + | // Abort doomed connection work first so shutdown and the engine-build | |||
| 178 | + | // error path are not stuck behind a hung open or migration. Safe to | |||
| 179 | + | // call repeatedly and on settled providers. | |||
| 180 | + | l.initCancel() | |||
| 181 | + | ||||
| 182 | + | l.mu.Lock() | |||
| 183 | + | l.closed = true | |||
| 184 | + | tasks := make([]chan struct{}, len(l.warmTasks)) | |||
| 185 | + | copy(tasks, l.warmTasks) | |||
| 186 | + | l.mu.Unlock() | |||
| 187 | + | for _, task := range tasks { | |||
| 188 | + | <-task | |||
| 189 | + | } | |||
| 190 | + | ||||
| 191 | + | select { | |||
| 192 | + | case <-l.started: | |||
| 193 | + | <-l.done | |||
| 194 | + | default: | |||
| 195 | + | return nil | |||
| 196 | + | } | |||
| 197 | + | ||||
| 73 | l.mu.Lock() | 198 | l.mu.Lock() | ||
| 74 | defer l.mu.Unlock() | 199 | defer l.mu.Unlock() | ||
| 75 | 200 | ||||
| 76 | - | if l.db != nil { | 201 | + | if l.dbClosed || l.db == nil { |
| 77 | - | return l.db.Close() | 202 | + | return nil |
| 78 | } | 203 | } | ||
| 79 | - | return nil | 204 | + | err := l.db.Close() |
| 205 | + | l.dbClosed = true | |||
| 206 | + | return err | |||
| 80 | } | 207 | } | ||
| 81 | 208 | ||||
| 82 | // IsInitialized returns true if the database has been initialized. | 209 | // IsInitialized returns true if the database has been initialized. | ||
A internal/infrastructure/persistence/sqlite/lazy_db_settle_internal_test.go +98 -0
| @@ -0,0 +1,98 @@ | |||
| 1 | + | package sqlite | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "path/filepath" | |
| 6 | + | "sync/atomic" | |
| 7 | + | "testing" | |
| 8 | + | "time" | |
| 9 | + | ||
| 10 | + | "github.com/stretchr/testify/assert" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // TestLazyDB_CloseSettlesAdmittedInit holds an admitted initialization open | |
| 15 | + | // with initGateForTest and proves Close seals admission, settles the | |
| 16 | + | // in-flight work, and leaves no usable connection behind: after Close, | |
| 17 | + | // every DB call fails instead of escaping with an orphaned pool. | |
| 18 | + | func TestLazyDB_CloseSettlesAdmittedInit(t *testing.T) { | |
| 19 | + | entered := make(chan struct{}, 1) | |
| 20 | + | release := make(chan struct{}) | |
| 21 | + | var enteredOnce atomic.Bool | |
| 22 | + | initGateForTest = func() { | |
| 23 | + | if enteredOnce.CompareAndSwap(false, true) { | |
| 24 | + | close(entered) | |
| 25 | + | } | |
| 26 | + | <-release | |
| 27 | + | } | |
| 28 | + | defer func() { initGateForTest = nil }() | |
| 29 | + | ||
| 30 | + | lazy := NewLazyDB(filepath.Join(t.TempDir(), "test.db")) | |
| 31 | + | ||
| 32 | + | dbErr := make(chan error, 1) | |
| 33 | + | go func() { | |
| 34 | + | _, err := lazy.DB(context.Background()) | |
| 35 | + | dbErr <- err | |
| 36 | + | }() | |
| 37 | + | ||
| 38 | + | select { | |
| 39 | + | case <-entered: | |
| 40 | + | case <-time.After(10 * time.Second): | |
| 41 | + | t.Fatal("admitted initializer did not start") | |
| 42 | + | } | |
| 43 | + | ||
| 44 | + | closeReturned := make(chan error, 1) | |
| 45 | + | go func() { | |
| 46 | + | closeReturned <- lazy.Close() | |
| 47 | + | }() | |
| 48 | + | ||
| 49 | + | select { | |
| 50 | + | case err := <-closeReturned: | |
| 51 | + | t.Fatalf("Close returned while admitted initialization was in flight: %v", err) | |
| 52 | + | case <-time.After(100 * time.Millisecond): | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | close(release) | |
| 56 | + | ||
| 57 | + | select { | |
| 58 | + | case err := <-dbErr: | |
| 59 | + | require.Error(t, err, "admitted init racing Close must not yield a usable connection") | |
| 60 | + | case <-time.After(10 * time.Second): | |
| 61 | + | t.Fatal("initializer did not finish after release") | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | select { | |
| 65 | + | case err := <-closeReturned: | |
| 66 | + | require.NoError(t, err, "Close must settle admitted init and release its connection") | |
| 67 | + | case <-time.After(10 * time.Second): | |
| 68 | + | t.Fatal("Close did not return after initialization settled") | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | _, err := lazy.DB(context.Background()) | |
| 72 | + | require.Error(t, err, "no usable connection may escape Close") | |
| 73 | + | ||
| 74 | + | require.NoError(t, lazy.Close(), "repeat Close must be idempotent") | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | // TestLazyDB_CloseRejectsLateAdmission proves the admission/closure | |
| 78 | + | // serialization: a DB call arriving after Close began never opens anything | |
| 79 | + | // and receives the closed-provider error. | |
| 80 | + | func TestLazyDB_CloseRejectsLateAdmission(t *testing.T) { | |
| 81 | + | entered := make(chan struct{}, 1) | |
| 82 | + | initGateForTest = func() { close(entered) } | |
| 83 | + | defer func() { initGateForTest = nil }() | |
| 84 | + | ||
| 85 | + | lazy := NewLazyDB(filepath.Join(t.TempDir(), "test.db")) | |
| 86 | + | ||
| 87 | + | require.NoError(t, lazy.Close()) | |
| 88 | + | ||
| 89 | + | _, err := lazy.DB(context.Background()) | |
| 90 | + | require.ErrorIs(t, err, ErrLazyDBClosed) | |
| 91 | + | ||
| 92 | + | select { | |
| 93 | + | case <-entered: | |
| 94 | + | t.Fatal("initialization must not be admitted after Close") | |
| 95 | + | default: | |
| 96 | + | } | |
| 97 | + | assert.False(t, lazy.IsInitialized()) | |
| 98 | + | } | |
| @@ -0,0 +1,98 @@ | |||||
| 1 | + | package sqlite | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "path/filepath" | |||
| 6 | + | "sync/atomic" | |||
| 7 | + | "testing" | |||
| 8 | + | "time" | |||
| 9 | + | ||||
| 10 | + | "github.com/stretchr/testify/assert" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // TestLazyDB_CloseSettlesAdmittedInit holds an admitted initialization open | |||
| 15 | + | // with initGateForTest and proves Close seals admission, settles the | |||
| 16 | + | // in-flight work, and leaves no usable connection behind: after Close, | |||
| 17 | + | // every DB call fails instead of escaping with an orphaned pool. | |||
| 18 | + | func TestLazyDB_CloseSettlesAdmittedInit(t *testing.T) { | |||
| 19 | + | entered := make(chan struct{}, 1) | |||
| 20 | + | release := make(chan struct{}) | |||
| 21 | + | var enteredOnce atomic.Bool | |||
| 22 | + | initGateForTest = func() { | |||
| 23 | + | if enteredOnce.CompareAndSwap(false, true) { | |||
| 24 | + | close(entered) | |||
| 25 | + | } | |||
| 26 | + | <-release | |||
| 27 | + | } | |||
| 28 | + | defer func() { initGateForTest = nil }() | |||
| 29 | + | ||||
| 30 | + | lazy := NewLazyDB(filepath.Join(t.TempDir(), "test.db")) | |||
| 31 | + | ||||
| 32 | + | dbErr := make(chan error, 1) | |||
| 33 | + | go func() { | |||
| 34 | + | _, err := lazy.DB(context.Background()) | |||
| 35 | + | dbErr <- err | |||
| 36 | + | }() | |||
| 37 | + | ||||
| 38 | + | select { | |||
| 39 | + | case <-entered: | |||
| 40 | + | case <-time.After(10 * time.Second): | |||
| 41 | + | t.Fatal("admitted initializer did not start") | |||
| 42 | + | } | |||
| 43 | + | ||||
| 44 | + | closeReturned := make(chan error, 1) | |||
| 45 | + | go func() { | |||
| 46 | + | closeReturned <- lazy.Close() | |||
| 47 | + | }() | |||
| 48 | + | ||||
| 49 | + | select { | |||
| 50 | + | case err := <-closeReturned: | |||
| 51 | + | t.Fatalf("Close returned while admitted initialization was in flight: %v", err) | |||
| 52 | + | case <-time.After(100 * time.Millisecond): | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | close(release) | |||
| 56 | + | ||||
| 57 | + | select { | |||
| 58 | + | case err := <-dbErr: | |||
| 59 | + | require.Error(t, err, "admitted init racing Close must not yield a usable connection") | |||
| 60 | + | case <-time.After(10 * time.Second): | |||
| 61 | + | t.Fatal("initializer did not finish after release") | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | select { | |||
| 65 | + | case err := <-closeReturned: | |||
| 66 | + | require.NoError(t, err, "Close must settle admitted init and release its connection") | |||
| 67 | + | case <-time.After(10 * time.Second): | |||
| 68 | + | t.Fatal("Close did not return after initialization settled") | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | _, err := lazy.DB(context.Background()) | |||
| 72 | + | require.Error(t, err, "no usable connection may escape Close") | |||
| 73 | + | ||||
| 74 | + | require.NoError(t, lazy.Close(), "repeat Close must be idempotent") | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | // TestLazyDB_CloseRejectsLateAdmission proves the admission/closure | |||
| 78 | + | // serialization: a DB call arriving after Close began never opens anything | |||
| 79 | + | // and receives the closed-provider error. | |||
| 80 | + | func TestLazyDB_CloseRejectsLateAdmission(t *testing.T) { | |||
| 81 | + | entered := make(chan struct{}, 1) | |||
| 82 | + | initGateForTest = func() { close(entered) } | |||
| 83 | + | defer func() { initGateForTest = nil }() | |||
| 84 | + | ||||
| 85 | + | lazy := NewLazyDB(filepath.Join(t.TempDir(), "test.db")) | |||
| 86 | + | ||||
| 87 | + | require.NoError(t, lazy.Close()) | |||
| 88 | + | ||||
| 89 | + | _, err := lazy.DB(context.Background()) | |||
| 90 | + | require.ErrorIs(t, err, ErrLazyDBClosed) | |||
| 91 | + | ||||
| 92 | + | select { | |||
| 93 | + | case <-entered: | |||
| 94 | + | t.Fatal("initialization must not be admitted after Close") | |||
| 95 | + | default: | |||
| 96 | + | } | |||
| 97 | + | assert.False(t, lazy.IsInitialized()) | |||
| 98 | + | } | |||
M internal/infrastructure/persistence/sqlite/lazy_db_test.go +84 -3
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "path/filepath" | |
| 5 | 5 | "sync" | |
| 6 | 6 | "testing" | |
| 7 | + | "time" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | |
| 9 | 10 | "github.com/stretchr/testify/assert" | |
| @@ -95,9 +96,12 @@ func TestLazyDB_CloseBeforeInit(t *testing.T) { | |||
| 95 | 96 | dbPath := filepath.Join(t.TempDir(), "test.db") | |
| 96 | 97 | lazy := sqlite.NewLazyDB(dbPath) | |
| 97 | 98 | ||
| 98 | - | // Close without ever calling DB() should not error | |
| 99 | - | err := lazy.Close() | |
| 100 | - | assert.NoError(t, err) | |
| 99 | + | // Close without ever calling DB() seals the provider without triggering | |
| 100 | + | // initialization; late callers are rejected with the closed error. | |
| 101 | + | require.NoError(t, lazy.Close()) | |
| 102 | + | assert.False(t, lazy.IsInitialized(), "Close must not trigger initialization") | |
| 103 | + | _, err := lazy.DB(testCtx()) | |
| 104 | + | require.ErrorIs(t, err, sqlite.ErrLazyDBClosed) | |
| 101 | 105 | } | |
| 102 | 106 | ||
| 103 | 107 | func TestLazyDB_Path(t *testing.T) { | |
| @@ -123,3 +127,80 @@ func TestLazyDB_DBIsUsable(t *testing.T) { | |||
| 123 | 127 | ||
| 124 | 128 | require.NoError(t, lazy.Close()) | |
| 125 | 129 | } | |
| 130 | + | ||
| 131 | + | func TestLazyDB_WarmupInitializesInBackground(t *testing.T) { | |
| 132 | + | ctx := testCtx() | |
| 133 | + | dbPath := filepath.Join(t.TempDir(), "test.db") | |
| 134 | + | lazy := sqlite.NewLazyDB(dbPath) | |
| 135 | + | ||
| 136 | + | // Warmup returns immediately and initializes the provider concurrently. | |
| 137 | + | lazy.Warmup(ctx) | |
| 138 | + | ||
| 139 | + | require.Eventually(t, func() bool { | |
| 140 | + | return lazy.IsInitialized() | |
| 141 | + | }, 10*time.Second, 5*time.Millisecond, "warmup should initialize the provider without a direct DB call") | |
| 142 | + | ||
| 143 | + | db, err := lazy.DB(ctx) | |
| 144 | + | require.NoError(t, err) | |
| 145 | + | require.NotNil(t, db) | |
| 146 | + | require.NoError(t, lazy.Close()) | |
| 147 | + | } | |
| 148 | + | ||
| 149 | + | func TestLazyDB_CloseWaitsForInflightInit(t *testing.T) { | |
| 150 | + | ctx := testCtx() | |
| 151 | + | // Sealing and settlement: whether the warmup task is admitted or loses | |
| 152 | + | // the admission race, an immediate Close leaves no usable connection | |
| 153 | + | // behind and every later call fails closed. | |
| 154 | + | dbPath := filepath.Join(t.TempDir(), "test.db") | |
| 155 | + | lazy := sqlite.NewLazyDB(dbPath) | |
| 156 | + | ||
| 157 | + | lazy.Warmup(ctx) | |
| 158 | + | require.NoError(t, lazy.Close()) | |
| 159 | + | ||
| 160 | + | _, err := lazy.DB(ctx) | |
| 161 | + | require.ErrorIs(t, err, sqlite.ErrLazyDBClosed) | |
| 162 | + | require.NoError(t, lazy.Close(), "repeat Close must be idempotent") | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | func TestLazyDB_ConcurrentWarmupAccessAndClose(t *testing.T) { | |
| 166 | + | ctx := testCtx() | |
| 167 | + | dbPath := filepath.Join(t.TempDir(), "test.db") | |
| 168 | + | lazy := sqlite.NewLazyDB(dbPath) | |
| 169 | + | ||
| 170 | + | const workers = 8 | |
| 171 | + | var wg sync.WaitGroup | |
| 172 | + | wg.Add(workers + 1) | |
| 173 | + | for range workers { | |
| 174 | + | go func() { | |
| 175 | + | defer wg.Done() | |
| 176 | + | _, _ = lazy.DB(ctx) | |
| 177 | + | }() | |
| 178 | + | } | |
| 179 | + | go func() { | |
| 180 | + | defer wg.Done() | |
| 181 | + | lazy.Warmup(ctx) | |
| 182 | + | }() | |
| 183 | + | wg.Wait() | |
| 184 | + | ||
| 185 | + | db, err := lazy.DB(ctx) | |
| 186 | + | require.NoError(t, err) | |
| 187 | + | require.NotNil(t, db) | |
| 188 | + | require.NoError(t, lazy.Close()) | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | func TestLazyDB_InitErrorPropagated(t *testing.T) { | |
| 192 | + | ctx := testCtx() | |
| 193 | + | // An empty path always fails connection setup; the error must be cached | |
| 194 | + | // and reported to every later caller without panicking. | |
| 195 | + | lazy := sqlite.NewLazyDB("") | |
| 196 | + | ||
| 197 | + | _, err := lazy.DB(ctx) | |
| 198 | + | require.Error(t, err, "initialization failure must propagate to the first caller") | |
| 199 | + | ||
| 200 | + | _, err = lazy.DB(ctx) | |
| 201 | + | require.Error(t, err, "initialization failure must propagate to later callers") | |
| 202 | + | ||
| 203 | + | // Closing a failed provider must not error: there is no connection. | |
| 204 | + | require.NoError(t, lazy.Close()) | |
| 205 | + | assert.False(t, lazy.IsInitialized()) | |
| 206 | + | } | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "path/filepath" | 4 | "path/filepath" | ||
| 5 | "sync" | 5 | "sync" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | + | "time" | |||
| 7 | 8 | ||||
| 8 | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | 9 | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | ||
| 9 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| @@ -95,9 +96,12 @@ func TestLazyDB_CloseBeforeInit(t *testing.T) { | |||||
| 95 | dbPath := filepath.Join(t.TempDir(), "test.db") | 96 | dbPath := filepath.Join(t.TempDir(), "test.db") | ||
| 96 | lazy := sqlite.NewLazyDB(dbPath) | 97 | lazy := sqlite.NewLazyDB(dbPath) | ||
| 97 | 98 | ||||
| 98 | - | // Close without ever calling DB() should not error | 99 | + | // Close without ever calling DB() seals the provider without triggering |
| 99 | - | err := lazy.Close() | 100 | + | // initialization; late callers are rejected with the closed error. |
| 100 | - | assert.NoError(t, err) | 101 | + | require.NoError(t, lazy.Close()) |
| 102 | + | assert.False(t, lazy.IsInitialized(), "Close must not trigger initialization") | |||
| 103 | + | _, err := lazy.DB(testCtx()) | |||
| 104 | + | require.ErrorIs(t, err, sqlite.ErrLazyDBClosed) | |||
| 101 | } | 105 | } | ||
| 102 | 106 | ||||
| 103 | func TestLazyDB_Path(t *testing.T) { | 107 | func TestLazyDB_Path(t *testing.T) { | ||
| @@ -123,3 +127,80 @@ func TestLazyDB_DBIsUsable(t *testing.T) { | |||||
| 123 | 127 | ||||
| 124 | require.NoError(t, lazy.Close()) | 128 | require.NoError(t, lazy.Close()) | ||
| 125 | } | 129 | } | ||
| 130 | + | ||||
| 131 | + | func TestLazyDB_WarmupInitializesInBackground(t *testing.T) { | |||
| 132 | + | ctx := testCtx() | |||
| 133 | + | dbPath := filepath.Join(t.TempDir(), "test.db") | |||
| 134 | + | lazy := sqlite.NewLazyDB(dbPath) | |||
| 135 | + | ||||
| 136 | + | // Warmup returns immediately and initializes the provider concurrently. | |||
| 137 | + | lazy.Warmup(ctx) | |||
| 138 | + | ||||
| 139 | + | require.Eventually(t, func() bool { | |||
| 140 | + | return lazy.IsInitialized() | |||
| 141 | + | }, 10*time.Second, 5*time.Millisecond, "warmup should initialize the provider without a direct DB call") | |||
| 142 | + | ||||
| 143 | + | db, err := lazy.DB(ctx) | |||
| 144 | + | require.NoError(t, err) | |||
| 145 | + | require.NotNil(t, db) | |||
| 146 | + | require.NoError(t, lazy.Close()) | |||
| 147 | + | } | |||
| 148 | + | ||||
| 149 | + | func TestLazyDB_CloseWaitsForInflightInit(t *testing.T) { | |||
| 150 | + | ctx := testCtx() | |||
| 151 | + | // Sealing and settlement: whether the warmup task is admitted or loses | |||
| 152 | + | // the admission race, an immediate Close leaves no usable connection | |||
| 153 | + | // behind and every later call fails closed. | |||
| 154 | + | dbPath := filepath.Join(t.TempDir(), "test.db") | |||
| 155 | + | lazy := sqlite.NewLazyDB(dbPath) | |||
| 156 | + | ||||
| 157 | + | lazy.Warmup(ctx) | |||
| 158 | + | require.NoError(t, lazy.Close()) | |||
| 159 | + | ||||
| 160 | + | _, err := lazy.DB(ctx) | |||
| 161 | + | require.ErrorIs(t, err, sqlite.ErrLazyDBClosed) | |||
| 162 | + | require.NoError(t, lazy.Close(), "repeat Close must be idempotent") | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | func TestLazyDB_ConcurrentWarmupAccessAndClose(t *testing.T) { | |||
| 166 | + | ctx := testCtx() | |||
| 167 | + | dbPath := filepath.Join(t.TempDir(), "test.db") | |||
| 168 | + | lazy := sqlite.NewLazyDB(dbPath) | |||
| 169 | + | ||||
| 170 | + | const workers = 8 | |||
| 171 | + | var wg sync.WaitGroup | |||
| 172 | + | wg.Add(workers + 1) | |||
| 173 | + | for range workers { | |||
| 174 | + | go func() { | |||
| 175 | + | defer wg.Done() | |||
| 176 | + | _, _ = lazy.DB(ctx) | |||
| 177 | + | }() | |||
| 178 | + | } | |||
| 179 | + | go func() { | |||
| 180 | + | defer wg.Done() | |||
| 181 | + | lazy.Warmup(ctx) | |||
| 182 | + | }() | |||
| 183 | + | wg.Wait() | |||
| 184 | + | ||||
| 185 | + | db, err := lazy.DB(ctx) | |||
| 186 | + | require.NoError(t, err) | |||
| 187 | + | require.NotNil(t, db) | |||
| 188 | + | require.NoError(t, lazy.Close()) | |||
| 189 | + | } | |||
| 190 | + | ||||
| 191 | + | func TestLazyDB_InitErrorPropagated(t *testing.T) { | |||
| 192 | + | ctx := testCtx() | |||
| 193 | + | // An empty path always fails connection setup; the error must be cached | |||
| 194 | + | // and reported to every later caller without panicking. | |||
| 195 | + | lazy := sqlite.NewLazyDB("") | |||
| 196 | + | ||||
| 197 | + | _, err := lazy.DB(ctx) | |||
| 198 | + | require.Error(t, err, "initialization failure must propagate to the first caller") | |||
| 199 | + | ||||
| 200 | + | _, err = lazy.DB(ctx) | |||
| 201 | + | require.Error(t, err, "initialization failure must propagate to later callers") | |||
| 202 | + | ||||
| 203 | + | // Closing a failed provider must not error: there is no connection. | |||
| 204 | + | require.NoError(t, lazy.Close()) | |||
| 205 | + | assert.False(t, lazy.IsInitialized()) | |||
| 206 | + | } | |||
M internal/ui/browser_window.go +3 -2
| @@ -126,14 +126,15 @@ func (bw *browserWindow) initChrome(ctx context.Context, a *App) { | |||
| 126 | 126 | return | |
| 127 | 127 | } | |
| 128 | 128 | ||
| 129 | + | // Hidden panels (history, favorites) are constructed on first request via | |
| 130 | + | // ensureHistorySidebar/ensureFavoritesSidebar so window creation performs | |
| 131 | + | // no hidden-panel queries or widget row rebuilds. | |
| 129 | 132 | bw.initToasterOverlay(a) | |
| 130 | 133 | bw.initTouchpadNavigationIndicator() | |
| 131 | 134 | bw.initBorderOverlay(a) | |
| 132 | 135 | bw.initAccentPicker(ctx, a) | |
| 133 | 136 | bw.initSessionManager(ctx, a) | |
| 134 | 137 | bw.initTabPicker(ctx, a) | |
| 135 | - | bw.initHistorySidebar(ctx, a) | |
| 136 | - | bw.initFavoritesSidebar(ctx, a) | |
| 137 | 138 | } | |
| 138 | 139 | ||
| 139 | 140 | func (bw *browserWindow) initToasterOverlay(a *App) { | |
| @@ -126,14 +126,15 @@ func (bw *browserWindow) initChrome(ctx context.Context, a *App) { | |||||
| 126 | return | 126 | return | ||
| 127 | } | 127 | } | ||
| 128 | 128 | ||||
| 129 | + | // Hidden panels (history, favorites) are constructed on first request via | |||
| 130 | + | // ensureHistorySidebar/ensureFavoritesSidebar so window creation performs | |||
| 131 | + | // no hidden-panel queries or widget row rebuilds. | |||
| 129 | bw.initToasterOverlay(a) | 132 | bw.initToasterOverlay(a) | ||
| 130 | bw.initTouchpadNavigationIndicator() | 133 | bw.initTouchpadNavigationIndicator() | ||
| 131 | bw.initBorderOverlay(a) | 134 | bw.initBorderOverlay(a) | ||
| 132 | bw.initAccentPicker(ctx, a) | 135 | bw.initAccentPicker(ctx, a) | ||
| 133 | bw.initSessionManager(ctx, a) | 136 | bw.initSessionManager(ctx, a) | ||
| 134 | bw.initTabPicker(ctx, a) | 137 | bw.initTabPicker(ctx, a) | ||
| 135 | - | bw.initHistorySidebar(ctx, a) | |||
| 136 | - | bw.initFavoritesSidebar(ctx, a) | |||
| 137 | } | 138 | } | ||
| 138 | 139 | ||||
| 139 | func (bw *browserWindow) initToasterOverlay(a *App) { | 140 | func (bw *browserWindow) initToasterOverlay(a *App) { | ||
M internal/ui/browser_window_favorites_sidebar.go +28 -10
| @@ -11,24 +11,42 @@ import ( | |||
| 11 | 11 | "github.com/bnema/dumber/internal/ui/component" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | - | // initFavoritesSidebar creates the native favorites sidebar when favorites are configured. | |
| 15 | - | func (bw *browserWindow) initFavoritesSidebar(ctx context.Context, a *App) { | |
| 16 | - | if bw == nil || a == nil || bw.mainWindow == nil || a.deps == nil || a.deps.FavoritesUC == nil { | |
| 17 | - | return | |
| 14 | + | // newFavoritesSidebarComponent constructs the native favorites sidebar. It is | |
| 15 | + | // a package-level seam so headless tests can observe on-demand construction | |
| 16 | + | // without instantiating GTK widgets. Mutable global: tests must restore it | |
| 17 | + | // via defer and must not use t.Parallel alongside seam users. | |
| 18 | + | var newFavoritesSidebarComponent = component.NewFavoritesSidebar | |
| 19 | + | ||
| 20 | + | // ensureFavoritesSidebar constructs the favorites sidebar on first request. | |
| 21 | + | // Construction is separated from mounting and shared visibility mutation: | |
| 22 | + | // the caller mounts and reveals through mountNativeSidebar/showFavoritesSidebar. | |
| 23 | + | // The component lifetime is detached from the caller's request context via | |
| 24 | + | // context.WithoutCancel so a short-lived window-creation or shortcut context | |
| 25 | + | // cannot cancel the sidebar's in-flight loads. It returns false when the | |
| 26 | + | // window, dependencies, or native construction are unavailable. | |
| 27 | + | func (bw *browserWindow) ensureFavoritesSidebar(ctx context.Context, a *App) bool { | |
| 28 | + | if bw == nil || a == nil || bw.mainWindow == nil { | |
| 29 | + | return false | |
| 30 | + | } | |
| 31 | + | if bw.favoritesSidebar != nil { | |
| 32 | + | return true | |
| 33 | + | } | |
| 34 | + | if a.deps == nil || a.deps.FavoritesUC == nil { | |
| 35 | + | return false | |
| 18 | 36 | } | |
| 19 | 37 | log := logging.FromContext(ctx) | |
| 20 | 38 | ||
| 21 | 39 | cfg := a.buildFavoritesSidebarConfig(bw) | |
| 22 | - | sidebar := component.NewFavoritesSidebar(ctx, cfg) | |
| 40 | + | sidebar := newFavoritesSidebarComponent(context.WithoutCancel(ctx), cfg) | |
| 23 | 41 | if sidebar == nil { | |
| 24 | 42 | log.Warn().Msg("failed to create favorites sidebar") | |
| 25 | - | return | |
| 43 | + | return false | |
| 26 | 44 | } | |
| 27 | 45 | ||
| 28 | 46 | bw.favoritesSidebar = sidebar | |
| 29 | - | bw.favoritesSidebar.Hide() | |
| 30 | 47 | bw.applySidebarWidthConfig(a) | |
| 31 | 48 | log.Debug().Msg("favorites sidebar initialized") | |
| 49 | + | return true | |
| 32 | 50 | } | |
| 33 | 51 | ||
| 34 | 52 | //nolint:dupl // Favorites and history sidebar configs intentionally mirror each other for separate component types. | |
| @@ -104,13 +122,13 @@ func (a *App) toggleFavoritesSidebarAction(ctx context.Context) error { | |||
| 104 | 122 | if bw == nil { | |
| 105 | 123 | return fmt.Errorf("favorites sidebar unavailable: no focused browser window") | |
| 106 | 124 | } | |
| 107 | - | if bw.favoritesSidebar == nil { | |
| 108 | - | return fmt.Errorf("favorites sidebar unavailable: native sidebar not initialized") | |
| 109 | - | } | |
| 110 | 125 | if bw.sidebarVisible && bw.activeSidebarKind == nativeSidebarFavorites { | |
| 111 | 126 | a.hideAndRestoreFocusForBrowserWindow(bw) | |
| 112 | 127 | return nil | |
| 113 | 128 | } | |
| 129 | + | if !bw.ensureFavoritesSidebar(ctx, a) { | |
| 130 | + | return fmt.Errorf("favorites sidebar unavailable: native sidebar not initialized") | |
| 131 | + | } | |
| 114 | 132 | bw.toggleFavoritesSidebar() | |
| 115 | 133 | return nil | |
| 116 | 134 | } | |
| @@ -11,24 +11,42 @@ import ( | |||||
| 11 | "github.com/bnema/dumber/internal/ui/component" | 11 | "github.com/bnema/dumber/internal/ui/component" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | - | // initFavoritesSidebar creates the native favorites sidebar when favorites are configured. | 14 | + | // newFavoritesSidebarComponent constructs the native favorites sidebar. It is |
| 15 | - | func (bw *browserWindow) initFavoritesSidebar(ctx context.Context, a *App) { | 15 | + | // a package-level seam so headless tests can observe on-demand construction |
| 16 | - | if bw == nil || a == nil || bw.mainWindow == nil || a.deps == nil || a.deps.FavoritesUC == nil { | 16 | + | // without instantiating GTK widgets. Mutable global: tests must restore it |
| 17 | - | return | 17 | + | // via defer and must not use t.Parallel alongside seam users. |
| 18 | + | var newFavoritesSidebarComponent = component.NewFavoritesSidebar | |||
| 19 | + | ||||
| 20 | + | // ensureFavoritesSidebar constructs the favorites sidebar on first request. | |||
| 21 | + | // Construction is separated from mounting and shared visibility mutation: | |||
| 22 | + | // the caller mounts and reveals through mountNativeSidebar/showFavoritesSidebar. | |||
| 23 | + | // The component lifetime is detached from the caller's request context via | |||
| 24 | + | // context.WithoutCancel so a short-lived window-creation or shortcut context | |||
| 25 | + | // cannot cancel the sidebar's in-flight loads. It returns false when the | |||
| 26 | + | // window, dependencies, or native construction are unavailable. | |||
| 27 | + | func (bw *browserWindow) ensureFavoritesSidebar(ctx context.Context, a *App) bool { | |||
| 28 | + | if bw == nil || a == nil || bw.mainWindow == nil { | |||
| 29 | + | return false | |||
| 30 | + | } | |||
| 31 | + | if bw.favoritesSidebar != nil { | |||
| 32 | + | return true | |||
| 33 | + | } | |||
| 34 | + | if a.deps == nil || a.deps.FavoritesUC == nil { | |||
| 35 | + | return false | |||
| 18 | } | 36 | } | ||
| 19 | log := logging.FromContext(ctx) | 37 | log := logging.FromContext(ctx) | ||
| 20 | 38 | ||||
| 21 | cfg := a.buildFavoritesSidebarConfig(bw) | 39 | cfg := a.buildFavoritesSidebarConfig(bw) | ||
| 22 | - | sidebar := component.NewFavoritesSidebar(ctx, cfg) | 40 | + | sidebar := newFavoritesSidebarComponent(context.WithoutCancel(ctx), cfg) |
| 23 | if sidebar == nil { | 41 | if sidebar == nil { | ||
| 24 | log.Warn().Msg("failed to create favorites sidebar") | 42 | log.Warn().Msg("failed to create favorites sidebar") | ||
| 25 | - | return | 43 | + | return false |
| 26 | } | 44 | } | ||
| 27 | 45 | ||||
| 28 | bw.favoritesSidebar = sidebar | 46 | bw.favoritesSidebar = sidebar | ||
| 29 | - | bw.favoritesSidebar.Hide() | |||
| 30 | bw.applySidebarWidthConfig(a) | 47 | bw.applySidebarWidthConfig(a) | ||
| 31 | log.Debug().Msg("favorites sidebar initialized") | 48 | log.Debug().Msg("favorites sidebar initialized") | ||
| 49 | + | return true | |||
| 32 | } | 50 | } | ||
| 33 | 51 | ||||
| 34 | //nolint:dupl // Favorites and history sidebar configs intentionally mirror each other for separate component types. | 52 | //nolint:dupl // Favorites and history sidebar configs intentionally mirror each other for separate component types. | ||
| @@ -104,13 +122,13 @@ func (a *App) toggleFavoritesSidebarAction(ctx context.Context) error { | |||||
| 104 | if bw == nil { | 122 | if bw == nil { | ||
| 105 | return fmt.Errorf("favorites sidebar unavailable: no focused browser window") | 123 | return fmt.Errorf("favorites sidebar unavailable: no focused browser window") | ||
| 106 | } | 124 | } | ||
| 107 | - | if bw.favoritesSidebar == nil { | |||
| 108 | - | return fmt.Errorf("favorites sidebar unavailable: native sidebar not initialized") | |||
| 109 | - | } | |||
| 110 | if bw.sidebarVisible && bw.activeSidebarKind == nativeSidebarFavorites { | 125 | if bw.sidebarVisible && bw.activeSidebarKind == nativeSidebarFavorites { | ||
| 111 | a.hideAndRestoreFocusForBrowserWindow(bw) | 126 | a.hideAndRestoreFocusForBrowserWindow(bw) | ||
| 112 | return nil | 127 | return nil | ||
| 113 | } | 128 | } | ||
| 129 | + | if !bw.ensureFavoritesSidebar(ctx, a) { | |||
| 130 | + | return fmt.Errorf("favorites sidebar unavailable: native sidebar not initialized") | |||
| 131 | + | } | |||
| 114 | bw.toggleFavoritesSidebar() | 132 | bw.toggleFavoritesSidebar() | ||
| 115 | return nil | 133 | return nil | ||
| 116 | } | 134 | } | ||
M internal/ui/browser_window_history_sidebar.go +33 -18
| @@ -11,36 +11,49 @@ import ( | |||
| 11 | 11 | "github.com/bnema/dumber/internal/ui/window" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | - | // initHistorySidebar creates and mounts the history sidebar into the | |
| 15 | - | // browser window's sidebar container. The sidebar is hidden by default. | |
| 16 | - | func (bw *browserWindow) initHistorySidebar(ctx context.Context, a *App) { | |
| 17 | - | if bw == nil || a == nil || bw.mainWindow == nil || a.deps == nil || a.deps.HistoryUC == nil { | |
| 18 | - | return | |
| 14 | + | // newHistorySidebarComponent constructs the native history sidebar. It is a | |
| 15 | + | // package-level seam so headless tests can observe on-demand construction | |
| 16 | + | // without instantiating GTK widgets. Mutable global: tests must restore it | |
| 17 | + | // via defer and must not use t.Parallel alongside seam users. | |
| 18 | + | var newHistorySidebarComponent = component.NewHistorySidebar | |
| 19 | + | ||
| 20 | + | // ensureHistorySidebar constructs the history sidebar on first request. | |
| 21 | + | // Construction is separated from mounting and shared visibility mutation: | |
| 22 | + | // the caller mounts and reveals through mountNativeSidebar/showHistorySidebar. | |
| 23 | + | // The component lifetime is detached from the caller's request context via | |
| 24 | + | // context.WithoutCancel so a short-lived window-creation or shortcut context | |
| 25 | + | // cannot cancel the sidebar's in-flight loads. It returns false when the | |
| 26 | + | // window, dependencies, or native construction are unavailable. | |
| 27 | + | func (bw *browserWindow) ensureHistorySidebar(ctx context.Context, a *App) bool { | |
| 28 | + | if bw == nil || a == nil || bw.mainWindow == nil { | |
| 29 | + | return false | |
| 30 | + | } | |
| 31 | + | if bw.historySidebar != nil { | |
| 32 | + | return true | |
| 33 | + | } | |
| 34 | + | if a.deps == nil || a.deps.HistoryUC == nil { | |
| 35 | + | return false | |
| 19 | 36 | } | |
| 20 | 37 | log := logging.FromContext(ctx) | |
| 21 | 38 | ||
| 22 | 39 | cfg := a.buildHistorySidebarConfig(bw) | |
| 23 | 40 | ||
| 24 | - | sidebar := component.NewHistorySidebar(ctx, cfg) | |
| 41 | + | sidebar := newHistorySidebarComponent(context.WithoutCancel(ctx), cfg) | |
| 25 | 42 | if sidebar == nil { | |
| 26 | 43 | log.Warn().Msg("failed to create history sidebar") | |
| 27 | - | return | |
| 44 | + | return false | |
| 28 | 45 | } | |
| 29 | 46 | ||
| 30 | 47 | bw.historySidebar = sidebar | |
| 31 | 48 | ||
| 32 | - | // Mount into the main window's sidebar box. | |
| 33 | - | bw.mainWindow.SetSidebarWidget(sidebar.Widget()) | |
| 34 | - | bw.historySidebar.Hide() | |
| 35 | - | bw.mainWindow.SetSidebarVisible(false) | |
| 36 | - | bw.sidebarVisible = false | |
| 37 | - | bw.activeSidebarKind = nativeSidebarNone | |
| 38 | - | ||
| 39 | 49 | // Apply sidebar width from config, falling back to the default 320px. | |
| 40 | 50 | // The width is clamped to [280, 380] by SetSidebarWidth internally. | |
| 51 | + | // Visibility is left untouched: the sidebar container keeps whatever | |
| 52 | + | // panel (or none) is currently mounted until the caller reveals history. | |
| 41 | 53 | bw.applySidebarWidthConfig(a) | |
| 42 | 54 | ||
| 43 | 55 | log.Debug().Msg("history sidebar initialized") | |
| 56 | + | return true | |
| 44 | 57 | } | |
| 45 | 58 | ||
| 46 | 59 | // buildHistorySidebarConfig constructs the HistorySidebarConfig for the given | |
| @@ -164,19 +177,21 @@ func (bw *browserWindow) applySidebarWidthConfig(a *App) { | |||
| 164 | 177 | } | |
| 165 | 178 | ||
| 166 | 179 | // toggleHistorySidebarAction is the keyboard-action handler for toggling the | |
| 167 | - | // history sidebar on the last focused browser window. | |
| 180 | + | // history sidebar on the last focused browser window. It ensures on-demand | |
| 181 | + | // construction before testing availability so shortcuts work without eager | |
| 182 | + | // window-creation work. | |
| 168 | 183 | func (a *App) toggleHistorySidebarAction(ctx context.Context) error { | |
| 169 | 184 | bw := a.lastFocusedBrowserWindow() | |
| 170 | 185 | if bw == nil { | |
| 171 | 186 | return fmt.Errorf("history sidebar unavailable: no focused browser window") | |
| 172 | 187 | } | |
| 173 | - | if bw.historySidebar == nil { | |
| 174 | - | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") | |
| 175 | - | } | |
| 176 | 188 | if bw.sidebarVisible && bw.activeSidebarKind == nativeSidebarHistory { | |
| 177 | 189 | a.hideAndRestoreFocusForBrowserWindow(bw) | |
| 178 | 190 | return nil | |
| 179 | 191 | } | |
| 192 | + | if !bw.ensureHistorySidebar(ctx, a) { | |
| 193 | + | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") | |
| 194 | + | } | |
| 180 | 195 | bw.toggleHistorySidebar() | |
| 181 | 196 | return nil | |
| 182 | 197 | } | |
| @@ -11,36 +11,49 @@ import ( | |||||
| 11 | "github.com/bnema/dumber/internal/ui/window" | 11 | "github.com/bnema/dumber/internal/ui/window" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | - | // initHistorySidebar creates and mounts the history sidebar into the | 14 | + | // newHistorySidebarComponent constructs the native history sidebar. It is a |
| 15 | - | // browser window's sidebar container. The sidebar is hidden by default. | 15 | + | // package-level seam so headless tests can observe on-demand construction |
| 16 | - | func (bw *browserWindow) initHistorySidebar(ctx context.Context, a *App) { | 16 | + | // without instantiating GTK widgets. Mutable global: tests must restore it |
| 17 | - | if bw == nil || a == nil || bw.mainWindow == nil || a.deps == nil || a.deps.HistoryUC == nil { | 17 | + | // via defer and must not use t.Parallel alongside seam users. |
| 18 | - | return | 18 | + | var newHistorySidebarComponent = component.NewHistorySidebar |
| 19 | + | ||||
| 20 | + | // ensureHistorySidebar constructs the history sidebar on first request. | |||
| 21 | + | // Construction is separated from mounting and shared visibility mutation: | |||
| 22 | + | // the caller mounts and reveals through mountNativeSidebar/showHistorySidebar. | |||
| 23 | + | // The component lifetime is detached from the caller's request context via | |||
| 24 | + | // context.WithoutCancel so a short-lived window-creation or shortcut context | |||
| 25 | + | // cannot cancel the sidebar's in-flight loads. It returns false when the | |||
| 26 | + | // window, dependencies, or native construction are unavailable. | |||
| 27 | + | func (bw *browserWindow) ensureHistorySidebar(ctx context.Context, a *App) bool { | |||
| 28 | + | if bw == nil || a == nil || bw.mainWindow == nil { | |||
| 29 | + | return false | |||
| 30 | + | } | |||
| 31 | + | if bw.historySidebar != nil { | |||
| 32 | + | return true | |||
| 33 | + | } | |||
| 34 | + | if a.deps == nil || a.deps.HistoryUC == nil { | |||
| 35 | + | return false | |||
| 19 | } | 36 | } | ||
| 20 | log := logging.FromContext(ctx) | 37 | log := logging.FromContext(ctx) | ||
| 21 | 38 | ||||
| 22 | cfg := a.buildHistorySidebarConfig(bw) | 39 | cfg := a.buildHistorySidebarConfig(bw) | ||
| 23 | 40 | ||||
| 24 | - | sidebar := component.NewHistorySidebar(ctx, cfg) | 41 | + | sidebar := newHistorySidebarComponent(context.WithoutCancel(ctx), cfg) |
| 25 | if sidebar == nil { | 42 | if sidebar == nil { | ||
| 26 | log.Warn().Msg("failed to create history sidebar") | 43 | log.Warn().Msg("failed to create history sidebar") | ||
| 27 | - | return | 44 | + | return false |
| 28 | } | 45 | } | ||
| 29 | 46 | ||||
| 30 | bw.historySidebar = sidebar | 47 | bw.historySidebar = sidebar | ||
| 31 | 48 | ||||
| 32 | - | // Mount into the main window's sidebar box. | |||
| 33 | - | bw.mainWindow.SetSidebarWidget(sidebar.Widget()) | |||
| 34 | - | bw.historySidebar.Hide() | |||
| 35 | - | bw.mainWindow.SetSidebarVisible(false) | |||
| 36 | - | bw.sidebarVisible = false | |||
| 37 | - | bw.activeSidebarKind = nativeSidebarNone | |||
| 38 | - | ||||
| 39 | // Apply sidebar width from config, falling back to the default 320px. | 49 | // Apply sidebar width from config, falling back to the default 320px. | ||
| 40 | // The width is clamped to [280, 380] by SetSidebarWidth internally. | 50 | // The width is clamped to [280, 380] by SetSidebarWidth internally. | ||
| 51 | + | // Visibility is left untouched: the sidebar container keeps whatever | |||
| 52 | + | // panel (or none) is currently mounted until the caller reveals history. | |||
| 41 | bw.applySidebarWidthConfig(a) | 53 | bw.applySidebarWidthConfig(a) | ||
| 42 | 54 | ||||
| 43 | log.Debug().Msg("history sidebar initialized") | 55 | log.Debug().Msg("history sidebar initialized") | ||
| 56 | + | return true | |||
| 44 | } | 57 | } | ||
| 45 | 58 | ||||
| 46 | // buildHistorySidebarConfig constructs the HistorySidebarConfig for the given | 59 | // buildHistorySidebarConfig constructs the HistorySidebarConfig for the given | ||
| @@ -164,19 +177,21 @@ func (bw *browserWindow) applySidebarWidthConfig(a *App) { | |||||
| 164 | } | 177 | } | ||
| 165 | 178 | ||||
| 166 | // toggleHistorySidebarAction is the keyboard-action handler for toggling the | 179 | // toggleHistorySidebarAction is the keyboard-action handler for toggling the | ||
| 167 | - | // history sidebar on the last focused browser window. | 180 | + | // history sidebar on the last focused browser window. It ensures on-demand |
| 181 | + | // construction before testing availability so shortcuts work without eager | |||
| 182 | + | // window-creation work. | |||
| 168 | func (a *App) toggleHistorySidebarAction(ctx context.Context) error { | 183 | func (a *App) toggleHistorySidebarAction(ctx context.Context) error { | ||
| 169 | bw := a.lastFocusedBrowserWindow() | 184 | bw := a.lastFocusedBrowserWindow() | ||
| 170 | if bw == nil { | 185 | if bw == nil { | ||
| 171 | return fmt.Errorf("history sidebar unavailable: no focused browser window") | 186 | return fmt.Errorf("history sidebar unavailable: no focused browser window") | ||
| 172 | } | 187 | } | ||
| 173 | - | if bw.historySidebar == nil { | |||
| 174 | - | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") | |||
| 175 | - | } | |||
| 176 | if bw.sidebarVisible && bw.activeSidebarKind == nativeSidebarHistory { | 188 | if bw.sidebarVisible && bw.activeSidebarKind == nativeSidebarHistory { | ||
| 177 | a.hideAndRestoreFocusForBrowserWindow(bw) | 189 | a.hideAndRestoreFocusForBrowserWindow(bw) | ||
| 178 | return nil | 190 | return nil | ||
| 179 | } | 191 | } | ||
| 192 | + | if !bw.ensureHistorySidebar(ctx, a) { | |||
| 193 | + | return fmt.Errorf("history sidebar unavailable: native sidebar not initialized") | |||
| 194 | + | } | |||
| 180 | bw.toggleHistorySidebar() | 195 | bw.toggleHistorySidebar() | ||
| 181 | return nil | 196 | return nil | ||
| 182 | } | 197 | } | ||
A internal/ui/browser_window_sidebar_laziness_test.go +193 -0
| @@ -0,0 +1,193 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "sync/atomic" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 9 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 10 | + | "github.com/bnema/dumber/internal/ui/component" | |
| 11 | + | "github.com/bnema/dumber/internal/ui/window" | |
| 12 | + | "github.com/stretchr/testify/assert" | |
| 13 | + | "github.com/stretchr/testify/require" | |
| 14 | + | ) | |
| 15 | + | ||
| 16 | + | func TestWindowCreation_DoesNotConstructHiddenSidebars(t *testing.T) { | |
| 17 | + | ctx := context.Background() | |
| 18 | + | app := &App{ | |
| 19 | + | deps: &Dependencies{ | |
| 20 | + | HistoryUC: usecase.NewSearchHistoryUseCase(nil), | |
| 21 | + | FavoritesUC: usecase.NewManageFavoritesUseCase(nil, nil), | |
| 22 | + | }, | |
| 23 | + | browserWindows: map[string]*browserWindow{}, | |
| 24 | + | } | |
| 25 | + | app.browserWindowFactory = func(ctx context.Context, url string) (*browserWindow, error) { | |
| 26 | + | return &browserWindow{id: "w-1", initialURL: url, mainWindow: &window.MainWindow{}}, nil | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | created, err := app.createBrowserWindow(ctx, "https://example.com") | |
| 30 | + | require.NoError(t, err) | |
| 31 | + | require.NotNil(t, created) | |
| 32 | + | assert.Nil(t, created.historySidebar, "window creation must not construct the hidden history panel") | |
| 33 | + | assert.Nil(t, created.favoritesSidebar, "window creation must not construct the hidden favorites panel") | |
| 34 | + | assert.False(t, created.sidebarVisible) | |
| 35 | + | assert.Equal(t, nativeSidebarNone, created.activeSidebarKind) | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | func TestEnsureSidebars_ConstructOnceAndReuse(t *testing.T) { | |
| 39 | + | ctx := context.Background() | |
| 40 | + | var historyConstructions atomic.Int64 | |
| 41 | + | var favoritesConstructions atomic.Int64 | |
| 42 | + | releaseHistorySeam := stubHistorySidebarConstructor(&historyConstructions) | |
| 43 | + | defer releaseHistorySeam() | |
| 44 | + | releaseFavoritesSeam := stubFavoritesSidebarConstructor(&favoritesConstructions) | |
| 45 | + | defer releaseFavoritesSeam() | |
| 46 | + | ||
| 47 | + | app := &App{ | |
| 48 | + | deps: &Dependencies{ | |
| 49 | + | HistoryUC: usecase.NewSearchHistoryUseCase(nil), | |
| 50 | + | FavoritesUC: usecase.NewManageFavoritesUseCase(nil, nil), | |
| 51 | + | }, | |
| 52 | + | browserWindows: map[string]*browserWindow{}, | |
| 53 | + | } | |
| 54 | + | bw := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |
| 55 | + | app.browserWindows[bw.id] = bw | |
| 56 | + | ||
| 57 | + | require.True(t, bw.ensureHistorySidebar(ctx, app)) | |
| 58 | + | require.True(t, bw.ensureFavoritesSidebar(ctx, app)) | |
| 59 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |
| 60 | + | assert.EqualValues(t, 1, favoritesConstructions.Load()) | |
| 61 | + | ||
| 62 | + | // Reopening resolves the same instance without reconstructing. | |
| 63 | + | firstHistory := bw.historySidebar | |
| 64 | + | firstFavorites := bw.favoritesSidebar | |
| 65 | + | require.True(t, bw.ensureHistorySidebar(ctx, app)) | |
| 66 | + | require.True(t, bw.ensureFavoritesSidebar(ctx, app)) | |
| 67 | + | assert.Same(t, firstHistory, bw.historySidebar) | |
| 68 | + | assert.Same(t, firstFavorites, bw.favoritesSidebar) | |
| 69 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |
| 70 | + | assert.EqualValues(t, 1, favoritesConstructions.Load()) | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | func TestEnsureSidebars_UnavailableWithoutDeps(t *testing.T) { | |
| 74 | + | ctx := context.Background() | |
| 75 | + | bw := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |
| 76 | + | assert.False(t, bw.ensureHistorySidebar(ctx, &App{})) | |
| 77 | + | assert.False(t, bw.ensureFavoritesSidebar(ctx, &App{})) | |
| 78 | + | assert.False(t, (&browserWindow{}).ensureHistorySidebar(ctx, &App{deps: &Dependencies{}})) | |
| 79 | + | assert.False(t, (&browserWindow{}).ensureFavoritesSidebar(ctx, &App{deps: &Dependencies{}})) | |
| 80 | + | assert.False(t, (*browserWindow)(nil).ensureHistorySidebar(ctx, &App{})) | |
| 81 | + | assert.False(t, (*browserWindow)(nil).ensureFavoritesSidebar(ctx, &App{})) | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | func TestToggleActions_EnsureOnDemandAndHideWithoutConstruction(t *testing.T) { | |
| 85 | + | ctx := context.Background() | |
| 86 | + | var historyConstructions atomic.Int64 | |
| 87 | + | releaseHistorySeam := stubHistorySidebarConstructor(&historyConstructions) | |
| 88 | + | defer releaseHistorySeam() | |
| 89 | + | ||
| 90 | + | // Close-before-open does nothing and constructs nothing. | |
| 91 | + | quiet := &browserWindow{id: "quiet", mainWindow: &window.MainWindow{}} | |
| 92 | + | quiet.hideHistorySidebar() | |
| 93 | + | quiet.hideFavoritesSidebar() | |
| 94 | + | quiet.toggleHistorySidebar() | |
| 95 | + | quiet.toggleFavoritesSidebar() | |
| 96 | + | assert.False(t, quiet.sidebarVisible) | |
| 97 | + | assert.Zero(t, historyConstructions.Load()) | |
| 98 | + | ||
| 99 | + | // Keyboard shortcut constructs on first open. | |
| 100 | + | app := &App{ | |
| 101 | + | deps: &Dependencies{HistoryUC: usecase.NewSearchHistoryUseCase(nil)}, | |
| 102 | + | browserWindows: map[string]*browserWindow{quiet.id: quiet}, | |
| 103 | + | lastFocusedWindowID: quiet.id, | |
| 104 | + | } | |
| 105 | + | require.NoError(t, app.toggleHistorySidebarAction(ctx)) | |
| 106 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |
| 107 | + | assert.True(t, quiet.sidebarVisible) | |
| 108 | + | assert.Equal(t, nativeSidebarHistory, quiet.activeSidebarKind) | |
| 109 | + | ||
| 110 | + | // Closing the visible panel hides without further construction. | |
| 111 | + | require.NoError(t, app.toggleHistorySidebarAction(ctx)) | |
| 112 | + | assert.False(t, quiet.sidebarVisible) | |
| 113 | + | assert.Equal(t, nativeSidebarNone, quiet.activeSidebarKind) | |
| 114 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |
| 115 | + | ||
| 116 | + | // Reopening reuses the retained instance. | |
| 117 | + | require.NoError(t, app.toggleHistorySidebarAction(ctx)) | |
| 118 | + | assert.True(t, quiet.sidebarVisible) | |
| 119 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | func TestToggleActions_ErrorWhenUnavailable(t *testing.T) { | |
| 123 | + | ctx := context.Background() | |
| 124 | + | app := &App{browserWindows: map[string]*browserWindow{"w": {id: "w"}}, lastFocusedWindowID: "w"} | |
| 125 | + | require.ErrorContains(t, app.toggleHistorySidebarAction(ctx), "history sidebar unavailable") | |
| 126 | + | require.ErrorContains(t, app.toggleFavoritesSidebarAction(ctx), "favorites sidebar unavailable") | |
| 127 | + | ||
| 128 | + | empty := &App{} | |
| 129 | + | require.ErrorContains(t, empty.toggleHistorySidebarAction(ctx), "no focused browser window") | |
| 130 | + | require.ErrorContains(t, empty.toggleFavoritesSidebarAction(ctx), "no focused browser window") | |
| 131 | + | } | |
| 132 | + | ||
| 133 | + | func TestSidebarConfigReload_DoesNotConstructPanels(t *testing.T) { | |
| 134 | + | ctx := context.Background() | |
| 135 | + | bw := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |
| 136 | + | app := &App{ | |
| 137 | + | deps: &Dependencies{ | |
| 138 | + | HistoryUC: usecase.NewSearchHistoryUseCase(nil), | |
| 139 | + | FavoritesUC: usecase.NewManageFavoritesUseCase(nil, nil), | |
| 140 | + | }, | |
| 141 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |
| 142 | + | lastFocusedWindowID: bw.id, | |
| 143 | + | } | |
| 144 | + | app.runtimeConfig = newRuntimeConfigState(nil) | |
| 145 | + | ||
| 146 | + | app.applyRuntimeConfigChange(ctx, app.runtimeConfigSnapshot()) | |
| 147 | + | assert.Nil(t, bw.historySidebar) | |
| 148 | + | assert.Nil(t, bw.favoritesSidebar) | |
| 149 | + | ||
| 150 | + | app.reloadVisibleFavoritesSidebars("config-reload") | |
| 151 | + | app.refreshVisibleHistorySidebars(dto.HistoryChange{}) | |
| 152 | + | assert.Nil(t, bw.historySidebar) | |
| 153 | + | assert.Nil(t, bw.favoritesSidebar) | |
| 154 | + | } | |
| 155 | + | ||
| 156 | + | func TestEnsureSidebars_MultipleWindowsIndependent(t *testing.T) { | |
| 157 | + | ctx := context.Background() | |
| 158 | + | var historyConstructions atomic.Int64 | |
| 159 | + | releaseHistorySeam := stubHistorySidebarConstructor(&historyConstructions) | |
| 160 | + | defer releaseHistorySeam() | |
| 161 | + | ||
| 162 | + | app := &App{ | |
| 163 | + | deps: &Dependencies{HistoryUC: usecase.NewSearchHistoryUseCase(nil)}, | |
| 164 | + | browserWindows: map[string]*browserWindow{}, | |
| 165 | + | } | |
| 166 | + | first := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |
| 167 | + | second := &browserWindow{id: "w-2", mainWindow: &window.MainWindow{}} | |
| 168 | + | app.browserWindows[first.id] = first | |
| 169 | + | app.browserWindows[second.id] = second | |
| 170 | + | ||
| 171 | + | require.True(t, first.ensureHistorySidebar(ctx, app)) | |
| 172 | + | assert.NotNil(t, first.historySidebar) | |
| 173 | + | assert.Nil(t, second.historySidebar) | |
| 174 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |
| 175 | + | } | |
| 176 | + | ||
| 177 | + | func stubHistorySidebarConstructor(calls *atomic.Int64) func() { | |
| 178 | + | previous := newHistorySidebarComponent | |
| 179 | + | newHistorySidebarComponent = func(ctx context.Context, cfg component.HistorySidebarConfig) *component.HistorySidebar { | |
| 180 | + | calls.Add(1) | |
| 181 | + | return &component.HistorySidebar{} | |
| 182 | + | } | |
| 183 | + | return func() { newHistorySidebarComponent = previous } | |
| 184 | + | } | |
| 185 | + | ||
| 186 | + | func stubFavoritesSidebarConstructor(calls *atomic.Int64) func() { | |
| 187 | + | previous := newFavoritesSidebarComponent | |
| 188 | + | newFavoritesSidebarComponent = func(ctx context.Context, cfg component.FavoritesSidebarConfig) *component.FavoritesSidebar { | |
| 189 | + | calls.Add(1) | |
| 190 | + | return &component.FavoritesSidebar{} | |
| 191 | + | } | |
| 192 | + | return func() { newFavoritesSidebarComponent = previous } | |
| 193 | + | } | |
| @@ -0,0 +1,193 @@ | |||||
| 1 | + | package ui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "sync/atomic" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/application/dto" | |||
| 9 | + | "github.com/bnema/dumber/internal/application/usecase" | |||
| 10 | + | "github.com/bnema/dumber/internal/ui/component" | |||
| 11 | + | "github.com/bnema/dumber/internal/ui/window" | |||
| 12 | + | "github.com/stretchr/testify/assert" | |||
| 13 | + | "github.com/stretchr/testify/require" | |||
| 14 | + | ) | |||
| 15 | + | ||||
| 16 | + | func TestWindowCreation_DoesNotConstructHiddenSidebars(t *testing.T) { | |||
| 17 | + | ctx := context.Background() | |||
| 18 | + | app := &App{ | |||
| 19 | + | deps: &Dependencies{ | |||
| 20 | + | HistoryUC: usecase.NewSearchHistoryUseCase(nil), | |||
| 21 | + | FavoritesUC: usecase.NewManageFavoritesUseCase(nil, nil), | |||
| 22 | + | }, | |||
| 23 | + | browserWindows: map[string]*browserWindow{}, | |||
| 24 | + | } | |||
| 25 | + | app.browserWindowFactory = func(ctx context.Context, url string) (*browserWindow, error) { | |||
| 26 | + | return &browserWindow{id: "w-1", initialURL: url, mainWindow: &window.MainWindow{}}, nil | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | created, err := app.createBrowserWindow(ctx, "https://example.com") | |||
| 30 | + | require.NoError(t, err) | |||
| 31 | + | require.NotNil(t, created) | |||
| 32 | + | assert.Nil(t, created.historySidebar, "window creation must not construct the hidden history panel") | |||
| 33 | + | assert.Nil(t, created.favoritesSidebar, "window creation must not construct the hidden favorites panel") | |||
| 34 | + | assert.False(t, created.sidebarVisible) | |||
| 35 | + | assert.Equal(t, nativeSidebarNone, created.activeSidebarKind) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | func TestEnsureSidebars_ConstructOnceAndReuse(t *testing.T) { | |||
| 39 | + | ctx := context.Background() | |||
| 40 | + | var historyConstructions atomic.Int64 | |||
| 41 | + | var favoritesConstructions atomic.Int64 | |||
| 42 | + | releaseHistorySeam := stubHistorySidebarConstructor(&historyConstructions) | |||
| 43 | + | defer releaseHistorySeam() | |||
| 44 | + | releaseFavoritesSeam := stubFavoritesSidebarConstructor(&favoritesConstructions) | |||
| 45 | + | defer releaseFavoritesSeam() | |||
| 46 | + | ||||
| 47 | + | app := &App{ | |||
| 48 | + | deps: &Dependencies{ | |||
| 49 | + | HistoryUC: usecase.NewSearchHistoryUseCase(nil), | |||
| 50 | + | FavoritesUC: usecase.NewManageFavoritesUseCase(nil, nil), | |||
| 51 | + | }, | |||
| 52 | + | browserWindows: map[string]*browserWindow{}, | |||
| 53 | + | } | |||
| 54 | + | bw := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |||
| 55 | + | app.browserWindows[bw.id] = bw | |||
| 56 | + | ||||
| 57 | + | require.True(t, bw.ensureHistorySidebar(ctx, app)) | |||
| 58 | + | require.True(t, bw.ensureFavoritesSidebar(ctx, app)) | |||
| 59 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |||
| 60 | + | assert.EqualValues(t, 1, favoritesConstructions.Load()) | |||
| 61 | + | ||||
| 62 | + | // Reopening resolves the same instance without reconstructing. | |||
| 63 | + | firstHistory := bw.historySidebar | |||
| 64 | + | firstFavorites := bw.favoritesSidebar | |||
| 65 | + | require.True(t, bw.ensureHistorySidebar(ctx, app)) | |||
| 66 | + | require.True(t, bw.ensureFavoritesSidebar(ctx, app)) | |||
| 67 | + | assert.Same(t, firstHistory, bw.historySidebar) | |||
| 68 | + | assert.Same(t, firstFavorites, bw.favoritesSidebar) | |||
| 69 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |||
| 70 | + | assert.EqualValues(t, 1, favoritesConstructions.Load()) | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | func TestEnsureSidebars_UnavailableWithoutDeps(t *testing.T) { | |||
| 74 | + | ctx := context.Background() | |||
| 75 | + | bw := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |||
| 76 | + | assert.False(t, bw.ensureHistorySidebar(ctx, &App{})) | |||
| 77 | + | assert.False(t, bw.ensureFavoritesSidebar(ctx, &App{})) | |||
| 78 | + | assert.False(t, (&browserWindow{}).ensureHistorySidebar(ctx, &App{deps: &Dependencies{}})) | |||
| 79 | + | assert.False(t, (&browserWindow{}).ensureFavoritesSidebar(ctx, &App{deps: &Dependencies{}})) | |||
| 80 | + | assert.False(t, (*browserWindow)(nil).ensureHistorySidebar(ctx, &App{})) | |||
| 81 | + | assert.False(t, (*browserWindow)(nil).ensureFavoritesSidebar(ctx, &App{})) | |||
| 82 | + | } | |||
| 83 | + | ||||
| 84 | + | func TestToggleActions_EnsureOnDemandAndHideWithoutConstruction(t *testing.T) { | |||
| 85 | + | ctx := context.Background() | |||
| 86 | + | var historyConstructions atomic.Int64 | |||
| 87 | + | releaseHistorySeam := stubHistorySidebarConstructor(&historyConstructions) | |||
| 88 | + | defer releaseHistorySeam() | |||
| 89 | + | ||||
| 90 | + | // Close-before-open does nothing and constructs nothing. | |||
| 91 | + | quiet := &browserWindow{id: "quiet", mainWindow: &window.MainWindow{}} | |||
| 92 | + | quiet.hideHistorySidebar() | |||
| 93 | + | quiet.hideFavoritesSidebar() | |||
| 94 | + | quiet.toggleHistorySidebar() | |||
| 95 | + | quiet.toggleFavoritesSidebar() | |||
| 96 | + | assert.False(t, quiet.sidebarVisible) | |||
| 97 | + | assert.Zero(t, historyConstructions.Load()) | |||
| 98 | + | ||||
| 99 | + | // Keyboard shortcut constructs on first open. | |||
| 100 | + | app := &App{ | |||
| 101 | + | deps: &Dependencies{HistoryUC: usecase.NewSearchHistoryUseCase(nil)}, | |||
| 102 | + | browserWindows: map[string]*browserWindow{quiet.id: quiet}, | |||
| 103 | + | lastFocusedWindowID: quiet.id, | |||
| 104 | + | } | |||
| 105 | + | require.NoError(t, app.toggleHistorySidebarAction(ctx)) | |||
| 106 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |||
| 107 | + | assert.True(t, quiet.sidebarVisible) | |||
| 108 | + | assert.Equal(t, nativeSidebarHistory, quiet.activeSidebarKind) | |||
| 109 | + | ||||
| 110 | + | // Closing the visible panel hides without further construction. | |||
| 111 | + | require.NoError(t, app.toggleHistorySidebarAction(ctx)) | |||
| 112 | + | assert.False(t, quiet.sidebarVisible) | |||
| 113 | + | assert.Equal(t, nativeSidebarNone, quiet.activeSidebarKind) | |||
| 114 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |||
| 115 | + | ||||
| 116 | + | // Reopening reuses the retained instance. | |||
| 117 | + | require.NoError(t, app.toggleHistorySidebarAction(ctx)) | |||
| 118 | + | assert.True(t, quiet.sidebarVisible) | |||
| 119 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | func TestToggleActions_ErrorWhenUnavailable(t *testing.T) { | |||
| 123 | + | ctx := context.Background() | |||
| 124 | + | app := &App{browserWindows: map[string]*browserWindow{"w": {id: "w"}}, lastFocusedWindowID: "w"} | |||
| 125 | + | require.ErrorContains(t, app.toggleHistorySidebarAction(ctx), "history sidebar unavailable") | |||
| 126 | + | require.ErrorContains(t, app.toggleFavoritesSidebarAction(ctx), "favorites sidebar unavailable") | |||
| 127 | + | ||||
| 128 | + | empty := &App{} | |||
| 129 | + | require.ErrorContains(t, empty.toggleHistorySidebarAction(ctx), "no focused browser window") | |||
| 130 | + | require.ErrorContains(t, empty.toggleFavoritesSidebarAction(ctx), "no focused browser window") | |||
| 131 | + | } | |||
| 132 | + | ||||
| 133 | + | func TestSidebarConfigReload_DoesNotConstructPanels(t *testing.T) { | |||
| 134 | + | ctx := context.Background() | |||
| 135 | + | bw := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |||
| 136 | + | app := &App{ | |||
| 137 | + | deps: &Dependencies{ | |||
| 138 | + | HistoryUC: usecase.NewSearchHistoryUseCase(nil), | |||
| 139 | + | FavoritesUC: usecase.NewManageFavoritesUseCase(nil, nil), | |||
| 140 | + | }, | |||
| 141 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |||
| 142 | + | lastFocusedWindowID: bw.id, | |||
| 143 | + | } | |||
| 144 | + | app.runtimeConfig = newRuntimeConfigState(nil) | |||
| 145 | + | ||||
| 146 | + | app.applyRuntimeConfigChange(ctx, app.runtimeConfigSnapshot()) | |||
| 147 | + | assert.Nil(t, bw.historySidebar) | |||
| 148 | + | assert.Nil(t, bw.favoritesSidebar) | |||
| 149 | + | ||||
| 150 | + | app.reloadVisibleFavoritesSidebars("config-reload") | |||
| 151 | + | app.refreshVisibleHistorySidebars(dto.HistoryChange{}) | |||
| 152 | + | assert.Nil(t, bw.historySidebar) | |||
| 153 | + | assert.Nil(t, bw.favoritesSidebar) | |||
| 154 | + | } | |||
| 155 | + | ||||
| 156 | + | func TestEnsureSidebars_MultipleWindowsIndependent(t *testing.T) { | |||
| 157 | + | ctx := context.Background() | |||
| 158 | + | var historyConstructions atomic.Int64 | |||
| 159 | + | releaseHistorySeam := stubHistorySidebarConstructor(&historyConstructions) | |||
| 160 | + | defer releaseHistorySeam() | |||
| 161 | + | ||||
| 162 | + | app := &App{ | |||
| 163 | + | deps: &Dependencies{HistoryUC: usecase.NewSearchHistoryUseCase(nil)}, | |||
| 164 | + | browserWindows: map[string]*browserWindow{}, | |||
| 165 | + | } | |||
| 166 | + | first := &browserWindow{id: "w-1", mainWindow: &window.MainWindow{}} | |||
| 167 | + | second := &browserWindow{id: "w-2", mainWindow: &window.MainWindow{}} | |||
| 168 | + | app.browserWindows[first.id] = first | |||
| 169 | + | app.browserWindows[second.id] = second | |||
| 170 | + | ||||
| 171 | + | require.True(t, first.ensureHistorySidebar(ctx, app)) | |||
| 172 | + | assert.NotNil(t, first.historySidebar) | |||
| 173 | + | assert.Nil(t, second.historySidebar) | |||
| 174 | + | assert.EqualValues(t, 1, historyConstructions.Load()) | |||
| 175 | + | } | |||
| 176 | + | ||||
| 177 | + | func stubHistorySidebarConstructor(calls *atomic.Int64) func() { | |||
| 178 | + | previous := newHistorySidebarComponent | |||
| 179 | + | newHistorySidebarComponent = func(ctx context.Context, cfg component.HistorySidebarConfig) *component.HistorySidebar { | |||
| 180 | + | calls.Add(1) | |||
| 181 | + | return &component.HistorySidebar{} | |||
| 182 | + | } | |||
| 183 | + | return func() { newHistorySidebarComponent = previous } | |||
| 184 | + | } | |||
| 185 | + | ||||
| 186 | + | func stubFavoritesSidebarConstructor(calls *atomic.Int64) func() { | |||
| 187 | + | previous := newFavoritesSidebarComponent | |||
| 188 | + | newFavoritesSidebarComponent = func(ctx context.Context, cfg component.FavoritesSidebarConfig) *component.FavoritesSidebar { | |||
| 189 | + | calls.Add(1) | |||
| 190 | + | return &component.FavoritesSidebar{} | |||
| 191 | + | } | |||
| 192 | + | return func() { newFavoritesSidebarComponent = previous } | |||
| 193 | + | } | |||
M internal/ui/component/favorites_sidebar.go +5 -2
| @@ -81,7 +81,11 @@ type FavoritesSidebar struct { | |||
| 81 | 81 | idleScheduler func(glib.SourceFunc) | |
| 82 | 82 | } | |
| 83 | 83 | ||
| 84 | - | // NewFavoritesSidebar creates a native favorites sidebar and starts loading data. | |
| 84 | + | // NewFavoritesSidebar creates a native favorites sidebar without loading | |
| 85 | + | // data. The initial load runs on the first Show so on-demand instances | |
| 86 | + | // (see the host ensure methods) issue exactly one GetAll/GetAllTags batch | |
| 87 | + | // instead of a discarded constructor generation plus the Show reload. | |
| 88 | + | // Reopening reloads through Show as before. | |
| 85 | 89 | func NewFavoritesSidebar(ctx context.Context, cfg FavoritesSidebarConfig) *FavoritesSidebar { | |
| 86 | 90 | ctx, cancel := context.WithCancel(ctx) | |
| 87 | 91 | fs := &FavoritesSidebar{ | |
| @@ -100,7 +104,6 @@ func NewFavoritesSidebar(ctx context.Context, cfg FavoritesSidebarConfig) *Favor | |||
| 100 | 104 | } | |
| 101 | 105 | fs.setupSearchHandler() | |
| 102 | 106 | fs.setupKeyboardNavigation() | |
| 103 | - | fs.startLoad() | |
| 104 | 107 | return fs | |
| 105 | 108 | } | |
| 106 | 109 | ||
| @@ -81,7 +81,11 @@ type FavoritesSidebar struct { | |||||
| 81 | idleScheduler func(glib.SourceFunc) | 81 | idleScheduler func(glib.SourceFunc) | ||
| 82 | } | 82 | } | ||
| 83 | 83 | ||||
| 84 | - | // NewFavoritesSidebar creates a native favorites sidebar and starts loading data. | 84 | + | // NewFavoritesSidebar creates a native favorites sidebar without loading |
| 85 | + | // data. The initial load runs on the first Show so on-demand instances | |||
| 86 | + | // (see the host ensure methods) issue exactly one GetAll/GetAllTags batch | |||
| 87 | + | // instead of a discarded constructor generation plus the Show reload. | |||
| 88 | + | // Reopening reloads through Show as before. | |||
| 85 | func NewFavoritesSidebar(ctx context.Context, cfg FavoritesSidebarConfig) *FavoritesSidebar { | 89 | func NewFavoritesSidebar(ctx context.Context, cfg FavoritesSidebarConfig) *FavoritesSidebar { | ||
| 86 | ctx, cancel := context.WithCancel(ctx) | 90 | ctx, cancel := context.WithCancel(ctx) | ||
| 87 | fs := &FavoritesSidebar{ | 91 | fs := &FavoritesSidebar{ | ||
| @@ -100,7 +104,6 @@ func NewFavoritesSidebar(ctx context.Context, cfg FavoritesSidebarConfig) *Favor | |||||
| 100 | } | 104 | } | ||
| 101 | fs.setupSearchHandler() | 105 | fs.setupSearchHandler() | ||
| 102 | fs.setupKeyboardNavigation() | 106 | fs.setupKeyboardNavigation() | ||
| 103 | - | fs.startLoad() | |||
| 104 | return fs | 107 | return fs | ||
| 105 | } | 108 | } | ||
| 106 | 109 | ||||
M internal/ui/component/favorites_sidebar_test.go +38 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "sync" | |
| 7 | + | "sync/atomic" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| @@ -45,9 +46,12 @@ type fakeFavoritesSidebarUC struct { | |||
| 45 | 46 | errOnUntag map[entity.TagID]error | |
| 46 | 47 | failUntagOnCall int | |
| 47 | 48 | getAllCalled chan struct{} | |
| 49 | + | getAllCalls atomic.Int64 | |
| 50 | + | getAllTagsCalls atomic.Int64 | |
| 48 | 51 | } | |
| 49 | 52 | ||
| 50 | 53 | func (f *fakeFavoritesSidebarUC) GetAll(context.Context) ([]*entity.Favorite, error) { | |
| 54 | + | f.getAllCalls.Add(1) | |
| 51 | 55 | if f.getAllCalled != nil { | |
| 52 | 56 | select { | |
| 53 | 57 | case f.getAllCalled <- struct{}{}: | |
| @@ -57,6 +61,7 @@ func (f *fakeFavoritesSidebarUC) GetAll(context.Context) ([]*entity.Favorite, er | |||
| 57 | 61 | return f.favorites, f.err | |
| 58 | 62 | } | |
| 59 | 63 | func (f *fakeFavoritesSidebarUC) GetAllTags(context.Context) ([]*entity.Tag, error) { | |
| 64 | + | f.getAllTagsCalls.Add(1) | |
| 60 | 65 | return f.tags, f.err | |
| 61 | 66 | } | |
| 62 | 67 | func (f *fakeFavoritesSidebarUC) AddTag(_ context.Context, name, color string) (*entity.Tag, error) { | |
| @@ -751,3 +756,36 @@ func TestFavoritesSidebarErrorsPreserveStateAndShowNotice(t *testing.T) { | |||
| 751 | 756 | assert.Equal(t, favoritesSidebarModeAdd, fs.mode) | |
| 752 | 757 | assert.Equal(t, "add failed", fs.notice) | |
| 753 | 758 | } | |
| 759 | + | ||
| 760 | + | // TestFavoritesSidebar_FirstShowIssuesSingleQueryBatch proves the lazy-load | |
| 761 | + | // contract behind the on-demand host: a fresh instance issues zero queries | |
| 762 | + | // (construction performs no load), the first Show-equivalent load issues | |
| 763 | + | // exactly one GetAll/GetAllTags batch, and reopening reloads with one more | |
| 764 | + | // batch according to existing semantics. | |
| 765 | + | func TestFavoritesSidebar_FirstShowIssuesSingleQueryBatch(t *testing.T) { | |
| 766 | + | fav := &entity.Favorite{ID: 1, URL: "https://example.com", Title: "Example"} | |
| 767 | + | fs := newFavoritesSidebarHarness([]*entity.Favorite{fav}, nil) | |
| 768 | + | uc := fs.favoritesUC.(*fakeFavoritesSidebarUC) | |
| 769 | + | fs.ctx = context.Background() | |
| 770 | + | ||
| 771 | + | // Fresh instance: construction must not have queried anything. | |
| 772 | + | assert.Zero(t, uc.getAllCalls.Load()) | |
| 773 | + | assert.Zero(t, uc.getAllTagsCalls.Load()) | |
| 774 | + | ||
| 775 | + | // First Show issues exactly one batch. | |
| 776 | + | fs.startLoad() | |
| 777 | + | require.Eventually(t, func() bool { | |
| 778 | + | fs.mu.RLock() | |
| 779 | + | defer fs.mu.RUnlock() | |
| 780 | + | return len(fs.allFavorites) == 1 | |
| 781 | + | }, 10*time.Second, 5*time.Millisecond, "first Show load must apply fetched favorites") | |
| 782 | + | assert.EqualValues(t, 1, uc.getAllCalls.Load(), "first Show must issue exactly one GetAll batch") | |
| 783 | + | assert.EqualValues(t, 1, uc.getAllTagsCalls.Load(), "first Show must issue exactly one GetAllTags batch") | |
| 784 | + | assert.Equal(t, "https://example.com", fs.allFavorites[0].URL) | |
| 785 | + | ||
| 786 | + | // Reopen reloads with one more batch. | |
| 787 | + | fs.startLoad() | |
| 788 | + | require.Eventually(t, func() bool { | |
| 789 | + | return uc.getAllCalls.Load() == 2 && uc.getAllTagsCalls.Load() == 2 | |
| 790 | + | }, 10*time.Second, 5*time.Millisecond, "reopen must reload with one more batch") | |
| 791 | + | } | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "sync" | 6 | "sync" | ||
| 7 | + | "sync/atomic" | |||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| @@ -45,9 +46,12 @@ type fakeFavoritesSidebarUC struct { | |||||
| 45 | errOnUntag map[entity.TagID]error | 46 | errOnUntag map[entity.TagID]error | ||
| 46 | failUntagOnCall int | 47 | failUntagOnCall int | ||
| 47 | getAllCalled chan struct{} | 48 | getAllCalled chan struct{} | ||
| 49 | + | getAllCalls atomic.Int64 | |||
| 50 | + | getAllTagsCalls atomic.Int64 | |||
| 48 | } | 51 | } | ||
| 49 | 52 | ||||
| 50 | func (f *fakeFavoritesSidebarUC) GetAll(context.Context) ([]*entity.Favorite, error) { | 53 | func (f *fakeFavoritesSidebarUC) GetAll(context.Context) ([]*entity.Favorite, error) { | ||
| 54 | + | f.getAllCalls.Add(1) | |||
| 51 | if f.getAllCalled != nil { | 55 | if f.getAllCalled != nil { | ||
| 52 | select { | 56 | select { | ||
| 53 | case f.getAllCalled <- struct{}{}: | 57 | case f.getAllCalled <- struct{}{}: | ||
| @@ -57,6 +61,7 @@ func (f *fakeFavoritesSidebarUC) GetAll(context.Context) ([]*entity.Favorite, er | |||||
| 57 | return f.favorites, f.err | 61 | return f.favorites, f.err | ||
| 58 | } | 62 | } | ||
| 59 | func (f *fakeFavoritesSidebarUC) GetAllTags(context.Context) ([]*entity.Tag, error) { | 63 | func (f *fakeFavoritesSidebarUC) GetAllTags(context.Context) ([]*entity.Tag, error) { | ||
| 64 | + | f.getAllTagsCalls.Add(1) | |||
| 60 | return f.tags, f.err | 65 | return f.tags, f.err | ||
| 61 | } | 66 | } | ||
| 62 | func (f *fakeFavoritesSidebarUC) AddTag(_ context.Context, name, color string) (*entity.Tag, error) { | 67 | func (f *fakeFavoritesSidebarUC) AddTag(_ context.Context, name, color string) (*entity.Tag, error) { | ||
| @@ -751,3 +756,36 @@ func TestFavoritesSidebarErrorsPreserveStateAndShowNotice(t *testing.T) { | |||||
| 751 | assert.Equal(t, favoritesSidebarModeAdd, fs.mode) | 756 | assert.Equal(t, favoritesSidebarModeAdd, fs.mode) | ||
| 752 | assert.Equal(t, "add failed", fs.notice) | 757 | assert.Equal(t, "add failed", fs.notice) | ||
| 753 | } | 758 | } | ||
| 759 | + | ||||
| 760 | + | // TestFavoritesSidebar_FirstShowIssuesSingleQueryBatch proves the lazy-load | |||
| 761 | + | // contract behind the on-demand host: a fresh instance issues zero queries | |||
| 762 | + | // (construction performs no load), the first Show-equivalent load issues | |||
| 763 | + | // exactly one GetAll/GetAllTags batch, and reopening reloads with one more | |||
| 764 | + | // batch according to existing semantics. | |||
| 765 | + | func TestFavoritesSidebar_FirstShowIssuesSingleQueryBatch(t *testing.T) { | |||
| 766 | + | fav := &entity.Favorite{ID: 1, URL: "https://example.com", Title: "Example"} | |||
| 767 | + | fs := newFavoritesSidebarHarness([]*entity.Favorite{fav}, nil) | |||
| 768 | + | uc := fs.favoritesUC.(*fakeFavoritesSidebarUC) | |||
| 769 | + | fs.ctx = context.Background() | |||
| 770 | + | ||||
| 771 | + | // Fresh instance: construction must not have queried anything. | |||
| 772 | + | assert.Zero(t, uc.getAllCalls.Load()) | |||
| 773 | + | assert.Zero(t, uc.getAllTagsCalls.Load()) | |||
| 774 | + | ||||
| 775 | + | // First Show issues exactly one batch. | |||
| 776 | + | fs.startLoad() | |||
| 777 | + | require.Eventually(t, func() bool { | |||
| 778 | + | fs.mu.RLock() | |||
| 779 | + | defer fs.mu.RUnlock() | |||
| 780 | + | return len(fs.allFavorites) == 1 | |||
| 781 | + | }, 10*time.Second, 5*time.Millisecond, "first Show load must apply fetched favorites") | |||
| 782 | + | assert.EqualValues(t, 1, uc.getAllCalls.Load(), "first Show must issue exactly one GetAll batch") | |||
| 783 | + | assert.EqualValues(t, 1, uc.getAllTagsCalls.Load(), "first Show must issue exactly one GetAllTags batch") | |||
| 784 | + | assert.Equal(t, "https://example.com", fs.allFavorites[0].URL) | |||
| 785 | + | ||||
| 786 | + | // Reopen reloads with one more batch. | |||
| 787 | + | fs.startLoad() | |||
| 788 | + | require.Eventually(t, func() bool { | |||
| 789 | + | return uc.getAllCalls.Load() == 2 && uc.getAllTagsCalls.Load() == 2 | |||
| 790 | + | }, 10*time.Second, 5*time.Millisecond, "reopen must reload with one more batch") | |||
| 791 | + | } | |||
M internal/ui/component/history_sidebar.go +5 -4
| @@ -138,7 +138,11 @@ type HistorySidebarConfig struct { | |||
| 138 | 138 | OnClose func() | |
| 139 | 139 | } | |
| 140 | 140 | ||
| 141 | - | // NewHistorySidebar creates a new HistorySidebar component. | |
| 141 | + | // NewHistorySidebar creates a new HistorySidebar component. Construction | |
| 142 | + | // performs no data queries: the initial load runs on the first Show so | |
| 143 | + | // on-demand instances (see the host ensure methods) never pay for a | |
| 144 | + | // discarded constructor generation before Show schedules its own Reload. | |
| 145 | + | // Reopening reloads through Show as before. | |
| 142 | 146 | func NewHistorySidebar(ctx context.Context, cfg HistorySidebarConfig) *HistorySidebar { | |
| 143 | 147 | ctx, cancel := context.WithCancel(ctx) | |
| 144 | 148 | log := logging.FromContext(ctx).With().Str("component", "history-sidebar").Logger() | |
| @@ -198,9 +202,6 @@ func NewHistorySidebar(ctx context.Context, cfg HistorySidebarConfig) *HistorySi | |||
| 198 | 202 | ||
| 199 | 203 | log.Debug().Msg("history sidebar created") | |
| 200 | 204 | ||
| 201 | - | // Start loading history asynchronously (background goroutine) | |
| 202 | - | hs.startLoadHistory() | |
| 203 | - | ||
| 204 | 205 | return hs | |
| 205 | 206 | } | |
| 206 | 207 | ||
| @@ -138,7 +138,11 @@ type HistorySidebarConfig struct { | |||||
| 138 | OnClose func() | 138 | OnClose func() | ||
| 139 | } | 139 | } | ||
| 140 | 140 | ||||
| 141 | - | // NewHistorySidebar creates a new HistorySidebar component. | 141 | + | // NewHistorySidebar creates a new HistorySidebar component. Construction |
| 142 | + | // performs no data queries: the initial load runs on the first Show so | |||
| 143 | + | // on-demand instances (see the host ensure methods) never pay for a | |||
| 144 | + | // discarded constructor generation before Show schedules its own Reload. | |||
| 145 | + | // Reopening reloads through Show as before. | |||
| 142 | func NewHistorySidebar(ctx context.Context, cfg HistorySidebarConfig) *HistorySidebar { | 146 | func NewHistorySidebar(ctx context.Context, cfg HistorySidebarConfig) *HistorySidebar { | ||
| 143 | ctx, cancel := context.WithCancel(ctx) | 147 | ctx, cancel := context.WithCancel(ctx) | ||
| 144 | log := logging.FromContext(ctx).With().Str("component", "history-sidebar").Logger() | 148 | log := logging.FromContext(ctx).With().Str("component", "history-sidebar").Logger() | ||
| @@ -198,9 +202,6 @@ func NewHistorySidebar(ctx context.Context, cfg HistorySidebarConfig) *HistorySi | |||||
| 198 | 202 | ||||
| 199 | log.Debug().Msg("history sidebar created") | 203 | log.Debug().Msg("history sidebar created") | ||
| 200 | 204 | ||||
| 201 | - | // Start loading history asynchronously (background goroutine) | |||
| 202 | - | hs.startLoadHistory() | |||
| 203 | - | ||||
| 204 | return hs | 205 | return hs | ||
| 205 | } | 206 | } | ||
| 206 | 207 | ||||
M internal/ui/component/history_sidebar_search_test.go +54 -0
| @@ -367,3 +367,57 @@ func TestFetchPage_StaleGenerationDoesNotMutateLoadingState(t *testing.T) { | |||
| 367 | 367 | assert.Equal(t, 0, hs.totalLoaded, "stale results must not update totalLoaded") | |
| 368 | 368 | hs.mu.RUnlock() | |
| 369 | 369 | } | |
| 370 | + | ||
| 371 | + | // ============================================================================= | |
| 372 | + | // First-Show single query batch: no constructor load, one batch per Show | |
| 373 | + | // ============================================================================= | |
| 374 | + | ||
| 375 | + | // TestHistorySidebar_FirstShowIssuesSingleQueryBatch proves the lazy-load | |
| 376 | + | // contract behind the on-demand host: a fresh instance issues zero queries | |
| 377 | + | // (construction performs no load), the first Show-equivalent Reload issues | |
| 378 | + | // exactly one GetRecent batch, and reopening reloads with one more batch | |
| 379 | + | // according to existing semantics. | |
| 380 | + | func TestHistorySidebar_FirstShowIssuesSingleQueryBatch(t *testing.T) { | |
| 381 | + | entries := []*entity.HistoryEntry{ | |
| 382 | + | {ID: 1, URL: "https://example.com", Title: "Example", LastVisited: time.Now()}, | |
| 383 | + | } | |
| 384 | + | history := portmocks.NewMockHistorySidebarHistory(t) | |
| 385 | + | history.EXPECT(). | |
| 386 | + | GetRecent(mock.Anything, sidebarPageSize, 0). | |
| 387 | + | RunAndReturn(func(context.Context, int, int) ([]*entity.HistoryEntry, error) { | |
| 388 | + | return entries, nil | |
| 389 | + | }). | |
| 390 | + | Times(2) | |
| 391 | + | ||
| 392 | + | hs := newTestSidebarSearchHarness() | |
| 393 | + | hs.historyUC = history | |
| 394 | + | hs.ctx = t.Context() | |
| 395 | + | hs.idleScheduler = func(cb glib.SourceFunc) { cb(0) } | |
| 396 | + | ||
| 397 | + | // Fresh instance: construction must not have queried anything (the mock | |
| 398 | + | // fails the test on any unexpected call at cleanup). | |
| 399 | + | hs.mu.RLock() | |
| 400 | + | assert.False(t, hs.loadStarted) | |
| 401 | + | assert.False(t, hs.loadDone) | |
| 402 | + | hs.mu.RUnlock() | |
| 403 | + | ||
| 404 | + | waitLoaded := func(want int) { | |
| 405 | + | t.Helper() | |
| 406 | + | require.Eventually(t, func() bool { | |
| 407 | + | hs.mu.RLock() | |
| 408 | + | defer hs.mu.RUnlock() | |
| 409 | + | return hs.loadDone && len(hs.allEntries) == want | |
| 410 | + | }, 10*time.Second, 5*time.Millisecond, "Show load must apply fetched entries") | |
| 411 | + | } | |
| 412 | + | ||
| 413 | + | // First Show loads exactly one batch. | |
| 414 | + | hs.Reload() | |
| 415 | + | waitLoaded(1) | |
| 416 | + | hs.mu.RLock() | |
| 417 | + | assert.Equal(t, "https://example.com", hs.allEntries[0].URL) | |
| 418 | + | hs.mu.RUnlock() | |
| 419 | + | ||
| 420 | + | // Reopen reloads with one more batch (Times(2) enforces the exact count). | |
| 421 | + | hs.Reload() | |
| 422 | + | waitLoaded(1) | |
| 423 | + | } | |
| @@ -367,3 +367,57 @@ func TestFetchPage_StaleGenerationDoesNotMutateLoadingState(t *testing.T) { | |||||
| 367 | assert.Equal(t, 0, hs.totalLoaded, "stale results must not update totalLoaded") | 367 | assert.Equal(t, 0, hs.totalLoaded, "stale results must not update totalLoaded") | ||
| 368 | hs.mu.RUnlock() | 368 | hs.mu.RUnlock() | ||
| 369 | } | 369 | } | ||
| 370 | + | ||||
| 371 | + | // ============================================================================= | |||
| 372 | + | // First-Show single query batch: no constructor load, one batch per Show | |||
| 373 | + | // ============================================================================= | |||
| 374 | + | ||||
| 375 | + | // TestHistorySidebar_FirstShowIssuesSingleQueryBatch proves the lazy-load | |||
| 376 | + | // contract behind the on-demand host: a fresh instance issues zero queries | |||
| 377 | + | // (construction performs no load), the first Show-equivalent Reload issues | |||
| 378 | + | // exactly one GetRecent batch, and reopening reloads with one more batch | |||
| 379 | + | // according to existing semantics. | |||
| 380 | + | func TestHistorySidebar_FirstShowIssuesSingleQueryBatch(t *testing.T) { | |||
| 381 | + | entries := []*entity.HistoryEntry{ | |||
| 382 | + | {ID: 1, URL: "https://example.com", Title: "Example", LastVisited: time.Now()}, | |||
| 383 | + | } | |||
| 384 | + | history := portmocks.NewMockHistorySidebarHistory(t) | |||
| 385 | + | history.EXPECT(). | |||
| 386 | + | GetRecent(mock.Anything, sidebarPageSize, 0). | |||
| 387 | + | RunAndReturn(func(context.Context, int, int) ([]*entity.HistoryEntry, error) { | |||
| 388 | + | return entries, nil | |||
| 389 | + | }). | |||
| 390 | + | Times(2) | |||
| 391 | + | ||||
| 392 | + | hs := newTestSidebarSearchHarness() | |||
| 393 | + | hs.historyUC = history | |||
| 394 | + | hs.ctx = t.Context() | |||
| 395 | + | hs.idleScheduler = func(cb glib.SourceFunc) { cb(0) } | |||
| 396 | + | ||||
| 397 | + | // Fresh instance: construction must not have queried anything (the mock | |||
| 398 | + | // fails the test on any unexpected call at cleanup). | |||
| 399 | + | hs.mu.RLock() | |||
| 400 | + | assert.False(t, hs.loadStarted) | |||
| 401 | + | assert.False(t, hs.loadDone) | |||
| 402 | + | hs.mu.RUnlock() | |||
| 403 | + | ||||
| 404 | + | waitLoaded := func(want int) { | |||
| 405 | + | t.Helper() | |||
| 406 | + | require.Eventually(t, func() bool { | |||
| 407 | + | hs.mu.RLock() | |||
| 408 | + | defer hs.mu.RUnlock() | |||
| 409 | + | return hs.loadDone && len(hs.allEntries) == want | |||
| 410 | + | }, 10*time.Second, 5*time.Millisecond, "Show load must apply fetched entries") | |||
| 411 | + | } | |||
| 412 | + | ||||
| 413 | + | // First Show loads exactly one batch. | |||
| 414 | + | hs.Reload() | |||
| 415 | + | waitLoaded(1) | |||
| 416 | + | hs.mu.RLock() | |||
| 417 | + | assert.Equal(t, "https://example.com", hs.allEntries[0].URL) | |||
| 418 | + | hs.mu.RUnlock() | |||
| 419 | + | ||||
| 420 | + | // Reopen reloads with one more batch (Times(2) enforces the exact count). | |||
| 421 | + | hs.Reload() | |||
| 422 | + | waitLoaded(1) | |||
| 423 | + | } | |||
M internal/ui/theme/manager.go +67 -6
| @@ -24,7 +24,20 @@ type Manager struct { | |||
| 24 | 24 | modeColors ModeColors // Modal mode indicator colors | |
| 25 | 25 | transitionDurationMs int | |
| 26 | 26 | cssProvider *gtk.CssProvider | |
| 27 | - | appliedFont string | |
| 27 | + | // appliedFonts records the GTK font name last set per display, keyed by | |
| 28 | + | // native display address. GtkSettings are per display, so the font must | |
| 29 | + | // be applied on every display even when the theme is unchanged. Keys | |
| 30 | + | // are plain addresses: no display reference is retained, so destroyed | |
| 31 | + | // displays cannot leak through the manager. | |
| 32 | + | appliedFonts map[uintptr]string | |
| 33 | + | // appliedCSS is the effective CSS identity last loaded into cssProvider. | |
| 34 | + | // appliedDisplayPtr is the native display address that provider was last | |
| 35 | + | // added for. Both are plain values: no display reference is retained, so | |
| 36 | + | // destroyed displays cannot leak through the manager. Native display | |
| 37 | + | // addresses are unique among live displays; address reuse after a display | |
| 38 | + | // is destroyed is pathological in single-display processes. | |
| 39 | + | appliedCSS string | |
| 40 | + | appliedDisplayPtr uintptr | |
| 28 | 41 | } | |
| 29 | 42 | ||
| 30 | 43 | // NewManager creates a new theme manager from an already-resolved theme. | |
| @@ -136,7 +149,38 @@ func shouldApplyGTKFontName(current, next string) bool { | |||
| 136 | 149 | return next != "" && next != current | |
| 137 | 150 | } | |
| 138 | 151 | ||
| 139 | - | // ApplyToDisplay loads the theme CSS into the display. | |
| 152 | + | // lastAppliedFontFor returns the GTK font name recorded for a display, or | |
| 153 | + | // "" when the manager has not applied a font there yet. | |
| 154 | + | func (m *Manager) lastAppliedFontFor(ptr uintptr) string { | |
| 155 | + | if m == nil || m.appliedFonts == nil { | |
| 156 | + | return "" | |
| 157 | + | } | |
| 158 | + | return m.appliedFonts[ptr] | |
| 159 | + | } | |
| 160 | + | ||
| 161 | + | // recordAppliedFontFor stores the GTK font name set on a display. The map | |
| 162 | + | // is created on first use so zero-value Managers stay usable. | |
| 163 | + | func (m *Manager) recordAppliedFontFor(ptr uintptr, font string) { | |
| 164 | + | if m == nil { | |
| 165 | + | return | |
| 166 | + | } | |
| 167 | + | if m.appliedFonts == nil { | |
| 168 | + | m.appliedFonts = make(map[uintptr]string) | |
| 169 | + | } | |
| 170 | + | m.appliedFonts[ptr] = font | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | // shouldSkipThemeReapply reports whether a theme application can be skipped | |
| 174 | + | // entirely: the provider exists, the effective CSS and font are unchanged, | |
| 175 | + | // and the target is the display the provider was last added for. | |
| 176 | + | func shouldSkipThemeReapply(hasProvider bool, appliedPtr, ptr uintptr, appliedCSS, css, appliedFont, font string) bool { | |
| 177 | + | return hasProvider && ptr == appliedPtr && css == appliedCSS && font != "" && font == appliedFont | |
| 178 | + | } | |
| 179 | + | ||
| 180 | + | // ApplyToDisplay loads the theme CSS into the display, skipping the reload | |
| 181 | + | // and re-add when the same provider already carries the effective CSS on the | |
| 182 | + | // same display. A different display always gets the provider added; the CSS | |
| 183 | + | // is only re-parsed when its identity changed. | |
| 140 | 184 | func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | |
| 141 | 185 | log := logging.FromContext(ctx) | |
| 142 | 186 | ||
| @@ -150,21 +194,31 @@ func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | |||
| 150 | 194 | palette := m.GetCurrentPalette() | |
| 151 | 195 | css := GenerateCSSFullWithTiming(palette, m.uiScale, m.fonts, m.modeColors, m.transitionDurationMs) | |
| 152 | 196 | fontName := formatGTKFontName(m.gtkFont, m.uiScale) | |
| 197 | + | displayFont := m.lastAppliedFontFor(display.Ptr) | |
| 198 | + | ||
| 199 | + | if shouldSkipThemeReapply(m.cssProvider != nil, m.appliedDisplayPtr, display.Ptr, m.appliedCSS, css, displayFont, fontName) { | |
| 200 | + | log.Debug(). | |
| 201 | + | Bool("dark_mode", m.prefersDark). | |
| 202 | + | Msg("theme CSS unchanged for display, skipping reapply") | |
| 203 | + | return | |
| 204 | + | } | |
| 153 | 205 | ||
| 154 | 206 | settings := gtk.SettingsGetForDisplay(display) | |
| 155 | 207 | if settings == nil { | |
| 156 | 208 | log.Warn().Msg("cannot apply theme font scaling: settings unavailable") | |
| 157 | - | } else if shouldApplyGTKFontName(m.appliedFont, fontName) { | |
| 209 | + | } else if shouldApplyGTKFontName(displayFont, fontName) { | |
| 158 | 210 | settings.SetPropertyGtkFontName(fontName) | |
| 159 | - | m.appliedFont = fontName | |
| 211 | + | m.recordAppliedFontFor(display.Ptr, fontName) | |
| 160 | 212 | settings.Unref() | |
| 161 | 213 | } else { | |
| 162 | 214 | settings.Unref() | |
| 163 | 215 | } | |
| 164 | 216 | ||
| 165 | 217 | // Create CSS provider if needed | |
| 218 | + | providerFresh := false | |
| 166 | 219 | if m.cssProvider == nil { | |
| 167 | 220 | m.cssProvider = gtk.NewCssProvider() | |
| 221 | + | providerFresh = true | |
| 168 | 222 | } | |
| 169 | 223 | ||
| 170 | 224 | if m.cssProvider == nil { | |
| @@ -172,13 +226,20 @@ func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | |||
| 172 | 226 | return | |
| 173 | 227 | } | |
| 174 | 228 | ||
| 175 | - | // Load CSS | |
| 176 | - | m.cssProvider.LoadFromString(css) | |
| 229 | + | // Load CSS only when its identity changed; the shared provider already | |
| 230 | + | // carries the previous CSS for a display change with an unchanged theme. | |
| 231 | + | // A freshly created provider always loads, even if the generated CSS is | |
| 232 | + | // empty, so the skip identity can never mask a missing load. | |
| 233 | + | if providerFresh || css != m.appliedCSS { | |
| 234 | + | m.cssProvider.LoadFromString(css) | |
| 235 | + | } | |
| 177 | 236 | gtk.StyleContextAddProviderForDisplay( | |
| 178 | 237 | display, | |
| 179 | 238 | m.cssProvider, | |
| 180 | 239 | uint(gtk.STYLE_PROVIDER_PRIORITY_APPLICATION), | |
| 181 | 240 | ) | |
| 241 | + | m.appliedCSS = css | |
| 242 | + | m.appliedDisplayPtr = display.Ptr | |
| 182 | 243 | ||
| 183 | 244 | log.Debug(). | |
| 184 | 245 | Bool("dark_mode", m.prefersDark). | |
| @@ -24,7 +24,20 @@ type Manager struct { | |||||
| 24 | modeColors ModeColors // Modal mode indicator colors | 24 | modeColors ModeColors // Modal mode indicator colors | ||
| 25 | transitionDurationMs int | 25 | transitionDurationMs int | ||
| 26 | cssProvider *gtk.CssProvider | 26 | cssProvider *gtk.CssProvider | ||
| 27 | - | appliedFont string | 27 | + | // appliedFonts records the GTK font name last set per display, keyed by |
| 28 | + | // native display address. GtkSettings are per display, so the font must | |||
| 29 | + | // be applied on every display even when the theme is unchanged. Keys | |||
| 30 | + | // are plain addresses: no display reference is retained, so destroyed | |||
| 31 | + | // displays cannot leak through the manager. | |||
| 32 | + | appliedFonts map[uintptr]string | |||
| 33 | + | // appliedCSS is the effective CSS identity last loaded into cssProvider. | |||
| 34 | + | // appliedDisplayPtr is the native display address that provider was last | |||
| 35 | + | // added for. Both are plain values: no display reference is retained, so | |||
| 36 | + | // destroyed displays cannot leak through the manager. Native display | |||
| 37 | + | // addresses are unique among live displays; address reuse after a display | |||
| 38 | + | // is destroyed is pathological in single-display processes. | |||
| 39 | + | appliedCSS string | |||
| 40 | + | appliedDisplayPtr uintptr | |||
| 28 | } | 41 | } | ||
| 29 | 42 | ||||
| 30 | // NewManager creates a new theme manager from an already-resolved theme. | 43 | // NewManager creates a new theme manager from an already-resolved theme. | ||
| @@ -136,7 +149,38 @@ func shouldApplyGTKFontName(current, next string) bool { | |||||
| 136 | return next != "" && next != current | 149 | return next != "" && next != current | ||
| 137 | } | 150 | } | ||
| 138 | 151 | ||||
| 139 | - | // ApplyToDisplay loads the theme CSS into the display. | 152 | + | // lastAppliedFontFor returns the GTK font name recorded for a display, or |
| 153 | + | // "" when the manager has not applied a font there yet. | |||
| 154 | + | func (m *Manager) lastAppliedFontFor(ptr uintptr) string { | |||
| 155 | + | if m == nil || m.appliedFonts == nil { | |||
| 156 | + | return "" | |||
| 157 | + | } | |||
| 158 | + | return m.appliedFonts[ptr] | |||
| 159 | + | } | |||
| 160 | + | ||||
| 161 | + | // recordAppliedFontFor stores the GTK font name set on a display. The map | |||
| 162 | + | // is created on first use so zero-value Managers stay usable. | |||
| 163 | + | func (m *Manager) recordAppliedFontFor(ptr uintptr, font string) { | |||
| 164 | + | if m == nil { | |||
| 165 | + | return | |||
| 166 | + | } | |||
| 167 | + | if m.appliedFonts == nil { | |||
| 168 | + | m.appliedFonts = make(map[uintptr]string) | |||
| 169 | + | } | |||
| 170 | + | m.appliedFonts[ptr] = font | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | // shouldSkipThemeReapply reports whether a theme application can be skipped | |||
| 174 | + | // entirely: the provider exists, the effective CSS and font are unchanged, | |||
| 175 | + | // and the target is the display the provider was last added for. | |||
| 176 | + | func shouldSkipThemeReapply(hasProvider bool, appliedPtr, ptr uintptr, appliedCSS, css, appliedFont, font string) bool { | |||
| 177 | + | return hasProvider && ptr == appliedPtr && css == appliedCSS && font != "" && font == appliedFont | |||
| 178 | + | } | |||
| 179 | + | ||||
| 180 | + | // ApplyToDisplay loads the theme CSS into the display, skipping the reload | |||
| 181 | + | // and re-add when the same provider already carries the effective CSS on the | |||
| 182 | + | // same display. A different display always gets the provider added; the CSS | |||
| 183 | + | // is only re-parsed when its identity changed. | |||
| 140 | func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | 184 | func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | ||
| 141 | log := logging.FromContext(ctx) | 185 | log := logging.FromContext(ctx) | ||
| 142 | 186 | ||||
| @@ -150,21 +194,31 @@ func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | |||||
| 150 | palette := m.GetCurrentPalette() | 194 | palette := m.GetCurrentPalette() | ||
| 151 | css := GenerateCSSFullWithTiming(palette, m.uiScale, m.fonts, m.modeColors, m.transitionDurationMs) | 195 | css := GenerateCSSFullWithTiming(palette, m.uiScale, m.fonts, m.modeColors, m.transitionDurationMs) | ||
| 152 | fontName := formatGTKFontName(m.gtkFont, m.uiScale) | 196 | fontName := formatGTKFontName(m.gtkFont, m.uiScale) | ||
| 197 | + | displayFont := m.lastAppliedFontFor(display.Ptr) | |||
| 198 | + | ||||
| 199 | + | if shouldSkipThemeReapply(m.cssProvider != nil, m.appliedDisplayPtr, display.Ptr, m.appliedCSS, css, displayFont, fontName) { | |||
| 200 | + | log.Debug(). | |||
| 201 | + | Bool("dark_mode", m.prefersDark). | |||
| 202 | + | Msg("theme CSS unchanged for display, skipping reapply") | |||
| 203 | + | return | |||
| 204 | + | } | |||
| 153 | 205 | ||||
| 154 | settings := gtk.SettingsGetForDisplay(display) | 206 | settings := gtk.SettingsGetForDisplay(display) | ||
| 155 | if settings == nil { | 207 | if settings == nil { | ||
| 156 | log.Warn().Msg("cannot apply theme font scaling: settings unavailable") | 208 | log.Warn().Msg("cannot apply theme font scaling: settings unavailable") | ||
| 157 | - | } else if shouldApplyGTKFontName(m.appliedFont, fontName) { | 209 | + | } else if shouldApplyGTKFontName(displayFont, fontName) { |
| 158 | settings.SetPropertyGtkFontName(fontName) | 210 | settings.SetPropertyGtkFontName(fontName) | ||
| 159 | - | m.appliedFont = fontName | 211 | + | m.recordAppliedFontFor(display.Ptr, fontName) |
| 160 | settings.Unref() | 212 | settings.Unref() | ||
| 161 | } else { | 213 | } else { | ||
| 162 | settings.Unref() | 214 | settings.Unref() | ||
| 163 | } | 215 | } | ||
| 164 | 216 | ||||
| 165 | // Create CSS provider if needed | 217 | // Create CSS provider if needed | ||
| 218 | + | providerFresh := false | |||
| 166 | if m.cssProvider == nil { | 219 | if m.cssProvider == nil { | ||
| 167 | m.cssProvider = gtk.NewCssProvider() | 220 | m.cssProvider = gtk.NewCssProvider() | ||
| 221 | + | providerFresh = true | |||
| 168 | } | 222 | } | ||
| 169 | 223 | ||||
| 170 | if m.cssProvider == nil { | 224 | if m.cssProvider == nil { | ||
| @@ -172,13 +226,20 @@ func (m *Manager) ApplyToDisplay(ctx context.Context, display *gdk.Display) { | |||||
| 172 | return | 226 | return | ||
| 173 | } | 227 | } | ||
| 174 | 228 | ||||
| 175 | - | // Load CSS | 229 | + | // Load CSS only when its identity changed; the shared provider already |
| 176 | - | m.cssProvider.LoadFromString(css) | 230 | + | // carries the previous CSS for a display change with an unchanged theme. |
| 231 | + | // A freshly created provider always loads, even if the generated CSS is | |||
| 232 | + | // empty, so the skip identity can never mask a missing load. | |||
| 233 | + | if providerFresh || css != m.appliedCSS { | |||
| 234 | + | m.cssProvider.LoadFromString(css) | |||
| 235 | + | } | |||
| 177 | gtk.StyleContextAddProviderForDisplay( | 236 | gtk.StyleContextAddProviderForDisplay( | ||
| 178 | display, | 237 | display, | ||
| 179 | m.cssProvider, | 238 | m.cssProvider, | ||
| 180 | uint(gtk.STYLE_PROVIDER_PRIORITY_APPLICATION), | 239 | uint(gtk.STYLE_PROVIDER_PRIORITY_APPLICATION), | ||
| 181 | ) | 240 | ) | ||
| 241 | + | m.appliedCSS = css | |||
| 242 | + | m.appliedDisplayPtr = display.Ptr | |||
| 182 | 243 | ||||
| 183 | log.Debug(). | 244 | log.Debug(). | ||
| 184 | Bool("dark_mode", m.prefersDark). | 245 | Bool("dark_mode", m.prefersDark). | ||
M internal/ui/theme/manager_test.go +80 -0
| @@ -182,3 +182,83 @@ func TestShouldApplyGTKFontName(t *testing.T) { | |||
| 182 | 182 | assert.False(t, shouldApplyGTKFontName("Fira Sans 14", "Fira Sans 14")) | |
| 183 | 183 | assert.False(t, shouldApplyGTKFontName("Fira Sans 14", "")) | |
| 184 | 184 | } | |
| 185 | + | ||
| 186 | + | func TestShouldSkipThemeReapply(t *testing.T) { | |
| 187 | + | const ( | |
| 188 | + | displayA = uintptr(0x1000) | |
| 189 | + | displayB = uintptr(0x2000) | |
| 190 | + | css = "window{background:#000;}" | |
| 191 | + | otherCSS = "window{background:#fff;}" | |
| 192 | + | font = "Adwaita Sans 14" | |
| 193 | + | otherFont = "Adwaita Sans 17" | |
| 194 | + | ) | |
| 195 | + | ||
| 196 | + | // Same provider, display, CSS and font: skip the reload and re-add. | |
| 197 | + | assert.True(t, shouldSkipThemeReapply(true, displayA, displayA, css, css, font, font), | |
| 198 | + | "repeated window creation with an unchanged theme must skip reapply") | |
| 199 | + | // No provider yet: full apply. | |
| 200 | + | assert.False(t, shouldSkipThemeReapply(false, 0, displayA, "", css, "", font)) | |
| 201 | + | // Different display: the provider must be added for the new display even | |
| 202 | + | // when the CSS is unchanged. Displays are never conflated. | |
| 203 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayB, css, css, font, font)) | |
| 204 | + | // Changed CSS (real theme change): reload. | |
| 205 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayA, css, otherCSS, font, font)) | |
| 206 | + | // Changed font (scale change): reapply for the font setting. | |
| 207 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayA, css, css, font, otherFont)) | |
| 208 | + | // Empty font identity never skips. | |
| 209 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayA, css, css, "", "")) | |
| 210 | + | } | |
| 211 | + | ||
| 212 | + | func TestEffectiveCSSIdentity_StableForUnchangedTheme(t *testing.T) { | |
| 213 | + | ctx := context.Background() | |
| 214 | + | first := NewManager(ctx, resolvedThemeFixture(true)) | |
| 215 | + | second := NewManager(ctx, resolvedThemeFixture(true)) | |
| 216 | + | ||
| 217 | + | cssOf := func(m *Manager) string { | |
| 218 | + | return GenerateCSSFullWithTiming(m.GetCurrentPalette(), m.uiScale, m.fonts, m.modeColors, m.transitionDurationMs) | |
| 219 | + | } | |
| 220 | + | ||
| 221 | + | // The dedup relies on CSS identity: identical resolved themes must | |
| 222 | + | // produce identical CSS so repeated window creation skips the reload. | |
| 223 | + | assert.Equal(t, cssOf(first), cssOf(second)) | |
| 224 | + | assert.NotEmpty(t, cssOf(first)) | |
| 225 | + | ||
| 226 | + | // A real theme change must change the identity so it is never skipped. | |
| 227 | + | changed := resolvedThemeFixture(true) | |
| 228 | + | changed.DarkPalette.Background = "#000001" | |
| 229 | + | other := NewManager(ctx, changed) | |
| 230 | + | assert.NotEqual(t, cssOf(first), cssOf(other)) | |
| 231 | + | } | |
| 232 | + | ||
| 233 | + | func TestManager_AppliedFontTrackedPerDisplay(t *testing.T) { | |
| 234 | + | ctx := context.Background() | |
| 235 | + | m := NewManager(ctx, resolvedThemeFixture(true)) | |
| 236 | + | ||
| 237 | + | const ( | |
| 238 | + | displayA = uintptr(0x1000) | |
| 239 | + | displayB = uintptr(0x2000) | |
| 240 | + | font = "Adwaita Sans 14" | |
| 241 | + | ) | |
| 242 | + | ||
| 243 | + | // No font recorded anywhere initially, including on a nil manager. | |
| 244 | + | assert.Empty(t, m.lastAppliedFontFor(displayA)) | |
| 245 | + | assert.Empty(t, m.lastAppliedFontFor(displayB)) | |
| 246 | + | assert.Empty(t, (*Manager)(nil).lastAppliedFontFor(displayA)) | |
| 247 | + | ||
| 248 | + | // Recording for one display leaves the other untouched so its | |
| 249 | + | // GtkSettings still get configured on first apply. | |
| 250 | + | m.recordAppliedFontFor(displayA, font) | |
| 251 | + | assert.Equal(t, font, m.lastAppliedFontFor(displayA)) | |
| 252 | + | assert.Empty(t, m.lastAppliedFontFor(displayB)) | |
| 253 | + | ||
| 254 | + | m.recordAppliedFontFor(displayB, font) | |
| 255 | + | assert.Equal(t, font, m.lastAppliedFontFor(displayB)) | |
| 256 | + | ||
| 257 | + | // Updating one display does not disturb the other. | |
| 258 | + | m.recordAppliedFontFor(displayA, "Adwaita Sans 17") | |
| 259 | + | assert.Equal(t, "Adwaita Sans 17", m.lastAppliedFontFor(displayA)) | |
| 260 | + | assert.Equal(t, font, m.lastAppliedFontFor(displayB)) | |
| 261 | + | ||
| 262 | + | // Recording on a nil manager is a safe no-op. | |
| 263 | + | assert.NotPanics(t, func() { (*Manager)(nil).recordAppliedFontFor(displayA, font) }) | |
| 264 | + | } | |
| @@ -182,3 +182,83 @@ func TestShouldApplyGTKFontName(t *testing.T) { | |||||
| 182 | assert.False(t, shouldApplyGTKFontName("Fira Sans 14", "Fira Sans 14")) | 182 | assert.False(t, shouldApplyGTKFontName("Fira Sans 14", "Fira Sans 14")) | ||
| 183 | assert.False(t, shouldApplyGTKFontName("Fira Sans 14", "")) | 183 | assert.False(t, shouldApplyGTKFontName("Fira Sans 14", "")) | ||
| 184 | } | 184 | } | ||
| 185 | + | ||||
| 186 | + | func TestShouldSkipThemeReapply(t *testing.T) { | |||
| 187 | + | const ( | |||
| 188 | + | displayA = uintptr(0x1000) | |||
| 189 | + | displayB = uintptr(0x2000) | |||
| 190 | + | css = "window{background:#000;}" | |||
| 191 | + | otherCSS = "window{background:#fff;}" | |||
| 192 | + | font = "Adwaita Sans 14" | |||
| 193 | + | otherFont = "Adwaita Sans 17" | |||
| 194 | + | ) | |||
| 195 | + | ||||
| 196 | + | // Same provider, display, CSS and font: skip the reload and re-add. | |||
| 197 | + | assert.True(t, shouldSkipThemeReapply(true, displayA, displayA, css, css, font, font), | |||
| 198 | + | "repeated window creation with an unchanged theme must skip reapply") | |||
| 199 | + | // No provider yet: full apply. | |||
| 200 | + | assert.False(t, shouldSkipThemeReapply(false, 0, displayA, "", css, "", font)) | |||
| 201 | + | // Different display: the provider must be added for the new display even | |||
| 202 | + | // when the CSS is unchanged. Displays are never conflated. | |||
| 203 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayB, css, css, font, font)) | |||
| 204 | + | // Changed CSS (real theme change): reload. | |||
| 205 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayA, css, otherCSS, font, font)) | |||
| 206 | + | // Changed font (scale change): reapply for the font setting. | |||
| 207 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayA, css, css, font, otherFont)) | |||
| 208 | + | // Empty font identity never skips. | |||
| 209 | + | assert.False(t, shouldSkipThemeReapply(true, displayA, displayA, css, css, "", "")) | |||
| 210 | + | } | |||
| 211 | + | ||||
| 212 | + | func TestEffectiveCSSIdentity_StableForUnchangedTheme(t *testing.T) { | |||
| 213 | + | ctx := context.Background() | |||
| 214 | + | first := NewManager(ctx, resolvedThemeFixture(true)) | |||
| 215 | + | second := NewManager(ctx, resolvedThemeFixture(true)) | |||
| 216 | + | ||||
| 217 | + | cssOf := func(m *Manager) string { | |||
| 218 | + | return GenerateCSSFullWithTiming(m.GetCurrentPalette(), m.uiScale, m.fonts, m.modeColors, m.transitionDurationMs) | |||
| 219 | + | } | |||
| 220 | + | ||||
| 221 | + | // The dedup relies on CSS identity: identical resolved themes must | |||
| 222 | + | // produce identical CSS so repeated window creation skips the reload. | |||
| 223 | + | assert.Equal(t, cssOf(first), cssOf(second)) | |||
| 224 | + | assert.NotEmpty(t, cssOf(first)) | |||
| 225 | + | ||||
| 226 | + | // A real theme change must change the identity so it is never skipped. | |||
| 227 | + | changed := resolvedThemeFixture(true) | |||
| 228 | + | changed.DarkPalette.Background = "#000001" | |||
| 229 | + | other := NewManager(ctx, changed) | |||
| 230 | + | assert.NotEqual(t, cssOf(first), cssOf(other)) | |||
| 231 | + | } | |||
| 232 | + | ||||
| 233 | + | func TestManager_AppliedFontTrackedPerDisplay(t *testing.T) { | |||
| 234 | + | ctx := context.Background() | |||
| 235 | + | m := NewManager(ctx, resolvedThemeFixture(true)) | |||
| 236 | + | ||||
| 237 | + | const ( | |||
| 238 | + | displayA = uintptr(0x1000) | |||
| 239 | + | displayB = uintptr(0x2000) | |||
| 240 | + | font = "Adwaita Sans 14" | |||
| 241 | + | ) | |||
| 242 | + | ||||
| 243 | + | // No font recorded anywhere initially, including on a nil manager. | |||
| 244 | + | assert.Empty(t, m.lastAppliedFontFor(displayA)) | |||
| 245 | + | assert.Empty(t, m.lastAppliedFontFor(displayB)) | |||
| 246 | + | assert.Empty(t, (*Manager)(nil).lastAppliedFontFor(displayA)) | |||
| 247 | + | ||||
| 248 | + | // Recording for one display leaves the other untouched so its | |||
| 249 | + | // GtkSettings still get configured on first apply. | |||
| 250 | + | m.recordAppliedFontFor(displayA, font) | |||
| 251 | + | assert.Equal(t, font, m.lastAppliedFontFor(displayA)) | |||
| 252 | + | assert.Empty(t, m.lastAppliedFontFor(displayB)) | |||
| 253 | + | ||||
| 254 | + | m.recordAppliedFontFor(displayB, font) | |||
| 255 | + | assert.Equal(t, font, m.lastAppliedFontFor(displayB)) | |||
| 256 | + | ||||
| 257 | + | // Updating one display does not disturb the other. | |||
| 258 | + | m.recordAppliedFontFor(displayA, "Adwaita Sans 17") | |||
| 259 | + | assert.Equal(t, "Adwaita Sans 17", m.lastAppliedFontFor(displayA)) | |||
| 260 | + | assert.Equal(t, font, m.lastAppliedFontFor(displayB)) | |||
| 261 | + | ||||
| 262 | + | // Recording on a nil manager is a safe no-op. | |||
| 263 | + | assert.NotPanics(t, func() { (*Manager)(nil).recordAppliedFontFor(displayA, font) }) | |||
| 264 | + | } | |||