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: remove unused non-rpc code, test options, infra, bots #3444

Merged
merged 1 commit into from
Aug 13, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,12 @@ jobs:
name: headful-${{ matrix.browser }}-linux-testrun.log
path: testrun.log

rpc_linux:
name: "RPC Linux"
wire_linux:
name: "Wire Linux"
strategy:
fail-fast: false
matrix:
browser: [chromium, firefox, webkit]
transport: [wire, none]
runs-on: ubuntu-18.04
steps:
- uses: actions/checkout@v2
Expand All @@ -204,14 +203,14 @@ jobs:
BROWSER: ${{ matrix.browser }}
DEBUG: "pw:*,-pw:wrapped*,-pw:test*"
DEBUG_FILE: "testrun.log"
PWCHANNEL: ${{ matrix.transport }}
PWWIRE: true
- uses: actions/upload-artifact@v1
if: failure()
with:
name: rpc-${{ matrix.transport }}-${{ matrix.browser }}-linux-output
name: wire-${{ matrix.browser }}-linux-output
path: test/output-${{ matrix.browser }}
- uses: actions/upload-artifact@v1
if: ${{ always() }}
with:
name: rpc-${{ matrix.transport }}-${{ matrix.browser }}-linux-testrun.log
name: wire-${{ matrix.browser }}-linux-testrun.log
path: testrun.log
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
"tsc": "tsc -p .",
"tsc-installer": "tsc -p ./src/install/tsconfig.json",
"doc": "node utils/doclint/cli.js",
"doc-no-channel": "node utils/doclint/cli.js --no-channel",
"test-infra": "node test/runner utils/doclint/check_public_api/test/test.js && node test/runner utils/doclint/preprocessor/test.js",
"lint": "npm run eslint && npm run tsc && npm run doc && npm run doc-no-channel && npm run check-deps && npm run generate-channels && npm run test-types && npm run test-infra",
"lint": "npm run eslint && npm run tsc && npm run doc && npm run check-deps && npm run generate-channels && npm run test-types && npm run test-infra",
"clean": "rimraf lib && rimraf types",
"prepare": "node install-from-github.js",
"build": "node utils/runWebpack.js --mode='development' && tsc -p . && npm run generate-types",
Expand Down
11 changes: 0 additions & 11 deletions src/server/browserServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@

dgozman marked this conversation as resolved.
Show resolved Hide resolved
import { ChildProcess } from 'child_process';
import { EventEmitter } from 'events';
import { WebSocketServer } from './webSocketServer';

export class BrowserServer extends EventEmitter {
private _process: ChildProcess;
private _gracefullyClose: () => Promise<void>;
private _kill: () => Promise<void>;
_webSocketServer: WebSocketServer | null = null;

constructor(process: ChildProcess, gracefullyClose: () => Promise<void>, kill: () => Promise<void>) {
super();
Expand All @@ -35,10 +33,6 @@ export class BrowserServer extends EventEmitter {
return this._process;
}

wsEndpoint(): string {
return this._webSocketServer ? this._webSocketServer.wsEndpoint : '';
}

async kill(): Promise<void> {
await this._kill();
}
Expand All @@ -47,11 +41,6 @@ export class BrowserServer extends EventEmitter {
await this._gracefullyClose();
}

async _checkLeaks(): Promise<void> {
if (this._webSocketServer)
await this._webSocketServer.checkLeaks();
}

async _closeOrKill(timeout: number): Promise<void> {
let timer: NodeJS.Timer;
try {
Expand Down
22 changes: 2 additions & 20 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, verifyProxySettings, validateBrowserContextOptions } from '../browserContext';
import { BrowserServer } from './browserServer';
import * as browserPaths from '../install/browserPaths';
import { Loggers, Logger } from '../logger';
import { Loggers } from '../logger';
import { ConnectionTransport, WebSocketTransport } from '../transport';
import { BrowserBase, BrowserOptions, Browser } from '../browser';
import { assert, helper } from '../helper';
Expand All @@ -31,13 +31,11 @@ import { PipeTransport } from './pipeTransport';
import { Progress, runAbortableTask } from '../progress';
import * as types from '../types';
import { TimeoutSettings } from '../timeoutSettings';
import { WebSocketServer } from './webSocketServer';
import { LoggerSink } from '../loggerSink';
import { validateHostRequirements } from './validateDependencies';

type FirefoxPrefsOptions = { firefoxUserPrefs?: { [key: string]: string | number | boolean } };
type LaunchOptions = types.LaunchOptions & { logger?: LoggerSink };
type ConnectOptions = types.ConnectOptions & { logger?: LoggerSink };


export type LaunchNonPersistentOptions = LaunchOptions & FirefoxPrefsOptions;
Expand All @@ -50,7 +48,6 @@ export interface BrowserType {
launch(options?: LaunchNonPersistentOptions): Promise<Browser>;
launchServer(options?: LaunchServerOptions): Promise<BrowserServer>;
launchPersistentContext(userDataDir: string, options?: LaunchPersistentOptions): Promise<BrowserContext>;
connect(options: ConnectOptions): Promise<Browser>;
}

const mkdirAsync = util.promisify(fs.mkdir);
Expand Down Expand Up @@ -134,26 +131,12 @@ export abstract class BrowserTypeBase implements BrowserType {
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);
const { port = 0 } = options;
return runAbortableTask(async progress => {
const { browserServer, transport } = await this._launchServer(progress, options, false, loggers);
browserServer._webSocketServer = this._startWebSocketServer(transport, loggers.browser, port);
const { browserServer } = await this._launchServer(progress, options, false, loggers);
return browserServer;
}, loggers.browser, TimeoutSettings.timeout(options), 'browserType.launchServer');
}

async connect(options: ConnectOptions): Promise<Browser> {
const loggers = new Loggers(options.logger);
return runAbortableTask(async progress => {
const transport = await WebSocketTransport.connect(progress, options.wsEndpoint);
progress.cleanupWhenAborted(() => transport.closeAndWait());
if ((options as any).__testHookBeforeCreateBrowser)
await (options as any).__testHookBeforeCreateBrowser();
const browser = await this._connectToTransport(transport, { name: this._name, slowMo: options.slowMo, loggers });
return browser;
}, loggers.browser, TimeoutSettings.timeout(options), 'browserType.connect');
}

private async _launchServer(progress: Progress, options: LaunchServerOptions, isPersistent: boolean, loggers: Loggers, userDataDir?: string): Promise<{ browserServer: BrowserServer, downloadsPath: string, transport: ConnectionTransport }> {
const {
ignoreDefaultArgs = false,
Expand Down Expand Up @@ -247,7 +230,6 @@ export abstract class BrowserTypeBase implements BrowserType {

abstract _defaultArgs(options: types.LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[];
abstract _connectToTransport(transport: ConnectionTransport, options: BrowserOptions): Promise<BrowserBase>;
abstract _startWebSocketServer(transport: ConnectionTransport, logger: Logger, port: number): WebSocketServer;
abstract _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env;
abstract _amendArguments(browserArguments: string[]): string[];
abstract _rewriteStartupError(error: Error, prefix: string): Error;
Expand Down
139 changes: 1 addition & 138 deletions src/server/chromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,14 @@ import * as path from 'path';
import * as os from 'os';
import { getFromENV, logPolitely, helper } from '../helper';
import { CRBrowser } from '../chromium/crBrowser';
import * as ws from 'ws';
import { Env } from './processLauncher';
import { kBrowserCloseMessageId } from '../chromium/crConnection';
import { rewriteErrorMessage } from '../utils/stackTrace';
import { BrowserTypeBase } from './browserType';
import { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport';
import { Logger } from '../logger';
import { ConnectionTransport, ProtocolRequest } from '../transport';
import { BrowserDescriptor } from '../install/browserPaths';
import { CRDevTools } from '../chromium/crDevTools';
import { BrowserOptions } from '../browser';
import { WebSocketServer } from './webSocketServer';
import { LaunchOptionsBase } from '../types';

export class Chromium extends BrowserTypeBase {
Expand Down Expand Up @@ -105,10 +102,6 @@ export class Chromium extends BrowserTypeBase {
transport.send(message);
}

_startWebSocketServer(transport: ConnectionTransport, logger: Logger, port: number): WebSocketServer {
return startWebSocketServer(transport, logger, port);
}

_defaultArgs(options: LaunchOptionsBase, isPersistent: boolean, userDataDir: string): string[] {
const { args = [], proxy } = options;
const userDataDirArg = args.find(arg => arg.startsWith('--user-data-dir'));
Expand Down Expand Up @@ -158,136 +151,6 @@ export class Chromium extends BrowserTypeBase {
}
}

type SessionData = {
socket: ws,
children: Set<string>,
isBrowserSession: boolean,
parent?: string,
};

function startWebSocketServer(transport: ConnectionTransport, logger: Logger, port: number): WebSocketServer {
const awaitingBrowserTarget = new Map<number, ws>();
const sessionToData = new Map<string, SessionData>();
const socketToBrowserSession = new Map<ws, { sessionId?: string, queue?: ProtocolRequest[] }>();

function addSession(sessionId: string, socket: ws, parentSessionId?: string) {
sessionToData.set(sessionId, {
socket,
children: new Set(),
isBrowserSession: !parentSessionId,
parent: parentSessionId,
});
if (parentSessionId)
sessionToData.get(parentSessionId)!.children.add(sessionId);
}

function removeSession(sessionId: string) {
const data = sessionToData.get(sessionId)!;
for (const child of data.children)
removeSession(child);
if (data.parent)
sessionToData.get(data.parent)!.children.delete(sessionId);
sessionToData.delete(sessionId);
}

const server = new WebSocketServer(transport, logger, port, {
onBrowserResponse(seqNum: number, source: ws, message: ProtocolResponse) {
if (awaitingBrowserTarget.has(seqNum)) {
const freshSocket = awaitingBrowserTarget.get(seqNum)!;
awaitingBrowserTarget.delete(seqNum);

const sessionId = message.result.sessionId;
if (freshSocket.readyState !== ws.CLOSED && freshSocket.readyState !== ws.CLOSING) {
const { queue } = socketToBrowserSession.get(freshSocket)!;
for (const item of queue!) {
item.sessionId = sessionId;
server.sendMessageToBrowser(item, source);
}
socketToBrowserSession.set(freshSocket, { sessionId });
addSession(sessionId, freshSocket);
} else {
server.sendMessageToBrowserOneWay('Target.detachFromTarget', { sessionId });
socketToBrowserSession.delete(freshSocket);
}
return;
}

if (message.id === -1)
return;

// At this point everything we care about has sessionId.
if (!message.sessionId)
return;

const data = sessionToData.get(message.sessionId);
if (data && data.socket.readyState !== ws.CLOSING) {
if (data.isBrowserSession)
delete message.sessionId;
data.socket.send(JSON.stringify(message));
}
},

onBrowserNotification(message: ProtocolResponse) {
// At this point everything we care about has sessionId.
if (!message.sessionId)
return;

const data = sessionToData.get(message.sessionId);
if (data && data.socket.readyState !== ws.CLOSING) {
if (message.method === 'Target.attachedToTarget')
addSession(message.params.sessionId, data.socket, message.sessionId);
if (message.method === 'Target.detachedFromTarget')
removeSession(message.params.sessionId);
// Strip session ids from the browser sessions.
if (data.isBrowserSession)
delete message.sessionId;
data.socket.send(JSON.stringify(message));
}
},

onClientAttached(socket: ws) {
socketToBrowserSession.set(socket, { queue: [] });

const seqNum = server.sendMessageToBrowser({
id: -1, // Proxy-initiated request.
method: 'Target.attachToBrowserTarget',
params: {}
}, socket);
awaitingBrowserTarget.set(seqNum, socket);
},

onClientRequest(socket: ws, message: ProtocolRequest) {
// If message has sessionId, pass through.
if (message.sessionId) {
server.sendMessageToBrowser(message, socket);
return;
}

// If message has no sessionId, look it up.
const session = socketToBrowserSession.get(socket)!;
if (session.sessionId) {
// We have it, use it.
message.sessionId = session.sessionId;
server.sendMessageToBrowser(message, socket);
return;
}
// Pending session id, queue the message.
session.queue!.push(message);
},

onClientDetached(socket: ws) {
const session = socketToBrowserSession.get(socket);
if (!session || !session.sessionId)
return;
removeSession(session.sessionId);
socketToBrowserSession.delete(socket);
server.sendMessageToBrowserOneWay('Target.detachFromTarget', { sessionId: session.sessionId });
}
});
return server;
}


const DEFAULT_ARGS = [
'--disable-background-networking',
'--enable-features=NetworkService,NetworkServiceInProcess',
Expand Down
Loading