From 1accb5141df4501961cfef475c84feaf647bd58f Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 3 Jun 2020 11:23:24 -0700 Subject: [PATCH] chore: convert more actions to Progress (#2444) --- src/dom.ts | 69 ++++++++++++++++++++++---------------- src/frames.ts | 55 +++++++++++++----------------- src/javascript.ts | 6 ++-- test/elementhandle.spec.js | 10 ++++++ test/page.spec.js | 2 +- 5 files changed, 77 insertions(+), 65 deletions(-) diff --git a/src/dom.ts b/src/dom.ts index 5e12036fba..c68a6b03d7 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -61,9 +61,9 @@ export class FrameExecutionContext extends js.ExecutionContext { } async doEvaluateInternal(returnByValue: boolean, waitForNavigations: boolean, pageFunction: string | Function, ...args: any[]): Promise { - return await this.frame._page._frameManager.waitForSignalsCreatedBy(async () => { + return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, !waitForNavigations, async () => { return this._delegate.evaluate(this, returnByValue, pageFunction, ...args); - }, Number.MAX_SAFE_INTEGER, waitForNavigations ? undefined : { noWaitAfter: true }); + }); } createHandle(remoteObject: js.RemoteObject): js.JSHandle { @@ -241,7 +241,6 @@ export class ElementHandle extends js.JSHandle { } async _retryPointerAction(progress: Progress, action: (point: types.Point) => Promise, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise { - progress.log(inputLog, progress.apiName); while (!progress.isCanceled()) { const result = await this._performPointerAction(progress, action, options); if (result === 'done') @@ -291,7 +290,7 @@ export class ElementHandle extends js.JSHandle { progress.log(inputLog, `...element does receive pointer events, continuing input action`); } - await this._page._frameManager.waitForSignalsCreatedBy(async () => { + await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { let restoreModifiers: input.Modifier[] | undefined; if (options && options.modifiers) restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers); @@ -301,7 +300,7 @@ export class ElementHandle extends js.JSHandle { progress.log(inputLog, 'waiting for scheduled navigations to finish...'); if (restoreModifiers) await this._page.keyboard._ensureModifiers(restoreModifiers); - }, progress.deadline, options, true); + }, 'input'); progress.log(inputLog, '...navigations have finished'); return 'done'; @@ -331,9 +330,12 @@ export class ElementHandle extends js.JSHandle { return this._retryPointerAction(progress, point => this._page.mouse.dblclick(point.x, point.y, options), options); } - async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options?: types.NavigatingActionWaitOptions): Promise { - this._page._log(inputLog, `elementHandle.selectOption(%s)`, values); - const deadline = this._page._timeoutSettings.computeDeadline(options); + async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise { + return Progress.runCancelableTask(progress => this._selectOption(progress, values, options), options, this._page, this._page._timeoutSettings); + } + + async _selectOption(progress: Progress, values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions): Promise { + progress.log(inputLog, progress.apiName); let vals: string[] | ElementHandle[] | types.SelectOption[]; if (!Array.isArray(values)) vals = [ values ] as (string[] | ElementHandle[] | types.SelectOption[]); @@ -350,10 +352,10 @@ export class ElementHandle extends js.JSHandle { if (option.index !== undefined) assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"'); } - return await this._page._frameManager.waitForSignalsCreatedBy(async () => { + return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { const injectedResult = await this._evaluateInUtility(({ injected, node }, selectOptions) => injected.selectOptions(node, selectOptions), selectOptions); return handleInjectedResult(injectedResult); - }, deadline, options); + }); } async fill(value: string, options: types.NavigatingActionWaitOptions = {}): Promise { @@ -363,7 +365,7 @@ export class ElementHandle extends js.JSHandle { async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise { progress.log(inputLog, `elementHandle.fill("${value}")`); assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"'); - await this._page._frameManager.waitForSignalsCreatedBy(async () => { + await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { const poll = await this._evaluateHandleInUtility(({ injected, node }, { value }) => { return injected.waitForEnabledAndFill(node, value); }, { value }); @@ -376,7 +378,7 @@ export class ElementHandle extends js.JSHandle { else await this._page.keyboard.press('Delete'); } - }, progress.deadline, options, true); + }, 'input'); } async selectText(): Promise { @@ -385,9 +387,12 @@ export class ElementHandle extends js.JSHandle { handleInjectedResult(injectedResult); } - async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options?: types.NavigatingActionWaitOptions) { - this._page._log(inputLog, `elementHandle.setInputFiles(...)`); - const deadline = this._page._timeoutSettings.computeDeadline(options); + async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}) { + return Progress.runCancelableTask(async progress => this._setInputFiles(progress, files, options), options, this._page, this._page._timeoutSettings); + } + + async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions) { + progress.log(inputLog, progress.apiName); const injectedResult = await this._evaluateInUtility(({ node }): types.InjectedScriptResult => { if (node.nodeType !== Node.ELEMENT_NODE || (node as Node as Element).tagName !== 'INPUT') return { status: 'error', error: 'Node is not an HTMLInputElement' }; @@ -416,9 +421,9 @@ export class ElementHandle extends js.JSHandle { filePayloads.push(item); } } - await this._page._frameManager.waitForSignalsCreatedBy(async () => { + await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { await this._page._delegate.setInputFiles(this as any as ElementHandle, filePayloads); - }, deadline, options); + }); } async focus() { @@ -427,22 +432,28 @@ export class ElementHandle extends js.JSHandle { handleInjectedResult(injectedResult); } - async type(text: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) { - this._page._log(inputLog, `elementHandle.type("${text}")`); - const deadline = this._page._timeoutSettings.computeDeadline(options); - await this._page._frameManager.waitForSignalsCreatedBy(async () => { - await this.focus(); - await this._page.keyboard.type(text, options); - }, deadline, options, true); + async type(text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) { + return Progress.runCancelableTask(progress => this._type(progress, text, options), options, this._page, this._page._timeoutSettings); } - async press(key: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) { - this._page._log(inputLog, `elementHandle.press("${key}")`); - const deadline = this._page._timeoutSettings.computeDeadline(options); - await this._page._frameManager.waitForSignalsCreatedBy(async () => { + async _type(progress: Progress, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions) { + progress.log(inputLog, `elementHandle.type("${text}")`); + return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { + await this.focus(); + await this._page.keyboard.type(text, options); + }, 'input'); + } + + async press(key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) { + return Progress.runCancelableTask(progress => this._press(progress, key, options), options, this._page, this._page._timeoutSettings); + } + + async _press(progress: Progress, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions) { + progress.log(inputLog, `elementHandle.press("${key}")`); + return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { await this.focus(); await this._page.keyboard.press(key, options); - }, deadline, options, true); + }, 'input'); } async check(options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) { diff --git a/src/frames.ts b/src/frames.ts index 38d21c52a1..c4ff98f2fe 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -105,23 +105,21 @@ export class FrameManager { } } - // TODO: take progress parameter. - async waitForSignalsCreatedBy(action: () => Promise, deadline: number, options: types.NavigatingActionWaitOptions = {}, input?: boolean): Promise { - if (options.noWaitAfter) + async waitForSignalsCreatedBy(progress: Progress | null, noWaitAfter: boolean | undefined, action: () => Promise, source?: 'input'): Promise { + if (noWaitAfter) return action(); - const barrier = new SignalBarrier(options, deadline); + const barrier = new SignalBarrier(progress); this._signalBarriers.add(barrier); - try { - const result = await action(); - if (input) - await this._page._delegate.inputActionEpilogue(); - await barrier.waitFor(); - // Resolve in the next task, after all waitForNavigations. - await new Promise(helper.makeWaitForNextTask()); - return result; - } finally { - this._signalBarriers.delete(barrier); - } + if (progress) + progress.cleanupWhenCanceled(() => this._signalBarriers.delete(barrier)); + const result = await action(); + if (source === 'input') + await this._page._delegate.inputActionEpilogue(); + await barrier.waitFor(); + this._signalBarriers.delete(barrier); + // Resolve in the next task, after all waitForNavigations. + await new Promise(helper.makeWaitForNextTask()); + return result; } frameWillPotentiallyRequestNavigation() { @@ -732,8 +730,7 @@ export class Frame { } async fill(selector: string, value: string, options: types.NavigatingActionWaitOptions = {}) { - await this._retryWithSelectorIfNotConnected(selector, options, - (progress, handle) => handle._fill(progress, value, options)); + await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._fill(progress, value, options)); } async focus(selector: string, options: types.TimeoutOptions = {}) { @@ -761,23 +758,19 @@ export class Frame { } async selectOption(selector: string, values: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise { - return await this._retryWithSelectorIfNotConnected(selector, options, - (progress, handle) => handle.selectOption(values, helper.optionsWithUpdatedTimeout(options, progress.deadline))); + return this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._selectOption(progress, values, options)); } async setInputFiles(selector: string, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}): Promise { - await this._retryWithSelectorIfNotConnected(selector, options, - (progress, handle) => handle.setInputFiles(files, helper.optionsWithUpdatedTimeout(options, progress.deadline))); + await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._setInputFiles(progress, files, options)); } async type(selector: string, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) { - await this._retryWithSelectorIfNotConnected(selector, options, - (progress, handle) => handle.type(text, helper.optionsWithUpdatedTimeout(options, progress.deadline))); + await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._type(progress, text, options)); } async press(selector: string, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) { - await this._retryWithSelectorIfNotConnected(selector, options, - (progress, handle) => handle.press(key, helper.optionsWithUpdatedTimeout(options, progress.deadline))); + await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._press(progress, key, options)); } async check(selector: string, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) { @@ -937,15 +930,13 @@ class RerunnableTask { } export class SignalBarrier { - private _options: types.NavigatingActionWaitOptions; + private _progress: Progress | null; private _protectCount = 0; private _promise: Promise; private _promiseCallback = () => {}; - private _deadline: number; - constructor(options: types.NavigatingActionWaitOptions, deadline: number) { - this._options = options; - this._deadline = deadline; + constructor(progress: Progress | null) { + this._progress = progress; this._promise = new Promise(f => this._promiseCallback = f); this.retain(); } @@ -957,8 +948,8 @@ export class SignalBarrier { async addFrameNavigation(frame: Frame) { this.retain(); - const options = helper.optionsWithUpdatedTimeout(this._options, this._deadline); - await frame._waitForNavigation({...options, waitUntil: 'commit'}).catch(e => {}); + const timeout = this._progress ? helper.timeUntilDeadline(this._progress.deadline) : undefined; + await frame._waitForNavigation({timeout, waitUntil: 'commit'}).catch(e => {}); this.release(); } diff --git a/src/javascript.ts b/src/javascript.ts index b1af5c9ec6..a00131260f 100644 --- a/src/javascript.ts +++ b/src/javascript.ts @@ -83,13 +83,13 @@ export class JSHandle { _disposed = false; readonly _objectId: string | undefined; readonly _value: any; - private _type: string; + private _objectType: string; constructor(context: ExecutionContext, type: string, objectId?: string, value?: any) { this._context = context; this._objectId = objectId; this._value = value; - this._type = type; + this._objectType = type; } async evaluate(pageFunction: types.FuncOn, arg: Arg): Promise; @@ -137,7 +137,7 @@ export class JSHandle { _handleToString(includeType: boolean): string { if (this._objectId) - return 'JSHandle@' + this._type; + return 'JSHandle@' + this._objectType; return (includeType ? 'JSHandle:' : '') + this._value; } diff --git a/test/elementhandle.spec.js b/test/elementhandle.spec.js index b26a2d3197..0785f02386 100644 --- a/test/elementhandle.spec.js +++ b/test/elementhandle.spec.js @@ -416,3 +416,13 @@ describe('ElementHandle.check', () => { expect(await page.evaluate(() => checkbox.checked)).toBe(false); }); }); + +describe('ElementHandle.selectOption', function() { + it('should select single option', async({page, server}) => { + await page.goto(server.PREFIX + '/input/select.html'); + const select = await page.$('select'); + await select.selectOption('blue'); + expect(await page.evaluate(() => result.onInput)).toEqual(['blue']); + expect(await page.evaluate(() => result.onChange)).toEqual(['blue']); + }); +}); diff --git a/test/page.spec.js b/test/page.spec.js index 60ed0c21f4..ddff9e9016 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -850,7 +850,7 @@ describe('Page.title', function() { }); }); -describe('Page.select', function() { +describe('Page.selectOption', function() { it('should select single option', async({page, server}) => { await page.goto(server.PREFIX + '/input/select.html'); await page.selectOption('select', 'blue');