From 21af495b653a35e2a783b4d10fcb8697a0de85af Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Tue, 18 Jul 2017 22:10:38 -0700 Subject: [PATCH] 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 | 19 ++- lib/Connection.js | 15 ++- lib/Page.js | 14 ++- test/golden/grid-cell-0.png | Bin 0 -> 436 bytes test/golden/grid-cell-1.png | Bin 0 -> 276 bytes test/golden/grid-cell-2.png | Bin 0 -> 428 bytes test/golden/grid-cell-3.png | Bin 0 -> 448 bytes test/golden/screenshot-parallel-calls.png | Bin 288 -> 0 bytes test/test.js | 143 ++++++++++++---------- utils/doclint/lint.js | 5 +- 10 files changed, 119 insertions(+), 77 deletions(-) create mode 100644 test/golden/grid-cell-0.png create mode 100644 test/golden/grid-cell-1.png create mode 100644 test/golden/grid-cell-2.png create mode 100644 test/golden/grid-cell-3.png delete mode 100644 test/golden/screenshot-parallel-calls.png diff --git a/lib/Browser.js b/lib/Browser.js index cc234312..3ff41f5f 100644 --- a/lib/Browser.js +++ b/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() { } diff --git a/lib/Connection.js b/lib/Connection.js index a5006df4..1d667d4e 100644 --- a/lib/Connection.js +++ b/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}*/ 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}`); } /** diff --git a/lib/Page.js b/lib/Page.js index b2652793..f1e5bd70 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -28,9 +28,10 @@ let helper = require('./helper'); class Page extends EventEmitter { /** * @param {!Connection} client + * @param {!TaskQueue} screenshotTaskQueue * @return {!Promise} */ - 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} */ 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); diff --git a/test/golden/grid-cell-0.png b/test/golden/grid-cell-0.png new file mode 100644 index 0000000000000000000000000000000000000000..ff282e989b7eae67216b80a63e7b9c5b55142e3d GIT binary patch literal 436 zcmeAS@N?(olHy`uVBq!ia0vp^Mj*_=1SBWM%0FXZV666ZaSVxQeS6K`gDFws_{aH0 z(R!vHR}>x=G`%ShT|3M6M)~r39RKrdD+Bs)`Xz0~#xd&&?IC@peb^jIqDWwG_iK(IytPYE>Pdz*19rMNPWSc@C S)&O8^FnGH9xvX@1PLn2z=UNPi4K-5=as z15f-nRN6H6eA~Gn0o!5?N)G3raWddOsKc^Z0K#Z_AG!Q@@}8*dwJTGP7OpW>)r#Ex zOh#wgJL~r%n>OiqUp6^k{9W_(4*x5qb6p<(D_XniRP9^Whg&CUh)mq~{_b*}`}{3> zX^J)Hjocnyi^z(c{qjkfO~%@v|2lWgXiX4nisVRBgfn_NrsSHJ9NVsba<;s^d-3Ic QKyNa5y85}Sb4q9e0L%S&y#N3J literal 0 HcmV?d00001 diff --git a/test/golden/grid-cell-2.png b/test/golden/grid-cell-2.png new file mode 100644 index 0000000000000000000000000000000000000000..7b01753b6a63d4b7bad71f18f0918edcf2304789 GIT binary patch literal 428 zcmeAS@N?(olHy`uVBq!ia0vp^Mj*_=1SBWM%0FXZU@Z1@aSVxQeS5>u`*46r+r!Hf zmO48aOv|%6a!^6GC&0KwVAes!fP;&gngUpMdn{|RcQEj7cUrc?!s5s!y_F(Mn!1C( zmuZG(w9IlWIQ2?^Q;Sb7Pr>z^b?mthA9W%p70Ixt>B}*k(hj=RlW*Ftte1KCy&$*n zMgwHx^8eJivG)rsTTg4gc0c}oeg2!~1LvadTATQ84qJV5+uO={%QELapWYJrZnC)q zPt@9LM-S$y?l*f|ni{z+*E%uGYVNrdquZywFS>??rU@}9ubwe?{zqd_3NHqC1#c++0%IRsR%>H z_S>qN6Ki#QrcTw(<81oy?}tFQT32W4y?a0R@iQEI8Y;l@Fi_;^=f87RA}60zsWFl2 zeX-)T6@#|+QwzSoQa`hg9CFd|K_iS$Jz`v7DWiJ!$4&M{?2)%sclf>q#sY(;UU#EbrvoWO}X4q=nxHU5xIDQ;9}4cH?~?LTn=(Ahb>ml zAv_Ehr*IHkqMW1QKuTI3f8#>_-|61L{rSMVd)$<5+xUM40T}AM-;5tsTAhI3hTmhB zOw5vrSu!z8CT7XRESZ=k)8w+utD8Vn3q-X;>ykdrD@c5PX?I@;k;~=6;czCCaU7>t zcYS6(zJ3THip64jRRy4F_lZQJ_R`;q%n@0nP28AOEX(pt-V#8!+a;Q)EC5HF>%;4< z<1B!KwH4w_(BCbu?UU15tp*?#i;ZRAcn%2n;;hkV7=|H)$lDHm;MmSBeP-IuXLVgK zl}hw!RMz?J-S=fY9v=(_#F-$n%FVWEn)Tk>@Q&-cp-_mdqOtat7Xc)b$#S#((0^Br zv=E|DD9~BdFY)vFNj1``R0@EiDAj6pXCdN03qY%KO&>DS)v}4mIS|+{4MZ)?^hOv!xN6? z^=H^@wEEW@=`rv5iA_ANff2qY7OR_AuX37oBx3G44#kMGduHsNXwu2ZP#PQUYx3e( z>S6&iuFTVxo2B<1v`I00@pw~Y^!XQ;7lDKm(TK+l&;R@Ff5*6Gxq{si3q4_=ml-@= L{an^LB{Ts5TpMzG diff --git a/test/test.js b/test/test.js index 9f69309d..2e182670 100644 --- a/test/test.js +++ b/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())); + })); + }); }); /** diff --git a/utils/doclint/lint.js b/utils/doclint/lint.js index 284d692c..729dee7f 100644 --- a/utils/doclint/lint.js +++ b/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', ]); /**