fix(dispose): do not await inner handle dispose (#1230)

This commit is contained in:
Dmitry Gozman 2020-03-04 17:57:35 -08:00 committed by GitHub
parent 5ee744cd26
commit 3bedc60b2b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 34 additions and 46 deletions

View File

@ -483,7 +483,7 @@ export class CRPage implements PageDelegate {
}); });
const frameId = nodeInfo && typeof nodeInfo.node.frameId === 'string' ? const frameId = nodeInfo && typeof nodeInfo.node.frameId === 'string' ?
nodeInfo.node.frameId : null; nodeInfo.node.frameId : null;
await documentElement.dispose(); documentElement.dispose();
return frameId; return frameId;
} }

View File

@ -15,7 +15,7 @@
* limitations under the License. * limitations under the License.
*/ */
import { assert, debugError } from '../helper'; import { assert } from '../helper';
import { CRSession } from './crConnection'; import { CRSession } from './crConnection';
import { Protocol } from './protocol'; import { Protocol } from './protocol';
import * as platform from '../platform'; import * as platform from '../platform';
@ -58,11 +58,7 @@ export function valueFromRemoteObject(remoteObject: Protocol.Runtime.RemoteObjec
export async function releaseObject(client: CRSession, remoteObject: Protocol.Runtime.RemoteObject) { export async function releaseObject(client: CRSession, remoteObject: Protocol.Runtime.RemoteObject) {
if (!remoteObject.objectId) if (!remoteObject.objectId)
return; return;
await client.send('Runtime.releaseObject', {objectId: remoteObject.objectId}).catch(error => { await client.send('Runtime.releaseObject', {objectId: remoteObject.objectId}).catch(error => {});
// Exceptions might happen in case of a page been navigated or closed.
// Swallow these since they are harmless and we don't leak anything in this case.
debugError(error);
});
} }
export async function readProtocolStream(client: CRSession, handle: string, path: string | null): Promise<platform.BufferType> { export async function readProtocolStream(client: CRSession, handle: string, path: string | null): Promise<platform.BufferType> {

View File

@ -67,7 +67,7 @@ export class FrameExecutionContext extends js.ExecutionContext {
try { try {
result = await this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted); result = await this._delegate.evaluate(this, returnByValue, pageFunction, ...adopted);
} finally { } finally {
await Promise.all(toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose()))); toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose()));
} }
return result; return result;
} }
@ -105,7 +105,7 @@ export class FrameExecutionContext extends js.ExecutionContext {
await this._injected(), selector, scope await this._injected(), selector, scope
); );
if (!handle.asElement()) if (!handle.asElement())
await handle.dispose(); handle.dispose();
return handle.asElement() as ElementHandle<Element>; return handle.asElement() as ElementHandle<Element>;
} }
@ -120,14 +120,14 @@ export class FrameExecutionContext extends js.ExecutionContext {
async _$$(selector: string, scope?: ElementHandle): Promise<ElementHandle<Element>[]> { async _$$(selector: string, scope?: ElementHandle): Promise<ElementHandle<Element>[]> {
const arrayHandle = await this._$array(selector, scope); const arrayHandle = await this._$array(selector, scope);
const properties = await arrayHandle.getProperties(); const properties = await arrayHandle.getProperties();
await arrayHandle.dispose(); arrayHandle.dispose();
const result: ElementHandle<Element>[] = []; const result: ElementHandle<Element>[] = [];
for (const property of properties.values()) { for (const property of properties.values()) {
const elementHandle = property.asElement() as ElementHandle<Element>; const elementHandle = property.asElement() as ElementHandle<Element>;
if (elementHandle) if (elementHandle)
result.push(elementHandle); result.push(elementHandle);
else else
await property.dispose(); property.dispose();
} }
return result; return result;
} }
@ -378,14 +378,14 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
if (!elementHandle) if (!elementHandle)
throw new Error(`Error: failed to find element matching selector "${selector}"`); throw new Error(`Error: failed to find element matching selector "${selector}"`);
const result = await elementHandle.evaluate(pageFunction, ...args as any); const result = await elementHandle.evaluate(pageFunction, ...args as any);
await elementHandle.dispose(); elementHandle.dispose();
return result; return result;
} }
$$eval: types.$$Eval = async (selector, pageFunction, ...args) => { $$eval: types.$$Eval = async (selector, pageFunction, ...args) => {
const arrayHandle = await this._context._$array(selector, this); const arrayHandle = await this._context._$array(selector, this);
const result = await arrayHandle.evaluate(pageFunction, ...args as any); const result = await arrayHandle.evaluate(pageFunction, ...args as any);
await arrayHandle.dispose(); arrayHandle.dispose();
return result; return result;
} }

View File

@ -15,7 +15,7 @@
* limitations under the License. * limitations under the License.
*/ */
import {helper, debugError} from '../helper'; import { helper } from '../helper';
import * as js from '../javascript'; import * as js from '../javascript';
import { FFSession } from './ffConnection'; import { FFSession } from './ffConnection';
import { Protocol } from './protocol'; import { Protocol } from './protocol';
@ -122,11 +122,7 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
await this._session.send('Runtime.disposeObject', { await this._session.send('Runtime.disposeObject', {
executionContextId: this._executionContextId, executionContextId: this._executionContextId,
objectId: handle._remoteObject.objectId, objectId: handle._remoteObject.objectId,
}).catch(error => { }).catch(error => {});
// Exceptions might happen in case of a page been navigated or closed.
// Swallow these since they are harmless and we don't leak anything in this case.
debugError(error);
});
} }
async handleJSONValue<T>(handle: js.JSHandle<T>): Promise<T> { async handleJSONValue<T>(handle: js.JSHandle<T>): Promise<T> {

View File

@ -447,7 +447,7 @@ export class FFPage implements PageDelegate {
return { handle, frame }; return { handle, frame };
})); }));
const result = items.find(item => item.frame === frame); const result = items.find(item => item.frame === frame);
await Promise.all(items.map(item => item === result ? Promise.resolve() : item.handle.dispose())); items.map(item => item === result ? Promise.resolve() : item.handle.dispose());
if (!result) if (!result)
throw new Error('Frame has been detached.'); throw new Error('Frame has been detached.');
return result.handle; return result.handle;

View File

@ -549,7 +549,7 @@ export class Frame {
const handle = await utilityContext._$(selector); const handle = await utilityContext._$(selector);
if (handle && handle._context !== mainContext) { if (handle && handle._context !== mainContext) {
const adopted = this._page._delegate.adoptElementHandle(handle, mainContext); const adopted = this._page._delegate.adoptElementHandle(handle, mainContext);
await handle.dispose(); handle.dispose();
return adopted; return adopted;
} }
return handle; return handle;
@ -561,7 +561,7 @@ export class Frame {
const mainContext = await this._mainContext(); const mainContext = await this._mainContext();
if (handle && handle._context !== mainContext) { if (handle && handle._context !== mainContext) {
const adopted = this._page._delegate.adoptElementHandle(handle, mainContext); const adopted = this._page._delegate.adoptElementHandle(handle, mainContext);
await handle.dispose(); handle.dispose();
return adopted; return adopted;
} }
return handle; return handle;
@ -577,7 +577,7 @@ export class Frame {
if (!elementHandle) if (!elementHandle)
throw new Error(`Error: failed to find element matching selector "${selector}"`); throw new Error(`Error: failed to find element matching selector "${selector}"`);
const result = await elementHandle.evaluate(pageFunction, ...args as any); const result = await elementHandle.evaluate(pageFunction, ...args as any);
await elementHandle.dispose(); elementHandle.dispose();
return result; return result;
} }
@ -585,7 +585,7 @@ export class Frame {
const context = await this._mainContext(); const context = await this._mainContext();
const arrayHandle = await context._$array(selector); const arrayHandle = await context._$array(selector);
const result = await arrayHandle.evaluate(pageFunction, ...args as any); const result = await arrayHandle.evaluate(pageFunction, ...args as any);
await arrayHandle.dispose(); arrayHandle.dispose();
return result; return result;
} }
@ -782,63 +782,63 @@ export class Frame {
async click(selector: string, options?: dom.ClickOptions & types.WaitForOptions) { async click(selector: string, options?: dom.ClickOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.click(options); await handle.click(options);
await handle.dispose(); handle.dispose();
} }
async dblclick(selector: string, options?: dom.MultiClickOptions & types.WaitForOptions) { async dblclick(selector: string, options?: dom.MultiClickOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.dblclick(options); await handle.dblclick(options);
await handle.dispose(); handle.dispose();
} }
async tripleclick(selector: string, options?: dom.MultiClickOptions & types.WaitForOptions) { async tripleclick(selector: string, options?: dom.MultiClickOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.tripleclick(options); await handle.tripleclick(options);
await handle.dispose(); handle.dispose();
} }
async fill(selector: string, value: string, options?: types.WaitForOptions) { async fill(selector: string, value: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.fill(value); await handle.fill(value);
await handle.dispose(); handle.dispose();
} }
async focus(selector: string, options?: types.WaitForOptions) { async focus(selector: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.focus(); await handle.focus();
await handle.dispose(); handle.dispose();
} }
async hover(selector: string, options?: dom.PointerActionOptions & types.WaitForOptions) { async hover(selector: string, options?: dom.PointerActionOptions & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.hover(options); await handle.hover(options);
await handle.dispose(); handle.dispose();
} }
async select(selector: string, value: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[], options?: types.WaitForOptions): Promise<string[]> { async select(selector: string, value: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[], options?: types.WaitForOptions): Promise<string[]> {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
const values = Array.isArray(value) ? value : [value]; const values = Array.isArray(value) ? value : [value];
const result = await handle.select(...values); const result = await handle.select(...values);
await handle.dispose(); handle.dispose();
return result; return result;
} }
async type(selector: string, text: string, options?: { delay?: number } & types.WaitForOptions) { async type(selector: string, text: string, options?: { delay?: number } & types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.type(text, options); await handle.type(text, options);
await handle.dispose(); handle.dispose();
} }
async check(selector: string, options?: types.WaitForOptions) { async check(selector: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.check(options); await handle.check(options);
await handle.dispose(); handle.dispose();
} }
async uncheck(selector: string, options?: types.WaitForOptions) { async uncheck(selector: string, options?: types.WaitForOptions) {
const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options); const handle = await this._optionallyWaitForSelectorInUtilityContext(selector, options);
await handle.uncheck(options); await handle.uncheck(options);
await handle.dispose(); handle.dispose();
} }
async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options: types.WaitForFunctionOptions & { visibility?: types.Visibility } = {}, ...args: any[]): Promise<js.JSHandle | null> { async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options: types.WaitForFunctionOptions & { visibility?: types.Visibility } = {}, ...args: any[]): Promise<js.JSHandle | null> {
@ -879,7 +879,7 @@ export class Frame {
const task = dom.waitForSelectorTask(selector, visibility, timeout); const task = dom.waitForSelectorTask(selector, visibility, timeout);
const result = await this._scheduleRerunnableTask(task, 'utility', timeout, `selector "${selectorToString(selector, visibility)}"`); const result = await this._scheduleRerunnableTask(task, 'utility', timeout, `selector "${selectorToString(selector, visibility)}"`);
if (!result.asElement()) { if (!result.asElement()) {
await result.dispose(); result.dispose();
return null; return null;
} }
return result.asElement() as dom.ElementHandle<Element>; return result.asElement() as dom.ElementHandle<Element>;
@ -993,7 +993,7 @@ class RerunnableTask {
if (this._terminated || runCount !== this._runCount) { if (this._terminated || runCount !== this._runCount) {
if (success) if (success)
await success.dispose(); success.dispose();
return; return;
} }
@ -1001,7 +1001,7 @@ class RerunnableTask {
// If execution context has been already destroyed, `context.evaluate` will // If execution context has been already destroyed, `context.evaluate` will
// throw an error - ignore this predicate run altogether. // throw an error - ignore this predicate run altogether.
if (!error && await context.evaluate(s => !s, success).catch(e => true)) { if (!error && await context.evaluate(s => !s, success).catch(e => true)) {
await success!.dispose(); success!.dispose();
return; return;
} }

View File

@ -75,7 +75,7 @@ export class JSHandle<T = any> {
}, propertyName); }, propertyName);
const properties = await objectHandle.getProperties(); const properties = await objectHandle.getProperties();
const result = properties.get(propertyName) || null; const result = properties.get(propertyName) || null;
await objectHandle.dispose(); objectHandle.dispose();
return result; return result;
} }

View File

@ -187,7 +187,7 @@ export class Page extends platform.EventEmitter {
async _onFileChooserOpened(handle: dom.ElementHandle) { async _onFileChooserOpened(handle: dom.ElementHandle) {
const multiple = await handle.evaluate(element => !!(element as HTMLInputElement).multiple); const multiple = await handle.evaluate(element => !!(element as HTMLInputElement).multiple);
if (!this.listenerCount(Events.Page.FileChooser)) { if (!this.listenerCount(Events.Page.FileChooser)) {
await handle.dispose(); handle.dispose();
return; return;
} }
const fileChooser: FileChooser = { element: handle, multiple }; const fileChooser: FileChooser = { element: handle, multiple };

View File

@ -630,7 +630,7 @@ export class WKPage implements PageDelegate {
return { handle, frame }; return { handle, frame };
})); }));
const result = items.find(item => item.frame === frame); const result = items.find(item => item.frame === frame);
await Promise.all(items.map(item => item === result ? Promise.resolve() : item.handle.dispose())); items.map(item => item === result ? Promise.resolve() : item.handle.dispose());
if (!result) if (!result)
throw new Error('Frame has been detached.'); throw new Error('Frame has been detached.');
return result.handle; return result.handle;

View File

@ -15,7 +15,7 @@
* limitations under the License. * limitations under the License.
*/ */
import { assert, debugError } from '../helper'; import { assert } from '../helper';
import { WKSession } from './wkConnection'; import { WKSession } from './wkConnection';
import { Protocol } from './protocol'; import { Protocol } from './protocol';
@ -46,10 +46,6 @@ export function valueFromRemoteObject(remoteObject: Protocol.Runtime.RemoteObjec
export async function releaseObject(client: WKSession, remoteObject: Protocol.Runtime.RemoteObject) { export async function releaseObject(client: WKSession, remoteObject: Protocol.Runtime.RemoteObject) {
if (!remoteObject.objectId) if (!remoteObject.objectId)
return; return;
await client.send('Runtime.releaseObject', {objectId: remoteObject.objectId}).catch(error => { await client.send('Runtime.releaseObject', {objectId: remoteObject.objectId}).catch(error => {});
// Exceptions might happen in case of a page been navigated or closed.
// Swallow these since they are harmless and we don't leak anything in this case.
debugError(error);
});
} }