From fe06c896eb2616899690f1070b6d394065a4a612 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 9 Aug 2017 16:14:00 -0700 Subject: [PATCH] Roll chromium to 492629 (#230) This patch - rolls chromium to 492629 - migrates connection establishing to use browser target. This migration means that now we have a single websocket connection to browser (implemented in Connection class). A connection to a particular target is incapsulated in a new Session class. --- lib/Browser.js | 17 ++-- lib/Connection.js | 129 ++++++++++++++++++++---- lib/Dialog.js | 2 +- lib/EmulationManager.js | 2 +- lib/FrameManager.js | 4 +- lib/Input.js | 4 +- lib/NavigatorWatcher.js | 2 +- lib/NetworkManager.js | 6 +- lib/Page.js | 5 +- lib/Tracing.js | 2 +- lib/helper.js | 4 +- package.json | 2 +- test/test.js | 24 +++++ utils/doclint/check_public_api/index.js | 1 + 14 files changed, 162 insertions(+), 42 deletions(-) diff --git a/lib/Browser.js b/lib/Browser.js index 40f414c0c19..72e5a3964f4 100644 --- a/lib/Browser.js +++ b/lib/Browser.js @@ -92,9 +92,10 @@ class Browser { await this._ensureChromeIsRunning(); if (!this._chromeProcess || this._terminated) throw new Error('ERROR: this chrome instance is not alive any more!'); - let client = await Connection.create(this._remoteDebuggingPort, this._connectionDelay); - let page = await Page.create(client, this._ignoreHTTPSErrors, this._screenshotTaskQueue); - return page; + + const {targetId} = await this._connection.send('Target.createTarget', {url: 'about:blank'}); + const client = await this._connection.createSession(targetId); + return await Page.create(client, this._ignoreHTTPSErrors, this._screenshotTaskQueue); } /** @@ -133,15 +134,17 @@ class Browser { this._chromeProcess.stderr.pipe(this.stderr); this._chromeProcess.stdout.pipe(this.stdout); - this._remoteDebuggingPort = await waitForRemoteDebuggingPort(this._chromeProcess); + let {port, browserTargetId} = await waitForRemoteDebuggingPort(this._chromeProcess); // Failed to connect to browser. - if (this._remoteDebuggingPort === -1) { + if (port === -1) { this._chromeProcess.kill(); throw new Error('Failed to connect to chrome!'); } if (this._terminated) throw new Error('Failed to launch chrome! ' + stderr); + this._remoteDebuggingPort = port; + this._connection = await Connection.create(port, browserTargetId, this._connectionDelay); } close() { @@ -168,11 +171,11 @@ function waitForRemoteDebuggingPort(chromeProcess) { * @param {string} line */ function onLine(line) { - const match = line.match(/^DevTools listening on .*:(\d+)$/); + const match = line.match(/^DevTools listening on .*:(\d+)(\/.*)$/); if (!match) return; rl.removeListener('line', onLine); - fulfill(Number.parseInt(match[1], 10)); + fulfill({port: Number.parseInt(match[1], 10), browserTargetId: match[2]}); } }); } diff --git a/lib/Connection.js b/lib/Connection.js index b1e1d4694aa..90ae5544f61 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -23,14 +23,11 @@ const COMMAND_TIMEOUT = 10000; class Connection extends EventEmitter { /** * @param {number} port - * @param {string} targetId * @param {!WebSocket} ws * @param {number=} delay */ - constructor(port, targetId, ws, delay) { + constructor(ws, delay) { super(); - this._port = port; - this._targetId = targetId; this._lastId = 0; /** @type {!Map}*/ this._callbacks = new Map(); @@ -39,13 +36,8 @@ class Connection extends EventEmitter { this._ws = ws; this._ws.on('message', this._onMessage.bind(this)); this._ws.on('close', this._onClose.bind(this)); - } - - /** - * @return {string} - */ - targetId() { - return this._targetId; + /** @type {!Map}*/ + this._sessions = new Map(); } /** @@ -80,22 +72,47 @@ class Connection extends EventEmitter { callback.resolve(object.result); } else { console.assert(!object.id); - this.emit(object.method, object.params); + if (object.method === 'Target.receivedMessageFromTarget') { + const session = this._sessions.get(object.params.sessionId); + if (session) + session._onMessage(object.params.message); + } else if (object.method === 'Target.detachedFromTarget') { + const session = this._sessions.get(object.params.sessionId); + if (session) + session._onClosed(); + this._sessions.delete(object.params.sessionId); + } else { + this.emit(object.method, object.params); + } } } _onClose() { this._ws.removeAllListeners(); - this._ws.close(); for (let callback of this._callbacks.values()) callback.reject(new Error(`Protocol error (${callback.method}): Target closed.`)); + this._callbacks.clear(); + for (let session of this._sessions.values()) + session._onClosed(); + this._sessions.clear(); } /** * @return {!Promise} */ async dispose() { - await runJsonCommand(this._port, `close/${this._targetId}`); + this._ws.close(); + } + + /** + * @param {string} targetId + * @return {!Promise} + */ + async createSession(targetId) { + const {sessionId} = await this.send('Target.attachToTarget', {targetId}); + const session = new Session(this, targetId, sessionId); + this._sessions.set(sessionId, session); + return session; } /** @@ -103,13 +120,11 @@ class Connection extends EventEmitter { * @param {number=} delay * @return {!Promise} */ - static async create(port, delay) { - let newTab = await runJsonCommand(port, 'new'); - let url = newTab.webSocketDebuggerUrl; - + static async create(port, targetId, delay) { + const url = `ws://localhost:${port}${targetId}`; return new Promise((resolve, reject) => { let ws = new WebSocket(url, { perMessageDeflate: false }); - ws.on('open', () => resolve(new Connection(port, newTab.id, ws, delay))); + ws.on('open', () => resolve(new Connection(ws, delay))); ws.on('error', reject); }); } @@ -123,6 +138,82 @@ class Connection extends EventEmitter { } } +class Session extends EventEmitter { + /** + * @param {!Connection} connection + * @param {string} targetId + * @param {string} sessionId + */ + constructor(connection, targetId, sessionId) { + super(); + this._lastId = 0; + /** @type {!Map}*/ + this._callbacks = new Map(); + this._connection = connection; + this._targetId = targetId; + this._sessionId = sessionId; + } + + /** + * @return {string} + */ + targetId() { + return this._targetId; + } + + /** + * @param {string} method + * @param {!Object=} params + * @return {!Promise} + */ + async send(method, params = {}) { + let id = ++this._lastId; + let message = JSON.stringify({id, method, params}); + this._connection.send('Target.sendMessageToTarget', {sessionId: this._sessionId, message}).catch(e => { + // The response from target might have been already dispatched. + if (!this._callbacks.has(id)) + return; + let callback = this._callbacks.get(id); + this._callbacks.delete(object.id); + callback.reject(e); + }); + return new Promise((resolve, reject) => { + this._callbacks.set(id, {resolve, reject, method}); + }); + } + + /** + * @param {string} message + */ + _onMessage(message) { + let object = JSON.parse(message); + if (object.id && this._callbacks.has(object.id)) { + let callback = this._callbacks.get(object.id); + this._callbacks.delete(object.id); + if (object.error) + callback.reject(new Error(`Protocol error (${callback.method}): ${object.error.message} ${object.error.data}`)); + else + callback.resolve(object.result); + } else { + console.assert(!object.id); + this.emit(object.method, object.params); + } + } + + /** + * @return {!Promise} + */ + async dispose() { + await this._connection.send('Target.closeTarget', {targetId: this._targetId}); + } + + _onClosed() { + for (let callback of this._callbacks.values()) + callback.reject(new Error(`Protocol error (${callback.method}): Target closed.`)); + this._callbacks.clear(); + } +} + /** * @param {number} port * @param {string} command diff --git a/lib/Dialog.js b/lib/Dialog.js index 0c14e4ac68b..0ad4b58c6cf 100644 --- a/lib/Dialog.js +++ b/lib/Dialog.js @@ -18,7 +18,7 @@ const helper = require('./helper'); class Dialog { /** - * @param {!Connection} client + * @param {!Session} client * @param {!Dialog.Type} type * @param {string} message */ diff --git a/lib/EmulationManager.js b/lib/EmulationManager.js index d5763770c63..23b1e80a3af 100644 --- a/lib/EmulationManager.js +++ b/lib/EmulationManager.js @@ -16,7 +16,7 @@ class EmulationManager { /** - * @param {!Connection} client + * @param {!Session} client */ constructor(client) { this._client = client; diff --git a/lib/FrameManager.js b/lib/FrameManager.js index 29325e3c29f..b81194ae52b 100644 --- a/lib/FrameManager.js +++ b/lib/FrameManager.js @@ -21,7 +21,7 @@ const helper = require('./helper'); class FrameManager extends EventEmitter { /** - * @param {!Connection} client + * @param {!Session} client * @param {!Object} frameTree * @param {!Mouse} mouse */ @@ -144,7 +144,7 @@ FrameManager.Events = { */ class Frame { /** - * @param {!Connection} client + * @param {!Session} client * @param {!Mouse} mouse * @param {?Frame} parentFrame * @param {string} frameId diff --git a/lib/Input.js b/lib/Input.js index 08aa0aa3ed9..c709ff2ea73 100644 --- a/lib/Input.js +++ b/lib/Input.js @@ -18,7 +18,7 @@ const helper = require('./helper'); class Keyboard { /** - * @param {!Connection} client + * @param {!Session} client */ constructor(client) { this._client = client; @@ -95,7 +95,7 @@ class Keyboard { class Mouse { /** - * @param {!Connection} client + * @param {!Session} client * @param {!Keyboard} keyboard */ constructor(client, keyboard) { diff --git a/lib/NavigatorWatcher.js b/lib/NavigatorWatcher.js index 18a591f02dd..b5abd0546eb 100644 --- a/lib/NavigatorWatcher.js +++ b/lib/NavigatorWatcher.js @@ -18,7 +18,7 @@ const helper = require('./helper'); class NavigatorWatcher { /** - * @param {!Connection} client + * @param {!Session} client * @param {boolean} ignoreHTTPSErrors * @param {!Object=} options */ diff --git a/lib/NetworkManager.js b/lib/NetworkManager.js index ab988f0bcc6..34c9001bcd5 100644 --- a/lib/NetworkManager.js +++ b/lib/NetworkManager.js @@ -18,7 +18,7 @@ const helper = require('./helper'); class NetworkManager extends EventEmitter { /** - * @param {!Connection} client + * @param {!Session} client */ constructor(client) { super(); @@ -165,7 +165,7 @@ helper.tracePublicAPI(Request); class Response { /** - * @param {!Connection} client + * @param {!Session} client * @param {?Request} request * @param {!Object} payload */ @@ -223,7 +223,7 @@ helper.tracePublicAPI(Response); class InterceptedRequest { /** - * @param {!Connection} client + * @param {!Session} client * @param {string} interceptionId * @param {!Object} payload */ diff --git a/lib/Page.js b/lib/Page.js index 1fb0c89b591..5284e10cac2 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -28,7 +28,8 @@ const helper = require('./helper'); class Page extends EventEmitter { /** - * @param {!Connection} client + * @param {!Session} client + * @param {string} sessionId * @param {boolean} ignoreHTTPSErrors * @param {!TaskQueue} screenshotTaskQueue * @return {!Promise} @@ -50,7 +51,7 @@ class Page extends EventEmitter { } /** - * @param {!Connection} client + * @param {!Session} client * @param {boolean} ignoreHTTPSErrors * @param {!TaskQueue} screenshotTaskQueue */ diff --git a/lib/Tracing.js b/lib/Tracing.js index 06b6c234fb4..61c80b6c3b8 100644 --- a/lib/Tracing.js +++ b/lib/Tracing.js @@ -18,7 +18,7 @@ const helper = require('./helper'); class Tracing { /** - * @param {!Connection} client + * @param {!Session} client */ constructor(client) { this._client = client; diff --git a/lib/helper.js b/lib/helper.js index aa68e5b2370..38f4ed801cc 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -49,7 +49,7 @@ class Helper { } /** - * @param {!Connection} client + * @param {!Session} client * @param {!Object} remoteObject * @return {!Promise} */ @@ -96,7 +96,7 @@ class Helper { } /** - * @param {!Connection} client + * @param {!Session} client * @param {!Object} remoteObject * @return {!Promise} */ diff --git a/package.json b/package.json index c85483af2bc..d3e6df8558d 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "ws": "^3.0.0" }, "puppeteer": { - "chromium_revision": "491334" + "chromium_revision": "492629" }, "devDependencies": { "commonmark": "^0.27.0", diff --git a/test/test.js b/test/test.js index dd4e2ad5cc5..dff3244e0c5 100644 --- a/test/test.js +++ b/test/test.js @@ -95,6 +95,15 @@ describe('Browser', function() { expect(response.ok).toBe(true); browser.close(); })); + it('should reject all promises when browser is closed', SX(async function() { + let browser = new Browser(defaultBrowserOptions); + let page = await browser.newPage(); + let error = null; + let neverResolves = page.evaluate(() => new Promise(r => {})).catch(e => error = e); + browser.close(); + await neverResolves; + expect(error.message).toContain('Protocol error'); + })); }); describe('Page', function() { @@ -131,6 +140,21 @@ describe('Page', function() { })); }); + describe('Page.close', function() { + it('should reject all promises when page is closed', SX(async function() { + let newPage = await browser.newPage(); + let neverResolves = newPage.evaluate(() => new Promise(r => {})); + newPage.close(); + let error = null; + try { + await neverResolves; + } catch (e) { + error = e; + } + expect(error.message).toContain('Protocol error'); + })); + }); + describe('Page.evaluate', function() { it('should work', SX(async function() { let result = await page.evaluate(() => 7 * 3); diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index 96e17a35bf3..71e88c37ac4 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -27,6 +27,7 @@ const EXCLUDE_CLASSES = new Set([ 'NavigatorWatcher', 'NetworkManager', 'ProxyStream', + 'Session', 'TaskQueue', 'WaitTask', ]);