From b5e90865762d9bd369ada18b635726d3560f9235 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 18 Dec 2019 16:23:05 -0800 Subject: [PATCH] chore: remove browser from the api (#296) --- src/browserContext.ts | 36 +++++++++++-------------------- src/chromium/Browser.ts | 8 +++---- src/chromium/FrameManager.ts | 7 ++++-- src/chromium/Target.ts | 11 +++++----- src/chromium/api.ts | 4 ---- src/firefox/Browser.ts | 6 +++--- src/firefox/api.ts | 3 --- src/page.ts | 8 +++---- src/screenshotter.ts | 6 +++--- src/webkit/Browser.ts | 8 +++---- src/webkit/FrameManager.ts | 6 ++++-- src/webkit/Target.ts | 7 ++++-- src/webkit/api.ts | 3 --- test/browsercontext.spec.js | 19 ++++++++-------- test/chromium/chromium.spec.js | 19 +++++++--------- test/chromium/connect.spec.js | 17 --------------- test/chromium/oopif.spec.js | 15 ++++++------- test/click.spec.js | 9 ++++---- test/cookies.spec.js | 11 ++++------ test/features/permissions.spec.js | 6 ++---- test/page.spec.js | 12 +++-------- 21 files changed, 86 insertions(+), 135 deletions(-) diff --git a/src/browserContext.ts b/src/browserContext.ts index 36cede7db2..bff7b6cad9 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -20,19 +20,6 @@ import { Page } from './page'; import * as input from './input'; import * as network from './network'; import * as types from './types'; -import * as childProcess from 'child_process'; - -export interface BrowserInterface { - browserContexts(): BrowserContext[]; - close(): Promise; - newContext(): Promise; - defaultContext(): BrowserContext; - newPage(): Promise; - pages(): Promise; - process(): childProcess.ChildProcess | null; - version(): Promise; - userAgent(): Promise; -} export interface BrowserDelegate { contextPages(): Promise; @@ -56,13 +43,12 @@ export type BrowserContextOptions = { export class BrowserContext { private readonly _delegate: BrowserDelegate; - private readonly _browser: BrowserInterface; private readonly _isIncognito: boolean; readonly _options: BrowserContextOptions; + private _closed = false; - constructor(delegate: BrowserDelegate, browser: BrowserInterface, isIncognito: boolean, options: BrowserContextOptions) { + constructor(delegate: BrowserDelegate, isIncognito: boolean, options: BrowserContextOptions) { this._delegate = delegate; - this._browser = browser; this._isIncognito = isIncognito; this._options = options; if (!options.viewport && options.viewport !== null) @@ -82,13 +68,14 @@ export class BrowserContext { } async _createOwnerPage(): Promise { - const page = await this._delegate.createPageInContext(); - page._isContextOwner = true; - return page; - } - - browser(): BrowserInterface { - return this._browser; + try { + const page = await this._delegate.createPageInContext(); + page._isContextOwner = true; + return page; + } catch (e) { + await this.close(); + throw e; + } } async cookies(...urls: string[]): Promise { @@ -104,7 +91,10 @@ export class BrowserContext { } async close() { + if (this._closed) + return; assert(this._isIncognito, 'Non-incognito profiles cannot be closed!'); await this._delegate.closeContext(); + this._closed = true; } } diff --git a/src/chromium/Browser.ts b/src/chromium/Browser.ts index 689ddd6f31..0649d9b985 100644 --- a/src/chromium/Browser.ts +++ b/src/chromium/Browser.ts @@ -19,7 +19,7 @@ import * as childProcess from 'child_process'; import { EventEmitter } from 'events'; import { Events } from './events'; import { assert, helper } from '../helper'; -import { BrowserContext, BrowserInterface, BrowserContextOptions } from '../browserContext'; +import { BrowserContext, BrowserContextOptions } from '../browserContext'; import { Connection, ConnectionEvents, CDPSession } from './Connection'; import { Page } from '../page'; import { Target } from './Target'; @@ -30,7 +30,7 @@ import * as network from '../network'; import { Permissions } from './features/permissions'; import { Overrides } from './features/overrides'; -export class Browser extends EventEmitter implements BrowserInterface { +export class Browser extends EventEmitter { private _process: childProcess.ChildProcess; _connection: Connection; _client: CDPSession; @@ -131,7 +131,7 @@ export class Browser extends EventEmitter implements BrowserInterface { setContextCookies: async (cookies: network.SetNetworkCookieParam[]): Promise => { await this._client.send('Storage.setCookies', { cookies, browserContextId: contextId || undefined }); }, - }, this, isIncognito, options); + }, isIncognito, options); overrides = new Overrides(context); (context as any).permissions = new Permissions(this._client, contextId); (context as any).overrides = overrides; @@ -162,7 +162,7 @@ export class Browser extends EventEmitter implements BrowserInterface { const {browserContextId} = targetInfo; const context = (browserContextId && this._contexts.has(browserContextId)) ? this._contexts.get(browserContextId) : this._defaultContext; - const target = new Target(targetInfo, context, () => this._connection.createSession(targetInfo)); + const target = new Target(this, targetInfo, context, () => this._connection.createSession(targetInfo)); assert(!this._targets.has(event.targetInfo.targetId), 'Target should not exist before targetCreated'); this._targets.set(event.targetInfo.targetId, target); diff --git a/src/chromium/FrameManager.ts b/src/chromium/FrameManager.ts index ac9bbab30f..25937d51cb 100644 --- a/src/chromium/FrameManager.ts +++ b/src/chromium/FrameManager.ts @@ -51,9 +51,11 @@ export class FrameManager implements PageDelegate { private _eventListeners: RegisteredListener[]; rawMouse: RawMouseImpl; rawKeyboard: RawKeyboardImpl; + private _browser: Browser; - constructor(client: CDPSession, browserContext: BrowserContext) { + constructor(client: CDPSession, browser: Browser, browserContext: BrowserContext) { this._client = client; + this._browser = browser; this.rawKeyboard = new RawKeyboardImpl(client); this.rawMouse = new RawMouseImpl(client); this._page = new Page(this, browserContext); @@ -330,7 +332,7 @@ export class FrameManager implements PageDelegate { if (runBeforeUnload) await this._client.send('Page.close'); else - await (this._page.browser() as Browser)._closePage(this._page); + await this._browser._closePage(this._page); } async getBoundingBoxForScreenshot(handle: dom.ElementHandle): Promise { @@ -352,6 +354,7 @@ export class FrameManager implements PageDelegate { } async takeScreenshot(format: 'png' | 'jpeg', options: types.ScreenshotOptions): Promise { + await this._client.send('Page.bringToFront', {}); const clip = options.clip ? { ...options.clip, scale: 1 } : undefined; const result = await this._client.send('Page.captureScreenshot', { format, quality: options.quality, clip }); return Buffer.from(result.data, 'base64'); diff --git a/src/chromium/Target.ts b/src/chromium/Target.ts index 4004260845..6d0beda65a 100644 --- a/src/chromium/Target.ts +++ b/src/chromium/Target.ts @@ -29,6 +29,7 @@ const targetSymbol = Symbol('target'); export class Target { private _targetInfo: Protocol.Target.TargetInfo; + private _browser: Browser; private _browserContext: BrowserContext; _targetId: string; private _sessionFactory: () => Promise; @@ -44,10 +45,12 @@ export class Target { } constructor( + browser: Browser, targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext, sessionFactory: () => Promise) { this._targetInfo = targetInfo; + this._browser = browser; this._browserContext = browserContext; this._targetId = targetInfo.targetId; this._sessionFactory = sessionFactory; @@ -77,7 +80,7 @@ export class Target { async page(): Promise { if ((this._targetInfo.type === 'page' || this._targetInfo.type === 'background_page') && !this._pagePromise) { this._pagePromise = this._sessionFactory().then(async client => { - this._frameManager = new FrameManager(client, this._browserContext); + this._frameManager = new FrameManager(client, this._browser, this._browserContext); const page = this._frameManager.page(); (page as any)[targetSymbol] = this; client.once(CDPSessionEvents.Disconnected, () => page._didDisconnect()); @@ -117,10 +120,6 @@ export class Target { return 'other'; } - browser(): Browser { - return this._browserContext.browser() as Browser; - } - browserContext(): BrowserContext { return this._browserContext; } @@ -129,7 +128,7 @@ export class Target { const { openerId } = this._targetInfo; if (!openerId) return null; - return this.browser()._targets.get(openerId); + return this._browser._targets.get(openerId); } createCDPSession(): Promise { diff --git a/src/chromium/api.ts b/src/chromium/api.ts index 2453ee25d2..7bbda2a8aa 100644 --- a/src/chromium/api.ts +++ b/src/chromium/api.ts @@ -9,9 +9,7 @@ export { Frame } from '../frames'; export { Keyboard, Mouse } from '../input'; export { JSHandle } from '../javascript'; export { Request, Response } from '../network'; -export { Browser } from './Browser'; export { BrowserContext } from '../browserContext'; -export { BrowserFetcher } from '../browserFetcher'; export { CDPSession } from './Connection'; export { Accessibility } from './features/accessibility'; export { Chromium } from './features/chromium'; @@ -22,6 +20,4 @@ export { PDF } from './features/pdf'; export { Permissions } from './features/permissions'; export { Worker, Workers } from './features/workers'; export { Page } from '../page'; -export { Playwright } from './Playwright'; export { Target } from './Target'; - diff --git a/src/firefox/Browser.ts b/src/firefox/Browser.ts index 3e9743f2e2..7b6822d481 100644 --- a/src/firefox/Browser.ts +++ b/src/firefox/Browser.ts @@ -25,9 +25,9 @@ import { Page } from '../page'; import { FrameManager } from './FrameManager'; import { Firefox } from './features/firefox'; import * as network from '../network'; -import { BrowserContext, BrowserInterface, BrowserContextOptions } from '../browserContext'; +import { BrowserContext, BrowserContextOptions } from '../browserContext'; -export class Browser extends EventEmitter implements BrowserInterface { +export class Browser extends EventEmitter { _connection: Connection; private _process: import('child_process').ChildProcess; private _closeCallback: () => Promise; @@ -226,7 +226,7 @@ export class Browser extends EventEmitter implements BrowserInterface { setContextCookies: async (cookies: network.SetNetworkCookieParam[]): Promise => { await this._connection.send('Browser.setCookies', { browserContextId: browserContextId || undefined, cookies }); }, - }, this, isIncognito, options); + }, isIncognito, options); (context as any).permissions = new Permissions(this._connection, browserContextId); return context; } diff --git a/src/firefox/api.ts b/src/firefox/api.ts index d355d5976a..393cbadd8e 100644 --- a/src/firefox/api.ts +++ b/src/firefox/api.ts @@ -3,9 +3,7 @@ export { TimeoutError } from '../errors'; export { Keyboard, Mouse } from '../input'; -export { Browser } from './Browser'; export { BrowserContext } from '../browserContext'; -export { BrowserFetcher } from '../browserFetcher'; export { Dialog } from '../dialog'; export { JSHandle } from '../javascript'; export { ElementHandle } from '../dom'; @@ -15,5 +13,4 @@ export { Permissions } from './features/permissions'; export { Frame } from '../frames'; export { Request, Response } from '../network'; export { Page } from '../page'; -export { Playwright } from './Playwright'; export { ConsoleMessage } from '../console'; diff --git a/src/page.ts b/src/page.ts index dbb9af1c60..a1851e28b5 100644 --- a/src/page.ts +++ b/src/page.ts @@ -26,7 +26,7 @@ import { Screenshotter } from './screenshotter'; import { TimeoutSettings } from './timeoutSettings'; import * as types from './types'; import { Events } from './events'; -import { BrowserContext, BrowserInterface } from './browserContext'; +import { BrowserContext } from './browserContext'; import { ConsoleMessage, ConsoleMessageLocation } from './console'; import Injected from './injected/injected'; @@ -138,10 +138,6 @@ export class Page extends EventEmitter { this.emit(Events.Page.FileChooser, fileChooser); } - browser(): BrowserInterface { - return this._browserContext.browser(); - } - browserContext(): BrowserContext { return this._browserContext; } @@ -401,6 +397,8 @@ export class Page extends EventEmitter { } async close(options: { runBeforeUnload: (boolean | undefined); } = {runBeforeUnload: undefined}) { + if (this._closed) + return; assert(!this._disconnected, 'Protocol error: Connection closed. Most likely the page has been closed.'); const runBeforeUnload = !!options.runBeforeUnload; await this._delegate.closePage(runBeforeUnload); diff --git a/src/screenshotter.ts b/src/screenshotter.ts index cc7923de8f..3ece8ab251 100644 --- a/src/screenshotter.ts +++ b/src/screenshotter.ts @@ -31,11 +31,11 @@ export class Screenshotter { constructor(page: Page) { this._page = page; - const browser = page.browser(); - this._queue = browser[taskQueueSymbol]; + const browserContext = page.browserContext(); + this._queue = browserContext[taskQueueSymbol]; if (!this._queue) { this._queue = new TaskQueue(); - browser[taskQueueSymbol] = this._queue; + browserContext[taskQueueSymbol] = this._queue; } } diff --git a/src/webkit/Browser.ts b/src/webkit/Browser.ts index 91a31656eb..5f8b4b2121 100644 --- a/src/webkit/Browser.ts +++ b/src/webkit/Browser.ts @@ -24,9 +24,9 @@ import { Page } from '../page'; import { Target } from './Target'; import { Protocol } from './protocol'; import { Events } from '../events'; -import { BrowserContext, BrowserInterface, BrowserContextOptions } from '../browserContext'; +import { BrowserContext, BrowserContextOptions } from '../browserContext'; -export class Browser extends EventEmitter implements BrowserInterface { +export class Browser extends EventEmitter { private readonly _process: childProcess.ChildProcess; readonly _connection: Connection; private _closeCallback: () => Promise; @@ -151,7 +151,7 @@ export class Browser extends EventEmitter implements BrowserInterface { } if (!context) context = this._defaultContext; - const target = new Target(session, targetInfo, context); + const target = new Target(this, session, targetInfo, context); this._targets.set(targetInfo.targetId, target); if (targetInfo.isProvisional) { const oldTarget = this._targets.get(targetInfo.oldTargetId); @@ -244,7 +244,7 @@ export class Browser extends EventEmitter implements BrowserInterface { const cc = cookies.map(c => ({ ...c, session: c.expires === -1 || c.expires === undefined })) as Protocol.Browser.SetCookieParam[]; await this._connection.send('Browser.setCookies', { cookies: cc, browserContextId }); }, - }, this, isIncognito, options); + }, isIncognito, options); return context; } } diff --git a/src/webkit/FrameManager.ts b/src/webkit/FrameManager.ts index dde87d88ee..f117c82c9b 100644 --- a/src/webkit/FrameManager.ts +++ b/src/webkit/FrameManager.ts @@ -42,13 +42,15 @@ export class FrameManager implements PageDelegate { readonly rawKeyboard: RawKeyboardImpl; _session: TargetSession; readonly _page: Page; + private _browser: Browser; private readonly _networkManager: NetworkManager; private readonly _contextIdToContext: Map; private _isolatedWorlds: Set; private _sessionListeners: RegisteredListener[] = []; private readonly _bootstrapScripts: string[] = []; - constructor(browserContext: BrowserContext) { + constructor(browser: Browser, browserContext: BrowserContext) { + this._browser = browser; this.rawKeyboard = new RawKeyboardImpl(); this.rawMouse = new RawMouseImpl(); this._contextIdToContext = new Map(); @@ -329,7 +331,7 @@ export class FrameManager implements PageDelegate { } async closePage(runBeforeUnload: boolean): Promise { - (this._page.browser() as Browser)._closePage(this._page, runBeforeUnload); + this._browser._closePage(this._page, runBeforeUnload); } getBoundingBoxForScreenshot(handle: dom.ElementHandle): Promise { diff --git a/src/webkit/Target.ts b/src/webkit/Target.ts index 2d89cf4a3e..40c89b2792 100644 --- a/src/webkit/Target.ts +++ b/src/webkit/Target.ts @@ -20,6 +20,7 @@ import { Page } from '../page'; import { Protocol } from './protocol'; import { TargetSession, TargetSessionEvents } from './Connection'; import { FrameManager } from './FrameManager'; +import { Browser } from './Browser'; const targetSymbol = Symbol('target'); @@ -29,14 +30,16 @@ export class Target { readonly _type: 'page' | 'service-worker' | 'worker'; private readonly _session: TargetSession; private _pagePromise: Promise | null = null; + private _browser: Browser; _frameManager: FrameManager | null = null; static fromPage(page: Page): Target { return (page as any)[targetSymbol]; } - constructor(session: TargetSession, targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext) { + constructor(browser: Browser, session: TargetSession, targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext) { const {targetId, type} = targetInfo; + this._browser = browser; this._session = session; this._browserContext = browserContext; this._targetId = targetId; @@ -85,7 +88,7 @@ export class Target { async page(): Promise { if (this._type === 'page' && !this._pagePromise) { - this._frameManager = new FrameManager(this._browserContext); + this._frameManager = new FrameManager(this._browser, this._browserContext); // Reference local page variable as |this._frameManager| may be // cleared on swap. const page = this._frameManager._page; diff --git a/src/webkit/api.ts b/src/webkit/api.ts index d6080780ce..bd18c96726 100644 --- a/src/webkit/api.ts +++ b/src/webkit/api.ts @@ -2,15 +2,12 @@ // Licensed under the MIT license. export { TimeoutError } from '../errors'; -export { Browser } from './Browser'; export { BrowserContext } from '../browserContext'; -export { BrowserFetcher } from '../browserFetcher'; export { JSHandle } from '../javascript'; export { ElementHandle } from '../dom'; export { Frame } from '../frames'; export { Mouse, Keyboard } from '../input'; export { Request, Response } from '../network'; export { Page } from '../page'; -export { Playwright } from './Playwright'; export { Dialog } from '../dialog'; export { ConsoleMessage } from '../console'; diff --git a/test/browsercontext.spec.js b/test/browsercontext.spec.js index 4a37dff644..2f85d85112 100644 --- a/test/browsercontext.spec.js +++ b/test/browsercontext.spec.js @@ -32,19 +32,19 @@ module.exports.addTests = function({testRunner, expect, playwright, WEBKIT}) { expect(browser.defaultContext()).toBe(defaultContext); expect(error.message).toContain('cannot be closed'); }); - it('should create new incognito context', async function({browser, server}) { + it('should create new incognito context', async function({browser, newContext}) { expect(browser.browserContexts().length).toBe(1); - const context = await browser.newContext(); + const context = await newContext(); expect(context.isIncognito()).toBe(true); expect(browser.browserContexts().length).toBe(2); expect(browser.browserContexts().indexOf(context) !== -1).toBe(true); await context.close(); expect(browser.browserContexts().length).toBe(1); }); - it('should close all belonging targets once closing context', async function({browser, server}) { + it('should close all belonging targets once closing context', async function({browser, newContext, server}) { expect((await browser.pages()).length).toBe(1); - const context = await browser.newContext(); + const context = await newContext(); await context.newPage(); expect((await browser.pages()).length).toBe(2); expect((await context.pages()).length).toBe(1); @@ -52,8 +52,8 @@ module.exports.addTests = function({testRunner, expect, playwright, WEBKIT}) { await context.close(); expect((await browser.pages()).length).toBe(1); }); - it('window.open should use parent tab context', async function({browser, server}) { - const context = await browser.newContext(); + it('window.open should use parent tab context', async function({browser, newContext, server}) { + const context = await newContext(); const page = await context.newPage(); await page.goto(server.EMPTY_PAGE); const [popupTarget] = await Promise.all([ @@ -61,12 +61,11 @@ module.exports.addTests = function({testRunner, expect, playwright, WEBKIT}) { page.evaluate(url => window.open(url), server.EMPTY_PAGE) ]); expect(popupTarget.browserContext()).toBe(context); - await context.close(); }); - it('should isolate localStorage and cookies', async function({browser, server}) { + it('should isolate localStorage and cookies', async function({browser, newContext, server}) { // Create two incognito contexts. - const context1 = await browser.newContext(); - const context2 = await browser.newContext(); + const context1 = await newContext(); + const context2 = await newContext(); expect((await context1.pages()).length).toBe(0); expect((await context2.pages()).length).toBe(0); diff --git a/test/chromium/chromium.spec.js b/test/chromium/chromium.spec.js index 0459f583a1..2c2ce4dc88 100644 --- a/test/chromium/chromium.spec.js +++ b/test/chromium/chromium.spec.js @@ -22,9 +22,9 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME const {beforeAll, beforeEach, afterAll, afterEach} = testRunner; describe('Chromium', function() { - it('should work across sessions', async function({browser, server}) { + it('should work across sessions', async function({browser, newContext}) { expect(browser.browserContexts().length).toBe(2); - const context = await browser.newContext(); + await newContext(); expect(browser.browserContexts().length).toBe(3); const remoteBrowser = await playwright.connect({ browserWSEndpoint: browser.chromium.wsEndpoint() @@ -32,7 +32,6 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME const contexts = remoteBrowser.browserContexts(); expect(contexts.length).toBe(3); remoteBrowser.disconnect(); - await context.close(); }); }); @@ -174,8 +173,8 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME }); describe('Chromium.waitForTarget', () => { - it('should wait for a target', async function({browser, server}) { - const context = await browser.newContext(); + it('should wait for a target', async function({browser, server, newContext}) { + const context = await newContext(); let resolved = false; const targetPromise = browser.chromium.waitForTarget(target => target.browserContext() === context && target.url() === server.EMPTY_PAGE); targetPromise.then(() => resolved = true); @@ -184,10 +183,9 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME await page.goto(server.EMPTY_PAGE); const target = await targetPromise; expect(await target.page()).toBe(page); - await context.close(); }); - it('should timeout waiting for a non-existent target', async function({browser, server}) { - const context = await browser.newContext(); + it('should timeout waiting for a non-existent target', async function({browser, server, newContext}) { + const context = await newContext(); const error = await browser.chromium.waitForTarget(target => target.browserContext() === context && target.url() === server.EMPTY_PAGE, {timeout: 1}).catch(e => e); expect(error).toBeInstanceOf(playwright.errors.TimeoutError); await context.close(); @@ -210,8 +208,8 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME }).catch(e => error = e); expect(error).toBeInstanceOf(playwright.errors.TimeoutError); }); - it('should fire target events', async function({browser, server}) { - const context = await browser.newContext(); + it('should fire target events', async function({browser, newContext, server}) { + const context = await newContext(); const events = []; browser.chromium.on('targetcreated', target => events.push('CREATED: ' + target.url())); browser.chromium.on('targetchanged', target => events.push('CHANGED: ' + target.url())); @@ -224,7 +222,6 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME `CHANGED: ${server.EMPTY_PAGE}`, `DESTROYED: ${server.EMPTY_PAGE}` ]); - await context.close(); }); }); diff --git a/test/chromium/connect.spec.js b/test/chromium/connect.spec.js index 79c1e27544..7028fe364f 100644 --- a/test/chromium/connect.spec.js +++ b/test/chromium/connect.spec.js @@ -51,23 +51,6 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p remoteBrowser.close(), ]); }); - it('should support ignoreHTTPSErrors option', async({httpsServer}) => { - const originalBrowser = await playwright.launch(defaultBrowserOptions); - const browserWSEndpoint = originalBrowser.chromium.wsEndpoint(); - - const browser = await playwright.connect({...defaultBrowserOptions, browserWSEndpoint}); - const context = await browser.newContext({ ignoreHTTPSErrors: true }); - const page = await context.newPage(); - let error = null; - const [, response] = await Promise.all([ - httpsServer.waitForRequest('/empty.html'), - page.goto(httpsServer.EMPTY_PAGE).catch(e => error = e) - ]); - expect(error).toBe(null); - expect(response.ok()).toBe(true); - await context.close(); - await browser.close(); - }); it('should be able to reconnect to a disconnected browser', async({server}) => { const originalBrowser = await playwright.launch(defaultBrowserOptions); const browserWSEndpoint = originalBrowser.chromium.wsEndpoint(); diff --git a/test/chromium/oopif.spec.js b/test/chromium/oopif.spec.js index 08e3b68386..0061acc5c5 100644 --- a/test/chromium/oopif.spec.js +++ b/test/chromium/oopif.spec.js @@ -38,24 +38,21 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p await state.browser.close(); state.browser = null; }); - xit('should report oopif frames', async function({page, server, context}) { + xit('should report oopif frames', async function({browser, page, server, context}) { await page.goto(server.PREFIX + '/dynamic-oopif.html'); - expect(oopifs(context).length).toBe(1); + expect(oopifs(browser).length).toBe(1); expect(page.frames().length).toBe(2); }); - it('should load oopif iframes with subresources and request interception', async function({page, server, context}) { + it('should load oopif iframes with subresources and request interception', async function({browser, page, server, context}) { await page.interception.enable(); page.on('request', request => page.interception.continue(request)); await page.goto(server.PREFIX + '/dynamic-oopif.html'); - expect(oopifs(context).length).toBe(1); + expect(oopifs(browser).length).toBe(1); }); }); }; -/** - * @param {!Playwright.BrowserContext} context - */ -function oopifs(context) { - return context.browser().chromium.targets().filter(target => target._targetInfo.type === 'iframe'); +function oopifs(browser) { + return browser.chromium.targets().filter(target => target._targetInfo.type === 'iframe'); } diff --git a/test/click.spec.js b/test/click.spec.js index 287b481291..0ce98ad9e3 100644 --- a/test/click.spec.js +++ b/test/click.spec.js @@ -56,11 +56,12 @@ module.exports.addTests = function({testRunner, expect, playwright, FFOX, CHROME await page.click('span'); expect(await page.evaluate(() => window.CLICKED)).toBe(42); }); - it('should not throw UnhandledPromiseRejection when page closes', async({page, server}) => { - const newPage = await page.browser().newPage(); + it('should not throw UnhandledPromiseRejection when page closes', async({newContext, server}) => { + const context = await newContext(); + const page = await context.newPage(); await Promise.all([ - newPage.close(), - newPage.mouse.click(1, 2), + page.close(), + page.mouse.click(1, 2), ]).catch(e => {}); }); it('should click the button after navigation ', async({page, server}) => { diff --git a/test/cookies.spec.js b/test/cookies.spec.js index 5db5bbee75..b2185f0dcc 100644 --- a/test/cookies.spec.js +++ b/test/cookies.spec.js @@ -154,8 +154,8 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { }]); expect(await page.evaluate(() => document.cookie)).toEqual('password=123456'); }); - it('should isolate cookies in browser contexts', async({context, server, browser}) => { - const anotherContext = await browser.newContext(); + it('should isolate cookies in browser contexts', async({context, server, newContext}) => { + const anotherContext = await newContext(); await context.setCookies([{url: server.EMPTY_PAGE, name: 'page1cookie', value: 'page1value'}]); await anotherContext.setCookies([{url: server.EMPTY_PAGE, name: 'page2cookie', value: 'page2value'}]); @@ -167,7 +167,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { expect(cookies1[0].value).toBe('page1value'); expect(cookies2[0].name).toBe('page2cookie'); expect(cookies2[0].value).toBe('page2value'); - await anotherContext.close(); }); it('should set multiple cookies', async({context, page, server}) => { await page.goto(server.EMPTY_PAGE); @@ -364,8 +363,8 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { await context.clearCookies(); expect(await page.evaluate('document.cookie')).toBe(''); }); - it('should isolate cookies when clearing', async({context, server, browser}) => { - const anotherContext = await browser.newContext(); + it('should isolate cookies when clearing', async({context, server, newContext}) => { + const anotherContext = await newContext(); await context.setCookies([{url: server.EMPTY_PAGE, name: 'page1cookie', value: 'page1value'}]); await anotherContext.setCookies([{url: server.EMPTY_PAGE, name: 'page2cookie', value: 'page2value'}]); @@ -379,8 +378,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { await anotherContext.clearCookies(); expect((await context.cookies()).length).toBe(0); expect((await anotherContext.cookies()).length).toBe(0); - - await anotherContext.close(); }); }); }; diff --git a/test/features/permissions.spec.js b/test/features/permissions.spec.js index 738809b2dc..78a95b804e 100644 --- a/test/features/permissions.spec.js +++ b/test/features/permissions.spec.js @@ -75,9 +75,9 @@ module.exports.addTests = function({testRunner, expect, WEBKIT}) { await context.permissions.clearOverrides(); expect(await page.evaluate(() => window['events'])).toEqual(['prompt', 'denied', 'granted', 'prompt']); }); - it.skip(WEBKIT)('should isolate permissions between browser contexs', async({page, server, context, browser}) => { + it.skip(WEBKIT)('should isolate permissions between browser contexs', async({page, server, context, newContext}) => { await page.goto(server.EMPTY_PAGE); - const otherContext = await browser.newContext(); + const otherContext = await newContext(); const otherPage = await otherContext.newPage(); await otherPage.goto(server.EMPTY_PAGE); expect(await getPermission(page, 'geolocation')).toBe('prompt'); @@ -91,8 +91,6 @@ module.exports.addTests = function({testRunner, expect, WEBKIT}) { await context.permissions.clearOverrides(); expect(await getPermission(page, 'geolocation')).toBe('prompt'); expect(await getPermission(otherPage, 'geolocation')).toBe('granted'); - - await otherContext.close(); }); }); }; diff --git a/test/page.spec.js b/test/page.spec.js index be4f477e49..7d8cd12363 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -261,7 +261,7 @@ module.exports.addTests = function({testRunner, expect, headless, playwright, FF }); }); // @see https://github.com/GoogleChrome/puppeteer/issues/3865 - it.skip(FFOX)('should not throw when there are console messages in detached iframes', async({browser, page, server}) => { + it.skip(FFOX)('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. @@ -1154,7 +1154,7 @@ module.exports.addTests = function({testRunner, expect, headless, playwright, FF }); describe('Page.Events.Close', function() { - it('should work with window.close', async function({ browser, page, context, server }) { + it('should work with window.close', async function({ page, context, server }) { const newPagePromise = new Promise(f => page.once('popup', f)); await page.evaluate(() => window['newPage'] = window.open('about:blank')); const newPage = await newPagePromise; @@ -1170,14 +1170,8 @@ module.exports.addTests = function({testRunner, expect, headless, playwright, FF }); }); - describe('Page.browser', function() { - it('should return the correct browser instance', async function({ page, browser }) { - expect(page.browser()).toBe(browser); - }); - }); - describe('Page.browserContext', function() { - it('should return the correct browser instance', async function({page, context, browser}) { + it('should return the correct browser instance', async function({page, context}) { expect(page.browserContext()).toBe(context); }); });