From 6eee6442c8b826a6d44e0b2716dd102b3f26fbbb Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 27 Aug 2024 17:18:14 -0700 Subject: [PATCH] chore: pass explicit recorder app factory --- packages/playwright-core/src/cli/program.ts | 1 - .../src/client/browserContext.ts | 1 - .../playwright-core/src/protocol/validator.ts | 1 - .../src/server/browserContext.ts | 9 ++-- .../src/server/debugController.ts | 4 +- .../dispatchers/browserContextDispatcher.ts | 3 +- .../playwright-core/src/server/recorder.ts | 42 ++++++++----------- .../src/server/recorder/recorderApp.ts | 18 +++++--- packages/protocol/src/channels.ts | 2 - packages/protocol/src/protocol.yml | 1 - 10 files changed, 39 insertions(+), 43 deletions(-) diff --git a/packages/playwright-core/src/cli/program.ts b/packages/playwright-core/src/cli/program.ts index 28cf15fddb038..9c68271e80b83 100644 --- a/packages/playwright-core/src/cli/program.ts +++ b/packages/playwright-core/src/cli/program.ts @@ -571,7 +571,6 @@ async function codegen(options: Options & { target: string, output?: string, tes mode: 'recording', testIdAttributeName, outputFile: outputFile ? path.resolve(outputFile) : undefined, - handleSIGINT: false, }); await openPage(context, url); } diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index c4f7827840454..ef222136dd673 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -481,7 +481,6 @@ export class BrowserContext extends ChannelOwner mode?: 'recording' | 'inspecting', testIdAttributeName?: string, outputFile?: string, - handleSIGINT?: boolean, }) { await this._channel.recorderSupplementEnable(params); } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index e0b4a4d3dfb22..1768380d3019c 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -961,7 +961,6 @@ scheme.BrowserContextRecorderSupplementEnableParams = tObject({ device: tOptional(tString), saveStorage: tOptional(tString), outputFile: tOptional(tString), - handleSIGINT: tOptional(tBoolean), omitCallTracking: tOptional(tBoolean), }); scheme.BrowserContextRecorderSupplementEnableResult = tOptional(tObject({})); diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 09b84b267fe9a..8ddbe68f89b3a 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -43,6 +43,7 @@ import { BrowserContextAPIRequestContext } from './fetch'; import type { Artifact } from './artifact'; import { Clock } from './clock'; import { ClientCertificatesProxy } from './socksClientCertificatesInterceptor'; +import { RecorderApp } from './recorder/recorderApp'; export abstract class BrowserContext extends SdkObject { static Events = { @@ -130,13 +131,15 @@ export abstract class BrowserContext extends SdkObject { // When PWDEBUG=1, show inspector for each context. if (debugMode() === 'inspector') - await Recorder.show(this, { pauseOnNextStatement: true }); + await Recorder.show(this, RecorderApp.factory(this), { pauseOnNextStatement: true }); // When paused, show inspector. if (this._debugger.isPaused()) - Recorder.showInspector(this); + Recorder.showInspector(this, RecorderApp.factory(this)); + this._debugger.on(Debugger.Events.PausedStateChanged, () => { - Recorder.showInspector(this); + if (this._debugger.isPaused()) + Recorder.showInspector(this, RecorderApp.factory(this)); }); if (debugMode() === 'console') diff --git a/packages/playwright-core/src/server/debugController.ts b/packages/playwright-core/src/server/debugController.ts index 1d87485571a25..2a950d7c6aa64 100644 --- a/packages/playwright-core/src/server/debugController.ts +++ b/packages/playwright-core/src/server/debugController.ts @@ -52,7 +52,6 @@ export class DebugController extends SdkObject { initialize(codegenId: string, sdkLanguage: Language) { this._codegenId = codegenId; this._sdkLanguage = sdkLanguage; - Recorder.setAppFactory(async () => new InspectingRecorderApp(this)); } setAutoCloseAllowed(allowed: boolean) { @@ -62,7 +61,6 @@ export class DebugController extends SdkObject { dispose() { this.setReportStateChanged(false); this.setAutoCloseAllowed(false); - Recorder.setAppFactory(undefined); } setReportStateChanged(enabled: boolean) { @@ -199,7 +197,7 @@ export class DebugController extends SdkObject { const contexts = new Set(); for (const page of this._playwright.allPages()) contexts.add(page.context()); - const result = await Promise.all([...contexts].map(c => Recorder.show(c, { omitCallTracking: true }))); + const result = await Promise.all([...contexts].map(c => Recorder.show(c, () => Promise.resolve(new InspectingRecorderApp(this)), { omitCallTracking: true }))); return result.filter(Boolean) as Recorder[]; } diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index 5654950360048..c70d8e825ac97 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -39,6 +39,7 @@ import type { Dialog } from '../dialog'; import type { ConsoleMessage } from '../console'; import { serializeError } from '../errors'; import { ElementHandleDispatcher } from './elementHandlerDispatcher'; +import { RecorderApp } from '../recorder/recorderApp'; export class BrowserContextDispatcher extends Dispatcher implements channels.BrowserContextChannel { _type_EventTarget = true; @@ -291,7 +292,7 @@ export class BrowserContextDispatcher extends Dispatcher { - await Recorder.show(this._context, params); + await Recorder.show(this._context, RecorderApp.factory(this._context), params); } async pause(params: channels.BrowserContextPauseParams, metadata: CallMetadata) { diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 857df392b3a53..79b1bde22e4e3 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -26,12 +26,13 @@ import { type Language } from './codegen/types'; import { Debugger } from './debugger'; import type { CallMetadata, InstrumentationListener, SdkObject } from './instrumentation'; import { ContextRecorder, generateFrameSelector } from './recorder/contextRecorder'; -import type { IRecorderApp } from './recorder/recorderApp'; -import { EmptyRecorderApp, RecorderApp } from './recorder/recorderApp'; +import { type IRecorderApp } from './recorder/recorderApp'; import { buildFullSelector, metadataToCallLog } from './recorder/recorderUtils'; const recorderSymbol = Symbol('recorderSymbol'); +export type RecorderAppFactory = (recorder: Recorder) => Promise; + export class Recorder implements InstrumentationListener { private _context: BrowserContext; private _mode: Mode; @@ -43,40 +44,38 @@ export class Recorder implements InstrumentationListener { private _userSources = new Map(); private _debugger: Debugger; private _contextRecorder: ContextRecorder; - private _handleSIGINT: boolean | undefined; private _omitCallTracking = false; private _currentLanguage: Language; - private static recorderAppFactory: ((recorder: Recorder) => Promise) | undefined; - - static setAppFactory(recorderAppFactory: ((recorder: Recorder) => Promise) | undefined) { - Recorder.recorderAppFactory = recorderAppFactory; - } - - static showInspector(context: BrowserContext) { + static showInspector(context: BrowserContext, recorderAppFactory: RecorderAppFactory) { const params: channels.BrowserContextRecorderSupplementEnableParams = {}; if (isUnderTest()) params.language = process.env.TEST_INSPECTOR_LANGUAGE; - Recorder.show(context, params).catch(() => {}); + Recorder.show(context, recorderAppFactory, params).catch(() => {}); } - static show(context: BrowserContext, params: channels.BrowserContextRecorderSupplementEnableParams = {}): Promise { + static show(context: BrowserContext, recorderAppFactory: RecorderAppFactory, params: channels.BrowserContextRecorderSupplementEnableParams = {}): Promise { let recorderPromise = (context as any)[recorderSymbol] as Promise; if (!recorderPromise) { - const recorder = new Recorder(context, params); - recorderPromise = recorder.install().then(() => recorder); + recorderPromise = Recorder._create(context, recorderAppFactory, params); (context as any)[recorderSymbol] = recorderPromise; } return recorderPromise; } + private static async _create(context: BrowserContext, recorderAppFactory: RecorderAppFactory, params: channels.BrowserContextRecorderSupplementEnableParams = {}): Promise { + const recorder = new Recorder(context, params); + const recorderApp = await recorderAppFactory(recorder); + await recorder._install(recorderApp); + return recorder; + } + constructor(context: BrowserContext, params: channels.BrowserContextRecorderSupplementEnableParams) { this._mode = params.mode || 'none'; this._contextRecorder = new ContextRecorder(context, params); this._context = context; this._omitCallTracking = !!params.omitCallTracking; this._debugger = context.debugger(); - this._handleSIGINT = params.handleSIGINT; context.instrumentation.addListener(this, context); this._currentLanguage = this._contextRecorder.languageName(); @@ -86,14 +85,7 @@ export class Recorder implements InstrumentationListener { } } - private static async defaultRecorderAppFactory(recorder: Recorder) { - if (process.env.PW_CODEGEN_NO_INSPECTOR) - return new EmptyRecorderApp(); - return await RecorderApp.open(recorder, recorder._context, recorder._handleSIGINT); - } - - async install() { - const recorderApp = await (Recorder.recorderAppFactory || Recorder.defaultRecorderAppFactory)(this); + private async _install(recorderApp: IRecorderApp) { this._recorderApp = recorderApp; recorderApp.once('close', () => { this._debugger.resume(false); @@ -140,7 +132,7 @@ export class Recorder implements InstrumentationListener { this._context.once(BrowserContext.Events.Close, () => { this._contextRecorder.dispose(); this._context.instrumentation.removeListener(this); - recorderApp.close().catch(() => {}); + this._recorderApp?.close().catch(() => {}); }); this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], primaryFileName: string }) => { this._recorderSources = data.sources; @@ -201,7 +193,7 @@ export class Recorder implements InstrumentationListener { this._pausedStateChanged(); this._debugger.on(Debugger.Events.PausedStateChanged, () => this._pausedStateChanged()); - (this._context as any).recorderAppForTest = recorderApp; + (this._context as any).recorderAppForTest = this._recorderApp; } _pausedStateChanged() { diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index 7f9166ae73084..0faf191ea5021 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -24,7 +24,7 @@ import type { CallLog, EventData, Mode, Source } from '@recorder/recorderTypes'; import { isUnderTest } from '../../utils'; import { mime } from '../../utilsBundle'; import { syncLocalStorageWithSettings } from '../launchApp'; -import type { Recorder } from '../recorder'; +import type { Recorder, RecorderAppFactory } from '../recorder'; import type { BrowserContext } from '../browserContext'; import { launchApp } from '../launchApp'; @@ -113,7 +113,15 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { await mainFrame.goto(serverSideCallMetadata(), 'https://playwright/index.html'); } - static async open(recorder: Recorder, inspectedContext: BrowserContext, handleSIGINT: boolean | undefined): Promise { + static factory(context: BrowserContext): RecorderAppFactory { + return async recorder => { + if (process.env.PW_CODEGEN_NO_INSPECTOR) + return new EmptyRecorderApp(); + return await RecorderApp._open(recorder, context); + }; + } + + private static async _open(recorder: Recorder, inspectedContext: BrowserContext): Promise { const sdkLanguage = inspectedContext.attribution.playwright.options.sdkLanguage; const headed = !!inspectedContext._browser.options.headful; const recorderPlaywright = (require('../playwright').createPlaywright as typeof import('../playwright').createPlaywright)({ sdkLanguage: 'javascript', isInternalPlaywright: true }); @@ -125,7 +133,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { noDefaultViewport: true, headless: !!process.env.PWTEST_CLI_HEADLESS || (isUnderTest() && !headed), useWebSocket: !!process.env.PWTEST_RECORDER_PORT, - handleSIGINT, + handleSIGINT: false, args: process.env.PWTEST_RECORDER_PORT ? [`--remote-debugging-port=${process.env.PWTEST_RECORDER_PORT}`] : [], executablePath: inspectedContext._browser.options.isChromium ? inspectedContext._browser.options.customExecutablePath : undefined, } @@ -170,11 +178,11 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { async setSelector(selector: string, userGesture?: boolean): Promise { if (userGesture) { - if (this._recorder.mode() === 'inspecting') { + if (this._recorder?.mode() === 'inspecting') { this._recorder.setMode('standby'); this._page.bringToFront(); } else { - this._recorder.setMode('recording'); + this._recorder?.setMode('recording'); } } await this._page.mainFrame().evaluateExpression(((data: { selector: string, userGesture?: boolean }) => { diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index cc3d07ba577b6..d0f5d43f795e6 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -1753,7 +1753,6 @@ export type BrowserContextRecorderSupplementEnableParams = { device?: string, saveStorage?: string, outputFile?: string, - handleSIGINT?: boolean, omitCallTracking?: boolean, }; export type BrowserContextRecorderSupplementEnableOptions = { @@ -1766,7 +1765,6 @@ export type BrowserContextRecorderSupplementEnableOptions = { device?: string, saveStorage?: string, outputFile?: string, - handleSIGINT?: boolean, omitCallTracking?: boolean, }; export type BrowserContextRecorderSupplementEnableResult = void; diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index c0a8d097951d6..d7c33b05d804e 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -1189,7 +1189,6 @@ BrowserContext: device: string? saveStorage: string? outputFile: string? - handleSIGINT: boolean? omitCallTracking: boolean? newCDPSession: