core(lantern): link layout nodes to root frame request (GoogleChrome#…

…9727)

.gitignore

@@ -29,6 +29,8 @@ last-run-results.html
 !lighthouse-core/test/results/artifacts/*.devtoolslog.json
 !lighthouse-core/test/fixtures/artifacts/**/*.trace.json
 !lighthouse-core/test/fixtures/artifacts/**/*.devtoolslog.json
+!lighthouse-core/test/fixtures/traces/**/*.trace.json
+!lighthouse-core/test/fixtures/traces/**/*.devtoolslog.json
 
 latest-run
 lantern-data

lighthouse-core/computed/page-dependency-graph.js

@@ -59,8 +59,12 @@ class PageDependencyGraph {
   static getNetworkNodeOutput(networkRecords) {
     /** @type {Array<NetworkNode>} */
     const nodes = [];
+    /** @type {Map<string, NetworkNode>} */
     const idToNodeMap = new Map();
+    /** @type {Map<string, Array<NetworkNode>>} */
     const urlToNodeMap = new Map();
+    /** @type {Map<string, NetworkNode|null>} */
+    const frameIdToNodeMap = new Map();
 
     networkRecords.forEach(record => {
       if (IGNORED_MIME_TYPES_REGEX.test(record.mimeType)) return;
@@ -76,14 +80,24 @@ class PageDependencyGraph {
       const node = new NetworkNode(record);
       nodes.push(node);
 
-      const list = urlToNodeMap.get(record.url) || [];
-      list.push(node);
+      const urlList = urlToNodeMap.get(record.url) || [];
+      urlList.push(node);
 
       idToNodeMap.set(record.requestId, node);
-      urlToNodeMap.set(record.url, list);
+      urlToNodeMap.set(record.url, urlList);
+
+      // If the request was for the root document of an iframe, save an entry in our
+      // map so we can link up the task `args.data.frame` dependencies later in graph creation.
+      if (record.frameId &&
+          record.resourceType === NetworkRequest.TYPES.Document &&
+          record.documentURL === record.url) {
+        // If there's ever any ambiguity, permanently set the value to `false` to avoid loops in the graph.
+        const value = frameIdToNodeMap.has(record.frameId) ? null : node;
+        frameIdToNodeMap.set(record.frameId, value);
+      }
     });
 
-    return {nodes, idToNodeMap, urlToNodeMap};
+    return {nodes, idToNodeMap, urlToNodeMap, frameIdToNodeMap};
   }
 
   /**
@@ -178,6 +192,23 @@ class PageDependencyGraph {
       cpuNode.addDependent(networkNode);
     }
 
+    /**
+     * If the node has an associated frameId, then create a dependency on the root document request
+     * for the frame. The task obviously couldn't have started before the frame was even downloaded.
+     *
+     * @param {CPUNode} cpuNode
+     * @param {string|undefined} frameId
+     */
+    function addDependencyOnFrame(cpuNode, frameId) {
+      if (!frameId) return;
+      const networkNode = networkNodeOutput.frameIdToNodeMap.get(frameId);
+      if (!networkNode) return;
+      // Ignore all network nodes that started after this CPU task started
+      // A network request that started after could not possibly be required this task
+      if (networkNode.startTime >= cpuNode.startTime) return;
+      cpuNode.addDependency(networkNode);
+    }
+
     /** @param {CPUNode} cpuNode @param {string} url */
     function addDependencyOnUrl(cpuNode, url) {
       if (!url) return;
@@ -230,10 +261,12 @@ class PageDependencyGraph {
 
           case 'InvalidateLayout':
           case 'ScheduleStyleRecalculation':
+            addDependencyOnFrame(node, evt.args.data.frame);
             stackTraceUrls.forEach(url => addDependencyOnUrl(node, url));
             break;
 
           case 'EvaluateScript':
+            addDependencyOnFrame(node, evt.args.data.frame);
             // @ts-ignore - 'EvaluateScript' event means argsUrl is defined.
             addDependencyOnUrl(node, argsUrl);
             stackTraceUrls.forEach(url => addDependencyOnUrl(node, url));
@@ -251,16 +284,19 @@ class PageDependencyGraph {
 
           case 'FunctionCall':
           case 'v8.compile':
+            addDependencyOnFrame(node, evt.args.data.frame);
             // @ts-ignore - events mean argsUrl is defined.
             addDependencyOnUrl(node, argsUrl);
             break;
 
           case 'ParseAuthorStyleSheet':
+            addDependencyOnFrame(node, evt.args.data.frame);
             // @ts-ignore - 'ParseAuthorStyleSheet' event means styleSheetUrl is defined.
             addDependencyOnUrl(node, evt.args.data.styleSheetUrl);
             break;
 
           case 'ResourceSendRequest':
+            addDependencyOnFrame(node, evt.args.data.frame);
             // @ts-ignore - 'ResourceSendRequest' event means requestId is defined.
             addDependentNetworkRequest(node, evt.args.data.requestId);
             stackTraceUrls.forEach(url => addDependencyOnUrl(node, url));
@@ -369,4 +405,5 @@ module.exports = makeComputedArtifact(PageDependencyGraph);
  * @property {Array<NetworkNode>} nodes
  * @property {Map<string, NetworkNode>} idToNodeMap
  * @property {Map<string, Array<NetworkNode>>} urlToNodeMap
+ * @property {Map<string, NetworkNode|null>} frameIdToNodeMap
  */

lighthouse-core/test/computed/metrics/__snapshots__/lantern-interactive-test.js.snap

@@ -7,3 +7,11 @@ Object {
   "timing": 3427,
 }
 `;
+
+exports[`Metrics: Lantern TTI should compute predicted value on iframes with substantial layout 1`] = `
+Object {
+  "optimistic": 6657,
+  "pessimistic": 6692,
+  "timing": 6674,
+}
+`;

lighthouse-core/test/computed/metrics/lantern-interactive-test.js

@@ -10,8 +10,11 @@ const assert = require('assert');
 
 const trace = require('../../fixtures/traces/progressive-app-m60.json');
 const devtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools.log.json');
+const iframeTrace = require('../../fixtures/traces/iframe-m79.trace.json');
+const iframeDevtoolsLog = require('../../fixtures/traces/iframe-m79.devtoolslog.json');
 
 /* eslint-env jest */
+
 describe('Metrics: Lantern TTI', () => {
   it('should compute predicted value', async () => {
     const settings = {};
@@ -29,4 +32,22 @@ describe('Metrics: Lantern TTI', () => {
     assert.ok(result.optimisticGraph, 'should have created optimistic graph');
     assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
   });
+
+  it('should compute predicted value on iframes with substantial layout', async () => {
+    const settings = {};
+    const context = {settings, computedCache: new Map()};
+    const result = await LanternInteractive.request({
+      trace: iframeTrace,
+      devtoolsLog: iframeDevtoolsLog,
+      settings,
+    }, context);
+
+    expect({
+      timing: Math.round(result.timing),
+      optimistic: Math.round(result.optimisticEstimate.timeInMs),
+      pessimistic: Math.round(result.pessimisticEstimate.timeInMs),
+    }).toMatchSnapshot();
+    assert.ok(result.optimisticGraph, 'should have created optimistic graph');
+    assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
+  });
 });

lighthouse-core/test/computed/page-dependency-graph-test.js

@@ -106,6 +106,27 @@ describe('PageDependencyGraph computed artifact:', () => {
       assert.deepEqual(indexedByUrl.get('urlA'), [nodes[0]]);
       assert.deepEqual(indexedByUrl.get('urlB'), [nodes[1], nodes[2]]);
     });
+
+    it('should index nodes by frame', () => {
+      const networkNodeOutput = PageDependencyGraph.getNetworkNodeOutput([
+        {...createRequest(1, 'urlA'), documentURL: 'urlA', frameId: 'A'},
+        {...createRequest(2, 'urlB'), documentURL: 'urlA', frameId: 'A'},
+        {...createRequest(3, 'urlC'), documentURL: 'urlC', frameId: 'C',
+          resourceType: NetworkRequest.TYPES.XHR},
+        {...createRequest(4, 'urlD'), documentURL: 'urlD', frameId: 'D'},
+        {...createRequest(4, 'urlE'), documentURL: 'urlE', frameId: undefined},
+        {...createRequest(4, 'urlF'), documentURL: 'urlF', frameId: 'collision'},
+        {...createRequest(4, 'urlG'), documentURL: 'urlG', frameId: 'collision'},
+      ]);
+
+      const nodes = networkNodeOutput.nodes;
+      const indexedByFrame = networkNodeOutput.frameIdToNodeMap;
+      expect([...indexedByFrame.entries()]).toEqual([
+        ['A', nodes[0]],
+        ['D', nodes[3]],
+        ['collision', null],
+      ]);
+    });
   });
 
   describe('#getCPUNodes', () => {