mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
Claude
2cd6569fe9
BrowserIconRegistry was the third of the trio flagged as still sitting in
amethyst/, and the only one with a real platform tie: unlike the other two, its
Context was not dead — it supplied filesDir for the icon directory.
It is a small tie, and commons already has the shape for it. AppPreferenceStores
takes `rootFilesDir: () -> Path`, so this takes `iconDir: () -> Path` and goes
through commons' platformFileSystem (the expect/actual that exists because okio
declares FileSystem.SYSTEM per platform). The Android app passes
`{ appContext.filesDir.toOkioPath() / BrowserIconRegistry.DIR }`, which is the
same filesDir/browser_icons the object used, so stored favicons are found where
they were left. No bitmaps are involved anywhere — it has always been ByteArray
in, `file://` string out.
One behaviour change, deliberate: the startup scan now merges into `keys`
instead of assigning it. A record() that landed while the scan was in flight had
already written its file and added its key, and the wholesale assignment dropped
it — the icon sat on disk unshown until the next launch. Not pinned by a test,
and that is on purpose: making the scan finish after a concurrent record is not
something I can force deterministically, so any test I wrote would pass with or
without the change and would only look like coverage.
8 new tests for what is deterministic: a recorded icon reaches disk with the
bytes given and is announced, icons already on disk are indexed by init(), an
unknown host has no model (iconModelFor is read from composition, so it answers
from `keys` rather than touching the filesystem), a missing icon directory is
created rather than dropping the icon, blank hosts and empty byte arrays are
ignored, and hosts are sanitized into one flat filename both when storing and
when looking up — "Example.COM:8080/../etc" cannot escape the directory.
Writing them repeated the lesson from f0c66955 in a new form: with the
registry's scope set to runTest's backgroundScope, none of the launched disk
work ran under advanceUntilIdle and every assertion failed as though the code
did nothing. Same `coroutineContext + Job()` session idiom as the other two
suites now.
Three call sites used it as a method reference (`::iconModelFor`), which has no
trailing dot and so was missed by the first pass over the callers — caught by
the compiler, not by grep.
Verified: :commons:jvmTest, :commons:verifyKmpPurity,
:commons:compileCommonMainKotlinMetadata, :amethyst:compilePlayDebugKotlin.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AXvKXakvup4inNFfAhhr4L