Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: wait for pending navigation to resolve before many actions #32899

Merged
merged 1 commit into from
Oct 4, 2024
+136 −49
Merged

feat: wait for pending navigation to resolve before many actions #32899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/playwright-core/src/server/dom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
};
}

async _retryAction(progress: Progress, actionName: string, action: (retry: number) => Promise<PerformActionResult>, options: { trial?: boolean, force?: boolean, skipLocatorHandlersCheckpoint?: boolean }): Promise<'error:notconnected' | 'done'> {
async _retryAction(progress: Progress, actionName: string, action: (retry: number) => Promise<PerformActionResult>, options: { trial?: boolean, force?: boolean, skipActionPreChecks?: boolean }): Promise<'error:notconnected' | 'done'> {
let retry = 0;
// We progressively wait longer between retries, up to 500ms.
const waitTime = [0, 20, 100, 100, 500];
Expand All @@ -310,8 +310,8 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
} else {
progress.log(`attempting ${actionName} action${options.trial ? ' (trial run)' : ''}`);
}
if (!options.skipLocatorHandlersCheckpoint && !options.force)
await this._frame._page.performLocatorHandlersCheckpoint(progress);
if (!options.skipActionPreChecks && !options.force)
await this._frame._page.performActionPreChecks(progress);
const result = await action(retry);
++retry;
if (result === 'error:notvisible') {
Expand Down Expand Up @@ -346,7 +346,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
async _retryPointerAction(progress: Progress, actionName: ActionName, waitForEnabled: boolean, action: (point: types.Point) => Promise<void>,
options: { waitAfter: boolean | 'disabled' } & types.PointerActionOptions & types.PointerActionWaitOptions): Promise<'error:notconnected' | 'done'> {
// Note: do not perform locator handlers checkpoint to avoid moving the mouse in the middle of a drag operation.
const skipLocatorHandlersCheckpoint = actionName === 'move and up';
const skipActionPreChecks = actionName === 'move and up';
return await this._retryAction(progress, actionName, async retry => {
// By default, we scroll with protocol method to reveal the action point.
// However, that might not work to scroll from under position:sticky elements
Expand All @@ -360,7 +360,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
];
const forceScrollOptions = scrollOptions[retry % scrollOptions.length];
return await this._performPointerAction(progress, actionName, waitForEnabled, action, forceScrollOptions, options);
}, { ...options, skipLocatorHandlersCheckpoint });
}, { ...options, skipActionPreChecks });
}

async _performPointerAction(
Expand Down
48 changes: 24 additions & 24 deletions packages/playwright-core/src/server/frames.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,11 +791,11 @@ export class Frame extends SdkObject {
}, this._page._timeoutSettings.timeout(options));
}

async waitForSelectorInternal(progress: Progress, selector: string, performLocatorHandlersCheckpoint: boolean, options: types.WaitForElementOptions, scope?: dom.ElementHandle): Promise<dom.ElementHandle<Element> | null> {
async waitForSelectorInternal(progress: Progress, selector: string, performActionPreChecks: boolean, options: types.WaitForElementOptions, scope?: dom.ElementHandle): Promise<dom.ElementHandle<Element> | null> {
const { state = 'visible' } = options;
const promise = this.retryWithProgressAndTimeouts(progress, [0, 20, 50, 100, 100, 500], async continuePolling => {
if (performLocatorHandlersCheckpoint)
await this._page.performLocatorHandlersCheckpoint(progress);
if (performActionPreChecks)
await this._page.performActionPreChecks(progress);

const resolved = await this.selectors.resolveInjectedForSelector(selector, options, scope);
progress.throwIfAborted();
Expand Down Expand Up @@ -1118,12 +1118,12 @@ export class Frame extends SdkObject {
progress: Progress,
selector: string,
strict: boolean | undefined,
performLocatorHandlersCheckpoint: boolean,
performActionPreChecks: boolean,
action: (handle: dom.ElementHandle<Element>) => Promise<R | 'error:notconnected'>): Promise<R> {
progress.log(`waiting for ${this._asLocator(selector)}`);
return this.retryWithProgressAndTimeouts(progress, [0, 20, 50, 100, 100, 500], async continuePolling => {
if (performLocatorHandlersCheckpoint)
await this._page.performLocatorHandlersCheckpoint(progress);
if (performActionPreChecks)
await this._page.performActionPreChecks(progress);

const resolved = await this.selectors.resolveInjectedForSelector(selector, { strict });
progress.throwIfAborted();
Expand Down Expand Up @@ -1167,7 +1167,7 @@ export class Frame extends SdkObject {
}

async rafrafTimeoutScreenshotElementWithProgress(progress: Progress, selector: string, timeout: number, options: ScreenshotOptions): Promise<Buffer> {
return await this._retryWithProgressIfNotConnected(progress, selector, true /* strict */, true /* performLocatorHandlersCheckpoint */, async handle => {
return await this._retryWithProgressIfNotConnected(progress, selector, true /* strict */, true /* performActionPreChecks */, async handle => {
await handle._frame.rafrafTimeout(timeout);
return await this._page._screenshotter.screenshotElement(progress, handle, options);
});
Expand All @@ -1176,21 +1176,21 @@ export class Frame extends SdkObject {
async click(metadata: CallMetadata, selector: string, options: { noWaitAfter?: boolean } & types.MouseClickOptions & types.PointerActionWaitOptions) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._click(progress, { ...options, waitAfter: !options.noWaitAfter })));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._click(progress, { ...options, waitAfter: !options.noWaitAfter })));
}, this._page._timeoutSettings.timeout(options));
}

async dblclick(metadata: CallMetadata, selector: string, options: types.MouseMultiClickOptions & types.PointerActionWaitOptions = {}) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._dblclick(progress, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._dblclick(progress, options)));
}, this._page._timeoutSettings.timeout(options));
}

async dragAndDrop(metadata: CallMetadata, source: string, target: string, options: types.DragActionOptions & types.PointerActionWaitOptions = {}) {
const controller = new ProgressController(metadata, this);
await controller.run(async progress => {
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, source, options.strict, !options.force /* performLocatorHandlersCheckpoint */, async handle => {
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, source, options.strict, !options.force /* performActionPreChecks */, async handle => {
return handle._retryPointerAction(progress, 'move and down', false, async point => {
await this._page.mouse.move(point.x, point.y);
await this._page.mouse.down();
Expand All @@ -1202,7 +1202,7 @@ export class Frame extends SdkObject {
});
}));
// Note: do not perform locator handlers checkpoint to avoid moving the mouse in the middle of a drag operation.
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, target, options.strict, false /* performLocatorHandlersCheckpoint */, async handle => {
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, target, options.strict, false /* performActionPreChecks */, async handle => {
return handle._retryPointerAction(progress, 'move and up', false, async point => {
await this._page.mouse.move(point.x, point.y);
await this._page.mouse.up();
Expand All @@ -1221,28 +1221,28 @@ export class Frame extends SdkObject {
throw new Error('The page does not support tap. Use hasTouch context option to enable touch support.');
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._tap(progress, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._tap(progress, options)));
}, this._page._timeoutSettings.timeout(options));
}

async fill(metadata: CallMetadata, selector: string, value: string, options: types.TimeoutOptions & types.StrictOptions & { force?: boolean }) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._fill(progress, value, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._fill(progress, value, options)));
}, this._page._timeoutSettings.timeout(options));
}

async focus(metadata: CallMetadata, selector: string, options: types.TimeoutOptions & types.StrictOptions = {}) {
const controller = new ProgressController(metadata, this);
await controller.run(async progress => {
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._focus(progress)));
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performActionPreChecks */, handle => handle._focus(progress)));
}, this._page._timeoutSettings.timeout(options));
}

async blur(metadata: CallMetadata, selector: string, options: types.TimeoutOptions & types.StrictOptions = {}) {
const controller = new ProgressController(metadata, this);
await controller.run(async progress => {
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._blur(progress)));
dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performActionPreChecks */, handle => handle._blur(progress)));
}, this._page._timeoutSettings.timeout(options));
}

Expand Down Expand Up @@ -1349,50 +1349,50 @@ export class Frame extends SdkObject {
async hover(metadata: CallMetadata, selector: string, options: types.PointerActionOptions & types.PointerActionWaitOptions = {}) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._hover(progress, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._hover(progress, options)));
}, this._page._timeoutSettings.timeout(options));
}

async selectOption(metadata: CallMetadata, selector: string, elements: dom.ElementHandle[], values: types.SelectOption[], options: types.CommonActionOptions = {}): Promise<string[]> {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._selectOption(progress, elements, values, options));
return await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._selectOption(progress, elements, values, options));
}, this._page._timeoutSettings.timeout(options));
}

async setInputFiles(metadata: CallMetadata, selector: string, params: channels.FrameSetInputFilesParams): Promise<channels.FrameSetInputFilesResult> {
const inputFileItems = await prepareFilesForUpload(this, params);
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, params.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._setInputFiles(progress, inputFileItems)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, params.strict, true /* performActionPreChecks */, handle => handle._setInputFiles(progress, inputFileItems)));
}, this._page._timeoutSettings.timeout(params));
}

async type(metadata: CallMetadata, selector: string, text: string, options: { delay?: number } & types.TimeoutOptions & types.StrictOptions = {}) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._type(progress, text, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performActionPreChecks */, handle => handle._type(progress, text, options)));
}, this._page._timeoutSettings.timeout(options));
}

async press(metadata: CallMetadata, selector: string, key: string, options: { delay?: number, noWaitAfter?: boolean } & types.TimeoutOptions & types.StrictOptions = {}) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performLocatorHandlersCheckpoint */, handle => handle._press(progress, key, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, true /* performActionPreChecks */, handle => handle._press(progress, key, options)));
}, this._page._timeoutSettings.timeout(options));
}

async check(metadata: CallMetadata, selector: string, options: types.PointerActionWaitOptions = {}) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._setChecked(progress, true, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._setChecked(progress, true, options)));
}, this._page._timeoutSettings.timeout(options));
}

async uncheck(metadata: CallMetadata, selector: string, options: types.PointerActionWaitOptions = {}) {
const controller = new ProgressController(metadata, this);
return controller.run(async progress => {
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performLocatorHandlersCheckpoint */, handle => handle._setChecked(progress, false, options)));
return dom.assertDone(await this._retryWithProgressIfNotConnected(progress, selector, options.strict, !options.force /* performActionPreChecks */, handle => handle._setChecked(progress, false, options)));
}, this._page._timeoutSettings.timeout(options));
}

Expand Down Expand Up @@ -1421,7 +1421,7 @@ export class Frame extends SdkObject {
await (new ProgressController(metadata, this)).run(async progress => {
progress.log(`${metadata.apiName}${timeout ? ` with timeout ${timeout}ms` : ''}`);
progress.log(`waiting for ${this._asLocator(selector)}`);
await this._page.performLocatorHandlersCheckpoint(progress);
await this._page.performActionPreChecks(progress);
}, timeout);

// Step 2: perform one-shot expect check without a timeout.
Expand All @@ -1448,7 +1448,7 @@ export class Frame extends SdkObject {
// Step 3: auto-retry expect with increasing timeouts. Bounded by the total remaining time.
return await (new ProgressController(metadata, this)).run(async progress => {
return await this.retryWithProgressAndTimeouts(progress, [100, 250, 500, 1000], async continuePolling => {
await this._page.performLocatorHandlersCheckpoint(progress);
await this._page.performActionPreChecks(progress);
const { matches, received } = await this._expectInternal(progress, selector, options, lastIntermediateResult);
if (matches === options.isNot) {
// Keep waiting in these cases:
Expand Down
Loading