Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: centralize playwright creation, bind context listeners to inst… #5217

Merged
merged 1 commit into from
Jan 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/browserServerImpl.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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