8305e33 fix(favicon): address PR review cache and href issues · dumber

M internal/infrastructure/cef/favicon_discovery.go +6 -2


@@ -87,8 +87,12 @@ func faviconRel(rel string) bool {
87	87		}
88	88
89	89		func resolveFaviconURL(pageURL, rawIconURL string) (string, bool) {
90		-	icon, err := url.Parse(strings.TrimSpace(rawIconURL))
91		-	if err != nil \|\| rawIconURL == "" {
	90	+	trimmed := strings.TrimSpace(rawIconURL)
	91	+	if trimmed == "" {
	92	+	return "", false
	93	+	}
	94	+	icon, err := url.Parse(trimmed)
	95	+	if err != nil {
92	96		return "", false
93	97		}
94	98		if icon.IsAbs() {

 }
 func resolveFaviconURL(pageURL, rawIconURL string) (string, bool) {
-	icon, err := url.Parse(strings.TrimSpace(rawIconURL))
+	trimmed := strings.TrimSpace(rawIconURL)
-	if err != nil || rawIconURL == "" {
+	if trimmed == "" {
+		return "", false
+	}
+	icon, err := url.Parse(trimmed)
+	if err != nil {
 		return "", false
 	}
 	if icon.IsAbs() {

M internal/infrastructure/cef/favicon_discovery_test.go +5 -0


@@ -45,6 +45,11 @@ func TestFaviconDiscoveryIgnoresUnsafeIconURLs(t *testing.T) {
45	45		require.Equal(t, []string{"https://cdn.example.com/icon.png", "https://example.com/favicon.ico"}, got)
46	46		}
47	47
	48	+	func TestFaviconDiscoveryIgnoresWhitespaceOnlyHrefs(t *testing.T) {
	49	+	got := DiscoverFaviconCandidates("https://example.com/path", `<link rel="icon" href=" ">`)
	50	+	require.Equal(t, []string{"https://example.com/favicon.ico"}, got)
	51	+	}
	52	+
48	53		func TestCEFOnFaviconUrlchangeForwardsOrderedCandidates(t *testing.T) {
49	54		prevDecode := decodeCEFStringList
50	55		decodeCEFStringList = func(list purecef.StringList) []string {


@@ -45,6 +45,11 @@ func TestFaviconDiscoveryIgnoresUnsafeIconURLs(t *testing.T) {
45	require.Equal(t, []string{"https://cdn.example.com/icon.png", "https://example.com/favicon.ico"}, got)	45		require.Equal(t, []string{"https://cdn.example.com/icon.png", "https://example.com/favicon.ico"}, got)
46	}	46		}
47		47
		48	+	func TestFaviconDiscoveryIgnoresWhitespaceOnlyHrefs(t *testing.T) {
		49	+	got := DiscoverFaviconCandidates("https://example.com/path", `<link rel="icon" href=" ">`)
		50	+	require.Equal(t, []string{"https://example.com/favicon.ico"}, got)
		51	+	}
		52	+
48	func TestCEFOnFaviconUrlchangeForwardsOrderedCandidates(t *testing.T) {	53		func TestCEFOnFaviconUrlchangeForwardsOrderedCandidates(t *testing.T) {
49	prevDecode := decodeCEFStringList	54		prevDecode := decodeCEFStringList
50	decodeCEFStringList = func(list purecef.StringList) []string {	55		decodeCEFStringList = func(list purecef.StringList) []string {

M internal/ui/adapter/favicon.go +21 -2


@@ -234,7 +234,7 @@ func (a *FaviconAdapter) resolveTexture(ctx context.Context, pageURL string, sch
234	234		}
235	235		texture := a.textureFromBytesOnGTK(ctx, resolved.Bytes)
236	236		if texture != nil {
237		-	a.setTexture(string(resolved.Key), texture)
	237	+	a.setResolvedTexture(pageURL, resolved.Key, texture)
238	238		}
239	239		return texture
240	240		}

@@ -512,10 +512,19 @@ func (a *FaviconAdapter) Close() {
512	512		}
513	513		}
514	514
515		-	// Invalidate clears one resolved-key texture cache entry.
	515	+	// Invalidate clears one resolved-key texture cache entry and any domain aliases
	516	+	// pointing at the same texture.
516	517		func (a *FaviconAdapter) Invalidate(_ context.Context, key favicon.Key) error {
517	518		a.mu.Lock()
	519	+	texture := a.textureCache[string(key)]
518	520		delete(a.textureCache, string(key))
	521	+	if texture != nil {
	522	+	for cacheKey, cached := range a.textureCache {
	523	+	if cached == texture {
	524	+	delete(a.textureCache, cacheKey)
	525	+	}
	526	+	}
	527	+	}
519	528		a.mu.Unlock()
520	529		return nil
521	530		}

@@ -535,6 +544,16 @@ func (a *FaviconAdapter) Size() int {
535	544		return size
536	545		}
537	546
	547	+	// setResolvedTexture stores resolver results under both the resolved favicon key
	548	+	// and the current page domain. The domain alias keeps UI lookups fast while
	549	+	// Invalidate removes aliases sharing the same texture during the migration path.
	550	+	func (a FaviconAdapter) setResolvedTexture(pageURL string, key favicon.Key, texture gdk.Texture) {
	551	+	a.setTexture(string(key), texture)
	552	+	if domain := domainurl.ExtractDomain(pageURL); domain != "" {
	553	+	a.setTexture(domain, texture)
	554	+	}
	555	+	}
	556	+
538	557		// setTexture stores a texture in the cache.
539	558		func (a FaviconAdapter) setTexture(domain string, texture gdk.Texture) {
540	559		if domain == "" \|\| texture == nil {

 	}
 	texture := a.textureFromBytesOnGTK(ctx, resolved.Bytes)
 	if texture != nil {
-		a.setTexture(string(resolved.Key), texture)
+		a.setResolvedTexture(pageURL, resolved.Key, texture)
 	}
 	return texture
 }
 	}
 }
-// Invalidate clears one resolved-key texture cache entry.
+// Invalidate clears one resolved-key texture cache entry and any domain aliases
+// pointing at the same texture.
 func (a *FaviconAdapter) Invalidate(_ context.Context, key favicon.Key) error {
 	a.mu.Lock()
+	texture := a.textureCache[string(key)]
 	delete(a.textureCache, string(key))
+	if texture != nil {
+		for cacheKey, cached := range a.textureCache {
+			if cached == texture {
+				delete(a.textureCache, cacheKey)
+			}
+		}
+	}
 	a.mu.Unlock()
 	return nil
 }
 	return size
 }
+// setResolvedTexture stores resolver results under both the resolved favicon key
+// and the current page domain. The domain alias keeps UI lookups fast while
+// Invalidate removes aliases sharing the same texture during the migration path.
+func (a *FaviconAdapter) setResolvedTexture(pageURL string, key favicon.Key, texture *gdk.Texture) {
+	a.setTexture(string(key), texture)
+	if domain := domainurl.ExtractDomain(pageURL); domain != "" {
+		a.setTexture(domain, texture)
+	}
+}
 // setTexture stores a texture in the cache.
 func (a *FaviconAdapter) setTexture(domain string, texture *gdk.Texture) {
 	if domain == "" || texture == nil {

M internal/ui/adapter/favicon_test.go +19 -0

@@ -1,12 +1,31 @@
1	1		package adapter
2	2
3	3		import (
	4	+	"context"
4	5		"sync/atomic"
5	6		"testing"
6	7
	8	+	"github.com/bnema/dumber/internal/domain/favicon"
	9	+	"github.com/bnema/puregotk/v4/gdk"
7	10		"github.com/rs/zerolog"
8	11		)
9	12
	13	+	func TestFaviconAdapterInvalidateRemovesResolvedTextureAliases(t *testing.T) {
	14	+	adapter := NewFaviconAdapter(nil, nil, FaviconAdapterConfig{})
	15	+	texture := &gdk.Texture{}
	16	+	adapter.setResolvedTexture("https://docs.example.com/page", favicon.Key("example.com"), texture)
	17	+
	18	+	if adapter.GetTexture("example.com") == nil \|\| adapter.GetTexture("docs.example.com") == nil {
	19	+	t.Fatal("expected both resolved key and domain alias to be cached")
	20	+	}
	21	+	if err := adapter.Invalidate(context.Background(), "example.com"); err != nil {
	22	+	t.Fatal(err)
	23	+	}
	24	+	if adapter.GetTexture("example.com") != nil \|\| adapter.GetTexture("docs.example.com") != nil {
	25	+	t.Fatal("expected invalidation to remove resolved key and alias")
	26	+	}
	27	+	}
	28	+
10	29		func TestFaviconWarningDedup_FirstAndRepeated(t *testing.T) {
11	30		adapter := NewFaviconAdapter(nil, nil, FaviconAdapterConfig{})
12	31

@@ -1,12 +1,31 @@
1	package adapter	1		package adapter
2		2
3	import (	3		import (
		4	+	"context"
4	"sync/atomic"	5		"sync/atomic"
5	"testing"	6		"testing"
6		7
		8	+	"github.com/bnema/dumber/internal/domain/favicon"
		9	+	"github.com/bnema/puregotk/v4/gdk"
7	"github.com/rs/zerolog"	10		"github.com/rs/zerolog"
8	)	11		)
9		12
		13	+	func TestFaviconAdapterInvalidateRemovesResolvedTextureAliases(t *testing.T) {
		14	+	adapter := NewFaviconAdapter(nil, nil, FaviconAdapterConfig{})
		15	+	texture := &gdk.Texture{}
		16	+	adapter.setResolvedTexture("https://docs.example.com/page", favicon.Key("example.com"), texture)
		17	+
		18	+	if adapter.GetTexture("example.com") == nil \|\| adapter.GetTexture("docs.example.com") == nil {
		19	+	t.Fatal("expected both resolved key and domain alias to be cached")
		20	+	}
		21	+	if err := adapter.Invalidate(context.Background(), "example.com"); err != nil {
		22	+	t.Fatal(err)
		23	+	}
		24	+	if adapter.GetTexture("example.com") != nil \|\| adapter.GetTexture("docs.example.com") != nil {
		25	+	t.Fatal("expected invalidation to remove resolved key and alias")
		26	+	}
		27	+	}
		28	+
10	func TestFaviconWarningDedup_FirstAndRepeated(t *testing.T) {	29		func TestFaviconWarningDedup_FirstAndRepeated(t *testing.T) {
11	adapter := NewFaviconAdapter(nil, nil, FaviconAdapterConfig{})	30		adapter := NewFaviconAdapter(nil, nil, FaviconAdapterConfig{})
12		31