From 9b86c631c8921c6c328dc5541537f856d950103b Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 13 Mar 2020 11:33:33 -0700 Subject: [PATCH] api: make BrowserContext.pages() synchronous (#1369) Returns all pages which have been initialized already. References #1348. --- docs/api.md | 10 ++-- src/browserContext.ts | 11 ++--- src/chromium/crBrowser.ts | 85 ++++++++++++++-------------------- src/chromium/crPage.ts | 2 + src/chromium/crTarget.ts | 6 +++ src/dom.ts | 3 +- src/firefox/ffBrowser.ts | 31 ++++++------- src/server/chromium.ts | 11 +---- src/webkit/wkBrowser.ts | 39 ++++++---------- test/browsercontext.spec.js | 26 +++++------ test/chromium/launcher.spec.js | 6 +-- test/headful.spec.js | 4 +- test/launcher.spec.js | 8 ++-- test/navigation.spec.js | 20 +++----- test/page.spec.js | 35 +++++++------- 15 files changed, 129 insertions(+), 168 deletions(-) diff --git a/docs/api.md b/docs/api.md index 39224fb3c4f89..943e5660d9262 100644 --- a/docs/api.md +++ b/docs/api.md @@ -452,10 +452,7 @@ const crypto = require('crypto'); Creates a new page in the browser context. #### browserContext.pages() -- returns: <[Promise]<[Array]<[Page]>>> Promise which resolves to an array of all open pages. Non visible pages, such as `"background_page"`, will not be listed here. You can find them using -[chromiumBrowserContext.backgroundPages()](#chromiumbrowsercontextbackgroundpages). - -An array of all pages inside the browser context. +- returns: <[Array]<[Page]>> All open pages in the context. Non visible pages, such as `"background_page"`, will not be listed here. You can find them using [chromiumBrowserContext.backgroundPages()](#chromiumbrowsercontextbackgroundpages). #### browserContext.route(url, handler) - `url` <[string]|[RegExp]|[function]\([string]\):[boolean]> A glob pattern, regex pattern or predicate receiving [URL] to match while routing. @@ -3804,7 +3801,7 @@ Emitted when new background page is created in the context. Emitted when new service worker is created in the context. #### chromiumBrowserContext.backgroundPages() -- returns: <[Promise]<[Array]<[Page]>>> Promise which resolves to an array of all existing background pages in the context. +- returns: <[Array]<[Page]>> All existing background pages in the context. #### chromiumBrowserContext.newCDPSession(page) - `page` <[Page]> Page to create new session for. @@ -4011,8 +4008,7 @@ const { chromium } = require('playwright'); `--load-extension=${pathToExtension}` ] }); - const backgroundPages = await browserContext.backgroundPages(); - const backgroundPage = backgroundPages[0]; + const backgroundPage = browserContext.backgroundPages()[0]; // Test the background page as you would any other page. await browserContext.close(); })(); diff --git a/src/browserContext.ts b/src/browserContext.ts index 005bbf7c890a7..6b690d6280f63 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -41,7 +41,7 @@ export type BrowserContextOptions = { export interface BrowserContext { setDefaultNavigationTimeout(timeout: number): void; setDefaultTimeout(timeout: number): void; - pages(): Promise; + pages(): Page[]; newPage(): Promise; cookies(urls?: string | string[]): Promise; addCookies(cookies: network.SetNetworkCookieParam[]): Promise; @@ -74,10 +74,8 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement this._closePromise = new Promise(fulfill => this._closePromiseFulfill = fulfill); } - abstract _existingPages(): Page[]; - _browserClosed() { - for (const page of this._existingPages()) + for (const page of this.pages()) page._didClose(); this._didCloseInternal(); } @@ -89,7 +87,7 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement } // BrowserContext methods. - abstract pages(): Promise; + abstract pages(): Page[]; abstract newPage(): Promise; abstract cookies(...urls: string[]): Promise; abstract addCookies(cookies: network.SetNetworkCookieParam[]): Promise; @@ -126,8 +124,7 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement } export function assertBrowserContextIsNotOwned(context: BrowserContextBase) { - const pages = context._existingPages(); - for (const page of pages) { + for (const page of context.pages()) { if (page._ownedContext) throw new Error('Please use browser.newContext() for multi-page scripts that share the context.'); } diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index b63884b9e9fb5..d764d705083f0 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -38,6 +38,8 @@ export class CRBrowser extends platform.EventEmitter implements Browser { readonly _defaultContext: CRBrowserContext; readonly _contexts = new Map(); _targets = new Map(); + readonly _firstPagePromise: Promise; + private _firstPageCallback = () => {}; private _tracingRecording = false; private _tracingPath: string | null = ''; @@ -83,6 +85,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser { this._session.on('Target.targetDestroyed', this._targetDestroyed.bind(this)); this._session.on('Target.targetInfoChanged', this._targetInfoChanged.bind(this)); this._session.on('Target.attachedToTarget', this._onAttachedToTarget.bind(this)); + this._firstPagePromise = new Promise(f => this._firstPageCallback = f); } async newContext(options: BrowserContextOptions = {}): Promise { @@ -102,7 +105,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser { return createPageInNewContext(this, options); } - async _onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) { + _onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) { const session = this._connection.session(sessionId)!; if (!CRTarget.isPageType(targetInfo.type)) { assert(targetInfo.type === 'service_worker' || targetInfo.type === 'browser' || targetInfo.type === 'other'); @@ -115,29 +118,24 @@ export class CRBrowser extends platform.EventEmitter implements Browser { return; } const { context, target } = this._createTarget(targetInfo, session); - try { - switch (targetInfo.type) { - case 'page': { - 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(context, target.pageOrError())); - break; - } - case 'background_page': { - const event = new PageEvent(context, target.pageOrError()); - context.emit(Events.CRBrowserContext.BackgroundPage, event); - break; - } + + if (!CRTarget.isPageType(targetInfo.type)) + return; + const pageEvent = new PageEvent(context, target.pageOrError()); + target.pageOrError().then(async () => { + if (targetInfo.type === 'page') { + this._firstPageCallback(); + context.emit(CommonEvents.BrowserContext.Page, pageEvent); + const opener = target.opener(); + if (!opener) + return; + const openerPage = await opener.pageOrError(); + if (openerPage instanceof Page && !openerPage.isClosed()) + openerPage.emit(CommonEvents.Page.Popup, pageEvent); + } else if (targetInfo.type === 'background_page') { + context.emit(Events.CRBrowserContext.BackgroundPage, pageEvent); } - } catch (e) { - // Do not dispatch the event if initialization failed. - debugError(e); - } + }); } async _targetCreated({targetInfo}: Protocol.Target.targetCreatedPayload) { @@ -176,10 +174,6 @@ export class CRBrowser extends platform.EventEmitter implements Browser { await this._session.send('Target.closeTarget', { targetId: CRTarget.fromPage(page)._targetId }); } - _allTargets(): CRTarget[] { - return Array.from(this._targets.values()); - } - async close() { const disconnected = new Promise(f => this._connection.once(ConnectionEvents.Disconnected, f)); await Promise.all(this.contexts().map(context => context.close())); @@ -268,19 +262,12 @@ export class CRBrowserContext extends BrowserContextBase { await this.setHTTPCredentials(this._options.httpCredentials); } - _existingPages(): Page[] { - const pages: Page[] = []; - for (const target of this._browser._allTargets()) { - if (target.context() === this && target._crPage) - pages.push(target._crPage.page()); - } - return pages; + _targets(): CRTarget[] { + return Array.from(this._browser._targets.values()).filter(target => target.context() === this); } - async pages(): Promise { - const targets = this._browser._allTargets().filter(target => target.context() === this && target.type() === 'page'); - const pages = await Promise.all(targets.map(target => target.pageOrError())); - return pages.filter(page => (page instanceof Page) && !page.isClosed()) as Page[]; + pages(): Page[] { + return this._targets().filter(target => target.type() === 'page').map(target => target._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; } async newPage(): Promise { @@ -351,37 +338,37 @@ export class CRBrowserContext extends BrowserContextBase { if (geolocation) geolocation = verifyGeolocation(geolocation); this._options.geolocation = geolocation || undefined; - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage)._client.send('Emulation.setGeolocationOverride', geolocation || {}); } async setExtraHTTPHeaders(headers: network.Headers): Promise { this._options.extraHTTPHeaders = network.verifyHeaders(headers); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage).updateExtraHTTPHeaders(); } async setOffline(offline: boolean): Promise { this._options.offline = offline; - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage)._networkManager.setOffline(offline); } async setHTTPCredentials(httpCredentials: types.Credentials | null): Promise { this._options.httpCredentials = httpCredentials || undefined; - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage)._networkManager.authenticate(httpCredentials); } async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) { const source = await helper.evaluationScript(script, args); this._evaluateOnNewDocumentSources.push(source); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage).evaluateOnNewDocument(source); } async exposeFunction(name: string, playwrightFunction: Function): Promise { - for (const page of this._existingPages()) { + for (const page of this.pages()) { if (page._pageBindings.has(name)) throw new Error(`Function "${name}" has been already registered in one of the pages`); } @@ -389,13 +376,13 @@ export class CRBrowserContext extends BrowserContextBase { throw new Error(`Function "${name}" has been already registered`); const binding = new PageBinding(name, playwrightFunction); this._pageBindings.set(name, binding); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage).exposeBinding(binding); } async route(url: types.URLMatch, handler: network.RouteHandler): Promise { this._routes.push({ url, handler }); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as CRPage).updateRequestInterception(); } @@ -413,10 +400,8 @@ export class CRBrowserContext extends BrowserContextBase { this._didCloseInternal(); } - async backgroundPages(): Promise { - const targets = this._browser._allTargets().filter(target => target.context() === this && target.type() === 'background_page'); - const pages = await Promise.all(targets.map(target => target.pageOrError())); - return pages.filter(page => (page instanceof Page) && !page.isClosed()) as Page[]; + backgroundPages(): Page[] { + return this._targets().filter(target => target.type() === 'background_page').map(target => target._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; } async newCDPSession(page: Page): Promise { diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index b342eaf44f585..d91689adaf909 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -43,6 +43,7 @@ const UTILITY_WORLD_NAME = '__playwright_utility_world__'; export class CRPage implements PageDelegate { _client: CRSession; + _initialized = false; private readonly _page: Page; readonly _networkManager: CRNetworkManager; private _contextIdToContext = new Map(); @@ -143,6 +144,7 @@ export class CRPage implements PageDelegate { promises.push(this.evaluateOnNewDocument(source)); promises.push(this._client.send('Runtime.runIfWaitingForDebugger')); await Promise.all(promises); + this._initialized = true; } didClose() { diff --git a/src/chromium/crTarget.ts b/src/chromium/crTarget.ts index 5dd964cacc92c..594c7b7b5d9eb 100644 --- a/src/chromium/crTarget.ts +++ b/src/chromium/crTarget.ts @@ -70,6 +70,12 @@ export class CRTarget { this._crPage.didClose(); } + _initializedPage(): Page | null { + if (this._crPage && this._crPage._initialized) + return this._crPage.page(); + return null; + } + async pageOrError(): Promise { if (CRTarget.isPageType(this.type())) return this._pagePromise!; diff --git a/src/dom.ts b/src/dom.ts index c5d9bec08e4d4..bb818416f62b4 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -160,8 +160,7 @@ export class ElementHandle extends js.JSHandle { const frameId = await this._page._delegate.getOwnerFrame(this); if (!frameId) return null; - const pages = this._page._browserContext._existingPages(); - for (const page of pages) { + for (const page of this._page._browserContext.pages()) { const frame = page._frameManager.frame(frameId); if (frame) return frame; diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 4e4583a4ebe9f..f51563cc0b16a 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -119,7 +119,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser { ffPage.didClose(); } - async _onAttachedToTarget(payload: Protocol.Browser.attachedToTargetPayload) { + _onAttachedToTarget(payload: Protocol.Browser.attachedToTargetPayload) { const {targetId, browserContextId, openerId, type} = payload.targetInfo; assert(type === 'page'); const context = browserContextId ? this._contexts.get(browserContextId)! : this._defaultContext; @@ -129,15 +129,15 @@ export class FFBrowser extends platform.EventEmitter implements Browser { this._ffPages.set(targetId, ffPage); const pageEvent = new PageEvent(context, ffPage.pageOrError()); - context.emit(Events.BrowserContext.Page, pageEvent); - - ffPage.pageOrError().then(() => this._firstPageCallback()); - - if (!opener) - return; - const openerPage = await opener.pageOrError(); - if (openerPage instanceof Page && !openerPage.isClosed()) - openerPage.emit(Events.Page.Popup, pageEvent); + ffPage.pageOrError().then(async () => { + this._firstPageCallback(); + context.emit(Events.BrowserContext.Page, pageEvent); + if (!opener) + return; + const openerPage = await opener.pageOrError(); + if (openerPage instanceof Page && !openerPage.isClosed()) + openerPage.emit(Events.Page.Popup, pageEvent); + }); } async close() { @@ -182,10 +182,6 @@ export class FFBrowserContext extends BrowserContextBase { return Array.from(this._browser._ffPages.values()).filter(ffPage => ffPage._browserContext === this); } - _existingPages(): Page[] { - return this._ffPages().map(ffPage => ffPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; - } - setDefaultNavigationTimeout(timeout: number) { this._timeoutSettings.setDefaultNavigationTimeout(timeout); } @@ -194,9 +190,8 @@ export class FFBrowserContext extends BrowserContextBase { this._timeoutSettings.setDefaultTimeout(timeout); } - async pages(): Promise { - const pagesOrErrors = await Promise.all(this._ffPages().map(ffPage => ffPage.pageOrError())); - return pagesOrErrors.filter(pageOrError => pageOrError instanceof Page && !pageOrError.isClosed()) as Page[]; + pages(): Page[] { + return this._ffPages().map(ffPage => ffPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; } async newPage(): Promise { @@ -279,7 +274,7 @@ export class FFBrowserContext extends BrowserContextBase { } async exposeFunction(name: string, playwrightFunction: Function): Promise { - for (const page of this._existingPages()) { + for (const page of this.pages()) { if (page._pageBindings.has(name)) throw new Error(`Function "${name}" has been already registered in one of the pages`); } diff --git a/src/server/chromium.ts b/src/server/chromium.ts index ca0cee06da366..2cb977334d2b6 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -66,15 +66,8 @@ export class Chromium implements BrowserType { const { timeout = 30000 } = options || {}; const { transport } = await this._launchServer(options, 'persistent', userDataDir); const browser = await CRBrowser.connect(transport!, true); - const browserContext = browser._defaultContext; - - function targets() { - return browser._allTargets().filter(target => target.context() === browserContext && target.type() === 'page'); - } - const firstTarget = targets().length ? Promise.resolve() : new Promise(f => browserContext.once('page', f)); - const firstPage = firstTarget.then(() => targets()[0].pageOrError()); - await helper.waitWithTimeout(firstPage, 'first page', timeout); - return browserContext; + await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout); + return browser._defaultContext; } private async _launchServer(options: LaunchOptions = {}, launchType: LaunchType, userDataDir?: string, port?: number): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport }> { diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 24182a7cca8aa..8569066570358 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -39,7 +39,7 @@ export class WKBrowser extends platform.EventEmitter implements Browser { readonly _wkPages = new Map(); private readonly _eventListeners: RegisteredListener[]; - private _firstPageCallback?: () => void; + private _firstPageCallback: () => void = () => {}; private readonly _firstPagePromise: Promise; static async connect(transport: ConnectionTransport, slowMo: number = 0, attachToDefaultContext: boolean = false): Promise { @@ -119,16 +119,13 @@ export class WKBrowser extends platform.EventEmitter implements Browser { const wkPage = new WKPage(context, pageProxySession, opener || null); this._wkPages.set(pageProxyId, wkPage); - if (this._firstPageCallback) { - this._firstPageCallback(); - this._firstPageCallback = undefined; - } - const pageEvent = new PageEvent(context, wkPage.pageOrError()); - context.emit(Events.BrowserContext.Page, pageEvent); - if (!opener) - return; - opener.pageOrError().then(openerPage => { + wkPage.pageOrError().then(async () => { + this._firstPageCallback(); + context!.emit(Events.BrowserContext.Page, pageEvent); + if (!opener) + return; + const openerPage = await opener.pageOrError(); if (openerPage instanceof Page && !openerPage.isClosed()) openerPage.emit(Events.Page.Popup, pageEvent); }); @@ -206,16 +203,10 @@ export class WKBrowserContext extends BrowserContextBase { return Array.from(this._browser._wkPages.values()).filter(wkPage => wkPage._browserContext === this); } - _existingPages(): Page[] { + pages(): Page[] { return this._wkPages().map(wkPage => wkPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; } - async pages(): Promise { - const wkPages = Array.from(this._browser._wkPages.values()).filter(wkPage => wkPage._browserContext === this); - const pages = await Promise.all(wkPages.map(wkPage => wkPage.pageOrError())); - return pages.filter(page => page instanceof Page && !page.isClosed()) as Page[]; - } - async newPage(): Promise { assertBrowserContextIsNotOwned(this); const { pageProxyId } = await this._browser._browserSession.send('Playwright.createPage', { browserContextId: this._browserContextId }); @@ -279,31 +270,31 @@ export class WKBrowserContext extends BrowserContextBase { async setExtraHTTPHeaders(headers: network.Headers): Promise { this._options.extraHTTPHeaders = network.verifyHeaders(headers); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as WKPage).updateExtraHTTPHeaders(); } async setOffline(offline: boolean): Promise { this._options.offline = offline; - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as WKPage).updateOffline(); } async setHTTPCredentials(httpCredentials: types.Credentials | null): Promise { this._options.httpCredentials = httpCredentials || undefined; - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as WKPage).updateHttpCredentials(); } async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) { const source = await helper.evaluationScript(script, args); this._evaluateOnNewDocumentSources.push(source); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as WKPage)._updateBootstrapScript(); } async exposeFunction(name: string, playwrightFunction: Function): Promise { - for (const page of this._existingPages()) { + for (const page of this.pages()) { if (page._pageBindings.has(name)) throw new Error(`Function "${name}" has been already registered in one of the pages`); } @@ -311,13 +302,13 @@ export class WKBrowserContext extends BrowserContextBase { throw new Error(`Function "${name}" has been already registered`); const binding = new PageBinding(name, playwrightFunction); this._pageBindings.set(name, binding); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as WKPage).exposeBinding(binding); } async route(url: types.URLMatch, handler: network.RouteHandler): Promise { this._routes.push({ url, handler }); - for (const page of this._existingPages()) + for (const page of this.pages()) await (page._delegate as WKPage).updateRequestInterception(); } diff --git a/test/browsercontext.spec.js b/test/browsercontext.spec.js index 3b660dd4788a7..5d3fe8a4db3f3 100644 --- a/test/browsercontext.spec.js +++ b/test/browsercontext.spec.js @@ -49,8 +49,8 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF // Create two incognito contexts. const context1 = await browser.newContext(); const context2 = await browser.newContext(); - expect((await context1.pages()).length).toBe(0); - expect((await context2.pages()).length).toBe(0); + expect(context1.pages().length).toBe(0); + expect(context2.pages().length).toBe(0); // Create a page in first incognito context. const page1 = await context1.newPage(); @@ -60,8 +60,8 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF document.cookie = 'name=page1'; }); - expect((await context1.pages()).length).toBe(1); - expect((await context2.pages()).length).toBe(0); + expect(context1.pages().length).toBe(1); + expect(context2.pages().length).toBe(0); // Create a page in second incognito context. const page2 = await context2.newPage(); @@ -71,10 +71,10 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF document.cookie = 'name=page2'; }); - expect((await context1.pages()).length).toBe(1); - expect((await context2.pages()).length).toBe(1); - expect((await context1.pages())[0]).toBe(page1); - expect((await context2.pages())[0]).toBe(page2); + expect(context1.pages().length).toBe(1); + expect(context2.pages().length).toBe(1); + expect(context1.pages()[0]).toBe(page1); + expect(context2.pages()[0]).toBe(page2); // Make sure pages don't share localstorage or cookies. expect(await page1.evaluate(() => localStorage.getItem('name'))).toBe('page1'); @@ -309,7 +309,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF const context = await browser.newContext(); const page = await context.newPage(); const second = await context.newPage(); - const allPages = await context.pages(); + const allPages = context.pages(); expect(allPages.length).toBe(2); expect(allPages).toContain(page); expect(allPages).toContain(second); @@ -318,10 +318,10 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF it('should close all belonging pages once closing context', async function({browser}) { const context = await browser.newContext(); await context.newPage(); - expect((await context.pages()).length).toBe(1); + expect(context.pages().length).toBe(1); await context.close(); - expect((await context.pages()).length).toBe(0); + expect(context.pages().length).toBe(0); }); }); @@ -490,7 +490,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF expect(await otherPage.evaluate(() => ['Hello', 'world'].join(' '))).toBe('Hello world'); expect(await otherPage.$('body')).toBeTruthy(); - let allPages = await context.pages(); + let allPages = context.pages(); expect(allPages).toContain(page); expect(allPages).toContain(otherPage); @@ -499,7 +499,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF await otherPage.close(); expect(closeEventReceived).toBeTruthy(); - allPages = await context.pages(); + allPages = context.pages(); expect(allPages).toContain(page); expect(allPages).not.toContain(otherPage); await context.close(); diff --git a/test/chromium/launcher.spec.js b/test/chromium/launcher.spec.js index 750a71027813d..cebb683d91cbd 100644 --- a/test/chromium/launcher.spec.js +++ b/test/chromium/launcher.spec.js @@ -80,13 +80,13 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, b it('should return background pages', async() => { const userDataDir = await makeUserDataDir(); const context = await browserType.launchPersistentContext(userDataDir, extensionOptions); - const backgroundPages = await context.backgroundPages(); + const backgroundPages = context.backgroundPages(); let backgroundPage = backgroundPages.length ? backgroundPages[0] : await context.waitForEvent('backgroundpage').then(event => event.page()); expect(backgroundPage).toBeTruthy(); - expect(await context.backgroundPages()).toContain(backgroundPage); - expect(await context.pages()).not.toContain(backgroundPage); + expect(context.backgroundPages()).toContain(backgroundPage); + expect(context.pages()).not.toContain(backgroundPage); await removeUserDataDir(userDataDir); }); }); diff --git a/test/headful.spec.js b/test/headful.spec.js index 53a0468162d1c..db1b5128aeed1 100644 --- a/test/headful.spec.js +++ b/test/headful.spec.js @@ -35,8 +35,8 @@ module.exports.describe = function({testRunner, expect, browserType, defaultBrow it('should have default url when launching browser', async function() { const userDataDir = await makeUserDataDir(); const browserContext = await browserType.launchPersistentContext(userDataDir, headfulOptions); - const pages = (await browserContext.pages()).map(page => page.url()); - expect(pages).toEqual(['about:blank']); + const urls = browserContext.pages().map(page => page.url()); + expect(urls).toEqual(['about:blank']); await browserContext.close(); await removeUserDataDir(userDataDir); }); diff --git a/test/launcher.spec.js b/test/launcher.spec.js index c2e10d92fd199..33af2c5a5b536 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -63,8 +63,8 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p it('should have default URL when launching browser', async function() { const userDataDir = await makeUserDataDir(); const browserContext = await browserType.launchPersistentContext(userDataDir, defaultBrowserOptions); - const pages = (await browserContext.pages()).map(page => page.url()); - expect(pages).toEqual(['about:blank']); + const urls = browserContext.pages().map(page => page.url()); + expect(urls).toEqual(['about:blank']); await browserContext.close(); await removeUserDataDir(userDataDir); }); @@ -73,7 +73,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p const options = Object.assign({}, defaultBrowserOptions); options.args = [server.EMPTY_PAGE].concat(options.args || []); const browserContext = await browserType.launchPersistentContext(userDataDir, options); - const pages = await browserContext.pages(); + const pages = browserContext.pages(); expect(pages.length).toBe(1); const page = pages[0]; if (page.url() !== server.EMPTY_PAGE) { @@ -241,7 +241,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p const browserServer = await browserType.launchServer(defaultBrowserOptions); const browser = await browserType.connect({ wsEndpoint: browserServer.wsEndpoint() }); const browserContext = await browser.newContext(); - expect((await browserContext.pages()).length).toBe(0); + expect(browserContext.pages().length).toBe(0); expect(browserServer.wsEndpoint()).not.toBe(null); const page = await browserContext.newPage(); expect(await page.evaluate('11 * 11')).toBe(121); diff --git a/test/navigation.spec.js b/test/navigation.spec.js index 350a445bb60ae..37ab40226e7cd 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -1042,19 +1042,13 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF }); it('should work with pages that have loaded before being connected to', async({page, context, server}) => { await page.goto(server.EMPTY_PAGE); - await page.evaluate(async () => { - const child = window.open(document.location.href); - while (child.document.readyState !== 'complete' || child.document.location.href === 'about:blank') - await new Promise(f => setTimeout(f, 100)); - }); - const pages = await context.pages(); - expect(pages.length).toBe(2); - expect(pages[0]).toBe(page); - expect(pages[0].url()).toBe(server.EMPTY_PAGE); - - expect(pages[1].url()).toBe(server.EMPTY_PAGE); - await pages[1].mainFrame()._waitForLoadState(); - expect(pages[1].url()).toBe(server.EMPTY_PAGE); + const [popup] = await Promise.all([ + page.waitForEvent('popup').then(e => e.page()), + page.evaluate(() => window._popup = window.open(document.location.href)), + ]); + expect(popup.url()).toBe(server.EMPTY_PAGE); + await popup.mainFrame()._waitForLoadState(); + expect(popup.url()).toBe(server.EMPTY_PAGE); }); }); diff --git a/test/page.spec.js b/test/page.spec.js index a0ce259836273..59fc8076ce015 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -40,9 +40,9 @@ module.exports.describe = function({testRunner, expect, headless, playwright, FF }); it('should not be visible in context.pages', async({context}) => { const newPage = await context.newPage(); - expect(await context.pages()).toContain(newPage); + expect(context.pages()).toContain(newPage); await newPage.close(); - expect(await context.pages()).not.toContain(newPage); + expect(context.pages()).not.toContain(newPage); }); it('should run beforeunload if asked for', async({context, server}) => { const newPage = await context.newPage(); @@ -226,21 +226,24 @@ module.exports.describe = function({testRunner, expect, headless, playwright, FF // @see https://github.com/GoogleChrome/puppeteer/issues/3865 it('should not throw when there are console messages in detached iframes', async({page, server}) => { await page.goto(server.EMPTY_PAGE); - await page.evaluate(async() => { - // 1. Create a popup that Playwright is not connected to. - const win = window.open(window.location.href, 'Title', 'toolbar=no,location=no,directories=no,status=no,menubar=no,scrollbars=yes,resizable=yes,width=780,height=200,top=0,left=0'); - if (window.document.readyState !== 'complete') - await new Promise(f => window.addEventListener('load', f)); - // 2. In this popup, create an iframe that console.logs a message. - win.document.body.innerHTML = ``; - const frame = win.document.querySelector('iframe'); - if (!frame.contentDocument || frame.contentDocument.readyState !== 'complete') - await new Promise(f => frame.addEventListener('load', f)); - // 3. After that, remove the iframe. - frame.remove(); - }); + const [popup] = await Promise.all([ + page.waitForEvent('popup').then(e => e.page()), + page.evaluate(async() => { + // 1. Create a popup that Playwright is not connected to. + const win = window.open(window.location.href); + if (window.document.readyState !== 'complete') + await new Promise(f => window.addEventListener('load', f)); + // 2. In this popup, create an iframe that console.logs a message. + win.document.body.innerHTML = ``; + const frame = win.document.querySelector('iframe'); + if (!frame.contentDocument || frame.contentDocument.readyState !== 'complete') + await new Promise(f => frame.addEventListener('load', f)); + // 3. After that, remove the iframe. + frame.remove(); + }), + ]); // 4. Connect to the popup and make sure it doesn't throw. - await page.context().pages(); + expect(await popup.evaluate('1 + 1')).toBe(2); }); });