Skip to content

Commit

Permalink
core(inspector-issues): collect all issue types (#13462)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamread committed Jan 11, 2022
1 parent ccbe8fc commit adb28ff
Show file tree
Hide file tree
Showing 18 changed files with 208 additions and 130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const expectations = {
RobotsTxt: {
status: 200,
},
InspectorIssues: {contentSecurityPolicy: []},
InspectorIssues: {contentSecurityPolicyIssue: []},
SourceMaps: [{
sourceMapUrl: 'http://localhost:10200/source-map/script.js.map',
map: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const expectations = {
status: 200,
},
InspectorIssues: {
contentSecurityPolicy: [],
contentSecurityPolicyIssue: [],
},
SourceMaps: [{
sourceMapUrl: 'http://localhost:10200/source-map/script.js.map',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
const expectations = {
artifacts: {
InspectorIssues: {
mixedContent: [
mixedContentIssue: [
{
_minChromiumMilestone: 88, // We went from Warning to AutoUpgrade in https://chromium-review.googlesource.com/c/chromium/src/+/2480817
resourceType: 'Image',
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/audits/deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class Deprecations extends Audit {
const bundles = await JsBundles.request(artifacts, context);

let deprecations;
if (artifacts.InspectorIssues.deprecations.length) {
deprecations = artifacts.InspectorIssues.deprecations
if (artifacts.InspectorIssues.deprecationIssue.length) {
deprecations = artifacts.InspectorIssues.deprecationIssue
.map(deprecation => {
const {url, lineNumber, columnNumber} = deprecation.sourceCodeLocation;
const bundle = bundles.find(bundle => bundle.script.src === url);
Expand Down
16 changes: 8 additions & 8 deletions lighthouse-core/audits/dobetterweb/inspector-issues.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,19 @@ class IssuesPanelEntries extends Audit {
/** @type LH.Audit.Details.TableItem[] */
const items = [];

if (issues.mixedContent.length) {
items.push(this.getMixedContentRow(issues.mixedContent));
if (issues.mixedContentIssue.length) {
items.push(this.getMixedContentRow(issues.mixedContentIssue));
}
if (issues.sameSiteCookies.length) {
items.push(this.getSameSiteCookieRow(issues.sameSiteCookies));
if (issues.sameSiteCookieIssue.length) {
items.push(this.getSameSiteCookieRow(issues.sameSiteCookieIssue));
}
if (issues.blockedByResponse.length) {
items.push(this.getBlockedByResponseRow(issues.blockedByResponse));
if (issues.blockedByResponseIssue.length) {
items.push(this.getBlockedByResponseRow(issues.blockedByResponseIssue));
}
if (issues.heavyAds.length) {
if (issues.heavyAdIssue.length) {
items.push({issueType: str_(UIStrings.issueTypeHeavyAds)});
}
const cspIssues = issues.contentSecurityPolicy.filter(issue => {
const cspIssues = issues.contentSecurityPolicyIssue.filter(issue => {
// kTrustedTypesSinkViolation and kTrustedTypesPolicyViolation aren't currently supported by the Issues panel
return issue.contentSecurityPolicyViolationType !== 'kTrustedTypesSinkViolation' &&
issue.contentSecurityPolicyViolationType !== 'kTrustedTypesPolicyViolation';
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/is-on-https.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class HTTPS extends Audit {
{key: 'resolution', itemType: 'text', text: str_(UIStrings.columnResolution)},
];

for (const details of artifacts.InspectorIssues.mixedContent) {
for (const details of artifacts.InspectorIssues.mixedContentIssue) {
let item = items.find(item => item.url === details.insecureURL);
if (!item) {
item = {url: details.insecureURL};
Expand Down
82 changes: 36 additions & 46 deletions lighthouse-core/gather/gatherers/inspector-issues.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,58 +58,48 @@ class InspectorIssues extends FRGatherer {
* @return {Promise<LH.Artifacts['InspectorIssues']>}
*/
async _getArtifact(networkRecords) {
/** @type {LH.Artifacts.InspectorIssues} */
const artifact = {
/** @type {Array<LH.Crdp.Audits.MixedContentIssueDetails>} */
mixedContent: [],
/** @type {Array<LH.Crdp.Audits.SameSiteCookieIssueDetails>} */
sameSiteCookies: [],
/** @type {Array<LH.Crdp.Audits.BlockedByResponseIssueDetails>} */
blockedByResponse: [],
/** @type {Array<LH.Crdp.Audits.HeavyAdIssueDetails>} */
heavyAds: [],
/** @type {Array<LH.Crdp.Audits.ContentSecurityPolicyIssueDetails>} */
contentSecurityPolicy: [],
/** @type {Array<LH.Crdp.Audits.DeprecationIssueDetails>} */
deprecations: [],
attributionReportingIssue: [],
blockedByResponseIssue: [],
clientHintIssue: [],
contentSecurityPolicyIssue: [],
corsIssue: [],
deprecationIssue: [],
genericIssue: [],
heavyAdIssue: [],
lowTextContrastIssue: [],
mixedContentIssue: [],
navigatorUserAgentIssue: [],
quirksModeIssue: [],
sameSiteCookieIssue: [],
sharedArrayBufferIssue: [],
twaQualityEnforcement: [],
wasmCrossOriginModuleSharingIssue: [],
};

for (const issue of this._issues) {
if (issue.details.mixedContentIssueDetails) {
const issueDetails = issue.details.mixedContentIssueDetails;
const issueReqId = issueDetails.request?.requestId;
// Duplicate issues can occur for the same request; only use the one with a matching networkRequest.
if (issueReqId &&
networkRecords.find(req => req.requestId === issueReqId)) {
artifact.mixedContent.push(issueDetails);
const keys = /** @type {Array<keyof LH.Artifacts['InspectorIssues']>} */(Object.keys(artifact));
for (const key of keys) {
// The wasmCrossOriginModuleSharingIssue key doesn't follow the pattern of the rest. See
// https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/devtools_protocol/browser_protocol.pdl;l=811;drc=9c10bf258928b5d169ba6a449f8f33958f7947ea
/** @type {`${key}Details` | `wasmCrossOriginModuleSharingIssue`} */
const detailsKey = (key === 'wasmCrossOriginModuleSharingIssue' ? `${key}` : `${key}Details`);
const allDetails = this._issues.map(issue => issue.details[detailsKey]);
for (const detail of allDetails) {
if (!detail) {
continue;
}
}
if (issue.details.sameSiteCookieIssueDetails) {
const issueDetails = issue.details.sameSiteCookieIssueDetails;
const issueReqId = issueDetails.request?.requestId;
// Duplicate issues can occur for the same request; only use the one with a matching networkRequest.
if (issueReqId &&
networkRecords.find(req => req.requestId === issueReqId)) {
artifact.sameSiteCookies.push(issueDetails);
const requestId = 'request' in detail && detail.request && detail.request.requestId;
if (requestId) {
if (networkRecords.find(req => req.requestId === requestId)) {
// @ts-expect-error - detail types are not all compatible
artifact[key].push(detail);
}
} else {
// @ts-expect-error - detail types are not all compatible
artifact[key].push(detail);
}
}
if (issue.details.blockedByResponseIssueDetails) {
const issueDetails = issue.details.blockedByResponseIssueDetails;
const issueReqId = issueDetails.request?.requestId;
// Duplicate issues can occur for the same request; only use the one with a matching networkRequest.
if (issueReqId &&
networkRecords.find(req => req.requestId === issueReqId)) {
artifact.blockedByResponse.push(issueDetails);
}
}
if (issue.details.heavyAdIssueDetails) {
artifact.heavyAds.push(issue.details.heavyAdIssueDetails);
}
if (issue.details.contentSecurityPolicyIssueDetails) {
artifact.contentSecurityPolicy.push(issue.details.contentSecurityPolicyIssueDetails);
}
if (issue.details.deprecationIssueDetails) {
artifact.deprecations.push(issue.details.deprecationIssueDetails);
}
}

return artifact;
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/test/audits/deprecations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('ConsoleMessages deprecations audit', () => {
const context = {computedCache: new Map()};
const auditResult = await DeprecationsAudit.audit({
ConsoleMessages: [],
InspectorIssues: {deprecations: []},
InspectorIssues: {deprecationIssue: []},
SourceMaps: [],
ScriptElements: [],
}, context);
Expand All @@ -32,7 +32,7 @@ describe('ConsoleMessages deprecations audit', () => {
text: 'Deprecation message',
},
],
InspectorIssues: {deprecations: []},
InspectorIssues: {deprecationIssue: []},
SourceMaps: [],
ScriptElements: [],
}, context);
Expand Down Expand Up @@ -65,7 +65,7 @@ describe('ConsoleMessages deprecations audit', () => {
text: 'Not a deprecation message 789',
},
],
InspectorIssues: {deprecations: []},
InspectorIssues: {deprecationIssue: []},
SourceMaps: [],
ScriptElements: [],
}, context);
Expand All @@ -90,7 +90,7 @@ describe('ConsoleMessages deprecations audit', () => {
},
],
InspectorIssues: {
deprecations: [
deprecationIssue: [
{
message: 'Deprecation message 123',
sourceCodeLocation: {
Expand Down
31 changes: 21 additions & 10 deletions lighthouse-core/test/audits/dobetterweb/inspector-issues-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,22 @@ describe('Has inspector issues audit', () => {
let issues;
beforeEach(() => {
issues = {
mixedContent: [],
sameSiteCookies: [],
blockedByResponse: [],
heavyAds: [],
contentSecurityPolicy: [],
attributionReportingIssue: [],
blockedByResponseIssue: [],
clientHintIssue: [],
contentSecurityPolicyIssue: [],
corsIssue: [],
deprecationIssue: [],
genericIssue: [],
heavyAdIssue: [],
lowTextContrastIssue: [],
mixedContentIssue: [],
navigatorUserAgentIssue: [],
quirksModeIssue: [],
sameSiteCookieIssue: [],
sharedArrayBufferIssue: [],
twaQualityEnforcement: [],
wasmCrossOriginModuleSharingIssue: [],
};
});

Expand Down Expand Up @@ -47,7 +58,7 @@ describe('Has inspector issues audit', () => {
},
},
];
issues.mixedContent.push(...mixedContentIssues);
issues.mixedContentIssue.push(...mixedContentIssues);

const auditResult = InspectorIssuesAudit.audit({
InspectorIssues: issues,
Expand Down Expand Up @@ -82,7 +93,7 @@ describe('Has inspector issues audit', () => {
},
},
];
issues.sameSiteCookies.push(...samesiteIssues);
issues.sameSiteCookieIssue.push(...samesiteIssues);

const auditResult = InspectorIssuesAudit.audit({
InspectorIssues: issues,
Expand Down Expand Up @@ -138,7 +149,7 @@ describe('Has inspector issues audit', () => {
},
},
];
issues.blockedByResponse.push(...blockedByResponseIssues);
issues.blockedByResponseIssue.push(...blockedByResponseIssues);

const auditResult = InspectorIssuesAudit.audit({
InspectorIssues: issues,
Expand Down Expand Up @@ -184,7 +195,7 @@ describe('Has inspector issues audit', () => {
reason: 'CpuPeakLimit',
},
];
issues.heavyAds.push(...heavyAdsIssues);
issues.heavyAdIssue.push(...heavyAdsIssues);

const auditResult = InspectorIssuesAudit.audit({
InspectorIssues: issues,
Expand Down Expand Up @@ -221,7 +232,7 @@ describe('Has inspector issues audit', () => {
blockedURL: 'www.csp.com/policy-violation',
},
];
issues.contentSecurityPolicy.push(...cspIssues);
issues.contentSecurityPolicyIssue.push(...cspIssues);

const auditResult = InspectorIssuesAudit.audit({
InspectorIssues: issues,
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/is-on-https-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Security: HTTPS audit', () => {
const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords);
return {
devtoolsLogs: {[Audit.DEFAULT_PASS]: devtoolsLog},
InspectorIssues: {mixedContent: mixedContentIssues || []},
InspectorIssues: {mixedContentIssue: mixedContentIssues || []},
};
}

Expand Down
Loading

0 comments on commit adb28ff

Please sign in to comment.