From c1ef68337369674989f2bbefe712043a906f4d54 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 11 Mar 2020 14:46:52 -0700 Subject: [PATCH] api: remove waitForLoadState() in favor of PageEvent.page(options) (#1323) --- docs/api.md | 69 ++++++++++++----------------------- src/chromium/crBrowser.ts | 6 +-- src/firefox/ffBrowser.ts | 2 +- src/frames.ts | 8 ++-- src/page.ts | 47 +++++++++++++++++------- src/types.ts | 1 + src/webkit/wkBrowser.ts | 2 +- test/browsercontext.spec.js | 18 ++++----- test/elementhandle.spec.js | 2 +- test/navigation.spec.js | 16 ++++---- test/popup.spec.js | 73 +++++++++++++++++++++++++++++++++---- 11 files changed, 147 insertions(+), 97 deletions(-) diff --git a/docs/api.md b/docs/api.md index b072f13c1a..cf57a2a639 100644 --- a/docs/api.md +++ b/docs/api.md @@ -525,7 +525,6 @@ This setting will change the default maximum navigation time for the following m - [page.goto(url[, options])](#pagegotourl-options) - [page.reload([options])](#pagereloadoptions) - [page.setContent(html[, options])](#pagesetcontenthtml-options) -- [page.waitForLoadState([options])](#pagewaitforloadstateoptions) - [page.waitForNavigation([options])](#pagewaitfornavigationoptions) > **NOTE** [`page.setDefaultNavigationTimeout`](#pagesetdefaultnavigationtimeouttimeout) and [`page.setDefaultTimeout`](#pagesetdefaulttimeouttimeout) take priority over [`browserContext.setDefaultNavigationTimeout`](#browsercontextsetdefaultnavigationtimeouttimeout). @@ -717,7 +716,6 @@ page.removeListener('request', logRequest); - [page.waitFor(selectorOrFunctionOrTimeout[, options[, ...args]])](#pagewaitforselectororfunctionortimeout-options-args) - [page.waitForEvent(event[, optionsOrPredicate])](#pagewaitforeventevent-optionsorpredicate) - [page.waitForFunction(pageFunction[, options[, ...args]])](#pagewaitforfunctionpagefunction-options-args) -- [page.waitForLoadState([options])](#pagewaitforloadstateoptions) - [page.waitForNavigation([options])](#pagewaitfornavigationoptions) - [page.waitForRequest(urlOrPredicate[, options])](#pagewaitforrequesturlorpredicate-options) - [page.waitForResponse(urlOrPredicate[, options])](#pagewaitforresponseurlorpredicate-options) @@ -1567,7 +1565,6 @@ This setting will change the default maximum navigation time for the following m - [page.goto(url[, options])](#pagegotourl-options) - [page.reload([options])](#pagereloadoptions) - [page.setContent(html[, options])](#pagesetcontenthtml-options) -- [page.waitForLoadState([options])](#pagewaitforloadstateoptions) - [page.waitForNavigation([options])](#pagewaitfornavigationoptions) > **NOTE** [`page.setDefaultNavigationTimeout`](#pagesetdefaultnavigationtimeouttimeout) takes priority over [`page.setDefaultTimeout`](#pagesetdefaulttimeouttimeout), [`browserContext.setDefaultTimeout`](#browsercontextsetdefaulttimeouttimeout) and [`browserContext.setDefaultNavigationTimeout`](#browsercontextsetdefaultnavigationtimeouttimeout). @@ -1782,27 +1779,6 @@ await page.waitForFunction(selector => !!document.querySelector(selector), {}, s Shortcut for [page.mainFrame().waitForFunction(pageFunction[, options[, ...args]])](#framewaitforfunctionpagefunction-options-args). -#### page.waitForLoadState([options]) -- `options` <[Object]> Navigation parameters which might have the following properties: - - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. - - `waitUntil` <"commit"|"load"|"domcontentloaded"|"networkidle0"|"networkidle2"> When to consider navigation succeeded, defaults to `load`. Events can be either: - - `'commit'` - navigation is committed, new url is displayed in the browser address bar. - - `'load'` - consider navigation to be finished when the `load` event is fired. - - `'domcontentloaded'` - consider navigation to be finished when the `DOMContentLoaded` event is fired. - - `'networkidle0'` - consider navigation to be finished when there are no more than 0 network connections for at least `500` ms. - - `'networkidle2'` - consider navigation to be finished when there are no more than 2 network connections for at least `500` ms. -- returns: <[Promise]> Promise which resolves when the load state has been achieved. - -This resolves when the page reaches a required load state, `load` by default. The navigation can be in progress when it is called. -If navigation is already at a required state, resolves immediately. - -```js -await page.click('button'); // Click triggers navigation. -await page.waitForLoadState(); // The promise resolves after navigation has finished. -``` - -Shortcut for [page.mainFrame().waitForLoadState([options])](#framewaitforloadstateoptions). - #### page.waitForNavigation([options]) - `options` <[Object]> Navigation parameters which might have the following properties: - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. @@ -1895,8 +1871,29 @@ This method returns all of the dedicated [WebWorkers](https://developer.mozilla. Event object passed to the listeners of [`browserContext.on('page')`](#event-page) and [`page.on('popup')`](#event-popup) events. Provides access to the newly created page. -#### pageEvent.page() -- returns: <[Promise]<[Page]>> Promise which resolves to the created page. +#### pageEvent.page([options]) +- `options` <[Object]> + - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. + - `waitUntil` <"commit"|"load"|"domcontentloaded"|"networkidle0"|"networkidle2"> When to consider the page to be loaded, defaults to `load`. Events can be either: + - `'commit'` - navigation is committed, new url is displayed in the browser address bar. + - `'load'` - consider navigation to be finished when the `load` event is fired. + - `'domcontentloaded'` - consider navigation to be finished when the `DOMContentLoaded` event is fired. + - `'networkidle0'` - consider navigation to be finished when there are no more than 0 network connections for at least `500` ms. + - `'networkidle2'` - consider navigation to be finished when there are no more than 2 network connections for at least `500` ms. +- returns: <[Promise]<[Page]>> Promise which resolves to the created page once it loads, according to `waitUntil` option. + +This resolves when the page reaches a required load state, `load` by default. The earliest moment that page is available is when it has committed navigation to the initial url (corresponds to `{waitUntil: 'commit'}` option). For example, when opening a popup with `window.open('http://example.com')`, this method will wait until the network request to "http://example.com" is done and its response has started loading in the popup. Passing different `waitUntil` options will also wait for the particular load state to happen, e.g. default `load` waits until the load event fires. + +```js +const [, popup] = await Promise.all([ + // Click opens a popup window. + page.click('button'), + // Resolves after popup has fired 'DOMContentLoaded' event. + page.waitForEvent('popup').then(event => event.page({ waitUntil: 'domcontentloaded' })), +]); +``` + +> **NOTE** Some pages will never fire `load` event. In this case, default `pageEvent.page()` without options will timeout - try using `pageEvent.page({ waitUntil: 'domcontentloaded' })` instead. ### class: Frame @@ -1968,7 +1965,6 @@ An example of getting text from an iframe element: - [frame.url()](#frameurl) - [frame.waitFor(selectorOrFunctionOrTimeout[, options[, ...args]])](#framewaitforselectororfunctionortimeout-options-args) - [frame.waitForFunction(pageFunction[, options[, ...args]])](#framewaitforfunctionpagefunction-options-args) -- [frame.waitForLoadState([options])](#framewaitforloadstateoptions) - [frame.waitForNavigation([options])](#framewaitfornavigationoptions) - [frame.waitForSelector(selector[, options])](#framewaitforselectorselector-options) @@ -2498,25 +2494,6 @@ const selector = '.foo'; await page.waitForFunction(selector => !!document.querySelector(selector), {}, selector); ``` -#### frame.waitForLoadState([options]) -- `options` <[Object]> Navigation parameters which might have the following properties: - - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. - - `waitUntil` <"commit"|"load"|"domcontentloaded"|"networkidle0"|"networkidle2"> When to consider navigation succeeded, defaults to `load`. Events can be either: - - `'commit'` - navigation is committed, new url is displayed in the browser address bar. - - `'load'` - consider navigation to be finished when the `load` event is fired. - - `'domcontentloaded'` - consider navigation to be finished when the `DOMContentLoaded` event is fired. - - `'networkidle0'` - consider navigation to be finished when there are no more than 0 network connections for at least `500` ms. - - `'networkidle2'` - consider navigation to be finished when there are no more than 2 network connections for at least `500` ms. -- returns: <[Promise]> Promise which resolves when the load state has been achieved. - -This resolves when the page reaches a required load state, `load` by default. The navigation can be in progress when it is called. -If navigation is already at a required state, resolves immediately. - -```js -await frame.click('button'); // Click triggers navigation. -await frame.waitForLoadState(); // The promise resolves after navigation has finished. -``` - #### frame.waitForNavigation([options]) - `options` <[Object]> Navigation parameters which might have the following properties: - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index bec8375e7d..8d7b1ab955 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -118,18 +118,18 @@ export class CRBrowser extends platform.EventEmitter implements Browser { try { switch (targetInfo.type) { case 'page': { - const event = new PageEvent(target.pageOrError()); + const event = new PageEvent(context, target.pageOrError()); context.emit(CommonEvents.BrowserContext.Page, event); const opener = target.opener(); if (!opener) break; const openerPage = await opener.pageOrError(); if (openerPage instanceof Page && !openerPage.isClosed()) - openerPage.emit(CommonEvents.Page.Popup, new PageEvent(target.pageOrError())); + openerPage.emit(CommonEvents.Page.Popup, new PageEvent(context, target.pageOrError())); break; } case 'background_page': { - const event = new PageEvent(target.pageOrError()); + const event = new PageEvent(context, target.pageOrError()); context.emit(Events.CRBrowserContext.BackgroundPage, event); break; } diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 5b90207f20..114c64fe47 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -128,7 +128,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser { const ffPage = new FFPage(session, context, opener); this._ffPages.set(targetId, ffPage); - const pageEvent = new PageEvent(ffPage.pageOrError()); + const pageEvent = new PageEvent(context, ffPage.pageOrError()); context.emit(Events.BrowserContext.Page, pageEvent); ffPage.pageOrError().then(() => this._firstPageCallback()); diff --git a/src/frames.ts b/src/frames.ts index facca80757..36894abe5f 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -41,8 +41,6 @@ export type GotoResult = { newDocumentId?: string, }; -const kLifecycleEvents: Set = new Set(['commit', 'load', 'domcontentloaded', 'networkidle0', 'networkidle2']); - type ConsoleTagHandler = () => void; export class FrameManager { @@ -437,7 +435,7 @@ export class Frame { return request ? request._finalRequest._waitForResponse() : null; } - async waitForLoadState(options: types.NavigateOptions = {}): Promise { + async _waitForLoadState(options: types.NavigateOptions = {}): Promise { const {timeout = this._page._timeoutSettings.navigationTimeout()} = options; const disposer = new Disposer(); const error = await Promise.race([ @@ -491,7 +489,7 @@ export class Frame { _waitForLifecycle(waitUntil: types.LifecycleEvent = 'load'): Disposable> { let resolve: () => void; - if (!kLifecycleEvents.has(waitUntil)) + if (!types.kLifecycleEvents.has(waitUntil)) throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`); const checkLifecycleComplete = () => { @@ -643,7 +641,7 @@ export class Frame { this._page._frameManager._consoleMessageTags.set(tag, () => { // Clear lifecycle right after document.open() - see 'tag' below. this._page._frameManager.clearFrameLifecycle(this); - resolve(this.waitForLoadState(options)); + resolve(this._waitForLoadState(options)); }); }); const contentPromise = context.evaluate((html, tag) => { diff --git a/src/page.ts b/src/page.ts index a66bb65a65..3e8eade749 100644 --- a/src/page.ts +++ b/src/page.ts @@ -88,20 +88,45 @@ export type FileChooser = { }; export class PageEvent { + private readonly _browserContext: BrowserContextBase; private readonly _pageOrError: Promise; + private readonly _lifecyclePromises = new Map>(); - constructor(pageOrErrorPromise: Promise) { + constructor(browserContext: BrowserContextBase, pageOrErrorPromise: Promise) { + this._browserContext = browserContext; this._pageOrError = pageOrErrorPromise; + for (const lifecycle of types.kLifecycleEvents) + this._lifecyclePromises.set(lifecycle, this._createLifecyclePromise(lifecycle)); } - async page(/* options?: frames.NavigateOptions */): Promise { - const result = await this._pageOrError; - if (result instanceof Page) { - if (result.isClosed()) - throw new Error('Page has been closed.'); - return result; - } - throw result; + async _createLifecyclePromise(lifecycle: types.LifecycleEvent): Promise { + const page = await this._pageOrError; + if (!(page instanceof Page)) + return page; + + const { dispose, value: waitForLifecycle } = page.mainFrame()._waitForLifecycle(lifecycle); + const error = await Promise.race([ + page.mainFrame()._createFrameDestroyedPromise(), + waitForLifecycle, + ]); + dispose(); + if (error) + return error; + return page; + } + + async page(options: types.NavigateOptions = {}): Promise { + const { + timeout = this._browserContext._timeoutSettings.navigationTimeout(), + waitUntil = 'load', + } = options; + const lifecyclePromise = this._lifecyclePromises.get(waitUntil); + if (!lifecyclePromise) + throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`); + const pageOrError = await helper.waitWithTimeout(lifecyclePromise, `"${waitUntil}"`, timeout); + if (pageOrError instanceof Page) + return pageOrError; + throw pageOrError; } } @@ -311,10 +336,6 @@ export class Page extends platform.EventEmitter { return this.mainFrame().waitForNavigation(options); } - async waitForLoadState(options?: types.NavigateOptions): Promise { - return this.mainFrame().waitForLoadState(options); - } - async waitForEvent(event: string, optionsOrPredicate: Function | (types.TimeoutOptions & { predicate?: Function }) = {}): Promise { if (typeof optionsOrPredicate === 'function') optionsOrPredicate = { predicate: optionsOrPredicate }; diff --git a/src/types.ts b/src/types.ts index fbd5e3b4a6..235c2418ff 100644 --- a/src/types.ts +++ b/src/types.ts @@ -46,6 +46,7 @@ export type Polling = 'raf' | 'mutation' | number; export type WaitForFunctionOptions = TimeoutOptions & { polling?: Polling }; export type LifecycleEvent = 'commit' | 'load' | 'domcontentloaded' | 'networkidle0' | 'networkidle2'; +export const kLifecycleEvents: Set = new Set(['commit', 'load', 'domcontentloaded', 'networkidle0', 'networkidle2']); export type NavigateOptions = TimeoutOptions & { waitUntil?: LifecycleEvent, diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index b84ab98b18..f32307ad39 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -124,7 +124,7 @@ export class WKBrowser extends platform.EventEmitter implements Browser { this._firstPageCallback = undefined; } - const pageEvent = new PageEvent(wkPage.pageOrError()); + const pageEvent = new PageEvent(context, wkPage.pageOrError()); context.emit(Events.BrowserContext.Page, pageEvent); if (!opener) return; diff --git a/test/browsercontext.spec.js b/test/browsercontext.spec.js index 019f965b45..af5bf09778 100644 --- a/test/browsercontext.spec.js +++ b/test/browsercontext.spec.js @@ -486,7 +486,6 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF context.waitForEvent('page').then(event => event.page()), page.evaluate(url => window.open(url), server.CROSS_PROCESS_PREFIX + '/empty.html').catch(e => console.log('eee = ' + e)), ]); - await otherPage.waitForLoadState(); expect(otherPage.url()).toContain(server.CROSS_PROCESS_PREFIX); expect(await otherPage.evaluate(() => ['Hello', 'world'].join(' '))).toBe('Hello world'); expect(await otherPage.$('body')).toBeTruthy(); @@ -507,15 +506,14 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF }); it('should report initialized pages', async({browser, server}) => { const context = await browser.newContext(); - const pagePromise = new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))); + const pagePromise = context.waitForEvent('page').then(e => e.page({ waitUntil: 'commit' })); context.newPage(); const newPage = await pagePromise; expect(newPage.url()).toBe('about:blank'); - const popupPromise = new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))); + const popupPromise = context.waitForEvent('page').then(e => e.page()); const evaluatePromise = newPage.evaluate(() => window.open('about:blank')); const popup = await popupPromise; - await popup.waitForLoadState(); expect(popup.url()).toBe('about:blank'); await evaluatePromise; await context.close(); @@ -526,16 +524,15 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF let serverResponse = null; server.setRoute('/one-style.css', (req, res) => serverResponse = res); // Open a new page. Use window.open to connect to the page later. - const [newPage] = await Promise.all([ - new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))), + const [newPageEvent] = await Promise.all([ + context.waitForEvent('page'), page.evaluate(url => window.open(url), server.PREFIX + '/one-style.html'), server.waitForRequest('/one-style.css') ]); // Issue a redirect. serverResponse.writeHead(302, { location: '/injectedstyle.css' }); serverResponse.end(); - // Wait for the new page to load. - await newPage.waitForLoadState(); + const newPage = await newPageEvent.page(); // Connect to the opened page. expect(newPage.url()).toBe(server.PREFIX + '/one-style.html'); // Cleanup. @@ -546,10 +543,9 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF const page = await context.newPage(); await page.goto(server.EMPTY_PAGE); const [popup] = await Promise.all([ - new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))), + context.waitForEvent('page').then(e => e.page()), page.goto(server.PREFIX + '/popup/window-open.html') ]); - await popup.waitForLoadState(); expect(popup.url()).toBe(server.PREFIX + '/popup/popup.html'); expect(await popup.opener()).toBe(page); expect(await page.opener()).toBe(null); @@ -559,7 +555,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF const context = await browser.newContext(); const events = []; context.on('page', async event => { - const page = await event.page(); + const page = await event.page({ waitUntil: 'commit' }); events.push('CREATED: ' + page.url()); page.on('close', () => events.push('DESTROYED: ' + page.url())); }); diff --git a/test/elementhandle.spec.js b/test/elementhandle.spec.js index 61472e4bca..9c9a432d4f 100644 --- a/test/elementhandle.spec.js +++ b/test/elementhandle.spec.js @@ -209,7 +209,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT}) it('should work for adopted elements', async({page,server}) => { await page.goto(server.EMPTY_PAGE); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(async e => { const popup = await e.page(); await popup.waitForLoadState(); return popup; }), + page.waitForEvent('popup').then(e => e.page()), page.evaluate(url => window.__popup = window.open(url), server.EMPTY_PAGE), ]); const divHandle = await page.evaluateHandle(() => { diff --git a/test/navigation.spec.js b/test/navigation.spec.js index f71c641dcd..6943b70006 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -969,7 +969,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF }); }); - describe('Page.waitForLoadState', () => { + describe('Page._waitForLoadState', () => { it('should pick up ongoing navigation', async({page, server}) => { let response = null; server.setRoute('/one-style.css', (req, res) => response = res); @@ -977,7 +977,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF server.waitForRequest('/one-style.css'), page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'commit'}), ]); - const waitPromise = page.waitForLoadState(); + const waitPromise = page.mainFrame()._waitForLoadState(); response.statusCode = 404; response.end('Not found'); await waitPromise; @@ -985,18 +985,18 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF it('should respect timeout', async({page, server}) => { server.setRoute('/one-style.css', (req, res) => response = res); await page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'commit'}); - const error = await page.waitForLoadState({ timeout: 1 }).catch(e => e); + const error = await page.mainFrame()._waitForLoadState({ timeout: 1 }).catch(e => e); expect(error.message).toBe('Navigation timeout of 1 ms exceeded'); }); it('should resolve immediately if loaded', async({page, server}) => { await page.goto(server.PREFIX + '/one-style.html'); - await page.waitForLoadState(); + await page.mainFrame()._waitForLoadState(); }); it('should resolve immediately if load state matches', async({page, server}) => { await page.goto(server.EMPTY_PAGE); server.setRoute('/one-style.css', (req, res) => response = res); await page.goto(server.PREFIX + '/one-style.html', {waitUntil: 'commit'}); - await page.waitForLoadState({ waitUntil: 'domcontentloaded' }); + await page.mainFrame()._waitForLoadState({ waitUntil: 'domcontentloaded' }); }); it('should work with pages that have loaded before being connected to', async({page, context, server}) => { await page.goto(server.EMPTY_PAGE); @@ -1011,7 +1011,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF expect(pages[0].url()).toBe(server.EMPTY_PAGE); expect(pages[1].url()).toBe(server.EMPTY_PAGE); - await pages[1].waitForLoadState(); + await pages[1].mainFrame()._waitForLoadState(); expect(pages[1].url()).toBe(server.EMPTY_PAGE); }); }); @@ -1126,7 +1126,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF }); }); - describe('Frame.waitForLodState', function() { + describe('Frame._waitForLodState', function() { it('should work', async({page, server}) => { await page.goto(server.PREFIX + '/frames/one-frame.html'); const frame = page.frames()[1]; @@ -1135,7 +1135,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF await frame.goto(server.PREFIX + '/one-style.html', {waitUntil: 'domcontentloaded'}); const request = await requestPromise; let resolved = false; - const loadPromise = frame.waitForLoadState().then(() => resolved = true); + 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); diff --git a/test/popup.spec.js b/test/popup.spec.js index 1bc21d406f..ffad027f8a 100644 --- a/test/popup.spec.js +++ b/test/popup.spec.js @@ -32,7 +32,6 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE context.waitForEvent('page').then(pageEvent => pageEvent.page()), page.click('a'), ]); - await popup.waitForLoadState(); const userAgent = await popup.evaluate(() => window.initialUserAgent); const request = await requestPromise; await context.close(); @@ -53,7 +52,6 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE context.waitForEvent('page').then(pageEvent => pageEvent.page()), page.click('a'), ]); - await popup.waitForLoadState(); await context.close(); expect(intercepted).toBe(true); }); @@ -108,7 +106,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE const page = await context.newPage(); await page.goto(server.EMPTY_PAGE); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(async e => { const popup = await e.page(); await popup.waitForLoadState(); return popup; }), + page.waitForEvent('popup').then(e => e.page()), page.evaluate(url => window._popup = window.open(url), server.PREFIX + '/title.html'), ]); expect(await popup.title()).toBe('Woof-Woof'); @@ -204,6 +202,65 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE expect(popupEvent).toBeTruthy(); await context.close(); }); + it('should resolve page() after initial navigation', async({browser, server}) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(server.EMPTY_PAGE); + // First popup navigation is about:blank. + const [popupEvent] = await Promise.all([ + page.waitForEvent('popup'), + page.evaluate(() => window.popup = window.open('about:blank')), + ]); + // Stall the 'load' for second navigation. + server.setRoute('/one-style.css', (req, res) => {}); + await Promise.all([ + server.waitForRequest('/one-style.css'), + page.evaluate(url => window.popup.location.href = url, server.PREFIX + '/one-style.html'), + ]); + // Second navigation should be committed, but page() should resolve because first navigation is done. + const popup = await popupEvent.page(); + expect(popup).toBeTruthy(); + await context.close(); + }); + it('should resolve page() after load', async({browser, server}) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(server.EMPTY_PAGE); + // Stall the 'load' by delaying css. + let cssResponse; + server.setRoute('/one-style.css', (req, res) => cssResponse = res); + const [popupEvent] = await Promise.all([ + page.waitForEvent('popup'), + server.waitForRequest('/one-style.css'), + page.evaluate(url => window.popup = window.open(url), server.PREFIX + '/one-style.html'), + ]); + let resolved = false; + const popupPromise = popupEvent.page().then(page => { resolved = true; return page; }); + expect(resolved).toBe(false); + // Round trips! + for (let i = 0; i < 5; i++) + await page.evaluate('window'); + expect(resolved).toBe(false); + cssResponse.end(''); + const popup = await popupPromise; + expect(resolved).toBe(true); + expect(popup.url()).toBe(server.PREFIX + '/one-style.html'); + await context.close(); + }); + it('should respect timeout in page()', async({browser, server}) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(server.EMPTY_PAGE); + // Stall the 'load' by delaying css. + server.setRoute('/one-style.css', (req, res) => {}); + const [popupEvent] = await Promise.all([ + page.waitForEvent('popup'), + page.evaluate(url => window.popup = window.open(url), server.PREFIX + '/one-style.html'), + ]); + const error = await popupEvent.page({ timeout: 1 }).catch(e => e); + expect(error.message).toBe('waiting for "load" failed: timeout 1ms exceeded'); + await context.close(); + }); it.fail(FFOX)('should be able to capture alert', async({browser}) => { // Firefox: // - immediately closes dialog by itself, without protocol call; @@ -226,7 +283,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE const context = await browser.newContext(); const page = await context.newPage(); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(e => e.page()), + page.waitForEvent('popup').then(e => e.page({ waitUntil: 'commit' })), page.evaluate(() => window.__popup = window.open('')), ]); expect(await page.evaluate(() => !!window.opener)).toBe(false); @@ -250,7 +307,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE await page.goto(server.EMPTY_PAGE); await page.setContent('yo'); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(async e => { const popup = await e.page(); await popup.waitForLoadState(); return popup; }), + page.waitForEvent('popup').then(e => e.page()), page.click('a'), ]); expect(await page.evaluate(() => !!window.opener)).toBe(false); @@ -264,7 +321,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE await page.goto(server.EMPTY_PAGE); await page.setContent('yo'); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(async e => { const popup = await e.page(); await popup.waitForLoadState(); return popup; }), + page.waitForEvent('popup').then(e => e.page()), page.$eval('a', a => a.click()), ]); expect(await page.evaluate(() => !!window.opener)).toBe(false); @@ -279,7 +336,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE await page.goto(server.EMPTY_PAGE); await page.setContent('yo'); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(async e => { const popup = await e.page(); await popup.waitForLoadState(); return popup; }), + page.waitForEvent('popup').then(e => e.page()), page.click('a'), ]); expect(await page.evaluate(() => !!window.opener)).toBe(false); @@ -292,7 +349,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE await page.goto(server.EMPTY_PAGE); await page.setContent('yo'); const [popup] = await Promise.all([ - page.waitForEvent('popup').then(async e => { const popup = await e.page(); await popup.waitForLoadState(); return popup; }), + page.waitForEvent('popup').then(e => e.page()), page.click('a'), ]); let badSecondPopup = false;