diff --git a/experimental/puppeteer-firefox/lib/Page.js b/experimental/puppeteer-firefox/lib/Page.js index 8c3898a811f64..c2ce1bdb3ed6f 100644 --- a/experimental/puppeteer-firefox/lib/Page.js +++ b/experimental/puppeteer-firefox/lib/Page.js @@ -241,6 +241,12 @@ class Page extends EventEmitter { } } + _sessionClosePromise() { + if (!this._disconnectPromise) + this._disconnectPromise = new Promise(fulfill => this._session.once(Events.JugglerSession.Disconnected, () => fulfill(new Error('Target closed')))); + return this._disconnectPromise; + } + /** * @param {(string|Function)} urlOrPredicate * @param {!{timeout?: number}=} options @@ -256,7 +262,7 @@ class Page extends EventEmitter { if (typeof urlOrPredicate === 'function') return !!(urlOrPredicate(request)); return false; - }, timeout); + }, timeout, this._sessionClosePromise()); } /** @@ -274,7 +280,7 @@ class Page extends EventEmitter { if (typeof urlOrPredicate === 'function') return !!(urlOrPredicate(response)); return false; - }, timeout); + }, timeout, this._sessionClosePromise()); } /** diff --git a/experimental/puppeteer-firefox/lib/TimeoutSettings.js b/experimental/puppeteer-firefox/lib/TimeoutSettings.js index c6b08d39671c9..de7cbbf47dae3 100644 --- a/experimental/puppeteer-firefox/lib/TimeoutSettings.js +++ b/experimental/puppeteer-firefox/lib/TimeoutSettings.js @@ -47,6 +47,9 @@ class TimeoutSettings { return DEFAULT_TIMEOUT; } + /** + * @return {number} + */ timeout() { if (this._defaultTimeout !== null) return this._defaultTimeout; diff --git a/experimental/puppeteer-firefox/lib/helper.js b/experimental/puppeteer-firefox/lib/helper.js index 78dd01d6889e0..1f417215c6a83 100644 --- a/experimental/puppeteer-firefox/lib/helper.js +++ b/experimental/puppeteer-firefox/lib/helper.js @@ -121,9 +121,11 @@ class Helper { * @param {!NodeJS.EventEmitter} emitter * @param {(string|symbol)} eventName * @param {function} predicate + * @param {number} timeout + * @param {!Promise} abortPromise * @return {!Promise} */ - static waitForEvent(emitter, eventName, predicate, timeout) { + static async waitForEvent(emitter, eventName, predicate, timeout, abortPromise) { let eventTimeout, resolveCallback, rejectCallback; const promise = new Promise((resolve, reject) => { resolveCallback = resolve; @@ -132,12 +134,10 @@ class Helper { const listener = Helper.addEventListener(emitter, eventName, event => { if (!predicate(event)) return; - cleanup(); resolveCallback(event); }); if (timeout) { eventTimeout = setTimeout(() => { - cleanup(); rejectCallback(new TimeoutError('Timeout exceeded while waiting for event')); }, timeout); } @@ -145,7 +145,16 @@ class Helper { Helper.removeEventListeners([listener]); clearTimeout(eventTimeout); } - return promise; + const result = await Promise.race([promise, abortPromise]).then(r => { + cleanup(); + return r; + }, e => { + cleanup(); + throw e; + }); + if (result instanceof Error) + throw result; + return result; } /** diff --git a/lib/Page.js b/lib/Page.js index dac0797f03f80..0ec70f53c56d7 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -694,6 +694,12 @@ class Page extends EventEmitter { return await this._frameManager.mainFrame().waitForNavigation(options); } + _sessionClosePromise() { + if (!this._disconnectPromise) + this._disconnectPromise = new Promise(fulfill => this._client.once(Events.CDPSession.Disconnected, () => fulfill(new Error('Target closed')))); + return this._disconnectPromise; + } + /** * @param {(string|Function)} urlOrPredicate * @param {!{timeout?: number}=} options @@ -709,7 +715,7 @@ class Page extends EventEmitter { if (typeof urlOrPredicate === 'function') return !!(urlOrPredicate(request)); return false; - }, timeout); + }, timeout, this._sessionClosePromise()); } /** @@ -727,7 +733,7 @@ class Page extends EventEmitter { if (typeof urlOrPredicate === 'function') return !!(urlOrPredicate(response)); return false; - }, timeout); + }, timeout, this._sessionClosePromise()); } /** diff --git a/lib/helper.js b/lib/helper.js index 217b04df5ccf4..d6bd31439e222 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -176,9 +176,11 @@ class Helper { * @param {!NodeJS.EventEmitter} emitter * @param {(string|symbol)} eventName * @param {function} predicate + * @param {number} timeout + * @param {!Promise} abortPromise * @return {!Promise} */ - static waitForEvent(emitter, eventName, predicate, timeout) { + static async waitForEvent(emitter, eventName, predicate, timeout, abortPromise) { let eventTimeout, resolveCallback, rejectCallback; const promise = new Promise((resolve, reject) => { resolveCallback = resolve; @@ -187,12 +189,10 @@ class Helper { const listener = Helper.addEventListener(emitter, eventName, event => { if (!predicate(event)) return; - cleanup(); resolveCallback(event); }); if (timeout) { eventTimeout = setTimeout(() => { - cleanup(); rejectCallback(new TimeoutError('Timeout exceeded while waiting for event')); }, timeout); } @@ -200,7 +200,16 @@ class Helper { Helper.removeEventListeners([listener]); clearTimeout(eventTimeout); } - return promise; + const result = await Promise.race([promise, abortPromise]).then(r => { + cleanup(); + return r; + }, e => { + cleanup(); + throw e; + }); + if (result instanceof Error) + throw result; + return result; } /** diff --git a/test/launcher.spec.js b/test/launcher.spec.js index 41a1e2b063802..b993eb2020d3c 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -84,6 +84,23 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p await browser.close(); }); }); + describe('Browser.close', function() { + it('should terminate network waiters', async({context, server}) => { + const browser = await puppeteer.launch(defaultBrowserOptions); + const remote = await puppeteer.connect({browserWSEndpoint: browser.wsEndpoint()}); + const newPage = await remote.newPage(); + const results = await Promise.all([ + newPage.waitForRequest(server.EMPTY_PAGE).catch(e => e), + newPage.waitForResponse(server.EMPTY_PAGE).catch(e => e), + browser.close() + ]); + for (let i = 0; i < 2; i++) { + const message = results[i].message; + expect(message).toContain('Target closed'); + expect(message).not.toContain('Timeout'); + } + }); + }); describe('Puppeteer.launch', function() { it('should reject all promises when browser is closed', async() => { const browser = await puppeteer.launch(defaultBrowserOptions); diff --git a/test/page.spec.js b/test/page.spec.js index b3fc082d6fc22..2bd3195eba9ee 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -77,6 +77,19 @@ module.exports.addTests = function({testRunner, expect, headless, puppeteer, CHR await newPage.close(); expect(newPage.isClosed()).toBe(true); }); + it('should terminate network waiters', async({context, server}) => { + const newPage = await context.newPage(); + const results = await Promise.all([ + newPage.waitForRequest(server.EMPTY_PAGE).catch(e => e), + newPage.waitForResponse(server.EMPTY_PAGE).catch(e => e), + newPage.close() + ]); + for (let i = 0; i < 2; i++) { + const message = results[i].message; + expect(message).toContain('Target closed'); + expect(message).not.toContain('Timeout'); + } + }); }); describe('Page.Events.Load', function() {