diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index a5a3090468..8ba6906722 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -193,7 +193,7 @@ export abstract class BrowserContext extends SdkObject { const [, ...otherPages] = this.pages(); for (const p of otherPages) await p.close(metadata); - if (page && page._crashedScope.isClosed()) { + if (page && page.hasCrashed()) { await page.close(metadata); page = undefined; } diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index b9e7e95804..8a5a2da95b 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -91,7 +91,7 @@ export class CRBrowser extends Browser { super(parent, options); this._connection = connection; this._session = this._connection.rootSession; - this._connection.on(ConnectionEvents.Disconnected, () => this._didClose()); + this._connection.on(ConnectionEvents.Disconnected, () => this._didDisconnect()); this._session.on('Target.attachedToTarget', this._onAttachedToTarget.bind(this)); this._session.on('Target.detachedFromTarget', this._onDetachedFromTarget.bind(this)); this._session.on('Browser.downloadWillBegin', this._onDownloadWillBegin.bind(this)); @@ -149,7 +149,7 @@ export class CRBrowser extends Browser { _onAttachedToTarget({ targetInfo, sessionId, waitingForDebugger }: Protocol.Target.attachedToTargetPayload) { if (targetInfo.type === 'browser') return; - const session = this._connection.session(sessionId)!; + const session = this._session.createChildSession(sessionId); assert(targetInfo.browserContextId, 'targetInfo: ' + JSON.stringify(targetInfo, null, 2)); let context = this._contexts.get(targetInfo.browserContextId) || null; if (!context) { @@ -234,6 +234,19 @@ export class CRBrowser extends Browser { } } + private _didDisconnect() { + for (const crPage of this._crPages.values()) + crPage.didClose(); + this._crPages.clear(); + for (const backgroundPage of this._backgroundPages.values()) + backgroundPage.didClose(); + this._backgroundPages.clear(); + for (const serviceWorker of this._serviceWorkers.values()) + serviceWorker.didClose(); + this._serviceWorkers.clear(); + this._didClose(); + } + private _findOwningPage(frameId: string) { for (const crPage of this._crPages.values()) { const frame = crPage._page._frameManager.frame(frameId); @@ -593,6 +606,6 @@ export class CRBrowserContext extends BrowserContext { const rootSession = await this._browser._clientRootSession(); const { sessionId } = await rootSession.send('Target.attachToTarget', { targetId, flatten: true }); - return this._browser._connection.session(sessionId)!; + return rootSession.createChildSession(sessionId); } } diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index 475010c960..7c579feafe 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -37,9 +37,10 @@ export const kBrowserCloseMessageId = -9999; export class CRConnection extends EventEmitter { private _lastId = 0; private readonly _transport: ConnectionTransport; - private readonly _sessions = new Map(); + readonly _sessions = new Map(); private readonly _protocolLogger: ProtocolLogger; private readonly _browserLogsCollector: RecentLogsCollector; + _browserDisconnectedLogs: string | undefined; readonly rootSession: CRSession; _closed = false; @@ -49,21 +50,13 @@ export class CRConnection extends EventEmitter { this._transport = transport; this._protocolLogger = protocolLogger; this._browserLogsCollector = browserLogsCollector; - this.rootSession = new CRSession(this, '', 'browser', ''); + this.rootSession = new CRSession(this, null, ''); this._sessions.set('', this.rootSession); this._transport.onmessage = this._onMessage.bind(this); // onclose should be set last, since it can be immediately called. this._transport.onclose = this._onClose.bind(this); } - static fromSession(session: CRSession): CRConnection { - return session._connection!; - } - - session(sessionId: string): CRSession | null { - return this._sessions.get(sessionId) || null; - } - _rawSend(sessionId: string, method: string, params: any): number { const id = ++this._lastId; const message: ProtocolRequest = { id, method, params }; @@ -78,18 +71,6 @@ export class CRConnection extends EventEmitter { this._protocolLogger('receive', message); if (message.id === kBrowserCloseMessageId) return; - if (message.method === 'Target.attachedToTarget') { - const sessionId = message.params.sessionId; - const rootSessionId = message.sessionId || ''; - const session = new CRSession(this, rootSessionId, message.params.targetInfo.type, sessionId); - this._sessions.set(sessionId, session); - } else if (message.method === 'Target.detachedFromTarget') { - const session = this._sessions.get(message.params.sessionId); - if (session) { - session._onClosed(undefined); - this._sessions.delete(message.params.sessionId); - } - } const session = this._sessions.get(message.sessionId || ''); if (session) session._onMessage(message); @@ -99,10 +80,8 @@ export class CRConnection extends EventEmitter { this._closed = true; this._transport.onmessage = undefined; this._transport.onclose = undefined; - const browserDisconnectedLogs = helper.formatBrowserLogs(this._browserLogsCollector.recentLogs()); - for (const session of this._sessions.values()) - session._onClosed(browserDisconnectedLogs); - this._sessions.clear(); + this._browserDisconnectedLogs = helper.formatBrowserLogs(this._browserLogsCollector.recentLogs()); + this.rootSession.dispose(); Promise.resolve().then(() => this.emit(ConnectionEvents.Disconnected)); } @@ -111,14 +90,9 @@ export class CRConnection extends EventEmitter { this._transport.close(); } - async createSession(targetInfo: Protocol.Target.TargetInfo): Promise { - const { sessionId } = await this.rootSession.send('Target.attachToTarget', { targetId: targetInfo.targetId, flatten: true }); - return this._sessions.get(sessionId)!; - } - async createBrowserSession(): Promise { const { sessionId } = await this.rootSession.send('Target.attachToBrowserTarget'); - return this._sessions.get(sessionId)!; + return this.rootSession.createChildSession(sessionId); } } @@ -127,14 +101,13 @@ export const CRSessionEvents = { }; export class CRSession extends EventEmitter { - _connection: CRConnection | null; + private readonly _connection: CRConnection; _eventListener?: (method: string, params?: Object) => void; private readonly _callbacks = new Map void, reject: (e: ProtocolError) => void, error: ProtocolError, method: string}>(); - private readonly _targetType: string; private readonly _sessionId: string; - private readonly _rootSessionId: string; + private readonly _parentSession: CRSession | null; private _crashed: boolean = false; - private _browserDisconnectedLogs: string | undefined; + private _closed = false; override on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; override addListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; override off: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -142,13 +115,12 @@ export class CRSession extends EventEmitter { override once: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; readonly guid: string; - constructor(connection: CRConnection, rootSessionId: string, targetType: string, sessionId: string) { + constructor(connection: CRConnection, parentSession: CRSession | null, sessionId: string) { super(); this.guid = `cdp-session@${sessionId}`; this.setMaxListeners(0); this._connection = connection; - this._rootSessionId = rootSessionId; - this._targetType = targetType; + this._parentSession = parentSession; this._sessionId = sessionId; this.on = super.on; @@ -162,16 +134,28 @@ export class CRSession extends EventEmitter { this._crashed = true; } + createChildSession(sessionId: string): CRSession { + const session = new CRSession(this._connection, this, sessionId); + this._connection._sessions.set(sessionId, session); + return session; + } + + private _closedErrorMessage() { + if (this._crashed) + return 'Target crashed'; + if (this._connection._browserDisconnectedLogs !== undefined) + return `Browser closed.` + this._connection._browserDisconnectedLogs; + if (this._closed) + return `Target closed`; + } + async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { - if (this._crashed) - throw new ProtocolError(true, 'Target crashed'); - if (this._browserDisconnectedLogs !== undefined) - throw new ProtocolError(true, `Browser closed.` + this._browserDisconnectedLogs); - if (!this._connection) - throw new ProtocolError(true, `Target closed`); + const closedErrorMessage = this._closedErrorMessage(); + if (closedErrorMessage) + throw new ProtocolError(true, closedErrorMessage); const id = this._connection._rawSend(this._sessionId, method, params); return new Promise((resolve, reject) => { this._callbacks.set(id, { resolve, reject, error: new ProtocolError(false), method }); @@ -203,23 +187,23 @@ export class CRSession extends EventEmitter { } async detach() { - if (!this._connection) - throw new Error(`Session already detached. Most likely the ${this._targetType} has been closed.`); - const rootSession = this._connection.session(this._rootSessionId); - if (!rootSession) - throw new Error('Root session has been closed'); - await rootSession.send('Target.detachFromTarget', { sessionId: this._sessionId }); + if (this._closed) + throw new Error(`Session already detached. Most likely the page has been closed.`); + if (!this._parentSession) + throw new Error('Root session cannot be closed'); + await this._parentSession.send('Target.detachFromTarget', { sessionId: this._sessionId }); + this.dispose(); } - _onClosed(browserDisconnectedLogs: string | undefined) { - this._browserDisconnectedLogs = browserDisconnectedLogs; - const errorMessage = browserDisconnectedLogs !== undefined ? 'Browser closed.' + browserDisconnectedLogs : 'Target closed'; + dispose() { + this._closed = true; + this._connection._sessions.delete(this._sessionId); + const errorMessage = this._closedErrorMessage()!; for (const callback of this._callbacks.values()) { callback.error.sessionClosed = true; callback.reject(rewriteErrorMessage(callback.error, errorMessage)); } this._callbacks.clear(); - this._connection = null; Promise.resolve().then(() => this.emit(CRSessionEvents.Disconnected)); } } diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 54f5e4459d..1566b722e7 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -34,7 +34,6 @@ import type * as channels from '@protocol/channels'; import { getAccessibilityTree } from './crAccessibility'; import { CRBrowserContext } from './crBrowser'; import type { CRSession } from './crConnection'; -import { CRConnection, CRSessionEvents } from './crConnection'; import { CRCoverage } from './crCoverage'; import { DragManager } from './crDragDrop'; import { CRExecutionContext } from './crExecutionContext'; @@ -93,7 +92,6 @@ export class CRPage implements PageDelegate { this._page = new Page(this, browserContext); this._mainFrameSession = new FrameSession(this, client, targetId, null); this._sessions.set(targetId, this._mainFrameSession); - client.once(CRSessionEvents.Disconnected, () => this._page._didDisconnect()); if (opener && !browserContext._options.noDefaultViewport) { const features = opener._nextWindowOpenPopupFeatures.shift() || []; const viewportSize = helper.getViewportSizeFromWindowFeatures(features); @@ -407,6 +405,7 @@ class FrameSession { private _evaluateOnNewDocumentIdentifiers: string[] = []; private _exposedBindingNames: string[] = []; private _metricsOverride: Protocol.Emulation.setDeviceMetricsOverrideParameters | undefined; + private _workerSessions = new Map(); constructor(crPage: CRPage, client: CRSession, targetId: string, parentSession: FrameSession | null) { this._client = client; @@ -421,9 +420,6 @@ class FrameSession { this._firstNonInitialNavigationCommittedFulfill = f; this._firstNonInitialNavigationCommittedReject = r; }); - client.once(CRSessionEvents.Disconnected, () => { - this._firstNonInitialNavigationCommittedReject(new Error('Page closed')); - }); } _isMainFrame(): boolean { @@ -578,6 +574,7 @@ class FrameSession { } dispose() { + this._firstNonInitialNavigationCommittedReject(new Error('Page closed')); for (const childSession of this._childSessions) childSession.dispose(); if (this._parentSession) @@ -585,6 +582,7 @@ class FrameSession { eventsHelper.removeEventListeners(this._eventListeners); this._networkManager.dispose(); this._crPage._sessions.delete(this._targetId); + this._client.dispose(); } async _navigate(frame: frames.Frame, url: string, referrer: string | undefined): Promise { @@ -719,7 +717,7 @@ class FrameSession { } _onAttachedToTarget(event: Protocol.Target.attachedToTargetPayload) { - const session = CRConnection.fromSession(this._client).session(event.sessionId)!; + const session = this._client.createChildSession(event.sessionId); if (event.targetInfo.type === 'iframe') { // Frame id equals target id. @@ -745,6 +743,7 @@ class FrameSession { const url = event.targetInfo.url; const worker = new Worker(this._page, url); this._page._addWorker(event.sessionId, worker); + this._workerSessions.set(event.sessionId, session); session.once('Runtime.executionContextCreated', async event => { worker._createExecutionContext(new CRExecutionContext(session, event.context)); }); @@ -763,7 +762,11 @@ class FrameSession { _onDetachedFromTarget(event: Protocol.Target.detachedFromTargetPayload) { // This might be a worker... - this._page._removeWorker(event.sessionId); + const workerSession = this._workerSessions.get(event.sessionId); + if (workerSession) { + workerSession.dispose(); + this._page._removeWorker(event.sessionId); + } // ... or an oopif. const childFrameSession = this._crPage._sessions.get(event.targetId!); diff --git a/packages/playwright-core/src/server/chromium/crServiceWorker.ts b/packages/playwright-core/src/server/chromium/crServiceWorker.ts index a9cddac9f9..b75f9922e9 100644 --- a/packages/playwright-core/src/server/chromium/crServiceWorker.ts +++ b/packages/playwright-core/src/server/chromium/crServiceWorker.ts @@ -55,6 +55,11 @@ export class CRServiceWorker extends Worker { }); } + override didClose() { + this._session.dispose(); + super.didClose(); + } + async updateOffline(initial: boolean): Promise { if (!this._isNetworkInspectionEnabled()) return; diff --git a/packages/playwright-core/src/server/firefox/ffBrowser.ts b/packages/playwright-core/src/server/firefox/ffBrowser.ts index 9db93752fe..3b13eda242 100644 --- a/packages/playwright-core/src/server/firefox/ffBrowser.ts +++ b/packages/playwright-core/src/server/firefox/ffBrowser.ts @@ -159,6 +159,9 @@ export class FFBrowser extends Browser { for (const video of this._idToVideo.values()) video.artifact.reportFinished(kBrowserClosedError); this._idToVideo.clear(); + for (const ffPage of this._ffPages.values()) + ffPage.didClose(); + this._ffPages.clear(); this._didClose(); } } diff --git a/packages/playwright-core/src/server/firefox/ffConnection.ts b/packages/playwright-core/src/server/firefox/ffConnection.ts index 3585570e92..90cfba66f0 100644 --- a/packages/playwright-core/src/server/firefox/ffConnection.ts +++ b/packages/playwright-core/src/server/firefox/ffConnection.ts @@ -126,9 +126,6 @@ export class FFConnection extends EventEmitter { this._transport.onmessage = undefined; this._transport.onclose = undefined; const formattedBrowserLogs = helper.formatBrowserLogs(this._browserLogsCollector.recentLogs()); - for (const session of this._sessions.values()) - session.dispose(); - this._sessions.clear(); for (const callback of this._callbacks.values()) { const error = rewriteErrorMessage(callback.error, `Protocol error (${callback.method}): Browser closed.` + formattedBrowserLogs); error.sessionClosed = true; @@ -150,10 +147,6 @@ export class FFConnection extends EventEmitter { } } -export const FFSessionEvents = { - Disconnected: Symbol('Disconnected') -}; - export class FFSession extends EventEmitter { _connection: FFConnection; _disposed = false; @@ -228,7 +221,6 @@ export class FFSession extends EventEmitter { this._callbacks.clear(); this._disposed = true; this._connection._sessions.delete(this._sessionId); - Promise.resolve().then(() => this.emit(FFSessionEvents.Disconnected)); } } diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts index c599d0e3e3..a47e590712 100644 --- a/packages/playwright-core/src/server/firefox/ffPage.ts +++ b/packages/playwright-core/src/server/firefox/ffPage.ts @@ -25,7 +25,7 @@ import { Page, Worker } from '../page'; import type * as types from '../types'; import { getAccessibilityTree } from './ffAccessibility'; import type { FFBrowserContext } from './ffBrowser'; -import { FFSession, FFSessionEvents } from './ffConnection'; +import { FFSession } from './ffConnection'; import { FFExecutionContext } from './ffExecutionContext'; import { RawKeyboardImpl, RawMouseImpl, RawTouchscreenImpl } from './ffInput'; import { FFNetworkManager } from './ffNetworkManager'; @@ -100,10 +100,6 @@ export class FFPage implements PageDelegate { eventsHelper.addEventListener(this._session, 'Page.screencastFrame', this._onScreencastFrame.bind(this)), ]; - session.once(FFSessionEvents.Disconnected, () => { - this._markAsError(new Error('Page closed')); - this._page._didDisconnect(); - }); this._session.once('Page.ready', async () => { await this._page.initOpener(this._opener); if (this._initializationFailed) @@ -346,6 +342,7 @@ export class FFPage implements PageDelegate { } didClose() { + this._markAsError(new Error('Page closed')); this._session.dispose(); eventsHelper.removeEventListeners(this._eventListeners); this._networkManager.dispose(); diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 1fed89ab69..008ed32407 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -617,8 +617,7 @@ export class Frame extends SdkObject { async raceNavigationAction(progress: Progress, options: types.GotoOptions, action: () => Promise): Promise { return LongStandingScope.raceMultiple([ this._detachedScope, - this._page._disconnectedScope, - this._page._crashedScope, + this._page.openScope, ], action().catch(e => { if (e instanceof NavigationAbortedError && e.documentId) { const data = this._redirectedNavigations.get(e.documentId); @@ -1055,8 +1054,7 @@ export class Frame extends SdkObject { // We need this to show expected/received values in time. const actionPromise = new Promise(f => setTimeout(f, timeout)); await LongStandingScope.raceMultiple([ - this._page._disconnectedScope, - this._page._crashedScope, + this._page.openScope, this._detachedScope, ], actionPromise); } @@ -1704,8 +1702,7 @@ class SignalBarrier { return true; }); await LongStandingScope.raceMultiple([ - frame._page._disconnectedScope, - frame._page._crashedScope, + frame._page.openScope, frame._detachedScope, ], waiter.promise).catch(() => {}); waiter.dispose(); diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 318835f52a..fa542e7bbb 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -136,11 +136,9 @@ export class Page extends SdkObject { private _closedState: 'open' | 'closing' | 'closed' = 'open'; private _closedPromise = new ManualPromise(); - private _disconnected = false; private _initialized = false; private _eventsToEmitAfterInitialized: { event: string | symbol, args: any[] }[] = []; - readonly _disconnectedScope = new LongStandingScope(); - readonly _crashedScope = new LongStandingScope(); + private _crashed = false; readonly openScope = new LongStandingScope(); readonly _browserContext: BrowserContext; readonly keyboard: input.Keyboard; @@ -284,18 +282,9 @@ export class Page extends SdkObject { this._frameManager.dispose(); this._frameThrottler.dispose(); this.emit(Page.Events.Crash); - this._crashedScope.close('Page crashed'); + this._crashed = true; this.instrumentation.onPageClose(this); - this.openScope.close('Page closed'); - } - - _didDisconnect() { - this._frameManager.dispose(); - this._frameThrottler.dispose(); - assert(!this._disconnected, 'Page disconnected twice'); - this._disconnected = true; - this._disconnectedScope.close('Page closed'); - this.openScope.close('Page closed'); + this.openScope.close('Page crashed'); } async _onFileChooserOpened(handle: dom.ElementHandle) { @@ -366,7 +355,7 @@ export class Page extends SdkObject { } async _onBindingCalled(payload: string, context: dom.FrameExecutionContext) { - if (this._disconnected || this._closedState === 'closed') + if (this._closedState === 'closed') return; await PageBinding.dispatch(this, payload, context); } @@ -612,7 +601,6 @@ export class Page extends SdkObject { const runBeforeUnload = !!options && !!options.runBeforeUnload; if (this._closedState !== 'closing') { this._closedState = 'closing'; - assert(!this._disconnected, 'Target closed'); // This might throw if the browser context containing the page closes // while we are trying to close the page. await this._delegate.closePage(runBeforeUnload).catch(e => debugLogger.log('error', e)); @@ -632,8 +620,12 @@ export class Page extends SdkObject { return this._closedState === 'closed'; } + hasCrashed() { + return this._crashed; + } + isClosedOrClosingOrCrashed() { - return this._closedState !== 'open' || this._crashedScope.isClosed(); + return this._closedState !== 'open' || this._crashed; } _addWorker(workerId: string, worker: Worker) { diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index 492eaa2929..0750e9f695 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -78,7 +78,8 @@ export class WKBrowser extends Browser { _onDisconnect() { for (const wkPage of this._wkPages.values()) - wkPage.dispose(true); + wkPage.didClose(); + this._wkPages.clear(); for (const video of this._idToVideo.values()) video.artifact.reportFinished(kBrowserClosedError); this._idToVideo.clear(); @@ -178,7 +179,6 @@ export class WKBrowser extends Browser { if (!wkPage) return; wkPage.didClose(); - wkPage.dispose(false); this._wkPages.delete(pageProxyId); } diff --git a/packages/playwright-core/src/server/webkit/wkConnection.ts b/packages/playwright-core/src/server/webkit/wkConnection.ts index cfa3848d53..6d5d0d8488 100644 --- a/packages/playwright-core/src/server/webkit/wkConnection.ts +++ b/packages/playwright-core/src/server/webkit/wkConnection.ts @@ -40,7 +40,8 @@ export class WKConnection { private readonly _transport: ConnectionTransport; private readonly _onDisconnect: () => void; private readonly _protocolLogger: ProtocolLogger; - readonly _browserLogsCollector: RecentLogsCollector; + private readonly _browserLogsCollector: RecentLogsCollector; + _browserDisconnectedLogs: string | undefined; private _lastId = 0; private _closed = false; readonly browserSession: WKSession; @@ -83,7 +84,8 @@ export class WKConnection { this._closed = true; this._transport.onmessage = undefined; this._transport.onclose = undefined; - this.browserSession.dispose(true); + this._browserDisconnectedLogs = helper.formatBrowserLogs(this._browserLogsCollector.recentLogs()); + this.browserSession.dispose(); this._onDisconnect(); } @@ -156,9 +158,9 @@ export class WKSession extends EventEmitter { return this._disposed; } - dispose(disconnected: boolean) { - if (disconnected) - this.errorText = 'Browser closed.' + helper.formatBrowserLogs(this.connection._browserLogsCollector.recentLogs()); + dispose() { + if (this.connection._browserDisconnectedLogs) + this.errorText = 'Browser closed.' + this.connection._browserDisconnectedLogs; for (const callback of this._callbacks.values()) { callback.error.sessionClosed = true; callback.reject(rewriteErrorMessage(callback.error, this.errorText)); diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index a2b24c0677..1d91e96337 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -247,11 +247,11 @@ export class WKPage implements PageDelegate { private _onTargetDestroyed(event: Protocol.Target.targetDestroyedPayload) { const { targetId, crashed } = event; if (this._provisionalPage && this._provisionalPage._session.sessionId === targetId) { - this._provisionalPage._session.dispose(false); + this._provisionalPage._session.dispose(); this._provisionalPage.dispose(); this._provisionalPage = null; } else if (this._session.sessionId === targetId) { - this._session.dispose(false); + this._session.dispose(); eventsHelper.removeEventListeners(this._sessionListeners); if (crashed) { this._session.markAsCrashed(); @@ -261,22 +261,18 @@ export class WKPage implements PageDelegate { } didClose() { - this._page._didClose(); - } - - dispose(disconnected: boolean) { - this._pageProxySession.dispose(disconnected); + this._pageProxySession.dispose(); eventsHelper.removeEventListeners(this._sessionListeners); eventsHelper.removeEventListeners(this._eventListeners); if (this._session) - this._session.dispose(disconnected); + this._session.dispose(); if (this._provisionalPage) { - this._provisionalPage._session.dispose(disconnected); + this._provisionalPage._session.dispose(); this._provisionalPage.dispose(); this._provisionalPage = null; } - this._page._didDisconnect(); this._firstNonInitialNavigationCommittedReject(new Error('Page closed')); + this._page._didClose(); } dispatchMessageToSession(message: any) { diff --git a/packages/playwright-core/src/server/webkit/wkWorkers.ts b/packages/playwright-core/src/server/webkit/wkWorkers.ts index f4b28588d1..cb02c8b32a 100644 --- a/packages/playwright-core/src/server/webkit/wkWorkers.ts +++ b/packages/playwright-core/src/server/webkit/wkWorkers.ts @@ -69,7 +69,7 @@ export class WKWorkers { const workerSession = this._workerSessions.get(event.workerId)!; if (!workerSession) return; - workerSession.dispose(false); + workerSession.dispose(); this._workerSessions.delete(event.workerId); this._page._removeWorker(event.workerId); }) diff --git a/tests/library/beforeunload.spec.ts b/tests/library/beforeunload.spec.ts index fb8b077d11..9f0982eea7 100644 --- a/tests/library/beforeunload.spec.ts +++ b/tests/library/beforeunload.spec.ts @@ -97,10 +97,10 @@ it('should not stall on evaluate when dismissing beforeunload', async ({ page, s await page.click('body'); await Promise.all([ + page.waitForEvent('dialog').then(dialog => dialog.dismiss()), page.evaluate(() => { window.location.reload(); }), - page.waitForEvent('dialog').then(dialog => dialog.dismiss()), ]); }); diff --git a/tests/library/browser.spec.ts b/tests/library/browser.spec.ts index df9d5c7b4b..c3fdc305d0 100644 --- a/tests/library/browser.spec.ts +++ b/tests/library/browser.spec.ts @@ -49,3 +49,15 @@ test('version should work', async function({ browser, browserName }) { else expect(version.match(/^\d+\.\d+/)).toBeTruthy(); }); + +test('should dispatch page.on(close) upon browser.close and reject evaluate', async ({ browserType }) => { + const browser = await browserType.launch(); + const page = await browser.newPage(); + let closed = false; + page.on('close', () => closed = true); + const promise = page.evaluate(() => new Promise(() => {})).catch(e => e); + await browser.close(); + expect(closed).toBe(true); + const error = await promise; + expect(error.message).toMatch(/(Target|Browser) closed/); +}); diff --git a/tests/page/page-close.spec.ts b/tests/page/page-close.spec.ts index f9ebd3dc81..cb9b028d97 100644 --- a/tests/page/page-close.spec.ts +++ b/tests/page/page-close.spec.ts @@ -26,10 +26,11 @@ it('should close page with active dialog', async ({ page }) => { await page.close(); }); -it('should not accept after close', async ({ page, mode }) => { +it('should not accept dialog after close', async ({ page, mode }) => { it.fixme(mode.startsWith('service2'), 'Times out'); + const promise = page.waitForEvent('dialog'); page.evaluate(() => alert()).catch(() => {}); - const dialog = await page.waitForEvent('dialog'); + const dialog = await promise; await page.close(); const e = await dialog.dismiss().catch(e => e); expect(e.message).toContain('Target page, context or browser has been closed'); diff --git a/tests/page/page-event-request.spec.ts b/tests/page/page-event-request.spec.ts index 4d8507e2bd..cd95dbddb9 100644 --- a/tests/page/page-event-request.spec.ts +++ b/tests/page/page-event-request.spec.ts @@ -47,9 +47,9 @@ it('should report requests and responses handled by service worker', async ({ pa await page.goto(server.PREFIX + '/serviceworkers/fetchdummy/sw.html'); await page.evaluate(() => window['activationPromise']); - const [swResponse, request] = await Promise.all([ - page.evaluate(() => window['fetchDummy']('foo')), + const [request, swResponse] = await Promise.all([ page.waitForEvent('request'), + page.evaluate(() => window['fetchDummy']('foo')), ]); expect(swResponse).toBe('responseFromServiceWorker:foo'); expect(request.url()).toBe(server.PREFIX + '/serviceworkers/fetchdummy/foo');