chore(evaluate): respect signals when evaluating on handle (#5847)

src/dispatchers/electronDispatcher.ts

@@ -51,12 +51,12 @@ export class ElectronApplicationDispatcher extends Dispatcher<ElectronApplicatio
 
   async evaluateExpression(params: channels.ElectronApplicationEvaluateExpressionParams): Promise<channels.ElectronApplicationEvaluateExpressionResult> {
     const handle = this._object._nodeElectronHandle!;
-    return { value: serializeResult(await handle.evaluateExpression(params.expression, params.isFunction, true /* returnByValue */, parseArgument(params.arg))) };
+    return { value: serializeResult(await handle.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, true /* returnByValue */, parseArgument(params.arg))) };
   }
 
   async evaluateExpressionHandle(params: channels.ElectronApplicationEvaluateExpressionHandleParams): Promise<channels.ElectronApplicationEvaluateExpressionHandleResult> {
     const handle = this._object._nodeElectronHandle!;
-    const result = await handle.evaluateExpression(params.expression, params.isFunction, false /* returnByValue */, parseArgument(params.arg));
+    const result = await handle.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, false /* returnByValue */, parseArgument(params.arg));
     return { handle: createHandle(this._scope, result) };
   }
 

src/dispatchers/elementHandlerDispatcher.ts

@@ -171,11 +171,11 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements chann
   }
 
   async evalOnSelector(params: channels.ElementHandleEvalOnSelectorParams, metadata: CallMetadata): Promise<channels.ElementHandleEvalOnSelectorResult> {
-    return { value: serializeResult(await this._elementHandle.$evalExpression(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
+    return { value: serializeResult(await this._elementHandle.evalOnSelectorAndWaitForSignals(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
   }
 
   async evalOnSelectorAll(params: channels.ElementHandleEvalOnSelectorAllParams, metadata: CallMetadata): Promise<channels.ElementHandleEvalOnSelectorAllResult> {
-    return { value: serializeResult(await this._elementHandle.$$evalExpression(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
+    return { value: serializeResult(await this._elementHandle.evalOnSelectorAllAndWaitForSignals(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
   }
 
   async waitForElementState(params: channels.ElementHandleWaitForElementStateParams, metadata: CallMetadata): Promise<void> {

src/dispatchers/frameDispatcher.ts

@@ -77,11 +77,11 @@ export class FrameDispatcher extends Dispatcher<Frame, channels.FrameInitializer
   }
 
   async evalOnSelector(params: channels.FrameEvalOnSelectorParams, metadata: CallMetadata): Promise<channels.FrameEvalOnSelectorResult> {
-    return { value: serializeResult(await this._frame._$evalExpression(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
+    return { value: serializeResult(await this._frame.evalOnSelectorAndWaitForSignals(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
   }
 
   async evalOnSelectorAll(params: channels.FrameEvalOnSelectorAllParams, metadata: CallMetadata): Promise<channels.FrameEvalOnSelectorAllResult> {
-    return { value: serializeResult(await this._frame._$$evalExpression(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
+    return { value: serializeResult(await this._frame.evalOnSelectorAllAndWaitForSignals(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) };
   }
 
   async querySelector(params: channels.FrameQuerySelectorParams, metadata: CallMetadata): Promise<channels.FrameQuerySelectorResult> {

src/dispatchers/jsHandleDispatcher.ts

@@ -31,11 +31,11 @@ export class JSHandleDispatcher extends Dispatcher<js.JSHandle, channels.JSHandl
   }
 
   async evaluateExpression(params: channels.JSHandleEvaluateExpressionParams): Promise<channels.JSHandleEvaluateExpressionResult> {
-    return { value: serializeResult(await this._object.evaluateExpression(params.expression, params.isFunction, true /* returnByValue */, parseArgument(params.arg))) };
+    return { value: serializeResult(await this._object.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, true /* returnByValue */, parseArgument(params.arg))) };
   }
 
   async evaluateExpressionHandle(params: channels.JSHandleEvaluateExpressionHandleParams): Promise<channels.JSHandleEvaluateExpressionHandleResult> {
-    const jsHandle = await this._object.evaluateExpression(params.expression, params.isFunction, false /* returnByValue */, parseArgument(params.arg));
+    const jsHandle = await this._object.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, false /* returnByValue */, parseArgument(params.arg));
     return { handle: createHandle(this._scope, jsHandle) };
   }
 

src/dispatchers/pageDispatcher.ts

@@ -251,11 +251,11 @@ export class WorkerDispatcher extends Dispatcher<Worker, channels.WorkerInitiali
   }
 
   async evaluateExpression(params: channels.WorkerEvaluateExpressionParams, metadata: CallMetadata): Promise<channels.WorkerEvaluateExpressionResult> {
-    return { value: serializeResult(await this._object._evaluateExpression(params.expression, params.isFunction, parseArgument(params.arg))) };
+    return { value: serializeResult(await this._object.evaluateExpression(params.expression, params.isFunction, parseArgument(params.arg))) };
   }
 
   async evaluateExpressionHandle(params: channels.WorkerEvaluateExpressionHandleParams, metadata: CallMetadata): Promise<channels.WorkerEvaluateExpressionHandleResult> {
-    return { handle: createHandle(this._scope, await this._object._evaluateExpressionHandle(params.expression, params.isFunction, parseArgument(params.arg))) };
+    return { handle: createHandle(this._scope, await this._object.evaluateExpressionHandle(params.expression, params.isFunction, parseArgument(params.arg))) };
   }
 }
 

src/server/dom.ts

@@ -37,6 +37,10 @@ export class FrameExecutionContext extends js.ExecutionContext {
     this.world = world;
   }
 
+  async waitForSignalsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
+    return this.frame._page._frameManager.waitForSignalsCreatedBy(null, false, action);
+  }
+
   adoptIfNeeded(handle: js.JSHandle): Promise<js.JSHandle> | null {
     if (handle instanceof ElementHandle && handle._context !== this)
       return this.frame._page._delegate.adoptElementHandle(handle, this);
@@ -654,18 +658,18 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return this._page.selectors._queryAll(this._context.frame, selector, this, true /* adoptToMain */);
   }
 
-  async $evalExpression(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
+  async evalOnSelectorAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     const handle = await this._page.selectors._query(this._context.frame, selector, this);
     if (!handle)
       throw new Error(`Error: failed to find element matching selector "${selector}"`);
-    const result = await handle.evaluateExpression(expression, isFunction, true, arg);
+    const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg);
     handle.dispose();
     return result;
   }
 
-  async $$evalExpression(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
+  async evalOnSelectorAllAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     const arrayHandle = await this._page.selectors._queryArray(this._context.frame, selector, this);
-    const result = await arrayHandle.evaluateExpression(expression, isFunction, true, arg);
+    const result = await arrayHandle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg);
     arrayHandle.dispose();
     return result;
   }

src/server/frames.ts

@@ -656,18 +656,18 @@ export class Frame extends SdkObject {
     await this._page._doSlowMo();
   }
 
-  async _$evalExpression(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
+  async evalOnSelectorAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     const handle = await this.$(selector);
     if (!handle)
       throw new Error(`Error: failed to find element matching selector "${selector}"`);
-    const result = await handle.evaluateExpression(expression, isFunction, true, arg);
+    const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg);
     handle.dispose();
     return result;
   }
 
-  async _$$evalExpression(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
+  async evalOnSelectorAllAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     const arrayHandle = await this._page.selectors._queryArray(this, selector);
-    const result = await arrayHandle.evaluateExpression(expression, isFunction, true, arg);
+    const result = await arrayHandle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg);
     arrayHandle.dispose();
     return result;
   }

src/server/javascript.ts

@@ -60,6 +60,10 @@ export class ExecutionContext extends SdkObject {
     this._delegate = delegate;
   }
 
+  async waitForSignalsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
+    return action();
+  }
+
   adoptIfNeeded(handle: JSHandle): Promise<JSHandle> | null {
     return null;
   }
@@ -122,8 +126,8 @@ export class JSHandle<T = any> extends SdkObject {
     return evaluate(this._context, false /* returnByValue */, pageFunction, this, arg);
   }
 
-  async evaluateExpression(expression: string, isFunction: boolean | undefined, returnByValue: boolean, arg: any) {
+  async evaluateExpressionAndWaitForSignals(expression: string, isFunction: boolean | undefined, returnByValue: boolean, arg: any) {
-    const value = await evaluateExpression(this._context, returnByValue, expression, isFunction, this, arg);
+    const value = await evaluateExpressionAndWaitForSignals(this._context, returnByValue, expression, isFunction, this, arg);
     await this._context.doSlowMo();
     return value;
   }
@@ -225,6 +229,10 @@ export async function evaluateExpression(context: ExecutionContext, returnByValu
   }
 }
 
+export async function evaluateExpressionAndWaitForSignals(context: ExecutionContext, returnByValue: boolean, expression: string, isFunction?: boolean, ...args: any[]): Promise<any> {
+  return await context.waitForSignalsCreatedBy(() => evaluateExpression(context, returnByValue, expression, isFunction, ...args));
+}
+
 export function parseUnserializableValue(unserializableValue: string): any {
   if (unserializableValue === 'NaN')
     return NaN;

src/server/page.ts

@@ -528,11 +528,11 @@ export class Worker extends SdkObject {
     return this._url;
   }
 
-  async _evaluateExpression(expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
+  async evaluateExpression(expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     return js.evaluateExpression(await this._executionContextPromise, true /* returnByValue */, expression, isFunction, arg);
   }
 
-  async _evaluateExpressionHandle(expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
+  async evaluateExpressionHandle(expression: string, isFunction: boolean | undefined, arg: any): Promise<any> {
     return js.evaluateExpression(await this._executionContextPromise, false /* returnByValue */, expression, isFunction, arg);
   }
 }

test/page-autowaiting-basic.spec.ts

@@ -15,17 +15,74 @@
  * limitations under the License.
  */
 
+import { TestServer } from '../utils/testserver';
 import { it, expect } from './fixtures';
 
-it('should await navigation when clicking anchor', async ({page, server}) => {
+function initServer(server: TestServer): string[] {
   const messages = [];
   server.setRoute('/empty.html', async (req, res) => {
     messages.push('route');
     res.setHeader('Content-Type', 'text/html');
     res.end(`<link rel='stylesheet' href='./one-style.css'>`);
   });
+  return messages;
+}
 
-  await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
+it('should await navigation when clicking anchor', async ({page, server}) => {
+  const messages = initServer(server);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
+  await Promise.all([
+    page.click('a').then(() => messages.push('click')),
+    page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
+  ]);
+  expect(messages.join('|')).toBe('route|navigated|click');
+});
+
+it('should await navigation when clicking anchor programmatically', async ({page, server}) => {
+  const messages = initServer(server);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
+  await Promise.all([
+    page.evaluate(() => (window as any).anchor.click()).then(() => messages.push('click')),
+    page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
+  ]);
+  expect(messages.join('|')).toBe('route|navigated|click');
+});
+
+it('should await navigation when clicking anchor via $eval', async ({page, server}) => {
+  const messages = initServer(server);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
+  await Promise.all([
+    page.$eval('#anchor', anchor => (anchor as any).click()).then(() => messages.push('click')),
+    page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
+  ]);
+  expect(messages.join('|')).toBe('route|navigated|click');
+});
+
+it('should await navigation when clicking anchor via handle.eval', async ({page, server}) => {
+  const messages = initServer(server);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
+  const handle = await page.evaluateHandle('document');
+  await Promise.all([
+    handle.evaluate(doc => (doc as any).getElementById('anchor').click()).then(() => messages.push('click')),
+    page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
+  ]);
+  expect(messages.join('|')).toBe('route|navigated|click');
+});
+
+it('should await navigation when clicking anchor via handle.$eval', async ({page, server}) => {
+  const messages = initServer(server);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
+  const handle = await page.$('body');
+  await Promise.all([
+    handle.$eval('#anchor', anchor => (anchor as any).click()).then(() => messages.push('click')),
+    page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
+  ]);
+  expect(messages.join('|')).toBe('route|navigated|click');
+});
+
+it('should await cross-process navigation when clicking anchor', async ({page, server}) => {
+  const messages = initServer(server);
+  await page.setContent(`<a href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);
 
   await Promise.all([
     page.click('a').then(() => messages.push('click')),
@@ -34,18 +91,12 @@ it('should await navigation when clicking anchor', async ({page, server}) => {
   expect(messages.join('|')).toBe('route|navigated|click');
 });
 
-it('should await cross-process navigation when clicking anchor', async ({page, server}) => {
+it('should await cross-process navigation when clicking anchor programatically', async ({page, server}) => {
-  const messages = [];
+  const messages = initServer(server);
-  server.setRoute('/empty.html', async (req, res) => {
+  await page.setContent(`<a id="anchor" href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);
-    messages.push('route');
-    res.setHeader('Content-Type', 'text/html');
-    res.end(`<link rel='stylesheet' href='./one-style.css'>`);
-  });
-
-  await page.setContent(`<a href="${server.CROSS_PROCESS_PREFIX + '/empty.html'}">empty.html</a>`);
 
   await Promise.all([
-    page.click('a').then(() => messages.push('click')),
+    page.evaluate(() => (window as any).anchor.click()).then(() => messages.push('click')),
     page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
   ]);
   expect(messages.join('|')).toBe('route|navigated|click');
@@ -73,13 +124,7 @@ it('should await form-get on click', async ({page, server}) => {
 });
 
 it('should await form-post on click', async ({page, server}) => {
-  const messages = [];
+  const messages = initServer(server);
-  server.setRoute('/empty.html', async (req, res) => {
-    messages.push('route');
-    res.setHeader('Content-Type', 'text/html');
-    res.end(`<link rel='stylesheet' href='./one-style.css'>`);
-  });
-
   await page.setContent(`
     <form action="${server.EMPTY_PAGE}" method="post">
       <input name="foo" value="bar">
@@ -94,12 +139,7 @@ it('should await form-post on click', async ({page, server}) => {
 });
 
 it('should await navigation when assigning location', async ({page, server}) => {
-  const messages = [];
+  const messages = initServer(server);
-  server.setRoute('/empty.html', async (req, res) => {
-    messages.push('route');
-    res.setHeader('Content-Type', 'text/html');
-    res.end(`<link rel='stylesheet' href='./one-style.css'>`);
-  });
   await Promise.all([
     page.evaluate(`window.location.href = "${server.EMPTY_PAGE}"`).then(() => messages.push('evaluate')),
     page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
@@ -120,14 +160,8 @@ it('should await navigation when assigning location twice', async ({page, server
 });
 
 it('should await navigation when evaluating reload', async ({page, server}) => {
-  const messages = [];
   await page.goto(server.EMPTY_PAGE);
-  server.setRoute('/empty.html', async (req, res) => {
+  const messages = initServer(server);
-    messages.push('route');
-    res.setHeader('Content-Type', 'text/html');
-    res.end(`<link rel='stylesheet' href='./one-style.css'>`);
-  });
-
   await Promise.all([
     page.evaluate(`window.location.reload()`).then(() => messages.push('evaluate')),
     page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
@@ -135,48 +169,21 @@ it('should await navigation when evaluating reload', async ({page, server}) => {
   expect(messages.join('|')).toBe('route|navigated|evaluate');
 });
 
-it('should await navigating specified target', async ({page, server}) => {
-  const messages = [];
-  server.setRoute('/empty.html', async (req, res) => {
-    messages.push('route');
-    res.setHeader('Content-Type', 'text/html');
-    res.end(`<link rel='stylesheet' href='./one-style.css'>`);
-  });
-
-  await page.setContent(`
-    <a href="${server.EMPTY_PAGE}" target=target>empty.html</a>
-    <iframe name=target></iframe>
-  `);
-  const frame = page.frame({ name: 'target' });
-  await Promise.all([
-    page.click('a').then(() => messages.push('click')),
-    page.waitForEvent('framenavigated').then(() => messages.push('navigated')),
-  ]);
-  expect(frame.url()).toBe(server.EMPTY_PAGE);
-  expect(messages.join('|')).toBe('route|navigated|click');
-});
-
 it('should work with noWaitAfter: true', async ({page, server}) => {
   server.setRoute('/empty.html', async () => {});
-  await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
   await page.click('a', { noWaitAfter: true });
 });
 
 it('should work with dblclick noWaitAfter: true', async ({page, server}) => {
   server.setRoute('/empty.html', async () => {});
-  await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
   await page.dblclick('a', { noWaitAfter: true });
 });
 
 it('should work with waitForLoadState(load)', async ({page, server}) => {
-  const messages = [];
+  const messages = initServer(server);
-  server.setRoute('/empty.html', async (req, res) => {
+  await page.setContent(`<a id="anchor" href="${server.EMPTY_PAGE}">empty.html</a>`);
-    messages.push('route');
-    res.setHeader('Content-Type', 'text/html');
-    res.end(`<link rel='stylesheet' href='./one-style.css'>`);
-  });
-
-  await page.setContent(`<a href="${server.EMPTY_PAGE}">empty.html</a>`);
   await Promise.all([
     page.click('a').then(() => page.waitForLoadState('load')).then(() => messages.push('clickload')),
     page.waitForEvent('framenavigated').then(() => page.waitForLoadState('domcontentloaded')).then(() => messages.push('domcontentloaded')),