Skip to content

Commit 1accb51

Browse files
dgozmandgozman
authored
chore: convert more actions to Progress (#2444)
1 parent f188b0a commit 1accb51

File tree

5 files changed

+75
-63
lines changed

5 files changed

+75
-63
lines changed

‎src/dom.ts

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ export class FrameExecutionContext extends js.ExecutionContext {
6161
}
6262

6363
async doEvaluateInternal(returnByValue: boolean, waitForNavigations: boolean, pageFunction: string | Function, ...args: any[]): Promise<any> {
64-
return await this.frame._page._frameManager.waitForSignalsCreatedBy(async () => {
64+
return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, !waitForNavigations, async () => {
6565
return this._delegate.evaluate(this, returnByValue, pageFunction, ...args);
66-
}, Number.MAX_SAFE_INTEGER, waitForNavigations ? undefined : { noWaitAfter: true });
66+
});
6767
}
6868

6969
createHandle(remoteObject: js.RemoteObject): js.JSHandle {
@@ -241,7 +241,6 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
241241
}
242242

243243
async _retryPointerAction(progress: Progress, action: (point: types.Point) => Promise<void>, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<void> {
244-
progress.log(inputLog, progress.apiName);
245244
while (!progress.isCanceled()) {
246245
const result = await this._performPointerAction(progress, action, options);
247246
if (result === 'done')
@@ -291,7 +290,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
291290
progress.log(inputLog, `...element does receive pointer events, continuing input action`);
292291
}
293292

294-
await this._page._frameManager.waitForSignalsCreatedBy(async () => {
293+
await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
295294
let restoreModifiers: input.Modifier[] | undefined;
296295
if (options && options.modifiers)
297296
restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
@@ -301,7 +300,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
301300
progress.log(inputLog, 'waiting for scheduled navigations to finish...');
302301
if (restoreModifiers)
303302
await this._page.keyboard._ensureModifiers(restoreModifiers);
304-
}, progress.deadline, options, true);
303+
}, 'input');
305304
progress.log(inputLog, '...navigations have finished');
306305

307306
return 'done';
@@ -331,9 +330,12 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
331330
return this._retryPointerAction(progress, point => this._page.mouse.dblclick(point.x, point.y, options), options);
332331
}
333332

334-
async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options?: types.NavigatingActionWaitOptions): Promise<string[]> {
335-
this._page._log(inputLog, `elementHandle.selectOption(%s)`, values);
336-
const deadline = this._page._timeoutSettings.computeDeadline(options);
333+
async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise<string[]> {
334+
return Progress.runCancelableTask(progress => this._selectOption(progress, values, options), options, this._page, this._page._timeoutSettings);
335+
}
336+
337+
async _selectOption(progress: Progress, values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions): Promise<string[]> {
338+
progress.log(inputLog, progress.apiName);
337339
let vals: string[] | ElementHandle[] | types.SelectOption[];
338340
if (!Array.isArray(values))
339341
vals = [ values ] as (string[] | ElementHandle[] | types.SelectOption[]);
@@ -350,10 +352,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
350352
if (option.index !== undefined)
351353
assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
352354
}
353-
return await this._page._frameManager.waitForSignalsCreatedBy<string[]>(async () => {
355+
return this._page._frameManager.waitForSignalsCreatedBy<string[]>(progress, options.noWaitAfter, async () => {
354356
const injectedResult = await this._evaluateInUtility(({ injected, node }, selectOptions) => injected.selectOptions(node, selectOptions), selectOptions);
355357
return handleInjectedResult(injectedResult);
356-
}, deadline, options);
358+
});
357359
}
358360

359361
async fill(value: string, options: types.NavigatingActionWaitOptions = {}): Promise<void> {
@@ -363,7 +365,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
363365
async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions): Promise<void> {
364366
progress.log(inputLog, `elementHandle.fill("${value}")`);
365367
assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
366-
await this._page._frameManager.waitForSignalsCreatedBy(async () => {
368+
await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
367369
const poll = await this._evaluateHandleInUtility(({ injected, node }, { value }) => {
368370
return injected.waitForEnabledAndFill(node, value);
369371
}, { value });
@@ -376,7 +378,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
376378
else
377379
await this._page.keyboard.press('Delete');
378380
}
379-
}, progress.deadline, options, true);
381+
}, 'input');
380382
}
381383

382384
async selectText(): Promise<void> {
@@ -385,9 +387,12 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
385387
handleInjectedResult(injectedResult);
386388
}
387389

388-
async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options?: types.NavigatingActionWaitOptions) {
389-
this._page._log(inputLog, `elementHandle.setInputFiles(...)`);
390-
const deadline = this._page._timeoutSettings.computeDeadline(options);
390+
async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}) {
391+
return Progress.runCancelableTask(async progress => this._setInputFiles(progress, files, options), options, this._page, this._page._timeoutSettings);
392+
}
393+
394+
async _setInputFiles(progress: Progress, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions) {
395+
progress.log(inputLog, progress.apiName);
391396
const injectedResult = await this._evaluateInUtility(({ node }): types.InjectedScriptResult<boolean> => {
392397
if (node.nodeType !== Node.ELEMENT_NODE || (node as Node as Element).tagName !== 'INPUT')
393398
return { status: 'error', error: 'Node is not an HTMLInputElement' };
@@ -416,9 +421,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
416421
filePayloads.push(item);
417422
}
418423
}
419-
await this._page._frameManager.waitForSignalsCreatedBy(async () => {
424+
await this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
420425
await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
421-
}, deadline, options);
426+
});
422427
}
423428

424429
async focus() {
@@ -427,22 +432,28 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
427432
handleInjectedResult(injectedResult);
428433
}
429434

430-
async type(text: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
431-
this._page._log(inputLog, `elementHandle.type("${text}")`);
432-
const deadline = this._page._timeoutSettings.computeDeadline(options);
433-
await this._page._frameManager.waitForSignalsCreatedBy(async () => {
435+
async type(text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
436+
return Progress.runCancelableTask(progress => this._type(progress, text, options), options, this._page, this._page._timeoutSettings);
437+
}
438+
439+
async _type(progress: Progress, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions) {
440+
progress.log(inputLog, `elementHandle.type("${text}")`);
441+
return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
434442
await this.focus();
435443
await this._page.keyboard.type(text, options);
436-
}, deadline, options, true);
444+
}, 'input');
445+
}
446+
447+
async press(key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
448+
return Progress.runCancelableTask(progress => this._press(progress, key, options), options, this._page, this._page._timeoutSettings);
437449
}
438450

439-
async press(key: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
440-
this._page._log(inputLog, `elementHandle.press("${key}")`);
441-
const deadline = this._page._timeoutSettings.computeDeadline(options);
442-
await this._page._frameManager.waitForSignalsCreatedBy(async () => {
451+
async _press(progress: Progress, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions) {
452+
progress.log(inputLog, `elementHandle.press("${key}")`);
453+
return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
443454
await this.focus();
444455
await this._page.keyboard.press(key, options);
445-
}, deadline, options, true);
456+
}, 'input');
446457
}
447458

448459
async check(options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {

‎src/frames.ts

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -105,23 +105,21 @@ export class FrameManager {
105105
}
106106
}
107107

108-
// TODO: take progress parameter.
109-
async waitForSignalsCreatedBy<T>(action: () => Promise<T>, deadline: number, options: types.NavigatingActionWaitOptions = {}, input?: boolean): Promise<T> {
110-
if (options.noWaitAfter)
108+
async waitForSignalsCreatedBy<T>(progress: Progress | null, noWaitAfter: boolean | undefined, action: () => Promise<T>, source?: 'input'): Promise<T> {
109+
if (noWaitAfter)
111110
return action();
112-
const barrier = new SignalBarrier(options, deadline);
111+
const barrier = new SignalBarrier(progress);
113112
this._signalBarriers.add(barrier);
114-
try {
115-
const result = await action();
116-
if (input)
117-
await this._page._delegate.inputActionEpilogue();
118-
await barrier.waitFor();
119-
// Resolve in the next task, after all waitForNavigations.
120-
await new Promise(helper.makeWaitForNextTask());
121-
return result;
122-
} finally {
123-
this._signalBarriers.delete(barrier);
124-
}
113+
if (progress)
114+
progress.cleanupWhenCanceled(() => this._signalBarriers.delete(barrier));
115+
const result = await action();
116+
if (source === 'input')
117+
await this._page._delegate.inputActionEpilogue();
118+
await barrier.waitFor();
119+
this._signalBarriers.delete(barrier);
120+
// Resolve in the next task, after all waitForNavigations.
121+
await new Promise(helper.makeWaitForNextTask());
122+
return result;
125123
}
126124

127125
frameWillPotentiallyRequestNavigation() {
@@ -732,8 +730,7 @@ export class Frame {
732730
}
733731

734732
async fill(selector: string, value: string, options: types.NavigatingActionWaitOptions = {}) {
735-
await this._retryWithSelectorIfNotConnected(selector, options,
736-
(progress, handle) => handle._fill(progress, value, options));
733+
await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._fill(progress, value, options));
737734
}
738735

739736
async focus(selector: string, options: types.TimeoutOptions = {}) {
@@ -761,23 +758,19 @@ export class Frame {
761758
}
762759

763760
async selectOption(selector: string, values: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise<string[]> {
764-
return await this._retryWithSelectorIfNotConnected(selector, options,
765-
(progress, handle) => handle.selectOption(values, helper.optionsWithUpdatedTimeout(options, progress.deadline)));
761+
return this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._selectOption(progress, values, options));
766762
}
767763

768764
async setInputFiles(selector: string, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}): Promise<void> {
769-
await this._retryWithSelectorIfNotConnected(selector, options,
770-
(progress, handle) => handle.setInputFiles(files, helper.optionsWithUpdatedTimeout(options, progress.deadline)));
765+
await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._setInputFiles(progress, files, options));
771766
}
772767

773768
async type(selector: string, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
774-
await this._retryWithSelectorIfNotConnected(selector, options,
775-
(progress, handle) => handle.type(text, helper.optionsWithUpdatedTimeout(options, progress.deadline)));
769+
await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._type(progress, text, options));
776770
}
777771

778772
async press(selector: string, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
779-
await this._retryWithSelectorIfNotConnected(selector, options,
780-
(progress, handle) => handle.press(key, helper.optionsWithUpdatedTimeout(options, progress.deadline)));
773+
await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle._press(progress, key, options));
781774
}
782775

783776
async check(selector: string, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
@@ -937,15 +930,13 @@ class RerunnableTask<T> {
937930
}
938931

939932
export class SignalBarrier {
940-
private _options: types.NavigatingActionWaitOptions;
933+
private _progress: Progress | null;
941934
private _protectCount = 0;
942935
private _promise: Promise<void>;
943936
private _promiseCallback = () => {};
944-
private _deadline: number;
945937

946-
constructor(options: types.NavigatingActionWaitOptions, deadline: number) {
947-
this._options = options;
948-
this._deadline = deadline;
938+
constructor(progress: Progress | null) {
939+
this._progress = progress;
949940
this._promise = new Promise(f => this._promiseCallback = f);
950941
this.retain();
951942
}
@@ -957,8 +948,8 @@ export class SignalBarrier {
957948

958949
async addFrameNavigation(frame: Frame) {
959950
this.retain();
960-
const options = helper.optionsWithUpdatedTimeout(this._options, this._deadline);
961-
await frame._waitForNavigation({...options, waitUntil: 'commit'}).catch(e => {});
951+
const timeout = this._progress ? helper.timeUntilDeadline(this._progress.deadline) : undefined;
952+
await frame._waitForNavigation({timeout, waitUntil: 'commit'}).catch(e => {});
962953
this.release();
963954
}
964955

‎src/javascript.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,13 @@ export class JSHandle<T = any> {
8383
_disposed = false;
8484
readonly _objectId: string | undefined;
8585
readonly _value: any;
86-
private _type: string;
86+
private _objectType: string;
8787

8888
constructor(context: ExecutionContext, type: string, objectId?: string, value?: any) {
8989
this._context = context;
9090
this._objectId = objectId;
9191
this._value = value;
92-
this._type = type;
92+
this._objectType = type;
9393
}
9494

9595
async evaluate<R, Arg>(pageFunction: types.FuncOn<T, Arg, R>, arg: Arg): Promise<R>;
@@ -137,7 +137,7 @@ export class JSHandle<T = any> {
137137

138138
_handleToString(includeType: boolean): string {
139139
if (this._objectId)
140-
return 'JSHandle@' + this._type;
140+
return 'JSHandle@' + this._objectType;
141141
return (includeType ? 'JSHandle:' : '') + this._value;
142142
}
143143

‎test/elementhandle.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,3 +416,13 @@ describe('ElementHandle.check', () => {
416416
expect(await page.evaluate(() => checkbox.checked)).toBe(false);
417417
});
418418
});
419+
420+
describe('ElementHandle.selectOption', function() {
421+
it('should select single option', async({page, server}) => {
422+
await page.goto(server.PREFIX + '/input/select.html');
423+
const select = await page.$('select');
424+
await select.selectOption('blue');
425+
expect(await page.evaluate(() => result.onInput)).toEqual(['blue']);
426+
expect(await page.evaluate(() => result.onChange)).toEqual(['blue']);
427+
});
428+
});

‎test/page.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ describe('Page.title', function() {
850850
});
851851
});
852852

853-
describe('Page.select', function() {
853+
describe('Page.selectOption', function() {
854854
it('should select single option', async({page, server}) => {
855855
await page.goto(server.PREFIX + '/input/select.html');
856856
await page.selectOption('select', 'blue');

0 commit comments

Comments
 (0)