core(full-page-screenshot): resolve node rects during emulation

lighthouse-cli/test/fixtures/perf/delayed-element.js

@@ -39,6 +39,14 @@ setTimeout(() => {
   sectionEl.append(imgEl, textEl);
   shadowRoot.append(sectionEl);
 
+  textEl.style.color = 'gray';
+  textEl.style.backgroundColor = 'lightgrey';
+  let padding = 1;
+  setInterval(() => {
+    textEl.style.paddingTop = padding + 'px';
+    padding += 1;
+  }, 50);
+
   // layout-shift-elements: ensure we can handle missing shift elements
   if (window.location.href.includes('?missing')) {
     stall(100); // force a long task to ensure we reach the rerendering stage
@@ -47,7 +55,7 @@ setTimeout(() => {
       document.body.textContent = 'Now it is all gone!';
     }, 50);
   }
-}, 1000);
+}, 1);
 
 // long-tasks: add a very long task at least 500ms
 stall(800);

lighthouse-core/gather/gather-runner.js

@@ -14,6 +14,7 @@ const NetworkRecorder = require('../lib/network-recorder.js');
 const constants = require('../config/constants.js');
 const i18n = require('../lib/i18n/i18n.js');
 const URL = require('../lib/url-shim.js');
+const pageFunctions = require('../lib/page-functions.js');
 
 const UIStrings = {
   /**
@@ -501,6 +502,60 @@ class GatherRunner {
     };
   }
 
+  /**
+   * Gatherers can collect details about DOM nodes, including their position on the page.
+   * Layout shifts occuring after a gatherer runs can cause these positions to be incorrect,
+   * resulting in a poor experience for element screenshots.
+   * `getNodeDetails` maintains a collection of DOM objects in the page, which we can iterate
+   * to re-collect the bounding client rectangle. We also update the devtools node path.
+   * The old devtools node path is used as a lookup key. We walk the entire artifacts object
+   * to update all objects that reference an old devtools node path.
+   * @param {Driver} driver
+   * @param {Partial<LH.Artifacts>} artifacts
+   */
+  static async resolveNodes(driver, artifacts) {
+    function resolveNodes() {
+      return (window.__nodes || []).map(({key, node}) => {
+        return {
+          key,
+          newBoundingRect: getBoundingClientRect(node),
+          newDevtoolsNodePath: getNodePath(node),
+        };
+      });
+    }
+    const expression = `(function () {
+      ${pageFunctions.getBoundingClientRectString};
+      ${pageFunctions.getNodePathString};
+      return (${resolveNodes.toString()}());
+    })()`;
+
+    /** @type {Array<{key: string, newBoundingRect: any, newDevtoolsNodePath: string}>} */
+    const resolved = await driver.evaluateAsync(expression, {useIsolation: true});
+
+    console.log(' resolved ====');
+    console.log(resolved);
+
+    const walk = require('../lib/sd-validation/helpers/walk-object.js');
+    walk(artifacts, (name, value) => {
+      if (!value || typeof value !== 'object') return;
+      if (!value.path && !value.devtoolsNodePath) return;
+
+      const resolvedNode = resolved.find(r => r.key === value.devtoolsNodePath);
+      if (!resolvedNode) return;
+
+      console.log(resolvedNode.key, 'set to rect', resolvedNode.newBoundingRect);
+
+      // Rects are stored on properties named either `boundingRect` or `clientRect`.
+      if (value.boundingRect) {
+        value.boundingRect = resolvedNode.newBoundingRect;
+      } else if (value.clientRect) {
+        value.clientRect = resolvedNode.newBoundingRect;
+      }
+
+      value.devtoolsNodePath = resolvedNode.newDevtoolsNodePath;
+    });
+  }
+
   /**
    * Return an initialized but mostly empty set of base artifacts, to be
    * populated as the run continues.
@@ -783,7 +838,11 @@ class GatherRunner {
 
     // Run `afterPass()` on gatherers and return collected artifacts.
     await GatherRunner.afterPass(passContext, loadData, gathererResults);
-    const artifacts = GatherRunner.collectArtifacts(gathererResults);
+    const artifacts = await GatherRunner.collectArtifacts(gathererResults);
+
+    if (process.env.RESOLVE_NODES) {
+      await this.resolveNodes(driver, artifacts.artifacts);
+    }
 
     log.timeEnd(status);
     return artifacts;

lighthouse-core/lib/page-functions.js

@@ -447,19 +447,30 @@ function wrapRequestIdleCallback(cpuSlowdownMultiplier) {
   };
 }
 
-const getNodeDetailsString = `function getNodeDetails(elem) {
+/**
+ * @param {HTMLElement} element
+ */
+function getNodeDetails(element) {
+  const details = {
+    devtoolsNodePath: getNodePath(element),
+    selector: getNodeSelector(element),
+    boundingRect: getBoundingClientRect(element),
+    snippet: getOuterHTMLSnippet(element),
+    nodeLabel: getNodeLabel(element),
+  };
+  window.__nodes = window.__nodes || [];
+  window.__nodes.push({key: details.devtoolsNodePath, node: element});
+  return details;
+}
+
+const getNodeDetailsString = `function getNodeDetails(element) {
   ${getNodePath.toString()};
   ${getNodeSelector.toString()};
   ${getBoundingClientRect.toString()};
   ${getOuterHTMLSnippet.toString()};
   ${getNodeLabel.toString()};
-  return {
-    devtoolsNodePath: getNodePath(elem),
-    selector: getNodeSelector(elem),
-    boundingRect: getBoundingClientRect(elem),
-    snippet: getOuterHTMLSnippet(elem),
-    nodeLabel: getNodeLabel(elem),
-  };
+  ${getNodeDetails.toString()};
+  return getNodeDetails(element);
 }`;
 
 module.exports = {

lighthouse-core/report/html/renderer/details-renderer.js

@@ -533,7 +533,7 @@ class DetailsRenderer {
       item.boundingRect,
       maxThumbnailSize
     );
-    element.prepend(elementScreenshot);
+    elementScreenshot && element.prepend(elementScreenshot);
 
     return element;
   }

lighthouse-core/report/html/renderer/element-screenshot-renderer.js

@@ -17,6 +17,22 @@
 /** @typedef {LH.Artifacts.Rect} Rect */
 /** @typedef {{width: number, height: number}} Size */
 
+/**
+ * Returns whether rect2 is contained entirely within rect1;
+ * @param {LH.Artifacts.Rect} rect1
+ * @param {LH.Artifacts.Rect} rect2
+ * @return {boolean}
+ */
+// We sometimes run this as a part of a gatherer script injected into the page, so prevent
+// renaming the function for code coverage.
+/* istanbul ignore next */
+function rectContains(rect1, rect2) {
+  return rect2.top >= rect1.top &&
+    rect2.right <= rect1.right &&
+    rect2.bottom <= rect1.bottom &&
+    rect2.left >= rect1.left;
+}
+
 /**
  * @param {number} value
  * @param {number} min
@@ -153,13 +169,16 @@ class ElementScreenshotRenderer {
         top: Number(el.dataset['rectTop']),
         bottom: Number(el.dataset['rectTop']) + Number(el.dataset['rectHeight']),
       };
-      overlay.appendChild(ElementScreenshotRenderer.render(
+      const screenshotElement = ElementScreenshotRenderer.render(
         dom,
         templateContext,
         fullPageScreenshot,
         elementRectSC,
         maxLightboxSize
-      ));
+      );
+      if (!screenshotElement) return;
+
+      overlay.appendChild(screenshotElement);
       overlay.addEventListener('click', () => {
         overlay.remove();
       });
@@ -188,14 +207,28 @@ class ElementScreenshotRenderer {
   /**
    * Renders an element with surrounding context from the full page screenshot.
    * Used to render both the thumbnail preview in details tables and the full-page screenshot in the lightbox.
+   * Returns null if element rect is outside screenshot bounds.
    * @param {DOM} dom
    * @param {ParentNode} templateContext
    * @param {LH.Audit.Details.FullPageScreenshot} fullPageScreenshot
    * @param {LH.Artifacts.Rect} elementRectSC Region of screenshot to highlight.
    * @param {Size} maxRenderSizeDC e.g. maxThumbnailSize or maxLightboxSize.
-   * @return {Element}
+   * @return {Element|null}
    */
   static render(dom, templateContext, fullPageScreenshot, elementRectSC, maxRenderSizeDC) {
+    const fullPageScreenshotRect = {
+      left: 0,
+      top: 0,
+      right: fullPageScreenshot.width,
+      bottom: fullPageScreenshot.height,
+      width: fullPageScreenshot.width,
+      height: fullPageScreenshot.height,
+    };
+    if (!rectContains(fullPageScreenshotRect, elementRectSC)) {
+      // Element is out of bounds of screenshot.
+      return null;
+    }
+
     const tmpl = dom.cloneTemplate('#tmpl-lh-element-screenshot', templateContext);
     const containerEl = dom.find('.lh-element-screenshot', tmpl);
 

lighthouse-core/test/report/html/renderer/element-screenshot-renderer-test.js

@@ -21,6 +21,18 @@ const TEMPLATE_FILE = fs.readFileSync(
   'utf8'
 );
 
+/**
+ * @param {{left: number, top: number, width: number, height:number}} opts
+ * @returns {LH.Artifacts.Rect}
+ */
+function makeRect(opts) {
+  return {
+    ...opts,
+    right: opts.left + opts.width,
+    bottom: opts.top + opts.height,
+  };
+}
+
 describe('ElementScreenshotRenderer', () => {
   let dom;
 
@@ -38,17 +50,17 @@ describe('ElementScreenshotRenderer', () => {
     global.Util = undefined;
   });
 
-  it('renders screenshot', () => {
+  it.only('renders screenshot', () => {
     const fullPageScreenshot = {
       width: 1000,
       height: 1000,
     };
-    const elementRectSC = {
+    const elementRectSC = makeRect({
       left: 50,
       top: 50,
       width: 200,
       height: 300,
-    };
+    });
     const renderContainerSizeDC = {
       width: 500,
       height: 500,
@@ -81,6 +93,30 @@ describe('ElementScreenshotRenderer', () => {
     /* eslint-enable max-len */
   });
 
+  it('returns null if element is out of bounds', () => {
+    const fullPageScreenshot = {
+      width: 1000,
+      height: 1000,
+    };
+    const elementRectSC = makeRect({
+      left: 50,
+      top: 5000,
+      width: 200,
+      height: 300,
+    });
+    const renderContainerSizeDC = {
+      width: 500,
+      height: 500,
+    };
+    expect(ElementScreenshotRenderer.render(
+      dom,
+      dom.document(),
+      fullPageScreenshot,
+      elementRectSC,
+      renderContainerSizeDC
+    )).toBe(null);
+  });
+
   describe('getScreenshotPositions', () => {
     it('centers the screenshot on the highlighted area', () => {
       expect(