chore: cleanup more non-rpc code (#3471)

- Replaces BrowserServer class with BrowserProcess struct.
- Removes src/api.ts.
- Removes helper.installApiHooks.
- Removes BrowserType.launchServer.
This commit is contained in:
Dmitry Gozman 2020-08-14 13:19:12 -07:00 committed by GitHub
parent dec8fb7890
commit ae4280a12b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 52 additions and 175 deletions

View File

@ -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.
*/
export { Accessibility } from './accessibility';
export { Browser } from './browser';
export { BrowserContext } from './browserContext';
export { ConsoleMessage } from './console';
export { Dialog } from './dialog';
export { Download } from './download';
export { ElementHandle } from './dom';
export { FileChooser } from './fileChooser';
export { Logger } from './loggerSink';
export { TimeoutError } from './errors';
export { Frame } from './frames';
export { Keyboard, Mouse } from './input';
export { JSHandle } from './javascript';
export { Request, Response, Route } from './network';
export { Page, Worker } from './page';
export { Selectors } from './selectors';
export { CRBrowser as ChromiumBrowser } from './chromium/crBrowser';
export { CRBrowserContext as ChromiumBrowserContext } from './chromium/crBrowser';
export { CRCoverage as ChromiumCoverage } from './chromium/crCoverage';
export { CRSession as CDPSession } from './chromium/crConnection';
export { FFBrowser as FirefoxBrowser } from './firefox/ffBrowser';
export { WKBrowser as WebKitBrowser } from './webkit/wkBrowser';
export { BrowserType } from './server/browserType';
export { BrowserServer } from './server/browserServer';

View File

@ -19,11 +19,18 @@ import { BrowserContext, BrowserContextBase } from './browserContext';
import { Page } from './page';
import { EventEmitter } from 'events';
import { Download } from './download';
import type { BrowserServer } from './server/browserServer';
import { Events } from './events';
import { Loggers } from './logger';
import { ProxySettings } from './types';
import { LoggerSink } from './loggerSink';
import { ChildProcess } from 'child_process';
export interface BrowserProcess {
onclose: ((exitCode: number | null, signal: string | null) => void) | undefined;
process: ChildProcess;
kill(): Promise<void>;
close(): Promise<void>;
}
export type BrowserOptions = {
name: string,
@ -32,7 +39,7 @@ export type BrowserOptions = {
headful?: boolean,
persistent?: types.BrowserContextOptions, // Undefined means no persistent context.
slowMo?: number,
ownedServer?: BrowserServer,
browserProcess: BrowserProcess,
proxy?: ProxySettings,
};
@ -102,12 +109,7 @@ export abstract class BrowserBase extends EventEmitter implements Browser {
async close() {
if (!this._startedClosing) {
this._startedClosing = true;
if (this._options.ownedServer) {
await this._options.ownedServer.close();
} else {
await Promise.all(this.contexts().map(context => context.close()));
this._disconnect();
}
await this._options.browserProcess.close();
}
if (this.isConnected())
await new Promise(x => this.once(Events.Browser.Disconnected, x));

View File

@ -80,30 +80,6 @@ class Helper {
return helper.evaluationString(fun, arg);
}
static installApiHooks(className: string, classType: any) {
for (const methodName of Reflect.ownKeys(classType.prototype)) {
const method = Reflect.get(classType.prototype, methodName);
if (methodName === 'constructor' || typeof methodName !== 'string' || methodName.startsWith('_') || typeof method !== 'function')
continue;
const isAsync = method.constructor.name === 'AsyncFunction';
if (!isAsync)
continue;
const override = function(this: any, ...args: any[]) {
const syncStack: any = {};
Error.captureStackTrace(syncStack);
return method.call(this, ...args).catch((e: any) => {
const stack = syncStack.stack.substring(syncStack.stack.indexOf('\n') + 1);
const clientStack = stack.substring(stack.indexOf('\n'));
if (e instanceof Error && e.stack && !e.stack.includes(clientStack))
e.stack += '\n -- ASYNC --\n' + stack;
throw e;
});
};
Object.defineProperty(override, 'name', { writable: false, value: methodName });
Reflect.set(classType.prototype, methodName, override);
}
}
static addEventListener(
emitter: EventEmitter,
eventName: (string | symbol),

View File

@ -20,7 +20,6 @@ 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';
@ -59,8 +58,7 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {
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._process = browser._options.browserProcess.process;
this._server.on('connection', (socket: ws, req) => {
if (req.url !== '/' + token) {
@ -70,10 +68,10 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {
this._clientAttached(socket);
});
browserServer.on(Events.BrowserServer.Close, (exitCode, signal) => {
browser._options.browserProcess.onclose = (exitCode, signal) => {
this._server.close();
this.emit('close', exitCode, signal);
});
};
}
process(): ChildProcess {
@ -85,13 +83,11 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {
}
async close(): Promise<void> {
const browserServer = this._browser._options.ownedServer!;
await browserServer.close();
await this._browser._options.browserProcess.close();
}
async kill(): Promise<void> {
const browserServer = this._browser._options.ownedServer!;
await browserServer.kill();
await this._browser._options.browserProcess.kill();
}
private _clientAttached(socket: ws) {

View File

@ -1,57 +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 { ChildProcess } from 'child_process';
import { EventEmitter } from 'events';
export class BrowserServer extends EventEmitter {
private _process: ChildProcess;
private _gracefullyClose: () => Promise<void>;
private _kill: () => Promise<void>;
constructor(process: ChildProcess, gracefullyClose: () => Promise<void>, kill: () => Promise<void>) {
super();
this._process = process;
this._gracefullyClose = gracefullyClose;
this._kill = kill;
}
process(): ChildProcess {
return this._process;
}
async kill(): Promise<void> {
await this._kill();
}
async close(): Promise<void> {
await this._gracefullyClose();
}
async _closeOrKill(timeout: number): Promise<void> {
let timer: NodeJS.Timer;
try {
await Promise.race([
this.close(),
new Promise((resolve, reject) => timer = setTimeout(reject, timeout)),
]);
} catch (ignored) {
await this.kill().catch(ignored => {}); // Make sure to await actual process exit.
} finally {
clearTimeout(timer!);
}
}
}

View File

@ -19,14 +19,12 @@ import * as os from 'os';
import * as path from 'path';
import * as util from 'util';
import { BrowserContext, verifyProxySettings, validateBrowserContextOptions } from '../browserContext';
import { BrowserServer } from './browserServer';
import * as browserPaths from '../install/browserPaths';
import { Loggers } from '../logger';
import { ConnectionTransport, WebSocketTransport } from '../transport';
import { BrowserBase, BrowserOptions, Browser } from '../browser';
import { BrowserBase, BrowserOptions, Browser, BrowserProcess } from '../browser';
import { assert, helper } from '../helper';
import { launchProcess, Env, waitForLine } from './processLauncher';
import { Events } from '../events';
import { PipeTransport } from './pipeTransport';
import { Progress, runAbortableTask } from '../progress';
import * as types from '../types';
@ -46,7 +44,6 @@ export interface BrowserType {
executablePath(): string;
name(): string;
launch(options?: LaunchNonPersistentOptions): Promise<Browser>;
launchServer(options?: LaunchServerOptions): Promise<BrowserServer>;
launchPersistentContext(userDataDir: string, options?: LaunchPersistentOptions): Promise<BrowserContext>;
}
@ -105,7 +102,7 @@ export abstract class BrowserTypeBase implements BrowserType {
async _innerLaunch(progress: Progress, options: LaunchOptions, logger: Loggers, persistent: types.BrowserContextOptions | undefined, userDataDir?: string): Promise<BrowserBase> {
options.proxy = options.proxy ? verifyProxySettings(options.proxy) : undefined;
const { browserServer, downloadsPath, transport } = await this._launchServer(progress, options, !!persistent, logger, userDataDir);
const { browserProcess, downloadsPath, transport } = await this._launchProcess(progress, options, !!persistent, logger, userDataDir);
if ((options as any).__testHookBeforeCreateBrowser)
await (options as any).__testHookBeforeCreateBrowser();
const browserOptions: BrowserOptions = {
@ -115,7 +112,7 @@ export abstract class BrowserTypeBase implements BrowserType {
headful: !options.headless,
loggers: logger,
downloadsPath,
ownedServer: browserServer,
browserProcess,
proxy: options.proxy,
};
copyTestHooks(options, browserOptions);
@ -127,17 +124,7 @@ export abstract class BrowserTypeBase implements BrowserType {
return browser;
}
async launchServer(options: LaunchServerOptions = {}): Promise<BrowserServer> {
assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launchServer`. Use `browserType.launchPersistentContext` instead');
options = validateLaunchOptions(options);
const loggers = new Loggers(options.logger);
return runAbortableTask(async progress => {
const { browserServer } = await this._launchServer(progress, options, false, loggers);
return browserServer;
}, loggers.browser, TimeoutSettings.timeout(options), 'browserType.launchServer');
}
private async _launchServer(progress: Progress, options: LaunchServerOptions, isPersistent: boolean, loggers: Loggers, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, transport: ConnectionTransport }> {
private async _launchProcess(progress: Progress, options: LaunchServerOptions, isPersistent: boolean, loggers: Loggers, userDataDir?: string): Promise<{ browserProcess: BrowserProcess, downloadsPath: string, transport: ConnectionTransport }> {
const {
ignoreDefaultArgs = false,
args = [],
@ -190,7 +177,7 @@ export abstract class BrowserTypeBase implements BrowserType {
// 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;
let browserServer: BrowserServer | undefined = undefined;
let browserProcess: BrowserProcess | undefined = undefined;
const { launchedProcess, gracefullyClose, kill } = await launchProcess({
executablePath: executable,
args: this._amendArguments(browserArguments),
@ -210,12 +197,17 @@ export abstract class BrowserTypeBase implements BrowserType {
this._attemptToGracefullyCloseBrowser(transport!);
},
onExit: (exitCode, signal) => {
if (browserServer)
browserServer.emit(Events.BrowserServer.Close, exitCode, signal);
if (browserProcess && browserProcess.onclose)
browserProcess.onclose(exitCode, signal);
},
});
browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill);
progress.cleanupWhenAborted(() => browserServer && browserServer._closeOrKill(progress.timeUntilDeadline()));
browserProcess = {
onclose: undefined,
process: launchedProcess,
close: gracefullyClose,
kill
};
progress.cleanupWhenAborted(() => browserProcess && closeOrKill(browserProcess, progress.timeUntilDeadline()));
if (this._webSocketNotPipe) {
const match = await waitForLine(progress, launchedProcess, this._webSocketNotPipe.stream === 'stdout' ? launchedProcess.stdout : launchedProcess.stderr, this._webSocketNotPipe.webSocketRegex);
@ -225,7 +217,7 @@ export abstract class BrowserTypeBase implements BrowserType {
const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
transport = new PipeTransport(stdio[3], stdio[4], loggers.browser);
}
return { browserServer, downloadsPath, transport };
return { browserProcess, downloadsPath, transport };
}
abstract _defaultArgs(options: types.LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[];
@ -247,3 +239,17 @@ function validateLaunchOptions<Options extends types.LaunchOptionsBase>(options:
const { devtools = false, headless = !helper.isDebugMode() && !devtools } = options;
return { ...options, devtools, headless };
}
async function closeOrKill(browserProcess: BrowserProcess, timeout: number): Promise<void> {
let timer: NodeJS.Timer;
try {
await Promise.race([
browserProcess.close(),
new Promise((resolve, reject) => timer = setTimeout(reject, timeout)),
]);
} catch (ignored) {
await browserProcess.kill().catch(ignored => {}); // Make sure to await actual process exit.
} finally {
clearTimeout(timer!);
}
}

View File

@ -25,7 +25,6 @@ import { Page } from '../page';
import { TimeoutSettings } from '../timeoutSettings';
import { WebSocketTransport } from '../transport';
import * as types from '../types';
import { BrowserServer } from './browserServer';
import { launchProcess, waitForLine } from './processLauncher';
import { BrowserContext } from '../browserContext';
import type {BrowserWindow} from 'electron';
@ -33,6 +32,7 @@ import { runAbortableTask, ProgressController } from '../progress';
import { EventEmitter } from 'events';
import { helper } from '../helper';
import { LoggerSink } from '../loggerSink';
import { BrowserProcess } from '../browser';
export type ElectronLaunchOptionsBase = {
args?: string[],
@ -202,8 +202,13 @@ export class Electron {
const chromeMatch = await waitForLine(progress, launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/);
const chromeTransport = await WebSocketTransport.connect(progress, chromeMatch[1]);
const browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill);
const browser = await CRBrowser.connect(chromeTransport, { name: 'electron', headful: true, loggers, persistent: { viewport: null }, ownedServer: browserServer });
const browserProcess: BrowserProcess = {
onclose: undefined,
process: launchedProcess,
close: gracefullyClose,
kill
};
const browser = await CRBrowser.connect(chromeTransport, { name: 'electron', headful: true, loggers, persistent: { viewport: null }, browserProcess });
app = new ElectronApplication(loggers, browser, nodeConnection);
await app._init();
return app;

View File

@ -15,8 +15,6 @@
*/
import * as types from '../types';
import * as api from '../api';
import { helper } from '../helper';
import { TimeoutError } from '../errors';
import { DeviceDescriptors } from '../deviceDescriptors';
import { Chromium } from './chromium';
@ -25,11 +23,6 @@ import { Firefox } from './firefox';
import { selectors } from '../selectors';
import * as browserPaths from '../install/browserPaths';
for (const className in api) {
if (typeof (api as any)[className] === 'function')
helper.installApiHooks(className[0].toLowerCase() + className.substring(1), (api as any)[className]);
}
export class Playwright {
readonly selectors = selectors;
readonly devices: types.Devices;