more tweaks to redirect stuff

core/lib/lantern/page-dependency-graph.js

@@ -624,6 +624,21 @@ class PageDependencyGraph {
     return serviceWorkerThreads;
   }
 
+  /**
+   * @param {URL|string} url
+   */
+  static _createParsedUrl(url) {
+    if (typeof url === 'string') {
+      url = new URL(url);
+    }
+    return {
+      scheme: url.protocol.split(':')[0],
+      // Intentional, DevTools uses different terminology
+      host: url.hostname,
+      securityOrigin: url.origin,
+    };
+  }
+
   /**
    * @param {LH.Artifacts.TraceEngineResult} traceEngineResult
    * @param {Map<number, number[]>} serviceWorkerThreads
@@ -643,13 +658,6 @@ class PageDependencyGraph {
       return;
     }
 
-    const parsedURL = {
-      scheme: url.protocol.split(':')[0],
-      // Intentional, DevTools uses different terminology
-      host: url.hostname,
-      securityOrigin: url.origin,
-    };
-
     const timing = request.args.data.timing ? {
       ...request.args.data.timing,
       // These two timings are not included in the trace.
@@ -704,7 +712,7 @@ class PageDependencyGraph {
       connectionReused: request.args.data.connectionReused,
       url: request.args.data.url,
       protocol: request.args.data.protocol,
-      parsedURL,
+      parsedURL: this._createParsedUrl(url),
       documentURL: request.args.data.requestingFrameUrl,
       rendererStartTime: request.ts / 1000,
       networkRequestTime,
@@ -726,7 +734,7 @@ class PageDependencyGraph {
       frameId: request.args.data.frame,
       fromWorker,
       record: request,
-      // Set below.
+      // Set later.
       redirects: undefined,
       redirectSource: undefined,
       redirectDestination: undefined,
@@ -782,16 +790,30 @@ class PageDependencyGraph {
       const redirects = request.record.args.data.redirects;
       if (!redirects.length) continue;
 
-      const requestChain = [request];
-
+      const requestChain = [];
       for (const redirect of redirects) {
         const redirectedRequest = structuredClone(request);
-        redirectedRequest.rendererStartTime = redirect.ts / 1000;
-        redirectedRequest.networkEndTime = (redirect.ts + redirect.dur) / 1000;
+        if (redirectedRequest.timing) {
+          // TODO: These are surely wrong for when there is more than one redirect. Would be
+          // simpler if each redirect remembered it's `timing` object in this `redirects` array.
+          redirectedRequest.timing.requestTime = redirect.ts / 1000 / 1000;
+          redirectedRequest.timing.receiveHeadersStart -= redirect.dur / 1000 / 1000;
+          redirectedRequest.timing.receiveHeadersEnd -= redirect.dur / 1000 / 1000;
+          redirectedRequest.rendererStartTime = redirect.ts / 1000;
+          redirectedRequest.networkRequestTime = redirect.ts / 1000;
+          redirectedRequest.networkEndTime = (redirect.ts + redirect.dur) / 1000;
+          redirectedRequest.responseHeadersEndTime =
+            redirectedRequest.timing.requestTime * 1000 +
+            redirectedRequest.timing.receiveHeadersEnd;
+        }
         redirectedRequest.url = redirect.url;
+        redirectedRequest.parsedURL = this._createParsedUrl(redirect.url);
+        // TODO: TraceEngine is not retaining the actual status code.
+        redirectedRequest.statusCode = 302;
         requestChain.push(redirectedRequest);
         lanternRequests.push(redirectedRequest);
       }
+      requestChain.push(request);
 
       for (let i = 0; i < requestChain.length; i++) {
         const request = requestChain[i];
@@ -806,7 +828,6 @@ class PageDependencyGraph {
 
       // Apply the `:redirect` requestId convention: only redirects[0].requestId is the actual
       // requestId, all the rest have n occurences of `:redirect` as a suffix.
-      requestChain.reverse();
       for (let i = 1; i < requestChain.length; i++) {
         requestChain[i].requestId = `${requestChain[i - 1].requestId}:redirect`;
       }

core/test/audits/largest-contentful-paint-element-test.js

@@ -74,6 +74,7 @@ describe('Performance: largest-contentful-paint-element audit', () => {
   }
 
   it('correctly surfaces the LCP element', async () => {
+    const networkRecords = mockNetworkRecords();
     const artifacts = {
       TraceElements: [{
         traceEventType: 'largest-contentful-paint',
@@ -90,10 +91,11 @@ describe('Performance: largest-contentful-paint-element audit', () => {
         defaultPass: createTestTrace({
           traceEnd: 6000,
           largestContentfulPaint: 8000,
+          networkRecords,
         }),
       },
       devtoolsLogs: {
-        defaultPass: networkRecordsToDevtoolsLog(mockNetworkRecords()),
+        defaultPass: networkRecordsToDevtoolsLog(networkRecords),
       },
       URL: {
         requestedUrl,

core/test/create-test-trace.js

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import {getNormalizedRequestTiming} from './network-records-to-devtools-log.js';
+
 const pid = 1111;
 const tid = 222;
 const browserPid = 13725;
@@ -264,8 +266,15 @@ function createTestTrace(options) {
       willBeRedirected = networkRecords.some(r => r.requestId === redirectedRequestId);
     }
 
-    const willSendTime = (record.rendererStartTime ?? record.networkRequestTime ?? 0) * 10000;
-    const sendTime = (record.networkRequestTime ?? 0) * 10000;
+    const times = getNormalizedRequestTiming(record);
+    const willSendTime = times.rendererStartTime * 1000;
+    const sendTime = times.networkRequestTime * 1000;
+    const recieveResponseTime = times.responseHeadersEndTime * 1000;
+    const endTime = times.networkEndTime * 1000;
+
+    if (times.timing.receiveHeadersStart === undefined) {
+      times.timing.receiveHeadersStart = times.timing.receiveHeadersEnd;
+    }
 
     if (!willBeRedirected) {
       traceEvents.push({
@@ -279,6 +288,8 @@ function createTestTrace(options) {
         args: {
           data: {
             requestId,
+            frame: record.frameId,
+            initiator: record.initiator ?? {type: 'other'},
           },
         },
       });
@@ -310,7 +321,7 @@ function createTestTrace(options) {
 
     traceEvents.push({
       name: 'ResourceReceiveResponse',
-      ts: sendTime,
+      ts: recieveResponseTime,
       pid,
       tid,
       ph: 'I',
@@ -322,11 +333,29 @@ function createTestTrace(options) {
           frame: record.frameId,
           fromCache: record.fromDiskCache || record.fromMemoryCache,
           fromServiceWorker: record.fromWorker,
-          mimeType: record.mimeType,
-          statusCode: record.statusCode,
-          timing: record.timing ?? {},
-          connectionId: record.connectionId ?? 0,
+          mimeType: record.mimeType ?? 'text/html',
+          statusCode: record.statusCode ?? 200,
+          timing: times.timing,
+          connectionId: record.connectionId ?? 140,
           connectionReused: record.connectionReused ?? false,
+          protocol: record.protocol ?? 'http/1.1',
+        },
+      },
+    });
+
+    traceEvents.push({
+      name: 'ResourceFinish',
+      ts: endTime,
+      pid,
+      tid,
+      ph: 'I',
+      cat: 'devtools.timeline',
+      dur: 0,
+      args: {
+        data: {
+          requestId,
+          frame: record.frameId,
+          finishTime: endTime / 1000 / 1000,
           encodedDataLength: record.transferSize,
           decodedBodyLength: record.resourceSize,
         },

core/test/network-records-to-devtools-log.js

@@ -284,13 +284,13 @@ function getResponseReceivedEvent(networkRecord, index, normalizedTiming) {
         status: networkRecord.statusCode || 200,
         headers,
         mimeType: typeof networkRecord.mimeType === 'string' ? networkRecord.mimeType : 'text/html',
-        connectionReused: networkRecord.connectionReused || false,
+        connectionReused: networkRecord.connectionReused ?? false,
         connectionId: networkRecord.connectionId ?? 140,
-        fromDiskCache: networkRecord.fromDiskCache || false,
-        fromServiceWorker: networkRecord.fetchedViaServiceWorker || false,
-        encodedDataLength: networkRecord.responseHeadersTransferSize || 0,
+        fromDiskCache: networkRecord.fromDiskCache ?? false,
+        fromServiceWorker: networkRecord.fetchedViaServiceWorker ?? false,
+        encodedDataLength: networkRecord.responseHeadersTransferSize ?? 0,
         timing: {...normalizedTiming.timing},
-        protocol: networkRecord.protocol || 'http/1.1',
+        protocol: networkRecord.protocol ?? 'http/1.1',
       },
       frameId: networkRecord.frameId,
     },
@@ -479,4 +479,7 @@ function networkRecordsToDevtoolsLog(networkRecords, options = {}) {
   return devtoolsLog;
 }
 
-export {networkRecordsToDevtoolsLog};
+export {
+  networkRecordsToDevtoolsLog,
+  getNormalizedRequestTiming,
+};

types/artifacts.d.ts

@@ -955,6 +955,9 @@ export interface TraceEvent {
       connectionReused?: boolean;
       encodedDataLength?: number;
       decodedBodyLength?: number;
+      initiator?: LH.Crdp.Network.Initiator;
+      protocol?: string;
+      finishTime?: number;
     };
     frame?: string;
     name?: string;