From 69151be1c94e5b3550cd440949404f92ebf90a23 Mon Sep 17 00:00:00 2001 From: brice Date: Wed, 15 Jul 2026 11:07:48 +0200 Subject: [PATCH] refactor: consolidate reveal state --- .../content/content_reveal_test.go | 16 +++++ .../ui/coordinator/content/coordinator.go | 58 +++++++------------ internal/ui/coordinator/content/navigation.go | 17 +++--- 3 files changed, 46 insertions(+), 45 deletions(-) diff --git a/internal/ui/coordinator/content/content_reveal_test.go b/internal/ui/coordinator/content/content_reveal_test.go index 911a3626..d349766d 100644 --- a/internal/ui/coordinator/content/content_reveal_test.go +++ b/internal/ui/coordinator/content/content_reveal_test.go @@ -54,6 +54,22 @@ func TestRevealState_IsBoundToCurrentWebViewAcrossReplacementPaths(t *testing.T) assert.False(t, c.WebViewRevealed(paneID), "popup registration must not inherit a prior WebView reveal") } +func TestRevealState_StoresIdentityAsTheOnlyStateValue(t *testing.T) { + paneID := entity.PaneID("pane-1") + wv := revealTestWebView(t, 101, 1) + c := newRevealTestCoordinator() + c.setWebViewLocked(paneID, wv) + identity, _ := identityForWebView(wv) + + c.markPendingReveal(paneID, identity) + c.markWebViewRevealed(paneID, identity) + + c.revealMu.Lock() + defer c.revealMu.Unlock() + assert.Equal(t, identity, c.pendingReveal[paneID]) + assert.Equal(t, identity, c.revealedWebViews[paneID]) +} + func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { paneID := entity.PaneID("pane-1") wv := revealTestWebView(t, 101, 1) diff --git a/internal/ui/coordinator/content/coordinator.go b/internal/ui/coordinator/content/coordinator.go index a33c6650..f072d00b 100644 --- a/internal/ui/coordinator/content/coordinator.go +++ b/internal/ui/coordinator/content/coordinator.go @@ -80,11 +80,9 @@ type Coordinator struct { // revealMu is always acquired after webViewsMu. Reveal state is keyed by the // WebView identity (ID plus pool-reuse generation), never by pane alone. - revealMu sync.Mutex - pendingReveal map[entity.PaneID]bool - revealedWebViews map[entity.PaneID]bool - revealedWebViewIdentity map[entity.PaneID]webViewIdentity - pendingRevealIdentity map[entity.PaneID]webViewIdentity + revealMu sync.Mutex + pendingReveal map[entity.PaneID]webViewIdentity + revealedWebViews map[entity.PaneID]webViewIdentity appearanceMu sync.Mutex pendingScriptRefresh map[entity.PaneID]bool @@ -149,25 +147,23 @@ func NewCoordinator( log.Debug().Msg("creating content coordinator") return &Coordinator{ - logger: log.With().Str("component", "content-coordinator").Logger(), - pool: pool, - injector: injector, - widgetFactory: widgetFactory, - faviconAdapter: faviconAdapter, - zoomUC: zoomUC, - permissionUC: permissionUC, - webViews: make(map[entity.PaneID]port.WebView), - webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), - paneTitles: make(map[entity.PaneID]string), - navOrigins: make(map[entity.PaneID]string), - pendingReveal: make(map[entity.PaneID]bool), - revealedWebViews: make(map[entity.PaneID]bool), - revealedWebViewIdentity: make(map[entity.PaneID]webViewIdentity), - pendingRevealIdentity: make(map[entity.PaneID]webViewIdentity), - pendingScriptRefresh: make(map[entity.PaneID]bool), - pendingThemePanes: make(map[entity.PaneID]bool), - getActiveWS: getActiveWS, - popups: newPopupManager(), + logger: log.With().Str("component", "content-coordinator").Logger(), + pool: pool, + injector: injector, + widgetFactory: widgetFactory, + faviconAdapter: faviconAdapter, + zoomUC: zoomUC, + permissionUC: permissionUC, + webViews: make(map[entity.PaneID]port.WebView), + webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), + paneTitles: make(map[entity.PaneID]string), + navOrigins: make(map[entity.PaneID]string), + pendingReveal: make(map[entity.PaneID]webViewIdentity), + revealedWebViews: make(map[entity.PaneID]webViewIdentity), + pendingScriptRefresh: make(map[entity.PaneID]bool), + pendingThemePanes: make(map[entity.PaneID]bool), + getActiveWS: getActiveWS, + popups: newPopupManager(), } } @@ -334,16 +330,10 @@ func identityForWebView(wv port.WebView) (webViewIdentity, bool) { func (c *Coordinator) ensureRevealMapsLocked() { if c.pendingReveal == nil { - c.pendingReveal = make(map[entity.PaneID]bool) + c.pendingReveal = make(map[entity.PaneID]webViewIdentity) } if c.revealedWebViews == nil { - c.revealedWebViews = make(map[entity.PaneID]bool) - } - if c.revealedWebViewIdentity == nil { - c.revealedWebViewIdentity = make(map[entity.PaneID]webViewIdentity) - } - if c.pendingRevealIdentity == nil { - c.pendingRevealIdentity = make(map[entity.PaneID]webViewIdentity) + c.revealedWebViews = make(map[entity.PaneID]webViewIdentity) } } @@ -368,9 +358,7 @@ func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { } c.ensureRevealMapsLocked() delete(c.pendingReveal, paneID) - delete(c.pendingRevealIdentity, paneID) delete(c.revealedWebViews, paneID) - delete(c.revealedWebViewIdentity, paneID) } // deleteWebViewLocked removes both the mapping and any presentation state, @@ -389,9 +377,7 @@ func (c *Coordinator) deleteWebViewLocked(paneID entity.PaneID) port.WebView { } c.ensureRevealMapsLocked() delete(c.pendingReveal, paneID) - delete(c.pendingRevealIdentity, paneID) delete(c.revealedWebViews, paneID) - delete(c.revealedWebViewIdentity, paneID) return wv } diff --git a/internal/ui/coordinator/content/navigation.go b/internal/ui/coordinator/content/navigation.go index 83e96408..1ffdb57f 100644 --- a/internal/ui/coordinator/content/navigation.go +++ b/internal/ui/coordinator/content/navigation.go @@ -255,8 +255,7 @@ func (c *Coordinator) markPendingReveal(paneID entity.PaneID, identity webViewId return } c.ensureRevealMapsLocked() - c.pendingReveal[paneID] = true - c.pendingRevealIdentity[paneID] = identity + c.pendingReveal[paneID] = identity } func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, identity webViewIdentity) { @@ -264,11 +263,11 @@ func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, identity webViewI c.revealMu.Lock() defer c.revealMu.Unlock() defer c.webViewsMu.RUnlock() - if c.pendingRevealIdentity[paneID] != identity { + pendingIdentity, pending := c.pendingReveal[paneID] + if !pending || pendingIdentity != identity { return } delete(c.pendingReveal, paneID) - delete(c.pendingRevealIdentity, paneID) } // WebViewRevealed reports whether this pane's current WebView has completed a @@ -293,7 +292,8 @@ func (c *Coordinator) webViewRevealed(paneID entity.PaneID, expected port.WebVie return false } } - return c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity + revealedIdentity, revealed := c.revealedWebViews[paneID] + return revealed && revealedIdentity == identity } func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, identity webViewIdentity) bool { @@ -306,8 +306,7 @@ func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, identity webView return false } c.ensureRevealMapsLocked() - c.revealedWebViews[paneID] = true - c.revealedWebViewIdentity[paneID] = identity + c.revealedWebViews[paneID] = identity return true } @@ -321,7 +320,8 @@ func (c *Coordinator) revealIfPending( c.webViewsMu.RLock() c.revealMu.Lock() - if c.pendingRevealIdentity[paneID] != identity || !c.pendingReveal[paneID] { + pendingIdentity, pending := c.pendingReveal[paneID] + if !pending || pendingIdentity != identity { c.revealMu.Unlock() c.webViewsMu.RUnlock() c.revealMutationMu.Unlock() @@ -335,7 +335,6 @@ func (c *Coordinator) revealIfPending( return } delete(c.pendingReveal, paneID) - delete(c.pendingRevealIdentity, paneID) c.revealMu.Unlock() c.webViewsMu.RUnlock()