Review changes

GoogleChrome · Nov 27, 2017 · de21178 · de21178
1 parent e40de79
commit de21178
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 49 deletions.
diff --git a/lighthouse-core/audits/webfonts.js b/lighthouse-core/audits/webfonts.js
@@ -9,6 +9,7 @@ const Audit = require('./audit');
 const Util = require('../report/v2/renderer/util');
 const WebInspector = require('../lib/web-inspector');
 const UnusedBytes = require('./byte-efficiency/byte-efficiency-audit');
+const allowedFontFaceDisplays = ['optional', 'swap', 'fallback'];
 
 class WebFonts extends Audit {
   /**
@@ -37,18 +38,18 @@ class WebFonts extends Audit {
 
     // Filter font-faces that do not have a display tag with optional or swap
     const fontsWithoutProperDisplay = fontFaces.filter(fontFace =>
-      !fontFace.display || !['optional', 'swap'].includes(fontFace.display)
+      !fontFace.display || !allowedFontFaceDisplays.includes(fontFace.display)
     );
 
 
     return Promise.all([traceOfTabPromise, networkPromise]).then(([tabTrace, networkRecords]) => {
       let totalWasted = 0;
-      const fcpInMS = tabTrace.timestamps.firstContentfulPaint / 1000000;
+      const fcpInMS = tabTrace.timestamps.firstContentfulPaint / 1000;
       const results = networkRecords.filter(record => {
         const isFont = record._resourceType === WebInspector.resourceTypes.Font;
-        const isLoadedBeforeFCP = record._endTime < fcpInMS || true;
+        //const isLoadedBeforeFCP = record._endTime * 1000 < fcpInMS;
 
-        return isFont && isLoadedBeforeFCP;
+        return isFont;// && isLoadedBeforeFCP;
       })
         .filter(fontRecord => {
           // find the fontRecord of a font

diff --git a/lighthouse-core/gather/gatherers/fonts.js b/lighthouse-core/gather/gatherers/fonts.js
@@ -18,36 +18,46 @@ function getAllLoadedFonts() {
     weight: fontFace.weight,
   });
 
-  if (document.fonts.status === 'loaded') {
-    return Promise.resolve(
-      Array.from(document.fonts).filter(fontFace => fontFace.status === 'loaded')
-        .map(getFont)
-    );
-  } else {
-    return document.fonts.ready.then(() => {
-      return Array.from(document.fonts).filter(fontFace => fontFace.status === 'loaded')
-        .map(getFont);
-    });
-  }
+  return document.fonts.ready.then(() => {
+    return Array.from(document.fonts).filter(fontFace => fontFace.status === 'loaded')
+      .map(getFont);
+  });
 }
+
 function getFontFaceFromStylesheets() {
+  function resolveUrl(url) {
+    const link = document.createElement('a');
+    link.href = url;
+
+    return link.href;
+  }
+
+  const fontUrlRegex = new RegExp('url\\((?:"|\')([^"]+)(?:"|\')\\)');
   const fontFaceRules = [];
+  // get all loaded stylesheets
   for (let sheet = 0; sheet < document.styleSheets.length; sheet++) {
-    for (var i = 0; document.styleSheets[sheet].cssRules && i < document.styleSheets[sheet].cssRules.length; i++) {
-      var rule = document.styleSheets[sheet].cssRules[i];
+    const stylesheet = document.styleSheets[sheet];
+    for (let i = 0; stylesheet.cssRules && i < stylesheet.cssRules.length; i++) {
+      var rule = stylesheet.cssRules[i];
 
       if (rule instanceof CSSFontFaceRule) {
-        const keys = Object.keys(rule.style);
-        const fontObject = keys.reduce((fontRules, propName) => {
-          if (!isNaN(propName)) {
-            const cssKey = rule.style[keys[propName]];
-            fontRules[cssKey.replace('font-', '')] = rule.style[cssKey];
-          }
+        const fontsObject = {
+          display: rule.style.fontDisplay || 'auto',
+          family: rule.style.fontFamily.replace(/"|'/g, ''),
+          stretch: rule.style.fontStretch || 'normal',
+          style: rule.style.fontStyle || 'normal',
+          weight: rule.style.fontWeight || 'normal',
+          src: [],
+        }
 
-          return fontRules;
-        }, {});
+        if (rule.style.src) {
+          const matches = rule.style.src.match(fontUrlRegex);
+          if (matches) {
+            fontsObject.src.push(resolveUrl(matches[1]));
+          }
+        }
 
-        fontFaceRules.push(fontObject);
+        fontFaceRules.push(fontsObject);
       }
     }
   }
@@ -63,16 +73,6 @@ class Fonts extends Gatherer {
     this.stylesheetIds = [];
   }
 
-  _onStyleSheetAdded({ header }) {
-    this.stylesheetIds.push(header.styleSheetId);
-    console.log('added', header.styleSheetId);
-  }
-
-  _onStyleSheetRemoved({ stylesheetId }) {
-    this.stylesheetIds.splice(this.stylesheetIds.indexOf(stylesheetId), 1);
-    console.log('removed', stylesheetId);
-  }
-
   _findSameFontFamily(fontFace, fontFacesList) {
     return fontFacesList.find(fontItem => {
       return fontFace.family === fontItem.family &&
@@ -90,19 +90,7 @@ class Fonts extends Gatherer {
     ).then(([loadedFonts, fontFaces ]) => {
       return loadedFonts.map(fontFace => {
         const fontFaceItem = this._findSameFontFamily(fontFace, fontFaces);
-
-        // if font-face hase an url
-        if (fontFaceItem && fontFaceItem.src) {
-          fontFace.src = [];
-
-          // Fetch font urls and add them to the font-face
-          fontFaceItem.src.split(',').forEach(src => {
-            const matches = src.match(fontUrlRegex);
-            if (matches) {
-              fontFace.src.push(matches[1]);
-            }
-          });
-        }
+        fontFace.src = fontFaceItem.src || [];
 
         return fontFace;
       });