From aaaf9fbb9a5c4a3c40c69260971d9435b70d6166 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 1 Nov 2018 10:29:10 -0500 Subject: [PATCH 1/3] core(driver): wait for Page.frameNavigated for about:blank --- lighthouse-core/gather/driver.js | 17 ++++++++++++++++- lighthouse-core/gather/gather-runner.js | 7 ++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 5317dfcaa5f6..39f37fc9f203 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -496,6 +496,16 @@ class Driver { }); } + /** + * Returns a promise that resolve when a frame has been navigated. + * Used for detecting that our about:blank reset has been completed. + */ + _waitForFrameNavigated() { + return new Promise(resolve => { + this.once('Page.frameNavigated', resolve); + }); + } + /** * Returns a promise that resolves when the network has been idle (after DCL) for * `networkQuietThresholdMs` ms and a method to cancel internal network listeners/timeout. @@ -824,10 +834,11 @@ class Driver { * possible workaround. * Resolves on the url of the loaded page, taking into account any redirects. * @param {string} url - * @param {{waitForLoad?: boolean, passContext?: LH.Gatherer.PassContext}} options + * @param {{waitForLoad?: boolean, waitForNavigated?: boolean, passContext?: LH.Gatherer.PassContext}} options * @return {Promise} */ async gotoURL(url, options = {}) { + const waitForNavigated = options.waitForNavigated || false; const waitForLoad = options.waitForLoad || false; const passContext = /** @type {Partial} */ (options.passContext || {}); const disableJS = passContext.disableJavaScript || false; @@ -843,6 +854,10 @@ class Driver { this.setNextProtocolTimeout(30 * 1000); this.sendCommand('Page.navigate', {url}); + if (waitForNavigated) { + await this._waitForFrameNavigated(); + } + if (waitForLoad) { const passConfig = /** @type {Partial} */ (passContext.passConfig || {}); let {pauseAfterLoadMs, networkQuietThresholdMs, cpuQuietThresholdMs} = passConfig; diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 7251e297672d..220d7c5d411b 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -68,18 +68,15 @@ class GatherRunner { * never fired on it. * @param {Driver} driver * @param {string=} url - * @param {number=} duration * @return {Promise} */ static async loadBlank( driver, url = constants.defaultPassConfig.blankPage, - duration = constants.defaultPassConfig.blankDuration ) { const status = {msg: 'Resetting state with about:blank', id: 'lh:gather:loadBlank'}; log.time(status); - await driver.gotoURL(url); - await new Promise(resolve => setTimeout(resolve, duration)); + await driver.gotoURL(url, {waitForNavigated: true}); log.timeEnd(status); } @@ -458,7 +455,7 @@ class GatherRunner { await driver.setThrottling(options.settings, passConfig); if (!isFirstPass) { // Already on blank page if driver was just set up. - await GatherRunner.loadBlank(driver, passConfig.blankPage, passConfig.blankDuration); + await GatherRunner.loadBlank(driver, passConfig.blankPage); } await GatherRunner.beforePass(passContext, gathererResults); await GatherRunner.pass(passContext, gathererResults); From 38797aea9363de192a8c93a2b32c008d2df9351b Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 1 Nov 2018 10:33:55 -0500 Subject: [PATCH 2/3] comma dangle --- lighthouse-core/gather/gather-runner.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 220d7c5d411b..bf5a0632f7d5 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -72,7 +72,7 @@ class GatherRunner { */ static async loadBlank( driver, - url = constants.defaultPassConfig.blankPage, + url = constants.defaultPassConfig.blankPage ) { const status = {msg: 'Resetting state with about:blank', id: 'lh:gather:loadBlank'}; log.time(status); From 2d096093340da04232ff9fb79518c344872e04ba Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 1 Nov 2018 14:57:06 -0500 Subject: [PATCH 3/3] more cleanup and feedback --- .../test/cli/__snapshots__/index-test.js.snap | 4 ---- lighthouse-core/config/constants.js | 1 - lighthouse-core/gather/driver.js | 4 ++++ lighthouse-core/gather/gather-runner.js | 5 +---- .../test/gather/gather-runner-test.js | 20 +++++++------------ typings/config.d.ts | 1 - 6 files changed, 12 insertions(+), 23 deletions(-) diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap index 749933aa4ab2..4a70b36d973a 100644 --- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap +++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap @@ -1029,7 +1029,6 @@ Object { }, "passes": Array [ Object { - "blankDuration": 300, "blankPage": "about:blank", "blockedUrlPatterns": Array [], "cpuQuietThresholdMs": 1000, @@ -1123,7 +1122,6 @@ Object { "useThrottling": true, }, Object { - "blankDuration": 300, "blankPage": "about:blank", "blockedUrlPatterns": Array [], "cpuQuietThresholdMs": 0, @@ -1145,7 +1143,6 @@ Object { "useThrottling": false, }, Object { - "blankDuration": 300, "blankPage": "about:blank", "blockedUrlPatterns": Array [ "*.css", @@ -1290,7 +1287,6 @@ Object { }, "passes": Array [ Object { - "blankDuration": 300, "blankPage": "about:blank", "blockedUrlPatterns": Array [], "cpuQuietThresholdMs": 1000, diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js index 0fcfdc60ba9e..46d19987f17e 100644 --- a/lighthouse-core/config/constants.js +++ b/lighthouse-core/config/constants.js @@ -59,7 +59,6 @@ const defaultPassConfig = { cpuQuietThresholdMs: 0, blockedUrlPatterns: [], blankPage: 'about:blank', - blankDuration: 300, gatherers: [], }; diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 39f37fc9f203..52a0b2ef3973 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -843,6 +843,10 @@ class Driver { const passContext = /** @type {Partial} */ (options.passContext || {}); const disableJS = passContext.disableJavaScript || false; + if (waitForNavigated && waitForLoad) { + throw new Error('Cannot use both waitForNavigated and waitForLoad, pick just one'); + } + await this._beginNetworkStatusMonitoring(url); await this._clearIsolatedContextId(); diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index bf5a0632f7d5..6eaeb4b079c9 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -70,10 +70,7 @@ class GatherRunner { * @param {string=} url * @return {Promise} */ - static async loadBlank( - driver, - url = constants.defaultPassConfig.blankPage - ) { + static async loadBlank(driver, url = constants.defaultPassConfig.blankPage) { const status = {msg: 'Resetting state with about:blank', id: 'lh:gather:loadBlank'}; log.time(status); await driver.gotoURL(url, {waitForNavigated: true}); diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 96dbf04c12b8..2b773a8d8faa 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -560,14 +560,12 @@ describe('GatherRunner', function() { const settings = {}; const passes = [{ - blankDuration: 0, recordTrace: true, passName: 'firstPass', gatherers: [ {instance: t1}, ], }, { - blankDuration: 0, passName: 'secondPass', gatherers: [ {instance: t2}, @@ -587,12 +585,10 @@ describe('GatherRunner', function() { it('respects trace names', () => { const passes = [{ - blankDuration: 0, recordTrace: true, passName: 'firstPass', gatherers: [{instance: new TestGatherer()}], }, { - blankDuration: 0, recordTrace: true, passName: 'secondPass', gatherers: [{instance: new TestGatherer()}], @@ -610,12 +606,10 @@ describe('GatherRunner', function() { it('doesn\'t leave networkRecords as an artifact', () => { const passes = [{ - blankDuration: 0, recordTrace: true, passName: 'firstPass', gatherers: [{instance: new TestGatherer()}], }, { - blankDuration: 0, recordTrace: true, passName: 'secondPass', gatherers: [{instance: new TestGatherer()}], @@ -790,7 +784,7 @@ describe('GatherRunner', function() { }, ]; const passes = [{ - blankDuration: 0, + gatherers: gatherers.map(G => ({instance: new G()})), }]; @@ -844,7 +838,7 @@ describe('GatherRunner', function() { ].map(instance => ({instance})); const gathererNames = gatherers.map(gatherer => gatherer.instance.name); const passes = [{ - blankDuration: 0, + gatherers, }]; @@ -881,7 +875,7 @@ describe('GatherRunner', function() { {instance: new class EavesdropGatherer3 extends EavesdropGatherer {}()}, ]; - const passes = [{blankDuration: 0, gatherers}]; + const passes = [{gatherers}]; return GatherRunner.run(passes, { driver: fakeDriver, requestedUrl: 'https://example.com', @@ -1002,7 +996,7 @@ describe('GatherRunner', function() { ].map(instance => ({instance})); const gathererNames = gatherers.map(gatherer => gatherer.instance.name); const passes = [{ - blankDuration: 0, + gatherers, }]; @@ -1022,7 +1016,7 @@ describe('GatherRunner', function() { it('rejects if a gatherer does not provide an artifact', () => { const passes = [{ - blankDuration: 0, + recordTrace: true, passName: 'firstPass', gatherers: [ @@ -1040,7 +1034,7 @@ describe('GatherRunner', function() { it('rejects when domain name can\'t be resolved', () => { const passes = [{ - blankDuration: 0, + recordTrace: true, passName: 'firstPass', gatherers: [], @@ -1071,7 +1065,7 @@ describe('GatherRunner', function() { it('resolves when domain name can\'t be resolved but is offline', () => { const passes = [{ - blankDuration: 0, + recordTrace: true, passName: 'firstPass', gatherers: [], diff --git a/typings/config.d.ts b/typings/config.d.ts index 1866a095549f..3992fb3d2ca0 100644 --- a/typings/config.d.ts +++ b/typings/config.d.ts @@ -35,7 +35,6 @@ declare global { cpuQuietThresholdMs?: number; blockedUrlPatterns?: string[]; blankPage?: string; - blankDuration?: number; gatherers?: GathererJson[]; }