From ce8a952044ef318baf3d9ff25157e41571293de5 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Mon, 23 Oct 2017 18:10:59 -0700 Subject: [PATCH] refactor: migrate NavigatorWatcher to lifecycle events (#1141) This patch: - migrates navigation watcher to use protocol-issued lifecycle events. - removes `networkIdleTimeout` and `networkIdleInflight` options for `page.goto` method - adds a new `networkidle0` value to the waitUntil option of navigation methods References #728. BREAKING CHANGE: As an implication of this new approach, the `networkIdleTimeout` and `networkIdleInflight` options are no longer supported. Interested clients should implement the behavior themselves using the `request` and `response` events. --- README.md | 2 +- docs/api.md | 25 ++++++------- examples/custom-event.js | 2 +- examples/detect-sniff.js | 2 +- examples/pdf.js | 2 +- examples/search.js | 2 +- lib/NavigatorWatcher.js | 78 +++++++++++++++++----------------------- lib/Page.js | 6 ++-- test/test.js | 21 +++++++---- 9 files changed, 67 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 32b073c6..1d407ec4 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ const puppeteer = require('puppeteer'); (async () => { const browser = await puppeteer.launch(); const page = await browser.newPage(); - await page.goto('https://news.ycombinator.com', {waitUntil: 'networkidle'}); + await page.goto('https://news.ycombinator.com', {waitUntil: 'networkidle2'}); await page.pdf({path: 'hn.pdf', format: 'A4'}); await browser.close(); diff --git a/docs/api.md b/docs/api.md index e876b1bb..2c37c613 100644 --- a/docs/api.md +++ b/docs/api.md @@ -763,9 +763,8 @@ If there's no element matching `selector`, the method throws an error. - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. - `waitUntil` <[string]> When to consider a navigation finished, defaults to `load`. Can be either: - `load` - consider navigation to be finished when the `load` event is fired. - - `networkidle` - consider navigation to be finished when the network activity stays "idle" for at least `networkIdleTimeout` ms. - - `networkIdleInflight` <[number]> Maximum amount of inflight requests which are considered "idle". Takes effect only with `waitUntil: 'networkidle'` parameter. - - `networkIdleTimeout` <[number]> A timeout to wait before completing navigation. Takes effect only with `waitUntil: 'networkidle'` parameter. + - `networkidle0` - consider navigation to be finished when there are no more then 0 network connections for at least `500` ms. + - `networkidle2` - consider navigation to be finished when there are no more then 2 network connections for at least `500` ms. - returns: <[Promise]<[Response]>> Promise which resolves to the main resource response. In case of multiple redirects, the navigation will resolve with the response of the last redirect. If can not go back, resolves to null. @@ -776,9 +775,8 @@ Navigate to the previous page in history. - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. - `waitUntil` <[string]> When to consider navigation succeeded, defaults to `load`. Can be either: - `load` - consider navigation to be finished when the `load` event is fired. - - `networkidle` - consider navigation to be finished when the network activity stays "idle" for at least `networkIdleTimeout` ms. - - `networkIdleInflight` <[number]> Maximum amount of inflight requests which are considered "idle". Takes effect only with `waitUntil: 'networkidle'` parameter. - - `networkIdleTimeout` <[number]> A timeout to wait before completing navigation. Takes effect only with `waitUntil: 'networkidle'` parameter. + - `networkidle0` - consider navigation to be finished when there are no more then 0 network connections for at least `500` ms. + - `networkidle2` - consider navigation to be finished when there are no more then 2 network connections for at least `500` ms. - returns: <[Promise]<[Response]>> Promise which resolves to the main resource response. In case of multiple redirects, the navigation will resolve with the response of the last redirect. If can not go back, resolves to null. @@ -790,9 +788,8 @@ Navigate to the next page in history. - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. - `waitUntil` <[string]> When to consider navigation succeeded, defaults to `load`. Can be either: - `load` - consider navigation to be finished when the `load` event is fired. - - `networkidle` - consider navigation to be finished when the network activity stays "idle" for at least `networkIdleTimeout` ms. - - `networkIdleInflight` <[number]> Maximum amount of inflight requests which are considered "idle". Takes effect only with `waitUntil: 'networkidle'` parameter. Defaults to 2. - - `networkIdleTimeout` <[number]> A timeout to wait before completing navigation. Takes effect only with `waitUntil: 'networkidle'` parameter. Defaults to 1000 ms. + - `networkidle0` - consider navigation to be finished when there are no more then 0 network connections for at least `500` ms. + - `networkidle2` - consider navigation to be finished when there are no more then 2 network connections for at least `500` ms. - returns: <[Promise]<[Response]>> Promise which resolves to the main resource response. In case of multiple redirects, the navigation will resolve with the response of the last redirect. The `page.goto` will throw an error if: @@ -905,9 +902,8 @@ Shortcut for [page.mainFrame().executionContext().queryObjects(prototypeHandle)] - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. - `waitUntil` <[string]> When to consider navigation succeeded, defaults to `load`. Can be either: - `load` - consider navigation to be finished when the `load` event is fired. - - `networkidle` - consider navigation to be finished when the network activity stays "idle" for at least `networkIdleTimeout` ms. - - `networkIdleInflight` <[number]> Maximum amount of inflight requests which are considered "idle". Takes effect only with `waitUntil: 'networkidle'` parameter. - - `networkIdleTimeout` <[number]> A timeout to wait before completing navigation. Takes effect only with `waitUntil: 'networkidle'` parameter. + - `networkidle0` - consider navigation to be finished when there are no more then 0 network connections for at least `500` ms. + - `networkidle2` - consider navigation to be finished when there are no more then 2 network connections for at least `500` ms. - returns: <[Promise]<[Response]>> Promise which resolves to the main resource response. In case of multiple redirects, the navigation will resolve with the response of the last redirect. #### page.screenshot([options]) @@ -1109,9 +1105,8 @@ Shortcut for [page.mainFrame().waitForFunction(pageFunction[, options[, ...args] - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. - `waitUntil` <[string]> When to consider navigation succeeded, defaults to `load`. Can be either: - `load` - consider navigation to be finished when the `load` event is fired. - - `networkidle` - consider navigation to be finished when the network activity stays "idle" for at least `networkIdleTimeout` ms. - - `networkIdleInflight` <[number]> Maximum amount of inflight requests which are considered "idle". Takes effect only with `waitUntil: 'networkidle'` parameter. - - `networkIdleTimeout` <[number]> A timeout to wait before completing navigation. Takes effect only with `waitUntil: 'networkidle'` parameter. + - `networkidle0` - consider navigation to be finished when there are no more then 0 network connections for at least `500` ms. + - `networkidle2` - consider navigation to be finished when there are no more then 2 network connections for at least `500` ms. - returns: <[Promise]<[Response]>> Promise which resolves to the main resource response. In case of multiple redirects, the navigation will resolve with the response of the last redirect. #### page.waitForSelector(selector[, options]) diff --git a/examples/custom-event.js b/examples/custom-event.js index 6b958b37..527612ad 100644 --- a/examples/custom-event.js +++ b/examples/custom-event.js @@ -43,7 +43,7 @@ function listenFor(type) { await listenFor('app-ready'); // Listen for "app-ready" custom event on page load. -await page.goto('https://www.chromestatus.com/features', {waitUntil: 'networkidle'}); +await page.goto('https://www.chromestatus.com/features', {waitUntil: 'networkidle2'}); await browser.close(); diff --git a/examples/detect-sniff.js b/examples/detect-sniff.js index f55fb88c..b8b37ad2 100644 --- a/examples/detect-sniff.js +++ b/examples/detect-sniff.js @@ -38,7 +38,7 @@ function sniffDetector() { const browser = await puppeteer.launch(); const page = await browser.newPage(); await page.evaluateOnNewDocument(sniffDetector); -await page.goto('https://www.google.com', {waitUntil: 'networkidle'}); +await page.goto('https://www.google.com', {waitUntil: 'networkidle2'}); console.log('Sniffed: ' + (await page.evaluate(() => !!navigator.sniffed))); await browser.close(); diff --git a/examples/pdf.js b/examples/pdf.js index 836d0b13..a31de285 100644 --- a/examples/pdf.js +++ b/examples/pdf.js @@ -22,7 +22,7 @@ const puppeteer = require('puppeteer'); const browser = await puppeteer.launch(); const page = await browser.newPage(); -await page.goto('https://news.ycombinator.com', {waitUntil: 'networkidle'}); +await page.goto('https://news.ycombinator.com', {waitUntil: 'networkidle2'}); // page.pdf() is currently supported only in headless mode. // @see https://bugs.chromium.org/p/chromium/issues/detail?id=753118 await page.pdf({ diff --git a/examples/search.js b/examples/search.js index 7c09a40f..794f0530 100644 --- a/examples/search.js +++ b/examples/search.js @@ -22,7 +22,7 @@ const puppeteer = require('puppeteer'); const browser = await puppeteer.launch(); const page = await browser.newPage(); -await page.goto('https://google.com', {waitUntil: 'networkidle'}); +await page.goto('https://google.com', {waitUntil: 'networkidle2'}); // Type our query into the search bar await page.type('input[name=q]', 'puppeteer'); diff --git a/lib/NavigatorWatcher.js b/lib/NavigatorWatcher.js index 5f8b6580..25d9a693 100644 --- a/lib/NavigatorWatcher.js +++ b/lib/NavigatorWatcher.js @@ -19,25 +19,28 @@ const {helper} = require('./helper'); class NavigatorWatcher { /** * @param {!Puppeteer.Session} client + * @param {string} frameId * @param {boolean} ignoreHTTPSErrors * @param {!Object=} options */ - constructor(client, ignoreHTTPSErrors, options = {}) { + constructor(client, frameId, ignoreHTTPSErrors, options = {}) { + console.assert(options.networkIdleTimeout === undefined, 'ERROR: networkIdleTimeout option is no longer supported.'); + console.assert(options.networkIdleInflight === undefined, 'ERROR: networkIdleInflight option is no longer supported.'); + console.assert(options.waitUntil !== 'networkidle', 'ERROR: "networkidle" option is no longer supported. Use "networkidle2" instead'); this._client = client; + this._frameId = frameId; this._ignoreHTTPSErrors = ignoreHTTPSErrors; this._timeout = typeof options['timeout'] === 'number' ? options['timeout'] : 30000; - this._idleTime = typeof options['networkIdleTimeout'] === 'number' ? options['networkIdleTimeout'] : 1000; - this._idleInflight = typeof options['networkIdleInflight'] === 'number' ? options['networkIdleInflight'] : 2; - this._waitUntil = typeof options['waitUntil'] === 'string' ? options['waitUntil'] : 'load'; - console.assert(this._waitUntil === 'load' || this._waitUntil === 'networkidle', 'Unknown value for options.waitUntil: ' + this._waitUntil); + const waitUntil = typeof options['waitUntil'] === 'string' ? options['waitUntil'] : 'load'; + const isAllowedWaitUntil = waitUntil === 'networkidle0' || waitUntil === 'networkidle2' || waitUntil === 'load'; + console.assert(isAllowedWaitUntil, 'Unknown value for options.waitUntil: ' + waitUntil); + this._pendingEvents = new Set([waitUntil]); } /** * @return {!Promise} */ async waitForNavigation() { - this._requestIds = new Set(); - this._eventListeners = []; const navigationPromises = []; @@ -54,58 +57,43 @@ class NavigatorWatcher { navigationPromises.push(certificateError); } - if (this._waitUntil === 'load') { - const loadEventFired = new Promise(fulfill => { - this._eventListeners.push(helper.addEventListener(this._client, 'Page.loadEventFired', fulfill)); - }).then(() => null); - navigationPromises.push(loadEventFired); - } else { - this._eventListeners.push(...[ - helper.addEventListener(this._client, 'Network.requestWillBeSent', this._onLoadingStarted.bind(this)), - helper.addEventListener(this._client, 'Network.loadingFinished', this._onLoadingCompleted.bind(this)), - helper.addEventListener(this._client, 'Network.loadingFailed', this._onLoadingCompleted.bind(this)), - helper.addEventListener(this._client, 'Network.webSocketCreated', this._onLoadingStarted.bind(this)), - helper.addEventListener(this._client, 'Network.webSocketClosed', this._onLoadingCompleted.bind(this)), - ]); - const networkIdle = new Promise(fulfill => this._networkIdleCallback = fulfill).then(() => null); - navigationPromises.push(networkIdle); - } + this._eventListeners.push(helper.addEventListener(this._client, 'Page.lifecycleEvent', this._onLifecycleEvent.bind(this))); + const pendingEventsFired = new Promise(fulfill => this._pendingEventsCallback = fulfill); + navigationPromises.push(pendingEventsFired); const error = await Promise.race(navigationPromises); this._cleanup(); return error ? new Error(error) : null; } + /** + * @param {!{frameId: string, name: string}} event + */ + _onLifecycleEvent(event) { + if (event.frameId !== this._frameId) + return; + const pptrName = protocolLifecycleToPuppeteer[event.name]; + if (!pptrName) + return; + this._pendingEvents.delete(pptrName); + if (this._pendingEvents.size === 0) + this._pendingEventsCallback(); + } + cancel() { this._cleanup(); } - /** - * @param {!Object} event - */ - _onLoadingStarted(event) { - this._requestIds.add(event.requestId); - if (this._requestIds.size > this._idleInflight) { - clearTimeout(this._idleTimer); - this._idleTimer = null; - } - } - - /** - * @param {!Object} event - */ - _onLoadingCompleted(event) { - this._requestIds.delete(event.requestId); - if (this._requestIds.size <= this._idleInflight && !this._idleTimer) - this._idleTimer = setTimeout(this._networkIdleCallback, this._idleTime); - } - _cleanup() { helper.removeEventListeners(this._eventListeners); - - clearTimeout(this._idleTimer); clearTimeout(this._maximumTimer); } } +const protocolLifecycleToPuppeteer = { + 'load': 'load', + 'networkIdle': 'networkidle0', + 'networkAlmostIdle': 'networkidle2' +}; + module.exports = NavigatorWatcher; diff --git a/lib/Page.js b/lib/Page.js index 4db2ecad..dea00d9b 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -452,7 +452,8 @@ class Page extends EventEmitter { * @return {!Promise} */ async goto(url, options) { - const watcher = new NavigatorWatcher(this._client, this._ignoreHTTPSErrors, options); + const mainFrame = this._frameManager.mainFrame(); + const watcher = new NavigatorWatcher(this._client, mainFrame._id, this._ignoreHTTPSErrors, options); const responses = new Map(); const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url, response)); const navigationPromise = watcher.waitForNavigation(); @@ -492,7 +493,8 @@ class Page extends EventEmitter { * @return {!Promise} */ async waitForNavigation(options) { - const watcher = new NavigatorWatcher(this._client, this._ignoreHTTPSErrors, options); + const mainFrame = this._frameManager.mainFrame(); + const watcher = new NavigatorWatcher(this._client, mainFrame._id, this._ignoreHTTPSErrors, options); const responses = new Map(); const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url, response)); diff --git a/test/test.js b/test/test.js index ded66436..37e3be27 100644 --- a/test/test.js +++ b/test/test.js @@ -884,6 +884,14 @@ describe('Page', function() { const response = await page.goto('about:blank'); expect(response).toBe(null); })); + it('should navigate to empty page with networkidle0', SX(async function() { + const response = await page.goto(EMPTY_PAGE, {waitUntil: 'networkidle0'}); + expect(response.status).toBe(200); + })); + it('should navigate to empty page with networkidle2', SX(async function() { + const response = await page.goto(EMPTY_PAGE, {waitUntil: 'networkidle2'}); + expect(response.status).toBe(200); + })); it('should fail when navigating to bad url', SX(async function() { let error = null; await page.goto('asdfasdf').catch(e => error = e); @@ -899,6 +907,11 @@ describe('Page', function() { await page.goto(HTTPS_PREFIX + '/empty.html').catch(e => error = e); expect(error.message).toContain('SSL Certificate error'); })); + it('should throw if networkidle is passed as an option', SX(async function() { + let error = null; + await page.goto(EMPTY_PAGE, {waitUntil: 'networkidle'}).catch(err => error = err); + expect(error.message).toContain('"networkidle" option is no longer supported'); + })); it('should fail when main resources failed to load', SX(async function() { let error = null; await page.goto('http://localhost:44123/non-existing-url').catch(e => error = e); @@ -959,9 +972,7 @@ describe('Page', function() { // Navigate to a page which loads immediately and then does a bunch of // requests via javascript's fetch method. const navigationPromise = page.goto(PREFIX + '/networkidle.html', { - waitUntil: 'networkidle', - networkIdleTimeout: 100, - networkIdleInflight: 0, // Only be idle when there are 0 inflight requests + waitUntil: 'networkidle0', }); // Track when the navigation gets completed. let navigationFinished = false; @@ -1009,9 +1020,7 @@ describe('Page', function() { // Navigate to a page which loads immediately and then opens a bunch of // websocket connections and then a fetch request. const navigationPromise = page.goto(PREFIX + '/websocket.html', { - waitUntil: 'networkidle', - networkIdleTimeout: 100, - networkIdleInflight: 0, // Only be idle when there are 0 inflight requests/connections + waitUntil: 'networkidle0', }); // Track when the navigation gets completed. let navigationFinished = false;