core(lantern): add edges from initiatorRequest when there are duplica…

…te records (#10097)

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

@@ -139,21 +139,27 @@ class PageDependencyGraph {
   }
 
   /**
-   * @param {Node} rootNode
+   * @param {NetworkNode} rootNode
    * @param {NetworkNodeOutput} networkNodeOutput
    */
   static linkNetworkNodes(rootNode, networkNodeOutput) {
     networkNodeOutput.nodes.forEach(node => {
+      const directInitiatorRequest = node.record.initiatorRequest || rootNode.record;
+      const directInitiatorNode =
+        networkNodeOutput.idToNodeMap.get(directInitiatorRequest.requestId) || rootNode;
       const initiators = PageDependencyGraph.getNetworkInitiators(node.record);
       if (initiators.length) {
         initiators.forEach(initiator => {
-          const parentCandidates = networkNodeOutput.urlToNodeMap.get(initiator) || [rootNode];
-          // Only add the initiator relationship if the initiator is unambiguous
-          const parent = parentCandidates.length === 1 ? parentCandidates[0] : rootNode;
-          node.addDependency(parent);
+          const parentCandidates = networkNodeOutput.urlToNodeMap.get(initiator) || [];
+          // Only add the edge if the parent is unambiguous with valid timing.
+          if (parentCandidates.length === 1 && parentCandidates[0].startTime <= node.startTime) {
+            node.addDependency(parentCandidates[0]);
+          } else {
+            directInitiatorNode.addDependent(node);
+          }
         });
-      } else if (node !== rootNode) {
-        rootNode.addDependent(node);
+      } else if (node !== directInitiatorNode) {
+        directInitiatorNode.addDependent(node);
       }
 
       if (!node.record.redirects) return;

lighthouse-core/lib/network-recorder.js

@@ -327,6 +327,42 @@ class NetworkRecorder extends EventEmitter {
     return request;
   }
 
+  /**
+   * @param {NetworkRequest} record The record to find the initiator of
+   * @param {Map<string, NetworkRequest[]>} recordsByURL
+   * @return {NetworkRequest|null}
+   * @private
+   */
+  static _chooseInitiator(record, recordsByURL) {
+    if (record.redirectSource) {
+      return record.redirectSource;
+    }
+    const stackFrames = (record.initiator.stack && record.initiator.stack.callFrames) || [];
+    const initiatorURL = record.initiator.url || (stackFrames[0] && stackFrames[0].url);
+
+    let candidates = recordsByURL.get(initiatorURL) || [];
+    // The initiator must come before the initiated request.
+    candidates = candidates.filter(cand => cand.responseReceivedTime <= record.startTime);
+    if (candidates.length > 1) {
+      // Disambiguate based on resource type. Prefetch requests have type 'Other' and cannot
+      // initiate requests, so we drop them here.
+      const nonPrefetchCandidates = candidates.filter(
+          cand => cand.resourceType !== NetworkRequest.TYPES.Other);
+      if (nonPrefetchCandidates.length) {
+        candidates = nonPrefetchCandidates;
+      }
+    }
+    if (candidates.length > 1) {
+      // Disambiguate based on frame. It's likely that the initiator comes from the same frame.
+      const sameFrameCandidates = candidates.filter(cand => cand.frameId === record.frameId);
+      if (sameFrameCandidates.length) {
+        candidates = sameFrameCandidates;
+      }
+    }
+
+    return candidates.length ? candidates[0] : null;
+  }
+
   /**
    * Construct network records from a log of devtools protocol messages.
    * @param {LH.DevtoolsLog} devtoolsLog
@@ -340,20 +376,17 @@ class NetworkRecorder extends EventEmitter {
     // get out the list of records & filter out invalid records
     const records = networkRecorder.getRecords().filter(record => record.isValid);
 
-    // create a map of all the records by URL to link up initiator
+    /** @type {Map<string, NetworkRequest[]>} */
     const recordsByURL = new Map();
     for (const record of records) {
-      if (recordsByURL.has(record.url)) continue;
-      recordsByURL.set(record.url, record);
+      const records = recordsByURL.get(record.url) || [];
+      records.push(record);
+      recordsByURL.set(record.url, records);
     }
 
     // set the initiator and redirects array
     for (const record of records) {
-      const stackFrames = (record.initiator.stack && record.initiator.stack.callFrames) || [];
-      const initiatorURL = record.initiator.url || (stackFrames[0] && stackFrames[0].url);
-      // If we were redirected to this request, our initiator is that redirect, otherwise, it's the
-      // initiator provided by the protocol. See https://github.com/GoogleChrome/lighthouse/pull/7352/
-      const initiator = record.redirectSource || recordsByURL.get(initiatorURL);
+      const initiator = NetworkRecorder._chooseInitiator(record, recordsByURL);
       if (initiator) {
         record.setInitiatorRequest(initiator);
       }

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

@@ -483,6 +483,57 @@ describe('PageDependencyGraph computed artifact:', () => {
       assert.deepEqual(nodes[3].getDependencies(), [nodes[1], nodes[2]]);
     });
 
+    it('should link up script initiators only when timing is valid', () => {
+      const request1 = createRequest(1, '1', 0);
+      const request2 = createRequest(2, '2', 500);
+      const request3 = createRequest(3, '3', 500);
+      const request4 = createRequest(4, '4', 20);
+      request4.initiator = {
+        type: 'script',
+        stack: {callFrames: [{url: '2'}], parent: {parent: {callFrames: [{url: '3'}]}}},
+      };
+      const networkRecords = [request1, request2, request3, request4];
+
+      addTaskEvents(0, 0, []);
+
+      const graph = PageDependencyGraph.createGraph(traceOfTab, networkRecords);
+      const nodes = [];
+      graph.traverse(node => nodes.push(node));
+
+      assert.equal(nodes.length, 4);
+      assert.deepEqual(nodes.map(node => node.id), [1, 2, 3, 4]);
+      assert.deepEqual(nodes[0].getDependencies(), []);
+      assert.deepEqual(nodes[1].getDependencies(), [nodes[0]]);
+      assert.deepEqual(nodes[2].getDependencies(), [nodes[0]]);
+      assert.deepEqual(nodes[3].getDependencies(), [nodes[0]]);
+    });
+
+    it('should link up script initiators with prefetch requests', () => {
+      const request1 = createRequest(1, 'a.com/1', 0);
+      const request2Prefetch = createRequest(2, 'a.com/js', 5);
+      const request2Fetch = createRequest(3, 'a.com/js', 10);
+      const request3 = createRequest(4, 'a.com/4', 20);
+      request3.initiator = {
+        type: 'script',
+        stack: {callFrames: [{url: 'a.com/js'}], parent: {parent: {callFrames: [{url: 'js'}]}}},
+      };
+      request3.initiatorRequest = request2Fetch;
+      const networkRecords = [request1, request2Prefetch, request2Fetch, request3];
+
+      addTaskEvents(0, 0, []);
+
+      const graph = PageDependencyGraph.createGraph(traceOfTab, networkRecords);
+      const nodes = [];
+      graph.traverse(node => nodes.push(node));
+
+      assert.equal(nodes.length, 4);
+      assert.deepEqual(nodes.map(node => node.id), [1, 2, 3, 4]);
+      assert.deepEqual(nodes[0].getDependencies(), []);
+      assert.deepEqual(nodes[1].getDependencies(), [nodes[0]]);
+      assert.deepEqual(nodes[2].getDependencies(), [nodes[0]]);
+      assert.deepEqual(nodes[3].getDependencies(), [nodes[2]]);
+    });
+
     it('should throw when root node is not related to main document', () => {
       const request1 = createRequest(1, '1', 0, null, NetworkRequest.TYPES.Other);
       const request2 = createRequest(2, '2', 5, null, NetworkRequest.TYPES.Document);

lighthouse-core/test/fixtures/lantern-master-accuracy.json

@@ -5,28 +5,28 @@
     "p95": 0.5795269168026101
   },
   "roughEstimateOfFMP": {
-    "p50": 0.3129196884233145,
+    "p50": 0.3114278173660596,
     "p90": 0.5385564466378778,
     "p95": 0.6641924269472893
   },
   "roughEstimateOfSI": {
     "p50": 0.27011225444340503,
     "p90": 0.6267744048918978,
-    "p95": 0.9847542003733665
+    "p95": 0.9915404288805824
   },
   "roughEstimateOfTTFCPUI": {
-    "p50": 0.307951137444225,
-    "p90": 0.6159518516510329,
-    "p95": 0.7404564989707104
+    "p50": 0.2951503181917928,
+    "p90": 0.6422540616604836,
+    "p95": 0.9250387487670847
   },
   "roughEstimateOfTTI": {
-    "p50": 0.3131914893617021,
+    "p50": 0.31149493746277546,
     "p90": 0.6774419418835337,
     "p95": 0.7467400959317818
   },
   "roughEstimateOfLCP": {
-    "p50": 0.2813953488372093,
-    "p90": 0.9142573930282505,
+    "p50": 0.30685920577617326,
+    "p90": 0.9659546061415221,
     "p95": 1.6544450025113009
   }
 }