d96730e fix: address code review findings (port abstraction, races, error handling, lint) · dumber

M internal/application/port/webview.go +25 -0


@@ -272,3 +272,28 @@ type DevToolsOpener interface {
272	272		type Printer interface {
273	273		PrintPage()
274	274		}
	275	+
	276	+	// PopupCapable is implemented by WebViews that support popup lifecycle callbacks.
	277	+	// SetOnClose composes the provided function with any existing close handler so
	278	+	// that multiple callers can each register a close callback without overwriting
	279	+	// one another.
	280	+	type PopupCapable interface {
	281	+	SetOnReadyToShow(fn func())
	282	+	SetOnClose(fn func())
	283	+	Show()
	284	+	}
	285	+
	286	+	// OAuthCallbackCapable is implemented by WebViews that support OAuth auto-close callbacks.
	287	+	// It is an optional capability; callers should type-assert port.WebView to this interface
	288	+	// before use and degrade gracefully when it is absent.
	289	+	type OAuthCallbackCapable interface {
	290	+	// AddCloseCallback registers fn to be called when the WebView is programmatically closed.
	291	+	// Multiple callbacks may be registered; each will be called in registration order.
	292	+	AddCloseCallback(fn func())
	293	+	// AddNavigationCallback registers fn to be called whenever the URI changes or a
	294	+	// committed load is detected. The uri parameter contains the new URL.
	295	+	// Multiple callbacks may be registered; each will be called in registration order.
	296	+	AddNavigationCallback(fn func(uri string))
	297	+	// Close triggers a programmatic close of the WebView (equivalent to window.close()).
	298	+	Close()
	299	+	}


@@ -272,3 +272,28 @@ type DevToolsOpener interface {
272	type Printer interface {	272		type Printer interface {
273	PrintPage()	273		PrintPage()
274	}	274		}
		275	+
		276	+	// PopupCapable is implemented by WebViews that support popup lifecycle callbacks.
		277	+	// SetOnClose composes the provided function with any existing close handler so
		278	+	// that multiple callers can each register a close callback without overwriting
		279	+	// one another.
		280	+	type PopupCapable interface {
		281	+	SetOnReadyToShow(fn func())
		282	+	SetOnClose(fn func())
		283	+	Show()
		284	+	}
		285	+
		286	+	// OAuthCallbackCapable is implemented by WebViews that support OAuth auto-close callbacks.
		287	+	// It is an optional capability; callers should type-assert port.WebView to this interface
		288	+	// before use and degrade gracefully when it is absent.
		289	+	type OAuthCallbackCapable interface {
		290	+	// AddCloseCallback registers fn to be called when the WebView is programmatically closed.
		291	+	// Multiple callbacks may be registered; each will be called in registration order.
		292	+	AddCloseCallback(fn func())
		293	+	// AddNavigationCallback registers fn to be called whenever the URI changes or a
		294	+	// committed load is detected. The uri parameter contains the new URL.
		295	+	// Multiple callbacks may be registered; each will be called in registration order.
		296	+	AddNavigationCallback(fn func(uri string))
		297	+	// Close triggers a programmatic close of the WebView (equivalent to window.close()).
		298	+	Close()
		299	+	}

M internal/infrastructure/config/migrate_engine_test.go +8 -2


@@ -240,8 +240,14 @@ disable_dmabuf_renderer = false
240	240		assert.NotContains(t, media, "gstreamer_debug_level")
241	241
242	242		// Migrated fields should be in engine.webkit
243		-	engine := raw["engine"].(map[string]any)
244		-	webkit := engine["webkit"].(map[string]any)
	243	+	engineAny, ok := raw["engine"]
	244	+	require.True(t, ok, "engine section must exist")
	245	+	engine, ok := engineAny.(map[string]any)
	246	+	require.True(t, ok, "engine section must be a map")
	247	+	webkitAny, ok := engine["webkit"]
	248	+	require.True(t, ok, "engine.webkit section must exist")
	249	+	webkit, ok := webkitAny.(map[string]any)
	250	+	require.True(t, ok, "engine.webkit section must be a map")
245	251		assert.Equal(t, true, webkit["force_vsync"])
246	252		assert.Equal(t, "auto", webkit["gl_rendering_mode"])
247	253		assert.Equal(t, int64(3), webkit["gstreamer_debug_level"])

 	assert.NotContains(t, media, "gstreamer_debug_level")
 	// Migrated fields should be in engine.webkit
-	engine := raw["engine"].(map[string]any)
+	engineAny, ok := raw["engine"]
-	webkit := engine["webkit"].(map[string]any)
+	require.True(t, ok, "engine section must exist")
+	engine, ok := engineAny.(map[string]any)
+	require.True(t, ok, "engine section must be a map")
+	webkitAny, ok := engine["webkit"]
+	require.True(t, ok, "engine.webkit section must exist")
+	webkit, ok := webkitAny.(map[string]any)
+	require.True(t, ok, "engine.webkit section must be a map")
 	assert.Equal(t, true, webkit["force_vsync"])
 	assert.Equal(t, "auto", webkit["gl_rendering_mode"])
 	assert.Equal(t, int64(3), webkit["gstreamer_debug_level"])

M internal/infrastructure/webkit/engine.go +5 -3


@@ -5,6 +5,7 @@ import (
5	5
6	6		"github.com/bnema/dumber/internal/application/port"
7	7		"github.com/bnema/dumber/internal/infrastructure/filtering"
	8	+	"github.com/rs/zerolog"
8	9		)
9	10
10	11		// Engine implements port.Engine for the WebKit browser engine.

@@ -18,13 +19,14 @@ type Engine struct {
18	19		filterManager *filtering.Manager
19	20		schemeHandler *DumbSchemeHandler
20	21		schemePath string
	22	+	logger zerolog.Logger
21	23		}
22	24
23	25		// Compile-time check that Engine implements port.Engine.
24	26		var _ port.Engine = (*Engine)(nil)
25	27
26	28		// Init is a no-op for WebKit — initialization happens in NewEngine.
27		-	func (e *Engine) Init(_ context.Context, _ port.EngineOptions) error {
	29	+	func (*Engine) Init(_ context.Context, _ port.EngineOptions) error {
28	30		return nil
29	31		}
30	32

@@ -35,7 +37,7 @@ func (e *Engine) Factory() port.WebViewFactory {
35	37
36	38		// Pool returns the WebViewPool wrapped as a port.WebViewPool.
37	39		func (e *Engine) Pool() port.WebViewPool {
38		-	return &webViewPoolAdapter{pool: e.pool}
	40	+	return &webViewPoolAdapter{pool: e.pool, logger: e.logger}
39	41		}
40	42
41	43		// ContentInjector returns the ContentInjector implementing port.ContentInjector.

@@ -58,7 +60,7 @@ func (e *Engine) Close() error {
58	60
59	61		// SchemeHandler returns a port.SchemeHandler adapter for the DumbSchemeHandler.
60	62		func (e *Engine) SchemeHandler() port.SchemeHandler {
61		-	return &schemeHandlerAdapter{handler: e.schemeHandler}
	63	+	return &schemeHandlerAdapter{handler: e.schemeHandler, logger: e.logger}
62	64		}
63	65
64	66		// MessageRouter returns a port.MessageRouter adapter for the internal MessageRouter.

 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/infrastructure/filtering"
+	"github.com/rs/zerolog"
 )
 // Engine implements port.Engine for the WebKit browser engine.
 	filterManager *filtering.Manager
 	schemeHandler *DumbSchemeHandler
 	schemePath    string
+	logger        zerolog.Logger
 }
 // Compile-time check that Engine implements port.Engine.
 var _ port.Engine = (*Engine)(nil)
 // Init is a no-op for WebKit — initialization happens in NewEngine.
-func (e *Engine) Init(_ context.Context, _ port.EngineOptions) error {
+func (*Engine) Init(_ context.Context, _ port.EngineOptions) error {
 	return nil
 }
 // Pool returns the WebViewPool wrapped as a port.WebViewPool.
 func (e *Engine) Pool() port.WebViewPool {
-	return &webViewPoolAdapter{pool: e.pool}
+	return &webViewPoolAdapter{pool: e.pool, logger: e.logger}
 }
 // ContentInjector returns the ContentInjector implementing port.ContentInjector.
 // SchemeHandler returns a port.SchemeHandler adapter for the DumbSchemeHandler.
 func (e *Engine) SchemeHandler() port.SchemeHandler {
-	return &schemeHandlerAdapter{handler: e.schemeHandler}
+	return &schemeHandlerAdapter{handler: e.schemeHandler, logger: e.logger}
 }
 // MessageRouter returns a port.MessageRouter adapter for the internal MessageRouter.

M internal/infrastructure/webkit/engine_adapters.go +18 -8


@@ -6,6 +6,7 @@ import (
6	6
7	7		"github.com/bnema/dumber/internal/application/port"
8	8		"github.com/bnema/dumber/internal/infrastructure/filtering"
	9	+	"github.com/rs/zerolog"
9	10		)
10	11
11	12		// --- WebViewFactory adapter ---

@@ -39,7 +40,8 @@ func (a *webViewFactoryAdapter) CreateRelated(ctx context.Context, parentID port
39	40		// WebViewPool methods use *WebView and require a context; this adapter adapts
40	41		// the signatures to match the port interface.
41	42		type webViewPoolAdapter struct {
42		-	pool *WebViewPool
	43	+	pool *WebViewPool
	44	+	logger zerolog.Logger
43	45		}
44	46
45	47		func (a *webViewPoolAdapter) Acquire(ctx context.Context) (port.WebView, error) {

@@ -54,9 +56,14 @@ func (a *webViewPoolAdapter) Release(wv port.WebView) {
54	56		if wv == nil {
55	57		return
56	58		}
57		-	if wwv, ok := wv.(*WebView); ok {
58		-	a.pool.Release(context.Background(), wwv)
	59	+	wwv, ok := wv.(*WebView)
	60	+	if !ok {
	61	+	a.logger.Warn().
	62	+	Str("concrete_type", fmt.Sprintf("%T", wv)).
	63	+	Msg("webViewPoolAdapter.Release: unexpected type, cannot release to pool")
	64	+	return
59	65		}
	66	+	a.pool.Release(context.Background(), wwv)
60	67		}
61	68
62	69		func (a *webViewPoolAdapter) Prewarm(count int) {

@@ -79,12 +86,15 @@ func (a *webViewPoolAdapter) Close() {
79	86		// consumers are migrated.
80	87		type schemeHandlerAdapter struct {
81	88		handler *DumbSchemeHandler
	89	+	logger zerolog.Logger
82	90		}
83	91
84		-	func (a *schemeHandlerAdapter) RegisterScheme(_ string, _ func(uri string) ([]byte, string, error)) {
	92	+	func (a *schemeHandlerAdapter) RegisterScheme(scheme string, _ func(uri string) ([]byte, string, error)) {
85	93		// DumbSchemeHandler handles the "dumb" scheme exclusively via RegisterPage/RegisterWithContext.
86	94		// Generic scheme registration is not yet wired through this adapter.
87		-	panic("not implemented — DumbSchemeHandler uses RegisterPage/RegisterWithContext pattern")
	95	+	a.logger.Warn().
	96	+	Str("scheme", scheme).
	97	+	Msg("schemeHandlerAdapter.RegisterScheme: not implemented — DumbSchemeHandler uses RegisterPage/RegisterWithContext pattern")
88	98		}
89	99
90	100		// --- MessageRouter adapter ---

@@ -97,13 +107,13 @@ type messageRouterAdapter struct {
97	107		router *MessageRouter
98	108		}
99	109
100		-	func (a *messageRouterAdapter) RegisterHandler(_ string, _ func(message string) (string, error)) {
	110	+	func (*messageRouterAdapter) RegisterHandler(_ string, _ func(message string) (string, error)) {
101	111		// Internal MessageRouter.RegisterHandler takes a MessageHandler interface, not a plain func.
102	112		// Wire up via RegisterHandler(msgType, MessageHandlerFunc{...}) when consumers are migrated.
103	113		panic("not implemented — use MessageRouter.RegisterHandler(msgType, MessageHandler) directly")
104	114		}
105	115
106		-	func (a *messageRouterAdapter) PostMessage(webviewID port.WebViewID, message string) error {
	116	+	func (*messageRouterAdapter) PostMessage(webviewID port.WebViewID, message string) error {
107	117		wv := LookupWebView(webviewID)
108	118		if wv == nil {
109	119		return fmt.Errorf("webview %d not found", webviewID)

@@ -151,7 +161,7 @@ type faviconDatabaseAdapter struct {
151	161		wkCtx *WebKitContext
152	162		}
153	163
154		-	func (a *faviconDatabaseAdapter) GetFaviconAsync(_ string, callback func(port.Texture)) {
	164	+	func (*faviconDatabaseAdapter) GetFaviconAsync(_ string, callback func(port.Texture)) {
155	165		// FaviconDatabase async lookup is not yet wired through this adapter.
156	166		// The underlying API is: wkCtx.FaviconDatabase().GetFavicon(uri, callback).
157	167		// This stub satisfies the interface contract; callers will receive nil.

 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/infrastructure/filtering"
+	"github.com/rs/zerolog"
 )
 // --- WebViewFactory adapter ---
 // WebViewPool methods use *WebView and require a context; this adapter adapts
 // the signatures to match the port interface.
 type webViewPoolAdapter struct {
-	pool *WebViewPool
+	pool   *WebViewPool
+	logger zerolog.Logger
 }
 func (a *webViewPoolAdapter) Acquire(ctx context.Context) (port.WebView, error) {
 	if wv == nil {
 		return
 	}
-	if wwv, ok := wv.(*WebView); ok {
+	wwv, ok := wv.(*WebView)
-		a.pool.Release(context.Background(), wwv)
+	if !ok {
+		a.logger.Warn().
+			Str("concrete_type", fmt.Sprintf("%T", wv)).
+			Msg("webViewPoolAdapter.Release: unexpected type, cannot release to pool")
+		return
 	}
+	a.pool.Release(context.Background(), wwv)
 }
 func (a *webViewPoolAdapter) Prewarm(count int) {
 // consumers are migrated.
 type schemeHandlerAdapter struct {
 	handler *DumbSchemeHandler
+	logger  zerolog.Logger
 }
-func (a *schemeHandlerAdapter) RegisterScheme(_ string, _ func(uri string) ([]byte, string, error)) {
+func (a *schemeHandlerAdapter) RegisterScheme(scheme string, _ func(uri string) ([]byte, string, error)) {
 	// DumbSchemeHandler handles the "dumb" scheme exclusively via RegisterPage/RegisterWithContext.
 	// Generic scheme registration is not yet wired through this adapter.
-	panic("not implemented — DumbSchemeHandler uses RegisterPage/RegisterWithContext pattern")
+	a.logger.Warn().
+		Str("scheme", scheme).
+		Msg("schemeHandlerAdapter.RegisterScheme: not implemented — DumbSchemeHandler uses RegisterPage/RegisterWithContext pattern")
 }
 // --- MessageRouter adapter ---
 	router *MessageRouter
 }
-func (a *messageRouterAdapter) RegisterHandler(_ string, _ func(message string) (string, error)) {
+func (*messageRouterAdapter) RegisterHandler(_ string, _ func(message string) (string, error)) {
 	// Internal MessageRouter.RegisterHandler takes a MessageHandler interface, not a plain func.
 	// Wire up via RegisterHandler(msgType, MessageHandlerFunc{...}) when consumers are migrated.
 	panic("not implemented — use MessageRouter.RegisterHandler(msgType, MessageHandler) directly")
 }
-func (a *messageRouterAdapter) PostMessage(webviewID port.WebViewID, message string) error {
+func (*messageRouterAdapter) PostMessage(webviewID port.WebViewID, message string) error {
 	wv := LookupWebView(webviewID)
 	if wv == nil {
 		return fmt.Errorf("webview %d not found", webviewID)
 	wkCtx *WebKitContext
 }
-func (a *faviconDatabaseAdapter) GetFaviconAsync(_ string, callback func(port.Texture)) {
+func (*faviconDatabaseAdapter) GetFaviconAsync(_ string, callback func(port.Texture)) {
 	// FaviconDatabase async lookup is not yet wired through this adapter.
 	// The underlying API is: wkCtx.FaviconDatabase().GetFavicon(uri, callback).
 	// This stub satisfies the interface contract; callers will receive nil.

M internal/infrastructure/webkit/engine_init.go +4 -1


@@ -113,13 +113,16 @@ func NewEngine(
113	113		filterManager: filterManager,
114	114		schemeHandler: schemeHandler,
115	115		schemePath: "dumb://",
	116	+	logger: logger,
116	117		}
117	118
118	119		return engine, nil
119	120		}
120	121
121	122		// engineSurveyHardwareAndResolveProfile surveys hardware and resolves the performance profile.
122		-	func engineSurveyHardwareAndResolveProfile(ctx context.Context, cfg *config.Config, logger zerolog.Logger) config.ResolvedPerformanceSettings {
	123	+	func engineSurveyHardwareAndResolveProfile(
	124	+	ctx context.Context, cfg *config.Config, logger zerolog.Logger,
	125	+	) config.ResolvedPerformanceSettings {
123	126		hwSurveyor := env.NewHardwareSurveyor()
124	127		hwInfo := hwSurveyor.Survey(ctx)
125	128		logging.Trace().Mark("hardware_survey")

 		filterManager: filterManager,
 		schemeHandler: schemeHandler,
 		schemePath:    "dumb://",
+		logger:        logger,
 	}
 	return engine, nil
 }
 // engineSurveyHardwareAndResolveProfile surveys hardware and resolves the performance profile.
-func engineSurveyHardwareAndResolveProfile(ctx context.Context, cfg *config.Config, logger zerolog.Logger) config.ResolvedPerformanceSettings {
+func engineSurveyHardwareAndResolveProfile(
+	ctx context.Context, cfg *config.Config, logger zerolog.Logger,
+) config.ResolvedPerformanceSettings {
 	hwSurveyor := env.NewHardwareSurveyor()
 	hwInfo := hwSurveyor.Survey(ctx)
 	logging.Trace().Mark("hardware_survey")

M internal/infrastructure/webkit/webview.go +79 -2


@@ -28,6 +28,7 @@ import (
28	28		var _ port.WebView = (*WebView)(nil)
29	29		var _ port.DevToolsOpener = (*WebView)(nil)
30	30		var _ port.Printer = (*WebView)(nil)
	31	+	var _ port.OAuthCallbackCapable = (*WebView)(nil)
31	32
32	33		// WebViewID is an alias to port.WebViewID for clean architecture compliance.
33	34		// Infrastructure layer uses the type defined in the application port.

@@ -1477,6 +1478,30 @@ func (wv *WebView) Show() {
1477	1478		wv.inner.SetVisible(true)
1478	1479		}
1479	1480
	1481	+	// SetOnReadyToShow sets the callback invoked when the popup WebView is ready to display.
	1482	+	// It implements port.PopupCapable.
	1483	+	func (wv *WebView) SetOnReadyToShow(fn func()) {
	1484	+	wv.OnReadyToShow = fn
	1485	+	}
	1486	+
	1487	+	// SetOnClose composes fn with any existing OnClose handler so multiple callers
	1488	+	// can each register a close callback without overwriting one another.
	1489	+	// It implements port.PopupCapable.
	1490	+	func (wv *WebView) SetOnClose(fn func()) {
	1491	+	existing := wv.OnClose
	1492	+	if existing == nil {
	1493	+	wv.OnClose = fn
	1494	+	return
	1495	+	}
	1496	+	if fn == nil {
	1497	+	return
	1498	+	}
	1499	+	wv.OnClose = func() {
	1500	+	existing()
	1501	+	fn()
	1502	+	}
	1503	+	}
	1504	+
1480	1505		// State returns the current WebView state as a snapshot.
1481	1506		func (wv *WebView) State() port.WebViewState {
1482	1507		return port.WebViewState{

@@ -1584,12 +1609,16 @@ func (wv *WebView) Print() error {
1584	1609
1585	1610		// OpenDevTools implements port.DevToolsOpener.
1586	1611		func (wv *WebView) OpenDevTools() {
1587		-	_ = wv.ShowDevTools()
	1612	+	if err := wv.ShowDevTools(); err != nil {
	1613	+	wv.logger.Error().Err(err).Uint64("id", uint64(wv.id)).Msg("failed to open dev tools")
	1614	+	}
1588	1615		}
1589	1616
1590	1617		// PrintPage implements port.Printer.
1591	1618		func (wv *WebView) PrintPage() {
1592		-	_ = wv.Print()
	1619	+	if err := wv.Print(); err != nil {
	1620	+	wv.logger.Error().Err(err).Uint64("id", uint64(wv.id)).Msg("failed to open print dialog")
	1621	+	}
1593	1622		}
1594	1623
1595	1624		// IsDestroyed returns true if the WebView has been destroyed.

@@ -1618,6 +1647,54 @@ func (wv *WebView) Close() {
1618	1647		}
1619	1648		}
1620	1649
	1650	+	// AddCloseCallback implements port.OAuthCallbackCapable.
	1651	+	// It composes fn with any existing OnClose handler so multiple callers can register
	1652	+	// close callbacks without overwriting one another.
	1653	+	func (wv *WebView) AddCloseCallback(fn func()) {
	1654	+	if wv.OnClose == nil {
	1655	+	wv.OnClose = fn
	1656	+	return
	1657	+	}
	1658	+	existing := wv.OnClose
	1659	+	wv.OnClose = func() {
	1660	+	existing()
	1661	+	fn()
	1662	+	}
	1663	+	}
	1664	+
	1665	+	// AddNavigationCallback implements port.OAuthCallbackCapable.
	1666	+	// fn is invoked when the URI changes (OnURIChanged) and when a page load is committed
	1667	+	// (OnLoadChanged with LoadCommitted), covering both redirect-based and postMessage OAuth flows.
	1668	+	func (wv *WebView) AddNavigationCallback(fn func(uri string)) {
	1669	+	// Compose with existing OnURIChanged.
	1670	+	if wv.OnURIChanged == nil {
	1671	+	wv.OnURIChanged = fn
	1672	+	} else {
	1673	+	existing := wv.OnURIChanged
	1674	+	wv.OnURIChanged = func(uri string) {
	1675	+	existing(uri)
	1676	+	fn(uri)
	1677	+	}
	1678	+	}
	1679	+
	1680	+	// Compose with existing OnLoadChanged to also fire on committed loads.
	1681	+	if wv.OnLoadChanged == nil {
	1682	+	wv.OnLoadChanged = func(event LoadEvent) {
	1683	+	if event == LoadCommitted {
	1684	+	fn(wv.URI())
	1685	+	}
	1686	+	}
	1687	+	} else {
	1688	+	existing := wv.OnLoadChanged
	1689	+	wv.OnLoadChanged = func(event LoadEvent) {
	1690	+	existing(event)
	1691	+	if event == LoadCommitted {
	1692	+	fn(wv.URI())
	1693	+	}
	1694	+	}
	1695	+	}
	1696	+	}
	1697	+
1621	1698		// DisconnectSignals disconnects all GLib signal handlers from the WebView.
1622	1699		// This must be called before releasing the WebView to the pool or destroying it
1623	1700		// to prevent callbacks from firing on freed/pooled WebViews.


@@ -28,6 +28,7 @@ import (
28		var _ port.WebView = (*WebView)(nil)	28		var _ port.WebView = (*WebView)(nil)
29		var _ port.DevToolsOpener = (*WebView)(nil)	29		var _ port.DevToolsOpener = (*WebView)(nil)
30		var _ port.Printer = (*WebView)(nil)	30		var _ port.Printer = (*WebView)(nil)
			31	+	var _ port.OAuthCallbackCapable = (*WebView)(nil)
31			32
32		// WebViewID is an alias to port.WebViewID for clean architecture compliance.	33		// WebViewID is an alias to port.WebViewID for clean architecture compliance.
33		// Infrastructure layer uses the type defined in the application port.	34		// Infrastructure layer uses the type defined in the application port.

@@ -1477,6 +1478,30 @@ func (wv *WebView) Show() {
1477		wv.inner.SetVisible(true)	1478		wv.inner.SetVisible(true)
1478		}	1479		}
1479			1480
			1481	+	// SetOnReadyToShow sets the callback invoked when the popup WebView is ready to display.
			1482	+	// It implements port.PopupCapable.
			1483	+	func (wv *WebView) SetOnReadyToShow(fn func()) {
			1484	+	wv.OnReadyToShow = fn
			1485	+	}
			1486	+
			1487	+	// SetOnClose composes fn with any existing OnClose handler so multiple callers
			1488	+	// can each register a close callback without overwriting one another.
			1489	+	// It implements port.PopupCapable.
			1490	+	func (wv *WebView) SetOnClose(fn func()) {
			1491	+	existing := wv.OnClose
			1492	+	if existing == nil {
			1493	+	wv.OnClose = fn
			1494	+	return
			1495	+	}
			1496	+	if fn == nil {
			1497	+	return
			1498	+	}
			1499	+	wv.OnClose = func() {
			1500	+	existing()
			1501	+	fn()
			1502	+	}
			1503	+	}
			1504	+
1480		// State returns the current WebView state as a snapshot.	1505		// State returns the current WebView state as a snapshot.
1481		func (wv *WebView) State() port.WebViewState {	1506		func (wv *WebView) State() port.WebViewState {
1482		return port.WebViewState{	1507		return port.WebViewState{

@@ -1584,12 +1609,16 @@ func (wv *WebView) Print() error {
1584			1609
1585		// OpenDevTools implements port.DevToolsOpener.	1610		// OpenDevTools implements port.DevToolsOpener.
1586		func (wv *WebView) OpenDevTools() {	1611		func (wv *WebView) OpenDevTools() {
1587	-	_ = wv.ShowDevTools()	1612	+	if err := wv.ShowDevTools(); err != nil {
			1613	+	wv.logger.Error().Err(err).Uint64("id", uint64(wv.id)).Msg("failed to open dev tools")
			1614	+	}
1588		}	1615		}
1589			1616
1590		// PrintPage implements port.Printer.	1617		// PrintPage implements port.Printer.
1591		func (wv *WebView) PrintPage() {	1618		func (wv *WebView) PrintPage() {
1592	-	_ = wv.Print()	1619	+	if err := wv.Print(); err != nil {
			1620	+	wv.logger.Error().Err(err).Uint64("id", uint64(wv.id)).Msg("failed to open print dialog")
			1621	+	}
1593		}	1622		}
1594			1623
1595		// IsDestroyed returns true if the WebView has been destroyed.	1624		// IsDestroyed returns true if the WebView has been destroyed.

@@ -1618,6 +1647,54 @@ func (wv *WebView) Close() {
1618		}	1647		}
1619		}	1648		}
1620			1649
			1650	+	// AddCloseCallback implements port.OAuthCallbackCapable.
			1651	+	// It composes fn with any existing OnClose handler so multiple callers can register
			1652	+	// close callbacks without overwriting one another.
			1653	+	func (wv *WebView) AddCloseCallback(fn func()) {
			1654	+	if wv.OnClose == nil {
			1655	+	wv.OnClose = fn
			1656	+	return
			1657	+	}
			1658	+	existing := wv.OnClose
			1659	+	wv.OnClose = func() {
			1660	+	existing()
			1661	+	fn()
			1662	+	}
			1663	+	}
			1664	+
			1665	+	// AddNavigationCallback implements port.OAuthCallbackCapable.
			1666	+	// fn is invoked when the URI changes (OnURIChanged) and when a page load is committed
			1667	+	// (OnLoadChanged with LoadCommitted), covering both redirect-based and postMessage OAuth flows.
			1668	+	func (wv *WebView) AddNavigationCallback(fn func(uri string)) {
			1669	+	// Compose with existing OnURIChanged.
			1670	+	if wv.OnURIChanged == nil {
			1671	+	wv.OnURIChanged = fn
			1672	+	} else {
			1673	+	existing := wv.OnURIChanged
			1674	+	wv.OnURIChanged = func(uri string) {
			1675	+	existing(uri)
			1676	+	fn(uri)
			1677	+	}
			1678	+	}
			1679	+
			1680	+	// Compose with existing OnLoadChanged to also fire on committed loads.
			1681	+	if wv.OnLoadChanged == nil {
			1682	+	wv.OnLoadChanged = func(event LoadEvent) {
			1683	+	if event == LoadCommitted {
			1684	+	fn(wv.URI())
			1685	+	}
			1686	+	}
			1687	+	} else {
			1688	+	existing := wv.OnLoadChanged
			1689	+	wv.OnLoadChanged = func(event LoadEvent) {
			1690	+	existing(event)
			1691	+	if event == LoadCommitted {
			1692	+	fn(wv.URI())
			1693	+	}
			1694	+	}
			1695	+	}
			1696	+	}
			1697	+
1621		// DisconnectSignals disconnects all GLib signal handlers from the WebView.	1698		// DisconnectSignals disconnects all GLib signal handlers from the WebView.
1622		// This must be called before releasing the WebView to the pool or destroying it	1699		// This must be called before releasing the WebView to the pool or destroying it
1623		// to prevent callbacks from firing on freed/pooled WebViews.	1700		// to prevent callbacks from firing on freed/pooled WebViews.

M internal/ui/coordinator/content/appearance.go +9 -3


@@ -262,14 +262,20 @@ func (c *Coordinator) shouldDeferAppearance(wv port.WebView) bool {
262	262
263	263		func (c *Coordinator) refreshInjectedScripts(
264	264		ctx context.Context,
265		-	injector *webkit.ContentInjector,
	265	+	injector port.ContentInjector,
266	266		paneID entity.PaneID,
267	267		wv port.WebView,
268	268		) {
269	269		if injector == nil \|\| wv == nil \|\| wv.IsDestroyed() {
270	270		return
271	271		}
272		-	// Type-assert to access webkit-specific UserContentManager
	272	+	// Type-assert to access webkit-specific UserContentManager and InjectScripts.
	273	+	// refreshInjectedScripts is a webkit-specific operation; other engines will
	274	+	// need their own implementation when wired through a port.ScriptRefresher.
	275	+	wkInjector, ok := injector.(*webkit.ContentInjector)
	276	+	if !ok {
	277	+	return
	278	+	}
273	279		wkWV, ok := wv.(*webkit.WebView)
274	280		if !ok {
275	281		return

@@ -280,7 +286,7 @@ func (c *Coordinator) refreshInjectedScripts(
280	286		}
281	287		ucm.RemoveAllScripts()
282	288		ucm.RemoveAllStyleSheets()
283		-	injector.InjectScripts(ctx, ucm, wv.ID())
	289	+	wkInjector.InjectScripts(ctx, ucm, wv.ID())
284	290		logging.FromContext(ctx).Debug().Str("pane_id", string(paneID)).Msg("refreshed injected scripts for webview")
285	291		}
286	292

 func (c *Coordinator) refreshInjectedScripts(
 	ctx context.Context,
-	injector *webkit.ContentInjector,
+	injector port.ContentInjector,
 	paneID entity.PaneID,
 	wv port.WebView,
 ) {
 	if injector == nil || wv == nil || wv.IsDestroyed() {
 		return
 	}
-	// Type-assert to access webkit-specific UserContentManager
+	// Type-assert to access webkit-specific UserContentManager and InjectScripts.
+	// refreshInjectedScripts is a webkit-specific operation; other engines will
+	// need their own implementation when wired through a port.ScriptRefresher.
+	wkInjector, ok := injector.(*webkit.ContentInjector)
+	if !ok {
+		return
+	}
 	wkWV, ok := wv.(*webkit.WebView)
 	if !ok {
 		return
 	}
 	ucm.RemoveAllScripts()
 	ucm.RemoveAllStyleSheets()
-	injector.InjectScripts(ctx, ucm, wv.ID())
+	wkInjector.InjectScripts(ctx, ucm, wv.ID())
 	logging.FromContext(ctx).Debug().Str("pane_id", string(paneID)).Msg("refreshed injected scripts for webview")
 }

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


@@ -9,7 +9,6 @@ import (
9	9		"github.com/bnema/dumber/internal/application/usecase"
10	10		"github.com/bnema/dumber/internal/domain/entity"
11	11		"github.com/bnema/dumber/internal/infrastructure/config"
12		-	"github.com/bnema/dumber/internal/infrastructure/webkit"
13	12		"github.com/bnema/dumber/internal/logging"
14	13		"github.com/bnema/dumber/internal/ui/adapter"
15	14		"github.com/bnema/dumber/internal/ui/component"

@@ -24,7 +23,7 @@ type Coordinator struct {
24	23		faviconAdapter *adapter.FaviconAdapter
25	24		zoomUC *usecase.ManageZoomUseCase
26	25		permissionUC *usecase.HandlePermissionUseCase
27		-	injector *webkit.ContentInjector
	26	+	injector port.ContentInjector
28	27
29	28		webViews map[entity.PaneID]port.WebView
30	29		webViewsMu sync.RWMutex

@@ -289,13 +288,3 @@ func (c *Coordinator) deleteWebViewLocked(paneID entity.PaneID) port.WebView {
289	288		delete(c.webViews, paneID)
290	289		return wv
291	290		}
292		-
293		-	func (c *Coordinator) snapshotWebViews() map[entity.PaneID]port.WebView {
294		-	c.webViewsMu.RLock()
295		-	defer c.webViewsMu.RUnlock()
296		-	snapshot := make(map[entity.PaneID]port.WebView, len(c.webViews))
297		-	for paneID, wv := range c.webViews {
298		-	snapshot[paneID] = wv
299		-	}
300		-	return snapshot
301		-	}

 	"github.com/bnema/dumber/internal/application/usecase"
 	"github.com/bnema/dumber/internal/domain/entity"
 	"github.com/bnema/dumber/internal/infrastructure/config"
-	"github.com/bnema/dumber/internal/infrastructure/webkit"
 	"github.com/bnema/dumber/internal/logging"
 	"github.com/bnema/dumber/internal/ui/adapter"
 	"github.com/bnema/dumber/internal/ui/component"
 	faviconAdapter *adapter.FaviconAdapter
 	zoomUC         *usecase.ManageZoomUseCase
 	permissionUC   *usecase.HandlePermissionUseCase
-	injector       *webkit.ContentInjector
+	injector       port.ContentInjector
 	webViews   map[entity.PaneID]port.WebView
 	webViewsMu sync.RWMutex
 	delete(c.webViews, paneID)
 	return wv
 }
-func (c *Coordinator) snapshotWebViews() map[entity.PaneID]port.WebView {
-	c.webViewsMu.RLock()
-	defer c.webViewsMu.RUnlock()
-	snapshot := make(map[entity.PaneID]port.WebView, len(c.webViews))
-	for paneID, wv := range c.webViews {
-		snapshot[paneID] = wv
-	}
-	return snapshot
-}

M internal/ui/coordinator/content/navigation.go +4 -2


@@ -205,7 +205,7 @@ func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID,
205	205		paneView.SetLoading(false)
206	206		}
207	207
208		-	c.revealIfPending(context.Background(), paneID, "", "load-finished")
	208	+	c.revealIfPending(ctx, paneID, "", "load-finished")
209	209		if c.shouldSkipAboutBlankAppearance(paneID, wv) {
210	210		return
211	211		}

@@ -321,7 +321,9 @@ func (c *Coordinator) handleURIChanged(ctx context.Context, paneID entity.PaneID
321	321
322	322		// Stop loading to prevent WebKit from showing an error page
323	323		// The page stays on the previous URL before the JS redirect
324		-	_ = wv.Stop(ctx)
	324	+	if err := wv.Stop(ctx); err != nil {
	325	+	log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for external URL")
	326	+	}
325	327
326	328		// Navigate back to avoid stale URI in omnibox/history
327	329		if wv.CanGoBack() {

 		paneView.SetLoading(false)
 	}
-	c.revealIfPending(context.Background(), paneID, "", "load-finished")
+	c.revealIfPending(ctx, paneID, "", "load-finished")
 	if c.shouldSkipAboutBlankAppearance(paneID, wv) {
 		return
 	}
 		// Stop loading to prevent WebKit from showing an error page
 		// The page stays on the previous URL before the JS redirect
-		_ = wv.Stop(ctx)
+		if err := wv.Stop(ctx); err != nil {
+			log.Warn().Str("pane_id", string(paneID)).Str("uri", uri).Err(err).Msg("stop webview for external URL")
+		}
 		// Navigate back to avoid stale URI in omnibox/history
 		if wv.CanGoBack() {

M internal/ui/coordinator/content/oauth.go +19 -26


@@ -8,7 +8,6 @@ import (
8	8
9	9		"github.com/bnema/dumber/internal/application/port"
10	10		"github.com/bnema/dumber/internal/domain/entity"
11		-	"github.com/bnema/dumber/internal/infrastructure/webkit"
12	11		"github.com/bnema/dumber/internal/logging"
13	12		"github.com/jwijenbergh/puregotk/v4/glib"
14	13		)

@@ -60,6 +59,15 @@ var oauthRequestPatterns = []string{
60	59
61	60		// IsOAuthURL checks if the URL is related to an OAuth flow.
62	61		// This includes authorization endpoints, login pages, and callback URLs.
	62	+	//
	63	+	// The matching is intentionally broad: both oauthFlowPatterns (path-based terms like
	64	+	// "login", "authorize", "callback") and oauthRequestPatterns (query parameters like
	65	+	// "client_id=" and "scope=") use simple substring matching without anchoring.
	66	+	// This maximizes recall — we prefer false positives over missed detections — so that
	67	+	// edge-case provider URLs and non-standard redirect paths are still caught. The
	68	+	// trade-off is reduced precision: some unrelated URLs that happen to contain these
	69	+	// substrings will be classified as OAuth URLs. Callers that need higher confidence
	70	+	// should combine this with IsOAuthCallback, which checks for concrete response params.
63	71		func IsOAuthURL(url string) bool {
64	72		if url == "" {
65	73		return false

@@ -212,10 +220,10 @@ func (c *Coordinator) setupOAuthAutoClose(
212	220		) {
213	221		log := logging.FromContext(ctx)
214	222
215		-	// Type-assert to access webkit-specific direct field callbacks and Close()
216		-	wkWV, ok := wv.(*webkit.WebView)
	223	+	// Use the optional OAuthCallbackCapable capability instead of a webkit-specific assertion.
	224	+	oauthWV, ok := wv.(port.OAuthCallbackCapable)
217	225		if !ok {
218		-	log.Debug().Str("pane_id", string(paneID)).Msg("oauth auto-close: webview does not support webkit callbacks")
	226	+	log.Debug().Str("pane_id", string(paneID)).Msg("oauth auto-close: webview does not support oauth callbacks")
219	227		return
220	228		}
221	229

@@ -239,7 +247,7 @@ func (c *Coordinator) setupOAuthAutoClose(
239	247		uri := wv.URI()
240	248		if shouldForceCloseOnSafetyTimeout(uri) {
241	249		log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup")
242		-	wkWV.Close()
	250	+	oauthWV.Close()
243	251		return false
244	252		}
245	253		log.Debug().

@@ -275,7 +283,7 @@ func (c *Coordinator) setupOAuthAutoClose(
275	283		time.AfterFunc(oauthCloseDelay, func() {
276	284		cb := glib.SourceFunc(func(_ uintptr) bool {
277	285		if wv != nil && !wv.IsDestroyed() {
278		-	wkWV.Close()
	286	+	oauthWV.Close()
279	287		}
280	288		return false
281	289		})

@@ -287,32 +295,17 @@ func (c *Coordinator) setupOAuthAutoClose(
287	295		// Start safety timer immediately.
288	296		startSafetyTimer()
289	297
290		-	// Wrap OnURIChanged to check for OAuth callbacks.
291		-	wkWV.OnURIChanged = composeOnURIChanged(wkWV.OnURIChanged, func(uri string) {
	298	+	// Register navigation callback to check for OAuth callbacks on URI changes and committed loads.
	299	+	oauthWV.AddNavigationCallback(func(uri string) {
292	300		if ShouldAutoClose(uri) {
293		-	requestOAuthClose(uri, "uri_changed")
	301	+	requestOAuthClose(uri, "navigation")
294	302		}
295	303		})
296	304
297		-	// Monitor load events for URL-based detection.
298		-	// Wrap webkit.LoadEvent to port.LoadEvent at the boundary.
299		-	existingOnLoad := wkWV.OnLoadChanged
300		-	wkWV.OnLoadChanged = func(event webkit.LoadEvent) {
301		-	if existingOnLoad != nil {
302		-	existingOnLoad(event)
303		-	}
304		-	if port.LoadEvent(event) == port.LoadCommitted {
305		-	uri := wv.URI()
306		-	if ShouldAutoClose(uri) {
307		-	requestOAuthClose(uri, "load_committed")
308		-	}
309		-	}
310		-	}
311		-
312	305		// Cancel safety timer on any close path.
313		-	wkWV.OnClose = composeOnClose(func() {
	306	+	oauthWV.AddCloseCallback(func() {
314	307		cancelSafetyTimer()
315		-	}, wkWV.OnClose)
	308	+	})
316	309		}
317	310
318	311		func (c *Coordinator) trackOAuthPopup(popupID port.WebViewID, parentPaneID entity.PaneID) {

 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/domain/entity"
-	"github.com/bnema/dumber/internal/infrastructure/webkit"
 	"github.com/bnema/dumber/internal/logging"
 	"github.com/jwijenbergh/puregotk/v4/glib"
 )
 // IsOAuthURL checks if the URL is related to an OAuth flow.
 // This includes authorization endpoints, login pages, and callback URLs.
+//
+// The matching is intentionally broad: both oauthFlowPatterns (path-based terms like
+// "login", "authorize", "callback") and oauthRequestPatterns (query parameters like
+// "client_id=" and "scope=") use simple substring matching without anchoring.
+// This maximizes recall — we prefer false positives over missed detections — so that
+// edge-case provider URLs and non-standard redirect paths are still caught. The
+// trade-off is reduced precision: some unrelated URLs that happen to contain these
+// substrings will be classified as OAuth URLs. Callers that need higher confidence
+// should combine this with IsOAuthCallback, which checks for concrete response params.
 func IsOAuthURL(url string) bool {
 	if url == "" {
 		return false
 ) {
 	log := logging.FromContext(ctx)
-	// Type-assert to access webkit-specific direct field callbacks and Close()
+	// Use the optional OAuthCallbackCapable capability instead of a webkit-specific assertion.
-	wkWV, ok := wv.(*webkit.WebView)
+	oauthWV, ok := wv.(port.OAuthCallbackCapable)
 	if !ok {
-		log.Debug().Str("pane_id", string(paneID)).Msg("oauth auto-close: webview does not support webkit callbacks")
+		log.Debug().Str("pane_id", string(paneID)).Msg("oauth auto-close: webview does not support oauth callbacks")
 		return
 	}
 					uri := wv.URI()
 					if shouldForceCloseOnSafetyTimeout(uri) {
 						log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup")
-						wkWV.Close()
+						oauthWV.Close()
 						return false
 					}
 					log.Debug().
 			time.AfterFunc(oauthCloseDelay, func() {
 				cb := glib.SourceFunc(func(_ uintptr) bool {
 					if wv != nil && !wv.IsDestroyed() {
-						wkWV.Close()
+						oauthWV.Close()
 					}
 					return false
 				})
 	// Start safety timer immediately.
 	startSafetyTimer()
-	// Wrap OnURIChanged to check for OAuth callbacks.
+	// Register navigation callback to check for OAuth callbacks on URI changes and committed loads.
-	wkWV.OnURIChanged = composeOnURIChanged(wkWV.OnURIChanged, func(uri string) {
+	oauthWV.AddNavigationCallback(func(uri string) {
 		if ShouldAutoClose(uri) {
-			requestOAuthClose(uri, "uri_changed")
+			requestOAuthClose(uri, "navigation")
 		}
 	})
-	// Monitor load events for URL-based detection.
-	// Wrap webkit.LoadEvent to port.LoadEvent at the boundary.
-	existingOnLoad := wkWV.OnLoadChanged
-	wkWV.OnLoadChanged = func(event webkit.LoadEvent) {
-		if existingOnLoad != nil {
-			existingOnLoad(event)
-		}
-		if port.LoadEvent(event) == port.LoadCommitted {
-			uri := wv.URI()
-			if ShouldAutoClose(uri) {
-				requestOAuthClose(uri, "load_committed")
-			}
-		}
-	}
 	// Cancel safety timer on any close path.
-	wkWV.OnClose = composeOnClose(func() {
+	oauthWV.AddCloseCallback(func() {
 		cancelSafetyTimer()
-	}, wkWV.OnClose)
+	})
 }
 func (c *Coordinator) trackOAuthPopup(popupID port.WebViewID, parentPaneID entity.PaneID) {

M internal/ui/coordinator/content/popup.go +18 -11


@@ -8,7 +8,6 @@ import (
8	8		"github.com/bnema/dumber/internal/application/port"
9	9		"github.com/bnema/dumber/internal/domain/entity"
10	10		"github.com/bnema/dumber/internal/infrastructure/config"
11		-	"github.com/bnema/dumber/internal/infrastructure/webkit"
12	11		"github.com/bnema/dumber/internal/logging"
13	12		)
14	13

@@ -153,7 +152,9 @@ func (c *Coordinator) SetOnClosePane(fn func(ctx context.Context, paneID entity.
153	152
154	153		// buildPopupCreateHandler returns the OnCreate callback for a WebView.
155	154		// Returns nil if popup handling is not configured.
156		-	func (c *Coordinator) buildPopupCreateHandler(ctx context.Context, paneID entity.PaneID, wv port.WebView) func(port.PopupRequest) port.WebView {
	155	+	func (c *Coordinator) buildPopupCreateHandler(
	156	+	ctx context.Context, paneID entity.PaneID, wv port.WebView,
	157	+	) func(port.PopupRequest) port.WebView {
157	158		if wv == nil {
158	159		return nil
159	160		}

@@ -304,14 +305,16 @@ func (c *Coordinator) handlePopupCreate(
304	305		c.pendingPopups[popupID] = pending
305	306		c.popupMu.Unlock()
306	307
307		-	// Wire ready-to-show and close signals via webkit-specific fields
308		-	if wkWV, ok := popupWV.(*webkit.WebView); ok {
309		-	wkWV.OnReadyToShow = func() {
	308	+	// Wire ready-to-show and close signals via the PopupCapable interface.
	309	+	if pc, ok := popupWV.(port.PopupCapable); ok {
	310	+	pc.SetOnReadyToShow(func() {
310	311		c.handlePopupReadyToShow(ctx, popupID)
311		-	}
312		-	wkWV.OnClose = composeOnClose(wkWV.OnClose, func() {
	312	+	})
	313	+	pc.SetOnClose(func() {
313	314		c.handlePopupClose(ctx, popupID)
314	315		})
	316	+	} else {
	317	+	log.Warn().Uint64("popup_id", uint64(popupID)).Msg("webview does not support popup lifecycle callbacks (PopupCapable)")
315	318		}
316	319
317	320		log.Info().

@@ -350,8 +353,10 @@ func (c *Coordinator) handlePopupReadyToShow(ctx context.Context, popupID port.W
350	353
351	354		// Make the WebView visible now that it's ready
352	355		if pending.WebView != nil {
353		-	if wkWV, ok := pending.WebView.(*webkit.WebView); ok {
354		-	wkWV.Show()
	356	+	if pc, ok := pending.WebView.(port.PopupCapable); ok {
	357	+	pc.Show()
	358	+	} else {
	359	+	log.Warn().Uint64("popup_id", uint64(popupID)).Msg("webview does not support Show() (PopupCapable)")
355	360		}
356	361		}
357	362

@@ -384,12 +389,14 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView
384	389
385	390		// Find pane by WebView ID
386	391		var paneID entity.PaneID
	392	+	c.webViewsMu.RLock()
387	393		for pid, wv := range c.webViews {
388	394		if wv != nil && wv.ID() == popupID {
389	395		paneID = pid
390	396		break
391	397		}
392	398		}
	399	+	c.webViewsMu.RUnlock()
393	400
394	401		if paneID == "" {
395	402		c.handlePopupOAuthClose(ctx, popupID)

@@ -484,8 +491,8 @@ func (c *Coordinator) handleLinkMiddleClick(ctx context.Context, parentPaneID en
484	491
485	492		if err := c.onInsertPopup(ctx, popupInput); err != nil {
486	493		log.Error().Err(err).Msg("failed to insert middle-click pane into workspace")
487		-	// Clean up on failure
488		-	delete(c.webViews, paneID)
	494	+	// Clean up on failure — use the locking helper to avoid a data race.
	495	+	c.deleteWebViewLocked(paneID)
489	496		newWV.Destroy()
490	497		return false
491	498		}

 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/domain/entity"
 	"github.com/bnema/dumber/internal/infrastructure/config"
-	"github.com/bnema/dumber/internal/infrastructure/webkit"
 	"github.com/bnema/dumber/internal/logging"
 )
 // buildPopupCreateHandler returns the OnCreate callback for a WebView.
 // Returns nil if popup handling is not configured.
-func (c *Coordinator) buildPopupCreateHandler(ctx context.Context, paneID entity.PaneID, wv port.WebView) func(port.PopupRequest) port.WebView {
+func (c *Coordinator) buildPopupCreateHandler(
+	ctx context.Context, paneID entity.PaneID, wv port.WebView,
+) func(port.PopupRequest) port.WebView {
 	if wv == nil {
 		return nil
 	}
 	c.pendingPopups[popupID] = pending
 	c.popupMu.Unlock()
-	// Wire ready-to-show and close signals via webkit-specific fields
+	// Wire ready-to-show and close signals via the PopupCapable interface.
-	if wkWV, ok := popupWV.(*webkit.WebView); ok {
+	if pc, ok := popupWV.(port.PopupCapable); ok {
-		wkWV.OnReadyToShow = func() {
+		pc.SetOnReadyToShow(func() {
 			c.handlePopupReadyToShow(ctx, popupID)
-		}
+		})
-		wkWV.OnClose = composeOnClose(wkWV.OnClose, func() {
+		pc.SetOnClose(func() {
 			c.handlePopupClose(ctx, popupID)
 		})
+	} else {
+		log.Warn().Uint64("popup_id", uint64(popupID)).Msg("webview does not support popup lifecycle callbacks (PopupCapable)")
 	}
 	log.Info().
 	// Make the WebView visible now that it's ready
 	if pending.WebView != nil {
-		if wkWV, ok := pending.WebView.(*webkit.WebView); ok {
+		if pc, ok := pending.WebView.(port.PopupCapable); ok {
-			wkWV.Show()
+			pc.Show()
+		} else {
+			log.Warn().Uint64("popup_id", uint64(popupID)).Msg("webview does not support Show() (PopupCapable)")
 		}
 	}
 	// Find pane by WebView ID
 	var paneID entity.PaneID
+	c.webViewsMu.RLock()
 	for pid, wv := range c.webViews {
 		if wv != nil && wv.ID() == popupID {
 			paneID = pid
 			break
 		}
 	}
+	c.webViewsMu.RUnlock()
 	if paneID == "" {
 		c.handlePopupOAuthClose(ctx, popupID)
 		if err := c.onInsertPopup(ctx, popupInput); err != nil {
 			log.Error().Err(err).Msg("failed to insert middle-click pane into workspace")
-			// Clean up on failure
+			// Clean up on failure — use the locking helper to avoid a data race.
-			delete(c.webViews, paneID)
+			c.deleteWebViewLocked(paneID)
 			newWV.Destroy()
 			return false
 		}