fix(ui): release webviews when closing tabs
- commit
- 52415e57b4ba54830b5ce5f4e86214a40db1df0b
- author
- bnema · · 2026-06-17 20:02 UTC
- parent
- 90363e4
- signature
- unsigned
4 files · +237 -7
M internal/ui/app.go +3 -0
| @@ -3222,6 +3222,9 @@ func (a *App) initTabCoordinator(ctx context.Context) { | |||
| 3222 | 3222 | } | |
| 3223 | 3223 | a.switchWorkspaceView(ctx, tab.ID) | |
| 3224 | 3224 | }) | |
| 3225 | + | a.tabCoord.SetOnTabClosed(func(ctx context.Context, _ coordinator.TabTarget, tab *entity.Tab) { | |
| 3226 | + | a.releaseTabWorkspace(ctx, tab) | |
| 3227 | + | }) | |
| 3225 | 3228 | a.tabCoord.SetOnStateChanged(a.MarkDirty) | |
| 3226 | 3229 | // Per-window tab ownership is now enforced through explicit TabTarget. | |
| 3227 | 3230 | // The per-window TabList (bw.tabs) is the source of truth; no scope filtering needed. | |
| @@ -3222,6 +3222,9 @@ func (a *App) initTabCoordinator(ctx context.Context) { | |||||
| 3222 | } | 3222 | } | ||
| 3223 | a.switchWorkspaceView(ctx, tab.ID) | 3223 | a.switchWorkspaceView(ctx, tab.ID) | ||
| 3224 | }) | 3224 | }) | ||
| 3225 | + | a.tabCoord.SetOnTabClosed(func(ctx context.Context, _ coordinator.TabTarget, tab *entity.Tab) { | |||
| 3226 | + | a.releaseTabWorkspace(ctx, tab) | |||
| 3227 | + | }) | |||
| 3225 | a.tabCoord.SetOnStateChanged(a.MarkDirty) | 3228 | a.tabCoord.SetOnStateChanged(a.MarkDirty) | ||
| 3226 | // Per-window tab ownership is now enforced through explicit TabTarget. | 3229 | // Per-window tab ownership is now enforced through explicit TabTarget. | ||
| 3227 | // The per-window TabList (bw.tabs) is the source of truth; no scope filtering needed. | 3230 | // The per-window TabList (bw.tabs) is the source of truth; no scope filtering needed. | ||
M internal/ui/browser_window.go +55 -7
| @@ -296,6 +296,30 @@ func (a *App) registerBrowserWindow(bw *browserWindow) { | |||
| 296 | 296 | a.browserWindowOrder = append(a.browserWindowOrder, bw.id) | |
| 297 | 297 | } | |
| 298 | 298 | ||
| 299 | + | func (a *App) releaseTabWorkspace(ctx context.Context, tab *entity.Tab) { | |
| 300 | + | if tab == nil { | |
| 301 | + | return | |
| 302 | + | } | |
| 303 | + | if ctx == nil { | |
| 304 | + | ctx = context.Background() | |
| 305 | + | } | |
| 306 | + | ||
| 307 | + | a.releaseFloatingSessionsForTab(ctx, tab.ID) | |
| 308 | + | if a.contentCoord != nil && tab.Workspace != nil { | |
| 309 | + | for _, pane := range tab.Workspace.AllPanes() { | |
| 310 | + | if pane == nil { | |
| 311 | + | continue | |
| 312 | + | } | |
| 313 | + | a.contentCoord.ReleaseWebView(ctx, pane.ID) | |
| 314 | + | } | |
| 315 | + | } | |
| 316 | + | delete(a.workspaceViews, tab.ID) | |
| 317 | + | delete(a.windowForTab, tab.ID) | |
| 318 | + | if a.tabs != nil && a.tabs.Find(tab.ID) != nil { | |
| 319 | + | a.tabs.Remove(tab.ID) | |
| 320 | + | } | |
| 321 | + | } | |
| 322 | + | ||
| 299 | 323 | func (a *App) removeBrowserWindow(id string) { | |
| 300 | 324 | if id == "" || a.browserWindows == nil { | |
| 301 | 325 | return | |
| @@ -310,18 +334,42 @@ func (a *App) removeBrowserWindow(id string) { | |||
| 310 | 334 | } | |
| 311 | 335 | } | |
| 312 | 336 | wasMainWindow := removed != nil && removed.mainWindow == a.mainWindow | |
| 313 | - | ownedTabIDs := make([]entity.TabID, 0) | |
| 337 | + | ownedTabs := make(map[entity.TabID]*entity.Tab) | |
| 338 | + | if removed != nil && removed.tabs != nil { | |
| 339 | + | for _, tab := range removed.tabs.Tabs { | |
| 340 | + | if tab != nil { | |
| 341 | + | ownedTabs[tab.ID] = tab | |
| 342 | + | } | |
| 343 | + | } | |
| 344 | + | } | |
| 314 | 345 | for tabID, bw := range a.windowForTab { | |
| 315 | 346 | if bw != nil && bw.id == id { | |
| 316 | - | ownedTabIDs = append(ownedTabIDs, tabID) | |
| 347 | + | if bw.tabs != nil { | |
| 348 | + | if tab := bw.tabs.Find(tabID); tab != nil { | |
| 349 | + | ownedTabs[tabID] = tab | |
| 350 | + | continue | |
| 351 | + | } | |
| 352 | + | } | |
| 353 | + | if a.tabs != nil { | |
| 354 | + | ownedTabs[tabID] = a.tabs.Find(tabID) | |
| 355 | + | } | |
| 317 | 356 | } | |
| 318 | 357 | } | |
| 358 | + | ownedTabIDs := make([]entity.TabID, 0, len(ownedTabs)) | |
| 359 | + | for tabID := range ownedTabs { | |
| 360 | + | ownedTabIDs = append(ownedTabIDs, tabID) | |
| 361 | + | } | |
| 362 | + | sort.Slice(ownedTabIDs, func(i, j int) bool { return ownedTabIDs[i] < ownedTabIDs[j] }) | |
| 319 | 363 | for _, tabID := range ownedTabIDs { | |
| 320 | - | a.releaseFloatingSessionsForTab(context.Background(), tabID) | |
| 321 | - | delete(a.workspaceViews, tabID) | |
| 322 | - | delete(a.windowForTab, tabID) | |
| 323 | - | if a.tabs != nil && a.tabs.Find(tabID) != nil { | |
| 324 | - | a.tabs.Remove(tabID) | |
| 364 | + | a.releaseTabWorkspace(context.Background(), ownedTabs[tabID]) | |
| 365 | + | } | |
| 366 | + | for tabID, bw := range a.windowForTab { | |
| 367 | + | if bw != nil && bw.id == id { | |
| 368 | + | delete(a.workspaceViews, tabID) | |
| 369 | + | delete(a.windowForTab, tabID) | |
| 370 | + | if a.tabs != nil && a.tabs.Find(tabID) != nil { | |
| 371 | + | a.tabs.Remove(tabID) | |
| 372 | + | } | |
| 325 | 373 | } | |
| 326 | 374 | } | |
| 327 | 375 | if removed != nil { | |
| @@ -296,6 +296,30 @@ func (a *App) registerBrowserWindow(bw *browserWindow) { | |||||
| 296 | a.browserWindowOrder = append(a.browserWindowOrder, bw.id) | 296 | a.browserWindowOrder = append(a.browserWindowOrder, bw.id) | ||
| 297 | } | 297 | } | ||
| 298 | 298 | ||||
| 299 | + | func (a *App) releaseTabWorkspace(ctx context.Context, tab *entity.Tab) { | |||
| 300 | + | if tab == nil { | |||
| 301 | + | return | |||
| 302 | + | } | |||
| 303 | + | if ctx == nil { | |||
| 304 | + | ctx = context.Background() | |||
| 305 | + | } | |||
| 306 | + | ||||
| 307 | + | a.releaseFloatingSessionsForTab(ctx, tab.ID) | |||
| 308 | + | if a.contentCoord != nil && tab.Workspace != nil { | |||
| 309 | + | for _, pane := range tab.Workspace.AllPanes() { | |||
| 310 | + | if pane == nil { | |||
| 311 | + | continue | |||
| 312 | + | } | |||
| 313 | + | a.contentCoord.ReleaseWebView(ctx, pane.ID) | |||
| 314 | + | } | |||
| 315 | + | } | |||
| 316 | + | delete(a.workspaceViews, tab.ID) | |||
| 317 | + | delete(a.windowForTab, tab.ID) | |||
| 318 | + | if a.tabs != nil && a.tabs.Find(tab.ID) != nil { | |||
| 319 | + | a.tabs.Remove(tab.ID) | |||
| 320 | + | } | |||
| 321 | + | } | |||
| 322 | + | ||||
| 299 | func (a *App) removeBrowserWindow(id string) { | 323 | func (a *App) removeBrowserWindow(id string) { | ||
| 300 | if id == "" || a.browserWindows == nil { | 324 | if id == "" || a.browserWindows == nil { | ||
| 301 | return | 325 | return | ||
| @@ -310,18 +334,42 @@ func (a *App) removeBrowserWindow(id string) { | |||||
| 310 | } | 334 | } | ||
| 311 | } | 335 | } | ||
| 312 | wasMainWindow := removed != nil && removed.mainWindow == a.mainWindow | 336 | wasMainWindow := removed != nil && removed.mainWindow == a.mainWindow | ||
| 313 | - | ownedTabIDs := make([]entity.TabID, 0) | 337 | + | ownedTabs := make(map[entity.TabID]*entity.Tab) |
| 338 | + | if removed != nil && removed.tabs != nil { | |||
| 339 | + | for _, tab := range removed.tabs.Tabs { | |||
| 340 | + | if tab != nil { | |||
| 341 | + | ownedTabs[tab.ID] = tab | |||
| 342 | + | } | |||
| 343 | + | } | |||
| 344 | + | } | |||
| 314 | for tabID, bw := range a.windowForTab { | 345 | for tabID, bw := range a.windowForTab { | ||
| 315 | if bw != nil && bw.id == id { | 346 | if bw != nil && bw.id == id { | ||
| 316 | - | ownedTabIDs = append(ownedTabIDs, tabID) | 347 | + | if bw.tabs != nil { |
| 348 | + | if tab := bw.tabs.Find(tabID); tab != nil { | |||
| 349 | + | ownedTabs[tabID] = tab | |||
| 350 | + | continue | |||
| 351 | + | } | |||
| 352 | + | } | |||
| 353 | + | if a.tabs != nil { | |||
| 354 | + | ownedTabs[tabID] = a.tabs.Find(tabID) | |||
| 355 | + | } | |||
| 317 | } | 356 | } | ||
| 318 | } | 357 | } | ||
| 358 | + | ownedTabIDs := make([]entity.TabID, 0, len(ownedTabs)) | |||
| 359 | + | for tabID := range ownedTabs { | |||
| 360 | + | ownedTabIDs = append(ownedTabIDs, tabID) | |||
| 361 | + | } | |||
| 362 | + | sort.Slice(ownedTabIDs, func(i, j int) bool { return ownedTabIDs[i] < ownedTabIDs[j] }) | |||
| 319 | for _, tabID := range ownedTabIDs { | 363 | for _, tabID := range ownedTabIDs { | ||
| 320 | - | a.releaseFloatingSessionsForTab(context.Background(), tabID) | 364 | + | a.releaseTabWorkspace(context.Background(), ownedTabs[tabID]) |
| 321 | - | delete(a.workspaceViews, tabID) | 365 | + | } |
| 322 | - | delete(a.windowForTab, tabID) | 366 | + | for tabID, bw := range a.windowForTab { |
| 323 | - | if a.tabs != nil && a.tabs.Find(tabID) != nil { | 367 | + | if bw != nil && bw.id == id { |
| 324 | - | a.tabs.Remove(tabID) | 368 | + | delete(a.workspaceViews, tabID) |
| 369 | + | delete(a.windowForTab, tabID) | |||
| 370 | + | if a.tabs != nil && a.tabs.Find(tabID) != nil { | |||
| 371 | + | a.tabs.Remove(tabID) | |||
| 372 | + | } | |||
| 325 | } | 373 | } | ||
| 326 | } | 374 | } | ||
| 327 | if removed != nil { | 375 | if removed != nil { | ||
M internal/ui/browser_window_test.go +168 -0
| @@ -8,9 +8,12 @@ import ( | |||
| 8 | 8 | "unsafe" | |
| 9 | 9 | ||
| 10 | 10 | "github.com/bnema/dumber/internal/application/port" | |
| 11 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 11 | 12 | "github.com/bnema/dumber/internal/domain/entity" | |
| 13 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 12 | 14 | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 13 | 15 | "github.com/bnema/dumber/internal/ui/component" | |
| 16 | + | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 14 | 17 | "github.com/bnema/dumber/internal/ui/focus" | |
| 15 | 18 | "github.com/bnema/dumber/internal/ui/input" | |
| 16 | 19 | "github.com/bnema/dumber/internal/ui/layout" | |
| @@ -103,6 +106,171 @@ func TestApp_CleanupCreatedBrowserWindowsDetachesUnregisteredWindowBeforeDestroy | |||
| 103 | 106 | } | |
| 104 | 107 | } | |
| 105 | 108 | ||
| 109 | + | type recordingWebViewPool struct { | |
| 110 | + | released []port.WebView | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | func (p *recordingWebViewPool) Acquire(context.Context) (port.WebView, error) { | |
| 114 | + | return nil, errors.New("not implemented") | |
| 115 | + | } | |
| 116 | + | func (p *recordingWebViewPool) Release(wv port.WebView) { p.released = append(p.released, wv) } | |
| 117 | + | func (p *recordingWebViewPool) Prewarm(int) {} | |
| 118 | + | func (p *recordingWebViewPool) PrewarmAsync(context.Context, int) {} | |
| 119 | + | func (p *recordingWebViewPool) Size() int { return 0 } | |
| 120 | + | func (p *recordingWebViewPool) Close() {} | |
| 121 | + | ||
| 122 | + | func TestTabCoordinator_CloseReleasesClosedTabWorkspaceWebViews(t *testing.T) { | |
| 123 | + | ctx := context.Background() | |
| 124 | + | closedTab := entity.NewTab(entity.TabID("tab-x"), entity.WorkspaceID("workspace-x"), entity.NewPane(entity.PaneID("pane-x"))) | |
| 125 | + | survivingTab := entity.NewTab(entity.TabID("tab-survivor"), entity.WorkspaceID("workspace-survivor"), entity.NewPane(entity.PaneID("pane-survivor"))) | |
| 126 | + | tabs := entity.NewTabList() | |
| 127 | + | tabs.Add(closedTab) | |
| 128 | + | tabs.Add(survivingTab) | |
| 129 | + | tabs.SetActive(closedTab.ID) | |
| 130 | + | ||
| 131 | + | pool := &recordingWebViewPool{} | |
| 132 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |
| 133 | + | closedWV := &fakeRecordingWebView{id: 101} | |
| 134 | + | survivingWV := &fakeRecordingWebView{id: 102} | |
| 135 | + | contentCoord.RegisterPopupWebView(closedTab.Workspace.ActivePaneID, closedWV) | |
| 136 | + | contentCoord.RegisterPopupWebView(survivingTab.Workspace.ActivePaneID, survivingWV) | |
| 137 | + | ||
| 138 | + | bw := &browserWindow{id: "window-1", tabs: tabs} | |
| 139 | + | app := &App{ | |
| 140 | + | deps: &Dependencies{Config: config.DefaultConfig()}, | |
| 141 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 142 | + | contentCoord: contentCoord, | |
| 143 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |
| 144 | + | tabs: entity.NewTabList(), | |
| 145 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{closedTab.ID: {}, survivingTab.ID: {}}, | |
| 146 | + | windowForTab: map[entity.TabID]*browserWindow{closedTab.ID: bw, survivingTab.ID: bw}, | |
| 147 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |
| 148 | + | } | |
| 149 | + | app.tabs.Add(closedTab) | |
| 150 | + | app.tabs.Add(survivingTab) | |
| 151 | + | app.initTabCoordinator(ctx) | |
| 152 | + | ||
| 153 | + | require.NoError(t, app.tabCoord.Close(ctx, app.ensureTabTargetForBrowserWindow(bw))) | |
| 154 | + | ||
| 155 | + | require.Len(t, pool.released, 1) | |
| 156 | + | assert.Same(t, closedWV, pool.released[0]) | |
| 157 | + | assert.Nil(t, contentCoord.GetWebView(closedTab.Workspace.ActivePaneID)) | |
| 158 | + | assert.Same(t, survivingWV, contentCoord.GetWebView(survivingTab.Workspace.ActivePaneID)) | |
| 159 | + | assert.Nil(t, app.workspaceViews[closedTab.ID]) | |
| 160 | + | assert.Nil(t, app.windowForTab[closedTab.ID]) | |
| 161 | + | assert.Nil(t, app.tabs.Find(closedTab.ID)) | |
| 162 | + | assert.Equal(t, survivingTab.ID, tabs.ActiveTabID) | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | func TestTabCoordinator_SwitchDoesNotReleaseWorkspaceWebViews(t *testing.T) { | |
| 166 | + | ctx := context.Background() | |
| 167 | + | firstTab := entity.NewTab(entity.TabID("tab-first"), entity.WorkspaceID("workspace-first"), entity.NewPane(entity.PaneID("pane-first"))) | |
| 168 | + | secondTab := entity.NewTab(entity.TabID("tab-second"), entity.WorkspaceID("workspace-second"), entity.NewPane(entity.PaneID("pane-second"))) | |
| 169 | + | tabs := entity.NewTabList() | |
| 170 | + | tabs.Add(firstTab) | |
| 171 | + | tabs.Add(secondTab) | |
| 172 | + | tabs.SetActive(firstTab.ID) | |
| 173 | + | ||
| 174 | + | pool := &recordingWebViewPool{} | |
| 175 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |
| 176 | + | firstWV := &fakeRecordingWebView{id: 111} | |
| 177 | + | secondWV := &fakeRecordingWebView{id: 112} | |
| 178 | + | contentCoord.RegisterPopupWebView(firstTab.Workspace.ActivePaneID, firstWV) | |
| 179 | + | contentCoord.RegisterPopupWebView(secondTab.Workspace.ActivePaneID, secondWV) | |
| 180 | + | ||
| 181 | + | bw := &browserWindow{id: "window-1", tabs: tabs} | |
| 182 | + | app := &App{ | |
| 183 | + | deps: &Dependencies{Config: config.DefaultConfig()}, | |
| 184 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 185 | + | contentCoord: contentCoord, | |
| 186 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |
| 187 | + | tabs: entity.NewTabList(), | |
| 188 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{firstTab.ID: {}, secondTab.ID: {}}, | |
| 189 | + | windowForTab: map[entity.TabID]*browserWindow{firstTab.ID: bw, secondTab.ID: bw}, | |
| 190 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |
| 191 | + | } | |
| 192 | + | app.tabs.Add(firstTab) | |
| 193 | + | app.tabs.Add(secondTab) | |
| 194 | + | app.initTabCoordinator(ctx) | |
| 195 | + | ||
| 196 | + | require.NoError(t, app.tabCoord.Switch(ctx, app.ensureTabTargetForBrowserWindow(bw), secondTab.ID)) | |
| 197 | + | ||
| 198 | + | assert.Empty(t, pool.released) | |
| 199 | + | assert.Same(t, firstWV, contentCoord.GetWebView(firstTab.Workspace.ActivePaneID)) | |
| 200 | + | assert.Same(t, secondWV, contentCoord.GetWebView(secondTab.Workspace.ActivePaneID)) | |
| 201 | + | } | |
| 202 | + | ||
| 203 | + | func TestTabCoordinator_CloseLastTabReleasesWorkspaceBeforeWindowRemoval(t *testing.T) { | |
| 204 | + | ctx := context.Background() | |
| 205 | + | closedTab := entity.NewTab(entity.TabID("tab-only"), entity.WorkspaceID("workspace-only"), entity.NewPane(entity.PaneID("pane-only"))) | |
| 206 | + | tabs := entity.NewTabList() | |
| 207 | + | tabs.Add(closedTab) | |
| 208 | + | tabs.SetActive(closedTab.ID) | |
| 209 | + | ||
| 210 | + | pool := &recordingWebViewPool{} | |
| 211 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |
| 212 | + | closedWV := &fakeRecordingWebView{id: 121} | |
| 213 | + | contentCoord.RegisterPopupWebView(closedTab.Workspace.ActivePaneID, closedWV) | |
| 214 | + | ||
| 215 | + | bw := &browserWindow{id: "window-1", tabs: tabs} | |
| 216 | + | app := &App{ | |
| 217 | + | deps: &Dependencies{Config: config.DefaultConfig()}, | |
| 218 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |
| 219 | + | contentCoord: contentCoord, | |
| 220 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |
| 221 | + | tabs: entity.NewTabList(), | |
| 222 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{closedTab.ID: {}}, | |
| 223 | + | windowForTab: map[entity.TabID]*browserWindow{closedTab.ID: bw}, | |
| 224 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |
| 225 | + | } | |
| 226 | + | app.tabs.Add(closedTab) | |
| 227 | + | app.initTabCoordinator(ctx) | |
| 228 | + | ||
| 229 | + | require.NoError(t, app.tabCoord.Close(ctx, app.ensureTabTargetForBrowserWindow(bw))) | |
| 230 | + | ||
| 231 | + | require.Len(t, pool.released, 1) | |
| 232 | + | assert.Same(t, closedWV, pool.released[0]) | |
| 233 | + | assert.Nil(t, contentCoord.GetWebView(closedTab.Workspace.ActivePaneID)) | |
| 234 | + | assert.Empty(t, app.browserWindows) | |
| 235 | + | assert.Nil(t, app.workspaceViews[closedTab.ID]) | |
| 236 | + | assert.Nil(t, app.windowForTab[closedTab.ID]) | |
| 237 | + | } | |
| 238 | + | ||
| 239 | + | func TestBrowserWindow_RemoveBrowserWindowReleasesOwnedTabWorkspaceWebViews(t *testing.T) { | |
| 240 | + | ctx := context.Background() | |
| 241 | + | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | |
| 242 | + | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | |
| 243 | + | removed := &browserWindow{id: "window-1", tabs: entity.NewTabList()} | |
| 244 | + | remaining := &browserWindow{id: "window-2", tabs: entity.NewTabList()} | |
| 245 | + | removed.tabs.Add(ownedTab) | |
| 246 | + | remaining.tabs.Add(otherTab) | |
| 247 | + | ||
| 248 | + | pool := &recordingWebViewPool{} | |
| 249 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |
| 250 | + | ownedWV := &fakeRecordingWebView{id: 201} | |
| 251 | + | otherWV := &fakeRecordingWebView{id: 202} | |
| 252 | + | contentCoord.RegisterPopupWebView(ownedTab.Workspace.ActivePaneID, ownedWV) | |
| 253 | + | contentCoord.RegisterPopupWebView(otherTab.Workspace.ActivePaneID, otherWV) | |
| 254 | + | ||
| 255 | + | app := &App{ | |
| 256 | + | contentCoord: contentCoord, | |
| 257 | + | browserWindows: map[string]*browserWindow{removed.id: removed, remaining.id: remaining}, | |
| 258 | + | tabs: entity.NewTabList(), | |
| 259 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ownedTab.ID: {}, otherTab.ID: {}}, | |
| 260 | + | windowForTab: map[entity.TabID]*browserWindow{ownedTab.ID: removed, otherTab.ID: remaining}, | |
| 261 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |
| 262 | + | } | |
| 263 | + | app.tabs.Add(ownedTab) | |
| 264 | + | app.tabs.Add(otherTab) | |
| 265 | + | ||
| 266 | + | app.removeBrowserWindow(removed.id) | |
| 267 | + | ||
| 268 | + | require.Len(t, pool.released, 1) | |
| 269 | + | assert.Same(t, ownedWV, pool.released[0]) | |
| 270 | + | assert.Nil(t, contentCoord.GetWebView(ownedTab.Workspace.ActivePaneID)) | |
| 271 | + | assert.Same(t, otherWV, contentCoord.GetWebView(otherTab.Workspace.ActivePaneID)) | |
| 272 | + | } | |
| 273 | + | ||
| 106 | 274 | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | |
| 107 | 275 | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | |
| 108 | 276 | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | |
| @@ -8,9 +8,12 @@ import ( | |||||
| 8 | "unsafe" | 8 | "unsafe" | ||
| 9 | 9 | ||||
| 10 | "github.com/bnema/dumber/internal/application/port" | 10 | "github.com/bnema/dumber/internal/application/port" | ||
| 11 | + | "github.com/bnema/dumber/internal/application/usecase" | |||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 12 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 13 | + | "github.com/bnema/dumber/internal/infrastructure/config" | |||
| 12 | "github.com/bnema/dumber/internal/shared/syncdispatch" | 14 | "github.com/bnema/dumber/internal/shared/syncdispatch" | ||
| 13 | "github.com/bnema/dumber/internal/ui/component" | 15 | "github.com/bnema/dumber/internal/ui/component" | ||
| 16 | + | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | |||
| 14 | "github.com/bnema/dumber/internal/ui/focus" | 17 | "github.com/bnema/dumber/internal/ui/focus" | ||
| 15 | "github.com/bnema/dumber/internal/ui/input" | 18 | "github.com/bnema/dumber/internal/ui/input" | ||
| 16 | "github.com/bnema/dumber/internal/ui/layout" | 19 | "github.com/bnema/dumber/internal/ui/layout" | ||
| @@ -103,6 +106,171 @@ func TestApp_CleanupCreatedBrowserWindowsDetachesUnregisteredWindowBeforeDestroy | |||||
| 103 | } | 106 | } | ||
| 104 | } | 107 | } | ||
| 105 | 108 | ||||
| 109 | + | type recordingWebViewPool struct { | |||
| 110 | + | released []port.WebView | |||
| 111 | + | } | |||
| 112 | + | ||||
| 113 | + | func (p *recordingWebViewPool) Acquire(context.Context) (port.WebView, error) { | |||
| 114 | + | return nil, errors.New("not implemented") | |||
| 115 | + | } | |||
| 116 | + | func (p *recordingWebViewPool) Release(wv port.WebView) { p.released = append(p.released, wv) } | |||
| 117 | + | func (p *recordingWebViewPool) Prewarm(int) {} | |||
| 118 | + | func (p *recordingWebViewPool) PrewarmAsync(context.Context, int) {} | |||
| 119 | + | func (p *recordingWebViewPool) Size() int { return 0 } | |||
| 120 | + | func (p *recordingWebViewPool) Close() {} | |||
| 121 | + | ||||
| 122 | + | func TestTabCoordinator_CloseReleasesClosedTabWorkspaceWebViews(t *testing.T) { | |||
| 123 | + | ctx := context.Background() | |||
| 124 | + | closedTab := entity.NewTab(entity.TabID("tab-x"), entity.WorkspaceID("workspace-x"), entity.NewPane(entity.PaneID("pane-x"))) | |||
| 125 | + | survivingTab := entity.NewTab(entity.TabID("tab-survivor"), entity.WorkspaceID("workspace-survivor"), entity.NewPane(entity.PaneID("pane-survivor"))) | |||
| 126 | + | tabs := entity.NewTabList() | |||
| 127 | + | tabs.Add(closedTab) | |||
| 128 | + | tabs.Add(survivingTab) | |||
| 129 | + | tabs.SetActive(closedTab.ID) | |||
| 130 | + | ||||
| 131 | + | pool := &recordingWebViewPool{} | |||
| 132 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |||
| 133 | + | closedWV := &fakeRecordingWebView{id: 101} | |||
| 134 | + | survivingWV := &fakeRecordingWebView{id: 102} | |||
| 135 | + | contentCoord.RegisterPopupWebView(closedTab.Workspace.ActivePaneID, closedWV) | |||
| 136 | + | contentCoord.RegisterPopupWebView(survivingTab.Workspace.ActivePaneID, survivingWV) | |||
| 137 | + | ||||
| 138 | + | bw := &browserWindow{id: "window-1", tabs: tabs} | |||
| 139 | + | app := &App{ | |||
| 140 | + | deps: &Dependencies{Config: config.DefaultConfig()}, | |||
| 141 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |||
| 142 | + | contentCoord: contentCoord, | |||
| 143 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |||
| 144 | + | tabs: entity.NewTabList(), | |||
| 145 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{closedTab.ID: {}, survivingTab.ID: {}}, | |||
| 146 | + | windowForTab: map[entity.TabID]*browserWindow{closedTab.ID: bw, survivingTab.ID: bw}, | |||
| 147 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |||
| 148 | + | } | |||
| 149 | + | app.tabs.Add(closedTab) | |||
| 150 | + | app.tabs.Add(survivingTab) | |||
| 151 | + | app.initTabCoordinator(ctx) | |||
| 152 | + | ||||
| 153 | + | require.NoError(t, app.tabCoord.Close(ctx, app.ensureTabTargetForBrowserWindow(bw))) | |||
| 154 | + | ||||
| 155 | + | require.Len(t, pool.released, 1) | |||
| 156 | + | assert.Same(t, closedWV, pool.released[0]) | |||
| 157 | + | assert.Nil(t, contentCoord.GetWebView(closedTab.Workspace.ActivePaneID)) | |||
| 158 | + | assert.Same(t, survivingWV, contentCoord.GetWebView(survivingTab.Workspace.ActivePaneID)) | |||
| 159 | + | assert.Nil(t, app.workspaceViews[closedTab.ID]) | |||
| 160 | + | assert.Nil(t, app.windowForTab[closedTab.ID]) | |||
| 161 | + | assert.Nil(t, app.tabs.Find(closedTab.ID)) | |||
| 162 | + | assert.Equal(t, survivingTab.ID, tabs.ActiveTabID) | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | func TestTabCoordinator_SwitchDoesNotReleaseWorkspaceWebViews(t *testing.T) { | |||
| 166 | + | ctx := context.Background() | |||
| 167 | + | firstTab := entity.NewTab(entity.TabID("tab-first"), entity.WorkspaceID("workspace-first"), entity.NewPane(entity.PaneID("pane-first"))) | |||
| 168 | + | secondTab := entity.NewTab(entity.TabID("tab-second"), entity.WorkspaceID("workspace-second"), entity.NewPane(entity.PaneID("pane-second"))) | |||
| 169 | + | tabs := entity.NewTabList() | |||
| 170 | + | tabs.Add(firstTab) | |||
| 171 | + | tabs.Add(secondTab) | |||
| 172 | + | tabs.SetActive(firstTab.ID) | |||
| 173 | + | ||||
| 174 | + | pool := &recordingWebViewPool{} | |||
| 175 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |||
| 176 | + | firstWV := &fakeRecordingWebView{id: 111} | |||
| 177 | + | secondWV := &fakeRecordingWebView{id: 112} | |||
| 178 | + | contentCoord.RegisterPopupWebView(firstTab.Workspace.ActivePaneID, firstWV) | |||
| 179 | + | contentCoord.RegisterPopupWebView(secondTab.Workspace.ActivePaneID, secondWV) | |||
| 180 | + | ||||
| 181 | + | bw := &browserWindow{id: "window-1", tabs: tabs} | |||
| 182 | + | app := &App{ | |||
| 183 | + | deps: &Dependencies{Config: config.DefaultConfig()}, | |||
| 184 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |||
| 185 | + | contentCoord: contentCoord, | |||
| 186 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |||
| 187 | + | tabs: entity.NewTabList(), | |||
| 188 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{firstTab.ID: {}, secondTab.ID: {}}, | |||
| 189 | + | windowForTab: map[entity.TabID]*browserWindow{firstTab.ID: bw, secondTab.ID: bw}, | |||
| 190 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |||
| 191 | + | } | |||
| 192 | + | app.tabs.Add(firstTab) | |||
| 193 | + | app.tabs.Add(secondTab) | |||
| 194 | + | app.initTabCoordinator(ctx) | |||
| 195 | + | ||||
| 196 | + | require.NoError(t, app.tabCoord.Switch(ctx, app.ensureTabTargetForBrowserWindow(bw), secondTab.ID)) | |||
| 197 | + | ||||
| 198 | + | assert.Empty(t, pool.released) | |||
| 199 | + | assert.Same(t, firstWV, contentCoord.GetWebView(firstTab.Workspace.ActivePaneID)) | |||
| 200 | + | assert.Same(t, secondWV, contentCoord.GetWebView(secondTab.Workspace.ActivePaneID)) | |||
| 201 | + | } | |||
| 202 | + | ||||
| 203 | + | func TestTabCoordinator_CloseLastTabReleasesWorkspaceBeforeWindowRemoval(t *testing.T) { | |||
| 204 | + | ctx := context.Background() | |||
| 205 | + | closedTab := entity.NewTab(entity.TabID("tab-only"), entity.WorkspaceID("workspace-only"), entity.NewPane(entity.PaneID("pane-only"))) | |||
| 206 | + | tabs := entity.NewTabList() | |||
| 207 | + | tabs.Add(closedTab) | |||
| 208 | + | tabs.SetActive(closedTab.ID) | |||
| 209 | + | ||||
| 210 | + | pool := &recordingWebViewPool{} | |||
| 211 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |||
| 212 | + | closedWV := &fakeRecordingWebView{id: 121} | |||
| 213 | + | contentCoord.RegisterPopupWebView(closedTab.Workspace.ActivePaneID, closedWV) | |||
| 214 | + | ||||
| 215 | + | bw := &browserWindow{id: "window-1", tabs: tabs} | |||
| 216 | + | app := &App{ | |||
| 217 | + | deps: &Dependencies{Config: config.DefaultConfig()}, | |||
| 218 | + | tabsUC: usecase.NewManageTabsUseCase(counterIDGen()), | |||
| 219 | + | contentCoord: contentCoord, | |||
| 220 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |||
| 221 | + | tabs: entity.NewTabList(), | |||
| 222 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{closedTab.ID: {}}, | |||
| 223 | + | windowForTab: map[entity.TabID]*browserWindow{closedTab.ID: bw}, | |||
| 224 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |||
| 225 | + | } | |||
| 226 | + | app.tabs.Add(closedTab) | |||
| 227 | + | app.initTabCoordinator(ctx) | |||
| 228 | + | ||||
| 229 | + | require.NoError(t, app.tabCoord.Close(ctx, app.ensureTabTargetForBrowserWindow(bw))) | |||
| 230 | + | ||||
| 231 | + | require.Len(t, pool.released, 1) | |||
| 232 | + | assert.Same(t, closedWV, pool.released[0]) | |||
| 233 | + | assert.Nil(t, contentCoord.GetWebView(closedTab.Workspace.ActivePaneID)) | |||
| 234 | + | assert.Empty(t, app.browserWindows) | |||
| 235 | + | assert.Nil(t, app.workspaceViews[closedTab.ID]) | |||
| 236 | + | assert.Nil(t, app.windowForTab[closedTab.ID]) | |||
| 237 | + | } | |||
| 238 | + | ||||
| 239 | + | func TestBrowserWindow_RemoveBrowserWindowReleasesOwnedTabWorkspaceWebViews(t *testing.T) { | |||
| 240 | + | ctx := context.Background() | |||
| 241 | + | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | |||
| 242 | + | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | |||
| 243 | + | removed := &browserWindow{id: "window-1", tabs: entity.NewTabList()} | |||
| 244 | + | remaining := &browserWindow{id: "window-2", tabs: entity.NewTabList()} | |||
| 245 | + | removed.tabs.Add(ownedTab) | |||
| 246 | + | remaining.tabs.Add(otherTab) | |||
| 247 | + | ||||
| 248 | + | pool := &recordingWebViewPool{} | |||
| 249 | + | contentCoord := contentcoord.NewCoordinator(ctx, pool, nil, nil, nil, nil, nil, nil) | |||
| 250 | + | ownedWV := &fakeRecordingWebView{id: 201} | |||
| 251 | + | otherWV := &fakeRecordingWebView{id: 202} | |||
| 252 | + | contentCoord.RegisterPopupWebView(ownedTab.Workspace.ActivePaneID, ownedWV) | |||
| 253 | + | contentCoord.RegisterPopupWebView(otherTab.Workspace.ActivePaneID, otherWV) | |||
| 254 | + | ||||
| 255 | + | app := &App{ | |||
| 256 | + | contentCoord: contentCoord, | |||
| 257 | + | browserWindows: map[string]*browserWindow{removed.id: removed, remaining.id: remaining}, | |||
| 258 | + | tabs: entity.NewTabList(), | |||
| 259 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ownedTab.ID: {}, otherTab.ID: {}}, | |||
| 260 | + | windowForTab: map[entity.TabID]*browserWindow{ownedTab.ID: removed, otherTab.ID: remaining}, | |||
| 261 | + | floatingSessions: map[floatingSessionKey]*floatingWorkspaceSession{}, | |||
| 262 | + | } | |||
| 263 | + | app.tabs.Add(ownedTab) | |||
| 264 | + | app.tabs.Add(otherTab) | |||
| 265 | + | ||||
| 266 | + | app.removeBrowserWindow(removed.id) | |||
| 267 | + | ||||
| 268 | + | require.Len(t, pool.released, 1) | |||
| 269 | + | assert.Same(t, ownedWV, pool.released[0]) | |||
| 270 | + | assert.Nil(t, contentCoord.GetWebView(ownedTab.Workspace.ActivePaneID)) | |||
| 271 | + | assert.Same(t, otherWV, contentCoord.GetWebView(otherTab.Workspace.ActivePaneID)) | |||
| 272 | + | } | |||
| 273 | + | ||||
| 106 | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | 274 | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | ||
| 107 | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | 275 | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | ||
| 108 | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | 276 | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | ||
M internal/ui/coordinator/tab.go +11 -0
| @@ -32,6 +32,7 @@ type TabCoordinator struct { | |||
| 32 | 32 | // Callbacks to avoid circular dependencies | |
| 33 | 33 | onTabCreated func(ctx context.Context, target TabTarget, tab *entity.Tab) | |
| 34 | 34 | onTabSwitched func(ctx context.Context, target TabTarget, tab *entity.Tab) | |
| 35 | + | onTabClosed func(ctx context.Context, target TabTarget, tab *entity.Tab) | |
| 35 | 36 | onQuit func() | |
| 36 | 37 | onCurrentWindowEmpty func(ctx context.Context, target TabTarget) | |
| 37 | 38 | onAttachPopupToTab func(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) // For popup tabs | |
| @@ -87,6 +88,11 @@ func (c *TabCoordinator) SetOnTabSwitched(fn func(ctx context.Context, target Ta | |||
| 87 | 88 | c.onTabSwitched = fn | |
| 88 | 89 | } | |
| 89 | 90 | ||
| 91 | + | // SetOnTabClosed sets the callback for when a tab is removed from its target. | |
| 92 | + | func (c *TabCoordinator) SetOnTabClosed(fn func(ctx context.Context, target TabTarget, tab *entity.Tab)) { | |
| 93 | + | c.onTabClosed = fn | |
| 94 | + | } | |
| 95 | + | ||
| 90 | 96 | // SetOnQuit sets the callback for when the last tab is closed. | |
| 91 | 97 | func (c *TabCoordinator) SetOnQuit(fn func()) { | |
| 92 | 98 | c.onQuit = fn | |
| @@ -236,12 +242,17 @@ func (c *TabCoordinator) Close(ctx context.Context, target TabTarget) error { | |||
| 236 | 242 | } | |
| 237 | 243 | } | |
| 238 | 244 | ||
| 245 | + | closedTab := target.Tabs.Find(activeID) | |
| 239 | 246 | wasLast, err := c.tabsUC.Close(ctx, target.Tabs, activeID) | |
| 240 | 247 | if err != nil { | |
| 241 | 248 | log.Error().Err(err).Msg("failed to close tab") | |
| 242 | 249 | return err | |
| 243 | 250 | } | |
| 244 | 251 | ||
| 252 | + | if c.onTabClosed != nil && closedTab != nil { | |
| 253 | + | c.onTabClosed(ctx, target, closedTab) | |
| 254 | + | } | |
| 255 | + | ||
| 245 | 256 | if target.MainWindow != nil && target.MainWindow.TabBar() != nil { | |
| 246 | 257 | target.MainWindow.TabBar().RemoveTab(activeID) | |
| 247 | 258 | } | |
| @@ -32,6 +32,7 @@ type TabCoordinator struct { | |||||
| 32 | // Callbacks to avoid circular dependencies | 32 | // Callbacks to avoid circular dependencies | ||
| 33 | onTabCreated func(ctx context.Context, target TabTarget, tab *entity.Tab) | 33 | onTabCreated func(ctx context.Context, target TabTarget, tab *entity.Tab) | ||
| 34 | onTabSwitched func(ctx context.Context, target TabTarget, tab *entity.Tab) | 34 | onTabSwitched func(ctx context.Context, target TabTarget, tab *entity.Tab) | ||
| 35 | + | onTabClosed func(ctx context.Context, target TabTarget, tab *entity.Tab) | |||
| 35 | onQuit func() | 36 | onQuit func() | ||
| 36 | onCurrentWindowEmpty func(ctx context.Context, target TabTarget) | 37 | onCurrentWindowEmpty func(ctx context.Context, target TabTarget) | ||
| 37 | onAttachPopupToTab func(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) // For popup tabs | 38 | onAttachPopupToTab func(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) // For popup tabs | ||
| @@ -87,6 +88,11 @@ func (c *TabCoordinator) SetOnTabSwitched(fn func(ctx context.Context, target Ta | |||||
| 87 | c.onTabSwitched = fn | 88 | c.onTabSwitched = fn | ||
| 88 | } | 89 | } | ||
| 89 | 90 | ||||
| 91 | + | // SetOnTabClosed sets the callback for when a tab is removed from its target. | |||
| 92 | + | func (c *TabCoordinator) SetOnTabClosed(fn func(ctx context.Context, target TabTarget, tab *entity.Tab)) { | |||
| 93 | + | c.onTabClosed = fn | |||
| 94 | + | } | |||
| 95 | + | ||||
| 90 | // SetOnQuit sets the callback for when the last tab is closed. | 96 | // SetOnQuit sets the callback for when the last tab is closed. | ||
| 91 | func (c *TabCoordinator) SetOnQuit(fn func()) { | 97 | func (c *TabCoordinator) SetOnQuit(fn func()) { | ||
| 92 | c.onQuit = fn | 98 | c.onQuit = fn | ||
| @@ -236,12 +242,17 @@ func (c *TabCoordinator) Close(ctx context.Context, target TabTarget) error { | |||||
| 236 | } | 242 | } | ||
| 237 | } | 243 | } | ||
| 238 | 244 | ||||
| 245 | + | closedTab := target.Tabs.Find(activeID) | |||
| 239 | wasLast, err := c.tabsUC.Close(ctx, target.Tabs, activeID) | 246 | wasLast, err := c.tabsUC.Close(ctx, target.Tabs, activeID) | ||
| 240 | if err != nil { | 247 | if err != nil { | ||
| 241 | log.Error().Err(err).Msg("failed to close tab") | 248 | log.Error().Err(err).Msg("failed to close tab") | ||
| 242 | return err | 249 | return err | ||
| 243 | } | 250 | } | ||
| 244 | 251 | ||||
| 252 | + | if c.onTabClosed != nil && closedTab != nil { | |||
| 253 | + | c.onTabClosed(ctx, target, closedTab) | |||
| 254 | + | } | |||
| 255 | + | ||||
| 245 | if target.MainWindow != nil && target.MainWindow.TabBar() != nil { | 256 | if target.MainWindow != nil && target.MainWindow.TabBar() != nil { | ||
| 246 | target.MainWindow.TabBar().RemoveTab(activeID) | 257 | target.MainWindow.TabBar().RemoveTab(activeID) | ||
| 247 | } | 258 | } | ||