Skip to content

Commit adf55dc

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: make args.data optional for console.timestamp events
We have a trace that has a console timestamp trace event with no `args.data`, and this breaks the code. This CL updates the types to mark `args.data` as optional and then updates the code accordingly. Bug: none Change-Id: I6167b016cee20da0fa70a82bcbe6f2565b4f654c Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6221296 Auto-Submit: Jack Franklin <[email protected]> Reviewed-by: Andres Olivares <[email protected]> Commit-Queue: Andres Olivares <[email protected]> Commit-Queue: Jack Franklin <[email protected]>
1 parent beeb30b commit adf55dc

File tree

7 files changed

+14
-8
lines changed

7 files changed

+14
-8
lines changed

front_end/models/trace/extras/TraceTree.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ export function generateEventID(event: Types.Events.Event): string {
589589
return `f:${name}@${location}`;
590590
}
591591

592-
if (Types.Events.isConsoleTimeStamp(event)) {
592+
if (Types.Events.isConsoleTimeStamp(event) && event.args.data) {
593593
return `${event.name}:${event.args.data.name}`;
594594
}
595595

front_end/models/trace/handlers/ExtensionTraceDataHandler.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ function createExtensionFlameChartEntries(): void {
8181
export function extractConsoleAPIExtensionEntries(): void {
8282
const consoleTimeStamps: readonly Types.Events.ConsoleTimeStamp[] = userTimingsData().timestampEvents;
8383
for (const currentTimeStamp of consoleTimeStamps) {
84+
if (!currentTimeStamp.args.data) {
85+
continue;
86+
}
8487
const timeStampName = String(currentTimeStamp.args.data.name);
8588
timeStampByName.set(timeStampName, currentTimeStamp);
8689
const extensionData = extensionDataInConsoleTimeStamp(currentTimeStamp);
@@ -254,6 +257,9 @@ export function extensionDataInPerformanceTiming(timing: Types.Events.SyntheticU
254257
*/
255258
export function extensionDataInConsoleTimeStamp(timeStamp: Types.Events.ConsoleTimeStamp):
256259
Types.Extensions.ExtensionTrackEntryPayload|null {
260+
if (!timeStamp.args.data) {
261+
return null;
262+
}
257263
const trackName = timeStamp.args.data.track;
258264
if (trackName === '' || trackName === undefined) {
259265
return null;

front_end/models/trace/handlers/UserTimingsHandler.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,9 @@ describeWithEnvironment('UserTimingsHandler', function() {
174174
describe('console.timestamp events parsing', function() {
175175
it('parses console.timestamp events correctly', async function() {
176176
assert.lengthOf(timingsData.timestampEvents, 3);
177-
assert.strictEqual(timingsData.timestampEvents[0].args.data.name, 'a timestamp');
178-
assert.strictEqual(timingsData.timestampEvents[1].args.data.name, 'another timestamp');
179-
assert.strictEqual(timingsData.timestampEvents[2].args.data.name, 'yet another timestamp');
177+
assert.strictEqual(timingsData.timestampEvents[0].args.data?.name, 'a timestamp');
178+
assert.strictEqual(timingsData.timestampEvents[1].args.data?.name, 'another timestamp');
179+
assert.strictEqual(timingsData.timestampEvents[2].args.data?.name, 'yet another timestamp');
180180
});
181181
});
182182
});

front_end/models/trace/types/TraceEvents.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1412,7 +1412,7 @@ export interface ConsoleTimeStamp extends Event {
14121412
name: Name.CONSOLE_TIME_STAMP;
14131413
ph: Phase.COMPLETE;
14141414
args: Args&{
1415-
data: ArgsData & {
1415+
data?: ArgsData & {
14161416
// The console.timeStamp allows to pass integers as values as well
14171417
// as strings
14181418
name: string | number,

front_end/panels/timeline/TimelineUIUtils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ export class TimelineUIUtils {
655655
if (Trace.Helpers.Trace.eventHasCategory(event, Trace.Types.Events.Categories.Console)) {
656656
return title;
657657
}
658-
if (Trace.Types.Events.isConsoleTimeStamp(event)) {
658+
if (Trace.Types.Events.isConsoleTimeStamp(event) && event.args.data) {
659659
return i18nString(UIStrings.sS, {PH1: title, PH2: event.args.data.name});
660660
}
661661
if (Trace.Types.Events.isAnimation(event) && event.args.data.name) {

front_end/panels/timeline/TimingsTrackAppender.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ export class TimingsTrackAppender implements TrackAppender {
244244
}
245245
}
246246
if (Trace.Types.Events.isConsoleTimeStamp(event)) {
247-
return `TimeStamp: ${event.args.data.name}`;
247+
return `TimeStamp: ${event.args.data?.name ?? '(name unknown)'}`;
248248
}
249249
if (Trace.Types.Events.isPerformanceMark(event)) {
250250
return `[mark]: ${event.name}`;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ describeWithEnvironment('TimingTrackAppender', function() {
154154
it('returns the correct title for console timestamps', () => {
155155
const traceMarkers = parsedTrace.UserTimings.timestampEvents;
156156
for (const mark of traceMarkers) {
157-
assert.strictEqual(timingsTrackAppender.titleForEvent(mark), `TimeStamp: ${mark.args.data.name}`);
157+
assert.strictEqual(timingsTrackAppender.titleForEvent(mark), `TimeStamp: ${mark.args.data?.name}`);
158158
}
159159
});
160160
});

0 commit comments

Comments
 (0)