Skip to content

Commit 245d363

Browse files
committed
fix(desktop): keep deferred popups in their opener's session
- A background-tab popup with no Chromium-supplied contents now opens in the opener's session rather than the agent partition - Driver tests expect the referrer options loadURL now receives
1 parent ddf4e2f commit 245d363

3 files changed

Lines changed: 40 additions & 15 deletions

File tree

‎apps/desktop/src/main/browser-agent/driver.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ describe('executeTool', () => {
804804
const popup = session.activeTab()?.view.webContents
805805
if (!popup) throw new Error('Expected user popup tab')
806806
expect(session.automationTab()?.view.webContents).toBe(source)
807-
expect(popup.loadURL).toHaveBeenCalledWith('https://user-popup.example/')
807+
expect(popup.loadURL).toHaveBeenCalledWith('https://user-popup.example/', expect.anything())
808808
})
809809

810810
it('keeps agent-opened target-blank tabs agent-owned after dispatch ends', async () => {
@@ -823,7 +823,7 @@ describe('executeTool', () => {
823823
expect(session.activeTab()?.view.webContents).toBe(source)
824824
session.setAutomationActive(false)
825825
expect(session.automationTab()?.view.webContents).toBe(popup)
826-
expect(popup.loadURL).toHaveBeenCalledWith('https://agent-popup.example/')
826+
expect(popup.loadURL).toHaveBeenCalledWith('https://agent-popup.example/', expect.anything())
827827
})
828828

829829
it('does not report a timed-out restored tab as ready to the model', async () => {

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function _setPlatform(platform: NodeJS.Platform): void {
3232
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
3333
}
3434

35-
type PopupHandler = (details: { url: string }) => {
35+
type PopupHandler = (details: { url: string; disposition?: string }) => {
3636
action: string
3737
outlivesOpener?: boolean
3838
createWindow?: (options: Record<string, unknown>) => unknown
@@ -2321,13 +2321,28 @@ describe('importAgentCookies', () => {
23212321

23222322
describe('first-party browser sessions', () => {
23232323
const origin = 'https://www.dev.sim.ai'
2324+
const appSession = new WebContentsView().webContents.session
23242325
function initialize(persistence?: sessionModule.BrowserSessionPersistence) {
23252326
return freshSession(null, {}, persistence, undefined, {
23262327
origin,
2327-
session: new WebContentsView().webContents.session,
2328+
session: appSession,
23282329
})
23292330
}
23302331

2332+
it("opens a deferred blank popup in its opener's session", () => {
2333+
const session = initialize()
2334+
const opener = session.addAutomationTab(`${origin}/home`).view.webContents
2335+
Object.assign(opener, { session: appSession })
2336+
const openWindow = vi.mocked(opener.setWindowOpenHandler).mock.calls[0]?.[0] as PopupHandler
2337+
2338+
const popup = openWindow({ url: 'about:blank', disposition: 'background-tab' }).createWindow?.(
2339+
{}
2340+
) as WebContents
2341+
expect(popup).not.toBe(opener)
2342+
expect(agentAppOrigin(popup)).toBe(origin)
2343+
expect(popup.loadURL).not.toHaveBeenCalled()
2344+
})
2345+
23312346
it('keeps a populated tab and its history when crossing the session boundary', () => {
23322347
const session = initialize()
23332348
const tab = session.addAutomationTab(`${origin}/home`)

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

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2318,7 +2318,7 @@ function adoptPopupTab(
23182318
}: { agentOwned: boolean; background: boolean; referrer?: Referrer }
23192319
): WebContents {
23202320
const openerTabId = tabForContents(opener)?.id
2321-
const tab = openLinkTab(url, { agentOwned, background }, popup)
2321+
const tab = openLinkTab(url, { agentOwned, background }, { opener, options: popup })
23222322
tab.openerTabId = openerTabId
23232323
tab.returnToOpenerOnClose = true
23242324
const contents = tab.view.webContents
@@ -2336,7 +2336,7 @@ function adoptPopupTab(
23362336
function openLinkTab(
23372337
url: string,
23382338
{ agentOwned, background }: { agentOwned: boolean; background: boolean },
2339-
popup?: PopupWindowOptions
2339+
popup?: AdoptedPopup
23402340
): AgentTab {
23412341
if (agentOwned) return addAutomationTab(url, popup)
23422342
if (background) {
@@ -2379,23 +2379,33 @@ function openTabWithUrl(
23792379
*/
23802380
type PopupWindowOptions = BrowserWindowConstructorOptions & WebContentsViewConstructorOptions
23812381

2382+
/** A page-opened window becoming a tab, with the page that opened it. */
2383+
interface AdoptedPopup {
2384+
opener: WebContents
2385+
options: PopupWindowOptions
2386+
}
2387+
23822388
/**
23832389
* Creates a tab's view. `popup` adopts a page-opened window's WebContents, which Chromium created
23842390
* with the opener's (identical) web preferences, so the opener relationship and its session stay
2385-
* intact; callers only adopt popups that are blank or whose URL belongs to the opener's session.
2391+
* intact; a deferred popup without one gets a fresh view in the opener's session. Callers only
2392+
* adopt popups that are blank or whose URL belongs to the opener's session.
23862393
*/
2387-
function createTabView(url?: string, popup?: PopupWindowOptions): WebContentsView {
2394+
function createTabView(url?: string, popup?: AdoptedPopup): WebContentsView {
23882395
let appSession: BrowserAppSession | undefined
2389-
if (popup?.webContents) {
2390-
// A blank popup's URL names no origin, so trust the session Chromium created it in.
2391-
if (browserAppSession && popup.webContents.session === browserAppSession.session) {
2396+
if (popup) {
2397+
// A popup follows its opener's session (Chromium creates it there); a blank URL names no origin.
2398+
const popupSession = popup.options.webContents?.session ?? popup.opener.session
2399+
if (browserAppSession && popupSession === browserAppSession.session) {
23922400
appSession = browserAppSession
23932401
}
23942402
} else if (url && browserAppSession && isAppOrigin(url, browserAppSession.origin)) {
23952403
appSession = browserAppSession
23962404
}
23972405
const scopeId = getBrowserScopeId()
2398-
const view = popup?.webContents ? new WebContentsView(popup) : createFreshTabView(appSession)
2406+
const view = popup?.options.webContents
2407+
? new WebContentsView(popup.options)
2408+
: createFreshTabView(appSession)
23992409
try {
24002410
/** Detached tabs must lay out before a foreground panel owns their native view. */
24012411
const [width, height] = getMainWindow()?.getContentSize() ?? [1280, 720]
@@ -2960,7 +2970,7 @@ interface AddTabOptions {
29602970
activate?: boolean
29612971
notify?: boolean
29622972
url?: string
2963-
popup?: PopupWindowOptions
2973+
popup?: AdoptedPopup
29642974
}
29652975

29662976
function addTabInternal({
@@ -3441,7 +3451,7 @@ export function restoreBrowserSession(): void {
34413451
}
34423452
}
34433453

3444-
export function addTab(url?: string, popup?: PopupWindowOptions): AgentTab {
3454+
export function addTab(url?: string, popup?: AdoptedPopup): AgentTab {
34453455
restoreBrowserSession()
34463456
currentScope.visibleTabUserSelected = true
34473457
return addTabInternal({ url, popup })
@@ -3452,7 +3462,7 @@ export function addTab(url?: string, popup?: PopupWindowOptions): AgentTab {
34523462
* renderer's decision: every page is a resource tab there, and it shows the
34533463
* agent's tab or badges it depending on what the user is doing.
34543464
*/
3455-
export function addAutomationTab(url?: string, popup?: PopupWindowOptions): AgentTab {
3465+
export function addAutomationTab(url?: string, popup?: AdoptedPopup): AgentTab {
34563466
restoreBrowserSession()
34573467
const tab = addTabInternal({ activate: false, notify: false, url, popup })
34583468
currentScope.automationTabId = tab.id

0 commit comments

Comments
 (0)