fix: improve reliability of exposeFunction (#11600)

This commit is contained in:
Alex Rudenko 2024-01-02 13:58:55 +01:00 committed by GitHub
parent 42b03a67d0
commit b0c5392cb3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 87 additions and 0 deletions

View File

@ -96,6 +96,11 @@ export class CdpFrame extends Frame {
updateClient(client: CDPSession, keepWorlds = false): void {
this.#client = client;
if (!keepWorlds) {
// Clear the current contexts on previous world instances.
if (this.worlds) {
this.worlds[MAIN_WORLD].clearContext();
this.worlds[PUPPETEER_WORLD].clearContext();
}
this.worlds = {
[MAIN_WORLD]: new IsolatedWorld(
this,

View File

@ -93,6 +93,8 @@ export class IsolatedWorld extends Realm {
}
clearContext(): void {
// The message has to match the CDP message expected by the WaitTask class.
this.#context?.reject(new Error('Execution context was destroyed'));
this.#context = Deferred.create();
if ('clearDocumentHandle' in this.#frameOrWorker) {
this.#frameOrWorker.clearDocumentHandle();

View File

@ -673,6 +673,9 @@ export class CdpPage extends Page {
const expression = pageBindingInitString('exposedFun', name);
await this.#primaryTargetClient.send('Runtime.addBinding', {name});
// TODO: investigate this as it appears to only apply to the main frame and
// local subframes instead of the entire frame tree (including future
// frame).
const {identifier} = await this.#primaryTargetClient.send(
'Page.addScriptToEvaluateOnNewDocument',
{
@ -684,6 +687,11 @@ export class CdpPage extends Page {
await Promise.all(
this.frames().map(frame => {
// If a frame has not started loading, it might never start. Rely on
// addScriptToEvaluateOnNewDocument in that case.
if (frame !== this.mainFrame() && !frame._hasStartedLoading) {
return;
}
return frame.evaluate(expression).catch(debugError);
})
);
@ -702,6 +710,11 @@ export class CdpPage extends Page {
await Promise.all(
this.frames().map(frame => {
// If a frame has not started loading, it might never start. Rely on
// addScriptToEvaluateOnNewDocument in that case.
if (frame !== this.mainFrame() && !frame._hasStartedLoading) {
return;
}
return frame
.evaluate(name => {
// Removes the dangling Puppeteer binding wrapper.

View File

@ -365,6 +365,13 @@ export function addPageBinding(type: string, name: string): void {
// @ts-expect-error: In a different context.
const callCdp = globalThis[name];
// Depending on the frame loading state either Runtime.evaluate or
// Page.addScriptToEvaluateOnNewDocument might succeed. Let's check that we
// don't re-wrap Puppeteer's binding.
if (callCdp[Symbol.toStringTag] === 'PuppeteerBinding') {
return;
}
// We replace the CDP binding with a Puppeteer binding.
Object.assign(globalThis, {
[name](...args: unknown[]): Promise<unknown> {
@ -404,6 +411,8 @@ export function addPageBinding(type: string, name: string): void {
});
},
});
// @ts-expect-error: In a different context.
globalThis[name][Symbol.toStringTag] = 'PuppeteerBinding';
}
/**

View File

@ -1044,6 +1044,13 @@
"parameters": ["chrome", "webDriverBiDi"],
"expectations": ["PASS"]
},
{
"testIdPattern": "[page.spec] Page Page.exposeFunction should work with loading frames",
"platforms": ["darwin", "linux", "win32"],
"parameters": ["webDriverBiDi"],
"expectations": ["SKIP"],
"comment": "Missing request interception"
},
{
"testIdPattern": "[page.spec] Page Page.pdf can print to PDF with accessible",
"platforms": ["darwin", "linux", "win32"],
@ -3127,6 +3134,12 @@
"parameters": ["cdp", "firefox"],
"expectations": ["SKIP"]
},
{
"testIdPattern": "[page.spec] Page Page.exposeFunction should work with loading frames",
"platforms": ["darwin", "linux", "win32"],
"parameters": ["cdp", "firefox"],
"expectations": ["SKIP"]
},
{
"testIdPattern": "[page.spec] Page Page.metrics metrics event fired on console.timeStamp",
"platforms": ["darwin", "linux", "win32"],

View File

@ -21,6 +21,7 @@ import path from 'path';
import expect from 'expect';
import {KnownDevices, TimeoutError} from 'puppeteer';
import {CDPSession} from 'puppeteer-core/internal/api/CDPSession.js';
import type {HTTPRequest} from 'puppeteer-core/internal/api/HTTPRequest.js';
import type {Metrics, Page} from 'puppeteer-core/internal/api/Page.js';
import type {CdpPage} from 'puppeteer-core/internal/cdp/Page.js';
import type {ConsoleMessage} from 'puppeteer-core/internal/common/ConsoleMessage.js';
@ -1177,6 +1178,50 @@ describe('Page', function () {
});
expect(result).toBe(15);
});
it('should work with loading frames', async () => {
// Tries to reproduce the scenario from
// https://github.com/puppeteer/puppeteer/issues/8106
const {page, server} = await getTestState();
await page.setRequestInterception(true);
let saveRequest: (value: HTTPRequest | PromiseLike<HTTPRequest>) => void;
const iframeRequest = new Promise<HTTPRequest>(resolve => {
saveRequest = resolve;
});
page.on('request', async req => {
if (req.url().endsWith('/frames/frame.html')) {
saveRequest(req);
} else {
await req.continue();
}
});
let error: Error | undefined;
const navPromise = page
.goto(server.PREFIX + '/frames/one-frame.html', {
waitUntil: 'networkidle0',
})
.catch(err => {
error = err;
});
const req = await iframeRequest;
// Expose function while the frame is being loaded. Loading process is
// controlled by interception.
const exposePromise = page.exposeFunction(
'compute',
function (a: number, b: number) {
return Promise.resolve(a * b);
}
);
await Promise.all([req.continue(), exposePromise]);
await navPromise;
expect(error).toBeUndefined();
const frame = page.frames()[1]!;
const result = await frame.evaluate(async function () {
return (globalThis as any).compute(3, 5);
});
expect(result).toBe(15);
});
it('should work on frames before navigation', async () => {
const {page, server} = await getTestState();