[issues] Introduce RelatedIssue for associating issues with objects
This introduces a mechanism to associate issues with other objects in
DevTools such as NetworkRequests. Whether an issue is associated may
be used for, e.g., filtering or providing links to the issue.
Bug: chromium:1057979
Change-Id: I208ac146b1f576a19d377f29adf266cacc1ef091
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2116439
Commit-Queue: Sigurd Schneider <[email protected]>
Reviewed-by: Tim van der Lippe <[email protected]>
Reviewed-by: Simon Zünd <[email protected]>
diff --git a/BUILD.gn b/BUILD.gn
index ea488a3..4383138 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1404,6 +1404,7 @@
"sdk/IsolateManager.js",
"sdk/Issue.js",
"sdk/IssuesModel.js",
+ "sdk/RelatedIssue.js",
"sdk/LayerTreeBase.js",
"sdk/LogModel.js",
"sdk/NetworkLog.js",
diff --git a/front_end/network/NetworkLogView.js b/front_end/network/NetworkLogView.js
index 2863f27..c0097db 100644
--- a/front_end/network/NetworkLogView.js
+++ b/front_end/network/NetworkLogView.js
@@ -1578,7 +1578,7 @@
if (this._dataURLFilterUI.checked() && (request.parsedURL.isDataURL() || request.parsedURL.isBlobURL())) {
return false;
}
- if (this._onlyIssuesFilterUI.checked() && !SDK.IssuesModel.IssuesModel.hasIssues(request)) {
+ if (this._onlyIssuesFilterUI.checked() && !SDK.RelatedIssue.hasIssues(request)) {
return false;
}
if (this._onlyBlockedRequestsUI.checked() && !request.wasBlocked()) {
diff --git a/front_end/resources/CookieItemsView.js b/front_end/resources/CookieItemsView.js
index 02464d7..fd2bee4 100644
--- a/front_end/resources/CookieItemsView.js
+++ b/front_end/resources/CookieItemsView.js
@@ -207,7 +207,7 @@
*/
filter(items, keyFunction) {
return super.filter(items, keyFunction)
- .filter(cookie => !this._onlyIssuesFilterUI.checked() || SDK.IssuesModel.IssuesModel.hasIssues(cookie));
+ .filter(cookie => !this._onlyIssuesFilterUI.checked() || SDK.RelatedIssue.hasIssues(cookie));
}
/**
diff --git a/front_end/sdk/BUILD.gn b/front_end/sdk/BUILD.gn
index 8d99d1a..74e4091 100644
--- a/front_end/sdk/BUILD.gn
+++ b/front_end/sdk/BUILD.gn
@@ -10,6 +10,7 @@
"CookieParser.js",
"NetworkManager.js",
"NetworkRequest.js",
+ "RelatedIssue.js",
"SDKModel.js",
"SecurityOriginManager.js",
"ServerTiming.js",
diff --git a/front_end/sdk/Issue.js b/front_end/sdk/Issue.js
index f609ecb..fcb2a85 100644
--- a/front_end/sdk/Issue.js
+++ b/front_end/sdk/Issue.js
@@ -3,7 +3,7 @@
// found in the LICENSE file.
import * as Common from '../common/common.js';
-import {IssuesModel} from './IssuesModel.js';
+import {IssueCategory} from './RelatedIssue.js';
/**
* @unrestricted
@@ -52,6 +52,20 @@
}
return false;
}
+
+ /**
+ * @return {symbol}
+ */
+ getCategory() {
+ const code = this.code();
+ if (code === 'SameSiteCookieIssue') {
+ return IssueCategory.SameSiteCookie;
+ }
+ if (code.startsWith('CrossOriginEmbedderPolicy')) {
+ return IssueCategory.CrossOriginEmbedderPolicy;
+ }
+ return IssueCategory.Other;
+ }
}
/**
@@ -107,7 +121,6 @@
}
if (resources.cookies) {
for (const cookie of resources.cookies) {
- IssuesModel.connectWithIssue(cookie, issue);
const key = JSON.stringify(cookie);
if (!this._cookies.has(key)) {
this._cookies.set(key, cookie);
diff --git a/front_end/sdk/IssuesModel.js b/front_end/sdk/IssuesModel.js
index 43781c1..01b34df 100644
--- a/front_end/sdk/IssuesModel.js
+++ b/front_end/sdk/IssuesModel.js
@@ -7,10 +7,11 @@
import {CookieModel} from './CookieModel.js';
import {AggregatedIssue, Issue} from './Issue.js';
import {NetworkManager} from './NetworkManager.js';
+import {NetworkRequest} from './NetworkRequest.js'; // eslint-disable-line no-unused-vars
+import * as RelatedIssue from './RelatedIssue.js';
import {Events as ResourceTreeModelEvents, ResourceTreeModel} from './ResourceTreeModel.js';
import {Capability, SDKModel, Target} from './SDKModel.js'; // eslint-disable-line no-unused-vars
-const connectedIssueSymbol = Symbol('issue');
/**
* @implements {Protocol.AuditsDispatcher}
@@ -43,7 +44,15 @@
*/
_onMainFrameNavigated(event) {
const mainFrame = /** @type {!SDK.ResourceTreeFrame} */ (event.data);
- this._issues = this._issues.filter(issue => issue.isAssociatedWithRequestId(mainFrame.loaderId));
+ const keptIssues = [];
+ for (const issue of this._issues) {
+ if (issue.isAssociatedWithRequestId(mainFrame.loaderId)) {
+ keptIssues.push(issue);
+ } else {
+ this._disconnectIssue(issue);
+ }
+ }
+ this._issues = keptIssues;
this._aggregatedIssuesByCode.clear();
for (const issue of this._issues) {
this._aggregateIssue(issue);
@@ -83,48 +92,69 @@
issueAdded(inspectorIssue) {
const issue = new Issue(inspectorIssue.code, inspectorIssue.resources);
this._issues.push(issue);
+ this._connectIssue(issue);
const aggregatedIssue = this._aggregateIssue(issue);
this.dispatchEventToListeners(Events.AggregatedIssueUpdated, aggregatedIssue);
}
/**
+ *
+ * @param {!Issue} issue
+ */
+ _connectIssue(issue) {
+ const resources = issue.resources();
+ if (!resources) {
+ return;
+ }
+ if (resources.requests) {
+ for (const resourceRequest of resources.requests) {
+ const request =
+ /** @type {?NetworkRequest} */ (
+ self.SDK.networkLog.requests().find(r => r.requestId() === resourceRequest.requestId));
+ if (request) {
+ // Connect the real network request with this issue and vice versa.
+ RelatedIssue.connect(request, issue.getCategory(), issue);
+ resourceRequest.request = request;
+ }
+ }
+ }
+ }
+
+ /**
+ *
+ * @param {!Issue} issue
+ */
+ _disconnectIssue(issue) {
+ const resources = issue.resources();
+ if (!resources) {
+ return;
+ }
+ if (resources.requests) {
+ for (const resourceRequest of resources.requests) {
+ const request =
+ /** @type {?NetworkRequest} */ (
+ self.SDK.networkLog.requests().find(r => r.requestId() === resourceRequest.requestId));
+ if (request) {
+ // Disconnect the real network request from this issue;
+ RelatedIssue.disconnect(request, issue.getCategory(), issue);
+ }
+ }
+ }
+ }
+
+ /**
* @returns {!Iterable<AggregatedIssue>}
*/
aggregatedIssues() {
return this._aggregatedIssuesByCode.values();
}
- /**
+ /**
* @return {number}
*/
numberOfAggregatedIssues() {
return this._aggregatedIssuesByCode.size;
}
-
- /**
- * @param {!*} obj
- * TODO(chromium:1063765): Strengthen types.
- * @param {!Issue} issue
- */
- static connectWithIssue(obj, issue) {
- if (!obj) {
- return;
- }
-
- if (!obj[connectedIssueSymbol]) {
- obj[connectedIssueSymbol] = new Set();
- }
-
- obj[connectedIssueSymbol].add(issue);
- }
-
- /**
- * @param {!*} obj
- * @returns {boolean}
- */
- static hasIssues(obj) {
- return !!obj && obj[connectedIssueSymbol] && obj[connectedIssueSymbol].size;
- }
}
/** @enum {symbol} */
@@ -133,4 +163,4 @@
FullUpdateRequired: Symbol('FullUpdateRequired'),
};
-SDKModel.register(IssuesModel, Capability.None, true);
+SDKModel.register(IssuesModel, Capability.Network, true);
diff --git a/front_end/sdk/RelatedIssue.js b/front_end/sdk/RelatedIssue.js
new file mode 100644
index 0000000..6f54d28
--- /dev/null
+++ b/front_end/sdk/RelatedIssue.js
@@ -0,0 +1,107 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import * as Common from '../common/common.js'; // eslint-disable-line no-unused-vars
+
+const connectedIssueSymbol = Symbol('issue');
+
+export const IssueCategory = {
+ CrossOriginEmbedderPolicy: Symbol('CrossOriginEmbedderPolicy'),
+ SameSiteCookie: Symbol('SameSiteCookie'),
+ Other: Symbol('Other')
+};
+
+/**
+ * @param {*} obj
+ * @param {symbol} category
+ * @param {*} issue
+ */
+export function connect(obj, category, issue) {
+ if (!obj) {
+ return;
+ }
+ if (!obj[connectedIssueSymbol]) {
+ obj[connectedIssueSymbol] = new Map();
+ }
+ const map = obj[connectedIssueSymbol];
+ if (!map.has(category)) {
+ map.set(category, new Set());
+ }
+ const set = map.get(category);
+ set.add(issue);
+}
+
+/**
+ * @param {*} obj
+ * @param {symbol} category
+ * @param {*} issue
+ */
+export function disconnect(obj, category, issue) {
+ if (!obj || !obj[connectedIssueSymbol]) {
+ return;
+ }
+ const map = obj[connectedIssueSymbol];
+ if (!map.has(category)) {
+ return;
+ }
+ const set = map.get(category);
+ set.delete(issue);
+}
+
+/**
+ * @param {*} obj
+ * @return {boolean}
+ */
+export function hasIssues(obj) {
+ if (!obj || !obj[connectedIssueSymbol]) {
+ return false;
+ }
+ const map = obj[connectedIssueSymbol];
+ if (map.size === 0) {
+ return false;
+ }
+ for (const set of map.values()) {
+ if (set.size > 0) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/**
+ * @param {*} obj
+ * @param {symbol} category
+ * @return {boolean}
+ */
+export function hasIssueOfCategory(obj, category) {
+ if (!obj || !obj[connectedIssueSymbol]) {
+ return false;
+ }
+ const map = obj[connectedIssueSymbol];
+ if (!map.has(category)) {
+ return false;
+ }
+ const set = map.get(category);
+ return set.size > 0;
+}
+
+/**
+ * @param {*} obj
+ * @param {symbol} category
+ * @return {!Promise<undefined>}
+ */
+export async function reveal(obj, category) {
+ if (!obj || !obj[connectedIssueSymbol]) {
+ return;
+ }
+ const map = obj[connectedIssueSymbol];
+ if (!map.has(category)) {
+ return;
+ }
+ const set = map.get(category);
+ if (set.size === 0) {
+ return;
+ }
+ return Common.Revealer.reveal(set.values().next().value);
+}
diff --git a/front_end/sdk/module.json b/front_end/sdk/module.json
index 46b1987..2e27ff2 100644
--- a/front_end/sdk/module.json
+++ b/front_end/sdk/module.json
@@ -420,6 +420,7 @@
"ProfileTreeModel.js",
"Issue.js",
"IssuesModel.js",
+ "RelatedIssue.js",
"ServerTiming.js",
"CPUProfileDataModel.js",
"CPUProfilerModel.js",
diff --git a/front_end/sdk/sdk.js b/front_end/sdk/sdk.js
index 8a7ab19..f70ff35 100644
--- a/front_end/sdk/sdk.js
+++ b/front_end/sdk/sdk.js
@@ -47,6 +47,7 @@
import * as PaintProfiler from './PaintProfiler.js';
import * as PerformanceMetricsModel from './PerformanceMetricsModel.js';
import * as ProfileTreeModel from './ProfileTreeModel.js';
+import * as RelatedIssue from './RelatedIssue.js';
import * as RemoteObject from './RemoteObject.js';
import * as Resource from './Resource.js';
import * as ResourceTreeModel from './ResourceTreeModel.js';
@@ -100,6 +101,7 @@
PaintProfiler,
PerformanceMetricsModel,
ProfileTreeModel,
+ RelatedIssue,
RemoteObject,
Resource,
ResourceTreeModel,
diff --git a/test/unittests/front_end/sdk/BUILD.gn b/test/unittests/front_end/sdk/BUILD.gn
index 1e9670c..2546751 100644
--- a/test/unittests/front_end/sdk/BUILD.gn
+++ b/test/unittests/front_end/sdk/BUILD.gn
@@ -4,6 +4,7 @@
testonly = true
sources = [
"Cookie_test.ts",
+ "RelatedIssue_test.ts",
"ServerTiming_test.ts",
]
diff --git a/test/unittests/front_end/sdk/RelatedIssue_test.ts b/test/unittests/front_end/sdk/RelatedIssue_test.ts
new file mode 100644
index 0000000..acbdb2e
--- /dev/null
+++ b/test/unittests/front_end/sdk/RelatedIssue_test.ts
@@ -0,0 +1,93 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const {assert} = chai;
+
+import {connect, disconnect, hasIssues, hasIssueOfCategory, IssueCategory} from '../../../../front_end/sdk/RelatedIssue.js';
+
+describe('hasIssues', () => {
+ it('should be false for fresh objects', () => {
+ const obj = {};
+ assert.isFalse(hasIssues(obj), 'obj should not have an issue');
+ });
+});
+
+describe('connect', () => {
+ it('should cause object to have issues', () => {
+ const obj = {};
+ const issue = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue);
+ assert.isTrue(hasIssues(obj), 'obj should have an issue');
+ });
+
+ it('should cause object to have a issue of a specific category', () => {
+ const obj = {};
+ const issue = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue);
+ assert.isTrue(hasIssueOfCategory(obj, IssueCategory.Other), 'obj should have an issue of the specified category');
+ assert.isFalse(
+ hasIssueOfCategory(obj, IssueCategory.CrossOriginEmbedderPolicy),
+ 'obj should not have an issue of a different category');
+ });
+
+ it('should cause object to have a issue after being called twice', () => {
+ const obj = {};
+ const issue = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue);
+ connect(obj, IssueCategory.Other, issue);
+ assert.isTrue(hasIssues(obj), 'obj should have an issue');
+ assert.isTrue(hasIssueOfCategory(obj, IssueCategory.Other), 'obj should have an issue of the specified category');
+ assert.isFalse(
+ hasIssueOfCategory(obj, IssueCategory.CrossOriginEmbedderPolicy),
+ 'obj should not have an issue of a different category');
+ });
+});
+
+describe('disconnect', () => {
+ it('should remove an issue', () => {
+ const obj = {};
+ const issue = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue);
+ assert.isTrue(hasIssues(obj), 'obj should have an issue');
+ disconnect(obj, IssueCategory.Other, issue);
+ assert.isFalse(hasIssues(obj), 'obj should not have an issue');
+ });
+
+ it('should remove an issue of a specific category', () => {
+ const obj = {};
+ const issue = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue);
+ connect(obj, IssueCategory.CrossOriginEmbedderPolicy, issue);
+ assert.isTrue(hasIssueOfCategory(obj, IssueCategory.Other), 'obj should have an issue of this category');
+ assert.isTrue(
+ hasIssueOfCategory(obj, IssueCategory.CrossOriginEmbedderPolicy),
+ 'obj should not have an issue of this category');
+ disconnect(obj, IssueCategory.Other, issue);
+ assert.isFalse(hasIssueOfCategory(obj, IssueCategory.Other), 'obj should no longer have an issue of this category');
+ assert.isTrue(
+ hasIssueOfCategory(obj, IssueCategory.CrossOriginEmbedderPolicy),
+ 'obj should still have an issue of this category');
+ });
+
+ it('should remove an issue even if connected twice', () => {
+ const obj = {};
+ const issue = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue);
+ connect(obj, IssueCategory.Other, issue);
+ disconnect(obj, IssueCategory.Other, issue);
+ assert.isFalse(hasIssues(obj), 'obj should not have an issue');
+ assert.isFalse(hasIssueOfCategory(obj, IssueCategory.Other), 'obj should not have an issue');
+ });
+
+ it('should remove only the specified issue', () => {
+ const obj = {};
+ const issue1 = {}; // Use a dummy object;
+ const issue2 = {}; // Use a dummy object;
+ connect(obj, IssueCategory.Other, issue1);
+ connect(obj, IssueCategory.Other, issue2);
+ disconnect(obj, IssueCategory.Other, issue1);
+ assert.isTrue(hasIssues(obj), 'obj should still have an issue');
+ assert.isTrue(hasIssueOfCategory(obj, IssueCategory.Other), 'obj should still have an issue');
+ });
+});
