Merge branch 'master' into missing-fcp

GoogleChrome · Feb 4, 2017 · 2e33372 · 2e33372
2 parents 36eb532 + a9e431d
commit 2e33372
Show file tree

Hide file tree

Showing 39 changed files with 127 additions and 480 deletions.
diff --git a/lighthouse-core/audits/accessibility/axe-audit.js b/lighthouse-core/audits/accessibility/axe-audit.js
@@ -32,7 +32,7 @@ class AxeAudit extends Audit {
    * @return {!AuditResult}
    */
   static audit(artifacts) {
-    const violations = artifacts.Accessibility.violations || [];
+    const violations = artifacts.Accessibility.violations;
     const rule = violations.find(result => result.id === this.meta.name);
 
     return this.generateAuditResult({

diff --git a/lighthouse-core/audits/content-width.js b/lighthouse-core/audits/content-width.js
@@ -42,7 +42,7 @@ class ContentWidth extends Audit {
   static audit(artifacts) {
     const scrollWidth = artifacts.ContentWidth.scrollWidth;
     const viewportWidth = artifacts.ContentWidth.viewportWidth;
-    const widthsMatch = scrollWidth === viewportWidth && scrollWidth !== -1;
+    const widthsMatch = scrollWidth === viewportWidth;
 
     return ContentWidth.generateAuditResult({
       rawValue: widthsMatch,

diff --git a/lighthouse-core/audits/deprecations.js b/lighthouse-core/audits/deprecations.js
@@ -45,10 +45,6 @@ class Deprecations extends Audit {
    * @return {!AuditResult}
    */
   static audit(artifacts) {
-    if (artifacts.ChromeConsoleMessages.rawValue === -1) {
-      return Deprecations.generateAuditResult(artifacts.ChromeConsoleMessages);
-    }
-
     const entries = artifacts.ChromeConsoleMessages;
 
     const deprecations = entries.filter(log => log.entry.source === 'deprecation')

diff --git a/lighthouse-core/audits/service-worker.js b/lighthouse-core/audits/service-worker.js
@@ -51,14 +51,6 @@ class ServiceWorker extends Audit {
    * @return {!AuditResult}
    */
   static audit(artifacts) {
-    if (!artifacts.ServiceWorker.versions) {
-      // Error in ServiceWorker gatherer.
-      return ServiceWorker.generateAuditResult({
-        rawValue: false,
-        debugString: artifacts.ServiceWorker.debugString
-      });
-    }
-
     // Find active service worker for this URL. Match against
     // artifacts.URL.finalUrl so audit accounts for any redirects.
     const version = getActivatedServiceWorker(

diff --git a/lighthouse-core/audits/without-javascript.js b/lighthouse-core/audits/without-javascript.js
@@ -41,13 +41,6 @@ class WithoutJavaScript extends Audit {
    */
   static audit(artifacts) {
     const artifact = artifacts.HTMLWithoutJavaScript;
-    if (typeof artifact.value !== 'string') {
-      return WithoutJavaScript.generateAuditResult({
-        rawValue: -1,
-        debugString: artifact.debugString ||
-            'HTMLWithoutJavaScript gatherer did not complete successfully'
-      });
-    }
 
     if (artifact.value.trim() === '') {
       return WithoutJavaScript.generateAuditResult({

diff --git a/lighthouse-core/gather/computed/trace-of-tab.js b/lighthouse-core/gather/computed/trace-of-tab.js
@@ -60,6 +60,10 @@ class TraceOfTab extends ComputedArtifact {
     const navigationStart = frameEvents.filter(e =>
         e.name === 'navigationStart' && e.ts < firstPaint.ts).pop();
 
+    // FCP will follow at/after the FP
+    const firstContentfulPaint = frameEvents.find(e =>
+      e.name === 'firstContentfulPaint' && e.ts >= firstPaint.ts);
+
     // fMP will follow at/after the FP
     let firstMeaningfulPaint = frameEvents.find(e =>
         e.name === 'firstMeaningfulPaint' && e.ts >= firstPaint.ts);
@@ -73,13 +77,10 @@ class TraceOfTab extends ComputedArtifact {
       log.verbose('trace-of-tab', `No firstMeaningfulPaint found, falling back to last ${fmpCand}`);
       const lastCandidate = frameEvents.filter(e => e.name === fmpCand).pop();
       if (!lastCandidate) {
-        log.verbose('No `firstMeaningfulPaintCandidate` events found in trace');
+        log.verbose('trace-of-tab', 'No `firstMeaningfulPaintCandidate` events found in trace');
       }
       firstMeaningfulPaint = lastCandidate;
     }
-    // Grab any FCP after the FP
-    const firstContentfulPaint = frameEvents.find(e =>
-      e.name === 'firstContentfulPaint' && e.ts >= firstPaint.ts);
 
     // subset all trace events to just our tab's process (incl threads other than main)
     const processEvents = trace.traceEvents.filter(e => {

diff --git a/lighthouse-core/gather/gatherers/accessibility.js b/lighthouse-core/gather/gatherers/accessibility.js
@@ -47,14 +47,10 @@ function runA11yChecks() {
 }
 
 class Accessibility extends Gatherer {
-  static _errorAccessibility(errorString) {
-    return {
-      raw: undefined,
-      value: undefined,
-      debugString: errorString
-    };
-  }
-
+  /**
+   * @param {!Object} options
+   * @return {!Promise<{violations: !Array}>}
+   */
   afterPass(options) {
     const driver = options.driver;
     const expression = `(function () {
@@ -63,20 +59,14 @@ class Accessibility extends Gatherer {
     })()`;
 
     return driver
-        .evaluateAsync(expression)
-        .then(returnedValue => {
-          if (!returnedValue) {
-            return Accessibility._errorAccessibility('Unable to parse axe results');
-          }
-
-          if (returnedValue.error) {
-            return Accessibility._errorAccessibility(returnedValue.error);
-          }
+      .evaluateAsync(expression)
+      .then(returnedValue => {
+        if (!returnedValue || !Array.isArray(returnedValue.violations)) {
+          throw new Error('Unable to parse axe results' + returnedValue);
+        }
 
-          return returnedValue;
-        }, _ => {
-          return Accessibility._errorAccessibility('Axe results timed out');
-        });
+        return returnedValue;
+      });
   }
 }
 

diff --git a/lighthouse-core/gather/gatherers/chrome-console-messages.js b/lighthouse-core/gather/gatherers/chrome-console-messages.js
@@ -43,13 +43,7 @@ class ChromeConsoleMessages extends Gatherer {
   afterPass(options) {
     options.driver.off('Log.entryAdded', this._onConsoleEntryAdded);
     return options.driver.sendCommand('Log.disable')
-        .then(_ => this._logEntries)
-        .catch(err => {
-          return {
-            rawValue: -1,
-            debugString: err.message
-          };
-        });
+        .then(_ => this._logEntries);
   }
 }
 

diff --git a/lighthouse-core/gather/gatherers/content-width.js b/lighthouse-core/gather/gatherers/content-width.js
@@ -34,6 +34,10 @@ function getContentWidth() {
 
 class ContentWidth extends Gatherer {
 
+  /**
+   * @param {!Object} options
+   * @return {!Promise<{scrollWidth: number, viewportWidth: number, devicePixelRatio: number}>}
+   */
   afterPass(options) {
     const driver = options.driver;
 
@@ -47,12 +51,6 @@ class ContentWidth extends Gatherer {
       }
 
       return returnedValue;
-    }, _ => {
-      return {
-        scrollWidth: -1,
-        viewportWidth: -1,
-        devicePixelRatio: -1,
-      };
     });
   }
 }

diff --git a/lighthouse-core/gather/gatherers/html-without-javascript.js b/lighthouse-core/gather/gatherers/html-without-javascript.js
@@ -44,18 +44,12 @@ class HTMLWithoutJavaScript extends Gatherer {
     return options.driver.evaluateAsync(`(${getBodyText.toString()}())`)
       .then(result => {
         if (typeof result !== 'string') {
-          throw new Error('result was not a string');
+          throw new Error('document body innerText returned by protocol was not a string');
         }
 
         return {
           value: result
         };
-      })
-      .catch(err => {
-        return {
-          value: -1,
-          debugString: `Unable to get document body innerText: ${err.message}`
-        };
       });
   }
 }

diff --git a/lighthouse-core/gather/gatherers/html.js b/lighthouse-core/gather/gatherers/html.js
diff --git a/lighthouse-core/gather/gatherers/http-redirect.js b/lighthouse-core/gather/gatherers/http-redirect.js
@@ -47,22 +47,14 @@ class HTTPRedirect extends Gatherer {
         return {
           value: state.schemeIsCryptographic
         };
-      }, _ => {
-        return {
-          value: false,
-          debugString: 'Error requesting security state'
-        };
       });
 
     let noSecurityChangesTimeout;
     const timeoutPromise = new Promise((resolve, reject) => {
       // Set up a timeout for ten seconds in case we don't get any
       // security events at all. If that happens, bail.
       noSecurityChangesTimeout = setTimeout(_ => {
-        resolve({
-          value: false,
-          debugString: 'Timed out waiting for HTTP redirection.'
-        });
+        reject(new Error('Timed out waiting for HTTP redirection.'));
       }, timeout);
     });
 
@@ -73,6 +65,9 @@ class HTTPRedirect extends Gatherer {
       // Clear timeout. No effect if it won, no need to wait if it lost.
       clearTimeout(noSecurityChangesTimeout);
       return result;
+    }).catch(err => {
+      clearTimeout(noSecurityChangesTimeout);
+      throw err;
     });
   }
 }

diff --git a/lighthouse-core/gather/gatherers/https.js b/lighthouse-core/gather/gatherers/https.js
@@ -39,22 +39,14 @@ class HTTPS extends Gatherer {
         return {
           value: state.schemeIsCryptographic
         };
-      }, _ => {
-        return {
-          value: false,
-          debugString: 'Error requesting page security state.'
-        };
       });
 
     let noSecurityChangesTimeout;
     const timeoutPromise = new Promise((resolve, reject) => {
       // Set up a timeout for ten seconds in case we don't get any
       // security events at all. If that happens, bail.
       noSecurityChangesTimeout = setTimeout(_ => {
-        resolve({
-          value: false,
-          debugString: 'Timed out waiting for page security state.'
-        });
+        reject(new Error('Timed out waiting for page security state.'));
       }, timeout);
     });
 
@@ -65,6 +57,9 @@ class HTTPS extends Gatherer {
       // Clear timeout. No effect if it won, no need to wait if it lost.
       clearTimeout(noSecurityChangesTimeout);
       return result;
+    }).catch(err => {
+      clearTimeout(noSecurityChangesTimeout);
+      throw err;
     });
   }
 }

diff --git a/lighthouse-core/gather/gatherers/service-worker.js b/lighthouse-core/gather/gatherers/service-worker.js
@@ -27,11 +27,6 @@ class ServiceWorker extends Gatherer {
         return {
           versions: data.versions
         };
-      })
-      .catch(err => {
-        return {
-          debugString: `Error in querying Service Worker status: ${err.message}`
-        };
       });
   }
 }

diff --git a/lighthouse-core/test/audits/accessibility/aria-allowed-attr-test.js b/lighthouse-core/test/audits/accessibility/aria-allowed-attr-test.js
@@ -53,15 +53,4 @@ describe('Accessibility: aria-allowed-attr audit', () => {
     assert.equal(output.rawValue, false);
     assert.equal(output.displayValue, '');
   });
-
-  it('doesn\'t throw an error when violations is undefined', () => {
-    const artifacts = {
-      Accessibility: {
-        violations: undefined
-      }
-    };
-
-    const output = Audit.audit(artifacts);
-    assert.equal(output.description, 'Element aria-* attributes are allowed for this role');
-  });
 });
diff --git a/lighthouse-core/test/audits/accessibility/aria-required-attr-test.js b/lighthouse-core/test/audits/accessibility/aria-required-attr-test.js
@@ -53,16 +53,4 @@ describe('Accessibility: aria-required-attr audit', () => {
     assert.equal(output.rawValue, false);
     assert.equal(output.displayValue, '');
   });
-
-  it('doesn\'t throw an error when violations is undefined', () => {
-    const artifacts = {
-      Accessibility: {
-        violations: undefined
-      }
-    };
-
-    const output = Audit.audit(artifacts);
-    assert.equal(output.description,
-        'Elements with ARIA roles have the required aria-* attributes');
-  });
 });
diff --git a/lighthouse-core/test/audits/accessibility/aria-valid-attr-test.js b/lighthouse-core/test/audits/accessibility/aria-valid-attr-test.js
@@ -53,16 +53,4 @@ describe('Accessibility: aria-valid-attr audit', () => {
     assert.equal(output.rawValue, false);
     assert.equal(output.displayValue, '');
   });
-
-  it('doesn\'t throw an error when violations is undefined', () => {
-    const artifacts = {
-      Accessibility: {
-        violations: undefined
-      }
-    };
-
-    const output = Audit.audit(artifacts);
-    assert.equal(output.description,
-        'Element aria-* attributes are valid and not misspelled or non-existent.');
-  });
 });
diff --git a/lighthouse-core/test/audits/accessibility/aria-valid-attr-value-test.js b/lighthouse-core/test/audits/accessibility/aria-valid-attr-value-test.js
@@ -53,15 +53,4 @@ describe('Accessibility: aria-valid-attr-value audit', () => {
     assert.equal(output.rawValue, false);
     assert.equal(output.displayValue, '');
   });
-
-  it('doesn\'t throw an error when violations is undefined', () => {
-    const artifacts = {
-      Accessibility: {
-        violations: undefined
-      }
-    };
-
-    const output = Audit.audit(artifacts);
-    assert.equal(output.description, 'Element aria-* attributes have valid values');
-  });
 });
diff --git a/lighthouse-core/test/audits/accessibility/color-contrast-test.js b/lighthouse-core/test/audits/accessibility/color-contrast-test.js
@@ -53,16 +53,4 @@ describe('Accessibility: color-contrast audit', () => {
     assert.equal(output.rawValue, false);
     assert.equal(output.displayValue, '');
   });
-
-  it('doesn\'t throw an error when violations is undefined', () => {
-    const artifacts = {
-      Accessibility: {
-        violations: undefined
-      }
-    };
-
-    const output = Audit.audit(artifacts);
-    assert.equal(output.description,
-        'Background and foreground colors have a sufficient contrast ratio');
-  });
 });