From c35821a1a1c0348c03effdf14cb0eb8186ecad32 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 15 Feb 2019 17:57:48 -0800 Subject: [PATCH] feat(firefox): switch over to WebSocket and support multiclient (#4022) - switch transport from TCP to WS (yay!) - implemenet `puppeter.connect()`, `browser.disconnect()`, `'disconnected'` event and `browser.wsEndpoint()` --- experimental/puppeteer-firefox/lib/Browser.js | 10 +++ .../puppeteer-firefox/lib/Connection.js | 7 +- experimental/puppeteer-firefox/lib/Events.js | 1 + .../puppeteer-firefox/lib/Launcher.js | 35 +++++++--- .../puppeteer-firefox/lib/NetworkManager.js | 6 +- .../puppeteer-firefox/lib/Puppeteer.js | 4 ++ ...efoxTransport.js => WebSocketTransport.js} | 68 ++++++------------- experimental/puppeteer-firefox/package.json | 2 +- test/browser.spec.js | 2 +- test/browsercontext.spec.js | 2 +- test/launcher.spec.js | 10 +-- 11 files changed, 81 insertions(+), 66 deletions(-) rename experimental/puppeteer-firefox/lib/{FirefoxTransport.js => WebSocketTransport.js} (57%) diff --git a/experimental/puppeteer-firefox/lib/Browser.js b/experimental/puppeteer-firefox/lib/Browser.js index a14ee71c..5ade6337 100644 --- a/experimental/puppeteer-firefox/lib/Browser.js +++ b/experimental/puppeteer-firefox/lib/Browser.js @@ -40,6 +40,8 @@ class Browser extends EventEmitter { for (const browserContextId of browserContextIds) this._contexts.set(browserContextId, new BrowserContext(this._connection, this, browserContextId)); + this._connection.on(Events.Connection.Disconnected, () => this.emit(Events.Browser.Disconnected)); + this._eventListeners = [ helper.addEventListener(this._connection, 'Browser.tabOpened', this._onTabOpened.bind(this)), helper.addEventListener(this._connection, 'Browser.tabClosed', this._onTabClosed.bind(this)), @@ -47,6 +49,14 @@ class Browser extends EventEmitter { ]; } + wsEndpoint() { + return this._connection.url(); + } + + disconnect() { + this._connection.dispose(); + } + /** * @return {!BrowserContext} */ diff --git a/experimental/puppeteer-firefox/lib/Connection.js b/experimental/puppeteer-firefox/lib/Connection.js index a1c18331..6fe261ff 100644 --- a/experimental/puppeteer-firefox/lib/Connection.js +++ b/experimental/puppeteer-firefox/lib/Connection.js @@ -25,8 +25,9 @@ class Connection extends EventEmitter { * @param {!Puppeteer.ConnectionTransport} transport * @param {number=} delay */ - constructor(transport, delay = 0) { + constructor(url, transport, delay = 0) { super(); + this._url = url; this._lastId = 0; /** @type {!Map}*/ this._callbacks = new Map(); @@ -38,6 +39,10 @@ class Connection extends EventEmitter { this._closed = false; } + url() { + return this._url; + } + /** * @param {string} method * @param {!Object=} params diff --git a/experimental/puppeteer-firefox/lib/Events.js b/experimental/puppeteer-firefox/lib/Events.js index b99bbf4f..1062606f 100644 --- a/experimental/puppeteer-firefox/lib/Events.js +++ b/experimental/puppeteer-firefox/lib/Events.js @@ -16,6 +16,7 @@ const Events = { RequestFailed: 'requestfailed', }, Browser: { + Disconnected: 'disconnected', TargetCreated: 'targetcreated', TargetChanged: 'targetchanged', TargetDestroyed: 'targetdestroyed', diff --git a/experimental/puppeteer-firefox/lib/Launcher.js b/experimental/puppeteer-firefox/lib/Launcher.js index 2786e064..9b1a1c97 100644 --- a/experimental/puppeteer-firefox/lib/Launcher.js +++ b/experimental/puppeteer-firefox/lib/Launcher.js @@ -23,9 +23,9 @@ const {BrowserFetcher} = require('./BrowserFetcher'); const readline = require('readline'); const fs = require('fs'); const util = require('util'); -const {helper} = require('./helper'); +const {helper, debugError} = require('./helper'); const {TimeoutError} = require('./Errors') -const FirefoxTransport = require('./FirefoxTransport'); +const WebSocketTransport = require('./WebSocketTransport'); const mkdtempAsync = util.promisify(fs.mkdtemp); const removeFolderAsync = util.promisify(removeFolder); @@ -121,14 +121,13 @@ class Launcher { /** @type {?Connection} */ let connection = null; try { - const port = await waitForWSEndpoint(firefoxProcess, 30000); - const transport = await FirefoxTransport.create(parseInt(port, 10)); - connection = new Connection(transport, slowMo); + const url = await waitForWSEndpoint(firefoxProcess, 30000); + const transport = await WebSocketTransport.create(url); + connection = new Connection(url, transport, slowMo); const browser = await Browser.create(connection, defaultViewport, firefoxProcess, killFirefox); if (ignoreHTTPSErrors) await connection.send('Browser.setIgnoreHTTPSErrors', {enabled: true}); - if (!browser.targets().length) - await new Promise(x => browser.once('targetcreated', x)); + await browser.waitForTarget(t => t.type() === 'page'); return browser; } catch (e) { killFirefox(); @@ -156,6 +155,26 @@ class Launcher { } } + /** + * @param {Object} options + * @return {!Promise} + */ + async connect(options = {}) { + const { + browserWSEndpoint, + slowMo = 0, + defaultViewport = {width: 800, height: 600}, + ignoreHTTPSErrors = false, + } = options; + let connection = null; + const transport = await WebSocketTransport.create(browserWSEndpoint); + connection = new Connection(browserWSEndpoint, transport, slowMo); + const browser = await Browser.create(connection, defaultViewport, null, () => connection.send('Browser.close').catch(debugError)); + if (ignoreHTTPSErrors) + await connection.send('Browser.setIgnoreHTTPSErrors', {enabled: true}); + return browser; + } + /** * @return {string} */ @@ -205,7 +224,7 @@ function waitForWSEndpoint(firefoxProcess, timeout) { */ function onLine(line) { stderr += line + '\n'; - const match = line.match(/^Juggler listening on (\d+)$/); + const match = line.match(/^Juggler listening on (ws:\/\/.*)$/); if (!match) return; cleanup(); diff --git a/experimental/puppeteer-firefox/lib/NetworkManager.js b/experimental/puppeteer-firefox/lib/NetworkManager.js index 9daaaf9d..be7b653d 100644 --- a/experimental/puppeteer-firefox/lib/NetworkManager.js +++ b/experimental/puppeteer-firefox/lib/NetworkManager.js @@ -12,9 +12,9 @@ class NetworkManager extends EventEmitter { this._frameManager = null; this._eventListeners = [ - helper.addEventListener(session, 'Page.requestWillBeSent', this._onRequestWillBeSent.bind(this)), - helper.addEventListener(session, 'Page.responseReceived', this._onResponseReceived.bind(this)), - helper.addEventListener(session, 'Page.requestFinished', this._onRequestFinished.bind(this)), + helper.addEventListener(session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this)), + helper.addEventListener(session, 'Network.responseReceived', this._onResponseReceived.bind(this)), + helper.addEventListener(session, 'Network.requestFinished', this._onRequestFinished.bind(this)), ]; } diff --git a/experimental/puppeteer-firefox/lib/Puppeteer.js b/experimental/puppeteer-firefox/lib/Puppeteer.js index b1beffe2..031aac34 100644 --- a/experimental/puppeteer-firefox/lib/Puppeteer.js +++ b/experimental/puppeteer-firefox/lib/Puppeteer.js @@ -15,6 +15,10 @@ class Puppeteer { return this._launcher.launch(options); } + async connect(options) { + return this._launcher.connect(options); + } + createBrowserFetcher(options) { return new BrowserFetcher(this._projectRoot, options); } diff --git a/experimental/puppeteer-firefox/lib/FirefoxTransport.js b/experimental/puppeteer-firefox/lib/WebSocketTransport.js similarity index 57% rename from experimental/puppeteer-firefox/lib/FirefoxTransport.js rename to experimental/puppeteer-firefox/lib/WebSocketTransport.js index 9997b390..f81f5a8c 100644 --- a/experimental/puppeteer-firefox/lib/FirefoxTransport.js +++ b/experimental/puppeteer-firefox/lib/WebSocketTransport.js @@ -13,63 +13,39 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -const {Socket} = require('net'); +const WebSocket = require('ws'); /** * @implements {!Puppeteer.ConnectionTransport} - * @internal */ -class FirefoxTransport { +class WebSocketTransport { /** - * @param {number} port - * @return {!Promise} + * @param {string} url + * @return {!Promise} */ - static async create(port) { - const socket = new Socket(); - try { - await new Promise((resolve, reject) => { - socket.once('connect', resolve); - socket.once('error', reject); - socket.connect({ - port, - host: 'localhost' - }); - }); - } catch (e) { - socket.destroy(); - throw e; - } - return new FirefoxTransport(socket); + static create(url) { + return new Promise((resolve, reject) => { + const ws = new WebSocket(url, [], { perMessageDeflate: false }); + ws.addEventListener('open', () => resolve(new WebSocketTransport(ws))); + ws.addEventListener('error', reject); + }); } /** - * @param {!Socket} socket + * @param {!WebSocket} ws */ - constructor(socket) { - this._socket = socket; - this._socket.once('close', had_error => { + constructor(ws) { + this._ws = ws; + this._dispatchQueue = new DispatchQueue(this); + this._ws.addEventListener('message', event => { + this._dispatchQueue.enqueue(event.data); + }); + this._ws.addEventListener('close', event => { if (this.onclose) this.onclose.call(null); }); - this._dispatchQueue = new DispatchQueue(this); - let buffer = Buffer.from(''); - socket.on('data', async data => { - buffer = Buffer.concat([buffer, data]); - while (true) { - const bufferString = buffer.toString(); - const seperatorIndex = bufferString.indexOf(':'); - if (seperatorIndex === -1) - return; - const length = parseInt(bufferString.substring(0, seperatorIndex), 10); - if (buffer.length < length + seperatorIndex) - return; - const message = buffer.slice(seperatorIndex + 1, seperatorIndex + 1 + length).toString(); - buffer = buffer.slice(seperatorIndex + 1 + length); - this._dispatchQueue.enqueue(message); - } - }); // Silently ignore all errors - we don't know what to do with them. - this._socket.on('error', () => {}); + this._ws.addEventListener('error', () => {}); this.onmessage = null; this.onclose = null; } @@ -78,11 +54,11 @@ class FirefoxTransport { * @param {string} message */ send(message) { - this._socket.write(Buffer.byteLength(message) + ':' + message); + this._ws.send(message); } close() { - this._socket.destroy(); + this._ws.close(); } } @@ -123,4 +99,4 @@ class DispatchQueue { } } -module.exports = FirefoxTransport; +module.exports = WebSocketTransport; diff --git a/experimental/puppeteer-firefox/package.json b/experimental/puppeteer-firefox/package.json index 224588db..617f4e23 100644 --- a/experimental/puppeteer-firefox/package.json +++ b/experimental/puppeteer-firefox/package.json @@ -9,7 +9,7 @@ "node": ">=8.9.4" }, "puppeteer": { - "firefox_revision": "fd017c27c17d0b4fa8bdea3ad40b88ca2addaeda" + "firefox_revision": "c74102def6c16584c155a98741e8143ab5d615b9" }, "scripts": { "install": "node install.js", diff --git a/test/browser.spec.js b/test/browser.spec.js index 5f373f70..e516151e 100644 --- a/test/browser.spec.js +++ b/test/browser.spec.js @@ -31,7 +31,7 @@ module.exports.addTests = function({testRunner, expect, headless, puppeteer}) { const process = await browser.process(); expect(process.pid).toBeGreaterThan(0); }); - it_fails_ffox('should not return child_process for remote browser', async function({browser}) { + it('should not return child_process for remote browser', async function({browser}) { const browserWSEndpoint = browser.wsEndpoint(); const remoteBrowser = await puppeteer.connect({browserWSEndpoint}); expect(remoteBrowser.process()).toBe(null); diff --git a/test/browsercontext.spec.js b/test/browsercontext.spec.js index 0accba66..5c365334 100644 --- a/test/browsercontext.spec.js +++ b/test/browsercontext.spec.js @@ -141,7 +141,7 @@ module.exports.addTests = function({testRunner, expect, puppeteer, Errors}) { ]); expect(browser.browserContexts().length).toBe(1); }); - it_fails_ffox('should work across sessions', async function({browser, server}) { + it('should work across sessions', async function({browser, server}) { expect(browser.browserContexts().length).toBe(1); const context = await browser.createIncognitoBrowserContext(); expect(browser.browserContexts().length).toBe(2); diff --git a/test/launcher.spec.js b/test/launcher.spec.js index 4553dc6b..f21fdf69 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -300,7 +300,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p await browser.close(); }); }); - describe_fails_ffox('Puppeteer.connect', function() { + describe('Puppeteer.connect', function() { it('should be able to connect multiple times to the same browser', async({server}) => { const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const browser = await puppeteer.connect({ @@ -349,7 +349,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p expect(await restoredPage.evaluate(() => 7 * 8)).toBe(56); await browser.close(); }); - it('should be able to connect using browserUrl, with and without trailing slash', async({server}) => { + it_fails_ffox('should be able to connect using browserUrl, with and without trailing slash', async({server}) => { const originalBrowser = await puppeteer.launch(Object.assign({}, defaultBrowserOptions, { args: ['--remote-debugging-port=21222'] })); @@ -366,7 +366,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p browser2.disconnect(); originalBrowser.close(); }); - it('should throw when using both browserWSEndpoint and browserURL', async({server}) => { + it_fails_ffox('should throw when using both browserWSEndpoint and browserURL', async({server}) => { const originalBrowser = await puppeteer.launch(Object.assign({}, defaultBrowserOptions, { args: ['--remote-debugging-port=21222'] })); @@ -378,7 +378,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p originalBrowser.close(); }); - it('should throw when trying to connect to non-existing browser', async({server}) => { + it_fails_ffox('should throw when trying to connect to non-existing browser', async({server}) => { const originalBrowser = await puppeteer.launch(Object.assign({}, defaultBrowserOptions, { args: ['--remote-debugging-port=21222'] })); @@ -414,7 +414,7 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p }); }); - describe_fails_ffox('Browser.Events.disconnected', function() { + describe('Browser.Events.disconnected', function() { it('should be emitted when: browser gets closed, disconnected or underlying websocket gets closed', async() => { const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const browserWSEndpoint = originalBrowser.wsEndpoint();