Repository: GoogleChrome/lighthouse
Pull request #8752 (2019-04-30 20:12:29)

clients(lr): revert Lightrider timings applied to NetworkRequests
This reverts commit ca1731aba7df31b16f68abc0de23f9fffac38efd, which _may have_, _slightly_, _kinda_, messed up some performance data.  Or whatever, I mean you never know.



Patch:
diff --git a/docs/releasing.md b/docs/releasing.md
index a1a026f0e..eabdf233d 100644
--- a/docs/releasing.md
+++ b/docs/releasing.md
@@ -32,17 +32,17 @@ We follow [semver](https://semver.org/) versioning semantics (`vMajor.Minor.Patc
 
 ```sh
 # Run the tests
-bash ./lighthouse-core/scripts/release/release-test.sh
+bash ./lighthouse-core/scripts/release/test.sh
 # Prepare the commit, replace x.x.x with the desired version
-bash ./lighthouse-core/scripts/release/release-prepare-commit.sh x.x.x
+bash ./lighthouse-core/scripts/release/prepare-commit.sh x.x.x
 
 # Open the PR and await merge...
 echo "It's been merged! 🎉"
 
 # Run the tests again :)
-bash ./lighthouse-core/scripts/release/release-test.sh
+bash ./lighthouse-core/scripts/release/test.sh
 # Package everything for publishing
-bash ./lighthouse-core/scripts/release/release-prepare-package.sh
+bash ./lighthouse-core/scripts/release/prepare-package.sh
 
 # Upload the extension
 node build/build-extension.js package; cd dist/extension-package/
diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js
index b93c7eeb1..c1be94fed 100644
--- a/lighthouse-core/lib/network-request.js
+++ b/lighthouse-core/lib/network-request.js
@@ -428,41 +428,8 @@ class NetworkRequest {
       return;
     }
 
-    // Init timing.
-    this.timing = {
-      requestTime: this.startTime,
-      proxyStart: -1,
-      proxyEnd: -1,
-      dnsStart: -1,
-      dnsEnd: -1,
-      connectStart: -1,
-      connectEnd: -1,
-      sslStart: -1,
-      sslEnd: -1,
-      workerStart: -1,
-      workerReady: -1,
-      sendStart: -1,
-      sendEnd: -1,
-      pushStart: -1,
-      pushEnd: -1,
-      receiveHeadersEnd: -1,
-    };
-
-    const origEnd = this.endTime;
-    // `this.endTime` and `this.responseReceivedTime` are in seconds, so convert from milliseconds.
-    this.endTime = this.startTime + (totalMs / 1000);
-    this.responseReceivedTime = this.startTime + ((TCPMs + requestMs) / 1000);
-
-    this.timing.connectStart = 0;
-    this.timing.connectEnd = TCPMs;
-    this.timing.sslStart = TCPMs - SSLMs;
-    this.timing.sslEnd = TCPMs;
-    this.timing.sendStart = TCPMs;
-    this.timing.sendEnd = TCPMs;
-    this.timing.receiveHeadersEnd = TCPMs + requestMs;
-
     this.lrStatistics = {
-      endTimeDeltaMs: (origEnd - this.endTime) * 1000,
+      endTimeDeltaMs: (this.endTime - (this.startTime + (totalMs / 1000))) * 1000,
       TCPMs: TCPMs,
       requestMs: requestMs,
       responseMs: responseMs,
diff --git a/lighthouse-core/scripts/release/publish-clean-pristine.sh b/lighthouse-core/scripts/release/clean-pristine.sh
similarity index 100%
rename from lighthouse-core/scripts/release/publish-clean-pristine.sh
rename to lighthouse-core/scripts/release/clean-pristine.sh
diff --git a/lighthouse-core/scripts/release/publish-prepare-commit.sh b/lighthouse-core/scripts/release/prepare-commit.sh
similarity index 100%
rename from lighthouse-core/scripts/release/publish-prepare-commit.sh
rename to lighthouse-core/scripts/release/prepare-commit.sh
diff --git a/lighthouse-core/scripts/release/publish-prepare-package.sh b/lighthouse-core/scripts/release/prepare-package.sh
similarity index 94%
rename from lighthouse-core/scripts/release/publish-prepare-package.sh
rename to lighthouse-core/scripts/release/prepare-package.sh
index c5398c5d7..ac175968e 100755
--- a/lighthouse-core/scripts/release/publish-prepare-package.sh
+++ b/lighthouse-core/scripts/release/prepare-package.sh
@@ -5,7 +5,7 @@ LH_PRISTINE_ROOT="$DIRNAME/../../../../lighthouse-pristine"
 
 set -euxo pipefail
 
-bash "$DIRNAME/release-prepare-pristine.sh"
+bash "$DIRNAME/prepare-pristine.sh"
 
 cd "$LH_PRISTINE_ROOT"
 


Test patch:
diff --git a/lighthouse-core/scripts/release/publish-prepare-pristine.sh b/lighthouse-core/scripts/release/prepare-pristine.sh
similarity index 100%
rename from lighthouse-core/scripts/release/publish-prepare-pristine.sh
rename to lighthouse-core/scripts/release/prepare-pristine.sh
diff --git a/lighthouse-core/scripts/release/publish-test.sh b/lighthouse-core/scripts/release/test.sh
similarity index 97%
rename from lighthouse-core/scripts/release/publish-test.sh
rename to lighthouse-core/scripts/release/test.sh
index 2a5b52765..271a08346 100755
--- a/lighthouse-core/scripts/release/publish-test.sh
+++ b/lighthouse-core/scripts/release/test.sh
@@ -9,7 +9,7 @@ LH_PRISTINE_ROOT="$DIRNAME/../../../../lighthouse-pristine"
 
 set -euxo pipefail
 
-bash "$DIRNAME/release-prepare-pristine.sh"
+bash "$DIRNAME/prepare-pristine.sh"
 
 cd "$LH_PRISTINE_ROOT"
 
diff --git a/lighthouse-core/test/lib/network-request-test.js b/lighthouse-core/test/lib/network-request-test.js
index 5abdfa5d6..542f676e4 100644
--- a/lighthouse-core/test/lib/network-request-test.js
+++ b/lighthouse-core/test/lib/network-request-test.js
@@ -90,7 +90,7 @@ describe('NetworkRequest', () => {
       };
     }
 
-    it('updates timings if in Lightrider', () => {
+    it('updates lrStatistics if in Lightrider', () => {
       const req = getRequest();
 
       const devtoolsLog = networkRecordsToDevtoolsLog([req]);
@@ -98,17 +98,8 @@ describe('NetworkRequest', () => {
       const record = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
 
       expect(record.startTime).toStrictEqual(0);
-      expect(record.endTime).toStrictEqual(10);
-      expect(record.responseReceivedTime).toStrictEqual(7.5);
-      expect(record.timing).toMatchObject({
-        connectStart: 0,
-        connectEnd: 5000,
-        sslStart: 4000,
-        sslEnd: 5000,
-        sendStart: 5000,
-        sendEnd: 5000,
-        receiveHeadersEnd: 7500,
-      });
+      expect(record.endTime).toStrictEqual(2);
+      expect(record.responseReceivedTime).toStrictEqual(1);
       expect(record.lrStatistics).toStrictEqual({
         endTimeDeltaMs: -8000,
         TCPMs: 5000,
@@ -198,18 +189,6 @@ describe('NetworkRequest', () => {
       global.isLightrider = true;
       const record = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
 
-      expect(record.startTime).toStrictEqual(0);
-      expect(record.endTime).toStrictEqual(10);
-      expect(record.responseReceivedTime).toStrictEqual(0);
-      expect(record.timing).toMatchObject({
-        connectStart: 0,
-        connectEnd: 0,
-        sslStart: 0,
-        sslEnd: 0,
-        sendStart: 0,
-        sendEnd: 0,
-        receiveHeadersEnd: 0,
-      });
       expect(record.lrStatistics).toStrictEqual({
         endTimeDeltaMs: -8000,
         TCPMs: 0,
@@ -228,18 +207,6 @@ describe('NetworkRequest', () => {
       global.isLightrider = true;
       const record = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
 
-      expect(record.startTime).toStrictEqual(0);
-      expect(record.endTime).toStrictEqual(10);
-      expect(record.responseReceivedTime).toStrictEqual(1);
-      expect(record.timing).toMatchObject({
-        connectStart: 0,
-        connectEnd: 1000,
-        sslStart: 1000,
-        sslEnd: 1000,
-        sendStart: 1000,
-        sendEnd: 1000,
-        receiveHeadersEnd: 1000,
-      });
       expect(record.lrStatistics).toStrictEqual({
         endTimeDeltaMs: -8000,
         TCPMs: 1000,
@@ -248,40 +215,7 @@ describe('NetworkRequest', () => {
       });
     });
 
-    it('created a new timing property if one did not exist', function() {
-      const req = getRequest();
-
-      const devtoolsLog = networkRecordsToDevtoolsLog([req]);
-
-      const noLRRecord = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
-      expect(noLRRecord.timing).toStrictEqual(undefined);
-
-      global.isLightrider = true;
-      const lrRecord = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
-      expect(lrRecord.timing.proxyStart).toStrictEqual(-1);
-
-      expect(lrRecord.startTime).toStrictEqual(0);
-      expect(lrRecord.endTime).toStrictEqual(10);
-      expect(lrRecord.responseReceivedTime).toStrictEqual(7.5);
-      expect(lrRecord.timing).toMatchObject({
-        proxyStart: -1,
-        connectStart: 0,
-        connectEnd: 5000,
-        sslStart: 4000,
-        sslEnd: 5000,
-        sendStart: 5000,
-        sendEnd: 5000,
-        receiveHeadersEnd: 7500,
-      });
-      expect(lrRecord.lrStatistics).toStrictEqual({
-        endTimeDeltaMs: -8000,
-        TCPMs: 5000,
-        requestMs: 2500,
-        responseMs: 2500,
-      });
-    });
-
-    it('overrides existing timing properties', function() {
+    it('does not override existing timing properties', function() {
       const req = getRequest();
       req.timing = {proxyStart: 17, sslStart: 35};
       const devtoolsLog = networkRecordsToDevtoolsLog([req]);
@@ -293,18 +227,9 @@ describe('NetworkRequest', () => {
       global.isLightrider = true;
       const lrRecord = NetworkRecorder.recordsFromLogs(devtoolsLog)[0];
 
-      expect(lrRecord.startTime).toStrictEqual(0);
-      expect(lrRecord.endTime).toStrictEqual(10);
-      expect(lrRecord.responseReceivedTime).toStrictEqual(7.5);
       expect(lrRecord.timing).toMatchObject({
-        proxyStart: -1,
-        connectStart: 0,
-        connectEnd: 5000,
-        sslStart: 4000,
-        sslEnd: 5000,
-        sendStart: 5000,
-        sendEnd: 5000,
-        receiveHeadersEnd: 7500,
+        proxyStart: 17,
+        sslStart: 35,
       });
       expect(lrRecord.lrStatistics).toStrictEqual({
         endTimeDeltaMs: -8000,