mirror of
https://github.com/microsoft/playwright.git
synced 2025-06-26 21:40:17 +00:00
fix(reuse): make sure all dispose and close sequences are executed (#19572)
- When disposing recursively, only the root dispatcher received `_dispose()` call, while some dispatchers need `_onDispose()` to clean things up. - When reusing the context, pages should be notified with `_onClose()` so that all client-side waiting promises could reject. Fixes #19216.
This commit is contained in:
parent
600d6bc635
commit
412c11db20
@ -65,6 +65,8 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
|
||||
async _newContextForReuse(options: BrowserContextOptions = {}): Promise<BrowserContext> {
|
||||
for (const context of this._contexts) {
|
||||
await this._browserType._onWillCloseContext?.(context);
|
||||
for (const page of context.pages())
|
||||
page._onClose();
|
||||
context._onClose();
|
||||
}
|
||||
this._contexts.clear();
|
||||
|
||||
@ -270,8 +270,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
|
||||
this._subscriptions.delete(params.event);
|
||||
}
|
||||
|
||||
override _dispose() {
|
||||
super._dispose();
|
||||
override _onDispose() {
|
||||
// Avoid protocol calls for the closed context.
|
||||
if (!this._context.isClosingOrClosed())
|
||||
this._context.setRequestInterceptor(undefined).catch(() => {});
|
||||
|
||||
@ -79,8 +79,7 @@ export class DebugControllerDispatcher extends Dispatcher<DebugController, chann
|
||||
await this._object.closeAllBrowsers();
|
||||
}
|
||||
|
||||
override _dispose() {
|
||||
super._dispose();
|
||||
override _onDispose() {
|
||||
this._object.dispose();
|
||||
}
|
||||
}
|
||||
|
||||
@ -102,8 +102,12 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
|
||||
this._connection.sendDispose(this);
|
||||
}
|
||||
|
||||
protected _onDispose() {
|
||||
}
|
||||
|
||||
private _disposeRecursively() {
|
||||
assert(!this._disposed, `${this._guid} is disposed more than once`);
|
||||
this._onDispose();
|
||||
this._disposed = true;
|
||||
eventsHelper.removeEventListeners(this._eventListeners);
|
||||
|
||||
|
||||
@ -299,9 +299,10 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
|
||||
this._dispatchEvent('frameDetached', { frame: FrameDispatcher.from(this, frame) });
|
||||
}
|
||||
|
||||
override _dispose() {
|
||||
super._dispose();
|
||||
this._page.setClientRequestInterceptor(undefined).catch(() => {});
|
||||
override _onDispose() {
|
||||
// Avoid protocol calls for the closed page.
|
||||
if (!this._page.isClosedOrClosingOrCrashed())
|
||||
this._page.setClientRequestInterceptor(undefined).catch(() => {});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -612,6 +612,10 @@ export class Page extends SdkObject {
|
||||
return this._closedState === 'closed';
|
||||
}
|
||||
|
||||
isClosedOrClosingOrCrashed() {
|
||||
return this._closedState !== 'open' || this._crashedPromise.isDone();
|
||||
}
|
||||
|
||||
_addWorker(workerId: string, worker: Worker) {
|
||||
this._workers.set(workerId, worker);
|
||||
this.emit(Page.Events.Worker, worker);
|
||||
|
||||
@ -20,10 +20,12 @@ import { createGuid } from '../../packages/playwright-core/lib/utils';
|
||||
import { Backend } from '../config/debugControllerBackend';
|
||||
import type { Browser, BrowserContext } from '@playwright/test';
|
||||
|
||||
type BrowserWithReuse = Browser & { _newContextForReuse: () => Promise<BrowserContext> };
|
||||
type Fixtures = {
|
||||
wsEndpoint: string;
|
||||
backend: Backend;
|
||||
connectedBrowser: Browser & { _newContextForReuse: () => Promise<BrowserContext> };
|
||||
connectedBrowserFactory: () => Promise<BrowserWithReuse>;
|
||||
connectedBrowser: BrowserWithReuse;
|
||||
};
|
||||
|
||||
const test = baseTest.extend<Fixtures>({
|
||||
@ -41,16 +43,24 @@ const test = baseTest.extend<Fixtures>({
|
||||
await use(backend);
|
||||
await backend.close();
|
||||
},
|
||||
connectedBrowser: async ({ wsEndpoint, browserType }, use) => {
|
||||
const oldValue = (browserType as any)._defaultConnectOptions;
|
||||
(browserType as any)._defaultConnectOptions = {
|
||||
wsEndpoint,
|
||||
headers: { 'x-playwright-reuse-context': '1', },
|
||||
};
|
||||
const browser = await browserType.launch();
|
||||
(browserType as any)._defaultConnectOptions = oldValue;
|
||||
await use(browser as any);
|
||||
await browser.close();
|
||||
connectedBrowserFactory: async ({ wsEndpoint, browserType }, use) => {
|
||||
const browsers: BrowserWithReuse [] = [];
|
||||
await use(async () => {
|
||||
const oldValue = (browserType as any)._defaultConnectOptions;
|
||||
(browserType as any)._defaultConnectOptions = {
|
||||
wsEndpoint,
|
||||
headers: { 'x-playwright-reuse-context': '1', },
|
||||
};
|
||||
const browser = await browserType.launch() as BrowserWithReuse;
|
||||
(browserType as any)._defaultConnectOptions = oldValue;
|
||||
browsers.push(browser);
|
||||
return browser;
|
||||
});
|
||||
for (const browser of browsers)
|
||||
await browser.close();
|
||||
},
|
||||
connectedBrowser: async ({ connectedBrowserFactory }, use) => {
|
||||
await use(await connectedBrowserFactory());
|
||||
},
|
||||
});
|
||||
|
||||
@ -231,3 +241,27 @@ test('should pause and resume', async ({ backend, connectedBrowser }) => {
|
||||
await backend.resume();
|
||||
await pausePromise;
|
||||
});
|
||||
|
||||
test('should reset routes before reuse', async ({ server, connectedBrowserFactory }) => {
|
||||
const browser1 = await connectedBrowserFactory();
|
||||
const context1 = await browser1._newContextForReuse();
|
||||
await context1.route(server.PREFIX + '/title.html', route => route.fulfill({ body: '<title>Hello</title>', contentType: 'text/html' }));
|
||||
const page1 = await context1.newPage();
|
||||
await page1.route(server.PREFIX + '/consolelog.html', route => route.fulfill({ body: '<title>World</title>', contentType: 'text/html' }));
|
||||
|
||||
await page1.goto(server.PREFIX + '/title.html');
|
||||
await expect(page1).toHaveTitle('Hello');
|
||||
await page1.goto(server.PREFIX + '/consolelog.html');
|
||||
await expect(page1).toHaveTitle('World');
|
||||
await browser1.close();
|
||||
|
||||
const browser2 = await connectedBrowserFactory();
|
||||
const context2 = await browser2._newContextForReuse();
|
||||
const page2 = await context2.newPage();
|
||||
|
||||
await page2.goto(server.PREFIX + '/title.html');
|
||||
await expect(page2).toHaveTitle('Woof-Woof');
|
||||
await page2.goto(server.PREFIX + '/consolelog.html');
|
||||
await expect(page2).toHaveTitle('console.log test');
|
||||
await browser2.close();
|
||||
});
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user