Move screenshot task chain in Browser

Currently, it's impossible to do screenshots in parallel.
This patch:
- makes all screenshot tasks sequential inside one browser
- starts activating target before taking screenshot
- adds a test to make sure it's possible to take screenshots across
  tabs
- starts waiting for a proper page closing after each test. This might
  finally solve the ECONNRESET issues in tests.

References #89

lib/Browser.js

@@ -65,6 +65,7 @@ class Browser {
     this._terminated = false;
     this._chromeProcess = null;
     this._launchPromise = null;
+    this._screenshotTaskQueue = new TaskQueue();
 
     this.stderr = new ProxyStream();
     this.stdout = new ProxyStream();
@@ -78,7 +79,7 @@ class Browser {
     if (!this._chromeProcess || this._terminated)
       throw new Error('ERROR: this chrome instance is not alive any more!');
     let client = await Connection.create(this._remoteDebuggingPort);
-    let page = await Page.create(client);
+    let page = await Page.create(client, this._screenshotTaskQueue);
     return page;
   }
 
@@ -162,6 +163,22 @@ function waitForRemoteDebuggingPort(chromeProcess) {
   }
 }
 
+class TaskQueue {
+  constructor() {
+    this._chain = Promise.resolve();
+  }
+
+  /**
+   * @param {function():!Promise} task
+   * @return {!Promise}
+   */
+  postTask(task) {
+    let result = this._chain.then(task);
+    this._chain = result.catch(() => {});
+    return result;
+  }
+}
+
 class ProxyStream extends Duplex {
   _read() { }
 

lib/Connection.js

@@ -23,13 +23,13 @@ const COMMAND_TIMEOUT = 10000;
 class Connection extends EventEmitter {
   /**
    * @param {number} port
-   * @param {string} pageId
+   * @param {string} targetId
    * @param {!WebSocket} ws
    */
-  constructor(port, pageId, ws) {
+  constructor(port, targetId, ws) {
     super();
     this._port = port;
-    this._pageId = pageId;
+    this._targetId = targetId;
     this._lastId = 0;
     /** @type {!Map<number, {resolve: function(*), reject: function(*), method: string}>}*/
     this._callbacks = new Map();
@@ -39,6 +39,13 @@ class Connection extends EventEmitter {
     this._ws.on('close', this._onClose.bind(this));
   }
 
+  /**
+   * @return {string}
+   */
+  targetId() {
+    return this._targetId;
+  }
+
   /**
    * @param {string} method
    * @param {(!Object|undefined)} params
@@ -84,7 +91,7 @@ class Connection extends EventEmitter {
    * @return {!Promise}
    */
   async dispose() {
-    await runJsonCommand(this._port, `close/${this._pageId}`);
+    await runJsonCommand(this._port, `close/${this._targetId}`);
   }
 
   /**

lib/Page.js

@@ -28,9 +28,10 @@ let helper = require('./helper');
 class Page extends EventEmitter {
   /**
    * @param {!Connection} client
+   * @param {!TaskQueue} screenshotTaskQueue
    * @return {!Promise<!Page>}
    */
-  static async create(client) {
+  static async create(client, screenshotTaskQueue) {
     await Promise.all([
       client.send('Network.enable', {}),
       client.send('Page.enable', {}),
@@ -41,7 +42,7 @@ class Page extends EventEmitter {
     let {result:{value: userAgent}} = await client.send('Runtime.evaluate', { expression: userAgentExpression, returnByValue: true });
     let frameManager = await FrameManager.create(client);
     let networkManager = new NetworkManager(client, userAgent);
-    let page = new Page(client, frameManager, networkManager);
+    let page = new Page(client, frameManager, networkManager, screenshotTaskQueue);
     // Initialize default page size.
     await page.setViewport({width: 400, height: 300});
     return page;
@@ -51,8 +52,9 @@ class Page extends EventEmitter {
    * @param {!Connection} client
    * @param {!FrameManager} frameManager
    * @param {!NetworkManager} networkManager
+   * @param {!TaskQueue} screenshotTaskQueue
    */
-  constructor(client, frameManager, networkManager) {
+  constructor(client, frameManager, networkManager, screenshotTaskQueue) {
     super();
     this._client = client;
     this._frameManager = frameManager;
@@ -62,7 +64,7 @@ class Page extends EventEmitter {
 
     this._keyboard = new Keyboard(this._client);
 
-    this._screenshotTaskChain = Promise.resolve();
+    this._screenshotTaskQueue = screenshotTaskQueue;
 
     this._frameManager.on(FrameManager.Events.FrameAttached, event => this.emit(Page.Events.FrameAttached, event));
     this._frameManager.on(FrameManager.Events.FrameDetached, event => this.emit(Page.Events.FrameDetached, event));
@@ -421,8 +423,7 @@ class Page extends EventEmitter {
       console.assert(typeof options.clip.width === 'number', 'Expected options.clip.width to be a number but found ' + (typeof options.clip.width));
       console.assert(typeof options.clip.height === 'number', 'Expected options.clip.height to be a number but found ' + (typeof options.clip.height));
     }
-    this._screenshotTaskChain = this._screenshotTaskChain.then(this._screenshotTask.bind(this, screenshotType, options));
-    return this._screenshotTaskChain;
+    return this._screenshotTaskQueue.postTask(this._screenshotTask.bind(this, screenshotType, options));
   }
 
   /**
@@ -431,6 +432,7 @@ class Page extends EventEmitter {
    * @return {!Promise<!Buffer>}
    */
   async _screenshotTask(format, options) {
+    await this._client.send('Target.activateTarget', {targetId: this._client.targetId()});
     if (options.fullPage) {
       const metrics = await this._client.send('Page.getLayoutMetrics');
       const width = Math.ceil(metrics.contentSize.width);

test/test.js

@@ -50,7 +50,7 @@ describe('Puppeteer', function() {
   let page;
 
   beforeAll(SX(async function() {
-    browser = new Browser({args: ['--no-sandbox']});
+    browser = new Browser({headless: true, args: ['--no-sandbox']});
     const assetsPath = path.join(__dirname, 'assets');
     server = await SimpleServer.create(assetsPath, PORT);
     httpsServer = await SimpleServer.createHTTPS(assetsPath, HTTPS_PORT);
@@ -73,9 +73,9 @@ describe('Puppeteer', function() {
     GoldenUtils.addMatchers(jasmine, GOLDEN_DIR, OUTPUT_DIR);
   }));
 
-  afterEach(function() {
-    page.close();
-  });
+  afterEach(SX(async function() {
+    await page.close();
+  }));
 
   describe('Page.evaluate', function() {
     it('should work', SX(async function() {
@@ -581,66 +581,6 @@ describe('Puppeteer', function() {
     }));
   });
 
-  describe('Page.screenshot', function() {
-    it('should work', SX(async function() {
-      await page.setViewport({width: 500, height: 500});
-      await page.navigate(PREFIX + '/grid.html');
-      let screenshot = await page.screenshot();
-      expect(screenshot).toBeGolden('screenshot-sanity.png');
-    }));
-    it('should clip rect', SX(async function() {
-      await page.setViewport({width: 500, height: 500});
-      await page.navigate(PREFIX + '/grid.html');
-      let screenshot = await page.screenshot({
-        clip: {
-          x: 50,
-          y: 100,
-          width: 150,
-          height: 100
-        }
-      });
-      expect(screenshot).toBeGolden('screenshot-clip-rect.png');
-    }));
-    it('should work for offscreen clip', SX(async function() {
-      await page.setViewport({width: 500, height: 500});
-      await page.navigate(PREFIX + '/grid.html');
-      let screenshot = await page.screenshot({
-        clip: {
-          x: 50,
-          y: 600,
-          width: 100,
-          height: 100
-        }
-      });
-      expect(screenshot).toBeGolden('screenshot-offscreen-clip.png');
-    }));
-    it('should run in parallel', SX(async function() {
-      await page.setViewport({width: 500, height: 500});
-      await page.navigate(PREFIX + '/grid.html');
-      let promises = [];
-      for (let i = 0; i < 3; ++i) {
-        promises.push(page.screenshot({
-          clip: {
-            x: 50 * i,
-            y: 0,
-            width: 50,
-            height: 50
-          }
-        }));
-      }
-      let screenshots = await Promise.all(promises);
-      expect(screenshots[1]).toBeGolden('screenshot-parallel-calls.png');
-    }));
-    it('should take fullPage screenshots', SX(async function() {
-      await page.setViewport({width: 500, height: 500});
-      await page.navigate(PREFIX + '/grid.html');
-      let screenshot = await page.screenshot({
-        fullPage: true
-      });
-      expect(screenshot).toBeGolden('screenshot-grid-fullpage.png');
-    }));
-  });
-
   describe('Frame Management', function() {
     let FrameUtils = require('./frame-utils');
     it('should handle nested frames', SX(async function() {
@@ -1064,6 +1004,81 @@ describe('Puppeteer', function() {
       expect((await page.$$('span', (element, index, arg1) => arg1, 'value1'))[0]).toBe('value1');
     }));
   });
+
+  describe('Page.screenshot', function() {
+    it('should work', SX(async function() {
+      await page.setViewport({width: 500, height: 500});
+      await page.navigate(PREFIX + '/grid.html');
+      let screenshot = await page.screenshot();
+      expect(screenshot).toBeGolden('screenshot-sanity.png');
+    }));
+    it('should clip rect', SX(async function() {
+      await page.setViewport({width: 500, height: 500});
+      await page.navigate(PREFIX + '/grid.html');
+      let screenshot = await page.screenshot({
+        clip: {
+          x: 50,
+          y: 100,
+          width: 150,
+          height: 100
+        }
+      });
+      expect(screenshot).toBeGolden('screenshot-clip-rect.png');
+    }));
+    it('should work for offscreen clip', SX(async function() {
+      await page.setViewport({width: 500, height: 500});
+      await page.navigate(PREFIX + '/grid.html');
+      let screenshot = await page.screenshot({
+        clip: {
+          x: 50,
+          y: 600,
+          width: 100,
+          height: 100
+        }
+      });
+      expect(screenshot).toBeGolden('screenshot-offscreen-clip.png');
+    }));
+    it('should run in parallel', SX(async function() {
+      await page.setViewport({width: 500, height: 500});
+      await page.navigate(PREFIX + '/grid.html');
+      let promises = [];
+      for (let i = 0; i < 3; ++i) {
+        promises.push(page.screenshot({
+          clip: {
+            x: 50 * i,
+            y: 0,
+            width: 50,
+            height: 50
+          }
+        }));
+      }
+      let screenshots = await Promise.all(promises);
+      expect(screenshots[1]).toBeGolden('grid-cell-1.png');
+    }));
+    it('should take fullPage screenshots', SX(async function() {
+      await page.setViewport({width: 500, height: 500});
+      await page.navigate(PREFIX + '/grid.html');
+      let screenshot = await page.screenshot({
+        fullPage: true
+      });
+      expect(screenshot).toBeGolden('screenshot-grid-fullpage.png');
+    }));
+    it('should run in parallel in multiple pages', SX(async function() {
+      const N = 2;
+      let pages = await Promise.all(Array(N).fill(0).map(async() => {
+        let page = await browser.newPage();
+        await page.navigate(PREFIX + '/grid.html');
+        return page;
+      }));
+      let promises = [];
+      for (let i = 0; i < N; ++i)
+        promises.push(pages[i].screenshot({ clip: { x: 50 * i, y: 0, width: 50, height: 50 } }));
+      let screenshots = await Promise.all(promises);
+      for (let i = 0; i < N; ++i)
+        expect(screenshots[i]).toBeGolden(`grid-cell-${i}.png`);
+      await Promise.all(pages.map(page => page.close()));
+    }));
+  });
 });
 
 /**

utils/doclint/lint.js

@@ -13,7 +13,8 @@ let EXCLUDE_CLASSES = new Set([
   'Helper',
   'NavigatorWatcher',
   'NetworkManager',
-  'ProxyStream'
+  'ProxyStream',
+  'TaskQueue',
 ]);
 
 let EXCLUDE_METHODS = new Set([
@@ -23,11 +24,11 @@ let EXCLUDE_METHODS = new Set([
   'Headers.constructor',
   'Headers.fromPayload',
   'InterceptedRequest.constructor',
+  'Keyboard.constructor',
   'Page.constructor',
   'Page.create',
   'Request.constructor',
   'Response.constructor',
-  'Keyboard.constructor',
 ]);
 
 /**