From 5f28ea7ada7bda157484389b1df842a9c68a29c7 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 23 Oct 2018 13:47:44 -0700 Subject: [PATCH 1/4] core: don't load blank page twice in first pass --- lighthouse-core/gather/gather-runner.js | 45 ++++++++++--------- .../test/gather/gather-runner-test.js | 2 + 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index bab5985c8440..f22370387ba0 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -26,22 +26,22 @@ const Driver = require('../gather/driver.js'); // eslint-disable-line no-unused- * Execution sequence when GatherRunner.run() is called: * * 1. Setup - * A. navigate to about:blank * B. driver.connect() * C. GatherRunner.setupDriver() - * i. assertNoSameOriginServiceWorkerClients - * ii. beginEmulation - * iii. enableRuntimeEvents - * iv. evaluateScriptOnLoad rescue native Promise from potential polyfill - * v. register a performance observer - * vi. register dialog dismisser - * vii. clearDataForOrigin + * i. navigate to a blank page + * ii. assertNoSameOriginServiceWorkerClients + * iii. retrieve and save userAgent + * iv. beginEmulation + * v. enableRuntimeEvents + * vi. evaluateScriptOnLoad rescue native Promise from potential polyfill + * vii. register a performance observer + * viii. register dialog dismisser + * iv. clearDataForOrigin * * 2. For each pass in the config: * A. GatherRunner.beforePass() - * i. navigate to about:blank - * ii. Enable network request blocking for specified patterns - * iii. all gatherers' beforePass() + * i. Enable network request blocking for specified patterns + * ii. all gatherers' beforePass() * B. GatherRunner.pass() * i. cleanBrowserCaches() (if it's a perf run) * ii. beginDevtoolsLog() @@ -105,7 +105,9 @@ class GatherRunner { static async setupDriver(driver, options) { log.log('status', 'Initializing…'); const resetStorage = !options.settings.disableStorageReset; - // Enable emulation based on settings + // In the devtools/extension case, we can't still be on the site while trying to clear state + // So we first navigate to a blank page, then apply our emulation & setup + await GatherRunner.loadBlank(driver); await driver.assertNoSameOriginServiceWorkerClients(options.requestedUrl); await driver.beginEmulation(options.settings); await driver.enableRuntimeEvents(); @@ -176,7 +178,7 @@ class GatherRunner { } /** - * Navigates to about:blank and calls beforePass() on gatherers before tracing + * Calls beforePass() on gatherers before tracing * has started and before navigation to the target page. * @param {LH.Gatherer.PassContext} passContext * @param {Partial} gathererResults @@ -185,9 +187,7 @@ class GatherRunner { static async beforePass(passContext, gathererResults) { const blockedUrls = (passContext.passConfig.blockedUrlPatterns || []) .concat(passContext.settings.blockedUrlPatterns || []); - const blankPage = passContext.passConfig.blankPage; - const blankDuration = passContext.passConfig.blankDuration; - await GatherRunner.loadBlank(passContext.driver, blankPage, blankDuration); + // Set request blocking before any network activity // No "clearing" is done at the end of the pass since blockUrlPatterns([]) will unset all if // neccessary at the beginning of the next pass. @@ -394,12 +394,11 @@ class GatherRunner { try { await driver.connect(); const baseArtifacts = await GatherRunner.getBaseArtifacts(options); - await GatherRunner.loadBlank(driver); - baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); await GatherRunner.setupDriver(driver, options); + baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); // Run each pass - let firstPass = true; + let isFirstPass = true; for (const passConfig of passes) { const passContext = { driver: options.driver, @@ -412,6 +411,10 @@ 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.beforePass(passContext, gathererResults); await GatherRunner.pass(passContext, gathererResults); const passData = await GatherRunner.afterPass(passContext, gathererResults); @@ -434,10 +437,10 @@ class GatherRunner { baseArtifacts.traces[passConfig.passName] = passData.trace; } - if (firstPass) { + if (isFirstPass) { // Copy redirected URL to artifact in the first pass only. baseArtifacts.URL.finalUrl = passContext.url; - firstPass = false; + isFirstPass = false; } } const resetStorage = !options.settings.disableStorageReset; diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index b5f94b39e1ff..96dbf04c12b8 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -316,6 +316,7 @@ describe('GatherRunner', function() { dismissJavaScriptDialogs: asyncFunc, enableRuntimeEvents: asyncFunc, cacheNatives: asyncFunc, + gotoURL: asyncFunc, registerPerformanceObserver: asyncFunc, cleanBrowserCaches: createCheck('calledCleanBrowserCaches'), clearDataForOrigin: createCheck('calledClearStorage'), @@ -375,6 +376,7 @@ describe('GatherRunner', function() { dismissJavaScriptDialogs: asyncFunc, enableRuntimeEvents: asyncFunc, cacheNatives: asyncFunc, + gotoURL: asyncFunc, registerPerformanceObserver: asyncFunc, cleanBrowserCaches: createCheck('calledCleanBrowserCaches'), clearDataForOrigin: createCheck('calledClearStorage'), From bfb3ac11b9582041ecb3b35084a4041d6b94ed33 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 23 Oct 2018 15:55:08 -0700 Subject: [PATCH 2/4] restore original loadBlank --- lighthouse-core/gather/gather-runner.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index f22370387ba0..ca1775e6710f 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -394,8 +394,9 @@ class GatherRunner { try { await driver.connect(); const baseArtifacts = await GatherRunner.getBaseArtifacts(options); - await GatherRunner.setupDriver(driver, options); + await GatherRunner.loadBlank(driver); baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); + await GatherRunner.setupDriver(driver, options); // Run each pass let isFirstPass = true; From 772754eb630acef32f007a21f72764bffebeadeb Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 24 Oct 2018 09:17:39 -0700 Subject: [PATCH 3/4] remove extra call to load blank --- lighthouse-core/gather/gather-runner.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index ca1775e6710f..33d00829e833 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -105,9 +105,6 @@ class GatherRunner { static async setupDriver(driver, options) { log.log('status', 'Initializing…'); const resetStorage = !options.settings.disableStorageReset; - // In the devtools/extension case, we can't still be on the site while trying to clear state - // So we first navigate to a blank page, then apply our emulation & setup - await GatherRunner.loadBlank(driver); await driver.assertNoSameOriginServiceWorkerClients(options.requestedUrl); await driver.beginEmulation(options.settings); await driver.enableRuntimeEvents(); @@ -394,6 +391,8 @@ class GatherRunner { try { await driver.connect(); const baseArtifacts = await GatherRunner.getBaseArtifacts(options); + // In the devtools/extension case, we can't still be on the site while trying to clear state + // So we first navigate to a blank page, then apply our emulation & setup await GatherRunner.loadBlank(driver); baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); await GatherRunner.setupDriver(driver, options); From c8b2c130a3c179311de97aae5adcab3d36cd7984 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 24 Oct 2018 14:42:37 -0700 Subject: [PATCH 4/4] edit mega comment --- lighthouse-core/gather/gather-runner.js | 28 ++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index 33d00829e833..66ce17a6d40a 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -26,22 +26,22 @@ const Driver = require('../gather/driver.js'); // eslint-disable-line no-unused- * Execution sequence when GatherRunner.run() is called: * * 1. Setup - * B. driver.connect() - * C. GatherRunner.setupDriver() - * i. navigate to a blank page - * ii. assertNoSameOriginServiceWorkerClients - * iii. retrieve and save userAgent - * iv. beginEmulation - * v. enableRuntimeEvents - * vi. evaluateScriptOnLoad rescue native Promise from potential polyfill - * vii. register a performance observer - * viii. register dialog dismisser - * iv. clearDataForOrigin + * A. driver.connect() + * B. GatherRunner.setupDriver() + * i. assertNoSameOriginServiceWorkerClients + * ii. retrieve and save userAgent + * iii. beginEmulation + * iv. enableRuntimeEvents + * v. evaluateScriptOnLoad rescue native Promise from potential polyfill + * vi. register a performance observer + * vii. register dialog dismisser + * viii. clearDataForOrigin * * 2. For each pass in the config: * A. GatherRunner.beforePass() - * i. Enable network request blocking for specified patterns - * ii. all gatherers' beforePass() + * i. navigate to about:blank + * ii. Enable network request blocking for specified patterns + * iii. all gatherers' beforePass() * B. GatherRunner.pass() * i. cleanBrowserCaches() (if it's a perf run) * ii. beginDevtoolsLog() @@ -392,7 +392,7 @@ class GatherRunner { await driver.connect(); const baseArtifacts = await GatherRunner.getBaseArtifacts(options); // In the devtools/extension case, we can't still be on the site while trying to clear state - // So we first navigate to a blank page, then apply our emulation & setup + // So we first navigate to about:blank, then apply our emulation & setup await GatherRunner.loadBlank(driver); baseArtifacts.BenchmarkIndex = await options.driver.getBenchmarkIndex(); await GatherRunner.setupDriver(driver, options);