Skip to content

Commit

Permalink
core(legacy-javascript): use third-party-web for scoring (#10849)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark authored May 27, 2020
1 parent 210ff35 commit 03f00c7
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 29 deletions.
11 changes: 3 additions & 8 deletions lighthouse-core/audits/legacy-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@

const Audit = require('./audit.js');
const NetworkRecords = require('../computed/network-records.js');
const MainResource = require('../computed/main-resource.js');
const JSBundles = require('../computed/js-bundles.js');
const URL = require('../lib/url-shim.js');
const i18n = require('../lib/i18n/i18n.js');
const thirdPartyWeb = require('../lib/third-party-web.js');

const UIStrings = {
/** Title of a Lighthouse audit that tells the user about legacy polyfills and transforms used on the page. This is displayed in a list of audit titles that Lighthouse generates. */
Expand Down Expand Up @@ -338,10 +337,6 @@ class LegacyJavascript extends Audit {
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[LegacyJavascript.DEFAULT_PASS];
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const mainResource = await MainResource.request({
URL: artifacts.URL,
devtoolsLog,
}, context);
const bundles = await JSBundles.request(artifacts, context);

/** @type {Array<{url: string, signals: string[], locations: LH.Audit.Details.SourceLocationValue[]}>} */
Expand Down Expand Up @@ -384,9 +379,9 @@ class LegacyJavascript extends Audit {
const details = Audit.makeTableDetails(headings, tableRows);

// Only fail if first party code has legacy code.
// TODO(cjamcl): Use third-party-web.
const mainDocumentEntity = thirdPartyWeb.getEntity(artifacts.URL.finalUrl);
const foundSignalInFirstPartyCode = tableRows.some(row => {
return URL.rootDomainsMatch(row.url, mainResource.url);
return thirdPartyWeb.isFirstParty(row.url, mainDocumentEntity);
});
return {
score: foundSignalInFirstPartyCode ? 0 : 1,
Expand Down
24 changes: 4 additions & 20 deletions lighthouse-core/audits/third-party-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
*/
'use strict';

const thirdPartyWeb = require('third-party-web/httparchive-nostats-subset');

const Audit = require('./audit.js');
const BootupTime = require('./bootup-time.js');
const i18n = require('../lib/i18n/i18n.js');
const thirdPartyWeb = require('../lib/third-party-web.js');
const NetworkRecords = require('../computed/network-records.js');
const MainResource = require('../computed/main-resource.js');
const MainThreadTasks = require('../computed/main-thread-tasks.js');
Expand Down Expand Up @@ -53,21 +52,6 @@ class ThirdPartySummary extends Audit {
};
}

/**
* `third-party-web` throws when the passed in string doesn't appear to have any domain whatsoever.
* We pass in some not-so-url-like things, so make the dependent-code simpler by making this call safe.
* @param {string} url
* @return {ThirdPartyEntity|undefined}
*/
static getEntitySafe(url) {
try {
return thirdPartyWeb.getEntity(url);
} catch (_) {
return undefined;
}
}


/**
*
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
Expand All @@ -81,7 +65,7 @@ class ThirdPartySummary extends Audit {
const defaultEntityStat = {mainThreadTime: 0, blockingTime: 0, transferSize: 0};

for (const request of networkRecords) {
const entity = ThirdPartySummary.getEntitySafe(request.url);
const entity = thirdPartyWeb.getEntity(request.url);
if (!entity) continue;

const entityStats = entities.get(entity) || {...defaultEntityStat};
Expand All @@ -93,7 +77,7 @@ class ThirdPartySummary extends Audit {

for (const task of mainThreadTasks) {
const attributeableURL = BootupTime.getAttributableURLForTask(task, jsURLs);
const entity = ThirdPartySummary.getEntitySafe(attributeableURL);
const entity = thirdPartyWeb.getEntity(attributeableURL);
if (!entity) continue;

const entityStats = entities.get(entity) || {...defaultEntityStat};
Expand Down Expand Up @@ -121,7 +105,7 @@ class ThirdPartySummary extends Audit {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);
const mainEntity = ThirdPartySummary.getEntitySafe(mainResource.url);
const mainEntity = thirdPartyWeb.getEntity(mainResource.url);
const tasks = await MainThreadTasks.request(trace, context);
const multiplier = settings.throttlingMethod === 'simulate' ?
settings.throttling.cpuSlowdownMultiplier : 1;
Expand Down
49 changes: 49 additions & 0 deletions lighthouse-core/lib/third-party-web.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* @license Copyright 2020 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

const thirdPartyWeb = require('third-party-web/httparchive-nostats-subset');

/** @typedef {import("third-party-web").IEntity} ThirdPartyEntity */

/**
* `third-party-web` throws when the passed in string doesn't appear to have any domain whatsoever.
* We pass in some not-so-url-like things, so make the dependent-code simpler by making this call safe.
* @param {string} url
* @return {ThirdPartyEntity|undefined}
*/
function getEntity(url) {
try {
return thirdPartyWeb.getEntity(url);
} catch (_) {
return undefined;
}
}

/**
* @param {string} url
* @param {ThirdPartyEntity | undefined} mainDocumentEntity
*/
function isThirdParty(url, mainDocumentEntity) {
const entity = getEntity(url);
if (!entity) return false;
if (entity === mainDocumentEntity) return false;
return true;
}

/**
* @param {string} url
* @param {ThirdPartyEntity | undefined} mainDocumentEntity
*/
function isFirstParty(url, mainDocumentEntity) {
return !isThirdParty(url, mainDocumentEntity);
}

module.exports = {
getEntity,
isThirdParty,
isFirstParty,
};
14 changes: 13 additions & 1 deletion lighthouse-core/test/audits/legacy-javascript-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const createArtifacts = (scripts) => {
url,
}));
return {
URL: {finalUrl: '', requestedUrl: ''},
URL: {finalUrl: 'https://www.example.com', requestedUrl: 'https://www.example.com'},
devtoolsLogs: {defaultPass: networkRecordsToDevtoolsLog(networkRecords)},
ScriptElements: scripts.map(({url, code}, index) => {
return {
Expand Down Expand Up @@ -83,6 +83,18 @@ describe('LegacyJavaScript audit', () => {
assert.equal(result.extendedInfo.signalCount, 0);
});

it('passes code with a legacy polyfill in third party resource', async () => {
const artifacts = createArtifacts([
{
code: 'String.prototype.repeat = function() {}',
url: 'https://www.googletagmanager.com/a.js',
},
]);
const result = await LegacyJavascript.audit(artifacts, {computedCache: new Map()});
assert.equal(result.score, 1);
assert.equal(result.extendedInfo.signalCount, 1);
});

it('fails code with a legacy polyfill', async () => {
const artifacts = createArtifacts([
{
Expand Down
27 changes: 27 additions & 0 deletions lighthouse-core/test/lib/third-party-web-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* @license Copyright 2020 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/* eslint-env jest */

const thirdPartyWeb = require('../../lib/third-party-web.js');

describe('third party web', () => {
it('basic', () => {
expect(thirdPartyWeb.isThirdParty('https://www.example.com', undefined)).toBe(false);
expect(thirdPartyWeb.isFirstParty('https://www.example.com', undefined)).toBe(true);

expect(thirdPartyWeb.isThirdParty('https://www.googletagmanager.com', undefined)).toBe(true);
expect(thirdPartyWeb.isFirstParty('https://www.googletagmanager.com', undefined)).toBe(false);
});

it('not third party if main document is same entity', () => {
const mainDocumentEntity = thirdPartyWeb.getEntity('https://www.googletagmanager.com');
expect(thirdPartyWeb.isThirdParty('https://www.googletagmanager.com/a.js', mainDocumentEntity)).toBe(false);
expect(thirdPartyWeb.isThirdParty('https://blah.atdmt.com', mainDocumentEntity)).toBe(true);
expect(thirdPartyWeb.isThirdParty('https://www.example.com', mainDocumentEntity)).toBe(false);
});
});

0 comments on commit 03f00c7

Please sign in to comment.