refactor(ui): make popup host ownership transactional

commit: 8b31f3218560c2190847b9516bc6bfa4a96f68a7
author: brice · 2mo ago · 2026-07-25 07:39 UTC
committer: brice · 2mo ago · 2026-07-26 16:42 UTC
parent: 192c70d
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

8 files · +316 -67

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


@@ -2948,7 +2948,7 @@ func (a *App) onShutdown(ctx context.Context) {
2948	2948		popupIDs = append(popupIDs, popupID)
2949	2949		}
2950	2950		for _, popupID := range popupIDs {
2951		-	a.releaseNativePopupWindow(popupID, false, true)
	2951	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
2952	2952		}
2953	2953		}
2954	2954		if a.engine != nil {


@@ -2948,7 +2948,7 @@ func (a *App) onShutdown(ctx context.Context) {
2948		popupIDs = append(popupIDs, popupID)	2948		popupIDs = append(popupIDs, popupID)
2949		}	2949		}
2950		for _, popupID := range popupIDs {	2950		for _, popupID := range popupIDs {
2951	-	a.releaseNativePopupWindow(popupID, false, true)	2951	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
2952		}	2952		}
2953		}	2953		}
2954		if a.engine != nil {	2954		if a.engine != nil {

M internal/ui/coordinator/content/popup.go +9 -7


@@ -134,13 +134,15 @@ type StagePopupInput struct {
134	134		}
135	135
136	136		type NativePopupInput struct {
137		-	ParentPaneID entity.PaneID
138		-	ParentWebViewID port.WebViewID
139		-	ParentURIAtOpen string
140		-	PopupWebView port.WebView
141		-	TargetURI string
142		-	Request port.PopupRequest
143		-	ObserveOAuthAutoClose bool
	137	+	ParentPaneID entity.PaneID
	138	+	ParentWebViewID port.WebViewID
	139	+	ParentURIAtOpen string
	140	+	PopupWebView port.WebView
	141	+	TargetURI string
	142	+	Request port.PopupRequest
	143	+	ObserveOAuthAutoClose bool
	144	+	AllowBrowserWindowFallback bool
	145	+	OnNativeHostAbort func(context.Context, port.WebView) bool
144	146		}
145	147
146	148		// GetBehavior returns the appropriate behavior based on popup type and config.


@@ -134,13 +134,15 @@ type StagePopupInput struct {
134		}	134		}
135			135
136		type NativePopupInput struct {	136		type NativePopupInput struct {
137	-	ParentPaneID entity.PaneID	137	+	ParentPaneID entity.PaneID
138	-	ParentWebViewID port.WebViewID	138	+	ParentWebViewID port.WebViewID
139	-	ParentURIAtOpen string	139	+	ParentURIAtOpen string
140	-	PopupWebView port.WebView	140	+	PopupWebView port.WebView
141	-	TargetURI string	141	+	TargetURI string
142	-	Request port.PopupRequest	142	+	Request port.PopupRequest
143	-	ObserveOAuthAutoClose bool	143	+	ObserveOAuthAutoClose bool
			144	+	AllowBrowserWindowFallback bool
			145	+	OnNativeHostAbort func(context.Context, port.WebView) bool
144		}	146		}
145			147
146		// GetBehavior returns the appropriate behavior based on popup type and config.	148		// GetBehavior returns the appropriate behavior based on popup type and config.

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


@@ -59,6 +59,47 @@ func (pm *popupManager) openBrowserWindow(
59	59		return popupWV
60	60		}
61	61
	62	+	// openExistingPopupInBrowserWindow attempts to transfer an already-created
	63	+	// WebView. On failure ownership remains with the caller.
	64	+	func (pm *popupManager) openExistingPopupInBrowserWindow(
	65	+	ctx context.Context,
	66	+	hooks popupCoordinatorHooks,
	67	+	parentPaneID entity.PaneID,
	68	+	parentWebViewID port.WebViewID,
	69	+	popupWV port.WebView,
	70	+	req port.PopupRequest,
	71	+	decision dto.HostDecision,
	72	+	ready bool,
	73	+	) bool {
	74	+	log := logging.FromContext(ctx)
	75	+	normalized := buildPopupBrowsingContextRequest(req)
	76	+	normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
	77	+	if popupWV == nil \|\| pm.onOpenBrowserWindow == nil {
	78	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureFallback, nil)
	79	+	return false
	80	+	}
	81	+
	82	+	paneID, popupPane := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
	83	+	if hooks.setupWebViewCallbacks != nil {
	84	+	hooks.setupWebViewCallbacks(ctx, paneID, popupWV)
	85	+	}
	86	+	result, err := pm.onOpenBrowserWindow(ctx, BrowserWindowInput{
	87	+	ParentPaneID: parentPaneID, ParentWebViewID: parentWebViewID, PopupPane: popupPane,
	88	+	PopupWebView: popupWV, TargetURI: req.TargetURI, Request: req, Ready: ready,
	89	+	})
	90	+	if err != nil \|\| result.WindowID == "" {
	91	+	if err == nil {
	92	+	err = fmt.Errorf("browser window fallback returned an empty window ID")
	93	+	}
	94	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureFallback, err)
	95	+	return false
	96	+	}
	97	+	if !req.NoJavaScriptAccess {
	98	+	pm.storeReusableNamedPopupWithHost(parentPaneID, req.FrameName, paneID, popupWV, result.WindowID)
	99	+	}
	100	+	return true
	101	+	}
	102	+
62	103		func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {
63	104		// Deferred WebKit requests are staged and routed by a later phase. Every
64	105		// immediate route must be visible without depending on another signal.


@@ -59,6 +59,47 @@ func (pm *popupManager) openBrowserWindow(
59	return popupWV	59		return popupWV
60	}	60		}
61		61
		62	+	// openExistingPopupInBrowserWindow attempts to transfer an already-created
		63	+	// WebView. On failure ownership remains with the caller.
		64	+	func (pm *popupManager) openExistingPopupInBrowserWindow(
		65	+	ctx context.Context,
		66	+	hooks popupCoordinatorHooks,
		67	+	parentPaneID entity.PaneID,
		68	+	parentWebViewID port.WebViewID,
		69	+	popupWV port.WebView,
		70	+	req port.PopupRequest,
		71	+	decision dto.HostDecision,
		72	+	ready bool,
		73	+	) bool {
		74	+	log := logging.FromContext(ctx)
		75	+	normalized := buildPopupBrowsingContextRequest(req)
		76	+	normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
		77	+	if popupWV == nil \|\| pm.onOpenBrowserWindow == nil {
		78	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureFallback, nil)
		79	+	return false
		80	+	}
		81	+
		82	+	paneID, popupPane := pm.createPopupPane(popupWV.ID(), parentPaneID, req.TargetURI)
		83	+	if hooks.setupWebViewCallbacks != nil {
		84	+	hooks.setupWebViewCallbacks(ctx, paneID, popupWV)
		85	+	}
		86	+	result, err := pm.onOpenBrowserWindow(ctx, BrowserWindowInput{
		87	+	ParentPaneID: parentPaneID, ParentWebViewID: parentWebViewID, PopupPane: popupPane,
		88	+	PopupWebView: popupWV, TargetURI: req.TargetURI, Request: req, Ready: ready,
		89	+	})
		90	+	if err != nil \|\| result.WindowID == "" {
		91	+	if err == nil {
		92	+	err = fmt.Errorf("browser window fallback returned an empty window ID")
		93	+	}
		94	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureFallback, err)
		95	+	return false
		96	+	}
		97	+	if !req.NoJavaScriptAccess {
		98	+	pm.storeReusableNamedPopupWithHost(parentPaneID, req.FrameName, paneID, popupWV, result.WindowID)
		99	+	}
		100	+	return true
		101	+	}
		102	+
62	func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {	103		func (*popupManager) readyOnCreate(dto.BrowserEngineKind) bool {
63	// Deferred WebKit requests are staged and routed by a later phase. Every	104		// Deferred WebKit requests are staged and routed by a later phase. Every
64	// immediate route must be visible without depending on another signal.	105		// immediate route must be visible without depending on another signal.

M internal/ui/coordinator/content/popup_manager.go +31 -11


@@ -514,6 +514,7 @@ func (pm *popupManager) popupParentURIAtOpen(
514	514
515	515		func (pm *popupManager) openNativePopup(
516	516		ctx context.Context,
	517	+	hooks popupCoordinatorHooks,
517	518		parentPaneID entity.PaneID,
518	519		parentID port.WebViewID,
519	520		parentURIAtOpen string,

@@ -521,27 +522,46 @@ func (pm *popupManager) openNativePopup(
521	522		decision dto.HostDecision,
522	523		) port.WebView {
523	524		log := logging.FromContext(ctx)
	525	+	normalized := buildPopupBrowsingContextRequest(req)
	526	+	normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
524	527		if pm.onOpenNativePopup == nil {
525		-	log.Warn().Msg("native popup host callback not configured")
	528	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostUnavailable, nil)
526	529		return nil
527	530		}
528	531		popupWV, err := pm.createPopupWebView(ctx, parentID, req.TargetURI, false)
529	532		if err != nil {
530		-	log.Error().Err(err).Msg("failed to create webview for native popup")
	533	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
531	534		return nil
532	535		}
533	536		cfg := pm.currentPopupConfig()
534	537		pm.setBrowsingContextDecision(popupWV, decision)
	538	+	allowFallback := !normalized.AuthIntent && !decision.RequiresNativeOpener && req.NoJavaScriptAccess
	539	+	var abortOnce sync.Once
	540	+	abortResult := false
	541	+	onAbort := func(abortCtx context.Context, abortedWV port.WebView) bool {
	542	+	abortOnce.Do(func() {
	543	+	if !allowFallback {
	544	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureNativeArm, nil)
	545	+	return
	546	+	}
	547	+	abortResult = pm.openExistingPopupInBrowserWindow(
	548	+	abortCtx, hooks, parentPaneID, parentID, abortedWV, req, decision, true,
	549	+	)
	550	+	})
	551	+	return abortResult
	552	+	}
535	553		if err := pm.onOpenNativePopup(ctx, NativePopupInput{
536		-	ParentPaneID: parentPaneID,
537		-	ParentWebViewID: parentID,
538		-	ParentURIAtOpen: parentURIAtOpen,
539		-	PopupWebView: popupWV,
540		-	TargetURI: req.TargetURI,
541		-	Request: req,
542		-	ObserveOAuthAutoClose: cfg != nil && cfg.OAuthAutoClose && IsOAuthURL(req.TargetURI),
	554	+	ParentPaneID: parentPaneID,
	555	+	ParentWebViewID: parentID,
	556	+	ParentURIAtOpen: parentURIAtOpen,
	557	+	PopupWebView: popupWV,
	558	+	TargetURI: req.TargetURI,
	559	+	Request: req,
	560	+	ObserveOAuthAutoClose: cfg != nil && cfg.OAuthAutoClose && IsOAuthURL(req.TargetURI),
	561	+	AllowBrowserWindowFallback: allowFallback,
	562	+	OnNativeHostAbort: onAbort,
543	563		}); err != nil {
544		-	log.Error().Err(err).Msg("failed to open native popup host")
	564	+	logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureNativeArm, err)
545	565		popupWV.Destroy()
546	566		return nil
547	567		}

@@ -597,7 +617,7 @@ func (pm *popupManager) handlePopupCreate(
597	617		case dto.HostDecisionCreateBrowserWindow:
598	618		return pm.openBrowserWindow(ctx, hooks, parentPaneID, parentID, req, decision, pm.readyOnCreate(req.Engine))
599	619		case dto.HostDecisionCreateNativePopup:
600		-	return pm.openNativePopup(ctx, parentPaneID, parentID, parentURIAtOpen, req, decision)
	620	+	return pm.openNativePopup(ctx, hooks, parentPaneID, parentID, parentURIAtOpen, req, decision)
601	621		case dto.HostDecisionAwaitPopupFeatures:
602	622		logBrowsingContextFailure(*log, request, decision, dto.BrowsingContextFailureHostUnavailable, nil)
603	623		return nil

 func (pm *popupManager) openNativePopup(
 	ctx context.Context,
+	hooks popupCoordinatorHooks,
 	parentPaneID entity.PaneID,
 	parentID port.WebViewID,
 	parentURIAtOpen string,
 	decision dto.HostDecision,
 ) port.WebView {
 	log := logging.FromContext(ctx)
+	normalized := buildPopupBrowsingContextRequest(req)
+	normalized.SourceHost = pm.resolveSourceHost(parentPaneID)
 	if pm.onOpenNativePopup == nil {
-		log.Warn().Msg("native popup host callback not configured")
+		logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostUnavailable, nil)
 		return nil
 	}
 	popupWV, err := pm.createPopupWebView(ctx, parentID, req.TargetURI, false)
 	if err != nil {
-		log.Error().Err(err).Msg("failed to create webview for native popup")
+		logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureHostFailed, err)
 		return nil
 	}
 	cfg := pm.currentPopupConfig()
 	pm.setBrowsingContextDecision(popupWV, decision)
+	allowFallback := !normalized.AuthIntent && !decision.RequiresNativeOpener && req.NoJavaScriptAccess
+	var abortOnce sync.Once
+	abortResult := false
+	onAbort := func(abortCtx context.Context, abortedWV port.WebView) bool {
+		abortOnce.Do(func() {
+			if !allowFallback {
+				logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureNativeArm, nil)
+				return
+			}
+			abortResult = pm.openExistingPopupInBrowserWindow(
+				abortCtx, hooks, parentPaneID, parentID, abortedWV, req, decision, true,
+			)
+		})
+		return abortResult
+	}
 	if err := pm.onOpenNativePopup(ctx, NativePopupInput{
-		ParentPaneID:          parentPaneID,
+		ParentPaneID:               parentPaneID,
-		ParentWebViewID:       parentID,
+		ParentWebViewID:            parentID,
-		ParentURIAtOpen:       parentURIAtOpen,
+		ParentURIAtOpen:            parentURIAtOpen,
-		PopupWebView:          popupWV,
+		PopupWebView:               popupWV,
-		TargetURI:             req.TargetURI,
+		TargetURI:                  req.TargetURI,
-		Request:               req,
+		Request:                    req,
-		ObserveOAuthAutoClose: cfg != nil && cfg.OAuthAutoClose && IsOAuthURL(req.TargetURI),
+		ObserveOAuthAutoClose:      cfg != nil && cfg.OAuthAutoClose && IsOAuthURL(req.TargetURI),
+		AllowBrowserWindowFallback: allowFallback,
+		OnNativeHostAbort:          onAbort,
 	}); err != nil {
-		log.Error().Err(err).Msg("failed to open native popup host")
+		logBrowsingContextFailure(*log, normalized, decision, dto.BrowsingContextFailureNativeArm, err)
 		popupWV.Destroy()
 		return nil
 	}
 	case dto.HostDecisionCreateBrowserWindow:
 		return pm.openBrowserWindow(ctx, hooks, parentPaneID, parentID, req, decision, pm.readyOnCreate(req.Engine))
 	case dto.HostDecisionCreateNativePopup:
-		return pm.openNativePopup(ctx, parentPaneID, parentID, parentURIAtOpen, req, decision)
+		return pm.openNativePopup(ctx, hooks, parentPaneID, parentID, parentURIAtOpen, req, decision)
 	case dto.HostDecisionAwaitPopupFeatures:
 		logBrowsingContextFailure(*log, request, decision, dto.BrowsingContextFailureHostUnavailable, nil)
 		return nil

M internal/ui/coordinator/content/popup_test.go +105 -0


@@ -644,6 +644,111 @@ func TestHandlePopupCreate_NativePopupDoesNotForceOAuthObservationWhenDisabled(t
644	644		assert.Equal(t, 0, insertCalls)
645	645		}
646	646
	647	+	func TestPopupNativeAbortFallsBackOnceForNoOpenerVisualPopup(t *testing.T) {
	648	+	ctx := context.Background()
	649	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
	650	+	popupWV.EXPECT().ID().Return(port.WebViewID(701)).Once()
	651	+	factory := mocks.NewMockWebViewFactory(t)
	652	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
	653	+
	654	+	pm := newPopupManager()
	655	+	pm.setConfig(factory, nil, nil)
	656	+	browserCalls := 0
	657	+	pm.setOnOpenBrowserWindow(func(_ context.Context, input BrowserWindowInput) (BrowserWindowResult, error) {
	658	+	browserCalls++
	659	+	assert.Same(t, popupWV, input.PopupWebView)
	660	+	assert.True(t, input.Ready)
	661	+	return BrowserWindowResult{WindowID: "fallback-window"}, nil
	662	+	})
	663	+	pm.setOnOpenNativePopup(func(ctx context.Context, input NativePopupInput) error {
	664	+	assert.True(t, input.AllowBrowserWindowFallback)
	665	+	assert.True(t, input.OnNativeHostAbort(ctx, input.PopupWebView))
	666	+	assert.True(t, input.OnNativeHostAbort(ctx, input.PopupWebView))
	667	+	return nil
	668	+	})
	669	+
	670	+	got := pm.openNativePopup(ctx, popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
	671	+	TargetURI: "https://example.com/visual", NoJavaScriptAccess: true,
	672	+	}, dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup})
	673	+	require.Same(t, popupWV, got)
	674	+	assert.Equal(t, 1, browserCalls)
	675	+	}
	676	+
	677	+	func TestPopupNativeAbortDoesNotFallbackForAuthOrRequiredOpener(t *testing.T) {
	678	+	for _, tc := range []struct {
	679	+	name string
	680	+	target string
	681	+	decision dto.HostDecision
	682	+	}{
	683	+	{name: "auth", target: "https://accounts.google.com/o/oauth2/auth", decision: dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup}},
	684	+	{name: "required opener", target: "https://example.com/opener", decision: dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup, RequiresNativeOpener: true}},
	685	+	} {
	686	+	t.Run(tc.name, func(t *testing.T) {
	687	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
	688	+	popupWV.EXPECT().Destroy().Once()
	689	+	factory := mocks.NewMockWebViewFactory(t)
	690	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
	691	+	pm := newPopupManager()
	692	+	pm.setConfig(factory, nil, nil)
	693	+	pm.setOnOpenBrowserWindow(func(context.Context, BrowserWindowInput) (BrowserWindowResult, error) {
	694	+	t.Fatal("auth/opener-required popup must not degrade to browser fallback")
	695	+	return BrowserWindowResult{}, nil
	696	+	})
	697	+	pm.setOnOpenNativePopup(func(ctx context.Context, input NativePopupInput) error {
	698	+	assert.False(t, input.AllowBrowserWindowFallback)
	699	+	if !input.OnNativeHostAbort(ctx, input.PopupWebView) {
	700	+	input.PopupWebView.Destroy()
	701	+	}
	702	+	return nil
	703	+	})
	704	+
	705	+	got := pm.openNativePopup(context.Background(), popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
	706	+	TargetURI: tc.target, NoJavaScriptAccess: true,
	707	+	}, tc.decision)
	708	+	require.Same(t, popupWV, got)
	709	+	})
	710	+	}
	711	+	}
	712	+
	713	+	func TestPopupNativeFallbackFailureLeavesDestructionToNativeHost(t *testing.T) {
	714	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
	715	+	popupWV.EXPECT().ID().Return(port.WebViewID(702)).Once()
	716	+	popupWV.EXPECT().Destroy().Once()
	717	+	factory := mocks.NewMockWebViewFactory(t)
	718	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
	719	+	pm := newPopupManager()
	720	+	pm.setConfig(factory, nil, nil)
	721	+	pm.setOnOpenBrowserWindow(func(context.Context, BrowserWindowInput) (BrowserWindowResult, error) {
	722	+	return BrowserWindowResult{}, errors.New("fallback failed")
	723	+	})
	724	+	pm.setOnOpenNativePopup(func(ctx context.Context, input NativePopupInput) error {
	725	+	if !input.OnNativeHostAbort(ctx, input.PopupWebView) {
	726	+	input.PopupWebView.Destroy()
	727	+	}
	728	+	return nil
	729	+	})
	730	+
	731	+	got := pm.openNativePopup(context.Background(), popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
	732	+	TargetURI: "https://example.com/visual", NoJavaScriptAccess: true,
	733	+	}, dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup})
	734	+	require.Same(t, popupWV, got)
	735	+	}
	736	+
	737	+	func TestPopupNativeSetupFailureDestroysCoordinatorOwnedWebViewOnce(t *testing.T) {
	738	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
	739	+	popupWV.EXPECT().Destroy().Once()
	740	+	factory := mocks.NewMockWebViewFactory(t)
	741	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
	742	+	pm := newPopupManager()
	743	+	pm.setConfig(factory, nil, nil)
	744	+	pm.setOnOpenNativePopup(func(context.Context, NativePopupInput) error { return errors.New("setup failed") })
	745	+
	746	+	got := pm.openNativePopup(context.Background(), popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
	747	+	TargetURI: "https://example.com/visual", NoJavaScriptAccess: true,
	748	+	}, dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup})
	749	+	assert.Nil(t, got)
	750	+	}
	751	+
647	752		func TestHandlePopupCreate_DoesNotReuseNamedPopupWhenNoJavaScriptAccess(t *testing.T) {
648	753		ctx := context.Background()
649	754		parentPaneID := entity.PaneID("parent-pane")


@@ -644,6 +644,111 @@ func TestHandlePopupCreate_NativePopupDoesNotForceOAuthObservationWhenDisabled(t
644	assert.Equal(t, 0, insertCalls)	644		assert.Equal(t, 0, insertCalls)
645	}	645		}
646		646
		647	+	func TestPopupNativeAbortFallsBackOnceForNoOpenerVisualPopup(t *testing.T) {
		648	+	ctx := context.Background()
		649	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
		650	+	popupWV.EXPECT().ID().Return(port.WebViewID(701)).Once()
		651	+	factory := mocks.NewMockWebViewFactory(t)
		652	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
		653	+
		654	+	pm := newPopupManager()
		655	+	pm.setConfig(factory, nil, nil)
		656	+	browserCalls := 0
		657	+	pm.setOnOpenBrowserWindow(func(_ context.Context, input BrowserWindowInput) (BrowserWindowResult, error) {
		658	+	browserCalls++
		659	+	assert.Same(t, popupWV, input.PopupWebView)
		660	+	assert.True(t, input.Ready)
		661	+	return BrowserWindowResult{WindowID: "fallback-window"}, nil
		662	+	})
		663	+	pm.setOnOpenNativePopup(func(ctx context.Context, input NativePopupInput) error {
		664	+	assert.True(t, input.AllowBrowserWindowFallback)
		665	+	assert.True(t, input.OnNativeHostAbort(ctx, input.PopupWebView))
		666	+	assert.True(t, input.OnNativeHostAbort(ctx, input.PopupWebView))
		667	+	return nil
		668	+	})
		669	+
		670	+	got := pm.openNativePopup(ctx, popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
		671	+	TargetURI: "https://example.com/visual", NoJavaScriptAccess: true,
		672	+	}, dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup})
		673	+	require.Same(t, popupWV, got)
		674	+	assert.Equal(t, 1, browserCalls)
		675	+	}
		676	+
		677	+	func TestPopupNativeAbortDoesNotFallbackForAuthOrRequiredOpener(t *testing.T) {
		678	+	for _, tc := range []struct {
		679	+	name string
		680	+	target string
		681	+	decision dto.HostDecision
		682	+	}{
		683	+	{name: "auth", target: "https://accounts.google.com/o/oauth2/auth", decision: dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup}},
		684	+	{name: "required opener", target: "https://example.com/opener", decision: dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup, RequiresNativeOpener: true}},
		685	+	} {
		686	+	t.Run(tc.name, func(t *testing.T) {
		687	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
		688	+	popupWV.EXPECT().Destroy().Once()
		689	+	factory := mocks.NewMockWebViewFactory(t)
		690	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
		691	+	pm := newPopupManager()
		692	+	pm.setConfig(factory, nil, nil)
		693	+	pm.setOnOpenBrowserWindow(func(context.Context, BrowserWindowInput) (BrowserWindowResult, error) {
		694	+	t.Fatal("auth/opener-required popup must not degrade to browser fallback")
		695	+	return BrowserWindowResult{}, nil
		696	+	})
		697	+	pm.setOnOpenNativePopup(func(ctx context.Context, input NativePopupInput) error {
		698	+	assert.False(t, input.AllowBrowserWindowFallback)
		699	+	if !input.OnNativeHostAbort(ctx, input.PopupWebView) {
		700	+	input.PopupWebView.Destroy()
		701	+	}
		702	+	return nil
		703	+	})
		704	+
		705	+	got := pm.openNativePopup(context.Background(), popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
		706	+	TargetURI: tc.target, NoJavaScriptAccess: true,
		707	+	}, tc.decision)
		708	+	require.Same(t, popupWV, got)
		709	+	})
		710	+	}
		711	+	}
		712	+
		713	+	func TestPopupNativeFallbackFailureLeavesDestructionToNativeHost(t *testing.T) {
		714	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
		715	+	popupWV.EXPECT().ID().Return(port.WebViewID(702)).Once()
		716	+	popupWV.EXPECT().Destroy().Once()
		717	+	factory := mocks.NewMockWebViewFactory(t)
		718	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
		719	+	pm := newPopupManager()
		720	+	pm.setConfig(factory, nil, nil)
		721	+	pm.setOnOpenBrowserWindow(func(context.Context, BrowserWindowInput) (BrowserWindowResult, error) {
		722	+	return BrowserWindowResult{}, errors.New("fallback failed")
		723	+	})
		724	+	pm.setOnOpenNativePopup(func(ctx context.Context, input NativePopupInput) error {
		725	+	if !input.OnNativeHostAbort(ctx, input.PopupWebView) {
		726	+	input.PopupWebView.Destroy()
		727	+	}
		728	+	return nil
		729	+	})
		730	+
		731	+	got := pm.openNativePopup(context.Background(), popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
		732	+	TargetURI: "https://example.com/visual", NoJavaScriptAccess: true,
		733	+	}, dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup})
		734	+	require.Same(t, popupWV, got)
		735	+	}
		736	+
		737	+	func TestPopupNativeSetupFailureDestroysCoordinatorOwnedWebViewOnce(t *testing.T) {
		738	+	popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)}
		739	+	popupWV.EXPECT().Destroy().Once()
		740	+	factory := mocks.NewMockWebViewFactory(t)
		741	+	factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once()
		742	+	pm := newPopupManager()
		743	+	pm.setConfig(factory, nil, nil)
		744	+	pm.setOnOpenNativePopup(func(context.Context, NativePopupInput) error { return errors.New("setup failed") })
		745	+
		746	+	got := pm.openNativePopup(context.Background(), popupCoordinatorHooks{}, "floating", 101, "", port.PopupRequest{
		747	+	TargetURI: "https://example.com/visual", NoJavaScriptAccess: true,
		748	+	}, dto.HostDecision{Kind: dto.HostDecisionCreateNativePopup})
		749	+	assert.Nil(t, got)
		750	+	}
		751	+
647	func TestHandlePopupCreate_DoesNotReuseNamedPopupWhenNoJavaScriptAccess(t *testing.T) {	752		func TestHandlePopupCreate_DoesNotReuseNamedPopupWhenNoJavaScriptAccess(t *testing.T) {
648	ctx := context.Background()	753		ctx := context.Background()
649	parentPaneID := entity.PaneID("parent-pane")	754		parentPaneID := entity.PaneID("parent-pane")

M internal/ui/native_popup_window.go +50 -25


@@ -14,25 +14,32 @@ import (
14	14		"github.com/bnema/puregotk/v4/gtk"
15	15		)
16	16
	17	+	type nativePopupShell interface {
	18	+	DetachContent() *gtk.Widget
	19	+	Destroy()
	20	+	Close()
	21	+	Show()
	22	+	}
	23	+
17	24		type nativePopupWindow struct {
18	25		popupID port.WebViewID
19	26		parentPaneID entity.PaneID
20	27		parentWindowID string
21		-	popupWindow *window.PopupWindow
	28	+	popupWindow nativePopupShell
22	29		webView port.WebView
23	30		closeOnce sync.Once
24	31		}
25	32
26		-	type nativePopupDestroyer interface {
27		-	Destroy()
28		-	}
	33	+	type nativePopupReleaseMode int
	34	+
	35	+	const (
	36	+	nativePopupReleaseDestroy nativePopupReleaseMode = iota
	37	+	nativePopupReleaseDetach
	38	+	)
29	39
30	40		var newPopupWindow = window.NewPopup
31	41
32		-	func destroyFailedNativePopupSetup(popupShell nativePopupDestroyer, wv port.WebView) {
33		-	if wv != nil && !wv.IsDestroyed() {
34		-	wv.Destroy()
35		-	}
	42	+	func destroyFailedNativePopupSetup(popupShell nativePopupShell) {
36	43		if popupShell != nil {
37	44		popupShell.Destroy()
38	45		}

@@ -67,17 +74,16 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
67	74
68	75		popupShell, err := newPopupWindow(ctx, a.gtkApp)
69	76		if err != nil {
70		-	destroyFailedNativePopupSetup(nil, input.PopupWebView)
71	77		return err
72	78		}
73	79		if a.contentCoord == nil {
74		-	destroyFailedNativePopupSetup(popupShell, input.PopupWebView)
	80	+	destroyFailedNativePopupSetup(popupShell)
75	81		return fmt.Errorf("content coordinator not available for native popup")
76	82		}
77	83		widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView)
78	84		gtkWidget, err := prepareNativePopupContentWidget(widget)
79	85		if err != nil {
80		-	destroyFailedNativePopupSetup(popupShell, input.PopupWebView)
	86	+	destroyFailedNativePopupSetup(popupShell)
81	87		return err
82	88		}
83	89		popupShell.SetContent(gtkWidget)

@@ -102,7 +108,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
102	108
103	109		if popupShell.Window() != nil {
104	110		closeRequestCb := func(_ gtk.Window) bool {
105		-	a.releaseNativePopupWindow(popupID, false, false)
	111	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
106	112		return false
107	113		}
108	114		popupShell.Window().ConnectCloseRequest(&closeRequestCb)

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

@@ -123,7 +129,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
123	129		})
124	130		lifecycle.SetOnClose(func() {
125	131		a.dispatchNativePopupLifecycle("ui.native_popup.close", popupID, func() {
126		-	a.releaseNativePopupWindow(popupID, true, false)
	132	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
127	133		})
128	134		})
129	135		} else {

@@ -132,7 +138,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
132	138		if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok {
133	139		oauthWV.AddCloseCallback(func() {
134	140		a.dispatchNativePopupLifecycle("ui.native_popup.oauth_close", popupID, func() {
135		-	a.releaseNativePopupWindow(popupID, true, false)
	141	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
136	142		})
137	143		})
138	144		}

@@ -148,6 +154,22 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
148	154		return nil
149	155		}
150	156
	157	+	func (a *App) abortNativePopupWindow(ctx context.Context, popupID port.WebViewID, input content.NativePopupInput) {
	158	+	wv := a.releaseNativePopupWindow(popupID, nativePopupReleaseDetach)
	159	+	if wv == nil {
	160	+	return
	161	+	}
	162	+	transferred := false
	163	+	if input.AllowBrowserWindowFallback && input.OnNativeHostAbort != nil {
	164	+	transferred = input.OnNativeHostAbort(ctx, wv)
	165	+	} else if input.OnNativeHostAbort != nil {
	166	+	input.OnNativeHostAbort(ctx, wv)
	167	+	}
	168	+	if !transferred && !wv.IsDestroyed() {
	169	+	wv.Destroy()
	170	+	}
	171	+	}
	172	+
151	173		func (a *App) dispatchNativePopupLifecycle(label string, popupID port.WebViewID, fn func()) {
152	174		if a == nil \|\| fn == nil {
153	175		return

@@ -181,26 +203,29 @@ func (a *App) showNativePopupWindow(popupID port.WebViewID) {
181	203		}
182	204		}
183	205
184		-	func (a *App) releaseNativePopupWindow(popupID port.WebViewID, closeWindow, destroyWindow bool) {
	206	+	func (a *App) releaseNativePopupWindow(popupID port.WebViewID, mode nativePopupReleaseMode) port.WebView {
185	207		if a == nil \|\| a.nativePopupWindows == nil {
186		-	return
	208	+	return nil
187	209		}
188	210		state := a.nativePopupWindows[popupID]
189	211		if state == nil {
190		-	return
	212	+	return nil
191	213		}
	214	+	var detached port.WebView
192	215		state.closeOnce.Do(func() {
193	216		delete(a.nativePopupWindows, popupID)
194		-	if state.webView != nil && !state.webView.IsDestroyed() {
	217	+	if mode == nativePopupReleaseDetach {
	218	+	if state.popupWindow != nil && state.popupWindow.DetachContent() != nil {
	219	+	detached = state.webView
	220	+	} else if state.webView != nil && !state.webView.IsDestroyed() {
	221	+	state.webView.Destroy()
	222	+	}
	223	+	} else if state.webView != nil && !state.webView.IsDestroyed() {
195	224		state.webView.Destroy()
196	225		}
197	226		if state.popupWindow != nil {
198		-	switch {
199		-	case destroyWindow:
200		-	state.popupWindow.Destroy()
201		-	case closeWindow:
202		-	state.popupWindow.Close()
203		-	}
	227	+	state.popupWindow.Destroy()
204	228		}
205	229		})
	230	+	return detached
206	231		}


@@ -14,25 +14,32 @@ import (
14		"github.com/bnema/puregotk/v4/gtk"	14		"github.com/bnema/puregotk/v4/gtk"
15		)	15		)
16			16
			17	+	type nativePopupShell interface {
			18	+	DetachContent() *gtk.Widget
			19	+	Destroy()
			20	+	Close()
			21	+	Show()
			22	+	}
			23	+
17		type nativePopupWindow struct {	24		type nativePopupWindow struct {
18		popupID port.WebViewID	25		popupID port.WebViewID
19		parentPaneID entity.PaneID	26		parentPaneID entity.PaneID
20		parentWindowID string	27		parentWindowID string
21	-	popupWindow *window.PopupWindow	28	+	popupWindow nativePopupShell
22		webView port.WebView	29		webView port.WebView
23		closeOnce sync.Once	30		closeOnce sync.Once
24		}	31		}
25			32
26	-	type nativePopupDestroyer interface {	33	+	type nativePopupReleaseMode int
27	-	Destroy()	34	+
28	-	}	35	+	const (
			36	+	nativePopupReleaseDestroy nativePopupReleaseMode = iota
			37	+	nativePopupReleaseDetach
			38	+	)
29			39
30		var newPopupWindow = window.NewPopup	40		var newPopupWindow = window.NewPopup
31			41
32	-	func destroyFailedNativePopupSetup(popupShell nativePopupDestroyer, wv port.WebView) {	42	+	func destroyFailedNativePopupSetup(popupShell nativePopupShell) {
33	-	if wv != nil && !wv.IsDestroyed() {
34	-	wv.Destroy()
35	-	}
36		if popupShell != nil {	43		if popupShell != nil {
37		popupShell.Destroy()	44		popupShell.Destroy()
38		}	45		}

@@ -67,17 +74,16 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
67			74
68		popupShell, err := newPopupWindow(ctx, a.gtkApp)	75		popupShell, err := newPopupWindow(ctx, a.gtkApp)
69		if err != nil {	76		if err != nil {
70	-	destroyFailedNativePopupSetup(nil, input.PopupWebView)
71		return err	77		return err
72		}	78		}
73		if a.contentCoord == nil {	79		if a.contentCoord == nil {
74	-	destroyFailedNativePopupSetup(popupShell, input.PopupWebView)	80	+	destroyFailedNativePopupSetup(popupShell)
75		return fmt.Errorf("content coordinator not available for native popup")	81		return fmt.Errorf("content coordinator not available for native popup")
76		}	82		}
77		widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView)	83		widget := a.contentCoord.WrapWidget(ctx, input.PopupWebView)
78		gtkWidget, err := prepareNativePopupContentWidget(widget)	84		gtkWidget, err := prepareNativePopupContentWidget(widget)
79		if err != nil {	85		if err != nil {
80	-	destroyFailedNativePopupSetup(popupShell, input.PopupWebView)	86	+	destroyFailedNativePopupSetup(popupShell)
81		return err	87		return err
82		}	88		}
83		popupShell.SetContent(gtkWidget)	89		popupShell.SetContent(gtkWidget)

@@ -102,7 +108,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
102			108
103		if popupShell.Window() != nil {	109		if popupShell.Window() != nil {
104		closeRequestCb := func(_ gtk.Window) bool {	110		closeRequestCb := func(_ gtk.Window) bool {
105	-	a.releaseNativePopupWindow(popupID, false, false)	111	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
106		return false	112		return false
107		}	113		}
108		popupShell.Window().ConnectCloseRequest(&closeRequestCb)	114		popupShell.Window().ConnectCloseRequest(&closeRequestCb)

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

@@ -123,7 +129,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
123		})	129		})
124		lifecycle.SetOnClose(func() {	130		lifecycle.SetOnClose(func() {
125		a.dispatchNativePopupLifecycle("ui.native_popup.close", popupID, func() {	131		a.dispatchNativePopupLifecycle("ui.native_popup.close", popupID, func() {
126	-	a.releaseNativePopupWindow(popupID, true, false)	132	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
127		})	133		})
128		})	134		})
129		} else {	135		} else {

@@ -132,7 +138,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
132		if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok {	138		if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok {
133		oauthWV.AddCloseCallback(func() {	139		oauthWV.AddCloseCallback(func() {
134		a.dispatchNativePopupLifecycle("ui.native_popup.oauth_close", popupID, func() {	140		a.dispatchNativePopupLifecycle("ui.native_popup.oauth_close", popupID, func() {
135	-	a.releaseNativePopupWindow(popupID, true, false)	141	+	a.releaseNativePopupWindow(popupID, nativePopupReleaseDestroy)
136		})	142		})
137		})	143		})
138		}	144		}

@@ -148,6 +154,22 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop
148		return nil	154		return nil
149		}	155		}
150			156
			157	+	func (a *App) abortNativePopupWindow(ctx context.Context, popupID port.WebViewID, input content.NativePopupInput) {
			158	+	wv := a.releaseNativePopupWindow(popupID, nativePopupReleaseDetach)
			159	+	if wv == nil {
			160	+	return
			161	+	}
			162	+	transferred := false
			163	+	if input.AllowBrowserWindowFallback && input.OnNativeHostAbort != nil {
			164	+	transferred = input.OnNativeHostAbort(ctx, wv)
			165	+	} else if input.OnNativeHostAbort != nil {
			166	+	input.OnNativeHostAbort(ctx, wv)
			167	+	}
			168	+	if !transferred && !wv.IsDestroyed() {
			169	+	wv.Destroy()
			170	+	}
			171	+	}
			172	+
151		func (a *App) dispatchNativePopupLifecycle(label string, popupID port.WebViewID, fn func()) {	173		func (a *App) dispatchNativePopupLifecycle(label string, popupID port.WebViewID, fn func()) {
152		if a == nil \|\| fn == nil {	174		if a == nil \|\| fn == nil {
153		return	175		return

@@ -181,26 +203,29 @@ func (a *App) showNativePopupWindow(popupID port.WebViewID) {
181		}	203		}
182		}	204		}
183			205
184	-	func (a *App) releaseNativePopupWindow(popupID port.WebViewID, closeWindow, destroyWindow bool) {	206	+	func (a *App) releaseNativePopupWindow(popupID port.WebViewID, mode nativePopupReleaseMode) port.WebView {
185		if a == nil \|\| a.nativePopupWindows == nil {	207		if a == nil \|\| a.nativePopupWindows == nil {
186	-	return	208	+	return nil
187		}	209		}
188		state := a.nativePopupWindows[popupID]	210		state := a.nativePopupWindows[popupID]
189		if state == nil {	211		if state == nil {
190	-	return	212	+	return nil
191		}	213		}
			214	+	var detached port.WebView
192		state.closeOnce.Do(func() {	215		state.closeOnce.Do(func() {
193		delete(a.nativePopupWindows, popupID)	216		delete(a.nativePopupWindows, popupID)
194	-	if state.webView != nil && !state.webView.IsDestroyed() {	217	+	if mode == nativePopupReleaseDetach {
			218	+	if state.popupWindow != nil && state.popupWindow.DetachContent() != nil {
			219	+	detached = state.webView
			220	+	} else if state.webView != nil && !state.webView.IsDestroyed() {
			221	+	state.webView.Destroy()
			222	+	}
			223	+	} else if state.webView != nil && !state.webView.IsDestroyed() {
195		state.webView.Destroy()	224		state.webView.Destroy()
196		}	225		}
197		if state.popupWindow != nil {	226		if state.popupWindow != nil {
198	-	switch {	227	+	state.popupWindow.Destroy()
199	-	case destroyWindow:
200	-	state.popupWindow.Destroy()
201	-	case closeWindow:
202	-	state.popupWindow.Close()
203	-	}
204		}	228		}
205		})	229		})
			230	+	return detached
206		}	231		}

M internal/ui/native_popup_window_test.go +67 -23


@@ -18,12 +18,16 @@ import (
18	18		)
19	19
20	20		type popupDestroySpy struct {
21		-	destroyed bool
	21	+	destroyed int
	22	+	detached int
	23	+	closed int
	24	+	shown int
22	25		}
23	26
24		-	func (p *popupDestroySpy) Destroy() {
25		-	p.destroyed = true
26		-	}
	27	+	func (p *popupDestroySpy) Destroy() { p.destroyed++ }
	28	+	func (p popupDestroySpy) DetachContent() gtk.Widget { p.detached++; return &gtk.Widget{} }
	29	+	func (p *popupDestroySpy) Close() { p.closed++ }
	30	+	func (p *popupDestroySpy) Show() { p.shown++ }
27	31
28	32		func TestPrepareNativePopupContentWidget_ExpandsWrappedWidget(t *testing.T) {
29	33		widget := layoutmocks.NewMockWidget(t)

@@ -46,7 +50,7 @@ func TestPrepareNativePopupContentWidget_ErrorsWhenWrappedGTKWidgetMissing(t *te
46	50		assert.Nil(t, got)
47	51		}
48	52
49		-	func TestOpenNativePopupWindow_DestroysWebViewWhenPopupShellCreationFails(t *testing.T) {
	53	+	func TestOpenNativePopupWindowLeavesWebViewOwnedByCallerOnSetupFailure(t *testing.T) {
50	54		oldNewPopupWindow := newPopupWindow
51	55		t.Cleanup(func() {
52	56		newPopupWindow = oldNewPopupWindow

@@ -58,23 +62,15 @@ func TestOpenNativePopupWindow_DestroysWebViewWhenPopupShellCreationFails(t *tes
58	62		}
59	63
60	64		wv := portmocks.NewMockWebView(t)
61		-	wv.EXPECT().IsDestroyed().Return(false).Once()
62		-	wv.EXPECT().Destroy().Once()
63		-
64	65		app := &App{gtkApp: &gtk.Application{}}
65	66		err := app.openNativePopupWindow(context.Background(), content.NativePopupInput{PopupWebView: wv})
66	67		require.ErrorIs(t, err, expectedErr)
67	68		}
68	69
69		-	func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) {
70		-	wv := portmocks.NewMockWebView(t)
71		-	wv.EXPECT().IsDestroyed().Return(false).Once()
72		-	wv.EXPECT().Destroy().Once()
73		-
	70	+	func TestDestroyFailedNativePopupSetupDestroysOnlyShell(t *testing.T) {
74	71		shell := &popupDestroySpy{}
75		-	destroyFailedNativePopupSetup(shell, wv)
76		-
77		-	assert.True(t, shell.destroyed)
	72	+	destroyFailedNativePopupSetup(shell)
	73	+	assert.Equal(t, 1, shell.destroyed)
78	74		}
79	75
80	76		func TestDispatchNativePopupLifecycleSkipsWorkWhenDispatchTimesOutBeforeStart(t *testing.T) {

@@ -104,19 +100,67 @@ func TestDispatchNativePopupLifecycleTreatsCompletedAfterTimeoutAsCompleted(t *t
104	100		assert.True(t, ran)
105	101		}
106	102
107		-	func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) {
	103	+	func TestDetachNativePopupWindowRemovesWidgetWithoutDestroyingWebView(t *testing.T) {
	104	+	wv := portmocks.NewMockWebView(t)
	105	+	shell := &popupDestroySpy{}
	106	+	app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
	107	+	1: {popupID: 1, webView: wv, popupWindow: shell},
	108	+	}}
	109	+
	110	+	detached := app.releaseNativePopupWindow(1, nativePopupReleaseDetach)
	111	+	assert.Same(t, wv, detached)
	112	+	assert.Equal(t, 1, shell.detached)
	113	+	assert.Equal(t, 1, shell.destroyed)
	114	+	}
	115	+
	116	+	func TestNativePopupAbortTransfersEligibleWebViewToBrowserFallback(t *testing.T) {
	117	+	wv := portmocks.NewMockWebView(t)
	118	+	shell := &popupDestroySpy{}
	119	+	app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
	120	+	1: {popupID: 1, webView: wv, popupWindow: shell},
	121	+	}}
	122	+	callbackCalls := 0
	123	+	app.abortNativePopupWindow(context.Background(), 1, content.NativePopupInput{
	124	+	AllowBrowserWindowFallback: true,
	125	+	OnNativeHostAbort: func(_ context.Context, got port.WebView) bool {
	126	+	callbackCalls++
	127	+	assert.Same(t, wv, got)
	128	+	assert.Equal(t, 1, shell.detached, "widget must detach before fallback adoption")
	129	+	return true
	130	+	},
	131	+	})
	132	+
	133	+	assert.Equal(t, 1, callbackCalls)
	134	+	assert.Equal(t, 1, shell.destroyed)
	135	+	}
	136	+
	137	+	func TestNativePopupAbortDestroysOpenerRequiredWebView(t *testing.T) {
108	138		wv := portmocks.NewMockWebView(t)
109	139		wv.EXPECT().IsDestroyed().Return(false).Once()
110	140		wv.EXPECT().Destroy().Once()
	141	+	shell := &popupDestroySpy{}
	142	+	app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
	143	+	1: {popupID: 1, webView: wv, popupWindow: shell},
	144	+	}}
	145	+	app.abortNativePopupWindow(context.Background(), 1, content.NativePopupInput{
	146	+	AllowBrowserWindowFallback: false,
	147	+	OnNativeHostAbort: func(context.Context, port.WebView) bool { return false },
	148	+	})
	149	+	assert.Equal(t, 1, shell.detached)
	150	+	}
111	151
	152	+	func TestReleaseNativePopupWindowDestroysOnlyOnce(t *testing.T) {
	153	+	wv := portmocks.NewMockWebView(t)
	154	+	wv.EXPECT().IsDestroyed().Return(false).Once()
	155	+	wv.EXPECT().Destroy().Once()
	156	+	shell := &popupDestroySpy{}
112	157		app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
113		-	port.WebViewID(1): {
114		-	popupID: port.WebViewID(1),
115		-	webView: wv,
116		-	},
	158	+	1: {popupID: 1, webView: wv, popupWindow: shell},
117	159		}}
118	160
119		-	app.releaseNativePopupWindow(port.WebViewID(1), false, false)
120		-	_, ok := app.nativePopupWindows[port.WebViewID(1)]
	161	+	app.releaseNativePopupWindow(1, nativePopupReleaseDestroy)
	162	+	app.releaseNativePopupWindow(1, nativePopupReleaseDestroy)
	163	+	_, ok := app.nativePopupWindows[1]
121	164		assert.False(t, ok)
	165	+	assert.Equal(t, 1, shell.destroyed)
122	166		}


@@ -18,12 +18,16 @@ import (
18		)	18		)
19			19
20		type popupDestroySpy struct {	20		type popupDestroySpy struct {
21	-	destroyed bool	21	+	destroyed int
			22	+	detached int
			23	+	closed int
			24	+	shown int
22		}	25		}
23			26
24	-	func (p *popupDestroySpy) Destroy() {	27	+	func (p *popupDestroySpy) Destroy() { p.destroyed++ }
25	-	p.destroyed = true	28	+	func (p popupDestroySpy) DetachContent() gtk.Widget { p.detached++; return &gtk.Widget{} }
26	-	}	29	+	func (p *popupDestroySpy) Close() { p.closed++ }
			30	+	func (p *popupDestroySpy) Show() { p.shown++ }
27			31
28		func TestPrepareNativePopupContentWidget_ExpandsWrappedWidget(t *testing.T) {	32		func TestPrepareNativePopupContentWidget_ExpandsWrappedWidget(t *testing.T) {
29		widget := layoutmocks.NewMockWidget(t)	33		widget := layoutmocks.NewMockWidget(t)

@@ -46,7 +50,7 @@ func TestPrepareNativePopupContentWidget_ErrorsWhenWrappedGTKWidgetMissing(t *te
46		assert.Nil(t, got)	50		assert.Nil(t, got)
47		}	51		}
48			52
49	-	func TestOpenNativePopupWindow_DestroysWebViewWhenPopupShellCreationFails(t *testing.T) {	53	+	func TestOpenNativePopupWindowLeavesWebViewOwnedByCallerOnSetupFailure(t *testing.T) {
50		oldNewPopupWindow := newPopupWindow	54		oldNewPopupWindow := newPopupWindow
51		t.Cleanup(func() {	55		t.Cleanup(func() {
52		newPopupWindow = oldNewPopupWindow	56		newPopupWindow = oldNewPopupWindow

@@ -58,23 +62,15 @@ func TestOpenNativePopupWindow_DestroysWebViewWhenPopupShellCreationFails(t *tes
58		}	62		}
59			63
60		wv := portmocks.NewMockWebView(t)	64		wv := portmocks.NewMockWebView(t)
61	-	wv.EXPECT().IsDestroyed().Return(false).Once()
62	-	wv.EXPECT().Destroy().Once()
63	-
64		app := &App{gtkApp: &gtk.Application{}}	65		app := &App{gtkApp: &gtk.Application{}}
65		err := app.openNativePopupWindow(context.Background(), content.NativePopupInput{PopupWebView: wv})	66		err := app.openNativePopupWindow(context.Background(), content.NativePopupInput{PopupWebView: wv})
66		require.ErrorIs(t, err, expectedErr)	67		require.ErrorIs(t, err, expectedErr)
67		}	68		}
68			69
69	-	func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) {	70	+	func TestDestroyFailedNativePopupSetupDestroysOnlyShell(t *testing.T) {
70	-	wv := portmocks.NewMockWebView(t)
71	-	wv.EXPECT().IsDestroyed().Return(false).Once()
72	-	wv.EXPECT().Destroy().Once()
73	-
74		shell := &popupDestroySpy{}	71		shell := &popupDestroySpy{}
75	-	destroyFailedNativePopupSetup(shell, wv)	72	+	destroyFailedNativePopupSetup(shell)
76	-		73	+	assert.Equal(t, 1, shell.destroyed)
77	-	assert.True(t, shell.destroyed)
78		}	74		}
79			75
80		func TestDispatchNativePopupLifecycleSkipsWorkWhenDispatchTimesOutBeforeStart(t *testing.T) {	76		func TestDispatchNativePopupLifecycleSkipsWorkWhenDispatchTimesOutBeforeStart(t *testing.T) {

@@ -104,19 +100,67 @@ func TestDispatchNativePopupLifecycleTreatsCompletedAfterTimeoutAsCompleted(t *t
104		assert.True(t, ran)	100		assert.True(t, ran)
105		}	101		}
106			102
107	-	func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) {	103	+	func TestDetachNativePopupWindowRemovesWidgetWithoutDestroyingWebView(t *testing.T) {
			104	+	wv := portmocks.NewMockWebView(t)
			105	+	shell := &popupDestroySpy{}
			106	+	app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
			107	+	1: {popupID: 1, webView: wv, popupWindow: shell},
			108	+	}}
			109	+
			110	+	detached := app.releaseNativePopupWindow(1, nativePopupReleaseDetach)
			111	+	assert.Same(t, wv, detached)
			112	+	assert.Equal(t, 1, shell.detached)
			113	+	assert.Equal(t, 1, shell.destroyed)
			114	+	}
			115	+
			116	+	func TestNativePopupAbortTransfersEligibleWebViewToBrowserFallback(t *testing.T) {
			117	+	wv := portmocks.NewMockWebView(t)
			118	+	shell := &popupDestroySpy{}
			119	+	app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
			120	+	1: {popupID: 1, webView: wv, popupWindow: shell},
			121	+	}}
			122	+	callbackCalls := 0
			123	+	app.abortNativePopupWindow(context.Background(), 1, content.NativePopupInput{
			124	+	AllowBrowserWindowFallback: true,
			125	+	OnNativeHostAbort: func(_ context.Context, got port.WebView) bool {
			126	+	callbackCalls++
			127	+	assert.Same(t, wv, got)
			128	+	assert.Equal(t, 1, shell.detached, "widget must detach before fallback adoption")
			129	+	return true
			130	+	},
			131	+	})
			132	+
			133	+	assert.Equal(t, 1, callbackCalls)
			134	+	assert.Equal(t, 1, shell.destroyed)
			135	+	}
			136	+
			137	+	func TestNativePopupAbortDestroysOpenerRequiredWebView(t *testing.T) {
108		wv := portmocks.NewMockWebView(t)	138		wv := portmocks.NewMockWebView(t)
109		wv.EXPECT().IsDestroyed().Return(false).Once()	139		wv.EXPECT().IsDestroyed().Return(false).Once()
110		wv.EXPECT().Destroy().Once()	140		wv.EXPECT().Destroy().Once()
			141	+	shell := &popupDestroySpy{}
			142	+	app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
			143	+	1: {popupID: 1, webView: wv, popupWindow: shell},
			144	+	}}
			145	+	app.abortNativePopupWindow(context.Background(), 1, content.NativePopupInput{
			146	+	AllowBrowserWindowFallback: false,
			147	+	OnNativeHostAbort: func(context.Context, port.WebView) bool { return false },
			148	+	})
			149	+	assert.Equal(t, 1, shell.detached)
			150	+	}
111			151
			152	+	func TestReleaseNativePopupWindowDestroysOnlyOnce(t *testing.T) {
			153	+	wv := portmocks.NewMockWebView(t)
			154	+	wv.EXPECT().IsDestroyed().Return(false).Once()
			155	+	wv.EXPECT().Destroy().Once()
			156	+	shell := &popupDestroySpy{}
112		app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{	157		app := &App{nativePopupWindows: map[port.WebViewID]*nativePopupWindow{
113	-	port.WebViewID(1): {	158	+	1: {popupID: 1, webView: wv, popupWindow: shell},
114	-	popupID: port.WebViewID(1),
115	-	webView: wv,
116	-	},
117		}}	159		}}
118			160
119	-	app.releaseNativePopupWindow(port.WebViewID(1), false, false)	161	+	app.releaseNativePopupWindow(1, nativePopupReleaseDestroy)
120	-	_, ok := app.nativePopupWindows[port.WebViewID(1)]	162	+	app.releaseNativePopupWindow(1, nativePopupReleaseDestroy)
			163	+	_, ok := app.nativePopupWindows[1]
121		assert.False(t, ok)	164		assert.False(t, ok)
			165	+	assert.Equal(t, 1, shell.destroyed)
122		}	166		}

M internal/ui/window/popup_window.go +12 -0


@@ -61,6 +61,18 @@ func (pw PopupWindow) SetContent(widget gtk.Widget) {
61	61		pw.currentContent = widget
62	62		}
63	63
	64	+	// DetachContent removes and returns the hosted widget without destroying it.
	65	+	// Callers must detach before reparenting the widget into another GTK host.
	66	+	func (pw PopupWindow) DetachContent() gtk.Widget {
	67	+	if pw == nil \|\| pw.content == nil \|\| pw.currentContent == nil {
	68	+	return nil
	69	+	}
	70	+	widget := pw.currentContent
	71	+	pw.content.Remove(widget)
	72	+	pw.currentContent = nil
	73	+	return widget
	74	+	}
	75	+
64	76		func (pw *PopupWindow) Show() {
65	77		if pw == nil \|\| pw.window == nil {
66	78		return


@@ -61,6 +61,18 @@ func (pw PopupWindow) SetContent(widget gtk.Widget) {
61	pw.currentContent = widget	61		pw.currentContent = widget
62	}	62		}
63		63
		64	+	// DetachContent removes and returns the hosted widget without destroying it.
		65	+	// Callers must detach before reparenting the widget into another GTK host.
		66	+	func (pw PopupWindow) DetachContent() gtk.Widget {
		67	+	if pw == nil \|\| pw.content == nil \|\| pw.currentContent == nil {
		68	+	return nil
		69	+	}
		70	+	widget := pw.currentContent
		71	+	pw.content.Remove(widget)
		72	+	pw.currentContent = nil
		73	+	return widget
		74	+	}
		75	+
64	func (pw *PopupWindow) Show() {	76		func (pw *PopupWindow) Show() {
65	if pw == nil \|\| pw.window == nil {	77		if pw == nil \|\| pw.window == nil {
66	return	78		return