From 9292a56eafabf8dbe8ef0fb0fffe0db0ca4020eb Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <aslushnikov@gmail.com>
Date: Mon, 11 Sep 2017 19:20:02 -0700
Subject: [PATCH] [api] Teach page.evaluate to accept element handles as
 parameters (#725)

This patch:
- teaches `page.evaluate` to accept ElementHandles as parameters
- removes `ElementHandle.evaluate` method since it's not needed any
  more

References #382
---
 docs/api.md          | 59 ++++++++++++++------------------
 lib/ElementHandle.js | 34 +++++++------------
 lib/FrameManager.js  | 51 ++++++++++++++++++++++++----
 lib/Page.js          |  2 +-
 test/test.js         | 80 +++++++++++++++++++-------------------------
 5 files changed, 118 insertions(+), 108 deletions(-)

diff --git a/docs/api.md b/docs/api.md
index 5e9be546..22b0b376 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -116,7 +116,6 @@
   * [class: ElementHandle](#class-elementhandle)
     + [elementHandle.click([options])](#elementhandleclickoptions)
     + [elementHandle.dispose()](#elementhandledispose)
-    + [elementHandle.evaluate(pageFunction, ...args)](#elementhandleevaluatepagefunction-args)
     + [elementHandle.hover()](#elementhandlehover)
     + [elementHandle.tap()](#elementhandletap)
     + [elementHandle.uploadFile(...filePaths)](#elementhandleuploadfilefilepaths)
@@ -333,7 +332,7 @@ Shortcut for [page.mainFrame().$$(selector)](#frameselector-1).
 #### page.$eval(selector, pageFunction[, ...args])
 - `selector` <[string]> A [selector] to query page for
 - `pageFunction` <[function]> Function to be evaluated in browser context
-- `...args` <...[Serializable]> Arguments to pass to `pageFunction`
+- `...args` <...[Serializable]|[ElementHandle]> Arguments to pass to `pageFunction`
 - returns: <[Promise]<[Serializable]>> Promise which resolves to the return value of `pageFunction`
 
 This method runs `document.querySelector` within the page and passes it as the first argument to `pageFunction`. If there's no element matching `selector`, the method throws an error.
@@ -450,22 +449,16 @@ List of all available devices is available in the source code: [DeviceDescriptor
 
 #### page.evaluate(pageFunction, ...args)
 - `pageFunction` <[function]|[string]> Function to be evaluated in the page context
-- `...args` <...[Serializable]> Arguments to pass to `pageFunction`
+- `...args` <...[Serializable]|[ElementHandle]> Arguments to pass to `pageFunction`
 - returns: <[Promise]<[Serializable]>> Resolves to the return value of `pageFunction`
 
 If the function, passed to the `page.evaluate`, returns a [Promise], then `page.evaluate` would wait for the promise to resolve and return it's value.
 
 ```js
-const puppeteer = require('puppeteer');
-
-puppeteer.launch().then(async browser => {
-  const page = await browser.newPage();
-  const result = await page.evaluate(() => {
-    return Promise.resolve(8 * 7);
-  });
-  console.log(result); // prints "56"
-  browser.close();
+const result = await page.evaluate(() => {
+  return Promise.resolve(8 * 7);
 });
+console.log(result); // prints "56"
 ```
 
 A string can also be passed in instead of a function.
@@ -474,6 +467,13 @@ A string can also be passed in instead of a function.
 console.log(await page.evaluate('1 + 2')); // prints "3"
 ```
 
+[ElementHandle] instances could be passed as arguments to the `page.evaluate`:
+```js
+const bodyHandle = await page.$('body');
+const html = await page.evaluate(body => body.innerHTML, bodyHandle);
+await bodyHandle.dispose();
+```
+
 Shortcut for [page.mainFrame().evaluate(pageFunction, ...args)](#frameevaluatepagefunction-args).
 
 #### page.evaluateOnNewDocument(pageFunction, ...args)
@@ -1113,7 +1113,7 @@ The method runs `document.querySelectorAll` within the frame. If no elements mat
 #### frame.$eval(selector, pageFunction[, ...args])
 - `selector` <[string]> A [selector] to query frame for
 - `pageFunction` <[function]> Function to be evaluated in browser context
-- `...args` <...[Serializable]> Arguments to pass to `pageFunction`
+- `...args` <...[Serializable]|[ElementHandle]> Arguments to pass to `pageFunction`
 - returns: <[Promise]<[Serializable]>> Promise which resolves to the return value of `pageFunction`
 
 This method runs `document.querySelector` within the frame and passes it as the first argument to `pageFunction`. If there's no element matching `selector`, the method throws an error.
@@ -1138,28 +1138,29 @@ Adds a `<script>` tag to the frame with the desired url. Alternatively, JavaScri
 
 #### frame.evaluate(pageFunction, ...args)
 - `pageFunction` <[function]|[string]> Function to be evaluated in browser context
-- `...args` <...[Serializable]> Arguments to pass to  `pageFunction`
+- `...args` <...[Serializable]|[ElementHandle]> Arguments to pass to  `pageFunction`
 - returns: <[Promise]<[Serializable]>> Promise which resolves to function return value
 
-If the function, passed to the `page.evaluate`, returns a [Promise], then `page.evaluate` would wait for the promise to resolve and return it's value.
+If the function, passed to the `frame.evaluate`, returns a [Promise], then `frame.evaluate` would wait for the promise to resolve and return it's value.
 
 ```js
-const puppeteer = require('puppeteer');
-
-puppeteer.launch().then(async browser => {
-  const page = await browser.newPage();
-  const result = await page.mainFrame().evaluate(() => {
-    return Promise.resolve(8 * 7);
-  });
-  console.log(result); // prints "56"
-  browser.close();
+const result = await frame.evaluate(() => {
+  return Promise.resolve(8 * 7);
 });
+console.log(result); // prints "56"
 ```
 
 A string can also be passed in instead of a function.
 
 ```js
-console.log(await page.mainFrame().evaluate('1 + 2')); // prints "3"
+console.log(await frame.evaluate('1 + 2')); // prints "3"
+```
+
+[ElementHandle] instances could be passed as arguments to the `frame.evaluate`:
+```js
+const bodyHandle = await frame.$('body');
+const html = await frame.evaluate(body => body.innerHTML, bodyHandle);
+await bodyHandle.dispose();
 ```
 
 #### frame.injectFile(filePath)
@@ -1286,14 +1287,6 @@ If the element is detached from DOM, the method throws an error.
 
 The `elementHandle.dispose` method stops referencing the element handle.
 
-#### elementHandle.evaluate(pageFunction, ...args)
-- `pageFunction` <[function]> Function to be evaluated in browser context
-- `...args` <...[Serializable]> Arguments to pass to  `pageFunction`
-- returns: <[Promise]<[Serializable]>> Promise which resolves to function return value
-
-If the function, passed to the `elementHandle.evaluate`, returns a [Promise], then `elementHandle.evaluate` would wait for the promise to resolve and return it's value.
-The element will be passed as the first argument to `pageFunction`, followed by any `args`.
-
 #### elementHandle.hover()
 - returns: <[Promise]> Promise which resolves when the element is successfully hovered.
 
diff --git a/lib/ElementHandle.js b/lib/ElementHandle.js
index a3c71334..ad4a5e41 100644
--- a/lib/ElementHandle.js
+++ b/lib/ElementHandle.js
@@ -18,12 +18,14 @@ const {helper} = require('./helper');
 
 class ElementHandle {
   /**
+   * @param {!Frame} frame
    * @param {!Connection} client
    * @param {!Object} remoteObject
    * @param {!Mouse} mouse
    * @param {!Touchscreen} touchscreen;
    */
-  constructor(client, remoteObject, mouse, touchscreen) {
+  constructor(frame, client, remoteObject, mouse, touchscreen) {
+    this._frame = frame;
     this._client = client;
     this._remoteObject = remoteObject;
     this._mouse = mouse;
@@ -31,6 +33,13 @@ class ElementHandle {
     this._disposed = false;
   }
 
+  /**
+   * @return {?string}
+   */
+  _remoteObjectId() {
+    return this._disposed ? null : this._remoteObject.objectId;
+  }
+
   async dispose() {
     if (this._disposed)
       return;
@@ -38,30 +47,11 @@ class ElementHandle {
     await helper.releaseObject(this._client, this._remoteObject);
   }
 
-  /**
-   * @param {function()} pageFunction
-   * @param {!Array<*>} args
-   * @return {!Promise<(!Object|undefined)>}
-   */
-  async evaluate(pageFunction, ...args) {
-    console.assert(!this._disposed, 'ElementHandle is disposed!');
-    console.assert(typeof pageFunction === 'function', 'First argument to ElementHandle.evaluate must be a function!');
-
-    const stringifiedArgs = ['this'];
-    stringifiedArgs.push(...args.map(x => JSON.stringify(x)));
-    const functionDeclaration = `function() { return (${pageFunction})(${stringifiedArgs.join(',')}) }`;
-    const objectId = this._remoteObject.objectId;
-    const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', { objectId, functionDeclaration, returnByValue: false, awaitPromise: true});
-    if (exceptionDetails)
-      throw new Error('Evaluation failed: ' + helper.getExceptionMessage(exceptionDetails));
-    return await helper.serializeRemoteObject(this._client, remoteObject);
-  }
-
   /**
    * @return {!Promise<{x: number, y: number}>}
    */
   async _visibleCenter() {
-    const center = await this.evaluate(element => {
+    const center = await this._frame.evaluate(element => {
       if (!element.ownerDocument.contains(element))
         return null;
       element.scrollIntoViewIfNeeded();
@@ -70,7 +60,7 @@ class ElementHandle {
         x: (Math.max(rect.left, 0) + Math.min(rect.right, window.innerWidth)) / 2,
         y: (Math.max(rect.top, 0) + Math.min(rect.bottom, window.innerHeight)) / 2
       };
-    });
+    }, this);
     if (!center)
       throw new Error('No node found for selector: ' + selector);
     return center;
diff --git a/lib/FrameManager.js b/lib/FrameManager.js
index e4ae6478..a98b37d6 100644
--- a/lib/FrameManager.js
+++ b/lib/FrameManager.js
@@ -194,7 +194,7 @@ class Frame {
   async $(selector) {
     const remoteObject = await this._rawEvaluate(selector => document.querySelector(selector), selector);
     if (remoteObject.subtype === 'node')
-      return new ElementHandle(this._client, remoteObject, this._mouse, this._touchscreen);
+      return new ElementHandle(this, this._client, remoteObject, this._mouse, this._touchscreen);
     await helper.releaseObject(this._client, remoteObject);
     return null;
   }
@@ -209,7 +209,8 @@ class Frame {
     const elementHandle = await this.$(selector);
     if (!elementHandle)
       throw new Error(`Error: failed to find element matching selector "${selector}"`);
-    const result = await elementHandle.evaluate(pageFunction, ...args);
+    args = [elementHandle].concat(args);
+    const result = await this.evaluate(pageFunction, ...args);
     await elementHandle.dispose();
     return result;
   }
@@ -229,7 +230,7 @@ class Frame {
     const releasePromises = [helper.releaseObject(this._client, remoteObject)];
     for (const property of properties) {
       if (property.enumerable && property.value.subtype === 'node')
-        result.push(new ElementHandle(this._client, property.value, this._mouse, this._touchscreen));
+        result.push(new ElementHandle(this, this._client, property.value, this._mouse, this._touchscreen));
       else
         releasePromises.push(helper.releaseObject(this._client, property.value));
     }
@@ -243,12 +244,50 @@ class Frame {
    * @return {!Promise<(!Object|undefined)>}
    */
   async _rawEvaluate(pageFunction, ...args) {
-    const expression = helper.evaluationString(pageFunction, ...args);
-    const contextId = this._defaultContextId;
-    const { exceptionDetails, result: remoteObject }  = await this._client.send('Runtime.evaluate', { expression, contextId, returnByValue: false, awaitPromise: true});
+    if (helper.isString(pageFunction)) {
+      const contextId = this._defaultContextId;
+      const expression = pageFunction;
+      const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.evaluate', { expression, contextId, returnByValue: false, awaitPromise: true});
+      if (exceptionDetails)
+        throw new Error('Evaluation failed: ' + helper.getExceptionMessage(exceptionDetails));
+      return remoteObject;
+    }
+
+    const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', {
+      functionDeclaration: pageFunction.toString(),
+      executionContextId: this._defaultContextId,
+      arguments: args.map(convertArgument.bind(this)),
+      returnByValue: false,
+      awaitPromise: true
+    });
     if (exceptionDetails)
       throw new Error('Evaluation failed: ' + helper.getExceptionMessage(exceptionDetails));
     return remoteObject;
+
+    /**
+     * @param {*} arg
+     * @return {*}
+     * @this {Frame}
+     */
+    function convertArgument(arg) {
+      if (Object.is(arg, -0))
+        return { unserializableValue: '-0' };
+      if (Object.is(arg, Infinity))
+        return { unserializableValue: 'Infinity' };
+      if (Object.is(arg, -Infinity))
+        return { unserializableValue: '-Infinity' };
+      if (Object.is(arg, NaN))
+        return { unserializableValue: 'NaN' };
+      if (arg instanceof ElementHandle) {
+        if (arg._frame !== this)
+          throw new Error('ElementHandles passed as arguments should belong to the frame that does evaluation');
+        const objectId = arg._remoteObjectId();
+        if (!objectId)
+          throw new Error('ElementHandle is disposed!');
+        return { objectId };
+      }
+      return { value: arg };
+    }
   }
 
   /**
diff --git a/lib/Page.js b/lib/Page.js
index c75d26c5..5f35d35a 100644
--- a/lib/Page.js
+++ b/lib/Page.js
@@ -689,7 +689,7 @@ class Page extends EventEmitter {
   async focus(selector) {
     const handle = await this.$(selector);
     console.assert(handle, 'No node found for selector: ' + selector);
-    await handle.evaluate(element => element.focus());
+    await this.evaluate(element => element.focus(), handle);
     await handle.dispose();
   }
 
diff --git a/test/test.js b/test/test.js
index 01634278..dc9b743c 100644
--- a/test/test.js
+++ b/test/test.js
@@ -266,6 +266,30 @@ describe('Page', function() {
       const result = await page.evaluate('2 + 5;\n// do some math!');
       expect(result).toBe(7);
     }));
+    it('should accept element handle as an argument', SX(async function() {
+      await page.setContent('<section>42</section>');
+      const element = await page.$('section');
+      const text = await page.evaluate(e => e.textContent, element);
+      expect(text).toBe('42');
+    }));
+    it('should throw if underlying element was disposed', SX(async function() {
+      await page.setContent('<section>39</section>');
+      const element = await page.$('section');
+      expect(element).toBeTruthy();
+      await element.dispose();
+      let error = null;
+      await page.evaluate(e => e.textContent, element).catch(e => error = e);
+      expect(error.message).toContain('ElementHandle is disposed');
+    }));
+    it('should throw if elementHandles are from other frames', SX(async function() {
+      const FrameUtils = require('./frame-utils');
+      await FrameUtils.attachFrame(page, 'frame1', EMPTY_PAGE);
+      const bodyHandle = await page.frames()[1].$('body');
+      let error = null;
+      await page.evaluate(body => body.innerHTML, bodyHandle).catch(e => error = e);
+      expect(error).toBeTruthy();
+      expect(error.message).toContain('ElementHandles passed as arguments should belong');
+    }));
   });
 
   describe('Page.injectFile', function() {
@@ -1151,6 +1175,12 @@ describe('Page', function() {
       const text = await page.$eval('section', (e, suffix) => e.textContent + suffix, ' world!');
       expect(text).toBe('hello world!');
     }));
+    it('should accept ElementHandles as arguments', SX(async function() {
+      await page.setContent('<section>hello</section><div> world</div>');
+      const divHandle = await page.$('div');
+      const text = await page.$eval('section', (e, div) => e.textContent + div.textContent, divHandle);
+      expect(text).toBe('hello world');
+    }));
     it('should throw error if no element is found', SX(async function() {
       let error = null;
       await page.$eval('section', e => e.id).catch(e => error = e);
@@ -1174,7 +1204,7 @@ describe('Page', function() {
       await page.setContent('<div>A</div><br/><div>B</div>');
       const elements = await page.$$('div');
       expect(elements.length).toBe(2);
-      const promises = elements.map(element => element.evaluate(e => e.textContent));
+      const promises = elements.map(element => page.evaluate(e => e.textContent, element));
       expect(await Promise.all(promises)).toEqual(['A', 'B']);
     }));
     it('should return empty array if nothing is found', SX(async function() {
@@ -1184,48 +1214,6 @@ describe('Page', function() {
     }));
   });
 
-  describe('ElementHandle.evaluate', function() {
-    it('should work', SX(async function() {
-      await page.setContent('<section>42</section>');
-      const element = await page.$('section');
-      const text = await element.evaluate(e => e.textContent);
-      expect(text).toBe('42');
-    }));
-    it('should await promise if any', SX(async function() {
-      await page.setContent('<section>39</section>');
-      const element = await page.$('section');
-      const text = await element.evaluate(e => Promise.resolve(e.textContent));
-      expect(text).toBe('39');
-    }));
-    it('should throw if underlying page got closed', SX(async function() {
-      const otherPage = await browser.newPage();
-      await otherPage.setContent('<section>88</section>');
-      const element = await otherPage.$('section');
-      expect(element).toBeTruthy();
-      await otherPage.close();
-      let error = null;
-      try {
-        await element.evaluate(e => e.textContent);
-      } catch (e) {
-        error = e;
-      }
-      expect(error.message).toContain('Session closed');
-    }));
-    it('should throw if underlying element was disposed', SX(async function() {
-      await page.setContent('<section>39</section>');
-      const element = await page.$('section');
-      expect(element).toBeTruthy();
-      await element.dispose();
-      let error = null;
-      try {
-        await element.evaluate(e => e.textContent);
-      } catch (e) {
-        error = e;
-      }
-      expect(error.message).toContain('ElementHandle is disposed');
-    }));
-  });
-
   describe('ElementHandle.click', function() {
     it('should work', SX(async function() {
       await page.goto(PREFIX + '/input/button.html');
@@ -1284,13 +1272,13 @@ describe('Page', function() {
       const filePath = path.relative(process.cwd(), __dirname + '/assets/file-to-upload.txt');
       const input = await page.$('input');
       await input.uploadFile(filePath);
-      expect(await input.evaluate(e => e.files[0].name)).toBe('file-to-upload.txt');
-      expect(await input.evaluate(e => {
+      expect(await page.evaluate(e => e.files[0].name, input)).toBe('file-to-upload.txt');
+      expect(await page.evaluate(e => {
         const reader = new FileReader();
         const promise = new Promise(fulfill => reader.onload = fulfill);
         reader.readAsText(e.files[0]);
         return promise.then(() => reader.result);
-      })).toBe('contents of the file');
+      }, input)).toBe('contents of the file');
     }));
     it('should move with the arrow keys', SX(async function(){
       await page.goto(PREFIX + '/input/textarea.html');