Skip to content

Commit 2bc8417

Browse files
and-oliDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Use user timing name as flamechart entry title for extension data
Instead of the tooltip text. This behavior was accidentally flipped in [1] and the tooltip text was used as the title of the event, resulting in the attached bug. [1] https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6007341/8/front_end/panels/timeline/ExtensionTrackAppender.ts Fixed: 384480570 Change-Id: Ie1f6d93bbe5afc964f6b5c16bb9856dcdf259cb5 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6098154 Auto-Submit: Andres Olivares <[email protected]> Reviewed-by: Nancy Li <[email protected]> Commit-Queue: Nancy Li <[email protected]>
1 parent 1edfffb commit 2bc8417

File tree

2 files changed

+29
-6
lines changed

2 files changed

+29
-6
lines changed

front_end/panels/timeline/ExtensionTrackAppender.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@ import * as i18n from '../../core/i18n/i18n.js';
55
import * as Trace from '../../models/trace/trace.js';
66
import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
77

8-
import {buildGroupStyle, buildTrackHeader} from './AppenderUtils.js';
8+
import {buildGroupStyle, buildTrackHeader, getFormattedTime} from './AppenderUtils.js';
99
import {
1010
type CompatibilityTracksAppender,
11+
type PopoverInfo,
1112
type TrackAppender,
1213
type TrackAppenderName,
1314
VisualLoggingTrackName,
@@ -99,9 +100,13 @@ export class ExtensionTrackAppender implements TrackAppender {
99100
}
100101

101102
titleForEvent(event: Trace.Types.Events.Event): string {
102-
if (Trace.Types.Extensions.isSyntheticExtensionEntry(event) && event.args.tooltipText) {
103-
return event.args.tooltipText;
104-
}
105103
return event.name;
106104
}
105+
106+
setPopoverInfo(event: Trace.Types.Events.Event, info: PopoverInfo): void {
107+
info.title = Trace.Types.Extensions.isSyntheticExtensionEntry(event) && event.args.tooltipText ?
108+
event.args.tooltipText :
109+
this.titleForEvent(event);
110+
info.formattedTime = getFormattedTime(event.dur);
111+
}
107112
}

front_end/panels/timeline/track_appenders/ExtensionTrackAppender.test.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ describeWithEnvironment('ExtensionTrackAppender', function() {
162162
const allExtensionTrackEntries =
163163
parsedTrace.ExtensionTraceData.extensionTrackData.map(track => Object.values(track.entriesByTrack)).flat(2);
164164
for (const event of allExtensionTrackEntries) {
165-
assert.notStrictEqual(extensionTrackAppenders[0].titleForEvent(event), event.name);
165+
assert.strictEqual(extensionTrackAppenders[0].titleForEvent(event), event.name);
166166
if (event.args.color === 'tertiary') {
167167
// "tertiary" color category is mapped to --ref-palette-green70
168168
// which is faked out to 10, 10, 10
@@ -222,8 +222,26 @@ describeWithEnvironment('ExtensionTrackAppender', function() {
222222
const allExtensionTrackEntries =
223223
parsedTrace.ExtensionTraceData.extensionTrackData.map(track => Object.values(track.entriesByTrack)).flat(2);
224224
const title = extensionTrackAppenders[0].titleForEvent(allExtensionTrackEntries[0]);
225+
assert.strictEqual(title, 'An extension measurement');
226+
});
227+
});
228+
229+
describe('setPopoverInfo', function() {
230+
it('build the tooltip content for an entry correctly', function() {
231+
const info: Timeline.CompatibilityTracksAppender.PopoverInfo = {
232+
title: 'title',
233+
formattedTime: 'time',
234+
warningElements: [],
235+
additionalElements: [],
236+
url: null,
237+
};
238+
const allExtensionTrackEntries =
239+
parsedTrace.ExtensionTraceData.extensionTrackData.map(track => Object.values(track.entriesByTrack)).flat(2);
240+
extensionTrackAppenders[0].setPopoverInfo(allExtensionTrackEntries[0], info);
241+
assert.strictEqual(info.title, 'A hint if needed');
225242
// The i18n encodes spaces using the u00A0 unicode character.
226-
assert.strictEqual(title, 'A hint if needed');
243+
assert.strictEqual(info.formattedTime, '1.00\u00A0s');
244+
assert.strictEqual(info.additionalElements?.at(0)?.nodeName, undefined);
227245
});
228246
});
229247
});

0 commit comments

Comments
 (0)