Skip to content

Commit

Permalink
feat(firefox): migrate to the pipe channel (#4068)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman authored Oct 6, 2020
1 parent e6869ed commit ce7aa7a
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 38 deletions.
2 changes: 1 addition & 1 deletion browsers.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
},
{
"name": "firefox",
"revision": "1182",
"revision": "1184",
"download": true
},
{
Expand Down
21 changes: 5 additions & 16 deletions src/server/browserType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import * as path from 'path';
import * as util from 'util';
import { BrowserContext, normalizeProxySettings, validateBrowserContextOptions } from './browserContext';
import * as browserPaths from '../utils/browserPaths';
import { ConnectionTransport, WebSocketTransport } from './transport';
import { ConnectionTransport } from './transport';
import { BrowserOptions, Browser, BrowserProcess } from './browser';
import { launchProcess, Env, waitForLine, envArrayToObject } from './processLauncher';
import { launchProcess, Env, envArrayToObject } from './processLauncher';
import { PipeTransport } from './pipeTransport';
import { Progress, ProgressController } from './progress';
import * as types from './types';
Expand All @@ -35,22 +35,18 @@ const mkdtempAsync = util.promisify(fs.mkdtemp);
const existsAsync = (path: string): Promise<boolean> => new Promise(resolve => fs.stat(path, err => resolve(!err)));
const DOWNLOADS_FOLDER = path.join(os.tmpdir(), 'playwright_downloads-');

type WebSocketNotPipe = { webSocketRegex: RegExp, stream: 'stdout' | 'stderr' };

export abstract class BrowserType {
private _name: string;
private _executablePath: string;
private _webSocketNotPipe: WebSocketNotPipe | null;
private _browserDescriptor: browserPaths.BrowserDescriptor;
readonly _browserPath: string;

constructor(packagePath: string, browser: browserPaths.BrowserDescriptor, webSocketOrPipe: WebSocketNotPipe | null) {
constructor(packagePath: string, browser: browserPaths.BrowserDescriptor) {
this._name = browser.name;
const browsersPath = browserPaths.browsersPath(packagePath);
this._browserDescriptor = browser;
this._browserPath = browserPaths.browserDirectory(browsersPath, browser);
this._executablePath = browserPaths.executablePath(this._browserPath, browser) || '';
this._webSocketNotPipe = webSocketOrPipe;
}

executablePath(): string {
Expand Down Expand Up @@ -175,7 +171,6 @@ export abstract class BrowserType {
handleSIGTERM,
handleSIGHUP,
progress,
pipe: !this._webSocketNotPipe,
tempDirectories,
attemptToGracefullyClose: async () => {
if ((options as any).__testHookGracefullyClose)
Expand All @@ -198,14 +193,8 @@ export abstract class BrowserType {
};
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);
const innerEndpoint = match[1];
transport = await WebSocketTransport.connect(progress, innerEndpoint);
} else {
const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
transport = new PipeTransport(stdio[3], stdio[4]);
}
const stdio = launchedProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
transport = new PipeTransport(stdio[3], stdio[4]);
return { browserProcess, downloadsPath, transport };
}

Expand Down
25 changes: 11 additions & 14 deletions src/server/chromium/chromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,9 @@ import { isDebugMode, getFromENV } from '../../utils/utils';

export class Chromium extends BrowserType {
private _devtools: CRDevTools | undefined;
private _debugPort: number | undefined;

constructor(packagePath: string, browser: BrowserDescriptor) {
const debugPortStr = getFromENV('PLAYWRIGHT_CHROMIUM_DEBUG_PORT');
const debugPort: number | undefined = debugPortStr ? +debugPortStr : undefined;
if (debugPort !== undefined) {
if (Number.isNaN(debugPort))
throw new Error(`PLAYWRIGHT_CHROMIUM_DEBUG_PORT must be a number, but is set to "${debugPortStr}"`);
}

super(packagePath, browser, debugPort ? { webSocketRegex: /^DevTools listening on (ws:\/\/.*)$/, stream: 'stderr' } : null);
this._debugPort = debugPort;
super(packagePath, browser);
if (isDebugMode())
this._devtools = this._createDevTools();
}
Expand Down Expand Up @@ -110,10 +101,16 @@ export class Chromium extends BrowserType {
throw new Error('Arguments can not specify page to be opened');
const chromeArguments = [...DEFAULT_ARGS];
chromeArguments.push(`--user-data-dir=${userDataDir}`);
if (this._debugPort !== undefined)
chromeArguments.push('--remote-debugging-port=' + this._debugPort);
else
chromeArguments.push('--remote-debugging-pipe');

const debugPortStr = getFromENV('PLAYWRIGHT_CHROMIUM_DEBUG_PORT');
if (debugPortStr) {
const debugPort = +debugPortStr;
if (Number.isNaN(debugPort))
throw new Error(`PLAYWRIGHT_CHROMIUM_DEBUG_PORT must be a number, but is set to "${debugPortStr}"`);
chromeArguments.push('--remote-debugging-port=' + debugPort);
}

chromeArguments.push('--remote-debugging-pipe');
if (options.devtools)
chromeArguments.push('--auto-open-devtools-for-tabs');
if (options.headless) {
Expand Down
1 change: 0 additions & 1 deletion src/server/electron/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export class Electron {
handleSIGTERM,
handleSIGHUP,
progress,
pipe: true,
cwd: options.cwd,
tempDirectories: [],
attemptToGracefullyClose: () => app!.close(),
Expand Down
5 changes: 2 additions & 3 deletions src/server/firefox/firefox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ import * as types from '../types';

export class Firefox extends BrowserType {
constructor(packagePath: string, browser: BrowserDescriptor) {
const webSocketRegex = /^Juggler listening on (ws:\/\/.*)$/;
super(packagePath, browser, { webSocketRegex, stream: 'stdout' });
super(packagePath, browser);
}

_connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<FFBrowser> {
Expand Down Expand Up @@ -82,7 +81,7 @@ export class Firefox extends BrowserType {
firefoxArguments.push('-foreground');
}
firefoxArguments.push(`-profile`, userDataDir);
firefoxArguments.push('-juggler', '0');
firefoxArguments.push('-juggler-pipe');
firefoxArguments.push(...args);
if (isPersistent)
firefoxArguments.push('about:blank');
Expand Down
3 changes: 1 addition & 2 deletions src/server/processLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export type LaunchProcessOptions = {
handleSIGINT?: boolean,
handleSIGTERM?: boolean,
handleSIGHUP?: boolean,
pipe?: boolean,
pipeStdin?: boolean,
tempDirectories: string[],

Expand Down Expand Up @@ -83,7 +82,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
const cleanup = () => helper.removeFolders(options.tempDirectories);

const progress = options.progress;
const stdio: ('ignore' | 'pipe')[] = options.pipe ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['ignore', 'pipe', 'pipe'];
const stdio: ('ignore' | 'pipe')[] = ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'];
if (options.pipeStdin)
stdio[0] = 'pipe';
progress.log(`<launching> ${options.executablePath} ${options.args.join(' ')}`);
Expand Down
2 changes: 1 addition & 1 deletion src/server/webkit/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import * as types from '../types';

export class WebKit extends BrowserType {
constructor(packagePath: string, browser: BrowserDescriptor) {
super(packagePath, browser, null /* use pipe not websocket */);
super(packagePath, browser);
}

_connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<WKBrowser> {
Expand Down

0 comments on commit ce7aa7a

Please sign in to comment.