From c06842628d8385ab97516036702d3e00518b7cc3 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 4 Mar 2026 17:40:08 +0000 Subject: [PATCH 1/3] chore: turn modifiers into hooks early (#39508) --- packages/playwright/src/common/fixtures.ts | 6 +- packages/playwright/src/common/poolBuilder.ts | 57 +++++++++++++------ packages/playwright/src/common/test.ts | 32 ++++++++--- packages/playwright/src/common/testType.ts | 2 +- .../playwright/src/worker/fixtureRunner.ts | 10 ---- .../playwright/src/worker/timeoutManager.ts | 6 -- packages/playwright/src/worker/workerMain.ts | 30 +--------- tests/playwright-test/test-modifiers.spec.ts | 6 +- 8 files changed, 75 insertions(+), 74 deletions(-) diff --git a/packages/playwright/src/common/fixtures.ts b/packages/playwright/src/common/fixtures.ts index 30b144e663465..dad97674a300b 100644 --- a/packages/playwright/src/common/fixtures.ts +++ b/packages/playwright/src/common/fixtures.ts @@ -228,12 +228,16 @@ export class FixturePool { return hash.digest('hex'); } - validateFunction(fn: Function, prefix: string, location: Location) { + validateFunction(fn: Function, prefix: string, location: Location): 'worker' | 'test' { + let scope: 'worker' | 'test' = 'worker'; for (const name of fixtureParameterNames(fn, location, e => this._onLoadError(e))) { const registration = this._registrations.get(name); if (!registration) this._addLoadError(`${prefix} has unknown parameter "${name}".`, location); + if (registration?.scope === 'test') + scope = 'test'; } + return scope; } resolve(name: string, forFixture?: FixtureRegistration): FixtureRegistration | undefined { diff --git a/packages/playwright/src/common/poolBuilder.ts b/packages/playwright/src/common/poolBuilder.ts index f975efe92f80d..2571747db91cb 100644 --- a/packages/playwright/src/common/poolBuilder.ts +++ b/packages/playwright/src/common/poolBuilder.ts @@ -14,8 +14,9 @@ * limitations under the License. */ -import { FixturePool } from './fixtures'; +import { FixturePool, inheritFixtureNames } from './fixtures'; import { formatLocation } from '../util'; +import { currentTestInfo } from './globals'; import type { FullProjectInternal } from './config'; import type { LoadError } from './fixtures'; @@ -41,8 +42,32 @@ export class PoolBuilder { this._project = project; } - buildPools(suite: Suite, testErrors?: TestError[]) { - suite.forEachTest(test => { + buildPools(topSuite: Suite, testErrors?: TestError[]) { + topSuite.forEachSuite(suite => { + const modifiers = suite._modifiers.slice(); + suite._modifiers = []; + + for (const modifier of modifiers.reverse()) { + let pool = this._buildTestTypePool(modifier.testType, testErrors); + pool = this._buildPoolForSuite(pool, suite, testErrors); + const scope = pool.validateFunction(modifier.fn, modifier.type + ' modifier', modifier.location); + + const fn = async (fixtures: any) => { + const result = await modifier.fn(fixtures); + currentTestInfo()?._modifier(modifier.type, modifier.location, [!!result, modifier.description]); + }; + inheritFixtureNames(modifier.fn, fn); + + suite._hooks.unshift({ + type: scope === 'worker' ? 'beforeAll' : 'beforeEach', + fn, + title: `${modifier.type} modifier`, + location: modifier.location, + }); + } + }); + + topSuite.forEachTest(test => { const pool = this._buildPoolForTest(test, testErrors); if (this._type === 'loader') test._poolDigest = pool.digest; @@ -53,25 +78,21 @@ export class PoolBuilder { private _buildPoolForTest(test: TestCase, testErrors?: TestError[]): FixturePool { let pool = this._buildTestTypePool(test._testType, testErrors); - - const parents: Suite[] = []; - for (let parent: Suite | undefined = test.parent; parent; parent = parent.parent) - parents.push(parent); - parents.reverse(); - - for (const parent of parents) { - if (parent._use.length) - pool = new FixturePool(parent._use, e => this._handleLoadError(e, testErrors), pool, parent._type === 'describe'); - for (const hook of parent._hooks) - pool.validateFunction(hook.fn, hook.type + ' hook', hook.location); - for (const modifier of parent._modifiers) - pool.validateFunction(modifier.fn, modifier.type + ' modifier', modifier.location); - } - + pool = this._buildPoolForSuite(pool, test.parent, testErrors); pool.validateFunction(test.fn, 'Test', test.location); return pool; } + private _buildPoolForSuite(pool: FixturePool, suite: Suite, testErrors?: TestError[]): FixturePool { + if (suite.parent) + pool = this._buildPoolForSuite(pool, suite.parent, testErrors); + if (suite._use.length) + pool = new FixturePool(suite._use, e => this._handleLoadError(e, testErrors), pool, suite._type === 'describe'); + for (const hook of suite._hooks) + pool.validateFunction(hook.fn, hook.type + ' hook', hook.location); + return pool; + } + private _buildTestTypePool(testType: TestTypeImpl, testErrors?: TestError[]): FixturePool { if (!this._testTypePools.has(testType)) { const optionOverrides = { diff --git a/packages/playwright/src/common/test.ts b/packages/playwright/src/common/test.ts index fa8d8c20032d0..e4b904e90879c 100644 --- a/packages/playwright/src/common/test.ts +++ b/packages/playwright/src/common/test.ts @@ -35,11 +35,19 @@ class Base { } } -export type Modifier = { +type Modifier = { type: 'slow' | 'fixme' | 'skip' | 'fail', fn: Function, location: Location, - description: string | undefined + description: string | undefined, + testType: TestTypeImpl, +}; + +export type SuiteHook = { + type: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll'; + fn: Function; + title: string; + location: Location; }; export class Suite extends Base { @@ -47,13 +55,14 @@ export class Suite extends Base { parent?: Suite; _use: FixturesWithLocation[] = []; _entries: (Suite | TestCase)[] = []; - _hooks: { type: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', fn: Function, title: string, location: Location }[] = []; + _hooks: SuiteHook[] = []; _timeout: number | undefined; _retries: number | undefined; // Annotations known statically before running the test, e.g. `test.describe.skip()` or `test.describe({ annotation }, body)`. _staticAnnotations: TestAnnotation[] = []; // Explicitly declared tags that are not a part of the title. _tags: string[] = []; + // Modifiers are converted into hooks after the fixture pool is built. _modifiers: Modifier[] = []; _parallelMode: 'none' | 'default' | 'serial' | 'parallel' = 'none'; _fullProject: FullProjectInternal | undefined; @@ -205,6 +214,14 @@ export class Suite extends Base { } } + forEachSuite(visitor: (suite: Suite) => void) { + visitor(this); + for (const entry of this._entries) { + if (entry instanceof Suite) + entry.forEachSuite(visitor); + } + } + _serialize(): any { return { kind: 'suite', @@ -217,9 +234,9 @@ export class Suite extends Base { retries: this._retries, staticAnnotations: this._staticAnnotations.slice(), tags: this._tags.slice(), - modifiers: this._modifiers.slice(), + modifiers: this._modifiers.map(m => ({ ...m, fn: undefined, testType: undefined })), parallelMode: this._parallelMode, - hooks: this._hooks.map(h => ({ type: h.type, location: h.location, title: h.title })), + hooks: this._hooks.map(h => ({ ...h, fn: undefined })), fileId: this._fileId, }; } @@ -233,9 +250,9 @@ export class Suite extends Base { suite._retries = data.retries; suite._staticAnnotations = data.staticAnnotations; suite._tags = data.tags; - suite._modifiers = data.modifiers; + suite._modifiers = data.modifiers.map((m: any) => ({ ...m, fn: () => { }, testType: rootTestType })); suite._parallelMode = data.parallelMode; - suite._hooks = data.hooks.map((h: any) => ({ type: h.type, location: h.location, title: h.title, fn: () => { } })); + suite._hooks = data.hooks.map((h: any) => ({ ...h, fn: () => { } })); suite._fileId = data.fileId; return suite; } @@ -245,6 +262,7 @@ export class Suite extends Base { const suite = Suite._parse(data); suite._use = this._use.slice(); suite._hooks = this._hooks.slice(); + suite._modifiers = this._modifiers.slice(); suite._fullProject = this._fullProject; return suite; } diff --git a/packages/playwright/src/common/testType.ts b/packages/playwright/src/common/testType.ts index e8736b667735b..7440c473e54f7 100644 --- a/packages/playwright/src/common/testType.ts +++ b/packages/playwright/src/common/testType.ts @@ -223,7 +223,7 @@ export class TestTypeImpl { } if (typeof modifierArgs[0] === 'function') { - suite._modifiers.push({ type, fn: modifierArgs[0], location, description: modifierArgs[1] }); + suite._modifiers.push({ type, fn: modifierArgs[0], location, description: modifierArgs[1], testType: this }); } else { if (modifierArgs.length >= 1 && !modifierArgs[0]) return; diff --git a/packages/playwright/src/worker/fixtureRunner.ts b/packages/playwright/src/worker/fixtureRunner.ts index c00b9fe626b87..7f8e0eacf79f1 100644 --- a/packages/playwright/src/worker/fixtureRunner.ts +++ b/packages/playwright/src/worker/fixtureRunner.ts @@ -289,16 +289,6 @@ export class FixtureRunner { await fixture.setup(testInfo, runnable); return fixture; } - - dependsOnWorkerFixturesOnly(fn: Function, location: Location): boolean { - const names = getRequiredFixtureNames(fn, location); - for (const name of names) { - const registration = this.pool!.resolve(name)!; - if (registration.scope !== 'worker') - return false; - } - return true; - } } function getRequiredFixtureNames(fn: Function, location?: Location) { diff --git a/packages/playwright/src/worker/timeoutManager.ts b/packages/playwright/src/worker/timeoutManager.ts index c003342d15806..8d18fe098d225 100644 --- a/packages/playwright/src/worker/timeoutManager.ts +++ b/packages/playwright/src/worker/timeoutManager.ts @@ -188,12 +188,6 @@ export class TimeoutManager { message = `Worker teardown timeout of ${timeout}ms exceeded.`; break; } - case 'skip': - case 'slow': - case 'fixme': - case 'fail': - message = `"${runnable.type}" modifier timeout of ${timeout}ms exceeded.`; - break; } const fixtureWithSlot = runnable.fixture?.slot ? runnable.fixture : undefined; if (fixtureWithSlot) diff --git a/packages/playwright/src/worker/workerMain.ts b/packages/playwright/src/worker/workerMain.ts index bdd46abaf00ad..8a7f5a960ce56 100644 --- a/packages/playwright/src/worker/workerMain.ts +++ b/packages/playwright/src/worker/workerMain.ts @@ -24,14 +24,12 @@ import { debugTest, relativeFilePath } from '../util'; import { FixtureRunner } from './fixtureRunner'; import { TestSkipError, TestInfoImpl, emtpyTestInfoCallbacks } from './testInfo'; import { testInfoError } from './util'; -import { inheritFixtureNames } from '../common/fixtures'; import { PoolBuilder } from '../common/poolBuilder'; import { ProcessRunner } from '../common/process'; import { applyRepeatEachIndex, bindFileSuiteToProject, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; import { loadTestFile } from '../common/testLoader'; import type { TimeSlot } from './timeoutManager'; -import type { Location } from '../../types/testReporter'; import type { FullConfigInternal, FullProjectInternal } from '../common/config'; import type * as ipc from '../common/ipc'; import type { Suite, TestCase } from '../common/test'; @@ -527,30 +525,6 @@ export class WorkerMain extends ProcessRunner { await removeFolders([testInfo.outputDir]); } - private _collectHooksAndModifiers(suite: Suite, type: 'beforeAll' | 'beforeEach' | 'afterAll' | 'afterEach', testInfo: TestInfoImpl) { - type Runnable = { type: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll' | 'fixme' | 'skip' | 'slow' | 'fail', fn: Function, title: string, location: Location }; - const runnables: Runnable[] = []; - for (const modifier of suite._modifiers) { - const modifierType = this._fixtureRunner.dependsOnWorkerFixturesOnly(modifier.fn, modifier.location) ? 'beforeAll' : 'beforeEach'; - if (modifierType !== type) - continue; - const fn = async (fixtures: any) => { - const result = await modifier.fn(fixtures); - testInfo._modifier(modifier.type, modifier.location, [!!result, modifier.description]); - }; - inheritFixtureNames(modifier.fn, fn); - runnables.push({ - title: `${modifier.type} modifier`, - location: modifier.location, - type: modifier.type, - fn, - }); - } - // Modifiers first, then hooks. - runnables.push(...suite._hooks.filter(hook => hook.type === type)); - return runnables; - } - private async _runBeforeAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl) { if (this._activeSuites.has(suite)) return; @@ -562,7 +536,7 @@ export class WorkerMain extends ProcessRunner { private async _runAllHooksForSuite(suite: Suite, testInfo: TestInfoImpl, type: 'beforeAll' | 'afterAll', extraAnnotations?: TestAnnotation[]) { // Always run all the hooks, and capture the first error. let firstError: Error | undefined; - for (const hook of this._collectHooksAndModifiers(suite, type, testInfo)) { + for (const hook of suite._hooks.filter(hook => hook.type === type)) { try { await testInfo._runAsStep({ title: hook.title, category: 'hook', location: hook.location }, async () => { // Separate time slot for each beforeAll/afterAll hook. @@ -609,7 +583,7 @@ export class WorkerMain extends ProcessRunner { private async _runEachHooksForSuites(suites: Suite[], type: 'beforeEach' | 'afterEach', testInfo: TestInfoImpl, slot?: TimeSlot) { // Always run all the hooks, unless one of the times out, and capture the first error. let firstError: Error | undefined; - const hooks = suites.map(suite => this._collectHooksAndModifiers(suite, type, testInfo)).flat(); + const hooks = suites.map(suite => suite._hooks.filter(hook => hook.type === type)).flat(); for (const hook of hooks) { const runnable = { type: hook.type, location: hook.location, slot }; if (testInfo._timeoutManager.isTimeExhaustedFor(runnable)) { diff --git a/tests/playwright-test/test-modifiers.spec.ts b/tests/playwright-test/test-modifiers.spec.ts index e73e483b278c4..b6ff2f35d6c35 100644 --- a/tests/playwright-test/test-modifiers.spec.ts +++ b/tests/playwright-test/test-modifiers.spec.ts @@ -283,13 +283,13 @@ test.describe('test modifier annotations', () => { const result = await runInlineTest({ 'a.test.ts': ` import { test, expect } from '@playwright/test'; - + test.describe.only("suite", () => { test.skip('focused skip by suite', () => {}); test.fixme('focused fixme by suite', () => {}); test.fail.only('focused fail by suite', () => { expect(1).toBe(2); }); }); - + test.describe.skip('not focused', () => { test('no marker', () => {}); }); @@ -527,7 +527,7 @@ test('modifier timeout should be reported', async ({ runInlineTest }) => { }, { timeout: 2000 }); expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.output).toContain('"skip" modifier timeout of 2000ms exceeded.'); + expect(result.output).toContain('"beforeAll" hook timeout of 2000ms exceeded.'); expect(result.output).toContain('3 | test.skip(async () => new Promise(() => {}));'); }); From 528c7044a8cc0ef020ba069db1b9ac0d1550c1cd Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 4 Mar 2026 10:40:15 -0800 Subject: [PATCH 2/3] feat(api): introduce disposable return values (#39504) --- docs/src/api/class-browsercontext.md | 3 + docs/src/api/class-disposable.md | 12 ++++ docs/src/api/class-page.md | 3 + packages/playwright-client/types/types.d.ts | 33 +++++++--- packages/playwright-core/src/client/api.ts | 1 + .../src/client/browserContext.ts | 15 +++-- .../playwright-core/src/client/connection.ts | 4 ++ .../playwright-core/src/client/disposable.ts | 40 ++++++++++++ packages/playwright-core/src/client/page.ts | 9 ++- .../playwright-core/src/protocol/validator.ts | 19 ++++-- .../src/server/browserContext.ts | 37 +++++------ packages/playwright-core/src/server/clock.ts | 64 +++++++++---------- .../dispatchers/browserContextDispatcher.ts | 28 ++++---- .../dispatchers/disposableDispatcher.ts | 36 +++++++++++ .../src/server/dispatchers/pageDispatcher.ts | 16 +++-- .../dispatchers/webSocketRouteDispatcher.ts | 6 +- .../playwright-core/src/server/disposable.ts | 31 +++++++++ .../src/server/firefox/ffPage.ts | 2 +- .../playwright-core/src/server/launchApp.ts | 2 +- packages/playwright-core/src/server/page.ts | 55 +++++++++------- .../src/server/trace/recorder/snapshotter.ts | 4 +- .../src/utils/isomorphic/protocolMetainfo.ts | 1 + packages/playwright-core/types/types.d.ts | 33 +++++++--- packages/protocol/src/channels.d.ts | 34 ++++++++-- packages/protocol/src/protocol.yml | 17 +++++ .../browsercontext-add-init-script.spec.ts | 19 ++++++ .../browsercontext-expose-function.spec.ts | 17 +++++ tests/library/channels.spec.ts | 4 ++ tests/page/page-add-init-script.spec.ts | 29 +++++++++ tests/page/page-expose-function.spec.ts | 16 +++++ utils/generate_types/overrides.d.ts | 12 ++-- 31 files changed, 464 insertions(+), 138 deletions(-) create mode 100644 docs/src/api/class-disposable.md create mode 100644 packages/playwright-core/src/client/disposable.ts create mode 100644 packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts create mode 100644 packages/playwright-core/src/server/disposable.ts diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index 384ae34abba0a..2f345a46e53c7 100644 --- a/docs/src/api/class-browsercontext.md +++ b/docs/src/api/class-browsercontext.md @@ -338,6 +338,7 @@ await context.AddCookiesAsync(new[] { cookie1, cookie2 }); ## async method: BrowserContext.addInitScript * since: v1.8 +- returns: <[Disposable]> Adds a script which would be evaluated in one of the following scenarios: * Whenever a page is created in the browser context or is navigated. @@ -584,6 +585,7 @@ Optional list of URLs. ## async method: BrowserContext.exposeBinding * since: v1.8 +- returns: <[Disposable]> The method adds a function called [`param: name`] on the `window` object of every frame in every page in the context. When called, the function executes [`param: callback`] and returns a [Promise] which resolves to the return value of @@ -735,6 +737,7 @@ supported. When passing by value, multiple arguments are supported. ## async method: BrowserContext.exposeFunction * since: v1.8 +- returns: <[Disposable]> The method adds a function called [`param: name`] on the `window` object of every frame in every page in the context. When called, the function executes [`param: callback`] and returns a [Promise] which resolves to the return value of diff --git a/docs/src/api/class-disposable.md b/docs/src/api/class-disposable.md new file mode 100644 index 0000000000000..2453f282529ac --- /dev/null +++ b/docs/src/api/class-disposable.md @@ -0,0 +1,12 @@ +# class: Disposable +* since: v1.59 +* langs: js + +[Disposable] is returned from various methods to allow undoing the corresponding action. For example, +[`method: Page.addInitScript`] returns a [Disposable] that can be used to remove the init script. + +## async method: Disposable.dispose +* since: v1.59 + +Removes the associated resource. For example, removes the init script installed via +[`method: Page.addInitScript`] or [`method: BrowserContext.addInitScript`]. diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 8e89b1af12a95..5150aaa51e997 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -559,6 +559,7 @@ page. ## async method: Page.addInitScript * since: v1.8 +- returns: <[Disposable]> Adds a script which would be evaluated in one of the following scenarios: * Whenever the page is navigated. @@ -1717,6 +1718,7 @@ Optional argument to pass to [`param: expression`]. ## async method: Page.exposeBinding * since: v1.8 +- returns: <[Disposable]> The method adds a function called [`param: name`] on the `window` object of every frame in this page. When called, the function executes [`param: callback`] and returns a [Promise] which resolves to the return value of [`param: callback`]. @@ -1882,6 +1884,7 @@ supported. When passing by value, multiple arguments are supported. ## async method: Page.exposeFunction * since: v1.8 +- returns: <[Disposable]> The method adds a function called [`param: name`] on the `window` object of every frame in the page. When called, the function executes [`param: callback`] and returns a [Promise] which resolves to the return value of [`param: callback`]. diff --git a/packages/playwright-client/types/types.d.ts b/packages/playwright-client/types/types.d.ts index e20be3e3d7c98..1dc4b7db3653e 100644 --- a/packages/playwright-client/types/types.d.ts +++ b/packages/playwright-client/types/types.d.ts @@ -315,7 +315,7 @@ export interface Page { * [`script`](https://playwright.dev/docs/api/class-page#page-add-init-script-option-script) (only supported when * passing a function). */ - addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; + addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; /** * **NOTE** Use locator-based [page.locator(selector[, options])](https://playwright.dev/docs/api/class-page#page-locator) @@ -920,7 +920,7 @@ export interface Page { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; /** * The method adds a function called * [`name`](https://playwright.dev/docs/api/class-page#page-expose-binding-option-name) on the `window` object of @@ -972,7 +972,7 @@ export interface Page { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; /** * Removes all the listeners of the given type (or all registered listeners if no type given). Allows to wait for @@ -2758,7 +2758,7 @@ export interface Page { * @param name Name of the function on the window object * @param callback Callback function which will be called in Playwright's context. */ - exposeFunction(name: string, callback: Function): Promise; + exposeFunction(name: string, callback: Function): Promise; /** * **NOTE** Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) @@ -8471,7 +8471,7 @@ export interface BrowserContext { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; /** * The method adds a function called * [`name`](https://playwright.dev/docs/api/class-browsercontext#browser-context-expose-binding-option-name) on the @@ -8519,7 +8519,7 @@ export interface BrowserContext { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; /** * Adds a script which would be evaluated in one of the following scenarios: @@ -8556,7 +8556,7 @@ export interface BrowserContext { * [`script`](https://playwright.dev/docs/api/class-browsercontext#browser-context-add-init-script-option-script) * (only supported when passing a function). */ - addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; + addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; /** * Removes all the listeners of the given type (or all registered listeners if no type given). Allows to wait for @@ -9345,7 +9345,7 @@ export interface BrowserContext { * @param name Name of the function on the window object. * @param callback Callback function that will be called in the Playwright's context. */ - exposeFunction(name: string, callback: Function): Promise; + exposeFunction(name: string, callback: Function): Promise; /** * Grants specified permissions to the browser context. Only grants corresponding permissions to the given origin if @@ -19531,6 +19531,23 @@ export interface Dialog { type(): string; } +/** + * [Disposable](https://playwright.dev/docs/api/class-disposable) is returned from various methods to allow undoing + * the corresponding action. For example, + * [page.addInitScript(script[, arg])](https://playwright.dev/docs/api/class-page#page-add-init-script) returns a + * [Disposable](https://playwright.dev/docs/api/class-disposable) that can be used to remove the init script. + */ +export interface Disposable { + /** + * Removes the associated resource. For example, removes the init script installed via + * [page.addInitScript(script[, arg])](https://playwright.dev/docs/api/class-page#page-add-init-script) or + * [browserContext.addInitScript(script[, arg])](https://playwright.dev/docs/api/class-browsercontext#browser-context-add-init-script). + */ + dispose(): Promise; + + [Symbol.asyncDispose](): Promise; +} + /** * [Download](https://playwright.dev/docs/api/class-download) objects are dispatched by page via the * [page.on('download')](https://playwright.dev/docs/api/class-page#page-event-download) event. diff --git a/packages/playwright-core/src/client/api.ts b/packages/playwright-core/src/client/api.ts index db6d504267a4f..d84904d0d1725 100644 --- a/packages/playwright-core/src/client/api.ts +++ b/packages/playwright-core/src/client/api.ts @@ -23,6 +23,7 @@ export { Clock } from './clock'; export { ConsoleMessage } from './consoleMessage'; export { Coverage } from './coverage'; export { Dialog } from './dialog'; +export { Disposable } from './disposable'; export { Download } from './download'; export { Electron, ElectronApplication } from './electron'; export { FrameLocator, Locator } from './locator'; diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index a123ae698bee6..2dab10473af31 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -23,6 +23,7 @@ import { evaluationScript } from './clientHelper'; import { Clock } from './clock'; import { ConsoleMessage } from './consoleMessage'; import { Dialog } from './dialog'; +import { Disposable } from './disposable'; import { TargetClosedError, parseError } from './errors'; import { Events } from './events'; import { APIRequestContext } from './fetch'; @@ -349,20 +350,22 @@ export class BrowserContext extends ChannelOwner await this._channel.setHTTPCredentials({ httpCredentials: httpCredentials || undefined }); } - async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any): Promise { + async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) { const source = await evaluationScript(this._platform, script, arg); - await this._channel.addInitScript({ source }); + return Disposable.from((await this._channel.addInitScript({ source })).disposable); } - async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}): Promise { - await this._channel.exposeBinding({ name, needsHandle: options.handle }); + async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}): Promise { + const result = await this._channel.exposeBinding({ name, needsHandle: options.handle }); this._bindings.set(name, callback); + return Disposable.from(result.disposable); } - async exposeFunction(name: string, callback: Function): Promise { - await this._channel.exposeBinding({ name }); + async exposeFunction(name: string, callback: Function): Promise { + const result = await this._channel.exposeBinding({ name }); const binding = (source: structs.BindingSource, ...args: any[]) => callback(...args); this._bindings.set(name, binding); + return Disposable.from(result.disposable); } async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise { diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index fcd56ee6e6032..6b794e68a7ed9 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -24,6 +24,7 @@ import { CDPSession } from './cdpSession'; import { ChannelOwner } from './channelOwner'; import { createInstrumentation } from './clientInstrumentation'; import { Dialog } from './dialog'; +import { Disposable } from './disposable'; import { Electron, ElectronApplication } from './electron'; import { ElementHandle } from './elementHandle'; import { TargetClosedError, parseError } from './errors'; @@ -270,6 +271,9 @@ export class Connection extends EventEmitter { case 'Dialog': result = new Dialog(parent, type, guid, initializer); break; + case 'Disposable': + result = new Disposable(parent, type, guid, initializer); + break; case 'Electron': result = new Electron(parent, type, guid, initializer); break; diff --git a/packages/playwright-core/src/client/disposable.ts b/packages/playwright-core/src/client/disposable.ts new file mode 100644 index 0000000000000..fe32640e1665d --- /dev/null +++ b/packages/playwright-core/src/client/disposable.ts @@ -0,0 +1,40 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ChannelOwner } from './channelOwner'; +import { isTargetClosedError } from './errors'; + +import type * as channels from '@protocol/channels'; + +export class Disposable extends ChannelOwner { + static from(channel: channels.DisposableChannel): Disposable { + return (channel as any)._object; + } + + async [Symbol.asyncDispose]() { + await this.dispose(); + } + + async dispose() { + try { + await this._channel.dispose(); + } catch (e) { + if (isTargetClosedError(e)) + return; + throw e; + } + } +} diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index 72a4f9eda5218..46dededd8a92b 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -19,6 +19,7 @@ import { Artifact } from './artifact'; import { ChannelOwner } from './channelOwner'; import { evaluationScript } from './clientHelper'; import { Coverage } from './coverage'; +import { Disposable } from './disposable'; import { Download } from './download'; import { ElementHandle, determineScreenshotType } from './elementHandle'; import { TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors'; @@ -332,14 +333,16 @@ export class Page extends ChannelOwner implements api.Page } async exposeFunction(name: string, callback: Function) { - await this._channel.exposeBinding({ name }); + const result = await this._channel.exposeBinding({ name }); const binding = (source: structs.BindingSource, ...args: any[]) => callback(...args); this._bindings.set(name, binding); + return Disposable.from(result.disposable); } async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}) { - await this._channel.exposeBinding({ name, needsHandle: options.handle }); + const result = await this._channel.exposeBinding({ name, needsHandle: options.handle }); this._bindings.set(name, callback); + return Disposable.from(result.disposable); } async setExtraHTTPHeaders(headers: Headers) { @@ -507,7 +510,7 @@ export class Page extends ChannelOwner implements api.Page async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) { const source = await evaluationScript(this._platform, script, arg); - await this._channel.addInitScript({ source }); + return Disposable.from((await this._channel.addInitScript({ source })).disposable); } async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index fc1fea1a46a52..a7b44c1542a19 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -987,7 +987,9 @@ scheme.BrowserContextAddCookiesResult = tOptional(tObject({})); scheme.BrowserContextAddInitScriptParams = tObject({ source: tString, }); -scheme.BrowserContextAddInitScriptResult = tOptional(tObject({})); +scheme.BrowserContextAddInitScriptResult = tObject({ + disposable: tChannel(['Disposable']), +}); scheme.BrowserContextClearCookiesParams = tObject({ name: tOptional(tString), nameRegexSource: tOptional(tString), @@ -1016,7 +1018,9 @@ scheme.BrowserContextExposeBindingParams = tObject({ name: tString, needsHandle: tOptional(tBoolean), }); -scheme.BrowserContextExposeBindingResult = tOptional(tObject({})); +scheme.BrowserContextExposeBindingResult = tObject({ + disposable: tChannel(['Disposable']), +}); scheme.BrowserContextGrantPermissionsParams = tObject({ permissions: tArray(tString), origin: tOptional(tString), @@ -1242,7 +1246,9 @@ scheme.PageWorkerEvent = tObject({ scheme.PageAddInitScriptParams = tObject({ source: tString, }); -scheme.PageAddInitScriptResult = tOptional(tObject({})); +scheme.PageAddInitScriptResult = tObject({ + disposable: tChannel(['Disposable']), +}); scheme.PageCloseParams = tObject({ runBeforeUnload: tOptional(tBoolean), reason: tOptional(tString), @@ -1276,7 +1282,9 @@ scheme.PageExposeBindingParams = tObject({ name: tString, needsHandle: tOptional(tBoolean), }); -scheme.PageExposeBindingResult = tOptional(tObject({})); +scheme.PageExposeBindingResult = tObject({ + disposable: tChannel(['Disposable']), +}); scheme.PageGoBackParams = tObject({ timeout: tFloat, waitUntil: tOptional(tType('LifecycleEvent')), @@ -2028,6 +2036,9 @@ scheme.WorkerUpdateSubscriptionParams = tObject({ enabled: tBoolean, }); scheme.WorkerUpdateSubscriptionResult = tOptional(tObject({})); +scheme.DisposableInitializer = tOptional(tObject({})); +scheme.DisposableDisposeParams = tOptional(tObject({})); +scheme.DisposableDisposeResult = tOptional(tObject({})); scheme.JSHandleInitializer = tObject({ preview: tString, }); diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 1dbb020a875db..6fd0c6e8f18f2 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -169,7 +169,7 @@ export abstract class BrowserContext extends Sdk await this.exposeConsoleApi(); if (this._options.serviceWorkers === 'block') - await this.addInitScript(undefined, `\nif (navigator.serviceWorker) navigator.serviceWorker.register = async () => { console.warn('Service Worker registration blocked by Playwright'); };\n`); + await this.addInitScript(`\nif (navigator.serviceWorker) navigator.serviceWorker.register = async () => { console.warn('Service Worker registration blocked by Playwright'); };\n`); if (this._options.permissions) await this.grantPermissions(this._options.permissions); @@ -340,7 +340,7 @@ export abstract class BrowserContext extends Sdk this._playwrightBindingExposed ??= (async () => { await this.doExposePlaywrightBinding(); - this.bindingsInitScript = PageBinding.createInitScript(); + this.bindingsInitScript = PageBinding.createInitScript(this); this.initScripts.push(this.bindingsInitScript); await this.doAddInitScript(this.bindingsInitScript); await this.safeNonStallingEvaluateInAllFrames(this.bindingsInitScript.source, 'main'); @@ -360,7 +360,7 @@ export abstract class BrowserContext extends Sdk throw new Error(`Function "${name}" has been already registered in one of the pages`); } await progress.race(this.exposePlaywrightBindingIfNeeded()); - const binding = new PageBinding(name, playwrightBinding, needsHandle); + const binding = new PageBinding(this, name, playwrightBinding, needsHandle); binding.forClient = forClient; this._pageBindings.set(name, binding); try { @@ -373,12 +373,12 @@ export abstract class BrowserContext extends Sdk } } - async removeExposedBindings(bindings: PageBinding[]) { - bindings = bindings.filter(binding => this._pageBindings.get(binding.name) === binding); - for (const binding of bindings) - this._pageBindings.delete(binding.name); - await this.doRemoveInitScripts(bindings.map(binding => binding.initScript)); - const cleanup = bindings.map(binding => `{ ${binding.cleanupScript} };\n`).join(''); + async removeExposedBinding(binding: PageBinding) { + if (this._pageBindings.get(binding.name) !== binding) + return; + this._pageBindings.delete(binding.name); + await this.doRemoveInitScripts([binding.initScript]); + const cleanup = `{ ${binding.cleanupScript} };`; await this.safeNonStallingEvaluateInAllFrames(cleanup, 'main'); } @@ -478,27 +478,22 @@ export abstract class BrowserContext extends Sdk this._options.httpCredentials = { username, password: password || '' }; } - async addInitScript(progress: Progress | undefined, source: string) { - const initScript = new InitScript(source); + async addInitScript(source: string) { + const initScript = new InitScript(this, source); this.initScripts.push(initScript); try { - const promise = this.doAddInitScript(initScript); - if (progress) - await progress.race(promise); - else - await promise; + await this.doAddInitScript(initScript); return initScript; } catch (error) { // Note: no await, init script will be removed in the background as soon as possible. - this.removeInitScripts([initScript]).catch(() => {}); + initScript.dispose().catch(() => {}); throw error; } } - async removeInitScripts(initScripts: InitScript[]) { - const set = new Set(initScripts); - this.initScripts = this.initScripts.filter(script => !set.has(script)); - await this.doRemoveInitScripts(initScripts); + async removeInitScript(initScript: InitScript) { + this.initScripts = this.initScripts.filter(script => initScript !== script); + await this.doRemoveInitScripts([initScript]); } async addRequestInterceptor(progress: Progress, handler: network.RouteHandler): Promise { diff --git a/packages/playwright-core/src/server/clock.ts b/packages/playwright-core/src/server/clock.ts index 86427c7b840e8..4c15f9ec13867 100644 --- a/packages/playwright-core/src/server/clock.ts +++ b/packages/playwright-core/src/server/clock.ts @@ -29,69 +29,69 @@ export class Clock { } async uninstall(progress: Progress) { - await progress.race(this._browserContext.removeInitScripts(this._initScripts)); + await progress.race(Promise.all(this._initScripts.map(script => script.dispose()))); this._initScripts = []; } - async fastForward(progress: Progress, ticks: number | string) { - await this._installIfNeeded(progress); + async fastForward(ticks: number | string) { + await this._installIfNeeded(); const ticksMillis = parseTicks(ticks); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('fastForward', ${Date.now()}, ${ticksMillis})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.fastForward(${ticksMillis})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('fastForward', ${Date.now()}, ${ticksMillis})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.fastForward(${ticksMillis})`); } - async install(progress: Progress, time: number | string | undefined) { - await this._installIfNeeded(progress); + async install(time: number | string | undefined) { + await this._installIfNeeded(); const timeMillis = time !== undefined ? parseTime(time) : Date.now(); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('install', ${Date.now()}, ${timeMillis})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.install(${timeMillis})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('install', ${Date.now()}, ${timeMillis})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.install(${timeMillis})`); } - async pauseAt(progress: Progress, ticks: number | string) { - await this._installIfNeeded(progress); + async pauseAt(ticks: number | string) { + await this._installIfNeeded(); const timeMillis = parseTime(ticks); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('pauseAt', ${Date.now()}, ${timeMillis})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.pauseAt(${timeMillis})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('pauseAt', ${Date.now()}, ${timeMillis})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.pauseAt(${timeMillis})`); } resumeNoReply() { if (!this._initScripts.length) return; const doResume = async () => { - this._initScripts.push(await this._browserContext.addInitScript(undefined, `globalThis.__pwClock.controller.log('resume', ${Date.now()})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('resume', ${Date.now()})`)); await this._evaluateInFrames(`globalThis.__pwClock.controller.resume()`); }; doResume().catch(() => {}); } async resume(progress: Progress) { - await this._installIfNeeded(progress); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('resume', ${Date.now()})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.resume()`)); + await this._installIfNeeded(); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('resume', ${Date.now()})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.resume()`); } - async setFixedTime(progress: Progress, time: string | number) { - await this._installIfNeeded(progress); + async setFixedTime(time: string | number) { + await this._installIfNeeded(); const timeMillis = parseTime(time); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('setFixedTime', ${Date.now()}, ${timeMillis})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.setFixedTime(${timeMillis})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('setFixedTime', ${Date.now()}, ${timeMillis})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.setFixedTime(${timeMillis})`); } - async setSystemTime(progress: Progress, time: string | number) { - await this._installIfNeeded(progress); + async setSystemTime(time: string | number) { + await this._installIfNeeded(); const timeMillis = parseTime(time); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('setSystemTime', ${Date.now()}, ${timeMillis})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.setSystemTime(${timeMillis})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('setSystemTime', ${Date.now()}, ${timeMillis})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.setSystemTime(${timeMillis})`); } - async runFor(progress: Progress, ticks: number | string) { - await this._installIfNeeded(progress); + async runFor(ticks: number | string) { + await this._installIfNeeded(); const ticksMillis = parseTicks(ticks); - this._initScripts.push(await this._browserContext.addInitScript(progress, `globalThis.__pwClock.controller.log('runFor', ${Date.now()}, ${ticksMillis})`)); - await progress.race(this._evaluateInFrames(`globalThis.__pwClock.controller.runFor(${ticksMillis})`)); + this._initScripts.push(await this._browserContext.addInitScript(`globalThis.__pwClock.controller.log('runFor', ${Date.now()}, ${ticksMillis})`)); + await this._evaluateInFrames(`globalThis.__pwClock.controller.runFor(${ticksMillis})`); } - private async _installIfNeeded(progress: Progress) { + private async _installIfNeeded() { if (this._initScripts.length) return; const script = `(() => { @@ -100,8 +100,8 @@ export class Clock { if (!globalThis.__pwClock) globalThis.__pwClock = (module.exports.inject())(globalThis, ${JSON.stringify(this._browserContext._browser.options.name)}); })();`; - const initScript = await this._browserContext.addInitScript(progress, script); - await progress.race(this._evaluateInFrames(script)); + const initScript = await this._browserContext.addInitScript(script); + await this._evaluateInFrames(script); this._initScripts.push(initScript); } diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index 49ba7cbda3307..82756d1fa814d 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -27,6 +27,7 @@ import { APIRequestContextDispatcher, RequestDispatcher, ResponseDispatcher, Rou import { BindingCallDispatcher, PageDispatcher, WorkerDispatcher } from './pageDispatcher'; import { CRBrowser, CRBrowserContext } from '../chromium/crBrowser'; import { serializeError } from '../errors'; +import { DisposableDispatcher } from './disposableDispatcher'; import { TracingDispatcher } from './tracingDispatcher'; import { WebSocketRouteDispatcher } from './webSocketRouteDispatcher'; import { WritableStreamDispatcher } from './writableStreamDispatcher'; @@ -227,7 +228,7 @@ export class BrowserContextDispatcher extends Dispatcher { + async exposeBinding(params: channels.BrowserContextExposeBindingParams, progress: Progress): Promise { const binding = await this._context.exposeBinding(progress, params.name, !!params.needsHandle, (source, ...args) => { // When reusing the context, we might have some bindings called late enough, // after context and page dispatchers have been disposed. @@ -239,6 +240,7 @@ export class BrowserContextDispatcher extends Dispatcher { @@ -294,8 +296,10 @@ export class BrowserContextDispatcher extends Dispatcher { - this._initScripts.push(await this._context.addInitScript(progress, params.source)); + async addInitScript(params: channels.BrowserContextAddInitScriptParams, progress: Progress): Promise { + const initScript = await this._context.addInitScript(params.source); + this._initScripts.push(initScript); + return { disposable: new DisposableDispatcher(this, initScript) }; } async setNetworkInterceptionPatterns(params: channels.BrowserContextSetNetworkInterceptionPatternsParams, progress: Progress): Promise { @@ -371,15 +375,15 @@ export class BrowserContextDispatcher extends Dispatcher { - await this._context.clock.fastForward(progress, params.ticksString ?? params.ticksNumber ?? 0); + await this._context.clock.fastForward(params.ticksString ?? params.ticksNumber ?? 0); } async clockInstall(params: channels.BrowserContextClockInstallParams, progress: Progress): Promise { - await this._context.clock.install(progress, params.timeString ?? params.timeNumber ?? undefined); + await this._context.clock.install(params.timeString ?? params.timeNumber ?? undefined); } async clockPauseAt(params: channels.BrowserContextClockPauseAtParams, progress: Progress): Promise { - await this._context.clock.pauseAt(progress, params.timeString ?? params.timeNumber ?? 0); + await this._context.clock.pauseAt(params.timeString ?? params.timeNumber ?? 0); this._clockPaused = true; } @@ -389,15 +393,15 @@ export class BrowserContextDispatcher extends Dispatcher { - await this._context.clock.runFor(progress, params.ticksString ?? params.ticksNumber ?? 0); + await this._context.clock.runFor(params.ticksString ?? params.ticksNumber ?? 0); } async clockSetFixedTime(params: channels.BrowserContextClockSetFixedTimeParams, progress: Progress): Promise { - await this._context.clock.setFixedTime(progress, params.timeString ?? params.timeNumber ?? 0); + await this._context.clock.setFixedTime(params.timeString ?? params.timeNumber ?? 0); } async clockSetSystemTime(params: channels.BrowserContextClockSetSystemTimeParams, progress: Progress): Promise { - await this._context.clock.setSystemTime(progress, params.timeString ?? params.timeNumber ?? 0); + await this._context.clock.setSystemTime(params.timeString ?? params.timeNumber ?? 0); } async devtoolsStart(params: channels.BrowserContextDevtoolsStartParams, progress: Progress): Promise { @@ -429,9 +433,11 @@ export class BrowserContextDispatcher extends Dispatcher {}); - this._context.removeExposedBindings(this._bindings).catch(() => {}); + for (const binding of this._bindings) + binding.dispose().catch(() => {}); this._bindings = []; - this._context.removeInitScripts(this._initScripts).catch(() => {}); + for (const initScript of this._initScripts) + initScript.dispose().catch(() => {}); this._initScripts = []; if (this._routeWebSocketInitScript) WebSocketRouteDispatcher.uninstall(this.connection, this._context, this._routeWebSocketInitScript).catch(() => {}); diff --git a/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts b/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts new file mode 100644 index 0000000000000..a262e0684e76d --- /dev/null +++ b/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts @@ -0,0 +1,36 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Dispatcher } from './dispatcher'; + +import type { Disposable } from '../disposable'; +import type { DispatcherScope } from './dispatcher'; +import type * as channels from '@protocol/channels'; +import type { Progress } from '@protocol/progress'; + +export class DisposableDispatcher extends Dispatcher implements channels.DisposableChannel { + _type_Disposable = true; + + constructor(scope: DispatcherScope, disposable: Disposable) { + super(scope, disposable, 'Disposable', {}); + } + + async dispose(_: any, progress: Progress) { + progress.metadata.potentiallyClosesScope = true; + await this._object.dispose(); + this._dispose(); + } +} diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts index ccfde8ec5a688..cd7a40962d0bd 100644 --- a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts @@ -26,6 +26,7 @@ import { RequestDispatcher } from './networkDispatchers'; import { ResponseDispatcher } from './networkDispatchers'; import { RouteDispatcher, WebSocketDispatcher } from './networkDispatchers'; import { WebSocketRouteDispatcher } from './webSocketRouteDispatcher'; +import { DisposableDispatcher } from './disposableDispatcher'; import { SdkObject } from '../instrumentation'; import { deserializeURLMatch, urlMatches } from '../../utils/isomorphic/urlMatch'; import { PageAgentDispatcher } from './pageAgentDispatcher'; @@ -129,7 +130,7 @@ export class PageDispatcher extends Dispatcher { + async exposeBinding(params: channels.PageExposeBindingParams, progress: Progress): Promise { const binding = await this._page.exposeBinding(progress, params.name, !!params.needsHandle, (source, ...args) => { // When reusing the context, we might have some bindings called late enough, // after context and page dispatchers have been disposed. @@ -140,6 +141,7 @@ export class PageDispatcher extends Dispatcher { @@ -191,8 +193,10 @@ export class PageDispatcher extends Dispatcher { - this._initScripts.push(await this._page.addInitScript(progress, params.source)); + async addInitScript(params: channels.PageAddInitScriptParams, progress: Progress): Promise { + const initScript = await this._page.addInitScript(params.source); + this._initScripts.push(initScript); + return { disposable: new DisposableDispatcher(this, initScript) }; } async setNetworkInterceptionPatterns(params: channels.PageSetNetworkInterceptionPatternsParams, progress: Progress): Promise { @@ -421,9 +425,11 @@ export class PageDispatcher extends Dispatcher {}); - this._page.removeExposedBindings(this._bindings).catch(() => {}); + for (const binding of this._bindings) + this._page.removeExposedBinding(binding).catch(() => {}); this._bindings = []; - this._page.removeInitScripts(this._initScripts).catch(() => {}); + for (const initScript of this._initScripts) + initScript.dispose().catch(() => {}); this._initScripts = []; if (this._routeWebSocketInitScript) WebSocketRouteDispatcher.uninstall(this.connection, this._page, this._routeWebSocketInitScript).catch(() => {}); diff --git a/packages/playwright-core/src/server/dispatchers/webSocketRouteDispatcher.ts b/packages/playwright-core/src/server/dispatchers/webSocketRouteDispatcher.ts index b32000341b9d7..9015f004ad991 100644 --- a/packages/playwright-core/src/server/dispatchers/webSocketRouteDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/webSocketRouteDispatcher.ts @@ -97,7 +97,7 @@ export class WebSocketRouteDispatcher extends Dispatcher { const module = {}; ${rawWebSocketMockSource.source} @@ -112,8 +112,8 @@ export class WebSocketRouteDispatcher extends Dispatcher; +} diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts index 9677ab4493743..f896a6196d7b9 100644 --- a/packages/playwright-core/src/server/firefox/ffPage.ts +++ b/packages/playwright-core/src/server/firefox/ffPage.ts @@ -106,7 +106,7 @@ export class FFPage implements PageDelegate { // Ideally, we somehow ensure that utility world is created before Page.ready arrives, but currently it is racy. // Even worse, sometimes this protocol call never returns, for example when popup opens a dialog synchronously. // Therefore, we can end up with an initialized page without utility world, although very unlikely. - this.addInitScript(new InitScript(''), UTILITY_WORLD_NAME).catch(e => this._reportAsNew(e)); + this.addInitScript(new InitScript(this._page, ''), UTILITY_WORLD_NAME).catch(e => this._reportAsNew(e)); } _reportAsNew(error?: Error) { diff --git a/packages/playwright-core/src/server/launchApp.ts b/packages/playwright-core/src/server/launchApp.ts index ee65f3ca27258..d92b83b7af53e 100644 --- a/packages/playwright-core/src/server/launchApp.ts +++ b/packages/playwright-core/src/server/launchApp.ts @@ -108,7 +108,7 @@ export async function syncLocalStorageWithSettings(page: Page, appName: string) }); const settings = await fs.promises.readFile(settingsFile, 'utf-8').catch(() => ('{}')); - await page.addInitScript(progress, + await page.addInitScript( `(${String((settings: any) => { // iframes w/ snapshots, etc. if (location && location.protocol === 'data:') diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index ec708bce58ec3..e756c6bebcf29 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -16,6 +16,7 @@ */ import { BrowserContext } from './browserContext'; +import { Disposable } from './disposable'; import { ConsoleMessage } from './console'; import { TargetClosedError, TimeoutError } from './errors'; import { FileChooser } from './fileChooser'; @@ -329,7 +330,7 @@ export class Page extends SdkObject { if (this.browserContext._pageBindings.has(name)) throw new Error(`Function "${name}" has been already registered in the browser context`); await progress.race(this.browserContext.exposePlaywrightBindingIfNeeded()); - const binding = new PageBinding(name, playwrightBinding, needsHandle); + const binding = new PageBinding(this, name, playwrightBinding, needsHandle); this._pageBindings.set(name, binding); try { await progress.race(this.delegate.addInitScript(binding.initScript)); @@ -341,12 +342,12 @@ export class Page extends SdkObject { } } - async removeExposedBindings(bindings: PageBinding[]) { - bindings = bindings.filter(binding => this._pageBindings.get(binding.name) === binding); - for (const binding of bindings) - this._pageBindings.delete(binding.name); - await this.delegate.removeInitScripts(bindings.map(binding => binding.initScript)); - const cleanup = bindings.map(binding => `{ ${binding.cleanupScript} };\n`).join(''); + async removeExposedBinding(binding: PageBinding) { + if (this._pageBindings.get(binding.name) !== binding) + return; + this._pageBindings.delete(binding.name); + await this.delegate.removeInitScripts([binding.initScript]); + const cleanup = `{ ${binding.cleanupScript} };`; await this.safeNonStallingEvaluateInAllFrames(cleanup, 'main'); } @@ -627,23 +628,22 @@ export class Page extends SdkObject { await this.delegate.bringToFront(); } - async addInitScript(progress: Progress, source: string) { - const initScript = new InitScript(source); + async addInitScript(source: string) { + const initScript = new InitScript(this, source); this.initScripts.push(initScript); try { - await progress.race(this.delegate.addInitScript(initScript)); + await this.delegate.addInitScript(initScript); } catch (error) { // Note: no await, script will be removed in the background as soon as possible. - this.removeInitScripts([initScript]).catch(() => {}); + initScript.dispose().catch(() => {}); throw error; } return initScript; } - async removeInitScripts(initScripts: InitScript[]) { - const set = new Set(initScripts); - this.initScripts = this.initScripts.filter(script => !set.has(script)); - await this.delegate.removeInitScripts(initScripts); + async removeInitScript(initScript: InitScript) { + this.initScripts = this.initScripts.filter(script => initScript !== script); + await this.delegate.removeInitScripts([initScript]); } needsRequestInterception(): boolean { @@ -933,12 +933,12 @@ export class Worker extends SdkObject { } } -export class PageBinding { +export class PageBinding extends Disposable { private static kController = '__playwright__binding__controller__'; static kBindingName = '__playwright__binding__'; - static createInitScript() { - return new InitScript(` + static createInitScript(browserContext: BrowserContext): InitScript { + return new InitScript(browserContext, ` (() => { const module = {}; ${rawBindingsControllerSource.source} @@ -956,10 +956,11 @@ export class PageBinding { readonly cleanupScript: string; forClient?: unknown; - constructor(name: string, playwrightFunction: frames.FunctionWithSource, needsHandle: boolean) { + constructor(parent: BrowserContext | Page, name: string, playwrightFunction: frames.FunctionWithSource, needsHandle: boolean) { + super(parent); this.name = name; this.playwrightFunction = playwrightFunction; - this.initScript = new InitScript(`globalThis['${PageBinding.kController}'].addBinding(${JSON.stringify(name)}, ${needsHandle})`); + this.initScript = new InitScript(parent, `globalThis['${PageBinding.kController}'].addBinding(${JSON.stringify(name)}, ${needsHandle})`); this.needsHandle = needsHandle; this.cleanupScript = `globalThis['${PageBinding.kController}'].removeBinding(${JSON.stringify(name)})`; } @@ -986,18 +987,26 @@ export class PageBinding { context.evaluateExpressionHandle(`arg => globalThis['${PageBinding.kController}'].deliverBindingResult(arg)`, { isFunction: true }, { name, seq, error }).catch(e => debugLogger.log('error', e)); } } + + override async dispose(): Promise { + await this.parent.removeExposedBinding(this); + } } -export class InitScript { +export class InitScript extends Disposable { readonly source: string; - constructor(source: string) { + constructor(owner: BrowserContext | Page, source: string) { + super(owner); this.source = `(() => { ${source} })();`; } -} + async dispose() { + await this.parent.removeInitScript(this); + } +} async function snapshotFrameForAI(progress: Progress, frame: frames.Frame, options: { track?: string, doNotRenderActive?: boolean } = {}): Promise<{ full: string[], incremental?: string[] }> { // Only await the topmost navigations, inner frames will be empty when racing. diff --git a/packages/playwright-core/src/server/trace/recorder/snapshotter.ts b/packages/playwright-core/src/server/trace/recorder/snapshotter.ts index 9b55439052dc5..6b0c558507891 100644 --- a/packages/playwright-core/src/server/trace/recorder/snapshotter.ts +++ b/packages/playwright-core/src/server/trace/recorder/snapshotter.ts @@ -78,7 +78,7 @@ export class Snapshotter { // Next time we start recording, we will call addInitScript again. if (this._initScript) { eventsHelper.removeEventListeners(this._eventListeners); - await this._context.removeInitScripts([this._initScript]); + await this._initScript.dispose(); this._initScript = undefined; } } @@ -92,7 +92,7 @@ export class Snapshotter { const { javaScriptEnabled } = this._context._options; const initScriptSource = `(${frameSnapshotStreamer})("${this._snapshotStreamer}", ${javaScriptEnabled || javaScriptEnabled === undefined})`; - this._initScript = await this._context.addInitScript(undefined, initScriptSource); + this._initScript = await this._context.addInitScript(initScriptSource); await this._context.safeNonStallingEvaluateInAllFrames(initScriptSource, 'main'); } diff --git a/packages/playwright-core/src/utils/isomorphic/protocolMetainfo.ts b/packages/playwright-core/src/utils/isomorphic/protocolMetainfo.ts index 8b5f29d09bcde..eecd0a029fbea 100644 --- a/packages/playwright-core/src/utils/isomorphic/protocolMetainfo.ts +++ b/packages/playwright-core/src/utils/isomorphic/protocolMetainfo.ts @@ -205,6 +205,7 @@ export const methodMetainfo = new Map(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; + addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; /** * **NOTE** Use locator-based [page.locator(selector[, options])](https://playwright.dev/docs/api/class-page#page-locator) @@ -920,7 +920,7 @@ export interface Page { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; /** * The method adds a function called * [`name`](https://playwright.dev/docs/api/class-page#page-expose-binding-option-name) on the `window` object of @@ -972,7 +972,7 @@ export interface Page { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; /** * Removes all the listeners of the given type (or all registered listeners if no type given). Allows to wait for @@ -2758,7 +2758,7 @@ export interface Page { * @param name Name of the function on the window object * @param callback Callback function which will be called in Playwright's context. */ - exposeFunction(name: string, callback: Function): Promise; + exposeFunction(name: string, callback: Function): Promise; /** * **NOTE** Use locator-based [locator.fill(value[, options])](https://playwright.dev/docs/api/class-locator#locator-fill) @@ -8471,7 +8471,7 @@ export interface BrowserContext { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; /** * The method adds a function called * [`name`](https://playwright.dev/docs/api/class-browsercontext#browser-context-expose-binding-option-name) on the @@ -8519,7 +8519,7 @@ export interface BrowserContext { * @param callback Callback function that will be called in the Playwright's context. * @param options */ - exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; /** * Adds a script which would be evaluated in one of the following scenarios: @@ -8556,7 +8556,7 @@ export interface BrowserContext { * [`script`](https://playwright.dev/docs/api/class-browsercontext#browser-context-add-init-script-option-script) * (only supported when passing a function). */ - addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; + addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; /** * Removes all the listeners of the given type (or all registered listeners if no type given). Allows to wait for @@ -9345,7 +9345,7 @@ export interface BrowserContext { * @param name Name of the function on the window object. * @param callback Callback function that will be called in the Playwright's context. */ - exposeFunction(name: string, callback: Function): Promise; + exposeFunction(name: string, callback: Function): Promise; /** * Grants specified permissions to the browser context. Only grants corresponding permissions to the given origin if @@ -19531,6 +19531,23 @@ export interface Dialog { type(): string; } +/** + * [Disposable](https://playwright.dev/docs/api/class-disposable) is returned from various methods to allow undoing + * the corresponding action. For example, + * [page.addInitScript(script[, arg])](https://playwright.dev/docs/api/class-page#page-add-init-script) returns a + * [Disposable](https://playwright.dev/docs/api/class-disposable) that can be used to remove the init script. + */ +export interface Disposable { + /** + * Removes the associated resource. For example, removes the init script installed via + * [page.addInitScript(script[, arg])](https://playwright.dev/docs/api/class-page#page-add-init-script) or + * [browserContext.addInitScript(script[, arg])](https://playwright.dev/docs/api/class-browsercontext#browser-context-add-init-script). + */ + dispose(): Promise; + + [Symbol.asyncDispose](): Promise; +} + /** * [Download](https://playwright.dev/docs/api/class-download) objects are dispatched by page via the * [page.on('download')](https://playwright.dev/docs/api/class-page#page-event-download) event. diff --git a/packages/protocol/src/channels.d.ts b/packages/protocol/src/channels.d.ts index 8f9ca58401488..b941c06c8705d 100644 --- a/packages/protocol/src/channels.d.ts +++ b/packages/protocol/src/channels.d.ts @@ -47,6 +47,7 @@ export type InitializerTraits = T extends RequestChannel ? RequestInitializer : T extends ElementHandleChannel ? ElementHandleInitializer : T extends JSHandleChannel ? JSHandleInitializer : + T extends DisposableChannel ? DisposableInitializer : T extends WorkerChannel ? WorkerInitializer : T extends FrameChannel ? FrameInitializer : T extends PageChannel ? PageInitializer : @@ -85,6 +86,7 @@ export type EventsTraits = T extends RequestChannel ? RequestEvents : T extends ElementHandleChannel ? ElementHandleEvents : T extends JSHandleChannel ? JSHandleEvents : + T extends DisposableChannel ? DisposableEvents : T extends WorkerChannel ? WorkerEvents : T extends FrameChannel ? FrameEvents : T extends PageChannel ? PageEvents : @@ -123,6 +125,7 @@ export type EventTargetTraits = T extends RequestChannel ? RequestEventTarget : T extends ElementHandleChannel ? ElementHandleEventTarget : T extends JSHandleChannel ? JSHandleEventTarget : + T extends DisposableChannel ? DisposableEventTarget : T extends WorkerChannel ? WorkerEventTarget : T extends FrameChannel ? FrameEventTarget : T extends PageChannel ? PageEventTarget : @@ -1739,7 +1742,9 @@ export type BrowserContextAddInitScriptParams = { export type BrowserContextAddInitScriptOptions = { }; -export type BrowserContextAddInitScriptResult = void; +export type BrowserContextAddInitScriptResult = { + disposable: DisposableChannel, +}; export type BrowserContextClearCookiesParams = { name?: string, nameRegexSource?: string, @@ -1789,7 +1794,9 @@ export type BrowserContextExposeBindingParams = { export type BrowserContextExposeBindingOptions = { needsHandle?: boolean, }; -export type BrowserContextExposeBindingResult = void; +export type BrowserContextExposeBindingResult = { + disposable: DisposableChannel, +}; export type BrowserContextGrantPermissionsParams = { permissions: string[], origin?: string, @@ -2209,7 +2216,9 @@ export type PageAddInitScriptParams = { export type PageAddInitScriptOptions = { }; -export type PageAddInitScriptResult = void; +export type PageAddInitScriptResult = { + disposable: DisposableChannel, +}; export type PageCloseParams = { runBeforeUnload?: boolean, reason?: string, @@ -2259,7 +2268,9 @@ export type PageExposeBindingParams = { export type PageExposeBindingOptions = { needsHandle?: boolean, }; -export type PageExposeBindingResult = void; +export type PageExposeBindingResult = { + disposable: DisposableChannel, +}; export type PageGoBackParams = { timeout: number, waitUntil?: LifecycleEvent, @@ -3500,6 +3511,21 @@ export interface WorkerEvents { 'close': WorkerCloseEvent; } +// ----------- Disposable ----------- +export type DisposableInitializer = {}; +export interface DisposableEventTarget { +} +export interface DisposableChannel extends DisposableEventTarget, Channel { + _type_Disposable: boolean; + dispose(params?: DisposableDisposeParams, progress?: Progress): Promise; +} +export type DisposableDisposeParams = {}; +export type DisposableDisposeOptions = {}; +export type DisposableDisposeResult = void; + +export interface DisposableEvents { +} + // ----------- JSHandle ----------- export type JSHandleInitializer = { preview: string, diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index 5add91162acb0..0f6c9a4d3244e 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -1206,6 +1206,8 @@ BrowserContext: group: configuration parameters: source: string + returns: + disposable: Disposable clearCookies: title: Clear cookies @@ -1250,6 +1252,8 @@ BrowserContext: parameters: name: string needsHandle: boolean? + returns: + disposable: Disposable grantPermissions: title: Grant permissions @@ -1595,6 +1599,8 @@ Page: group: configuration parameters: source: string + returns: + disposable: Disposable close: title: Close page @@ -1662,6 +1668,8 @@ Page: parameters: name: string needsHandle: boolean? + returns: + disposable: Disposable goBack: title: Go back @@ -2959,6 +2967,15 @@ Worker: close: +Disposable: + type: interface + + commands: + + dispose: + internal: true + + JSHandle: type: interface diff --git a/tests/library/browsercontext-add-init-script.spec.ts b/tests/library/browsercontext-add-init-script.spec.ts index 53aafd2b0e7ab..c2fd6cf83840f 100644 --- a/tests/library/browsercontext-add-init-script.spec.ts +++ b/tests/library/browsercontext-add-init-script.spec.ts @@ -67,6 +67,25 @@ it('should work with browser context scripts for already created pages', async ( expect(await page.evaluate(() => (window as any)['result'])).toBe(123); }); +it('should remove context init script after dispose', async ({ context, server }) => { + const disposable = await context.addInitScript(() => (window as any)['temp'] = 123); + const page = await context.newPage(); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => (window as any)['temp'])).toBe(123); + + await disposable.dispose(); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => (window as any)['temp'])).toBe(undefined); +}); + +it('should remove context init script and keep working in new pages', async ({ context, server }) => { + const disposable = await context.addInitScript(() => (window as any)['temp'] = 123); + await disposable.dispose(); + const page = await context.newPage(); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => (window as any)['temp'])).toBe(undefined); +}); + it('init script should run only once in popup', async ({ context }) => { await context.addInitScript(() => { window['callCount'] = (window['callCount'] || 0) + 1; diff --git a/tests/library/browsercontext-expose-function.spec.ts b/tests/library/browsercontext-expose-function.spec.ts index c242c8582e06b..c485dce08bf46 100644 --- a/tests/library/browsercontext-expose-function.spec.ts +++ b/tests/library/browsercontext-expose-function.spec.ts @@ -45,6 +45,23 @@ it('should work', async ({ context, server }) => { expect(result).toEqual({ mul: 36, add: 13, sub: 5, addHandle: 11 }); }); +it('should dispose', async ({ context, server }) => { + const binding = await context.exposeFunction('compute', function(a, b) { + return a * b; + }); + const page = await context.newPage(); + const result = await page.evaluate(async function() { + return await window['compute'](9, 4); + }); + expect(result).toBe(36); + await binding.dispose(); + + const e = await page.evaluate(async function() { + return await window['compute'](9, 4); + }).catch(e => e); + expect(e.message).toContain('is not a function'); +}); + it('should throw for duplicate registrations', async ({ context, server }) => { await context.exposeFunction('foo', () => {}); await context.exposeFunction('bar', () => {}); diff --git a/tests/library/channels.spec.ts b/tests/library/channels.spec.ts index 17901000f7e6d..63741d7555f80 100644 --- a/tests/library/channels.spec.ts +++ b/tests/library/channels.spec.ts @@ -283,6 +283,10 @@ it('exposeFunction should not leak', async ({ page, expectScopeState, server }) { '_guid': 'page', 'objects': [ + { + '_guid': 'disposable', + 'objects': [], + }, { '_guid': 'frame', 'objects': [], diff --git a/tests/page/page-add-init-script.spec.ts b/tests/page/page-add-init-script.spec.ts index be4469e6ed56f..5e569f922f80f 100644 --- a/tests/page/page-add-init-script.spec.ts +++ b/tests/page/page-add-init-script.spec.ts @@ -84,6 +84,35 @@ it('should work after a cross origin navigation', async ({ page, server }) => { expect(await page.evaluate(() => window['result'])).toBe(123); }); +it('should remove init script after dispose', async ({ page, server }) => { + const disposable = await page.addInitScript(function() { + window['injected'] = 123; + }); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => window['result'])).toBe(123); + + await disposable.dispose(); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => window['result'])).toBe(undefined); +}); + +it('should remove one of multiple init scripts after dispose', async ({ page, server }) => { + const disposable1 = await page.addInitScript(function() { + window['script1'] = 1; + }); + await page.addInitScript(function() { + window['script2'] = 2; + }); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => window['script1'])).toBe(1); + expect(await page.evaluate(() => window['script2'])).toBe(2); + + await disposable1.dispose(); + await page.goto(server.PREFIX + '/tamperable.html'); + expect(await page.evaluate(() => window['script1'])).toBe(undefined); + expect(await page.evaluate(() => window['script2'])).toBe(2); +}); + it('init script should run only once in iframe', async ({ page, server, browserName, isBidi }) => { it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/26992' }); const messages = []; diff --git a/tests/page/page-expose-function.spec.ts b/tests/page/page-expose-function.spec.ts index 8bafdff2b3cef..8e92215ff3391 100644 --- a/tests/page/page-expose-function.spec.ts +++ b/tests/page/page-expose-function.spec.ts @@ -43,6 +43,22 @@ it('should work', async ({ page, server }) => { expect(result).toBe(36); }); +it('should dispose', async ({ page, server }) => { + const binding = await page.exposeFunction('compute', function(a, b) { + return a * b; + }); + const result = await page.evaluate(async function() { + return await window['compute'](9, 4); + }); + expect(result).toBe(36); + await binding.dispose(); + + const e = await page.evaluate(async function() { + return await window['compute'](9, 4); + }).catch(e => e); + expect(e.message).toContain('is not a function'); +}); + it('should work with handles and complex objects', async ({ page, server }) => { const fooHandle = await page.evaluateHandle(() => { window['fooValue'] = { bar: 2 }; diff --git a/utils/generate_types/overrides.d.ts b/utils/generate_types/overrides.d.ts index 07a961d7692bd..d34403fb5d731 100644 --- a/utils/generate_types/overrides.d.ts +++ b/utils/generate_types/overrides.d.ts @@ -44,7 +44,7 @@ export interface Page { evaluateHandle(pageFunction: PageFunction, arg: Arg): Promise>; evaluateHandle(pageFunction: PageFunction, arg?: any): Promise>; - addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; + addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; $(selector: K, options?: { strict: boolean }): Promise | null>; $(selector: string, options?: { strict: boolean }): Promise | null>; @@ -70,8 +70,8 @@ export interface Page { waitForSelector(selector: K, options: PageWaitForSelectorOptions): Promise | null>; waitForSelector(selector: string, options: PageWaitForSelectorOptions): Promise>; - exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; - exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; removeAllListeners(type?: string): this; removeAllListeners(type: string | undefined, options: { @@ -122,10 +122,10 @@ export interface Frame { } export interface BrowserContext { - exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; - exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; + exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; - addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; + addInitScript(script: PageFunction | { path?: string, content?: string }, arg?: Arg): Promise; removeAllListeners(type?: string): this; removeAllListeners(type: string | undefined, options: { From b87fa03fc60102c6d18e8d5a14dd8df51ddcd288 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 4 Mar 2026 15:20:14 -0800 Subject: [PATCH 3/3] chore: follow up to disposables (#39510) --- docs/src/api/class-browsercontext.md | 1 + docs/src/api/class-page.md | 1 + packages/playwright-client/types/types.d.ts | 4 +- packages/playwright-core/src/client/api.ts | 2 +- .../src/client/browserContext.ts | 15 ++++---- .../playwright-core/src/client/connection.ts | 4 +- .../playwright-core/src/client/disposable.ts | 38 ++++++++++++++++++- .../src/client/eventEmitter.ts | 7 +--- packages/playwright-core/src/client/page.ts | 11 +++--- .../src/mcp/browser/context.ts | 32 +++++++++------- .../playwright-core/src/mcp/browser/tab.ts | 6 +-- .../dispatchers/browserContextDispatcher.ts | 19 ++++------ .../dispatchers/disposableDispatcher.ts | 6 +-- .../src/server/dispatchers/pageDispatcher.ts | 19 ++++------ .../playwright-core/src/server/disposable.ts | 6 ++- packages/playwright-core/src/server/page.ts | 6 +-- packages/playwright-core/types/types.d.ts | 4 +- packages/playwright-ct-core/index.d.ts | 2 +- 18 files changed, 110 insertions(+), 73 deletions(-) diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index 2f345a46e53c7..24f07b5a67f34 100644 --- a/docs/src/api/class-browsercontext.md +++ b/docs/src/api/class-browsercontext.md @@ -1021,6 +1021,7 @@ API testing helper associated with this context. Requests made with this API wil ## async method: BrowserContext.route * since: v1.8 +- returns: <[Disposable]> Routing provides the capability to modify network requests that are made by any page in the browser context. Once route is enabled, every request matching the url pattern will stall unless it's continued, fulfilled or aborted. diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 5150aaa51e997..1b3093a52ebc8 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -3566,6 +3566,7 @@ API testing helper associated with this page. This method returns the same insta ## async method: Page.route * since: v1.8 +- returns: <[Disposable]> Routing provides the capability to modify network requests that are made by a page. diff --git a/packages/playwright-client/types/types.d.ts b/packages/playwright-client/types/types.d.ts index 1dc4b7db3653e..9227156952db2 100644 --- a/packages/playwright-client/types/types.d.ts +++ b/packages/playwright-client/types/types.d.ts @@ -4147,7 +4147,7 @@ export interface Page { * How often a route should be used. By default it will be used every time. */ times?: number; - }): Promise; + }): Promise; /** * If specified the network requests that are made in the page will be served from the HAR file. Read more about @@ -9469,7 +9469,7 @@ export interface BrowserContext { * How often a route should be used. By default it will be used every time. */ times?: number; - }): Promise; + }): Promise; /** * If specified the network requests that are made in the context will be served from the HAR file. Read more about diff --git a/packages/playwright-core/src/client/api.ts b/packages/playwright-core/src/client/api.ts index d84904d0d1725..86b4eb2be37a0 100644 --- a/packages/playwright-core/src/client/api.ts +++ b/packages/playwright-core/src/client/api.ts @@ -23,7 +23,7 @@ export { Clock } from './clock'; export { ConsoleMessage } from './consoleMessage'; export { Coverage } from './coverage'; export { Dialog } from './dialog'; -export { Disposable } from './disposable'; +export type { Disposable } from './disposable'; export { Download } from './download'; export { Electron, ElectronApplication } from './electron'; export { FrameLocator, Locator } from './locator'; diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 2dab10473af31..45d137c9c9a8d 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -23,7 +23,7 @@ import { evaluationScript } from './clientHelper'; import { Clock } from './clock'; import { ConsoleMessage } from './consoleMessage'; import { Dialog } from './dialog'; -import { Disposable } from './disposable'; +import { DisposableObject, DisposableStub } from './disposable'; import { TargetClosedError, parseError } from './errors'; import { Events } from './events'; import { APIRequestContext } from './fetch'; @@ -352,25 +352,26 @@ export class BrowserContext extends ChannelOwner async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) { const source = await evaluationScript(this._platform, script, arg); - return Disposable.from((await this._channel.addInitScript({ source })).disposable); + return DisposableObject.from((await this._channel.addInitScript({ source })).disposable); } - async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}): Promise { + async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}): Promise { const result = await this._channel.exposeBinding({ name, needsHandle: options.handle }); this._bindings.set(name, callback); - return Disposable.from(result.disposable); + return DisposableObject.from(result.disposable); } - async exposeFunction(name: string, callback: Function): Promise { + async exposeFunction(name: string, callback: Function): Promise { const result = await this._channel.exposeBinding({ name }); const binding = (source: structs.BindingSource, ...args: any[]) => callback(...args); this._bindings.set(name, binding); - return Disposable.from(result.disposable); + return DisposableObject.from(result.disposable); } - async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise { + async route(url: URLMatch, handler: network.RouteHandlerCallback, options: { times?: number } = {}): Promise { this._routes.unshift(new network.RouteHandler(this._platform, this._options.baseURL, url, handler, options.times)); await this._updateInterceptionPatterns({ title: 'Route requests' }); + return new DisposableStub(() => this.unroute(url, handler)); } async routeWebSocket(url: URLMatch, handler: network.WebSocketRouteHandlerCallback): Promise { diff --git a/packages/playwright-core/src/client/connection.ts b/packages/playwright-core/src/client/connection.ts index 6b794e68a7ed9..bc48d781595e2 100644 --- a/packages/playwright-core/src/client/connection.ts +++ b/packages/playwright-core/src/client/connection.ts @@ -24,7 +24,7 @@ import { CDPSession } from './cdpSession'; import { ChannelOwner } from './channelOwner'; import { createInstrumentation } from './clientInstrumentation'; import { Dialog } from './dialog'; -import { Disposable } from './disposable'; +import { DisposableObject } from './disposable'; import { Electron, ElectronApplication } from './electron'; import { ElementHandle } from './elementHandle'; import { TargetClosedError, parseError } from './errors'; @@ -272,7 +272,7 @@ export class Connection extends EventEmitter { result = new Dialog(parent, type, guid, initializer); break; case 'Disposable': - result = new Disposable(parent, type, guid, initializer); + result = new DisposableObject(parent, type, guid, initializer); break; case 'Electron': result = new Electron(parent, type, guid, initializer); diff --git a/packages/playwright-core/src/client/disposable.ts b/packages/playwright-core/src/client/disposable.ts index fe32640e1665d..1c6856834f9eb 100644 --- a/packages/playwright-core/src/client/disposable.ts +++ b/packages/playwright-core/src/client/disposable.ts @@ -19,8 +19,12 @@ import { isTargetClosedError } from './errors'; import type * as channels from '@protocol/channels'; -export class Disposable extends ChannelOwner { - static from(channel: channels.DisposableChannel): Disposable { +export interface Disposable { + dispose: () => Promise; +} + +export class DisposableObject extends ChannelOwner implements Disposable { + static from(channel: channels.DisposableChannel): DisposableObject { return (channel as any)._object; } @@ -38,3 +42,33 @@ export class Disposable Promise) | undefined; + + constructor(dispose: () => Promise) { + this._dispose = dispose; + } + + async [Symbol.asyncDispose]() { + await this.dispose(); + } + + async dispose() { + if (!this._dispose) + return; + try { + const dispose = this._dispose; + this._dispose = undefined; + await dispose(); + } catch (e) { + if (isTargetClosedError(e)) + return; + throw e; + } + } +} + +export function disposeAll(disposables: Disposable[]) { + return Promise.all(disposables.map(d => d.dispose())); +} diff --git a/packages/playwright-core/src/client/eventEmitter.ts b/packages/playwright-core/src/client/eventEmitter.ts index cf2dc55f88107..49c38c88aee05 100644 --- a/packages/playwright-core/src/client/eventEmitter.ts +++ b/packages/playwright-core/src/client/eventEmitter.ts @@ -24,6 +24,7 @@ import type { EventEmitter as EventEmitterType } from 'events'; import type { Platform } from './platform'; +import type { Disposable } from './disposable'; type EventType = string | symbol; type Listener = (...args: any[]) => any; @@ -397,10 +398,6 @@ function wrappedListener(l: Listener): Listener { return (l as any).listener; } -export type Disposable = { - dispose: () => void; -}; - class EventsHelper { static addEventListener( emitter: EventEmitterType, @@ -408,7 +405,7 @@ class EventsHelper { handler: (...args: any[]) => void): Disposable { emitter.on(eventName, handler); return { - dispose: () => emitter.removeListener(eventName, handler) + dispose: async () => { emitter.removeListener(eventName, handler); } }; } } diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index 46dededd8a92b..ffc58e30b74d2 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -19,7 +19,7 @@ import { Artifact } from './artifact'; import { ChannelOwner } from './channelOwner'; import { evaluationScript } from './clientHelper'; import { Coverage } from './coverage'; -import { Disposable } from './disposable'; +import { DisposableObject, DisposableStub } from './disposable'; import { Download } from './download'; import { ElementHandle, determineScreenshotType } from './elementHandle'; import { TargetClosedError, isTargetClosedError, parseError, serializeError } from './errors'; @@ -336,13 +336,13 @@ export class Page extends ChannelOwner implements api.Page const result = await this._channel.exposeBinding({ name }); const binding = (source: structs.BindingSource, ...args: any[]) => callback(...args); this._bindings.set(name, binding); - return Disposable.from(result.disposable); + return DisposableObject.from(result.disposable); } async exposeBinding(name: string, callback: (source: structs.BindingSource, ...args: any[]) => any, options: { handle?: boolean } = {}) { const result = await this._channel.exposeBinding({ name, needsHandle: options.handle }); this._bindings.set(name, callback); - return Disposable.from(result.disposable); + return DisposableObject.from(result.disposable); } async setExtraHTTPHeaders(headers: Headers) { @@ -510,12 +510,13 @@ export class Page extends ChannelOwner implements api.Page async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) { const source = await evaluationScript(this._platform, script, arg); - return Disposable.from((await this._channel.addInitScript({ source })).disposable); + return DisposableObject.from((await this._channel.addInitScript({ source })).disposable); } - async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { + async route(url: URLMatch, handler: RouteHandlerCallback, options: { times?: number } = {}): Promise { this._routes.unshift(new RouteHandler(this._platform, this._browserContext._options.baseURL, url, handler, options.times)); await this._updateInterceptionPatterns({ title: 'Route requests' }); + return new DisposableStub(() => this.unroute(url, handler)); } async routeFromHAR(har: string, options: { url?: string | RegExp, notFound?: 'abort' | 'fallback', update?: boolean, updateContent?: 'attach' | 'embed', updateMode?: 'minimal' | 'full'} = {}): Promise { diff --git a/packages/playwright-core/src/mcp/browser/context.ts b/packages/playwright-core/src/mcp/browser/context.ts index 2490841d2ea43..c439cc3a3673f 100644 --- a/packages/playwright-core/src/mcp/browser/context.ts +++ b/packages/playwright-core/src/mcp/browser/context.ts @@ -16,6 +16,7 @@ import path from 'path'; +import { disposeAll } from '../../client/disposable'; import { eventsHelper } from '../../client/eventEmitter'; import { debug } from '../../utilsBundle'; import { escapeWithQuotes } from '../../utils/isomorphic/stringUtils'; @@ -29,7 +30,7 @@ import type * as playwright from '../../..'; import type { FullConfig } from './config'; import type { SessionLog } from './sessionLog'; import type { Tracing } from '../../client/tracing'; -import type { Disposable } from '../../client/eventEmitter'; +import type { Disposable } from '../../client/disposable'; import type { BrowserContext } from '../../client/browserContext'; import type { ClientInfo } from '../sdk/server'; @@ -88,14 +89,12 @@ export class Context { } async dispose() { - for (const disposable of this._disposables) - disposable.dispose(); - this._disposables = []; + disposeAll(this._disposables); for (const tab of this._tabs) - tab.dispose(); + await tab.dispose(); this._tabs.length = 0; this._currentTab = undefined; - this.stopVideoRecording(); + await this.stopVideoRecording(); } tabs(): Tab[] { @@ -235,15 +234,17 @@ export class Context { private async _setupRequestInterception(context: playwright.BrowserContext) { if (this.config.network?.allowedOrigins?.length) { - await context.route('**', route => route.abort('blockedbyclient')); + this._disposables.push(await context.route('**', route => route.abort('blockedbyclient'))); - for (const origin of this.config.network.allowedOrigins) - await context.route(originOrHostGlob(origin), route => route.continue()); + for (const origin of this.config.network.allowedOrigins) { + const glob = originOrHostGlob(origin); + this._disposables.push(await context.route(glob, route => route.continue())); + } } if (this.config.network?.blockedOrigins?.length) { for (const origin of this.config.network.blockedOrigins) - await context.route(originOrHostGlob(origin), route => route.abort('blockedbyclient')); + this._disposables.push(await context.route(originOrHostGlob(origin), route => route.abort('blockedbyclient'))); } } @@ -263,9 +264,7 @@ export class Context { (browserContext as any)._setAllowedDirectories(allRootPaths(this._clientInfo)); } await this._setupRequestInterception(browserContext); - for (const page of browserContext.pages()) - this._onPageCreated(page); - this._disposables.push(eventsHelper.addEventListener(browserContext as BrowserContext, 'page', page => this._onPageCreated(page))); + if (this.config.saveTrace) { await (browserContext.tracing as Tracing).start({ name: 'trace-' + Date.now(), @@ -281,7 +280,12 @@ export class Context { } const rootPath = firstRootPath(this._clientInfo); for (const initScript of this.config.browser.initScript || []) - await browserContext.addInitScript({ path: path.resolve(rootPath, initScript) }); + this._disposables.push(await browserContext.addInitScript({ path: path.resolve(rootPath, initScript) })); + + for (const page of browserContext.pages()) + this._onPageCreated(page); + this._disposables.push(eventsHelper.addEventListener(browserContext as BrowserContext, 'page', page => this._onPageCreated(page))); + return browserContext; } diff --git a/packages/playwright-core/src/mcp/browser/tab.ts b/packages/playwright-core/src/mcp/browser/tab.ts index 6f474ceb78a56..7e31209505196 100644 --- a/packages/playwright-core/src/mcp/browser/tab.ts +++ b/packages/playwright-core/src/mcp/browser/tab.ts @@ -28,7 +28,7 @@ import { LogFile } from './logFile'; import { ModalState } from './tools/tool'; import { handleDialog } from './tools/dialogs'; import { uploadFile } from './tools/files'; -import type { Disposable } from '../../client/eventEmitter'; +import type { Disposable } from '../../client/disposable'; import type { Context } from './context'; import type { Page } from '../../client/page'; import type { Locator } from '../../client/locator'; @@ -142,9 +142,9 @@ export class Tab extends EventEmitter { this.expectTimeoutOptions = { timeout: context.config.timeouts.expect }; } - dispose() { + async dispose() { for (const disposable of this._disposables) - disposable.dispose(); + await disposable.dispose(); this._disposables = []; this._consoleLog.stop(); } diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index 82756d1fa814d..511b529e870bc 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -41,7 +41,8 @@ import { JSHandleDispatcher } from './jsHandleDispatcher'; import type { ConsoleMessage } from '../console'; import type { Dialog } from '../dialog'; import type { Request, Response, RouteHandler } from '../network'; -import type { InitScript, Page, PageBinding } from '../page'; +import type { InitScript, Page } from '../page'; +import type { Disposable } from '../disposable'; import type { DispatcherScope } from './dispatcher'; import type * as channels from '@protocol/channels'; import type { Progress } from '@protocol/progress'; @@ -53,8 +54,7 @@ export class BrowserContextDispatcher extends Dispatcher(); _webSocketInterceptionPatterns: channels.BrowserContextSetWebSocketInterceptionPatternsParams['patterns'] = []; - private _bindings: PageBinding[] = []; - private _initScripts: InitScript[] = []; + private _disposables: Disposable[] = []; private _dialogHandler: (dialog: Dialog) => boolean; private _clockPaused = false; private _requestInterceptor: RouteHandler; @@ -239,7 +239,7 @@ export class BrowserContextDispatcher extends Dispatcher { const initScript = await this._context.addInitScript(params.source); - this._initScripts.push(initScript); + this._disposables.push(initScript); return { disposable: new DisposableDispatcher(this, initScript) }; } @@ -433,12 +433,9 @@ export class BrowserContextDispatcher extends Dispatcher {}); - for (const binding of this._bindings) - binding.dispose().catch(() => {}); - this._bindings = []; - for (const initScript of this._initScripts) - initScript.dispose().catch(() => {}); - this._initScripts = []; + for (const disposable of this._disposables) + disposable.dispose().catch(() => {}); + this._disposables = []; if (this._routeWebSocketInitScript) WebSocketRouteDispatcher.uninstall(this.connection, this._context, this._routeWebSocketInitScript).catch(() => {}); this._routeWebSocketInitScript = undefined; diff --git a/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts b/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts index a262e0684e76d..344f8e41a0f63 100644 --- a/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/disposableDispatcher.ts @@ -16,15 +16,15 @@ import { Dispatcher } from './dispatcher'; -import type { Disposable } from '../disposable'; +import type { DisposableObject } from '../disposable'; import type { DispatcherScope } from './dispatcher'; import type * as channels from '@protocol/channels'; import type { Progress } from '@protocol/progress'; -export class DisposableDispatcher extends Dispatcher implements channels.DisposableChannel { +export class DisposableDispatcher extends Dispatcher implements channels.DisposableChannel { _type_Disposable = true; - constructor(scope: DispatcherScope, disposable: Disposable) { + constructor(scope: DispatcherScope, disposable: DisposableObject) { super(scope, disposable, 'Disposable', {}); } diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts index cd7a40962d0bd..df6fb0e7540c5 100644 --- a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts @@ -41,7 +41,8 @@ import type { JSHandle } from '../javascript'; import type { BrowserContextDispatcher } from './browserContextDispatcher'; import type { Frame } from '../frames'; import type { RouteHandler } from '../network'; -import type { InitScript, PageBinding } from '../page'; +import type { InitScript } from '../page'; +import type { Disposable } from '../disposable'; import type * as channels from '@protocol/channels'; import type { Progress } from '@protocol/progress'; import type { URLMatch } from '../../utils/isomorphic/urlMatch'; @@ -53,8 +54,7 @@ export class PageDispatcher extends Dispatcher(); _webSocketInterceptionPatterns: channels.PageSetWebSocketInterceptionPatternsParams['patterns'] = []; - private _bindings: PageBinding[] = []; - private _initScripts: InitScript[] = []; + private _disposables: Disposable[] = []; private _requestInterceptor: RouteHandler; private _interceptionUrlMatchers: URLMatch[] = []; private _routeWebSocketInitScript: InitScript | undefined; @@ -140,7 +140,7 @@ export class PageDispatcher extends Dispatcher { const initScript = await this._page.addInitScript(params.source); - this._initScripts.push(initScript); + this._disposables.push(initScript); return { disposable: new DisposableDispatcher(this, initScript) }; } @@ -425,12 +425,9 @@ export class PageDispatcher extends Dispatcher {}); - for (const binding of this._bindings) - this._page.removeExposedBinding(binding).catch(() => {}); - this._bindings = []; - for (const initScript of this._initScripts) - initScript.dispose().catch(() => {}); - this._initScripts = []; + for (const disposable of this._disposables) + disposable.dispose().catch(() => {}); + this._disposables = []; if (this._routeWebSocketInitScript) WebSocketRouteDispatcher.uninstall(this.connection, this._page, this._routeWebSocketInitScript).catch(() => {}); this._routeWebSocketInitScript = undefined; diff --git a/packages/playwright-core/src/server/disposable.ts b/packages/playwright-core/src/server/disposable.ts index 7d7c708f9a6af..dd3a04e36a2a6 100644 --- a/packages/playwright-core/src/server/disposable.ts +++ b/packages/playwright-core/src/server/disposable.ts @@ -19,7 +19,11 @@ import { SdkObject } from './instrumentation'; import type { Page } from './page'; import type { BrowserContext } from './browserContext'; -export abstract class Disposable extends SdkObject { +export interface Disposable { + dispose(): Promise; +} + +export abstract class DisposableObject extends SdkObject implements Disposable { readonly parent: Page | BrowserContext; constructor(parent: Page | BrowserContext) { diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index e756c6bebcf29..51a91df429bee 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -16,7 +16,7 @@ */ import { BrowserContext } from './browserContext'; -import { Disposable } from './disposable'; +import { DisposableObject } from './disposable'; import { ConsoleMessage } from './console'; import { TargetClosedError, TimeoutError } from './errors'; import { FileChooser } from './fileChooser'; @@ -933,7 +933,7 @@ export class Worker extends SdkObject { } } -export class PageBinding extends Disposable { +export class PageBinding extends DisposableObject { private static kController = '__playwright__binding__controller__'; static kBindingName = '__playwright__binding__'; @@ -993,7 +993,7 @@ export class PageBinding extends Disposable { } } -export class InitScript extends Disposable { +export class InitScript extends DisposableObject { readonly source: string; constructor(owner: BrowserContext | Page, source: string) { diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 1dc4b7db3653e..9227156952db2 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -4147,7 +4147,7 @@ export interface Page { * How often a route should be used. By default it will be used every time. */ times?: number; - }): Promise; + }): Promise; /** * If specified the network requests that are made in the page will be served from the HAR file. Read more about @@ -9469,7 +9469,7 @@ export interface BrowserContext { * How often a route should be used. By default it will be used every time. */ times?: number; - }): Promise; + }): Promise; /** * If specified the network requests that are made in the context will be served from the HAR file. Read more about diff --git a/packages/playwright-ct-core/index.d.ts b/packages/playwright-ct-core/index.d.ts index f6710ee1f6fe6..a7a3320af0e97 100644 --- a/packages/playwright-ct-core/index.d.ts +++ b/packages/playwright-ct-core/index.d.ts @@ -39,7 +39,7 @@ interface RequestHandler { } export interface RouterFixture { - route(...args: Parameters): Promise; + route(...args: Parameters): Promise; use(...handlers: RequestHandler[]): Promise; }