Skip to content

Commit 46a62bd

Browse files
committed
Fix electron bug
1 parent fec8b4b commit 46a62bd

2 files changed

Lines changed: 88 additions & 8 deletions

File tree

apps/desktop/src/main/browser-agent/session.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ interface MockView {
1515
}
1616
setWindowOpenHandler: ReturnType<typeof vi.fn>
1717
loadURL: ReturnType<typeof vi.fn>
18+
isDestroyed: ReturnType<typeof vi.fn>
1819
setBackgroundThrottling: ReturnType<typeof vi.fn>
1920
capturePage: ReturnType<typeof vi.fn>
2021
}
@@ -166,6 +167,67 @@ describe('browser-agent session', () => {
166167
expect(removeChildView).toHaveBeenCalledWith(tab.view)
167168
})
168169

170+
it('clears a stale attachment without touching a destroyed host window', () => {
171+
const tab = session.ensureTab()
172+
session.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 })
173+
const staleContent = (
174+
win as unknown as {
175+
contentView: {
176+
removeChildView: ReturnType<typeof vi.fn>
177+
}
178+
}
179+
).contentView
180+
staleContent.removeChildView.mockClear()
181+
staleContent.removeChildView.mockImplementation(() => {
182+
throw new Error('Object has been destroyed')
183+
})
184+
vi.mocked(win.isDestroyed).mockReturnValue(true)
185+
186+
const replacement = mainWindowMock()
187+
session.initSession(
188+
{
189+
onSessionClosed: vi.fn(),
190+
onTabCreated: vi.fn(),
191+
onActiveTabChanged: vi.fn(),
192+
onTabsChanged: vi.fn(),
193+
onTabThemeChanged: vi.fn(),
194+
onDownloadBlocked: vi.fn(),
195+
},
196+
() => replacement
197+
)
198+
199+
expect(() => session.setPanelBounds(null)).not.toThrow()
200+
expect(staleContent.removeChildView).not.toHaveBeenCalled()
201+
202+
session.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 })
203+
const replacementContent = (
204+
replacement as unknown as {
205+
contentView: {
206+
addChildView: ReturnType<typeof vi.fn>
207+
}
208+
}
209+
).contentView
210+
expect(replacementContent.addChildView).toHaveBeenCalledWith(tab.view)
211+
})
212+
213+
it('clears a stale attachment without touching a destroyed child view', () => {
214+
const tab = session.ensureTab()
215+
const view = tab.view as unknown as MockView
216+
session.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 })
217+
const content = (
218+
win as unknown as {
219+
contentView: {
220+
removeChildView: ReturnType<typeof vi.fn>
221+
}
222+
}
223+
).contentView
224+
content.removeChildView.mockClear()
225+
view.webContents.isDestroyed.mockReturnValue(true)
226+
227+
expect(() => session.setPanelBounds(null)).not.toThrow()
228+
expect(content.removeChildView).not.toHaveBeenCalled()
229+
})
230+
169231
it('scales panel bounds by the main window zoom factor', () => {
170232
const winZoomed = mainWindowMock()
171233
;(

apps/desktop/src/main/browser-agent/session.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
MAX_BROWSER_TABS,
88
} from '@sim/browser-protocol'
99
import { createLogger } from '@sim/logger'
10+
import { getErrorMessage } from '@sim/utils/errors'
1011
import type { BrowserWindow, Session, WebContents } from 'electron'
1112
import { nativeTheme, WebContentsView } from 'electron'
1213
import { registerAgentWebContents } from '@/main/browser-agent/registry'
@@ -206,6 +207,29 @@ let attachedView: WebContentsView | null = null
206207
let lastAppliedBounds = ''
207208
let lastAppliedVisibility: boolean | null = null
208209

210+
/**
211+
* Clears the tracked attachment before touching Electron objects so a stale
212+
* host or child view cannot leave layout permanently wedged after teardown.
213+
*/
214+
function detachAttachedView(): void {
215+
const view = attachedView
216+
const win = hostedWindow
217+
attachedView = null
218+
hostedWindow = null
219+
lastAppliedBounds = ''
220+
lastAppliedVisibility = null
221+
222+
if (!view || !win) return
223+
try {
224+
if (win.isDestroyed() || view.webContents.isDestroyed()) return
225+
win.contentView.removeChildView(view)
226+
} catch (error) {
227+
logger.warn('Could not detach embedded browser view', {
228+
error: getErrorMessage(error, 'unknown'),
229+
})
230+
}
231+
}
232+
209233
/**
210234
* Captures the current browser frame for the renderer to display while the
211235
* native view is hidden beneath an overlay. Captures stay hidden so Chromium
@@ -246,10 +270,7 @@ function layout(): void {
246270

247271
if (!showing || hostedWindow !== win || attachedView !== active?.view) {
248272
if (attachedView) {
249-
hostedWindow?.contentView.removeChildView(attachedView)
250-
attachedView = null
251-
lastAppliedBounds = ''
252-
lastAppliedVisibility = null
273+
detachAttachedView()
253274
}
254275
}
255276
if (!showing || !active || !win || panelBounds === null) {
@@ -369,10 +390,7 @@ export function closeTab(tabId: string): void {
369390
if (index < 0) throw new SessionError(`No tab with id ${tabId} — call browser_list_tabs.`)
370391
const [tab] = tabs.splice(index, 1)
371392
if (attachedView === tab.view) {
372-
hostedWindow?.contentView.removeChildView(tab.view)
373-
attachedView = null
374-
lastAppliedBounds = ''
375-
lastAppliedVisibility = null
393+
detachAttachedView()
376394
}
377395
tab.view.webContents.close()
378396
if (activeTabId === tab.id) {

0 commit comments

Comments
 (0)