diff --git a/src/chromium/crConnection.ts b/src/chromium/crConnection.ts index eeb1fe9c58..2a1d30c5e1 100644 --- a/src/chromium/crConnection.ts +++ b/src/chromium/crConnection.ts @@ -121,6 +121,7 @@ export class CRSession extends EventEmitter { private readonly _targetType: string; private readonly _sessionId: string; private readonly _rootSessionId: string; + private _unhandledException: Error | undefined; on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; addListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; off: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -145,6 +146,8 @@ export class CRSession extends EventEmitter { method: T, params?: Protocol.CommandParameters[T] ): Promise { + if (this._unhandledException) + throw this._unhandledException; if (!this._connection) throw new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`); const id = this._connection._rawSend(this._sessionId, method, params); @@ -163,7 +166,7 @@ export class CRSession extends EventEmitter { callback.resolve(object.result); } else { assert(!object.id); - Promise.resolve().then(() => this.emit(object.method!, object.params)); + Promise.resolve().then(() => this.emit(object.method!, object.params)).catch(e => this._unhandledException = this._unhandledException || e); } } diff --git a/src/chromium/crNetworkManager.ts b/src/chromium/crNetworkManager.ts index f75cfe4733..64d2433d83 100644 --- a/src/chromium/crNetworkManager.ts +++ b/src/chromium/crNetworkManager.ts @@ -182,6 +182,8 @@ export class CRNetworkManager { } const isNavigationRequest = event.requestId === event.loaderId && event.type === 'Document'; const documentId = isNavigationRequest ? event.loaderId : undefined; + if (isNavigationRequest) + this._page._frameManager.frameUpdatedDocumentIdForNavigation(event.frameId!, documentId!); const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectedFrom); this._requestIdToRequest.set(event.requestId, request); this._page._frameManager.requestStarted(request.request); diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 359593e384..2dc335ddf8 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -482,7 +482,7 @@ class FrameSession { } _onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) { - this._page._frameManager.frameRequestedNavigation(payload.frameId); + this._page._frameManager.frameRequestedNavigation(payload.frameId, ''); } _onFrameNavigatedWithinDocument(frameId: string, url: string) { diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 0474d6ea3d..97c03069be 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -65,7 +65,7 @@ export class FFPage implements PageDelegate { helper.addEventListener(this._session, 'Page.frameDetached', this._onFrameDetached.bind(this)), helper.addEventListener(this._session, 'Page.navigationAborted', this._onNavigationAborted.bind(this)), helper.addEventListener(this._session, 'Page.navigationCommitted', this._onNavigationCommitted.bind(this)), - helper.addEventListener(this._session, 'Page.navigationStarted', event => this._onNavigationStarted(event.frameId)), + helper.addEventListener(this._session, 'Page.navigationStarted', this._onNavigationStarted.bind(this)), helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)), @@ -136,8 +136,8 @@ export class FFPage implements PageDelegate { this._page._frameManager.frameDidPotentiallyRequestNavigation(); } - _onNavigationStarted(frameId: string) { - this._page._frameManager.frameRequestedNavigation(frameId); + _onNavigationStarted(params: Protocol.Page.navigationStartedPayload) { + this._page._frameManager.frameRequestedNavigation(params.frameId, params.navigationId); } _onNavigationAborted(params: Protocol.Page.navigationAbortedPayload) { diff --git a/src/frames.ts b/src/frames.ts index 4cab0384bc..a335b1922c 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -127,12 +127,20 @@ export class FrameManager { barrier.release(); } - frameRequestedNavigation(frameId: string) { + frameRequestedNavigation(frameId: string, documentId: string) { const frame = this._frames.get(frameId); if (!frame) return; for (const barrier of this._pendingNavigationBarriers) barrier.addFrame(frame); + frame._pendingDocumentId = documentId; + } + + frameUpdatedDocumentIdForNavigation(frameId: string, documentId: string) { + const frame = this._frames.get(frameId); + if (!frame) + return; + frame._pendingDocumentId = documentId; } frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) { @@ -140,7 +148,9 @@ export class FrameManager { this.removeChildFramesRecursively(frame); frame._url = url; frame._name = name; + assert(!frame._pendingDocumentId || frame._pendingDocumentId === documentId); frame._lastDocumentId = documentId; + frame._pendingDocumentId = ''; for (const task of frame._frameTasks) task.onNewDocument(documentId); this.clearFrameLifecycle(frame); @@ -225,8 +235,9 @@ export class FrameManager { requestFailed(request: network.Request, canceled: boolean) { this._inflightRequestFinished(request); if (request._documentId) { - const isCurrentDocument = request.frame()._lastDocumentId === request._documentId; - if (!isCurrentDocument) { + const isPendingDocument = request.frame()._pendingDocumentId === request._documentId; + if (isPendingDocument) { + request.frame()._pendingDocumentId = ''; let errorText = request.failure()!.errorText; if (canceled) errorText += '; maybe frame was detached?'; @@ -319,6 +330,7 @@ export class Frame { _id: string; readonly _firedLifecycleEvents: Set; _lastDocumentId = ''; + _pendingDocumentId = ''; _frameTasks = new Set(); readonly _page: Page; private _parentFrame: Frame | null; diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index a3cfd59d2b..d212e3c3ba 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -329,7 +329,7 @@ export class WKPage implements PageDelegate { } private _onFrameScheduledNavigation(frameId: string) { - this._page._frameManager.frameRequestedNavigation(frameId); + this._page._frameManager.frameRequestedNavigation(frameId, ''); } private _onFrameStoppedLoading(frameId: string) { @@ -761,6 +761,8 @@ export class WKPage implements PageDelegate { // TODO(einbinder) this will fail if we are an XHR document request const isNavigationRequest = event.type === 'Document'; const documentId = isNavigationRequest ? event.loaderId : undefined; + if (isNavigationRequest) + this._page._frameManager.frameUpdatedDocumentIdForNavigation(event.frameId, documentId!); const request = new WKInterceptableRequest(session, this._page._needsRequestInterception(), frame, event, redirectedFrom, documentId); this._requestIdToRequest.set(event.requestId, request); this._page._frameManager.requestStarted(request.request); diff --git a/test/autowaiting.spec.js b/test/autowaiting.spec.js index 3ba2a4ed4a..5995e88e6e 100644 --- a/test/autowaiting.spec.js +++ b/test/autowaiting.spec.js @@ -103,7 +103,7 @@ describe('Auto waiting', () => { ]); expect(messages.join('|')).toBe('route|domcontentloaded|evaluate'); }); - it.fail(CHROMIUM)('should await navigation when assigning location twice', async({page, server}) => { + it('should await navigation when assigning location twice', async({page, server}) => { const messages = []; server.setRoute('/empty.html?cancel', async (req, res) => { res.end('done'); }); server.setRoute('/empty.html?override', async (req, res) => { messages.push('routeoverride'); res.end('done'); }); @@ -196,10 +196,7 @@ describe('Auto waiting should not hang when', () => { setTimeout(() => window.stop(), 100); }, server.EMPTY_PAGE); }); - it.fail(CHROMIUM)('calling window.stop sync', async({page, server, httpsServer}) => { - // Flaky, see https://github.com/microsoft/playwright/pull/1630/checks?check_run_id=553475173. - // We only get Page.frameStoppedLoading, but do not know that navigation was aborted or - // that navigation request was cancelled. + it('calling window.stop sync', async({page, server, httpsServer}) => { await page.evaluate((url) => { window.location.href = url; window.stop(); diff --git a/test/launcher.spec.js b/test/launcher.spec.js index d993a602a6..33cc7c6ce3 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -316,8 +316,7 @@ describe('browserType.launchPersistentContext', function() { await removeUserDataDir(userDataDir); await removeUserDataDir(userDataDir2); }); - // See https://github.com/microsoft/playwright/issues/717 - it.slow().fail(WIN && CHROMIUM)('userDataDir option should restore cookies', async({browserType, server}) => { + it.slow()('userDataDir option should restore cookies', async({browserType, server}) => { const userDataDir = await makeUserDataDir(); const browserContext = await browserType.launchPersistentContext(userDataDir, defaultBrowserOptions); const page = await browserContext.newPage();