From 8cf3841e32560dc42961399e748880f1b4677b2f Mon Sep 17 00:00:00 2001
From: Brendan Kenny <bckenny@google.com>
Date: Mon, 29 Aug 2016 15:38:02 -0700
Subject: [PATCH] about:blank navigation moved to before gatherer.beforeClass()

---
 lighthouse-core/gather/gather-runner.js       | 41 ++++++++++++-------
 .../test/gather/gather-runner-test.js         | 23 -----------
 2 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index 6321913df70f..c48d1193ac5f 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -33,10 +33,10 @@ const path = require('path');
  *
  * 2. For each pass in the config:
  *   A. GatherRunner.beforePass()
- *     i. all gatherer's beforePass()
+ *     i. navigate to about:blank
+ *     ii. all gatherer's beforePass()
  *   B. GatherRunner.pass()
  *     i. GatherRunner.loadPage()
- *       a. navigate to about:blank
  *       b. beginTrace (if requested) & beginNetworkCollect
  *       c. navigate to options.url (and wait for onload)
  *     ii. all gatherer's pass()
@@ -50,13 +50,27 @@ const path = require('path');
  *   C. collect all artifacts and return them
  */
 class GatherRunner {
-  static loadPage(driver, options) {
-    // Since a Page.reload command does not let a service worker take over, we
-    // navigate away and then come back to reload. We do not `waitForLoad` on
-    // about:blank since a page load event is never fired on it.
+  /**
+   * Loads about:blank and waits there briefly. Since a Page.reload command does
+   * not let a service worker take over, we navigate away and then come back to
+   * reload. We do not `waitForLoad` on about:blank since a page load event is
+   * never fired on it.
+   * @param {!Driver} driver
+   * @return {!Promise}
+   */
+  static loadBlank(driver) {
     return driver.gotoURL('about:blank')
-      // Wait a bit for about:blank to "take hold" before switching back to the page.
-      .then(_ => new Promise((resolve, reject) => setTimeout(resolve, 300)))
+      .then(_ => new Promise((resolve, reject) => setTimeout(resolve, 300)));
+  }
+
+  /**
+   * Loads options.url with specified options.
+   * @param {!Driver} driver
+   * @param {!Object} options
+   * @return {!Promise}
+   */
+  static loadPage(driver, options) {
+    return Promise.resolve()
       // Begin tracing only if requested by config.
       .then(_ => options.config.trace && driver.beginTrace())
       // Network is always recorded for internal use, even if not saved as artifact.
@@ -81,20 +95,19 @@ class GatherRunner {
   }
 
   /**
-   * Calls beforePass() on gatherers before navigation and before tracing has
-   * started (if requested).
+   * Navigates to about:blank and calls beforePass() on gatherers before tracing
+   * has started and before navigation to the target page.
    * @param {!Object} options
    * @return {!Promise}
    */
   static beforePass(options) {
-    const config = options.config;
-    const gatherers = config.gatherers;
+    const pass = GatherRunner.loadBlank(options.driver);
 
-    return gatherers.reduce((chain, gatherer) => {
+    return options.config.gatherers.reduce((chain, gatherer) => {
       return chain.then(_ => {
         return gatherer.beforePass(options);
       });
-    }, Promise.resolve());
+    }, pass);
   }
 
   /**
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 8c0910be6826..4c4650b3e84a 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -79,29 +79,6 @@ describe('GatherRunner', function() {
     });
   });
 
-  it('reloads a page via about:blank', () => {
-    const expected = [
-      'https://example.com',
-      'about:blank'
-    ];
-    const driver = {
-      gotoURL(url) {
-        assert(url, expected.pop());
-        return Promise.resolve(true);
-      },
-      beginNetworkCollect() {
-        return Promise.resolve();
-      }
-    };
-
-    return GatherRunner.loadPage(driver, {
-      url: 'https://example.com',
-      config: {}
-    }).then(res => {
-      assert.equal(res, true);
-    });
-  });
-
   it('sets up the driver to begin emulation when mobile == true', () => {
     let calledEmulation = false;
     const driver = {