From b466513aaa6e28a8d7fda9ef14c444b548cbe090 Mon Sep 17 00:00:00 2001
From: Ward Peeters <ward@coding-tech.com>
Date: Mon, 9 Jan 2017 22:34:43 +0100
Subject: [PATCH] Updated review

---
 lighthouse-core/gather/gatherers/offline.js   |  6 +----
 lighthouse-core/lib/url-shim.js               | 10 ++++++++
 .../test/gather/gatherers/offline-test.js     | 24 ++++++++++---------
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/offline.js b/lighthouse-core/gather/gatherers/offline.js
index 55276b764e7b..25fbe4a100d3 100644
--- a/lighthouse-core/gather/gatherers/offline.js
+++ b/lighthouse-core/gather/gatherers/offline.js
@@ -26,7 +26,7 @@ class Offline extends Gatherer {
 
   afterPass(options, tracingData) {
     const navigationRecord = tracingData.networkRecords.filter(record => {
-      return cleanHash(new URL(record._url)) === cleanHash(new URL(options.url)) &&
+      return new URL(record._url).stripHash().href === new URL(options.url).stripHash().href &&
         record._fetchedViaServiceWorker;
     }).pop(); // Take the last record that matches.
 
@@ -36,8 +36,4 @@ class Offline extends Gatherer {
   }
 }
 
-function cleanHash(url) {
-  return url.href.removeURLFragment();
-}
-
 module.exports = Offline;
diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js
index 2075882da2b8..9d03cf2c04d3 100644
--- a/lighthouse-core/lib/url-shim.js
+++ b/lighthouse-core/lib/url-shim.js
@@ -57,4 +57,14 @@ URL.hostsMatch = function hostsMatch(urlA, urlB) {
   }
 };
 
+/**
+ * Removes hash from URL
+ * @return {URL} Url object chaining
+ */
+URL.prototype.stripHash = function stripHash() {
+  this.hash = '';
+
+  return this;
+};
+
 module.exports = URL;
diff --git a/lighthouse-core/test/gather/gatherers/offline-test.js b/lighthouse-core/test/gather/gatherers/offline-test.js
index c6ca86f814a8..f6d2b6ace828 100644
--- a/lighthouse-core/test/gather/gatherers/offline-test.js
+++ b/lighthouse-core/test/gather/gatherers/offline-test.js
@@ -37,9 +37,19 @@ describe('Offline gatherer', () => {
       url: 'https://do-not-match.com/',
       driver: mockDriver
     };
-    return offlineGather.afterPass(options, tracingData).then(artifact => {
-      assert.strictEqual(artifact, -1);
-    });
+    const optionsWithQueryString = {
+      url: 'https://ifixit-pwa.appspot.com/?history',
+      driver: mockDriver
+    };
+
+    return Promise.all([
+      offlineGather.afterPass(options, tracingData).then(artifact => {
+        assert.strictEqual(artifact, -1);
+      }),
+      offlineGather.afterPass(optionsWithQueryString, tracingData).then(artifact => {
+        assert.strictEqual(artifact, -1);
+      }),
+    ]);
   });
 
   it('returns an artifact set to 200 when offline loading succeeds', () => {
@@ -52,11 +62,6 @@ describe('Offline gatherer', () => {
       url: 'https://ifixit-pwa.appspot.com/#/history',
       driver: mockDriver
     };
-    const optionsWithQueryString = {
-      url: 'https://ifixit-pwa.appspot.com/?history',
-      driver: mockDriver
-    };
-
     return Promise.all([
       offlineGather.afterPass(options, tracingData).then(artifact => {
         assert.strictEqual(artifact, 200);
@@ -64,9 +69,6 @@ describe('Offline gatherer', () => {
       offlineGather.afterPass(optionsWithFragment, tracingData).then(artifact => {
         assert.strictEqual(artifact, 200);
       }),
-      offlineGather.afterPass(optionsWithQueryString, tracingData).then(artifact => {
-        assert.strictEqual(artifact, -1);
-      }),
     ]);
   });
 });