fix: rebind promoted browser windows and roll back failed opens

commit: 7ef5ca76ba23a39e0ecd7a284b2d66a808aeb6f6
author: brice · 6mo ago · 2026-04-14 16:02 UTC
parent: f7d1190
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +107 -1

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


@@ -528,7 +528,7 @@ func (a *App) openFreshWindow(ctx context.Context, url string) error {
528	528		return nil
529	529		}
530	530
531		-	if a.tabCoord != nil && a.widgetFactory != nil {
	531	+	if a.tabCoord != nil {
532	532		a.tabCreationWindow = created
533	533		defer func() {
534	534		a.tabCreationWindow = nil

@@ -536,8 +536,25 @@ func (a *App) openFreshWindow(ctx context.Context, url string) error {
536	536
537	537		tab, err := a.tabCoord.Create(ctx, url)
538	538		if err != nil {
	539	+	a.removeBrowserWindow(created.id)
	540	+	if created.mainWindow != nil {
	541	+	created.mainWindow.Destroy()
	542	+	}
539	543		return err
540	544		}
	545	+	if a.workspaceViews[tab.ID] == nil {
	546	+	if a.tabs != nil {
	547	+	a.tabs.Remove(tab.ID)
	548	+	}
	549	+	if created.mainWindow != nil && created.mainWindow.TabBar() != nil {
	550	+	created.mainWindow.TabBar().RemoveTab(tab.ID)
	551	+	}
	552	+	a.removeBrowserWindow(created.id)
	553	+	if created.mainWindow != nil {
	554	+	created.mainWindow.Destroy()
	555	+	}
	556	+	return fmt.Errorf("workspace view not created for tab %s", tab.ID)
	557	+	}
541	558		a.setBrowserWindowForTab(tab.ID, created)
542	559		if created.mainWindow != nil {
543	560		created.mainWindow.Show()


@@ -528,7 +528,7 @@ func (a *App) openFreshWindow(ctx context.Context, url string) error {
528		return nil	528		return nil
529		}	529		}
530			530
531	-	if a.tabCoord != nil && a.widgetFactory != nil {	531	+	if a.tabCoord != nil {
532		a.tabCreationWindow = created	532		a.tabCreationWindow = created
533		defer func() {	533		defer func() {
534		a.tabCreationWindow = nil	534		a.tabCreationWindow = nil

@@ -536,8 +536,25 @@ func (a *App) openFreshWindow(ctx context.Context, url string) error {
536			536
537		tab, err := a.tabCoord.Create(ctx, url)	537		tab, err := a.tabCoord.Create(ctx, url)
538		if err != nil {	538		if err != nil {
			539	+	a.removeBrowserWindow(created.id)
			540	+	if created.mainWindow != nil {
			541	+	created.mainWindow.Destroy()
			542	+	}
539		return err	543		return err
540		}	544		}
			545	+	if a.workspaceViews[tab.ID] == nil {
			546	+	if a.tabs != nil {
			547	+	a.tabs.Remove(tab.ID)
			548	+	}
			549	+	if created.mainWindow != nil && created.mainWindow.TabBar() != nil {
			550	+	created.mainWindow.TabBar().RemoveTab(tab.ID)
			551	+	}
			552	+	a.removeBrowserWindow(created.id)
			553	+	if created.mainWindow != nil {
			554	+	created.mainWindow.Destroy()
			555	+	}
			556	+	return fmt.Errorf("workspace view not created for tab %s", tab.ID)
			557	+	}
541		a.setBrowserWindowForTab(tab.ID, created)	558		a.setBrowserWindowForTab(tab.ID, created)
542		if created.mainWindow != nil {	559		if created.mainWindow != nil {
543		created.mainWindow.Show()	560		created.mainWindow.Show()

M internal/ui/app_browser_launch_test.go +78 -0


@@ -9,6 +9,10 @@ import (
9	9		"github.com/bnema/dumber/internal/application/port"
10	10		"github.com/bnema/dumber/internal/application/usecase"
11	11		"github.com/bnema/dumber/internal/domain/entity"
	12	+	"github.com/bnema/dumber/internal/ui/coordinator"
	13	+	"github.com/bnema/dumber/internal/ui/layout"
	14	+	"github.com/bnema/dumber/internal/ui/window"
	15	+	"github.com/bnema/puregotk/v4/gtk"
12	16		)
13	17
14	18		type testBrowserLaunchRelay struct {

@@ -57,6 +61,29 @@ func windowForTabCount(t testing.T, app App) int {
57	61		return fv.Len()
58	62		}
59	63
	64	+	func tabCoordinatorMainWindowPtr(t testing.T, tc coordinator.TabCoordinator) uintptr {
	65	+	t.Helper()
	66	+
	67	+	rv := reflect.ValueOf(tc).Elem()
	68	+	fv := rv.FieldByName("mainWindow")
	69	+	if !fv.IsValid() {
	70	+	t.Fatalf("TabCoordinator missing mainWindow field")
	71	+	}
	72	+	return fv.Pointer()
	73	+	}
	74	+
	75	+	type fakeWidgetFactory struct{}
	76	+
	77	+	func (fakeWidgetFactory) NewPaned(layout.Orientation) layout.PanedWidget { return nil }
	78	+	func (fakeWidgetFactory) NewBox(layout.Orientation, int) layout.BoxWidget { return nil }
	79	+	func (fakeWidgetFactory) NewOverlay() layout.OverlayWidget { return nil }
	80	+	func (fakeWidgetFactory) NewLabel(string) layout.LabelWidget { return nil }
	81	+	func (fakeWidgetFactory) NewButton() layout.ButtonWidget { return nil }
	82	+	func (fakeWidgetFactory) NewImage() layout.ImageWidget { return nil }
	83	+	func (fakeWidgetFactory) NewProgressBar() layout.ProgressBarWidget { return nil }
	84	+	func (fakeWidgetFactory) NewSpinner() layout.SpinnerWidget { return nil }
	85	+	func (fakeWidgetFactory) WrapWidget(*gtk.Widget) layout.Widget { return nil }
	86	+
60	87		func TestApp_FinalizeActivationStartsBrowserLaunchRelayOnceAndClosesOnShutdown(t *testing.T) {
61	88		relay := &testBrowserLaunchRelay{closer: &testCloser{}}
62	89		deps := &Dependencies{}

@@ -97,3 +124,54 @@ func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) {
97	124		t.Fatalf("windowForTab length = %d, want 1", got)
98	125		}
99	126		}
	127	+
	128	+	func TestApp_RemoveBrowserWindowRebindsPromotedTabCoordinatorWindow(t *testing.T) {
	129	+	oldWindow := &browserWindow{id: "window-1", mainWindow: &window.MainWindow{}}
	130	+	newWindow := &browserWindow{id: "window-2", mainWindow: &window.MainWindow{}}
	131	+	tc := coordinator.NewTabCoordinator(context.Background(), coordinator.TabCoordinatorConfig{
	132	+	MainWindow: oldWindow.mainWindow,
	133	+	})
	134	+	app := &App{
	135	+	mainWindow: oldWindow.mainWindow,
	136	+	browserWindows: map[string]*browserWindow{oldWindow.id: oldWindow, newWindow.id: newWindow},
	137	+	tabCoord: tc,
	138	+	}
	139	+
	140	+	app.removeBrowserWindow(oldWindow.id)
	141	+
	142	+	if app.mainWindow != newWindow.mainWindow {
	143	+	t.Fatalf("mainWindow = %p, want %p", app.mainWindow, newWindow.mainWindow)
	144	+	}
	145	+	if got := tabCoordinatorMainWindowPtr(t, tc); got != reflect.ValueOf(newWindow.mainWindow).Pointer() {
	146	+	t.Fatalf("tab coordinator mainWindow = %x, want %x", got, reflect.ValueOf(newWindow.mainWindow).Pointer())
	147	+	}
	148	+	}
	149	+
	150	+	func TestApp_OpenFreshWindowRollsBackOnTabCreationFailure(t *testing.T) {
	151	+	created := &browserWindow{id: "window-1"}
	152	+	app := &App{
	153	+	tabs: entity.NewTabList(),
	154	+	tabsUC: usecase.NewManageTabsUseCase(func() string { return "id-1" }),
	155	+	browserWindows: make(map[string]*browserWindow),
	156	+	browserWindowFactory: func(context.Context, string) (*browserWindow, error) {
	157	+	return created, nil
	158	+	},
	159	+	tabCoord: coordinator.NewTabCoordinator(context.Background(), coordinator.TabCoordinatorConfig{
	160	+	TabsUC: usecase.NewManageTabsUseCase(func() string { return "id-2" }),
	161	+	Tabs: nil,
	162	+	MainWindow: &window.MainWindow{},
	163	+	}),
	164	+	widgetFactory: fakeWidgetFactory{},
	165	+	}
	166	+	app.tabs.Add(entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))))
	167	+
	168	+	if err := app.OpenFreshWindow(context.Background(), "https://example.com/fail"); err == nil {
	169	+	t.Fatalf("OpenFreshWindow = nil error, want failure")
	170	+	}
	171	+	if got := len(app.browserWindows); got != 0 {
	172	+	t.Fatalf("browserWindows length = %d, want 0", got)
	173	+	}
	174	+	if got := windowForTabCount(t, app); got != 0 {
	175	+	t.Fatalf("windowForTab length = %d, want 0", got)
	176	+	}
	177	+	}


@@ -9,6 +9,10 @@ import (
9	"github.com/bnema/dumber/internal/application/port"	9		"github.com/bnema/dumber/internal/application/port"
10	"github.com/bnema/dumber/internal/application/usecase"	10		"github.com/bnema/dumber/internal/application/usecase"
11	"github.com/bnema/dumber/internal/domain/entity"	11		"github.com/bnema/dumber/internal/domain/entity"
		12	+	"github.com/bnema/dumber/internal/ui/coordinator"
		13	+	"github.com/bnema/dumber/internal/ui/layout"
		14	+	"github.com/bnema/dumber/internal/ui/window"
		15	+	"github.com/bnema/puregotk/v4/gtk"
12	)	16		)
13		17
14	type testBrowserLaunchRelay struct {	18		type testBrowserLaunchRelay struct {

@@ -57,6 +61,29 @@ func windowForTabCount(t testing.T, app App) int {
57	return fv.Len()	61		return fv.Len()
58	}	62		}
59		63
		64	+	func tabCoordinatorMainWindowPtr(t testing.T, tc coordinator.TabCoordinator) uintptr {
		65	+	t.Helper()
		66	+
		67	+	rv := reflect.ValueOf(tc).Elem()
		68	+	fv := rv.FieldByName("mainWindow")
		69	+	if !fv.IsValid() {
		70	+	t.Fatalf("TabCoordinator missing mainWindow field")
		71	+	}
		72	+	return fv.Pointer()
		73	+	}
		74	+
		75	+	type fakeWidgetFactory struct{}
		76	+
		77	+	func (fakeWidgetFactory) NewPaned(layout.Orientation) layout.PanedWidget { return nil }
		78	+	func (fakeWidgetFactory) NewBox(layout.Orientation, int) layout.BoxWidget { return nil }
		79	+	func (fakeWidgetFactory) NewOverlay() layout.OverlayWidget { return nil }
		80	+	func (fakeWidgetFactory) NewLabel(string) layout.LabelWidget { return nil }
		81	+	func (fakeWidgetFactory) NewButton() layout.ButtonWidget { return nil }
		82	+	func (fakeWidgetFactory) NewImage() layout.ImageWidget { return nil }
		83	+	func (fakeWidgetFactory) NewProgressBar() layout.ProgressBarWidget { return nil }
		84	+	func (fakeWidgetFactory) NewSpinner() layout.SpinnerWidget { return nil }
		85	+	func (fakeWidgetFactory) WrapWidget(*gtk.Widget) layout.Widget { return nil }
		86	+
60	func TestApp_FinalizeActivationStartsBrowserLaunchRelayOnceAndClosesOnShutdown(t *testing.T) {	87		func TestApp_FinalizeActivationStartsBrowserLaunchRelayOnceAndClosesOnShutdown(t *testing.T) {
61	relay := &testBrowserLaunchRelay{closer: &testCloser{}}	88		relay := &testBrowserLaunchRelay{closer: &testCloser{}}
62	deps := &Dependencies{}	89		deps := &Dependencies{}

@@ -97,3 +124,54 @@ func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) {
97	t.Fatalf("windowForTab length = %d, want 1", got)	124		t.Fatalf("windowForTab length = %d, want 1", got)
98	}	125		}
99	}	126		}
		127	+
		128	+	func TestApp_RemoveBrowserWindowRebindsPromotedTabCoordinatorWindow(t *testing.T) {
		129	+	oldWindow := &browserWindow{id: "window-1", mainWindow: &window.MainWindow{}}
		130	+	newWindow := &browserWindow{id: "window-2", mainWindow: &window.MainWindow{}}
		131	+	tc := coordinator.NewTabCoordinator(context.Background(), coordinator.TabCoordinatorConfig{
		132	+	MainWindow: oldWindow.mainWindow,
		133	+	})
		134	+	app := &App{
		135	+	mainWindow: oldWindow.mainWindow,
		136	+	browserWindows: map[string]*browserWindow{oldWindow.id: oldWindow, newWindow.id: newWindow},
		137	+	tabCoord: tc,
		138	+	}
		139	+
		140	+	app.removeBrowserWindow(oldWindow.id)
		141	+
		142	+	if app.mainWindow != newWindow.mainWindow {
		143	+	t.Fatalf("mainWindow = %p, want %p", app.mainWindow, newWindow.mainWindow)
		144	+	}
		145	+	if got := tabCoordinatorMainWindowPtr(t, tc); got != reflect.ValueOf(newWindow.mainWindow).Pointer() {
		146	+	t.Fatalf("tab coordinator mainWindow = %x, want %x", got, reflect.ValueOf(newWindow.mainWindow).Pointer())
		147	+	}
		148	+	}
		149	+
		150	+	func TestApp_OpenFreshWindowRollsBackOnTabCreationFailure(t *testing.T) {
		151	+	created := &browserWindow{id: "window-1"}
		152	+	app := &App{
		153	+	tabs: entity.NewTabList(),
		154	+	tabsUC: usecase.NewManageTabsUseCase(func() string { return "id-1" }),
		155	+	browserWindows: make(map[string]*browserWindow),
		156	+	browserWindowFactory: func(context.Context, string) (*browserWindow, error) {
		157	+	return created, nil
		158	+	},
		159	+	tabCoord: coordinator.NewTabCoordinator(context.Background(), coordinator.TabCoordinatorConfig{
		160	+	TabsUC: usecase.NewManageTabsUseCase(func() string { return "id-2" }),
		161	+	Tabs: nil,
		162	+	MainWindow: &window.MainWindow{},
		163	+	}),
		164	+	widgetFactory: fakeWidgetFactory{},
		165	+	}
		166	+	app.tabs.Add(entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))))
		167	+
		168	+	if err := app.OpenFreshWindow(context.Background(), "https://example.com/fail"); err == nil {
		169	+	t.Fatalf("OpenFreshWindow = nil error, want failure")
		170	+	}
		171	+	if got := len(app.browserWindows); got != 0 {
		172	+	t.Fatalf("browserWindows length = %d, want 0", got)
		173	+	}
		174	+	if got := windowForTabCount(t, app); got != 0 {
		175	+	t.Fatalf("windowForTab length = %d, want 0", got)
		176	+	}
		177	+	}

M internal/ui/browser_window.go +6 -0


@@ -36,10 +36,16 @@ func (a *App) removeBrowserWindow(id string) {
36	36		for _, remaining := range a.browserWindows {
37	37		if remaining != nil && remaining.mainWindow != nil {
38	38		a.mainWindow = remaining.mainWindow
	39	+	if a.tabCoord != nil {
	40	+	a.tabCoord.SetMainWindow(remaining.mainWindow)
	41	+	}
39	42		return
40	43		}
41	44		}
42	45		a.mainWindow = nil
	46	+	if a.tabCoord != nil {
	47	+	a.tabCoord.SetMainWindow(nil)
	48	+	}
43	49		return
44	50		}
45	51		delete(a.browserWindows, id)


@@ -36,10 +36,16 @@ func (a *App) removeBrowserWindow(id string) {
36	for _, remaining := range a.browserWindows {	36		for _, remaining := range a.browserWindows {
37	if remaining != nil && remaining.mainWindow != nil {	37		if remaining != nil && remaining.mainWindow != nil {
38	a.mainWindow = remaining.mainWindow	38		a.mainWindow = remaining.mainWindow
		39	+	if a.tabCoord != nil {
		40	+	a.tabCoord.SetMainWindow(remaining.mainWindow)
		41	+	}
39	return	42		return
40	}	43		}
41	}	44		}
42	a.mainWindow = nil	45		a.mainWindow = nil
		46	+	if a.tabCoord != nil {
		47	+	a.tabCoord.SetMainWindow(nil)
		48	+	}
43	return	49		return
44	}	50		}
45	delete(a.browserWindows, id)	51		delete(a.browserWindows, id)

M internal/ui/coordinator/tab.go +5 -0


@@ -69,6 +69,11 @@ func (c *TabCoordinator) SetOnStateChanged(fn func()) {
69	69		c.onStateChanged = fn
70	70		}
71	71
	72	+	// SetMainWindow updates the window targeted by tab UI operations.
	73	+	func (c TabCoordinator) SetMainWindow(mainWindow window.MainWindow) {
	74	+	c.mainWindow = mainWindow
	75	+	}
	76	+
72	77		// notifyStateChanged triggers the state changed callback if set.
73	78		func (c *TabCoordinator) notifyStateChanged() {
74	79		if c.onStateChanged != nil {


@@ -69,6 +69,11 @@ func (c *TabCoordinator) SetOnStateChanged(fn func()) {
69	c.onStateChanged = fn	69		c.onStateChanged = fn
70	}	70		}
71		71
		72	+	// SetMainWindow updates the window targeted by tab UI operations.
		73	+	func (c TabCoordinator) SetMainWindow(mainWindow window.MainWindow) {
		74	+	c.mainWindow = mainWindow
		75	+	}
		76	+
72	// notifyStateChanged triggers the state changed callback if set.	77		// notifyStateChanged triggers the state changed callback if set.
73	func (c *TabCoordinator) notifyStateChanged() {	78		func (c *TabCoordinator) notifyStateChanged() {
74	if c.onStateChanged != nil {	79		if c.onStateChanged != nil {