diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index 8df03bb9f1..8ca25fbf74 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -38,6 +38,7 @@ import { helper } from './helper'; import { RecentLogsCollector } from '../common/debugLogger'; import type { CallMetadata } from './instrumentation'; import { SdkObject } from './instrumentation'; +import { ManualPromise } from '../utils/manualPromise'; export const kNoXServerRunningError = 'Looks like you launched a headed browser without having a XServer running.\n' + 'Set either \'headless: true\' or use \'xvfb-run \' before running Playwright.\n\n<3 Playwright Team'; @@ -186,9 +187,8 @@ export abstract class BrowserType extends SdkObject { await registryExecutable.validateHostRequirements(this._playwrightOptions.sdkLanguage); } - let wsEndpointCallback: ((wsEndpoint: string) => void) | undefined; - const shouldWaitForWSListening = options.useWebSocket || options.args?.some(a => a.startsWith('--remote-debugging-port')); - const waitForWSEndpoint = shouldWaitForWSListening ? new Promise(f => wsEndpointCallback = f) : undefined; + const waitForWSEndpoint = (options.useWebSocket || options.args?.some(a => a.startsWith('--remote-debugging-port'))) ? new ManualPromise() : undefined; + const waitForJuggler = this._name === 'firefox' ? new ManualPromise() : undefined; // Note: it is important to define these variables before launchProcess, so that we don't get // "Cannot access 'browserServer' before initialization" if something went wrong. let transport: ConnectionTransport | undefined = undefined; @@ -201,11 +201,13 @@ export abstract class BrowserType extends SdkObject { handleSIGTERM, handleSIGHUP, log: (message: string) => { - if (wsEndpointCallback) { + if (waitForWSEndpoint) { const match = message.match(/DevTools listening on (.*)/); if (match) - wsEndpointCallback(match[1]); + waitForWSEndpoint.resolve(match[1]); } + if (waitForJuggler && message.includes('Juggler listening to the pipe')) + waitForJuggler.resolve(); progress.log(message); browserLogsCollector.log(message); }, @@ -220,6 +222,8 @@ export abstract class BrowserType extends SdkObject { this._attemptToGracefullyCloseBrowser(transport!); }, onExit: (exitCode, signal) => { + // Unblock launch when browser prematurely exits. + waitForJuggler?.resolve(); if (browserProcess && browserProcess.onclose) browserProcess.onclose(exitCode, signal); }, @@ -244,9 +248,8 @@ export abstract class BrowserType extends SdkObject { kill }; progress.cleanupWhenAborted(() => closeOrKill(progress.timeUntilDeadline())); - let wsEndpoint: string | undefined; - if (shouldWaitForWSListening) - wsEndpoint = await waitForWSEndpoint; + const wsEndpoint = await waitForWSEndpoint; + await waitForJuggler; if (options.useWebSocket) { transport = await WebSocketTransport.connect(progress, wsEndpoint!); } else { diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index 791f91a902..761b426155 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -49,10 +49,11 @@ export class CRConnection extends EventEmitter { this._transport = transport; this._protocolLogger = protocolLogger; this._browserLogsCollector = browserLogsCollector; - this._transport.onmessage = this._onMessage.bind(this); - this._transport.onclose = this._onClose.bind(this); this.rootSession = new CRSession(this, '', 'browser', ''); this._sessions.set('', this.rootSession); + this._transport.onmessage = this._onMessage.bind(this); + // onclose should be set last, since it can be immediately called. + this._transport.onclose = this._onClose.bind(this); } static fromSession(session: CRSession): CRConnection { diff --git a/packages/playwright-core/src/server/firefox/ffConnection.ts b/packages/playwright-core/src/server/firefox/ffConnection.ts index 41bcfe4636..3585570e92 100644 --- a/packages/playwright-core/src/server/firefox/ffConnection.ts +++ b/packages/playwright-core/src/server/firefox/ffConnection.ts @@ -58,8 +58,6 @@ export class FFConnection extends EventEmitter { this._lastId = 0; this._callbacks = new Map(); - this._transport.onmessage = this._onMessage.bind(this); - this._transport.onclose = this._onClose.bind(this); this._sessions = new Map(); this._closed = false; @@ -68,6 +66,10 @@ export class FFConnection extends EventEmitter { this.off = super.removeListener; this.removeListener = super.removeListener; this.once = super.once; + + this._transport.onmessage = this._onMessage.bind(this); + // onclose should be set last, since it can be immediately called. + this._transport.onclose = this._onClose.bind(this); } async send( diff --git a/packages/playwright-core/src/server/pipeTransport.ts b/packages/playwright-core/src/server/pipeTransport.ts index eb5c63cee3..c873dd7d92 100644 --- a/packages/playwright-core/src/server/pipeTransport.ts +++ b/packages/playwright-core/src/server/pipeTransport.ts @@ -20,26 +20,37 @@ import { makeWaitForNextTask } from '../utils'; import { debugLogger } from '../common/debugLogger'; export class PipeTransport implements ConnectionTransport { + private _pipeRead: NodeJS.ReadableStream; private _pipeWrite: NodeJS.WritableStream; private _pendingMessage = ''; private _waitForNextTask = makeWaitForNextTask(); private _closed = false; + private _onclose?: () => void; onmessage?: (message: ProtocolResponse) => void; - onclose?: () => void; constructor(pipeWrite: NodeJS.WritableStream, pipeRead: NodeJS.ReadableStream) { + this._pipeRead = pipeRead; this._pipeWrite = pipeWrite; pipeRead.on('data', buffer => this._dispatch(buffer)); pipeRead.on('close', () => { this._closed = true; - if (this.onclose) - this.onclose.call(null); + if (this._onclose) + this._onclose.call(null); }); pipeRead.on('error', e => debugLogger.log('error', e)); pipeWrite.on('error', e => debugLogger.log('error', e)); this.onmessage = undefined; - this.onclose = undefined; + } + + get onclose() { + return this._onclose; + } + + set onclose(onclose: undefined | (() => void)) { + this._onclose = onclose; + if (onclose && !this._pipeRead.readable) + onclose(); } send(message: ProtocolRequest) { diff --git a/packages/playwright-core/src/server/webkit/wkConnection.ts b/packages/playwright-core/src/server/webkit/wkConnection.ts index a2f6759f6b..cfa3848d53 100644 --- a/packages/playwright-core/src/server/webkit/wkConnection.ts +++ b/packages/playwright-core/src/server/webkit/wkConnection.ts @@ -47,14 +47,15 @@ export class WKConnection { constructor(transport: ConnectionTransport, onDisconnect: () => void, protocolLogger: ProtocolLogger, browserLogsCollector: RecentLogsCollector) { this._transport = transport; - this._transport.onmessage = this._dispatchMessage.bind(this); - this._transport.onclose = this._onClose.bind(this); this._onDisconnect = onDisconnect; this._protocolLogger = protocolLogger; this._browserLogsCollector = browserLogsCollector; this.browserSession = new WKSession(this, '', kBrowserClosedError, (message: any) => { this.rawSend(message); }); + this._transport.onmessage = this._dispatchMessage.bind(this); + // onclose should be set last, since it can be immediately called. + this._transport.onclose = this._onClose.bind(this); } nextMessageId(): number {