Skip to content

Commit

Permalink
fix: improve reliability of exposeFunction (#11600)
Browse files Browse the repository at this point in the history
  • Loading branch information
OrKoN committed Jan 2, 2024
1 parent 42b03a6 commit b0c5392
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 0 deletions.
5 changes: 5 additions & 0 deletions packages/puppeteer-core/src/cdp/Frame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions packages/puppeteer-core/src/cdp/IsolatedWorld.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
13 changes: 13 additions & 0 deletions packages/puppeteer-core/src/cdp/Page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
{
Expand All @@ -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);
})
);
Expand All @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions packages/puppeteer-core/src/common/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -404,6 +411,8 @@ export function addPageBinding(type: string, name: string): void {
});
},
});
// @ts-expect-error: In a different context.
globalThis[name][Symbol.toStringTag] = 'PuppeteerBinding';
}

/**
Expand Down
13 changes: 13 additions & 0 deletions test/TestExpectations.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down Expand Up @@ -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"],
Expand Down
45 changes: 45 additions & 0 deletions test/src/page.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit b0c5392

Please sign in to comment.