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

deps: upgrade puppeteer to v21.3.6 #15490

Merged
merged 12 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
// resolved eventually.
plugins.partialLoaders.inlineFs({
verbose: Boolean(process.env.DEBUG),
ignorePaths: [require.resolve('puppeteer-core/lib/esm/puppeteer/common/Page.js')],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Page.js has moved but it looks like the comment that contained fs.readFileSync is not longer there so we should be good to remove.

}),
plugins.partialLoaders.rmGetModuleDirectory,
plugins.partialLoaders.replaceText({
Expand Down
12 changes: 0 additions & 12 deletions cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,18 +485,6 @@ const expectations = {
}],
},
},
{
// Support for this was added in M109
// https://crbug.com/1350944
_maxChromiumVersion: '108',
reason: 'Pages that have requested notifications permissions are not currently eligible for back/forward cache.',
failureType: 'Pending browser support',
subItems: {
items: [{
frameUrl: 'http://localhost:10200/dobetterweb/dbw_tester.html',
}],
},
},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive-by outdated bfcache failure

{
// This issue only appears in the DevTools runner for some reason.
// TODO: Investigate why this doesn't happen on the CLI runner.
Expand Down
6 changes: 3 additions & 3 deletions clients/lightrider/lightrider-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import {Buffer} from 'buffer';

import log from 'lighthouse-logger';
import {CDPBrowser} from 'puppeteer-core/lib/esm/puppeteer/common/Browser.js';
import {Connection as PptrConnection} from 'puppeteer-core/lib/esm/puppeteer/common/Connection.js';
import {CdpBrowser} from 'puppeteer-core/lib/esm/puppeteer/cdp/Browser.js';
import {Connection as PptrConnection} from 'puppeteer-core/lib/esm/puppeteer/cdp/Connection.js';

import lighthouse, * as api from '../../core/index.js';
import {LighthouseError} from '../../core/lib/lh-error.js';
Expand Down Expand Up @@ -46,7 +46,7 @@ async function getPageFromConnection(connection) {

const pptrConnection = new PptrConnection(mainTargetInfo.url, transport);

const browser = await CDPBrowser._create(
const browser = await CdpBrowser._create(
'chrome',
pptrConnection,
[] /* contextIds */,
Expand Down
2 changes: 2 additions & 0 deletions core/gather/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class ProtocolSession extends CrdpEventEmitter {
this._nextProtocolTimeout = undefined;

this._handleProtocolEvent = this._handleProtocolEvent.bind(this);
// @ts-expect-error Puppeteer expects the handler params to be type `unknown`
this._cdpSession.on('*', this._handleProtocolEvent);
}

Expand Down Expand Up @@ -106,6 +107,7 @@ class ProtocolSession extends CrdpEventEmitter {
* @return {Promise<void>}
*/
async dispose() {
// @ts-expect-error Puppeteer expects the handler params to be type `unknown`
this._cdpSession.off('*', this._handleProtocolEvent);
await this._cdpSession.detach();
}
Expand Down
17 changes: 11 additions & 6 deletions core/scripts/pptr-run-devtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {fileURLToPath} from 'url';
import * as puppeteer from 'puppeteer-core';
import yargs from 'yargs';
import * as yargsHelpers from 'yargs/helpers';
import {getChromePath} from 'chrome-launcher';
import {launch} from 'chrome-launcher';
import esMain from 'es-main';

import {parseChromeFlags} from '../../cli/run.js';
Expand Down Expand Up @@ -300,12 +300,17 @@ function dismissDialog(dialog) {
* @return {Promise<{lhr: LH.Result, artifacts: LH.Artifacts, logs: string[]}>}
*/
async function testUrlFromDevtools(url, options = {}) {
const {config, chromeFlags} = options;
const {config, chromeFlags = []} = options;

const browser = await puppeteer.launch({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puppeteer will now disable the bfcache if we aren't using tab target which is currently only available through an environment variable PUPPETEER_INTERNAL_TAB_TARGET. The env variable needs to be set before puppeteer is imported so we would need to do some annoying dynamic imports.

This is an alternative approach which just uses chrome-launcher with our desired flags. This should be better for our smoke tests in general because it's closer to how Lighthouse launches Chrome.

executablePath: getChromePath(),
args: chromeFlags,
devtools: true,
const newChromeFlags = [
...chromeFlags,
'--auto-open-devtools-for-tabs',
];

const chrome = await launch({chromeFlags: newChromeFlags});

const browser = await puppeteer.connect({
browserURL: `http://127.0.0.1:${chrome.port}`,
defaultViewport: null,
});

Expand Down
6 changes: 3 additions & 3 deletions core/test/gather/driver/target-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import {EventEmitter} from 'events';

import {CDPSessionImpl} from 'puppeteer-core/lib/cjs/puppeteer/common/Connection.js';
import {CdpCDPSession} from 'puppeteer-core/lib/cjs/puppeteer/cdp/CDPSession.js';

import {TargetManager} from '../../../gather/driver/target-manager.js';
import {createMockCdpSession} from '../mock-driver.js';
Expand Down Expand Up @@ -251,9 +251,9 @@ describe('TargetManager', () => {
}

const mockCdpConnection = new MockCdpConnection();
/** @type {LH.Puppeteer.CDPSession} */
/** @type {import('puppeteer-core').CDPSession} */
// @ts-expect-error - close enough to the real thing.
const cdpSession = new CDPSessionImpl(mockCdpConnection, '', sessionId);
const cdpSession = new CdpCDPSession(mockCdpConnection, '', sessionId);
return cdpSession;
}

Expand Down
48 changes: 36 additions & 12 deletions core/test/gather/session-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import {EventEmitter} from 'events';

import {CDPSessionImpl} from 'puppeteer-core/lib/cjs/puppeteer/common/Connection.js';
import {CdpCDPSession} from 'puppeteer-core/lib/cjs/puppeteer/cdp/CDPSession.js';

import {ProtocolSession} from '../../gather/session.js';
import {
Expand All @@ -17,6 +17,27 @@ import {
timers,
} from '../test-utils.js';

/**
* @param {number} id
* @return {LH.Crdp.Page.FrameNavigatedEvent}
*/
function mockFrameNavigated(id) {
return {
frame: {
id: String(id),
loaderId: String(id),
url: `https://example.com/page${id}`,
domainAndRegistry: 'example.com',
securityOrigin: 'https://example.com',
mimeType: 'text/html',
secureContextType: 'Secure',
crossOriginIsolatedContextType: 'NotIsolated',
gatedAPIFeatures: [],
},
type: 'Navigation',
};
}

describe('ProtocolSession', () => {
before(() => timers.useFakeTimers());
after(() => timers.dispose());
Expand All @@ -30,46 +51,49 @@ describe('ProtocolSession', () => {

beforeEach(() => {
// @ts-expect-error - Individual mock functions are applied as necessary.
puppeteerSession = new CDPSessionImpl({_rawSend: fnAny(), send: fnAny()}, '', 'root');
puppeteerSession = new CdpCDPSession({_rawSend: fnAny(), send: fnAny()}, '', 'root');
session = new ProtocolSession(puppeteerSession);
});

describe('responds to events from the underlying CDPSession', () => {
const mockNavigated1 = mockFrameNavigated(1);
const mockNavigated2 = mockFrameNavigated(2);

it('once', async () => {
const callback = fnAny();

session.once('Page.frameNavigated', callback);
puppeteerSession.emit('Page.frameNavigated', {id: 1});
puppeteerSession.emit('Page.frameNavigated', mockNavigated1);
expect(callback).toHaveBeenCalledTimes(1);
expect(callback).toHaveBeenCalledWith({id: 1});
expect(callback).toHaveBeenCalledWith(mockNavigated1);

puppeteerSession.emit('Page.frameNavigated', {id: 2});
puppeteerSession.emit('Page.frameNavigated', mockNavigated2);
expect(callback).toHaveBeenCalledTimes(1);
});

it('on', async () => {
const callback = fnAny();

session.on('Page.frameNavigated', callback);
puppeteerSession.emit('Page.frameNavigated', {id: 1});
puppeteerSession.emit('Page.frameNavigated', mockNavigated1);
expect(callback).toHaveBeenCalledTimes(1);
expect(callback).toHaveBeenCalledWith({id: 1});
expect(callback).toHaveBeenCalledWith(mockNavigated1);

puppeteerSession.emit('Page.frameNavigated', {id: 2});
puppeteerSession.emit('Page.frameNavigated', mockNavigated2);
expect(callback).toHaveBeenCalledTimes(2);
expect(callback).toHaveBeenCalledWith({id: 2});
expect(callback).toHaveBeenCalledWith(mockNavigated2);
});

it('off', async () => {
const callback = fnAny();

session.on('Page.frameNavigated', callback);
puppeteerSession.emit('Page.frameNavigated', {id: 1});
puppeteerSession.emit('Page.frameNavigated', mockNavigated1);
expect(callback).toHaveBeenCalledTimes(1);
expect(callback).toHaveBeenCalledWith({id: 1});
expect(callback).toHaveBeenCalledWith(mockNavigated1);

session.off('Page.frameNavigated', callback);
puppeteerSession.emit('Page.frameNavigated', {id: 2});
puppeteerSession.emit('Page.frameNavigated', mockNavigated2);
expect(callback).toHaveBeenCalledTimes(1);
});
});
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
"pako": "^2.0.3",
"preact": "^10.7.2",
"pretty-json-stringify": "^0.0.2",
"puppeteer": "21.1.1",
"puppeteer": "^21.3.4",
"resolve": "^1.22.1",
"rollup": "^2.52.7",
"rollup-plugin-polyfill-node": "^0.12.0",
Expand Down Expand Up @@ -199,7 +199,7 @@
"open": "^8.4.0",
"parse-cache-control": "1.0.1",
"ps-list": "^8.0.0",
"puppeteer-core": "21.1.1",
"puppeteer-core": "^21.3.4",
"robots-parser": "^3.0.1",
"semver": "^5.3.0",
"speedline-core": "^1.4.3",
Expand Down
3 changes: 3 additions & 0 deletions types/internal/rxjs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,8 @@ declare module 'rxjs' {
export const tap: any;
export const throwIfEmpty: any;
export const firstValueFrom: any;
export const delay: any;
export const startWith: any;
export const switchMap: any;
}

Loading
Loading