chore: migrate waitForSelector to not use rerunnable task (#19715)

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<T> = (injectedScript: js.JSHandle<InjectedScript>) => Promise<js.JSHandle<InjectedScriptPoll<T>>>;
 
-export function waitForSelectorTask(selector: SelectorInfo, state: 'attached' | 'detached' | 'visible' | 'hidden', omitReturnValue?: boolean, root?: ElementHandle): SchedulableTask<Element | undefined> {
-  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(', ');

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 { InjectedScript, ElementStateWithoutStable, FrameExpectParams, InjectedScriptPoll, InjectedScriptProgress } from './injected/injectedScript';
-import type { ElementStateWithoutStable, FrameExpectParams, InjectedScriptPoll, InjectedScriptProgress } from './injected/injectedScript';
 import { isSessionClosedError } from './protocolError';
-import type { ParsedSelector } from './isomorphic/selectorParser';
+import { type ParsedSelector, isInvalidSelectorError, splitSelectorByFrame, stringifySelector } from './isomorphic/selectorParser';
-import { 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<dom.ElementHandle<Element> | null> {
+  async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions, scope?: dom.ElementHandle): Promise<dom.ElementHandle<Element> | 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) => {
+      const promise = this.retryWithProgressAndTimeouts(progress, [0, 20, 50, 100, 100, 500], async continuePolling => {
-        // Be careful, |this| can be different from |frame|.
+        const resolved = await this._resolveInjectedForSelector(progress, selector, options, scope);
-        // We did not pass omitAttached, so it is non-null.
+        if (!resolved)
-        const { frame, info } = selectorInFrame!;
+          return continuePolling;
-        const actualScope = this === frame ? scope : undefined;
+        const result = await resolved.injected.evaluateHandle((injected, { info, root }) => {
-        const task = dom.waitForSelectorTask(info, state, options.omitReturnValue, actualScope);
+          const elements = injected.querySelectorAll(info.parsed, root || document);
-        const result = actualScope ? await frame._runWaitForSelectorTaskOnce(progress, stringifySelector(info.parsed), info.world, task)
+          const element: Element | undefined  = elements[0];
-          : await frame._scheduleRerunnableHandleTask(progress, info.world, task);
+          const visible = element ? injected.isVisible(element) : false;
-        if (!result.asElement()) {
+          let log = '';
-          result.dispose();
+          if (elements.length > 1) {
-          return null;
+            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 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<Element>;
         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<R>(
-    progress: Progress,
-    selector: string,
-    options: types.StrictOptions & types.TimeoutOptions & { omitAttached?: boolean },
-    action: (selector: SelectorInFrame | null, continuePolling: symbol) => Promise<R | symbol>,
-    scope?: dom.ElementHandle): Promise<R> {
-    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<R>(progress: Progress, timeouts: number[], action: (continuePolling: symbol) => Promise<R | symbol>): Promise<R> {
     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<InjectedScript>, info: SelectorInfo } | undefined> {
+  private async _resolveInjectedForSelector(progress: Progress, selector: string, options: { strict?: boolean, mainWorld?: boolean }, scope?: dom.ElementHandle): Promise<{ injected: js.JSHandle<InjectedScript>, info: SelectorInfo, frame: Frame } | undefined> {
-    const selectorInFrame = await this.resolveFrameForSelectorNoWait(selector, options);
+    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<R>(
@@ -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<SelectorInFrame | null> {
-    const elementPath: dom.ElementHandle<Element>[] = [];
-    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<Element>;
-      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()}, <iframe> was expected`);
-      frame = await element.contentFrame();
-      element.dispose();
-      if (!frame)
-        return null;  // retry
-    }
-    return { frame, info: this._page.parseSelector(frameChunks[frameChunks.length - 1], options) };
-  }
-
   async resolveFrameForSelectorNoWait(selector: string, options: types.StrictOptions = {}, scope?: dom.ElementHandle): Promise<SelectorInFrame | null> {
-    let frame: Frame | null = this;
+    let frame: Frame = this;
     const frameChunks = splitSelectorByFrame(selector);
 
     for (let i = 0; i < frameChunks.length - 1; ++i) {
       const info = this._page.parseSelector(frameChunks[i], options);
-      const element: dom.ElementHandle<Element> | null = await this._page.selectors.query(frame, info, i === 0 ? scope : undefined);
+      const context = await frame._context(info.world);
+      const injectedScript = await context.injectedScript();
+      const handle = await injectedScript.evaluateHandle((injected, { info, scope, selectorString }) => {
+        const element = injected.querySelector(info.parsed, scope || document, info.strict);
+        if (element && element.nodeName !== 'IFRAME' && element.nodeName !== 'FRAME')
+          throw injected.createStacklessError(`Selector "${selectorString}" resolved to ${injected.previewNode(element)}, <iframe> was expected`);
+        return element;
+      }, { info, scope: i === 0 ? scope : undefined, selectorString: stringifySelector(info.parsed) });
+      const element = handle.asElement() as dom.ElementHandle<Element> | null;
       if (!element)
         return null;
-      frame = await element.contentFrame();
+      const maybeFrame = await this._page._delegate.getContentFrame(element);
       element.dispose();
-      if (!frame)
+      if (!maybeFrame)
-        throw new Error(`Selector "${stringifySelector(info.parsed)}" resolved to ${element.preview()}, <iframe> was expected`);
+        return null;
+      frame = maybeFrame;
     }
     return { frame, info: this._page.parseSelector(frameChunks[frameChunks.length - 1], options) };
   }
 
-  private async _runWaitForSelectorTaskOnce<T>(progress: Progress, selector: string, world: types.World, task: dom.SchedulableTask<T>): Promise<js.SmartHandle<T>> {
-    const context = await this._context(world);
-    const injected = await context.injectedScript();
-    try {
-      const pollHandler = new dom.InjectedScriptPollHandler(progress, await task(injected));
-      const result = await pollHandler.finishHandle();
-      progress.cleanupWhenAborted(() => result.dispose());
-      return result;
-    } catch (e) {
-      throw new Error(`Error: frame navigated while waiting for ${this._asLocator(selector)}`);
-    }
-  }
-
   async resetStorageForCurrentOriginBestEffort(newStorage: channels.OriginStorage | undefined) {
     const context = await this._utilityContext();
     await context.evaluate(async ({ ls }) => {

packages/playwright-core/src/server/javascript.ts

@@ -73,7 +73,7 @@ export class ExecutionContext extends SdkObject {
     this._destroyedPromise.resolve(error);
   }
 
-  private _raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> {
+  _raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> {
     return Promise.race([
       this._destroyedPromise.then(e => { throw e; }),
       promise,

tests/page/expect-boolean.spec.ts

@@ -474,9 +474,10 @@ test.describe(() => {
   });
 
   test('toBeOK fail with promise', async ({ page, server }) => {
-    const res = page.request.get(server.EMPTY_PAGE).catch(e => {});
+    const res = page.request.get(server.EMPTY_PAGE);
     const error = await (expect(res) as any).toBeOK().catch(e => e);
     expect(error.message).toContain('toBeOK can be only used with APIResponse object');
+    await res;
   });
 
   test.describe('toBeOK should print response with text content type when fails', () => {

tests/page/page-wait-for-selector-1.spec.ts

@@ -81,7 +81,7 @@ it('elementHandle.waitForSelector should throw on navigation', async ({ page, se
     await page.evaluate(() => 1);
   await page.goto(server.EMPTY_PAGE);
   const error = await promise;
-  expect(error.message).toContain('Error: frame navigated while waiting for locator(\'span\')');
+  expect(error.message).toContain(`waiting for locator('span') to be visible`);
 });
 
 it('should work with removed MutationObserver', async ({ page, server }) => {
@@ -135,7 +135,6 @@ it('should report logs while waiting for visible', async ({ page, server }) => {
   expect(error.message).toContain(`frame.waitForSelector: Timeout 5000ms exceeded.`);
   expect(error.message).toContain(`waiting for locator(\'div\') to be visible`);
   expect(error.message).toContain(`locator resolved to hidden <div id="mydiv" class="foo bar" foo="1234567890123456…>abcdefghijklmnopqrstuvwyxzabcdefghijklmnopqrstuvw…</div>`);
-  expect(error.message).toContain(`locator did not resolve to any element`);
   expect(error.message).toContain(`locator resolved to hidden <div class="another"></div>`);
 });
 

tests/page/page-wait-for-selector-2.spec.ts

@@ -326,5 +326,5 @@ it('should fail when navigating while on handle', async ({ page, mode, server })
 
   const body = await page.waitForSelector('body');
   const error = await body.waitForSelector('div', { __testHookBeforeAdoptNode } as any).catch(e => e);
-  expect(error.message).toContain('Error: frame navigated while waiting for locator(\'div\')');
+  expect(error.message).toContain(`waiting for locator('div') to be visible`);
 });