From 3f99b76da16567c60750f71254a27230756dadf9 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Tue, 19 Feb 2019 08:19:45 -0600 Subject: [PATCH] refactor(firefox): migrate onto Juggler flatten protocol Juggler now implements the same "flatten" protocol as CDP. This patch copies `Connection.js` from original Puppeteer (with a few renames, e.g. `CDPSesssion` -> `JugglerSession`). --- experimental/puppeteer-firefox/lib/Browser.js | 24 +-- .../puppeteer-firefox/lib/Connection.js | 139 ++++++++++++++++-- experimental/puppeteer-firefox/lib/Events.js | 4 + experimental/puppeteer-firefox/lib/Page.js | 64 +++----- experimental/puppeteer-firefox/package.json | 2 +- test/navigation.spec.js | 2 +- test/page.spec.js | 7 +- 7 files changed, 171 insertions(+), 71 deletions(-) diff --git a/experimental/puppeteer-firefox/lib/Browser.js b/experimental/puppeteer-firefox/lib/Browser.js index 084c863ddca1f..34dbae03c7cff 100644 --- a/experimental/puppeteer-firefox/lib/Browser.js +++ b/experimental/puppeteer-firefox/lib/Browser.js @@ -11,9 +11,9 @@ class Browser extends EventEmitter { * @param {function():void} closeCallback */ static async create(connection, defaultViewport, process, closeCallback) { - const {browserContextIds} = await connection.send('Browser.getBrowserContexts'); + const {browserContextIds} = await connection.send('Target.getBrowserContexts'); const browser = new Browser(connection, browserContextIds, defaultViewport, process, closeCallback); - await connection.send('Browser.enable'); + await connection.send('Target.enable'); return browser; } @@ -43,9 +43,9 @@ class Browser extends EventEmitter { this._connection.on(Events.Connection.Disconnected, () => this.emit(Events.Browser.Disconnected)); this._eventListeners = [ - helper.addEventListener(this._connection, 'Browser.targetCreated', this._onTargetCreated.bind(this)), - helper.addEventListener(this._connection, 'Browser.targetDestroyed', this._onTargetDestroyed.bind(this)), - helper.addEventListener(this._connection, 'Browser.targetInfoChanged', this._onTargetInfoChanged.bind(this)), + helper.addEventListener(this._connection, 'Target.targetCreated', this._onTargetCreated.bind(this)), + helper.addEventListener(this._connection, 'Target.targetDestroyed', this._onTargetDestroyed.bind(this)), + helper.addEventListener(this._connection, 'Target.targetInfoChanged', this._onTargetInfoChanged.bind(this)), ]; } @@ -61,7 +61,7 @@ class Browser extends EventEmitter { * @return {!BrowserContext} */ async createIncognitoBrowserContext() { - const {browserContextId} = await this._connection.send('Browser.createBrowserContext'); + const {browserContextId} = await this._connection.send('Target.createBrowserContext'); const context = new BrowserContext(this._connection, this, browserContextId); this._contexts.set(browserContextId, context); return context; @@ -79,7 +79,7 @@ class Browser extends EventEmitter { } async _disposeContext(browserContextId) { - await this._connection.send('Browser.removeBrowserContext', {browserContextId}); + await this._connection.send('Target.removeBrowserContext', {browserContextId}); this._contexts.delete(browserContextId); } @@ -152,7 +152,7 @@ class Browser extends EventEmitter { * @return {Promise} */ async _createPageInContext(browserContextId) { - const {targetId} = await this._connection.send('Browser.newPage', { + const {targetId} = await this._connection.send('Target.newPage', { browserContextId: browserContextId || undefined }); const target = this._targets.get(targetId); @@ -190,6 +190,7 @@ class Browser extends EventEmitter { _onTargetDestroyed({targetId}) { const target = this._targets.get(targetId); this._targets.delete(targetId); + target._closedCallback(); this.emit(Events.Browser.TargetDestroyed, target); target.browserContext().emit(Events.BrowserContext.TargetDestroyed, target); } @@ -228,6 +229,7 @@ class Target { this._pagePromise = null; this._url = url; this._openerId = openerId; + this._isClosedPromise = new Promise(fulfill => this._closedCallback = fulfill); } /** @@ -256,8 +258,10 @@ class Target { } async page() { - if (this._type === 'page' && !this._pagePromise) - this._pagePromise = Page.create(this._connection, this, this._targetId, this._browser._defaultViewport); + if (this._type === 'page' && !this._pagePromise) { + const session = await this._connection.createSession(this._targetId); + this._pagePromise = Page.create(session, this, this._browser._defaultViewport); + } return this._pagePromise; } diff --git a/experimental/puppeteer-firefox/lib/Connection.js b/experimental/puppeteer-firefox/lib/Connection.js index 6fe261ff174da..d058575815a0e 100644 --- a/experimental/puppeteer-firefox/lib/Connection.js +++ b/experimental/puppeteer-firefox/lib/Connection.js @@ -13,15 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -const debugProtocol = require('debug')('hdfox:protocol'); -const EventEmitter = require('events'); +const {assert} = require('./helper'); const {Events} = require('./Events'); +const debugProtocol = require('debug')('puppeteer:protocol'); +const EventEmitter = require('events'); -/** - * @internal - */ class Connection extends EventEmitter { /** + * @param {string} url * @param {!Puppeteer.ConnectionTransport} transport * @param {number=} delay */ @@ -36,9 +35,30 @@ class Connection extends EventEmitter { this._transport = transport; this._transport.onmessage = this._onMessage.bind(this); this._transport.onclose = this._onClose.bind(this); + /** @type {!Map}*/ + this._sessions = new Map(); this._closed = false; } + /** + * @param {!JugglerSession} session + * @return {!Connection} + */ + static fromSession(session) { + return session._connection; + } + + /** + * @param {string} sessionId + * @return {?JugglerSession} + */ + session(sessionId) { + return this._sessions.get(sessionId) || null; + } + + /** + * @return {string} + */ url() { return this._url; } @@ -49,15 +69,24 @@ class Connection extends EventEmitter { * @return {!Promise} */ send(method, params = {}) { - const id = ++this._lastId; - const message = JSON.stringify({id, method, params}); - debugProtocol('SEND ► ' + message); - this._transport.send(message); + const id = this._rawSend({method, params}); return new Promise((resolve, reject) => { this._callbacks.set(id, {resolve, reject, error: new Error(), method}); }); } + /** + * @param {*} message + * @return {number} + */ + _rawSend(message) { + const id = ++this._lastId; + message = JSON.stringify(Object.assign({}, message, {id})); + debugProtocol('SEND ► ' + message); + this._transport.send(message); + return id; + } + /** * @param {string} message */ @@ -66,7 +95,22 @@ class Connection extends EventEmitter { await new Promise(f => setTimeout(f, this._delay)); debugProtocol('◀ RECV ' + message); const object = JSON.parse(message); - if (object.id) { + if (object.method === 'Target.attachedToTarget') { + const sessionId = object.params.sessionId; + const session = new JugglerSession(this, object.params.targetInfo.type, sessionId); + this._sessions.set(sessionId, session); + } else if (object.method === 'Browser.detachedFromTarget') { + const session = this._sessions.get(object.params.sessionId); + if (session) { + session._onClosed(); + this._sessions.delete(object.params.sessionId); + } + } + if (object.sessionId) { + const session = this._sessions.get(object.sessionId); + if (session) + session._onMessage(object); + } else if (object.id) { const callback = this._callbacks.get(object.id); // Callbacks could be all rejected if someone has called `.dispose()`. if (callback) { @@ -90,6 +134,9 @@ class Connection extends EventEmitter { for (const callback of this._callbacks.values()) callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`)); this._callbacks.clear(); + for (const session of this._sessions.values()) + session._onClosed(); + this._sessions.clear(); this.emit(Events.Connection.Disconnected); } @@ -97,6 +144,76 @@ class Connection extends EventEmitter { this._onClose(); this._transport.close(); } + + /** + * @param {string} targetId + * @return {!Promise} + */ + async createSession(targetId) { + const {sessionId} = await this.send('Target.attachToTarget', {targetId}); + return this._sessions.get(sessionId); + } +} + +class JugglerSession extends EventEmitter { + /** + * @param {!Connection} connection + * @param {string} targetType + * @param {string} sessionId + */ + constructor(connection, targetType, sessionId) { + super(); + /** @type {!Map}*/ + this._callbacks = new Map(); + this._connection = connection; + this._targetType = targetType; + this._sessionId = sessionId; + } + + /** + * @param {string} method + * @param {!Object=} params + * @return {!Promise} + */ + send(method, params = {}) { + if (!this._connection) + return Promise.reject(new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`)); + const id = this._connection._rawSend({sessionId: this._sessionId, method, params}); + return new Promise((resolve, reject) => { + this._callbacks.set(id, {resolve, reject, error: new Error(), method}); + }); + } + + /** + * @param {{id?: number, method: string, params: Object, error: {message: string, data: any}, result?: *}} object + */ + _onMessage(object) { + if (object.id && this._callbacks.has(object.id)) { + const callback = this._callbacks.get(object.id); + this._callbacks.delete(object.id); + if (object.error) + callback.reject(createProtocolError(callback.error, callback.method, object)); + else + callback.resolve(object.result); + } else { + assert(!object.id); + this.emit(object.method, object.params); + } + } + + async detach() { + if (!this._connection) + throw new Error(`Session already detached. Most likely the ${this._targetType} has been closed.`); + await this._connection.send('Target.detachFromTarget', {sessionId: this._sessionId}); + } + + _onClosed() { + for (const callback of this._callbacks.values()) + callback.reject(rewriteError(callback.error, `Protocol error (${callback.method}): Target closed.`)); + this._callbacks.clear(); + this._connection = null; + this.emit(Events.JugglerSession.Disconnected); + } } /** @@ -122,4 +239,4 @@ function rewriteError(error, message) { return error; } -module.exports = {Connection}; +module.exports = {Connection, JugglerSession}; diff --git a/experimental/puppeteer-firefox/lib/Events.js b/experimental/puppeteer-firefox/lib/Events.js index 1062606f4920d..2ff4f6cdcc3a7 100644 --- a/experimental/puppeteer-firefox/lib/Events.js +++ b/experimental/puppeteer-firefox/lib/Events.js @@ -31,6 +31,10 @@ const Events = { Disconnected: Symbol('Events.Connection.Disconnected'), }, + JugglerSession: { + Disconnected: Symbol('Events.JugglerSession.Disconnected'), + }, + FrameManager: { Load: Symbol('Events.FrameManager.Load'), DOMContentLoaded: Symbol('Events.FrameManager.DOMContentLoaded'), diff --git a/experimental/puppeteer-firefox/lib/Page.js b/experimental/puppeteer-firefox/lib/Page.js index 5ab24924ee1df..906f368829a3a 100644 --- a/experimental/puppeteer-firefox/lib/Page.js +++ b/experimental/puppeteer-firefox/lib/Page.js @@ -15,49 +15,20 @@ const {NavigationWatchdog, NextNavigationWatchdog} = require('./NavigationWatchd const writeFileAsync = util.promisify(fs.writeFile); -/** - * @internal - */ -class PageSession extends EventEmitter { - constructor(connection, targetId) { - super(); - this._connection = connection; - this._targetId = targetId; - const wrapperSymbol = Symbol('listenerWrapper'); - - function wrapperListener(listener, params) { - if (params.targetId === targetId) - listener.call(null, params); - } - - this.on('removeListener', (eventName, listener) => { - this._connection.removeListener(eventName, listener[wrapperSymbol]); - }); - this.on('newListener', (eventName, listener) => { - if (!listener[wrapperSymbol]) - listener[wrapperSymbol] = wrapperListener.bind(null, listener); - this._connection.on(eventName, listener[wrapperSymbol]); - }); - } - - async send(method, params = {}) { - params = Object.assign({}, params, {targetId: this._targetId}); - return await this._connection.send(method, params); - } -} - class Page extends EventEmitter { /** * - * @param {!Puppeteer.Connection} connection + * @param {!Puppeteer.JugglerSession} connection * @param {!Puppeteer.Target} target - * @param {string} targetId * @param {?Puppeteer.Viewport} defaultViewport */ - static async create(connection, target, targetId, defaultViewport) { - const session = new PageSession(connection, targetId); + static async create(session, target, defaultViewport) { const page = new Page(session, target); - await session.send('Page.enable'); + await Promise.all([ + session.send('Page.enable'), + session.send('Network.enable'), + ]); + if (defaultViewport) await page.setViewport(defaultViewport); return page; @@ -74,7 +45,7 @@ class Page extends EventEmitter { this._target = target; this._keyboard = new Keyboard(session); this._mouse = new Mouse(session, this._keyboard); - this._isClosed = false; + this._closed = false; this._networkManager = new NetworkManager(session); this._frameManager = new FrameManager(session, this, this._networkManager, this._timeoutSettings); this._networkManager.setFrameManager(this._frameManager); @@ -82,7 +53,6 @@ class Page extends EventEmitter { helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)), helper.addEventListener(this._session, 'Page.console', this._onConsole.bind(this)), helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)), - helper.addEventListener(this._session, 'Browser.targetDestroyed', this._onClosed.bind(this)), helper.addEventListener(this._frameManager, Events.FrameManager.Load, () => this.emit(Events.Page.Load)), helper.addEventListener(this._frameManager, Events.FrameManager.DOMContentLoaded, () => this.emit(Events.Page.DOMContentLoaded)), helper.addEventListener(this._frameManager, Events.FrameManager.FrameAttached, frame => this.emit(Events.Page.FrameAttached, frame)), @@ -94,6 +64,13 @@ class Page extends EventEmitter { helper.addEventListener(this._networkManager, Events.NetworkManager.RequestFailed, request => this.emit(Events.Page.RequestFailed, request)), ]; this._viewport = null; + this._target._isClosedPromise.then(() => { + this._closed = true; + this._frameManager.dispose(); + this._networkManager.dispose(); + helper.removeEventListeners(this._eventListeners); + this.emit(Events.Page.Close); + }); } /** @@ -553,7 +530,9 @@ class Page extends EventEmitter { const { runBeforeUnload = false, } = options; - await this._session.send('Browser.closePage', { runBeforeUnload }); + await this._session.send('Page.close', { runBeforeUnload }); + if (!runBeforeUnload) + await this._target._isClosedPromise; } async content() { @@ -568,11 +547,6 @@ class Page extends EventEmitter { } _onClosed() { - this._isClosed = true; - this._frameManager.dispose(); - this._networkManager.dispose(); - helper.removeEventListeners(this._eventListeners); - this.emit(Events.Page.Close); } _onConsole({type, args, frameId, location}) { @@ -584,7 +558,7 @@ class Page extends EventEmitter { * @return {boolean} */ isClosed() { - return this._isClosed; + return this._closed; } } diff --git a/experimental/puppeteer-firefox/package.json b/experimental/puppeteer-firefox/package.json index 6719af1240373..99894e2ddf7a1 100644 --- a/experimental/puppeteer-firefox/package.json +++ b/experimental/puppeteer-firefox/package.json @@ -9,7 +9,7 @@ "node": ">=8.9.4" }, "puppeteer": { - "firefox_revision": "2ede4ae19f39ec7a1b73162a6004235908260dfe" + "firefox_revision": "387ac6bbbe5357d174e9fb3aa9b6f935113c315d" }, "scripts": { "install": "node install.js", diff --git a/test/navigation.spec.js b/test/navigation.spec.js index dd1a0fe5a415e..a69a9140b4ac9 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -265,7 +265,7 @@ module.exports.addTests = function({testRunner, expect, Errors, CHROME}) { process.removeListener('warning', warningHandler); expect(warning).toBe(null); }); - it_fails_ffox('should not leak listeners during navigation of 11 pages', async({page, context, server}) => { + it('should not leak listeners during navigation of 11 pages', async({page, context, server}) => { let warning = null; const warningHandler = w => warning = w; process.on('warning', warningHandler); diff --git a/test/page.spec.js b/test/page.spec.js index 395361ecf0be1..8fe8a7f56b99f 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -36,10 +36,11 @@ module.exports.addTests = function({testRunner, expect, headless, Errors, Device describe('Page.close', function() { it('should reject all promises when page is closed', async({context}) => { const newPage = await context.newPage(); - const neverResolves = newPage.evaluate(() => new Promise(r => {})); - newPage.close(); let error = null; - await neverResolves.catch(e => error = e); + await Promise.all([ + newPage.evaluate(() => new Promise(r => {})).catch(e => error = e), + newPage.close(), + ]); expect(error.message).toContain('Protocol error'); }); it('should not be visible in browser.pages', async({browser}) => {