diff --git a/src/chromium/FrameManager.ts b/src/chromium/FrameManager.ts index 73dda0f7a6..f2009056de 100644 --- a/src/chromium/FrameManager.ts +++ b/src/chromium/FrameManager.ts @@ -81,7 +81,7 @@ export class FrameManager extends EventEmitter implements PageDelegate { this._client.on('Page.fileChooserOpened', event => this._onFileChooserOpened(event)); this._client.on('Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId)); this._client.on('Page.frameDetached', event => this._onFrameDetached(event.frameId)); - this._client.on('Page.frameNavigated', event => this._onFrameNavigated(event.frame)); + this._client.on('Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)); this._client.on('Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)); this._client.on('Page.javascriptDialogOpening', event => this._onDialog(event)); this._client.on('Page.lifecycleEvent', event => this._onLifecycleEvent(event)); @@ -199,14 +199,12 @@ export class FrameManager extends EventEmitter implements PageDelegate { const frame = this._frames.get(event.frameId); if (!frame) return; - if (event.name === 'init') { + if (event.name === 'init') frame._firedLifecycleEvents.clear(); - frame._expectNewDocumentNavigation(event.loaderId); - } else if (event.name === 'load') { + else if (event.name === 'load') frame._lifecycleEvent('load'); - } else if (event.name === 'DOMContentLoaded') { + else if (event.name === 'DOMContentLoaded') frame._lifecycleEvent('domcontentloaded'); - } } _onFrameStoppedLoading(frameId: string) { @@ -220,7 +218,7 @@ export class FrameManager extends EventEmitter implements PageDelegate { _handleFrameTree(frameTree: Protocol.Page.FrameTree) { if (frameTree.frame.parentId) this._onFrameAttached(frameTree.frame.id, frameTree.frame.parentId); - this._onFrameNavigated(frameTree.frame); + this._onFrameNavigated(frameTree.frame, true); if (!frameTree.childFrames) return; @@ -254,7 +252,7 @@ export class FrameManager extends EventEmitter implements PageDelegate { this._page.emit(Events.Page.FrameAttached, frame); } - _onFrameNavigated(framePayload: Protocol.Page.Frame) { + _onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) { const isMainFrame = !framePayload.parentId; let frame = isMainFrame ? this._mainFrame : this._frames.get(framePayload.id); assert(isMainFrame || frame, 'We either navigate top level or have old version of the navigated frame'); @@ -279,7 +277,7 @@ export class FrameManager extends EventEmitter implements PageDelegate { this._mainFrame = frame; } - frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, framePayload.loaderId); + frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, framePayload.loaderId, initial); this._page.emit(Events.Page.FrameNavigated, frame); } diff --git a/src/chromium/NetworkManager.ts b/src/chromium/NetworkManager.ts index ae105cc479..798a6b27d6 100644 --- a/src/chromium/NetworkManager.ts +++ b/src/chromium/NetworkManager.ts @@ -201,8 +201,6 @@ export class NetworkManager extends EventEmitter { const frame = event.frameId ? this._frameManager.frame(event.frameId) : null; const isNavigationRequest = event.requestId === event.loaderId && event.type === 'Document'; const documentId = isNavigationRequest ? event.loaderId : undefined; - if (documentId && frame) - frame._expectNewDocumentNavigation(documentId, event.request.url); const request = new InterceptableRequest(this._client, frame, interceptionId, documentId, this._userRequestInterceptionEnabled, event, redirectChain); this._requestIdToRequest.set(event.requestId, request); this.emit(NetworkManagerEvents.Request, request.request); @@ -258,20 +256,12 @@ export class NetworkManager extends EventEmitter { // @see https://crbug.com/750469 if (!request) return; - request.request._setFailureText(event.errorText); const response = request.request.response(); if (response) response._requestFinished(); this._requestIdToRequest.delete(request._requestId); this._attemptedAuthentications.delete(request._interceptionId); - if (request._documentId && request.request.frame()) { - const isCurrentDocument = request.request.frame()._lastDocumentId === request._documentId; - let errorText = event.errorText; - if (event.canceled) - errorText += '; maybe frame was detached?'; - if (!isCurrentDocument) - request.request.frame()._onAbortedNewDocumentNavigation(request._documentId, errorText); - } + request.request._setFailureText(event.errorText, event.canceled); this.emit(NetworkManagerEvents.RequestFailed, request.request); } } @@ -298,7 +288,7 @@ class InterceptableRequest { this._documentId = documentId; this._allowInterception = allowInterception; - this.request = new network.Request(frame, redirectChain, !!documentId, + this.request = new network.Request(frame, redirectChain, documentId, event.request.url, event.type.toLowerCase(), event.request.method, event.request.postData, headersObject(event.request.headers)); (this.request as any)[interceptableRequestSymbol] = this; } diff --git a/src/firefox/FrameManager.ts b/src/firefox/FrameManager.ts index ac3cfa6919..f7fdc015bd 100644 --- a/src/firefox/FrameManager.ts +++ b/src/firefox/FrameManager.ts @@ -137,8 +137,6 @@ export class FrameManager extends EventEmitter implements PageDelegate { } _onNavigationStarted(params) { - const frame = this._frames.get(params.frameId); - frame._expectNewDocumentNavigation(params.navigationId, params.url); } _onNavigationAborted(params) { @@ -148,7 +146,7 @@ export class FrameManager extends EventEmitter implements PageDelegate { _onNavigationCommitted(params) { const frame = this._frames.get(params.frameId); - frame._onCommittedNewDocumentNavigation(params.url, params.name, params.navigationId); + frame._onCommittedNewDocumentNavigation(params.url, params.name, params.navigationId, false); this._page.emit(Events.Page.FrameNavigated, frame); } diff --git a/src/firefox/NetworkManager.ts b/src/firefox/NetworkManager.ts index 09a56ba4f7..c2d2b58bdc 100644 --- a/src/firefox/NetworkManager.ts +++ b/src/firefox/NetworkManager.ts @@ -126,7 +126,7 @@ export class NetworkManager extends EventEmitter { this._requests.delete(request._id); if (request.request.response()) request.request.response()._requestFinished(); - request.request._setFailureText(event.errorCode); + request.request._setFailureText(event.errorCode, event.errorCode === 'NS_BINDING_ABORTED'); this.emit(NetworkManagerEvents.RequestFailed, request.request); } } @@ -180,7 +180,7 @@ class InterceptableRequest { for (const {name, value} of payload.headers) headers[name.toLowerCase()] = value; - this.request = new network.Request(frame, redirectChain, payload.isNavigationRequest, + this.request = new network.Request(frame, redirectChain, payload.navigationId, payload.url, causeToResourceType[payload.cause] || 'other', payload.method, payload.postData, headers); (this.request as any)[interceptableRequestSymbol] = this; } diff --git a/src/frames.ts b/src/frames.ts index 90bd56f21a..e3a7815147 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -422,9 +422,9 @@ export class Frame { return context.evaluate(() => document.title); } - _expectNewDocumentNavigation(documentId: string, url?: string) { + _onNavigationRequest(request: network.Request) { for (const watcher of this._page._lifecycleWatchers) - watcher._onExpectedNewDocumentNavigation(this, documentId, url); + watcher._onNavigationRequest(this, request); } _onAbortedNewDocumentNavigation(documentId: string, errorText: string) { @@ -432,11 +432,15 @@ export class Frame { watcher._onAbortedNewDocumentNavigation(this, documentId, errorText); } - _onCommittedNewDocumentNavigation(url: string, name: string, documentId: string) { + _onCommittedNewDocumentNavigation(url: string, name: string, documentId: string, initial: boolean) { this._url = url; this._name = name; this._lastDocumentId = documentId; this._firedLifecycleEvents.clear(); + if (!initial) { + for (const watcher of this._page._lifecycleWatchers) + watcher._onCommittedNewDocumentNavigation(this); + } } _onCommittedSameDocumentNavigation(url: string) { @@ -600,7 +604,6 @@ export class LifecycleWatcher { private _navigationAbortedCallback: (err: Error) => void; private _maximumTimer: NodeJS.Timer; private _hasSameDocumentNavigation: boolean; - private _listeners: RegisteredListener[]; private _targetUrl?: string; private _expectedDocumentId?: string; @@ -623,12 +626,6 @@ export class LifecycleWatcher { this._frame._page._disconnectedPromise.then(() => new Error('Navigation failed because browser has disconnected!')), ]); frame._page._lifecycleWatchers.add(this); - this._listeners = [ - helper.addEventListener(this._frame._page, Events.Page.Request, (request: network.Request) => { - if (request.frame() === this._frame && request.isNavigationRequest() && (!this._navigationRequest || request.redirectChain().includes(this._navigationRequest))) - this._navigationRequest = request; - }), - ]; this._checkLifecycleComplete(); } @@ -647,10 +644,19 @@ export class LifecycleWatcher { this._checkLifecycleComplete(); } - _onExpectedNewDocumentNavigation(frame: Frame, documentId: string, url?: string) { + _onNavigationRequest(frame: Frame, request: network.Request) { + assert(request._documentId); if (frame === this._frame && this._expectedDocumentId === undefined) { - this._expectedDocumentId = documentId; - this._targetUrl = url; + this._navigationRequest = request; + this._expectedDocumentId = request._documentId; + this._targetUrl = request.url(); + } + } + + _onCommittedNewDocumentNavigation(frame: Frame) { + if (frame === this._frame && this._expectedDocumentId === undefined) { + this._expectedDocumentId = frame._lastDocumentId; + this._targetUrl = frame.url(); } } @@ -668,7 +674,7 @@ export class LifecycleWatcher { } navigationResponse(): Promise { - return this._navigationRequest ? this._navigationRequest._waitForFinished() : null; + return this._navigationRequest ? this._navigationRequest._finalRequest._waitForFinished() : null; } private _createTimeoutPromise(timeout: number): Promise { @@ -704,7 +710,6 @@ export class LifecycleWatcher { dispose() { this._frame._page._lifecycleWatchers.delete(this); - helper.removeEventListeners(this._listeners); clearTimeout(this._maximumTimer); } } diff --git a/src/network.ts b/src/network.ts index 834b7f4216..d3ea79b917 100644 --- a/src/network.ts +++ b/src/network.ts @@ -72,7 +72,8 @@ export type Headers = { [key: string]: string }; export class Request { private _response: Response | null = null; _redirectChain: Request[]; - private _isNavigationRequest: boolean; + _finalRequest: Request; + readonly _documentId?: string; private _failureText: string | null = null; private _url: string; private _resourceType: string; @@ -85,11 +86,14 @@ export class Request { private _waitForFinishedPromise: Promise; private _waitForFinishedPromiseCallback: (value?: Response | undefined) => void; - constructor(frame: frames.Frame | null, redirectChain: Request[], isNavigationRequest: boolean, + constructor(frame: frames.Frame | null, redirectChain: Request[], documentId: string, url: string, resourceType: string, method: string, postData: string, headers: Headers) { this._frame = frame; this._redirectChain = redirectChain; - this._isNavigationRequest = isNavigationRequest; + this._finalRequest = this; + for (const request of redirectChain) + request._finalRequest = this; + this._documentId = documentId; this._url = url; this._resourceType = resourceType; this._method = method; @@ -97,10 +101,20 @@ export class Request { this._headers = headers; this._waitForResponsePromise = new Promise(f => this._waitForResponsePromiseCallback = f); this._waitForFinishedPromise = new Promise(f => this._waitForFinishedPromiseCallback = f); + if (documentId && frame) + frame._onNavigationRequest(this); } - _setFailureText(failureText: string) { + _setFailureText(failureText: string, canceled: boolean) { this._failureText = failureText; + if (this._documentId && this._frame) { + const isCurrentDocument = this._frame._lastDocumentId === this._documentId; + let errorText = failureText; + if (canceled) + errorText += '; maybe frame was detached?'; + if (!isCurrentDocument) + this._frame._onAbortedNewDocumentNavigation(this._documentId, errorText); + } this._waitForFinishedPromiseCallback(); } @@ -149,7 +163,7 @@ export class Request { } isNavigationRequest(): boolean { - return this._isNavigationRequest; + return !!this._documentId; } redirectChain(): Request[] { diff --git a/src/webkit/FrameManager.ts b/src/webkit/FrameManager.ts index 9564b55997..5307aab128 100644 --- a/src/webkit/FrameManager.ts +++ b/src/webkit/FrameManager.ts @@ -230,9 +230,7 @@ export class FrameManager extends EventEmitter implements PageDelegate { // Append session id to avoid cross-process loaderId clash. const documentId = this._session._sessionId + '::' + framePayload.loaderId; - if (!initial) - frame._expectNewDocumentNavigation(documentId); - frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, documentId); + frame._onCommittedNewDocumentNavigation(framePayload.url, framePayload.name, documentId, initial); this._page.emit(Events.Page.FrameNavigated, frame); } diff --git a/src/webkit/NetworkManager.ts b/src/webkit/NetworkManager.ts index 2fa1fb6896..2595dba7a7 100644 --- a/src/webkit/NetworkManager.ts +++ b/src/webkit/NetworkManager.ts @@ -101,8 +101,6 @@ export class NetworkManager extends EventEmitter { // TODO(einbinder) this will fail if we are an XHR document request const isNavigationRequest = event.type === 'Document'; const documentId = isNavigationRequest ? this._session._sessionId + '::' + event.loaderId : undefined; - if (documentId) - frame._expectNewDocumentNavigation(documentId, event.request.url); const request = new InterceptableRequest(frame, undefined, event, redirectChain, documentId); this._requestIdToRequest.set(event.requestId, request); this.emit(NetworkManagerEvents.Request, request.request); @@ -158,20 +156,12 @@ export class NetworkManager extends EventEmitter { // @see https://crbug.com/750469 if (!request) return; - request.request._setFailureText(event.errorText); const response = request.request.response(); if (response) response._requestFinished(); this._requestIdToRequest.delete(request._requestId); this._attemptedAuthentications.delete(request._interceptionId); - if (request._documentId) { - const isCurrentDocument = request.request.frame()._lastDocumentId === request._documentId; - let errorText = event.errorText; - if (errorText.includes('cancelled')) - errorText += '; maybe frame was detached?'; - if (!isCurrentDocument) - request.request.frame()._onAbortedNewDocumentNavigation(request._documentId, errorText); - } + request.request._setFailureText(event.errorText, event.errorText.includes('cancelled')); this.emit(NetworkManagerEvents.RequestFailed, request.request); } } @@ -192,7 +182,7 @@ class InterceptableRequest { this._requestId = event.requestId; this._interceptionId = interceptionId; this._documentId = documentId; - this.request = new network.Request(frame, redirectChain, !!documentId, event.request.url, + this.request = new network.Request(frame, redirectChain, documentId, event.request.url, event.type ? event.type.toLowerCase() : 'Unknown', event.request.method, event.request.postData, headersObject(event.request.headers)); (this.request as any)[interceptableRequestSymbol] = this; } diff --git a/test/network.spec.js b/test/network.spec.js index b9d736db62..9a152825fa 100644 --- a/test/network.spec.js +++ b/test/network.spec.js @@ -208,7 +208,7 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { describe('Network Events', function() { it('Page.Events.Request', async({page, server}) => { const requests = []; - page.on('request', request => requests.push(request)); + page.on('request', request => !utils.isFavicon(request) && requests.push(request)); await page.goto(server.EMPTY_PAGE); expect(requests.length).toBe(1); expect(requests[0].url()).toBe(server.EMPTY_PAGE); @@ -278,10 +278,10 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { }); it('should support redirects', async({page, server}) => { const events = []; - page.on('request', request => events.push(`${request.method()} ${request.url()}`)); - page.on('response', response => events.push(`${response.status()} ${response.url()}`)); - page.on('requestfinished', request => events.push(`DONE ${request.url()}`)); - page.on('requestfailed', request => events.push(`FAIL ${request.url()}`)); + page.on('request', request => !utils.isFavicon(request) && events.push(`${request.method()} ${request.url()}`)); + page.on('response', response => !utils.isFavicon(response.request()) && events.push(`${response.status()} ${response.url()}`)); + page.on('requestfinished', request => !utils.isFavicon(request) && events.push(`DONE ${request.url()}`)); + page.on('requestfailed', request => !utils.isFavicon(request) && events.push(`FAIL ${request.url()}`)); server.setRedirect('/foo.html', '/empty.html'); const FOO_URL = server.PREFIX + '/foo.html'; const response = await page.goto(FOO_URL);