From c741df0cca8fee85aec67a82d1b2233a74e75535 Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Mon, 12 Jun 2023 12:56:50 -0700
Subject: [PATCH] core(time-to-first-byte): use receiveHeadersStart (#15126)

---
 core/computed/metrics/time-to-first-byte.js      | 14 ++++++++------
 core/lib/network-request.js                      | 16 ++++++++++++++++
 .../computed/metrics/time-to-first-byte-test.js  |  2 +-
 core/test/lib/network-request-test.js            | 13 ++++++++++++-
 4 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/core/computed/metrics/time-to-first-byte.js b/core/computed/metrics/time-to-first-byte.js
index 74ac7ff5b172..40de6b061cae 100644
--- a/core/computed/metrics/time-to-first-byte.js
+++ b/core/computed/metrics/time-to-first-byte.js
@@ -41,15 +41,17 @@ class TimeToFirstByte extends NavigationMetric {
    * @return {Promise<LH.Artifacts.Metric>}
    */
   static async computeObservedMetric(data, context) {
-    const {processedNavigation} = data;
-    const timeOriginTs = processedNavigation.timestamps.timeOrigin;
     const mainResource = await MainResource.request(data, context);
+    if (!mainResource.timing) {
+      throw new Error('missing timing for main resource');
+    }
 
-    // Technically TTFB is the start of the response headers not the end.
-    // That signal isn't available to us so we use header end time as a best guess.
-    const timestamp = mainResource.responseHeadersEndTime * 1000;
+    const {processedNavigation} = data;
+    const timeOriginTs = processedNavigation.timestamps.timeOrigin;
+    const timestampMs =
+      mainResource.timing.requestTime * 1000 + mainResource.timing.receiveHeadersStart;
+    const timestamp = timestampMs * 1000;
     const timing = (timestamp - timeOriginTs) / 1000;
-
     return {timing, timestamp};
   }
 }
diff --git a/core/lib/network-request.js b/core/lib/network-request.js
index 990b64e12c30..f7fa5345c0a5 100644
--- a/core/lib/network-request.js
+++ b/core/lib/network-request.js
@@ -274,6 +274,7 @@ class NetworkRequest {
     }
 
     this._updateResponseHeadersEndTimeIfNecessary();
+    this._backfillReceiveHeaderStartTiming();
     this._updateTransferSizeForLightrider();
     this._updateTimingsForLightrider();
   }
@@ -293,6 +294,7 @@ class NetworkRequest {
     this.localizedFailDescription = data.errorText;
 
     this._updateResponseHeadersEndTimeIfNecessary();
+    this._backfillReceiveHeaderStartTiming();
     this._updateTransferSizeForLightrider();
     this._updateTimingsForLightrider();
   }
@@ -315,6 +317,7 @@ class NetworkRequest {
     this.networkEndTime = data.timestamp * 1000;
 
     this._updateResponseHeadersEndTimeIfNecessary();
+    this._backfillReceiveHeaderStartTiming();
   }
 
   /**
@@ -449,6 +452,19 @@ class NetworkRequest {
     }
   }
 
+  /**
+   * TODO(compat): remove M116.
+   * `timing.receiveHeadersStart` was added recently, and will be in M116. Until then,
+   * set it to receiveHeadersEnd, which is close enough, to allow consumers of NetworkRequest
+   * to use the new field without accounting for this backcompat.
+   */
+  _backfillReceiveHeaderStartTiming() {
+    // Do nothing if a value is already present!
+    if (!this.timing || this.timing.receiveHeadersStart !== undefined) return;
+
+    this.timing.receiveHeadersStart = this.timing.receiveHeadersEnd;
+  }
+
   /**
    * LR gets additional, accurate timing information from its underlying fetch infrastructure.  This
    * is passed in via X-Headers similar to 'X-TotalFetchedSize'.
diff --git a/core/test/computed/metrics/time-to-first-byte-test.js b/core/test/computed/metrics/time-to-first-byte-test.js
index d257d6562c50..e2f111200910 100644
--- a/core/test/computed/metrics/time-to-first-byte-test.js
+++ b/core/test/computed/metrics/time-to-first-byte-test.js
@@ -53,7 +53,7 @@ function mockNetworkRecords() {
     networkRequestTime: 300,
     responseHeadersEndTime: 400,
     networkEndTime: 500,
-    timing: {sendEnd: 0, receiveHeadersEnd: 100},
+    timing: {sendEnd: 0, receiveHeadersStart: 100},
     transferSize: 16_000,
     url: mainDocumentUrl,
     frameId: 'ROOT_FRAME',
diff --git a/core/test/lib/network-request-test.js b/core/test/lib/network-request-test.js
index c22888443ec9..cf963de66851 100644
--- a/core/test/lib/network-request-test.js
+++ b/core/test/lib/network-request-test.js
@@ -13,6 +13,17 @@ describe('NetworkRequest', () => {
     global.isLightrider = undefined;
   });
 
+  it('backcompat for receiveHeadersStart', function() {
+    const req = {
+      timing: {receiveHeadersEnd: 123},
+    };
+    const devtoolsLog = networkRecordsToDevtoolsLog([req]);
+    const record = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
+
+    expect(record.timing.receiveHeadersStart).toEqual(123);
+    expect(record.timing.receiveHeadersEnd).toEqual(123);
+  });
+
   describe('update transfer size for Lightrider', () => {
     function getRequest() {
       return {
@@ -108,7 +119,7 @@ describe('NetworkRequest', () => {
     });
   });
 
-  describe('update fetch stats for Lightrider', () => {
+  describe('update timings for Lightrider', () => {
     function getRequest() {
       return {
         rendererStartTime: 0,