feat(api): make browser.newPage own the created context (#930)

This commit is contained in:
Pavel Feldman 2020-02-11 12:06:58 -08:00 committed by GitHub
parent ad9d6cc31f
commit aae5fca237
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 32 additions and 49 deletions

View File

@ -153,7 +153,6 @@ See [ChromiumBrowser], [FirefoxBrowser] and [WebKitBrowser] for browser-specific
- [browser.isConnected()](#browserisconnected)
- [browser.newContext(options)](#browsernewcontextoptions)
- [browser.newPage([options])](#browsernewpageoptions)
- [browser.pages()](#browserpages)
<!-- GEN:stop -->
#### event: 'disconnected'
@ -233,12 +232,9 @@ Creates a new browser context. It won't share cookies/cache with other browser c
- `permissions` <[Object]> A map from origin keys to permissions values. See [browserContext.setPermissions](#browsercontextsetpermissionsorigin-permissions) for more details.
- returns: <[Promise]<[Page]>>
Creates a new page in a new browser context.
Creates a new page in a new browser context. Closing this page will close the context as well.
#### browser.pages()
- returns: <[Promise]<[Array]<[Page]>>> Promise which resolves to an array of all open pages.
An array of all the pages inside all the browser contexts.
This is a convenience API that should only be used for the single-page scenarios and short snippets. Production code and testing frameworks should explicitly create [browser.newContext](#browsernewcontextoptions) followed by the [browserContext.newPage](#browsercontextnewpage) to control their exact life times.
### class: BrowserContext
@ -3667,7 +3663,6 @@ await browser.stopTracing();
- [browser.isConnected()](#browserisconnected)
- [browser.newContext(options)](#browsernewcontextoptions)
- [browser.newPage([options])](#browsernewpageoptions)
- [browser.pages()](#browserpages)
<!-- GEN:stop -->
#### event: 'targetchanged'
@ -3834,7 +3829,6 @@ Firefox browser instance does not expose Firefox-specific features.
- [browser.isConnected()](#browserisconnected)
- [browser.newContext(options)](#browsernewcontextoptions)
- [browser.newPage([options])](#browsernewpageoptions)
- [browser.pages()](#browserpages)
<!-- GEN:stop -->
### class: WebKitBrowser
@ -3850,7 +3844,6 @@ WebKit browser instance does not expose WebKit-specific features.
- [browser.isConnected()](#browserisconnected)
- [browser.newContext(options)](#browsernewcontextoptions)
- [browser.newPage([options])](#browsernewpageoptions)
- [browser.pages()](#browserpages)
<!-- GEN:stop -->
### Working with selectors

View File

@ -21,7 +21,6 @@ import { Page } from './page';
export interface Browser extends platform.EventEmitterType {
newContext(options?: BrowserContextOptions): Promise<BrowserContext>;
contexts(): BrowserContext[];
pages(): Promise<Page[]>;
newPage(options?: BrowserContextOptions): Promise<Page>;
isConnected(): boolean;
close(): Promise<void>;
@ -32,14 +31,11 @@ export type ConnectOptions = {
wsEndpoint: string
};
export async function collectPages(browser: Browser): Promise<Page[]> {
const result: Promise<Page[]>[] = [];
for (const browserContext of browser.contexts())
result.push(browserContext.pages());
const pages: Page[] = [];
for (const group of await Promise.all(result))
pages.push(...group);
return pages;
export async function createPageInNewContext(browser: Browser, options?: BrowserContextOptions): Promise<Page> {
const context = await browser.newContext(options);
const page = await context.newPage();
page._ownedContext = context;
return page;
}
export type LaunchType = 'local' | 'server' | 'persistent';

View File

@ -82,6 +82,11 @@ export class BrowserContext extends platform.EventEmitter {
}
async newPage(): Promise<Page> {
const pages = this._delegate.existingPages();
for (const page of pages) {
if (page._ownedContext)
throw new Error('Please use browser.newContext() for multi-page scripts that share the context.');
}
return this._delegate.newPage();
}

View File

@ -24,7 +24,7 @@ import { Page, Worker } from '../page';
import { CRTarget } from './crTarget';
import { Protocol } from './protocol';
import { CRPage } from './crPage';
import { Browser, collectPages } from '../browser';
import { Browser, createPageInNewContext } from '../browser';
import * as network from '../network';
import * as types from '../types';
import * as platform from '../platform';
@ -168,13 +168,8 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
return Array.from(this._contexts.values());
}
async pages(): Promise<Page[]> {
return collectPages(this);
}
async newPage(options?: BrowserContextOptions): Promise<Page> {
const context = await this.newContext(options);
return context.newPage();
return createPageInNewContext(this, options);
}
async _targetCreated(event: Protocol.Target.targetCreatedPayload) {

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
import { Browser, collectPages } from '../browser';
import { Browser, createPageInNewContext } from '../browser';
import { BrowserContext, BrowserContextOptions } from '../browserContext';
import { Events } from '../events';
import { assert, helper, RegisteredListener } from '../helper';
@ -84,13 +84,8 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
return Array.from(this._contexts.values());
}
async pages(): Promise<Page[]> {
return collectPages(this);
}
async newPage(options?: BrowserContextOptions): Promise<Page> {
const context = await this.newContext(options);
return context.newPage();
return createPageInNewContext(this, options);
}
async _waitForTarget(predicate: (target: Target) => boolean, options: { timeout?: number; } = {}): Promise<Target> {

View File

@ -114,6 +114,7 @@ export class Page extends platform.EventEmitter {
readonly pdf: ((options?: types.PDFOptions) => Promise<platform.BufferType>) | undefined;
readonly coverage: Coverage | undefined;
readonly _requestHandlers: { url: types.URLMatch, handler: (request: network.Request) => void }[] = [];
_ownedContext: BrowserContext | undefined;
constructor(delegate: PageDelegate, browserContext: BrowserContext) {
super();
@ -472,6 +473,8 @@ export class Page extends platform.EventEmitter {
await this._delegate.closePage(runBeforeUnload);
if (!runBeforeUnload)
await this._closedPromise;
if (this._ownedContext)
await this._ownedContext.close();
}
isClosed(): boolean {

View File

@ -15,7 +15,7 @@
* limitations under the License.
*/
import { Browser, collectPages } from '../browser';
import { Browser, createPageInNewContext } from '../browser';
import { BrowserContext, BrowserContextOptions } from '../browserContext';
import { assert, helper, RegisteredListener } from '../helper';
import * as network from '../network';
@ -89,13 +89,8 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
return Array.from(this._contexts.values());
}
async pages(): Promise<Page[]> {
return collectPages(this);
}
async newPage(options?: BrowserContextOptions): Promise<Page> {
const context = await this.newContext(options);
return context.newPage();
return createPageInNewContext(this, options);
}
async _waitForFirstPageTarget(timeout: number): Promise<void> {

View File

@ -24,24 +24,25 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
const {it, fit, xit, dit} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
describe('Browser', function() {
describe('Browser.newPage', function() {
it('should create new page', async function({browser}) {
expect((await browser.pages()).length).toBe(0);
const page1 = await browser.newPage();
expect((await browser.pages()).length).toBe(1);
expect(browser.contexts().length).toBe(1);
const page2 = await browser.newPage();
expect((await browser.pages()).length).toBe(2);
expect(browser.contexts().length).toBe(2);
await page1.context().close();
expect((await browser.pages()).length).toBe(1);
await page1.close();
expect(browser.contexts().length).toBe(1);
await page2.context().close();
expect((await browser.pages()).length).toBe(0);
await page2.close();
expect(browser.contexts().length).toBe(0);
});
it('should throw upon second create new page', async function({browser}) {
const page = await browser.newPage();
let error;
await page.context().newPage().catch(e => error = e);
expect(error.message).toContain('Please use browser.newContext()');
});
});
};

View File

@ -58,7 +58,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
const newPage = await browser.newPage();
let error = null;
await browser.startTracing(newPage, {path: outputFile}).catch(e => error = e);
await newPage.context().close();
await newPage.close();
expect(error).toBeTruthy();
await browser.stopTracing();
});

View File

@ -46,7 +46,7 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p
afterEach(async state => {
await state.page.evaluate(() => teardown());
await state.page.context().close();
await state.page.close();
state.page = null;
});