Merge a9f4a0f into 636b55d

lighthouse-core/gather/gather-runner.js

@@ -225,20 +225,21 @@ class GatherRunner {
 
   /**
    * Returns an error if we try to load a non-HTML page.
-   * @param {LH.Artifacts.NetworkRequest|undefined} mainRecord
+   * Expects a network request with all redirects resolved, otherwise the MIME type may be incorrect.
+   * @param {LH.Artifacts.NetworkRequest|undefined} finalRecord
    * @return {LH.LighthouseError|undefined}
    */
-  static getNonHtmlError(mainRecord) {
+  static getNonHtmlError(finalRecord) {
     // MIME types are case-insenstive but Chrome normalizes MIME types to be lowercase.
     const HTML_MIME_TYPE = 'text/html';
 
     // If we never requested a document, there's no doctype error, let other cases handle it.
-    if (!mainRecord) return undefined;
+    if (!finalRecord) return undefined;
 
     // mimeType is determined by the browser, we assume Chrome is determining mimeType correctly,
     // independently of 'Content-Type' response headers, and always sending mimeType if well-formed.
-    if (HTML_MIME_TYPE !== mainRecord.mimeType) {
-      return new LHError(LHError.errors.NOT_HTML, {mimeType: mainRecord.mimeType});
+    if (HTML_MIME_TYPE !== finalRecord.mimeType) {
+      return new LHError(LHError.errors.NOT_HTML, {mimeType: finalRecord.mimeType});
     }
     return undefined;
   }
@@ -259,9 +260,12 @@ class GatherRunner {
       mainRecord = NetworkAnalyzer.findMainDocument(networkRecords, passContext.url);
     } catch (_) {}
 
+    // MIME Type is only set on the final redirected document request. Use this for the HTML check instead of root.
+    const finalRecord = NetworkAnalyzer.resolveRedirects(mainRecord);
+
     const networkError = GatherRunner.getNetworkError(mainRecord);
     const interstitialError = GatherRunner.getInterstitialError(mainRecord, networkRecords);
-    const nonHtmlError = GatherRunner.getNonHtmlError(mainRecord);
+    const nonHtmlError = GatherRunner.getNonHtmlError(finalRecord);
 
     // Check to see if we need to ignore the page load failure.
     // e.g. When the driver is offline, the load will fail without page offline support.

lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js

@@ -452,6 +452,19 @@ class NetworkAnalyzer {
     // The main document is the earliest document request, using position in networkRecords array to break ties.
     return documentRequests.reduce((min, r) => (r.startTime < min.startTime ? r : min));
   }
+
+  /**
+   * Resolves redirect chain given a main document.
+   * See: {@link NetworkAnalyzer.findMainDocument}) for how to retrieve main document.
+   *
+   * @param {LH.Artifacts.NetworkRequest|undefined} request
+   * @returns {LH.Artifacts.NetworkRequest|undefined}
+   */
+  static resolveRedirects(request) {
+    if (!request) return undefined;
+    while (request.redirectDestination) request = request.redirectDestination;
+    return request;
+  }
 }
 
 module.exports = NetworkAnalyzer;

lighthouse-core/test/gather/gather-runner-test.js

@@ -1198,6 +1198,24 @@ describe('GatherRunner', function() {
       expect(error).toBeUndefined();
     });
 
+    it('passes when the page redirects to MIME type text/html', () => {
+      const passContext = {
+        url: 'http://the-page.com',
+        passConfig: {loadFailureMode: LoadFailureMode.fatal},
+      };
+      const mainRecord = new NetworkRequest();
+      const finalRecord = new NetworkRequest();
+      const loadData = {networkRecords: [mainRecord]};
+
+      mainRecord.url = passContext.url;
+      mainRecord.redirectDestination = finalRecord;
+      finalRecord.url = 'http://the-redirected-page.com';
+      finalRecord.mimeType = 'text/html';
+
+      const error = GatherRunner.getPageLoadError(passContext, loadData, undefined);
+      expect(error).toBeUndefined();
+    });
+
     it('fails with interstitial error first', () => {
       const passContext = {
         url: 'http://the-page.com',
@@ -1275,6 +1293,23 @@ describe('GatherRunner', function() {
       const error = getAndExpectError(passContext, loadData, navigationError);
       expect(error.message).toEqual('NAVIGATION_ERROR');
     });
+
+    it('fails with non-HTML when redirect is not HTML', () => {
+      const passContext = {
+        url: 'http://the-page.com',
+        passConfig: {loadFailureMode: LoadFailureMode.fatal},
+      };
+      const mainRecord = new NetworkRequest();
+      const finalRecord = new NetworkRequest();
+      const loadData = {networkRecords: [mainRecord]};
+
+      mainRecord.url = passContext.url;
+      mainRecord.redirectDestination = finalRecord;
+      finalRecord.url = 'http://the-redirected-page.com';
+
+      const error = getAndExpectError(passContext, loadData, navigationError);
+      expect(error.message).toEqual('NOT_HTML');
+    });
   });
 
   describe('artifact collection', () => {

lighthouse-core/test/lib/dependency-graph/simulator/network-analyzer-test.js

@@ -10,6 +10,9 @@ const assert = require('assert').strict;
 const NetworkAnalyzer = require('../../../../lib/dependency-graph/simulator/network-analyzer.js');
 const NetworkRecords = require('../../../../computed/network-records.js');
 const devtoolsLog = require('../../../fixtures/traces/progressive-app-m60.devtools.log.json');
+const devtoolsLogWithRedirect = require(
+  '../../../fixtures/traces/site-with-redirect.devtools.log.json'
+);
 
 /* eslint-env jest */
 describe('DependencyGraph/Simulator/NetworkAnalyzer', () => {
@@ -385,4 +388,30 @@ describe('DependencyGraph/Simulator/NetworkAnalyzer', () => {
       assert.equal(mainDocument.url, 'https://www.example.com');
     });
   });
+
+  describe('#resolveRedirects', () => {
+    it('should resolve to the same document when no redirect', async () => {
+      const records = await NetworkRecords.request(devtoolsLog, {computedCache: new Map()});
+
+      const mainDocument = NetworkAnalyzer.findMainDocument(records, 'https://pwa.rocks/');
+      const finalDocument = NetworkAnalyzer.resolveRedirects(mainDocument);
+      assert.equal(mainDocument.url, finalDocument.url);
+      assert.equal(finalDocument.url, 'https://pwa.rocks/');
+    });
+
+    it('should resolve to the final document with redirects', async () => {
+      const records = await NetworkRecords.request(devtoolsLogWithRedirect, {
+        computedCache: new Map(),
+      });
+
+      const mainDocument = NetworkAnalyzer.findMainDocument(records, 'http://www.vkontakte.ru/');
+      const finalDocument = NetworkAnalyzer.resolveRedirects(mainDocument);
+      assert.notEqual(mainDocument.url, finalDocument.url);
+      assert.equal(finalDocument.url, 'https://m.vk.com/');
+    });
+
+    it('should return undefined if not given a request', () => {
+      assert.equal(NetworkAnalyzer.resolveRedirects(), undefined);
+    });
+  });
 });