Skip to content

Commit b4a178e

Browse files
thefrog-ghDevtools-frontend LUCI CQ
authored andcommitted
Add warning icons for error events
All changes in this CL are scoped to the Application panel. Bug: 471021582, 471017387 Change-Id: I98ea0c8aaff947f0cd224b0549dfb837cd10a7bf Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7511225 Reviewed-by: Paul Irish <paulirish@chromium.org> Commit-Queue: thefrog <thefrog@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org>
1 parent 0e56341 commit b4a178e

5 files changed

Lines changed: 184 additions & 13 deletions

File tree

front_end/panels/application/DeviceBoundSessionsModel.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,75 @@ describeWithMockConnection('DeviceBoundSessionsModel', () => {
371371
assert.isFalse(session?.isSessionTerminated);
372372
});
373373

374+
it('updates hasErrors status correctly on failed events and clear events', () => {
375+
const site = 'example.com';
376+
const sessionId = 'session_error_test';
377+
378+
// New session does not have errors.
379+
const createEvent: Protocol.Network.DeviceBoundSessionEventOccurredEvent = {
380+
eventId: 'event1' as Protocol.Network.DeviceBoundSessionEventId,
381+
site,
382+
sessionId,
383+
succeeded: true,
384+
creationEventDetails: {
385+
newSession: makeSession(site, sessionId),
386+
fetchResult: Protocol.Network.DeviceBoundSessionFetchResult.Success
387+
}
388+
};
389+
networkManager.dispatchEventToListeners(SDK.NetworkManager.Events.DeviceBoundSessionEventOccurred, createEvent);
390+
391+
assert.isFalse(model.sessionHasErrors(site, sessionId));
392+
const session = model.getSession(site, sessionId);
393+
assert.isFalse(session?.hasErrors);
394+
395+
// It has errors for a failed event.
396+
const failedEvent: Protocol.Network.DeviceBoundSessionEventOccurredEvent = {
397+
eventId: 'event2' as Protocol.Network.DeviceBoundSessionEventId,
398+
site,
399+
sessionId,
400+
succeeded: false,
401+
creationEventDetails: {fetchResult: Protocol.Network.DeviceBoundSessionFetchResult.InvalidConfigJson}
402+
};
403+
networkManager.dispatchEventToListeners(SDK.NetworkManager.Events.DeviceBoundSessionEventOccurred, failedEvent);
404+
405+
assert.isTrue(model.sessionHasErrors(site, sessionId));
406+
assert.isTrue(session?.hasErrors);
407+
408+
// It still has errors after a subsequent successful event.
409+
const successEvent: Protocol.Network.DeviceBoundSessionEventOccurredEvent = {
410+
eventId: 'event3' as Protocol.Network.DeviceBoundSessionEventId,
411+
site,
412+
sessionId,
413+
succeeded: true,
414+
challengeEventDetails:
415+
{challenge: 'challenge', challengeResult: Protocol.Network.ChallengeEventDetailsChallengeResult.Success}
416+
};
417+
networkManager.dispatchEventToListeners(SDK.NetworkManager.Events.DeviceBoundSessionEventOccurred, successEvent);
418+
419+
assert.isTrue(model.sessionHasErrors(site, sessionId));
420+
assert.isTrue(session?.hasErrors);
421+
422+
const listener = sinon.spy();
423+
model.addEventListener(Application.DeviceBoundSessionsModel.DeviceBoundSessionModelEvents.CLEAR_EVENTS, listener);
424+
425+
// Errors are not cleared when clearEvents is called when preserving the log.
426+
Common.Settings.moduleSetting('device-bound-sessions-preserve-log').set(true);
427+
model.clearEvents();
428+
sinon.assert.notCalled(listener);
429+
assert.isTrue(model.sessionHasErrors(site, sessionId));
430+
assert.isTrue(session?.hasErrors);
431+
432+
// Errors are cleared when clearEvents is called when not preserving the log.
433+
Common.Settings.moduleSetting('device-bound-sessions-preserve-log').set(false);
434+
model.clearEvents();
435+
sinon.assert.calledOnce(listener);
436+
const noLongerFailedSessions = listener.firstCall.args[0].data.noLongerFailedSessions;
437+
assert.strictEqual(noLongerFailedSessions.size, 1);
438+
assert.deepEqual(noLongerFailedSessions.get(site), [sessionId]);
439+
assert.isFalse(model.sessionHasErrors(site, sessionId));
440+
assert.isFalse(session?.hasErrors);
441+
});
442+
374443
it('returns false for isSessionTerminated when session does not exist', () => {
375444
assert.isFalse(model.isSessionTerminated('unknown-site', 'unknown-session'));
376445
});

front_end/panels/application/DeviceBoundSessionsModel.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ interface EventWithTimestamp {
1313
export interface SessionAndEvents {
1414
session?: Protocol.Network.DeviceBoundSession;
1515
isSessionTerminated: boolean;
16+
hasErrors: boolean;
1617
eventsById: Map<string, EventWithTimestamp>;
1718
}
1819
type SessionIdToSessionMap = Map<string|undefined, SessionAndEvents>;
@@ -61,11 +62,21 @@ export class DeviceBoundSessionsModel extends Common.ObjectWrapper.ObjectWrapper
6162
return;
6263
}
6364
const emptySessions = new Map<string, Array<string|undefined>>();
65+
const noLongerFailedSessions = new Map<string, Array<string|undefined>>();
6466
const emptySites = new Set<string>();
6567
for (const [site, sessionIdToSessionMap] of [...this.#siteSessions]) {
6668
let emptySessionsSiteEntry = emptySessions.get(site);
69+
let noLongerFailedSessionsSiteEntry = noLongerFailedSessions.get(site);
6770
for (const [sessionId, sessionAndEvents] of sessionIdToSessionMap) {
6871
sessionAndEvents.eventsById.clear();
72+
if (sessionAndEvents.hasErrors) {
73+
sessionAndEvents.hasErrors = false;
74+
if (!noLongerFailedSessionsSiteEntry) {
75+
noLongerFailedSessionsSiteEntry = [];
76+
noLongerFailedSessions.set(site, noLongerFailedSessionsSiteEntry);
77+
}
78+
noLongerFailedSessionsSiteEntry.push(sessionId);
79+
}
6980
if (sessionAndEvents.session) {
7081
continue;
7182
}
@@ -85,7 +96,8 @@ export class DeviceBoundSessionsModel extends Common.ObjectWrapper.ObjectWrapper
8596
}
8697
}
8798

88-
this.dispatchEventToListeners(DeviceBoundSessionModelEvents.CLEAR_EVENTS, {emptySessions, emptySites});
99+
this.dispatchEventToListeners(
100+
DeviceBoundSessionModelEvents.CLEAR_EVENTS, {emptySessions, emptySites, noLongerFailedSessions});
89101
}
90102

91103
isSiteVisible(site: string): boolean {
@@ -100,6 +112,14 @@ export class DeviceBoundSessionsModel extends Common.ObjectWrapper.ObjectWrapper
100112
return session.isSessionTerminated;
101113
}
102114

115+
sessionHasErrors(site: string, sessionId?: string): boolean {
116+
const session = this.getSession(site, sessionId);
117+
if (session === undefined) {
118+
return false;
119+
}
120+
return session.hasErrors;
121+
}
122+
103123
getSession(site: string, sessionId?: string): SessionAndEvents|undefined {
104124
return this.#siteSessions.get(site)?.get(sessionId);
105125
}
@@ -128,6 +148,7 @@ export class DeviceBoundSessionsModel extends Common.ObjectWrapper.ObjectWrapper
128148
sessionAndEvent = {
129149
session: undefined,
130150
isSessionTerminated: false,
151+
hasErrors: false,
131152
eventsById: new Map<string, EventWithTimestamp>()
132153
};
133154
sessionIdToSessionMap.set(sessionId, sessionAndEvent);
@@ -167,6 +188,11 @@ export class DeviceBoundSessionsModel extends Common.ObjectWrapper.ObjectWrapper
167188
}
168189
}
169190

191+
// Set that the session has errors if the latest event failed.
192+
if (!event.succeeded) {
193+
sessionAndEvent.hasErrors = true;
194+
}
195+
170196
this.dispatchEventToListeners(
171197
DeviceBoundSessionModelEvents.EVENT_OCCURRED,
172198
{site: eventWithTimestamp.event.site, sessionId: eventWithTimestamp.event.sessionId});
@@ -186,6 +212,9 @@ export interface DeviceBoundSessionModelEventTypes {
186212
[DeviceBoundSessionModelEvents.ADD_VISIBLE_SITE]: {site: string};
187213
[DeviceBoundSessionModelEvents.CLEAR_VISIBLE_SITES]: void;
188214
[DeviceBoundSessionModelEvents.EVENT_OCCURRED]: {site: string, sessionId?: string};
189-
[DeviceBoundSessionModelEvents.CLEAR_EVENTS]:
190-
{emptySessions: Map<string, Array<string|undefined>>, emptySites: Set<string>};
215+
[DeviceBoundSessionModelEvents.CLEAR_EVENTS]: {
216+
emptySessions: Map<string, Array<string|undefined>>,
217+
emptySites: Set<string>,
218+
noLongerFailedSessions: Map<string, Array<string|undefined>>,
219+
};
191220
}

front_end/panels/application/DeviceBoundSessionsTreeElement.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as SDK from '../../core/sdk/sdk.js';
66
import type * as Protocol from '../../generated/protocol.js';
77
import {createTarget} from '../../testing/EnvironmentHelpers.js';
88
import {describeWithMockConnection} from '../../testing/MockConnection.js';
9+
import type {TreeElement} from '../../ui/legacy/Treeoutline.js';
910

1011
import * as Application from './application.js';
1112
import type {ResourcesPanel} from './ResourcesPanel.js';
@@ -135,7 +136,8 @@ describeWithMockConnection('DeviceBoundSessionsTreeElement', () => {
135136
['example2.com', ['session_1']],
136137
['hidden.com', ['session_1']],
137138
]),
138-
emptySites: new Set(['example2.com', 'hidden.com'])
139+
emptySites: new Set(['example2.com', 'hidden.com']),
140+
noLongerFailedSessions: new Map(),
139141
});
140142

141143
model.addVisibleSite('hidden.com');
@@ -339,4 +341,51 @@ describeWithMockConnection('DeviceBoundSessionsTreeElement', () => {
339341
assert.isFalse(session1Node.listItemElement.classList.contains('device-bound-session-terminated'));
340342
assert.isFalse(session2Node.listItemElement.classList.contains('device-bound-session-terminated'));
341343
});
344+
345+
it('updates the session tree element visual state when a session has errors', () => {
346+
const root = new Application.DeviceBoundSessionsTreeElement.RootTreeElement(mockPanel, model);
347+
root.onbind();
348+
349+
const site = 'example.com';
350+
const sessionId = 'session_1';
351+
const sessionId2 = 'session_2';
352+
model.addVisibleSite(site);
353+
354+
const session = makeSession(site, sessionId);
355+
const session2 = makeSession(site, sessionId2);
356+
model.dispatchEventToListeners(
357+
Application.DeviceBoundSessionsModel.DeviceBoundSessionModelEvents.INITIALIZE_SESSIONS,
358+
{sessions: [session, session2]});
359+
const siteNode = root.children()[0];
360+
const sessionNode = siteNode.children()[0];
361+
const sessionNode2 = siteNode.children()[1];
362+
363+
function checkIcon(node: TreeElement, expectedIcon: string) {
364+
const icon = node.listItemElement.querySelector('devtools-icon');
365+
assert.exists(icon);
366+
assert.strictEqual(icon.getAttribute('name'), expectedIcon);
367+
}
368+
369+
// Initially has database icon.
370+
checkIcon(sessionNode, 'database');
371+
checkIcon(sessionNode2, 'database');
372+
373+
// A failed event should change it to a warning icon.
374+
const sessionHasErrorsStub = sinon.stub(model, 'sessionHasErrors');
375+
sessionHasErrorsStub.withArgs(site, sessionId).returns(true);
376+
model.dispatchEventToListeners(
377+
Application.DeviceBoundSessionsModel.DeviceBoundSessionModelEvents.EVENT_OCCURRED, {site, sessionId});
378+
checkIcon(sessionNode, 'warning');
379+
checkIcon(sessionNode2, 'database');
380+
381+
// Clearing events should change it back to a database icon.
382+
sessionHasErrorsStub.withArgs(site, sessionId).returns(false);
383+
model.dispatchEventToListeners(Application.DeviceBoundSessionsModel.DeviceBoundSessionModelEvents.CLEAR_EVENTS, {
384+
emptySessions: new Map(),
385+
emptySites: new Set(),
386+
noLongerFailedSessions: new Map([[site, [sessionId]]]),
387+
});
388+
checkIcon(sessionNode, 'database');
389+
checkIcon(sessionNode2, 'database');
390+
});
342391
});

front_end/panels/application/DeviceBoundSessionsTreeElement.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,20 @@ export class RootTreeElement extends ApplicationPanelTreeElement {
120120
}
121121
}
122122

123-
#updateTerminatedSessionDisplay(site: string, sessionId: string|undefined): void {
123+
#updateElementIconAndStyling(
124+
sessionElement: ApplicationPanelTreeElement, isSessionTerminated: boolean, sessionHasErrors: boolean): void {
125+
if (isSessionTerminated) {
126+
sessionElement.listItemElement.classList.add('device-bound-session-terminated');
127+
sessionElement.setLeadingIcons([createIcon('database-off')]);
128+
return;
129+
}
130+
sessionElement.listItemElement.classList.remove('device-bound-session-terminated');
131+
sessionElement.setLeadingIcons([createIcon(sessionHasErrors ? 'warning' : 'database')]);
132+
}
133+
134+
#updateIconAndStyling(site: string, sessionId: string|undefined): void {
124135
const isSessionTerminated = this.#model.isSessionTerminated(site, sessionId);
136+
const sessionHasErrors = this.#model.sessionHasErrors(site, sessionId);
125137
const siteMapEntry = this.#sites.get(site);
126138
if (!siteMapEntry) {
127139
return;
@@ -130,12 +142,21 @@ export class RootTreeElement extends ApplicationPanelTreeElement {
130142
if (!sessionElement) {
131143
return;
132144
}
133-
if (isSessionTerminated) {
134-
sessionElement.listItemElement.classList.add('device-bound-session-terminated');
135-
sessionElement.setLeadingIcons([createIcon('database-off')]);
136-
} else {
137-
sessionElement.listItemElement.classList.remove('device-bound-session-terminated');
138-
sessionElement.setLeadingIcons([createIcon('database')]);
145+
this.#updateElementIconAndStyling(sessionElement, isSessionTerminated, sessionHasErrors);
146+
}
147+
148+
#removeWarningIcons(noLongerFailedSessions: Map<string, Array<string|undefined>>): void {
149+
for (const [site, noLongerFailedSessionIds] of noLongerFailedSessions) {
150+
const siteData = this.#sites.get(site);
151+
if (siteData) {
152+
for (const noLongerFailedSessionId of noLongerFailedSessionIds) {
153+
const sessionElement = siteData.sessions.get(noLongerFailedSessionId);
154+
if (sessionElement) {
155+
const isSessionTerminated = this.#model.isSessionTerminated(site, noLongerFailedSessionId);
156+
this.#updateElementIconAndStyling(sessionElement, isSessionTerminated, /* sessionHasErrors=*/ false);
157+
}
158+
}
159+
}
139160
}
140161
}
141162

@@ -231,12 +252,13 @@ export class RootTreeElement extends ApplicationPanelTreeElement {
231252
{data: {site, sessionId}}: Common.EventTarget
232253
.EventTargetEvent<DeviceBoundSessionModelEventTypes[DeviceBoundSessionModelEvents.EVENT_OCCURRED]>): void {
233254
this.#addSiteSessionIfMissing(site, sessionId);
234-
this.#updateTerminatedSessionDisplay(site, sessionId);
255+
this.#updateIconAndStyling(site, sessionId);
235256
}
236257

237-
#onClearEvents({data: {emptySessions, emptySites}}: Common.EventTarget
258+
#onClearEvents({data: {emptySessions, emptySites, noLongerFailedSessions}}: Common.EventTarget
238259
.EventTargetEvent<DeviceBoundSessionModelEventTypes[DeviceBoundSessionModelEvents.CLEAR_EVENTS]>):
239260
void {
240261
this.#removeEmptyElements(emptySessions, emptySites);
262+
this.#removeWarningIcons(noLongerFailedSessions);
241263
}
242264
}

front_end/panels/application/DeviceBoundSessionsView.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ describeWithMockConnection('DeviceBoundSessionsView', () => {
2929
return {
3030
eventsById: new Map(),
3131
isSessionTerminated: false,
32+
hasErrors: false,
3233
session: {
3334
key: {site: mockSite, id: mockSessionId},
3435
refreshUrl: 'https://example.com/refresh',
@@ -326,6 +327,7 @@ describeWithMockConnection('DeviceBoundSessionsView', () => {
326327
const sessionAndEvents: Application.DeviceBoundSessionsModel.SessionAndEvents = {
327328
eventsById: new Map(),
328329
isSessionTerminated: false,
330+
hasErrors: false,
329331
session: undefined,
330332
};
331333
const date = new Date('2026-01-01T10:00:00.000Z');

0 commit comments

Comments
 (0)