Skip to content

Commit

Permalink
chore: centralize playwright creation, bind context listeners to inst…
Browse files Browse the repository at this point in the history
…ance (#5217)
  • Loading branch information
pavelfeldman committed Jan 30, 2021
1 parent 7fe7d0e commit 9755191
Show file tree
Hide file tree
Showing 26 changed files with 104 additions and 112 deletions.
10 changes: 5 additions & 5 deletions src/browserServerImpl.ts
Expand Up @@ -70,7 +70,7 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {

this._browser = browser;
this._wsEndpoint = '';
this._process = browser._options.browserProcess.process!;
this._process = browser.options.browserProcess.process!;

let readyCallback = () => {};
this._ready = new Promise<void>(f => readyCallback = f);
Expand All @@ -86,7 +86,7 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {
this._clientAttached(socket);
});

browser._options.browserProcess.onclose = (exitCode, signal) => {
browser.options.browserProcess.onclose = (exitCode, signal) => {
this._server.close();
this.emit('close', exitCode, signal);
};
Expand All @@ -101,11 +101,11 @@ export class BrowserServerImpl extends EventEmitter implements BrowserServer {
}

async close(): Promise<void> {
await this._browser._options.browserProcess.close();
await this._browser.options.browserProcess.close();
}

async kill(): Promise<void> {
await this._browser._options.browserProcess.kill();
await this._browser.options.browserProcess.kill();
}

private _clientAttached(socket: ws) {
Expand Down Expand Up @@ -158,7 +158,7 @@ class ConnectedBrowser extends BrowserDispatcher {
async newContext(params: channels.BrowserNewContextParams): Promise<{ context: channels.BrowserContextChannel }> {
if (params.recordVideo) {
// TODO: we should create a separate temp directory or accept a launchServer parameter.
params.recordVideo.dir = this._object._options.downloadsPath!;
params.recordVideo.dir = this._object.options.downloadsPath!;
}
const result = await super.newContext(params);
const dispatcher = result.context as BrowserContextDispatcher;
Expand Down
11 changes: 2 additions & 9 deletions src/cli/driver.ts
Expand Up @@ -18,15 +18,12 @@

import * as fs from 'fs';
import * as path from 'path';
import { installInspectorController } from '../server/supplements/inspectorController';
import { DispatcherConnection } from '../dispatchers/dispatcher';
import { PlaywrightDispatcher } from '../dispatchers/playwrightDispatcher';
import { installBrowsersWithProgressBar } from '../install/installer';
import { Transport } from '../protocol/transport';
import { Playwright } from '../server/playwright';
import { createPlaywright } from '../server/playwright';
import { gracefullyCloseAll } from '../server/processLauncher';
import { installHarTracer } from '../trace/harTracer';
import { installTracer } from '../trace/tracer';
import { BrowserName } from '../utils/browserPaths';

export function printApiJson() {
Expand All @@ -38,10 +35,6 @@ export function printProtocol() {
}

export function runServer() {
installInspectorController();
installTracer();
installHarTracer();

const dispatcherConnection = new DispatcherConnection();
const transport = new Transport(process.stdout, process.stdin);
transport.onmessage = message => dispatcherConnection.dispatch(JSON.parse(message));
Expand All @@ -56,7 +49,7 @@ export function runServer() {
process.exit(0);
};

const playwright = new Playwright(__dirname, require('../../browsers.json')['browsers']);
const playwright = createPlaywright();
new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), playwright);
}

Expand Down
8 changes: 4 additions & 4 deletions src/dispatchers/browserContextDispatcher.ts
Expand Up @@ -28,7 +28,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
private _context: BrowserContext;

constructor(scope: DispatcherScope, context: BrowserContext) {
super(scope, context, 'BrowserContext', { isChromium: context._browser._options.isChromium }, true);
super(scope, context, 'BrowserContext', { isChromium: context._browser.options.isChromium }, true);
this._context = context;

for (const page of context.pages())
Expand All @@ -41,7 +41,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
context.on(BrowserContext.Events.StdOut, data => this._dispatchEvent('stdout', { data: Buffer.from(data, 'utf8').toString('base64') }));
context.on(BrowserContext.Events.StdErr, data => this._dispatchEvent('stderr', { data: Buffer.from(data, 'utf8').toString('base64') }));

if (context._browser._options.name === 'chromium') {
if (context._browser.options.name === 'chromium') {
for (const page of (context as CRBrowserContext).backgroundPages())
this._dispatchEvent('crBackgroundPage', { page: new PageDispatcher(this._scope, page) });
context.on(CRBrowserContext.CREvents.BackgroundPage, page => this._dispatchEvent('crBackgroundPage', { page: new PageDispatcher(this._scope, page) }));
Expand Down Expand Up @@ -139,7 +139,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
}

async pause() {
if (!this._context._browser._options.headful)
if (!this._context._browser.options.headful)
return;
const recorder = await RecorderSupplement.getOrCreate(this._context, 'pause', {
language: 'javascript',
Expand All @@ -149,7 +149,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
}

async crNewCDPSession(params: channels.BrowserContextCrNewCDPSessionParams): Promise<channels.BrowserContextCrNewCDPSessionResult> {
if (!this._object._browser._options.isChromium)
if (!this._object._browser.options.isChromium)
throw new Error(`CDP session is only available in Chromium`);
const crBrowserContext = this._object as CRBrowserContext;
return { session: new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession((params.page as PageDispatcher)._object)) };
Expand Down
8 changes: 4 additions & 4 deletions src/dispatchers/browserDispatcher.ts
Expand Up @@ -24,7 +24,7 @@ import { PageDispatcher } from './pageDispatcher';

export class BrowserDispatcher extends Dispatcher<Browser, channels.BrowserInitializer> implements channels.BrowserChannel {
constructor(scope: DispatcherScope, browser: Browser) {
super(scope, browser, 'Browser', { version: browser.version(), name: browser._options.name }, true);
super(scope, browser, 'Browser', { version: browser.version(), name: browser.options.name }, true);
browser.on(Browser.Events.Disconnected, () => this._didClose());
}

Expand All @@ -45,21 +45,21 @@ export class BrowserDispatcher extends Dispatcher<Browser, channels.BrowserIniti
}

async crNewBrowserCDPSession(): Promise<channels.BrowserCrNewBrowserCDPSessionResult> {
if (!this._object._options.isChromium)
if (!this._object.options.isChromium)
throw new Error(`CDP session is only available in Chromium`);
const crBrowser = this._object as CRBrowser;
return { session: new CDPSessionDispatcher(this._scope, await crBrowser.newBrowserCDPSession()) };
}

async crStartTracing(params: channels.BrowserCrStartTracingParams): Promise<void> {
if (!this._object._options.isChromium)
if (!this._object.options.isChromium)
throw new Error(`Tracing is only available in Chromium`);
const crBrowser = this._object as CRBrowser;
await crBrowser.startTracing(params.page ? (params.page as PageDispatcher)._object : undefined, params);
}

async crStopTracing(): Promise<channels.BrowserCrStopTracingResult> {
if (!this._object._options.isChromium)
if (!this._object.options.isChromium)
throw new Error(`Tracing is only available in Chromium`);
const crBrowser = this._object as CRBrowser;
const buffer = await crBrowser.stopTracing();
Expand Down
12 changes: 2 additions & 10 deletions src/inprocess.ts
Expand Up @@ -15,22 +15,14 @@
*/

import { DispatcherConnection } from './dispatchers/dispatcher';
import { Playwright as PlaywrightImpl } from './server/playwright';
import { createPlaywright } from './server/playwright';
import type { Playwright as PlaywrightAPI } from './client/playwright';
import { PlaywrightDispatcher } from './dispatchers/playwrightDispatcher';
import { Connection } from './client/connection';
import { BrowserServerLauncherImpl } from './browserServerImpl';
import { installInspectorController } from './server/supplements/inspectorController';
import { installTracer } from './trace/tracer';
import { installHarTracer } from './trace/harTracer';
import * as path from 'path';

function setupInProcess(): PlaywrightAPI {
const playwright = new PlaywrightImpl(path.join(__dirname, '..'), require('../browsers.json')['browsers']);

installInspectorController();
installTracer();
installHarTracer();
const playwright = createPlaywright();

const clientConnection = new Connection();
const dispatcherConnection = new DispatcherConnection();
Expand Down
12 changes: 2 additions & 10 deletions src/remote/playwrightServer.ts
Expand Up @@ -17,20 +17,13 @@
import * as debug from 'debug';
import * as http from 'http';
import * as WebSocket from 'ws';
import { installInspectorController } from '../server/supplements/inspectorController';
import { DispatcherConnection } from '../dispatchers/dispatcher';
import { PlaywrightDispatcher } from '../dispatchers/playwrightDispatcher';
import { Playwright } from '../server/playwright';
import { createPlaywright } from '../server/playwright';
import { gracefullyCloseAll } from '../server/processLauncher';
import { installTracer } from '../trace/tracer';
import { installHarTracer } from '../trace/harTracer';

const debugLog = debug('pw:server');

installInspectorController();
installTracer();
installHarTracer();

export class PlaywrightServer {
private _server: http.Server | undefined;
private _client: WebSocket | undefined;
Expand Down Expand Up @@ -62,8 +55,7 @@ export class PlaywrightServer {
this._onDisconnect();
});
dispatcherConnection.onmessage = message => ws.send(JSON.stringify(message));
const playwright = new Playwright(__dirname, require('../../browsers.json')['browsers']);
new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), playwright);
new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), createPlaywright());
});
}

Expand Down
7 changes: 5 additions & 2 deletions src/server/android/android.ts
Expand Up @@ -22,7 +22,7 @@ import * as stream from 'stream';
import * as util from 'util';
import * as ws from 'ws';
import { createGuid, makeWaitForNextTask } from '../../utils/utils';
import { BrowserOptions, BrowserProcess } from '../browser';
import { BrowserOptions, BrowserProcess, PlaywrightOptions } from '../browser';
import { BrowserContext, validateBrowserContextOptions } from '../browserContext';
import { ProgressController } from '../progress';
import { CRBrowser } from '../chromium/crBrowser';
Expand Down Expand Up @@ -57,9 +57,11 @@ export class Android {
private _backend: Backend;
private _devices = new Map<string, AndroidDevice>();
readonly _timeoutSettings: TimeoutSettings;
readonly _playwrightOptions: PlaywrightOptions;

constructor(backend: Backend) {
constructor(backend: Backend, playwrightOptions: PlaywrightOptions) {
this._backend = backend;
this._playwrightOptions = playwrightOptions;
this._timeoutSettings = new TimeoutSettings();
}

Expand Down Expand Up @@ -255,6 +257,7 @@ export class AndroidDevice extends EventEmitter {
this._browserConnections.add(androidBrowser);

const browserOptions: BrowserOptions = {
...this._android._playwrightOptions,
name: 'clank',
isChromium: true,
slowMo: 0,
Expand Down
17 changes: 11 additions & 6 deletions src/server/browser.ts
Expand Up @@ -15,7 +15,7 @@
*/

import * as types from './types';
import { BrowserContext, Video } from './browserContext';
import { BrowserContext, ContextListener, Video } from './browserContext';
import { Page } from './page';
import { EventEmitter } from 'events';
import { Download } from './download';
Expand All @@ -30,7 +30,11 @@ export interface BrowserProcess {
close(): Promise<void>;
}

export type BrowserOptions = types.UIOptions & {
export type PlaywrightOptions = {
contextListeners: ContextListener[]
};

export type BrowserOptions = PlaywrightOptions & {
name: string,
isChromium: boolean,
downloadsPath?: string,
Expand All @@ -40,22 +44,23 @@ export type BrowserOptions = types.UIOptions & {
proxy?: ProxySettings,
protocolLogger: types.ProtocolLogger,
browserLogsCollector: RecentLogsCollector,
slowMo?: number,
};

export abstract class Browser extends EventEmitter {
static Events = {
Disconnected: 'disconnected',
};

readonly _options: BrowserOptions;
readonly options: BrowserOptions;
private _downloads = new Map<string, Download>();
_defaultContext: BrowserContext | null = null;
private _startedClosing = false;
readonly _idToVideo = new Map<string, Video>();

constructor(options: BrowserOptions) {
super();
this._options = options;
this.options = options;
}

abstract newContext(options?: types.BrowserContextOptions): Promise<BrowserContext>;
Expand All @@ -71,7 +76,7 @@ export abstract class Browser extends EventEmitter {
}

_downloadCreated(page: Page, uuid: string, url: string, suggestedFilename?: string) {
const download = new Download(page, this._options.downloadsPath || '', uuid, url, suggestedFilename);
const download = new Download(page, this.options.downloadsPath || '', uuid, url, suggestedFilename);
this._downloads.set(uuid, download);
}

Expand Down Expand Up @@ -117,7 +122,7 @@ export abstract class Browser extends EventEmitter {
async close() {
if (!this._startedClosing) {
this._startedClosing = true;
await this._options.browserProcess.close();
await this.options.browserProcess.close();
}
if (this.isConnected())
await new Promise(x => this.once(Browser.Events.Disconnected, x));
Expand Down
12 changes: 5 additions & 7 deletions src/server/browserContext.ts
Expand Up @@ -94,8 +94,6 @@ export interface ContextListener {
onContextDidDestroy(context: BrowserContext): Promise<void>;
}

export const contextListeners = new Set<ContextListener>();

export abstract class BrowserContext extends EventEmitter {
static Events = {
Close: 'close',
Expand Down Expand Up @@ -140,7 +138,7 @@ export abstract class BrowserContext extends EventEmitter {
}

async _initialize() {
for (const listener of contextListeners)
for (const listener of this._browser.options.contextListeners)
await listener.onContextCreated(this);
}

Expand Down Expand Up @@ -259,7 +257,7 @@ export abstract class BrowserContext extends EventEmitter {
}

protected _authenticateProxyViaHeader() {
const proxy = this._options.proxy || this._browser._options.proxy || { username: undefined, password: undefined };
const proxy = this._options.proxy || this._browser.options.proxy || { username: undefined, password: undefined };
const { username, password } = proxy;
if (username) {
this._options.httpCredentials = { username, password: password! };
Expand All @@ -272,7 +270,7 @@ export abstract class BrowserContext extends EventEmitter {
}

protected _authenticateProxyViaCredentials() {
const proxy = this._options.proxy || this._browser._options.proxy;
const proxy = this._options.proxy || this._browser.options.proxy;
if (!proxy)
return;
const { username, password } = proxy;
Expand All @@ -294,7 +292,7 @@ export abstract class BrowserContext extends EventEmitter {
this.emit(BrowserContext.Events.BeforeClose);
this._closedStatus = 'closing';

for (const listener of contextListeners)
for (const listener of this._browser.options.contextListeners)
await listener.onContextWillDestroy(this);

// Collect videos/downloads that we will await.
Expand Down Expand Up @@ -323,7 +321,7 @@ export abstract class BrowserContext extends EventEmitter {
await this._browser.close();

// Bookkeeping.
for (const listener of contextListeners)
for (const listener of this._browser.options.contextListeners)
await listener.onContextDidDestroy(this);
this._didCloseInternal();
}
Expand Down
7 changes: 5 additions & 2 deletions src/server/browserType.ts
Expand Up @@ -21,7 +21,7 @@ import * as util from 'util';
import { BrowserContext, normalizeProxySettings, validateBrowserContextOptions } from './browserContext';
import * as browserPaths from '../utils/browserPaths';
import { ConnectionTransport } from './transport';
import { BrowserOptions, Browser, BrowserProcess } from './browser';
import { BrowserOptions, Browser, BrowserProcess, PlaywrightOptions } from './browser';
import { launchProcess, Env, envArrayToObject } from './processLauncher';
import { PipeTransport } from './pipeTransport';
import { Progress, ProgressController } from './progress';
Expand All @@ -42,8 +42,10 @@ export abstract class BrowserType {
private _executablePath: string;
private _browserDescriptor: browserPaths.BrowserDescriptor;
readonly _browserPath: string;
readonly _playwrightOptions: PlaywrightOptions;

constructor(packagePath: string, browser: browserPaths.BrowserDescriptor) {
constructor(packagePath: string, browser: browserPaths.BrowserDescriptor, playwrightOptions: PlaywrightOptions) {
this._playwrightOptions = playwrightOptions;
this._name = browser.name;
const browsersPath = browserPaths.browsersPath(packagePath);
this._browserDescriptor = browser;
Expand Down Expand Up @@ -87,6 +89,7 @@ export abstract class BrowserType {
if ((options as any).__testHookBeforeCreateBrowser)
await (options as any).__testHookBeforeCreateBrowser();
const browserOptions: BrowserOptions = {
...this._playwrightOptions,
name: this._name,
isChromium: this._name === 'chromium',
slowMo: options.slowMo,
Expand Down

0 comments on commit 9755191

Please sign in to comment.