core(network): fix time units in network quiet calc (#16013)

Co-authored-by: Paul Irish <paulirish@google.com>
GoogleChrome · May 24, 2024 · 8d1d78b · 8d1d78b
1 parent c73113d
commit 8d1d78b
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 32 deletions.
diff --git a/core/computed/metrics/interactive.js b/core/computed/metrics/interactive.js
@@ -187,6 +187,6 @@ export {InteractiveComputed as Interactive};
 
 /**
  * @typedef TimePeriod
- * @property {number} start
- * @property {number} end
+ * @property {number} start In ms.
+ * @property {number} end In ms.
  */
diff --git a/core/gather/driver/network-monitor.js b/core/gather/driver/network-monitor.js
@@ -202,9 +202,10 @@ class NetworkMonitor extends NetworkMonitorEventEmitter {
   /**
    * Finds all time periods where the number of inflight requests is less than or equal to the
    * number of allowed concurrent requests.
+   * The time periods returned are in ms.
    * @param {Array<LH.Artifacts.NetworkRequest>} requests
    * @param {number} allowedConcurrentRequests
-   * @param {number=} endTime
+   * @param {number=} endTime In ms
    * @return {Array<{start: number, end: number}>}
    */
   static findNetworkQuietPeriods(requests, allowedConcurrentRequests, endTime = Infinity) {
@@ -215,10 +216,9 @@ class NetworkMonitor extends NetworkMonitorEventEmitter {
       if (UrlUtils.isNonNetworkProtocol(request.protocol)) return;
       if (request.protocol === 'ws' || request.protocol === 'wss') return;
 
-      // convert the network timestamp to ms
-      timeBoundaries.push({time: request.networkRequestTime * 1000, isStart: true});
+      timeBoundaries.push({time: request.networkRequestTime, isStart: true});
       if (request.finished) {
-        timeBoundaries.push({time: request.networkEndTime * 1000, isStart: false});
+        timeBoundaries.push({time: request.networkEndTime, isStart: false});
       }
     });
 

diff --git a/core/test/computed/metrics/interactive-test.js b/core/test/computed/metrics/interactive-test.js
@@ -37,8 +37,8 @@ function generateNetworkRecords(partialRecords, timeOrigin) {
       statusCode: item.statusCode || 200,
       requestMethod: item.requestMethod || 'GET',
       finished: typeof item.finished === 'undefined' ? true : item.finished,
-      networkRequestTime: (item.start + timeOriginInMs) / 1000,
-      networkEndTime: item.end === -1 ? -1 : (item.end + timeOriginInMs) / 1000,
+      networkRequestTime: item.start + timeOriginInMs,
+      networkEndTime: item.end === -1 ? -1 : item.end + timeOriginInMs,
     };
     return /** @type {LH.Artifacts.NetworkRequest} */ (record);
   });

diff --git a/core/test/gather/driver/network-monitor-test.js b/core/test/gather/driver/network-monitor-test.js
@@ -381,9 +381,9 @@ describe('NetworkMonitor', () => {
 
     it('should find the 0-quiet periods', () => {
       const records = [
-        record({networkRequestTime: 0, networkEndTime: 1}),
-        record({networkRequestTime: 2, networkEndTime: 3}),
-        record({networkRequestTime: 4, networkEndTime: 5}),
+        record({networkRequestTime: 0, networkEndTime: 1000}),
+        record({networkRequestTime: 2000, networkEndTime: 3000}),
+        record({networkRequestTime: 4000, networkEndTime: 5000}),
       ];
 
       const periods = NetworkMonitor.findNetworkQuietPeriods(records, 0);
@@ -392,15 +392,22 @@ describe('NetworkMonitor', () => {
         {start: 3000, end: 4000},
         {start: 5000, end: Infinity},
       ]);
+      // Same thing but verifying these numbers round trip without a problem.
+      expect(periods).toEqual([
+        // The time between the first two, and so on…
+        {start: records[0].networkEndTime, end: records[1].networkRequestTime},
+        {start: records[1].networkEndTime, end: records[2].networkRequestTime},
+        {start: records[2].networkEndTime, end: Infinity},
+      ]);
     });
 
     it('should find the 2-quiet periods', () => {
       const records = [
-        record({networkRequestTime: 0, networkEndTime: 1.5}),
-        record({networkRequestTime: 0, networkEndTime: 2}),
-        record({networkRequestTime: 0, networkEndTime: 2.5}),
-        record({networkRequestTime: 2, networkEndTime: 3}),
-        record({networkRequestTime: 4, networkEndTime: 5}),
+        record({networkRequestTime: 0, networkEndTime: 1500}),
+        record({networkRequestTime: 0, networkEndTime: 2000}),
+        record({networkRequestTime: 0, networkEndTime: 2500}),
+        record({networkRequestTime: 2000, networkEndTime: 3000}),
+        record({networkRequestTime: 4000, networkEndTime: 5000}),
       ];
 
       const periods = NetworkMonitor.findNetworkQuietPeriods(records, 2);
@@ -409,14 +416,14 @@ describe('NetworkMonitor', () => {
 
     it('should handle unfinished requests', () => {
       const records = [
-        record({networkRequestTime: 0, networkEndTime: 1.5}),
-        record({networkRequestTime: 0, networkEndTime: 2}),
-        record({networkRequestTime: 0, networkEndTime: 2.5}),
-        record({networkRequestTime: 2, networkEndTime: 3}),
-        record({networkRequestTime: 2}),
-        record({networkRequestTime: 2}),
-        record({networkRequestTime: 4, networkEndTime: 5}),
-        record({networkRequestTime: 5.5}),
+        record({networkRequestTime: 0, networkEndTime: 1500}),
+        record({networkRequestTime: 0, networkEndTime: 2000}),
+        record({networkRequestTime: 0, networkEndTime: 2500}),
+        record({networkRequestTime: 2000, networkEndTime: 3000}),
+        record({networkRequestTime: 2000}),
+        record({networkRequestTime: 2000}),
+        record({networkRequestTime: 4000, networkEndTime: 5000}),
+        record({networkRequestTime: 5500}),
       ];
 
       const periods = NetworkMonitor.findNetworkQuietPeriods(records, 2);
@@ -429,8 +436,8 @@ describe('NetworkMonitor', () => {
 
     it('should ignore data URIs', () => {
       const records = [
-        record({networkRequestTime: 0, networkEndTime: 1}),
-        record({networkRequestTime: 0, networkEndTime: 2, url: 'data:image/png;base64,',
+        record({networkRequestTime: 0, networkEndTime: 1000}),
+        record({networkRequestTime: 0, networkEndTime: 2000, url: 'data:image/png;base64,',
           protocol: 'data'}),
       ];
 
@@ -443,12 +450,12 @@ describe('NetworkMonitor', () => {
         finished: false,
         url: 'https://iframe.com',
         documentURL: 'https://iframe.com',
-        responseHeadersEndTime: 1.2,
+        responseHeadersEndTime: 1200,
       };
 
       const records = [
-        record({networkRequestTime: 0, networkEndTime: 1}),
-        record({networkRequestTime: 0, networkEndTime: 1.2, ...iframeRequest}),
+        record({networkRequestTime: 0, networkEndTime: 1000}),
+        record({networkRequestTime: 0, networkEndTime: 1200, ...iframeRequest}),
       ];
 
       const periods = NetworkMonitor.findNetworkQuietPeriods(records, 0);
@@ -460,12 +467,12 @@ describe('NetworkMonitor', () => {
       const quicRequest = {
         finished: false,
         responseHeaders: [{name: 'ALT-SVC', value: 'hq=":49288";quic="1,1abadaba,51303334,0"'}],
-        timing: /** @type {*} */ ({receiveHeadersEnd: 1.28}),
+        timing: /** @type {*} */ ({receiveHeadersEnd: 1280}),
       };
 
       const records = [
-        record({networkRequestTime: 0, networkEndTime: 1}),
-        record({networkRequestTime: 0, networkEndTime: 2, ...quicRequest}),
+        record({networkRequestTime: 0, networkEndTime: 1000}),
+        record({networkRequestTime: 0, networkEndTime: 2000, ...quicRequest}),
       ];
 
       const periods = NetworkMonitor.findNetworkQuietPeriods(records, 0);