From c1b9a560792296c9721525f9f7a92db70c9217fb Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 27 Dec 2022 13:39:35 -0800 Subject: [PATCH] chore: migrate waitForSelector to not use rerunnable task (#19715) --- packages/playwright-core/src/server/dom.ts | 42 ----- packages/playwright-core/src/server/frames.ts | 161 ++++++------------ .../playwright-core/src/server/javascript.ts | 2 +- tests/page/expect-boolean.spec.ts | 3 +- tests/page/page-wait-for-selector-1.spec.ts | 3 +- tests/page/page-wait-for-selector-2.spec.ts | 2 +- 6 files changed, 56 insertions(+), 157 deletions(-) diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index d8a3896b34..be472cb86a 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -26,7 +26,6 @@ import * as js from './javascript'; import type { Page } from './page'; import type { Progress } from './progress'; import { ProgressController } from './progress'; -import type { SelectorInfo } from './selectors'; import type * as types from './types'; import type { TimeoutOptions } from '../common/types'; import { isUnderTest } from '../utils'; @@ -1004,47 +1003,6 @@ function compensateHalfIntegerRoundingError(point: types.Point) { export type SchedulableTask = (injectedScript: js.JSHandle) => Promise>>; -export function waitForSelectorTask(selector: SelectorInfo, state: 'attached' | 'detached' | 'visible' | 'hidden', omitReturnValue?: boolean, root?: ElementHandle): SchedulableTask { - return injectedScript => injectedScript.evaluateHandle((injected, { parsed, strict, state, omitReturnValue, root }) => { - let lastElement: Element | undefined; - - return injected.pollRaf(progress => { - const elements = injected.querySelectorAll(parsed, root || document); - let element: Element | undefined = elements[0]; - const visible = element ? injected.isVisible(element) : false; - - if (lastElement !== element) { - lastElement = element; - if (!element) { - progress.log(` locator did not resolve to any element`); - } else { - if (elements.length > 1) { - if (strict) - throw injected.strictModeViolationError(parsed, elements); - progress.log(` locator resolved to ${elements.length} elements. Proceeding with the first one.`); - } - progress.log(` locator resolved to ${visible ? 'visible' : 'hidden'} ${injected.previewNode(element)}`); - } - } - - const hasElement = !!element; - if (omitReturnValue) - element = undefined; - - switch (state) { - case 'attached': - return hasElement ? element : progress.continuePolling; - case 'detached': - return !hasElement ? undefined : progress.continuePolling; - case 'visible': - return visible ? element : progress.continuePolling; - case 'hidden': - return !visible ? undefined : progress.continuePolling; - } - }); - }, { parsed: selector.parsed, strict: selector.strict, state, omitReturnValue, root }); -} - function joinWithAnd(strings: string[]): string { if (strings.length < 1) return strings.join(', '); diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index a3be598fd3..9f80c01d95 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -34,11 +34,9 @@ import { ManualPromise } from '../utils/manualPromise'; import { debugLogger } from '../common/debugLogger'; import type { CallMetadata } from './instrumentation'; import { serverSideCallMetadata, SdkObject } from './instrumentation'; -import { type InjectedScript } from './injected/injectedScript'; -import type { ElementStateWithoutStable, FrameExpectParams, InjectedScriptPoll, InjectedScriptProgress } from './injected/injectedScript'; +import type { InjectedScript, ElementStateWithoutStable, FrameExpectParams, InjectedScriptPoll, InjectedScriptProgress } from './injected/injectedScript'; import { isSessionClosedError } from './protocolError'; -import type { ParsedSelector } from './isomorphic/selectorParser'; -import { isInvalidSelectorError, splitSelectorByFrame, stringifySelector } from './isomorphic/selectorParser'; +import { type ParsedSelector, isInvalidSelectorError, splitSelectorByFrame, stringifySelector } from './isomorphic/selectorParser'; import type { SelectorInfo } from './selectors'; import type { ScreenshotOptions } from './screenshotter'; import type { InputFilesItems } from './dom'; @@ -788,7 +786,7 @@ export class Frame extends SdkObject { return this._page.selectors.query(result.frame, result.info); } - async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions & { omitReturnValue?: boolean }, scope?: dom.ElementHandle): Promise | null> { + async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions, scope?: dom.ElementHandle): Promise | null> { const controller = new ProgressController(metadata, this); if ((options as any).visibility) throw new Error('options.visibility is not supported, did you mean options.state?'); @@ -799,27 +797,45 @@ export class Frame extends SdkObject { throw new Error(`state: expected one of (attached|detached|visible|hidden)`); return controller.run(async progress => { progress.log(`waiting for ${this._asLocator(selector)}${state === 'attached' ? '' : ' to be ' + state}`); - return this.retryWithProgress(progress, selector, options, async (selectorInFrame, continuePolling) => { - // Be careful, |this| can be different from |frame|. - // We did not pass omitAttached, so it is non-null. - const { frame, info } = selectorInFrame!; - const actualScope = this === frame ? scope : undefined; - const task = dom.waitForSelectorTask(info, state, options.omitReturnValue, actualScope); - const result = actualScope ? await frame._runWaitForSelectorTaskOnce(progress, stringifySelector(info.parsed), info.world, task) - : await frame._scheduleRerunnableHandleTask(progress, info.world, task); - if (!result.asElement()) { + const promise = this.retryWithProgressAndTimeouts(progress, [0, 20, 50, 100, 100, 500], async continuePolling => { + const resolved = await this._resolveInjectedForSelector(progress, selector, options, scope); + if (!resolved) + return continuePolling; + const result = await resolved.injected.evaluateHandle((injected, { info, root }) => { + const elements = injected.querySelectorAll(info.parsed, root || document); + const element: Element | undefined = elements[0]; + const visible = element ? injected.isVisible(element) : false; + let log = ''; + if (elements.length > 1) { + if (info.strict) + throw injected.strictModeViolationError(info.parsed, elements); + log = ` locator resolved to ${elements.length} elements. Proceeding with the first one: ${injected.previewNode(elements[0])}`; + } else if (element) { + log = ` locator resolved to ${visible ? 'visible' : 'hidden'} ${injected.previewNode(element)}`; + } + return { log, element, visible, attached: !!element }; + }, { info: resolved.info, root: resolved.frame === this ? scope : undefined }); + const { log, visible, attached } = await result.evaluate(r => ({ log: r.log, visible: r.visible, attached: r.attached })); + if (log) + progress.log(log); + const success = { attached, detached: !attached, visible, hidden: !visible }[state]; + if (!success) { result.dispose(); - return null; + return continuePolling; } + const element = state === 'attached' || state === 'visible' ? await result.evaluateHandle(r => r.element) : null; + result.dispose(); + if (!element) + return null; if ((options as any).__testHookBeforeAdoptNode) await (options as any).__testHookBeforeAdoptNode(); - const handle = result.asElement() as dom.ElementHandle; try { - return await handle._adoptTo(await frame._mainContext()); + return await element._adoptTo(await resolved.frame._mainContext()); } catch (e) { return continuePolling; } - }, scope); + }); + return scope ? scope._context._raceAgainstContextDestroyed(promise) : promise; }, this._page._timeoutSettings.timeout(options)); } @@ -1050,45 +1066,6 @@ export class Frame extends SdkObject { return result!; } - async retryWithProgress( - progress: Progress, - selector: string, - options: types.StrictOptions & types.TimeoutOptions & { omitAttached?: boolean }, - action: (selector: SelectorInFrame | null, continuePolling: symbol) => Promise, - scope?: dom.ElementHandle): Promise { - const continuePolling = Symbol('continuePolling'); - while (progress.isRunning()) { - let selectorInFrame: SelectorInFrame | null; - if (options.omitAttached) { - selectorInFrame = await this.resolveFrameForSelectorNoWait(selector, options, scope); - } else { - selectorInFrame = await this._resolveFrameForSelector(progress, selector, options, scope); - if (!selectorInFrame) { - // Missing content frame. - await new Promise(f => setTimeout(f, 100)); - continue; - } - } - try { - const result = await action(selectorInFrame, continuePolling); - if (result === continuePolling) - continue; - return result as R; - } catch (e) { - if (this._isErrorThatCannotBeRetried(e)) - throw e; - // If there is scope, and scope is within the frame we use to select, assume context is destroyed and - // operation is not recoverable. - if (scope && scope._context.frame === selectorInFrame?.frame) - throw e; - // Retry upon all other errors. - continue; - } - } - progress.throwIfAborted(); - return undefined as any; - } - async retryWithProgressAndTimeouts(progress: Progress, timeouts: number[], action: (continuePolling: symbol) => Promise): Promise { const continuePolling = Symbol('continuePolling'); timeouts = [0, ...timeouts]; @@ -1135,8 +1112,8 @@ export class Frame extends SdkObject { return false; } - private async _resolveInjectedForSelector(progress: Progress, selector: string, options: { strict?: boolean, mainWorld?: boolean }): Promise<{ injected: js.JSHandle, info: SelectorInfo } | undefined> { - const selectorInFrame = await this.resolveFrameForSelectorNoWait(selector, options); + private async _resolveInjectedForSelector(progress: Progress, selector: string, options: { strict?: boolean, mainWorld?: boolean }, scope?: dom.ElementHandle): Promise<{ injected: js.JSHandle, info: SelectorInfo, frame: Frame } | undefined> { + const selectorInFrame = await this.resolveFrameForSelectorNoWait(selector, options, scope); if (!selectorInFrame) return; progress.throwIfAborted(); @@ -1145,7 +1122,7 @@ export class Frame extends SdkObject { const context = await selectorInFrame.frame._context(options.mainWorld ? 'main' : selectorInFrame.info.world); const injected = await context.injectedScript(); progress.throwIfAborted(); - return { injected, info: selectorInFrame.info }; + return { injected, info: selectorInFrame.info, frame: selectorInFrame.frame }; } private async _retryWithProgressIfNotConnected( @@ -1674,68 +1651,32 @@ export class Frame extends SdkObject { }, { source, arg }); } - private async _resolveFrameForSelector(progress: Progress, selector: string, options: types.StrictOptions & types.TimeoutOptions, scope?: dom.ElementHandle): Promise { - const elementPath: dom.ElementHandle[] = []; - progress.cleanupWhenAborted(() => { - // Do not await here to avoid being blocked, either by stalled - // page (e.g. alert) or unresolved navigation in Chromium. - for (const element of elementPath) - element.dispose(); - }); - - let frame: Frame | null = this; - const frameChunks = splitSelectorByFrame(selector); - - for (let i = 0; i < frameChunks.length - 1 && progress.isRunning(); ++i) { - const info = this._page.parseSelector(frameChunks[i], options); - const task = dom.waitForSelectorTask(info, 'attached', false, i === 0 ? scope : undefined); - progress.log(` waiting for ${this._asLocator(stringifySelector(frameChunks[i]) + ' >> internal:control=enter-frame')}`); - const handle = i === 0 && scope ? await frame._runWaitForSelectorTaskOnce(progress, stringifySelector(info.parsed), info.world, task) - : await frame._scheduleRerunnableHandleTask(progress, info.world, task); - const element = handle.asElement() as dom.ElementHandle; - const isIframe = await element.isIframeElement(); - if (isIframe === 'error:notconnected') - return null; // retry - if (!isIframe) - throw new Error(`Selector "${stringifySelector(info.parsed)}" resolved to ${element.preview()},