From 90a9de72573735568dce559d7fca0ab69623ba17 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Fri, 16 Jun 2023 15:27:31 +0200 Subject: [PATCH] chore: implement default context and newPage/pages (#10397) --- packages/puppeteer-core/src/api/Browser.ts | 12 +- packages/puppeteer-core/src/common/Browser.ts | 21 --- .../src/common/bidi/BidiOverCDP.ts | 2 +- .../puppeteer-core/src/common/bidi/Browser.ts | 34 ++++ .../src/common/bidi/BrowserContext.ts | 31 ++++ .../src/common/bidi/Connection.ts | 13 +- .../src/node/ProductLauncher.ts | 1 + test/TestExpectations.json | 166 +++++++----------- test/src/bidi/Connection.spec.ts | 2 +- test/src/chromiumonly.spec.ts | 2 + 10 files changed, 154 insertions(+), 130 deletions(-) diff --git a/packages/puppeteer-core/src/api/Browser.ts b/packages/puppeteer-core/src/api/Browser.ts index 3ff78585f03..584cada58de 100644 --- a/packages/puppeteer-core/src/api/Browser.ts +++ b/packages/puppeteer-core/src/api/Browser.ts @@ -395,8 +395,16 @@ export class Browser extends EventEmitter { * browser contexts. Non-visible pages, such as `"background_page"`, will not be listed * here. You can find them using {@link Target.page}. */ - pages(): Promise { - throw new Error('Not implemented'); + async pages(): Promise { + const contextPages = await Promise.all( + this.browserContexts().map(context => { + return context.pages(); + }) + ); + // Flatten array. + return contextPages.reduce((acc, x) => { + return acc.concat(x); + }, []); } /** diff --git a/packages/puppeteer-core/src/common/Browser.ts b/packages/puppeteer-core/src/common/Browser.ts index 9dd83035515..10926fb7d92 100644 --- a/packages/puppeteer-core/src/common/Browser.ts +++ b/packages/puppeteer-core/src/common/Browser.ts @@ -527,27 +527,6 @@ export class CDPBrowser extends BrowserBase { } } - /** - * An array of all open pages inside the Browser. - * - * @remarks - * - * In case of multiple browser contexts, returns an array with all the pages in all - * browser contexts. Non-visible pages, such as `"background_page"`, will not be listed - * here. You can find them using {@link Target.page}. - */ - override async pages(): Promise { - const contextPages = await Promise.all( - this.browserContexts().map(context => { - return context.pages(); - }) - ); - // Flatten array. - return contextPages.reduce((acc, x) => { - return acc.concat(x); - }, []); - } - override async version(): Promise { const version = await this.#getVersion(); return version.product; diff --git a/packages/puppeteer-core/src/common/bidi/BidiOverCDP.ts b/packages/puppeteer-core/src/common/bidi/BidiOverCDP.ts index 62166fd9202..95a13ceb035 100644 --- a/packages/puppeteer-core/src/common/bidi/BidiOverCDP.ts +++ b/packages/puppeteer-core/src/common/bidi/BidiOverCDP.ts @@ -53,7 +53,7 @@ export async function connectBidiOverCDP( // Forwards a BiDi event sent by BidiServer to Puppeteer. pptrTransport.onmessage(JSON.stringify(message)); }); - const pptrBiDiConnection = new BidiPPtrConnection(pptrTransport); + const pptrBiDiConnection = new BidiPPtrConnection(cdp.url(), pptrTransport); const bidiServer = await BidiMapper.BidiServer.createAndStart( transportBiDi, cdpConnectionAdapter, diff --git a/packages/puppeteer-core/src/common/bidi/Browser.ts b/packages/puppeteer-core/src/common/bidi/Browser.ts index 76eb1ed214a..9d6a28d783a 100644 --- a/packages/puppeteer-core/src/common/bidi/Browser.ts +++ b/packages/puppeteer-core/src/common/bidi/Browser.ts @@ -25,6 +25,7 @@ import { BrowserEmittedEvents, } from '../../api/Browser.js'; import {BrowserContext as BrowserContextBase} from '../../api/BrowserContext.js'; +import {Page} from '../../api/Page.js'; import {Viewport} from '../PuppeteerViewport.js'; import {BrowserContext} from './BrowserContext.js'; @@ -84,6 +85,7 @@ export class Browser extends BrowserBase { #closeCallback?: BrowserCloseCallback; #connection: Connection; #defaultViewport: Viewport | null; + #defaultContext: BrowserContext; constructor( opts: Options & { @@ -103,16 +105,26 @@ export class Browser extends BrowserBase { this.#connection.dispose(); this.emit(BrowserEmittedEvents.Disconnected); }); + this.#defaultContext = new BrowserContext(this, { + defaultViewport: this.#defaultViewport, + isDefault: true, + }); } get connection(): Connection { return this.#connection; } + override wsEndpoint(): string { + return this.#connection.url; + } + override async close(): Promise { if (this.#connection.closed) { return; } + // TODO: implement browser.close. + // await this.#connection.send('browser.close', {}); this.#connection.dispose(); await this.#closeCallback?.call(null); } @@ -128,14 +140,36 @@ export class Browser extends BrowserBase { override async createIncognitoBrowserContext( _options?: BrowserContextOptions ): Promise { + // TODO: implement incognito context https://github.com/w3c/webdriver-bidi/issues/289. return new BrowserContext(this, { defaultViewport: this.#defaultViewport, + isDefault: false, }); } override async version(): Promise { return `${this.#browserName}/${this.#browserVersion}`; } + + /** + * Returns an array of all open browser contexts. In a newly created browser, this will + * return a single instance of {@link BrowserContext}. + */ + override browserContexts(): BrowserContext[] { + // TODO: implement incognito context https://github.com/w3c/webdriver-bidi/issues/289. + return [this.#defaultContext]; + } + + /** + * Returns the default browser context. The default browser context cannot be closed. + */ + override defaultBrowserContext(): BrowserContext { + return this.#defaultContext; + } + + override newPage(): Promise { + return this.#defaultContext.newPage(); + } } interface Options { diff --git a/packages/puppeteer-core/src/common/bidi/BrowserContext.ts b/packages/puppeteer-core/src/common/bidi/BrowserContext.ts index 08386305090..4c80ba680de 100644 --- a/packages/puppeteer-core/src/common/bidi/BrowserContext.ts +++ b/packages/puppeteer-core/src/common/bidi/BrowserContext.ts @@ -18,6 +18,7 @@ import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; import {BrowserContext as BrowserContextBase} from '../../api/BrowserContext.js'; import {Page as PageBase} from '../../api/Page.js'; +import {Deferred} from '../../util/Deferred.js'; import {Viewport} from '../PuppeteerViewport.js'; import {Browser} from './Browser.js'; @@ -27,6 +28,7 @@ import {debugError} from './utils.js'; interface BrowserContextOptions { defaultViewport: Viewport | null; + isDefault: boolean; } /** @@ -38,6 +40,8 @@ export class BrowserContext extends BrowserContextBase { #defaultViewport: Viewport | null; #pages = new Map(); #onContextDestroyedBind = this.#onContextDestroyed.bind(this); + #init = Deferred.create(); + #isDefault = false; constructor(browser: Browser, options: BrowserContextOptions) { super(); @@ -48,12 +52,34 @@ export class BrowserContext extends BrowserContextBase { 'browsingContext.contextDestroyed', this.#onContextDestroyedBind ); + this.#isDefault = options.isDefault; + this.#getTree().catch(debugError); } get connection(): Connection { return this.#connection; } + async #getTree(): Promise { + if (!this.#isDefault) { + this.#init.resolve(); + return; + } + try { + const {result} = await this.#connection.send( + 'browsingContext.getTree', + {} + ); + for (const context of result.contexts) { + const page = new Page(this, context); + this.#pages.set(context.context, page); + } + this.#init.resolve(); + } catch (err) { + this.#init.reject(err as Error); + } + } + async #onContextDestroyed( event: Bidi.BrowsingContext.ContextDestroyedEvent['params'] ) { @@ -65,6 +91,8 @@ export class BrowserContext extends BrowserContextBase { } override async newPage(): Promise { + await this.#init.valueOrThrow(); + const {result} = await this.#connection.send('browsingContext.create', { type: 'tab', }); @@ -83,6 +111,8 @@ export class BrowserContext extends BrowserContextBase { } override async close(): Promise { + await this.#init.valueOrThrow(); + for (const page of this.#pages.values()) { await page?.close().catch(error => { debugError(error); @@ -96,6 +126,7 @@ export class BrowserContext extends BrowserContextBase { } override async pages(): Promise { + await this.#init.valueOrThrow(); return [...this.#pages.values()]; } } diff --git a/packages/puppeteer-core/src/common/bidi/Connection.ts b/packages/puppeteer-core/src/common/bidi/Connection.ts index ed08033c544..f21668e909a 100644 --- a/packages/puppeteer-core/src/common/bidi/Connection.ts +++ b/packages/puppeteer-core/src/common/bidi/Connection.ts @@ -140,6 +140,7 @@ interface Commands { * @internal */ export class Connection extends EventEmitter { + #url: string; #transport: ConnectionTransport; #delay: number; #timeout? = 0; @@ -147,8 +148,14 @@ export class Connection extends EventEmitter { #callbacks = new CallbackRegistry(); #browsingContexts: Map = new Map(); - constructor(transport: ConnectionTransport, delay = 0, timeout?: number) { + constructor( + url: string, + transport: ConnectionTransport, + delay = 0, + timeout?: number + ) { super(); + this.#url = url; this.#delay = delay; this.#timeout = timeout ?? 180_000; @@ -161,6 +168,10 @@ export class Connection extends EventEmitter { return this.#closed; } + get url(): string { + return this.#url; + } + send( method: T, params: Commands[T]['params'] diff --git a/packages/puppeteer-core/src/node/ProductLauncher.ts b/packages/puppeteer-core/src/node/ProductLauncher.ts index 5910cf0545a..f11f9f0c3af 100644 --- a/packages/puppeteer-core/src/node/ProductLauncher.ts +++ b/packages/puppeteer-core/src/node/ProductLauncher.ts @@ -372,6 +372,7 @@ export class ProductLauncher { /* webpackIgnore: true */ '../common/bidi/bidi.js' ); const bidiConnection = new BiDi.Connection( + browserWSEndpoint, transport, opts.slowMo, opts.protocolTimeout diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 1f79beff572..03bebd41290 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -5,6 +5,12 @@ "parameters": ["webDriverBiDi"], "expectations": ["SKIP", "TIMEOUT"] }, + { + "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests *", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["webDriverBiDi"], + "expectations": ["PASS"] + }, { "testIdPattern": "[DeviceRequestPrompt.spec] DeviceRequestPrompt *", "platforms": ["darwin", "linux", "win32"], @@ -251,36 +257,6 @@ "parameters": ["webDriverBiDi"], "expectations": ["PASS"] }, - { - "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should work when resolved right before execution context disposal", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["SKIP"] - }, - { - "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should work with strict CSP policy", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["SKIP"] - }, - { - "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should disable timeout when its set to 0", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["SKIP"] - }, - { - "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should survive cross-process navigation", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["SKIP"] - }, - { - "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should survive navigations", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["SKIP"] - }, { "testIdPattern": "[accessibility.spec] *", "platforms": ["darwin", "linux", "win32"], @@ -330,10 +306,10 @@ "expectations": ["SKIP"] }, { - "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |pipe| option should fire \"disconnected\" when closing with pipe", + "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |pipe| option *", "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["PASS"] + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["SKIP"] }, { "testIdPattern": "[Connection.spec] WebDriver BiDi Connection should work", @@ -455,12 +431,6 @@ "parameters": ["firefox"], "expectations": ["SKIP"] }, - { - "testIdPattern": "[fixtures.spec] Fixtures should close the browser when the node process closes", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["TIMEOUT"] - }, { "testIdPattern": "[frame.spec] Frame specs Frame Management should not send attach/detach events for main frame", "platforms": ["darwin", "linux", "win32"], @@ -587,24 +557,6 @@ "parameters": ["webDriverBiDi"], "expectations": ["FAIL"] }, - { - "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch should have default URL when launching browser", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, - { - "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch should launch Chrome properly with --no-startup-window and waitForInitialPage=false", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, - { - "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch should set the debugging port", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, { "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch should take fullPage screenshots when defaultViewport is null", "platforms": ["darwin", "linux", "win32"], @@ -629,18 +581,6 @@ "parameters": ["firefox"], "expectations": ["SKIP"] }, - { - "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch userDataDir option should restore cookies", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, - { - "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch userDataDir option should restore state", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, { "testIdPattern": "[navigation.spec] navigation \"after each\" hook for \"should work with both domcontentloaded and load\"", "platforms": ["darwin", "linux", "win32"], @@ -936,10 +876,34 @@ "expectations": ["SKIP"] }, { - "testIdPattern": "[tracing.spec] Tracing should throw if tracing on two pages", + "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should disable timeout when its set to 0", "platforms": ["darwin", "linux", "win32"], "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] + "expectations": ["SKIP"] + }, + { + "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should survive cross-process navigation", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["webDriverBiDi"], + "expectations": ["SKIP"] + }, + { + "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should survive navigations", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["webDriverBiDi"], + "expectations": ["SKIP"] + }, + { + "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should work when resolved right before execution context disposal", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["webDriverBiDi"], + "expectations": ["SKIP"] + }, + { + "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should work with strict CSP policy", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["webDriverBiDi"], + "expectations": ["SKIP"] }, { "testIdPattern": "[waittask.spec] waittask specs Frame.waitForSelector should be cancellable", @@ -1145,36 +1109,6 @@ "parameters": ["cdp", "firefox"], "expectations": ["FAIL", "PASS"] }, - { - "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |browserURL| option should be able to connect using browserUrl, with and without trailing slash", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "webDriverBiDi"], - "expectations": ["PASS"] - }, - { - "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |browserURL| option should be able to connect using browserUrl, with and without trailing slash", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["PASS"] - }, - { - "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |browserURL| option should throw when trying to connect to non-existing browser", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "webDriverBiDi"], - "expectations": ["PASS"] - }, - { - "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |browserURL| option should throw when trying to connect to non-existing browser", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["PASS"] - }, - { - "testIdPattern": "[chromiumonly.spec] Chromium-Specific Launcher tests Puppeteer.launch |pipe| option should fire \"disconnected\" when closing with pipe", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"] - }, { "testIdPattern": "[click.spec] Page.click should click on checkbox label and toggle", "platforms": ["darwin", "linux", "win32"], @@ -1319,6 +1253,12 @@ "parameters": ["chrome", "webDriverBiDi"], "expectations": ["FAIL"] }, + { + "testIdPattern": "[coverage.spec] Coverage specs JSCoverage shouldn't ignore eval() scripts if reportAnonymousScripts is true", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["chrome", "webDriverBiDi"], + "expectations": ["FAIL", "PASS"] + }, { "testIdPattern": "[defaultbrowsercontext.spec] DefaultBrowserContext page.deleteCookie() should work", "platforms": ["darwin", "linux", "win32"], @@ -1506,10 +1446,10 @@ "expectations": ["SKIP"] }, { - "testIdPattern": "[fixtures.spec] Fixtures dumpio option should work with pipe option", + "testIdPattern": "[fixtures.spec] Fixtures should close the browser when the node process closes", "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "webDriverBiDi"], - "expectations": ["FAIL"] + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL", "TIMEOUT"] }, { "testIdPattern": "[frame.spec] Frame specs Frame Management should report different frame instance when frame re-attaches", @@ -1739,6 +1679,12 @@ "parameters": ["cdp", "firefox"], "expectations": ["FAIL", "PASS"] }, + { + "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch should launch Chrome properly with --no-startup-window and waitForInitialPage=false", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"] + }, { "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch should launch Chrome properly with --no-startup-window and waitForInitialPage=false", "platforms": ["darwin", "linux", "win32"], @@ -1769,6 +1715,12 @@ "parameters": ["firefox", "webDriverBiDi"], "expectations": ["FAIL"] }, + { + "testIdPattern": "[launcher.spec] Launcher specs Puppeteer Puppeteer.launch userDataDir option should restore state", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["SKIP"] + }, { "testIdPattern": "[locator.spec] Locator Locator.click can be aborted", "platforms": ["darwin", "linux", "win32"], @@ -2969,6 +2921,12 @@ "parameters": ["cdp", "firefox"], "expectations": ["FAIL"] }, + { + "testIdPattern": "[tracing.spec] Tracing should throw if tracing on two pages", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"] + }, { "testIdPattern": "[waittask.spec] waittask specs Frame.waitForFunction should survive cross-process navigation", "platforms": ["darwin", "linux", "win32"], diff --git a/test/src/bidi/Connection.spec.ts b/test/src/bidi/Connection.spec.ts index 1baafa40639..2554f26e58c 100644 --- a/test/src/bidi/Connection.spec.ts +++ b/test/src/bidi/Connection.spec.ts @@ -35,7 +35,7 @@ describe('WebDriver BiDi', () => { it('should work', async () => { const transport = new TestConnectionTransport(); - const connection = new Connection(transport); + const connection = new Connection('ws://127.0.0.1', transport); const responsePromise = connection.send('session.new', { capabilities: {}, }); diff --git a/test/src/chromiumonly.spec.ts b/test/src/chromiumonly.spec.ts index ae69926f7b4..3831bd727fc 100644 --- a/test/src/chromiumonly.spec.ts +++ b/test/src/chromiumonly.spec.ts @@ -25,6 +25,8 @@ import { } from './mocha-utils.js'; import {waitEvent} from './utils.js'; +// TODO: rename this test suite to launch/connect test suite as it actually +// works across browsers. describe('Chromium-Specific Launcher tests', function () { describe('Puppeteer.launch |browserURL| option', function () { it('should be able to connect using browserUrl, with and without trailing slash', async () => {