detached 499f6a4

Stabilize pane-hosted popup fallback cleanup

Reason:
Two remaining popup follow-ups were still valid on this branch. CEF popup shells could retain stale native fallback state after switching to pane-hosted handling, and named browsing-context reuse could drop the popup entirely when the previously registered pane was no longer available.

Scope:
- clear stale CEF native popup fallback state when preparing pane-hosted browsing contexts
- fall back to creating a new pane when named browsing-context reuse fails at runtime
- add focused regression coverage for both cases

Description:
This commit makes pane-hosted CEF popup preparation stop and clear pending native fallback timers, reset the fallback-started flag, and clear stale opener-bridge state so reused popup shells cannot leak native-popup arming state. It also changes named browsing-context reuse so an unavailable target no longer drops the popup request; instead Dumber logs the miss and creates a replacement pane using the existing creation path. Regression tests lock both behaviors down.
commit
499f6a4649fe9e552be2cedd092e949c08857acd
author
brice · · 2026-05-13 17:47 UTC
parent
d0ff807
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +100 -5
Unified
UnifiedSplit
M internal/infrastructure/cef/webview.go +10 -0
@@ -664,11 +664,21 @@ func (wv *WebView) discardNativePopupCandidate() {
664664 if wv == nil {
665665 return
666666 }
667+ var timer stoppableTimer
667668 wv.mu.Lock()
669+ timer = wv.nativePopupFallbackTimer
670+ wv.nativePopupFallbackTimer = nil
668671 wv.nativePopupCandidate = false
669672 wv.nativePopupParent = nil
670673 wv.nativePopupID = 0
674+ wv.nativePopupFallbackStarted = false
675+ wv.popupOpenerBridgeParent = nil
676+ wv.popupOpenerBridgeParentURI = ""
677+ wv.syncPopupOpenerBridgeExtraInfoLocked()
671678 wv.mu.Unlock()
679+ if timer != nil {
680+ timer.Stop()
681+ }
672682 }
673683
674684 func (wv *WebView) PreparePaneHostedBrowsingContext() {
@@ -664,11 +664,21 @@ func (wv *WebView) discardNativePopupCandidate() {
664 if wv == nil {664 if wv == nil {
665 return665 return
666 }666 }
667+ var timer stoppableTimer
667 wv.mu.Lock()668 wv.mu.Lock()
669+ timer = wv.nativePopupFallbackTimer
670+ wv.nativePopupFallbackTimer = nil
668 wv.nativePopupCandidate = false671 wv.nativePopupCandidate = false
669 wv.nativePopupParent = nil672 wv.nativePopupParent = nil
670 wv.nativePopupID = 0673 wv.nativePopupID = 0
674+ wv.nativePopupFallbackStarted = false
675+ wv.popupOpenerBridgeParent = nil
676+ wv.popupOpenerBridgeParentURI = ""
677+ wv.syncPopupOpenerBridgeExtraInfoLocked()
671 wv.mu.Unlock()678 wv.mu.Unlock()
679+ if timer != nil {
680+ timer.Stop()
681+ }
672 }682 }
673 683
674 func (wv *WebView) PreparePaneHostedBrowsingContext() {684 func (wv *WebView) PreparePaneHostedBrowsingContext() {
M internal/infrastructure/cef/webview_handlers_test.go +33 -0
@@ -98,6 +98,39 @@ type stubStoppableTimer struct{}
9898
9999 func (stubStoppableTimer) Stop() bool { return true }
100100
101+type spyStoppableTimer struct{ stopped bool }
102+
103+func (t *spyStoppableTimer) Stop() bool {
104+ t.stopped = true
105+ return true
106+}
107+
108+func TestPreparePaneHostedBrowsingContext_ClearsNativePopupFallbackState(t *testing.T) {
109+ parent := &WebView{ctx: context.Background()}
110+ timer := &spyStoppableTimer{}
111+ wv := &WebView{
112+ nativePopupCandidate: true,
113+ nativePopupParent: parent,
114+ nativePopupID: 77,
115+ nativePopupFallbackStarted: true,
116+ nativePopupFallbackTimer: timer,
117+ pendingCreate: &pendingBrowserCreate{},
118+ popupOpenerBridgeParent: parent,
119+ popupOpenerBridgeParentURI: "https://example.com/login",
120+ }
121+
122+ wv.PreparePaneHostedBrowsingContext()
123+
124+ require.False(t, wv.nativePopupCandidate)
125+ require.Nil(t, wv.nativePopupParent)
126+ require.Zero(t, wv.nativePopupID)
127+ require.False(t, wv.nativePopupFallbackStarted)
128+ require.Nil(t, wv.nativePopupFallbackTimer)
129+ require.True(t, timer.stopped)
130+ require.Nil(t, wv.popupOpenerBridgeParent)
131+ require.Empty(t, wv.popupOpenerBridgeParentURI)
132+}
133+
101134 func TestStartNativePopupFallback_AllowsPopupShellAwaitingArm(t *testing.T) {
102135 wv := &WebView{
103136 nativePopupCandidate: true,
@@ -98,6 +98,39 @@ type stubStoppableTimer struct{}
98 98
99 func (stubStoppableTimer) Stop() bool { return true }99 func (stubStoppableTimer) Stop() bool { return true }
100 100
101+type spyStoppableTimer struct{ stopped bool }
102+
103+func (t *spyStoppableTimer) Stop() bool {
104+ t.stopped = true
105+ return true
106+}
107+
108+func TestPreparePaneHostedBrowsingContext_ClearsNativePopupFallbackState(t *testing.T) {
109+ parent := &WebView{ctx: context.Background()}
110+ timer := &spyStoppableTimer{}
111+ wv := &WebView{
112+ nativePopupCandidate: true,
113+ nativePopupParent: parent,
114+ nativePopupID: 77,
115+ nativePopupFallbackStarted: true,
116+ nativePopupFallbackTimer: timer,
117+ pendingCreate: &pendingBrowserCreate{},
118+ popupOpenerBridgeParent: parent,
119+ popupOpenerBridgeParentURI: "https://example.com/login",
120+ }
121+
122+ wv.PreparePaneHostedBrowsingContext()
123+
124+ require.False(t, wv.nativePopupCandidate)
125+ require.Nil(t, wv.nativePopupParent)
126+ require.Zero(t, wv.nativePopupID)
127+ require.False(t, wv.nativePopupFallbackStarted)
128+ require.Nil(t, wv.nativePopupFallbackTimer)
129+ require.True(t, timer.stopped)
130+ require.Nil(t, wv.popupOpenerBridgeParent)
131+ require.Empty(t, wv.popupOpenerBridgeParentURI)
132+}
133+
101 func TestStartNativePopupFallback_AllowsPopupShellAwaitingArm(t *testing.T) {134 func TestStartNativePopupFallback_AllowsPopupShellAwaitingArm(t *testing.T) {
102 wv := &WebView{135 wv := &WebView{
103 nativePopupCandidate: true,136 nativePopupCandidate: true,
M internal/ui/coordinator/content/popup_manager.go +7 -5
@@ -509,12 +509,14 @@ func (pm *popupManager) handlePopupCreate(
509509 return nil
510510 }
511511 reused, ok := pm.reuseNamedPopup(ctx, hooks, parentPaneID, req.FrameName, req.TargetURI)
512- if !ok {
513- log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable")
514- return nil
512+ if ok {
513+ pm.setBrowsingContextDecision(reused, decision)
514+ return reused
515515 }
516- pm.setBrowsingContextDecision(reused, decision)
517- return reused
516+ log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable")
517+ decision.Kind = dto.HostDecisionCreatePane
518+ decision.ReuseContextName = ""
519+ decision.Reason = "named browsing context unavailable; creating replacement pane"
518520 case dto.HostDecisionCreateNativeWin:
519521 return pm.openNativePopup(ctx, parentPaneID, parentID, parentURIAtOpen, req, decision)
520522 case dto.HostDecisionCreatePane:
@@ -509,12 +509,14 @@ func (pm *popupManager) handlePopupCreate(
509 return nil509 return nil
510 }510 }
511 reused, ok := pm.reuseNamedPopup(ctx, hooks, parentPaneID, req.FrameName, req.TargetURI)511 reused, ok := pm.reuseNamedPopup(ctx, hooks, parentPaneID, req.FrameName, req.TargetURI)
512- if !ok {512+ if ok {
513- log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable")513+ pm.setBrowsingContextDecision(reused, decision)
514- return nil514+ return reused
515 }515 }
516- pm.setBrowsingContextDecision(reused, decision)516+ log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable")
517- return reused517+ decision.Kind = dto.HostDecisionCreatePane
518+ decision.ReuseContextName = ""
519+ decision.Reason = "named browsing context unavailable; creating replacement pane"
518 case dto.HostDecisionCreateNativeWin:520 case dto.HostDecisionCreateNativeWin:
519 return pm.openNativePopup(ctx, parentPaneID, parentID, parentURIAtOpen, req, decision)521 return pm.openNativePopup(ctx, parentPaneID, parentID, parentURIAtOpen, req, decision)
520 case dto.HostDecisionCreatePane:522 case dto.HostDecisionCreatePane:
M internal/ui/coordinator/content/popup_test.go +50 -0
@@ -562,6 +562,56 @@ func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) {
562562 assert.Equal(t, "https://example.com/second", currentURI)
563563 }
564564
565+func TestHandlePopupCreate_FallsBackToNewPaneWhenNamedReuseBecomesUnavailable(t *testing.T) {
566+ ctx := context.Background()
567+ parentPaneID := entity.PaneID("parent-pane")
568+ stalePaneID := entity.PaneID("stale-pane")
569+ parentWV := mocks.NewMockWebView(t)
570+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once()
571+
572+ staleWV := mocks.NewMockWebView(t)
573+ staleWV.EXPECT().IsDestroyed().Return(false).Once()
574+ staleWV.EXPECT().ID().Return(port.WebViewID(202)).Once()
575+ staleWV.EXPECT().IsDestroyed().Return(true).Once()
576+
577+ newWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
578+ newWV.EXPECT().ID().Return(port.WebViewID(303)).Maybe()
579+ newWV.EXPECT().Generation().Return(uint64(1)).Maybe()
580+ newWV.EXPECT().SetCallbacks(mock.Anything).Maybe()
581+
582+ factory := mocks.NewMockWebViewFactory(t)
583+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(newWV, nil).Once()
584+
585+ insertCalls := 0
586+ c := &Coordinator{
587+ webViews: map[entity.PaneID]port.WebView{
588+ stalePaneID: staleWV,
589+ },
590+ popups: newPopupManager(),
591+ }
592+ c.SetPopupConfig(factory, nil, nil)
593+ c.SetPopupWindowIDResolver(func(entity.PaneID) (string, bool) { return "window-1", true })
594+ c.popups.namedContexts.Register("window-1", "shared-pane", stalePaneID, port.WebViewID(202))
595+ c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error {
596+ insertCalls++
597+ return nil
598+ })
599+
600+ created := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{
601+ TargetURI: "https://example.com/replacement",
602+ FrameName: "shared-pane",
603+ IsUserGesture: true,
604+ })
605+
606+ require.Same(t, newWV, created)
607+ assert.Equal(t, 1, insertCalls)
608+ decision, hasDecision := newWV.BrowsingContextHostDecision()
609+ require.True(t, hasDecision)
610+ assert.Equal(t, dto.HostDecisionCreatePane, decision.Kind)
611+ assert.Empty(t, decision.ReuseContextName)
612+ assert.Equal(t, "shared-pane", decision.BrowsingContextName)
613+}
614+
565615 // ---------------------------------------------------------------------------
566616 // trackOAuthPopup / capturePopupOAuthState
567617 // ---------------------------------------------------------------------------
@@ -562,6 +562,56 @@ func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) {
562 assert.Equal(t, "https://example.com/second", currentURI)562 assert.Equal(t, "https://example.com/second", currentURI)
563 }563 }
564 564
565+func TestHandlePopupCreate_FallsBackToNewPaneWhenNamedReuseBecomesUnavailable(t *testing.T) {
566+ ctx := context.Background()
567+ parentPaneID := entity.PaneID("parent-pane")
568+ stalePaneID := entity.PaneID("stale-pane")
569+ parentWV := mocks.NewMockWebView(t)
570+ parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once()
571+
572+ staleWV := mocks.NewMockWebView(t)
573+ staleWV.EXPECT().IsDestroyed().Return(false).Once()
574+ staleWV.EXPECT().ID().Return(port.WebViewID(202)).Once()
575+ staleWV.EXPECT().IsDestroyed().Return(true).Once()
576+
577+ newWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
578+ newWV.EXPECT().ID().Return(port.WebViewID(303)).Maybe()
579+ newWV.EXPECT().Generation().Return(uint64(1)).Maybe()
580+ newWV.EXPECT().SetCallbacks(mock.Anything).Maybe()
581+
582+ factory := mocks.NewMockWebViewFactory(t)
583+ factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(newWV, nil).Once()
584+
585+ insertCalls := 0
586+ c := &Coordinator{
587+ webViews: map[entity.PaneID]port.WebView{
588+ stalePaneID: staleWV,
589+ },
590+ popups: newPopupManager(),
591+ }
592+ c.SetPopupConfig(factory, nil, nil)
593+ c.SetPopupWindowIDResolver(func(entity.PaneID) (string, bool) { return "window-1", true })
594+ c.popups.namedContexts.Register("window-1", "shared-pane", stalePaneID, port.WebViewID(202))
595+ c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error {
596+ insertCalls++
597+ return nil
598+ })
599+
600+ created := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{
601+ TargetURI: "https://example.com/replacement",
602+ FrameName: "shared-pane",
603+ IsUserGesture: true,
604+ })
605+
606+ require.Same(t, newWV, created)
607+ assert.Equal(t, 1, insertCalls)
608+ decision, hasDecision := newWV.BrowsingContextHostDecision()
609+ require.True(t, hasDecision)
610+ assert.Equal(t, dto.HostDecisionCreatePane, decision.Kind)
611+ assert.Empty(t, decision.ReuseContextName)
612+ assert.Equal(t, "shared-pane", decision.BrowsingContextName)
613+}
614+
565 // ---------------------------------------------------------------------------615 // ---------------------------------------------------------------------------
566 // trackOAuthPopup / capturePopupOAuthState616 // trackOAuthPopup / capturePopupOAuthState
567 // ---------------------------------------------------------------------------617 // ---------------------------------------------------------------------------

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