detached 9a38c58

fix(popup): tear down unresolved staged views

commit
9a38c58ced29178afbbce1e1aece5b35a1683556
author
brice · · 2026-07-25 08:19 UTC
committer
brice · · 2026-07-26 16:42 UTC
parent
2b549d6
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
6 files · +111 -6
Unified
UnifiedSplit
M internal/ui/app.go +3 -0
@@ -2956,6 +2956,9 @@ func (a *App) onShutdown(ctx context.Context) {
29562956 for _, tabID := range tabIDs {
29572957 a.releaseFloatingSessionsForTab(ctx, tabID)
29582958 }
2959+ if a.contentCoord != nil {
2960+ a.contentCoord.ShutdownPopups()
2961+ }
29592962 if len(a.nativePopupWindows) > 0 {
29602963 popupIDs := make([]port.WebViewID, 0, len(a.nativePopupWindows))
29612964 for popupID := range a.nativePopupWindows {
@@ -2956,6 +2956,9 @@ func (a *App) onShutdown(ctx context.Context) {
2956 for _, tabID := range tabIDs {2956 for _, tabID := range tabIDs {
2957 a.releaseFloatingSessionsForTab(ctx, tabID)2957 a.releaseFloatingSessionsForTab(ctx, tabID)
2958 }2958 }
2959+ if a.contentCoord != nil {
2960+ a.contentCoord.ShutdownPopups()
2961+ }
2959 if len(a.nativePopupWindows) > 0 {2962 if len(a.nativePopupWindows) > 0 {
2960 popupIDs := make([]port.WebViewID, 0, len(a.nativePopupWindows))2963 popupIDs := make([]port.WebViewID, 0, len(a.nativePopupWindows))
2961 for popupID := range a.nativePopupWindows {2964 for popupID := range a.nativePopupWindows {
M internal/ui/coordinator/content/lifecycle.go +1 -0
@@ -48,6 +48,7 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (
4848 // ReleaseWebView returns the WebView for a pane to the pool.
4949 func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) {
5050 log := logging.FromContext(ctx)
51+ c.ensurePopupManager().cancelDeferredPopupsForParent(paneID)
5152
5253 // deleteWebViewLocked clears reveal state before returning, even when a
5354 // previous cleanup already removed this pane's mapping.
@@ -48,6 +48,7 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) (
48 // ReleaseWebView returns the WebView for a pane to the pool.48 // ReleaseWebView returns the WebView for a pane to the pool.
49 func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) {49 func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) {
50 log := logging.FromContext(ctx)50 log := logging.FromContext(ctx)
51+ c.ensurePopupManager().cancelDeferredPopupsForParent(paneID)
51 52
52 // deleteWebViewLocked clears reveal state before returning, even when a53 // deleteWebViewLocked clears reveal state before returning, even when a
53 // previous cleanup already removed this pane's mapping.54 // previous cleanup already removed this pane's mapping.
M internal/ui/coordinator/content/popup.go +8 -1
@@ -202,7 +202,14 @@ func (c *Coordinator) SetOnStagePopup(fn func(context.Context, StagePopupInput)
202202 }
203203
204204 func (c *Coordinator) ClearPopupNamedContextsForWindow(windowID string) {
205- c.ensurePopupManager().clearReusableNamedPopupsForWindow(windowID)
205+ pm := c.ensurePopupManager()
206+ pm.cancelDeferredPopupsForWindow(windowID)
207+ pm.clearReusableNamedPopupsForWindow(windowID)
208+}
209+
210+// ShutdownPopups releases unresolved staged popups before the browser engine exits.
211+func (c *Coordinator) ShutdownPopups() {
212+ c.ensurePopupManager().cancelAllDeferredPopups()
206213 }
207214
208215 // SetOnInsertPopup sets the callback to insert popups into the workspace.
@@ -202,7 +202,14 @@ func (c *Coordinator) SetOnStagePopup(fn func(context.Context, StagePopupInput)
202 }202 }
203 203
204 func (c *Coordinator) ClearPopupNamedContextsForWindow(windowID string) {204 func (c *Coordinator) ClearPopupNamedContextsForWindow(windowID string) {
205- c.ensurePopupManager().clearReusableNamedPopupsForWindow(windowID)205+ pm := c.ensurePopupManager()
206+ pm.cancelDeferredPopupsForWindow(windowID)
207+ pm.clearReusableNamedPopupsForWindow(windowID)
208+}
209+
210+// ShutdownPopups releases unresolved staged popups before the browser engine exits.
211+func (c *Coordinator) ShutdownPopups() {
212+ c.ensurePopupManager().cancelAllDeferredPopups()
206 }213 }
207 214
208 // SetOnInsertPopup sets the callback to insert popups into the workspace.215 // SetOnInsertPopup sets the callback to insert popups into the workspace.
M internal/ui/coordinator/content/popup_deferred.go +48 -1
@@ -43,13 +43,17 @@ func (pm *popupManager) awaitPopupFeatures(
4343 }
4444 pm.setBrowsingContextDecision(popupWV, decision)
4545 paneID, _ := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
46+ ownerWindowID := ""
47+ if pm.windowIDForPane != nil {
48+ ownerWindowID, _ = pm.windowIDForPane(parentPaneID)
49+ }
4650 pending := &deferredPendingPopup{
4751 PendingPopup: &PendingPopup{
4852 PaneID: paneID, WebView: popupWV, ParentPaneID: parentPaneID,
4953 ParentWebViewID: parentWebViewID, TargetURI: req.TargetURI, FrameName: req.FrameName,
5054 IsUserGesture: req.IsUserGesture, PopupType: DetectPopupType(req.FrameName), CreatedAt: time.Now(),
5155 },
52- Request: req, Decision: decision, StagingHost: staging,
56+ Request: req, Decision: decision, StagingHost: staging, OwnerWindowID: ownerWindowID,
5357 }
5458 pm.storeDeferredPopup(popupWV.ID(), pending)
5559 callbackCtx := logging.WithContext(context.Background(), *log)
@@ -67,6 +71,10 @@ func (pm *popupManager) cleanupDeferredPending(pending *deferredPendingPopup) {
6771 return
6872 }
6973 pending.cleanupOnce.Do(func() {
74+ if lifecycle, ok := pending.WebView.(port.PopupLifecycleCapable); ok {
75+ lifecycle.SetOnReadyToShow(nil)
76+ lifecycle.SetOnClose(nil)
77+ }
7078 if pending.StagingHost != nil {
7179 pending.StagingHost.Destroy()
7280 }
@@ -76,6 +84,45 @@ func (pm *popupManager) cleanupDeferredPending(pending *deferredPendingPopup) {
7684 })
7785 }
7886
87+func (pm *popupManager) cancelDeferredPopupsMatching(match func(*deferredPendingPopup) bool) {
88+ if pm == nil || match == nil {
89+ return
90+ }
91+ pm.mu.Lock()
92+ cancelled := make([]*deferredPendingPopup, 0)
93+ for popupID, pending := range pm.deferredPopups {
94+ if pending == nil || !match(pending) {
95+ continue
96+ }
97+ delete(pm.deferredPopups, popupID)
98+ delete(pm.pendingPopups, popupID)
99+ cancelled = append(cancelled, pending)
100+ }
101+ pm.mu.Unlock()
102+ for _, pending := range cancelled {
103+ pm.cleanupDeferredPending(pending)
104+ }
105+}
106+
107+func (pm *popupManager) cancelDeferredPopupsForParent(parentPaneID entity.PaneID) {
108+ pm.cancelDeferredPopupsMatching(func(pending *deferredPendingPopup) bool {
109+ return pending.ParentPaneID == parentPaneID
110+ })
111+}
112+
113+func (pm *popupManager) cancelDeferredPopupsForWindow(windowID string) {
114+ if pm == nil || windowID == "" {
115+ return
116+ }
117+ pm.cancelDeferredPopupsMatching(func(pending *deferredPendingPopup) bool {
118+ return pending.OwnerWindowID == windowID
119+ })
120+}
121+
122+func (pm *popupManager) cancelAllDeferredPopups() {
123+ pm.cancelDeferredPopupsMatching(func(*deferredPendingPopup) bool { return true })
124+}
125+
79126 func (pm *popupManager) failPendingPopup(
80127 ctx context.Context,
81128 popupID port.WebViewID,
@@ -43,13 +43,17 @@ func (pm *popupManager) awaitPopupFeatures(
43 }43 }
44 pm.setBrowsingContextDecision(popupWV, decision)44 pm.setBrowsingContextDecision(popupWV, decision)
45 paneID, _ := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)45 paneID, _ := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
46+ ownerWindowID := ""
47+ if pm.windowIDForPane != nil {
48+ ownerWindowID, _ = pm.windowIDForPane(parentPaneID)
49+ }
46 pending := &deferredPendingPopup{50 pending := &deferredPendingPopup{
47 PendingPopup: &PendingPopup{51 PendingPopup: &PendingPopup{
48 PaneID: paneID, WebView: popupWV, ParentPaneID: parentPaneID,52 PaneID: paneID, WebView: popupWV, ParentPaneID: parentPaneID,
49 ParentWebViewID: parentWebViewID, TargetURI: req.TargetURI, FrameName: req.FrameName,53 ParentWebViewID: parentWebViewID, TargetURI: req.TargetURI, FrameName: req.FrameName,
50 IsUserGesture: req.IsUserGesture, PopupType: DetectPopupType(req.FrameName), CreatedAt: time.Now(),54 IsUserGesture: req.IsUserGesture, PopupType: DetectPopupType(req.FrameName), CreatedAt: time.Now(),
51 },55 },
52- Request: req, Decision: decision, StagingHost: staging,56+ Request: req, Decision: decision, StagingHost: staging, OwnerWindowID: ownerWindowID,
53 }57 }
54 pm.storeDeferredPopup(popupWV.ID(), pending)58 pm.storeDeferredPopup(popupWV.ID(), pending)
55 callbackCtx := logging.WithContext(context.Background(), *log)59 callbackCtx := logging.WithContext(context.Background(), *log)
@@ -67,6 +71,10 @@ func (pm *popupManager) cleanupDeferredPending(pending *deferredPendingPopup) {
67 return71 return
68 }72 }
69 pending.cleanupOnce.Do(func() {73 pending.cleanupOnce.Do(func() {
74+ if lifecycle, ok := pending.WebView.(port.PopupLifecycleCapable); ok {
75+ lifecycle.SetOnReadyToShow(nil)
76+ lifecycle.SetOnClose(nil)
77+ }
70 if pending.StagingHost != nil {78 if pending.StagingHost != nil {
71 pending.StagingHost.Destroy()79 pending.StagingHost.Destroy()
72 }80 }
@@ -76,6 +84,45 @@ func (pm *popupManager) cleanupDeferredPending(pending *deferredPendingPopup) {
76 })84 })
77 }85 }
78 86
87+func (pm *popupManager) cancelDeferredPopupsMatching(match func(*deferredPendingPopup) bool) {
88+ if pm == nil || match == nil {
89+ return
90+ }
91+ pm.mu.Lock()
92+ cancelled := make([]*deferredPendingPopup, 0)
93+ for popupID, pending := range pm.deferredPopups {
94+ if pending == nil || !match(pending) {
95+ continue
96+ }
97+ delete(pm.deferredPopups, popupID)
98+ delete(pm.pendingPopups, popupID)
99+ cancelled = append(cancelled, pending)
100+ }
101+ pm.mu.Unlock()
102+ for _, pending := range cancelled {
103+ pm.cleanupDeferredPending(pending)
104+ }
105+}
106+
107+func (pm *popupManager) cancelDeferredPopupsForParent(parentPaneID entity.PaneID) {
108+ pm.cancelDeferredPopupsMatching(func(pending *deferredPendingPopup) bool {
109+ return pending.ParentPaneID == parentPaneID
110+ })
111+}
112+
113+func (pm *popupManager) cancelDeferredPopupsForWindow(windowID string) {
114+ if pm == nil || windowID == "" {
115+ return
116+ }
117+ pm.cancelDeferredPopupsMatching(func(pending *deferredPendingPopup) bool {
118+ return pending.OwnerWindowID == windowID
119+ })
120+}
121+
122+func (pm *popupManager) cancelAllDeferredPopups() {
123+ pm.cancelDeferredPopupsMatching(func(*deferredPendingPopup) bool { return true })
124+}
125+
79 func (pm *popupManager) failPendingPopup(126 func (pm *popupManager) failPendingPopup(
80 ctx context.Context,127 ctx context.Context,
81 popupID port.WebViewID,128 popupID port.WebViewID,
M internal/ui/coordinator/content/popup_manager.go +5 -4
@@ -39,10 +39,11 @@ type popupManager struct {
3939
4040 type deferredPendingPopup struct {
4141 *PendingPopup
42- Request port.PopupRequest
43- Decision dto.HostDecision
44- StagingHost PopupStagingHost
45- cleanupOnce sync.Once
42+ Request port.PopupRequest
43+ Decision dto.HostDecision
44+ StagingHost PopupStagingHost
45+ OwnerWindowID string
46+ cleanupOnce sync.Once
4647 }
4748
4849 type popupOAuthState struct {
@@ -39,10 +39,11 @@ type popupManager struct {
39 39
40 type deferredPendingPopup struct {40 type deferredPendingPopup struct {
41 *PendingPopup41 *PendingPopup
42- Request port.PopupRequest42+ Request port.PopupRequest
43- Decision dto.HostDecision43+ Decision dto.HostDecision
44- StagingHost PopupStagingHost44+ StagingHost PopupStagingHost
45- cleanupOnce sync.Once45+ OwnerWindowID string
46+ cleanupOnce sync.Once
46 }47 }
47 48
48 type popupOAuthState struct {49 type popupOAuthState struct {
M internal/ui/coordinator/content/popup_test.go +46 -0
@@ -1417,6 +1417,52 @@ func TestPopupDeferredStagingFailureDestroysCallerOwnedWebViewOnce(t *testing.T)
14171417 assert.Nil(t, c.handlePopupCreate(context.Background(), "floating", parent, port.PopupRequest{Engine: dto.BrowserEngineWebKit, TargetURI: "https://example.com/open", PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesUnknown}}))
14181418 }
14191419
1420+func TestPopupDeferredExplicitTeardownCleansUpWithoutLifecycleSignals(t *testing.T) {
1421+ for _, tc := range []struct {
1422+ name string
1423+ prepare func(*Coordinator)
1424+ trigger func(*Coordinator)
1425+ }{
1426+ {name: "parent release", trigger: func(c *Coordinator) { c.ReleaseWebView(context.Background(), "floating") }},
1427+ {name: "owner window teardown", prepare: func(c *Coordinator) {
1428+ c.SetPopupWindowIDResolver(func(entity.PaneID) (string, bool) { return "owner", true })
1429+ }, trigger: func(c *Coordinator) { c.ClearPopupNamedContextsForWindow("owner") }},
1430+ {name: "coordinator shutdown", trigger: func(c *Coordinator) { c.ShutdownPopups() }},
1431+ } {
1432+ t.Run(tc.name, func(t *testing.T) {
1433+ parent := mocks.NewMockWebView(t)
1434+ parent.EXPECT().ID().Return(port.WebViewID(101)).Once()
1435+ popup := &deferredPopupWebViewStub{MockWebView: mocks.NewMockWebView(t), features: dto.PopupFeatures{State: dto.PopupFeaturesNone}}
1436+ popup.EXPECT().ID().Return(port.WebViewID(207)).Maybe()
1437+ popup.EXPECT().Destroy().Once()
1438+ factory := mocks.NewMockWebViewFactory(t)
1439+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popup, nil).Once()
1440+ staging := &popupStagingHostStub{}
1441+ c := &Coordinator{popups: newPopupManager()}
1442+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, nil)
1443+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
1444+ c.SetOnStagePopup(func(context.Context, StagePopupInput) (PopupStagingHost, error) { return staging, nil })
1445+ if tc.prepare != nil {
1446+ tc.prepare(c)
1447+ }
1448+
1449+ require.Same(t, popup, c.handlePopupCreate(context.Background(), "floating", parent, port.PopupRequest{
1450+ Engine: dto.BrowserEngineWebKit, TargetURI: "https://example.com/open",
1451+ PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesUnknown},
1452+ }))
1453+ require.Len(t, c.popups.deferredPopups, 1)
1454+ tc.trigger(c)
1455+ tc.trigger(c)
1456+
1457+ assert.Empty(t, c.popups.deferredPopups)
1458+ assert.Empty(t, c.popups.pendingPopups)
1459+ assert.Equal(t, 1, staging.destroyCalls)
1460+ assert.Nil(t, popup.onReady)
1461+ assert.Nil(t, popup.onClose)
1462+ })
1463+ }
1464+}
1465+
14201466 func TestPopupDeferredUnknownAndCloseCleanupExactlyOnce(t *testing.T) {
14211467 for _, tc := range []struct {
14221468 name string
@@ -1417,6 +1417,52 @@ func TestPopupDeferredStagingFailureDestroysCallerOwnedWebViewOnce(t *testing.T)
1417 assert.Nil(t, c.handlePopupCreate(context.Background(), "floating", parent, port.PopupRequest{Engine: dto.BrowserEngineWebKit, TargetURI: "https://example.com/open", PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesUnknown}}))1417 assert.Nil(t, c.handlePopupCreate(context.Background(), "floating", parent, port.PopupRequest{Engine: dto.BrowserEngineWebKit, TargetURI: "https://example.com/open", PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesUnknown}}))
1418 }1418 }
1419 1419
1420+func TestPopupDeferredExplicitTeardownCleansUpWithoutLifecycleSignals(t *testing.T) {
1421+ for _, tc := range []struct {
1422+ name string
1423+ prepare func(*Coordinator)
1424+ trigger func(*Coordinator)
1425+ }{
1426+ {name: "parent release", trigger: func(c *Coordinator) { c.ReleaseWebView(context.Background(), "floating") }},
1427+ {name: "owner window teardown", prepare: func(c *Coordinator) {
1428+ c.SetPopupWindowIDResolver(func(entity.PaneID) (string, bool) { return "owner", true })
1429+ }, trigger: func(c *Coordinator) { c.ClearPopupNamedContextsForWindow("owner") }},
1430+ {name: "coordinator shutdown", trigger: func(c *Coordinator) { c.ShutdownPopups() }},
1431+ } {
1432+ t.Run(tc.name, func(t *testing.T) {
1433+ parent := mocks.NewMockWebView(t)
1434+ parent.EXPECT().ID().Return(port.WebViewID(101)).Once()
1435+ popup := &deferredPopupWebViewStub{MockWebView: mocks.NewMockWebView(t), features: dto.PopupFeatures{State: dto.PopupFeaturesNone}}
1436+ popup.EXPECT().ID().Return(port.WebViewID(207)).Maybe()
1437+ popup.EXPECT().Destroy().Once()
1438+ factory := mocks.NewMockWebViewFactory(t)
1439+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popup, nil).Once()
1440+ staging := &popupStagingHostStub{}
1441+ c := &Coordinator{popups: newPopupManager()}
1442+ c.SetPopupConfig(factory, &entity.BrowsingContextConfig{OpenInNewPane: false}, nil)
1443+ c.SetPopupSourceHostResolver(func(entity.PaneID) dto.SourceHostKind { return dto.SourceHostFloating })
1444+ c.SetOnStagePopup(func(context.Context, StagePopupInput) (PopupStagingHost, error) { return staging, nil })
1445+ if tc.prepare != nil {
1446+ tc.prepare(c)
1447+ }
1448+
1449+ require.Same(t, popup, c.handlePopupCreate(context.Background(), "floating", parent, port.PopupRequest{
1450+ Engine: dto.BrowserEngineWebKit, TargetURI: "https://example.com/open",
1451+ PopupFeatures: dto.PopupFeatures{State: dto.PopupFeaturesUnknown},
1452+ }))
1453+ require.Len(t, c.popups.deferredPopups, 1)
1454+ tc.trigger(c)
1455+ tc.trigger(c)
1456+
1457+ assert.Empty(t, c.popups.deferredPopups)
1458+ assert.Empty(t, c.popups.pendingPopups)
1459+ assert.Equal(t, 1, staging.destroyCalls)
1460+ assert.Nil(t, popup.onReady)
1461+ assert.Nil(t, popup.onClose)
1462+ })
1463+ }
1464+}
1465+
1420 func TestPopupDeferredUnknownAndCloseCleanupExactlyOnce(t *testing.T) {1466 func TestPopupDeferredUnknownAndCloseCleanupExactlyOnce(t *testing.T) {
1421 for _, tc := range []struct {1467 for _, tc := range []struct {
1422 name string1468 name string

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close