From 4d7e531520993b815f92e09fde5cb2aefbaa3478 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 13 Feb 2020 17:46:40 -0800 Subject: [PATCH] fix(webkit): wait for the pipe ready on windows (#997) --- src/server/webkit.ts | 10 +++------- src/transport.ts | 32 ++++++++++++++++++++++++++++++++ test/playwright.spec.js | 1 - 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/server/webkit.ts b/src/server/webkit.ts index c94ae57ef1..7482296771 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -31,7 +31,7 @@ import * as os from 'os'; import { assert, helper } from '../helper'; import { kBrowserCloseMessageId } from '../webkit/wkConnection'; import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType'; -import { ConnectionTransport } from '../transport'; +import { ConnectionTransport, DeferWriteTransport } from '../transport'; import * as ws from 'ws'; import * as uuidv4 from 'uuid/v4'; import { ConnectOptions, LaunchType } from '../browser'; @@ -122,7 +122,7 @@ export class WebKit implements BrowserType { webkitExecutable = executablePath; } - let transport: PipeTransport | undefined = undefined; + let transport: ConnectionTransport | undefined = undefined; let browserServer: BrowserServer | undefined = undefined; const { launchedProcess, gracefullyClose } = await launchProcess({ executablePath: webkitExecutable!, @@ -151,7 +151,7 @@ export class WebKit implements BrowserType { const timeoutError = new TimeoutError(`Timed out after ${timeout} ms while trying to connect to WebKit! The only WebKit revision guaranteed to work is r${this._revision}`); await waitForLine(launchedProcess, launchedProcess.stdout, /^Web Inspector is reading from pipe #3$/, timeout, timeoutError); - transport = new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream); + transport = new DeferWriteTransport(new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream)); browserServer = new BrowserServer(launchedProcess, gracefullyClose, launchType === 'server' ? await wrapTransportWithWebSocket(transport, port || 0) : null); return { browserServer, transport }; } @@ -278,11 +278,8 @@ async function wrapTransportWithWebSocket(transport: ConnectionTransport, port: const browserContextIds = new Map(); const pageProxyIds = new Map(); const sockets = new Set(); - let transportReadyCallback: () => void; - const transportReady = new Promise(f => transportReadyCallback = f); transport.onmessage = message => { - transportReadyCallback(); const parsedMessage = JSON.parse(message); if ('id' in parsedMessage) { if (parsedMessage.id === -9999) @@ -418,6 +415,5 @@ async function wrapTransportWithWebSocket(transport: ConnectionTransport, port: const address = server.address(); if (typeof address === 'string') return address + '/' + guid; - await transportReady; return 'ws://127.0.0.1:' + address.port + '/' + guid; } diff --git a/src/transport.ts b/src/transport.ts index f6d4feb7ba..a472687149 100644 --- a/src/transport.ts +++ b/src/transport.ts @@ -83,3 +83,35 @@ export class SlowMoTransport { this._delegate.close(); } } + +export class DeferWriteTransport implements ConnectionTransport { + private _delegate: ConnectionTransport; + private _readPromise: Promise; + + onmessage?: (message: string) => void; + onclose?: () => void; + + constructor(transport: ConnectionTransport) { + this._delegate = transport; + let callback: () => void; + this._readPromise = new Promise(f => callback = f); + this._delegate.onmessage = s => { + callback(); + if (this.onmessage) + this.onmessage(s); + }; + this._delegate.onclose = () => { + if (this.onclose) + this.onclose(); + }; + } + + async send(s: string) { + await this._readPromise; + this._delegate.send(s); + } + + close() { + this._delegate.close(); + } +} diff --git a/test/playwright.spec.js b/test/playwright.spec.js index 4cb870eb5e..05bbbc9f57 100644 --- a/test/playwright.spec.js +++ b/test/playwright.spec.js @@ -127,7 +127,6 @@ module.exports.describe = ({testRunner, product, playwrightPath}) => { state.tearDown = async () => { await Promise.all(contexts.map(c => c.close())); - expect((await state.browser.contexts()).length).toBe(0, `"${test.fullName}" leaked a context`); if (rl) { rl.removeListener('line', onLine); rl.close();