fix: use LazyArg for puppeteer utilities (#9575)

This PR fixes the following edge case:

 - `const oldPromise = world.puppeteerUtil`. 
- setContext occurs but context is immediately destroyed, i.e.
`world.#puppeteerUtil === oldPromise` is not resolved.
- clearContext occurs due to destruction, i.e. `world.#puppeteerUtil` is
replaced (`world.#puppeteerUtil !== oldPromise`).
 - `oldPromise` never resolves.
This commit is contained in:
jrandolf 2023-01-25 23:56:33 -08:00 committed by GitHub
parent 0b717000ac
commit 496658f029
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 52 additions and 36 deletions

View File

@ -34,6 +34,7 @@ import {Page} from '../api/Page.js';
import {getQueryHandlerAndSelector} from './QueryHandler.js'; import {getQueryHandlerAndSelector} from './QueryHandler.js';
import {EvaluateFunc, HandleFor, NodeFor} from './types.js'; import {EvaluateFunc, HandleFor, NodeFor} from './types.js';
import {importFS} from './util.js'; import {importFS} from './util.js';
import {LazyArg} from './LazyArg.js';
/** /**
* @public * @public
@ -804,8 +805,9 @@ export class Frame {
type = type ?? 'text/javascript'; type = type ?? 'text/javascript';
const puppeteerWorld = this.worlds[PUPPETEER_WORLD];
return this.worlds[MAIN_WORLD].transferHandle( return this.worlds[MAIN_WORLD].transferHandle(
await this.worlds[PUPPETEER_WORLD].evaluateHandle( await puppeteerWorld.evaluateHandle(
async ({createDeferredPromise}, {url, id, type, content}) => { async ({createDeferredPromise}, {url, id, type, content}) => {
const promise = createDeferredPromise<void>(); const promise = createDeferredPromise<void>();
const script = document.createElement('script'); const script = document.createElement('script');
@ -839,7 +841,9 @@ export class Frame {
await promise; await promise;
return script; return script;
}, },
await this.worlds[PUPPETEER_WORLD].puppeteerUtil, LazyArg.create(() => {
return puppeteerWorld.puppeteerUtil;
}),
{...options, type, content} {...options, type, content}
) )
); );
@ -887,8 +891,9 @@ export class Frame {
options.content = content; options.content = content;
} }
const puppeteerWorld = this.worlds[PUPPETEER_WORLD];
return this.worlds[MAIN_WORLD].transferHandle( return this.worlds[MAIN_WORLD].transferHandle(
await this.worlds[PUPPETEER_WORLD].evaluateHandle( await puppeteerWorld.evaluateHandle(
async ({createDeferredPromise}, {url, content}) => { async ({createDeferredPromise}, {url, content}) => {
const promise = createDeferredPromise<void>(); const promise = createDeferredPromise<void>();
let element: HTMLStyleElement | HTMLLinkElement; let element: HTMLStyleElement | HTMLLinkElement;
@ -923,7 +928,9 @@ export class Frame {
await promise; await promise;
return element; return element;
}, },
await this.worlds[PUPPETEER_WORLD].puppeteerUtil, LazyArg.create(() => {
return puppeteerWorld.puppeteerUtil;
}),
options options
) )
); );

View File

@ -28,7 +28,7 @@ import {JSHandle} from './JSHandle.js';
import {LazyArg} from './LazyArg.js'; import {LazyArg} from './LazyArg.js';
import {LifecycleWatcher, PuppeteerLifeCycleEvent} from './LifecycleWatcher.js'; import {LifecycleWatcher, PuppeteerLifeCycleEvent} from './LifecycleWatcher.js';
import {TimeoutSettings} from './TimeoutSettings.js'; import {TimeoutSettings} from './TimeoutSettings.js';
import {EvaluateFunc, HandleFor, InnerLazyParams, NodeFor} from './types.js'; import {EvaluateFunc, HandleFor, NodeFor} from './types.js';
import {createJSHandle, debugError, pageBindingInitString} from './util.js'; import {createJSHandle, debugError, pageBindingInitString} from './util.js';
import {TaskManager, WaitTask} from './WaitTask.js'; import {TaskManager, WaitTask} from './WaitTask.js';
import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js'; import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js';
@ -138,8 +138,11 @@ export class IsolatedWorld {
} }
clearContext(): void { clearContext(): void {
// Only create a new promise if the old one was resolved.
if (this.#puppeteerUtil.resolved()) {
this.#puppeteerUtil = createDeferredPromise();
}
this.#document = undefined; this.#document = undefined;
this.#puppeteerUtil = createDeferredPromise();
this.#context = createDeferredPromise(); this.#context = createDeferredPromise();
} }
@ -517,13 +520,8 @@ export class IsolatedWorld {
root, root,
timeout, timeout,
}, },
new LazyArg(async () => { LazyArg.create(() => {
try { return this.puppeteerUtil;
// In case CDP fails.
return await this.puppeteerUtil;
} catch {
return undefined;
}
}), }),
queryOne.toString(), queryOne.toString(),
selector, selector,
@ -547,9 +545,7 @@ export class IsolatedWorld {
waitForFunction< waitForFunction<
Params extends unknown[], Params extends unknown[],
Func extends EvaluateFunc<InnerLazyParams<Params>> = EvaluateFunc< Func extends EvaluateFunc<Params> = EvaluateFunc<Params>
InnerLazyParams<Params>
>
>( >(
pageFunction: Func | string, pageFunction: Func | string,
options: { options: {

View File

@ -18,8 +18,14 @@
* @internal * @internal
*/ */
export class LazyArg<T> { export class LazyArg<T> {
static create = <T>(callback: () => Promise<T>): T => {
// We do type coercion here because we don't want to introduce LazyArgs to
// the type system.
return new LazyArg(callback) as unknown as T;
};
#get: () => Promise<T>; #get: () => Promise<T>;
constructor(get: () => Promise<T>) { private constructor(get: () => Promise<T>) {
this.#get = get; this.#get = get;
} }

View File

@ -15,11 +15,13 @@
*/ */
import PuppeteerUtil from '../injected/injected.js'; import PuppeteerUtil from '../injected/injected.js';
import {assert} from '../util/assert.js';
import {ariaHandler} from './AriaQueryHandler.js'; import {ariaHandler} from './AriaQueryHandler.js';
import {ElementHandle} from './ElementHandle.js'; import {ElementHandle} from './ElementHandle.js';
import {Frame} from './Frame.js'; import {Frame} from './Frame.js';
import {WaitForSelectorOptions} from './IsolatedWorld.js'; import {WaitForSelectorOptions} from './IsolatedWorld.js';
import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js'; import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js';
import {LazyArg} from './LazyArg.js';
/** /**
* @public * @public
@ -102,7 +104,11 @@ function createPuppeteerQueryHandler(
const jsHandle = await element.evaluateHandle( const jsHandle = await element.evaluateHandle(
queryOne, queryOne,
selector, selector,
await element.executionContext()._world!.puppeteerUtil LazyArg.create(() => {
const world = element.executionContext()._world;
assert(world);
return world.puppeteerUtil;
})
); );
const elementHandle = jsHandle.asElement(); const elementHandle = jsHandle.asElement();
if (elementHandle) { if (elementHandle) {
@ -148,7 +154,11 @@ function createPuppeteerQueryHandler(
const jsHandle = await element.evaluateHandle( const jsHandle = await element.evaluateHandle(
queryAll, queryAll,
selector, selector,
await element.executionContext()._world!.puppeteerUtil LazyArg.create(() => {
const world = element.executionContext()._world;
assert(world);
return world.puppeteerUtil;
})
); );
const properties = await jsHandle.getProperties(); const properties = await jsHandle.getProperties();
await jsHandle.dispose(); await jsHandle.dispose();

View File

@ -20,6 +20,7 @@ import {ElementHandle} from './ElementHandle.js';
import {TimeoutError} from './Errors.js'; import {TimeoutError} from './Errors.js';
import {IsolatedWorld} from './IsolatedWorld.js'; import {IsolatedWorld} from './IsolatedWorld.js';
import {JSHandle} from './JSHandle.js'; import {JSHandle} from './JSHandle.js';
import {LazyArg} from './LazyArg.js';
import {HandleFor} from './types.js'; import {HandleFor} from './types.js';
/** /**
@ -114,7 +115,9 @@ export class WaitTask<T = unknown> {
return fun(...args) as Promise<T>; return fun(...args) as Promise<T>;
}); });
}, },
await this.#world.puppeteerUtil, LazyArg.create(() => {
return this.#world.puppeteerUtil;
}),
this.#fn, this.#fn,
...this.#args ...this.#args
); );
@ -127,7 +130,9 @@ export class WaitTask<T = unknown> {
return fun(...args) as Promise<T>; return fun(...args) as Promise<T>;
}, root || document); }, root || document);
}, },
await this.#world.puppeteerUtil, LazyArg.create(() => {
return this.#world.puppeteerUtil;
}),
this.#root, this.#root,
this.#fn, this.#fn,
...this.#args ...this.#args
@ -141,7 +146,9 @@ export class WaitTask<T = unknown> {
return fun(...args) as Promise<T>; return fun(...args) as Promise<T>;
}, ms); }, ms);
}, },
await this.#world.puppeteerUtil, LazyArg.create(() => {
return this.#world.puppeteerUtil;
}),
this.#polling, this.#polling,
this.#fn, this.#fn,
...this.#args ...this.#args

View File

@ -16,7 +16,6 @@
import {JSHandle} from './JSHandle.js'; import {JSHandle} from './JSHandle.js';
import {ElementHandle} from './ElementHandle.js'; import {ElementHandle} from './ElementHandle.js';
import {LazyArg} from './LazyArg.js';
/** /**
* @public * @public
@ -38,18 +37,6 @@ export type HandleOr<T> = HandleFor<T> | JSHandle<T> | T;
*/ */
export type FlattenHandle<T> = T extends HandleOr<infer U> ? U : never; export type FlattenHandle<T> = T extends HandleOr<infer U> ? U : never;
/**
* @internal
*/
export type FlattenLazyArg<T> = T extends LazyArg<infer U> ? U : T;
/**
* @internal
*/
export type InnerLazyParams<T extends unknown[]> = {
[K in keyof T]: FlattenLazyArg<T[K]>;
};
/** /**
* @public * @public
*/ */

View File

@ -16,6 +16,7 @@
import expect from 'expect'; import expect from 'expect';
import {PUPPETEER_WORLD} from 'puppeteer-core/internal/common/IsolatedWorlds.js'; import {PUPPETEER_WORLD} from 'puppeteer-core/internal/common/IsolatedWorlds.js';
import {LazyArg} from 'puppeteer-core/internal/common/LazyArg.js';
import { import {
getTestState, getTestState,
setupTestBrowserHooks, setupTestBrowserHooks,
@ -45,7 +46,9 @@ describe('PuppeteerUtil tests', function () {
({createFunction}, fnString) => { ({createFunction}, fnString) => {
return createFunction(fnString)(4); return createFunction(fnString)(4);
}, },
await world.puppeteerUtil, LazyArg.create(() => {
return world.puppeteerUtil;
}),
(() => { (() => {
return 4; return 4;
}).toString() }).toString()