chore: improve client-side options plumbing (#21697)

This commit is contained in:
Dmitry Gozman 2023-03-16 07:03:33 -07:00 committed by GitHub
parent c45d8749b0
commit ea85242941
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 58 additions and 50 deletions

View file

@ -19,7 +19,7 @@ import { BrowserContext, prepareBrowserContextParams } from './browserContext';
import type { Page } from './page';
import { ChannelOwner } from './channelOwner';
import { Events } from './events';
import type { BrowserContextOptions } from './types';
import type { LaunchOptions, BrowserContextOptions } from './types';
import { isSafeCloseError, kBrowserClosedError } from '../common/errors';
import type * as api from '../../types/types';
import { CDPSession } from './cdpSession';
@ -30,17 +30,14 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
private _isConnected = true;
private _closedPromise: Promise<void>;
_shouldCloseConnectionOnClose = false;
private _browserType!: BrowserType;
_browserType!: BrowserType;
_options: LaunchOptions = {};
readonly _name: string;
static from(browser: channels.BrowserChannel): Browser {
return (browser as any)._object;
}
static fromNullable(browser: channels.BrowserChannel | null): Browser | null {
return browser ? Browser.from(browser) : null;
}
constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.BrowserInitializer) {
super(parent, type, guid, initializer);
this._name = initializer.name;
@ -48,12 +45,6 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
this._closedPromise = new Promise(f => this.once(Events.Browser.Disconnected, f));
}
_setBrowserType(browserType: BrowserType) {
this._browserType = browserType;
for (const context of this._contexts)
context._setBrowserType(browserType);
}
browserType(): BrowserType {
return this._browserType;
}
@ -65,13 +56,12 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
async _newContextForReuse(options: BrowserContextOptions = {}): Promise<BrowserContext> {
for (const context of this._contexts) {
await this._wrapApiCall(async () => {
await this._browserType._onWillCloseContext?.(context);
await this._browserType._willCloseContext(context);
}, true);
for (const page of context.pages())
page._onClose();
context._onClose();
}
this._contexts.clear();
return await this._innerNewContext(options, true);
}
@ -80,11 +70,7 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
const contextOptions = await prepareBrowserContextParams(options);
const response = forReuse ? await this._channel.newContextForReuse(contextOptions) : await this._channel.newContext(contextOptions);
const context = BrowserContext.from(response.context);
context._options = contextOptions;
this._contexts.add(context);
context._logger = options.logger || this._logger;
context._setBrowserType(this._browserType);
await this._browserType._onDidCreateContext?.(context);
await this._browserType._didCreateContext(context, contextOptions, this._options, options.logger || this._logger);
return context;
}

View file

@ -45,7 +45,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
_pages = new Set<Page>();
private _routes: network.RouteHandler[] = [];
readonly _browser: Browser | null = null;
private _browserType: BrowserType | undefined;
_browserType: BrowserType | undefined;
readonly _bindings = new Map<string, (source: structs.BindingSource, ...args: any[]) => any>();
_timeoutSettings = new TimeoutSettings();
_ownerPage: Page | undefined;
@ -72,6 +72,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
super(parent, type, guid, initializer, createInstrumentation());
if (parent instanceof Browser)
this._browser = parent;
this._browser?._contexts.add(this);
this._isChromium = this._browser?._name === 'chromium';
this.tracing = Tracing.from(initializer.tracing);
this.request = APIRequestContext.from(initializer.requestContext);
@ -105,11 +106,11 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
]));
}
_setBrowserType(browserType: BrowserType) {
this._browserType = browserType;
browserType._contexts.add(this);
_setOptions(contextOptions: channels.BrowserNewContextParams, browserOptions: LaunchOptions) {
this._options = contextOptions;
if (this._options.recordHar)
this._harRecorders.set('', { path: this._options.recordHar.path, content: this._options.recordHar.content });
this.tracing._tracesDir = browserOptions.tracesDir;
}
private _onPage(page: Page): void {
@ -348,7 +349,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
return;
this._closeWasCalled = true;
await this._wrapApiCall(async () => {
await this._browserType?._onWillCloseContext?.(this);
await this._browserType?._willCloseContext(this);
for (const [harId, harParams] of this._harRecorders) {
const har = await this._channel.harExport({ harId });
const artifact = Artifact.from(har.artifact);

View file

@ -18,7 +18,7 @@ import type * as channels from '@protocol/channels';
import { Browser } from './browser';
import { BrowserContext, prepareBrowserContextParams } from './browserContext';
import { ChannelOwner } from './channelOwner';
import type { LaunchOptions, LaunchServerOptions, ConnectOptions, LaunchPersistentContextOptions, BrowserContextOptions } from './types';
import type { LaunchOptions, LaunchServerOptions, ConnectOptions, LaunchPersistentContextOptions, BrowserContextOptions, Logger } from './types';
import { Connection } from './connection';
import { Events } from './events';
import type { ChildProcess } from 'child_process';
@ -48,10 +48,10 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
// Instrumentation.
_defaultContextOptions?: BrowserContextOptions;
_defaultLaunchOptions?: LaunchOptions;
_defaultConnectOptions?: ConnectOptions;
_onDidCreateContext?: (context: BrowserContext) => Promise<void>;
_onWillCloseContext?: (context: BrowserContext) => Promise<void>;
private _defaultLaunchOptions?: LaunchOptions;
private _defaultConnectOptions?: ConnectOptions;
private _onDidCreateContext?: (context: BrowserContext) => Promise<void>;
private _onWillCloseContext?: (context: BrowserContext) => Promise<void>;
static from(browserType: channels.BrowserTypeChannel): BrowserType {
return (browserType as any)._object;
@ -84,8 +84,7 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
};
return await this._wrapApiCall(async () => {
const browser = Browser.from((await this._channel.launch(launchOptions)).browser);
browser._logger = logger;
browser._setBrowserType(this);
this._didLaunchBrowser(browser, options, logger);
return browser;
});
}
@ -126,10 +125,7 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
return await this._wrapApiCall(async () => {
const result = await this._channel.launchPersistentContext(persistentParams);
const context = BrowserContext.from(result.context);
context._options = contextParams;
context._logger = logger;
context._setBrowserType(this);
await this._onDidCreateContext?.(context);
await this._didCreateContext(context, contextParams, options, logger);
return context;
});
}
@ -200,9 +196,8 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
}
playwright._setSelectors(this._playwright.selectors);
browser = Browser.from(playwright._initializer.preLaunchedBrowser!);
browser._logger = logger;
this._didLaunchBrowser(browser, {}, logger);
browser._shouldCloseConnectionOnClose = true;
browser._setBrowserType(this);
browser.on(Events.Browser.Disconnected, closePipe);
return browser;
}, deadline ? deadline - monotonicTime() : 0);
@ -236,10 +231,28 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel> imple
timeout: params.timeout
});
const browser = Browser.from(result.browser);
this._didLaunchBrowser(browser, {}, params.logger);
if (result.defaultContext)
browser._contexts.add(BrowserContext.from(result.defaultContext));
browser._logger = params.logger;
browser._setBrowserType(this);
await this._didCreateContext(BrowserContext.from(result.defaultContext), {}, {}, undefined);
return browser;
}
_didLaunchBrowser(browser: Browser, browserOptions: LaunchOptions, logger: Logger | undefined) {
browser._browserType = this;
browser._options = browserOptions;
browser._logger = logger;
}
async _didCreateContext(context: BrowserContext, contextOptions: channels.BrowserNewContextParams, browserOptions: LaunchOptions, logger: Logger | undefined) {
context._logger = logger;
context._browserType = this;
this._contexts.add(context);
context._setOptions(contextOptions, browserOptions);
await this._onDidCreateContext?.(context);
}
async _willCloseContext(context: BrowserContext) {
this._contexts.delete(context);
await this._onWillCloseContext?.(context);
}
}

View file

@ -44,7 +44,7 @@ export type FetchOptions = {
maxRedirects?: number,
};
type NewContextOptions = Omit<channels.PlaywrightNewRequestOptions, 'extraHTTPHeaders' | 'storageState'> & {
type NewContextOptions = Omit<channels.PlaywrightNewRequestOptions, 'extraHTTPHeaders' | 'storageState' | 'tracesDir'> & {
extraHTTPHeaders?: Headers,
storageState?: string | StorageState,
};
@ -56,6 +56,7 @@ export class APIRequest implements api.APIRequest {
readonly _contexts = new Set<APIRequestContext>();
// Instrumentation.
_defaultContextOptions?: NewContextOptions & { tracesDir?: string };
_onDidCreateContext?: (context: APIRequestContext) => Promise<void>;
_onWillCloseContext?: (context: APIRequestContext) => Promise<void>;
@ -64,16 +65,21 @@ export class APIRequest implements api.APIRequest {
}
async newContext(options: NewContextOptions = {}): Promise<APIRequestContext> {
options = { ...this._defaultContextOptions, ...options };
const storageState = typeof options.storageState === 'string' ?
JSON.parse(await fs.promises.readFile(options.storageState, 'utf8')) :
options.storageState;
// We do not expose tracesDir in the API, so do not allow options to accidentally override it.
const tracesDir = this._defaultContextOptions?.tracesDir;
const context = APIRequestContext.from((await this._playwright._channel.newRequest({
...options,
extraHTTPHeaders: options.extraHTTPHeaders ? headersObjectToArray(options.extraHTTPHeaders) : undefined,
storageState,
tracesDir,
})).request);
this._contexts.add(context);
context._request = this;
context._tracing._tracesDir = tracesDir;
await this._onDidCreateContext?.(context);
return context;
}

View file

@ -22,6 +22,8 @@ import { ChannelOwner } from './channelOwner';
export class Tracing extends ChannelOwner<channels.TracingChannel> implements api.Tracing {
private _includeSources = false;
private _metadataCollector: channels.ClientSideCallMetadata[] = [];
_tracesDir: string | undefined;
static from(channel: channels.TracingChannel): Tracing {
return (channel as any)._object;
}

View file

@ -17,7 +17,6 @@
import * as fs from 'fs';
import * as path from 'path';
import type { APIRequestContext, BrowserContext, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core';
import type { BrowserType as BrowserTypeImpl } from 'playwright-core/lib/client/browserType';
import * as playwrightLibrary from 'playwright-core';
import { createGuid, debugMode, removeFolders, addInternalStackPrefix, mergeTraceFiles, saveTraceFile } from 'playwright-core/lib/utils';
import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, TraceMode, VideoMode } from '../types/test';
@ -102,13 +101,13 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
options.tracesDir = path.join(_artifactsDir(), 'traces');
for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) {
(browserType as BrowserTypeImpl)._defaultLaunchOptions = options;
(browserType as BrowserTypeImpl)._defaultConnectOptions = connectOptions;
(browserType as any)._defaultLaunchOptions = options;
(browserType as any)._defaultConnectOptions = connectOptions;
}
await use(options);
for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) {
(browserType as BrowserTypeImpl)._defaultLaunchOptions = undefined;
(browserType as BrowserTypeImpl)._defaultConnectOptions = undefined;
(browserType as any)._defaultLaunchOptions = undefined;
(browserType as any)._defaultConnectOptions = undefined;
}
}, { scope: 'worker', auto: true }],
@ -386,6 +385,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
{
(playwright.request as any)._onDidCreateContext = onDidCreateRequestContext;
(playwright.request as any)._onWillCloseContext = onWillCloseRequestContext;
(playwright.request as any)._defaultContextOptions = _combinedContextOptions;
const existingApiRequests: APIRequestContext[] = Array.from((playwright.request as any)._contexts as Set<APIRequestContext>);
await Promise.all(existingApiRequests.map(onDidCreateRequestContext));
}
@ -422,6 +422,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const leftoverApiRequests: APIRequestContext[] = Array.from((playwright.request as any)._contexts as Set<APIRequestContext>);
(playwright.request as any)._onDidCreateContext = undefined;
(playwright.request as any)._onWillCloseContext = undefined;
(playwright.request as any)._defaultContextOptions = undefined;
testInfoImpl._onTestFailureImmediateCallbacks.delete(screenshotOnTestFailure);
// 5. Collect artifacts from any non-closed contexts.
@ -555,12 +556,11 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
await use(page);
},
request: async ({ playwright, _combinedContextOptions }, use) => {
const request = await playwright.request.newContext(_combinedContextOptions);
request: async ({ playwright }, use) => {
const request = await playwright.request.newContext();
await use(request);
await request.dispose();
}
},
});