From 18b2cf5ec70ef74ea3143cb7922a7a9c933d3b34 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 13 Aug 2020 13:24:49 -0700 Subject: [PATCH] feat(rpc): use rpc protocol for browserType.connect (#3380) --- src/rpc/browserServerImpl.ts | 147 ++++++++++++++++++++++ src/rpc/channels.ts | 90 ------------- src/rpc/client/api.ts | 2 +- src/rpc/client/browser.ts | 12 +- src/rpc/client/browserContext.ts | 2 +- src/rpc/client/browserServer.ts | 53 -------- src/rpc/client/browserType.ts | 99 ++++++++++++--- src/rpc/client/connection.ts | 4 - src/rpc/client/page.ts | 2 +- src/rpc/client/types.ts | 35 +++++- src/rpc/inprocess.ts | 8 +- src/rpc/protocol.yml | 67 ---------- src/rpc/server/browserDispatcher.ts | 14 ++- src/rpc/server/browserServerDispatcher.ts | 44 ------- src/rpc/server/browserTypeDispatcher.ts | 18 +-- src/rpc/validator.ts | 33 ----- test/browsertype-connect.spec.ts | 14 +-- test/browsertype-launch-server.spec.ts | 28 ++--- test/chromium/launcher.spec.ts | 4 +- test/multiclient.spec.ts | 16 +-- 20 files changed, 306 insertions(+), 386 deletions(-) create mode 100644 src/rpc/browserServerImpl.ts delete mode 100644 src/rpc/client/browserServer.ts delete mode 100644 src/rpc/server/browserServerDispatcher.ts diff --git a/src/rpc/browserServerImpl.ts b/src/rpc/browserServerImpl.ts new file mode 100644 index 0000000000..015a302726 --- /dev/null +++ b/src/rpc/browserServerImpl.ts @@ -0,0 +1,147 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the 'License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { LaunchServerOptions } from './client/types'; +import { BrowserTypeBase } from '../server/browserType'; +import * as ws from 'ws'; +import { helper } from '../helper'; +import { BrowserBase } from '../browser'; +import { ChildProcess } from 'child_process'; +import { Events } from '../events'; +import { EventEmitter } from 'ws'; +import { DispatcherScope, DispatcherConnection } from './server/dispatcher'; +import { BrowserTypeDispatcher } from './server/browserTypeDispatcher'; +import { BrowserDispatcher } from './server/browserDispatcher'; +import { BrowserContextDispatcher } from './server/browserContextDispatcher'; +import { BrowserNewContextParams, BrowserContextChannel } from './channels'; +import { BrowserServerLauncher, BrowserServer } from './client/browserType'; + +export class BrowserServerLauncherImpl implements BrowserServerLauncher { + private _browserType: BrowserTypeBase; + + constructor(browserType: BrowserTypeBase) { + this._browserType = browserType; + } + + async launchServer(options: LaunchServerOptions = {}): Promise { + const browser = await this._browserType.launch(options); + return new BrowserServerImpl(this._browserType, browser as BrowserBase, options.port); + } +} + +export class BrowserServerImpl extends EventEmitter implements BrowserServer { + private _server: ws.Server; + private _browserType: BrowserTypeBase; + private _browser: BrowserBase; + private _wsEndpoint: string; + private _process: ChildProcess; + + constructor(browserType: BrowserTypeBase, browser: BrowserBase, port: number = 0) { + super(); + + this._browserType = browserType; + this._browser = browser; + + const token = helper.guid(); + this._server = new ws.Server({ port }); + const address = this._server.address(); + this._wsEndpoint = typeof address === 'string' ? `${address}/${token}` : `ws://127.0.0.1:${address.port}/${token}`; + const browserServer = browser._options.ownedServer!; + this._process = browserServer.process(); + + this._server.on('connection', (socket: ws, req) => { + if (req.url !== '/' + token) { + socket.close(); + return; + } + this._clientAttached(socket); + }); + + browserServer.on(Events.BrowserServer.Close, (exitCode, signal) => { + this._server.close(); + this.emit('close', exitCode, signal); + }); + } + + process(): ChildProcess { + return this._process; + } + + wsEndpoint(): string { + return this._wsEndpoint; + } + + async close(): Promise { + const browserServer = this._browser._options.ownedServer!; + await browserServer.close(); + } + + async kill(): Promise { + const browserServer = this._browser._options.ownedServer!; + await browserServer.kill(); + } + + private _clientAttached(socket: ws) { + const connection = new DispatcherConnection(); + connection.onmessage = message => { + if (socket.readyState !== ws.CLOSING) + socket.send(JSON.stringify(message)); + }; + socket.on('message', (message: string) => { + connection.dispatch(JSON.parse(Buffer.from(message).toString())); + }); + socket.on('error', () => {}); + const browserType = new BrowserTypeDispatcher(connection.rootDispatcher(), this._browserType); + const browser = new ConnectedBrowser(browserType._scope, this._browser); + socket.on('close', () => { + // Avoid sending any more messages over closed socket. + connection.onmessage = () => {}; + // Cleanup contexts upon disconnect. + browser.close().catch(e => {}); + }); + } +} + +class ConnectedBrowser extends BrowserDispatcher { + private _contexts: BrowserContextDispatcher[] = []; + _closed = false; + + constructor(scope: DispatcherScope, browser: BrowserBase) { + super(scope, browser, 'connectedBrowser'); + } + + async newContext(params: BrowserNewContextParams): Promise<{ context: BrowserContextChannel }> { + const result = await super.newContext(params); + this._contexts.push(result.context as BrowserContextDispatcher); + return result; + } + + async close(): Promise { + // Only close our own contexts. + await Promise.all(this._contexts.map(context => context.close())); + this._didClose(); + } + + _didClose() { + if (!this._closed) { + // We come here multiple times: + // - from ConnectedBrowser.close(); + // - from underlying Browser.on('close'). + this._closed = true; + super._didClose(); + } + } +} diff --git a/src/rpc/channels.ts b/src/rpc/channels.ts index 232a813cd6..47095bdd3f 100644 --- a/src/rpc/channels.ts +++ b/src/rpc/channels.ts @@ -141,23 +141,9 @@ export type BrowserTypeInitializer = { name: string, }; export interface BrowserTypeChannel extends Channel { - connect(params: BrowserTypeConnectParams): Promise; launch(params: BrowserTypeLaunchParams): Promise; - launchServer(params: BrowserTypeLaunchServerParams): Promise; launchPersistentContext(params: BrowserTypeLaunchPersistentContextParams): Promise; } -export type BrowserTypeConnectParams = { - wsEndpoint: string, - slowMo?: number, - timeout?: number, -}; -export type BrowserTypeConnectOptions = { - slowMo?: number, - timeout?: number, -}; -export type BrowserTypeConnectResult = { - browser: BrowserChannel, -}; export type BrowserTypeLaunchParams = { executablePath?: string, args?: string[], @@ -213,61 +199,6 @@ export type BrowserTypeLaunchOptions = { export type BrowserTypeLaunchResult = { browser: BrowserChannel, }; -export type BrowserTypeLaunchServerParams = { - executablePath?: string, - args?: string[], - ignoreAllDefaultArgs?: boolean, - ignoreDefaultArgs?: string[], - handleSIGINT?: boolean, - handleSIGTERM?: boolean, - handleSIGHUP?: boolean, - timeout?: number, - env?: { - name: string, - value: string, - }[], - headless?: boolean, - devtools?: boolean, - proxy?: { - server: string, - bypass?: string, - username?: string, - password?: string, - }, - downloadsPath?: string, - firefoxUserPrefs?: any, - chromiumSandbox?: boolean, - port?: number, -}; -export type BrowserTypeLaunchServerOptions = { - executablePath?: string, - args?: string[], - ignoreAllDefaultArgs?: boolean, - ignoreDefaultArgs?: string[], - handleSIGINT?: boolean, - handleSIGTERM?: boolean, - handleSIGHUP?: boolean, - timeout?: number, - env?: { - name: string, - value: string, - }[], - headless?: boolean, - devtools?: boolean, - proxy?: { - server: string, - bypass?: string, - username?: string, - password?: string, - }, - downloadsPath?: string, - firefoxUserPrefs?: any, - chromiumSandbox?: boolean, - port?: number, -}; -export type BrowserTypeLaunchServerResult = { - server: BrowserServerChannel, -}; export type BrowserTypeLaunchPersistentContextParams = { userDataDir: string, executablePath?: string, @@ -385,27 +316,6 @@ export type BrowserTypeLaunchPersistentContextResult = { context: BrowserContextChannel, }; -// ----------- BrowserServer ----------- -export type BrowserServerInitializer = { - wsEndpoint: string, - pid: number, -}; -export interface BrowserServerChannel extends Channel { - on(event: 'close', callback: (params: BrowserServerCloseEvent) => void): this; - close(params?: BrowserServerCloseParams): Promise; - kill(params?: BrowserServerKillParams): Promise; -} -export type BrowserServerCloseEvent = { - exitCode?: number, - signal?: string, -}; -export type BrowserServerCloseParams = {}; -export type BrowserServerCloseOptions = {}; -export type BrowserServerCloseResult = void; -export type BrowserServerKillParams = {}; -export type BrowserServerKillOptions = {}; -export type BrowserServerKillResult = void; - // ----------- Browser ----------- export type BrowserInitializer = { version: string, diff --git a/src/rpc/client/api.ts b/src/rpc/client/api.ts index de10c3e9ab..9ade7ef1ee 100644 --- a/src/rpc/client/api.ts +++ b/src/rpc/client/api.ts @@ -17,7 +17,7 @@ export { Accessibility } from './accessibility'; export { Browser } from './browser'; export { BrowserContext } from './browserContext'; -export { BrowserServer } from './browserServer'; +export { BrowserServer } from './browserType'; export { BrowserType } from './browserType'; export { ConsoleMessage } from './consoleMessage'; export { Dialog } from './dialog'; diff --git a/src/rpc/client/browser.ts b/src/rpc/client/browser.ts index acba1f4254..2596fd789d 100644 --- a/src/rpc/client/browser.ts +++ b/src/rpc/client/browser.ts @@ -41,11 +41,7 @@ export class Browser extends ChannelOwner { constructor(parent: ChannelOwner, type: string, guid: string, initializer: BrowserInitializer) { super(parent, type, guid, initializer); this._browserType = parent as BrowserType; - this._channel.on('close', () => { - this._isConnected = false; - this.emit(Events.Browser.Disconnected); - this._isClosedOrClosing = true; - }); + this._channel.on('close', () => this._didClose()); this._closedPromise = new Promise(f => this.once(Events.Browser.Disconnected, f)); } @@ -95,4 +91,10 @@ export class Browser extends ChannelOwner { await this._closedPromise; }); } + + _didClose() { + this._isConnected = false; + this.emit(Events.Browser.Disconnected); + this._isClosedOrClosing = true; + } } diff --git a/src/rpc/client/browserContext.ts b/src/rpc/client/browserContext.ts index 8f7725d823..e74cfc2437 100644 --- a/src/rpc/client/browserContext.ts +++ b/src/rpc/client/browserContext.ts @@ -214,7 +214,7 @@ export class BrowserContext extends ChannelOwner { - static from(server: BrowserServerChannel): BrowserServer { - return (server as any)._object; - } - - constructor(parent: ChannelOwner, type: string, guid: string, initializer: BrowserServerInitializer) { - super(parent, type, guid, initializer); - this._channel.on('close', ({ exitCode, signal }) => { - this.emit(Events.BrowserServer.Close, exitCode === undefined ? null : exitCode, signal === undefined ? null : signal); - }); - } - - process(): ChildProcess { - return { pid: this._initializer.pid } as any; - } - - wsEndpoint(): string { - return this._initializer.wsEndpoint; - } - - async kill(): Promise { - return this._wrapApiCall('browserServer.kill', async () => { - await this._channel.kill(); - }); - } - - async close(): Promise { - return this._wrapApiCall('browserServer.close', async () => { - await this._channel.close(); - }); - } -} diff --git a/src/rpc/client/browserType.ts b/src/rpc/client/browserType.ts index c040c55907..f3fcb30555 100644 --- a/src/rpc/client/browserType.ts +++ b/src/rpc/client/browserType.ts @@ -14,16 +14,34 @@ * limitations under the License. */ -import { BrowserTypeChannel, BrowserTypeInitializer, BrowserTypeLaunchParams, BrowserTypeLaunchServerParams, BrowserTypeLaunchPersistentContextParams } from '../channels'; +import { BrowserTypeChannel, BrowserTypeInitializer, BrowserTypeLaunchParams, BrowserTypeLaunchPersistentContextParams } from '../channels'; import { Browser } from './browser'; import { BrowserContext } from './browserContext'; import { ChannelOwner } from './channelOwner'; -import { BrowserServer } from './browserServer'; import { headersObjectToArray, envObjectToArray } from '../../converters'; -import { assert } from '../../helper'; +import { assert, helper } from '../../helper'; import { LaunchOptions, LaunchServerOptions, ConnectOptions, LaunchPersistentContextOptions } from './types'; +import * as WebSocket from 'ws'; +import { Connection } from './connection'; +import { serializeError } from '../serializers'; +import { Events } from './events'; +import { TimeoutSettings } from '../../timeoutSettings'; +import { ChildProcess } from 'child_process'; + +export interface BrowserServerLauncher { + launchServer(options?: LaunchServerOptions): Promise; +} + +export interface BrowserServer { + process(): ChildProcess; + wsEndpoint(): string; + close(): Promise; + kill(): Promise; +} export class BrowserType extends ChannelOwner { + private _timeoutSettings = new TimeoutSettings(); + _serverLauncher?: BrowserServerLauncher; static from(browserType: BrowserTypeChannel): BrowserType { return (browserType as any)._object; @@ -60,17 +78,9 @@ export class BrowserType extends ChannelOwner { - const logger = options.logger; - options = { ...options, logger: undefined }; - return this._wrapApiCall('browserType.launchServer', async () => { - const launchServerOptions: BrowserTypeLaunchServerParams = { - ...options, - ignoreDefaultArgs: Array.isArray(options.ignoreDefaultArgs) ? options.ignoreDefaultArgs : undefined, - ignoreAllDefaultArgs: !!options.ignoreDefaultArgs && !Array.isArray(options.ignoreDefaultArgs), - env: options.env ? envObjectToArray(options.env) : undefined, - }; - return BrowserServer.from((await this._channel.launchServer(launchServerOptions)).server); - }, logger); + if (!this._serverLauncher) + throw new Error('Launching server is not supported'); + return this._serverLauncher.launchServer(options); } async launchPersistentContext(userDataDir: string, options: LaunchPersistentContextOptions = {}): Promise { @@ -96,11 +106,64 @@ export class BrowserType extends ChannelOwner { const logger = options.logger; - options = { ...options, logger: undefined }; return this._wrapApiCall('browserType.connect', async () => { - const browser = Browser.from((await this._channel.connect(options)).browser); - browser._logger = logger; - return browser; + const connection = new Connection(); + + const ws = new WebSocket(options.wsEndpoint, [], { + perMessageDeflate: false, + maxPayload: 256 * 1024 * 1024, // 256Mb, + handshakeTimeout: this._timeoutSettings.timeout(options), + }); + + // The 'ws' module in node sometimes sends us multiple messages in a single task. + const waitForNextTask = options.slowMo + ? (cb: () => any) => setTimeout(cb, options.slowMo) + : helper.makeWaitForNextTask(); + connection.onmessage = message => { + if (ws.readyState !== WebSocket.OPEN) { + setTimeout(() => { + connection.dispatch({ id: (message as any).id, error: serializeError(new Error('Browser has been closed')) }); + }, 0); + return; + } + ws.send(JSON.stringify(message)); + }; + ws.addEventListener('message', event => { + waitForNextTask(() => connection.dispatch(JSON.parse(event.data))); + }); + + return await new Promise(async (fulfill, reject) => { + if ((options as any).__testHookBeforeCreateBrowser) { + try { + await (options as any).__testHookBeforeCreateBrowser(); + } catch (e) { + reject(e); + } + } + ws.addEventListener('open', async () => { + const browser = (await connection.waitForObjectWithKnownName('connectedBrowser')) as Browser; + browser._logger = logger; + const closeListener = () => { + // Emulate all pages, contexts and the browser closing upon disconnect. + for (const context of browser.contexts()) { + for (const page of context.pages()) + page._onClose(); + context._onClose(); + } + browser._didClose(); + }; + ws.addEventListener('close', closeListener); + browser.on(Events.Browser.Disconnected, () => { + ws.removeEventListener('close', closeListener); + ws.close(); + }); + fulfill(browser); + }); + ws.addEventListener('error', event => { + ws.close(); + reject(new Error('WebSocket error: ' + event.message)); + }); + }); }, logger); } } diff --git a/src/rpc/client/connection.ts b/src/rpc/client/connection.ts index 14d539aed0..63f7ade700 100644 --- a/src/rpc/client/connection.ts +++ b/src/rpc/client/connection.ts @@ -29,7 +29,6 @@ import { ConsoleMessage } from './consoleMessage'; import { Dialog } from './dialog'; import { Download } from './download'; import { parseError } from '../serializers'; -import { BrowserServer } from './browserServer'; import { CDPSession } from './cdpSession'; import { Playwright } from './playwright'; import { Electron, ElectronApplication } from './electron'; @@ -160,9 +159,6 @@ export class Connection { else result = new BrowserContext(parent, type, guid, initializer, browserName); break; - case 'BrowserServer': - result = new BrowserServer(parent, type, guid, initializer); - break; case 'BrowserType': result = new BrowserType(parent, type, guid, initializer); break; diff --git a/src/rpc/client/page.ts b/src/rpc/client/page.ts index 820b155457..646026fe4e 100644 --- a/src/rpc/client/page.ts +++ b/src/rpc/client/page.ts @@ -173,7 +173,7 @@ export class Page extends ChannelOwner { this.emit(Events.Page.Worker, worker); } - private _onClose() { + _onClose() { this._closed = true; this._browserContext._pages.delete(this); this.emit(Events.Page.Close); diff --git a/src/rpc/client/types.ts b/src/rpc/client/types.ts index 30105e6d7b..eda04a8b93 100644 --- a/src/rpc/client/types.ts +++ b/src/rpc/client/types.ts @@ -15,13 +15,15 @@ * limitations under the License. */ -import { BrowserNewContextOptions, BrowserTypeLaunchOptions, BrowserTypeLaunchServerOptions, BrowserTypeConnectParams } from '../channels'; +import { BrowserNewContextOptions, BrowserTypeLaunchOptions } from '../channels'; type LoggerSeverity = 'verbose' | 'info' | 'warning' | 'error'; export interface LoggerSink { isEnabled(name: string, severity: LoggerSeverity): boolean; log(name: string, severity: LoggerSeverity, message: string | Error, args: any[], hints: { color?: string }): void; } +// This is a workaround for the documentation generation. +export interface Logger extends LoggerSink {} export type Size = { width: number, height: number }; export type Point = { x: number, y: number }; @@ -56,6 +58,33 @@ type FirefoxUserPrefs = { }; type LaunchOptionsBase = Omit & LaunchOverrides; export type LaunchOptions = LaunchOptionsBase & FirefoxUserPrefs; -export type LaunchServerOptions = Omit & LaunchOverrides & FirefoxUserPrefs; export type LaunchPersistentContextOptions = LaunchOptionsBase & BrowserContextOptions; -export type ConnectOptions = BrowserTypeConnectParams & { logger?: LoggerSink }; + +export type ConnectOptions = { + wsEndpoint: string, + slowMo?: number, + timeout?: number, + logger?: LoggerSink, +}; +export type LaunchServerOptions = { + executablePath?: string, + args?: string[], + ignoreDefaultArgs?: boolean | string[], + handleSIGINT?: boolean, + handleSIGTERM?: boolean, + handleSIGHUP?: boolean, + timeout?: number, + env?: Env, + headless?: boolean, + devtools?: boolean, + proxy?: { + server: string, + bypass?: string, + username?: string, + password?: string + }, + downloadsPath?: string, + chromiumSandbox?: boolean, + port?: number, + logger?: LoggerSink, +} & FirefoxUserPrefs; diff --git a/src/rpc/inprocess.ts b/src/rpc/inprocess.ts index bc8425ed22..27ecd26eab 100644 --- a/src/rpc/inprocess.ts +++ b/src/rpc/inprocess.ts @@ -21,6 +21,7 @@ import { PlaywrightDispatcher } from './server/playwrightDispatcher'; import { setUseApiName } from '../progress'; import { Connection } from './client/connection'; import { isUnderTest } from '../helper'; +import { BrowserServerLauncherImpl } from './browserServerImpl'; export function setupInProcess(playwright: PlaywrightImpl): PlaywrightAPI { setUseApiName(false); @@ -34,13 +35,16 @@ export function setupInProcess(playwright: PlaywrightImpl): PlaywrightAPI { // Initialize Playwright channel. new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), playwright); - const playwrightAPI = clientConnection.getObjectWithKnownName('Playwright'); + const playwrightAPI = clientConnection.getObjectWithKnownName('Playwright') as PlaywrightAPI; + playwrightAPI.chromium._serverLauncher = new BrowserServerLauncherImpl(playwright.chromium); + playwrightAPI.firefox._serverLauncher = new BrowserServerLauncherImpl(playwright.firefox); + playwrightAPI.webkit._serverLauncher = new BrowserServerLauncherImpl(playwright.webkit); // Switch to async dispatch after we got Playwright object. dispatcherConnection.onmessage = message => setImmediate(() => clientConnection.dispatch(message)); clientConnection.onmessage = message => setImmediate(() => dispatcherConnection.dispatch(message)); if (isUnderTest()) - playwrightAPI._toImpl = (x: any) => dispatcherConnection._dispatchers.get(x._guid)!._object; + (playwrightAPI as any)._toImpl = (x: any) => dispatcherConnection._dispatchers.get(x._guid)!._object; return playwrightAPI; } diff --git a/src/rpc/protocol.yml b/src/rpc/protocol.yml index 9a862f3881..1a25f6cef2 100644 --- a/src/rpc/protocol.yml +++ b/src/rpc/protocol.yml @@ -176,14 +176,6 @@ BrowserType: commands: - connect: - parameters: - wsEndpoint: string - slowMo: number? - timeout: number? - returns: - browser: Browser - launch: parameters: executablePath: string? @@ -221,43 +213,6 @@ BrowserType: returns: browser: Browser - launchServer: - parameters: - executablePath: string? - args: - type: array? - items: string - ignoreAllDefaultArgs: boolean? - ignoreDefaultArgs: - type: array? - items: string - handleSIGINT: boolean? - handleSIGTERM: boolean? - handleSIGHUP: boolean? - timeout: number? - env: - type: array? - items: - type: object - properties: - name: string - value: string - headless: boolean? - devtools: boolean? - proxy: - type: object? - properties: - server: string - bypass: string? - username: string? - password: string? - downloadsPath: string? - firefoxUserPrefs: json? - chromiumSandbox: boolean? - port: number? - returns: - server: BrowserServer - launchPersistentContext: parameters: userDataDir: string @@ -340,28 +295,6 @@ BrowserType: context: BrowserContext - -BrowserServer: - type: interface - - initializer: - wsEndpoint: string - pid: number - - commands: - - close: - - kill: - - events: - - close: - parameters: - exitCode: number? - signal: string? - - Browser: type: interface diff --git a/src/rpc/server/browserDispatcher.ts b/src/rpc/server/browserDispatcher.ts index 9a5857a68e..0c7ca8acb6 100644 --- a/src/rpc/server/browserDispatcher.ts +++ b/src/rpc/server/browserDispatcher.ts @@ -26,12 +26,14 @@ import { PageDispatcher } from './pageDispatcher'; import { headersArrayToObject } from '../../converters'; export class BrowserDispatcher extends Dispatcher implements BrowserChannel { - constructor(scope: DispatcherScope, browser: BrowserBase) { - super(scope, browser, 'Browser', { version: browser.version() }, true); - browser.on(Events.Browser.Disconnected, () => { - this._dispatchEvent('close'); - this._dispose(); - }); + constructor(scope: DispatcherScope, browser: BrowserBase, guid?: string) { + super(scope, browser, 'Browser', { version: browser.version() }, true, guid); + browser.on(Events.Browser.Disconnected, () => this._didClose()); + } + + _didClose() { + this._dispatchEvent('close'); + this._dispose(); } async newContext(params: BrowserNewContextParams): Promise<{ context: BrowserContextChannel }> { diff --git a/src/rpc/server/browserServerDispatcher.ts b/src/rpc/server/browserServerDispatcher.ts deleted file mode 100644 index e01717d526..0000000000 --- a/src/rpc/server/browserServerDispatcher.ts +++ /dev/null @@ -1,44 +0,0 @@ -/** - * Copyright (c) Microsoft Corporation. - * - * Licensed under the Apache License, Version 2.0 (the 'License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { BrowserServer } from '../../server/browserServer'; -import { BrowserServerChannel, BrowserServerInitializer } from '../channels'; -import { Dispatcher, DispatcherScope } from './dispatcher'; -import { Events } from '../../events'; - -export class BrowserServerDispatcher extends Dispatcher implements BrowserServerChannel { - constructor(scope: DispatcherScope, browserServer: BrowserServer) { - super(scope, browserServer, 'BrowserServer', { - wsEndpoint: browserServer.wsEndpoint(), - pid: browserServer.process().pid - }, true); - browserServer.on(Events.BrowserServer.Close, (exitCode, signal) => { - this._dispatchEvent('close', { - exitCode: exitCode === null ? undefined : exitCode, - signal: signal === null ? undefined : signal, - }); - this._dispose(); - }); - } - - async close(): Promise { - await this._object.close(); - } - - async kill(): Promise { - await this._object.close(); - } -} diff --git a/src/rpc/server/browserTypeDispatcher.ts b/src/rpc/server/browserTypeDispatcher.ts index affdc082e1..f27e389cc6 100644 --- a/src/rpc/server/browserTypeDispatcher.ts +++ b/src/rpc/server/browserTypeDispatcher.ts @@ -16,13 +16,11 @@ import { BrowserBase } from '../../browser'; import { BrowserTypeBase, BrowserType } from '../../server/browserType'; -import * as types from '../../types'; import { BrowserDispatcher } from './browserDispatcher'; -import { BrowserChannel, BrowserTypeChannel, BrowserContextChannel, BrowserTypeInitializer, BrowserServerChannel, BrowserTypeLaunchParams, BrowserTypeLaunchPersistentContextParams, BrowserTypeLaunchServerParams } from '../channels'; +import { BrowserChannel, BrowserTypeChannel, BrowserContextChannel, BrowserTypeInitializer, BrowserTypeLaunchParams, BrowserTypeLaunchPersistentContextParams } from '../channels'; import { Dispatcher, DispatcherScope } from './dispatcher'; import { BrowserContextBase } from '../../browserContext'; import { BrowserContextDispatcher } from './browserContextDispatcher'; -import { BrowserServerDispatcher } from './browserServerDispatcher'; import { headersArrayToObject, envArrayToObject } from '../../converters'; export class BrowserTypeDispatcher extends Dispatcher implements BrowserTypeChannel { @@ -54,18 +52,4 @@ export class BrowserTypeDispatcher extends Dispatcher { - const options = { - ...params, - ignoreDefaultArgs: params.ignoreAllDefaultArgs ? true : params.ignoreDefaultArgs, - env: params.env ? envArrayToObject(params.env) : undefined, - }; - return { server: new BrowserServerDispatcher(this._scope, await this._object.launchServer(options)) }; - } - - async connect(params: types.ConnectOptions): Promise<{ browser: BrowserChannel }> { - const browser = await this._object.connect(params); - return { browser: new BrowserDispatcher(this._scope, browser as BrowserBase) }; - } } diff --git a/src/rpc/validator.ts b/src/rpc/validator.ts index 5a2a0d63c6..1815ac1683 100644 --- a/src/rpc/validator.ts +++ b/src/rpc/validator.ts @@ -100,11 +100,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { name: tString, handle: tChannel('ElementHandle'), }); - scheme.BrowserTypeConnectParams = tObject({ - wsEndpoint: tString, - slowMo: tOptional(tNumber), - timeout: tOptional(tNumber), - }); scheme.BrowserTypeLaunchParams = tObject({ executablePath: tOptional(tString), args: tOptional(tArray(tString)), @@ -131,32 +126,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { chromiumSandbox: tOptional(tBoolean), slowMo: tOptional(tNumber), }); - scheme.BrowserTypeLaunchServerParams = tObject({ - executablePath: tOptional(tString), - args: tOptional(tArray(tString)), - ignoreAllDefaultArgs: tOptional(tBoolean), - ignoreDefaultArgs: tOptional(tArray(tString)), - handleSIGINT: tOptional(tBoolean), - handleSIGTERM: tOptional(tBoolean), - handleSIGHUP: tOptional(tBoolean), - timeout: tOptional(tNumber), - env: tOptional(tArray(tObject({ - name: tString, - value: tString, - }))), - headless: tOptional(tBoolean), - devtools: tOptional(tBoolean), - proxy: tOptional(tObject({ - server: tString, - bypass: tOptional(tString), - username: tOptional(tString), - password: tOptional(tString), - })), - downloadsPath: tOptional(tString), - firefoxUserPrefs: tOptional(tAny), - chromiumSandbox: tOptional(tBoolean), - port: tOptional(tNumber), - }); scheme.BrowserTypeLaunchPersistentContextParams = tObject({ userDataDir: tString, executablePath: tOptional(tString), @@ -214,8 +183,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { colorScheme: tOptional(tEnum(['light', 'dark', 'no-preference'])), acceptDownloads: tOptional(tBoolean), }); - scheme.BrowserServerCloseParams = tOptional(tObject({})); - scheme.BrowserServerKillParams = tOptional(tObject({})); scheme.BrowserCloseParams = tOptional(tObject({})); scheme.BrowserNewContextParams = tObject({ noDefaultViewport: tOptional(tBoolean), diff --git a/test/browsertype-connect.spec.ts b/test/browsertype-connect.spec.ts index 8b3c33c409..95d3e134df 100644 --- a/test/browsertype-connect.spec.ts +++ b/test/browsertype-connect.spec.ts @@ -19,7 +19,7 @@ import './base.fixture'; import utils from './utils'; const {FFOX, CHROMIUM, WEBKIT, WIN, USES_HOOKS, CHANNEL} = testOptions; -it.slow()('should be able to reconnect to a browser', async({browserType, defaultBrowserOptions, server, toImpl}) => { +it.skip(USES_HOOKS).slow()('should be able to reconnect to a browser', async({browserType, defaultBrowserOptions, server}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); { const browser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); @@ -35,33 +35,27 @@ it.slow()('should be able to reconnect to a browser', async({browserType, defaul await page.goto(server.EMPTY_PAGE); await browser.close(); } - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it.fail(USES_HOOKS || (CHROMIUM && WIN)).slow()('should handle exceptions during connect', async({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS).fail(CHROMIUM && WIN).slow()('should handle exceptions during connect', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const __testHookBeforeCreateBrowser = () => { throw new Error('Dummy') }; const error = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint(), __testHookBeforeCreateBrowser } as any).catch(e => e); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); expect(error.message).toContain('Dummy'); }); -it('should set the browser connected state', async ({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS)('should set the browser connected state', async ({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); expect(remote.isConnected()).toBe(true); await remote.close(); expect(remote.isConnected()).toBe(false); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it('should throw when used after isConnected returns false', async({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS)('should throw when used after isConnected returns false', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const page = await remote.newPage(); diff --git a/test/browsertype-launch-server.spec.ts b/test/browsertype-launch-server.spec.ts index e5edf6cd31..1e20ab5a13 100644 --- a/test/browsertype-launch-server.spec.ts +++ b/test/browsertype-launch-server.spec.ts @@ -21,7 +21,7 @@ import fs from 'fs'; import utils from './utils'; const {FFOX, CHROMIUM, WEBKIT, WIN, USES_HOOKS, CHANNEL} = testOptions; -it('should work', async({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS)('should work', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const browserContext = await browser.newContext(); @@ -31,12 +31,10 @@ it('should work', async({browserType, defaultBrowserOptions, toImpl}) => { expect(await page.evaluate('11 * 11')).toBe(121); await page.close(); await browser.close(); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it('should fire "disconnected" when closing the server', async({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS)('should fire "disconnected" when closing the server', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const disconnectedEventPromise = new Promise(resolve => browser.once('disconnected', resolve)); @@ -48,7 +46,7 @@ it('should fire "disconnected" when closing the server', async({browserType, def ]); }); -it('should fire "close" event during kill', async({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS)('should fire "close" event during kill', async({browserType, defaultBrowserOptions}) => { const order = []; const browserServer = await browserType.launchServer(defaultBrowserOptions); const closedPromise = new Promise(f => browserServer.on('close', () => { @@ -62,13 +60,13 @@ it('should fire "close" event during kill', async({browserType, defaultBrowserOp expect(order).toEqual(['closed', 'killed']); }); -it('should return child_process instance', async ({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS)('should return child_process instance', async ({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); expect(browserServer.process().pid).toBeGreaterThan(0); await browserServer.close(); }); -it('should fire close event', async ({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS)('should fire close event', async ({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const [result] = await Promise.all([ new Promise(f => (browserServer as any).on('close', (exitCode, signal) => f({ exitCode, signal }))), @@ -78,7 +76,7 @@ it('should fire close event', async ({browserType, defaultBrowserOptions}) => { expect(result['signal']).toBe(null); }); -it('should reject navigation when browser closes', async({browserType, defaultBrowserOptions, server, toImpl}) => { +it.skip(USES_HOOKS)('should reject navigation when browser closes', async({browserType, defaultBrowserOptions, server}) => { server.setRoute('/one-style.css', () => {}); const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); @@ -88,12 +86,10 @@ it('should reject navigation when browser closes', async({browserType, defaultBr await remote.close(); const error = await navigationPromise; expect(error.message).toContain('Navigation failed because page was closed!'); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it('should reject waitForSelector when browser closes', async({browserType, defaultBrowserOptions, server, toImpl}) => { +it.skip(USES_HOOKS)('should reject waitForSelector when browser closes', async({browserType, defaultBrowserOptions, server}) => { server.setRoute('/empty.html', () => {}); const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); @@ -106,24 +102,20 @@ it('should reject waitForSelector when browser closes', async({browserType, defa await remote.close(); const error = await watchdog; expect(error.message).toContain('Protocol error'); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it('should throw if used after disconnect', async({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS)('should throw if used after disconnect', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const page = await remote.newPage(); await remote.close(); const error = await page.evaluate('1 + 1').catch(e => e); expect((error as Error).message).toContain('has been closed'); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it('should emit close events on pages and contexts', async({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS)('should emit close events on pages and contexts', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const context = await remote.newContext(); @@ -137,7 +129,7 @@ it('should emit close events on pages and contexts', async({browserType, default expect(pageClosed).toBeTruthy(); }); -it('should terminate network waiters', async({browserType, defaultBrowserOptions, server}) => { +it.skip(USES_HOOKS)('should terminate network waiters', async({browserType, defaultBrowserOptions, server}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const newPage = await remote.newPage(); diff --git a/test/chromium/launcher.spec.ts b/test/chromium/launcher.spec.ts index 568b500e31..85b7a7adcd 100644 --- a/test/chromium/launcher.spec.ts +++ b/test/chromium/launcher.spec.ts @@ -21,14 +21,14 @@ import { ChromiumBrowser, ChromiumBrowserContext } from '../..'; const {makeUserDataDir, removeUserDataDir} = utils; const {FFOX, CHROMIUM, WEBKIT, WIN, USES_HOOKS} = testOptions; -it.skip(!CHROMIUM)('should throw with remote-debugging-pipe argument', async({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS || !CHROMIUM)('should throw with remote-debugging-pipe argument', async({browserType, defaultBrowserOptions}) => { const options = Object.assign({}, defaultBrowserOptions); options.args = ['--remote-debugging-pipe'].concat(options.args || []); const error = await browserType.launchServer(options).catch(e => e); expect(error.message).toContain('Playwright manages remote debugging connection itself'); }); -it.skip(!CHROMIUM)('should not throw with remote-debugging-port argument', async({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS || !CHROMIUM)('should not throw with remote-debugging-port argument', async({browserType, defaultBrowserOptions}) => { const options = Object.assign({}, defaultBrowserOptions); options.args = ['--remote-debugging-port=0'].concat(options.args || []); const browser = await browserType.launchServer(options); diff --git a/test/multiclient.spec.ts b/test/multiclient.spec.ts index c17c112315..89069de94c 100644 --- a/test/multiclient.spec.ts +++ b/test/multiclient.spec.ts @@ -16,9 +16,9 @@ */ import './base.fixture'; -const {FFOX, CHROMIUM, WEBKIT} = testOptions; +const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = testOptions; -it('should work across sessions', async ({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS)('should work across sessions', async ({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser1 = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); expect(browser1.contexts().length).toBe(0); @@ -35,12 +35,10 @@ it('should work across sessions', async ({browserType, defaultBrowserOptions, to await browser1.close(); await browser2.close(); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it.slow()('should be emitted when: browser gets closed, disconnected or underlying websocket gets closed', async ({browserType, defaultBrowserOptions}) => { +it.skip(USES_HOOKS).slow()('should be emitted when: browser gets closed, disconnected or underlying websocket gets closed', async ({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const originalBrowser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const wsEndpoint = browserServer.wsEndpoint(); @@ -74,7 +72,7 @@ it.slow()('should be emitted when: browser gets closed, disconnected or underlyi expect(disconnectedRemote2).toBe(1); }); -it('should be able to connect multiple times to the same browser', async({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS)('should be able to connect multiple times to the same browser', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser1 = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const browser2 = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); @@ -85,12 +83,10 @@ it('should be able to connect multiple times to the same browser', async({browse const page2 = await browser2.newPage(); expect(await page2.evaluate(() => 7 * 6)).toBe(42); // original browser should still work await browser2.close(); - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); }); -it('should not be able to close remote browser', async({browserType, defaultBrowserOptions, toImpl}) => { +it.skip(USES_HOOKS)('should not be able to close remote browser', async({browserType, defaultBrowserOptions}) => { const browserServer = await browserType.launchServer(defaultBrowserOptions); { const remote = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); @@ -102,7 +98,5 @@ it('should not be able to close remote browser', async({browserType, defaultBrow await remote.newContext(); await remote.close(); } - if (toImpl) - await toImpl(browserServer)._checkLeaks(); await browserServer.close(); });