fix(popup): release aborted popup shells and guard popup config

Aborted pane-hosted popups that can no longer get a browser are destroyed so their hidden pane, pending entry and OAuth state are cleaned up. Popup config reads and writes now hold the popup manager mutex.

commit: 2baceedb5027d6d3ef8ba72faf5d2cdaedd6ef12
author: brice · 7d ago · 2026-10-15 06:06 UTC
parent: 230caa1
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +203 -4

M internal/infrastructure/cef/webview.go +24 -0


@@ -1214,6 +1214,30 @@ func (wv *WebView) handleNativePopupAborted() {
1214	1214		wv.mu.Unlock()
1215	1215		}
1216	1216
	1217	+	// releaseUnrecoverableAbortedPopup destroys a pane-hosted popup shell whose
	1218	+	// CEF popup was aborted and that can no longer obtain a browser. While
	1219	+	// pendingCreate is set the shell still recovers through direct browser
	1220	+	// creation (see preparePopupShellDirectBrowserCreation), so it is left alone.
	1221	+	// Destroy runs the lifecycle close callback registered by the content layer,
	1222	+	// which drops its pending-popup and OAuth state, closes the hidden pane and
	1223	+	// releases the view. Shells owned by a native popup host (nativePopupHostAbort
	1224	+	// installed) or without a content-layer close callback are never touched: the
	1225	+	// host owns their teardown.
	1226	+	func (wv *WebView) releaseUnrecoverableAbortedPopup() bool {
	1227	+	if wv == nil {
	1228	+	return false
	1229	+	}
	1230	+	wv.mu.RLock()
	1231	+	release := wv.nativePopupHostAbort == nil && wv.popupLifecycleClose != nil &&
	1232	+	wv.browser == nil && wv.pendingCreate == nil
	1233	+	wv.mu.RUnlock()
	1234	+	if !release {
	1235	+	return false
	1236	+	}
	1237	+	wv.Destroy()
	1238	+	return true
	1239	+	}
	1240	+
1217	1241		// ---------------------------------------------------------------------------
1218	1242		// JavaScript / Appearance
1219	1243		// ---------------------------------------------------------------------------


@@ -1214,6 +1214,30 @@ func (wv *WebView) handleNativePopupAborted() {
1214	wv.mu.Unlock()	1214		wv.mu.Unlock()
1215	}	1215		}
1216		1216
		1217	+	// releaseUnrecoverableAbortedPopup destroys a pane-hosted popup shell whose
		1218	+	// CEF popup was aborted and that can no longer obtain a browser. While
		1219	+	// pendingCreate is set the shell still recovers through direct browser
		1220	+	// creation (see preparePopupShellDirectBrowserCreation), so it is left alone.
		1221	+	// Destroy runs the lifecycle close callback registered by the content layer,
		1222	+	// which drops its pending-popup and OAuth state, closes the hidden pane and
		1223	+	// releases the view. Shells owned by a native popup host (nativePopupHostAbort
		1224	+	// installed) or without a content-layer close callback are never touched: the
		1225	+	// host owns their teardown.
		1226	+	func (wv *WebView) releaseUnrecoverableAbortedPopup() bool {
		1227	+	if wv == nil {
		1228	+	return false
		1229	+	}
		1230	+	wv.mu.RLock()
		1231	+	release := wv.nativePopupHostAbort == nil && wv.popupLifecycleClose != nil &&
		1232	+	wv.browser == nil && wv.pendingCreate == nil
		1233	+	wv.mu.RUnlock()
		1234	+	if !release {
		1235	+	return false
		1236	+	}
		1237	+	wv.Destroy()
		1238	+	return true
		1239	+	}
		1240	+
1217	// ---------------------------------------------------------------------------	1241		// ---------------------------------------------------------------------------
1218	// JavaScript / Appearance	1242		// JavaScript / Appearance
1219	// ---------------------------------------------------------------------------	1243		// ---------------------------------------------------------------------------

M internal/infrastructure/cef/webview_handlers.go +15 -2


@@ -903,9 +903,22 @@ func (h *handlerSet) handlePopupHostDecision(
903	903		}
904	904		}
905	905
	906	+	// OnBeforePopupAborted is keyed by CEF's popup_id, which only maps to a popup
	907	+	// shell through the parent's pendingNativePopups. The content layer keys its
	908	+	// pending popup and OAuth state by the shell's WebViewID, so cleanup of a shell
	909	+	// that can no longer get a browser goes through its lifecycle close callback
	910	+	// (content handlePopupClose) rather than by id. Recoverable shells keep using
	911	+	// the direct-creation fallback.
906	912		func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) {
907		-	if popup := h.wv.takePendingNativePopup(popupID); popup != nil {
908		-	popup.handleNativePopupAborted()
	913	+	popup := h.wv.takePendingNativePopup(popupID)
	914	+	if popup == nil {
	915	+	return
	916	+	}
	917	+	popup.handleNativePopupAborted()
	918	+	if popup.releaseUnrecoverableAbortedPopup() {
	919	+	logging.FromContext(h.currentContext()).Debug().
	920	+	Int32("popup_id", popupID).
	921	+	Msg("cef: popup aborted and unrecoverable, released popup shell")
909	922		}
910	923		}
911	924


@@ -903,9 +903,22 @@ func (h *handlerSet) handlePopupHostDecision(
903		}	903		}
904		}	904		}
905			905
			906	+	// OnBeforePopupAborted is keyed by CEF's popup_id, which only maps to a popup
			907	+	// shell through the parent's pendingNativePopups. The content layer keys its
			908	+	// pending popup and OAuth state by the shell's WebViewID, so cleanup of a shell
			909	+	// that can no longer get a browser goes through its lifecycle close callback
			910	+	// (content handlePopupClose) rather than by id. Recoverable shells keep using
			911	+	// the direct-creation fallback.
906		func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) {	912		func (h *handlerSet) OnBeforePopupAborted(_ purecef.Browser, popupID int32) {
907	-	if popup := h.wv.takePendingNativePopup(popupID); popup != nil {	913	+	popup := h.wv.takePendingNativePopup(popupID)
908	-	popup.handleNativePopupAborted()	914	+	if popup == nil {
			915	+	return
			916	+	}
			917	+	popup.handleNativePopupAborted()
			918	+	if popup.releaseUnrecoverableAbortedPopup() {
			919	+	logging.FromContext(h.currentContext()).Debug().
			920	+	Int32("popup_id", popupID).
			921	+	Msg("cef: popup aborted and unrecoverable, released popup shell")
909		}	922		}
910		}	923		}
911			924

M internal/infrastructure/cef/webview_popup_lifecycle_test.go +50 -0

@@ -1,6 +1,7 @@
1	1		package cef
2	2
3	3		import (
	4	+	"context"
4	5		"testing"
5	6
6	7		"github.com/stretchr/testify/assert"

@@ -24,3 +25,52 @@ func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) {
24	25		assert.Zero(t, lifecycleCloseCalls)
25	26		assert.Equal(t, 1, oauthCloseCalls)
26	27		}
	28	+
	29	+	func TestOnBeforePopupAborted_ReleasesUnrecoverablePaneHostedPopup(t *testing.T) {
	30	+	parent := &WebView{ctx: context.Background()}
	31	+	popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 77}
	32	+	closeCalls := 0
	33	+	popup.SetOnClose(func() { closeCalls++ })
	34	+	parent.trackPendingNativePopup(77, popup)
	35	+
	36	+	(&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 77)
	37	+
	38	+	assert.Equal(t, 1, closeCalls)
	39	+	assert.True(t, popup.IsDestroyed())
	40	+	assert.Nil(t, parent.takePendingNativePopup(77))
	41	+	}
	42	+
	43	+	func TestOnBeforePopupAborted_KeepsRecoverablePopupForDirectFallback(t *testing.T) {
	44	+	parent := &WebView{ctx: context.Background()}
	45	+	popup := &WebView{
	46	+	ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 78,
	47	+	pendingCreate: &pendingBrowserCreate{},
	48	+	}
	49	+	closeCalls := 0
	50	+	popup.SetOnClose(func() { closeCalls++ })
	51	+	parent.trackPendingNativePopup(78, popup)
	52	+
	53	+	(&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 78)
	54	+
	55	+	assert.Zero(t, closeCalls)
	56	+	assert.False(t, popup.IsDestroyed())
	57	+	assert.False(t, popup.isNativePopupCandidate())
	58	+	}
	59	+
	60	+	func TestOnBeforePopupAborted_LeavesNativeHostedPopupToHost(t *testing.T) {
	61	+	parent := &WebView{ctx: context.Background()}
	62	+	popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 79}
	63	+	closeCalls := 0
	64	+	popup.SetOnClose(func() { closeCalls++ })
	65	+	popup.SetNativePopupHostAbort(func() {})
	66	+	parent.trackPendingNativePopup(79, popup)
	67	+
	68	+	(&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 79)
	69	+
	70	+	assert.Zero(t, closeCalls)
	71	+	assert.False(t, popup.IsDestroyed())
	72	+	}
	73	+
	74	+	func TestOnBeforePopupAborted_UnknownPopupIsNoop(_ *testing.T) {
	75	+	(&handlerSet{wv: &WebView{ctx: context.Background()}}).OnBeforePopupAborted(nil, 123)
	76	+	}

@@ -1,6 +1,7 @@
1	package cef	1		package cef
2		2
3	import (	3		import (
		4	+	"context"
4	"testing"	5		"testing"
5		6
6	"github.com/stretchr/testify/assert"	7		"github.com/stretchr/testify/assert"

@@ -24,3 +25,52 @@ func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) {
24	assert.Zero(t, lifecycleCloseCalls)	25		assert.Zero(t, lifecycleCloseCalls)
25	assert.Equal(t, 1, oauthCloseCalls)	26		assert.Equal(t, 1, oauthCloseCalls)
26	}	27		}
		28	+
		29	+	func TestOnBeforePopupAborted_ReleasesUnrecoverablePaneHostedPopup(t *testing.T) {
		30	+	parent := &WebView{ctx: context.Background()}
		31	+	popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 77}
		32	+	closeCalls := 0
		33	+	popup.SetOnClose(func() { closeCalls++ })
		34	+	parent.trackPendingNativePopup(77, popup)
		35	+
		36	+	(&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 77)
		37	+
		38	+	assert.Equal(t, 1, closeCalls)
		39	+	assert.True(t, popup.IsDestroyed())
		40	+	assert.Nil(t, parent.takePendingNativePopup(77))
		41	+	}
		42	+
		43	+	func TestOnBeforePopupAborted_KeepsRecoverablePopupForDirectFallback(t *testing.T) {
		44	+	parent := &WebView{ctx: context.Background()}
		45	+	popup := &WebView{
		46	+	ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 78,
		47	+	pendingCreate: &pendingBrowserCreate{},
		48	+	}
		49	+	closeCalls := 0
		50	+	popup.SetOnClose(func() { closeCalls++ })
		51	+	parent.trackPendingNativePopup(78, popup)
		52	+
		53	+	(&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 78)
		54	+
		55	+	assert.Zero(t, closeCalls)
		56	+	assert.False(t, popup.IsDestroyed())
		57	+	assert.False(t, popup.isNativePopupCandidate())
		58	+	}
		59	+
		60	+	func TestOnBeforePopupAborted_LeavesNativeHostedPopupToHost(t *testing.T) {
		61	+	parent := &WebView{ctx: context.Background()}
		62	+	popup := &WebView{ctx: context.Background(), nativePopupCandidate: true, nativePopupParent: parent, nativePopupID: 79}
		63	+	closeCalls := 0
		64	+	popup.SetOnClose(func() { closeCalls++ })
		65	+	popup.SetNativePopupHostAbort(func() {})
		66	+	parent.trackPendingNativePopup(79, popup)
		67	+
		68	+	(&handlerSet{wv: parent}).OnBeforePopupAborted(nil, 79)
		69	+
		70	+	assert.Zero(t, closeCalls)
		71	+	assert.False(t, popup.IsDestroyed())
		72	+	}
		73	+
		74	+	func TestOnBeforePopupAborted_UnknownPopupIsNoop(_ *testing.T) {
		75	+	(&handlerSet{wv: &WebView{ctx: context.Background()}}).OnBeforePopupAborted(nil, 123)
		76	+	}

M internal/ui/coordinator/content/popup_lifecycle_test.go +100 -0


@@ -683,3 +683,103 @@ type popupStagingHostStub struct {
683	683
684	684		func (s *popupStagingHostStub) Detach() error { s.detached = true; return s.detachErr }
685	685		func (s *popupStagingHostStub) Destroy() { s.destroyCalls++ }
	686	+
	687	+	// ---------------------------------------------------------------------------
	688	+	// Popup config concurrency / aborted popup cleanup
	689	+	// ---------------------------------------------------------------------------
	690	+
	691	+	func TestPopupConfig_ConcurrentUpdateAndRead(t *testing.T) {
	692	+	t.Parallel()
	693	+
	694	+	pm := newPopupManager()
	695	+	var wg sync.WaitGroup
	696	+	for i := range 20 {
	697	+	wg.Add(3)
	698	+	go func() {
	699	+	defer wg.Done()
	700	+	pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: i%2 == 0})
	701	+	}()
	702	+	go func() {
	703	+	defer wg.Done()
	704	+	pm.setPopupConfig(&entity.BrowsingContextConfig{OAuthAutoClose: i%2 == 0})
	705	+	}()
	706	+	go func() {
	707	+	defer wg.Done()
	708	+	if cfg := pm.currentPopupConfig(); cfg != nil {
	709	+	_ = cfg.OpenInNewPane
	710	+	}
	711	+	}()
	712	+	}
	713	+	wg.Wait()
	714	+	}
	715	+
	716	+	func TestCurrentPopupConfig_ReturnsSnapshot(t *testing.T) {
	717	+	t.Parallel()
	718	+
	719	+	pm := newPopupManager()
	720	+	pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: true})
	721	+	snapshot := pm.currentPopupConfig()
	722	+	require.NotNil(t, snapshot)
	723	+	snapshot.OpenInNewPane = false
	724	+
	725	+	require.True(t, pm.currentPopupConfig().OpenInNewPane)
	726	+	}
	727	+
	728	+	func TestPopupClose_AbortedPendingPopupCleansPendingOAuthAndPane(t *testing.T) {
	729	+	ctx := context.Background()
	730	+	parent := mocks.NewMockWebView(t)
	731	+	parent.EXPECT().ID().Return(port.WebViewID(101)).Once()
	732	+	popup := &closableLifecyclePopupStub{popupNavigationWebViewStub: &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}}
	733	+	popup.EXPECT().ID().Return(port.WebViewID(260)).Maybe()
	734	+	popup.EXPECT().URI().Return("").Maybe()
	735	+	popup.EXPECT().IsLoading().Return(false).Maybe()
	736	+	popup.EXPECT().Generation().Return(uint64(1)).Maybe()
	737	+	popup.EXPECT().SetCallbacks(mock.Anything).Once()
	738	+	popup.EXPECT().Destroy().Maybe()
	739	+	factory := mocks.NewMockWebViewFactory(t)
	740	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popup, nil).Once()
	741	+
	742	+	c := &Coordinator{webViews: make(map[entity.PaneID]port.WebView), popups: newPopupManager()}
	743	+	c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: true, OAuthAutoClose: true}, func() string { return "popup-pane" })
	744	+	var closedPanes []entity.PaneID
	745	+	c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { return nil })
	746	+	c.SetOnClosePane(func(_ context.Context, paneID entity.PaneID) error {
	747	+	closedPanes = append(closedPanes, paneID)
	748	+	return nil
	749	+	})
	750	+
	751	+	created := c.handlePopupCreate(ctx, "parent-pane", parent, port.PopupRequest{
	752	+	TargetURI: "https://example.com/popup", FrameName: "popup", IsUserGesture: true,
	753	+	})
	754	+	require.Same(t, popup, created)
	755	+	require.Len(t, c.popups.pendingPopups, 1)
	756	+	// OAuth tracking is normally armed for auth-intent popups; seed it so the
	757	+	// cleanup contract is covered regardless of the policy's current routing.
	758	+	c.popups.trackOAuthPopup(260, "parent-pane", "https://app.example.com/login")
	759	+	require.Len(t, c.popups.popupOAuth, 1)
	760	+	require.NotNil(t, c.getWebViewLocked("popup-pane"))
	761	+	require.NotNil(t, popup.onClose)
	762	+
	763	+	// CEF aborts the popup before ready-to-show: the engine fires the lifecycle
	764	+	// close callback exactly as it does for a destroyed shell.
	765	+	popup.close()
	766	+
	767	+	assert.Empty(t, c.popups.pendingPopups)
	768	+	assert.Empty(t, c.popups.popupOAuth)
	769	+	assert.Equal(t, []entity.PaneID{"popup-pane"}, closedPanes)
	770	+	assert.Nil(t, c.getWebViewLocked("popup-pane"))
	771	+	}
	772	+
	773	+	// closableLifecyclePopupStub records the lifecycle close callback the content
	774	+	// layer installs so tests can simulate the engine tearing the shell down.
	775	+	type closableLifecyclePopupStub struct {
	776	+	*popupNavigationWebViewStub
	777	+	onClose func()
	778	+	}
	779	+
	780	+	func (s *closableLifecyclePopupStub) SetOnClose(fn func()) { s.onClose = fn }
	781	+	func (s *closableLifecyclePopupStub) close() {
	782	+	if s.onClose != nil {
	783	+	s.onClose()
	784	+	}
	785	+	}


@@ -683,3 +683,103 @@ type popupStagingHostStub struct {
683		683
684	func (s *popupStagingHostStub) Detach() error { s.detached = true; return s.detachErr }	684		func (s *popupStagingHostStub) Detach() error { s.detached = true; return s.detachErr }
685	func (s *popupStagingHostStub) Destroy() { s.destroyCalls++ }	685		func (s *popupStagingHostStub) Destroy() { s.destroyCalls++ }
		686	+
		687	+	// ---------------------------------------------------------------------------
		688	+	// Popup config concurrency / aborted popup cleanup
		689	+	// ---------------------------------------------------------------------------
		690	+
		691	+	func TestPopupConfig_ConcurrentUpdateAndRead(t *testing.T) {
		692	+	t.Parallel()
		693	+
		694	+	pm := newPopupManager()
		695	+	var wg sync.WaitGroup
		696	+	for i := range 20 {
		697	+	wg.Add(3)
		698	+	go func() {
		699	+	defer wg.Done()
		700	+	pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: i%2 == 0})
		701	+	}()
		702	+	go func() {
		703	+	defer wg.Done()
		704	+	pm.setPopupConfig(&entity.BrowsingContextConfig{OAuthAutoClose: i%2 == 0})
		705	+	}()
		706	+	go func() {
		707	+	defer wg.Done()
		708	+	if cfg := pm.currentPopupConfig(); cfg != nil {
		709	+	_ = cfg.OpenInNewPane
		710	+	}
		711	+	}()
		712	+	}
		713	+	wg.Wait()
		714	+	}
		715	+
		716	+	func TestCurrentPopupConfig_ReturnsSnapshot(t *testing.T) {
		717	+	t.Parallel()
		718	+
		719	+	pm := newPopupManager()
		720	+	pm.updatePopupConfig(entity.BrowsingContextConfig{OpenInNewPane: true})
		721	+	snapshot := pm.currentPopupConfig()
		722	+	require.NotNil(t, snapshot)
		723	+	snapshot.OpenInNewPane = false
		724	+
		725	+	require.True(t, pm.currentPopupConfig().OpenInNewPane)
		726	+	}
		727	+
		728	+	func TestPopupClose_AbortedPendingPopupCleansPendingOAuthAndPane(t *testing.T) {
		729	+	ctx := context.Background()
		730	+	parent := mocks.NewMockWebView(t)
		731	+	parent.EXPECT().ID().Return(port.WebViewID(101)).Once()
		732	+	popup := &closableLifecyclePopupStub{popupNavigationWebViewStub: &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}}
		733	+	popup.EXPECT().ID().Return(port.WebViewID(260)).Maybe()
		734	+	popup.EXPECT().URI().Return("").Maybe()
		735	+	popup.EXPECT().IsLoading().Return(false).Maybe()
		736	+	popup.EXPECT().Generation().Return(uint64(1)).Maybe()
		737	+	popup.EXPECT().SetCallbacks(mock.Anything).Once()
		738	+	popup.EXPECT().Destroy().Maybe()
		739	+	factory := mocks.NewMockWebViewFactory(t)
		740	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popup, nil).Once()
		741	+
		742	+	c := &Coordinator{webViews: make(map[entity.PaneID]port.WebView), popups: newPopupManager()}
		743	+	c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: true, OAuthAutoClose: true}, func() string { return "popup-pane" })
		744	+	var closedPanes []entity.PaneID
		745	+	c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { return nil })
		746	+	c.SetOnClosePane(func(_ context.Context, paneID entity.PaneID) error {
		747	+	closedPanes = append(closedPanes, paneID)
		748	+	return nil
		749	+	})
		750	+
		751	+	created := c.handlePopupCreate(ctx, "parent-pane", parent, port.PopupRequest{
		752	+	TargetURI: "https://example.com/popup", FrameName: "popup", IsUserGesture: true,
		753	+	})
		754	+	require.Same(t, popup, created)
		755	+	require.Len(t, c.popups.pendingPopups, 1)
		756	+	// OAuth tracking is normally armed for auth-intent popups; seed it so the
		757	+	// cleanup contract is covered regardless of the policy's current routing.
		758	+	c.popups.trackOAuthPopup(260, "parent-pane", "https://app.example.com/login")
		759	+	require.Len(t, c.popups.popupOAuth, 1)
		760	+	require.NotNil(t, c.getWebViewLocked("popup-pane"))
		761	+	require.NotNil(t, popup.onClose)
		762	+
		763	+	// CEF aborts the popup before ready-to-show: the engine fires the lifecycle
		764	+	// close callback exactly as it does for a destroyed shell.
		765	+	popup.close()
		766	+
		767	+	assert.Empty(t, c.popups.pendingPopups)
		768	+	assert.Empty(t, c.popups.popupOAuth)
		769	+	assert.Equal(t, []entity.PaneID{"popup-pane"}, closedPanes)
		770	+	assert.Nil(t, c.getWebViewLocked("popup-pane"))
		771	+	}
		772	+
		773	+	// closableLifecyclePopupStub records the lifecycle close callback the content
		774	+	// layer installs so tests can simulate the engine tearing the shell down.
		775	+	type closableLifecyclePopupStub struct {
		776	+	*popupNavigationWebViewStub
		777	+	onClose func()
		778	+	}
		779	+
		780	+	func (s *closableLifecyclePopupStub) SetOnClose(fn func()) { s.onClose = fn }
		781	+	func (s *closableLifecyclePopupStub) close() {
		782	+	if s.onClose != nil {
		783	+	s.onClose()
		784	+	}
		785	+	}

M internal/ui/coordinator/content/popup_manager.go +14 -2


@@ -130,6 +130,8 @@ func (pm popupManager) setPopupConfig(popupConfig entity.BrowsingContextConfig
130	130		if pm == nil {
131	131		return
132	132		}
	133	+	pm.mu.Lock()
	134	+	defer pm.mu.Unlock()
133	135		if popupConfig == nil {
134	136		pm.popupConfig = entity.BrowsingContextConfig{}
135	137		pm.hasPopupConfig = false

@@ -143,15 +145,25 @@ func (pm *popupManager) updatePopupConfig(popupConfig entity.BrowsingContextConf
143	145		if pm == nil {
144	146		return
145	147		}
	148	+	pm.mu.Lock()
	149	+	defer pm.mu.Unlock()
146	150		pm.popupConfig = popupConfig
147	151		pm.hasPopupConfig = true
148	152		}
149	153
	154	+	// currentPopupConfig returns a snapshot copy of the popup config (nil when
	155	+	// unset). The copy keeps callers from racing with concurrent config updates.
150	156		func (pm popupManager) currentPopupConfig() entity.BrowsingContextConfig {
151		-	if pm == nil \|\| !pm.hasPopupConfig {
	157	+	if pm == nil {
	158	+	return nil
	159	+	}
	160	+	pm.mu.RLock()
	161	+	defer pm.mu.RUnlock()
	162	+	if !pm.hasPopupConfig {
152	163		return nil
153	164		}
154		-	return &pm.popupConfig
	165	+	cfg := pm.popupConfig
	166	+	return &cfg
155	167		}
156	168
157	169		func (pm *popupManager) setWindowIDResolver(fn func(entity.PaneID) (string, bool)) {

 	if pm == nil {
 		return
 	}
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
 	if popupConfig == nil {
 		pm.popupConfig = entity.BrowsingContextConfig{}
 		pm.hasPopupConfig = false
 	if pm == nil {
 		return
 	}
+	pm.mu.Lock()
+	defer pm.mu.Unlock()
 	pm.popupConfig = popupConfig
 	pm.hasPopupConfig = true
 }
+// currentPopupConfig returns a snapshot copy of the popup config (nil when
+// unset). The copy keeps callers from racing with concurrent config updates.
 func (pm *popupManager) currentPopupConfig() *entity.BrowsingContextConfig {
-	if pm == nil || !pm.hasPopupConfig {
+	if pm == nil {
+		return nil
+	}
+	pm.mu.RLock()
+	defer pm.mu.RUnlock()
+	if !pm.hasPopupConfig {
 		return nil
 	}
-	return &pm.popupConfig
+	cfg := pm.popupConfig
+	return &cfg
 }
 func (pm *popupManager) setWindowIDResolver(fn func(entity.PaneID) (string, bool)) {