perf(cef): web navigation, filtering and cache policy (#414)
* feat(cef): integrate residency lifecycle with quiescence * fix(cef): address senior review blockers on residency * feat(cef): share favicon refresh work with bounded admission Direct candidate refreshes join shared in-flight operations keyed by validated request identity (page origin plus ordered normalized candidates), capped at 16 candidates, 8 active operations and a 5s whole-operation deadline. Waiters cancel independently; the operation aborts with its last waiter. Overload reports ErrFaviconBusy, never false success. Only verified 404/410 populate the 30s/256-entry negative cache; auth, transient, transport and validation failures always refetch. Invalidation and explicit refresh advance an epoch that drops stale completions. The use case derives an owned cancelable background scope, aborted and drained by Close on shutdown. Covers issues #400 and #403 (favicon lots). * feat(cef): suppress stale load-end script installation Main-frame load-end captures browser instance plus navigation intent for the GTK hop; the dispatched installation drops provably stale events (old frame during process swap, callback outlived by a newer navigation) and installs exactly as before otherwise. Repeated same-document load-ends still install idempotent scripts. * feat(cef): cache contract note and filtering feasibility seam Lock the cache contract in code: only RootCachePath is configured, CachePath stays at the CEF default, and docs/cef-cache-contract.md records configured semantics plus the display-side probe plan with read-before-write and egress controls. Add the P4 test-only filtering seam: a ContentFilter decision port plus a CEF request-handler adapter mapping allow/block onto continue/cancel with main-frame, navigation, redirect, fail-open and shutdown coverage, deliberately unwired from production. Pin the P3/P5 experiment API surface (global/request contexts, GetCachePath, ResolveHost) at compile time without initializing CEF. * fix(cef): address Go review on navigation lot Serialize coordinator close with admission (late refreshes fail shut down instead of racing the drain) and cancel the last-waiter decision atomically so joiners never observe a dying call. The creating waiter advances the epoch; request misses record only fully missed requests with success clearing the entry; the order backlog compacts. Identify injection events by CEF browser identifier instead of interface identity, skip identity-less captures, and cover shutdown admission, epoch sharing, miss poisoning, fresh-join and identifier replacement with tests. * fix(cef): address Copilot and CodeRabbit review on navigation lot
- commit
- acef41d92c312271ee2148faa94464a80522d34e
- author
- bnema · · 2026-09-08 11:10 UTC
- committer
- GitHub · · 2026-09-08 11:10 UTC
- parent
- 66de0ba
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
16 files · +1964 -32
M cmd/dumber/main.go +13 -4
| @@ -328,7 +328,7 @@ func runGUI(cfg *config.Config, timing startupTiming) int { | |||
| 328 | 328 | ||
| 329 | 329 | engine.SetHandlerContext(ctx) | |
| 330 | 330 | ||
| 331 | - | useCases := createUseCases(repos, cfg) | |
| 331 | + | useCases := createUseCases(ctx, repos, cfg) | |
| 332 | 332 | defer useCases.Close() | |
| 333 | 333 | if needsEagerDB { | |
| 334 | 334 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | |
| @@ -388,7 +388,7 @@ func runStandaloneOmnibox() int { | |||
| 388 | 388 | defer dbCleanup() | |
| 389 | 389 | } | |
| 390 | 390 | ||
| 391 | - | useCases := createUseCases(repos, cfg) | |
| 391 | + | useCases := createUseCases(ctx, repos, cfg) | |
| 392 | 392 | defer useCases.Close() | |
| 393 | 393 | uiDeps, err := buildUIDependencies( | |
| 394 | 394 | ctx, | |
| @@ -898,13 +898,19 @@ type useCases struct { | |||
| 898 | 898 | } | |
| 899 | 899 | ||
| 900 | 900 | func (uc *useCases) Close() { | |
| 901 | - | if uc == nil || uc.historyRecorder == nil { | |
| 901 | + | if uc == nil { | |
| 902 | + | return | |
| 903 | + | } | |
| 904 | + | if uc.faviconUC != nil { | |
| 905 | + | uc.faviconUC.Close() | |
| 906 | + | } | |
| 907 | + | if uc.historyRecorder == nil { | |
| 902 | 908 | return | |
| 903 | 909 | } | |
| 904 | 910 | uc.historyRecorder.Close() | |
| 905 | 911 | } | |
| 906 | 912 | ||
| 907 | - | func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |
| 913 | + | func createUseCases(ctx context.Context, repos *repositories, cfg *config.Config) *useCases { | |
| 908 | 914 | const idAlphabetSize = 26 | |
| 909 | 915 | idCounter := uint64(0) | |
| 910 | 916 | idGenerator := func() string { | |
| @@ -923,6 +929,9 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |||
| 923 | 929 | Converter: infrafavicon.NewImageConverter(), | |
| 924 | 930 | Fetcher: infrafavicon.NewFetcher(), | |
| 925 | 931 | Scheduler: infrafavicon.NewRefreshScheduler(), | |
| 932 | + | // Owned lifetime: the use case derives a cancelable scope | |
| 933 | + | // for background refresh, aborted by Close on shutdown. | |
| 934 | + | Background: ctx, | |
| 926 | 935 | }) | |
| 927 | 936 | } | |
| 928 | 937 | xdgDirs, _ := config.GetXDGDirs() | |
| @@ -328,7 +328,7 @@ func runGUI(cfg *config.Config, timing startupTiming) int { | |||||
| 328 | 328 | ||||
| 329 | engine.SetHandlerContext(ctx) | 329 | engine.SetHandlerContext(ctx) | ||
| 330 | 330 | ||||
| 331 | - | useCases := createUseCases(repos, cfg) | 331 | + | useCases := createUseCases(ctx, repos, cfg) |
| 332 | defer useCases.Close() | 332 | defer useCases.Close() | ||
| 333 | if needsEagerDB { | 333 | if needsEagerDB { | ||
| 334 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | 334 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | ||
| @@ -388,7 +388,7 @@ func runStandaloneOmnibox() int { | |||||
| 388 | defer dbCleanup() | 388 | defer dbCleanup() | ||
| 389 | } | 389 | } | ||
| 390 | 390 | ||||
| 391 | - | useCases := createUseCases(repos, cfg) | 391 | + | useCases := createUseCases(ctx, repos, cfg) |
| 392 | defer useCases.Close() | 392 | defer useCases.Close() | ||
| 393 | uiDeps, err := buildUIDependencies( | 393 | uiDeps, err := buildUIDependencies( | ||
| 394 | ctx, | 394 | ctx, | ||
| @@ -898,13 +898,19 @@ type useCases struct { | |||||
| 898 | } | 898 | } | ||
| 899 | 899 | ||||
| 900 | func (uc *useCases) Close() { | 900 | func (uc *useCases) Close() { | ||
| 901 | - | if uc == nil || uc.historyRecorder == nil { | 901 | + | if uc == nil { |
| 902 | + | return | |||
| 903 | + | } | |||
| 904 | + | if uc.faviconUC != nil { | |||
| 905 | + | uc.faviconUC.Close() | |||
| 906 | + | } | |||
| 907 | + | if uc.historyRecorder == nil { | |||
| 902 | return | 908 | return | ||
| 903 | } | 909 | } | ||
| 904 | uc.historyRecorder.Close() | 910 | uc.historyRecorder.Close() | ||
| 905 | } | 911 | } | ||
| 906 | 912 | ||||
| 907 | - | func createUseCases(repos *repositories, cfg *config.Config) *useCases { | 913 | + | func createUseCases(ctx context.Context, repos *repositories, cfg *config.Config) *useCases { |
| 908 | const idAlphabetSize = 26 | 914 | const idAlphabetSize = 26 | ||
| 909 | idCounter := uint64(0) | 915 | idCounter := uint64(0) | ||
| 910 | idGenerator := func() string { | 916 | idGenerator := func() string { | ||
| @@ -923,6 +929,9 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |||||
| 923 | Converter: infrafavicon.NewImageConverter(), | 929 | Converter: infrafavicon.NewImageConverter(), | ||
| 924 | Fetcher: infrafavicon.NewFetcher(), | 930 | Fetcher: infrafavicon.NewFetcher(), | ||
| 925 | Scheduler: infrafavicon.NewRefreshScheduler(), | 931 | Scheduler: infrafavicon.NewRefreshScheduler(), | ||
| 932 | + | // Owned lifetime: the use case derives a cancelable scope | |||
| 933 | + | // for background refresh, aborted by Close on shutdown. | |||
| 934 | + | Background: ctx, | |||
| 926 | }) | 935 | }) | ||
| 927 | } | 936 | } | ||
| 928 | xdgDirs, _ := config.GetXDGDirs() | 937 | xdgDirs, _ := config.GetXDGDirs() | ||
A docs/cef-cache-contract.md +50 -0
| @@ -0,0 +1,50 @@ | |||
| 1 | + | # CEF cache contract | |
| 2 | + | ||
| 3 | + | Current product behavior for the CEF engine. No configuration change is | |
| 4 | + | proposed here; setting an explicit `CachePath` requires the P3.3 | |
| 5 | + | compatibility decision with data-preservation tests and approval. | |
| 6 | + | ||
| 7 | + | ## Configured settings | |
| 8 | + | ||
| 9 | + | - `RootCachePath` is set to the resolved state root (`DUMBER_CEF_ROOT_CACHE_PATH` | |
| 10 | + | override, engine cache dir, data dir, or profile default, in that order). | |
| 11 | + | - `CachePath` is intentionally left empty: with an empty `CachePath` CEF runs | |
| 12 | + | the global request context incognito-style, with in-memory-only storage. | |
| 13 | + | `RootCachePath` does NOT implicitly supply `CachePath` and does not grant | |
| 14 | + | persistent `localStorage`: HTML5 storage only persists across sessions with | |
| 15 | + | a non-empty `CachePath`. Locked by | |
| 16 | + | `TestPrepareCEFSettings_LeavesCachePathToCEFDefault`. | |
| 17 | + | - Browsers are created with a nil request context (`factory.go`), so every | |
| 18 | + | window shares the global request context and its storage. No per-window | |
| 19 | + | isolation is claimed. | |
| 20 | + | ||
| 21 | + | ## Observed behavior | |
| 22 | + | ||
| 23 | + | Orderly-restart runs once suggested HTTP asset reuse and localStorage | |
| 24 | + | persistence across restarts with this configuration, but that observation is | |
| 25 | + | unverified against CEF semantics: with an empty `CachePath` the global | |
| 26 | + | context is incognito-style and profile data (including `localStorage`) is | |
| 27 | + | expected to be memory-only, so restart persistence must NOT be assumed. The | |
| 28 | + | seed/read-only probes below are the only accepted proof; until they run | |
| 29 | + | green on a display host, treat this configuration as non-persistent. | |
| 30 | + | Setting an explicit `CachePath` under `RootCachePath` requires the P3.3 | |
| 31 | + | compatibility decision with data-preservation tests and approval. | |
| 32 | + | ||
| 33 | + | ## Display-side verification (pending) | |
| 34 | + | ||
| 35 | + | The binding exposes everything the probe needs; no new API is required: | |
| 36 | + | ||
| 37 | + | 1. `RequestContextGetGlobalContext()` plus | |
| 38 | + | `BrowserHost.GetRequestContext()` identity comparison proves windows | |
| 39 | + | share the global context. | |
| 40 | + | 2. `RequestContext.GetCachePath()` equality/containment against the | |
| 41 | + | configured state root proves the effective on-disk location. Export | |
| 42 | + | only equality and persistence assertions, never paths. | |
| 43 | + | 3. Seed/read-only probes: seed cacheable assets, localStorage, and | |
| 44 | + | synthetic cookies at a stable fixture origin, restart orderly, and read | |
| 45 | + | before any reseeding. Distinct-profile probes also read before writing. | |
| 46 | + | Include a no-store control and distinguish no-request hits from | |
| 47 | + | conditional revalidation. Assert no non-fixture destination is | |
| 48 | + | contacted (favicon-provider egress control included). | |
| 49 | + | 4. Never infer storage behavior from forcibly killed helpers; always shut | |
| 50 | + | down gracefully and let CEF stop its helpers. | |
| @@ -0,0 +1,50 @@ | |||||
| 1 | + | # CEF cache contract | |||
| 2 | + | ||||
| 3 | + | Current product behavior for the CEF engine. No configuration change is | |||
| 4 | + | proposed here; setting an explicit `CachePath` requires the P3.3 | |||
| 5 | + | compatibility decision with data-preservation tests and approval. | |||
| 6 | + | ||||
| 7 | + | ## Configured settings | |||
| 8 | + | ||||
| 9 | + | - `RootCachePath` is set to the resolved state root (`DUMBER_CEF_ROOT_CACHE_PATH` | |||
| 10 | + | override, engine cache dir, data dir, or profile default, in that order). | |||
| 11 | + | - `CachePath` is intentionally left empty: with an empty `CachePath` CEF runs | |||
| 12 | + | the global request context incognito-style, with in-memory-only storage. | |||
| 13 | + | `RootCachePath` does NOT implicitly supply `CachePath` and does not grant | |||
| 14 | + | persistent `localStorage`: HTML5 storage only persists across sessions with | |||
| 15 | + | a non-empty `CachePath`. Locked by | |||
| 16 | + | `TestPrepareCEFSettings_LeavesCachePathToCEFDefault`. | |||
| 17 | + | - Browsers are created with a nil request context (`factory.go`), so every | |||
| 18 | + | window shares the global request context and its storage. No per-window | |||
| 19 | + | isolation is claimed. | |||
| 20 | + | ||||
| 21 | + | ## Observed behavior | |||
| 22 | + | ||||
| 23 | + | Orderly-restart runs once suggested HTTP asset reuse and localStorage | |||
| 24 | + | persistence across restarts with this configuration, but that observation is | |||
| 25 | + | unverified against CEF semantics: with an empty `CachePath` the global | |||
| 26 | + | context is incognito-style and profile data (including `localStorage`) is | |||
| 27 | + | expected to be memory-only, so restart persistence must NOT be assumed. The | |||
| 28 | + | seed/read-only probes below are the only accepted proof; until they run | |||
| 29 | + | green on a display host, treat this configuration as non-persistent. | |||
| 30 | + | Setting an explicit `CachePath` under `RootCachePath` requires the P3.3 | |||
| 31 | + | compatibility decision with data-preservation tests and approval. | |||
| 32 | + | ||||
| 33 | + | ## Display-side verification (pending) | |||
| 34 | + | ||||
| 35 | + | The binding exposes everything the probe needs; no new API is required: | |||
| 36 | + | ||||
| 37 | + | 1. `RequestContextGetGlobalContext()` plus | |||
| 38 | + | `BrowserHost.GetRequestContext()` identity comparison proves windows | |||
| 39 | + | share the global context. | |||
| 40 | + | 2. `RequestContext.GetCachePath()` equality/containment against the | |||
| 41 | + | configured state root proves the effective on-disk location. Export | |||
| 42 | + | only equality and persistence assertions, never paths. | |||
| 43 | + | 3. Seed/read-only probes: seed cacheable assets, localStorage, and | |||
| 44 | + | synthetic cookies at a stable fixture origin, restart orderly, and read | |||
| 45 | + | before any reseeding. Distinct-profile probes also read before writing. | |||
| 46 | + | Include a no-store control and distinguish no-request hits from | |||
| 47 | + | conditional revalidation. Assert no non-fixture destination is | |||
| 48 | + | contacted (favicon-provider egress control included). | |||
| 49 | + | 4. Never infer storage behavior from forcibly killed helpers; always shut | |||
| 50 | + | down gracefully and let CEF stop its helpers. | |||
A internal/application/port/content_filter.go +32 -0
| @@ -0,0 +1,32 @@ | |||
| 1 | + | package port | |
| 2 | + | ||
| 3 | + | // FilterAction is the content-filter decision for one request. | |
| 4 | + | type FilterAction int | |
| 5 | + | ||
| 6 | + | const ( | |
| 7 | + | // FilterAllow lets the request continue. | |
| 8 | + | FilterAllow FilterAction = iota | |
| 9 | + | // FilterBlock cancels the request. | |
| 10 | + | FilterBlock | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // FilterRequest describes one request offered to a ContentFilter. URL is | |
| 14 | + | // the full request URL; IsMainFrame reports that the request belongs to the | |
| 15 | + | // main frame (a main-frame document load or one of its subresources), as | |
| 16 | + | // opposed to an iframe/subframe document or its resources; IsNavigation | |
| 17 | + | // reports a navigation versus an in-page resource load. | |
| 18 | + | type FilterRequest struct { | |
| 19 | + | URL string | |
| 20 | + | IsMainFrame bool | |
| 21 | + | IsNavigation bool | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | // ContentFilter decides synchronously whether one request may proceed. | |
| 25 | + | // This is a test-only feasibility seam for the CEF filtering gate: no | |
| 26 | + | // production matcher is wired, and none may be added without the separately | |
| 27 | + | // approved implementation design. Matchers must be pure decision functions | |
| 28 | + | // over the request; they never fetch lists, mutate state, or touch the | |
| 29 | + | // network. | |
| 30 | + | type ContentFilter interface { | |
| 31 | + | Decide(req FilterRequest) FilterAction | |
| 32 | + | } | |
| @@ -0,0 +1,32 @@ | |||||
| 1 | + | package port | |||
| 2 | + | ||||
| 3 | + | // FilterAction is the content-filter decision for one request. | |||
| 4 | + | type FilterAction int | |||
| 5 | + | ||||
| 6 | + | const ( | |||
| 7 | + | // FilterAllow lets the request continue. | |||
| 8 | + | FilterAllow FilterAction = iota | |||
| 9 | + | // FilterBlock cancels the request. | |||
| 10 | + | FilterBlock | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // FilterRequest describes one request offered to a ContentFilter. URL is | |||
| 14 | + | // the full request URL; IsMainFrame reports that the request belongs to the | |||
| 15 | + | // main frame (a main-frame document load or one of its subresources), as | |||
| 16 | + | // opposed to an iframe/subframe document or its resources; IsNavigation | |||
| 17 | + | // reports a navigation versus an in-page resource load. | |||
| 18 | + | type FilterRequest struct { | |||
| 19 | + | URL string | |||
| 20 | + | IsMainFrame bool | |||
| 21 | + | IsNavigation bool | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | // ContentFilter decides synchronously whether one request may proceed. | |||
| 25 | + | // This is a test-only feasibility seam for the CEF filtering gate: no | |||
| 26 | + | // production matcher is wired, and none may be added without the separately | |||
| 27 | + | // approved implementation design. Matchers must be pure decision functions | |||
| 28 | + | // over the request; they never fetch lists, mutate state, or touch the | |||
| 29 | + | // network. | |||
| 30 | + | type ContentFilter interface { | |||
| 31 | + | Decide(req FilterRequest) FilterAction | |||
| 32 | + | } | |||
M internal/application/port/favicon_errors.go +39 -0
| @@ -5,5 +5,44 @@ import "errors" | |||
| 5 | 5 | // ErrFaviconMiss is returned when a favicon cannot be found or served from allowed sources. | |
| 6 | 6 | var ErrFaviconMiss = errors.New("favicon miss") | |
| 7 | 7 | ||
| 8 | + | // ErrFaviconBusy is returned to direct refresh callers when the shared | |
| 9 | + | // refresh admission budget is exhausted. It is deliberately distinct from | |
| 10 | + | // ErrFaviconMiss: overload is not a miss and must never read as success. | |
| 11 | + | var ErrFaviconBusy = errors.New("favicon refresh overloaded") | |
| 12 | + | ||
| 13 | + | // ErrFaviconShutdown is returned to refresh callers arriving after the | |
| 14 | + | // use case began closing. Like overload, it is distinct from a miss. | |
| 15 | + | var ErrFaviconShutdown = errors.New("favicon refresh shutting down") | |
| 16 | + | ||
| 17 | + | // FaviconFetchError classifies a failed fetch while preserving | |
| 18 | + | // ErrFaviconMiss matching for existing callers. StatusCode carries the | |
| 19 | + | // HTTP status for HTTP failures and is zero for validation, transport, | |
| 20 | + | // cancellation, or DNS failures, which must never be negatively cached. | |
| 21 | + | type FaviconFetchError struct { | |
| 22 | + | StatusCode int | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func (e *FaviconFetchError) Error() string { | |
| 26 | + | if e == nil || e.StatusCode == 0 { | |
| 27 | + | return "favicon fetch failed" | |
| 28 | + | } | |
| 29 | + | return "favicon fetch failed: unexpected HTTP status" | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | // Is preserves ErrFaviconMiss matching so callers that treat every fetch | |
| 33 | + | // failure as a miss keep working; use FetchStatusCode for classification. | |
| 34 | + | func (*FaviconFetchError) Is(target error) bool { | |
| 35 | + | return target == ErrFaviconMiss | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | // FetchStatusCode returns the HTTP status carried by err, if any. | |
| 39 | + | func FetchStatusCode(err error) (int, bool) { | |
| 40 | + | var fetchErr *FaviconFetchError | |
| 41 | + | if errors.As(err, &fetchErr) && fetchErr != nil { | |
| 42 | + | return fetchErr.StatusCode, true | |
| 43 | + | } | |
| 44 | + | return 0, false | |
| 45 | + | } | |
| 46 | + | ||
| 8 | 47 | // ErrFaviconInvalidInput is returned when favicon storage receives invalid input. | |
| 9 | 48 | var ErrFaviconInvalidInput = errors.New("invalid favicon input") | |
| @@ -5,5 +5,44 @@ import "errors" | |||||
| 5 | // ErrFaviconMiss is returned when a favicon cannot be found or served from allowed sources. | 5 | // ErrFaviconMiss is returned when a favicon cannot be found or served from allowed sources. | ||
| 6 | var ErrFaviconMiss = errors.New("favicon miss") | 6 | var ErrFaviconMiss = errors.New("favicon miss") | ||
| 7 | 7 | ||||
| 8 | + | // ErrFaviconBusy is returned to direct refresh callers when the shared | |||
| 9 | + | // refresh admission budget is exhausted. It is deliberately distinct from | |||
| 10 | + | // ErrFaviconMiss: overload is not a miss and must never read as success. | |||
| 11 | + | var ErrFaviconBusy = errors.New("favicon refresh overloaded") | |||
| 12 | + | ||||
| 13 | + | // ErrFaviconShutdown is returned to refresh callers arriving after the | |||
| 14 | + | // use case began closing. Like overload, it is distinct from a miss. | |||
| 15 | + | var ErrFaviconShutdown = errors.New("favicon refresh shutting down") | |||
| 16 | + | ||||
| 17 | + | // FaviconFetchError classifies a failed fetch while preserving | |||
| 18 | + | // ErrFaviconMiss matching for existing callers. StatusCode carries the | |||
| 19 | + | // HTTP status for HTTP failures and is zero for validation, transport, | |||
| 20 | + | // cancellation, or DNS failures, which must never be negatively cached. | |||
| 21 | + | type FaviconFetchError struct { | |||
| 22 | + | StatusCode int | |||
| 23 | + | } | |||
| 24 | + | ||||
| 25 | + | func (e *FaviconFetchError) Error() string { | |||
| 26 | + | if e == nil || e.StatusCode == 0 { | |||
| 27 | + | return "favicon fetch failed" | |||
| 28 | + | } | |||
| 29 | + | return "favicon fetch failed: unexpected HTTP status" | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | // Is preserves ErrFaviconMiss matching so callers that treat every fetch | |||
| 33 | + | // failure as a miss keep working; use FetchStatusCode for classification. | |||
| 34 | + | func (*FaviconFetchError) Is(target error) bool { | |||
| 35 | + | return target == ErrFaviconMiss | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | // FetchStatusCode returns the HTTP status carried by err, if any. | |||
| 39 | + | func FetchStatusCode(err error) (int, bool) { | |||
| 40 | + | var fetchErr *FaviconFetchError | |||
| 41 | + | if errors.As(err, &fetchErr) && fetchErr != nil { | |||
| 42 | + | return fetchErr.StatusCode, true | |||
| 43 | + | } | |||
| 44 | + | return 0, false | |||
| 45 | + | } | |||
| 46 | + | ||||
| 8 | // ErrFaviconInvalidInput is returned when favicon storage receives invalid input. | 47 | // ErrFaviconInvalidInput is returned when favicon storage receives invalid input. | ||
| 9 | var ErrFaviconInvalidInput = errors.New("invalid favicon input") | 48 | var ErrFaviconInvalidInput = errors.New("invalid favicon input") | ||
M internal/application/usecase/favicon.go +118 -25
| @@ -78,6 +78,8 @@ type FaviconUseCase struct { | |||
| 78 | 78 | now func() time.Time | |
| 79 | 79 | ttl time.Duration | |
| 80 | 80 | background context.Context | |
| 81 | + | bgCancel context.CancelFunc | |
| 82 | + | shared *refreshCoordinator | |
| 81 | 83 | } | |
| 82 | 84 | ||
| 83 | 85 | type FaviconDeps struct { | |
| @@ -90,7 +92,11 @@ type FaviconDeps struct { | |||
| 90 | 92 | Invalidators appport.FaviconInvalidators | |
| 91 | 93 | Now func() time.Time | |
| 92 | 94 | TTL time.Duration | |
| 93 | - | Background context.Context | |
| 95 | + | // Background scopes background refresh work to the application | |
| 96 | + | // lifetime. The use case derives its own cancelable scope from it, | |
| 97 | + | // aborted by Close; callers pass the owning context (nil defaults to | |
| 98 | + | // context.Background with shutdown drain only). | |
| 99 | + | Background context.Context | |
| 94 | 100 | } | |
| 95 | 101 | ||
| 96 | 102 | func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | |
| @@ -100,9 +106,13 @@ func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | |||
| 100 | 106 | if deps.TTL == 0 { | |
| 101 | 107 | deps.TTL = favicon.DefaultTTL | |
| 102 | 108 | } | |
| 103 | - | if deps.Background == nil { | |
| 104 | - | deps.Background = context.Background() | |
| 109 | + | background := deps.Background | |
| 110 | + | if background == nil { | |
| 111 | + | background = context.Background() | |
| 105 | 112 | } | |
| 113 | + | // Owned cancelable scope for background refresh: parent cancellation | |
| 114 | + | // propagates, and Close aborts it before draining shared operations. | |
| 115 | + | background, bgCancel := context.WithCancel(background) | |
| 106 | 116 | registry := &faviconInvalidatorRegistry{} | |
| 107 | 117 | if deps.Invalidators != nil { | |
| 108 | 118 | registry.fallback = deps.Invalidators | |
| @@ -117,7 +127,9 @@ func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | |||
| 117 | 127 | invalidators: registry, | |
| 118 | 128 | now: deps.Now, | |
| 119 | 129 | ttl: deps.TTL, | |
| 120 | - | background: deps.Background, | |
| 130 | + | background: background, | |
| 131 | + | bgCancel: bgCancel, | |
| 132 | + | shared: newRefreshCoordinator(), | |
| 121 | 133 | } | |
| 122 | 134 | } | |
| 123 | 135 | ||
| @@ -312,30 +324,76 @@ func (uc *FaviconUseCase) RefreshFromIconURLs(ctx context.Context, pageURL strin | |||
| 312 | 324 | if uc.fetcher == nil { | |
| 313 | 325 | return ErrFaviconMiss | |
| 314 | 326 | } | |
| 315 | - | var lastMiss error | |
| 316 | - | for _, iconURL := range iconURLs { | |
| 317 | - | if err := ctx.Err(); err != nil { | |
| 327 | + | key, candidates, ok := refreshRequestKey(pageURL, iconURLs) | |
| 328 | + | if !ok { | |
| 329 | + | return ErrFaviconMiss | |
| 330 | + | } | |
| 331 | + | // Sealed admission wins over a cached absence: one atomic check so a | |
| 332 | + | // refresh arriving after Close reports shutdown, never a stale miss. | |
| 333 | + | if closed, miss := uc.sealedOrMiss(key); closed { | |
| 334 | + | return appport.ErrFaviconShutdown | |
| 335 | + | } else if miss { | |
| 336 | + | return ErrFaviconMiss | |
| 337 | + | } | |
| 338 | + | return uc.joinRefreshOp(ctx, key, func(opCtx context.Context, startEpoch uint64) error { | |
| 339 | + | var lastMiss error | |
| 340 | + | // allMissesCacheable tracks whether EVERY attempted candidate | |
| 341 | + | // failed with a cacheable genuine absence (verified 404/410). | |
| 342 | + | // A single non-cacheable miss (401/403, 5xx, validation, | |
| 343 | + | // transport) poisons the batch: the next call must retry the | |
| 344 | + | // transient candidate instead of reading a cached absence. | |
| 345 | + | allMissesCacheable := true | |
| 346 | + | for _, iconURL := range candidates { | |
| 347 | + | if err := opCtx.Err(); err != nil { | |
| 348 | + | return err | |
| 349 | + | } | |
| 350 | + | fetched, err := uc.fetcher.Fetch(opCtx, appport.FaviconFetchRequest{PageURL: pageURL, IconURL: iconURL}) | |
| 351 | + | if err != nil { | |
| 352 | + | if status, classified := appport.FetchStatusCode(err); classified { | |
| 353 | + | if status != 404 && status != 410 { | |
| 354 | + | allMissesCacheable = false | |
| 355 | + | } | |
| 356 | + | lastMiss = err | |
| 357 | + | continue | |
| 358 | + | } | |
| 359 | + | if errors.Is(err, ErrFaviconMiss) { | |
| 360 | + | allMissesCacheable = false | |
| 361 | + | lastMiss = err | |
| 362 | + | continue | |
| 363 | + | } | |
| 364 | + | return err | |
| 365 | + | } | |
| 366 | + | if uc.epochValue() != startEpoch { | |
| 367 | + | return nil | |
| 368 | + | } | |
| 369 | + | err = uc.observeFetched(opCtx, pageURL, fetched) | |
| 370 | + | if errors.Is(err, ErrFaviconMiss) { | |
| 371 | + | allMissesCacheable = false | |
| 372 | + | lastMiss = err | |
| 373 | + | continue | |
| 374 | + | } | |
| 375 | + | if err == nil { | |
| 376 | + | // Success contradicts any earlier absence record for | |
| 377 | + | // this request: drop it instead of letting it linger. | |
| 378 | + | uc.clearMiss(key) | |
| 379 | + | } | |
| 318 | 380 | return err | |
| 319 | 381 | } | |
| 320 | - | fetched, err := uc.fetcher.Fetch(ctx, appport.FaviconFetchRequest{PageURL: pageURL, IconURL: iconURL}) | |
| 321 | - | if errors.Is(err, ErrFaviconMiss) { | |
| 322 | - | lastMiss = err | |
| 323 | - | continue | |
| 324 | - | } | |
| 325 | - | if err != nil { | |
| 326 | - | return err | |
| 382 | + | if lastMiss != nil { | |
| 383 | + | // Only a fully missed request populates the negative cache: | |
| 384 | + | // recording per candidate would poison later candidates | |
| 385 | + | // that were never tried, and a mixed batch must never | |
| 386 | + | // suppress retry of its transient candidates. | |
| 387 | + | if allMissesCacheable { | |
| 388 | + | uc.noteMiss(key, lastMiss) | |
| 389 | + | } | |
| 390 | + | return lastMiss | |
| 327 | 391 | } | |
| 328 | - | err = uc.observeFetched(ctx, pageURL, fetched) | |
| 329 | - | if errors.Is(err, ErrFaviconMiss) { | |
| 330 | - | lastMiss = err | |
| 331 | - | continue | |
| 392 | + | if uc.epochValue() != startEpoch { | |
| 393 | + | return nil | |
| 332 | 394 | } | |
| 333 | - | return err | |
| 334 | - | } | |
| 335 | - | if lastMiss != nil { | |
| 336 | - | return lastMiss | |
| 337 | - | } | |
| 338 | - | return uc.RefreshIfStale(ctx, pageURL) | |
| 395 | + | return uc.RefreshIfStale(opCtx, pageURL) | |
| 396 | + | }) | |
| 339 | 397 | } | |
| 340 | 398 | ||
| 341 | 399 | func (uc *FaviconUseCase) refresh(ctx context.Context, pageURL string, force bool) error { | |
| @@ -363,7 +421,16 @@ func (uc *FaviconUseCase) refreshKey(ctx context.Context, key favicon.Key, pageU | |||
| 363 | 421 | if meta != nil && !favicon.ShouldRefresh(meta, uc.now(), uc.ttl) { | |
| 364 | 422 | return nil | |
| 365 | 423 | } | |
| 366 | - | return uc.fetchAndObserve(ctx, pageURL) | |
| 424 | + | // Scheduler-backed work joins the same admission budget as direct | |
| 425 | + | // refreshes; discovery has no upfront candidates, so it shares the | |
| 426 | + | // page-scoped discovery identity. | |
| 427 | + | discoveryKey, _, ok := refreshRequestKey(pageURL, nil) | |
| 428 | + | if !ok { | |
| 429 | + | return ErrFaviconMiss | |
| 430 | + | } | |
| 431 | + | return uc.joinRefreshOp(ctx, discoveryKey, func(opCtx context.Context, _ uint64) error { | |
| 432 | + | return uc.fetchAndObserve(opCtx, pageURL) | |
| 433 | + | }) | |
| 367 | 434 | } | |
| 368 | 435 | ||
| 369 | 436 | func (uc *FaviconUseCase) anyCandidateFresh(ctx context.Context, keys []favicon.Key) (bool, error) { | |
| @@ -436,6 +503,15 @@ func (uc *FaviconUseCase) EnsureSized(ctx context.Context, key favicon.Key, size | |||
| 436 | 503 | } | |
| 437 | 504 | ||
| 438 | 505 | func (uc *FaviconUseCase) Invalidate(ctx context.Context, key favicon.Key) error { | |
| 506 | + | // Invalidation advances the epoch so in-flight completions that have not | |
| 507 | + | // yet passed their pre-commit epoch check cannot repopulate the cleared | |
| 508 | + | // entries. This is best-effort suppression, not a transactional commit: | |
| 509 | + | // a completion that already passed its check may still write after the | |
| 510 | + | // clear (a transient stale icon until the next refresh), because the | |
| 511 | + | // check and the blob/repository writes are not atomic. Closing that | |
| 512 | + | // residual window would require versioned commits in the repository | |
| 513 | + | // layer and is out of scope for this stack. | |
| 514 | + | uc.bumpEpoch() | |
| 439 | 515 | if uc.blobs != nil { | |
| 440 | 516 | if err := uc.blobs.RemoveDerived(ctx, key); err != nil { | |
| 441 | 517 | return err | |
| @@ -470,3 +546,20 @@ func (uc *FaviconUseCase) scheduleRefresh(key favicon.Key, pageURL string) bool | |||
| 470 | 546 | } | |
| 471 | 547 | return uc.scheduler.Schedule(uc.background, key, func(ctx context.Context) { _ = uc.refreshKey(ctx, key, pageURL) }) | |
| 472 | 548 | } | |
| 549 | + | ||
| 550 | + | // Close aborts owned background refresh work and drains shared in-flight | |
| 551 | + | // operations. Admission is sealed first so no refresh can start after the | |
| 552 | + | // drain observes zero active operations. It is safe to call on a nil | |
| 553 | + | // receiver and more than once. | |
| 554 | + | func (uc *FaviconUseCase) Close() { | |
| 555 | + | if uc == nil { | |
| 556 | + | return | |
| 557 | + | } | |
| 558 | + | uc.shared.mu.Lock() | |
| 559 | + | uc.shared.closed = true | |
| 560 | + | uc.shared.mu.Unlock() | |
| 561 | + | if uc.bgCancel != nil { | |
| 562 | + | uc.bgCancel() | |
| 563 | + | } | |
| 564 | + | uc.drainRefreshOps() | |
| 565 | + | } | |
| @@ -78,6 +78,8 @@ type FaviconUseCase struct { | |||||
| 78 | now func() time.Time | 78 | now func() time.Time | ||
| 79 | ttl time.Duration | 79 | ttl time.Duration | ||
| 80 | background context.Context | 80 | background context.Context | ||
| 81 | + | bgCancel context.CancelFunc | |||
| 82 | + | shared *refreshCoordinator | |||
| 81 | } | 83 | } | ||
| 82 | 84 | ||||
| 83 | type FaviconDeps struct { | 85 | type FaviconDeps struct { | ||
| @@ -90,7 +92,11 @@ type FaviconDeps struct { | |||||
| 90 | Invalidators appport.FaviconInvalidators | 92 | Invalidators appport.FaviconInvalidators | ||
| 91 | Now func() time.Time | 93 | Now func() time.Time | ||
| 92 | TTL time.Duration | 94 | TTL time.Duration | ||
| 93 | - | Background context.Context | 95 | + | // Background scopes background refresh work to the application |
| 96 | + | // lifetime. The use case derives its own cancelable scope from it, | |||
| 97 | + | // aborted by Close; callers pass the owning context (nil defaults to | |||
| 98 | + | // context.Background with shutdown drain only). | |||
| 99 | + | Background context.Context | |||
| 94 | } | 100 | } | ||
| 95 | 101 | ||||
| 96 | func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | 102 | func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | ||
| @@ -100,9 +106,13 @@ func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | |||||
| 100 | if deps.TTL == 0 { | 106 | if deps.TTL == 0 { | ||
| 101 | deps.TTL = favicon.DefaultTTL | 107 | deps.TTL = favicon.DefaultTTL | ||
| 102 | } | 108 | } | ||
| 103 | - | if deps.Background == nil { | 109 | + | background := deps.Background |
| 104 | - | deps.Background = context.Background() | 110 | + | if background == nil { |
| 111 | + | background = context.Background() | |||
| 105 | } | 112 | } | ||
| 113 | + | // Owned cancelable scope for background refresh: parent cancellation | |||
| 114 | + | // propagates, and Close aborts it before draining shared operations. | |||
| 115 | + | background, bgCancel := context.WithCancel(background) | |||
| 106 | registry := &faviconInvalidatorRegistry{} | 116 | registry := &faviconInvalidatorRegistry{} | ||
| 107 | if deps.Invalidators != nil { | 117 | if deps.Invalidators != nil { | ||
| 108 | registry.fallback = deps.Invalidators | 118 | registry.fallback = deps.Invalidators | ||
| @@ -117,7 +127,9 @@ func NewFaviconUseCase(deps FaviconDeps) *FaviconUseCase { | |||||
| 117 | invalidators: registry, | 127 | invalidators: registry, | ||
| 118 | now: deps.Now, | 128 | now: deps.Now, | ||
| 119 | ttl: deps.TTL, | 129 | ttl: deps.TTL, | ||
| 120 | - | background: deps.Background, | 130 | + | background: background, |
| 131 | + | bgCancel: bgCancel, | |||
| 132 | + | shared: newRefreshCoordinator(), | |||
| 121 | } | 133 | } | ||
| 122 | } | 134 | } | ||
| 123 | 135 | ||||
| @@ -312,30 +324,76 @@ func (uc *FaviconUseCase) RefreshFromIconURLs(ctx context.Context, pageURL strin | |||||
| 312 | if uc.fetcher == nil { | 324 | if uc.fetcher == nil { | ||
| 313 | return ErrFaviconMiss | 325 | return ErrFaviconMiss | ||
| 314 | } | 326 | } | ||
| 315 | - | var lastMiss error | 327 | + | key, candidates, ok := refreshRequestKey(pageURL, iconURLs) |
| 316 | - | for _, iconURL := range iconURLs { | 328 | + | if !ok { |
| 317 | - | if err := ctx.Err(); err != nil { | 329 | + | return ErrFaviconMiss |
| 330 | + | } | |||
| 331 | + | // Sealed admission wins over a cached absence: one atomic check so a | |||
| 332 | + | // refresh arriving after Close reports shutdown, never a stale miss. | |||
| 333 | + | if closed, miss := uc.sealedOrMiss(key); closed { | |||
| 334 | + | return appport.ErrFaviconShutdown | |||
| 335 | + | } else if miss { | |||
| 336 | + | return ErrFaviconMiss | |||
| 337 | + | } | |||
| 338 | + | return uc.joinRefreshOp(ctx, key, func(opCtx context.Context, startEpoch uint64) error { | |||
| 339 | + | var lastMiss error | |||
| 340 | + | // allMissesCacheable tracks whether EVERY attempted candidate | |||
| 341 | + | // failed with a cacheable genuine absence (verified 404/410). | |||
| 342 | + | // A single non-cacheable miss (401/403, 5xx, validation, | |||
| 343 | + | // transport) poisons the batch: the next call must retry the | |||
| 344 | + | // transient candidate instead of reading a cached absence. | |||
| 345 | + | allMissesCacheable := true | |||
| 346 | + | for _, iconURL := range candidates { | |||
| 347 | + | if err := opCtx.Err(); err != nil { | |||
| 348 | + | return err | |||
| 349 | + | } | |||
| 350 | + | fetched, err := uc.fetcher.Fetch(opCtx, appport.FaviconFetchRequest{PageURL: pageURL, IconURL: iconURL}) | |||
| 351 | + | if err != nil { | |||
| 352 | + | if status, classified := appport.FetchStatusCode(err); classified { | |||
| 353 | + | if status != 404 && status != 410 { | |||
| 354 | + | allMissesCacheable = false | |||
| 355 | + | } | |||
| 356 | + | lastMiss = err | |||
| 357 | + | continue | |||
| 358 | + | } | |||
| 359 | + | if errors.Is(err, ErrFaviconMiss) { | |||
| 360 | + | allMissesCacheable = false | |||
| 361 | + | lastMiss = err | |||
| 362 | + | continue | |||
| 363 | + | } | |||
| 364 | + | return err | |||
| 365 | + | } | |||
| 366 | + | if uc.epochValue() != startEpoch { | |||
| 367 | + | return nil | |||
| 368 | + | } | |||
| 369 | + | err = uc.observeFetched(opCtx, pageURL, fetched) | |||
| 370 | + | if errors.Is(err, ErrFaviconMiss) { | |||
| 371 | + | allMissesCacheable = false | |||
| 372 | + | lastMiss = err | |||
| 373 | + | continue | |||
| 374 | + | } | |||
| 375 | + | if err == nil { | |||
| 376 | + | // Success contradicts any earlier absence record for | |||
| 377 | + | // this request: drop it instead of letting it linger. | |||
| 378 | + | uc.clearMiss(key) | |||
| 379 | + | } | |||
| 318 | return err | 380 | return err | ||
| 319 | } | 381 | } | ||
| 320 | - | fetched, err := uc.fetcher.Fetch(ctx, appport.FaviconFetchRequest{PageURL: pageURL, IconURL: iconURL}) | 382 | + | if lastMiss != nil { |
| 321 | - | if errors.Is(err, ErrFaviconMiss) { | 383 | + | // Only a fully missed request populates the negative cache: |
| 322 | - | lastMiss = err | 384 | + | // recording per candidate would poison later candidates |
| 323 | - | continue | 385 | + | // that were never tried, and a mixed batch must never |
| 324 | - | } | 386 | + | // suppress retry of its transient candidates. |
| 325 | - | if err != nil { | 387 | + | if allMissesCacheable { |
| 326 | - | return err | 388 | + | uc.noteMiss(key, lastMiss) |
| 389 | + | } | |||
| 390 | + | return lastMiss | |||
| 327 | } | 391 | } | ||
| 328 | - | err = uc.observeFetched(ctx, pageURL, fetched) | 392 | + | if uc.epochValue() != startEpoch { |
| 329 | - | if errors.Is(err, ErrFaviconMiss) { | 393 | + | return nil |
| 330 | - | lastMiss = err | |||
| 331 | - | continue | |||
| 332 | } | 394 | } | ||
| 333 | - | return err | 395 | + | return uc.RefreshIfStale(opCtx, pageURL) |
| 334 | - | } | 396 | + | }) |
| 335 | - | if lastMiss != nil { | |||
| 336 | - | return lastMiss | |||
| 337 | - | } | |||
| 338 | - | return uc.RefreshIfStale(ctx, pageURL) | |||
| 339 | } | 397 | } | ||
| 340 | 398 | ||||
| 341 | func (uc *FaviconUseCase) refresh(ctx context.Context, pageURL string, force bool) error { | 399 | func (uc *FaviconUseCase) refresh(ctx context.Context, pageURL string, force bool) error { | ||
| @@ -363,7 +421,16 @@ func (uc *FaviconUseCase) refreshKey(ctx context.Context, key favicon.Key, pageU | |||||
| 363 | if meta != nil && !favicon.ShouldRefresh(meta, uc.now(), uc.ttl) { | 421 | if meta != nil && !favicon.ShouldRefresh(meta, uc.now(), uc.ttl) { | ||
| 364 | return nil | 422 | return nil | ||
| 365 | } | 423 | } | ||
| 366 | - | return uc.fetchAndObserve(ctx, pageURL) | 424 | + | // Scheduler-backed work joins the same admission budget as direct |
| 425 | + | // refreshes; discovery has no upfront candidates, so it shares the | |||
| 426 | + | // page-scoped discovery identity. | |||
| 427 | + | discoveryKey, _, ok := refreshRequestKey(pageURL, nil) | |||
| 428 | + | if !ok { | |||
| 429 | + | return ErrFaviconMiss | |||
| 430 | + | } | |||
| 431 | + | return uc.joinRefreshOp(ctx, discoveryKey, func(opCtx context.Context, _ uint64) error { | |||
| 432 | + | return uc.fetchAndObserve(opCtx, pageURL) | |||
| 433 | + | }) | |||
| 367 | } | 434 | } | ||
| 368 | 435 | ||||
| 369 | func (uc *FaviconUseCase) anyCandidateFresh(ctx context.Context, keys []favicon.Key) (bool, error) { | 436 | func (uc *FaviconUseCase) anyCandidateFresh(ctx context.Context, keys []favicon.Key) (bool, error) { | ||
| @@ -436,6 +503,15 @@ func (uc *FaviconUseCase) EnsureSized(ctx context.Context, key favicon.Key, size | |||||
| 436 | } | 503 | } | ||
| 437 | 504 | ||||
| 438 | func (uc *FaviconUseCase) Invalidate(ctx context.Context, key favicon.Key) error { | 505 | func (uc *FaviconUseCase) Invalidate(ctx context.Context, key favicon.Key) error { | ||
| 506 | + | // Invalidation advances the epoch so in-flight completions that have not | |||
| 507 | + | // yet passed their pre-commit epoch check cannot repopulate the cleared | |||
| 508 | + | // entries. This is best-effort suppression, not a transactional commit: | |||
| 509 | + | // a completion that already passed its check may still write after the | |||
| 510 | + | // clear (a transient stale icon until the next refresh), because the | |||
| 511 | + | // check and the blob/repository writes are not atomic. Closing that | |||
| 512 | + | // residual window would require versioned commits in the repository | |||
| 513 | + | // layer and is out of scope for this stack. | |||
| 514 | + | uc.bumpEpoch() | |||
| 439 | if uc.blobs != nil { | 515 | if uc.blobs != nil { | ||
| 440 | if err := uc.blobs.RemoveDerived(ctx, key); err != nil { | 516 | if err := uc.blobs.RemoveDerived(ctx, key); err != nil { | ||
| 441 | return err | 517 | return err | ||
| @@ -470,3 +546,20 @@ func (uc *FaviconUseCase) scheduleRefresh(key favicon.Key, pageURL string) bool | |||||
| 470 | } | 546 | } | ||
| 471 | return uc.scheduler.Schedule(uc.background, key, func(ctx context.Context) { _ = uc.refreshKey(ctx, key, pageURL) }) | 547 | return uc.scheduler.Schedule(uc.background, key, func(ctx context.Context) { _ = uc.refreshKey(ctx, key, pageURL) }) | ||
| 472 | } | 548 | } | ||
| 549 | + | ||||
| 550 | + | // Close aborts owned background refresh work and drains shared in-flight | |||
| 551 | + | // operations. Admission is sealed first so no refresh can start after the | |||
| 552 | + | // drain observes zero active operations. It is safe to call on a nil | |||
| 553 | + | // receiver and more than once. | |||
| 554 | + | func (uc *FaviconUseCase) Close() { | |||
| 555 | + | if uc == nil { | |||
| 556 | + | return | |||
| 557 | + | } | |||
| 558 | + | uc.shared.mu.Lock() | |||
| 559 | + | uc.shared.closed = true | |||
| 560 | + | uc.shared.mu.Unlock() | |||
| 561 | + | if uc.bgCancel != nil { | |||
| 562 | + | uc.bgCancel() | |||
| 563 | + | } | |||
| 564 | + | uc.drainRefreshOps() | |||
| 565 | + | } | |||
A internal/application/usecase/favicon_refresh.go +379 -0
| @@ -0,0 +1,379 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "crypto/sha256" | |
| 6 | + | "encoding/hex" | |
| 7 | + | "net" | |
| 8 | + | "net/url" | |
| 9 | + | "slices" | |
| 10 | + | "strings" | |
| 11 | + | "sync" | |
| 12 | + | "time" | |
| 13 | + | ||
| 14 | + | appport "github.com/bnema/dumber/internal/application/port" | |
| 15 | + | "github.com/bnema/dumber/internal/domain/favicon" | |
| 16 | + | ) | |
| 17 | + | ||
| 18 | + | const ( | |
| 19 | + | // maxRefreshCandidates bounds how many icon URLs one refresh operation | |
| 20 | + | // accepts; extras are skipped, never queued. | |
| 21 | + | maxRefreshCandidates = 16 | |
| 22 | + | // maxRefreshCandidateLen bounds a single candidate URL length; longer | |
| 23 | + | // inputs are skipped as invalid rather than fetched. | |
| 24 | + | maxRefreshCandidateLen = 8 * 1024 | |
| 25 | + | // maxActiveRefreshes bounds distinct concurrent refresh operations | |
| 26 | + | // across direct and scheduler-backed paths. Overload skips optional | |
| 27 | + | // background work or reports ErrFaviconBusy to direct callers. | |
| 28 | + | maxActiveRefreshes = 8 | |
| 29 | + | // refreshOpTimeout bounds a whole refresh operation, including | |
| 30 | + | // context-aware DNS/validation, not per candidate. | |
| 31 | + | refreshOpTimeout = 5 * time.Second | |
| 32 | + | // faviconMissTTL bounds negative-cache entries for genuine absence. | |
| 33 | + | faviconMissTTL = 30 * time.Second | |
| 34 | + | // maxMissEntries bounds the negative cache; oldest entries are evicted. | |
| 35 | + | maxMissEntries = 256 | |
| 36 | + | ) | |
| 37 | + | ||
| 38 | + | // refreshRequestKey builds a fixed-size digest identifying request-equivalent | |
| 39 | + | // refresh work: the validated page origin plus the ordered, normalized | |
| 40 | + | // candidate list. Scheme, host, port, path, and query are significant; | |
| 41 | + | // fragments are dropped and hosts lowercased. Relative candidates resolve | |
| 42 | + | // against the page URL. At most maxRefreshCandidates URLs of at most | |
| 43 | + | // maxRefreshCandidateLen bytes are accepted; the rest are skipped. It | |
| 44 | + | // returns false when the page URL has no valid origin hierarchy. | |
| 45 | + | func refreshRequestKey(pageURL string, iconURLs []string) (string, []string, bool) { | |
| 46 | + | pageKey, ok := favicon.CanonicalKey(pageURL) | |
| 47 | + | if !ok { | |
| 48 | + | return "", nil, false | |
| 49 | + | } | |
| 50 | + | page, err := url.Parse(strings.TrimSpace(pageURL)) | |
| 51 | + | if err != nil || page.Hostname() == "" { | |
| 52 | + | return "", nil, false | |
| 53 | + | } | |
| 54 | + | accepted := make([]string, 0, min(len(iconURLs), maxRefreshCandidates)) | |
| 55 | + | for _, raw := range iconURLs { | |
| 56 | + | raw = strings.TrimSpace(raw) | |
| 57 | + | if raw == "" || len(raw) > maxRefreshCandidateLen { | |
| 58 | + | continue | |
| 59 | + | } | |
| 60 | + | candidate, err := page.Parse(raw) | |
| 61 | + | if err != nil { | |
| 62 | + | continue | |
| 63 | + | } | |
| 64 | + | if candidate.Scheme != "http" && candidate.Scheme != "https" { | |
| 65 | + | continue | |
| 66 | + | } | |
| 67 | + | if candidate.Hostname() == "" { | |
| 68 | + | continue | |
| 69 | + | } | |
| 70 | + | candidate.Fragment = "" | |
| 71 | + | candidate.Host = strings.ToLower(candidate.Host) | |
| 72 | + | if host, port, ok := splitHostPort(candidate.Host); ok { | |
| 73 | + | if isDefaultPort(candidate.Scheme, port) { | |
| 74 | + | // Re-bracket IPv6 literals: SplitHostPort strips | |
| 75 | + | // brackets, and url.URL requires them in Host. | |
| 76 | + | if strings.Contains(host, ":") { | |
| 77 | + | candidate.Host = "[" + host + "]" | |
| 78 | + | } else { | |
| 79 | + | candidate.Host = host | |
| 80 | + | } | |
| 81 | + | } | |
| 82 | + | } | |
| 83 | + | accepted = append(accepted, candidate.String()) | |
| 84 | + | if len(accepted) >= maxRefreshCandidates { | |
| 85 | + | break | |
| 86 | + | } | |
| 87 | + | } | |
| 88 | + | sum := sha256.Sum256([]byte(string(pageKey) + "\x00" + strings.Join(accepted, "\x00"))) | |
| 89 | + | return hex.EncodeToString(sum[:]), accepted, true | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | // splitHostPort splits an explicit host:port pair. Bracketed IPv6 literals | |
| 93 | + | // (as produced by url.URL.Host) require net.SplitHostPort; a bare | |
| 94 | + | // strings.Cut on ":" misparses them. Hosts without a port report false. | |
| 95 | + | func splitHostPort(hostport string) (string, string, bool) { | |
| 96 | + | host, port, err := net.SplitHostPort(hostport) | |
| 97 | + | if err != nil { | |
| 98 | + | return "", "", false | |
| 99 | + | } | |
| 100 | + | return host, port, true | |
| 101 | + | } | |
| 102 | + | ||
| 103 | + | func isDefaultPort(scheme, port string) bool { | |
| 104 | + | return (scheme == "http" && port == "80") || (scheme == "https" && port == "443") | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | // refreshCall is one shared in-flight refresh operation. Waiters join by | |
| 108 | + | // request key; the first waiter executes while the rest observe. One | |
| 109 | + | // canceled waiter never cancels the others; the operation is canceled only | |
| 110 | + | // when the last waiter leaves, decided atomically under the coordinator | |
| 111 | + | // lock so no joiner can observe a half-canceled call. | |
| 112 | + | type refreshCall struct { | |
| 113 | + | done chan struct{} | |
| 114 | + | err error | |
| 115 | + | waiters int | |
| 116 | + | cancel context.CancelFunc | |
| 117 | + | // closing marks a call whose last waiter left: it still occupies its | |
| 118 | + | // slot and map entry until the executor finishes, but no new waiter | |
| 119 | + | // may join it; joiners create a fresh operation instead. | |
| 120 | + | closing bool | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | // refreshMiss records a negatively cached genuine absence. | |
| 124 | + | type refreshMiss struct { | |
| 125 | + | expiresAt time.Time | |
| 126 | + | } | |
| 127 | + | ||
| 128 | + | // refreshCoordinator state lives on FaviconUseCase; see favicon.go. | |
| 129 | + | type refreshCoordinator struct { | |
| 130 | + | mu sync.Mutex | |
| 131 | + | active map[string]*refreshCall | |
| 132 | + | slots chan struct{} | |
| 133 | + | // wg tracks every spawned executor independently of the active map: | |
| 134 | + | // a fresh same-key operation may replace a canceled-but-still-running | |
| 135 | + | // call in active, so the map alone cannot prove shutdown quiescence. | |
| 136 | + | // Adds happen under mu before the goroutine starts; Done runs when the | |
| 137 | + | // executor finishes. Sealed admission (closed) guarantees no Add can | |
| 138 | + | // race a completed Wait. | |
| 139 | + | wg sync.WaitGroup | |
| 140 | + | epoch uint64 | |
| 141 | + | // closed seals admission: set by Close before cancel/drain, so no | |
| 142 | + | // refresh can start after the drain observes zero active operations. | |
| 143 | + | closed bool | |
| 144 | + | misses map[string]refreshMiss | |
| 145 | + | order []string | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | func newRefreshCoordinator() *refreshCoordinator { | |
| 149 | + | return &refreshCoordinator{ | |
| 150 | + | active: make(map[string]*refreshCall), | |
| 151 | + | slots: make(chan struct{}, maxActiveRefreshes), | |
| 152 | + | misses: make(map[string]refreshMiss), | |
| 153 | + | } | |
| 154 | + | } | |
| 155 | + | ||
| 156 | + | // epochValue returns the current invalidation epoch. | |
| 157 | + | func (uc *FaviconUseCase) epochValue() uint64 { | |
| 158 | + | uc.shared.mu.Lock() | |
| 159 | + | defer uc.shared.mu.Unlock() | |
| 160 | + | return uc.shared.epoch | |
| 161 | + | } | |
| 162 | + | ||
| 163 | + | // bumpEpoch advances the invalidation epoch so older in-flight completions | |
| 164 | + | // cannot repopulate invalidated entries. | |
| 165 | + | func (uc *FaviconUseCase) bumpEpoch() { | |
| 166 | + | uc.shared.mu.Lock() | |
| 167 | + | defer uc.shared.mu.Unlock() | |
| 168 | + | uc.shared.epoch++ | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | // cachedMiss reports whether key holds an unexpired genuine-absence entry. | |
| 172 | + | func (uc *FaviconUseCase) cachedMiss(key string) bool { | |
| 173 | + | _, miss := uc.sealedOrMiss(key) | |
| 174 | + | return miss | |
| 175 | + | } | |
| 176 | + | ||
| 177 | + | // sealedOrMiss atomically reports coordinator shutdown and negative-cache | |
| 178 | + | // state under one lock acquisition, giving late refreshes a single | |
| 179 | + | // linearization point: sealed admission always wins over a cached absence, | |
| 180 | + | // so a refresh arriving after Close reports ErrFaviconShutdown even when | |
| 181 | + | // the key holds a cached 404/410. | |
| 182 | + | func (uc *FaviconUseCase) sealedOrMiss(key string) (closed, miss bool) { | |
| 183 | + | now := uc.now() | |
| 184 | + | uc.shared.mu.Lock() | |
| 185 | + | defer uc.shared.mu.Unlock() | |
| 186 | + | if uc.shared.closed { | |
| 187 | + | return true, false | |
| 188 | + | } | |
| 189 | + | m, ok := uc.shared.misses[key] | |
| 190 | + | if !ok { | |
| 191 | + | return false, false | |
| 192 | + | } | |
| 193 | + | if !now.Before(m.expiresAt) { | |
| 194 | + | delete(uc.shared.misses, key) | |
| 195 | + | return false, false | |
| 196 | + | } | |
| 197 | + | return false, true | |
| 198 | + | } | |
| 199 | + | ||
| 200 | + | // clearMiss drops any absence record for key: a later success proves the | |
| 201 | + | // resource exists and must not stay suppressed. | |
| 202 | + | func (uc *FaviconUseCase) clearMiss(key string) { | |
| 203 | + | uc.shared.mu.Lock() | |
| 204 | + | defer uc.shared.mu.Unlock() | |
| 205 | + | delete(uc.shared.misses, key) | |
| 206 | + | } | |
| 207 | + | ||
| 208 | + | // noteMiss records a genuine absence (verified 404/410) for request key. | |
| 209 | + | // Only 404/410 are cacheable: 401/403, 5xx, cancellation, DNS/transport, | |
| 210 | + | // and validation failures must never suppress a later retry. | |
| 211 | + | func (uc *FaviconUseCase) noteMiss(key string, fetchErr error) { | |
| 212 | + | status, ok := appport.FetchStatusCode(fetchErr) | |
| 213 | + | if !ok || (status != 404 && status != 410) { | |
| 214 | + | return | |
| 215 | + | } | |
| 216 | + | uc.shared.mu.Lock() | |
| 217 | + | defer uc.shared.mu.Unlock() | |
| 218 | + | if _, exists := uc.shared.misses[key]; !exists { | |
| 219 | + | for len(uc.shared.misses) >= maxMissEntries { | |
| 220 | + | uc.evictMissLocked() | |
| 221 | + | } | |
| 222 | + | uc.shared.order = append(uc.shared.order, key) | |
| 223 | + | } | |
| 224 | + | uc.shared.misses[key] = refreshMiss{expiresAt: uc.now().Add(faviconMissTTL)} | |
| 225 | + | // Expired names linger in order until evicted; compact before the | |
| 226 | + | // backlog can grow without bound. | |
| 227 | + | if len(uc.shared.order) > 4*maxMissEntries { | |
| 228 | + | uc.compactMissOrderLocked() | |
| 229 | + | } | |
| 230 | + | } | |
| 231 | + | ||
| 232 | + | // compactMissOrderLocked drops order names with no live entry and collapses | |
| 233 | + | // duplicates to one entry per live key, keeping the newest occurrence. | |
| 234 | + | // Eviction removes entries from the map but not from order, so an evicted | |
| 235 | + | // key that is reinserted would otherwise leave a stale duplicate behind: | |
| 236 | + | // repeated cycles grow order without bound and let an old duplicate evict | |
| 237 | + | // the live entry early. Caller must hold refresh.mu. | |
| 238 | + | func (uc *FaviconUseCase) compactMissOrderLocked() { | |
| 239 | + | last := make(map[string]int, len(uc.shared.misses)) | |
| 240 | + | for i, key := range uc.shared.order { | |
| 241 | + | if _, ok := uc.shared.misses[key]; ok { | |
| 242 | + | last[key] = i | |
| 243 | + | } | |
| 244 | + | } | |
| 245 | + | ordered := make([]string, 0, len(last)) | |
| 246 | + | for key := range last { | |
| 247 | + | ordered = append(ordered, key) | |
| 248 | + | } | |
| 249 | + | slices.SortFunc(ordered, func(a, b string) int { return last[a] - last[b] }) | |
| 250 | + | for i := len(ordered); i < len(uc.shared.order); i++ { | |
| 251 | + | uc.shared.order[i] = "" | |
| 252 | + | } | |
| 253 | + | uc.shared.order = append(uc.shared.order[:0], ordered...) | |
| 254 | + | } | |
| 255 | + | ||
| 256 | + | // evictMissLocked removes one entry: expired first, oldest otherwise. | |
| 257 | + | // Caller must hold refresh.mu. | |
| 258 | + | func (uc *FaviconUseCase) evictMissLocked() { | |
| 259 | + | now := uc.now() | |
| 260 | + | for key, miss := range uc.shared.misses { | |
| 261 | + | if !now.Before(miss.expiresAt) { | |
| 262 | + | delete(uc.shared.misses, key) | |
| 263 | + | return | |
| 264 | + | } | |
| 265 | + | } | |
| 266 | + | for len(uc.shared.order) > 0 { | |
| 267 | + | oldest := uc.shared.order[0] | |
| 268 | + | uc.shared.order = uc.shared.order[1:] | |
| 269 | + | if _, ok := uc.shared.misses[oldest]; ok { | |
| 270 | + | delete(uc.shared.misses, oldest) | |
| 271 | + | return | |
| 272 | + | } | |
| 273 | + | } | |
| 274 | + | for key := range uc.shared.misses { | |
| 275 | + | delete(uc.shared.misses, key) | |
| 276 | + | return | |
| 277 | + | } | |
| 278 | + | } | |
| 279 | + | ||
| 280 | + | // joinRefreshOp shares equivalent in-flight work identified by key. The | |
| 281 | + | // first waiter executes exec under a whole-operation deadline derived from | |
| 282 | + | // the owned background context; joiners observe the result. The creating | |
| 283 | + | // waiter also advances the invalidation epoch, so concurrent joiners never | |
| 284 | + | // invalidate the operation they joined. A canceled waiter leaves without | |
| 285 | + | // affecting others; the operation is canceled atomically when the last | |
| 286 | + | // waiter leaves, and late joiners start a fresh operation instead of | |
| 287 | + | // observing a dying call. When the admission budget is exhausted, or the | |
| 288 | + | // coordinator is closed, direct callers receive ErrFaviconBusy or | |
| 289 | + | // ErrFaviconShutdown respectively (never false success). | |
| 290 | + | func (uc *FaviconUseCase) joinRefreshOp(ctx context.Context, key string, exec func(opCtx context.Context, startEpoch uint64) error) error { | |
| 291 | + | uc.shared.mu.Lock() | |
| 292 | + | if uc.shared.closed { | |
| 293 | + | uc.shared.mu.Unlock() | |
| 294 | + | return appport.ErrFaviconShutdown | |
| 295 | + | } | |
| 296 | + | if call, ok := uc.shared.active[key]; ok && !call.closing { | |
| 297 | + | call.waiters++ | |
| 298 | + | uc.shared.mu.Unlock() | |
| 299 | + | return uc.awaitRefreshCall(ctx, call) | |
| 300 | + | } | |
| 301 | + | select { | |
| 302 | + | case uc.shared.slots <- struct{}{}: | |
| 303 | + | default: | |
| 304 | + | uc.shared.mu.Unlock() | |
| 305 | + | return appport.ErrFaviconBusy | |
| 306 | + | } | |
| 307 | + | opCtx, cancel := context.WithTimeout(uc.background, refreshOpTimeout) | |
| 308 | + | // The creator advances the epoch with creation itself: older | |
| 309 | + | // completions cannot repopulate entries this operation supersedes, | |
| 310 | + | // and joiners share the creator's epoch instead of invalidating it. | |
| 311 | + | uc.shared.epoch++ | |
| 312 | + | startEpoch := uc.shared.epoch | |
| 313 | + | call := &refreshCall{done: make(chan struct{}), waiters: 1, cancel: cancel} | |
| 314 | + | uc.shared.active[key] = call | |
| 315 | + | uc.shared.wg.Add(1) | |
| 316 | + | uc.shared.mu.Unlock() | |
| 317 | + | ||
| 318 | + | go func() { | |
| 319 | + | defer uc.shared.wg.Done() | |
| 320 | + | err := exec(opCtx, startEpoch) | |
| 321 | + | <-uc.shared.slots | |
| 322 | + | cancel() | |
| 323 | + | uc.shared.mu.Lock() | |
| 324 | + | call.err = err | |
| 325 | + | if uc.shared.active[key] == call { | |
| 326 | + | delete(uc.shared.active, key) | |
| 327 | + | } | |
| 328 | + | uc.shared.mu.Unlock() | |
| 329 | + | close(call.done) | |
| 330 | + | }() | |
| 331 | + | ||
| 332 | + | return uc.awaitRefreshCall(ctx, call) | |
| 333 | + | } | |
| 334 | + | ||
| 335 | + | // awaitRefreshCall waits for a shared operation or the waiter's own | |
| 336 | + | // cancellation, releasing the waiter slot exactly once. The last waiter to | |
| 337 | + | // leave marks the call closing and cancels it atomically under the lock: | |
| 338 | + | // context cancellation only signals, so invoking it while holding the | |
| 339 | + | // coordinator mutex cannot block. | |
| 340 | + | func (uc *FaviconUseCase) awaitRefreshCall(ctx context.Context, call *refreshCall) error { | |
| 341 | + | select { | |
| 342 | + | case <-call.done: | |
| 343 | + | uc.shared.mu.Lock() | |
| 344 | + | err := call.err | |
| 345 | + | uc.shared.mu.Unlock() | |
| 346 | + | return err | |
| 347 | + | case <-ctx.Done(): | |
| 348 | + | uc.shared.mu.Lock() | |
| 349 | + | call.waiters-- | |
| 350 | + | if call.waiters <= 0 && !call.closing { | |
| 351 | + | call.closing = true | |
| 352 | + | call.cancel() | |
| 353 | + | } | |
| 354 | + | uc.shared.mu.Unlock() | |
| 355 | + | return ctx.Err() | |
| 356 | + | } | |
| 357 | + | } | |
| 358 | + | ||
| 359 | + | // drainRefreshOps waits until every spawned executor finished, up to a | |
| 360 | + | // bounded multiple of the operation deadline. It waits on the executor | |
| 361 | + | // wait group rather than the active map: a replaced | |
| 362 | + | // canceled-but-still-running call no longer occupies its map entry, but | |
| 363 | + | // its executor may still touch repositories. Callers cancel first (see | |
| 364 | + | // Close); cooperative operations observe the background cancellation and | |
| 365 | + | // finish promptly, so hitting the bound means stuck work that shutdown | |
| 366 | + | // must not wait out. | |
| 367 | + | func (uc *FaviconUseCase) drainRefreshOps() { | |
| 368 | + | done := make(chan struct{}) | |
| 369 | + | go func() { | |
| 370 | + | uc.shared.wg.Wait() | |
| 371 | + | close(done) | |
| 372 | + | }() | |
| 373 | + | deadline := time.NewTimer(3 * refreshOpTimeout) | |
| 374 | + | defer deadline.Stop() | |
| 375 | + | select { | |
| 376 | + | case <-done: | |
| 377 | + | case <-deadline.C: | |
| 378 | + | } | |
| 379 | + | } | |
| @@ -0,0 +1,379 @@ | |||||
| 1 | + | package usecase | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "crypto/sha256" | |||
| 6 | + | "encoding/hex" | |||
| 7 | + | "net" | |||
| 8 | + | "net/url" | |||
| 9 | + | "slices" | |||
| 10 | + | "strings" | |||
| 11 | + | "sync" | |||
| 12 | + | "time" | |||
| 13 | + | ||||
| 14 | + | appport "github.com/bnema/dumber/internal/application/port" | |||
| 15 | + | "github.com/bnema/dumber/internal/domain/favicon" | |||
| 16 | + | ) | |||
| 17 | + | ||||
| 18 | + | const ( | |||
| 19 | + | // maxRefreshCandidates bounds how many icon URLs one refresh operation | |||
| 20 | + | // accepts; extras are skipped, never queued. | |||
| 21 | + | maxRefreshCandidates = 16 | |||
| 22 | + | // maxRefreshCandidateLen bounds a single candidate URL length; longer | |||
| 23 | + | // inputs are skipped as invalid rather than fetched. | |||
| 24 | + | maxRefreshCandidateLen = 8 * 1024 | |||
| 25 | + | // maxActiveRefreshes bounds distinct concurrent refresh operations | |||
| 26 | + | // across direct and scheduler-backed paths. Overload skips optional | |||
| 27 | + | // background work or reports ErrFaviconBusy to direct callers. | |||
| 28 | + | maxActiveRefreshes = 8 | |||
| 29 | + | // refreshOpTimeout bounds a whole refresh operation, including | |||
| 30 | + | // context-aware DNS/validation, not per candidate. | |||
| 31 | + | refreshOpTimeout = 5 * time.Second | |||
| 32 | + | // faviconMissTTL bounds negative-cache entries for genuine absence. | |||
| 33 | + | faviconMissTTL = 30 * time.Second | |||
| 34 | + | // maxMissEntries bounds the negative cache; oldest entries are evicted. | |||
| 35 | + | maxMissEntries = 256 | |||
| 36 | + | ) | |||
| 37 | + | ||||
| 38 | + | // refreshRequestKey builds a fixed-size digest identifying request-equivalent | |||
| 39 | + | // refresh work: the validated page origin plus the ordered, normalized | |||
| 40 | + | // candidate list. Scheme, host, port, path, and query are significant; | |||
| 41 | + | // fragments are dropped and hosts lowercased. Relative candidates resolve | |||
| 42 | + | // against the page URL. At most maxRefreshCandidates URLs of at most | |||
| 43 | + | // maxRefreshCandidateLen bytes are accepted; the rest are skipped. It | |||
| 44 | + | // returns false when the page URL has no valid origin hierarchy. | |||
| 45 | + | func refreshRequestKey(pageURL string, iconURLs []string) (string, []string, bool) { | |||
| 46 | + | pageKey, ok := favicon.CanonicalKey(pageURL) | |||
| 47 | + | if !ok { | |||
| 48 | + | return "", nil, false | |||
| 49 | + | } | |||
| 50 | + | page, err := url.Parse(strings.TrimSpace(pageURL)) | |||
| 51 | + | if err != nil || page.Hostname() == "" { | |||
| 52 | + | return "", nil, false | |||
| 53 | + | } | |||
| 54 | + | accepted := make([]string, 0, min(len(iconURLs), maxRefreshCandidates)) | |||
| 55 | + | for _, raw := range iconURLs { | |||
| 56 | + | raw = strings.TrimSpace(raw) | |||
| 57 | + | if raw == "" || len(raw) > maxRefreshCandidateLen { | |||
| 58 | + | continue | |||
| 59 | + | } | |||
| 60 | + | candidate, err := page.Parse(raw) | |||
| 61 | + | if err != nil { | |||
| 62 | + | continue | |||
| 63 | + | } | |||
| 64 | + | if candidate.Scheme != "http" && candidate.Scheme != "https" { | |||
| 65 | + | continue | |||
| 66 | + | } | |||
| 67 | + | if candidate.Hostname() == "" { | |||
| 68 | + | continue | |||
| 69 | + | } | |||
| 70 | + | candidate.Fragment = "" | |||
| 71 | + | candidate.Host = strings.ToLower(candidate.Host) | |||
| 72 | + | if host, port, ok := splitHostPort(candidate.Host); ok { | |||
| 73 | + | if isDefaultPort(candidate.Scheme, port) { | |||
| 74 | + | // Re-bracket IPv6 literals: SplitHostPort strips | |||
| 75 | + | // brackets, and url.URL requires them in Host. | |||
| 76 | + | if strings.Contains(host, ":") { | |||
| 77 | + | candidate.Host = "[" + host + "]" | |||
| 78 | + | } else { | |||
| 79 | + | candidate.Host = host | |||
| 80 | + | } | |||
| 81 | + | } | |||
| 82 | + | } | |||
| 83 | + | accepted = append(accepted, candidate.String()) | |||
| 84 | + | if len(accepted) >= maxRefreshCandidates { | |||
| 85 | + | break | |||
| 86 | + | } | |||
| 87 | + | } | |||
| 88 | + | sum := sha256.Sum256([]byte(string(pageKey) + "\x00" + strings.Join(accepted, "\x00"))) | |||
| 89 | + | return hex.EncodeToString(sum[:]), accepted, true | |||
| 90 | + | } | |||
| 91 | + | ||||
| 92 | + | // splitHostPort splits an explicit host:port pair. Bracketed IPv6 literals | |||
| 93 | + | // (as produced by url.URL.Host) require net.SplitHostPort; a bare | |||
| 94 | + | // strings.Cut on ":" misparses them. Hosts without a port report false. | |||
| 95 | + | func splitHostPort(hostport string) (string, string, bool) { | |||
| 96 | + | host, port, err := net.SplitHostPort(hostport) | |||
| 97 | + | if err != nil { | |||
| 98 | + | return "", "", false | |||
| 99 | + | } | |||
| 100 | + | return host, port, true | |||
| 101 | + | } | |||
| 102 | + | ||||
| 103 | + | func isDefaultPort(scheme, port string) bool { | |||
| 104 | + | return (scheme == "http" && port == "80") || (scheme == "https" && port == "443") | |||
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | // refreshCall is one shared in-flight refresh operation. Waiters join by | |||
| 108 | + | // request key; the first waiter executes while the rest observe. One | |||
| 109 | + | // canceled waiter never cancels the others; the operation is canceled only | |||
| 110 | + | // when the last waiter leaves, decided atomically under the coordinator | |||
| 111 | + | // lock so no joiner can observe a half-canceled call. | |||
| 112 | + | type refreshCall struct { | |||
| 113 | + | done chan struct{} | |||
| 114 | + | err error | |||
| 115 | + | waiters int | |||
| 116 | + | cancel context.CancelFunc | |||
| 117 | + | // closing marks a call whose last waiter left: it still occupies its | |||
| 118 | + | // slot and map entry until the executor finishes, but no new waiter | |||
| 119 | + | // may join it; joiners create a fresh operation instead. | |||
| 120 | + | closing bool | |||
| 121 | + | } | |||
| 122 | + | ||||
| 123 | + | // refreshMiss records a negatively cached genuine absence. | |||
| 124 | + | type refreshMiss struct { | |||
| 125 | + | expiresAt time.Time | |||
| 126 | + | } | |||
| 127 | + | ||||
| 128 | + | // refreshCoordinator state lives on FaviconUseCase; see favicon.go. | |||
| 129 | + | type refreshCoordinator struct { | |||
| 130 | + | mu sync.Mutex | |||
| 131 | + | active map[string]*refreshCall | |||
| 132 | + | slots chan struct{} | |||
| 133 | + | // wg tracks every spawned executor independently of the active map: | |||
| 134 | + | // a fresh same-key operation may replace a canceled-but-still-running | |||
| 135 | + | // call in active, so the map alone cannot prove shutdown quiescence. | |||
| 136 | + | // Adds happen under mu before the goroutine starts; Done runs when the | |||
| 137 | + | // executor finishes. Sealed admission (closed) guarantees no Add can | |||
| 138 | + | // race a completed Wait. | |||
| 139 | + | wg sync.WaitGroup | |||
| 140 | + | epoch uint64 | |||
| 141 | + | // closed seals admission: set by Close before cancel/drain, so no | |||
| 142 | + | // refresh can start after the drain observes zero active operations. | |||
| 143 | + | closed bool | |||
| 144 | + | misses map[string]refreshMiss | |||
| 145 | + | order []string | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | func newRefreshCoordinator() *refreshCoordinator { | |||
| 149 | + | return &refreshCoordinator{ | |||
| 150 | + | active: make(map[string]*refreshCall), | |||
| 151 | + | slots: make(chan struct{}, maxActiveRefreshes), | |||
| 152 | + | misses: make(map[string]refreshMiss), | |||
| 153 | + | } | |||
| 154 | + | } | |||
| 155 | + | ||||
| 156 | + | // epochValue returns the current invalidation epoch. | |||
| 157 | + | func (uc *FaviconUseCase) epochValue() uint64 { | |||
| 158 | + | uc.shared.mu.Lock() | |||
| 159 | + | defer uc.shared.mu.Unlock() | |||
| 160 | + | return uc.shared.epoch | |||
| 161 | + | } | |||
| 162 | + | ||||
| 163 | + | // bumpEpoch advances the invalidation epoch so older in-flight completions | |||
| 164 | + | // cannot repopulate invalidated entries. | |||
| 165 | + | func (uc *FaviconUseCase) bumpEpoch() { | |||
| 166 | + | uc.shared.mu.Lock() | |||
| 167 | + | defer uc.shared.mu.Unlock() | |||
| 168 | + | uc.shared.epoch++ | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | // cachedMiss reports whether key holds an unexpired genuine-absence entry. | |||
| 172 | + | func (uc *FaviconUseCase) cachedMiss(key string) bool { | |||
| 173 | + | _, miss := uc.sealedOrMiss(key) | |||
| 174 | + | return miss | |||
| 175 | + | } | |||
| 176 | + | ||||
| 177 | + | // sealedOrMiss atomically reports coordinator shutdown and negative-cache | |||
| 178 | + | // state under one lock acquisition, giving late refreshes a single | |||
| 179 | + | // linearization point: sealed admission always wins over a cached absence, | |||
| 180 | + | // so a refresh arriving after Close reports ErrFaviconShutdown even when | |||
| 181 | + | // the key holds a cached 404/410. | |||
| 182 | + | func (uc *FaviconUseCase) sealedOrMiss(key string) (closed, miss bool) { | |||
| 183 | + | now := uc.now() | |||
| 184 | + | uc.shared.mu.Lock() | |||
| 185 | + | defer uc.shared.mu.Unlock() | |||
| 186 | + | if uc.shared.closed { | |||
| 187 | + | return true, false | |||
| 188 | + | } | |||
| 189 | + | m, ok := uc.shared.misses[key] | |||
| 190 | + | if !ok { | |||
| 191 | + | return false, false | |||
| 192 | + | } | |||
| 193 | + | if !now.Before(m.expiresAt) { | |||
| 194 | + | delete(uc.shared.misses, key) | |||
| 195 | + | return false, false | |||
| 196 | + | } | |||
| 197 | + | return false, true | |||
| 198 | + | } | |||
| 199 | + | ||||
| 200 | + | // clearMiss drops any absence record for key: a later success proves the | |||
| 201 | + | // resource exists and must not stay suppressed. | |||
| 202 | + | func (uc *FaviconUseCase) clearMiss(key string) { | |||
| 203 | + | uc.shared.mu.Lock() | |||
| 204 | + | defer uc.shared.mu.Unlock() | |||
| 205 | + | delete(uc.shared.misses, key) | |||
| 206 | + | } | |||
| 207 | + | ||||
| 208 | + | // noteMiss records a genuine absence (verified 404/410) for request key. | |||
| 209 | + | // Only 404/410 are cacheable: 401/403, 5xx, cancellation, DNS/transport, | |||
| 210 | + | // and validation failures must never suppress a later retry. | |||
| 211 | + | func (uc *FaviconUseCase) noteMiss(key string, fetchErr error) { | |||
| 212 | + | status, ok := appport.FetchStatusCode(fetchErr) | |||
| 213 | + | if !ok || (status != 404 && status != 410) { | |||
| 214 | + | return | |||
| 215 | + | } | |||
| 216 | + | uc.shared.mu.Lock() | |||
| 217 | + | defer uc.shared.mu.Unlock() | |||
| 218 | + | if _, exists := uc.shared.misses[key]; !exists { | |||
| 219 | + | for len(uc.shared.misses) >= maxMissEntries { | |||
| 220 | + | uc.evictMissLocked() | |||
| 221 | + | } | |||
| 222 | + | uc.shared.order = append(uc.shared.order, key) | |||
| 223 | + | } | |||
| 224 | + | uc.shared.misses[key] = refreshMiss{expiresAt: uc.now().Add(faviconMissTTL)} | |||
| 225 | + | // Expired names linger in order until evicted; compact before the | |||
| 226 | + | // backlog can grow without bound. | |||
| 227 | + | if len(uc.shared.order) > 4*maxMissEntries { | |||
| 228 | + | uc.compactMissOrderLocked() | |||
| 229 | + | } | |||
| 230 | + | } | |||
| 231 | + | ||||
| 232 | + | // compactMissOrderLocked drops order names with no live entry and collapses | |||
| 233 | + | // duplicates to one entry per live key, keeping the newest occurrence. | |||
| 234 | + | // Eviction removes entries from the map but not from order, so an evicted | |||
| 235 | + | // key that is reinserted would otherwise leave a stale duplicate behind: | |||
| 236 | + | // repeated cycles grow order without bound and let an old duplicate evict | |||
| 237 | + | // the live entry early. Caller must hold refresh.mu. | |||
| 238 | + | func (uc *FaviconUseCase) compactMissOrderLocked() { | |||
| 239 | + | last := make(map[string]int, len(uc.shared.misses)) | |||
| 240 | + | for i, key := range uc.shared.order { | |||
| 241 | + | if _, ok := uc.shared.misses[key]; ok { | |||
| 242 | + | last[key] = i | |||
| 243 | + | } | |||
| 244 | + | } | |||
| 245 | + | ordered := make([]string, 0, len(last)) | |||
| 246 | + | for key := range last { | |||
| 247 | + | ordered = append(ordered, key) | |||
| 248 | + | } | |||
| 249 | + | slices.SortFunc(ordered, func(a, b string) int { return last[a] - last[b] }) | |||
| 250 | + | for i := len(ordered); i < len(uc.shared.order); i++ { | |||
| 251 | + | uc.shared.order[i] = "" | |||
| 252 | + | } | |||
| 253 | + | uc.shared.order = append(uc.shared.order[:0], ordered...) | |||
| 254 | + | } | |||
| 255 | + | ||||
| 256 | + | // evictMissLocked removes one entry: expired first, oldest otherwise. | |||
| 257 | + | // Caller must hold refresh.mu. | |||
| 258 | + | func (uc *FaviconUseCase) evictMissLocked() { | |||
| 259 | + | now := uc.now() | |||
| 260 | + | for key, miss := range uc.shared.misses { | |||
| 261 | + | if !now.Before(miss.expiresAt) { | |||
| 262 | + | delete(uc.shared.misses, key) | |||
| 263 | + | return | |||
| 264 | + | } | |||
| 265 | + | } | |||
| 266 | + | for len(uc.shared.order) > 0 { | |||
| 267 | + | oldest := uc.shared.order[0] | |||
| 268 | + | uc.shared.order = uc.shared.order[1:] | |||
| 269 | + | if _, ok := uc.shared.misses[oldest]; ok { | |||
| 270 | + | delete(uc.shared.misses, oldest) | |||
| 271 | + | return | |||
| 272 | + | } | |||
| 273 | + | } | |||
| 274 | + | for key := range uc.shared.misses { | |||
| 275 | + | delete(uc.shared.misses, key) | |||
| 276 | + | return | |||
| 277 | + | } | |||
| 278 | + | } | |||
| 279 | + | ||||
| 280 | + | // joinRefreshOp shares equivalent in-flight work identified by key. The | |||
| 281 | + | // first waiter executes exec under a whole-operation deadline derived from | |||
| 282 | + | // the owned background context; joiners observe the result. The creating | |||
| 283 | + | // waiter also advances the invalidation epoch, so concurrent joiners never | |||
| 284 | + | // invalidate the operation they joined. A canceled waiter leaves without | |||
| 285 | + | // affecting others; the operation is canceled atomically when the last | |||
| 286 | + | // waiter leaves, and late joiners start a fresh operation instead of | |||
| 287 | + | // observing a dying call. When the admission budget is exhausted, or the | |||
| 288 | + | // coordinator is closed, direct callers receive ErrFaviconBusy or | |||
| 289 | + | // ErrFaviconShutdown respectively (never false success). | |||
| 290 | + | func (uc *FaviconUseCase) joinRefreshOp(ctx context.Context, key string, exec func(opCtx context.Context, startEpoch uint64) error) error { | |||
| 291 | + | uc.shared.mu.Lock() | |||
| 292 | + | if uc.shared.closed { | |||
| 293 | + | uc.shared.mu.Unlock() | |||
| 294 | + | return appport.ErrFaviconShutdown | |||
| 295 | + | } | |||
| 296 | + | if call, ok := uc.shared.active[key]; ok && !call.closing { | |||
| 297 | + | call.waiters++ | |||
| 298 | + | uc.shared.mu.Unlock() | |||
| 299 | + | return uc.awaitRefreshCall(ctx, call) | |||
| 300 | + | } | |||
| 301 | + | select { | |||
| 302 | + | case uc.shared.slots <- struct{}{}: | |||
| 303 | + | default: | |||
| 304 | + | uc.shared.mu.Unlock() | |||
| 305 | + | return appport.ErrFaviconBusy | |||
| 306 | + | } | |||
| 307 | + | opCtx, cancel := context.WithTimeout(uc.background, refreshOpTimeout) | |||
| 308 | + | // The creator advances the epoch with creation itself: older | |||
| 309 | + | // completions cannot repopulate entries this operation supersedes, | |||
| 310 | + | // and joiners share the creator's epoch instead of invalidating it. | |||
| 311 | + | uc.shared.epoch++ | |||
| 312 | + | startEpoch := uc.shared.epoch | |||
| 313 | + | call := &refreshCall{done: make(chan struct{}), waiters: 1, cancel: cancel} | |||
| 314 | + | uc.shared.active[key] = call | |||
| 315 | + | uc.shared.wg.Add(1) | |||
| 316 | + | uc.shared.mu.Unlock() | |||
| 317 | + | ||||
| 318 | + | go func() { | |||
| 319 | + | defer uc.shared.wg.Done() | |||
| 320 | + | err := exec(opCtx, startEpoch) | |||
| 321 | + | <-uc.shared.slots | |||
| 322 | + | cancel() | |||
| 323 | + | uc.shared.mu.Lock() | |||
| 324 | + | call.err = err | |||
| 325 | + | if uc.shared.active[key] == call { | |||
| 326 | + | delete(uc.shared.active, key) | |||
| 327 | + | } | |||
| 328 | + | uc.shared.mu.Unlock() | |||
| 329 | + | close(call.done) | |||
| 330 | + | }() | |||
| 331 | + | ||||
| 332 | + | return uc.awaitRefreshCall(ctx, call) | |||
| 333 | + | } | |||
| 334 | + | ||||
| 335 | + | // awaitRefreshCall waits for a shared operation or the waiter's own | |||
| 336 | + | // cancellation, releasing the waiter slot exactly once. The last waiter to | |||
| 337 | + | // leave marks the call closing and cancels it atomically under the lock: | |||
| 338 | + | // context cancellation only signals, so invoking it while holding the | |||
| 339 | + | // coordinator mutex cannot block. | |||
| 340 | + | func (uc *FaviconUseCase) awaitRefreshCall(ctx context.Context, call *refreshCall) error { | |||
| 341 | + | select { | |||
| 342 | + | case <-call.done: | |||
| 343 | + | uc.shared.mu.Lock() | |||
| 344 | + | err := call.err | |||
| 345 | + | uc.shared.mu.Unlock() | |||
| 346 | + | return err | |||
| 347 | + | case <-ctx.Done(): | |||
| 348 | + | uc.shared.mu.Lock() | |||
| 349 | + | call.waiters-- | |||
| 350 | + | if call.waiters <= 0 && !call.closing { | |||
| 351 | + | call.closing = true | |||
| 352 | + | call.cancel() | |||
| 353 | + | } | |||
| 354 | + | uc.shared.mu.Unlock() | |||
| 355 | + | return ctx.Err() | |||
| 356 | + | } | |||
| 357 | + | } | |||
| 358 | + | ||||
| 359 | + | // drainRefreshOps waits until every spawned executor finished, up to a | |||
| 360 | + | // bounded multiple of the operation deadline. It waits on the executor | |||
| 361 | + | // wait group rather than the active map: a replaced | |||
| 362 | + | // canceled-but-still-running call no longer occupies its map entry, but | |||
| 363 | + | // its executor may still touch repositories. Callers cancel first (see | |||
| 364 | + | // Close); cooperative operations observe the background cancellation and | |||
| 365 | + | // finish promptly, so hitting the bound means stuck work that shutdown | |||
| 366 | + | // must not wait out. | |||
| 367 | + | func (uc *FaviconUseCase) drainRefreshOps() { | |||
| 368 | + | done := make(chan struct{}) | |||
| 369 | + | go func() { | |||
| 370 | + | uc.shared.wg.Wait() | |||
| 371 | + | close(done) | |||
| 372 | + | }() | |||
| 373 | + | deadline := time.NewTimer(3 * refreshOpTimeout) | |||
| 374 | + | defer deadline.Stop() | |||
| 375 | + | select { | |||
| 376 | + | case <-done: | |||
| 377 | + | case <-deadline.C: | |||
| 378 | + | } | |||
| 379 | + | } | |||
A internal/application/usecase/favicon_refresh_test.go +798 -0
| @@ -0,0 +1,798 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "fmt" | |
| 6 | + | "sync" | |
| 7 | + | "sync/atomic" | |
| 8 | + | "testing" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | appport "github.com/bnema/dumber/internal/application/port" | |
| 12 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 13 | + | "github.com/bnema/dumber/internal/domain/favicon" | |
| 14 | + | "github.com/stretchr/testify/mock" | |
| 15 | + | "github.com/stretchr/testify/require" | |
| 16 | + | ) | |
| 17 | + | ||
| 18 | + | // refreshTestUC builds a FaviconUseCase with a custom fetcher and | |
| 19 | + | // controllable clock for refresh-coordinator tests. | |
| 20 | + | func refreshTestUC(t *testing.T, fetcher appport.FaviconFetcher, now *time.Time, background context.Context) *FaviconUseCase { | |
| 21 | + | t.Helper() | |
| 22 | + | if background == nil { | |
| 23 | + | background = context.Background() | |
| 24 | + | } | |
| 25 | + | nowFn := time.Now | |
| 26 | + | if now != nil { | |
| 27 | + | nowFn = func() time.Time { return *now } | |
| 28 | + | } | |
| 29 | + | return NewFaviconUseCase(FaviconDeps{ | |
| 30 | + | Repository: mockFaviconRepository(t, newFaviconRepoState()), | |
| 31 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |
| 32 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |
| 33 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |
| 34 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |
| 35 | + | Fetcher: fetcher, | |
| 36 | + | Now: nowFn, | |
| 37 | + | Background: background, | |
| 38 | + | }) | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | func refreshFetcherMock(t *testing.T, fetch func(context.Context, appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error)) *portmocks.MockFaviconFetcher { | |
| 42 | + | t.Helper() | |
| 43 | + | fetcher := portmocks.NewMockFaviconFetcher(t) | |
| 44 | + | fetcher.EXPECT().Fetch(mock.Anything, mock.Anything).RunAndReturn(fetch).Maybe() | |
| 45 | + | return fetcher | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | func fetchedIcon(iconURL string) *appport.FaviconFetchedIcon { | |
| 49 | + | return &appport.FaviconFetchedIcon{ | |
| 50 | + | IconURL: iconURL, | |
| 51 | + | Bytes: []byte("icon-bytes"), | |
| 52 | + | ContentType: "image/png", | |
| 53 | + | } | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | // TestRefreshFromIconURLs_ConcurrentIdenticalSharesOneFetch reproduces the | |
| 57 | + | // P1.1 bypass: concurrent identical candidate requests must share a single | |
| 58 | + | // fetch instead of issuing one fetch per caller. | |
| 59 | + | func TestRefreshFromIconURLs_ConcurrentIdenticalSharesOneFetch(t *testing.T) { | |
| 60 | + | var calls atomic.Int64 | |
| 61 | + | entered := make(chan struct{}, 8) | |
| 62 | + | release := make(chan struct{}) | |
| 63 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 64 | + | calls.Add(1) | |
| 65 | + | entered <- struct{}{} | |
| 66 | + | <-release | |
| 67 | + | return fetchedIcon(req.IconURL), nil | |
| 68 | + | }) | |
| 69 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 70 | + | ||
| 71 | + | const waiters = 5 | |
| 72 | + | page := "https://example.com/a" | |
| 73 | + | candidates := []string{"https://example.com/favicon.ico"} | |
| 74 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 75 | + | require.True(t, ok) | |
| 76 | + | results := make(chan error, waiters) | |
| 77 | + | var wg sync.WaitGroup | |
| 78 | + | for range waiters { | |
| 79 | + | wg.Add(1) | |
| 80 | + | go func() { | |
| 81 | + | defer wg.Done() | |
| 82 | + | results <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 83 | + | }() | |
| 84 | + | } | |
| 85 | + | // All five waiters must join the single shared operation before the | |
| 86 | + | // fetch completes; the waiter count is the deterministic join signal. | |
| 87 | + | require.Eventually(t, func() bool { | |
| 88 | + | uc.shared.mu.Lock() | |
| 89 | + | defer uc.shared.mu.Unlock() | |
| 90 | + | call, ok := uc.shared.active[key] | |
| 91 | + | return ok && call.waiters == waiters | |
| 92 | + | }, 10*time.Second, 5*time.Millisecond, "waiters did not join the shared operation") | |
| 93 | + | close(release) | |
| 94 | + | wg.Wait() | |
| 95 | + | close(results) | |
| 96 | + | for err := range results { | |
| 97 | + | require.NoError(t, err) | |
| 98 | + | } | |
| 99 | + | require.EqualValues(t, 1, calls.Load(), "identical concurrent refreshes must share one fetch") | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | // TestRefreshFromIconURLs_ChangingCandidatesDoNotShare verifies that | |
| 103 | + | // different candidate lists are distinct operations. | |
| 104 | + | func TestRefreshFromIconURLs_ChangingCandidatesDoNotShare(t *testing.T) { | |
| 105 | + | var calls atomic.Int64 | |
| 106 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 107 | + | calls.Add(1) | |
| 108 | + | return fetchedIcon(req.IconURL), nil | |
| 109 | + | }) | |
| 110 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 111 | + | ||
| 112 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/one.ico"})) | |
| 113 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/two.ico"})) | |
| 114 | + | require.EqualValues(t, 2, calls.Load()) | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | // TestRefreshFromIconURLs_CancelOneWaiterKeepsOthers proves waiter | |
| 118 | + | // independence: a canceled waiter leaves while the shared operation serves | |
| 119 | + | // the rest. | |
| 120 | + | func TestRefreshFromIconURLs_CancelOneWaiterKeepsOthers(t *testing.T) { | |
| 121 | + | release := make(chan struct{}) | |
| 122 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 123 | + | <-release | |
| 124 | + | return fetchedIcon(req.IconURL), nil | |
| 125 | + | }) | |
| 126 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 127 | + | ||
| 128 | + | page := "https://example.com/a" | |
| 129 | + | candidates := []string{"https://example.com/favicon.ico"} | |
| 130 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 131 | + | require.True(t, ok, "test page and candidates must form a refresh key") | |
| 132 | + | leaving, cancelLeaving := context.WithCancel(context.Background()) | |
| 133 | + | left := make(chan error, 1) | |
| 134 | + | go func() { | |
| 135 | + | left <- uc.RefreshFromIconURLs(leaving, page, candidates) | |
| 136 | + | }() | |
| 137 | + | staying := make(chan error, 1) | |
| 138 | + | go func() { | |
| 139 | + | staying <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 140 | + | }() | |
| 141 | + | // Both waiters must share one operation before either leaves: two | |
| 142 | + | // waiters on the same call proves sharing, not two separate fetches. | |
| 143 | + | require.Eventually(t, func() bool { | |
| 144 | + | uc.shared.mu.Lock() | |
| 145 | + | defer uc.shared.mu.Unlock() | |
| 146 | + | call, ok := uc.shared.active[key] | |
| 147 | + | return ok && !call.closing && call.waiters == 2 | |
| 148 | + | }, 10*time.Second, 5*time.Millisecond, "both waiters must join one shared operation") | |
| 149 | + | cancelLeaving() | |
| 150 | + | select { | |
| 151 | + | case err := <-left: | |
| 152 | + | require.ErrorIs(t, err, context.Canceled) | |
| 153 | + | case <-time.After(10 * time.Second): | |
| 154 | + | t.Fatal("canceled waiter did not return") | |
| 155 | + | } | |
| 156 | + | close(release) | |
| 157 | + | select { | |
| 158 | + | case err := <-staying: | |
| 159 | + | require.NoError(t, err, "remaining waiter must still be served") | |
| 160 | + | case <-time.After(10 * time.Second): | |
| 161 | + | t.Fatal("remaining waiter was dropped with the canceled one") | |
| 162 | + | } | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | // TestRefreshFromIconURLs_InvalidPageHierarchyRejected verifies invalid | |
| 166 | + | // origins never reach the fetcher. | |
| 167 | + | func TestRefreshFromIconURLs_InvalidPageHierarchyRejected(t *testing.T) { | |
| 168 | + | var calls atomic.Int64 | |
| 169 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 170 | + | calls.Add(1) | |
| 171 | + | return fetchedIcon(req.IconURL), nil | |
| 172 | + | }) | |
| 173 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 174 | + | ||
| 175 | + | for _, page := range []string{"", "not a url", "ftp://example.com/a", "dumb://internal"} { | |
| 176 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), page, []string{"https://example.com/favicon.ico"}), ErrFaviconMiss) | |
| 177 | + | } | |
| 178 | + | require.Zero(t, calls.Load()) | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | // TestRefreshRequestKey_Distinctions covers origin and candidate | |
| 182 | + | // significance: scheme/port/query matter, fragments/case/default ports do | |
| 183 | + | // not, relative candidates resolve against the page, over-long and excess | |
| 184 | + | // candidates are skipped. | |
| 185 | + | func TestRefreshRequestKey_Distinctions(t *testing.T) { | |
| 186 | + | page := "https://example.com:8443/a?x=1" | |
| 187 | + | keyOf := func(candidates ...string) (string, bool) { | |
| 188 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 189 | + | return key, ok | |
| 190 | + | } | |
| 191 | + | base, ok := keyOf("https://example.com:8443/icon.png") | |
| 192 | + | require.True(t, ok) | |
| 193 | + | ||
| 194 | + | other, _ := keyOf("https://example.com:8443/other.png") | |
| 195 | + | require.NotEqual(t, base, other, "different candidates must not share") | |
| 196 | + | ||
| 197 | + | portKey, _ := keyOf("https://example.com:9443/icon.png") | |
| 198 | + | require.NotEqual(t, base, portKey, "ports are significant") | |
| 199 | + | ||
| 200 | + | queryKey, _ := keyOf("https://example.com:8443/icon.png?v=2") | |
| 201 | + | require.NotEqual(t, base, queryKey, "queries are significant") | |
| 202 | + | ||
| 203 | + | fragKey, _ := keyOf("https://example.com:8443/icon.png#frag") | |
| 204 | + | require.Equal(t, base, fragKey, "fragments must not split sharing") | |
| 205 | + | ||
| 206 | + | caseKey, _ := keyOf("HTTPS://EXAMPLE.COM:8443/icon.png") | |
| 207 | + | require.Equal(t, base, caseKey, "scheme/host case must not split sharing") | |
| 208 | + | ||
| 209 | + | defaultPort, _ := keyOf("https://example.com:443/icon.png") | |
| 210 | + | otherPage, _, ok := refreshRequestKey("https://example.com/icon", []string{"https://example.com:443/icon.png"}) | |
| 211 | + | require.True(t, ok) | |
| 212 | + | require.NotEqual(t, defaultPort, otherPage, "page origin is part of the identity") | |
| 213 | + | ||
| 214 | + | relKey, relCandidates, ok := refreshRequestKey("https://example.com/a", []string{"/icon.png"}) | |
| 215 | + | require.True(t, ok) | |
| 216 | + | require.Equal(t, []string{"https://example.com/icon.png"}, relCandidates, "relative candidates resolve against the page") | |
| 217 | + | require.NotEqual(t, relKey, base, "different pages must not share") | |
| 218 | + | ||
| 219 | + | long := "https://example.com/" + string(make([]byte, maxRefreshCandidateLen)) | |
| 220 | + | _, kept, ok := refreshRequestKey(page, []string{long, "https://example.com:8443/icon.png"}) | |
| 221 | + | require.True(t, ok) | |
| 222 | + | require.Len(t, kept, 1, "over-long candidates are skipped") | |
| 223 | + | ||
| 224 | + | many := make([]string, maxRefreshCandidates+4) | |
| 225 | + | for i := range many { | |
| 226 | + | many[i] = "https://example.com:8443/icon.png" | |
| 227 | + | } | |
| 228 | + | _, kept, ok = refreshRequestKey(page, many) | |
| 229 | + | require.True(t, ok) | |
| 230 | + | require.Len(t, kept, maxRefreshCandidates, "candidates are capped") | |
| 231 | + | ||
| 232 | + | _, _, ok = refreshRequestKey("not a url", []string{"https://example.com/icon.png"}) | |
| 233 | + | require.False(t, ok, "invalid page hierarchy is rejected") | |
| 234 | + | } | |
| 235 | + | ||
| 236 | + | // TestRefreshAdmission_BusyWhenExhausted fills the shared budget with | |
| 237 | + | // blocked operations and proves the next direct caller gets a distinct | |
| 238 | + | // busy outcome instead of false success. | |
| 239 | + | func TestRefreshAdmission_BusyWhenExhausted(t *testing.T) { | |
| 240 | + | release := make(chan struct{}) | |
| 241 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 242 | + | <-release | |
| 243 | + | return fetchedIcon(req.IconURL), nil | |
| 244 | + | }) | |
| 245 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 246 | + | ||
| 247 | + | var wg sync.WaitGroup | |
| 248 | + | for i := range maxActiveRefreshes { | |
| 249 | + | wg.Add(1) | |
| 250 | + | go func(i int) { | |
| 251 | + | defer wg.Done() | |
| 252 | + | // Distinct candidates per holder fill distinct budget slots; | |
| 253 | + | // identical requests would share one. | |
| 254 | + | icon := fmt.Sprintf("https://example.com/icon-%d.png", i) | |
| 255 | + | _ = uc.RefreshFromIconURLs(context.Background(), "https://example.com/page", []string{icon}) | |
| 256 | + | }(i) | |
| 257 | + | } | |
| 258 | + | require.Eventually(t, func() bool { | |
| 259 | + | uc.shared.mu.Lock() | |
| 260 | + | defer uc.shared.mu.Unlock() | |
| 261 | + | return len(uc.shared.active) == maxActiveRefreshes | |
| 262 | + | }, 10*time.Second, 5*time.Millisecond, "budget must fill") | |
| 263 | + | ||
| 264 | + | err := uc.RefreshFromIconURLs(context.Background(), "https://other.example/", []string{"https://other.example/icon.png"}) | |
| 265 | + | require.ErrorIs(t, err, appport.ErrFaviconBusy) | |
| 266 | + | require.NotErrorIs(t, err, ErrFaviconMiss, "overload must stay distinct from a miss") | |
| 267 | + | ||
| 268 | + | close(release) | |
| 269 | + | wg.Wait() | |
| 270 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://other.example/", []string{"https://other.example/icon.png"})) | |
| 271 | + | } | |
| 272 | + | ||
| 273 | + | // TestRefreshOp_WholeDeadline verifies the shared operation carries the | |
| 274 | + | // bounded whole-operation deadline without waiting it out. | |
| 275 | + | func TestRefreshOp_WholeDeadline(t *testing.T) { | |
| 276 | + | deadlines := make(chan time.Time, 1) | |
| 277 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 278 | + | if deadline, ok := ctx.Deadline(); ok { | |
| 279 | + | deadlines <- deadline | |
| 280 | + | } | |
| 281 | + | return fetchedIcon(req.IconURL), nil | |
| 282 | + | }) | |
| 283 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 284 | + | ||
| 285 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/favicon.ico"})) | |
| 286 | + | select { | |
| 287 | + | case deadline := <-deadlines: | |
| 288 | + | remaining := time.Until(deadline) | |
| 289 | + | require.Greater(t, remaining, 4*time.Second) | |
| 290 | + | require.LessOrEqual(t, remaining, refreshOpTimeout) | |
| 291 | + | case <-time.After(10 * time.Second): | |
| 292 | + | t.Fatal("fetch did not observe the operation deadline") | |
| 293 | + | } | |
| 294 | + | } | |
| 295 | + | ||
| 296 | + | // TestRefreshLastWaiterCancelAbortsOp proves a lone waiter that leaves | |
| 297 | + | // cancels the shared operation instead of leaking it. | |
| 298 | + | func TestRefreshLastWaiterCancelAbortsOp(t *testing.T) { | |
| 299 | + | observedCancel := make(chan struct{}, 1) | |
| 300 | + | entered := make(chan struct{}, 1) | |
| 301 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 302 | + | select { | |
| 303 | + | case entered <- struct{}{}: | |
| 304 | + | default: | |
| 305 | + | } | |
| 306 | + | select { | |
| 307 | + | case <-ctx.Done(): | |
| 308 | + | observedCancel <- struct{}{} | |
| 309 | + | return nil, ctx.Err() | |
| 310 | + | case <-time.After(10 * time.Second): | |
| 311 | + | return fetchedIcon(req.IconURL), nil | |
| 312 | + | } | |
| 313 | + | }) | |
| 314 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 315 | + | ||
| 316 | + | waiter, cancel := context.WithCancel(context.Background()) | |
| 317 | + | done := make(chan error, 1) | |
| 318 | + | go func() { | |
| 319 | + | done <- uc.RefreshFromIconURLs(waiter, "https://example.com/a", []string{"https://example.com/favicon.ico"}) | |
| 320 | + | }() | |
| 321 | + | // Cancel only after the executor entered the fetch: len(active) == 1 | |
| 322 | + | // alone cannot prove the executor is blocked inside Fetch, and an early | |
| 323 | + | // cancel could race operation creation instead of aborting shared work. | |
| 324 | + | select { | |
| 325 | + | case <-entered: | |
| 326 | + | case <-time.After(10 * time.Second): | |
| 327 | + | t.Fatal("executor did not enter the fetch") | |
| 328 | + | } | |
| 329 | + | cancel() | |
| 330 | + | select { | |
| 331 | + | case err := <-done: | |
| 332 | + | require.ErrorIs(t, err, context.Canceled) | |
| 333 | + | case <-time.After(10 * time.Second): | |
| 334 | + | t.Fatal("canceled lone waiter did not return") | |
| 335 | + | } | |
| 336 | + | select { | |
| 337 | + | case <-observedCancel: | |
| 338 | + | case <-time.After(10 * time.Second): | |
| 339 | + | t.Fatal("operation was not canceled with its last waiter") | |
| 340 | + | } | |
| 341 | + | require.Eventually(t, func() bool { | |
| 342 | + | uc.shared.mu.Lock() | |
| 343 | + | defer uc.shared.mu.Unlock() | |
| 344 | + | return len(uc.shared.active) == 0 | |
| 345 | + | }, 10*time.Second, 5*time.Millisecond, "canceled operation must release its slot") | |
| 346 | + | } | |
| 347 | + | ||
| 348 | + | // TestFaviconUseCase_CloseCancelsAndDrains proves shutdown wiring: Close | |
| 349 | + | // aborts owned background work and returns once nothing is active. | |
| 350 | + | func TestFaviconUseCase_CloseCancelsAndDrains(t *testing.T) { | |
| 351 | + | background, cancel := context.WithCancel(context.Background()) | |
| 352 | + | defer cancel() | |
| 353 | + | sawCancel := make(chan struct{}, 1) | |
| 354 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 355 | + | select { | |
| 356 | + | case <-ctx.Done(): | |
| 357 | + | sawCancel <- struct{}{} | |
| 358 | + | return nil, ctx.Err() | |
| 359 | + | case <-time.After(10 * time.Second): | |
| 360 | + | return fetchedIcon(req.IconURL), nil | |
| 361 | + | } | |
| 362 | + | }) | |
| 363 | + | uc := refreshTestUC(t, fetcher, nil, background) | |
| 364 | + | ||
| 365 | + | done := make(chan error, 1) | |
| 366 | + | go func() { | |
| 367 | + | done <- uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/favicon.ico"}) | |
| 368 | + | }() | |
| 369 | + | require.Eventually(t, func() bool { | |
| 370 | + | uc.shared.mu.Lock() | |
| 371 | + | defer uc.shared.mu.Unlock() | |
| 372 | + | return len(uc.shared.active) == 1 | |
| 373 | + | }, 10*time.Second, 5*time.Millisecond) | |
| 374 | + | ||
| 375 | + | closed := make(chan struct{}) | |
| 376 | + | go func() { | |
| 377 | + | uc.Close() | |
| 378 | + | close(closed) | |
| 379 | + | }() | |
| 380 | + | select { | |
| 381 | + | case <-sawCancel: | |
| 382 | + | case <-time.After(10 * time.Second): | |
| 383 | + | t.Fatal("Close did not cancel background work") | |
| 384 | + | } | |
| 385 | + | select { | |
| 386 | + | case <-closed: | |
| 387 | + | case <-time.After(10 * time.Second): | |
| 388 | + | t.Fatal("Close did not drain") | |
| 389 | + | } | |
| 390 | + | select { | |
| 391 | + | case err := <-done: | |
| 392 | + | require.Error(t, err) | |
| 393 | + | case <-time.After(10 * time.Second): | |
| 394 | + | t.Fatal("refresh caller did not return after Close") | |
| 395 | + | } | |
| 396 | + | // Repeat Close must be safe (idempotent, no error to report). | |
| 397 | + | uc.Close() | |
| 398 | + | } | |
| 399 | + | ||
| 400 | + | // TestRefreshMissClassification_HTTPClasses proves only genuine absence | |
| 401 | + | // (404/410) is negatively cached: auth/transient failures refetch, and a | |
| 402 | + | // repeated miss within TTL issues no fetch. | |
| 403 | + | func TestRefreshMissClassification_HTTPClasses(t *testing.T) { | |
| 404 | + | now := time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC) | |
| 405 | + | statusByURL := map[string]int{ | |
| 406 | + | "https://example.com/gone.ico": 404, | |
| 407 | + | "https://example.com/removed.ico": 410, | |
| 408 | + | "https://example.com/denied.ico": 403, | |
| 409 | + | "https://example.com/login.ico": 401, | |
| 410 | + | "https://example.com/broken.ico": 500, | |
| 411 | + | "https://example.com/unstable.ico": 503, | |
| 412 | + | "https://example.com/validation.ico": 0, | |
| 413 | + | } | |
| 414 | + | var calls atomic.Int64 | |
| 415 | + | var candidateCalls atomic.Int64 | |
| 416 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 417 | + | calls.Add(1) | |
| 418 | + | // The all-miss fallback runs a candidate-less discovery fetch; | |
| 419 | + | // assertions below count candidate batches only. | |
| 420 | + | if req.IconURL != "" { | |
| 421 | + | candidateCalls.Add(1) | |
| 422 | + | } | |
| 423 | + | status := statusByURL[req.IconURL] | |
| 424 | + | if status == 0 { | |
| 425 | + | return nil, appport.ErrFaviconMiss | |
| 426 | + | } | |
| 427 | + | return nil, &appport.FaviconFetchError{StatusCode: status} | |
| 428 | + | }) | |
| 429 | + | uc := refreshTestUC(t, fetcher, &now, nil) | |
| 430 | + | page := "https://example.com/a" | |
| 431 | + | ||
| 432 | + | refresh := func(icon string) error { | |
| 433 | + | return uc.RefreshFromIconURLs(context.Background(), page, []string{icon}) | |
| 434 | + | } | |
| 435 | + | for icon := range statusByURL { | |
| 436 | + | err := refresh(icon) | |
| 437 | + | require.ErrorIs(t, err, ErrFaviconMiss, "every class must preserve miss matching for %s", icon) | |
| 438 | + | } | |
| 439 | + | firstCalls := candidateCalls.Load() | |
| 440 | + | require.EqualValues(t, len(statusByURL), firstCalls) | |
| 441 | + | ||
| 442 | + | // Genuine absence is cached: 404/410 refetch nothing within TTL. | |
| 443 | + | require.ErrorIs(t, refresh("https://example.com/gone.ico"), ErrFaviconMiss) | |
| 444 | + | require.ErrorIs(t, refresh("https://example.com/removed.ico"), ErrFaviconMiss) | |
| 445 | + | require.Equal(t, firstCalls, candidateCalls.Load(), "cached genuine absence must not refetch") | |
| 446 | + | ||
| 447 | + | // Auth/transient/validation failures are never cached: they refetch. | |
| 448 | + | require.ErrorIs(t, refresh("https://example.com/denied.ico"), ErrFaviconMiss) | |
| 449 | + | require.ErrorIs(t, refresh("https://example.com/broken.ico"), ErrFaviconMiss) | |
| 450 | + | require.ErrorIs(t, refresh("https://example.com/validation.ico"), ErrFaviconMiss) | |
| 451 | + | require.Equal(t, firstCalls+3, candidateCalls.Load()) | |
| 452 | + | ||
| 453 | + | // After TTL expiry the genuine absence is eligible again. | |
| 454 | + | now = now.Add(faviconMissTTL + time.Second) | |
| 455 | + | require.ErrorIs(t, refresh("https://example.com/gone.ico"), ErrFaviconMiss) | |
| 456 | + | require.Equal(t, firstCalls+4, candidateCalls.Load()) | |
| 457 | + | } | |
| 458 | + | ||
| 459 | + | // TestRefreshInvalidate_DropsStaleCompletion proves the epoch rule: an | |
| 460 | + | // invalidation racing an in-flight fetch prevents the stale bytes from | |
| 461 | + | // being stored. | |
| 462 | + | func TestRefreshInvalidate_DropsStaleCompletion(t *testing.T) { | |
| 463 | + | release := make(chan struct{}) | |
| 464 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 465 | + | <-release | |
| 466 | + | return fetchedIcon(req.IconURL), nil | |
| 467 | + | }) | |
| 468 | + | repo := newFaviconRepoState() | |
| 469 | + | uc := NewFaviconUseCase(FaviconDeps{ | |
| 470 | + | Repository: mockFaviconRepository(t, repo), | |
| 471 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |
| 472 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |
| 473 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |
| 474 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |
| 475 | + | Fetcher: fetcher, | |
| 476 | + | Now: time.Now, | |
| 477 | + | Background: context.Background(), | |
| 478 | + | }) | |
| 479 | + | ||
| 480 | + | page := "https://example.com/a" | |
| 481 | + | done := make(chan error, 1) | |
| 482 | + | go func() { | |
| 483 | + | done <- uc.RefreshFromIconURLs(context.Background(), page, []string{"https://example.com/favicon.ico"}) | |
| 484 | + | }() | |
| 485 | + | require.Eventually(t, func() bool { | |
| 486 | + | uc.shared.mu.Lock() | |
| 487 | + | defer uc.shared.mu.Unlock() | |
| 488 | + | return len(uc.shared.active) == 1 | |
| 489 | + | }, 10*time.Second, 5*time.Millisecond) | |
| 490 | + | ||
| 491 | + | key, ok := favicon.CanonicalKey(page) | |
| 492 | + | require.True(t, ok) | |
| 493 | + | require.NoError(t, uc.Invalidate(context.Background(), key)) | |
| 494 | + | close(release) | |
| 495 | + | select { | |
| 496 | + | case <-done: | |
| 497 | + | case <-time.After(10 * time.Second): | |
| 498 | + | t.Fatal("refresh did not complete after release") | |
| 499 | + | } | |
| 500 | + | require.Empty(t, repo.byKey, "stale completion must not repopulate invalidated entries") | |
| 501 | + | } | |
| 502 | + | ||
| 503 | + | // TestRefreshJoinerSharesCreatorEpoch proves concurrent joiners never | |
| 504 | + | // invalidate the operation they joined: overlapping identical refreshes | |
| 505 | + | // both succeed and the fetched icon is stored. | |
| 506 | + | func TestRefreshJoinerSharesCreatorEpoch(t *testing.T) { | |
| 507 | + | release := make(chan struct{}) | |
| 508 | + | var calls atomic.Int64 | |
| 509 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 510 | + | calls.Add(1) | |
| 511 | + | <-release | |
| 512 | + | return fetchedIcon(req.IconURL), nil | |
| 513 | + | }) | |
| 514 | + | repo := newFaviconRepoState() | |
| 515 | + | uc := NewFaviconUseCase(FaviconDeps{ | |
| 516 | + | Repository: mockFaviconRepository(t, repo), | |
| 517 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |
| 518 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |
| 519 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |
| 520 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |
| 521 | + | Fetcher: fetcher, | |
| 522 | + | Now: time.Now, | |
| 523 | + | Background: context.Background(), | |
| 524 | + | }) | |
| 525 | + | ||
| 526 | + | page := "https://example.com/a" | |
| 527 | + | candidates := []string{"https://example.com/favicon.ico"} | |
| 528 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 529 | + | require.True(t, ok) | |
| 530 | + | first := make(chan error, 1) | |
| 531 | + | go func() { | |
| 532 | + | first <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 533 | + | }() | |
| 534 | + | require.Eventually(t, func() bool { | |
| 535 | + | uc.shared.mu.Lock() | |
| 536 | + | defer uc.shared.mu.Unlock() | |
| 537 | + | return len(uc.shared.active) == 1 | |
| 538 | + | }, 10*time.Second, 5*time.Millisecond) | |
| 539 | + | second := make(chan error, 1) | |
| 540 | + | go func() { | |
| 541 | + | second <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 542 | + | }() | |
| 543 | + | require.Eventually(t, func() bool { | |
| 544 | + | uc.shared.mu.Lock() | |
| 545 | + | defer uc.shared.mu.Unlock() | |
| 546 | + | call, ok := uc.shared.active[key] | |
| 547 | + | return ok && call.waiters == 2 | |
| 548 | + | }, 10*time.Second, 5*time.Millisecond, "joiner must share the creator operation") | |
| 549 | + | close(release) | |
| 550 | + | select { | |
| 551 | + | case err := <-first: | |
| 552 | + | require.NoError(t, err) | |
| 553 | + | case <-time.After(10 * time.Second): | |
| 554 | + | t.Fatal("creator did not complete") | |
| 555 | + | } | |
| 556 | + | select { | |
| 557 | + | case err := <-second: | |
| 558 | + | require.NoError(t, err) | |
| 559 | + | case <-time.After(10 * time.Second): | |
| 560 | + | t.Fatal("joiner did not complete") | |
| 561 | + | } | |
| 562 | + | require.EqualValues(t, 1, calls.Load()) | |
| 563 | + | require.NotEmpty(t, repo.byKey, "shared success must be stored, not dropped by a joiner epoch bump") | |
| 564 | + | } | |
| 565 | + | ||
| 566 | + | // TestRefreshPartialMissDoesNotPoisonLaterCandidates proves request-level | |
| 567 | + | // negative caching happens only for fully missed requests: a 404 on the | |
| 568 | + | // first candidate followed by a successful second candidate stores the | |
| 569 | + | // icon and records no miss. | |
| 570 | + | func TestRefreshPartialMissDoesNotPoisonLaterCandidates(t *testing.T) { | |
| 571 | + | var calls atomic.Int64 | |
| 572 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 573 | + | calls.Add(1) | |
| 574 | + | if req.IconURL == "https://example.com/gone.ico" { | |
| 575 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |
| 576 | + | } | |
| 577 | + | return fetchedIcon(req.IconURL), nil | |
| 578 | + | }) | |
| 579 | + | repo := newFaviconRepoState() | |
| 580 | + | uc := NewFaviconUseCase(FaviconDeps{ | |
| 581 | + | Repository: mockFaviconRepository(t, repo), | |
| 582 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |
| 583 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |
| 584 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |
| 585 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |
| 586 | + | Fetcher: fetcher, | |
| 587 | + | Now: time.Now, | |
| 588 | + | Background: context.Background(), | |
| 589 | + | }) | |
| 590 | + | ||
| 591 | + | page := "https://example.com/a" | |
| 592 | + | candidates := []string{"https://example.com/gone.ico", "https://example.com/good.ico"} | |
| 593 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), page, candidates)) | |
| 594 | + | require.NotEmpty(t, repo.byKey, "successful candidate must be stored") | |
| 595 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 596 | + | require.True(t, ok) | |
| 597 | + | require.False(t, uc.cachedMiss(key), "partial miss must not populate the request miss cache") | |
| 598 | + | } | |
| 599 | + | ||
| 600 | + | // TestRefreshCanceledCallNotJoined proves a waiter arriving after the last | |
| 601 | + | // waiter canceled starts a fresh operation instead of observing the dying | |
| 602 | + | // call. | |
| 603 | + | func TestRefreshCanceledCallNotJoined(t *testing.T) { | |
| 604 | + | release := make(chan struct{}) | |
| 605 | + | var calls atomic.Int64 | |
| 606 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 607 | + | calls.Add(1) | |
| 608 | + | select { | |
| 609 | + | case <-release: | |
| 610 | + | return fetchedIcon(req.IconURL), nil | |
| 611 | + | case <-ctx.Done(): | |
| 612 | + | return nil, ctx.Err() | |
| 613 | + | } | |
| 614 | + | }) | |
| 615 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 616 | + | ||
| 617 | + | page := "https://example.com/a" | |
| 618 | + | candidates := []string{"https://example.com/favicon.ico"} | |
| 619 | + | leaving, cancel := context.WithCancel(context.Background()) | |
| 620 | + | left := make(chan error, 1) | |
| 621 | + | go func() { | |
| 622 | + | left <- uc.RefreshFromIconURLs(leaving, page, candidates) | |
| 623 | + | }() | |
| 624 | + | require.Eventually(t, func() bool { | |
| 625 | + | uc.shared.mu.Lock() | |
| 626 | + | defer uc.shared.mu.Unlock() | |
| 627 | + | return len(uc.shared.active) == 1 | |
| 628 | + | }, 10*time.Second, 5*time.Millisecond) | |
| 629 | + | cancel() | |
| 630 | + | select { | |
| 631 | + | case err := <-left: | |
| 632 | + | require.ErrorIs(t, err, context.Canceled) | |
| 633 | + | case <-time.After(10 * time.Second): | |
| 634 | + | t.Fatal("canceled waiter did not return") | |
| 635 | + | } | |
| 636 | + | // The dying call still occupies the map entry until its executor | |
| 637 | + | // finishes; the next waiter must start fresh rather than join it. | |
| 638 | + | fresh := make(chan error, 1) | |
| 639 | + | go func() { | |
| 640 | + | fresh <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 641 | + | }() | |
| 642 | + | require.Eventually(t, func() bool { | |
| 643 | + | return calls.Load() == 2 | |
| 644 | + | }, 10*time.Second, 5*time.Millisecond, "late joiner must start a fresh operation") | |
| 645 | + | close(release) | |
| 646 | + | select { | |
| 647 | + | case err := <-fresh: | |
| 648 | + | require.NoError(t, err) | |
| 649 | + | case <-time.After(10 * time.Second): | |
| 650 | + | t.Fatal("fresh operation did not complete") | |
| 651 | + | } | |
| 652 | + | } | |
| 653 | + | ||
| 654 | + | // TestRefreshCloseSealsAdmission proves no refresh can start after Close | |
| 655 | + | // began: late callers receive a distinct shutdown error instead of racing | |
| 656 | + | // the drain. | |
| 657 | + | func TestRefreshCloseSealsAdmission(t *testing.T) { | |
| 658 | + | uc := refreshTestUC(t, refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 659 | + | return fetchedIcon(req.IconURL), nil | |
| 660 | + | }), nil, nil) | |
| 661 | + | ||
| 662 | + | uc.Close() | |
| 663 | + | err := uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/favicon.ico"}) | |
| 664 | + | require.ErrorIs(t, err, appport.ErrFaviconShutdown) | |
| 665 | + | require.NotErrorIs(t, err, ErrFaviconMiss, "shutdown must stay distinct from a miss") | |
| 666 | + | } | |
| 667 | + | ||
| 668 | + | // TestRefreshMissOrderBounded proves the negative-cache order backlog | |
| 669 | + | // cannot grow without bound under distinct misses. | |
| 670 | + | func TestRefreshMissOrderBounded(t *testing.T) { | |
| 671 | + | now := time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC) | |
| 672 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, _ appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 673 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |
| 674 | + | }) | |
| 675 | + | uc := refreshTestUC(t, fetcher, &now, nil) | |
| 676 | + | ||
| 677 | + | for i := range 3 * maxMissEntries { | |
| 678 | + | icon := "https://example.com/missing-" + string(rune('a'+i%26)) + "-" + string(rune('0'+i/26%10)) + ".ico" | |
| 679 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{icon}), ErrFaviconMiss) | |
| 680 | + | } | |
| 681 | + | uc.shared.mu.Lock() | |
| 682 | + | defer uc.shared.mu.Unlock() | |
| 683 | + | require.LessOrEqual(t, len(uc.shared.misses), maxMissEntries) | |
| 684 | + | require.LessOrEqual(t, len(uc.shared.order), 4*maxMissEntries, "order backlog must stay bounded") | |
| 685 | + | } | |
| 686 | + | ||
| 687 | + | // TestRefreshSharing_ScopedToUseCaseInstance proves profile isolation: | |
| 688 | + | // identical requests on different use-case instances never share fetches. | |
| 689 | + | func TestRefreshSharing_ScopedToUseCaseInstance(t *testing.T) { | |
| 690 | + | var calls atomic.Int64 | |
| 691 | + | newUC := func() *FaviconUseCase { | |
| 692 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 693 | + | calls.Add(1) | |
| 694 | + | return fetchedIcon(req.IconURL), nil | |
| 695 | + | }) | |
| 696 | + | return refreshTestUC(t, fetcher, nil, nil) | |
| 697 | + | } | |
| 698 | + | first, second := newUC(), newUC() | |
| 699 | + | page := "https://example.com/a" | |
| 700 | + | candidates := []string{"https://example.com/favicon.ico"} | |
| 701 | + | var wg sync.WaitGroup | |
| 702 | + | errs := make(chan error, 2) | |
| 703 | + | for _, uc := range []*FaviconUseCase{first, second} { | |
| 704 | + | wg.Add(1) | |
| 705 | + | go func(uc *FaviconUseCase) { | |
| 706 | + | defer wg.Done() | |
| 707 | + | errs <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 708 | + | }(uc) | |
| 709 | + | } | |
| 710 | + | wg.Wait() | |
| 711 | + | close(errs) | |
| 712 | + | for err := range errs { | |
| 713 | + | require.NoError(t, err) | |
| 714 | + | } | |
| 715 | + | require.EqualValues(t, 2, calls.Load(), "sharing must not cross use-case instances") | |
| 716 | + | } | |
| 717 | + | ||
| 718 | + | // TestRefreshMixedStatusBatchNotCached proves a batch mixing a transient | |
| 719 | + | // failure with a genuine absence never populates the negative cache: the | |
| 720 | + | // last miss alone must not decide cacheability. | |
| 721 | + | func TestRefreshMixedStatusBatchNotCached(t *testing.T) { | |
| 722 | + | var calls atomic.Int64 | |
| 723 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 724 | + | calls.Add(1) | |
| 725 | + | if req.IconURL == "https://example.com/denied.ico" { | |
| 726 | + | return nil, &appport.FaviconFetchError{StatusCode: 403} | |
| 727 | + | } | |
| 728 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |
| 729 | + | }) | |
| 730 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 731 | + | ||
| 732 | + | page := "https://example.com/a" | |
| 733 | + | candidates := []string{"https://example.com/denied.ico", "https://example.com/gone.ico"} | |
| 734 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), page, candidates), ErrFaviconMiss) | |
| 735 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 736 | + | require.True(t, ok) | |
| 737 | + | require.False(t, uc.cachedMiss(key), "mixed 403+404 batch must not be negatively cached") | |
| 738 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), page, candidates), ErrFaviconMiss) | |
| 739 | + | require.EqualValues(t, 4, calls.Load(), "second refresh must retry instead of reading a cached absence") | |
| 740 | + | } | |
| 741 | + | ||
| 742 | + | // TestRefreshClosedWinsOverCachedMiss proves sealed admission takes | |
| 743 | + | // precedence over a cached absence: after Close, refresh reports shutdown | |
| 744 | + | // even for a key holding a genuine 404. | |
| 745 | + | func TestRefreshClosedWinsOverCachedMiss(t *testing.T) { | |
| 746 | + | var calls atomic.Int64 | |
| 747 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, _ appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 748 | + | calls.Add(1) | |
| 749 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |
| 750 | + | }) | |
| 751 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |
| 752 | + | ||
| 753 | + | page := "https://example.com/a" | |
| 754 | + | candidates := []string{"https://example.com/gone.ico"} | |
| 755 | + | key, _, ok := refreshRequestKey(page, candidates) | |
| 756 | + | require.True(t, ok) | |
| 757 | + | uc.noteMiss(key, &appport.FaviconFetchError{StatusCode: 404}) | |
| 758 | + | require.True(t, uc.cachedMiss(key)) | |
| 759 | + | uc.Close() | |
| 760 | + | err := uc.RefreshFromIconURLs(context.Background(), page, candidates) | |
| 761 | + | require.ErrorIs(t, err, appport.ErrFaviconShutdown) | |
| 762 | + | require.NotErrorIs(t, err, ErrFaviconMiss, "shutdown must stay distinct from a miss") | |
| 763 | + | require.Zero(t, calls.Load(), "sealed refresh must not reach the fetcher") | |
| 764 | + | } | |
| 765 | + | ||
| 766 | + | // TestCompactMissOrderDedupsReinsertedKeys proves compaction collapses | |
| 767 | + | // stale duplicates: an evicted key that is reinserted keeps exactly one | |
| 768 | + | // order entry, so the backlog cannot grow across expire/reinsert cycles. | |
| 769 | + | func TestCompactMissOrderDedupsReinsertedKeys(t *testing.T) { | |
| 770 | + | now := time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC) | |
| 771 | + | uc := refreshTestUC(t, refreshFetcherMock(t, func(_ context.Context, _ appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |
| 772 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |
| 773 | + | }), &now, nil) | |
| 774 | + | ||
| 775 | + | uc.shared.mu.Lock() | |
| 776 | + | uc.shared.misses["live"] = refreshMiss{expiresAt: now.Add(faviconMissTTL)} | |
| 777 | + | // Simulate eviction (map entry removed, order name left behind) plus | |
| 778 | + | // reinsertion: the same key appears twice in order. | |
| 779 | + | uc.shared.order = []string{"stale-a", "live", "stale-b", "live"} | |
| 780 | + | uc.compactMissOrderLocked() | |
| 781 | + | order := append([]string(nil), uc.shared.order...) | |
| 782 | + | uc.shared.mu.Unlock() | |
| 783 | + | require.Equal(t, []string{"live"}, order, "compaction must keep one entry per live key") | |
| 784 | + | } | |
| 785 | + | ||
| 786 | + | // TestRefreshRequestKey_IPv6DefaultPortEquivalence proves bracketed IPv6 | |
| 787 | + | // hosts with an explicit default port share identity with the bare form: | |
| 788 | + | // without bracket-aware parsing the two spellings hash differently and | |
| 789 | + | // bypass in-flight sharing. | |
| 790 | + | func TestRefreshRequestKey_IPv6DefaultPortEquivalence(t *testing.T) { | |
| 791 | + | page := "https://[2001:db8::1]/a" | |
| 792 | + | withPort, withPortAccepted, ok := refreshRequestKey(page, []string{"https://[2001:db8::1]:443/favicon.ico"}) | |
| 793 | + | require.True(t, ok) | |
| 794 | + | bare, bareAccepted, ok := refreshRequestKey(page, []string{"https://[2001:db8::1]/favicon.ico"}) | |
| 795 | + | require.True(t, ok) | |
| 796 | + | require.Equal(t, bare, withPort, "explicit default port must not change the request key") | |
| 797 | + | require.Equal(t, bareAccepted, withPortAccepted) | |
| 798 | + | } | |
| @@ -0,0 +1,798 @@ | |||||
| 1 | + | package usecase | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "fmt" | |||
| 6 | + | "sync" | |||
| 7 | + | "sync/atomic" | |||
| 8 | + | "testing" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | appport "github.com/bnema/dumber/internal/application/port" | |||
| 12 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |||
| 13 | + | "github.com/bnema/dumber/internal/domain/favicon" | |||
| 14 | + | "github.com/stretchr/testify/mock" | |||
| 15 | + | "github.com/stretchr/testify/require" | |||
| 16 | + | ) | |||
| 17 | + | ||||
| 18 | + | // refreshTestUC builds a FaviconUseCase with a custom fetcher and | |||
| 19 | + | // controllable clock for refresh-coordinator tests. | |||
| 20 | + | func refreshTestUC(t *testing.T, fetcher appport.FaviconFetcher, now *time.Time, background context.Context) *FaviconUseCase { | |||
| 21 | + | t.Helper() | |||
| 22 | + | if background == nil { | |||
| 23 | + | background = context.Background() | |||
| 24 | + | } | |||
| 25 | + | nowFn := time.Now | |||
| 26 | + | if now != nil { | |||
| 27 | + | nowFn = func() time.Time { return *now } | |||
| 28 | + | } | |||
| 29 | + | return NewFaviconUseCase(FaviconDeps{ | |||
| 30 | + | Repository: mockFaviconRepository(t, newFaviconRepoState()), | |||
| 31 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |||
| 32 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |||
| 33 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |||
| 34 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |||
| 35 | + | Fetcher: fetcher, | |||
| 36 | + | Now: nowFn, | |||
| 37 | + | Background: background, | |||
| 38 | + | }) | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | func refreshFetcherMock(t *testing.T, fetch func(context.Context, appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error)) *portmocks.MockFaviconFetcher { | |||
| 42 | + | t.Helper() | |||
| 43 | + | fetcher := portmocks.NewMockFaviconFetcher(t) | |||
| 44 | + | fetcher.EXPECT().Fetch(mock.Anything, mock.Anything).RunAndReturn(fetch).Maybe() | |||
| 45 | + | return fetcher | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | func fetchedIcon(iconURL string) *appport.FaviconFetchedIcon { | |||
| 49 | + | return &appport.FaviconFetchedIcon{ | |||
| 50 | + | IconURL: iconURL, | |||
| 51 | + | Bytes: []byte("icon-bytes"), | |||
| 52 | + | ContentType: "image/png", | |||
| 53 | + | } | |||
| 54 | + | } | |||
| 55 | + | ||||
| 56 | + | // TestRefreshFromIconURLs_ConcurrentIdenticalSharesOneFetch reproduces the | |||
| 57 | + | // P1.1 bypass: concurrent identical candidate requests must share a single | |||
| 58 | + | // fetch instead of issuing one fetch per caller. | |||
| 59 | + | func TestRefreshFromIconURLs_ConcurrentIdenticalSharesOneFetch(t *testing.T) { | |||
| 60 | + | var calls atomic.Int64 | |||
| 61 | + | entered := make(chan struct{}, 8) | |||
| 62 | + | release := make(chan struct{}) | |||
| 63 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 64 | + | calls.Add(1) | |||
| 65 | + | entered <- struct{}{} | |||
| 66 | + | <-release | |||
| 67 | + | return fetchedIcon(req.IconURL), nil | |||
| 68 | + | }) | |||
| 69 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 70 | + | ||||
| 71 | + | const waiters = 5 | |||
| 72 | + | page := "https://example.com/a" | |||
| 73 | + | candidates := []string{"https://example.com/favicon.ico"} | |||
| 74 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 75 | + | require.True(t, ok) | |||
| 76 | + | results := make(chan error, waiters) | |||
| 77 | + | var wg sync.WaitGroup | |||
| 78 | + | for range waiters { | |||
| 79 | + | wg.Add(1) | |||
| 80 | + | go func() { | |||
| 81 | + | defer wg.Done() | |||
| 82 | + | results <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 83 | + | }() | |||
| 84 | + | } | |||
| 85 | + | // All five waiters must join the single shared operation before the | |||
| 86 | + | // fetch completes; the waiter count is the deterministic join signal. | |||
| 87 | + | require.Eventually(t, func() bool { | |||
| 88 | + | uc.shared.mu.Lock() | |||
| 89 | + | defer uc.shared.mu.Unlock() | |||
| 90 | + | call, ok := uc.shared.active[key] | |||
| 91 | + | return ok && call.waiters == waiters | |||
| 92 | + | }, 10*time.Second, 5*time.Millisecond, "waiters did not join the shared operation") | |||
| 93 | + | close(release) | |||
| 94 | + | wg.Wait() | |||
| 95 | + | close(results) | |||
| 96 | + | for err := range results { | |||
| 97 | + | require.NoError(t, err) | |||
| 98 | + | } | |||
| 99 | + | require.EqualValues(t, 1, calls.Load(), "identical concurrent refreshes must share one fetch") | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | // TestRefreshFromIconURLs_ChangingCandidatesDoNotShare verifies that | |||
| 103 | + | // different candidate lists are distinct operations. | |||
| 104 | + | func TestRefreshFromIconURLs_ChangingCandidatesDoNotShare(t *testing.T) { | |||
| 105 | + | var calls atomic.Int64 | |||
| 106 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 107 | + | calls.Add(1) | |||
| 108 | + | return fetchedIcon(req.IconURL), nil | |||
| 109 | + | }) | |||
| 110 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 111 | + | ||||
| 112 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/one.ico"})) | |||
| 113 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/two.ico"})) | |||
| 114 | + | require.EqualValues(t, 2, calls.Load()) | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | // TestRefreshFromIconURLs_CancelOneWaiterKeepsOthers proves waiter | |||
| 118 | + | // independence: a canceled waiter leaves while the shared operation serves | |||
| 119 | + | // the rest. | |||
| 120 | + | func TestRefreshFromIconURLs_CancelOneWaiterKeepsOthers(t *testing.T) { | |||
| 121 | + | release := make(chan struct{}) | |||
| 122 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 123 | + | <-release | |||
| 124 | + | return fetchedIcon(req.IconURL), nil | |||
| 125 | + | }) | |||
| 126 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 127 | + | ||||
| 128 | + | page := "https://example.com/a" | |||
| 129 | + | candidates := []string{"https://example.com/favicon.ico"} | |||
| 130 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 131 | + | require.True(t, ok, "test page and candidates must form a refresh key") | |||
| 132 | + | leaving, cancelLeaving := context.WithCancel(context.Background()) | |||
| 133 | + | left := make(chan error, 1) | |||
| 134 | + | go func() { | |||
| 135 | + | left <- uc.RefreshFromIconURLs(leaving, page, candidates) | |||
| 136 | + | }() | |||
| 137 | + | staying := make(chan error, 1) | |||
| 138 | + | go func() { | |||
| 139 | + | staying <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 140 | + | }() | |||
| 141 | + | // Both waiters must share one operation before either leaves: two | |||
| 142 | + | // waiters on the same call proves sharing, not two separate fetches. | |||
| 143 | + | require.Eventually(t, func() bool { | |||
| 144 | + | uc.shared.mu.Lock() | |||
| 145 | + | defer uc.shared.mu.Unlock() | |||
| 146 | + | call, ok := uc.shared.active[key] | |||
| 147 | + | return ok && !call.closing && call.waiters == 2 | |||
| 148 | + | }, 10*time.Second, 5*time.Millisecond, "both waiters must join one shared operation") | |||
| 149 | + | cancelLeaving() | |||
| 150 | + | select { | |||
| 151 | + | case err := <-left: | |||
| 152 | + | require.ErrorIs(t, err, context.Canceled) | |||
| 153 | + | case <-time.After(10 * time.Second): | |||
| 154 | + | t.Fatal("canceled waiter did not return") | |||
| 155 | + | } | |||
| 156 | + | close(release) | |||
| 157 | + | select { | |||
| 158 | + | case err := <-staying: | |||
| 159 | + | require.NoError(t, err, "remaining waiter must still be served") | |||
| 160 | + | case <-time.After(10 * time.Second): | |||
| 161 | + | t.Fatal("remaining waiter was dropped with the canceled one") | |||
| 162 | + | } | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | // TestRefreshFromIconURLs_InvalidPageHierarchyRejected verifies invalid | |||
| 166 | + | // origins never reach the fetcher. | |||
| 167 | + | func TestRefreshFromIconURLs_InvalidPageHierarchyRejected(t *testing.T) { | |||
| 168 | + | var calls atomic.Int64 | |||
| 169 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 170 | + | calls.Add(1) | |||
| 171 | + | return fetchedIcon(req.IconURL), nil | |||
| 172 | + | }) | |||
| 173 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 174 | + | ||||
| 175 | + | for _, page := range []string{"", "not a url", "ftp://example.com/a", "dumb://internal"} { | |||
| 176 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), page, []string{"https://example.com/favicon.ico"}), ErrFaviconMiss) | |||
| 177 | + | } | |||
| 178 | + | require.Zero(t, calls.Load()) | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | // TestRefreshRequestKey_Distinctions covers origin and candidate | |||
| 182 | + | // significance: scheme/port/query matter, fragments/case/default ports do | |||
| 183 | + | // not, relative candidates resolve against the page, over-long and excess | |||
| 184 | + | // candidates are skipped. | |||
| 185 | + | func TestRefreshRequestKey_Distinctions(t *testing.T) { | |||
| 186 | + | page := "https://example.com:8443/a?x=1" | |||
| 187 | + | keyOf := func(candidates ...string) (string, bool) { | |||
| 188 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 189 | + | return key, ok | |||
| 190 | + | } | |||
| 191 | + | base, ok := keyOf("https://example.com:8443/icon.png") | |||
| 192 | + | require.True(t, ok) | |||
| 193 | + | ||||
| 194 | + | other, _ := keyOf("https://example.com:8443/other.png") | |||
| 195 | + | require.NotEqual(t, base, other, "different candidates must not share") | |||
| 196 | + | ||||
| 197 | + | portKey, _ := keyOf("https://example.com:9443/icon.png") | |||
| 198 | + | require.NotEqual(t, base, portKey, "ports are significant") | |||
| 199 | + | ||||
| 200 | + | queryKey, _ := keyOf("https://example.com:8443/icon.png?v=2") | |||
| 201 | + | require.NotEqual(t, base, queryKey, "queries are significant") | |||
| 202 | + | ||||
| 203 | + | fragKey, _ := keyOf("https://example.com:8443/icon.png#frag") | |||
| 204 | + | require.Equal(t, base, fragKey, "fragments must not split sharing") | |||
| 205 | + | ||||
| 206 | + | caseKey, _ := keyOf("HTTPS://EXAMPLE.COM:8443/icon.png") | |||
| 207 | + | require.Equal(t, base, caseKey, "scheme/host case must not split sharing") | |||
| 208 | + | ||||
| 209 | + | defaultPort, _ := keyOf("https://example.com:443/icon.png") | |||
| 210 | + | otherPage, _, ok := refreshRequestKey("https://example.com/icon", []string{"https://example.com:443/icon.png"}) | |||
| 211 | + | require.True(t, ok) | |||
| 212 | + | require.NotEqual(t, defaultPort, otherPage, "page origin is part of the identity") | |||
| 213 | + | ||||
| 214 | + | relKey, relCandidates, ok := refreshRequestKey("https://example.com/a", []string{"/icon.png"}) | |||
| 215 | + | require.True(t, ok) | |||
| 216 | + | require.Equal(t, []string{"https://example.com/icon.png"}, relCandidates, "relative candidates resolve against the page") | |||
| 217 | + | require.NotEqual(t, relKey, base, "different pages must not share") | |||
| 218 | + | ||||
| 219 | + | long := "https://example.com/" + string(make([]byte, maxRefreshCandidateLen)) | |||
| 220 | + | _, kept, ok := refreshRequestKey(page, []string{long, "https://example.com:8443/icon.png"}) | |||
| 221 | + | require.True(t, ok) | |||
| 222 | + | require.Len(t, kept, 1, "over-long candidates are skipped") | |||
| 223 | + | ||||
| 224 | + | many := make([]string, maxRefreshCandidates+4) | |||
| 225 | + | for i := range many { | |||
| 226 | + | many[i] = "https://example.com:8443/icon.png" | |||
| 227 | + | } | |||
| 228 | + | _, kept, ok = refreshRequestKey(page, many) | |||
| 229 | + | require.True(t, ok) | |||
| 230 | + | require.Len(t, kept, maxRefreshCandidates, "candidates are capped") | |||
| 231 | + | ||||
| 232 | + | _, _, ok = refreshRequestKey("not a url", []string{"https://example.com/icon.png"}) | |||
| 233 | + | require.False(t, ok, "invalid page hierarchy is rejected") | |||
| 234 | + | } | |||
| 235 | + | ||||
| 236 | + | // TestRefreshAdmission_BusyWhenExhausted fills the shared budget with | |||
| 237 | + | // blocked operations and proves the next direct caller gets a distinct | |||
| 238 | + | // busy outcome instead of false success. | |||
| 239 | + | func TestRefreshAdmission_BusyWhenExhausted(t *testing.T) { | |||
| 240 | + | release := make(chan struct{}) | |||
| 241 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 242 | + | <-release | |||
| 243 | + | return fetchedIcon(req.IconURL), nil | |||
| 244 | + | }) | |||
| 245 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 246 | + | ||||
| 247 | + | var wg sync.WaitGroup | |||
| 248 | + | for i := range maxActiveRefreshes { | |||
| 249 | + | wg.Add(1) | |||
| 250 | + | go func(i int) { | |||
| 251 | + | defer wg.Done() | |||
| 252 | + | // Distinct candidates per holder fill distinct budget slots; | |||
| 253 | + | // identical requests would share one. | |||
| 254 | + | icon := fmt.Sprintf("https://example.com/icon-%d.png", i) | |||
| 255 | + | _ = uc.RefreshFromIconURLs(context.Background(), "https://example.com/page", []string{icon}) | |||
| 256 | + | }(i) | |||
| 257 | + | } | |||
| 258 | + | require.Eventually(t, func() bool { | |||
| 259 | + | uc.shared.mu.Lock() | |||
| 260 | + | defer uc.shared.mu.Unlock() | |||
| 261 | + | return len(uc.shared.active) == maxActiveRefreshes | |||
| 262 | + | }, 10*time.Second, 5*time.Millisecond, "budget must fill") | |||
| 263 | + | ||||
| 264 | + | err := uc.RefreshFromIconURLs(context.Background(), "https://other.example/", []string{"https://other.example/icon.png"}) | |||
| 265 | + | require.ErrorIs(t, err, appport.ErrFaviconBusy) | |||
| 266 | + | require.NotErrorIs(t, err, ErrFaviconMiss, "overload must stay distinct from a miss") | |||
| 267 | + | ||||
| 268 | + | close(release) | |||
| 269 | + | wg.Wait() | |||
| 270 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://other.example/", []string{"https://other.example/icon.png"})) | |||
| 271 | + | } | |||
| 272 | + | ||||
| 273 | + | // TestRefreshOp_WholeDeadline verifies the shared operation carries the | |||
| 274 | + | // bounded whole-operation deadline without waiting it out. | |||
| 275 | + | func TestRefreshOp_WholeDeadline(t *testing.T) { | |||
| 276 | + | deadlines := make(chan time.Time, 1) | |||
| 277 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 278 | + | if deadline, ok := ctx.Deadline(); ok { | |||
| 279 | + | deadlines <- deadline | |||
| 280 | + | } | |||
| 281 | + | return fetchedIcon(req.IconURL), nil | |||
| 282 | + | }) | |||
| 283 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 284 | + | ||||
| 285 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/favicon.ico"})) | |||
| 286 | + | select { | |||
| 287 | + | case deadline := <-deadlines: | |||
| 288 | + | remaining := time.Until(deadline) | |||
| 289 | + | require.Greater(t, remaining, 4*time.Second) | |||
| 290 | + | require.LessOrEqual(t, remaining, refreshOpTimeout) | |||
| 291 | + | case <-time.After(10 * time.Second): | |||
| 292 | + | t.Fatal("fetch did not observe the operation deadline") | |||
| 293 | + | } | |||
| 294 | + | } | |||
| 295 | + | ||||
| 296 | + | // TestRefreshLastWaiterCancelAbortsOp proves a lone waiter that leaves | |||
| 297 | + | // cancels the shared operation instead of leaking it. | |||
| 298 | + | func TestRefreshLastWaiterCancelAbortsOp(t *testing.T) { | |||
| 299 | + | observedCancel := make(chan struct{}, 1) | |||
| 300 | + | entered := make(chan struct{}, 1) | |||
| 301 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 302 | + | select { | |||
| 303 | + | case entered <- struct{}{}: | |||
| 304 | + | default: | |||
| 305 | + | } | |||
| 306 | + | select { | |||
| 307 | + | case <-ctx.Done(): | |||
| 308 | + | observedCancel <- struct{}{} | |||
| 309 | + | return nil, ctx.Err() | |||
| 310 | + | case <-time.After(10 * time.Second): | |||
| 311 | + | return fetchedIcon(req.IconURL), nil | |||
| 312 | + | } | |||
| 313 | + | }) | |||
| 314 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 315 | + | ||||
| 316 | + | waiter, cancel := context.WithCancel(context.Background()) | |||
| 317 | + | done := make(chan error, 1) | |||
| 318 | + | go func() { | |||
| 319 | + | done <- uc.RefreshFromIconURLs(waiter, "https://example.com/a", []string{"https://example.com/favicon.ico"}) | |||
| 320 | + | }() | |||
| 321 | + | // Cancel only after the executor entered the fetch: len(active) == 1 | |||
| 322 | + | // alone cannot prove the executor is blocked inside Fetch, and an early | |||
| 323 | + | // cancel could race operation creation instead of aborting shared work. | |||
| 324 | + | select { | |||
| 325 | + | case <-entered: | |||
| 326 | + | case <-time.After(10 * time.Second): | |||
| 327 | + | t.Fatal("executor did not enter the fetch") | |||
| 328 | + | } | |||
| 329 | + | cancel() | |||
| 330 | + | select { | |||
| 331 | + | case err := <-done: | |||
| 332 | + | require.ErrorIs(t, err, context.Canceled) | |||
| 333 | + | case <-time.After(10 * time.Second): | |||
| 334 | + | t.Fatal("canceled lone waiter did not return") | |||
| 335 | + | } | |||
| 336 | + | select { | |||
| 337 | + | case <-observedCancel: | |||
| 338 | + | case <-time.After(10 * time.Second): | |||
| 339 | + | t.Fatal("operation was not canceled with its last waiter") | |||
| 340 | + | } | |||
| 341 | + | require.Eventually(t, func() bool { | |||
| 342 | + | uc.shared.mu.Lock() | |||
| 343 | + | defer uc.shared.mu.Unlock() | |||
| 344 | + | return len(uc.shared.active) == 0 | |||
| 345 | + | }, 10*time.Second, 5*time.Millisecond, "canceled operation must release its slot") | |||
| 346 | + | } | |||
| 347 | + | ||||
| 348 | + | // TestFaviconUseCase_CloseCancelsAndDrains proves shutdown wiring: Close | |||
| 349 | + | // aborts owned background work and returns once nothing is active. | |||
| 350 | + | func TestFaviconUseCase_CloseCancelsAndDrains(t *testing.T) { | |||
| 351 | + | background, cancel := context.WithCancel(context.Background()) | |||
| 352 | + | defer cancel() | |||
| 353 | + | sawCancel := make(chan struct{}, 1) | |||
| 354 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 355 | + | select { | |||
| 356 | + | case <-ctx.Done(): | |||
| 357 | + | sawCancel <- struct{}{} | |||
| 358 | + | return nil, ctx.Err() | |||
| 359 | + | case <-time.After(10 * time.Second): | |||
| 360 | + | return fetchedIcon(req.IconURL), nil | |||
| 361 | + | } | |||
| 362 | + | }) | |||
| 363 | + | uc := refreshTestUC(t, fetcher, nil, background) | |||
| 364 | + | ||||
| 365 | + | done := make(chan error, 1) | |||
| 366 | + | go func() { | |||
| 367 | + | done <- uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/favicon.ico"}) | |||
| 368 | + | }() | |||
| 369 | + | require.Eventually(t, func() bool { | |||
| 370 | + | uc.shared.mu.Lock() | |||
| 371 | + | defer uc.shared.mu.Unlock() | |||
| 372 | + | return len(uc.shared.active) == 1 | |||
| 373 | + | }, 10*time.Second, 5*time.Millisecond) | |||
| 374 | + | ||||
| 375 | + | closed := make(chan struct{}) | |||
| 376 | + | go func() { | |||
| 377 | + | uc.Close() | |||
| 378 | + | close(closed) | |||
| 379 | + | }() | |||
| 380 | + | select { | |||
| 381 | + | case <-sawCancel: | |||
| 382 | + | case <-time.After(10 * time.Second): | |||
| 383 | + | t.Fatal("Close did not cancel background work") | |||
| 384 | + | } | |||
| 385 | + | select { | |||
| 386 | + | case <-closed: | |||
| 387 | + | case <-time.After(10 * time.Second): | |||
| 388 | + | t.Fatal("Close did not drain") | |||
| 389 | + | } | |||
| 390 | + | select { | |||
| 391 | + | case err := <-done: | |||
| 392 | + | require.Error(t, err) | |||
| 393 | + | case <-time.After(10 * time.Second): | |||
| 394 | + | t.Fatal("refresh caller did not return after Close") | |||
| 395 | + | } | |||
| 396 | + | // Repeat Close must be safe (idempotent, no error to report). | |||
| 397 | + | uc.Close() | |||
| 398 | + | } | |||
| 399 | + | ||||
| 400 | + | // TestRefreshMissClassification_HTTPClasses proves only genuine absence | |||
| 401 | + | // (404/410) is negatively cached: auth/transient failures refetch, and a | |||
| 402 | + | // repeated miss within TTL issues no fetch. | |||
| 403 | + | func TestRefreshMissClassification_HTTPClasses(t *testing.T) { | |||
| 404 | + | now := time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC) | |||
| 405 | + | statusByURL := map[string]int{ | |||
| 406 | + | "https://example.com/gone.ico": 404, | |||
| 407 | + | "https://example.com/removed.ico": 410, | |||
| 408 | + | "https://example.com/denied.ico": 403, | |||
| 409 | + | "https://example.com/login.ico": 401, | |||
| 410 | + | "https://example.com/broken.ico": 500, | |||
| 411 | + | "https://example.com/unstable.ico": 503, | |||
| 412 | + | "https://example.com/validation.ico": 0, | |||
| 413 | + | } | |||
| 414 | + | var calls atomic.Int64 | |||
| 415 | + | var candidateCalls atomic.Int64 | |||
| 416 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 417 | + | calls.Add(1) | |||
| 418 | + | // The all-miss fallback runs a candidate-less discovery fetch; | |||
| 419 | + | // assertions below count candidate batches only. | |||
| 420 | + | if req.IconURL != "" { | |||
| 421 | + | candidateCalls.Add(1) | |||
| 422 | + | } | |||
| 423 | + | status := statusByURL[req.IconURL] | |||
| 424 | + | if status == 0 { | |||
| 425 | + | return nil, appport.ErrFaviconMiss | |||
| 426 | + | } | |||
| 427 | + | return nil, &appport.FaviconFetchError{StatusCode: status} | |||
| 428 | + | }) | |||
| 429 | + | uc := refreshTestUC(t, fetcher, &now, nil) | |||
| 430 | + | page := "https://example.com/a" | |||
| 431 | + | ||||
| 432 | + | refresh := func(icon string) error { | |||
| 433 | + | return uc.RefreshFromIconURLs(context.Background(), page, []string{icon}) | |||
| 434 | + | } | |||
| 435 | + | for icon := range statusByURL { | |||
| 436 | + | err := refresh(icon) | |||
| 437 | + | require.ErrorIs(t, err, ErrFaviconMiss, "every class must preserve miss matching for %s", icon) | |||
| 438 | + | } | |||
| 439 | + | firstCalls := candidateCalls.Load() | |||
| 440 | + | require.EqualValues(t, len(statusByURL), firstCalls) | |||
| 441 | + | ||||
| 442 | + | // Genuine absence is cached: 404/410 refetch nothing within TTL. | |||
| 443 | + | require.ErrorIs(t, refresh("https://example.com/gone.ico"), ErrFaviconMiss) | |||
| 444 | + | require.ErrorIs(t, refresh("https://example.com/removed.ico"), ErrFaviconMiss) | |||
| 445 | + | require.Equal(t, firstCalls, candidateCalls.Load(), "cached genuine absence must not refetch") | |||
| 446 | + | ||||
| 447 | + | // Auth/transient/validation failures are never cached: they refetch. | |||
| 448 | + | require.ErrorIs(t, refresh("https://example.com/denied.ico"), ErrFaviconMiss) | |||
| 449 | + | require.ErrorIs(t, refresh("https://example.com/broken.ico"), ErrFaviconMiss) | |||
| 450 | + | require.ErrorIs(t, refresh("https://example.com/validation.ico"), ErrFaviconMiss) | |||
| 451 | + | require.Equal(t, firstCalls+3, candidateCalls.Load()) | |||
| 452 | + | ||||
| 453 | + | // After TTL expiry the genuine absence is eligible again. | |||
| 454 | + | now = now.Add(faviconMissTTL + time.Second) | |||
| 455 | + | require.ErrorIs(t, refresh("https://example.com/gone.ico"), ErrFaviconMiss) | |||
| 456 | + | require.Equal(t, firstCalls+4, candidateCalls.Load()) | |||
| 457 | + | } | |||
| 458 | + | ||||
| 459 | + | // TestRefreshInvalidate_DropsStaleCompletion proves the epoch rule: an | |||
| 460 | + | // invalidation racing an in-flight fetch prevents the stale bytes from | |||
| 461 | + | // being stored. | |||
| 462 | + | func TestRefreshInvalidate_DropsStaleCompletion(t *testing.T) { | |||
| 463 | + | release := make(chan struct{}) | |||
| 464 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 465 | + | <-release | |||
| 466 | + | return fetchedIcon(req.IconURL), nil | |||
| 467 | + | }) | |||
| 468 | + | repo := newFaviconRepoState() | |||
| 469 | + | uc := NewFaviconUseCase(FaviconDeps{ | |||
| 470 | + | Repository: mockFaviconRepository(t, repo), | |||
| 471 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |||
| 472 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |||
| 473 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |||
| 474 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |||
| 475 | + | Fetcher: fetcher, | |||
| 476 | + | Now: time.Now, | |||
| 477 | + | Background: context.Background(), | |||
| 478 | + | }) | |||
| 479 | + | ||||
| 480 | + | page := "https://example.com/a" | |||
| 481 | + | done := make(chan error, 1) | |||
| 482 | + | go func() { | |||
| 483 | + | done <- uc.RefreshFromIconURLs(context.Background(), page, []string{"https://example.com/favicon.ico"}) | |||
| 484 | + | }() | |||
| 485 | + | require.Eventually(t, func() bool { | |||
| 486 | + | uc.shared.mu.Lock() | |||
| 487 | + | defer uc.shared.mu.Unlock() | |||
| 488 | + | return len(uc.shared.active) == 1 | |||
| 489 | + | }, 10*time.Second, 5*time.Millisecond) | |||
| 490 | + | ||||
| 491 | + | key, ok := favicon.CanonicalKey(page) | |||
| 492 | + | require.True(t, ok) | |||
| 493 | + | require.NoError(t, uc.Invalidate(context.Background(), key)) | |||
| 494 | + | close(release) | |||
| 495 | + | select { | |||
| 496 | + | case <-done: | |||
| 497 | + | case <-time.After(10 * time.Second): | |||
| 498 | + | t.Fatal("refresh did not complete after release") | |||
| 499 | + | } | |||
| 500 | + | require.Empty(t, repo.byKey, "stale completion must not repopulate invalidated entries") | |||
| 501 | + | } | |||
| 502 | + | ||||
| 503 | + | // TestRefreshJoinerSharesCreatorEpoch proves concurrent joiners never | |||
| 504 | + | // invalidate the operation they joined: overlapping identical refreshes | |||
| 505 | + | // both succeed and the fetched icon is stored. | |||
| 506 | + | func TestRefreshJoinerSharesCreatorEpoch(t *testing.T) { | |||
| 507 | + | release := make(chan struct{}) | |||
| 508 | + | var calls atomic.Int64 | |||
| 509 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 510 | + | calls.Add(1) | |||
| 511 | + | <-release | |||
| 512 | + | return fetchedIcon(req.IconURL), nil | |||
| 513 | + | }) | |||
| 514 | + | repo := newFaviconRepoState() | |||
| 515 | + | uc := NewFaviconUseCase(FaviconDeps{ | |||
| 516 | + | Repository: mockFaviconRepository(t, repo), | |||
| 517 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |||
| 518 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |||
| 519 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |||
| 520 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |||
| 521 | + | Fetcher: fetcher, | |||
| 522 | + | Now: time.Now, | |||
| 523 | + | Background: context.Background(), | |||
| 524 | + | }) | |||
| 525 | + | ||||
| 526 | + | page := "https://example.com/a" | |||
| 527 | + | candidates := []string{"https://example.com/favicon.ico"} | |||
| 528 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 529 | + | require.True(t, ok) | |||
| 530 | + | first := make(chan error, 1) | |||
| 531 | + | go func() { | |||
| 532 | + | first <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 533 | + | }() | |||
| 534 | + | require.Eventually(t, func() bool { | |||
| 535 | + | uc.shared.mu.Lock() | |||
| 536 | + | defer uc.shared.mu.Unlock() | |||
| 537 | + | return len(uc.shared.active) == 1 | |||
| 538 | + | }, 10*time.Second, 5*time.Millisecond) | |||
| 539 | + | second := make(chan error, 1) | |||
| 540 | + | go func() { | |||
| 541 | + | second <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 542 | + | }() | |||
| 543 | + | require.Eventually(t, func() bool { | |||
| 544 | + | uc.shared.mu.Lock() | |||
| 545 | + | defer uc.shared.mu.Unlock() | |||
| 546 | + | call, ok := uc.shared.active[key] | |||
| 547 | + | return ok && call.waiters == 2 | |||
| 548 | + | }, 10*time.Second, 5*time.Millisecond, "joiner must share the creator operation") | |||
| 549 | + | close(release) | |||
| 550 | + | select { | |||
| 551 | + | case err := <-first: | |||
| 552 | + | require.NoError(t, err) | |||
| 553 | + | case <-time.After(10 * time.Second): | |||
| 554 | + | t.Fatal("creator did not complete") | |||
| 555 | + | } | |||
| 556 | + | select { | |||
| 557 | + | case err := <-second: | |||
| 558 | + | require.NoError(t, err) | |||
| 559 | + | case <-time.After(10 * time.Second): | |||
| 560 | + | t.Fatal("joiner did not complete") | |||
| 561 | + | } | |||
| 562 | + | require.EqualValues(t, 1, calls.Load()) | |||
| 563 | + | require.NotEmpty(t, repo.byKey, "shared success must be stored, not dropped by a joiner epoch bump") | |||
| 564 | + | } | |||
| 565 | + | ||||
| 566 | + | // TestRefreshPartialMissDoesNotPoisonLaterCandidates proves request-level | |||
| 567 | + | // negative caching happens only for fully missed requests: a 404 on the | |||
| 568 | + | // first candidate followed by a successful second candidate stores the | |||
| 569 | + | // icon and records no miss. | |||
| 570 | + | func TestRefreshPartialMissDoesNotPoisonLaterCandidates(t *testing.T) { | |||
| 571 | + | var calls atomic.Int64 | |||
| 572 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 573 | + | calls.Add(1) | |||
| 574 | + | if req.IconURL == "https://example.com/gone.ico" { | |||
| 575 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |||
| 576 | + | } | |||
| 577 | + | return fetchedIcon(req.IconURL), nil | |||
| 578 | + | }) | |||
| 579 | + | repo := newFaviconRepoState() | |||
| 580 | + | uc := NewFaviconUseCase(FaviconDeps{ | |||
| 581 | + | Repository: mockFaviconRepository(t, repo), | |||
| 582 | + | BlobStore: mockFaviconBlobStore(t, newFaviconBlobStoreState()), | |||
| 583 | + | Converter: mockFaviconConverter(t, &faviconConverterState{}), | |||
| 584 | + | Scheduler: mockFaviconScheduler(t, &faviconSchedulerState{seen: map[favicon.Key]bool{}}), | |||
| 585 | + | Invalidators: mockFaviconInvalidators(t, &faviconInvalidatorsState{}), | |||
| 586 | + | Fetcher: fetcher, | |||
| 587 | + | Now: time.Now, | |||
| 588 | + | Background: context.Background(), | |||
| 589 | + | }) | |||
| 590 | + | ||||
| 591 | + | page := "https://example.com/a" | |||
| 592 | + | candidates := []string{"https://example.com/gone.ico", "https://example.com/good.ico"} | |||
| 593 | + | require.NoError(t, uc.RefreshFromIconURLs(context.Background(), page, candidates)) | |||
| 594 | + | require.NotEmpty(t, repo.byKey, "successful candidate must be stored") | |||
| 595 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 596 | + | require.True(t, ok) | |||
| 597 | + | require.False(t, uc.cachedMiss(key), "partial miss must not populate the request miss cache") | |||
| 598 | + | } | |||
| 599 | + | ||||
| 600 | + | // TestRefreshCanceledCallNotJoined proves a waiter arriving after the last | |||
| 601 | + | // waiter canceled starts a fresh operation instead of observing the dying | |||
| 602 | + | // call. | |||
| 603 | + | func TestRefreshCanceledCallNotJoined(t *testing.T) { | |||
| 604 | + | release := make(chan struct{}) | |||
| 605 | + | var calls atomic.Int64 | |||
| 606 | + | fetcher := refreshFetcherMock(t, func(ctx context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 607 | + | calls.Add(1) | |||
| 608 | + | select { | |||
| 609 | + | case <-release: | |||
| 610 | + | return fetchedIcon(req.IconURL), nil | |||
| 611 | + | case <-ctx.Done(): | |||
| 612 | + | return nil, ctx.Err() | |||
| 613 | + | } | |||
| 614 | + | }) | |||
| 615 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 616 | + | ||||
| 617 | + | page := "https://example.com/a" | |||
| 618 | + | candidates := []string{"https://example.com/favicon.ico"} | |||
| 619 | + | leaving, cancel := context.WithCancel(context.Background()) | |||
| 620 | + | left := make(chan error, 1) | |||
| 621 | + | go func() { | |||
| 622 | + | left <- uc.RefreshFromIconURLs(leaving, page, candidates) | |||
| 623 | + | }() | |||
| 624 | + | require.Eventually(t, func() bool { | |||
| 625 | + | uc.shared.mu.Lock() | |||
| 626 | + | defer uc.shared.mu.Unlock() | |||
| 627 | + | return len(uc.shared.active) == 1 | |||
| 628 | + | }, 10*time.Second, 5*time.Millisecond) | |||
| 629 | + | cancel() | |||
| 630 | + | select { | |||
| 631 | + | case err := <-left: | |||
| 632 | + | require.ErrorIs(t, err, context.Canceled) | |||
| 633 | + | case <-time.After(10 * time.Second): | |||
| 634 | + | t.Fatal("canceled waiter did not return") | |||
| 635 | + | } | |||
| 636 | + | // The dying call still occupies the map entry until its executor | |||
| 637 | + | // finishes; the next waiter must start fresh rather than join it. | |||
| 638 | + | fresh := make(chan error, 1) | |||
| 639 | + | go func() { | |||
| 640 | + | fresh <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 641 | + | }() | |||
| 642 | + | require.Eventually(t, func() bool { | |||
| 643 | + | return calls.Load() == 2 | |||
| 644 | + | }, 10*time.Second, 5*time.Millisecond, "late joiner must start a fresh operation") | |||
| 645 | + | close(release) | |||
| 646 | + | select { | |||
| 647 | + | case err := <-fresh: | |||
| 648 | + | require.NoError(t, err) | |||
| 649 | + | case <-time.After(10 * time.Second): | |||
| 650 | + | t.Fatal("fresh operation did not complete") | |||
| 651 | + | } | |||
| 652 | + | } | |||
| 653 | + | ||||
| 654 | + | // TestRefreshCloseSealsAdmission proves no refresh can start after Close | |||
| 655 | + | // began: late callers receive a distinct shutdown error instead of racing | |||
| 656 | + | // the drain. | |||
| 657 | + | func TestRefreshCloseSealsAdmission(t *testing.T) { | |||
| 658 | + | uc := refreshTestUC(t, refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 659 | + | return fetchedIcon(req.IconURL), nil | |||
| 660 | + | }), nil, nil) | |||
| 661 | + | ||||
| 662 | + | uc.Close() | |||
| 663 | + | err := uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{"https://example.com/favicon.ico"}) | |||
| 664 | + | require.ErrorIs(t, err, appport.ErrFaviconShutdown) | |||
| 665 | + | require.NotErrorIs(t, err, ErrFaviconMiss, "shutdown must stay distinct from a miss") | |||
| 666 | + | } | |||
| 667 | + | ||||
| 668 | + | // TestRefreshMissOrderBounded proves the negative-cache order backlog | |||
| 669 | + | // cannot grow without bound under distinct misses. | |||
| 670 | + | func TestRefreshMissOrderBounded(t *testing.T) { | |||
| 671 | + | now := time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC) | |||
| 672 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, _ appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 673 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |||
| 674 | + | }) | |||
| 675 | + | uc := refreshTestUC(t, fetcher, &now, nil) | |||
| 676 | + | ||||
| 677 | + | for i := range 3 * maxMissEntries { | |||
| 678 | + | icon := "https://example.com/missing-" + string(rune('a'+i%26)) + "-" + string(rune('0'+i/26%10)) + ".ico" | |||
| 679 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), "https://example.com/a", []string{icon}), ErrFaviconMiss) | |||
| 680 | + | } | |||
| 681 | + | uc.shared.mu.Lock() | |||
| 682 | + | defer uc.shared.mu.Unlock() | |||
| 683 | + | require.LessOrEqual(t, len(uc.shared.misses), maxMissEntries) | |||
| 684 | + | require.LessOrEqual(t, len(uc.shared.order), 4*maxMissEntries, "order backlog must stay bounded") | |||
| 685 | + | } | |||
| 686 | + | ||||
| 687 | + | // TestRefreshSharing_ScopedToUseCaseInstance proves profile isolation: | |||
| 688 | + | // identical requests on different use-case instances never share fetches. | |||
| 689 | + | func TestRefreshSharing_ScopedToUseCaseInstance(t *testing.T) { | |||
| 690 | + | var calls atomic.Int64 | |||
| 691 | + | newUC := func() *FaviconUseCase { | |||
| 692 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 693 | + | calls.Add(1) | |||
| 694 | + | return fetchedIcon(req.IconURL), nil | |||
| 695 | + | }) | |||
| 696 | + | return refreshTestUC(t, fetcher, nil, nil) | |||
| 697 | + | } | |||
| 698 | + | first, second := newUC(), newUC() | |||
| 699 | + | page := "https://example.com/a" | |||
| 700 | + | candidates := []string{"https://example.com/favicon.ico"} | |||
| 701 | + | var wg sync.WaitGroup | |||
| 702 | + | errs := make(chan error, 2) | |||
| 703 | + | for _, uc := range []*FaviconUseCase{first, second} { | |||
| 704 | + | wg.Add(1) | |||
| 705 | + | go func(uc *FaviconUseCase) { | |||
| 706 | + | defer wg.Done() | |||
| 707 | + | errs <- uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 708 | + | }(uc) | |||
| 709 | + | } | |||
| 710 | + | wg.Wait() | |||
| 711 | + | close(errs) | |||
| 712 | + | for err := range errs { | |||
| 713 | + | require.NoError(t, err) | |||
| 714 | + | } | |||
| 715 | + | require.EqualValues(t, 2, calls.Load(), "sharing must not cross use-case instances") | |||
| 716 | + | } | |||
| 717 | + | ||||
| 718 | + | // TestRefreshMixedStatusBatchNotCached proves a batch mixing a transient | |||
| 719 | + | // failure with a genuine absence never populates the negative cache: the | |||
| 720 | + | // last miss alone must not decide cacheability. | |||
| 721 | + | func TestRefreshMixedStatusBatchNotCached(t *testing.T) { | |||
| 722 | + | var calls atomic.Int64 | |||
| 723 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, req appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 724 | + | calls.Add(1) | |||
| 725 | + | if req.IconURL == "https://example.com/denied.ico" { | |||
| 726 | + | return nil, &appport.FaviconFetchError{StatusCode: 403} | |||
| 727 | + | } | |||
| 728 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |||
| 729 | + | }) | |||
| 730 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 731 | + | ||||
| 732 | + | page := "https://example.com/a" | |||
| 733 | + | candidates := []string{"https://example.com/denied.ico", "https://example.com/gone.ico"} | |||
| 734 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), page, candidates), ErrFaviconMiss) | |||
| 735 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 736 | + | require.True(t, ok) | |||
| 737 | + | require.False(t, uc.cachedMiss(key), "mixed 403+404 batch must not be negatively cached") | |||
| 738 | + | require.ErrorIs(t, uc.RefreshFromIconURLs(context.Background(), page, candidates), ErrFaviconMiss) | |||
| 739 | + | require.EqualValues(t, 4, calls.Load(), "second refresh must retry instead of reading a cached absence") | |||
| 740 | + | } | |||
| 741 | + | ||||
| 742 | + | // TestRefreshClosedWinsOverCachedMiss proves sealed admission takes | |||
| 743 | + | // precedence over a cached absence: after Close, refresh reports shutdown | |||
| 744 | + | // even for a key holding a genuine 404. | |||
| 745 | + | func TestRefreshClosedWinsOverCachedMiss(t *testing.T) { | |||
| 746 | + | var calls atomic.Int64 | |||
| 747 | + | fetcher := refreshFetcherMock(t, func(_ context.Context, _ appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 748 | + | calls.Add(1) | |||
| 749 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |||
| 750 | + | }) | |||
| 751 | + | uc := refreshTestUC(t, fetcher, nil, nil) | |||
| 752 | + | ||||
| 753 | + | page := "https://example.com/a" | |||
| 754 | + | candidates := []string{"https://example.com/gone.ico"} | |||
| 755 | + | key, _, ok := refreshRequestKey(page, candidates) | |||
| 756 | + | require.True(t, ok) | |||
| 757 | + | uc.noteMiss(key, &appport.FaviconFetchError{StatusCode: 404}) | |||
| 758 | + | require.True(t, uc.cachedMiss(key)) | |||
| 759 | + | uc.Close() | |||
| 760 | + | err := uc.RefreshFromIconURLs(context.Background(), page, candidates) | |||
| 761 | + | require.ErrorIs(t, err, appport.ErrFaviconShutdown) | |||
| 762 | + | require.NotErrorIs(t, err, ErrFaviconMiss, "shutdown must stay distinct from a miss") | |||
| 763 | + | require.Zero(t, calls.Load(), "sealed refresh must not reach the fetcher") | |||
| 764 | + | } | |||
| 765 | + | ||||
| 766 | + | // TestCompactMissOrderDedupsReinsertedKeys proves compaction collapses | |||
| 767 | + | // stale duplicates: an evicted key that is reinserted keeps exactly one | |||
| 768 | + | // order entry, so the backlog cannot grow across expire/reinsert cycles. | |||
| 769 | + | func TestCompactMissOrderDedupsReinsertedKeys(t *testing.T) { | |||
| 770 | + | now := time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC) | |||
| 771 | + | uc := refreshTestUC(t, refreshFetcherMock(t, func(_ context.Context, _ appport.FaviconFetchRequest) (*appport.FaviconFetchedIcon, error) { | |||
| 772 | + | return nil, &appport.FaviconFetchError{StatusCode: 404} | |||
| 773 | + | }), &now, nil) | |||
| 774 | + | ||||
| 775 | + | uc.shared.mu.Lock() | |||
| 776 | + | uc.shared.misses["live"] = refreshMiss{expiresAt: now.Add(faviconMissTTL)} | |||
| 777 | + | // Simulate eviction (map entry removed, order name left behind) plus | |||
| 778 | + | // reinsertion: the same key appears twice in order. | |||
| 779 | + | uc.shared.order = []string{"stale-a", "live", "stale-b", "live"} | |||
| 780 | + | uc.compactMissOrderLocked() | |||
| 781 | + | order := append([]string(nil), uc.shared.order...) | |||
| 782 | + | uc.shared.mu.Unlock() | |||
| 783 | + | require.Equal(t, []string{"live"}, order, "compaction must keep one entry per live key") | |||
| 784 | + | } | |||
| 785 | + | ||||
| 786 | + | // TestRefreshRequestKey_IPv6DefaultPortEquivalence proves bracketed IPv6 | |||
| 787 | + | // hosts with an explicit default port share identity with the bare form: | |||
| 788 | + | // without bracket-aware parsing the two spellings hash differently and | |||
| 789 | + | // bypass in-flight sharing. | |||
| 790 | + | func TestRefreshRequestKey_IPv6DefaultPortEquivalence(t *testing.T) { | |||
| 791 | + | page := "https://[2001:db8::1]/a" | |||
| 792 | + | withPort, withPortAccepted, ok := refreshRequestKey(page, []string{"https://[2001:db8::1]:443/favicon.ico"}) | |||
| 793 | + | require.True(t, ok) | |||
| 794 | + | bare, bareAccepted, ok := refreshRequestKey(page, []string{"https://[2001:db8::1]/favicon.ico"}) | |||
| 795 | + | require.True(t, ok) | |||
| 796 | + | require.Equal(t, bare, withPort, "explicit default port must not change the request key") | |||
| 797 | + | require.Equal(t, bareAccepted, withPortAccepted) | |||
| 798 | + | } | |||
M internal/infrastructure/cef/content_injector.go +30 -0
| @@ -249,6 +249,36 @@ func (ci *contentInjector) RefreshScripts(ctx context.Context, wv port.WebView) | |||
| 249 | 249 | return nil | |
| 250 | 250 | } | |
| 251 | 251 | ||
| 252 | + | // onLoadEndForEvent installs scripts for a captured load-end event after | |
| 253 | + | // revalidating it on the GTK thread. The event identity comes from the | |
| 254 | + | // OnLoadEnd callback's browser (not mutable WebView state), so events that | |
| 255 | + | // are provably stale are skipped: a replaced browser identifier (old main | |
| 256 | + | // frame during process swap) or a superseded intent ID (a newer navigation | |
| 257 | + | // committed while the callback was queued). Ambiguous cases fall back to skipping rather than | |
| 258 | + | // installing blind: an event captured without a browser cannot prove | |
| 259 | + | // identity, and the current browser gets its own load-end installation. | |
| 260 | + | // The single snapshot bounds the residual check-then-install window to | |
| 261 | + | // nanoseconds, and any race there can only repeat idempotent scripts. | |
| 262 | + | // Repeated same-document load-ends still install. | |
| 263 | + | func (ci *contentInjector) onLoadEndForEvent(wv *WebView, event injectionEvent) { | |
| 264 | + | if wv == nil || ci == nil || event.browserID == noBrowserID { | |
| 265 | + | return | |
| 266 | + | } | |
| 267 | + | wv.mu.RLock() | |
| 268 | + | browser, currentIntent := wv.browser, wv.pendingIntentID | |
| 269 | + | wv.mu.RUnlock() | |
| 270 | + | if browser == nil { | |
| 271 | + | return | |
| 272 | + | } | |
| 273 | + | if browser.GetIdentifier() != event.browserID { | |
| 274 | + | return | |
| 275 | + | } | |
| 276 | + | if currentIntent != event.intentID { | |
| 277 | + | return | |
| 278 | + | } | |
| 279 | + | ci.onLoadEnd(wv) | |
| 280 | + | } | |
| 281 | + | ||
| 252 | 282 | // onLoadEnd is called from the load handler after a page finishes loading. | |
| 253 | 283 | // It injects the appropriate scripts based on whether the page is internal. | |
| 254 | 284 | func (ci *contentInjector) onLoadEnd(wv *WebView) { | |
| @@ -249,6 +249,36 @@ func (ci *contentInjector) RefreshScripts(ctx context.Context, wv port.WebView) | |||||
| 249 | return nil | 249 | return nil | ||
| 250 | } | 250 | } | ||
| 251 | 251 | ||||
| 252 | + | // onLoadEndForEvent installs scripts for a captured load-end event after | |||
| 253 | + | // revalidating it on the GTK thread. The event identity comes from the | |||
| 254 | + | // OnLoadEnd callback's browser (not mutable WebView state), so events that | |||
| 255 | + | // are provably stale are skipped: a replaced browser identifier (old main | |||
| 256 | + | // frame during process swap) or a superseded intent ID (a newer navigation | |||
| 257 | + | // committed while the callback was queued). Ambiguous cases fall back to skipping rather than | |||
| 258 | + | // installing blind: an event captured without a browser cannot prove | |||
| 259 | + | // identity, and the current browser gets its own load-end installation. | |||
| 260 | + | // The single snapshot bounds the residual check-then-install window to | |||
| 261 | + | // nanoseconds, and any race there can only repeat idempotent scripts. | |||
| 262 | + | // Repeated same-document load-ends still install. | |||
| 263 | + | func (ci *contentInjector) onLoadEndForEvent(wv *WebView, event injectionEvent) { | |||
| 264 | + | if wv == nil || ci == nil || event.browserID == noBrowserID { | |||
| 265 | + | return | |||
| 266 | + | } | |||
| 267 | + | wv.mu.RLock() | |||
| 268 | + | browser, currentIntent := wv.browser, wv.pendingIntentID | |||
| 269 | + | wv.mu.RUnlock() | |||
| 270 | + | if browser == nil { | |||
| 271 | + | return | |||
| 272 | + | } | |||
| 273 | + | if browser.GetIdentifier() != event.browserID { | |||
| 274 | + | return | |||
| 275 | + | } | |||
| 276 | + | if currentIntent != event.intentID { | |||
| 277 | + | return | |||
| 278 | + | } | |||
| 279 | + | ci.onLoadEnd(wv) | |||
| 280 | + | } | |||
| 281 | + | ||||
| 252 | // onLoadEnd is called from the load handler after a page finishes loading. | 282 | // onLoadEnd is called from the load handler after a page finishes loading. | ||
| 253 | // It injects the appropriate scripts based on whether the page is internal. | 283 | // It injects the appropriate scripts based on whether the page is internal. | ||
| 254 | func (ci *contentInjector) onLoadEnd(wv *WebView) { | 284 | func (ci *contentInjector) onLoadEnd(wv *WebView) { | ||
M internal/infrastructure/cef/engine_init_test.go +16 -0
| @@ -62,6 +62,22 @@ func TestPrepareCEFSettings_UsesResolvedProfilePaths(t *testing.T) { | |||
| 62 | 62 | require.Empty(t, settings.BrowserSubprocessPath) | |
| 63 | 63 | } | |
| 64 | 64 | ||
| 65 | + | // TestPrepareCEFSettings_LeavesCachePathToCEFDefault locks the P3 cache | |
| 66 | + | // contract: only RootCachePath is configured and CachePath stays empty, so | |
| 67 | + | // the global request context runs incognito-style with in-memory storage. | |
| 68 | + | // RootCachePath does not supply CachePath and grants no persistent | |
| 69 | + | // localStorage. Setting an explicit CachePath requires the P3.3 | |
| 70 | + | // compatibility decision with data-preservation tests; see | |
| 71 | + | // docs/cef-cache-contract.md. | |
| 72 | + | func TestPrepareCEFSettings_LeavesCachePathToCEFDefault(t *testing.T) { | |
| 73 | + | logger := zerolog.Nop() | |
| 74 | + | profile := testCEFDevProfile(t) | |
| 75 | + | settings, err := prepareCEFSettings(port.EngineOptions{}, RuntimePaths{StateRoot: profile.CEFUserDataDir(), LogFile: profile.CEFLogFile()}, RuntimeConfig{}, &logger) | |
| 76 | + | require.NoError(t, err) | |
| 77 | + | require.NotEmpty(t, settings.RootCachePath) | |
| 78 | + | require.Empty(t, settings.CachePath, "explicit CachePath needs the P3.3 decision; empty CachePath means in-memory global context") | |
| 79 | + | } | |
| 80 | + | ||
| 65 | 81 | func TestParseLoadedLibCEFPath(t *testing.T) { | |
| 66 | 82 | maps := "7f5fd4000000-7f5fd4200000 r--p 00000000 00:00 0 /usr/lib/libvulkan.so.1\n" + | |
| 67 | 83 | "7f5fe0000000-7f5fe8200000 r-xp 00000000 00:00 0 /opt/cef-vaapi-runtime/usr/lib/cef/libcef.so\n" + | |
| @@ -62,6 +62,22 @@ func TestPrepareCEFSettings_UsesResolvedProfilePaths(t *testing.T) { | |||||
| 62 | require.Empty(t, settings.BrowserSubprocessPath) | 62 | require.Empty(t, settings.BrowserSubprocessPath) | ||
| 63 | } | 63 | } | ||
| 64 | 64 | ||||
| 65 | + | // TestPrepareCEFSettings_LeavesCachePathToCEFDefault locks the P3 cache | |||
| 66 | + | // contract: only RootCachePath is configured and CachePath stays empty, so | |||
| 67 | + | // the global request context runs incognito-style with in-memory storage. | |||
| 68 | + | // RootCachePath does not supply CachePath and grants no persistent | |||
| 69 | + | // localStorage. Setting an explicit CachePath requires the P3.3 | |||
| 70 | + | // compatibility decision with data-preservation tests; see | |||
| 71 | + | // docs/cef-cache-contract.md. | |||
| 72 | + | func TestPrepareCEFSettings_LeavesCachePathToCEFDefault(t *testing.T) { | |||
| 73 | + | logger := zerolog.Nop() | |||
| 74 | + | profile := testCEFDevProfile(t) | |||
| 75 | + | settings, err := prepareCEFSettings(port.EngineOptions{}, RuntimePaths{StateRoot: profile.CEFUserDataDir(), LogFile: profile.CEFLogFile()}, RuntimeConfig{}, &logger) | |||
| 76 | + | require.NoError(t, err) | |||
| 77 | + | require.NotEmpty(t, settings.RootCachePath) | |||
| 78 | + | require.Empty(t, settings.CachePath, "explicit CachePath needs the P3.3 decision; empty CachePath means in-memory global context") | |||
| 79 | + | } | |||
| 80 | + | ||||
| 65 | func TestParseLoadedLibCEFPath(t *testing.T) { | 81 | func TestParseLoadedLibCEFPath(t *testing.T) { | ||
| 66 | maps := "7f5fd4000000-7f5fd4200000 r--p 00000000 00:00 0 /usr/lib/libvulkan.so.1\n" + | 82 | maps := "7f5fd4000000-7f5fd4200000 r--p 00000000 00:00 0 /usr/lib/libvulkan.so.1\n" + | ||
| 67 | "7f5fe0000000-7f5fe8200000 r-xp 00000000 00:00 0 /opt/cef-vaapi-runtime/usr/lib/cef/libcef.so\n" + | 83 | "7f5fe0000000-7f5fe8200000 r-xp 00000000 00:00 0 /opt/cef-vaapi-runtime/usr/lib/cef/libcef.so\n" + | ||
A internal/infrastructure/cef/experiment_surface_test.go +23 -0
| @@ -0,0 +1,23 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 5 | + | ) | |
| 6 | + | ||
| 7 | + | // Compile-time pins for the Plan 04 experiment surfaces. These assert the | |
| 8 | + | // selected binding exposes the exact API shape the display-side probes were | |
| 9 | + | // designed against, without initializing CEF. Evaluating a method | |
| 10 | + | // expression performs no call. | |
| 11 | + | // | |
| 12 | + | // P3 cache probe: RequestContextGetGlobalContext, | |
| 13 | + | // BrowserHost.GetRequestContext, RequestContext.GetCachePath. | |
| 14 | + | // P5 DNS experiment: RequestContext.ResolveHost. Note the fire-and-forget | |
| 15 | + | // signature: ResolveHost carries no cancellation handle, so the experiment | |
| 16 | + | // design must recheck selection eligibility before issuance and only ignore | |
| 17 | + | // stale completions afterwards. | |
| 18 | + | var ( | |
| 19 | + | _ func() purecef.RequestContext = purecef.RequestContextGetGlobalContext | |
| 20 | + | _ func(purecef.BrowserHost) purecef.RequestContext = purecef.BrowserHost.GetRequestContext | |
| 21 | + | _ func(purecef.RequestContext) string = purecef.RequestContext.GetCachePath | |
| 22 | + | _ func(purecef.RequestContext, string, purecef.ResolveCallback) = purecef.RequestContext.ResolveHost | |
| 23 | + | ) | |
| @@ -0,0 +1,23 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 5 | + | ) | |||
| 6 | + | ||||
| 7 | + | // Compile-time pins for the Plan 04 experiment surfaces. These assert the | |||
| 8 | + | // selected binding exposes the exact API shape the display-side probes were | |||
| 9 | + | // designed against, without initializing CEF. Evaluating a method | |||
| 10 | + | // expression performs no call. | |||
| 11 | + | // | |||
| 12 | + | // P3 cache probe: RequestContextGetGlobalContext, | |||
| 13 | + | // BrowserHost.GetRequestContext, RequestContext.GetCachePath. | |||
| 14 | + | // P5 DNS experiment: RequestContext.ResolveHost. Note the fire-and-forget | |||
| 15 | + | // signature: ResolveHost carries no cancellation handle, so the experiment | |||
| 16 | + | // design must recheck selection eligibility before issuance and only ignore | |||
| 17 | + | // stale completions afterwards. | |||
| 18 | + | var ( | |||
| 19 | + | _ func() purecef.RequestContext = purecef.RequestContextGetGlobalContext | |||
| 20 | + | _ func(purecef.BrowserHost) purecef.RequestContext = purecef.BrowserHost.GetRequestContext | |||
| 21 | + | _ func(purecef.RequestContext) string = purecef.RequestContext.GetCachePath | |||
| 22 | + | _ func(purecef.RequestContext, string, purecef.ResolveCallback) = purecef.RequestContext.ResolveHost | |||
| 23 | + | ) | |||
A internal/infrastructure/cef/filter_request_handler.go +65 -0
| @@ -0,0 +1,65 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync/atomic" | |
| 5 | + | ||
| 6 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/application/port" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | // filterRequestHandler adapts a port.ContentFilter to CEF resource loading. | |
| 12 | + | // This is the test-only feasibility seam for the filtering gate: it is | |
| 13 | + | // deliberately NOT wired into client/handler creation, so it has no | |
| 14 | + | // production effect. Decisions are synchronous; asynchronous rule loading, | |
| 15 | + | // atomic updates, and corruption recovery belong to the separately approved | |
| 16 | + | // production design. | |
| 17 | + | type filterRequestHandler struct { | |
| 18 | + | filter port.ContentFilter | |
| 19 | + | closed atomic.Bool | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | // newFilterRequestHandler builds the seam adapter around matcher. A nil | |
| 23 | + | // matcher allows everything. | |
| 24 | + | func newFilterRequestHandler(matcher port.ContentFilter) *filterRequestHandler { | |
| 25 | + | return &filterRequestHandler{filter: matcher} | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | // Close retires the seam: later requests are allowed without consulting the | |
| 29 | + | // matcher (fail-open is correct for a test seam; production UX for a | |
| 30 | + | // missing/errored policy belongs to the follow-up design). | |
| 31 | + | func (h *filterRequestHandler) Close() { | |
| 32 | + | if h == nil { | |
| 33 | + | return | |
| 34 | + | } | |
| 35 | + | h.closed.Store(true) | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | // OnBeforeResourceLoad maps one filter decision onto CEF's synchronous | |
| 39 | + | // return value. Unclassifiable requests (nil request/empty URL) and a | |
| 40 | + | // retired/nil matcher allow the request. The CEF continuation callback is | |
| 41 | + | // untouched: this seam never defers decisions asynchronously. | |
| 42 | + | func (h *filterRequestHandler) OnBeforeResourceLoad( | |
| 43 | + | browser purecef.Browser, frame purecef.Frame, request purecef.Request, _ purecef.Callback, | |
| 44 | + | ) purecef.ReturnValue { | |
| 45 | + | if h == nil || h.closed.Load() || h.filter == nil || request == nil { | |
| 46 | + | return purecef.ReturnValueRvContinue | |
| 47 | + | } | |
| 48 | + | var isMain bool | |
| 49 | + | if frame != nil { | |
| 50 | + | isMain = frame.IsMain() | |
| 51 | + | } | |
| 52 | + | url := request.GetURL() | |
| 53 | + | if url == "" { | |
| 54 | + | return purecef.ReturnValueRvContinue | |
| 55 | + | } | |
| 56 | + | // Document loads are main/sub-frame resource types; everything else | |
| 57 | + | // is a subresource of the current document. | |
| 58 | + | resourceType := request.GetResourceType() | |
| 59 | + | isNavigation := resourceType == purecef.ResourceTypeRtMainFrame || resourceType == purecef.ResourceTypeRtSubFrame | |
| 60 | + | _ = browser | |
| 61 | + | if h.filter.Decide(port.FilterRequest{URL: url, IsMainFrame: isMain, IsNavigation: isNavigation}) == port.FilterBlock { | |
| 62 | + | return purecef.ReturnValueRvCancel | |
| 63 | + | } | |
| 64 | + | return purecef.ReturnValueRvContinue | |
| 65 | + | } | |
| @@ -0,0 +1,65 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync/atomic" | |||
| 5 | + | ||||
| 6 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | // filterRequestHandler adapts a port.ContentFilter to CEF resource loading. | |||
| 12 | + | // This is the test-only feasibility seam for the filtering gate: it is | |||
| 13 | + | // deliberately NOT wired into client/handler creation, so it has no | |||
| 14 | + | // production effect. Decisions are synchronous; asynchronous rule loading, | |||
| 15 | + | // atomic updates, and corruption recovery belong to the separately approved | |||
| 16 | + | // production design. | |||
| 17 | + | type filterRequestHandler struct { | |||
| 18 | + | filter port.ContentFilter | |||
| 19 | + | closed atomic.Bool | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | // newFilterRequestHandler builds the seam adapter around matcher. A nil | |||
| 23 | + | // matcher allows everything. | |||
| 24 | + | func newFilterRequestHandler(matcher port.ContentFilter) *filterRequestHandler { | |||
| 25 | + | return &filterRequestHandler{filter: matcher} | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | // Close retires the seam: later requests are allowed without consulting the | |||
| 29 | + | // matcher (fail-open is correct for a test seam; production UX for a | |||
| 30 | + | // missing/errored policy belongs to the follow-up design). | |||
| 31 | + | func (h *filterRequestHandler) Close() { | |||
| 32 | + | if h == nil { | |||
| 33 | + | return | |||
| 34 | + | } | |||
| 35 | + | h.closed.Store(true) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | // OnBeforeResourceLoad maps one filter decision onto CEF's synchronous | |||
| 39 | + | // return value. Unclassifiable requests (nil request/empty URL) and a | |||
| 40 | + | // retired/nil matcher allow the request. The CEF continuation callback is | |||
| 41 | + | // untouched: this seam never defers decisions asynchronously. | |||
| 42 | + | func (h *filterRequestHandler) OnBeforeResourceLoad( | |||
| 43 | + | browser purecef.Browser, frame purecef.Frame, request purecef.Request, _ purecef.Callback, | |||
| 44 | + | ) purecef.ReturnValue { | |||
| 45 | + | if h == nil || h.closed.Load() || h.filter == nil || request == nil { | |||
| 46 | + | return purecef.ReturnValueRvContinue | |||
| 47 | + | } | |||
| 48 | + | var isMain bool | |||
| 49 | + | if frame != nil { | |||
| 50 | + | isMain = frame.IsMain() | |||
| 51 | + | } | |||
| 52 | + | url := request.GetURL() | |||
| 53 | + | if url == "" { | |||
| 54 | + | return purecef.ReturnValueRvContinue | |||
| 55 | + | } | |||
| 56 | + | // Document loads are main/sub-frame resource types; everything else | |||
| 57 | + | // is a subresource of the current document. | |||
| 58 | + | resourceType := request.GetResourceType() | |||
| 59 | + | isNavigation := resourceType == purecef.ResourceTypeRtMainFrame || resourceType == purecef.ResourceTypeRtSubFrame | |||
| 60 | + | _ = browser | |||
| 61 | + | if h.filter.Decide(port.FilterRequest{URL: url, IsMainFrame: isMain, IsNavigation: isNavigation}) == port.FilterBlock { | |||
| 62 | + | return purecef.ReturnValueRvCancel | |||
| 63 | + | } | |||
| 64 | + | return purecef.ReturnValueRvContinue | |||
| 65 | + | } | |||
A internal/infrastructure/cef/filter_request_handler_test.go +172 -0
| @@ -0,0 +1,172 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 8 | + | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |
| 9 | + | "github.com/stretchr/testify/require" | |
| 10 | + | ||
| 11 | + | "github.com/bnema/dumber/internal/application/port" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // listMatcher is the P4.2 fake matcher: synthetic block-list decisions over | |
| 15 | + | // loopback fixtures, recording every offered request. | |
| 16 | + | type listMatcher struct { | |
| 17 | + | mu sync.Mutex | |
| 18 | + | blocked map[string]bool | |
| 19 | + | seen []port.FilterRequest | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | func (m *listMatcher) Decide(req port.FilterRequest) port.FilterAction { | |
| 23 | + | m.mu.Lock() | |
| 24 | + | defer m.mu.Unlock() | |
| 25 | + | m.seen = append(m.seen, req) | |
| 26 | + | if m.blocked[req.URL] { | |
| 27 | + | return port.FilterBlock | |
| 28 | + | } | |
| 29 | + | return port.FilterAllow | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | func (m *listMatcher) seenCount() int { | |
| 33 | + | m.mu.Lock() | |
| 34 | + | defer m.mu.Unlock() | |
| 35 | + | return len(m.seen) | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | func filterRequest(t *testing.T, url string, resourceType purecef.ResourceType, isMain bool) (purecef.Frame, purecef.Request) { | |
| 39 | + | t.Helper() | |
| 40 | + | frame := cefmocks.NewMockFrame(t) | |
| 41 | + | frame.EXPECT().IsMain().Return(isMain).Maybe() | |
| 42 | + | request := cefmocks.NewMockRequest(t) | |
| 43 | + | request.EXPECT().GetURL().Return(url).Maybe() | |
| 44 | + | request.EXPECT().GetResourceType().Return(resourceType).Maybe() | |
| 45 | + | return frame, request | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | // TestFilterSeam_AllowsMainDocument verifies the baseline: an unlisted main | |
| 49 | + | // document proceeds. | |
| 50 | + | func TestFilterSeam_AllowsMainDocument(t *testing.T) { | |
| 51 | + | matcher := &listMatcher{blocked: map[string]bool{}} | |
| 52 | + | handler := newFilterRequestHandler(matcher) | |
| 53 | + | ||
| 54 | + | frame, request := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |
| 55 | + | rv := handler.OnBeforeResourceLoad(nil, frame, request, nil) | |
| 56 | + | require.Equal(t, purecef.ReturnValueRvContinue, rv) | |
| 57 | + | require.Equal(t, 1, matcher.seenCount()) | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | // TestFilterSeam_BlocksListedSubresource verifies subresource blocking while | |
| 61 | + | // the document itself stays allowed. | |
| 62 | + | func TestFilterSeam_BlocksListedSubresource(t *testing.T) { | |
| 63 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/tracker.js": true}} | |
| 64 | + | handler := newFilterRequestHandler(matcher) | |
| 65 | + | ||
| 66 | + | frame, _ := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |
| 67 | + | require.Equal(t, purecef.ReturnValueRvContinue, | |
| 68 | + | handler.OnBeforeResourceLoad(nil, frame, mustFilterRequest(t, "http://127.0.0.1:8471/app.js", purecef.ResourceTypeRtScript), nil)) | |
| 69 | + | ||
| 70 | + | _, blocked := filterRequest(t, "http://127.0.0.1:8471/tracker.js", purecef.ResourceTypeRtScript, false) | |
| 71 | + | require.Equal(t, purecef.ReturnValueRvCancel, | |
| 72 | + | handler.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func mustFilterRequest(t *testing.T, url string, resourceType purecef.ResourceType) purecef.Request { | |
| 76 | + | t.Helper() | |
| 77 | + | _, request := filterRequest(t, url, resourceType, false) | |
| 78 | + | return request | |
| 79 | + | } | |
| 80 | + | ||
| 81 | + | // TestFilterSeam_ClassifiesNavigationVsSubresource proves main/sub-frame | |
| 82 | + | // document loads arrive as navigations and scripts as subresources. | |
| 83 | + | func TestFilterSeam_ClassifiesNavigationVsSubresource(t *testing.T) { | |
| 84 | + | var got []port.FilterRequest | |
| 85 | + | matcher := &listMatcher{blocked: map[string]bool{}} | |
| 86 | + | handler := newFilterRequestHandler(recordingMatcher(matcher, &got)) | |
| 87 | + | ||
| 88 | + | frame, mainReq := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |
| 89 | + | handler.OnBeforeResourceLoad(nil, frame, mainReq, nil) | |
| 90 | + | // Page scripts belong to the main frame yet are subresources: frame | |
| 91 | + | // identity and navigation classification are independent axes. | |
| 92 | + | handler.OnBeforeResourceLoad(nil, frame, mustFilterRequest(t, "http://127.0.0.1:8471/app.js", purecef.ResourceTypeRtScript), nil) | |
| 93 | + | subFrame, _ := filterRequest(t, "http://127.0.0.1:8471/frame", purecef.ResourceTypeRtSubFrame, false) | |
| 94 | + | _, frameReq := filterRequest(t, "http://127.0.0.1:8471/frame", purecef.ResourceTypeRtSubFrame, false) | |
| 95 | + | handler.OnBeforeResourceLoad(nil, subFrame, frameReq, nil) | |
| 96 | + | ||
| 97 | + | require.Len(t, got, 3) | |
| 98 | + | require.True(t, got[0].IsMainFrame && got[0].IsNavigation, "main document must be a main-frame navigation") | |
| 99 | + | require.True(t, got[1].IsMainFrame, "page scripts belong to the main frame") | |
| 100 | + | require.False(t, got[1].IsNavigation, "scripts must be subresources") | |
| 101 | + | require.True(t, got[2].IsNavigation, "sub-frame documents are navigations") | |
| 102 | + | require.False(t, got[2].IsMainFrame) | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | func recordingMatcher(inner *listMatcher, out *[]port.FilterRequest) port.ContentFilter { | |
| 106 | + | return filterFunc(func(req port.FilterRequest) port.FilterAction { | |
| 107 | + | *out = append(*out, req) | |
| 108 | + | return inner.Decide(req) | |
| 109 | + | }) | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | type filterFunc func(port.FilterRequest) port.FilterAction | |
| 113 | + | ||
| 114 | + | func (f filterFunc) Decide(req port.FilterRequest) port.FilterAction { return f(req) } | |
| 115 | + | ||
| 116 | + | // TestFilterSeam_RedirectTargetEvaluatedIndependently verifies each request | |
| 117 | + | // in a redirect chain is offered separately: blocking the redirect target | |
| 118 | + | // cancels it without retroactively affecting the source decision. | |
| 119 | + | func TestFilterSeam_RedirectTargetEvaluatedIndependently(t *testing.T) { | |
| 120 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/target": true}} | |
| 121 | + | handler := newFilterRequestHandler(matcher) | |
| 122 | + | ||
| 123 | + | frame, source := filterRequest(t, "http://127.0.0.1:8471/redirect", purecef.ResourceTypeRtMainFrame, true) | |
| 124 | + | require.Equal(t, purecef.ReturnValueRvContinue, | |
| 125 | + | handler.OnBeforeResourceLoad(nil, frame, source, nil)) | |
| 126 | + | _, target := filterRequest(t, "http://127.0.0.1:8471/target", purecef.ResourceTypeRtMainFrame, true) | |
| 127 | + | require.Equal(t, purecef.ReturnValueRvCancel, | |
| 128 | + | handler.OnBeforeResourceLoad(nil, frame, target, nil)) | |
| 129 | + | require.Equal(t, 2, matcher.seenCount()) | |
| 130 | + | } | |
| 131 | + | ||
| 132 | + | // TestFilterSeam_UnclassifiableAllows verifies fail-open guards: nil | |
| 133 | + | // request, empty URL, nil frame, and nil matcher never cancel. | |
| 134 | + | func TestFilterSeam_UnclassifiableAllows(t *testing.T) { | |
| 135 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/x": true}} | |
| 136 | + | handler := newFilterRequestHandler(matcher) | |
| 137 | + | ||
| 138 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, nil, nil, nil)) | |
| 139 | + | ||
| 140 | + | // A nil frame does not make a listed URL unclassifiable: the URL | |
| 141 | + | // decision still applies. | |
| 142 | + | require.Equal(t, purecef.ReturnValueRvCancel, handler.OnBeforeResourceLoad(nil, nil, mustFilterRequest(t, "http://127.0.0.1:8471/x", purecef.ResourceTypeRtScript), nil)) | |
| 143 | + | ||
| 144 | + | frame, _ := filterRequest(t, "http://127.0.0.1:8471/x", purecef.ResourceTypeRtScript, false) | |
| 145 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, frame, nil, nil)) | |
| 146 | + | ||
| 147 | + | emptyURL := cefmocks.NewMockRequest(t) | |
| 148 | + | emptyURL.EXPECT().GetURL().Return("").Once() | |
| 149 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, frame, emptyURL, nil)) | |
| 150 | + | ||
| 151 | + | allowAll := newFilterRequestHandler(nil) | |
| 152 | + | _, blocked := filterRequest(t, "http://127.0.0.1:8471/x", purecef.ResourceTypeRtScript, false) | |
| 153 | + | require.Equal(t, purecef.ReturnValueRvContinue, allowAll.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |
| 154 | + | require.Equal(t, purecef.ReturnValueRvContinue, (*filterRequestHandler)(nil).OnBeforeResourceLoad(nil, frame, blocked, nil)) | |
| 155 | + | } | |
| 156 | + | ||
| 157 | + | // TestFilterSeam_CloseRetiresMatcher verifies shutdown semantics: after | |
| 158 | + | // Close the matcher is no longer consulted and everything is allowed. | |
| 159 | + | func TestFilterSeam_CloseRetiresMatcher(t *testing.T) { | |
| 160 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/tracker.js": true}} | |
| 161 | + | handler := newFilterRequestHandler(matcher) | |
| 162 | + | ||
| 163 | + | _, blocked := filterRequest(t, "http://127.0.0.1:8471/tracker.js", purecef.ResourceTypeRtScript, false) | |
| 164 | + | frame, _ := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |
| 165 | + | require.Equal(t, purecef.ReturnValueRvCancel, handler.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |
| 166 | + | before := matcher.seenCount() | |
| 167 | + | ||
| 168 | + | handler.Close() | |
| 169 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |
| 170 | + | require.Equal(t, before, matcher.seenCount(), "retired matcher must not be consulted") | |
| 171 | + | handler.Close() | |
| 172 | + | } | |
| @@ -0,0 +1,172 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 8 | + | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |||
| 9 | + | "github.com/stretchr/testify/require" | |||
| 10 | + | ||||
| 11 | + | "github.com/bnema/dumber/internal/application/port" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // listMatcher is the P4.2 fake matcher: synthetic block-list decisions over | |||
| 15 | + | // loopback fixtures, recording every offered request. | |||
| 16 | + | type listMatcher struct { | |||
| 17 | + | mu sync.Mutex | |||
| 18 | + | blocked map[string]bool | |||
| 19 | + | seen []port.FilterRequest | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | func (m *listMatcher) Decide(req port.FilterRequest) port.FilterAction { | |||
| 23 | + | m.mu.Lock() | |||
| 24 | + | defer m.mu.Unlock() | |||
| 25 | + | m.seen = append(m.seen, req) | |||
| 26 | + | if m.blocked[req.URL] { | |||
| 27 | + | return port.FilterBlock | |||
| 28 | + | } | |||
| 29 | + | return port.FilterAllow | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | func (m *listMatcher) seenCount() int { | |||
| 33 | + | m.mu.Lock() | |||
| 34 | + | defer m.mu.Unlock() | |||
| 35 | + | return len(m.seen) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | func filterRequest(t *testing.T, url string, resourceType purecef.ResourceType, isMain bool) (purecef.Frame, purecef.Request) { | |||
| 39 | + | t.Helper() | |||
| 40 | + | frame := cefmocks.NewMockFrame(t) | |||
| 41 | + | frame.EXPECT().IsMain().Return(isMain).Maybe() | |||
| 42 | + | request := cefmocks.NewMockRequest(t) | |||
| 43 | + | request.EXPECT().GetURL().Return(url).Maybe() | |||
| 44 | + | request.EXPECT().GetResourceType().Return(resourceType).Maybe() | |||
| 45 | + | return frame, request | |||
| 46 | + | } | |||
| 47 | + | ||||
| 48 | + | // TestFilterSeam_AllowsMainDocument verifies the baseline: an unlisted main | |||
| 49 | + | // document proceeds. | |||
| 50 | + | func TestFilterSeam_AllowsMainDocument(t *testing.T) { | |||
| 51 | + | matcher := &listMatcher{blocked: map[string]bool{}} | |||
| 52 | + | handler := newFilterRequestHandler(matcher) | |||
| 53 | + | ||||
| 54 | + | frame, request := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |||
| 55 | + | rv := handler.OnBeforeResourceLoad(nil, frame, request, nil) | |||
| 56 | + | require.Equal(t, purecef.ReturnValueRvContinue, rv) | |||
| 57 | + | require.Equal(t, 1, matcher.seenCount()) | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | // TestFilterSeam_BlocksListedSubresource verifies subresource blocking while | |||
| 61 | + | // the document itself stays allowed. | |||
| 62 | + | func TestFilterSeam_BlocksListedSubresource(t *testing.T) { | |||
| 63 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/tracker.js": true}} | |||
| 64 | + | handler := newFilterRequestHandler(matcher) | |||
| 65 | + | ||||
| 66 | + | frame, _ := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |||
| 67 | + | require.Equal(t, purecef.ReturnValueRvContinue, | |||
| 68 | + | handler.OnBeforeResourceLoad(nil, frame, mustFilterRequest(t, "http://127.0.0.1:8471/app.js", purecef.ResourceTypeRtScript), nil)) | |||
| 69 | + | ||||
| 70 | + | _, blocked := filterRequest(t, "http://127.0.0.1:8471/tracker.js", purecef.ResourceTypeRtScript, false) | |||
| 71 | + | require.Equal(t, purecef.ReturnValueRvCancel, | |||
| 72 | + | handler.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func mustFilterRequest(t *testing.T, url string, resourceType purecef.ResourceType) purecef.Request { | |||
| 76 | + | t.Helper() | |||
| 77 | + | _, request := filterRequest(t, url, resourceType, false) | |||
| 78 | + | return request | |||
| 79 | + | } | |||
| 80 | + | ||||
| 81 | + | // TestFilterSeam_ClassifiesNavigationVsSubresource proves main/sub-frame | |||
| 82 | + | // document loads arrive as navigations and scripts as subresources. | |||
| 83 | + | func TestFilterSeam_ClassifiesNavigationVsSubresource(t *testing.T) { | |||
| 84 | + | var got []port.FilterRequest | |||
| 85 | + | matcher := &listMatcher{blocked: map[string]bool{}} | |||
| 86 | + | handler := newFilterRequestHandler(recordingMatcher(matcher, &got)) | |||
| 87 | + | ||||
| 88 | + | frame, mainReq := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |||
| 89 | + | handler.OnBeforeResourceLoad(nil, frame, mainReq, nil) | |||
| 90 | + | // Page scripts belong to the main frame yet are subresources: frame | |||
| 91 | + | // identity and navigation classification are independent axes. | |||
| 92 | + | handler.OnBeforeResourceLoad(nil, frame, mustFilterRequest(t, "http://127.0.0.1:8471/app.js", purecef.ResourceTypeRtScript), nil) | |||
| 93 | + | subFrame, _ := filterRequest(t, "http://127.0.0.1:8471/frame", purecef.ResourceTypeRtSubFrame, false) | |||
| 94 | + | _, frameReq := filterRequest(t, "http://127.0.0.1:8471/frame", purecef.ResourceTypeRtSubFrame, false) | |||
| 95 | + | handler.OnBeforeResourceLoad(nil, subFrame, frameReq, nil) | |||
| 96 | + | ||||
| 97 | + | require.Len(t, got, 3) | |||
| 98 | + | require.True(t, got[0].IsMainFrame && got[0].IsNavigation, "main document must be a main-frame navigation") | |||
| 99 | + | require.True(t, got[1].IsMainFrame, "page scripts belong to the main frame") | |||
| 100 | + | require.False(t, got[1].IsNavigation, "scripts must be subresources") | |||
| 101 | + | require.True(t, got[2].IsNavigation, "sub-frame documents are navigations") | |||
| 102 | + | require.False(t, got[2].IsMainFrame) | |||
| 103 | + | } | |||
| 104 | + | ||||
| 105 | + | func recordingMatcher(inner *listMatcher, out *[]port.FilterRequest) port.ContentFilter { | |||
| 106 | + | return filterFunc(func(req port.FilterRequest) port.FilterAction { | |||
| 107 | + | *out = append(*out, req) | |||
| 108 | + | return inner.Decide(req) | |||
| 109 | + | }) | |||
| 110 | + | } | |||
| 111 | + | ||||
| 112 | + | type filterFunc func(port.FilterRequest) port.FilterAction | |||
| 113 | + | ||||
| 114 | + | func (f filterFunc) Decide(req port.FilterRequest) port.FilterAction { return f(req) } | |||
| 115 | + | ||||
| 116 | + | // TestFilterSeam_RedirectTargetEvaluatedIndependently verifies each request | |||
| 117 | + | // in a redirect chain is offered separately: blocking the redirect target | |||
| 118 | + | // cancels it without retroactively affecting the source decision. | |||
| 119 | + | func TestFilterSeam_RedirectTargetEvaluatedIndependently(t *testing.T) { | |||
| 120 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/target": true}} | |||
| 121 | + | handler := newFilterRequestHandler(matcher) | |||
| 122 | + | ||||
| 123 | + | frame, source := filterRequest(t, "http://127.0.0.1:8471/redirect", purecef.ResourceTypeRtMainFrame, true) | |||
| 124 | + | require.Equal(t, purecef.ReturnValueRvContinue, | |||
| 125 | + | handler.OnBeforeResourceLoad(nil, frame, source, nil)) | |||
| 126 | + | _, target := filterRequest(t, "http://127.0.0.1:8471/target", purecef.ResourceTypeRtMainFrame, true) | |||
| 127 | + | require.Equal(t, purecef.ReturnValueRvCancel, | |||
| 128 | + | handler.OnBeforeResourceLoad(nil, frame, target, nil)) | |||
| 129 | + | require.Equal(t, 2, matcher.seenCount()) | |||
| 130 | + | } | |||
| 131 | + | ||||
| 132 | + | // TestFilterSeam_UnclassifiableAllows verifies fail-open guards: nil | |||
| 133 | + | // request, empty URL, nil frame, and nil matcher never cancel. | |||
| 134 | + | func TestFilterSeam_UnclassifiableAllows(t *testing.T) { | |||
| 135 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/x": true}} | |||
| 136 | + | handler := newFilterRequestHandler(matcher) | |||
| 137 | + | ||||
| 138 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, nil, nil, nil)) | |||
| 139 | + | ||||
| 140 | + | // A nil frame does not make a listed URL unclassifiable: the URL | |||
| 141 | + | // decision still applies. | |||
| 142 | + | require.Equal(t, purecef.ReturnValueRvCancel, handler.OnBeforeResourceLoad(nil, nil, mustFilterRequest(t, "http://127.0.0.1:8471/x", purecef.ResourceTypeRtScript), nil)) | |||
| 143 | + | ||||
| 144 | + | frame, _ := filterRequest(t, "http://127.0.0.1:8471/x", purecef.ResourceTypeRtScript, false) | |||
| 145 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, frame, nil, nil)) | |||
| 146 | + | ||||
| 147 | + | emptyURL := cefmocks.NewMockRequest(t) | |||
| 148 | + | emptyURL.EXPECT().GetURL().Return("").Once() | |||
| 149 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, frame, emptyURL, nil)) | |||
| 150 | + | ||||
| 151 | + | allowAll := newFilterRequestHandler(nil) | |||
| 152 | + | _, blocked := filterRequest(t, "http://127.0.0.1:8471/x", purecef.ResourceTypeRtScript, false) | |||
| 153 | + | require.Equal(t, purecef.ReturnValueRvContinue, allowAll.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |||
| 154 | + | require.Equal(t, purecef.ReturnValueRvContinue, (*filterRequestHandler)(nil).OnBeforeResourceLoad(nil, frame, blocked, nil)) | |||
| 155 | + | } | |||
| 156 | + | ||||
| 157 | + | // TestFilterSeam_CloseRetiresMatcher verifies shutdown semantics: after | |||
| 158 | + | // Close the matcher is no longer consulted and everything is allowed. | |||
| 159 | + | func TestFilterSeam_CloseRetiresMatcher(t *testing.T) { | |||
| 160 | + | matcher := &listMatcher{blocked: map[string]bool{"http://127.0.0.1:8471/tracker.js": true}} | |||
| 161 | + | handler := newFilterRequestHandler(matcher) | |||
| 162 | + | ||||
| 163 | + | _, blocked := filterRequest(t, "http://127.0.0.1:8471/tracker.js", purecef.ResourceTypeRtScript, false) | |||
| 164 | + | frame, _ := filterRequest(t, "http://127.0.0.1:8471/page", purecef.ResourceTypeRtMainFrame, true) | |||
| 165 | + | require.Equal(t, purecef.ReturnValueRvCancel, handler.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |||
| 166 | + | before := matcher.seenCount() | |||
| 167 | + | ||||
| 168 | + | handler.Close() | |||
| 169 | + | require.Equal(t, purecef.ReturnValueRvContinue, handler.OnBeforeResourceLoad(nil, frame, blocked, nil)) | |||
| 170 | + | require.Equal(t, before, matcher.seenCount(), "retired matcher must not be consulted") | |||
| 171 | + | handler.Close() | |||
| 172 | + | } | |||
A internal/infrastructure/cef/injection_event_test.go +174 -0
| @@ -0,0 +1,174 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "sync" | |
| 6 | + | "testing" | |
| 7 | + | ||
| 8 | + | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // injectionHarness builds a WebView with a recording main frame. Engine is | |
| 14 | + | // nil so RunJavaScript executes synchronously without GTK dispatch. | |
| 15 | + | type injectionHarness struct { | |
| 16 | + | wv *WebView | |
| 17 | + | browser *cefmocks.MockBrowser | |
| 18 | + | mu sync.Mutex | |
| 19 | + | scripts []string | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | func newInjectionHarness(t *testing.T, uri string) *injectionHarness { | |
| 23 | + | t.Helper() | |
| 24 | + | return newInjectionHarnessWithID(t, uri, 7) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | func newInjectionHarnessWithID(t *testing.T, uri string, browserID int32) *injectionHarness { | |
| 28 | + | t.Helper() | |
| 29 | + | h := &injectionHarness{} | |
| 30 | + | frame := cefmocks.NewMockFrame(t) | |
| 31 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(script string, _ string, _ int32) { | |
| 32 | + | h.mu.Lock() | |
| 33 | + | defer h.mu.Unlock() | |
| 34 | + | h.scripts = append(h.scripts, script) | |
| 35 | + | }).Maybe() | |
| 36 | + | h.browser = cefmocks.NewMockBrowser(t) | |
| 37 | + | h.browser.EXPECT().GetMainFrame().Return(frame).Maybe() | |
| 38 | + | h.browser.EXPECT().GetIdentifier().Return(browserID).Maybe() | |
| 39 | + | h.wv = &WebView{ctx: context.Background(), browser: h.browser} | |
| 40 | + | h.wv.uri = uri | |
| 41 | + | return h | |
| 42 | + | } | |
| 43 | + | ||
| 44 | + | func (h *injectionHarness) scriptCount() int { | |
| 45 | + | h.mu.Lock() | |
| 46 | + | defer h.mu.Unlock() | |
| 47 | + | return len(h.scripts) | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | func testInjector() *contentInjector { | |
| 51 | + | return &contentInjector{themeCSS: ":root{--t:1}", findHighlightCSS: ".find{--f:1}"} | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | // TestInjectionEvent_ExternalPageScriptSet counts the installation for an | |
| 55 | + | // external document: scrollbar styling, its auto-hide behavior, the | |
| 56 | + | // clipboard bridge, plus configured find CSS. No dark-mode or message | |
| 57 | + | // bridge shim belongs to external pages. | |
| 58 | + | func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | |
| 59 | + | h := newInjectionHarness(t, "https://example.com/page") | |
| 60 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 61 | + | ||
| 62 | + | require.Equal(t, 4, h.scriptCount(), "external install must stay a fixed small script set") | |
| 63 | + | h.mu.Lock() | |
| 64 | + | defer h.mu.Unlock() | |
| 65 | + | joined := "" | |
| 66 | + | for _, script := range h.scripts { | |
| 67 | + | joined += script | |
| 68 | + | } | |
| 69 | + | require.Contains(t, joined, "dumber-scrollbar") | |
| 70 | + | require.Contains(t, joined, "dumber-find-highlight") | |
| 71 | + | require.NotContains(t, joined, "dumber-theme-vars") | |
| 72 | + | require.NotContains(t, joined, "__dumber_cef_prefers_dark") | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | // TestInjectionEvent_InternalPageScriptSet counts the installation for an | |
| 76 | + | // internal document, including dark-mode handling and the message bridge. | |
| 77 | + | func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | |
| 78 | + | h := newInjectionHarness(t, "dumb://home") | |
| 79 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 80 | + | ||
| 81 | + | require.Equal(t, 7, h.scriptCount(), "internal install must stay a fixed small script set") | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | // TestInjectionEvent_StaleBrowserSkipped reproduces an old main frame firing | |
| 85 | + | // during process swap: the GTK-dispatched callback must install nothing. | |
| 86 | + | func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | |
| 87 | + | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | |
| 88 | + | event := h.wv.captureInjectionEvent(h.browser) | |
| 89 | + | ||
| 90 | + | replacement := cefmocks.NewMockBrowser(t) | |
| 91 | + | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | |
| 92 | + | h.wv.mu.Lock() | |
| 93 | + | h.wv.browser = replacement | |
| 94 | + | h.wv.mu.Unlock() | |
| 95 | + | ||
| 96 | + | testInjector().onLoadEndForEvent(h.wv, event) | |
| 97 | + | require.Zero(t, h.scriptCount(), "old-frame event must not install against the replacement browser") | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | // TestInjectionEvent_StaleIntentSkipped reproduces a queued callback | |
| 101 | + | // outlived by a newer navigation: installation must not run for the | |
| 102 | + | // superseded intent. | |
| 103 | + | func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | |
| 104 | + | h := newInjectionHarness(t, "https://example.com/a") | |
| 105 | + | event := h.wv.captureInjectionEvent(h.browser) | |
| 106 | + | ||
| 107 | + | h.wv.mu.Lock() | |
| 108 | + | h.wv.pendingIntentID++ | |
| 109 | + | h.wv.mu.Unlock() | |
| 110 | + | ||
| 111 | + | testInjector().onLoadEndForEvent(h.wv, event) | |
| 112 | + | require.Zero(t, h.scriptCount(), "superseded intent must not install") | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | // TestInjectionEvent_CurrentInstalls verifies matching identity installs | |
| 116 | + | // exactly as the legacy path, including repeated same-document load-ends | |
| 117 | + | // whose scripts are idempotent by element ID. | |
| 118 | + | func TestInjectionEvent_CurrentInstalls(t *testing.T) { | |
| 119 | + | h := newInjectionHarness(t, "https://example.com/a") | |
| 120 | + | ci := testInjector() | |
| 121 | + | ||
| 122 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 123 | + | first := h.scriptCount() | |
| 124 | + | require.Positive(t, first) | |
| 125 | + | ||
| 126 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |
| 127 | + | require.Equal(t, 2*first, h.scriptCount(), "repeated same-document events reinstall idempotent scripts") | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | // TestInjectionEvent_NilCaptureSkips verifies the conservative fallback: a | |
| 131 | + | // capture from a nil callback browser cannot prove identity, so it installs | |
| 132 | + | // nothing; the current browser gets its own load-end installation. | |
| 133 | + | func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | |
| 134 | + | h := newInjectionHarness(t, "https://example.com/a") | |
| 135 | + | event := h.wv.captureInjectionEvent(nil) | |
| 136 | + | require.Equal(t, noBrowserID, event.browserID) | |
| 137 | + | ||
| 138 | + | testInjector().onLoadEndForEvent(h.wv, event) | |
| 139 | + | require.Zero(t, h.scriptCount(), "identity-less capture must not install blind") | |
| 140 | + | } | |
| 141 | + | ||
| 142 | + | // TestInjectionEvent_NilCurrentBrowserSkips verifies dispatch against a | |
| 143 | + | // destroyed browser installs nothing instead of dereferencing it. | |
| 144 | + | func TestInjectionEvent_NilCurrentBrowserSkips(t *testing.T) { | |
| 145 | + | h := newInjectionHarness(t, "https://example.com/a") | |
| 146 | + | event := h.wv.captureInjectionEvent(h.browser) | |
| 147 | + | ||
| 148 | + | h.wv.mu.Lock() | |
| 149 | + | h.wv.browser = nil | |
| 150 | + | h.wv.mu.Unlock() | |
| 151 | + | testInjector().onLoadEndForEvent(h.wv, event) | |
| 152 | + | require.Zero(t, h.scriptCount()) | |
| 153 | + | } | |
| 154 | + | ||
| 155 | + | // TestInjectionEvent_StaleCallbackBrowserSkipped reproduces the exact | |
| 156 | + | // reviewer scenario: WebView state advances to a replacement browser, then | |
| 157 | + | // the OLD callback browser fires. Identity must come from the callback, so | |
| 158 | + | // the event keeps the old identifier and is rejected. Stamping the current | |
| 159 | + | // wv.browser instead would bless it with the replacement identity. | |
| 160 | + | func TestInjectionEvent_StaleCallbackBrowserSkipped(t *testing.T) { | |
| 161 | + | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | |
| 162 | + | oldCallbackBrowser := h.browser | |
| 163 | + | ||
| 164 | + | replacement := cefmocks.NewMockBrowser(t) | |
| 165 | + | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | |
| 166 | + | h.wv.mu.Lock() | |
| 167 | + | h.wv.browser = replacement | |
| 168 | + | h.wv.mu.Unlock() | |
| 169 | + | ||
| 170 | + | event := h.wv.captureInjectionEvent(oldCallbackBrowser) | |
| 171 | + | require.Equal(t, int32(7), event.browserID, "callback-sourced capture must keep the old identity") | |
| 172 | + | testInjector().onLoadEndForEvent(h.wv, event) | |
| 173 | + | require.Zero(t, h.scriptCount(), "old-callback event must not install against the replacement browser") | |
| 174 | + | } | |
| @@ -0,0 +1,174 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "sync" | |||
| 6 | + | "testing" | |||
| 7 | + | ||||
| 8 | + | cefmocks "github.com/bnema/purego-cef/cef/mocks" | |||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // injectionHarness builds a WebView with a recording main frame. Engine is | |||
| 14 | + | // nil so RunJavaScript executes synchronously without GTK dispatch. | |||
| 15 | + | type injectionHarness struct { | |||
| 16 | + | wv *WebView | |||
| 17 | + | browser *cefmocks.MockBrowser | |||
| 18 | + | mu sync.Mutex | |||
| 19 | + | scripts []string | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | func newInjectionHarness(t *testing.T, uri string) *injectionHarness { | |||
| 23 | + | t.Helper() | |||
| 24 | + | return newInjectionHarnessWithID(t, uri, 7) | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | func newInjectionHarnessWithID(t *testing.T, uri string, browserID int32) *injectionHarness { | |||
| 28 | + | t.Helper() | |||
| 29 | + | h := &injectionHarness{} | |||
| 30 | + | frame := cefmocks.NewMockFrame(t) | |||
| 31 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(script string, _ string, _ int32) { | |||
| 32 | + | h.mu.Lock() | |||
| 33 | + | defer h.mu.Unlock() | |||
| 34 | + | h.scripts = append(h.scripts, script) | |||
| 35 | + | }).Maybe() | |||
| 36 | + | h.browser = cefmocks.NewMockBrowser(t) | |||
| 37 | + | h.browser.EXPECT().GetMainFrame().Return(frame).Maybe() | |||
| 38 | + | h.browser.EXPECT().GetIdentifier().Return(browserID).Maybe() | |||
| 39 | + | h.wv = &WebView{ctx: context.Background(), browser: h.browser} | |||
| 40 | + | h.wv.uri = uri | |||
| 41 | + | return h | |||
| 42 | + | } | |||
| 43 | + | ||||
| 44 | + | func (h *injectionHarness) scriptCount() int { | |||
| 45 | + | h.mu.Lock() | |||
| 46 | + | defer h.mu.Unlock() | |||
| 47 | + | return len(h.scripts) | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | func testInjector() *contentInjector { | |||
| 51 | + | return &contentInjector{themeCSS: ":root{--t:1}", findHighlightCSS: ".find{--f:1}"} | |||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | // TestInjectionEvent_ExternalPageScriptSet counts the installation for an | |||
| 55 | + | // external document: scrollbar styling, its auto-hide behavior, the | |||
| 56 | + | // clipboard bridge, plus configured find CSS. No dark-mode or message | |||
| 57 | + | // bridge shim belongs to external pages. | |||
| 58 | + | func TestInjectionEvent_ExternalPageScriptSet(t *testing.T) { | |||
| 59 | + | h := newInjectionHarness(t, "https://example.com/page") | |||
| 60 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |||
| 61 | + | ||||
| 62 | + | require.Equal(t, 4, h.scriptCount(), "external install must stay a fixed small script set") | |||
| 63 | + | h.mu.Lock() | |||
| 64 | + | defer h.mu.Unlock() | |||
| 65 | + | joined := "" | |||
| 66 | + | for _, script := range h.scripts { | |||
| 67 | + | joined += script | |||
| 68 | + | } | |||
| 69 | + | require.Contains(t, joined, "dumber-scrollbar") | |||
| 70 | + | require.Contains(t, joined, "dumber-find-highlight") | |||
| 71 | + | require.NotContains(t, joined, "dumber-theme-vars") | |||
| 72 | + | require.NotContains(t, joined, "__dumber_cef_prefers_dark") | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | // TestInjectionEvent_InternalPageScriptSet counts the installation for an | |||
| 76 | + | // internal document, including dark-mode handling and the message bridge. | |||
| 77 | + | func TestInjectionEvent_InternalPageScriptSet(t *testing.T) { | |||
| 78 | + | h := newInjectionHarness(t, "dumb://home") | |||
| 79 | + | testInjector().onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |||
| 80 | + | ||||
| 81 | + | require.Equal(t, 7, h.scriptCount(), "internal install must stay a fixed small script set") | |||
| 82 | + | } | |||
| 83 | + | ||||
| 84 | + | // TestInjectionEvent_StaleBrowserSkipped reproduces an old main frame firing | |||
| 85 | + | // during process swap: the GTK-dispatched callback must install nothing. | |||
| 86 | + | func TestInjectionEvent_StaleBrowserSkipped(t *testing.T) { | |||
| 87 | + | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | |||
| 88 | + | event := h.wv.captureInjectionEvent(h.browser) | |||
| 89 | + | ||||
| 90 | + | replacement := cefmocks.NewMockBrowser(t) | |||
| 91 | + | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | |||
| 92 | + | h.wv.mu.Lock() | |||
| 93 | + | h.wv.browser = replacement | |||
| 94 | + | h.wv.mu.Unlock() | |||
| 95 | + | ||||
| 96 | + | testInjector().onLoadEndForEvent(h.wv, event) | |||
| 97 | + | require.Zero(t, h.scriptCount(), "old-frame event must not install against the replacement browser") | |||
| 98 | + | } | |||
| 99 | + | ||||
| 100 | + | // TestInjectionEvent_StaleIntentSkipped reproduces a queued callback | |||
| 101 | + | // outlived by a newer navigation: installation must not run for the | |||
| 102 | + | // superseded intent. | |||
| 103 | + | func TestInjectionEvent_StaleIntentSkipped(t *testing.T) { | |||
| 104 | + | h := newInjectionHarness(t, "https://example.com/a") | |||
| 105 | + | event := h.wv.captureInjectionEvent(h.browser) | |||
| 106 | + | ||||
| 107 | + | h.wv.mu.Lock() | |||
| 108 | + | h.wv.pendingIntentID++ | |||
| 109 | + | h.wv.mu.Unlock() | |||
| 110 | + | ||||
| 111 | + | testInjector().onLoadEndForEvent(h.wv, event) | |||
| 112 | + | require.Zero(t, h.scriptCount(), "superseded intent must not install") | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | // TestInjectionEvent_CurrentInstalls verifies matching identity installs | |||
| 116 | + | // exactly as the legacy path, including repeated same-document load-ends | |||
| 117 | + | // whose scripts are idempotent by element ID. | |||
| 118 | + | func TestInjectionEvent_CurrentInstalls(t *testing.T) { | |||
| 119 | + | h := newInjectionHarness(t, "https://example.com/a") | |||
| 120 | + | ci := testInjector() | |||
| 121 | + | ||||
| 122 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |||
| 123 | + | first := h.scriptCount() | |||
| 124 | + | require.Positive(t, first) | |||
| 125 | + | ||||
| 126 | + | ci.onLoadEndForEvent(h.wv, h.wv.captureInjectionEvent(h.browser)) | |||
| 127 | + | require.Equal(t, 2*first, h.scriptCount(), "repeated same-document events reinstall idempotent scripts") | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | // TestInjectionEvent_NilCaptureSkips verifies the conservative fallback: a | |||
| 131 | + | // capture from a nil callback browser cannot prove identity, so it installs | |||
| 132 | + | // nothing; the current browser gets its own load-end installation. | |||
| 133 | + | func TestInjectionEvent_NilCaptureSkips(t *testing.T) { | |||
| 134 | + | h := newInjectionHarness(t, "https://example.com/a") | |||
| 135 | + | event := h.wv.captureInjectionEvent(nil) | |||
| 136 | + | require.Equal(t, noBrowserID, event.browserID) | |||
| 137 | + | ||||
| 138 | + | testInjector().onLoadEndForEvent(h.wv, event) | |||
| 139 | + | require.Zero(t, h.scriptCount(), "identity-less capture must not install blind") | |||
| 140 | + | } | |||
| 141 | + | ||||
| 142 | + | // TestInjectionEvent_NilCurrentBrowserSkips verifies dispatch against a | |||
| 143 | + | // destroyed browser installs nothing instead of dereferencing it. | |||
| 144 | + | func TestInjectionEvent_NilCurrentBrowserSkips(t *testing.T) { | |||
| 145 | + | h := newInjectionHarness(t, "https://example.com/a") | |||
| 146 | + | event := h.wv.captureInjectionEvent(h.browser) | |||
| 147 | + | ||||
| 148 | + | h.wv.mu.Lock() | |||
| 149 | + | h.wv.browser = nil | |||
| 150 | + | h.wv.mu.Unlock() | |||
| 151 | + | testInjector().onLoadEndForEvent(h.wv, event) | |||
| 152 | + | require.Zero(t, h.scriptCount()) | |||
| 153 | + | } | |||
| 154 | + | ||||
| 155 | + | // TestInjectionEvent_StaleCallbackBrowserSkipped reproduces the exact | |||
| 156 | + | // reviewer scenario: WebView state advances to a replacement browser, then | |||
| 157 | + | // the OLD callback browser fires. Identity must come from the callback, so | |||
| 158 | + | // the event keeps the old identifier and is rejected. Stamping the current | |||
| 159 | + | // wv.browser instead would bless it with the replacement identity. | |||
| 160 | + | func TestInjectionEvent_StaleCallbackBrowserSkipped(t *testing.T) { | |||
| 161 | + | h := newInjectionHarnessWithID(t, "https://example.com/a", 7) | |||
| 162 | + | oldCallbackBrowser := h.browser | |||
| 163 | + | ||||
| 164 | + | replacement := cefmocks.NewMockBrowser(t) | |||
| 165 | + | replacement.EXPECT().GetIdentifier().Return(int32(8)).Maybe() | |||
| 166 | + | h.wv.mu.Lock() | |||
| 167 | + | h.wv.browser = replacement | |||
| 168 | + | h.wv.mu.Unlock() | |||
| 169 | + | ||||
| 170 | + | event := h.wv.captureInjectionEvent(oldCallbackBrowser) | |||
| 171 | + | require.Equal(t, int32(7), event.browserID, "callback-sourced capture must keep the old identity") | |||
| 172 | + | testInjector().onLoadEndForEvent(h.wv, event) | |||
| 173 | + | require.Zero(t, h.scriptCount(), "old-callback event must not install against the replacement browser") | |||
| 174 | + | } | |||
M internal/infrastructure/cef/webview.go +45 -0
| @@ -1268,6 +1268,11 @@ func (wv *WebView) Destroy() { | |||
| 1268 | 1268 | if !wv.destroyed.CompareAndSwap(false, true) { | |
| 1269 | 1269 | return | |
| 1270 | 1270 | } | |
| 1271 | + | // Native close begins here: the view leaves the active set and enters | |
| 1272 | + | // GTK-teardown outstanding for the quiescence boundary. | |
| 1273 | + | if wv.engine != nil { | |
| 1274 | + | wv.engine.activity.NoteViewCloseStarted() | |
| 1275 | + | } | |
| 1271 | 1276 | // Retire scroll motion the moment destruction begins, regardless of | |
| 1272 | 1277 | // calling thread; GTK-only cleanup follows through the owning | |
| 1273 | 1278 | // dispatcher without waiting for the deferred native browser close. | |
| @@ -2187,6 +2192,46 @@ func (wv *WebView) claimPendingNavigationSubmission(intentID uint64, browserID i | |||
| 2187 | 2192 | return uri, pendingClaimReady | |
| 2188 | 2193 | } | |
| 2189 | 2194 | ||
| 2195 | + | // injectionEvent captures the navigation identity observed at main-frame | |
| 2196 | + | // load-end so the GTK-dispatched installation can drop stale events: an | |
| 2197 | + | // old main frame firing during process swap, or a queued callback outlived | |
| 2198 | + | // by a newer navigation. The browser is identified by its CEF identifier | |
| 2199 | + | // (matching codebase convention) rather than interface identity, which is | |
| 2200 | + | // neither panic-safe for arbitrary dynamic types nor proof of CEF object | |
| 2201 | + | // identity. noBrowserID marks captures taken while no browser was | |
| 2202 | + | // attached. The current URL alone never proves document identity. | |
| 2203 | + | type injectionEvent struct { | |
| 2204 | + | browserID int32 | |
| 2205 | + | intentID uint64 | |
| 2206 | + | } | |
| 2207 | + | ||
| 2208 | + | // noBrowserID marks an injection event captured while no browser was | |
| 2209 | + | // attached; such events cannot prove identity and are skipped. | |
| 2210 | + | const noBrowserID = int32(-1) | |
| 2211 | + | ||
| 2212 | + | // captureInjectionEvent snapshots the navigation identity for a load-end | |
| 2213 | + | // event from the callback's browser, not from mutable WebView state. Called | |
| 2214 | + | // on the CEF thread; the identifier read is a foreign call made outside any | |
| 2215 | + | // state lock. Sourcing identity from the callback is what makes staleness | |
| 2216 | + | // detection work: a queued event from a replaced browser keeps the OLD | |
| 2217 | + | // identifier and fails revalidation, while stamping the current wv.browser | |
| 2218 | + | // would bless it. Same-browser navigations are distinguished by the pending | |
| 2219 | + | // intent generation; repeated same-document load-ends share it and still | |
| 2220 | + | // install (their scripts are idempotent). | |
| 2221 | + | func (wv *WebView) captureInjectionEvent(browser purecef.Browser) injectionEvent { | |
| 2222 | + | if wv == nil { | |
| 2223 | + | return injectionEvent{browserID: noBrowserID} | |
| 2224 | + | } | |
| 2225 | + | if browser == nil { | |
| 2226 | + | return injectionEvent{browserID: noBrowserID} | |
| 2227 | + | } | |
| 2228 | + | browserID := browser.GetIdentifier() | |
| 2229 | + | wv.mu.RLock() | |
| 2230 | + | intentID := wv.pendingIntentID | |
| 2231 | + | wv.mu.RUnlock() | |
| 2232 | + | return injectionEvent{browserID: browserID, intentID: intentID} | |
| 2233 | + | } | |
| 2234 | + | ||
| 2190 | 2235 | func pendingURIEquivalent(a, b string) bool { | |
| 2191 | 2236 | if a == "" || b == "" { | |
| 2192 | 2237 | return false | |
| @@ -1268,6 +1268,11 @@ func (wv *WebView) Destroy() { | |||||
| 1268 | if !wv.destroyed.CompareAndSwap(false, true) { | 1268 | if !wv.destroyed.CompareAndSwap(false, true) { | ||
| 1269 | return | 1269 | return | ||
| 1270 | } | 1270 | } | ||
| 1271 | + | // Native close begins here: the view leaves the active set and enters | |||
| 1272 | + | // GTK-teardown outstanding for the quiescence boundary. | |||
| 1273 | + | if wv.engine != nil { | |||
| 1274 | + | wv.engine.activity.NoteViewCloseStarted() | |||
| 1275 | + | } | |||
| 1271 | // Retire scroll motion the moment destruction begins, regardless of | 1276 | // Retire scroll motion the moment destruction begins, regardless of | ||
| 1272 | // calling thread; GTK-only cleanup follows through the owning | 1277 | // calling thread; GTK-only cleanup follows through the owning | ||
| 1273 | // dispatcher without waiting for the deferred native browser close. | 1278 | // dispatcher without waiting for the deferred native browser close. | ||
| @@ -2187,6 +2192,46 @@ func (wv *WebView) claimPendingNavigationSubmission(intentID uint64, browserID i | |||||
| 2187 | return uri, pendingClaimReady | 2192 | return uri, pendingClaimReady | ||
| 2188 | } | 2193 | } | ||
| 2189 | 2194 | ||||
| 2195 | + | // injectionEvent captures the navigation identity observed at main-frame | |||
| 2196 | + | // load-end so the GTK-dispatched installation can drop stale events: an | |||
| 2197 | + | // old main frame firing during process swap, or a queued callback outlived | |||
| 2198 | + | // by a newer navigation. The browser is identified by its CEF identifier | |||
| 2199 | + | // (matching codebase convention) rather than interface identity, which is | |||
| 2200 | + | // neither panic-safe for arbitrary dynamic types nor proof of CEF object | |||
| 2201 | + | // identity. noBrowserID marks captures taken while no browser was | |||
| 2202 | + | // attached. The current URL alone never proves document identity. | |||
| 2203 | + | type injectionEvent struct { | |||
| 2204 | + | browserID int32 | |||
| 2205 | + | intentID uint64 | |||
| 2206 | + | } | |||
| 2207 | + | ||||
| 2208 | + | // noBrowserID marks an injection event captured while no browser was | |||
| 2209 | + | // attached; such events cannot prove identity and are skipped. | |||
| 2210 | + | const noBrowserID = int32(-1) | |||
| 2211 | + | ||||
| 2212 | + | // captureInjectionEvent snapshots the navigation identity for a load-end | |||
| 2213 | + | // event from the callback's browser, not from mutable WebView state. Called | |||
| 2214 | + | // on the CEF thread; the identifier read is a foreign call made outside any | |||
| 2215 | + | // state lock. Sourcing identity from the callback is what makes staleness | |||
| 2216 | + | // detection work: a queued event from a replaced browser keeps the OLD | |||
| 2217 | + | // identifier and fails revalidation, while stamping the current wv.browser | |||
| 2218 | + | // would bless it. Same-browser navigations are distinguished by the pending | |||
| 2219 | + | // intent generation; repeated same-document load-ends share it and still | |||
| 2220 | + | // install (their scripts are idempotent). | |||
| 2221 | + | func (wv *WebView) captureInjectionEvent(browser purecef.Browser) injectionEvent { | |||
| 2222 | + | if wv == nil { | |||
| 2223 | + | return injectionEvent{browserID: noBrowserID} | |||
| 2224 | + | } | |||
| 2225 | + | if browser == nil { | |||
| 2226 | + | return injectionEvent{browserID: noBrowserID} | |||
| 2227 | + | } | |||
| 2228 | + | browserID := browser.GetIdentifier() | |||
| 2229 | + | wv.mu.RLock() | |||
| 2230 | + | intentID := wv.pendingIntentID | |||
| 2231 | + | wv.mu.RUnlock() | |||
| 2232 | + | return injectionEvent{browserID: browserID, intentID: intentID} | |||
| 2233 | + | } | |||
| 2234 | + | ||||
| 2190 | func pendingURIEquivalent(a, b string) bool { | 2235 | func pendingURIEquivalent(a, b string) bool { | ||
| 2191 | if a == "" || b == "" { | 2236 | if a == "" || b == "" { | ||
| 2192 | return false | 2237 | return false | ||
M internal/infrastructure/cef/webview_handlers.go +6 -2
| @@ -490,7 +490,7 @@ func (h *handlerSet) OnLoadStart(_ purecef.Browser, frame purecef.Frame, _ purec | |||
| 490 | 490 | } | |
| 491 | 491 | ||
| 492 | 492 | // OnLoadEnd resets crash count and injects content scripts for the main frame. | |
| 493 | - | func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatusCode int32) { | |
| 493 | + | func (h *handlerSet) OnLoadEnd(browser purecef.Browser, frame purecef.Frame, httpStatusCode int32) { | |
| 494 | 494 | log := logging.FromContext(h.wv.ctx) | |
| 495 | 495 | log.Debug(). | |
| 496 | 496 | Bool("frame_nil", frame == nil). | |
| @@ -532,8 +532,12 @@ func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatu | |||
| 532 | 532 | }) | |
| 533 | 533 | } | |
| 534 | 534 | if h.wv.engine != nil && h.wv.engine.contentInj != nil { | |
| 535 | + | // Capture navigation identity for the GTK hop from the callback's | |
| 536 | + | // browser: a process swap or a newer navigation can stale this | |
| 537 | + | // event before it runs. | |
| 538 | + | event := h.wv.captureInjectionEvent(browser) | |
| 535 | 539 | h.wv.runOnGTK(func() { | |
| 536 | - | h.wv.engine.contentInj.onLoadEnd(h.wv) | |
| 540 | + | h.wv.engine.contentInj.onLoadEndForEvent(h.wv, event) | |
| 537 | 541 | }) | |
| 538 | 542 | } | |
| 539 | 543 | ||
| @@ -490,7 +490,7 @@ func (h *handlerSet) OnLoadStart(_ purecef.Browser, frame purecef.Frame, _ purec | |||||
| 490 | } | 490 | } | ||
| 491 | 491 | ||||
| 492 | // OnLoadEnd resets crash count and injects content scripts for the main frame. | 492 | // OnLoadEnd resets crash count and injects content scripts for the main frame. | ||
| 493 | - | func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatusCode int32) { | 493 | + | func (h *handlerSet) OnLoadEnd(browser purecef.Browser, frame purecef.Frame, httpStatusCode int32) { |
| 494 | log := logging.FromContext(h.wv.ctx) | 494 | log := logging.FromContext(h.wv.ctx) | ||
| 495 | log.Debug(). | 495 | log.Debug(). | ||
| 496 | Bool("frame_nil", frame == nil). | 496 | Bool("frame_nil", frame == nil). | ||
| @@ -532,8 +532,12 @@ func (h *handlerSet) OnLoadEnd(_ purecef.Browser, frame purecef.Frame, httpStatu | |||||
| 532 | }) | 532 | }) | ||
| 533 | } | 533 | } | ||
| 534 | if h.wv.engine != nil && h.wv.engine.contentInj != nil { | 534 | if h.wv.engine != nil && h.wv.engine.contentInj != nil { | ||
| 535 | + | // Capture navigation identity for the GTK hop from the callback's | |||
| 536 | + | // browser: a process swap or a newer navigation can stale this | |||
| 537 | + | // event before it runs. | |||
| 538 | + | event := h.wv.captureInjectionEvent(browser) | |||
| 535 | h.wv.runOnGTK(func() { | 539 | h.wv.runOnGTK(func() { | ||
| 536 | - | h.wv.engine.contentInj.onLoadEnd(h.wv) | 540 | + | h.wv.engine.contentInj.onLoadEndForEvent(h.wv, event) |
| 537 | }) | 541 | }) | ||
| 538 | } | 542 | } | ||
| 539 | 543 | ||||
M internal/infrastructure/favicon/fetcher.go +4 -1
| @@ -131,7 +131,10 @@ func (f *Fetcher) fetchURL(ctx context.Context, raw, pageURL string) ([]byte, st | |||
| 131 | 131 | } | |
| 132 | 132 | defer func() { _ = resp.Body.Close() }() | |
| 133 | 133 | if resp.StatusCode != http.StatusOK { | |
| 134 | - | return nil, "", appport.ErrFaviconMiss | |
| 134 | + | // Classified miss: callers keep ErrFaviconMiss matching via Is, | |
| 135 | + | // while the use case can distinguish genuine absence (404/410) | |
| 136 | + | // from auth/transient failures for negative caching. | |
| 137 | + | return nil, "", &appport.FaviconFetchError{StatusCode: resp.StatusCode} | |
| 135 | 138 | } | |
| 136 | 139 | limit := f.maxBytes | |
| 137 | 140 | if limit <= 0 { | |
| @@ -131,7 +131,10 @@ func (f *Fetcher) fetchURL(ctx context.Context, raw, pageURL string) ([]byte, st | |||||
| 131 | } | 131 | } | ||
| 132 | defer func() { _ = resp.Body.Close() }() | 132 | defer func() { _ = resp.Body.Close() }() | ||
| 133 | if resp.StatusCode != http.StatusOK { | 133 | if resp.StatusCode != http.StatusOK { | ||
| 134 | - | return nil, "", appport.ErrFaviconMiss | 134 | + | // Classified miss: callers keep ErrFaviconMiss matching via Is, |
| 135 | + | // while the use case can distinguish genuine absence (404/410) | |||
| 136 | + | // from auth/transient failures for negative caching. | |||
| 137 | + | return nil, "", &appport.FaviconFetchError{StatusCode: resp.StatusCode} | |||
| 135 | } | 138 | } | ||
| 136 | limit := f.maxBytes | 139 | limit := f.maxBytes | ||
| 137 | if limit <= 0 { | 140 | if limit <= 0 { | ||