DOMStat Audit: shadow roots don't have .classList (#2131)

* DOMStat Audit: handle shadow roots (dont have .classList) * Users should target < optimalVal * isShadowHost
GoogleChrome · May 3, 2017 · d3a0692 · d3a0692
1 parent 4775908
commit d3a0692
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 13 deletions.
diff --git a/lighthouse-cli/test/fixtures/dobetterweb/domtester.html b/lighthouse-cli/test/fixtures/dobetterweb/domtester.html
@@ -40,6 +40,7 @@
   </main>
   <footer>
     <div id="attachshadow-big" class="test this out">4</div>
+    <div id="attachshadow-big-2" class="root with many children">5</div>
     <div>3</div>
     <div></div>
     <div></div>
@@ -72,6 +73,21 @@
     `;
   }
 
+  function withShadowDOMTest2() {
+    const el3 = document.querySelector('#attachshadow-big-2');
+    el3.attachShadow({mode: 'open'}).innerHTML = `
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+      <div>6</div>
+    `;
+  }
+
   function domTest(numNodes=1500) {
     const frag = new DocumentFragment();
     for (let i = 0; i < numNodes; ++i) {
@@ -89,6 +105,9 @@
   if (params.has('withShadowDOM')) {
     withShadowDOMTest();
   }
+  if (params.has('ShadowRootWithManyChildren')) {
+    withShadowDOMTest2();
+  }
 </script>
 </body>
 </html>
diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -158,7 +158,7 @@ module.exports = [
         score: 100,
         extendedInfo: {
           value: {
-            0: {value: '1,323'},
+            0: {value: '1,324'},
             1: {value: '7'},
             2: {value: '1,303'}
           }
@@ -173,7 +173,7 @@ module.exports = [
         score: 0,
         extendedInfo: {
           value: {
-            0: {value: '6,024'},
+            0: {value: '6,025'},
             1: {value: '9'},
             2: {value: '6,003'}
           }
@@ -188,13 +188,28 @@ module.exports = [
         score: 100,
         extendedInfo: {
           value: {
-            0: {value: '24'},
+            0: {value: '25'},
             1: {value: '9'},
             2: {value: '9'}
           }
         }
       }
     }
+  }, {
+    initialUrl: 'http://localhost:10200/dobetterweb/domtester.html?ShadowRootWithManyChildren',
+    url: 'http://localhost:10200/dobetterweb/domtester.html?ShadowRootWithManyChildren',
+    audits: {
+      'dom-size': {
+        score: 100,
+        extendedInfo: {
+          value: {
+            0: {value: '24'},
+            1: {value: '7'},
+            2: {value: '9'}
+          }
+        }
+      }
+    }
   }, {
     initialUrl: 'http://localhost:10200/online-only.html',
     url: 'http://localhost:10200/online-only.html',

diff --git a/lighthouse-core/audits/dobetterweb/dom-size.js b/lighthouse-core/audits/dobetterweb/dom-size.js
@@ -48,7 +48,7 @@ class DOMSize extends Audit {
       category: 'Performance',
       name: 'dom-size',
       description: 'Avoids an excessive DOM size',
-      optimalValue: DOMSize.MAX_DOM_NODES.toLocaleString() + ' nodes',
+      optimalValue: `< ${DOMSize.MAX_DOM_NODES.toLocaleString()} nodes`,
       helpText: 'Browser engineers recommend pages contain fewer than ' +
         `~${DOMSize.MAX_DOM_NODES.toLocaleString()} DOM nodes. The sweet spot is a tree depth < ` +
         `${MAX_DOM_TREE_DEPTH} elements and fewer than ${MAX_DOM_TREE_WIDTH} ` +

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/domstats.js b/lighthouse-core/gather/gatherers/dobetterweb/domstats.js
@@ -20,7 +20,7 @@
  * and total number of nodes used on the page.
  */
 
-/* global document */
+/* global document ShadowRoot */
 
 'use strict';
 
@@ -34,17 +34,22 @@ const Gatherer = require('../gatherer');
  */
 /* istanbul ignore next */
 function createSelectorsLabel(element) {
-  let name = element.localName;
+  let name = element.localName || '';
   const idAttr = element.getAttribute && element.getAttribute('id');
   if (idAttr) {
     name += `#${idAttr}`;
   }
   // svg elements return SVGAnimatedString for .className, which is an object.
   // Stringify classList instead.
-  const className = element.classList.toString();
-  if (className) {
-    name += `.${className.trim().replace(/\s+/g, '.')}`;
+  if (element.classList) {
+    const className = element.classList.toString();
+    if (className) {
+      name += `.${className.trim().replace(/\s+/g, '.')}`;
+    }
+  } else if (ShadowRoot.prototype.isPrototypeOf(element)) {
+    name += '#shadow-root';
   }
+
   return name;
 }
 
@@ -59,9 +64,15 @@ function elementPathInDOM(element) {
   while (node) {
     // Anchor elements have a .host property. Be sure we've found a shadow root
     // host and not an anchor.
-    const shadowHost = node.parentNode && node.parentNode.host &&
-                       node.parentNode.localName !== 'a';
-    node = shadowHost ? node.parentNode.host : node.parentElement;
+    if (ShadowRoot.prototype.isPrototypeOf(node)) {
+      const isShadowHost = node.host && node.localName !== 'a';
+      node = isShadowHost ? node.host : node.parentElement;
+    } else {
+      const isShadowHost = node.parentNode && node.parentNode.host &&
+                           node.parentNode.localName !== 'a';
+      node = isShadowHost ? node.parentNode.host : node.parentElement;
+    }
+
     if (node) {
       path.unshift(createSelectorsLabel(node));
     }

diff --git a/lighthouse-core/test/audits/dobetterweb/dom-size-test.js b/lighthouse-core/test/audits/dobetterweb/dom-size-test.js
@@ -39,7 +39,7 @@ describe('Num DOM nodes audit', () => {
     const auditResult = DOMSize.audit(artifact);
     assert.equal(auditResult.score, 100);
     assert.equal(auditResult.rawValue, numNodes);
-    assert.equal(auditResult.optimalValue, `${DOMSize.MAX_DOM_NODES.toLocaleString()} nodes`);
+    assert.equal(auditResult.optimalValue, `< ${DOMSize.MAX_DOM_NODES.toLocaleString()} nodes`);
     assert.equal(auditResult.displayValue, `${numNodes.toLocaleString()} nodes`);
     assert.equal(auditResult.extendedInfo.value[0].title, 'Total DOM Nodes');
     assert.equal(auditResult.extendedInfo.value[0].value, numNodes.toLocaleString());