diff --git a/core/audits/third-party-summary.js b/core/audits/third-party-summary.js index 9fb32436eb6b..a0de4c74e51b 100644 --- a/core/audits/third-party-summary.js +++ b/core/audits/third-party-summary.js @@ -55,15 +55,6 @@ const PASS_THRESHOLD_IN_MS = 250; * @property {Map} urls Map of URLs under each entity. */ -/** - * Don't bother showing resources smaller than 4KiB since they're likely to be pixels, which isn't - * too actionable. - */ -const MIN_TRANSFER_SIZE_FOR_SUBITEMS = 4096; - -/** Show at most 5 sub items in the resource breakdown. */ -const MAX_SUBITEMS = 5; - class ThirdPartySummary extends Audit { /** * @return {LH.Audit.Meta} @@ -146,51 +137,15 @@ class ThirdPartySummary extends Audit { /** * @param {LH.Artifacts.Entity} entity * @param {SummaryMaps} summaries - * @param {Summary} stats * @return {Array} */ - static makeSubItems(entity, summaries, stats) { + static makeSubItems(entity, summaries) { const entityURLs = summaries.urls.get(entity) || []; - let items = entityURLs + const items = entityURLs .map(url => /** @type {URLSummary} */ ({url, ...summaries.byURL.get(url)})) - // Filter out any cases where byURL was missing entries. - .filter((stat) => stat.transferSize > 0) // Sort by blocking time first, then transfer size to break ties. .sort((a, b) => (b.blockingTime - a.blockingTime) || (b.transferSize - a.transferSize)); - const subitemSummary = {transferSize: 0, blockingTime: 0, tbtImpact: 0}; - const minTransferSize = Math.max(MIN_TRANSFER_SIZE_FOR_SUBITEMS, stats.transferSize / 20); - const maxSubItems = Math.min(MAX_SUBITEMS, items.length); - let numSubItems = 0; - while (numSubItems < maxSubItems) { - const nextSubItem = items[numSubItems]; - if (nextSubItem.blockingTime === 0 && nextSubItem.transferSize < minTransferSize) { - // Don't include the resource in the sub-item breakdown because it didn't have a big - // enough impact on its own. - break; - } - - numSubItems++; - subitemSummary.transferSize += nextSubItem.transferSize; - subitemSummary.blockingTime += nextSubItem.blockingTime; - subitemSummary.tbtImpact += nextSubItem.tbtImpact; - } - if (!subitemSummary.blockingTime && !subitemSummary.transferSize) { - // Don't bother breaking down if there are no large resources. - return []; - } - // Only show the top N entries for brevity. If there is more than one remaining entry - // we'll replace the tail entries with single remainder entry. - items = items.slice(0, numSubItems); - const remainder = { - url: str_(i18n.UIStrings.otherResourcesLabel), - transferSize: stats.transferSize - subitemSummary.transferSize, - blockingTime: stats.blockingTime - subitemSummary.blockingTime, - tbtImpact: stats.tbtImpact - subitemSummary.tbtImpact, - }; - if (remainder.transferSize > minTransferSize) { - items.push(remainder); - } return items; } @@ -231,7 +186,7 @@ class ThirdPartySummary extends Audit { entity: entity.name, subItems: { type: /** @type {const} */ ('subitems'), - items: ThirdPartySummary.makeSubItems(entity, summaries, stats), + items: ThirdPartySummary.makeSubItems(entity, summaries), }, }; }) diff --git a/core/test/audits/third-party-summary-test.js b/core/test/audits/third-party-summary-test.js index b9c26fd43c34..f718800f1785 100644 --- a/core/test/audits/third-party-summary-test.js +++ b/core/test/audits/third-party-summary-test.js @@ -67,16 +67,37 @@ describe('Third party summary', () => { }, { blockingTime: 0, + mainThreadTime: 5.094, tbtImpact: 0, - transferSize: 8007, - url: expect.toBeDisplayString('Other resources'), + transferSize: 3161, + url: 'https://www.google-analytics.com/cx/api.js?experiment=qvpc5qIfRC2EMnbn6bbN5A', + }, + { + blockingTime: 0, + mainThreadTime: 7.01, + tbtImpact: 0, + transferSize: 2949, + url: 'https://www.google-analytics.com/cx/api.js?experiment=jdCfRmudTmy-0USnJ8xPbw', + }, + { + blockingTime: 0, + mainThreadTime: 27.805000000000007, + tbtImpact: 0, + transferSize: 1425, + url: 'https://www.google-analytics.com/plugins/ua/linkid.js', + }, + { + blockingTime: 0, + mainThreadTime: 0, + tbtImpact: 0, + transferSize: 472, + url: 'https://www.google-analytics.com/r/collect?v=1&_v=j53&aip=1&a=749319045&t=pageview&_s=1&dl=https%3A%2F%2Fpwa.rocks%2F&ul=en-us&de=UTF-8&dt=A%20selection%20of%20Progressive%20Web%20Apps&sd=24-bit&sr=412x732&vp=412x732&je=0&_u=aGAAAAAjI~&jid=2098926753&gjid=1553792979&cid=601220267.1494017431&tid=UA-58419726-4&_gid=445409680.1494017431&_r=1>m=GTM-Q5SW&z=1395339825', }, ], type: 'subitems', }, }, ]); - expect(results.details.items[1].subItems.items[1].url).toBeDisplayString('Other resources'); }); it('account for simulated throttling', async () => { diff --git a/core/test/fixtures/user-flows/reports/sample-flow-result.json b/core/test/fixtures/user-flows/reports/sample-flow-result.json index 7fa1b54128fc..ba17a862bf16 100644 --- a/core/test/fixtures/user-flows/reports/sample-flow-result.json +++ b/core/test/fixtures/user-flows/reports/sample-flow-result.json @@ -1479,6 +1479,13 @@ "blockingTime": 0, "transferSize": 8273, "tbtImpact": 0 + }, + { + "url": "https://fonts.googleapis.com/css2?family=Poppins:wght@400;500;700&display=swap", + "mainThreadTime": 4.868, + "blockingTime": 0, + "transferSize": 2506, + "tbtImpact": 0 } ] } @@ -19056,7 +19063,29 @@ "entity": "Google Fonts", "subItems": { "type": "subitems", - "items": [] + "items": [ + { + "url": "https://fonts.googleapis.com/css2?family=Poppins:wght@400;500;700&display=swap", + "mainThreadTime": 2.98, + "blockingTime": 0, + "transferSize": 0, + "tbtImpact": 0 + }, + { + "url": "https://fonts.gstatic.com/s/poppins/v20/pxiByp8kv8JHgFVrLCz7Z1xlFd2JQEk.woff2", + "mainThreadTime": 0, + "blockingTime": 0, + "transferSize": 0, + "tbtImpact": 0 + }, + { + "url": "https://fonts.gstatic.com/s/poppins/v20/pxiEyp8kv8JHgFVrJJfecnFHGPc.woff2", + "mainThreadTime": 0, + "blockingTime": 0, + "transferSize": 0, + "tbtImpact": 0 + } + ] } } ],