diff --git a/lib/Browser.js b/lib/Browser.js index 8bc2fc3db2b..7553d15ec12 100644 --- a/lib/Browser.js +++ b/lib/Browser.js @@ -18,6 +18,7 @@ const { helper, assert } = require('./helper'); const {Target} = require('./Target'); const EventEmitter = require('events'); const {TaskQueue} = require('./TaskQueue'); +const {Connection} = require('./Connection'); class Browser extends EventEmitter { /** @@ -45,9 +46,7 @@ class Browser extends EventEmitter { /** @type {Map} */ this._targets = new Map(); - this._connection.setClosedCallback(() => { - this.emit(Browser.Events.Disconnected); - }); + this._connection.on(Connection.Events.Disconnected, () => this.emit(Browser.Events.Disconnected)); this._connection.on('Target.targetCreated', this._targetCreated.bind(this)); this._connection.on('Target.targetDestroyed', this._targetDestroyed.bind(this)); this._connection.on('Target.targetInfoChanged', this._targetInfoChanged.bind(this)); diff --git a/lib/Connection.js b/lib/Connection.js index 58f37aa1d20..2d648cc8388 100644 --- a/lib/Connection.js +++ b/lib/Connection.js @@ -37,6 +37,19 @@ class Connection extends EventEmitter { this._transport.onclose = this._onClose.bind(this); /** @type {!Map}*/ this._sessions = new Map(); + this._closed = false; + } + + /** + * @param {!CDPSession} session + * @return {!Connection} + */ + static fromSession(session) { + let connection = session._connection; + // TODO(lushnikov): move to flatten protocol to avoid this. + while (connection instanceof CDPSession) + connection = connection._connection; + return connection; } /** @@ -61,13 +74,6 @@ class Connection extends EventEmitter { }); } - /** - * @param {function()} callback - */ - setClosedCallback(callback) { - this._closeCallback = callback; - } - /** * @param {string} message */ @@ -103,10 +109,9 @@ class Connection extends EventEmitter { } _onClose() { - if (this._closeCallback) { - this._closeCallback(); - this._closeCallback = null; - } + if (this._closed) + return; + this._closed = true; this._transport.onmessage = null; this._transport.onclose = null; for (const callback of this._callbacks.values()) @@ -115,6 +120,7 @@ class Connection extends EventEmitter { for (const session of this._sessions.values()) session._onClosed(); this._sessions.clear(); + this.emit(Connection.Events.Disconnected); } dispose() { @@ -134,6 +140,10 @@ class Connection extends EventEmitter { } } +Connection.Events = { + Disconnected: Symbol('Connection.Events.Disconnected'), +}; + class CDPSession extends EventEmitter { /** * @param {!Connection|!CDPSession} connection diff --git a/lib/NavigatorWatcher.js b/lib/NavigatorWatcher.js index 92008ab8da7..e0ebf6c1515 100644 --- a/lib/NavigatorWatcher.js +++ b/lib/NavigatorWatcher.js @@ -17,15 +17,17 @@ const {helper, assert} = require('./helper'); const {FrameManager} = require('./FrameManager'); const {TimeoutError} = require('./Errors'); +const {Connection} = require('./Connection'); class NavigatorWatcher { /** + * @param {!Puppeteer.CDPSession} client * @param {!FrameManager} frameManager * @param {!Puppeteer.Frame} frame * @param {number} timeout * @param {!Object=} options */ - constructor(frameManager, frame, timeout, options = {}) { + constructor(client, frameManager, frame, timeout, options = {}) { assert(options.networkIdleTimeout === undefined, 'ERROR: networkIdleTimeout option is no longer supported.'); assert(options.networkIdleInflight === undefined, 'ERROR: networkIdleInflight option is no longer supported.'); assert(options.waitUntil !== 'networkidle', 'ERROR: "networkidle" option is no longer supported. Use "networkidle2" instead'); @@ -46,6 +48,7 @@ class NavigatorWatcher { this._timeout = timeout; this._hasSameDocumentNavigation = false; this._eventListeners = [ + helper.addEventListener(Connection.fromSession(client), Connection.Events.Disconnected, () => this._terminate(new Error('Navigation failed because browser has disconnected!'))), helper.addEventListener(this._frameManager, FrameManager.Events.LifecycleEvent, this._checkLifecycleComplete.bind(this)), helper.addEventListener(this._frameManager, FrameManager.Events.FrameNavigatedWithinDocument, this._navigatedWithinDocument.bind(this)), helper.addEventListener(this._frameManager, FrameManager.Events.FrameDetached, this._checkLifecycleComplete.bind(this)) @@ -60,6 +63,16 @@ class NavigatorWatcher { }); this._timeoutPromise = this._createTimeoutPromise(); + this._terminationPromise = new Promise(fulfill => { + this._terminationCallback = fulfill; + }); + } + + /** + * @param {!Error} error + */ + _terminate(error) { + this._terminationCallback.call(null, error); } /** @@ -79,8 +92,8 @@ class NavigatorWatcher { /** * @return {!Promise} */ - timeoutPromise() { - return this._timeoutPromise; + timeoutOrTerminationPromise() { + return Promise.race([this._timeoutPromise, this._terminationPromise]); } /** diff --git a/lib/Page.js b/lib/Page.js index 591077ed2d5..b73fde753b2 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -590,15 +590,15 @@ class Page extends EventEmitter { const mainFrame = this._frameManager.mainFrame(); const timeout = typeof options.timeout === 'number' ? options.timeout : this._defaultNavigationTimeout; - const watcher = new NavigatorWatcher(this._frameManager, mainFrame, timeout, options); + const watcher = new NavigatorWatcher(this._client, this._frameManager, mainFrame, timeout, options); let ensureNewDocumentNavigation = false; let error = await Promise.race([ navigate(this._client, url, referrer), - watcher.timeoutPromise(), + watcher.timeoutOrTerminationPromise(), ]); if (!error) { error = await Promise.race([ - watcher.timeoutPromise(), + watcher.timeoutOrTerminationPromise(), ensureNewDocumentNavigation ? watcher.newDocumentNavigationPromise() : watcher.sameDocumentNavigationPromise(), ]); } @@ -645,12 +645,12 @@ class Page extends EventEmitter { async waitForNavigation(options = {}) { const mainFrame = this._frameManager.mainFrame(); const timeout = typeof options.timeout === 'number' ? options.timeout : this._defaultNavigationTimeout; - const watcher = new NavigatorWatcher(this._frameManager, mainFrame, timeout, options); + const watcher = new NavigatorWatcher(this._client, this._frameManager, mainFrame, timeout, options); const responses = new Map(); const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url(), response)); const error = await Promise.race([ - watcher.timeoutPromise(), + watcher.timeoutOrTerminationPromise(), watcher.sameDocumentNavigationPromise(), watcher.newDocumentNavigationPromise() ]); diff --git a/lib/helper.js b/lib/helper.js index d6f49da0541..0c5f87ff3ee 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -133,9 +133,9 @@ class Helper { /** * @param {!NodeJS.EventEmitter} emitter - * @param {string} eventName + * @param {(string|symbol)} eventName * @param {function(?)} handler - * @return {{emitter: !NodeJS.EventEmitter, eventName: string, handler: function(?)}} + * @return {{emitter: !NodeJS.EventEmitter, eventName: (string|symbol), handler: function(?)}} */ static addEventListener(emitter, eventName, handler) { emitter.on(eventName, handler); @@ -143,7 +143,7 @@ class Helper { } /** - * @param {!Array<{emitter: !NodeJS.EventEmitter, eventName: string, handler: function(?)}>} listeners + * @param {!Array<{emitter: !NodeJS.EventEmitter, eventName: (string|symbol), handler: function(?)}>} listeners */ static removeEventListeners(listeners) { for (const listener of listeners) diff --git a/test/puppeteer.spec.js b/test/puppeteer.spec.js index 032e91978ca..8dcc0549da3 100644 --- a/test/puppeteer.spec.js +++ b/test/puppeteer.spec.js @@ -60,6 +60,31 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions}) await rmAsync(downloadsFolder); }); }); + describe('Browser.disconnect', function() { + it('should reject navigation when browser closes', async({server}) => { + server.setRoute('/one-style.css', () => {}); + const browser = await puppeteer.launch(defaultBrowserOptions); + const remote = await puppeteer.connect({browserWSEndpoint: browser.wsEndpoint()}); + const page = await remote.newPage(); + const navigationPromise = page.goto(server.PREFIX + '/one-style.html', {timeout: 60000}).catch(e => e); + await server.waitForRequest('/one-style.css'); + await remote.disconnect(); + const error = await navigationPromise; + expect(error.message).toBe('Navigation failed because browser has disconnected!'); + await browser.close(); + }); + it('should reject waitForSelector when browser closes', async({server}) => { + server.setRoute('/empty.html', () => {}); + const browser = await puppeteer.launch(defaultBrowserOptions); + const remote = await puppeteer.connect({browserWSEndpoint: browser.wsEndpoint()}); + const page = await remote.newPage(); + const watchdog = page.waitForSelector('div', {timeout: 60000}).catch(e => e); + await remote.disconnect(); + const error = await watchdog; + expect(error.message).toBe('Protocol error (Runtime.callFunctionOn): Session closed. Most likely the page has been closed.'); + await browser.close(); + }); + }); describe('Puppeteer.launch', function() { it('should reject all promises when browser is closed', async() => { const browser = await puppeteer.launch(defaultBrowserOptions);