Skip to content

Commit

Permalink
core(uses-http2): include multiplexable assets when 1p is a known 3p …
Browse files Browse the repository at this point in the history
…origin (#15638)

Co-authored-by: Adam Raine <6752989+adamraine@users.noreply.github.com>
  • Loading branch information
alexnj and adamraine committed Dec 5, 2023
1 parent 4e09806 commit 4c5e1c2
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 5 deletions.
13 changes: 8 additions & 5 deletions core/audits/dobetterweb/uses-http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,19 @@ class UsesHTTP2Audit extends Audit {
* @param {LH.Artifacts.EntityClassification} classifiedEntities
* @return {boolean}
*/
static isStaticAsset(networkRequest, classifiedEntities) {
static isMultiplexableStaticAsset(networkRequest, classifiedEntities) {
if (!STATIC_RESOURCE_TYPES.has(networkRequest.resourceType)) return false;

// Resources from third-parties that are less than 100 bytes are usually tracking pixels, not actual resources.
// They can masquerade as static types though (gifs, documents, etc)
if (networkRequest.resourceSize < 100) {
// This logic needs to be revisited.
// See https://github.com/GoogleChrome/lighthouse/issues/14661
const entity = classifiedEntities.entityByUrl.get(networkRequest.url);
if (entity && !entity.isUnrecognized) return false;
if (entity) {
// Third-party assets are multiplexable in their first-party context.
if (classifiedEntities.firstParty?.name === entity.name) return true;
// Skip recognizable third-parties' requests.
if (!entity.isUnrecognized) return false;
}
}

return true;
Expand Down Expand Up @@ -199,7 +202,7 @@ class UsesHTTP2Audit extends Audit {
/** @type {Map<string, Array<LH.Artifacts.NetworkRequest>>} */
const groupedByOrigin = new Map();
for (const record of networkRecords) {
if (!UsesHTTP2Audit.isStaticAsset(record, classifiedEntities)) continue;
if (!UsesHTTP2Audit.isMultiplexableStaticAsset(record, classifiedEntities)) continue;
if (UrlUtils.isLikeLocalhost(record.parsedURL.host)) continue;
const existing = groupedByOrigin.get(record.parsedURL.securityOrigin) || [];
existing.push(record);
Expand Down
54 changes: 54 additions & 0 deletions core/test/audits/dobetterweb/uses-http2-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,58 @@ describe('Resources are fetched over http/2', () => {
expect(result.score).toEqual(0);
expect(result.metricSavings).toBeUndefined();
});

it('should identify multiplexable assets when run on recognizable 3p origins', async () => {
const networkRecords = [
{
url: 'https://www.twitter.com/',
priority: 'High',
protocol: 'HTTP/1.1',
},
{
url: 'https://www.twitter.com/2',
priority: 'High',
protocol: 'HTTP/1.1',
},
{
url: 'https://www.twitter.com/3',
priority: 'High',
protocol: 'HTTP/1.1',
},
{
url: 'https://www.twitter.com/4',
priority: 'High',
protocol: 'HTTP/1.1',
},
{
url: 'https://www.twitter.com/5',
priority: 'High',
protocol: 'HTTP/1.1',
},
{
url: 'https://www.twitter.com/embed/foo',
priority: 'High',
protocol: 'HTTP/1.1',
},
{
url: 'https://www.facebook.com/embed',
protocol: 'HTTP/1.1',
priority: 'High',
},
];
const artifacts = buildArtifacts(networkRecords);
artifacts.devtoolsLogs.defaultPass = networkRecordsToDevtoolsLog(networkRecords);

const result = await UsesHTTP2Audit.audit(artifacts, context);
const urls = new Set(result.details.items.map(item => item.url));
const hosts = new Set(result.details.items.map(item => new URL(item.url).host));

// Make sure we don't pull in actual 3p domains.
expect(hosts).toEqual(new Set(['www.twitter.com']));

// Make sure we dont flag the 3rd party request for multiplexing.
expect(urls).not.toContain('https://www.facebook.com/embed');

expect(result.details.items).toHaveLength(6);
});
});

0 comments on commit 4c5e1c2

Please sign in to comment.