From dbb374d4af344d510aea72a0c30de80aae6b77c0 Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <lushnikov@chromium.org>
Date: Fri, 16 Jun 2017 20:20:36 -0700
Subject: [PATCH] Fix racy condition in case of multiple parallel screenshots

Page.screenshot operates the global state of the page. In case of
multiple Page.screenshot() commands running in parallel with different
clipping rects, they interfere with each other.

This patch makes Page.screenshot() commands run sequencially
even though they were called in parallel.

Fixes #15.
---
 lib/Page.js        | 12 ++++++++++++
 test/goldentest.js | 17 +++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/lib/Page.js b/lib/Page.js
index 8adff1c8..77b5fa0a 100644
--- a/lib/Page.js
+++ b/lib/Page.js
@@ -56,6 +56,8 @@ class Page extends EventEmitter {
         /** @type {?function(!Request)} */
         this._requestInterceptor = null;
 
+        this._screenshotTaskChain = Promise.resolve();
+
         client.on('Debugger.paused', event => this._onDebuggerPaused(event));
         client.on('Debugger.scriptParsed', event => this._onScriptParsed(event));
         client.on('Network.responseReceived', event => this.emit(Page.Events.ResponseReceived, event.response));
@@ -391,6 +393,7 @@ class Page extends EventEmitter {
      * @return {!Promise<!Buffer>}
      */
     async screenshot(options) {
+        options = options || {};
         var screenshotType = null;
         if (options.path) {
             var mimeType = mime.lookup(options.path);
@@ -423,7 +426,16 @@ 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;
+    }
 
+    /**
+     * @param {string} screenshotType
+     * @param {!Object=} options
+     * @return {!Promise<!Buffer>}
+     */
+    async _screenshotTask(screenshotType, options) {
         if (options.clip) {
             await Promise.all([
                 this._client.send('Emulation.setVisibleSize', {
diff --git a/test/goldentest.js b/test/goldentest.js
index d5f417ed..4335bf5b 100644
--- a/test/goldentest.js
+++ b/test/goldentest.js
@@ -70,6 +70,23 @@ describe('GoldenTests', function() {
             }
         });
     });
+
+    imageTest('screenshot-parallel-calls.png', async function() {
+        await page.setViewportSize({width: 500, height: 500});
+        await page.navigate(STATIC_PREFIX + '/grid.html');
+        var promises = [];
+        for (var i = 0; i < 3; ++i) {
+            promises.push(page.screenshot({
+                clip: {
+                    x: 50 * i,
+                    y: 0,
+                    width: 50,
+                    height: 50
+                }
+            }));
+        }
+        return await promises[1];
+    });
 });
 
 /**