Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow handling other targets as pages internally #8336

Merged
merged 2 commits into from May 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 32 additions & 4 deletions src/common/Browser.ts
Expand Up @@ -54,6 +54,13 @@ export type TargetFilterCallback = (
target: Protocol.Target.TargetInfo
) => boolean;

/**
* @internal
*/
export type IsPageTargetCallback = (
target: Protocol.Target.TargetInfo
) => boolean;

const WEB_PERMISSION_TO_PROTOCOL_PERMISSION = new Map<
Permission,
Protocol.Browser.PermissionType
Expand Down Expand Up @@ -217,7 +224,8 @@ export class Browser extends EventEmitter {
defaultViewport?: Viewport | null,
process?: ChildProcess,
closeCallback?: BrowserCloseCallback,
targetFilterCallback?: TargetFilterCallback
targetFilterCallback?: TargetFilterCallback,
isPageTargetCallback?: IsPageTargetCallback
): Promise<Browser> {
const browser = new Browser(
connection,
Expand All @@ -226,7 +234,8 @@ export class Browser extends EventEmitter {
defaultViewport,
process,
closeCallback,
targetFilterCallback
targetFilterCallback,
isPageTargetCallback
);
await connection.send('Target.setDiscoverTargets', { discover: true });
return browser;
Expand All @@ -237,6 +246,7 @@ export class Browser extends EventEmitter {
private _connection: Connection;
private _closeCallback: BrowserCloseCallback;
private _targetFilterCallback: TargetFilterCallback;
private _isPageTargetCallback: IsPageTargetCallback;
private _defaultContext: BrowserContext;
private _contexts: Map<string, BrowserContext>;
private _screenshotTaskQueue: TaskQueue;
Expand All @@ -257,7 +267,8 @@ export class Browser extends EventEmitter {
defaultViewport?: Viewport | null,
process?: ChildProcess,
closeCallback?: BrowserCloseCallback,
targetFilterCallback?: TargetFilterCallback
targetFilterCallback?: TargetFilterCallback,
isPageTargetCallback?: IsPageTargetCallback
) {
super();
this._ignoreHTTPSErrors = ignoreHTTPSErrors;
Expand All @@ -267,6 +278,7 @@ export class Browser extends EventEmitter {
this._connection = connection;
this._closeCallback = closeCallback || function (): void {};
this._targetFilterCallback = targetFilterCallback || ((): boolean => true);
this._setIsPageTargetCallback(isPageTargetCallback);

this._defaultContext = new BrowserContext(this._connection, this);
this._contexts = new Map();
Expand Down Expand Up @@ -299,6 +311,21 @@ export class Browser extends EventEmitter {
return this._process ?? null;
}

/**
* @internal
*/
_setIsPageTargetCallback(isPageTargetCallback?: IsPageTargetCallback): void {
this._isPageTargetCallback =
isPageTargetCallback ||
((target: Protocol.Target.TargetInfo): boolean => {
return (
target.type === 'page' ||
target.type === 'background_page' ||
target.type === 'webview'
);
});
}

/**
* Creates a new incognito browser context. This won't share cookies/cache with other
* browser contexts.
Expand Down Expand Up @@ -392,7 +419,8 @@ export class Browser extends EventEmitter {
() => this._connection.createSession(targetInfo),
this._ignoreHTTPSErrors,
this._defaultViewport ?? null,
this._screenshotTaskQueue
this._screenshotTaskQueue,
this._isPageTargetCallback
);
assert(
!this._targets.has(event.targetInfo.targetId),
Expand Down
14 changes: 12 additions & 2 deletions src/common/BrowserConnector.ts
Expand Up @@ -15,7 +15,11 @@
*/

import { ConnectionTransport } from './ConnectionTransport.js';
import { Browser, TargetFilterCallback } from './Browser.js';
import {
Browser,
TargetFilterCallback,
IsPageTargetCallback,
} from './Browser.js';
import { assert } from './assert.js';
import { debugError } from '../common/helper.js';
import { Connection } from './Connection.js';
Expand Down Expand Up @@ -47,6 +51,10 @@ export interface BrowserConnectOptions {
* Callback to decide if Puppeteer should connect to a given target or not.
*/
targetFilter?: TargetFilterCallback;
/**
* @internal
*/
isPageTarget?: IsPageTargetCallback;
}

const getWebSocketTransportClass = async () => {
Expand Down Expand Up @@ -76,6 +84,7 @@ export const connectToBrowser = async (
transport,
slowMo = 0,
targetFilter,
isPageTarget,
} = options;

assert(
Expand Down Expand Up @@ -110,7 +119,8 @@ export const connectToBrowser = async (
defaultViewport,
null,
() => connection.send('Browser.close').catch(debugError),
targetFilter
targetFilter,
isPageTarget
);
};

Expand Down
23 changes: 13 additions & 10 deletions src/common/Target.ts
Expand Up @@ -17,7 +17,7 @@
import { Page, PageEmittedEvents } from './Page.js';
import { WebWorker } from './WebWorker.js';
import { CDPSession } from './Connection.js';
import { Browser, BrowserContext } from './Browser.js';
import { Browser, BrowserContext, IsPageTargetCallback } from './Browser.js';
import { Viewport } from './PuppeteerViewport.js';
import { Protocol } from 'devtools-protocol';
import { TaskQueue } from './TaskQueue.js';
Expand Down Expand Up @@ -59,6 +59,10 @@ export class Target {
* @internal
*/
_targetId: string;
/**
* @internal
*/
_isPageTargetCallback: IsPageTargetCallback;

/**
* @internal
Expand All @@ -69,7 +73,8 @@ export class Target {
sessionFactory: () => Promise<CDPSession>,
ignoreHTTPSErrors: boolean,
defaultViewport: Viewport | null,
screenshotTaskQueue: TaskQueue
screenshotTaskQueue: TaskQueue,
isPageTargetCallback: IsPageTargetCallback
) {
this._targetInfo = targetInfo;
this._browserContext = browserContext;
Expand All @@ -78,6 +83,7 @@ export class Target {
this._ignoreHTTPSErrors = ignoreHTTPSErrors;
this._defaultViewport = defaultViewport;
this._screenshotTaskQueue = screenshotTaskQueue;
this._isPageTargetCallback = isPageTargetCallback;
/** @type {?Promise<!Puppeteer.Page>} */
this._pagePromise = null;
/** @type {?Promise<!WebWorker>} */
Expand All @@ -99,7 +105,8 @@ export class Target {
(fulfill) => (this._closedCallback = fulfill)
);
this._isInitialized =
this._targetInfo.type !== 'page' || this._targetInfo.url !== '';
!this._isPageTargetCallback(this._targetInfo) ||
this._targetInfo.url !== '';
if (this._isInitialized) this._initializedCallback(true);
}

Expand All @@ -114,12 +121,7 @@ export class Target {
* If the target is not of type `"page"` or `"background_page"`, returns `null`.
*/
async page(): Promise<Page | null> {
if (
(this._targetInfo.type === 'page' ||
this._targetInfo.type === 'background_page' ||
this._targetInfo.type === 'webview') &&
!this._pagePromise
) {
if (this._isPageTargetCallback(this._targetInfo) && !this._pagePromise) {
this._pagePromise = this._sessionFactory().then((client) =>
Page.create(
client,
Expand Down Expand Up @@ -220,7 +222,8 @@ export class Target {

if (
!this._isInitialized &&
(this._targetInfo.type !== 'page' || this._targetInfo.url !== '')
(!this._isPageTargetCallback(this._targetInfo) ||
this._targetInfo.url !== '')
) {
this._isInitialized = true;
this._initializedCallback(true);
Expand Down
28 changes: 28 additions & 0 deletions test/headful.spec.ts
Expand Up @@ -43,6 +43,7 @@ describeChromeOnly('headful tests', function () {
let headlessOptions;
let extensionOptions;
let forcedOopifOptions;
let devtoolsOptions;
const browsers = [];

beforeEach(() => {
Expand Down Expand Up @@ -73,6 +74,11 @@ describeChromeOnly('headful tests', function () {
)}`,
],
});

devtoolsOptions = Object.assign({}, defaultBrowserOptions, {
headless: false,
devtools: true,
});
});

async function launchBrowser(puppeteer, options) {
Expand Down Expand Up @@ -120,6 +126,28 @@ describeChromeOnly('headful tests', function () {
expect(await page.evaluate(() => globalThis.MAGIC)).toBe(42);
await browserWithExtension.close();
});
it('target.page() should return a DevTools page if custom isPageTarget is provided', async function () {
const { puppeteer } = getTestState();
const originalBrowser = await launchBrowser(puppeteer, devtoolsOptions);

const browserWSEndpoint = originalBrowser.wsEndpoint();

const browser = await puppeteer.connect({
browserWSEndpoint,
isPageTarget: (target) => {
return (
target.type === 'other' && target.url.startsWith('devtools://')
);
},
});
const devtoolsPageTarget = await browser.waitForTarget(
(target) => target.type() === 'other'
);
console.log(devtoolsPageTarget);
const page = await devtoolsPageTarget.page();
expect(await page.evaluate(() => 2 * 3)).toBe(6);
await browser.close();
});
it('should have default url when launching browser', async function () {
const { puppeteer } = getTestState();
const browser = await launchBrowser(puppeteer, extensionOptions);
Expand Down