From a4c806a68e6124f819228644f041f6641f2d5f0d Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Fri, 7 Feb 2020 15:41:25 -0800 Subject: [PATCH] frame.waitForLoadState test, convert waiters to methods --- src/frames.ts | 225 +++++++++++++++++++--------------------- test/navigation.spec.js | 40 ++++--- 2 files changed, 134 insertions(+), 131 deletions(-) diff --git a/src/frames.ts b/src/frames.ts index 27924dad30..39959e2162 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -355,6 +355,8 @@ export class Frame { _inflightRequests = new Set(); readonly _networkIdleTimers = new Map(); private _setContentCounter = 0; + private _detachedPromise: Promise; + private _detachedCallback = () => {}; constructor(page: Page, id: string, parentFrame: Frame | null) { this._id = id; @@ -362,6 +364,8 @@ export class Frame { this._page = page; this._parentFrame = parentFrame; + this._detachedPromise = new Promise(x => this._detachedCallback = x); + this._contextData.set('main', { contextPromise: new Promise(() => {}), contextResolveCallback: () => {}, context: null, rerunnableTasks: new Set() }); this._contextData.set('utility', { contextPromise: new Promise(() => {}), contextResolveCallback: () => {}, context: null, rerunnableTasks: new Set() }); this._setContext('main', null); @@ -383,9 +387,9 @@ export class Frame { const disposer = new Disposer(); const timeoutPromise = disposer.add(createTimeoutPromise(timeout)); - const frameDestroyedPromise = disposer.add(createFrameDestroyedPromise(this)); - const sameDocumentPromise = disposer.add(waitForSameDocumentNavigation(this)); - const requestWatcher = disposer.add(trackDocumentRequests(this)); + const frameDestroyedPromise = this._createFrameDestroyedPromise(); + const sameDocumentPromise = disposer.add(this._waitForSameDocumentNavigation()); + const requestWatcher = disposer.add(this._trackDocumentRequests()); let navigateResult: GotoResult; const navigate = async () => { try { @@ -403,13 +407,13 @@ export class Frame { const promises: Promise[] = [timeoutPromise, frameDestroyedPromise]; if (navigateResult!.newDocumentId) - promises.push(disposer.add(waitForSpecificDocument(this, navigateResult!.newDocumentId))); + promises.push(disposer.add(this._waitForSpecificDocument(navigateResult!.newDocumentId))); else promises.push(sameDocumentPromise); throwIfError(await Promise.race(promises)); const request = (navigateResult! && navigateResult!.newDocumentId) ? requestWatcher.get(navigateResult!.newDocumentId) : null; - const waitForLifecyclePromise = disposer.add(waitForLifecycle(this, options.waitUntil)); + const waitForLifecyclePromise = disposer.add(this._waitForLifecycle(options.waitUntil)); throwIfError(await Promise.race([timeoutPromise, frameDestroyedPromise, waitForLifecyclePromise])); disposer.dispose(); @@ -429,28 +433,28 @@ export class Frame { async waitForNavigation(options: WaitForNavigationOptions = {}): Promise { const disposer = new Disposer(); - const requestWatcher = disposer.add(trackDocumentRequests(this)); + const requestWatcher = disposer.add(this._trackDocumentRequests()); const {timeout = this._page._timeoutSettings.navigationTimeout()} = options; const failurePromise = Promise.race([ + this._createFrameDestroyedPromise(), disposer.add(createTimeoutPromise(timeout)), - disposer.add(createFrameDestroyedPromise(this)), ]); let documentId: string|null = null; let error: void|Error = await Promise.race([ failurePromise, - disposer.add(waitForNewDocument(this, options.url)).then(result => { + disposer.add(this._waitForNewDocument(options.url)).then(result => { if (result.error) return result.error; documentId = result.documentId; }), - disposer.add(waitForSameDocumentNavigation(this, options.url)), + disposer.add(this._waitForSameDocumentNavigation(options.url)), ]); const request = requestWatcher.get(documentId!); if (!error) { error = await Promise.race([ failurePromise, - disposer.add(waitForLifecycle(this, options.waitUntil)), + disposer.add(this._waitForLifecycle(options.waitUntil)), ]); } disposer.dispose(); @@ -463,14 +467,107 @@ export class Frame { const {timeout = this._page._timeoutSettings.navigationTimeout()} = options; const disposer = new Disposer(); const error = await Promise.race([ + this._createFrameDestroyedPromise(), disposer.add(createTimeoutPromise(timeout)), - disposer.add(waitForLifecycle(this, options.waitUntil)), + disposer.add(this._waitForLifecycle(options.waitUntil)), ]); disposer.dispose(); if (error) throw error; } + _waitForSpecificDocument(expectedDocumentId: string): Disposable> { + let resolve: (error: Error|void) => void; + const promise = new Promise(x => resolve = x); + const watch = (documentId: string, error?: Error) => { + if (documentId !== expectedDocumentId) + return resolve(new Error('Navigation interrupted by another one')); + resolve(error); + }; + const dispose = () => this._documentWatchers.delete(watch); + this._documentWatchers.add(watch); + return {value: promise, dispose}; + } + + _waitForNewDocument(url?: types.URLMatch): Disposable> { + let resolve: (error: {error?: Error, documentId: string}) => void; + const promise = new Promise<{error?: Error, documentId: string}>(x => resolve = x); + const watch = (documentId: string, error?: Error) => { + if (!error && !platform.urlMatches(this.url(), url)) + return; + resolve({error, documentId}); + }; + const dispose = () => this._documentWatchers.delete(watch); + this._documentWatchers.add(watch); + return {value: promise, dispose}; + } + + _waitForSameDocumentNavigation(url?: types.URLMatch): Disposable> { + let resolve: () => void; + const promise = new Promise(x => resolve = x); + const watch = () => { + if (platform.urlMatches(this.url(), url)) + resolve(); + }; + const dispose = () => this._sameDocumentNavigationWatchers.delete(watch); + this._sameDocumentNavigationWatchers.add(watch); + return {value: promise, dispose}; + } + + _waitForLifecycle(waitUntil: LifecycleEvent|LifecycleEvent[] = 'load'): Disposable> { + let resolve: () => void; + const expectedLifecycle = typeof waitUntil === 'string' ? [waitUntil] : waitUntil; + for (const event of expectedLifecycle) { + if (!kLifecycleEvents.has(event)) + throw new Error(`Unsupported waitUntil option ${String(event)}`); + } + + const checkLifecycleComplete = () => { + if (!checkLifecycleRecursively(this)) + return; + resolve(); + }; + + const promise = new Promise(x => resolve = x); + const dispose = () => this._page._frameManager._lifecycleWatchers.delete(checkLifecycleComplete); + this._page._frameManager._lifecycleWatchers.add(checkLifecycleComplete); + checkLifecycleComplete(); + return {value: promise, dispose}; + + function checkLifecycleRecursively(frame: Frame): boolean { + for (const event of expectedLifecycle) { + if (!frame._firedLifecycleEvents.has(event)) + return false; + } + for (const child of frame.childFrames()) { + if (!checkLifecycleRecursively(child)) + return false; + } + return true; + } + } + + _trackDocumentRequests(): Disposable> { + const requestMap = new Map(); + const dispose = () => { + this._requestWatchers.delete(onRequest); + }; + const onRequest = (request: network.Request) => { + if (!request._documentId || request.redirectChain().length) + return; + requestMap.set(request._documentId, request); + }; + this._requestWatchers.add(onRequest); + return {dispose, value: requestMap}; + } + + _createFrameDestroyedPromise(): Promise { + return Promise.race([ + this._page._disconnectedPromise.then(() => new Error('Navigation failed because browser has disconnected!')), + this._detachedPromise.then(() => new Error('Navigating frame was detached!')), + ]); + } + async frameElement(): Promise { return this._page._delegate.getFrameElement(this); } @@ -854,6 +951,7 @@ export class Frame { _onDetached() { this._detached = true; + this._detachedCallback(); for (const data of this._contextData.values()) { for (const rerunnableTask of data.rerunnableTasks) rerunnableTask.terminate(new Error('waitForFunction failed: frame got detached.')); @@ -999,26 +1097,6 @@ class Disposer { } } -function createFrameDestroyedPromise(frame: Frame): Disposable> { - let detachedCallback: (error: Error) => void; - const detachedPromise = new Promise(x => detachedCallback = x); - const listener = (detachedFrame: Frame) => { - if (detachedFrame !== frame) - return; - detachedCallback(new Error('Navigating frame was detached!')); - dispose(); - }; - const dispose = () => { - frame._page.removeListener(Events.Page.FrameDetached, listener); - }; - frame._page.addListener(Events.Page.FrameDetached, listener); - const promise = Promise.race([ - frame._page._disconnectedPromise.then(() => new Error('Navigation failed because browser has disconnected!')), - detachedPromise, - ]); - return {value: promise, dispose}; -} - function createTimeoutPromise(timeout: number): Disposable> { if (!timeout) return { value: new Promise(() => {}), dispose: () => void 0 }; @@ -1036,91 +1114,6 @@ function createTimeoutPromise(timeout: number): Disposable }; } -function waitForSpecificDocument(frame: Frame, expectedDocumentId: string): Disposable> { - let resolve: (error: Error|void) => void; - const promise = new Promise(x => resolve = x); - const watch = (documentId: string, error?: Error) => { - if (documentId !== expectedDocumentId) - return resolve(new Error('Navigation interrupted by another one')); - resolve(error); - }; - const dispose = () => frame._documentWatchers.delete(watch); - frame._documentWatchers.add(watch); - return {value: promise, dispose}; - -} -function waitForNewDocument(frame: Frame, url?: types.URLMatch): Disposable> { - let resolve: (error: {error?: Error, documentId: string}) => void; - const promise = new Promise<{error?: Error, documentId: string}>(x => resolve = x); - const watch = (documentId: string, error?: Error) => { - if (!error && !platform.urlMatches(frame.url(), url)) - return; - resolve({error, documentId}); - }; - const dispose = () => frame._documentWatchers.delete(watch); - frame._documentWatchers.add(watch); - return {value: promise, dispose}; -} - -function waitForSameDocumentNavigation(frame: Frame, url?: types.URLMatch): Disposable> { - let resolve: () => void; - const promise = new Promise(x => resolve = x); - const watch = () => { - if (platform.urlMatches(frame.url(), url)) - resolve(); - }; - const dispose = () => frame._sameDocumentNavigationWatchers.delete(watch); - frame._sameDocumentNavigationWatchers.add(watch); - return {value: promise, dispose}; -} - -function waitForLifecycle(frame: Frame, waitUntil: LifecycleEvent|LifecycleEvent[] = 'load'): Disposable> { - let resolve: () => void; - const expectedLifecycle = typeof waitUntil === 'string' ? [waitUntil] : waitUntil; - for (const event of expectedLifecycle) { - if (!kLifecycleEvents.has(event)) - throw new Error(`Unsupported waitUntil option ${String(event)}`); - } - - const promise = new Promise(x => resolve = x); - const dispose = () => frame._page._frameManager._lifecycleWatchers.delete(checkLifecycleComplete); - frame._page._frameManager._lifecycleWatchers.add(checkLifecycleComplete); - checkLifecycleComplete(); - return {value: promise, dispose}; - - function checkLifecycleRecursively(frame: Frame): boolean { - for (const event of expectedLifecycle) { - if (!frame._firedLifecycleEvents.has(event)) - return false; - } - for (const child of frame.childFrames()) { - if (!checkLifecycleRecursively(child)) - return false; - } - return true; - } - - function checkLifecycleComplete() { - if (!checkLifecycleRecursively(frame)) - return; - resolve(); - } -} - -function trackDocumentRequests(frame: Frame): Disposable> { - const requestMap = new Map(); - const dispose = () => { - frame._requestWatchers.delete(onRequest); - }; - const onRequest = (request: network.Request) => { - if (!request._documentId || request.redirectChain().length) - return; - requestMap.set(request._documentId, request); - }; - frame._requestWatchers.add(onRequest); - return {dispose, value: requestMap}; -} - function selectorToString(selector: string, visibility: types.Visibility): string { let label; switch (visibility) { diff --git a/test/navigation.spec.js b/test/navigation.spec.js index 356582c78d..3b8b8abfff 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -739,24 +739,20 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI it('should pick up ongoing navigation', async({page, server}) => { let response = null; server.setRoute('/one-style.css', (req, res) => response = res); - const navigationPromise = page.goto(server.PREFIX + '/one-style.html'); - await server.waitForRequest('/one-style.css'); + await Promise.all([ + server.waitForRequest('/one-style.css'), + page.goto(server.PREFIX + '/one-style.html', {waitUntil: []}), + ]); const waitPromise = page.waitForLoadState(); response.statusCode = 404; response.end('Not found'); await waitPromise; - await navigationPromise; }); it('should respect timeout', async({page, server}) => { - let response = null; server.setRoute('/one-style.css', (req, res) => response = res); - const navigationPromise = page.goto(server.PREFIX + '/one-style.html'); - await server.waitForRequest('/one-style.css'); + await page.goto(server.PREFIX + '/one-style.html', {waitUntil: []}); const error = await page.waitForLoadState({ timeout: 1 }).catch(e => e); expect(error.message).toBe('Navigation timeout of 1 ms exceeded'); - response.statusCode = 404; - response.end('Not found'); - await navigationPromise; }); it('should resolve immediately if loaded', async({page, server}) => { await page.goto(server.PREFIX + '/one-style.html'); @@ -764,14 +760,9 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI }); it('should resolve immediately if load state matches', async({page, server}) => { await page.goto(server.EMPTY_PAGE); - let response = null; server.setRoute('/one-style.css', (req, res) => response = res); - const navigationPromise = page.goto(server.PREFIX + '/one-style.html'); - await server.waitForRequest('/one-style.css'); + await page.goto(server.PREFIX + '/one-style.html', {waitUntil: []}); await page.waitForLoadState({ waitUntil: 'domcontentloaded' }); - response.statusCode = 404; - response.end('Not found'); - await navigationPromise; }); it.skip(FFOX)('should work with pages that have loaded before being connected to', async({page, context, server}) => { await page.goto(server.EMPTY_PAGE); @@ -844,6 +835,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI await page.$eval('iframe', frame => frame.remove()); const error = await navigationPromise; expect(error.message).toContain('frame was detached'); + expect(error.stack).toContain('Frame.goto') }); it('should return matching responses', async({page, server}) => { // Disable cache: otherwise, chromium will cache similar requests. @@ -904,6 +896,24 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI }); }); + describe('Frame.waitForLodState', function() { + it('should work', async({page, server}) => { + await page.goto(server.PREFIX + '/frames/one-frame.html'); + const frame = page.frames()[1]; + + const requestPromise = new Promise(resolve => page.route(server.PREFIX + '/one-style.css',resolve)); + await frame.goto(server.PREFIX + '/one-style.html', {waitUntil: 'domcontentloaded'}); + const request = await requestPromise; + let resolved = false; + const loadPromise = frame.waitForLoadState().then(() => resolved = true); + // give the promise a chance to resolve, even though it shouldn't + await page.evaluate('1'); + expect(resolved).toBe(false); + request.continue(); + await loadPromise; + }); + }); + describe('Page.reload', function() { it('should work', async({page, server}) => { await page.goto(server.EMPTY_PAGE);