refactor: makes non-finished network records available (#2197)

* refactor: makes non-finished network records available internally * update comments
GoogleChrome · May 9, 2017 · fb3cfbd · fb3cfbd
1 parent efaeb13
commit fb3cfbd
Show file tree

Hide file tree

Showing 13 changed files with 69 additions and 14 deletions.
diff --git a/lighthouse-core/audits/byte-efficiency/total-byte-weight.js b/lighthouse-core/audits/byte-efficiency/total-byte-weight.js
@@ -61,7 +61,8 @@ class TotalByteWeight extends ByteEfficiencyAudit {
         let results = [];
         networkRecords.forEach(record => {
           // exclude data URIs since their size is reflected in other resources
-          if (record.scheme === 'data') return;
+          // exclude unfinished requests since they won't have transfer size information
+          if (record.scheme === 'data' || !record.finished) return;
 
           const result = {
             url: URL.getDisplayName(record.url),

diff --git a/lighthouse-core/audits/load-fast-enough-for-pwa.js b/lighthouse-core/audits/load-fast-enough-for-pwa.js
@@ -55,8 +55,9 @@ class LoadFastEnough4Pwa extends Audit {
       const allRequestLatencies = networkRecords.map(record => {
         // Ignore requests that don't have timing data or resources that have
         // previously been requested and are coming from the cache.
+        // Also ignore unfinished requests since they won't have timing information.
         const fromCache = record._fromDiskCache || record._fromMemoryCache;
-        if (!record._timing || fromCache) {
+        if (!record._timing || fromCache || !record.finished) {
           return undefined;
         }
 

diff --git a/lighthouse-core/gather/computed/critical-request-chains.js b/lighthouse-core/gather/computed/critical-request-chains.js
@@ -55,6 +55,8 @@ class CriticalRequestChains extends ComputedArtifact {
   }
 
   compute_(networkRecords) {
+    networkRecords = networkRecords.filter(req => req.finished);
+
     // Build a map of requestID -> Node.
     const requestIdToRequests = new Map();
     for (const request of networkRecords) {

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
@@ -73,7 +73,7 @@ class OptimizedImages extends Gatherer {
   static filterImageRequests(pageUrl, networkRecords) {
     const seenUrls = new Set();
     return networkRecords.reduce((prev, record) => {
-      if (seenUrls.has(record._url)) {
+      if (seenUrls.has(record._url) || !record.finished) {
         return prev;
       }
 

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js
@@ -36,7 +36,7 @@ class ResponseCompression extends Gatherer {
 
     networkRecords.forEach(record => {
       const isTextBasedResource = record.resourceType() && record.resourceType().isTextType();
-      if (!isTextBasedResource || !record.resourceSize) {
+      if (!isTextBasedResource || !record.resourceSize || !record.finished) {
         return;
       }
 

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js b/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js
@@ -93,6 +93,10 @@ function collectTagsThatBlockFirstPaint() {
 
 function filteredAndIndexedByUrl(networkRecords) {
   return networkRecords.reduce((prev, record) => {
+    if (!record.finished) {
+      return prev;
+    }
+
     const isParserGenerated = record._initiator.type === 'parser';
     // A stylesheet only blocks script if it was initiated by the parser
     // https://html.spec.whatwg.org/multipage/semantics.html#interactions-of-styling-and-scripting

diff --git a/lighthouse-core/gather/gatherers/image-usage.js b/lighthouse-core/gather/gatherers/image-usage.js
@@ -125,7 +125,7 @@ class ImageUsage extends Gatherer {
   afterPass(options, traceData) {
     const driver = this.driver = options.driver;
     const indexedNetworkRecords = traceData.networkRecords.reduce((map, record) => {
-      if (/^image/.test(record._mimeType)) {
+      if (/^image/.test(record._mimeType) && record.finished) {
         map[record._url] = {
           url: record.url,
           resourceSize: record.resourceSize,

diff --git a/lighthouse-core/lib/network-recorder.js b/lighthouse-core/lib/network-recorder.js
@@ -57,8 +57,9 @@ class NetworkRecorder extends EventEmitter {
    * web socket and normal request creation.
    * @private
    */
-  onRequestStarted() {
+  onRequestStarted(request) {
     this.startedRequestCount++;
+    this._records.push(request.data);
 
     const activeCount = this.activeRequestCount();
     log.verbose('NetworkRecorder', `Request started. ${activeCount} requests in progress` +
@@ -80,7 +81,6 @@ class NetworkRecorder extends EventEmitter {
    */
   onRequestFinished(request) {
     this.finishedRequestCount++;
-    this._records.push(request.data);
     this.emit('requestloaded', request.data);
 
     const activeCount = this.activeRequestCount();

diff --git a/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js b/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js
@@ -56,10 +56,10 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {
   it('fails a good TTI value with no throttling', () => {
     // latencies are very short
     const mockNetworkRecords = [
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}},
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 75}},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: 75}, finished: true},
       { },
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true},
     ];
     return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
       assert.equal(result.rawValue, false);
@@ -81,10 +81,11 @@ describe('PWA: load-fast-enough-for-pwa audit', () => {
   it('passes a good TTI value and WITH throttling', () => {
     // latencies are very long
     const mockNetworkRecords = [
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 250}},
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 175}},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: 175}, finished: true},
       { },
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 250}},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true},
+      {_timing: {sendEnd: 0, receiveHeadersEnd: -1}, finished: false}, // ignored for not finishing
     ];
     return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
       assert.equal(result.rawValue, true);

diff --git a/lighthouse-core/test/gather/computed/critical-request-chains-test.js b/lighthouse-core/test/gather/computed/critical-request-chains-test.js
@@ -33,6 +33,7 @@ function mockTracingData(prioritiesList, edges) {
         _resourceType: {
           _category: 'fake'
         },
+        finished: true,
         priority: () => priority,
         initiatorRequest: () => null
       }));
@@ -59,7 +60,7 @@ function constructEmptyRequest() {
     responseReceivedTime: undefined,
     startTime: undefined,
     url: undefined,
-    transferSize: undefined
+    transferSize: undefined,
   };
 }
 

diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js
@@ -33,36 +33,49 @@ const traceData = {
       _url: 'http://google.com/image.jpg',
       _mimeType: 'image/jpeg',
       _resourceSize: 10,
+      finished: true,
     },
     {
       _url: 'http://google.com/transparent.png',
       _mimeType: 'image/png',
       _resourceSize: 11,
+      finished: true,
     },
     {
       _url: 'http://google.com/image.bmp',
       _mimeType: 'image/bmp',
       _resourceSize: 12,
+      finished: true,
     },
     {
       _url: 'http://google.com/image.bmp',
       _mimeType: 'image/bmp',
       _resourceSize: 12,
+      finished: true,
     },
     {
       _url: 'http://google.com/vector.svg',
       _mimeType: 'image/svg+xml',
       _resourceSize: 13,
+      finished: true,
     },
     {
       _url: 'http://gmail.com/image.jpg',
       _mimeType: 'image/jpeg',
       _resourceSize: 15,
+      finished: true,
     },
     {
       _url: 'data: image/jpeg ; base64 ,SgVcAT32587935321...',
       _mimeType: 'image/jpeg',
       _resourceSize: 14,
+      finished: true,
+    },
+    {
+      _url: 'http://google.com/big-image.bmp',
+      _mimeType: 'image/bmp',
+      _resourceSize: 12,
+      finished: false, // ignore for not finishing
     },
   ]
 };

diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js
@@ -39,6 +39,7 @@ const traceData = {
         value: 'gzip',
       }],
       content: 'aaabbbccc',
+      finished: true,
     },
     {
       _url: 'http://google.com/index.css',
@@ -50,6 +51,7 @@ const traceData = {
       },
       _responseHeaders: [],
       content: 'abcabc',
+      finished: true,
     },
     {
       _url: 'http://google.com/index.json',
@@ -61,6 +63,19 @@ const traceData = {
       },
       _responseHeaders: [],
       content: '1234567',
+      finished: true,
+    },
+    {
+      _url: 'http://google.com/other.json',
+      _mimeType: 'application/json',
+      _requestId: 2,
+      _resourceSize: 7,
+      _resourceType: {
+        _isTextType: true,
+      },
+      _responseHeaders: [],
+      content: '1234567',
+      finished: false, // ignore for not finishing
     },
     {
       _url: 'http://google.com/index.jpg',
@@ -72,6 +87,7 @@ const traceData = {
       },
       _responseHeaders: [],
       content: 'aaaaaaaaaa',
+      finished: true,
     }
   ]
 };

diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/tags-blocking-first-paint-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/tags-blocking-first-paint-test.js
@@ -29,6 +29,7 @@ const traceData = {
       _transferSize: 10,
       _startTime: 10,
       _endTime: 10,
+      finished: true,
       _initiator: {type: 'parser'}
     },
     {
@@ -37,6 +38,7 @@ const traceData = {
       _transferSize: 11,
       _startTime: 11,
       _endTime: 11,
+      finished: true,
       _initiator: {type: 'other'}
     },
     {
@@ -45,6 +47,7 @@ const traceData = {
       _transferSize: 24,
       _startTime: 24,
       _endTime: 24,
+      finished: true,
       _initiator: {type: 'script'}
     },
     {
@@ -53,6 +56,7 @@ const traceData = {
       _transferSize: 12,
       _startTime: 12,
       _endTime: 22,
+      finished: true,
       _initiator: {type: 'parser'}
     },
     {
@@ -61,6 +65,7 @@ const traceData = {
       _transferSize: 13,
       _startTime: 13,
       _endTime: 13,
+      finished: true,
       _initiator: {type: 'script'}
     },
     {
@@ -69,6 +74,7 @@ const traceData = {
       _transferSize: 16,
       _startTime: 16,
       _endTime: 16,
+      finished: true,
       _initiator: {type: 'script'}
     },
     {
@@ -77,8 +83,18 @@ const traceData = {
       _transferSize: 16,
       _startTime: 16,
       _endTime: 16,
+      finished: true,
       _initiator: {type: 'script'}
     },
+    {
+      _url: 'http://google.com/js/also-ignored.js',
+      _mimeType: 'text/javascript',
+      _transferSize: 12,
+      _startTime: 12,
+      _endTime: 22,
+      finished: false,
+      _initiator: {type: 'parser'}
+    },
   ]
 };