Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

chore: extract CDPSession on the server side, remove duplicate code (#27349)

Browse source 
This makes sure that protocol calls after target close are rejected
correctly.
This commit is contained in:
Dmitry Gozman 2023-09-29 12:50:02 -07:00 committed by GitHub
parent 2a8d6a8207
commit 55c4bb97af
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
packages/playwright-core/src/server/chromium/crBrowser.ts
View file

@ -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<CRSession> | null = null;
private _clientRootSessionPromise: Promise<CDPSession> | null = null;
readonly _contexts = new Map<string, CRBrowserContext>();
_crPages = new Map<string, CRPage>();
_backgroundPages = new Map<string, CRPage>();
@ -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<CRSession> {
async newBrowserCDPSession(): Promise<CDPSession> {
return await this._connection.createBrowserSession();
}
@ -333,7 +323,7 @@ export class CRBrowser extends Browser {
return !this._connection._closed;
}
async _clientRootSession(): Promise<CRSession> {
async _clientRootSession(): Promise<CDPSession> {
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<CRSession> {
async newCDPSession(page: Page | Frame): Promise<CDPSession> {
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);
}
}

67
packages/playwright-core/src/server/chromium/crConnection.ts
View file

@ -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<CRSession> {
async createBrowserSession(): Promise<CDPSession> {
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<number, {resolve: (o: any) => 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: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;
override removeListener: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;
override once: <T extends keyof Protocol.Events | symbol>(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<T extends keyof Protocol.CommandParameters>(
@ -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);
}
}

5
packages/playwright-core/src/server/chromium/crPage.ts
View file

@ -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;
}

15
packages/playwright-core/src/server/dispatchers/cdpSessionDispatcher.ts
View file

@ -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<CRSession, channels.CDPSessionChannel, BrowserDispatcher | BrowserContextDispatcher> implements channels.CDPSessionChannel {
export class CDPSessionDispatcher extends Dispatcher<CDPSession, channels.CDPSessionChannel, BrowserDispatcher | BrowserContextDispatcher> 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<channels.CDPSessionSendResult> {

13
tests/library/chromium/session.spec.ts
View file

@ -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();