core(start-url): switch to plain old fetch (#4301)

GoogleChrome · Jan 19, 2018 · 550b0c4 · 550b0c4
1 parent e8ce460
commit 550b0c4
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 64 deletions.
diff --git a/lighthouse-core/audits/webapp-install-banner.js b/lighthouse-core/audits/webapp-install-banner.js
@@ -80,6 +80,7 @@ class WebappInstallBanner extends MultiCheckAudit {
 
     if (!hasOfflineStartUrl) {
       result.failures.push('Service worker does not successfully serve the manifest\'s start_url');
+      if (artifacts.StartUrl.debugString) result.failures.push(artifacts.StartUrl.debugString);
     }
 
     if (artifacts.StartUrl.debugString) {

diff --git a/lighthouse-core/gather/gatherers/start-url.js b/lighthouse-core/gather/gatherers/start-url.js
@@ -18,32 +18,11 @@ class StartUrl extends Gatherer {
   }
 
   executeFetchRequest(driver, url) {
-    return new Promise((resolve, reject) => {
-      let requestId;
-      const fetchRequestId = (data) => {
-        if (URL.equalWithExcludedFragments(data.request.url, url)) {
-          requestId = data.requestId;
-          driver.off('Network.requestWillBeSent', fetchRequestId);
-        }
-      };
-      const fetchDone = (data) => {
-        if (data.requestId === requestId) {
-          driver.off('Network.loadingFinished', fetchDone);
-          driver.off('Network.loadingFailed', fetchDone);
-
-          resolve();
-        }
-      };
-
-      driver.on('Network.requestWillBeSent', fetchRequestId);
-      driver.on('Network.loadingFinished', fetchDone);
-      driver.on('Network.loadingFailed', fetchDone);
-      driver.evaluateAsync(
-        `fetch('${url}')
-          .then(response => response.status)
-          .catch(err => -1)`
-      ).catch(err => reject(err));
-    });
+    return driver.evaluateAsync(
+      `fetch('${url}')
+        .then(response => response.status)
+        .catch(err => ({fetchFailed: true, message: err.message}))`
+    );
   }
 
   pass(options) {
@@ -78,17 +57,18 @@ class StartUrl extends Gatherer {
         record._fetchedViaServiceWorker;
     }).pop(); // Take the last record that matches.
 
+    const msgWithExtraDebugString = msg => this.debugString ? `${msg}: ${this.debugString}` : msg;
     return options.driver.goOnline(options)
       .then(_ => {
         if (!this.startUrl) {
           return {
             statusCode: -1,
-            debugString: this.debugString || 'No start URL to fetch',
+            debugString: msgWithExtraDebugString('No start URL to fetch'),
           };
         } else if (!navigationRecord) {
           return {
             statusCode: -1,
-            debugString: this.debugString || 'Did not fetch start URL from service worker',
+            debugString: msgWithExtraDebugString('Unable to fetch start URL via service worker'),
           };
         } else {
           return {

diff --git a/lighthouse-core/test/gather/gatherers/start-url-test.js b/lighthouse-core/test/gather/gatherers/start-url-test.js
@@ -7,7 +7,6 @@
 
 /* eslint-env mocha */
 
-const URL = require('../../../lib/url-shim');
 const StartUrlGatherer = require('../../../gather/gatherers/start-url');
 const assert = require('assert');
 const tracingData = require('../../fixtures/traces/network-records.json');
@@ -24,37 +23,7 @@ const mockDriver = {
 
 const wrapSendCommand = (mockDriver, url) => {
   mockDriver = Object.assign({}, mockDriver);
-  mockDriver.evaluateAsync = () => {
-    url = new URL(url);
-    url.hash = '';
-
-    const record = findRequestByUrl(url.href);
-    if (!record) {
-      return Promise.reject(-1);
-    }
-
-    return Promise.resolve(record.statusCode);
-  };
-
-  mockDriver.on = (name, cb) => {
-    if (name === 'Network.requestWillBeSent') {
-      cb({
-        request: {
-          url,
-          requestId: 1,
-        },
-      });
-    }
-
-    if (name === 'Network.loadingFinished') {
-      cb({
-        request: {
-          url,
-          requestId: 1,
-        },
-      });
-    }
-  };
+  mockDriver.evaluateAsync = () => Promise.resolve();
 
   mockDriver.getAppManifest = () => {
     return Promise.resolve({
@@ -67,10 +36,6 @@ const wrapSendCommand = (mockDriver, url) => {
   return mockDriver;
 };
 
-const findRequestByUrl = (url) => {
-  return tracingData.networkRecords.find(record => record._url === url);
-};
-
 describe('Start-url gatherer', () => {
   it('returns an artifact set to -1 when offline loading fails', () => {
     const startUrlGatherer = new StartUrlGatherer();