From 2cd6569fe97a8b09032e58e993be3beea0ca620d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 20:04:34 +0000 Subject: [PATCH] refactor(commons): move the browser favicon registry out of the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01AXvKXakvup4inNFfAhhr4L --- .../com/vitorpamplona/amethyst/Amethyst.kt | 3 +- .../com/vitorpamplona/amethyst/AppModules.kt | 7 + .../amethyst/favorites/BrowserIconRegistry.kt | 124 ----------- .../amethyst/favorites/NappletFavoriteIcon.kt | 7 +- .../amethyst/napplet/NappletBrokerService.kt | 6 +- .../amethyst/napplet/NappletConsentSummary.kt | 4 +- .../amethyst/napplet/NostrSignerOpLabels.kt | 6 +- .../ui/navigation/bottombars/AppBottomBar.kt | 6 +- .../screen/loggedIn/browser/BrowserScreen.kt | 8 +- .../loggedIn/favorites/FavoriteAppsScreen.kt | 6 +- .../napplets/ConnectedAppDetailScreen.kt | 3 +- .../commons/browser/BrowserIconRegistry.kt | 141 +++++++++++++ .../browser/BrowserIconRegistryTest.kt | 197 ++++++++++++++++++ 13 files changed, 369 insertions(+), 149 deletions(-) delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/BrowserIconRegistry.kt create mode 100644 commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistry.kt create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistryTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt index 7266fdc6cc..5715d4e98b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt @@ -25,7 +25,6 @@ import android.content.ComponentCallbacks2 import android.os.Build import com.vitorpamplona.amethyst.commons.service.http.HttpClientEnvironment import com.vitorpamplona.amethyst.commons.service.http.MediaCallEventListener -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.napplet.WebAppNetworkRegistry import com.vitorpamplona.amethyst.service.logging.Logging import com.vitorpamplona.amethyst.service.nests.AppForegroundRecycleHook @@ -147,7 +146,7 @@ class Amethyst : Application() { instance.browserHistory.init() // Index device-local captured favicons (main process only; decorates favorites + suggestions). - BrowserIconRegistry.init(this) + instance.browserIcons.init() // Warm the global-settings prefs off-main so the first (deliberately synchronous) read of // them does not hit disk on the main thread. See LocalPreferences.warmGlobalSettings. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index 2bfc9ff3be..8c98c1f1b4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -28,6 +28,7 @@ import androidx.security.crypto.EncryptedSharedPreferences import coil3.disk.DiskCache import coil3.memory.MemoryCache import com.vitorpamplona.amethyst.commons.browser.BrowserHistoryRegistry +import com.vitorpamplona.amethyst.commons.browser.BrowserIconRegistry import com.vitorpamplona.amethyst.commons.connectedApps.DataStoreNostrSignerPermissionStore import com.vitorpamplona.amethyst.commons.connectedApps.nip46.DataStoreNip46ClientStore import com.vitorpamplona.amethyst.commons.favorites.FavoriteAppsRegistry @@ -899,6 +900,12 @@ class AppModules( val browserHistory by lazy { BrowserHistoryRegistry(appStores.getDataStore(BrowserHistoryRegistry.FILE_NAME), applicationIOScope) } + // Favicons captured by the browser host, one PNG per host. Not a DataStore — it takes the directory + // to keep them in, the same way AppPreferenceStores takes rootFilesDir. + val browserIcons by lazy { + BrowserIconRegistry({ appContext.filesDir.toOkioPath() / BrowserIconRegistry.DIR }, applicationIOScope) + } + // Authenticates with relays. val authCoordinator = AuthCoordinator(client, applicationIOScope) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/BrowserIconRegistry.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/BrowserIconRegistry.kt deleted file mode 100644 index f2d37b5453..0000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/BrowserIconRegistry.kt +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.favorites - -import android.content.Context -import com.vitorpamplona.quartz.utils.Log -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.update -import kotlinx.coroutines.launch -import java.io.File - -/** - * Device-local favicon store for browsed sites, keyed by host. Favicons are **captured from the WebView - * that already loaded the page** in the keyless `:napplet` browser host (where they ride the page's own — - * Tor-routed — network path) and relayed here as PNG bytes over IPC; this is the privacy-preserving - * alternative to the main app fetching `host/favicon.ico` itself, which would bypass Tor and leak the - * visit. Used to decorate favorite cards and omnibox suggestion rows. - * - * Lives only in the **main process**. Bytes are persisted as one small PNG per host under - * `filesDir/browser_icons`; the deterministic path means the only in-memory state is [keys] — the set of - * hosts that currently have an icon — which exists purely to drive Compose recomposition (and to keep - * `File.exists()` disk checks out of composition). - */ -object BrowserIconRegistry { - private const val DIR = "browser_icons" - - private val _keys = MutableStateFlow>(emptySet()) - - /** Sanitized host keys that currently have a stored icon. Observe to recompose when an icon arrives. */ - val keys: StateFlow> = _keys.asStateFlow() - - @Volatile private var iconDir: File? = null - - // Disk work runs here, never on the caller's thread. Both entry points are reached from threads - // that must not block: init() from app startup and record() from the broker's IPC handler, which - // is the main looper — StrictMode flagged the write, and a slow filesystem would have stalled the - // UI while a favicon was saved. - private val io = CoroutineScope(SupervisorJob() + Dispatchers.IO) - - /** - * Binds the app context and indexes already-stored icons. Idempotent. - * - * [iconDir] is published synchronously so [iconModelFor] and [record] work immediately; only the - * directory scan is deferred. Until it lands [keys] is empty, so an icon simply renders its - * placeholder for one frame and then recomposes — [keys] is a StateFlow precisely so that arrival - * drives recomposition. - */ - fun init(context: Context) { - if (iconDir != null) return - val dir = File(context.applicationContext.filesDir, DIR) - iconDir = dir - io.launch { - dir.mkdirs() - _keys.value = dir.listFiles()?.mapNotNull { it.name.removeSuffix(PNG).takeIf { n -> n.isNotBlank() } }?.toSet() ?: emptySet() - } - } - - /** Persists [bytes] as the favicon for [host] and marks it available. Called from the broker on IPC. */ - fun record( - host: String, - bytes: ByteArray, - ) { - val dir = iconDir ?: return - if (host.isBlank() || bytes.isEmpty()) return - val key = sanitize(host) - // Fire-and-forget: a favicon is a decoration, and the IPC handler must not wait on disk. - // [keys] updates only after the bytes are actually on disk, so a reader can never be told an - // icon exists before the file backing it does. - io.launch { - try { - dir.mkdirs() - File(dir, key + PNG).writeBytes(bytes) - _keys.update { it + key } - } catch (e: Exception) { - Log.w("BrowserIconRegistry", "Failed to store favicon for $host", e) - } - } - } - - /** - * A Coil model (`file://…`) for [host]'s favicon, or null when none is stored. Reads [keys] so callers - * that observe the flow recompose as icons arrive — pass [keys]'s value as a `remember` key. - */ - fun iconModelFor(host: String): String? { - val dir = iconDir ?: return null - val key = sanitize(host) - if (key !in _keys.value) return null - return "file://" + File(dir, key + PNG).absolutePath - } - - // Hosts map to a flat, filesystem-safe filename. Collisions (two hosts → one key) only mean a shared - // icon file, which is harmless for a decoration. - private fun sanitize(host: String): String = - host - .lowercase() - .map { if (it.isLetterOrDigit() || it == '.' || it == '-') it else '_' } - .joinToString("") - .take(120) - - private const val PNG = ".png" -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/NappletFavoriteIcon.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/NappletFavoriteIcon.kt index 330f648f09..f6b1e609e7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/NappletFavoriteIcon.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/favorites/NappletFavoriteIcon.kt @@ -111,7 +111,7 @@ private fun resolveIconBlob(event: Event?): IconBlob? = /** * A Coil model (`file://…`) for the cached favicon of [url]'s host, or null when no favicon - * has been captured yet. The favicon is stored by [BrowserIconRegistry] at browse time (the + * has been captured yet. The favicon is stored by [com.vitorpamplona.amethyst.commons.browser.BrowserIconRegistry] at browse time (the * WebView captures it in the sandboxed `:napplet` process); this composable just reads the cache. * * Early-returns null when [url] is blank or has no parseable host — this early return is stable @@ -121,8 +121,9 @@ private fun resolveIconBlob(event: Event?): IconBlob? = @Composable fun rememberWebAppIconModel(url: String): String? { val host = remember(url) { OmniboxInput.hostOf(url) } ?: return null - val iconKeys by BrowserIconRegistry.keys.collectAsStateWithLifecycle() - return remember(host, iconKeys) { BrowserIconRegistry.iconModelFor(host) } + val iconKeys by Amethyst.instance.browserIcons.keys + .collectAsStateWithLifecycle() + return remember(host, iconKeys) { Amethyst.instance.browserIcons.iconModelFor(host) } } /** diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt index c6badf72e0..3340367115 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletBrokerService.kt @@ -42,7 +42,6 @@ import com.vitorpamplona.amethyst.commons.napplet.NappletIdentityWatch import com.vitorpamplona.amethyst.commons.napplet.NappletRequestRouter import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletProtocolJson import com.vitorpamplona.amethyst.commons.napplet.protocol.NappletResponse -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.amethyst.napplet.gateways.AccountNappletGateways import com.vitorpamplona.amethyst.napplethost.NappletIpc @@ -212,8 +211,9 @@ class NappletBrokerService : Service() { val data = msg.data ?: return true val host = data.getString(NappletIpc.KEY_ICON_HOST)?.takeIf { it.isNotBlank() } ?: return true val bytes = data.getByteArray(NappletIpc.KEY_ICON_BYTES) ?: return true - BrowserIconRegistry.init(applicationContext) - BrowserIconRegistry.record(host, bytes) + val icons = Amethyst.instance.browserIcons + icons.init() + icons.record(host, bytes) return true } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletConsentSummary.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletConsentSummary.kt index 500b133ef6..3c0d13871e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletConsentSummary.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NappletConsentSummary.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.amethyst.napplet import android.content.Context +import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.commons.browser.OmniboxInput import com.vitorpamplona.amethyst.commons.napplet.NappletCapability import com.vitorpamplona.amethyst.commons.napplet.NappletIdentity @@ -62,7 +63,6 @@ import com.vitorpamplona.amethyst.commons.resources.napplet_consent_upload import com.vitorpamplona.amethyst.commons.resources.napplet_fallback_title import com.vitorpamplona.amethyst.commons.ui.loadPluralStringRes import com.vitorpamplona.amethyst.commons.ui.loadStringRes -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.model.Account import com.vitorpamplona.quartz.lightning.LnInvoiceUtil import com.vitorpamplona.quartz.nip01Core.core.fastForEach @@ -95,7 +95,7 @@ class NappletConsentSummary( val (title, iconUrl) = if (identity.authorPubKey == "browser") { val host = OmniboxInput.hostOf(identity.identifier) ?: identity.identifier - host to BrowserIconRegistry.iconModelFor(host) + host to Amethyst.instance.browserIcons.iconModelFor(host) } else { resolveNappletMeta(identity.authorPubKey, identity.identifier, untitled) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt index ce636d427a..9ff3361771 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/napplet/NostrSignerOpLabels.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.amethyst.napplet import android.content.Context +import com.vitorpamplona.amethyst.Amethyst import com.vitorpamplona.amethyst.commons.browser.OmniboxInput import com.vitorpamplona.amethyst.commons.connectedApps.signers.NostrSignerOp import com.vitorpamplona.amethyst.commons.model.cache.LocalCache @@ -39,7 +40,6 @@ import com.vitorpamplona.amethyst.commons.resources.nip46_signer_allow_always_fo import com.vitorpamplona.amethyst.commons.ui.loadStringRes import com.vitorpamplona.amethyst.connectedApps.consent.SignerConnectInfo import com.vitorpamplona.amethyst.connectedApps.consent.SignerConsentInfo -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.ui.screen.loggedIn.relays.kindNameFor import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey @@ -83,7 +83,7 @@ suspend fun buildSignerConsentInfo( val (title, iconUrl) = if (identity.authorPubKey == "browser") { val host = OmniboxInput.hostOf(identity.identifier) ?: identity.identifier - host to BrowserIconRegistry.iconModelFor(host) + host to Amethyst.instance.browserIcons.iconModelFor(host) } else { resolveNappletMeta(identity.authorPubKey, identity.identifier, untitled) } @@ -183,7 +183,7 @@ suspend fun buildConnectInfo( val (title, iconUrl) = if (identity.authorPubKey == "browser") { val host = OmniboxInput.hostOf(identity.identifier) ?: identity.identifier - host to BrowserIconRegistry.iconModelFor(host) + host to Amethyst.instance.browserIcons.iconModelFor(host) } else { resolveNappletMeta(identity.authorPubKey, identity.identifier, untitled) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt index 14d81fdaee..c91ee5a633 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/bottombars/AppBottomBar.kt @@ -60,7 +60,6 @@ import com.vitorpamplona.amethyst.commons.ui.theme.Size10Modifier import com.vitorpamplona.amethyst.commons.ui.theme.Size25Modifier import com.vitorpamplona.amethyst.commons.ui.theme.Size27Modifier import com.vitorpamplona.amethyst.commons.ui.theme.onSurface65 -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.favorites.rememberNappletIconModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel @@ -132,9 +131,10 @@ internal fun rememberFavoriteIconModel(fav: FavoriteApp): Any? = when (fav) { is FavoriteApp.WebApp -> { // Captured favicons, keyed so the icon appears once the site's capture lands. - val iconKeys by BrowserIconRegistry.keys.collectAsStateWithLifecycle() + val iconKeys by Amethyst.instance.browserIcons.keys + .collectAsStateWithLifecycle() remember(fav, iconKeys) { - OmniboxInput.hostOf(fav.url)?.let(BrowserIconRegistry::iconModelFor) + OmniboxInput.hostOf(fav.url)?.let(Amethyst.instance.browserIcons::iconModelFor) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/BrowserScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/BrowserScreen.kt index d4e10ff587..cc5430711b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/BrowserScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/browser/BrowserScreen.kt @@ -102,7 +102,6 @@ import com.vitorpamplona.amethyst.commons.resources.favorite_app_remove import com.vitorpamplona.amethyst.commons.resources.favorite_app_still_loading import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.commons.ui.note.ArrowBackIcon -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.favorites.FavoriteAppLauncher import com.vitorpamplona.amethyst.favorites.PreloadFavoriteNostrApps import com.vitorpamplona.amethyst.favorites.rememberNappletIconModel @@ -156,7 +155,8 @@ private fun BrowserLauncher( .collectAsStateWithLifecycle() val history by Amethyst.instance.browserHistory.history .collectAsStateWithLifecycle() - val iconKeys by BrowserIconRegistry.keys.collectAsStateWithLifecycle() + val iconKeys by Amethyst.instance.browserIcons.keys + .collectAsStateWithLifecycle() // Fetch favorited nsite/napplet manifests up front so tapping one launches immediately instead of // showing "isn't loaded yet" until the user happens to visit the nsite/napplet feed. @@ -571,7 +571,7 @@ private fun SuggestedRow( onClick: () -> Unit, onAddFavorite: () -> Unit, ) { - val iconModel = remember(entry, iconKeys) { OmniboxInput.hostOf(entry.app.url)?.let(BrowserIconRegistry::iconModelFor) } + val iconModel = remember(entry, iconKeys) { OmniboxInput.hostOf(entry.app.url)?.let(Amethyst.instance.browserIcons::iconModelFor) } Row( modifier = Modifier @@ -798,7 +798,7 @@ private fun SiteIcon( iconKeys: Set, modifier: Modifier = Modifier, ) { - val model = remember(host, iconKeys) { BrowserIconRegistry.iconModelFor(host) } + val model = remember(host, iconKeys) { Amethyst.instance.browserIcons.iconModelFor(host) } val symbol = if (isFavorite) MaterialSymbols.Star else MaterialSymbols.Public val tint = MaterialTheme.colorScheme.onSurfaceVariant if (model == null) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/favorites/FavoriteAppsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/favorites/FavoriteAppsScreen.kt index c4e69e44f4..f3e9e9f896 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/favorites/FavoriteAppsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/favorites/FavoriteAppsScreen.kt @@ -73,7 +73,6 @@ import com.vitorpamplona.amethyst.commons.resources.favorite_apps import com.vitorpamplona.amethyst.commons.resources.favorite_apps_empty import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.commons.ui.stringRes -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.favorites.FavoriteAppLauncher import com.vitorpamplona.amethyst.favorites.PreloadFavoriteNostrApps import com.vitorpamplona.amethyst.favorites.rememberNappletIconModel @@ -190,12 +189,13 @@ internal fun FavoriteAppCell( // For a plain web favorite, prefer the favicon captured when its site was opened; an nsite/napplet uses // the verified icon blob bundled in its own content. Observing the key set recomputes the model as a // captured favicon arrives. - val iconKeys by BrowserIconRegistry.keys.collectAsStateWithLifecycle() + val iconKeys by Amethyst.instance.browserIcons.keys + .collectAsStateWithLifecycle() val faviconModel = when (app) { is FavoriteApp.WebApp -> remember(app, iconKeys) { - OmniboxInput.hostOf(app.url)?.let(BrowserIconRegistry::iconModelFor) + OmniboxInput.hostOf(app.url)?.let(Amethyst.instance.browserIcons::iconModelFor) } is FavoriteApp.NostrApp -> rememberNappletIconModel(app.coordinate) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt index ed86de0cb2..9e7491a45d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/napplets/ConnectedAppDetailScreen.kt @@ -110,7 +110,6 @@ import com.vitorpamplona.amethyst.commons.resources.nip46_signer_reconnecting import com.vitorpamplona.amethyst.commons.resources.nip46_signer_remote_app import com.vitorpamplona.amethyst.commons.ui.navigation.navs.INav import com.vitorpamplona.amethyst.commons.ui.navigation.topbars.TopBarWithBackButton -import com.vitorpamplona.amethyst.favorites.BrowserIconRegistry import com.vitorpamplona.amethyst.favorites.rememberManifestIconModel import com.vitorpamplona.amethyst.favorites.rememberWebAppIconModel import com.vitorpamplona.amethyst.napplet.NappletBrokerService @@ -790,7 +789,7 @@ private suspend fun loadDetailState( val (title, iconUrl) = if (author == "browser") { val host = OmniboxInput.hostOf(identifier) ?: identifier - host to BrowserIconRegistry.iconModelFor(host) + host to Amethyst.instance.browserIcons.iconModelFor(host) } else { resolveNappletMeta(author, identifier, untitled) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistry.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistry.kt new file mode 100644 index 0000000000..f7f6c4bb69 --- /dev/null +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistry.kt @@ -0,0 +1,141 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.browser + +import com.vitorpamplona.amethyst.commons.util.platformFileSystem +import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import okio.Path +import kotlin.concurrent.Volatile + +/** + * Device-local favicon store for browsed sites, keyed by host. Favicons are **captured from the WebView + * that already loaded the page** — on Android, in the keyless `:napplet` browser host, where they ride the + * page's own (Tor-routed) network path — and handed here as PNG bytes; this is the privacy-preserving + * alternative to the app fetching `host/favicon.ico` itself, which would bypass Tor and leak the visit. + * Used to decorate favorite cards and omnibox suggestion rows. + * + * Bytes are persisted as one small PNG per host under [iconDir], so the only in-memory state is [keys] — + * the set of hosts that currently have an icon — which exists purely to drive Compose recomposition (and + * to keep filesystem existence checks out of composition). + * + * [iconDir] is a function rather than a path for the same reason [com.vitorpamplona.amethyst.commons.model.preferences.AppPreferenceStores] + * takes `rootFilesDir`: the front end owns where its files live, and resolving it lazily keeps this class + * free of any platform's notion of an app directory. The Android app passes + * `{ appContext.filesDir.toOkioPath() / DIR }`. + * + * One instance per process. On Android the launcher/UI in the **main** process own it; the keyless + * `:napplet` sandbox never builds one and relays captured bytes over IPC instead. + */ +class BrowserIconRegistry( + private val iconDir: () -> Path, + private val scope: CoroutineScope, +) { + private val _keys = MutableStateFlow>(emptySet()) + + /** Sanitized host keys that currently have a stored icon. Observe to recompose when an icon arrives. */ + val keys: StateFlow> = _keys.asStateFlow() + + @Volatile private var started = false + + /** + * Indexes already-stored icons. Idempotent. + * + * Only the directory scan is deferred; [iconModelFor] and [record] resolve [iconDir] themselves and + * work immediately. Until the scan lands [keys] is empty, so an icon renders its placeholder for one + * frame and then recomposes — [keys] is a StateFlow precisely so that arrival drives recomposition. + */ + fun init() { + if (started) return + started = true + scope.launch { + try { + val dir = iconDir() + platformFileSystem.createDirectories(dir) + val scanned = + platformFileSystem + .list(dir) + .mapNotNull { it.name.removeSuffix(PNG).takeIf { name -> name.isNotBlank() } } + .toSet() + // Merged rather than assigned: a record() that lands while the scan is in flight has + // already written its file and added its key, and overwriting the set wholesale would + // drop it — the icon would sit on disk unshown until the next launch. + _keys.update { it + scanned } + } catch (e: Exception) { + Log.w("BrowserIconRegistry", "Failed to index stored favicons", e) + } + } + } + + /** Persists [bytes] as the favicon for [host] and marks it available. */ + fun record( + host: String, + bytes: ByteArray, + ) { + if (host.isBlank() || bytes.isEmpty()) return + val key = sanitize(host) + // Fire-and-forget: a favicon is a decoration, and the caller (on Android, the broker's IPC + // handler, which runs on the main looper) must not wait on disk. + // [keys] updates only after the bytes are actually on disk, so a reader can never be told an + // icon exists before the file backing it does. + scope.launch { + try { + val dir = iconDir() + platformFileSystem.createDirectories(dir) + platformFileSystem.write(dir / (key + PNG)) { write(bytes) } + _keys.update { it + key } + } catch (e: Exception) { + Log.w("BrowserIconRegistry", "Failed to store favicon for $host", e) + } + } + } + + /** + * A Coil model (`file://…`) for [host]'s favicon, or null when none is stored. Reads [keys] so callers + * that observe the flow recompose as icons arrive — pass [keys]'s value as a `remember` key. + */ + fun iconModelFor(host: String): String? { + val key = sanitize(host) + if (key !in _keys.value) return null + return "file://" + (iconDir() / (key + PNG)) + } + + companion object { + /** Same directory the Android registry used: `filesDir/browser_icons`. */ + const val DIR = "browser_icons" + + private const val PNG = ".png" + + // Hosts map to a flat, filesystem-safe filename. Collisions (two hosts → one key) only mean a + // shared icon file, which is harmless for a decoration. + private fun sanitize(host: String): String = + host + .lowercase() + .map { if (it.isLetterOrDigit() || it == '.' || it == '-') it else '_' } + .joinToString("") + .take(120) + } +} diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistryTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistryTest.kt new file mode 100644 index 0000000000..1e5593e921 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/browser/BrowserIconRegistryTest.kt @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.browser + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import okio.Path.Companion.toOkioPath +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +/** + * The favicon store's disk behaviour. Untestable while it was an Android `object` taking a `Context` + * for its `filesDir`; taking `iconDir: () -> Path` is what opens it up. + */ +class BrowserIconRegistryTest { + @get:Rule + val folder = TemporaryFolder() + + private var seq = 0 + + private fun newDir(): File = folder.newFolder("icons_${seq++}") + + private val png = byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47) + + /** One app "session" over [dir], on the test scheduler so [advanceUntilIdle] drives its disk work. */ + private fun TestScope.registryOver(dir: File) = BrowserIconRegistry({ dir.toOkioPath() }, CoroutineScope(coroutineContext + Job())) + + @Test + fun aRecordedIconBecomesAvailableAndReachesDisk() = + runTest { + val dir = newDir() + val registry = registryOver(dir) + registry.init() + advanceUntilIdle() + + registry.record("example.com", png) + advanceUntilIdle() + + assertEquals("the host is announced", setOf("example.com"), registry.keys.value) + assertEquals( + "and the model points at the file", + "file://" + File(dir, "example.com.png").absolutePath, + registry.iconModelFor("example.com"), + ) + assertTrue("which exists", File(dir, "example.com.png").exists()) + assertEquals("with the bytes given", png.toList(), File(dir, "example.com.png").readBytes().toList()) + } + + /** A cold start has to find what earlier sessions stored, or every icon redownloads on first paint. */ + @Test + fun iconsAlreadyOnDiskAreIndexedByInit() = + runTest { + val dir = newDir() + File(dir, "already.example.png").writeBytes(png) + + val registry = registryOver(dir) + assertTrue("nothing is known before init", registry.keys.value.isEmpty()) + + registry.init() + advanceUntilIdle() + + assertEquals("the stored icon is indexed", setOf("already.example"), registry.keys.value) + } + + /** + * [BrowserIconRegistry.iconModelFor] is read from composition, so it must answer from [keys] rather + * than touch the filesystem — a host with no icon is null, not a path to a file that is not there. + */ + @Test + fun aHostWithNoStoredIconHasNoModel() = + runTest { + val registry = registryOver(newDir()) + registry.init() + advanceUntilIdle() + + assertNull(registry.iconModelFor("never-visited.example")) + } + + /** Host keys become one flat filename, so a port or an uppercase host cannot escape the directory. */ + @Test + fun hostsAreSanitizedIntoASingleFlatFilename() = + runTest { + val dir = newDir() + val registry = registryOver(dir) + registry.init() + advanceUntilIdle() + + registry.record("Example.COM:8080/../etc", png) + advanceUntilIdle() + + assertEquals( + "lowercased, and everything but letters/digits/dot/dash replaced", + setOf("example.com_8080_.._etc"), + registry.keys.value, + ) + assertEquals( + "one file, directly in the icon dir", + listOf("example.com_8080_.._etc.png"), + dir.listFiles()?.map { it.name }, + ) + } + + /** Lookups are sanitized the same way, so the caller passes the raw host and still finds it. */ + @Test + fun aLookupSanitizesTheHostTheSameWay() = + runTest { + val dir = newDir() + val registry = registryOver(dir) + registry.init() + advanceUntilIdle() + + registry.record("Example.COM", png) + advanceUntilIdle() + + assertEquals( + "the raw host resolves to the sanitized file", + "file://" + File(dir, "example.com.png").absolutePath, + registry.iconModelFor("Example.COM"), + ) + } + + @Test + fun aBlankHostOrEmptyBytesAreIgnored() = + runTest { + val dir = newDir() + val registry = registryOver(dir) + registry.init() + advanceUntilIdle() + + registry.record(" ", png) + registry.record("example.com", ByteArray(0)) + advanceUntilIdle() + + assertTrue("nothing announced", registry.keys.value.isEmpty()) + assertEquals("nothing written", emptyList(), dir.listFiles()?.map { it.name }) + } + + @Test + fun aRecordedIconSurvivesARestart() = + runTest { + val dir = newDir() + + val first = registryOver(dir) + first.init() + advanceUntilIdle() + first.record("example.com", png) + advanceUntilIdle() + + val second = registryOver(dir) + second.init() + advanceUntilIdle() + + assertEquals("indexed again from disk", setOf("example.com"), second.keys.value) + } + + /** The icon dir need not exist yet: a first run must create it rather than drop the icon. */ + @Test + fun aMissingIconDirectoryIsCreated() = + runTest { + val dir = File(folder.root, "not_yet_${seq++}") + val registry = registryOver(dir) + registry.init() + advanceUntilIdle() + + registry.record("example.com", png) + advanceUntilIdle() + + assertTrue("the directory was created", dir.isDirectory) + assertEquals("and the icon landed in it", setOf("example.com"), registry.keys.value) + } +}