From 82f6c36ea2797ec4711a0e073a6fbb3a93bac6e6 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 25 Oct 2018 01:08:18 -0700
Subject: [PATCH 01/16] core(font-size): speed up font-size gatherer (#6386)

---
 lighthouse-core/audits/seo/font-size.js       |  16 +-
 .../gather/gatherers/seo/font-size.js         | 137 +++++++++---------
 .../gather/gatherers/seo/font-size-test.js    |  72 ++++++---
 typings/artifacts.d.ts                        |   1 -
 4 files changed, 118 insertions(+), 108 deletions(-)

diff --git a/lighthouse-core/audits/seo/font-size.js b/lighthouse-core/audits/seo/font-size.js
index 9a08eea1e74a..48a78ad097ad 100644
--- a/lighthouse-core/audits/seo/font-size.js
+++ b/lighthouse-core/audits/seo/font-size.js
@@ -208,7 +208,6 @@ class FontSize extends Audit {
       analyzedFailingNodesData,
       analyzedFailingTextLength,
       failingTextLength,
-      visitedTextLength,
       totalTextLength,
     } = artifacts.FontSize;
 
@@ -220,7 +219,7 @@ class FontSize extends Audit {
 
     const failingRules = getUniqueFailingRules(analyzedFailingNodesData);
     const percentageOfPassingText =
-      (visitedTextLength - failingTextLength) / visitedTextLength * 100;
+      (totalTextLength - failingTextLength) / totalTextLength * 100;
     const pageUrl = artifacts.URL.finalUrl;
 
     const headings = [
@@ -232,7 +231,7 @@ class FontSize extends Audit {
 
     const tableData = failingRules.sort((a, b) => b.textLength - a.textLength)
       .map(({cssRule, textLength, fontSize, node}) => {
-        const percentageOfAffectedText = textLength / visitedTextLength * 100;
+        const percentageOfAffectedText = textLength / totalTextLength * 100;
         const origin = findStyleRuleSource(pageUrl, cssRule, node);
 
         return {
@@ -246,7 +245,7 @@ class FontSize extends Audit {
     // all failing nodes that were not fully analyzed will be displayed in a single row
     if (analyzedFailingTextLength < failingTextLength) {
       const percentageOfUnanalyzedFailingText =
-        (failingTextLength - analyzedFailingTextLength) / visitedTextLength * 100;
+        (failingTextLength - analyzedFailingTextLength) / totalTextLength * 100;
 
       tableData.push({
         source: 'Add\'l illegible text',
@@ -273,14 +272,7 @@ class FontSize extends Audit {
     let explanation;
     if (!passed) {
       const percentageOfFailingText = parseFloat((100 - percentageOfPassingText).toFixed(2));
-      let disclaimer = '';
-
-      // if we were unable to visit all text nodes we should disclose that information
-      if (visitedTextLength < totalTextLength) {
-        const percentageOfVisitedText = visitedTextLength / totalTextLength * 100;
-        disclaimer = ` (based on ${percentageOfVisitedText.toFixed()}% sample)`;
-      }
-
+      const disclaimer = '';
       explanation = `${percentageOfFailingText}% of text is too small${disclaimer}.`;
     }
 
diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 5fe2bdeba18a..7908083f9224 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -17,12 +17,10 @@
  */
 
 const Gatherer = require('../gatherer');
-const Sentry = require('../../../lib/sentry.js');
 const FONT_SIZE_PROPERTY_NAME = 'font-size';
 const TEXT_NODE_BLOCK_LIST = new Set(['SCRIPT', 'STYLE', 'NOSCRIPT']);
 const MINIMAL_LEGIBLE_FONT_SIZE_PX = 12;
 // limit number of protocol calls to make sure that gatherer doesn't take more than 1-2s
-const MAX_NODES_VISITED = 500; // number of nodes to get the text length and compute font-size
 const MAX_NODES_SOURCE_RULE_FETCHED = 50; // number of nodes to fetch the source font-size rule
 
 /** DevTools uses a numeric enum for nodeType */
@@ -178,7 +176,7 @@ function getEffectiveFontRule({inlineStyle, matchedCSSRules, inherited}) {
 }
 
 /**
- * @param {LH.Crdp.DOM.Node} node
+ * @param {{nodeValue: string}} node
  * @returns {number}
  */
 function getNodeTextLength(node) {
@@ -209,32 +207,7 @@ async function fetchSourceRule(driver, node) {
 }
 
 /**
- * @param {Driver} driver
- * @param {LH.Artifacts.FontSize.DomNodeWithParent} textNode text node
- * @returns {Promise<?NodeFontData>}
- */
-async function fetchComputedFontSize(driver, textNode) {
-  try {
-    const {computedStyle} = await driver.sendCommand('CSS.getComputedStyleForNode', {
-      nodeId: textNode.parentId,
-    });
-
-    const fontSizeProperty = computedStyle.find(({name}) => name === FONT_SIZE_PROPERTY_NAME);
-
-    return {
-      // @ts-ignore - font size property guaranteed to be returned in getComputedStyle
-      fontSize: parseInt(fontSizeProperty.value, 10),
-      textLength: getNodeTextLength(textNode),
-      node: textNode.parentNode,
-    };
-  } catch (err) {
-    Sentry.captureException(err, {tags: {gatherer: 'FontSize'}});
-    return null;
-  }
-}
-
-/**
- * @param {LH.Artifacts.FontSize.DomNodeWithParent} node
+ * @param {{nodeType: number, nodeValue: string, parentNode: {nodeName: string}}} node
  * @returns {boolean}
  */
 function isNonEmptyTextNode(node) {
@@ -246,41 +219,6 @@ function isNonEmptyTextNode(node) {
 }
 
 class FontSize extends Gatherer {
-  /**
-   * @param {LH.Gatherer.PassContext['driver']} driver
-   */
-  static async fetchNodesToAnalyze(driver) {
-    let failingTextLength = 0;
-    let visitedTextLength = 0;
-    let totalTextLength = 0;
-
-    const nodes = await getAllNodesFromBody(driver);
-
-    const textNodes = nodes.filter(isNonEmptyTextNode);
-    totalTextLength = textNodes.reduce((sum, node) => (sum += getNodeTextLength(node)), 0);
-
-    const nodesToVisit = textNodes
-      .sort((a, b) => getNodeTextLength(b) - getNodeTextLength(a))
-      .slice(0, MAX_NODES_VISITED);
-
-    const nodePromises = nodesToVisit.map(node => fetchComputedFontSize(driver, node));
-    const visitedNodes = await Promise.all(nodePromises);
-
-    /** @type {Array<NodeFontData>} */
-    const failingNodes = [];
-    for (const visitedNode of visitedNodes) {
-      if (!visitedNode) continue;
-      visitedTextLength += visitedNode.textLength;
-
-      if (visitedNode.fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
-        failingNodes.push(visitedNode);
-        failingTextLength += visitedNode.textLength;
-      }
-    }
-
-    return {totalTextLength, visitedTextLength, failingTextLength, failingNodes};
-  }
-
   /**
    * @param {LH.Gatherer.PassContext['driver']} driver
    * @param {Array<NodeFontData>} failingNodes
@@ -316,16 +254,73 @@ class FontSize extends Gatherer {
     passContext.driver.on('CSS.styleSheetAdded', onStylesheetAdd);
 
     await Promise.all([
+      passContext.driver.sendCommand('DOMSnapshot.enable'),
       passContext.driver.sendCommand('DOM.enable'),
       passContext.driver.sendCommand('CSS.enable'),
     ]);
 
-    const {
-      totalTextLength,
-      visitedTextLength,
-      failingTextLength,
-      failingNodes,
-    } = await FontSize.fetchNodesToAnalyze(passContext.driver);
+    const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
+      computedStyles: ['font-size'],
+    });
+    /** @param {number} index */
+    const lookup = (index) => snapshot.strings[index];
+    // TODO: this needs to use frameId
+    const doc = snapshot.documents.find(doc => lookup(doc.documentURL) === passContext.url);
+
+    if (!doc || !doc.nodes.nodeType || !doc.nodes.nodeName || !doc.nodes.backendNodeId
+      || !doc.nodes.nodeValue || !doc.nodes.parentIndex) {
+      throw new Error('Unexpected response from DOMSnapshot.captureSnapshot.');
+    }
+
+    /** @type {Map<number, number>} */
+    const nodeIndexToStyleIndex = new Map();
+    for (let i = 0; i < doc.layout.nodeIndex.length; i++) {
+      nodeIndexToStyleIndex.set(doc.layout.nodeIndex[i], i);
+    }
+
+    /** @type {Map<number, number>} */
+    const failingBackendIdsToFontSize = new Map();
+    for (let i = 0; i < doc.nodes.nodeType.length; i++) {
+      const nodeType = doc.nodes.nodeType[i];
+      const nodeValue = lookup(doc.nodes.nodeValue[i]);
+      if (!isNonEmptyTextNode({
+        nodeType,
+        nodeValue,
+        parentNode: {
+          nodeName: lookup(doc.nodes.nodeName[doc.nodes.parentIndex[i]]),
+        },
+      })) continue;
+
+      const styleIndex = nodeIndexToStyleIndex.get(doc.nodes.parentIndex[i]);
+      if (!styleIndex) continue;
+      const parentStyles = doc.layout.styles[styleIndex];
+      const [fontSizeStringId] = parentStyles;
+      const fontSize = parseInt(lookup(fontSizeStringId), 10);
+      if (fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
+        failingBackendIdsToFontSize.set(doc.nodes.backendNodeId[i], fontSize);
+      }
+    }
+
+    const nodes = (await getAllNodesFromBody(passContext.driver))
+      .filter(node => isNonEmptyTextNode(node));
+
+    /** @type {NodeFontData[]} */
+    const failingNodes = [];
+    let totalTextLength = 0;
+    let failingTextLength = 0;
+    for (const node of nodes) {
+      const textLength = getNodeTextLength(node);
+      totalTextLength += textLength;
+      const badFontSize = failingBackendIdsToFontSize.get(node.backendNodeId);
+      if (badFontSize) {
+        failingTextLength += textLength;
+        failingNodes.push({
+          node: node.parentNode,
+          textLength,
+          fontSize: badFontSize,
+        });
+      }
+    }
 
     const {
       analyzedFailingNodesData,
@@ -340,6 +335,7 @@ class FontSize extends Gatherer {
       .forEach(data => (data.cssRule.stylesheet = stylesheets.get(data.cssRule.styleSheetId)));
 
     await Promise.all([
+      passContext.driver.sendCommand('DOMSnapshot.disable'),
       passContext.driver.sendCommand('DOM.disable'),
       passContext.driver.sendCommand('CSS.disable'),
     ]);
@@ -348,7 +344,6 @@ class FontSize extends Gatherer {
       analyzedFailingNodesData,
       analyzedFailingTextLength,
       failingTextLength,
-      visitedTextLength,
       totalTextLength,
     };
   }
diff --git a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
index 4f0b74b2dd31..38e5c8e9d38b 100644
--- a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
+++ b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
@@ -13,34 +13,38 @@ let fontSizeGather;
 const smallText = ' body small text ';
 const bigText = 'body big text';
 const failingText = 'failing text';
-const bodyNode = {nodeId: 3, nodeName: 'BODY', parentId: 1};
-const failingNode = {nodeId: 10, nodeName: 'P', parentId: 3};
+const bodyNode = {nodeId: 3, backendNodeId: 102, nodeName: 'BODY', parentId: 1, fontSize: '10px'};
+const failingNode = {nodeId: 10, backendNodeId: 109, nodeName: 'P', parentId: 3};
 const nodes = [
-  {nodeId: 1, nodeName: 'HTML'},
-  {nodeId: 2, nodeName: 'HEAD', parentId: 1},
+  {nodeId: 1, backendNodeId: 100, nodeName: 'HTML'},
+  {nodeId: 2, backendNodeId: 101, nodeName: 'HEAD', parentId: 1},
   bodyNode,
   {
     nodeId: 4,
+    backendNodeId: 103,
     nodeValue: 'head text',
     nodeType: FontSizeGather.TEXT_NODE_TYPE,
     parentId: 2,
   },
   {
     nodeId: 5,
+    backendNodeId: 104,
     nodeValue: smallText,
     nodeType: FontSizeGather.TEXT_NODE_TYPE,
     parentId: 3,
   },
-  {nodeId: 6, nodeName: 'H1', parentId: 3},
+  {nodeId: 6, backendNodeId: 105, nodeName: 'H1', parentId: 3},
   {
     nodeId: 7,
+    backendNodeId: 106,
     nodeValue: bigText,
     nodeType: FontSizeGather.TEXT_NODE_TYPE,
     parentId: 6,
   },
-  {nodeId: 8, nodeName: 'SCRIPT', parentId: 3},
+  {nodeId: 8, backendNodeId: 107, nodeName: 'SCRIPT', parentId: 3},
   {
     nodeId: 9,
+    backendNodeId: 108,
     nodeValue: 'script text',
     nodeType: FontSizeGather.TEXT_NODE_TYPE,
     parentId: 8,
@@ -48,12 +52,44 @@ const nodes = [
   failingNode,
   {
     nodeId: 11,
+    backendNodeId: 110,
     nodeValue: failingText,
     nodeType: FontSizeGather.TEXT_NODE_TYPE,
     parentId: 10,
   },
 ];
 
+const stringsMap = {};
+const strings = [];
+const dedupe = (value) => {
+  if (stringsMap[value]) {
+    return stringsMap[value];
+  }
+
+  const index = strings.length;
+  stringsMap[value] = index;
+  strings.push(value);
+  return index;
+};
+const snapshot = {
+  documents: [
+    {
+      nodes: {
+        nodeType: nodes.map(node => node.nodeType),
+        nodeName: nodes.map(node => dedupe(node.nodeName)),
+        nodeValue: nodes.map(node => dedupe(node.nodeValue)),
+        backendNodeId: nodes.map(node => node.backendNodeId),
+        parentIndex: nodes.map(node => nodes.findIndex(pNode => node.parentId === pNode.nodeId)),
+      },
+      layout: {
+        nodeIndex: nodes.map((_, i) => i), // this isn't very accurate
+        styles: nodes.map(node => [dedupe(`${node.nodeId === bodyNode.nodeId ? 10 : 20}px`)]),
+      },
+    },
+  ],
+  strings,
+};
+
 describe('Font size gatherer', () => {
   // Reset the Gatherer before each test.
   beforeEach(() => {
@@ -64,27 +100,16 @@ describe('Font size gatherer', () => {
     const driver = {
       on() {},
       off() {},
-      async sendCommand(command, params) {
-        if (command === 'CSS.getComputedStyleForNode') {
-          if (params.nodeId === failingNode.nodeId) {
-            throw new Error('This is the failing node');
-          }
-
-          return {
-            computedStyle: [
-              {
-                name: 'font-size',
-                value: params.nodeId === bodyNode.nodeId ? 10 : 20,
-              },
-            ],
-          };
-        } else if (command === 'CSS.getMatchedStylesForNode') {
+      async sendCommand(command) {
+        if (command === 'CSS.getMatchedStylesForNode') {
           return {
             inlineStyle: null,
             attributesStyle: null,
             matchedCSSRules: [],
             inherited: [],
           };
+        } else if (command === 'DOMSnapshot.captureSnapshot') {
+          return snapshot;
         }
       },
       async getNodesInDocument() {
@@ -94,14 +119,13 @@ describe('Font size gatherer', () => {
 
     const artifact = await fontSizeGather.afterPass({driver});
     const expectedFailingTextLength = smallText.trim().length;
-    const expectedVisitedTextLength = bigText.trim().length + expectedFailingTextLength;
-    const expectedTotalTextLength = failingText.trim().length + expectedVisitedTextLength;
+    const expectedTotalTextLength = bigText.trim().length +
+      failingText.trim().length + expectedFailingTextLength;
     const expectedAnalyzedFailingTextLength = expectedFailingTextLength;
 
     expect(artifact).toEqual({
       analyzedFailingTextLength: expectedAnalyzedFailingTextLength,
       failingTextLength: expectedFailingTextLength,
-      visitedTextLength: expectedVisitedTextLength,
       totalTextLength: expectedTotalTextLength,
       analyzedFailingNodesData: [
         {
diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts
index 9caaf9e99b9e..18e654c674c5 100644
--- a/typings/artifacts.d.ts
+++ b/typings/artifacts.d.ts
@@ -182,7 +182,6 @@ declare global {
       export interface FontSize {
         totalTextLength: number;
         failingTextLength: number;
-        visitedTextLength: number;
         analyzedFailingTextLength: number;
         analyzedFailingNodesData: Array<{
           fontSize: number;

From 17413ec719f171faf6dc9971b42891af947a9060 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 25 Oct 2018 09:39:52 -0700
Subject: [PATCH 02/16] more comments

---
 .../gather/gatherers/seo/font-size.js           | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 7908083f9224..b96b4df0319b 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -259,19 +259,36 @@ class FontSize extends Gatherer {
       passContext.driver.sendCommand('CSS.enable'),
     ]);
 
+    // We need to find all TextNodes that do not have legible text. DOMSnapshot.captureSnapshot is the
+    // fastest way to get the computed styles of every Node. Bonus, it allows for whitelisting properties.
+    // Once a bad TextNode is identified, its parent Node is needed. DOMSnapshot.captureSnapshot doesn't
+    // give the entire Node object, so DOM.getFlattenedDocument is used. The only connection between a snapshot
+    // Node and an actual Protocol Node is backendId, so that is used to join the two data structures.
     const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
       computedStyles: ['font-size'],
     });
+    
+    // Makes the strings access code easier to read.
     /** @param {number} index */
     const lookup = (index) => snapshot.strings[index];
+    
+    // Locate the document under analysis.
     // TODO: this needs to use frameId
     const doc = snapshot.documents.find(doc => lookup(doc.documentURL) === passContext.url);
 
+    // doc is a flattened property list describing all the Nodes in a document, with all string values
+    // deduped in a strings array.
+    // Implementation:
+    // https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc?sq=package:chromium&g=0&l=534
+
     if (!doc || !doc.nodes.nodeType || !doc.nodes.nodeName || !doc.nodes.backendNodeId
       || !doc.nodes.nodeValue || !doc.nodes.parentIndex) {
       throw new Error('Unexpected response from DOMSnapshot.captureSnapshot.');
     }
 
+    // Not all nodes have computed styles (ex: TextNodes), so doc.layout.* is smaller than doc.nodes.*
+    // doc.layout.nodeIndex maps the index into doc.nodes.* to an index into doc.layout.styles.
+    // nodeIndexToStyleIndex inverses that mapping.
     /** @type {Map<number, number>} */
     const nodeIndexToStyleIndex = new Map();
     for (let i = 0; i < doc.layout.nodeIndex.length; i++) {

From 99ce40ebf1ad35a265322b9a7350e93f6dc44f55 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 25 Oct 2018 10:02:04 -0700
Subject: [PATCH 03/16] simplify isNonEmptyTextNode by restructuring backend id
 mapping

---
 .../gather/gatherers/seo/font-size.js         | 47 ++++++++++---------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index b96b4df0319b..f93a111b6cb3 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -176,11 +176,11 @@ function getEffectiveFontRule({inlineStyle, matchedCSSRules, inherited}) {
 }
 
 /**
- * @param {{nodeValue: string}} node
+ * @param {string} nodeValue
  * @returns {number}
  */
-function getNodeTextLength(node) {
-  return !node.nodeValue ? 0 : node.nodeValue.trim().length;
+function getNodeTextLength(nodeValue) {
+  return !nodeValue ? 0 : nodeValue.trim().length;
 }
 
 /**
@@ -207,14 +207,14 @@ async function fetchSourceRule(driver, node) {
 }
 
 /**
- * @param {{nodeType: number, nodeValue: string, parentNode: {nodeName: string}}} node
+ * @param {{nodeType: number, nodeValue: string, parentNodeName: string}} node
  * @returns {boolean}
  */
 function isNonEmptyTextNode(node) {
   return (
     node.nodeType === TEXT_NODE_TYPE &&
-    !TEXT_NODE_BLOCK_LIST.has(node.parentNode.nodeName) &&
-    getNodeTextLength(node) > 0
+    !TEXT_NODE_BLOCK_LIST.has(node.parentNodeName) &&
+    getNodeTextLength(node.nodeValue) > 0
   );
 }
 
@@ -267,11 +267,11 @@ class FontSize extends Gatherer {
     const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
       computedStyles: ['font-size'],
     });
-    
+
     // Makes the strings access code easier to read.
     /** @param {number} index */
     const lookup = (index) => snapshot.strings[index];
-    
+
     // Locate the document under analysis.
     // TODO: this needs to use frameId
     const doc = snapshot.documents.find(doc => lookup(doc.documentURL) === passContext.url);
@@ -295,17 +295,15 @@ class FontSize extends Gatherer {
       nodeIndexToStyleIndex.set(doc.layout.nodeIndex[i], i);
     }
 
-    /** @type {Map<number, number>} */
-    const failingBackendIdsToFontSize = new Map();
+    /** @type {Map<number, {fontSize: number, textLength: number}>} */
+    const backendIdsToPartialFontData = new Map();
     for (let i = 0; i < doc.nodes.nodeType.length; i++) {
       const nodeType = doc.nodes.nodeType[i];
       const nodeValue = lookup(doc.nodes.nodeValue[i]);
       if (!isNonEmptyTextNode({
         nodeType,
         nodeValue,
-        parentNode: {
-          nodeName: lookup(doc.nodes.nodeName[doc.nodes.parentIndex[i]]),
-        },
+        parentNodeName: lookup(doc.nodes.nodeName[doc.nodes.parentIndex[i]]),
       })) continue;
 
       const styleIndex = nodeIndexToStyleIndex.get(doc.nodes.parentIndex[i]);
@@ -313,28 +311,33 @@ class FontSize extends Gatherer {
       const parentStyles = doc.layout.styles[styleIndex];
       const [fontSizeStringId] = parentStyles;
       const fontSize = parseInt(lookup(fontSizeStringId), 10);
-      if (fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
-        failingBackendIdsToFontSize.set(doc.nodes.backendNodeId[i], fontSize);
-      }
+      backendIdsToPartialFontData.set(doc.nodes.backendNodeId[i], {
+        fontSize,
+        // TODO: trimming this for a second time. maybe don't?
+        textLength: getNodeTextLength(nodeValue),
+      });
     }
 
-    const nodes = (await getAllNodesFromBody(passContext.driver))
-      .filter(node => isNonEmptyTextNode(node));
+    const nodes = await getAllNodesFromBody(passContext.driver);
+
+    // backendIdsToPartialFontData will include all Nodes,
+    // but nodes will only contain the Body node and its descendants.
 
     /** @type {NodeFontData[]} */
     const failingNodes = [];
     let totalTextLength = 0;
     let failingTextLength = 0;
     for (const node of nodes) {
-      const textLength = getNodeTextLength(node);
+      const partialFontData = backendIdsToPartialFontData.get(node.backendNodeId);
+      if (!partialFontData) continue; // wasn't a non-empty TextNode
+      const {fontSize, textLength} = partialFontData;
       totalTextLength += textLength;
-      const badFontSize = failingBackendIdsToFontSize.get(node.backendNodeId);
-      if (badFontSize) {
+      if (fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
         failingTextLength += textLength;
         failingNodes.push({
           node: node.parentNode,
           textLength,
-          fontSize: badFontSize,
+          fontSize,
         });
       }
     }

From 80b0510943395c6edeccb3d587e49467debebd4e Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 25 Oct 2018 12:26:47 -0700
Subject: [PATCH 04/16] remove extra string trimming

---
 .../gather/gatherers/seo/font-size.js          | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index f93a111b6cb3..4f61de92dcd4 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -207,15 +207,11 @@ async function fetchSourceRule(driver, node) {
 }
 
 /**
- * @param {{nodeType: number, nodeValue: string, parentNodeName: string}} node
+ * @param {{nodeType: number, parentNodeName: string}} node
  * @returns {boolean}
  */
-function isNonEmptyTextNode(node) {
-  return (
-    node.nodeType === TEXT_NODE_TYPE &&
-    !TEXT_NODE_BLOCK_LIST.has(node.parentNodeName) &&
-    getNodeTextLength(node.nodeValue) > 0
-  );
+function isTextNode(node) {
+  return node.nodeType === TEXT_NODE_TYPE && !TEXT_NODE_BLOCK_LIST.has(node.parentNodeName);
 }
 
 class FontSize extends Gatherer {
@@ -300,21 +296,21 @@ class FontSize extends Gatherer {
     for (let i = 0; i < doc.nodes.nodeType.length; i++) {
       const nodeType = doc.nodes.nodeType[i];
       const nodeValue = lookup(doc.nodes.nodeValue[i]);
-      if (!isNonEmptyTextNode({
+      if (!isTextNode({
         nodeType,
-        nodeValue,
         parentNodeName: lookup(doc.nodes.nodeName[doc.nodes.parentIndex[i]]),
       })) continue;
 
       const styleIndex = nodeIndexToStyleIndex.get(doc.nodes.parentIndex[i]);
       if (!styleIndex) continue;
+      const textLength = getNodeTextLength(nodeValue);
+      if (!textLength) continue; // ignore empty TextNodes
       const parentStyles = doc.layout.styles[styleIndex];
       const [fontSizeStringId] = parentStyles;
       const fontSize = parseInt(lookup(fontSizeStringId), 10);
       backendIdsToPartialFontData.set(doc.nodes.backendNodeId[i], {
         fontSize,
-        // TODO: trimming this for a second time. maybe don't?
-        textLength: getNodeTextLength(nodeValue),
+        textLength,
       });
     }
 

From 70c976822d41d9c9440d6289f51b15504d6d344f Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 25 Oct 2018 12:28:17 -0700
Subject: [PATCH 05/16] remove disclaimer

---
 lighthouse-core/audits/seo/font-size.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lighthouse-core/audits/seo/font-size.js b/lighthouse-core/audits/seo/font-size.js
index 48a78ad097ad..a089397e1b1f 100644
--- a/lighthouse-core/audits/seo/font-size.js
+++ b/lighthouse-core/audits/seo/font-size.js
@@ -272,8 +272,7 @@ class FontSize extends Audit {
     let explanation;
     if (!passed) {
       const percentageOfFailingText = parseFloat((100 - percentageOfPassingText).toFixed(2));
-      const disclaimer = '';
-      explanation = `${percentageOfFailingText}% of text is too small${disclaimer}.`;
+      explanation = `${percentageOfFailingText}% of text is too small.`;
     }
 
     return {

From b3b5779156a967f90b63e65ffa03dac18d6590be Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 25 Oct 2018 12:29:38 -0700
Subject: [PATCH 06/16] pr stuff

---
 .../test/gather/gatherers/seo/font-size-test.js           | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
index 38e5c8e9d38b..619b23275ece 100644
--- a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
+++ b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
@@ -61,7 +61,7 @@ const nodes = [
 
 const stringsMap = {};
 const strings = [];
-const dedupe = (value) => {
+const getOrCreateStringIndex = (value) => {
   if (stringsMap[value]) {
     return stringsMap[value];
   }
@@ -76,14 +76,14 @@ const snapshot = {
     {
       nodes: {
         nodeType: nodes.map(node => node.nodeType),
-        nodeName: nodes.map(node => dedupe(node.nodeName)),
-        nodeValue: nodes.map(node => dedupe(node.nodeValue)),
+        nodeName: nodes.map(node => getOrCreateStringIndex(node.nodeName)),
+        nodeValue: nodes.map(node => getOrCreateStringIndex(node.nodeValue)),
         backendNodeId: nodes.map(node => node.backendNodeId),
         parentIndex: nodes.map(node => nodes.findIndex(pNode => node.parentId === pNode.nodeId)),
       },
       layout: {
         nodeIndex: nodes.map((_, i) => i), // this isn't very accurate
-        styles: nodes.map(node => [dedupe(`${node.nodeId === bodyNode.nodeId ? 10 : 20}px`)]),
+        styles: nodes.map(node => [getOrCreateStringIndex(`${node.nodeId === bodyNode.nodeId ? 10 : 20}px`)]),
       },
     },
   ],

From 4a31a3e4e3b4735a6c6b118a7832a3c8d6d53936 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Fri, 26 Oct 2018 10:48:56 -0700
Subject: [PATCH 07/16] lint

---
 .../test/gather/gatherers/seo/font-size-test.js           | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
index 619b23275ece..ad73ccb26e8a 100644
--- a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
+++ b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
@@ -82,8 +82,12 @@ const snapshot = {
         parentIndex: nodes.map(node => nodes.findIndex(pNode => node.parentId === pNode.nodeId)),
       },
       layout: {
-        nodeIndex: nodes.map((_, i) => i), // this isn't very accurate
-        styles: nodes.map(node => [getOrCreateStringIndex(`${node.nodeId === bodyNode.nodeId ? 10 : 20}px`)]),
+        nodeIndex: nodes.map((_, i) => i),
+        // this isn't very accurate.
+        // see https://github.com/GoogleChrome/lighthouse/pull/6391#discussion_r228267483
+        styles: nodes.map(node => [
+          getOrCreateStringIndex(`${node.nodeId === bodyNode.nodeId ? 10 : 20}px`),
+        ]),
       },
     },
   ],

From e5aaa34da83f86706f62caf4192e51fc92eb1642 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Fri, 26 Oct 2018 10:59:02 -0700
Subject: [PATCH 08/16] first doc in snapshot is the one we wat

---
 lighthouse-core/gather/gatherers/seo/font-size.js | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 4f61de92dcd4..5b4c1e2f2fc5 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -268,9 +268,8 @@ class FontSize extends Gatherer {
     /** @param {number} index */
     const lookup = (index) => snapshot.strings[index];
 
-    // Locate the document under analysis.
-    // TODO: this needs to use frameId
-    const doc = snapshot.documents.find(doc => lookup(doc.documentURL) === passContext.url);
+    // The document under analysis is the root document.
+    const doc = snapshot.documents[0];
 
     // doc is a flattened property list describing all the Nodes in a document, with all string values
     // deduped in a strings array.

From 3c01962ecb70a5782a22f26aa0ab119fc214847d Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Fri, 26 Oct 2018 11:36:14 -0700
Subject: [PATCH 09/16] break into functions

---
 .../gather/gatherers/seo/font-size.js         | 78 ++++++++++++-------
 1 file changed, 49 insertions(+), 29 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 5b4c1e2f2fc5..5951212df697 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -29,6 +29,7 @@ const TEXT_NODE_TYPE = 3;
 /** @typedef {import('../../driver.js')} Driver */
 /** @typedef {LH.Artifacts.FontSize['analyzedFailingNodesData'][0]} NodeFontData */
 /** @typedef {LH.Artifacts.FontSize.DomNodeMaybeWithParent} DomNodeMaybeWithParent*/
+/** @typedef {Map<number, {fontSize: number, textLength: number}>} BackendIdsToPartialFontData */
 
 /**
  * @param {LH.Artifacts.FontSize.DomNodeMaybeWithParent=} node
@@ -239,31 +240,10 @@ class FontSize extends Gatherer {
   }
 
   /**
-   * @param {LH.Gatherer.PassContext} passContext
-   * @return {Promise<LH.Artifacts.FontSize>} font-size analysis
+   * @param {LH.Crdp.DOMSnapshot.CaptureSnapshotResponse} snapshot
+   * @return {BackendIdsToPartialFontData}
    */
-  async afterPass(passContext) {
-    /** @type {Map<string, LH.Crdp.CSS.CSSStyleSheetHeader>} */
-    const stylesheets = new Map();
-    /** @param {LH.Crdp.CSS.StyleSheetAddedEvent} sheet */
-    const onStylesheetAdd = sheet => stylesheets.set(sheet.header.styleSheetId, sheet.header);
-    passContext.driver.on('CSS.styleSheetAdded', onStylesheetAdd);
-
-    await Promise.all([
-      passContext.driver.sendCommand('DOMSnapshot.enable'),
-      passContext.driver.sendCommand('DOM.enable'),
-      passContext.driver.sendCommand('CSS.enable'),
-    ]);
-
-    // We need to find all TextNodes that do not have legible text. DOMSnapshot.captureSnapshot is the
-    // fastest way to get the computed styles of every Node. Bonus, it allows for whitelisting properties.
-    // Once a bad TextNode is identified, its parent Node is needed. DOMSnapshot.captureSnapshot doesn't
-    // give the entire Node object, so DOM.getFlattenedDocument is used. The only connection between a snapshot
-    // Node and an actual Protocol Node is backendId, so that is used to join the two data structures.
-    const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
-      computedStyles: ['font-size'],
-    });
-
+  calculateBackendIdsToPartialFontData(snapshot) {
     // Makes the strings access code easier to read.
     /** @param {number} index */
     const lookup = (index) => snapshot.strings[index];
@@ -290,7 +270,7 @@ class FontSize extends Gatherer {
       nodeIndexToStyleIndex.set(doc.layout.nodeIndex[i], i);
     }
 
-    /** @type {Map<number, {fontSize: number, textLength: number}>} */
+    /** @type {BackendIdsToPartialFontData} */
     const backendIdsToPartialFontData = new Map();
     for (let i = 0; i < doc.nodes.nodeType.length; i++) {
       const nodeType = doc.nodes.nodeType[i];
@@ -313,11 +293,14 @@ class FontSize extends Gatherer {
       });
     }
 
-    const nodes = await getAllNodesFromBody(passContext.driver);
-
-    // backendIdsToPartialFontData will include all Nodes,
-    // but nodes will only contain the Body node and its descendants.
+    return backendIdsToPartialFontData;
+  }
 
+  /**
+   * @param {BackendIdsToPartialFontData} backendIdsToPartialFontData
+   * @param {LH.Artifacts.FontSize.DomNodeWithParent[]} nodes
+   */
+  findFailingNodes(backendIdsToPartialFontData, nodes) {
     /** @type {NodeFontData[]} */
     const failingNodes = [];
     let totalTextLength = 0;
@@ -337,6 +320,43 @@ class FontSize extends Gatherer {
       }
     }
 
+    return {totalTextLength, failingTextLength, failingNodes};
+  }
+
+  /**
+   * @param {LH.Gatherer.PassContext} passContext
+   * @return {Promise<LH.Artifacts.FontSize>} font-size analysis
+   */
+  async afterPass(passContext) {
+    /** @type {Map<string, LH.Crdp.CSS.CSSStyleSheetHeader>} */
+    const stylesheets = new Map();
+    /** @param {LH.Crdp.CSS.StyleSheetAddedEvent} sheet */
+    const onStylesheetAdd = sheet => stylesheets.set(sheet.header.styleSheetId, sheet.header);
+    passContext.driver.on('CSS.styleSheetAdded', onStylesheetAdd);
+
+    await Promise.all([
+      passContext.driver.sendCommand('DOMSnapshot.enable'),
+      passContext.driver.sendCommand('DOM.enable'),
+      passContext.driver.sendCommand('CSS.enable'),
+    ]);
+
+    // We need to find all TextNodes that do not have legible text. DOMSnapshot.captureSnapshot is the
+    // fastest way to get the computed styles of every Node. Bonus, it allows for whitelisting properties.
+    // Once a bad TextNode is identified, its parent Node is needed. DOMSnapshot.captureSnapshot doesn't
+    // give the entire Node object, so DOM.getFlattenedDocument is used. The only connection between a snapshot
+    // Node and an actual Protocol Node is backendId, so that is used to join the two data structures.
+    const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
+      computedStyles: ['font-size'],
+    });
+    // backendIdsToPartialFontData will include all Nodes,
+    const backendIdsToPartialFontData = this.calculateBackendIdsToPartialFontData(snapshot);
+    // but nodes will only contain the Body node and its descendants.
+    const nodes = await getAllNodesFromBody(passContext.driver);
+    const {
+      totalTextLength,
+      failingTextLength,
+      failingNodes,
+    } = this.findFailingNodes(backendIdsToPartialFontData, nodes);
     const {
       analyzedFailingNodesData,
       analyzedFailingTextLength,

From 3555192c0fc13dfec08f4cbd4073f1010fe6eb19 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Tue, 30 Oct 2018 16:11:07 -0700
Subject: [PATCH 10/16] pr changes

---
 .../gather/gatherers/seo/font-size.js         | 60 +++++++++----------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 5951212df697..4fd5be388b58 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -29,7 +29,7 @@ const TEXT_NODE_TYPE = 3;
 /** @typedef {import('../../driver.js')} Driver */
 /** @typedef {LH.Artifacts.FontSize['analyzedFailingNodesData'][0]} NodeFontData */
 /** @typedef {LH.Artifacts.FontSize.DomNodeMaybeWithParent} DomNodeMaybeWithParent*/
-/** @typedef {Map<number, {fontSize: number, textLength: number}>} BackendIdsToPartialFontData */
+/** @typedef {Map<number, {fontSize: number, textLength: number}>} BackendIdsToFontData */
 
 /**
  * @param {LH.Artifacts.FontSize.DomNodeMaybeWithParent=} node
@@ -181,7 +181,7 @@ function getEffectiveFontRule({inlineStyle, matchedCSSRules, inherited}) {
  * @returns {number}
  */
 function getNodeTextLength(nodeValue) {
-  return !nodeValue ? 0 : nodeValue.trim().length;
+  return nodeValue.trim().length;
 }
 
 /**
@@ -241,12 +241,10 @@ class FontSize extends Gatherer {
 
   /**
    * @param {LH.Crdp.DOMSnapshot.CaptureSnapshotResponse} snapshot
-   * @return {BackendIdsToPartialFontData}
+   * @return {BackendIdsToFontData}
    */
-  calculateBackendIdsToPartialFontData(snapshot) {
-    // Makes the strings access code easier to read.
-    /** @param {number} index */
-    const lookup = (index) => snapshot.strings[index];
+  calculateBackendIdsToFontData(snapshot) {
+    const strings = snapshot.strings;
 
     // The document under analysis is the root document.
     const doc = snapshot.documents[0];
@@ -256,13 +254,14 @@ class FontSize extends Gatherer {
     // Implementation:
     // https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc?sq=package:chromium&g=0&l=534
 
+    // satisfy the type checker that all expected values exist
     if (!doc || !doc.nodes.nodeType || !doc.nodes.nodeName || !doc.nodes.backendNodeId
       || !doc.nodes.nodeValue || !doc.nodes.parentIndex) {
       throw new Error('Unexpected response from DOMSnapshot.captureSnapshot.');
     }
 
     // Not all nodes have computed styles (ex: TextNodes), so doc.layout.* is smaller than doc.nodes.*
-    // doc.layout.nodeIndex maps the index into doc.nodes.* to an index into doc.layout.styles.
+    // doc.layout.nodeIndex maps the index into doc.layout.styles to an index into doc.nodes.*
     // nodeIndexToStyleIndex inverses that mapping.
     /** @type {Map<number, number>} */
     const nodeIndexToStyleIndex = new Map();
@@ -270,14 +269,14 @@ class FontSize extends Gatherer {
       nodeIndexToStyleIndex.set(doc.layout.nodeIndex[i], i);
     }
 
-    /** @type {BackendIdsToPartialFontData} */
-    const backendIdsToPartialFontData = new Map();
+    /** @type {BackendIdsToFontData} */
+    const backendIdsToFontData = new Map();
     for (let i = 0; i < doc.nodes.nodeType.length; i++) {
       const nodeType = doc.nodes.nodeType[i];
-      const nodeValue = lookup(doc.nodes.nodeValue[i]);
+      const nodeValue = strings[doc.nodes.nodeValue[i]];
       if (!isTextNode({
         nodeType,
-        parentNodeName: lookup(doc.nodes.nodeName[doc.nodes.parentIndex[i]]),
+        parentNodeName: strings[doc.nodes.nodeName[doc.nodes.parentIndex[i]]],
       })) continue;
 
       const styleIndex = nodeIndexToStyleIndex.get(doc.nodes.parentIndex[i]);
@@ -286,34 +285,39 @@ class FontSize extends Gatherer {
       if (!textLength) continue; // ignore empty TextNodes
       const parentStyles = doc.layout.styles[styleIndex];
       const [fontSizeStringId] = parentStyles;
-      const fontSize = parseInt(lookup(fontSizeStringId), 10);
-      backendIdsToPartialFontData.set(doc.nodes.backendNodeId[i], {
+      // expects values like '11.5px' here
+      const fontSize = parseFloat(strings[fontSizeStringId]);
+      backendIdsToFontData.set(doc.nodes.backendNodeId[i], {
         fontSize,
         textLength,
       });
     }
 
-    return backendIdsToPartialFontData;
+    return backendIdsToFontData;
   }
 
   /**
-   * @param {BackendIdsToPartialFontData} backendIdsToPartialFontData
-   * @param {LH.Artifacts.FontSize.DomNodeWithParent[]} nodes
+   * The only connection between a snapshot Node and an actual Protocol Node is backendId,
+   * so that is used to join the two data structures. DOMSnapshot.captureSnapshot doesn't
+   * give the entire Node object, so DOM.getFlattenedDocument is used.
+   * @param {BackendIdsToFontData} backendIdsToFontData
+   * @param {LH.Artifacts.FontSize.DomNodeWithParent[]} crdpNodes
    */
-  findFailingNodes(backendIdsToPartialFontData, nodes) {
+  findFailingNodes(backendIdsToFontData, crdpNodes) {
     /** @type {NodeFontData[]} */
     const failingNodes = [];
     let totalTextLength = 0;
     let failingTextLength = 0;
-    for (const node of nodes) {
-      const partialFontData = backendIdsToPartialFontData.get(node.backendNodeId);
+    for (const crdpNode of crdpNodes) {
+      const partialFontData = backendIdsToFontData.get(crdpNode.backendNodeId);
       if (!partialFontData) continue; // wasn't a non-empty TextNode
       const {fontSize, textLength} = partialFontData;
       totalTextLength += textLength;
       if (fontSize < MINIMAL_LEGIBLE_FONT_SIZE_PX) {
+        // Once a bad TextNode is identified, its parent Node is needed.
         failingTextLength += textLength;
         failingNodes.push({
-          node: node.parentNode,
+          node: crdpNode.parentNode,
           textLength,
           fontSize,
         });
@@ -340,23 +344,19 @@ class FontSize extends Gatherer {
       passContext.driver.sendCommand('CSS.enable'),
     ]);
 
-    // We need to find all TextNodes that do not have legible text. DOMSnapshot.captureSnapshot is the
-    // fastest way to get the computed styles of every Node. Bonus, it allows for whitelisting properties.
-    // Once a bad TextNode is identified, its parent Node is needed. DOMSnapshot.captureSnapshot doesn't
-    // give the entire Node object, so DOM.getFlattenedDocument is used. The only connection between a snapshot
-    // Node and an actual Protocol Node is backendId, so that is used to join the two data structures.
+    // Use DOMSnapshot.captureSnapshot to get the computed font-size style of every node.
     const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
       computedStyles: ['font-size'],
     });
-    // backendIdsToPartialFontData will include all Nodes,
-    const backendIdsToPartialFontData = this.calculateBackendIdsToPartialFontData(snapshot);
+    // backendIdsToFontData will include all non-empty TextNodes,
+    const backendIdsToFontData = this.calculateBackendIdsToFontData(snapshot);
     // but nodes will only contain the Body node and its descendants.
-    const nodes = await getAllNodesFromBody(passContext.driver);
+    const crdpNodes = await getAllNodesFromBody(passContext.driver);
     const {
       totalTextLength,
       failingTextLength,
       failingNodes,
-    } = this.findFailingNodes(backendIdsToPartialFontData, nodes);
+    } = this.findFailingNodes(backendIdsToFontData, crdpNodes);
     const {
       analyzedFailingNodesData,
       analyzedFailingTextLength,

From 2e7fa881903e91c0aa6151eac1d8ba28917a452b Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Wed, 31 Oct 2018 07:36:07 -0700
Subject: [PATCH 11/16] comments

---
 lighthouse-core/gather/gatherers/seo/font-size.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 4fd5be388b58..f25185958a69 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -284,6 +284,9 @@ class FontSize extends Gatherer {
       const textLength = getNodeTextLength(nodeValue);
       if (!textLength) continue; // ignore empty TextNodes
       const parentStyles = doc.layout.styles[styleIndex];
+      // each styles is an array of string indices, one for each property given
+      // to DOMSnapshot.captureSnapshot. We only requested font-size, so there's just
+      // the one string index here.
       const [fontSizeStringId] = parentStyles;
       // expects values like '11.5px' here
       const fontSize = parseFloat(strings[fontSizeStringId]);

From ec3da81ddda4d2c072513fd275e40e4a1c931a89 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Wed, 31 Oct 2018 07:37:37 -0700
Subject: [PATCH 12/16] forEach

---
 lighthouse-core/gather/gatherers/seo/font-size.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index f25185958a69..c0f18234d11d 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -265,9 +265,9 @@ class FontSize extends Gatherer {
     // nodeIndexToStyleIndex inverses that mapping.
     /** @type {Map<number, number>} */
     const nodeIndexToStyleIndex = new Map();
-    for (let i = 0; i < doc.layout.nodeIndex.length; i++) {
-      nodeIndexToStyleIndex.set(doc.layout.nodeIndex[i], i);
-    }
+    doc.layout.nodeIndex.forEach((nodeIndex, styleIndex) => {
+      nodeIndexToStyleIndex.set(nodeIndex, styleIndex);
+    });
 
     /** @type {BackendIdsToFontData} */
     const backendIdsToFontData = new Map();

From d6698dd8c4da292941478dc0e8693e3e517f7cf8 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Wed, 31 Oct 2018 07:41:44 -0700
Subject: [PATCH 13/16] pr

---
 lighthouse-core/gather/gatherers/seo/font-size.js | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index c0f18234d11d..e6d0dbe03d61 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -274,12 +274,14 @@ class FontSize extends Gatherer {
     for (let i = 0; i < doc.nodes.nodeType.length; i++) {
       const nodeType = doc.nodes.nodeType[i];
       const nodeValue = strings[doc.nodes.nodeValue[i]];
+      const parentIndex = doc.nodes.parentIndex[i];
+      const parentNodeName = strings[doc.nodes.nodeName[parentIndex]];
       if (!isTextNode({
         nodeType,
-        parentNodeName: strings[doc.nodes.nodeName[doc.nodes.parentIndex[i]]],
+        parentNodeName,
       })) continue;
 
-      const styleIndex = nodeIndexToStyleIndex.get(doc.nodes.parentIndex[i]);
+      const styleIndex = nodeIndexToStyleIndex.get(parentIndex);
       if (!styleIndex) continue;
       const textLength = getNodeTextLength(nodeValue);
       if (!textLength) continue; // ignore empty TextNodes

From 09b6e93d311a0cdcd0c2c420ca9fa165ded51ae9 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 1 Nov 2018 13:56:14 -0700
Subject: [PATCH 14/16] snapshot and all nodes concurrent

---
 lighthouse-core/gather/gatherers/seo/font-size.js | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index e6d0dbe03d61..39358f400122 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -350,13 +350,14 @@ class FontSize extends Gatherer {
     ]);
 
     // Use DOMSnapshot.captureSnapshot to get the computed font-size style of every node.
-    const snapshot = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
+    const snapshotPromise = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
       computedStyles: ['font-size'],
     });
-    // backendIdsToFontData will include all non-empty TextNodes,
+    const allNodesPromise = await getAllNodesFromBody(passContext.driver);
+    const [snapshot, crdpNodes] = await Promise.all([snapshotPromise, allNodesPromise]);
     const backendIdsToFontData = this.calculateBackendIdsToFontData(snapshot);
-    // but nodes will only contain the Body node and its descendants.
-    const crdpNodes = await getAllNodesFromBody(passContext.driver);
+    // backendIdsToFontData will include all non-empty TextNodes,
+    // but crdpNodes will only contain the Body node and its descendants.
     const {
       totalTextLength,
       failingTextLength,

From 3da4c8b849dacd5679f7f2f3caa8367f5591c143 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Thu, 1 Nov 2018 13:58:40 -0700
Subject: [PATCH 15/16] snapshot and all nodes concurrent

---
 lighthouse-core/gather/gatherers/seo/font-size.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 39358f400122..3e5d38e1d750 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -350,10 +350,10 @@ class FontSize extends Gatherer {
     ]);
 
     // Use DOMSnapshot.captureSnapshot to get the computed font-size style of every node.
-    const snapshotPromise = await passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
+    const snapshotPromise = passContext.driver.sendCommand('DOMSnapshot.captureSnapshot', {
       computedStyles: ['font-size'],
     });
-    const allNodesPromise = await getAllNodesFromBody(passContext.driver);
+    const allNodesPromise = getAllNodesFromBody(passContext.driver);
     const [snapshot, crdpNodes] = await Promise.all([snapshotPromise, allNodesPromise]);
     const backendIdsToFontData = this.calculateBackendIdsToFontData(snapshot);
     // backendIdsToFontData will include all non-empty TextNodes,

From 098508df08388583403c5f2b5027bb4025b515b5 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Fri, 2 Nov 2018 11:17:13 -0700
Subject: [PATCH 16/16] look at all documents in snapshot. look at all
 ancestors to find style info

---
 .../gather/gatherers/seo/font-size.js         | 107 ++++++++++--------
 1 file changed, 60 insertions(+), 47 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/font-size.js b/lighthouse-core/gather/gatherers/seo/font-size.js
index 3e5d38e1d750..596856bb171d 100644
--- a/lighthouse-core/gather/gatherers/seo/font-size.js
+++ b/lighthouse-core/gather/gatherers/seo/font-size.js
@@ -246,56 +246,69 @@ class FontSize extends Gatherer {
   calculateBackendIdsToFontData(snapshot) {
     const strings = snapshot.strings;
 
-    // The document under analysis is the root document.
-    const doc = snapshot.documents[0];
-
-    // doc is a flattened property list describing all the Nodes in a document, with all string values
-    // deduped in a strings array.
-    // Implementation:
-    // https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc?sq=package:chromium&g=0&l=534
-
-    // satisfy the type checker that all expected values exist
-    if (!doc || !doc.nodes.nodeType || !doc.nodes.nodeName || !doc.nodes.backendNodeId
-      || !doc.nodes.nodeValue || !doc.nodes.parentIndex) {
-      throw new Error('Unexpected response from DOMSnapshot.captureSnapshot.');
-    }
-
-    // Not all nodes have computed styles (ex: TextNodes), so doc.layout.* is smaller than doc.nodes.*
-    // doc.layout.nodeIndex maps the index into doc.layout.styles to an index into doc.nodes.*
-    // nodeIndexToStyleIndex inverses that mapping.
-    /** @type {Map<number, number>} */
-    const nodeIndexToStyleIndex = new Map();
-    doc.layout.nodeIndex.forEach((nodeIndex, styleIndex) => {
-      nodeIndexToStyleIndex.set(nodeIndex, styleIndex);
-    });
-
     /** @type {BackendIdsToFontData} */
     const backendIdsToFontData = new Map();
-    for (let i = 0; i < doc.nodes.nodeType.length; i++) {
-      const nodeType = doc.nodes.nodeType[i];
-      const nodeValue = strings[doc.nodes.nodeValue[i]];
-      const parentIndex = doc.nodes.parentIndex[i];
-      const parentNodeName = strings[doc.nodes.nodeName[parentIndex]];
-      if (!isTextNode({
-        nodeType,
-        parentNodeName,
-      })) continue;
-
-      const styleIndex = nodeIndexToStyleIndex.get(parentIndex);
-      if (!styleIndex) continue;
-      const textLength = getNodeTextLength(nodeValue);
-      if (!textLength) continue; // ignore empty TextNodes
-      const parentStyles = doc.layout.styles[styleIndex];
-      // each styles is an array of string indices, one for each property given
-      // to DOMSnapshot.captureSnapshot. We only requested font-size, so there's just
-      // the one string index here.
-      const [fontSizeStringId] = parentStyles;
-      // expects values like '11.5px' here
-      const fontSize = parseFloat(strings[fontSizeStringId]);
-      backendIdsToFontData.set(doc.nodes.backendNodeId[i], {
-        fontSize,
-        textLength,
+
+    for (let j = 0; j < snapshot.documents.length; j++) {
+      const doc = snapshot.documents[j];
+      // doc is a flattened property list describing all the Nodes in a document, with all string values
+      // deduped in a strings array.
+      // Implementation:
+      // https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc?sq=package:chromium&g=0&l=534
+
+      // satisfy the type checker that all expected values exist
+      if (!doc || !doc.nodes.nodeType || !doc.nodes.nodeName || !doc.nodes.backendNodeId
+        || !doc.nodes.nodeValue || !doc.nodes.parentIndex) {
+        throw new Error('Unexpected response from DOMSnapshot.captureSnapshot.');
+      }
+
+      // Not all nodes have computed styles (ex: TextNodes), so doc.layout.* is smaller than doc.nodes.*
+      // doc.layout.nodeIndex maps the index into doc.layout.styles to an index into doc.nodes.*
+      // nodeIndexToStyleIndex inverses that mapping.
+      /** @type {Map<number, number>} */
+      const nodeIndexToStyleIndex = new Map();
+      doc.layout.nodeIndex.forEach((nodeIndex, styleIndex) => {
+        nodeIndexToStyleIndex.set(nodeIndex, styleIndex);
       });
+
+      for (let i = 0; i < doc.nodes.nodeType.length; i++) {
+        const nodeType = doc.nodes.nodeType[i];
+        const nodeValue = strings[doc.nodes.nodeValue[i]];
+        const parentIndex = doc.nodes.parentIndex[i];
+        const parentNodeName = strings[doc.nodes.nodeName[parentIndex]];
+        if (!isTextNode({
+          nodeType,
+          parentNodeName,
+        })) continue;
+
+        const textLength = getNodeTextLength(nodeValue);
+        if (!textLength) continue; // ignore empty TextNodes
+
+        // walk up the hierarchy until a Node with a style is found
+        let ancestorIndex = parentIndex;
+        while (!nodeIndexToStyleIndex.has(ancestorIndex)) {
+          ancestorIndex = doc.nodes.parentIndex[ancestorIndex];
+          if (ancestorIndex === -1) {
+            // this shouldn't happen - at the very least, the root Node should have a style
+            throw new Error('Could not find style for a TextNode.');
+          }
+        }
+        /** @type {number} */
+        // @ts-ignore: if styleIndex would be undefined, the above error would have thrown
+        const styleIndex = nodeIndexToStyleIndex.get(ancestorIndex);
+        const styles = doc.layout.styles[styleIndex];
+
+        // each styles is an array of string indices, one for each property given
+        // to DOMSnapshot.captureSnapshot. We only requested font-size, so there's just
+        // the one string index here.
+        const [fontSizeStringId] = styles;
+        // expects values like '11.5px' here
+        const fontSize = parseFloat(strings[fontSizeStringId]);
+        backendIdsToFontData.set(doc.nodes.backendNodeId[i], {
+          fontSize,
+          textLength,
+        });
+      }
     }
 
     return backendIdsToFontData;