diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index 8a5a2da95b..af2716b588 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -28,7 +28,7 @@ import type { Dialog } from '../dialog'; import type { ConnectionTransport } from '../transport'; import type * as types from '../types'; import type * as channels from '@protocol/channels'; -import type { CRSession } from './crConnection'; +import type { CRSession, CDPSession } from './crConnection'; import { ConnectionEvents, CRConnection } from './crConnection'; import { CRPage } from './crPage'; import { saveProtocolStream } from './crProtocolHelper'; @@ -41,7 +41,7 @@ import { Artifact } from '../artifact'; export class CRBrowser extends Browser { readonly _connection: CRConnection; _session: CRSession; - private _clientRootSessionPromise: Promise | null = null; + private _clientRootSessionPromise: Promise | null = null; readonly _contexts = new Map(); _crPages = new Map(); _backgroundPages = new Map(); @@ -166,12 +166,7 @@ export class CRBrowser extends Browser { const treatOtherAsPage = targetInfo.type === 'other' && process.env.PW_CHROMIUM_ATTACH_TO_OTHER; if (!context || (targetInfo.type === 'other' && !treatOtherAsPage)) { - if (waitingForDebugger) { - // Ideally, detaching should resume any target, but there is a bug in the backend. - session._sendMayFail('Runtime.runIfWaitingForDebugger').then(() => { - this._session._sendMayFail('Target.detachFromTarget', { sessionId }); - }); - } + session.detach().catch(() => {}); return; } @@ -204,15 +199,10 @@ export class CRBrowser extends Browser { // One example of a side effect: upon shared worker restart, we receive // Inspector.targetReloadedAfterCrash and backend waits for Runtime.runIfWaitingForDebugger // from any attached client. If we do not resume, shared worker will stall. - // - // Ideally, detaching should resume any target, but there is a bug in the backend, - // so we must Runtime.runIfWaitingForDebugger first. - session._sendMayFail('Runtime.runIfWaitingForDebugger').then(() => { - this._session._sendMayFail('Target.detachFromTarget', { sessionId }); - }); + session.detach().catch(() => {}); } - _onDetachedFromTarget(payload: Protocol.Target.detachFromTargetParameters) { + _onDetachedFromTarget(payload: Protocol.Target.detachedFromTargetPayload) { const targetId = payload.targetId!; const crPage = this._crPages.get(targetId); if (crPage) { @@ -286,7 +276,7 @@ export class CRBrowser extends Browser { await this._session.send('Target.closeTarget', { targetId: crPage._targetId }); } - async newBrowserCDPSession(): Promise { + async newBrowserCDPSession(): Promise { return await this._connection.createBrowserSession(); } @@ -333,7 +323,7 @@ export class CRBrowser extends Browser { return !this._connection._closed; } - async _clientRootSession(): Promise { + async _clientRootSession(): Promise { if (!this._clientRootSessionPromise) this._clientRootSessionPromise = this._connection.createBrowserSession(); return this._clientRootSessionPromise; @@ -592,7 +582,7 @@ export class CRBrowserContext extends BrowserContext { return Array.from(this._browser._serviceWorkers.values()).filter(serviceWorker => serviceWorker._browserContext === this); } - async newCDPSession(page: Page | Frame): Promise { + async newCDPSession(page: Page | Frame): Promise { let targetId: string | null = null; if (page instanceof Page) { targetId = (page._delegate as CRPage)._targetId; @@ -605,7 +595,6 @@ export class CRBrowserContext extends BrowserContext { } const rootSession = await this._browser._clientRootSession(); - const { sessionId } = await rootSession.send('Target.attachToTarget', { targetId, flatten: true }); - return rootSession.createChildSession(sessionId); + return rootSession.attachToTarget(targetId); } } diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index 7c579feafe..4e2c0d0f08 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { assert } from '../../utils'; +import { type RegisteredListener, assert, eventsHelper } from '../../utils'; import type { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport'; import type { Protocol } from './protocol'; import { EventEmitter } from 'events'; @@ -90,19 +90,17 @@ export class CRConnection extends EventEmitter { this._transport.close(); } - async createBrowserSession(): Promise { + async createBrowserSession(): Promise { const { sessionId } = await this.rootSession.send('Target.attachToBrowserTarget'); - return this.rootSession.createChildSession(sessionId); + return new CDPSession(this.rootSession, sessionId); } } -export const CRSessionEvents = { - Disconnected: Symbol('Events.CDPSession.Disconnected') -}; +type SessionEventListener = (method: string, params?: Object) => void; export class CRSession extends EventEmitter { private readonly _connection: CRConnection; - _eventListener?: (method: string, params?: Object) => void; + private _eventListener?: SessionEventListener; private readonly _callbacks = new Map void, reject: (e: ProtocolError) => void, error: ProtocolError, method: string}>(); private readonly _sessionId: string; private readonly _parentSession: CRSession | null; @@ -113,15 +111,14 @@ export class CRSession extends EventEmitter { override off: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; override removeListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; override once: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; - readonly guid: string; - constructor(connection: CRConnection, parentSession: CRSession | null, sessionId: string) { + constructor(connection: CRConnection, parentSession: CRSession | null, sessionId: string, eventListener?: SessionEventListener) { super(); - this.guid = `cdp-session@${sessionId}`; this.setMaxListeners(0); this._connection = connection; this._parentSession = parentSession; this._sessionId = sessionId; + this._eventListener = eventListener; this.on = super.on; this.addListener = super.addListener; @@ -134,8 +131,8 @@ export class CRSession extends EventEmitter { this._crashed = true; } - createChildSession(sessionId: string): CRSession { - const session = new CRSession(this._connection, this, sessionId); + createChildSession(sessionId: string, eventListener?: SessionEventListener): CRSession { + const session = new CRSession(this._connection, this, sessionId, eventListener); this._connection._sessions.set(sessionId, session); return session; } @@ -147,6 +144,8 @@ export class CRSession extends EventEmitter { return `Browser closed.` + this._connection._browserDisconnectedLogs; if (this._closed) return `Target closed`; + if (this._connection._closed) + return 'Browser closed'; } async send( @@ -191,6 +190,9 @@ export class CRSession extends EventEmitter { throw new Error(`Session already detached. Most likely the page has been closed.`); if (!this._parentSession) throw new Error('Root session cannot be closed'); + // Ideally, detaching should resume any target, but there is a bug in the backend, + // so we must Runtime.runIfWaitingForDebugger first. + await this._sendMayFail('Runtime.runIfWaitingForDebugger'); await this._parentSession.send('Target.detachFromTarget', { sessionId: this._sessionId }); this.dispose(); } @@ -204,7 +206,46 @@ export class CRSession extends EventEmitter { callback.reject(rewriteErrorMessage(callback.error, errorMessage)); } this._callbacks.clear(); - Promise.resolve().then(() => this.emit(CRSessionEvents.Disconnected)); + } +} + +export class CDPSession extends EventEmitter { + static Events = { + Event: 'event', + Closed: 'close', + }; + + readonly guid: string; + private _session: CRSession; + private _listeners: RegisteredListener[] = []; + + constructor(parentSession: CRSession, sessionId: string) { + super(); + this.guid = `cdp-session@${sessionId}`; + this._session = parentSession.createChildSession(sessionId, (method, params) => this.emit(CDPSession.Events.Event, { method, params })); + this._listeners = [eventsHelper.addEventListener(parentSession, 'Target.detachedFromTarget', (event: Protocol.Target.detachedFromTargetPayload) => { + if (event.sessionId === sessionId) + this._onClose(); + })]; + } + + async send(method: string, params?: any) { + return await this._session.send(method as any, params); + } + + async detach() { + return await this._session.detach(); + } + + async attachToTarget(targetId: string) { + const { sessionId } = await this.send('Target.attachToTarget', { targetId, flatten: true }); + return new CDPSession(this._session, sessionId); + } + + private _onClose() { + eventsHelper.removeEventListeners(this._listeners); + this._session.dispose(); + this.emit(CDPSession.Events.Closed); } } diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 1566b722e7..f41cb51959 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -733,10 +733,7 @@ class FrameSession { } if (event.targetInfo.type !== 'worker') { - // Ideally, detaching should resume any target, but there is a bug in the backend. - session._sendMayFail('Runtime.runIfWaitingForDebugger').then(() => { - this._client._sendMayFail('Target.detachFromTarget', { sessionId: event.sessionId }); - }); + session.detach().catch(() => {}); return; } diff --git a/packages/playwright-core/src/server/dispatchers/cdpSessionDispatcher.ts b/packages/playwright-core/src/server/dispatchers/cdpSessionDispatcher.ts index 8d254531b1..4450e48d14 100644 --- a/packages/playwright-core/src/server/dispatchers/cdpSessionDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/cdpSessionDispatcher.ts @@ -14,22 +14,19 @@ * limitations under the License. */ -import type { CRSession } from '../chromium/crConnection'; -import { CRSessionEvents } from '../chromium/crConnection'; +import { CDPSession } from '../chromium/crConnection'; import type * as channels from '@protocol/channels'; import { Dispatcher } from './dispatcher'; import type { BrowserDispatcher } from './browserDispatcher'; import type { BrowserContextDispatcher } from './browserContextDispatcher'; -export class CDPSessionDispatcher extends Dispatcher implements channels.CDPSessionChannel { +export class CDPSessionDispatcher extends Dispatcher implements channels.CDPSessionChannel { _type_CDPSession = true; - constructor(scope: BrowserDispatcher | BrowserContextDispatcher, crSession: CRSession) { - super(scope, crSession, 'CDPSession', {}); - crSession._eventListener = (method, params) => { - this._dispatchEvent('event', { method, params }); - }; - this.addObjectListener(CRSessionEvents.Disconnected, () => this._dispose()); + constructor(scope: BrowserDispatcher | BrowserContextDispatcher, cdpSession: CDPSession) { + super(scope, cdpSession, 'CDPSession', {}); + this.addObjectListener(CDPSession.Events.Event, ({ method, params }) => this._dispatchEvent('event', { method, params })); + this.addObjectListener(CDPSession.Events.Closed, () => this._dispose()); } async send(params: channels.CDPSessionSendParams): Promise { diff --git a/tests/library/chromium/session.spec.ts b/tests/library/chromium/session.spec.ts index eb611b8cda..e5c6830ab0 100644 --- a/tests/library/chromium/session.spec.ts +++ b/tests/library/chromium/session.spec.ts @@ -132,6 +132,19 @@ browserTest('should detach when page closes', async function({ browser }) { await context.close(); }); +browserTest('should reject protocol calls when page closes', async function({ browser }) { + const context = await browser.newContext(); + const page = await context.newPage(); + const session = await context.newCDPSession(page); + const promise = session.send('Runtime.evaluate', { expression: 'new Promise(() => {})', awaitPromise: true }).catch(e => e); + await page.close(); + const error1 = await promise; + expect(error1.message).toContain('Target closed'); + const error2 = await session.send('Runtime.evaluate', { expression: 'new Promise(() => {})', awaitPromise: true }).catch(e => e); + expect(error2.message).toContain('Target page, context or browser has been closed'); + await context.close(); +}); + browserTest('should work with newBrowserCDPSession', async function({ browser }) { const session = await browser.newBrowserCDPSession();