feat: worker convenience methods (#2677)

This patch:
- adds `worker.evaluate` and `worker.evaluateHandle` methods as a shortcut to their execution context equivalents.
- makes the error messages a bit nicer when interacting with a closed worker (as opposed to a closed page).
- moves the worker tests into their own spec file.

docs/api.md

@@ -132,6 +132,8 @@
   * [page.waitForXPath(xpath[, options])](#pagewaitforxpathxpath-options)
   * [page.workers()](#pageworkers)
 - [class: Worker](#class-worker)
+  * [worker.evaluate(pageFunction, ...args)](#workerevaluatepagefunction-args)
+  * [worker.evaluateHandle(pageFunction, ...args)](#workerevaluatehandlepagefunction-args)
   * [worker.executionContext()](#workerexecutioncontext)
   * [worker.url()](#workerurl)
 - [class: Keyboard](#class-keyboard)
@@ -1645,6 +1647,28 @@ for (const worker of page.workers())
   console.log('  ' + worker.url());
 ```
 
+#### worker.evaluate(pageFunction, ...args)
+- `pageFunction` <[function]|[string]> Function to be evaluated in the worker context
+- `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction`
+- returns: <[Promise]<[Serializable]>> Promise which resolves to the return value of `pageFunction`
+
+If the function passed to the `worker.evaluate` returns a [Promise], then `worker.evaluate` would wait for the promise to resolve and return its value.
+
+If the function passed to the `worker.evaluate` returns a non-[Serializable] value, then `worker.evaluate` resolves to `undefined`.
+
+Shortcut for [(await worker.executionContext()).evaluate(pageFunction, ...args)](#executioncontextevaluatepagefunction-args).
+
+#### worker.evaluateHandle(pageFunction, ...args)
+- `pageFunction` <[function]|[string]> Function to be evaluated in the page context
+- `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction`
+- returns: <[Promise]<[JSHandle]>> Promise which resolves to the return value of `pageFunction` as in-page object (JSHandle)
+
+The only difference between `worker.evaluate` and `worker.evaluateHandle` is that `worker.evaluateHandle` returns in-page object (JSHandle).
+
+If the function passed to the `worker.evaluateHandle` returns a [Promise], then `worker.evaluateHandle` would wait for the promise to resolve and return its value.
+
+Shortcut for [(await worker.executionContext()).evaluateHandle(pageFunction, ...args)](#executioncontextevaluatehandlepagefunction-args).
+
 #### worker.executionContext()
 - returns: <[Promise]<[ExecutionContext]>>
 

lib/Browser.js

@@ -107,7 +107,7 @@ class Browser extends EventEmitter {
     const {browserContextId} = targetInfo;
     const context = (browserContextId && this._contexts.has(browserContextId)) ? this._contexts.get(browserContextId) : this._defaultContext;
 
-    const target = new Target(targetInfo, context, () => this._connection.createSession(targetInfo.targetId), this._ignoreHTTPSErrors, this._setDefaultViewport, this._screenshotTaskQueue);
+    const target = new Target(targetInfo, context, () => this._connection.createSession(targetInfo), this._ignoreHTTPSErrors, this._setDefaultViewport, this._screenshotTaskQueue);
     assert(!this._targets.has(event.targetInfo.targetId), 'Target should not exist before targetCreated');
     this._targets.set(event.targetInfo.targetId, target);
 

lib/Connection.js

@@ -150,12 +150,12 @@ class Connection extends EventEmitter {
   }
 
   /**
-   * @param {string} targetId
+   * @param {Protocol.Target.TargetInfo} targetInfo
    * @return {!Promise<!CDPSession>}
    */
-  async createSession(targetId) {
+  async createSession(targetInfo) {
-    const {sessionId} = await this.send('Target.attachToTarget', {targetId});
+    const {sessionId} = await this.send('Target.attachToTarget', {targetId: targetInfo.targetId});
-    const session = new CDPSession(this, targetId, sessionId);
+    const session = new CDPSession(this, targetInfo.type, sessionId);
     this._sessions.set(sessionId, session);
     return session;
   }
@@ -164,16 +164,16 @@ class Connection extends EventEmitter {
 class CDPSession extends EventEmitter {
   /**
    * @param {!Connection|!CDPSession} connection
-   * @param {string} targetId
+   * @param {string} targetType
    * @param {string} sessionId
    */
-  constructor(connection, targetId, sessionId) {
+  constructor(connection, targetType, sessionId) {
     super();
     this._lastId = 0;
     /** @type {!Map<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
     this._callbacks = new Map();
     this._connection = connection;
-    this._targetId = targetId;
+    this._targetType = targetType;
     this._sessionId = sessionId;
     /** @type {!Map<string, !CDPSession>}*/
     this._sessions = new Map();
@@ -186,7 +186,7 @@ class CDPSession extends EventEmitter {
    */
   send(method, params = {}) {
     if (!this._connection)
-      return Promise.reject(new Error(`Protocol error (${method}): Session closed. Most likely the page has been closed.`));
+      return Promise.reject(new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`));
     const id = ++this._lastId;
     const message = JSON.stringify({id, method, params});
     debugSession('SEND ► ' + message);
@@ -245,11 +245,11 @@ class CDPSession extends EventEmitter {
   }
 
   /**
-   * @param {string} targetId
+   * @param {string} targetType
    * @param {string} sessionId
    */
-  _createSession(targetId, sessionId) {
+  _createSession(targetType, sessionId) {
-    const session = new CDPSession(this, targetId, sessionId);
+    const session = new CDPSession(this, targetType, sessionId);
     this._sessions.set(sessionId, session);
     return session;
   }

lib/Page.js

@@ -100,7 +100,7 @@ class Page extends EventEmitter {
         }).catch(debugError);
         return;
       }
-      const session = client._createSession(event.targetInfo.targetId, event.sessionId);
+      const session = client._createSession(event.targetInfo.type, event.sessionId);
       const worker = new Worker(session, event.targetInfo.url, this._onLogEntryAdded.bind(this, session));
       this._workers.set(event.sessionId, worker);
       this.emit(Page.Events.WorkerCreated, worker);

lib/Worker.js

@@ -53,6 +53,24 @@ class Worker extends EventEmitter {
   async executionContext() {
     return this._executionContextPromise;
   }
+
+  /**
+   * @param {function()|string} pageFunction
+   * @param {!Array<*>} args
+   * @return {!Promise<*>}
+   */
+  async evaluate(pageFunction, ...args) {
+    return (await this._executionContextPromise).evaluate(pageFunction, ...args);
+  }
+
+  /**
+   * @param {function()|string} pageFunction
+   * @param {!Array<*>} args
+   * @return {!Promise<!JSHandle>}
+   */
+  async evaluateHandle(pageFunction, ...args) {
+    return (await this._executionContextPromise).evaluateHandle(pageFunction, ...args);
+  }
 }
 
 module.exports = Worker;

test/page.spec.js

@@ -1625,34 +1625,6 @@ module.exports.addTests = function({testRunner, expect, puppeteer, DeviceDescrip
       await closedPromise;
     });
   });
-  describe('Workers', function() {
-    it('Page.workers', async function({page, server}) {
-      await Promise.all([
-        new Promise(x => page.once('workercreated', x)),
-        page.goto(server.PREFIX + '/worker/worker.html')]);
-      const worker = page.workers()[0];
-      expect(worker.url()).toContain('worker.js');
-      const executionContext = await worker.executionContext();
-      expect(await executionContext.evaluate(() => self.workerFunction())).toBe('worker function result');
-
-      await page.goto(server.EMPTY_PAGE);
-      expect(page.workers()).toEqual([]);
-    });
-    it('should emit created and destroyed events', async function({page}) {
-      const workerCreatedPromise = new Promise(x => page.once('workercreated', x));
-      const workerObj = await page.evaluateHandle(() => new Worker('data:text/javascript,1'));
-      const worker = await workerCreatedPromise;
-      const workerDestroyedPromise = new Promise(x => page.once('workerdestroyed', x));
-      await page.evaluate(workerObj => workerObj.terminate(), workerObj);
-      expect(await workerDestroyedPromise).toBe(worker);
-    });
-    it('should report console logs', async function({page}) {
-      const logPromise = new Promise(x => page.on('console', x));
-      await page.evaluate(() => new Worker(`data:text/javascript,console.log(1)`));
-      const log = await logPromise;
-      expect(log.text()).toBe('1');
-    });
-  });
 
   describe('Page.browser', function() {
     it('should return the correct browser instance', async function({ page, browser }) {

test/test.js

@@ -145,6 +145,7 @@ describe('Page', function() {
   require('./page.spec.js').addTests({testRunner, expect, puppeteer, DeviceDescriptors, headless});
   require('./target.spec.js').addTests({testRunner, expect, puppeteer});
   require('./tracing.spec.js').addTests({testRunner, expect});
+  require('./worker.spec.js').addTests({testRunner, expect});
 });
 
 // Top-level tests that launch Browser themselves.

test/worker.spec.js

@@ -0,0 +1,44 @@
+
+module.exports.addTests = function({testRunner, expect}) {
+  const {describe, xdescribe, fdescribe} = testRunner;
+  const {it, fit, xit} = testRunner;
+  const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
+
+  describe('Workers', function() {
+    it('Page.workers', async function({page, server}) {
+      await Promise.all([
+        new Promise(x => page.once('workercreated', x)),
+        page.goto(server.PREFIX + '/worker/worker.html')]);
+      const worker = page.workers()[0];
+      expect(worker.url()).toContain('worker.js');
+
+      expect(await worker.evaluate(() => self.workerFunction())).toBe('worker function result');
+
+      await page.goto(server.EMPTY_PAGE);
+      expect(page.workers()).toEqual([]);
+    });
+    it('should emit created and destroyed events', async function({page}) {
+      const workerCreatedPromise = new Promise(x => page.once('workercreated', x));
+      const workerObj = await page.evaluateHandle(() => new Worker('data:text/javascript,1'));
+      const worker = await workerCreatedPromise;
+      const workerThisObj = await worker.evaluateHandle(() => this);
+      const workerDestroyedPromise = new Promise(x => page.once('workerdestroyed', x));
+      await page.evaluate(workerObj => workerObj.terminate(), workerObj);
+      expect(await workerDestroyedPromise).toBe(worker);
+      const error = await workerThisObj.getProperty('self').catch(error => error);
+      expect(error.message).toContain('Most likely the worker has been closed.');
+    });
+    it('should report console logs', async function({page}) {
+      const logPromise = new Promise(x => page.on('console', x));
+      await page.evaluate(() => new Worker(`data:text/javascript,console.log(1)`));
+      const log = await logPromise;
+      expect(log.text()).toBe('1');
+    });
+    it('should have an execution context', async function({page}) {
+      const workerCreatedPromise = new Promise(x => page.once('workercreated', x));
+      await page.evaluate(() => new Worker(`data:text/javascript,console.log(1)`));
+      const worker = await workerCreatedPromise;
+      expect(await (await worker.executionContext()).evaluate('1+1')).toBe(2);
+    });
+  });
+};