b95b119 fix(popup): preserve lifecycle and fallback contracts · dumber

M internal/infrastructure/webkit/webview.go +1 -0


@@ -1700,6 +1700,7 @@ func (wv *WebView) runCloseCallbacks() {
1700	1700		wv.mu.Lock()
1701	1701		base := wv.OnClose
1702	1702		lifecycle := wv.popupLifecycleClose
	1703	+	wv.OnClose = nil
1703	1704		wv.popupLifecycleClose = nil
1704	1705		wv.mu.Unlock()
1705	1706		if base != nil {


@@ -1700,6 +1700,7 @@ func (wv *WebView) runCloseCallbacks() {
1700	wv.mu.Lock()	1700		wv.mu.Lock()
1701	base := wv.OnClose	1701		base := wv.OnClose
1702	lifecycle := wv.popupLifecycleClose	1702		lifecycle := wv.popupLifecycleClose
		1703	+	wv.OnClose = nil
1703	wv.popupLifecycleClose = nil	1704		wv.popupLifecycleClose = nil
1704	wv.mu.Unlock()	1705		wv.mu.Unlock()
1705	if base != nil {	1706		if base != nil {

M internal/infrastructure/webkit/webview_popup_lifecycle_test.go +14 -0


@@ -6,6 +6,20 @@ import (
6	6		"github.com/stretchr/testify/assert"
7	7		)
8	8
	9	+	func TestPopupCloseCallbacksRunExactlyOnce(t *testing.T) {
	10	+	wv := &WebView{}
	11	+	baseCalls := 0
	12	+	lifecycleCalls := 0
	13	+	wv.AddCloseCallback(func() { baseCalls++ })
	14	+	wv.SetOnClose(func() { lifecycleCalls++ })
	15	+
	16	+	wv.runCloseCallbacks()
	17	+	wv.runCloseCallbacks()
	18	+
	19	+	assert.Equal(t, 1, baseCalls)
	20	+	assert.Equal(t, 1, lifecycleCalls)
	21	+	}
	22	+
9	23		func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) {
10	24		wv := &WebView{}
11	25		readyCalls := 0


@@ -6,6 +6,20 @@ import (
6	"github.com/stretchr/testify/assert"	6		"github.com/stretchr/testify/assert"
7	)	7		)
8		8
		9	+	func TestPopupCloseCallbacksRunExactlyOnce(t *testing.T) {
		10	+	wv := &WebView{}
		11	+	baseCalls := 0
		12	+	lifecycleCalls := 0
		13	+	wv.AddCloseCallback(func() { baseCalls++ })
		14	+	wv.SetOnClose(func() { lifecycleCalls++ })
		15	+
		16	+	wv.runCloseCallbacks()
		17	+	wv.runCloseCallbacks()
		18	+
		19	+	assert.Equal(t, 1, baseCalls)
		20	+	assert.Equal(t, 1, lifecycleCalls)
		21	+	}
		22	+
9	func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) {	23		func TestPopupLifecycleCallbacksCanBeDisarmed(t *testing.T) {
10	wv := &WebView{}	24		wv := &WebView{}
11	readyCalls := 0	25		readyCalls := 0

M internal/ui/coordinator/content/browsing_context_policy.go +4 -15


@@ -80,26 +80,15 @@ func inferPopupWindowDisposition(req port.PopupRequest) dto.WindowDisposition {
80	80		}
81	81
82	82		func inferPopupTriggerKind(req port.PopupRequest) dto.TriggerKind {
	83	+	if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
	84	+	return dto.TriggerNamedTargetNavigation
	85	+	}
83	86		switch inferPopupWindowDisposition(req) {
84		-	case dto.WindowDispositionCurrentTab:
85		-	if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
86		-	return dto.TriggerNamedTargetNavigation
87		-	}
88		-	return dto.TriggerLinkNewPage
89		-	case dto.WindowDispositionNewTab:
90		-	if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
91		-	return dto.TriggerNamedTargetNavigation
92		-	}
	87	+	case dto.WindowDispositionCurrentTab, dto.WindowDispositionNewTab:
93	88		return dto.TriggerLinkNewPage
94	89		case dto.WindowDispositionNewPopup, dto.WindowDispositionNewWindow:
95		-	if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
96		-	return dto.TriggerNamedTargetNavigation
97		-	}
98	90		return dto.TriggerScriptWindowOpen
99	91		default:
100		-	if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
101		-	return dto.TriggerNamedTargetNavigation
102		-	}
103	92		return dto.TriggerUnknown
104	93		}
105	94		}

 }
 func inferPopupTriggerKind(req port.PopupRequest) dto.TriggerKind {
+	if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
+		return dto.TriggerNamedTargetNavigation
+	}
 	switch inferPopupWindowDisposition(req) {
-	case dto.WindowDispositionCurrentTab:
+	case dto.WindowDispositionCurrentTab, dto.WindowDispositionNewTab:
-		if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
-			return dto.TriggerNamedTargetNavigation
-		}
-		return dto.TriggerLinkNewPage
-	case dto.WindowDispositionNewTab:
-		if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
-			return dto.TriggerNamedTargetNavigation
-		}
 		return dto.TriggerLinkNewPage
 	case dto.WindowDispositionNewPopup, dto.WindowDispositionNewWindow:
-		if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
-			return dto.TriggerNamedTargetNavigation
-		}
 		return dto.TriggerScriptWindowOpen
 	default:
-		if usecase.ReusableBrowsingContextName(req.FrameName) != "" {
-			return dto.TriggerNamedTargetNavigation
-		}
 		return dto.TriggerUnknown
 	}
 }

M internal/ui/coordinator/content/popup.go +3 -1


@@ -142,7 +142,9 @@ type NativePopupInput struct {
142	142		Request port.PopupRequest
143	143		ObserveOAuthAutoClose bool
144	144		AllowBrowserWindowFallback bool
145		-	OnNativeHostAbort func(context.Context, port.WebView) bool
	145	+	// OnNativeHostAbort returns true when fallback adopted the WebView. A false
	146	+	// result leaves destruction responsibility with the native host.
	147	+	OnNativeHostAbort func(context.Context, port.WebView) bool
146	148		}
147	149
148	150		// GetBehavior returns the appropriate behavior based on popup type and config.

 	Request                    port.PopupRequest
 	ObserveOAuthAutoClose      bool
 	AllowBrowserWindowFallback bool
-	OnNativeHostAbort          func(context.Context, port.WebView) bool
+	// OnNativeHostAbort returns true when fallback adopted the WebView. A false
+	// result leaves destruction responsibility with the native host.
+	OnNativeHostAbort func(context.Context, port.WebView) bool
 }
 // GetBehavior returns the appropriate behavior based on popup type and config.

M internal/ui/coordinator/content/popup_browsing_context_test.go +1 -1


@@ -796,7 +796,7 @@ func TestHandlePopupCreate_FallsBackToNewPaneWhenNamedReuseBecomesUnavailable(t
796	796		parentPaneID := entity.PaneID("parent-pane")
797	797		stalePaneID := entity.PaneID("stale-pane")
798	798		parentWV := mocks.NewMockWebView(t)
799		-	parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once()
	799	+	parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice()
800	800
801	801		staleWV := mocks.NewMockWebView(t)
802	802		staleWV.EXPECT().IsDestroyed().Return(false).Once()


@@ -796,7 +796,7 @@ func TestHandlePopupCreate_FallsBackToNewPaneWhenNamedReuseBecomesUnavailable(t
796		parentPaneID := entity.PaneID("parent-pane")	796		parentPaneID := entity.PaneID("parent-pane")
797		stalePaneID := entity.PaneID("stale-pane")	797		stalePaneID := entity.PaneID("stale-pane")
798		parentWV := mocks.NewMockWebView(t)	798		parentWV := mocks.NewMockWebView(t)
799	-	parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once()	799	+	parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice()
800			800
801		staleWV := mocks.NewMockWebView(t)	801		staleWV := mocks.NewMockWebView(t)
802		staleWV.EXPECT().IsDestroyed().Return(false).Once()	802		staleWV.EXPECT().IsDestroyed().Return(false).Once()

M internal/ui/coordinator/content/popup_hosting.go +0 -6


@@ -245,9 +245,3 @@ func (pm *popupManager) adoptPopupInNativePopup(
245	245		}
246	246		return true
247	247		}
248		-
249		-	func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {
250		-	// Deferred WebKit requests are staged and routed by a later phase. Every
251		-	// immediate route must be visible without depending on another signal.
252		-	return true
253		-	}


@@ -245,9 +245,3 @@ func (pm *popupManager) adoptPopupInNativePopup(
245		}	245	}
246		return true	246	return true
247		}	247	}
248	-
249	-	func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {
250	-	// Deferred WebKit requests are staged and routed by a later phase. Every
251	-	// immediate route must be visible without depending on another signal.
252	-	return true
253	-	}

M internal/ui/coordinator/content/popup_manager.go +4 -8


@@ -592,14 +592,14 @@ func (pm *popupManager) handlePopupCreate(
592	592		return nil
593	593		}
594	594		log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable")
595		-	decision.Kind = dto.HostDecisionCreatePane
596		-	decision.ReuseContextName = ""
597		-	decision.Reason = "named browsing context unavailable; creating replacement pane"
	595	+	// Lookup removes the stale registry entry. Re-enter policy once so the
	596	+	// replacement host remains source-aware instead of forcing a workspace pane.
	597	+	return pm.handlePopupCreate(ctx, hooks, parentPaneID, parentWV, req)
598	598		case dto.HostDecisionNavigateSource:
599	599		pm.navigatePopupSource(ctx, parentID, parentWV, req, request, decision)
600	600		return nil
601	601		case dto.HostDecisionCreateBrowserWindow:
602		-	return pm.openBrowserWindow(ctx, hooks, parentPaneID, parentID, req, decision, pm.readyOnCreate(req.Engine))
	602	+	return pm.openBrowserWindow(ctx, hooks, parentPaneID, parentID, req, decision, true)
603	603		case dto.HostDecisionCreateNativePopup:
604	604		return pm.openNativePopup(ctx, hooks, parentPaneID, parentID, parentURIAtOpen, req, decision)
605	605		case dto.HostDecisionAwaitPopupFeatures:

@@ -922,10 +922,6 @@ func (pm *popupManager) handleLinkMiddleClick(
922	922		request.SourceHost = sourceHost
923	923		decision := pm.policy.Decide(request, false)
924	924		logBrowsingContextDecision(*log, request, decision)
925		-	log.Debug().
926		-	Str("decision", string(decision.Kind)).
927		-	Str("reason", decision.Reason).
928		-	Msg("middle-click browsing context host decision")
929	925		if decision.Kind == dto.HostDecisionNavigateSource {
930	926		return pm.navigateSource(ctx, parentWV, uri)
931	927		}

 			return nil
 		}
 		log.Warn().Str("frame_name", req.FrameName).Msg("named browsing context reuse requested but target was unavailable")
-		decision.Kind = dto.HostDecisionCreatePane
+		// Lookup removes the stale registry entry. Re-enter policy once so the
-		decision.ReuseContextName = ""
+		// replacement host remains source-aware instead of forcing a workspace pane.
-		decision.Reason = "named browsing context unavailable; creating replacement pane"
+		return pm.handlePopupCreate(ctx, hooks, parentPaneID, parentWV, req)
 	case dto.HostDecisionNavigateSource:
 		pm.navigatePopupSource(ctx, parentID, parentWV, req, request, decision)
 		return nil
 	case dto.HostDecisionCreateBrowserWindow:
-		return pm.openBrowserWindow(ctx, hooks, parentPaneID, parentID, req, decision, pm.readyOnCreate(req.Engine))
+		return pm.openBrowserWindow(ctx, hooks, parentPaneID, parentID, req, decision, true)
 	case dto.HostDecisionCreateNativePopup:
 		return pm.openNativePopup(ctx, hooks, parentPaneID, parentID, parentURIAtOpen, req, decision)
 	case dto.HostDecisionAwaitPopupFeatures:
 	request.SourceHost = sourceHost
 	decision := pm.policy.Decide(request, false)
 	logBrowsingContextDecision(*log, request, decision)
-	log.Debug().
-		Str("decision", string(decision.Kind)).
-		Str("reason", decision.Reason).
-		Msg("middle-click browsing context host decision")
 	if decision.Kind == dto.HostDecisionNavigateSource {
 		return pm.navigateSource(ctx, parentWV, uri)
 	}

M internal/ui/native_popup_window.go +1 -1


@@ -117,7 +117,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
117	117		if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok {
118	118		aborter.SetNativePopupHostAbort(func() {
119	119		a.dispatchNativePopupLifecycle("ui.native_popup.abort", popupID, func() {
120		-	a.abortNativePopupWindow(ctx, popupID, input)
	120	+	a.abortNativePopupWindow(context.Background(), popupID, input)
121	121		})
122	122		})
123	123		}


@@ -117,7 +117,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
117		if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok {	117		if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok {
118		aborter.SetNativePopupHostAbort(func() {	118		aborter.SetNativePopupHostAbort(func() {
119		a.dispatchNativePopupLifecycle("ui.native_popup.abort", popupID, func() {	119		a.dispatchNativePopupLifecycle("ui.native_popup.abort", popupID, func() {
120	-	a.abortNativePopupWindow(ctx, popupID, input)	120	+	a.abortNativePopupWindow(context.Background(), popupID, input)
121		})	121		})
122		})	122		})
123		}	123		}