core(fonts): handle CORS cssRules (#5592)

GoogleChrome · Jul 2, 2018 · 7b19e91 · 7b19e91
1 parent 5539ce1
commit 7b19e91
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 15 deletions.
diff --git a/lighthouse-cli/test/fixtures/perf/cors-fonts.css b/lighthouse-cli/test/fixtures/perf/cors-fonts.css
@@ -0,0 +1,10 @@
+@font-face {
+  font-family: 'Lobster Three';
+  font-style: normal;
+  font-weight: 700;
+  src: url("http://localhost:10503/perf/lobster-two-v10-latin-700.woff2?cors=true") format('woff2');
+}
+.corsfont {
+  font-family: "Lobster Three", sans-serif;
+  color: red;
+}
diff --git a/lighthouse-cli/test/fixtures/perf/fonts.html b/lighthouse-cli/test/fixtures/perf/fonts.html
@@ -6,13 +6,15 @@
         font-family: 'Lobster';
         font-style: normal;
         font-weight: 400;
-        src: local('Lobster'), url('./lobster-v20-latin-regular.eot?#iefix') format('eot'), url('./lobster-v20-latin-regular.woff2') format('woff2');
+        /* We don't need `local` but keep around for testing robustness of our regex */
+        src: local('Lobster'), url('./lobster-v20-latin-regular.woff2') format('woff2');
       }
       @font-face {
         font-family: 'Lobster Two';
         font-style: normal;
         font-weight: 700;
         font-display: optional;
+        /* We don't need `local` but keep around for testing robustness of our regex */
         src: local("Lobster Two"), url("./lobster-two-v10-latin-700.woff2?delay=4000") format('woff2');
       }
       .webfont {
@@ -25,10 +27,12 @@
         font-family: Unknown, sans-serif;
       }
     </style>
+    <link rel="stylesheet" href="http://localhost:10503/perf/cors-fonts.css">
   </head>
   <body>
     <p class="webfont">Let's load some sweet webfonts...</p>
     <p><strong class="webfont">Let's load some sweet webfonts...</strong></p>
-    <p class"nofont">Some lovely text that uses the fallback font</p>
+    <p class="nofont">Some lovely text that uses the fallback font</p>
+    <p class="corsfont">Some lovely text that uses a CORS font</p>
   </body>
 </html>
diff --git a/lighthouse-cli/test/fixtures/static-server.js b/lighthouse-cli/test/fixtures/static-server.js
@@ -60,7 +60,7 @@ function requestHandler(request, response) {
   }
 
   function sendResponse(statusCode, data) {
-    const headers = {};
+    const headers = {'Access-Control-Allow-Origin': '*'};
 
     if (filePath.endsWith('.js')) {
       headers['Content-Type'] = 'text/javascript';

diff --git a/lighthouse-cli/test/smokehouse/perf/expectations.js b/lighthouse-cli/test/smokehouse/perf/expectations.js
@@ -64,7 +64,7 @@ module.exports = [
         rawValue: false,
         details: {
           items: {
-            length: 1,
+            length: 2,
           },
         },
       },

diff --git a/lighthouse-core/gather/gatherers/fonts.js b/lighthouse-core/gather/gatherers/fonts.js
@@ -13,7 +13,7 @@ const Sentry = require('../../lib/sentry');
 // All the property keys of FontFace where the value is a string and are worth
 // using for finding font matches (see _findSameFontFamily).
 /** @typedef {'family'|'style'|'weight'|'stretch'|'unicodeRange'|'variant'|'featureSettings'|'display'} FontFaceStringKeys */
-/** @typedef {{err: {message: string, stack: string}}} FontGatherError */
+/** @typedef {{err: {message: string, stack?: string}}} FontGatherError */
 
 /** @type {Array<FontFaceStringKeys>} */
 const fontFaceDescriptors = [
@@ -124,11 +124,24 @@ function getFontFaceFromStylesheets() {
     return new Promise(resolve => {
       newNode.addEventListener('load', function onload() {
         newNode.removeEventListener('load', onload);
-        resolve(getFontFaceFromStylesheets());
+        try {
+          const stylesheet = Array.from(document.styleSheets).find(s => s.ownerNode === newNode);
+          if (stylesheet) {
+            const cssStylesheet = /** @type {CSSStyleSheet} */ (stylesheet);
+            resolve(getSheetsFontFaces(cssStylesheet));
+          } else {
+            resolve([{err: {message: 'Could not load stylesheet with CORS'}}]);
+          }
+        } catch (err) {
+          resolve([{err: {message: err.message, stack: err.stack}}]);
+        }
       });
       newNode.crossOrigin = 'anonymous';
       oldNode.parentNode && oldNode.parentNode.insertBefore(newNode, oldNode);
       oldNode.remove();
+
+      // Give each stylesheet 1s to load before giving up
+      setTimeout(() => resolve([{err: {message: 'Could not load stylesheet (timeout)'}}]), 1000);
     });
   }
 
@@ -138,19 +151,28 @@ function getFontFaceFromStylesheets() {
   const corsDataPromises = [];
   // Get all loaded stylesheets
   for (const stylesheet of Array.from(document.styleSheets)) {
+    const cssStylesheet = /** @type {CSSStyleSheet} */ (stylesheet);
+
     try {
-      const cssStylesheet = /** @type {CSSStyleSheet} */ (stylesheet);
-      // Cross-origin stylesheets don't expose cssRules by default. We reload them w/ CORS headers.
-      if (cssStylesheet.cssRules === null && cssStylesheet.href && cssStylesheet.ownerNode &&
-        // @ts-ignore - crossOrigin exists if ownerNode is an HTMLLinkElement
-        !cssStylesheet.ownerNode.crossOrigin) {
+      // cssRules can be null or this access can throw when CORS isn't enabled; throw a matching error message.
+      if (!cssStylesheet.cssRules) {
+        throw new Error('Failed to read cssRules');
+      }
+
+      data.push(...getSheetsFontFaces(cssStylesheet));
+    } catch (err) {
+      const failedToReadRules = /Failed to read.*cssRules/.test(err.message);
+      // @ts-ignore - crossOrigin exists if ownerNode is an HTMLLinkElement
+      const alreadyCORS = !cssStylesheet.ownerNode || !!cssStylesheet.ownerNode.crossOrigin;
+
+      if (failedToReadRules && !alreadyCORS && cssStylesheet.href) {
+        // Cross-origin stylesheets don't expose cssRules by default. We reload them w/ CORS headers.
         const ownerLinkEl = /** @type {HTMLLinkElement} */ (cssStylesheet.ownerNode);
         corsDataPromises.push(loadStylesheetWithCORS(ownerLinkEl));
       } else {
-        data.push(...getSheetsFontFaces(cssStylesheet));
+        // Otherwise this is a legit error we should report back to the gatherer.
+        data.push({err: {message: err.message, stack: err.stack}});
       }
-    } catch (err) {
-      data.push({err: {message: err.message, stack: err.stack}});
     }
   }
   // Flatten results
@@ -194,7 +216,7 @@ class Fonts extends Gatherer {
           const dataError = /** @type {FontGatherError} */ (fontOrError);
           if (dataError.err) {
             const err = new Error(dataError.err.message);
-            err.stack = dataError.err.stack;
+            err.stack = dataError.err.stack || err.stack;
             // @ts-ignore TODO(bckenny): Sentry type checking
             Sentry.captureException(err, {tags: {gatherer: 'Fonts'}, level: 'warning'});
             return false;

diff --git a/lighthouse-core/lib/sentry.js b/lighthouse-core/lib/sentry.js
@@ -26,6 +26,7 @@ const SAMPLED_ERRORS = [
   {pattern: /(IDLE_PERIOD|FMP_TOO_LATE)/, rate: 0.1},
   {pattern: /^NO_.*/, rate: 0.1},
   // Message based sampling
+  {pattern: /Could not load stylesheet/, rate: 0.1},
   {pattern: /Failed to decode/, rate: 0.1},
   {pattern: /All image optimizations failed/, rate: 0.1},
   {pattern: /No.*resource with given/, rate: 0.01},