Skip to content

Commit b947b50

Browse files
Natallia HarshunovaDevtools-frontend LUCI CQ
authored andcommitted
Implement getAllIssues on AggregatedIssue. Centralize issue details in
generic base Issue class. The base `Issue` class is now centralizes the `issueDetails` property, providing improved type-safety and inference for all issue aubclasses. In the future it will be easier to reuse that classes in the mcp Fixed: 463602403 Change-Id: Ie9c8ce88aa455b768a056708ec1ad30da68cad38 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7206122 Reviewed-by: Alex Rudenko <[email protected]> Commit-Queue: Alex Rudenko <[email protected]>
1 parent 8c32ced commit b947b50

29 files changed

+178
-317
lines changed

front_end/models/issues_manager/AttributionReportingIssue.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,10 @@ const structuredHeaderLink = {
8989
linkTitle: 'Structured Headers RFC',
9090
};
9191

92-
export class AttributionReportingIssue extends Issue<IssueCode> {
93-
issueDetails: Readonly<Protocol.Audits.AttributionReportingIssueDetails>;
94-
92+
export class AttributionReportingIssue extends Issue<Protocol.Audits.AttributionReportingIssueDetails, IssueCode> {
9593
constructor(
9694
issueDetails: Protocol.Audits.AttributionReportingIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel|null) {
97-
super(getIssueCode(issueDetails), issuesModel);
98-
this.issueDetails = issueDetails;
95+
super(getIssueCode(issueDetails), issueDetails, issuesModel);
9996
}
10097

10198
getCategory(): IssueCategory {
@@ -106,8 +103,9 @@ export class AttributionReportingIssue extends Issue<IssueCode> {
106103
const url = new URL('https://wicg.github.io/attribution-reporting-api/validate-headers');
107104
url.searchParams.set('header', name);
108105

109-
if (this.issueDetails.invalidParameter) {
110-
url.searchParams.set('json', this.issueDetails.invalidParameter);
106+
const details = this.details();
107+
if (details.invalidParameter) {
108+
url.searchParams.set('json', details.invalidParameter);
111109
}
112110

113111
return {
@@ -229,7 +227,7 @@ export class AttributionReportingIssue extends Issue<IssueCode> {
229227
}
230228

231229
primaryKey(): string {
232-
return JSON.stringify(this.issueDetails);
230+
return JSON.stringify(this.details());
233231
}
234232

235233
getKind(): IssueKind {

front_end/models/issues_manager/BounceTrackingIssue.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,9 @@ const UIStrings = {
1818
const str_ = i18n.i18n.registerUIStrings('models/issues_manager/BounceTrackingIssue.ts', UIStrings);
1919
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
2020

21-
export class BounceTrackingIssue extends Issue {
22-
readonly #issueDetails: Protocol.Audits.BounceTrackingIssueDetails;
23-
21+
export class BounceTrackingIssue extends Issue<Protocol.Audits.BounceTrackingIssueDetails> {
2422
constructor(issueDetails: Protocol.Audits.BounceTrackingIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel|null) {
25-
super(Protocol.Audits.InspectorIssueCode.BounceTrackingIssue, issuesModel);
26-
this.#issueDetails = issueDetails;
23+
super(Protocol.Audits.InspectorIssueCode.BounceTrackingIssue, issueDetails, issuesModel);
2724
}
2825

2926
getCategory(): IssueCategory {
@@ -42,23 +39,16 @@ export class BounceTrackingIssue extends Issue {
4239
};
4340
}
4441

45-
details(): Protocol.Audits.BounceTrackingIssueDetails {
46-
return this.#issueDetails;
47-
}
48-
4942
getKind(): IssueKind {
5043
return IssueKind.BREAKING_CHANGE;
5144
}
5245

5346
primaryKey(): string {
54-
return JSON.stringify(this.#issueDetails);
47+
return JSON.stringify(this.details());
5548
}
5649

5750
override trackingSites(): Iterable<string> {
58-
if (this.#issueDetails.trackingSites) {
59-
return this.#issueDetails.trackingSites;
60-
}
61-
return [];
51+
return this.details().trackingSites;
6252
}
6353

6454
static fromInspectorIssue(

front_end/models/issues_manager/ClientHintIssue.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,41 +22,34 @@ const UIStrings = {
2222
const str_ = i18n.i18n.registerUIStrings('models/issues_manager/ClientHintIssue.ts', UIStrings);
2323
const i18nLazyString = i18n.i18n.getLazilyComputedLocalizedString.bind(undefined, str_);
2424

25-
export class ClientHintIssue extends Issue {
26-
private issueDetails: Protocol.Audits.ClientHintIssueDetails;
27-
25+
export class ClientHintIssue extends Issue<Protocol.Audits.ClientHintIssueDetails> {
2826
constructor(issueDetails: Protocol.Audits.ClientHintIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel|null) {
2927
super(
3028
{
3129
code: Protocol.Audits.InspectorIssueCode.ClientHintIssue,
3230
umaCode: [Protocol.Audits.InspectorIssueCode.ClientHintIssue, issueDetails.clientHintIssueReason].join('::'),
3331
},
34-
issuesModel);
35-
this.issueDetails = issueDetails;
32+
issueDetails, issuesModel);
3633
}
3734

3835
getCategory(): IssueCategory {
3936
return IssueCategory.OTHER;
4037
}
4138

42-
details(): Protocol.Audits.ClientHintIssueDetails {
43-
return this.issueDetails;
44-
}
45-
4639
getDescription(): MarkdownIssueDescription|null {
47-
const description = issueDescriptions.get(this.issueDetails.clientHintIssueReason);
40+
const description = issueDescriptions.get(this.details().clientHintIssueReason);
4841
if (!description) {
4942
return null;
5043
}
5144
return resolveLazyDescription(description);
5245
}
5346

5447
override sources(): Iterable<Protocol.Audits.SourceCodeLocation> {
55-
return [this.issueDetails.sourceCodeLocation];
48+
return [this.details().sourceCodeLocation];
5649
}
5750

5851
primaryKey(): string {
59-
return JSON.stringify(this.issueDetails);
52+
return JSON.stringify(this.details());
6053
}
6154

6255
getKind(): IssueKind {

front_end/models/issues_manager/ContentSecurityPolicyIssue.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,23 @@ const UIStrings = {
3838
const str_ = i18n.i18n.registerUIStrings('models/issues_manager/ContentSecurityPolicyIssue.ts', UIStrings);
3939
const i18nLazyString = i18n.i18n.getLazilyComputedLocalizedString.bind(undefined, str_);
4040

41-
export class ContentSecurityPolicyIssue extends Issue {
42-
#issueDetails: Protocol.Audits.ContentSecurityPolicyIssueDetails;
43-
41+
export class ContentSecurityPolicyIssue extends Issue<Protocol.Audits.ContentSecurityPolicyIssueDetails> {
4442
constructor(
4543
issueDetails: Protocol.Audits.ContentSecurityPolicyIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel|null,
4644
issueId?: Protocol.Audits.IssueId) {
4745
const issueCode = [
4846
Protocol.Audits.InspectorIssueCode.ContentSecurityPolicyIssue,
4947
issueDetails.contentSecurityPolicyViolationType,
5048
].join('::');
51-
super(issueCode, issuesModel, issueId);
52-
this.#issueDetails = issueDetails;
49+
super(issueCode, issueDetails, issuesModel, issueId);
5350
}
5451

5552
getCategory(): IssueCategory {
5653
return IssueCategory.CONTENT_SECURITY_POLICY;
5754
}
5855

5956
primaryKey(): string {
60-
return JSON.stringify(this.#issueDetails, [
57+
return JSON.stringify(this.details(), [
6158
'blockedURL',
6259
'contentSecurityPolicyViolationType',
6360
'violatedDirective',
@@ -71,19 +68,15 @@ export class ContentSecurityPolicyIssue extends Issue {
7168
}
7269

7370
getDescription(): MarkdownIssueDescription|null {
74-
const description = issueDescriptions.get(this.#issueDetails.contentSecurityPolicyViolationType);
71+
const description = issueDescriptions.get(this.details().contentSecurityPolicyViolationType);
7572
if (!description) {
7673
return null;
7774
}
7875
return resolveLazyDescription(description);
7976
}
8077

81-
details(): Protocol.Audits.ContentSecurityPolicyIssueDetails {
82-
return this.#issueDetails;
83-
}
84-
8578
getKind(): IssueKind {
86-
if (this.#issueDetails.isReportOnly) {
79+
if (this.details().isReportOnly) {
8780
return IssueKind.IMPROVEMENT;
8881
}
8982
return IssueKind.PAGE_ERROR;

front_end/models/issues_manager/CookieDeprecationMetadataIssue.ts

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,29 +19,26 @@ const str_ = i18n.i18n.registerUIStrings('models/issues_manager/CookieDeprecatio
1919
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
2020

2121
/** TODO(b/305738703): Move this issue into a warning on CookieIssue. **/
22-
export class CookieDeprecationMetadataIssue extends Issue {
23-
readonly #issueDetails: Protocol.Audits.CookieDeprecationMetadataIssueDetails;
24-
22+
export class CookieDeprecationMetadataIssue extends Issue<Protocol.Audits.CookieDeprecationMetadataIssueDetails> {
2523
constructor(
2624
issueDetails: Protocol.Audits.CookieDeprecationMetadataIssueDetails,
2725
issuesModel: SDK.IssuesModel.IssuesModel|null) {
2826
// Set a distinct code for ReadCookie and SetCookie issues, so they are grouped separately.
2927
const issueCode = Protocol.Audits.InspectorIssueCode.CookieDeprecationMetadataIssue + '_' + issueDetails.operation;
30-
super(issueCode, issuesModel);
31-
this.#issueDetails = issueDetails;
28+
super(issueCode, issueDetails, issuesModel);
3229
}
3330

3431
getCategory(): IssueCategory {
3532
return IssueCategory.OTHER;
3633
}
3734

3835
getDescription(): MarkdownIssueDescription {
39-
const fileName = this.#issueDetails.operation === 'SetCookie' ? 'cookieWarnMetadataGrantSet.md' :
40-
'cookieWarnMetadataGrantRead.md';
36+
const fileName =
37+
this.details().operation === 'SetCookie' ? 'cookieWarnMetadataGrantSet.md' : 'cookieWarnMetadataGrantRead.md';
4138

4239
let optOutText = '';
43-
if (this.#issueDetails.isOptOutTopLevel) {
44-
optOutText = '\n\n (Top level site opt-out: ' + this.#issueDetails.optOutPercentage +
40+
if (this.details().isOptOutTopLevel) {
41+
optOutText = '\n\n (Top level site opt-out: ' + this.details().optOutPercentage +
4542
'% - [learn more](gracePeriodStagedControlExplainer))';
4643
}
4744

@@ -59,16 +56,12 @@ export class CookieDeprecationMetadataIssue extends Issue {
5956
};
6057
}
6158

62-
details(): Protocol.Audits.CookieDeprecationMetadataIssueDetails {
63-
return this.#issueDetails;
64-
}
65-
6659
getKind(): IssueKind {
6760
return IssueKind.BREAKING_CHANGE;
6861
}
6962

7063
primaryKey(): string {
71-
return JSON.stringify(this.#issueDetails);
64+
return JSON.stringify(this.details());
7265
}
7366

7467
static fromInspectorIssue(

front_end/models/issues_manager/CookieIssue.ts

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -68,27 +68,20 @@ export interface CookieReportInfo {
6868
insight?: Protocol.Audits.CookieIssueInsight;
6969
}
7070

71-
export class CookieIssue extends Issue {
72-
#issueDetails: Protocol.Audits.CookieIssueDetails;
73-
74-
constructor(
75-
code: string, issueDetails: Protocol.Audits.CookieIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel|null,
76-
issueId: Protocol.Audits.IssueId|undefined) {
77-
super(code, issuesModel, issueId);
78-
this.#issueDetails = issueDetails;
79-
}
80-
71+
export class CookieIssue extends Issue<Protocol.Audits.CookieIssueDetails> {
8172
cookieId(): string {
82-
if (this.#issueDetails.cookie) {
83-
const {domain, path, name} = this.#issueDetails.cookie;
73+
const details = this.details();
74+
if (details.cookie) {
75+
const {domain, path, name} = details.cookie;
8476
const cookieId = `${domain};${path};${name}`;
8577
return cookieId;
8678
}
87-
return this.#issueDetails.rawCookieLine ?? 'no-cookie-info';
79+
return this.details().rawCookieLine ?? 'no-cookie-info';
8880
}
8981

9082
primaryKey(): string {
91-
const requestId = this.#issueDetails.request ? this.#issueDetails.request.requestId : 'no-request';
83+
const details = this.details();
84+
const requestId = details.request ? details.request.requestId : 'no-request';
9285
return `${this.code()}-(${this.cookieId()})-(${requestId})`;
9386
}
9487

@@ -210,22 +203,25 @@ export class CookieIssue extends Issue {
210203
}
211204

212205
override cookies(): Iterable<Protocol.Audits.AffectedCookie> {
213-
if (this.#issueDetails.cookie) {
214-
return [this.#issueDetails.cookie];
206+
const details = this.details();
207+
if (details.cookie) {
208+
return [details.cookie];
215209
}
216210
return [];
217211
}
218212

219213
override rawCookieLines(): Iterable<string> {
220-
if (this.#issueDetails.rawCookieLine) {
221-
return [this.#issueDetails.rawCookieLine];
214+
const details = this.details();
215+
if (details.rawCookieLine) {
216+
return [details.rawCookieLine];
222217
}
223218
return [];
224219
}
225220

226221
override requests(): Iterable<Protocol.Audits.AffectedRequest> {
227-
if (this.#issueDetails.request) {
228-
return [this.#issueDetails.request];
222+
const details = this.details();
223+
if (details.request) {
224+
return [details.request];
229225
}
230226
return [];
231227
}
@@ -244,27 +240,28 @@ export class CookieIssue extends Issue {
244240

245241
override isCausedByThirdParty(): boolean {
246242
const outermostFrame = SDK.FrameManager.FrameManager.instance().getOutermostFrame();
247-
return isCausedByThirdParty(outermostFrame, this.#issueDetails.cookieUrl, this.#issueDetails.siteForCookies);
243+
return isCausedByThirdParty(outermostFrame, this.details().cookieUrl, this.details().siteForCookies);
248244
}
249245

250246
getKind(): IssueKind {
251-
if (this.#issueDetails.cookieExclusionReasons?.length > 0) {
247+
if (this.details().cookieExclusionReasons?.length > 0) {
252248
return IssueKind.PAGE_ERROR;
253249
}
254250
return IssueKind.BREAKING_CHANGE;
255251
}
256252

257253
makeCookieReportEntry(): CookieReportInfo|undefined {
258-
const status = CookieIssue.getCookieStatus(this.#issueDetails);
259-
if (this.#issueDetails.cookie && this.#issueDetails.cookieUrl && status !== undefined) {
260-
const entity = ThirdPartyWeb.ThirdPartyWeb.getEntity(this.#issueDetails.cookieUrl);
254+
const status = CookieIssue.getCookieStatus(this.details());
255+
const details = this.details();
256+
if (details.cookie && details.cookieUrl && status !== undefined) {
257+
const entity = ThirdPartyWeb.ThirdPartyWeb.getEntity(details.cookieUrl);
261258
return {
262-
name: this.#issueDetails.cookie.name,
263-
domain: this.#issueDetails.cookie.domain,
259+
name: details.cookie.name,
260+
domain: details.cookie.domain,
264261
type: entity?.category,
265262
platform: entity?.name,
266263
status,
267-
insight: this.#issueDetails.insight,
264+
insight: this.details().insight,
268265
};
269266
}
270267

@@ -332,8 +329,8 @@ export class CookieIssue extends Issue {
332329
return new SDK.ConsoleModel.ConsoleMessage(
333330
issuesModel.target().model(SDK.RuntimeModel.RuntimeModel), Common.Console.FrontendMessageSource.ISSUE_PANEL,
334331
Protocol.Log.LogEntryLevel.Warning, UIStrings.consoleTpcdErrorMessage, {
335-
url: this.#issueDetails.request?.url as Platform.DevToolsPath.UrlString | undefined,
336-
affectedResources: {requestId: this.#issueDetails.request?.requestId, issueId: this.issueId},
332+
url: this.details().request?.url as Platform.DevToolsPath.UrlString | undefined,
333+
affectedResources: {requestId: this.details().request?.requestId, issueId: this.issueId},
337334
isCookieReportIssue: true
338335
});
339336
}

front_end/models/issues_manager/CorsIssue.ts

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -115,26 +115,19 @@ function getIssueCode(details: Protocol.Audits.CorsIssueDetails): IssueCode {
115115
}
116116
}
117117

118-
export class CorsIssue extends Issue<IssueCode> {
119-
#issueDetails: Protocol.Audits.CorsIssueDetails;
120-
118+
export class CorsIssue extends Issue<Protocol.Audits.CorsIssueDetails, IssueCode> {
121119
constructor(
122120
issueDetails: Protocol.Audits.CorsIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel|null,
123121
issueId: Protocol.Audits.IssueId|undefined) {
124-
super(getIssueCode(issueDetails), issuesModel, issueId);
125-
this.#issueDetails = issueDetails;
122+
super(getIssueCode(issueDetails), issueDetails, issuesModel, issueId);
126123
}
127124

128125
getCategory(): IssueCategory {
129126
return IssueCategory.CORS;
130127
}
131128

132-
details(): Protocol.Audits.CorsIssueDetails {
133-
return this.#issueDetails;
134-
}
135-
136129
getDescription(): MarkdownIssueDescription|null {
137-
switch (getIssueCode(this.#issueDetails)) {
130+
switch (getIssueCode(this.details())) {
138131
case IssueCode.INSECURE_PRIVATE_NETWORK:
139132
return {
140133
file: 'corsInsecurePrivateNetwork.md',
@@ -269,16 +262,14 @@ export class CorsIssue extends Issue<IssueCode> {
269262
}
270263

271264
primaryKey(): string {
272-
return JSON.stringify(this.#issueDetails);
265+
return JSON.stringify(this.details());
273266
}
274267

275268
getKind(): IssueKind {
276-
if (this.#issueDetails.isWarning &&
277-
(this.#issueDetails.corsErrorStatus.corsError === Protocol.Network.CorsError.InsecurePrivateNetwork ||
278-
this.#issueDetails.corsErrorStatus.corsError ===
279-
Protocol.Network.CorsError.PreflightMissingAllowPrivateNetwork ||
280-
this.#issueDetails.corsErrorStatus.corsError ===
281-
Protocol.Network.CorsError.PreflightInvalidAllowPrivateNetwork)) {
269+
if (this.details().isWarning &&
270+
(this.details().corsErrorStatus.corsError === Protocol.Network.CorsError.InsecurePrivateNetwork ||
271+
this.details().corsErrorStatus.corsError === Protocol.Network.CorsError.PreflightMissingAllowPrivateNetwork ||
272+
this.details().corsErrorStatus.corsError === Protocol.Network.CorsError.PreflightInvalidAllowPrivateNetwork)) {
282273
return IssueKind.BREAKING_CHANGE;
283274
}
284275
return IssueKind.PAGE_ERROR;

0 commit comments

Comments
 (0)