From fa6e524a2e6f4fc1848ff15bc183de3be33ccc06 Mon Sep 17 00:00:00 2001
From: Brendan Kenny <bckenny@google.com>
Date: Mon, 29 Aug 2016 15:03:03 -0700
Subject: [PATCH] update offline gatherer to use network recording changes

---
 .../fixtures/smoketest-offline-config.json    |  1 +
 lighthouse-core/config/default.json           |  1 +
 lighthouse-core/gather/gatherers/offline.js   | 17 +++-------
 lighthouse-core/lib/network-recorder.js       |  8 +----
 .../test/gather/gatherers/offline-test.js     | 32 +++++++++++++------
 5 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/lighthouse-cli/test/fixtures/smoketest-offline-config.json b/lighthouse-cli/test/fixtures/smoketest-offline-config.json
index 2381769a838a..86110663c46b 100644
--- a/lighthouse-cli/test/fixtures/smoketest-offline-config.json
+++ b/lighthouse-cli/test/fixtures/smoketest-offline-config.json
@@ -6,6 +6,7 @@
     ]
   },{
     "loadPage": true,
+    "network": true,
     "gatherers": [
       "service-worker",
       "offline"
diff --git a/lighthouse-core/config/default.json b/lighthouse-core/config/default.json
index 82483dc07f48..f1d542ed7967 100644
--- a/lighthouse-core/config/default.json
+++ b/lighthouse-core/config/default.json
@@ -17,6 +17,7 @@
     ]
   },
   {
+    "passName": "offlinePass",
     "loadPage": true,
     "network": true,
     "gatherers": [
diff --git a/lighthouse-core/gather/gatherers/offline.js b/lighthouse-core/gather/gatherers/offline.js
index 3941f5ea9031..7d0be3eef8f4 100644
--- a/lighthouse-core/gather/gatherers/offline.js
+++ b/lighthouse-core/gather/gatherers/offline.js
@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 'use strict';
 
 const Gatherer = require('./gatherer');
@@ -43,26 +42,20 @@ class Offline extends Gatherer {
   }
 
   beforePass(options) {
-    const driver = options.driver;
-
-    return driver.gotoURL(options.url, {waitForLoad: true})
-      .then(_ => Offline.goOffline(driver))
-      // Navigate away, then back, to allow a service worker that doesn't call
-      // clients.claim() to take control of the page load.
-      .then(_ => driver.gotoURL('about:blank'))
-      .then(_ => driver.gotoURL(options.url, {waitForLoad: true}))
-      .then(_ => Offline.goOnline(driver));
+    return Offline.goOffline(options.driver);
   }
 
   afterPass(options, tracingData) {
     const navigationRecord = tracingData.networkRecords.filter(record => {
-      // If options.url is just an origin without a path, the Chrome will
-      // implicitly add in a path of '/'.
+      // If options.url is just an origin without a path, Chrome will implicitly
+      // add in a path of '/'.
       return (record._url === options.url || record._url === options.url + '/') &&
         record._fetchedViaServiceWorker;
     }).pop(); // Take the last record that matches.
 
     this.artifact = navigationRecord ? navigationRecord.statusCode : -1;
+
+    return Offline.goOnline(options.driver);
   }
 }
 
diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js
index 1b6cff9a4f0c..f8bf8cdacb14 100644
--- a/lighthouse-core/lib/network-recorder.js
+++ b/lighthouse-core/lib/network-recorder.js
@@ -87,14 +87,8 @@ class NetworkRecorder extends EventEmitter {
   }
 
   onResourceChangedPriority(data) {
-    // TODO: this.networkManager._dispatcher.resourceChangedPriority is set to
-    // undefined when onResourceChangedPriority is triggered in
-    // gatherers/offline.js. The underlying cause may need to be investigated.
-    // In the meantime, explicitly check that it's a function.
-    if (typeof this.networkManager._dispatcher.resourceChangedPriority === 'function') {
-      this.networkManager._dispatcher.resourceChangedPriority(data.requestId,
+    this.networkManager._dispatcher.resourceChangedPriority(data.requestId,
         data.newPriority, data.timestamp);
-    }
   }
 
   static recordsFromLogs(logs) {
diff --git a/lighthouse-core/test/gather/gatherers/offline-test.js b/lighthouse-core/test/gather/gatherers/offline-test.js
index 2604200be991..3130959171dc 100644
--- a/lighthouse-core/test/gather/gatherers/offline-test.js
+++ b/lighthouse-core/test/gather/gatherers/offline-test.js
@@ -20,21 +20,33 @@
 const OfflineGather = require('../../../gather/gatherers/offline');
 const assert = require('assert');
 const tracingData = require('../../fixtures/traces/network-records.json');
-let offlineGather;
 
-describe('Offline gatherer', () => {
-  // Reset the Gatherer before each test.
-  beforeEach(() => {
-    offlineGather = new OfflineGather();
-  });
+const mockDriver = {
+  sendCommand() {
+    return Promise.resolve();
+  }
+};
 
+describe('Offline gatherer', () => {
   it('returns an artifact set to -1 when offline loading fails', () => {
-    offlineGather.afterPass({url: 'https://do-not-match.com'}, tracingData);
-    assert.deepEqual(offlineGather.artifact, -1);
+    const offlineGather = new OfflineGather();
+    const options = {
+      url: 'https://do-not-match.com',
+      driver: mockDriver
+    };
+    return offlineGather.afterPass(options, tracingData).then(_ => {
+      assert.strictEqual(offlineGather.artifact, -1);
+    });
   });
 
   it('returns an artifact set to 200 when offline loading succeeds', () => {
-    offlineGather.afterPass({url: 'https://ifixit-pwa.appspot.com'}, tracingData);
-    assert.deepEqual(offlineGather.artifact, 200);
+    const offlineGather = new OfflineGather();
+    const options = {
+      url: 'https://ifixit-pwa.appspot.com',
+      driver: mockDriver
+    };
+    return offlineGather.afterPass(options, tracingData).then(_ => {
+      assert.strictEqual(offlineGather.artifact, 200);
+    });
   });
 });