review feedback

This commit is contained in:
Max Schmitt 2024-09-13 16:41:13 +02:00
parent 6747844a33
commit 73ea4ce524
13 changed files with 30 additions and 29 deletions

View file

@ -29,6 +29,7 @@ import { validateBrowserContextOptions } from '../browserContext';
import { ProgressController } from '../progress'; import { ProgressController } from '../progress';
import { CRBrowser } from '../chromium/crBrowser'; import { CRBrowser } from '../chromium/crBrowser';
import { helper } from '../helper'; import { helper } from '../helper';
import type * as types from '../types';
import { PipeTransport } from '../../protocol/transport'; import { PipeTransport } from '../../protocol/transport';
import { RecentLogsCollector } from '../../utils/debugLogger'; import { RecentLogsCollector } from '../../utils/debugLogger';
import { gracefullyCloseSet } from '../../utils/processLauncher'; import { gracefullyCloseSet } from '../../utils/processLauncher';
@ -309,7 +310,7 @@ export class AndroidDevice extends SdkObject {
return await this._connectToBrowser(socketName); return await this._connectToBrowser(socketName);
} }
private async _connectToBrowser(socketName: string, options: channels.BrowserNewContextParams = {}): Promise<BrowserContext> { private async _connectToBrowser(socketName: string, options: types.BrowserContextOptions = {}): Promise<BrowserContext> {
const socket = await this._waitForLocalAbstract(socketName); const socket = await this._waitForLocalAbstract(socketName);
const androidBrowser = new AndroidBrowser(this, socket); const androidBrowser = new AndroidBrowser(this, socket);
await androidBrowser._init(); await androidBrowser._init();

View file

@ -111,7 +111,7 @@ export class BidiBrowser extends Browser {
this._didClose(); this._didClose();
} }
async doCreateNewContext(options: channels.BrowserNewContextParams): Promise<BrowserContext> { async doCreateNewContext(options: types.BrowserContextOptions): Promise<BrowserContext> {
const { userContext } = await this._browserSession.send('browser.createUserContext', {}); const { userContext } = await this._browserSession.send('browser.createUserContext', {});
const context = new BidiBrowserContext(this, userContext, options); const context = new BidiBrowserContext(this, userContext, options);
await context._initialize(); await context._initialize();
@ -190,7 +190,7 @@ export class BidiBrowser extends Browser {
export class BidiBrowserContext extends BrowserContext { export class BidiBrowserContext extends BrowserContext {
declare readonly _browser: BidiBrowser; declare readonly _browser: BidiBrowser;
constructor(browser: BidiBrowser, browserContextId: string | undefined, options: channels.BrowserNewContextParams) { constructor(browser: BidiBrowser, browserContextId: string | undefined, options: types.BrowserContextOptions) {
super(browser, options, browserContextId); super(browser, options, browserContextId);
this._authenticateProxyViaHeader(); this._authenticateProxyViaHeader();
} }

View file

@ -41,7 +41,7 @@ export type BrowserOptions = {
downloadsPath: string, downloadsPath: string,
tracesDir: string, tracesDir: string,
headful?: boolean, headful?: boolean,
persistent?: channels.BrowserNewContextParams, // Undefined means no persistent context. persistent?: types.BrowserContextOptions, // Undefined means no persistent context.
browserProcess: BrowserProcess, browserProcess: BrowserProcess,
customExecutablePath?: string; customExecutablePath?: string;
proxy?: ProxySettings, proxy?: ProxySettings,
@ -85,7 +85,7 @@ export abstract class Browser extends SdkObject {
const clientCertificatesProxy = await createClientCertificatesProxyIfNeeded(options, this.options); const clientCertificatesProxy = await createClientCertificatesProxyIfNeeded(options, this.options);
if (clientCertificatesProxy) { if (clientCertificatesProxy) {
options.proxyOverride = await clientCertificatesProxy.listen(); options.proxyOverride = await clientCertificatesProxy.listen();
options.ignoreHTTPSErrorsOverride = true; options.internalIgnoreHTTPSErrors = true;
} }
let context; let context;
try { try {

View file

@ -93,7 +93,7 @@ export abstract class BrowserContext extends SdkObject {
readonly clock: Clock; readonly clock: Clock;
_clientCertificatesProxy: ClientCertificatesProxy | undefined; _clientCertificatesProxy: ClientCertificatesProxy | undefined;
constructor(browser: Browser, options: channels.BrowserNewContextParams, browserContextId: string | undefined) { constructor(browser: Browser, options: types.BrowserContextOptions, browserContextId: string | undefined) {
super(browser, 'browser-context'); super(browser, 'browser-context');
this.attribution.context = this; this.attribution.context = this;
this._browser = browser; this._browser = browser;
@ -659,7 +659,7 @@ export function assertBrowserContextIsNotOwned(context: BrowserContext) {
} }
} }
export async function createClientCertificatesProxyIfNeeded(options: channels.BrowserNewContextOptions, browserOptions?: BrowserOptions) { export async function createClientCertificatesProxyIfNeeded(options: types.BrowserContextOptions, browserOptions?: BrowserOptions) {
if (!options.clientCertificates?.length) if (!options.clientCertificates?.length)
return; return;
if ((options.proxy?.server && options.proxy?.server !== 'per-context') || (browserOptions?.proxy?.server && browserOptions?.proxy?.server !== 'http://per-context')) if ((options.proxy?.server && options.proxy?.server !== 'per-context') || (browserOptions?.proxy?.server && browserOptions?.proxy?.server !== 'http://per-context'))
@ -668,7 +668,7 @@ export async function createClientCertificatesProxyIfNeeded(options: channels.Br
return new ClientCertificatesProxy(options); return new ClientCertificatesProxy(options);
} }
export function validateBrowserContextOptions(options: channels.BrowserNewContextParams, browserOptions: BrowserOptions) { export function validateBrowserContextOptions(options: types.BrowserContextOptions, browserOptions: BrowserOptions) {
if (options.noDefaultViewport && options.deviceScaleFactor !== undefined) if (options.noDefaultViewport && options.deviceScaleFactor !== undefined)
throw new Error(`"deviceScaleFactor" option is not supported with null "viewport"`); throw new Error(`"deviceScaleFactor" option is not supported with null "viewport"`);
if (options.noDefaultViewport && !!options.isMobile) if (options.noDefaultViewport && !!options.isMobile)
@ -717,7 +717,7 @@ export function verifyGeolocation(geolocation?: types.Geolocation) {
throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`); throw new Error(`geolocation.accuracy: precondition 0 <= ACCURACY failed.`);
} }
export function verifyClientCertificates(clientCertificates?: channels.BrowserNewContextParams['clientCertificates']) { export function verifyClientCertificates(clientCertificates?: types.BrowserContextOptions['clientCertificates']) {
if (!clientCertificates) if (!clientCertificates)
return; return;
for (const cert of clientCertificates) { for (const cert of clientCertificates) {

View file

@ -111,7 +111,7 @@ export abstract class BrowserType extends SdkObject {
return browser._defaultContext!; return browser._defaultContext!;
} }
async _innerLaunchWithRetries(progress: Progress, options: types.LaunchOptions, persistent: channels.BrowserNewContextParams | undefined, protocolLogger: types.ProtocolLogger, userDataDir?: string): Promise<Browser> { async _innerLaunchWithRetries(progress: Progress, options: types.LaunchOptions, persistent: types.BrowserContextOptions | undefined, protocolLogger: types.ProtocolLogger, userDataDir?: string): Promise<Browser> {
try { try {
return await this._innerLaunch(progress, options, persistent, protocolLogger, userDataDir); return await this._innerLaunch(progress, options, persistent, protocolLogger, userDataDir);
} catch (error) { } catch (error) {
@ -125,7 +125,7 @@ export abstract class BrowserType extends SdkObject {
} }
} }
async _innerLaunch(progress: Progress, options: types.LaunchOptions, persistent: channels.BrowserNewContextParams | undefined, protocolLogger: types.ProtocolLogger, maybeUserDataDir?: string): Promise<Browser> { async _innerLaunch(progress: Progress, options: types.LaunchOptions, persistent: types.BrowserContextOptions | undefined, protocolLogger: types.ProtocolLogger, maybeUserDataDir?: string): Promise<Browser> {
options.proxy = options.proxy ? normalizeProxySettings(options.proxy) : undefined; options.proxy = options.proxy ? normalizeProxySettings(options.proxy) : undefined;
const browserLogsCollector = new RecentLogsCollector(); const browserLogsCollector = new RecentLogsCollector();
const { browserProcess, userDataDir, artifactsDir, transport } = await this._launchProcess(progress, options, !!persistent, browserLogsCollector, maybeUserDataDir); const { browserProcess, userDataDir, artifactsDir, transport } = await this._launchProcess(progress, options, !!persistent, browserLogsCollector, maybeUserDataDir);

View file

@ -31,7 +31,6 @@ import { CRDevTools } from './crDevTools';
import type { BrowserOptions, BrowserProcess } from '../browser'; import type { BrowserOptions, BrowserProcess } from '../browser';
import { Browser } from '../browser'; import { Browser } from '../browser';
import type * as types from '../types'; import type * as types from '../types';
import type * as channels from '@protocol/channels';
import type { HTTPRequestParams } from '../../utils/network'; import type { HTTPRequestParams } from '../../utils/network';
import { fetchData } from '../../utils/network'; import { fetchData } from '../../utils/network';
import { getUserAgent } from '../../utils/userAgent'; import { getUserAgent } from '../../utils/userAgent';
@ -98,7 +97,7 @@ export class Chromium extends BrowserType {
await cleanedUp; await cleanedUp;
}; };
const browserProcess: BrowserProcess = { close: doClose, kill: doClose }; const browserProcess: BrowserProcess = { close: doClose, kill: doClose };
const persistent: channels.BrowserNewContextParams = { noDefaultViewport: true }; const persistent: types.BrowserContextOptions = { noDefaultViewport: true };
const browserOptions: BrowserOptions = { const browserOptions: BrowserOptions = {
slowMo: options.slowMo, slowMo: options.slowMo,
name: 'chromium', name: 'chromium',

View file

@ -543,7 +543,7 @@ class FrameSession {
const options = this._crPage._browserContext._options; const options = this._crPage._browserContext._options;
if (options.bypassCSP) if (options.bypassCSP)
promises.push(this._client.send('Page.setBypassCSP', { enabled: true })); promises.push(this._client.send('Page.setBypassCSP', { enabled: true }));
if (options.ignoreHTTPSErrors || options.ignoreHTTPSErrorsOverride) if (options.ignoreHTTPSErrors || options.internalIgnoreHTTPSErrors)
promises.push(this._client.send('Security.setIgnoreCertificateErrors', { ignore: true })); promises.push(this._client.send('Security.setIgnoreCertificateErrors', { ignore: true }));
if (this._isMainFrame()) if (this._isMainFrame())
promises.push(this._updateViewport()); promises.push(this._updateViewport());
@ -1213,7 +1213,7 @@ async function emulateTimezone(session: CRSession, timezoneId: string) {
const contextDelegateSymbol = Symbol('delegate'); const contextDelegateSymbol = Symbol('delegate');
// Chromium reference: https://source.chromium.org/chromium/chromium/src/+/main:components/embedder_support/user_agent_utils.cc;l=434;drc=70a6711e08e9f9e0d8e4c48e9ba5cab62eb010c2 // Chromium reference: https://source.chromium.org/chromium/chromium/src/+/main:components/embedder_support/user_agent_utils.cc;l=434;drc=70a6711e08e9f9e0d8e4c48e9ba5cab62eb010c2
function calculateUserAgentMetadata(options: channels.BrowserNewContextParams) { function calculateUserAgentMetadata(options: types.BrowserContextOptions) {
const ua = options.userAgent; const ua = options.userAgent;
if (!ua) if (!ua)
return undefined; return undefined;

View file

@ -36,6 +36,7 @@ import type { BrowserWindow } from 'electron';
import type { Progress } from '../progress'; import type { Progress } from '../progress';
import { ProgressController } from '../progress'; import { ProgressController } from '../progress';
import { helper } from '../helper'; import { helper } from '../helper';
import type * as types from '../types';
import { eventsHelper } from '../../utils/eventsHelper'; import { eventsHelper } from '../../utils/eventsHelper';
import type { BrowserOptions, BrowserProcess } from '../browser'; import type { BrowserOptions, BrowserProcess } from '../browser';
import type { Playwright } from '../playwright'; import type { Playwright } from '../playwright';
@ -265,7 +266,7 @@ export class Electron extends SdkObject {
close: gracefullyClose, close: gracefullyClose,
kill kill
}; };
const contextOptions: channels.BrowserNewContextParams = { const contextOptions: types.BrowserContextOptions = {
...options, ...options,
noDefaultViewport: true, noDefaultViewport: true,
}; };

View file

@ -50,7 +50,7 @@ type FetchRequestOptions = {
timeoutSettings: TimeoutSettings; timeoutSettings: TimeoutSettings;
ignoreHTTPSErrors?: boolean; ignoreHTTPSErrors?: boolean;
baseURL?: string; baseURL?: string;
clientCertificates?: channels.BrowserNewContextOptions['clientCertificates']; clientCertificates?: types.BrowserContextOptions['clientCertificates'];
}; };
type HeadersObject = Readonly<{ [name: string]: string }>; type HeadersObject = Readonly<{ [name: string]: string }>;

View file

@ -89,7 +89,7 @@ export class FFBrowser extends Browser {
return !this._connection._closed; return !this._connection._closed;
} }
async doCreateNewContext(options: channels.BrowserNewContextParams): Promise<BrowserContext> { async doCreateNewContext(options: types.BrowserContextOptions): Promise<BrowserContext> {
if (options.isMobile) if (options.isMobile)
throw new Error('options.isMobile is not supported in Firefox'); throw new Error('options.isMobile is not supported in Firefox');
const { browserContextId } = await this.session.send('Browser.createBrowserContext', { removeOnDetach: true }); const { browserContextId } = await this.session.send('Browser.createBrowserContext', { removeOnDetach: true });
@ -173,7 +173,7 @@ export class FFBrowser extends Browser {
export class FFBrowserContext extends BrowserContext { export class FFBrowserContext extends BrowserContext {
declare readonly _browser: FFBrowser; declare readonly _browser: FFBrowser;
constructor(browser: FFBrowser, browserContextId: string | undefined, options: channels.BrowserNewContextParams) { constructor(browser: FFBrowser, browserContextId: string | undefined, options: types.BrowserContextOptions) {
super(browser, options, browserContextId); super(browser, options, browserContextId);
} }
@ -206,7 +206,7 @@ export class FFBrowserContext extends BrowserContext {
promises.push(this._browser.session.send('Browser.setUserAgentOverride', { browserContextId, userAgent: this._options.userAgent })); promises.push(this._browser.session.send('Browser.setUserAgentOverride', { browserContextId, userAgent: this._options.userAgent }));
if (this._options.bypassCSP) if (this._options.bypassCSP)
promises.push(this._browser.session.send('Browser.setBypassCSP', { browserContextId, bypassCSP: true })); promises.push(this._browser.session.send('Browser.setBypassCSP', { browserContextId, bypassCSP: true }));
if (this._options.ignoreHTTPSErrors || this._options.ignoreHTTPSErrorsOverride) if (this._options.ignoreHTTPSErrors || this._options.internalIgnoreHTTPSErrors)
promises.push(this._browser.session.send('Browser.setIgnoreHTTPSErrors', { browserContextId, ignoreHTTPSErrors: true })); promises.push(this._browser.session.send('Browser.setIgnoreHTTPSErrors', { browserContextId, ignoreHTTPSErrors: true }));
if (this._options.javaScriptEnabled === false) if (this._options.javaScriptEnabled === false)
promises.push(this._browser.session.send('Browser.setJavaScriptDisabled', { browserContextId, javaScriptDisabled: true })); promises.push(this._browser.session.send('Browser.setJavaScriptDisabled', { browserContextId, javaScriptDisabled: true }));

View file

@ -23,7 +23,7 @@ import { createSocket, createTLSSocket } from '../utils/happy-eyeballs';
import { escapeHTML, generateSelfSignedCertificate, ManualPromise, rewriteErrorMessage } from '../utils'; import { escapeHTML, generateSelfSignedCertificate, ManualPromise, rewriteErrorMessage } from '../utils';
import type { SocksSocketClosedPayload, SocksSocketDataPayload, SocksSocketRequestedPayload } from '../common/socksProxy'; import type { SocksSocketClosedPayload, SocksSocketDataPayload, SocksSocketRequestedPayload } from '../common/socksProxy';
import { SocksProxy } from '../common/socksProxy'; import { SocksProxy } from '../common/socksProxy';
import type * as channels from '@protocol/channels'; import type * as types from './types';
import { debugLogger } from '../utils/debugLogger'; import { debugLogger } from '../utils/debugLogger';
let dummyServerTlsOptions: tls.TlsOptions | undefined = undefined; let dummyServerTlsOptions: tls.TlsOptions | undefined = undefined;
@ -235,7 +235,7 @@ export class ClientCertificatesProxy {
alpnCache: ALPNCache; alpnCache: ALPNCache;
constructor( constructor(
contextOptions: Pick<channels.BrowserNewContextOptions, 'clientCertificates' | 'ignoreHTTPSErrors'> contextOptions: Pick<types.BrowserContextOptions, 'clientCertificates' | 'ignoreHTTPSErrors'>
) { ) {
this.alpnCache = new ALPNCache(); this.alpnCache = new ALPNCache();
this.ignoreHTTPSErrors = contextOptions.ignoreHTTPSErrors; this.ignoreHTTPSErrors = contextOptions.ignoreHTTPSErrors;
@ -261,9 +261,9 @@ export class ClientCertificatesProxy {
loadDummyServerCertsIfNeeded(); loadDummyServerCertsIfNeeded();
} }
_initSecureContexts(clientCertificates: channels.BrowserNewContextOptions['clientCertificates']) { _initSecureContexts(clientCertificates: types.BrowserContextOptions['clientCertificates']) {
// Step 1. Group certificates by origin. // Step 1. Group certificates by origin.
const origin2certs = new Map<string, channels.BrowserNewContextOptions['clientCertificates']>(); const origin2certs = new Map<string, types.BrowserContextOptions['clientCertificates']>();
for (const cert of clientCertificates || []) { for (const cert of clientCertificates || []) {
const origin = normalizeOrigin(cert.origin); const origin = normalizeOrigin(cert.origin);
const certs = origin2certs.get(origin) || []; const certs = origin2certs.get(origin) || [];
@ -301,7 +301,7 @@ function normalizeOrigin(origin: string): string {
} }
function convertClientCertificatesToTLSOptions( function convertClientCertificatesToTLSOptions(
clientCertificates: channels.BrowserNewContextOptions['clientCertificates'] clientCertificates: types.BrowserContextOptions['clientCertificates']
): Pick<https.RequestOptions, 'pfx' | 'key' | 'cert'> | undefined { ): Pick<https.RequestOptions, 'pfx' | 'key' | 'cert'> | undefined {
if (!clientCertificates || !clientCertificates.length) if (!clientCertificates || !clientCertificates.length)
return; return;
@ -322,7 +322,7 @@ function convertClientCertificatesToTLSOptions(
} }
export function getMatchingTLSOptionsForOrigin( export function getMatchingTLSOptionsForOrigin(
clientCertificates: channels.BrowserNewContextOptions['clientCertificates'], clientCertificates: types.BrowserContextOptions['clientCertificates'],
origin: string origin: string
): Pick<https.RequestOptions, 'pfx' | 'key' | 'cert'> | undefined { ): Pick<https.RequestOptions, 'pfx' | 'key' | 'cert'> | undefined {
const matchingCerts = clientCertificates?.filter(c => const matchingCerts = clientCertificates?.filter(c =>

View file

@ -157,7 +157,7 @@ export type LaunchOptions = channels.BrowserTypeLaunchOptions & {
export type BrowserContextOptions = channels.BrowserNewContextOptions & { export type BrowserContextOptions = channels.BrowserNewContextOptions & {
proxyOverride?: ProxySettings; proxyOverride?: ProxySettings;
ignoreHTTPSErrorsOverride?: boolean; internalIgnoreHTTPSErrors?: boolean;
}; };
export type ProtocolLogger = (direction: 'send' | 'receive', message: object) => void; export type ProtocolLogger = (direction: 'send' | 'receive', message: object) => void;

View file

@ -207,7 +207,7 @@ export class WKBrowser extends Browser {
export class WKBrowserContext extends BrowserContext { export class WKBrowserContext extends BrowserContext {
declare readonly _browser: WKBrowser; declare readonly _browser: WKBrowser;
constructor(browser: WKBrowser, browserContextId: string | undefined, options: channels.BrowserNewContextParams) { constructor(browser: WKBrowser, browserContextId: string | undefined, options: types.BrowserContextOptions) {
super(browser, options, browserContextId); super(browser, options, browserContextId);
this._validateEmulatedViewport(options.viewport); this._validateEmulatedViewport(options.viewport);
this._authenticateProxyViaHeader(); this._authenticateProxyViaHeader();
@ -222,7 +222,7 @@ export class WKBrowserContext extends BrowserContext {
downloadPath: this._browser.options.downloadsPath, downloadPath: this._browser.options.downloadsPath,
browserContextId browserContextId
})); }));
if (this._options.ignoreHTTPSErrors || this._options.ignoreHTTPSErrorsOverride) if (this._options.ignoreHTTPSErrors || this._options.internalIgnoreHTTPSErrors)
promises.push(this._browser._browserSession.send('Playwright.setIgnoreCertificateErrors', { browserContextId, ignore: true })); promises.push(this._browser._browserSession.send('Playwright.setIgnoreCertificateErrors', { browserContextId, ignore: true }));
if (this._options.locale) if (this._options.locale)
promises.push(this._browser._browserSession.send('Playwright.setLanguages', { browserContextId, languages: [this._options.locale] })); promises.push(this._browser._browserSession.send('Playwright.setLanguages', { browserContextId, languages: [this._options.locale] }));