Skip to content

Commit 64efc03

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Revert "Reland "Support console timestamp measurements with performance.now() timestamps""
This reverts commit 5bc0a06. Reason for revert: Compile errors on CI Original change's description: > Reland "Support console timestamp measurements with performance.now() timestamps" > > This is a reland of commit 1693f4f > > The culprit stems from a stale build state triggered by the update to a value our Event.Name const enum, which isn't always picked up by release builds. This causes timestamps events to not be added to the flamechart due to the lack of a style for it (since event styles are defined by event name). To work around this, the key of the updated enum entry is updated > (See https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6372399/3..12) > Original change's description: > > Support console timestamp measurements with performance.now() timestamps > > > > Relevant Chromium CL: https://chromium-review.googlesource.com/c/chromium/src/+/6311629 > > > > Bug: 399851200 > > Change-Id: I082d27bf0b791a439748f5c100bb791c1588145e > > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6308724 > > Reviewed-by: Nancy Li <[email protected]> > > Commit-Queue: Nancy Li <[email protected]> > > Auto-Submit: Andres Olivares <[email protected]> > > Bug: 399851200 > Change-Id: I9353bfb41250a7de6923613d371dbe29deda9f3f > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6372399 > Commit-Queue: Nancy Li <[email protected]> > Reviewed-by: Nancy Li <[email protected]> > Auto-Submit: Andres Olivares <[email protected]> Bug: 399851200 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: I581168e23b533da24639903902504caf7b7b6e02 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6373029 Commit-Queue: Rubber Stamper <[email protected]> Auto-Submit: Philip Pfaffe <[email protected]> Bot-Commit: Rubber Stamper <[email protected]>
1 parent 5bc0a06 commit 64efc03

File tree

13 files changed

+59
-89
lines changed

13 files changed

+59
-89
lines changed

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

Lines changed: 26 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ export interface PerformanceAPIExtensionTestData {
1414

1515
export interface ConsoleAPIExtensionTestData {
1616
name: string;
17-
start?: string|number;
18-
end?: string|number;
17+
start?: string;
18+
end?: string;
1919
track?: string;
2020
trackGroup?: string;
2121
color?: string;
@@ -59,13 +59,13 @@ function makeTimingEventWithPerformanceExtensionData(
5959
function makeTimingEventWithConsoleExtensionData({name, ts, start, end, track, trackGroup, color}:
6060
ConsoleAPIExtensionTestData): Trace.Types.Events.ConsoleTimeStamp {
6161
return {
62-
cat: 'devtools.timeline',
62+
cat: 'disabled-by-default-v8.inspector',
6363
pid: Trace.Types.Events.ProcessID(2017),
6464
tid: Trace.Types.Events.ThreadID(259),
65-
name: Trace.Types.Events.Name.TIME_STAMP,
65+
name: Trace.Types.Events.Name.CONSOLE_TIME_STAMP,
6666
args: {
6767
data: {
68-
message: name,
68+
name,
6969
start,
7070
end,
7171
track,
@@ -74,7 +74,7 @@ function makeTimingEventWithConsoleExtensionData({name, ts, start, end, track, t
7474
}
7575
},
7676
ts: Trace.Types.Timing.Micro(ts),
77-
ph: Trace.Types.Events.Phase.INSTANT,
77+
ph: Trace.Types.Events.Phase.COMPLETE,
7878

7979
};
8080
}
@@ -441,7 +441,7 @@ describe('ExtensionTraceDataHandler', function() {
441441
assert.deepEqual(Object.keys(fourthTrackData.entriesByTrack), ['Ungrouped Track 2']);
442442
assert.deepEqual(Object.values(fourthTrackData.entriesByTrack).map(entries => entries.length), [2]);
443443
});
444-
it('calculates self time sub track by sub track for events added with the performance API', async function() {
444+
it('calculates self time sub track by sub track', async function() {
445445
const extensionDevToolsObjects: PerformanceAPIExtensionTestData[] = [
446446
// Track group 1
447447
{
@@ -622,20 +622,6 @@ describe('ExtensionTraceDataHandler', function() {
622622
end: 'Mark 4',
623623
ts: 300,
624624
},
625-
// numeric start and end
626-
{
627-
track: 'Custom track 1',
628-
name: 'Measure 6',
629-
start: 300,
630-
end: 400,
631-
ts: 300,
632-
},
633-
{
634-
track: 'Custom track 1',
635-
name: 'Measure 7',
636-
start: 350,
637-
ts: 400,
638-
}
639625
];
640626
return createTraceExtensionDataFromConsoleAPITestInput(extensionData);
641627
}
@@ -646,26 +632,23 @@ describe('ExtensionTraceDataHandler', function() {
646632
});
647633
it('parses track data correctly', async () => {
648634
assert.lengthOf(extensionHandlerOutput.extensionTrackData, 3);
649-
const expectedData: Record<string, Array<{name: string, ts: number, dur: number}>> = {
650-
'Custom track 3': [{name: 'Measure 3', ts: 100, dur: 200}, {name: 'Measure 4', ts: 100, dur: 100}],
651-
'Custom track 1': [
652-
{name: 'Measure 1', ts: 100, dur: 100}, {name: 'Measure 6', ts: 300, dur: 100},
653-
{name: 'Measure 7', ts: 350, dur: 50}
654-
],
655-
'Custom track 2': [{name: 'Measure 2', ts: 100, dur: 100}]
656-
};
657-
for (let i = 0; i < extensionHandlerOutput.extensionTrackData.length; i++) {
658-
const track = extensionHandlerOutput.extensionTrackData[i];
659-
assert.strictEqual(track.name, Object.keys(expectedData)[i]);
660-
const actualTrackData = track.entriesByTrack[track.name];
661-
const expectedTrackData = expectedData[track.name];
662-
for (let j = 0; j < actualTrackData.length; j++) {
663-
const {name, ts, dur} = actualTrackData[j];
664-
assert.strictEqual(name, expectedTrackData[j].name);
665-
assert.strictEqual(ts, expectedTrackData[j].ts);
666-
assert.strictEqual(dur, expectedTrackData[j].dur);
667-
}
668-
}
635+
636+
assert.strictEqual(extensionHandlerOutput.extensionTrackData[0].name, 'Custom track 3');
637+
assert.lengthOf(extensionHandlerOutput.extensionTrackData[0].entriesByTrack['Custom track 3'], 2);
638+
assert.strictEqual(
639+
extensionHandlerOutput.extensionTrackData[0].entriesByTrack['Custom track 3'][0].name, 'Measure 3');
640+
assert.strictEqual(
641+
extensionHandlerOutput.extensionTrackData[0].entriesByTrack['Custom track 3'][1].name, 'Measure 4');
642+
643+
assert.strictEqual(extensionHandlerOutput.extensionTrackData[1].name, 'Custom track 1');
644+
assert.lengthOf(extensionHandlerOutput.extensionTrackData[1].entriesByTrack['Custom track 1'], 1);
645+
assert.strictEqual(
646+
extensionHandlerOutput.extensionTrackData[1].entriesByTrack['Custom track 1'][0].name, 'Measure 1');
647+
648+
assert.strictEqual(extensionHandlerOutput.extensionTrackData[2].name, 'Custom track 2');
649+
assert.lengthOf(extensionHandlerOutput.extensionTrackData[2].entriesByTrack['Custom track 2'], 1);
650+
assert.strictEqual(
651+
extensionHandlerOutput.extensionTrackData[2].entriesByTrack['Custom track 2'][0].name, 'Measure 2');
669652
});
670653
it('parses synthetic console timings for the timings track', async () => {
671654
assert.lengthOf(extensionHandlerOutput.syntheticConsoleEntriesForTimingsTrack, 1);
@@ -752,7 +735,7 @@ describe('ExtensionTraceDataHandler', function() {
752735
assert.deepEqual(Object.keys(fourthTrackData.entriesByTrack), ['Ungrouped Track 2']);
753736
assert.deepEqual(Object.values(fourthTrackData.entriesByTrack).map(entries => entries.length), [2]);
754737
});
755-
it('calculates self time sub track by sub track for events added with the console API', async function() {
738+
it('calculates self time sub track by sub track', async function() {
756739
const extensionDevToolsObjects: ConsoleAPIExtensionTestData[] = [
757740
// Track group 1
758741
{
@@ -809,13 +792,6 @@ describe('ExtensionTraceDataHandler', function() {
809792
start: 'F',
810793
ts: 80,
811794
},
812-
{
813-
trackGroup: 'Group 1',
814-
track: 'Track 1',
815-
name: 'Measurement 5',
816-
start: 75,
817-
ts: 80,
818-
},
819795
{
820796
name: 'G',
821797
ts: 0,
@@ -872,8 +848,7 @@ describe('ExtensionTraceDataHandler', function() {
872848
{name: 'Measurement 1', selfTime: 40},
873849
{name: 'Measurement 2', selfTime: 20},
874850
{name: 'Measurement 3', selfTime: 30},
875-
{name: 'Measurement 4', selfTime: 5},
876-
{name: 'Measurement 5', selfTime: 5},
851+
{name: 'Measurement 4', selfTime: 10},
877852
]);
878853

879854
const testDataTrack2 = trackGroupData.entriesByTrack['Track 2'].map(entry => {

front_end/models/trace/handlers/ExtensionTraceDataHandler.ts

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,27 +85,22 @@ export function extractConsoleAPIExtensionEntries(): void {
8585
if (!currentTimeStamp.args.data) {
8686
continue;
8787
}
88-
const timeStampName = String(currentTimeStamp.args.data.name ?? currentTimeStamp.args.data.message);
88+
const timeStampName = String(currentTimeStamp.args.data.name);
8989
timeStampByName.set(timeStampName, currentTimeStamp);
9090
const extensionData = extensionDataInConsoleTimeStamp(currentTimeStamp);
91-
const start = currentTimeStamp.args.data.start;
92-
const end = currentTimeStamp.args.data.end;
93-
if (!extensionData && !start && !end) {
91+
const startName = currentTimeStamp.args.data.start;
92+
const endName = currentTimeStamp.args.data.end;
93+
if (!extensionData && !startName && !endName) {
9494
continue;
9595
}
96-
// If the start or end is a number, it's assumed to be a timestamp
97-
// from the tracing clock, so we use that directly, otherwise we
98-
// assume it's the label of a previous console timestamp, in which
99-
// case we use its corresponding timestamp.
100-
const startTimeStamp =
101-
typeof start === 'number' ? Types.Timing.Micro(start) : timeStampByName.get(String(start))?.ts;
102-
const endTimeStamp = typeof end === 'number' ? Types.Timing.Micro(end) : timeStampByName.get(String(end))?.ts;
103-
if (endTimeStamp !== undefined && startTimeStamp === undefined) {
96+
const startTimeStamp = startName ? timeStampByName.get(String(startName)) : undefined;
97+
const endTimeStamp = endName ? timeStampByName.get(String(endName)) : undefined;
98+
if (endTimeStamp && !startTimeStamp) {
10499
// Invalid data
105100
continue;
106101
}
107-
const entryStartTime = startTimeStamp ?? currentTimeStamp.ts;
108-
const entryEndTime = endTimeStamp ?? currentTimeStamp.ts;
102+
const entryStartTime = startTimeStamp?.ts ?? currentTimeStamp.ts;
103+
const entryEndTime = endTimeStamp?.ts ?? currentTimeStamp.ts;
109104
if (extensionData) {
110105
const unregisteredExtensionEntry: Omit<Types.Extensions.SyntheticExtensionTrackEntry, '_tag'> = {
111106
...currentTimeStamp,
@@ -115,7 +110,6 @@ export function extractConsoleAPIExtensionEntries(): void {
115110
rawSourceEvent: currentTimeStamp,
116111
dur: Types.Timing.Micro(entryEndTime - entryStartTime),
117112
ts: entryStartTime,
118-
ph: Types.Events.Phase.COMPLETE,
119113
};
120114
const extensionEntry =
121115
Helpers.SyntheticEvents.SyntheticEventsManager.getActiveManager()

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ describeWithEnvironment('UserTimingsHandler', function() {
177177
describe('console.timestamp events parsing', function() {
178178
it('parses console.timestamp events correctly', async function() {
179179
assert.lengthOf(timingsData.timestampEvents, 3);
180-
assert.strictEqual(timingsData.timestampEvents[0].args.data?.message, 'a timestamp');
181-
assert.strictEqual(timingsData.timestampEvents[1].args.data?.message, 'another timestamp');
182-
assert.strictEqual(timingsData.timestampEvents[2].args.data?.message, 'yet another timestamp');
180+
assert.strictEqual(timingsData.timestampEvents[0].args.data?.name, 'a timestamp');
181+
assert.strictEqual(timingsData.timestampEvents[1].args.data?.name, 'another timestamp');
182+
assert.strictEqual(timingsData.timestampEvents[2].args.data?.name, 'yet another timestamp');
183183
});
184184
});
185185
});

front_end/models/trace/helpers/SamplesIntegrator.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import type * as Protocol from '../../../generated/protocol.js';
66
import * as CPUProfile from '../../../models/cpu_profile/cpu_profile.js';
77
import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
8-
import {makeCompleteEvent, makeInstantEvent} from '../../../testing/TraceHelpers.js';
8+
import {makeCompleteEvent} from '../../../testing/TraceHelpers.js';
99
import {TraceLoader} from '../../../testing/TraceLoader.js';
1010
import * as Trace from '../trace.js';
1111

@@ -272,8 +272,8 @@ describeWithEnvironment('SamplesIntegrator', function() {
272272
const evaluateScript = makeCompleteEvent(Trace.Types.Events.Name.EVALUATE_SCRIPT, 0, 500);
273273
const v8Run = makeCompleteEvent('v8.run', 10, 490);
274274
const consoleTimeStamp =
275-
makeInstantEvent(Trace.Types.Events.Name.TIME_STAMP, 350) as Trace.Types.Events.ConsoleTimeStamp;
276-
consoleTimeStamp.args = {data: {message: 'A timestamp', sampleTraceId: traceId}};
275+
makeCompleteEvent(Trace.Types.Events.Name.CONSOLE_TIME_STAMP, 350, 10) as Trace.Types.Events.ConsoleTimeStamp;
276+
consoleTimeStamp.args = {data: {name: 'A timestamp', sampleTraceId: traceId}};
277277
const traceEvents = [evaluateScript, v8Run, consoleTimeStamp];
278278
const constructedCalls = integrator.buildProfileCalls(traceEvents);
279279
assert.lengthOf(constructedCalls, 4);

front_end/models/trace/helpers/SamplesIntegrator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class SamplesIntegrator {
126126
// Because instant trace events have no duration, they don't provide
127127
// useful information for possible changes in the duration of calls
128128
// in the JS stack.
129-
if (event.ph === Types.Events.Phase.INSTANT && !extractSampleTraceId(event)) {
129+
if (event.ph === Types.Events.Phase.INSTANT) {
130130
continue;
131131
}
132132
if (stack.length === 0) {

front_end/models/trace/types/TraceEvents.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,15 +1424,14 @@ export interface ConsoleTimeEnd extends PairableAsyncEnd {
14241424
export type ConsoleTime = ConsoleTimeBegin|ConsoleTimeEnd;
14251425

14261426
export interface ConsoleTimeStamp extends Event {
1427-
cat: 'devtools.timeline';
1428-
name: Name.TIME_STAMP;
1429-
ph: Phase.INSTANT;
1427+
cat: 'disabled-by-default-v8.inspector';
1428+
name: Name.CONSOLE_TIME_STAMP;
1429+
ph: Phase.COMPLETE;
14301430
args: Args&{
14311431
data?: ArgsData & {
14321432
// The console.timeStamp allows to pass integers as values as well
14331433
// as strings
1434-
message: string,
1435-
name?: string|number,
1434+
name: string | number,
14361435
start?: string|number,
14371436
end?: string|number,
14381437
track?: string|number,
@@ -2324,7 +2323,7 @@ export function isConsoleTime(event: Event): event is ConsoleTime {
23242323
}
23252324

23262325
export function isConsoleTimeStamp(event: Event): event is ConsoleTimeStamp {
2327-
return event.ph === Phase.INSTANT && event.name === Name.TIME_STAMP;
2326+
return event.ph === Phase.COMPLETE && event.name === Name.CONSOLE_TIME_STAMP;
23282327
}
23292328

23302329
export function isUserTimingMeasure(event: Event): event is UserTimingMeasure {
@@ -2989,7 +2988,7 @@ export const enum Name {
29892988
CONSOLE_TIME = 'ConsoleTime',
29902989
USER_TIMING = 'UserTiming',
29912990
INTERACTIVE_TIME = 'InteractiveTime',
2992-
TIME_STAMP = 'TimeStamp',
2991+
CONSOLE_TIME_STAMP = 'V8Console::TimeStamp',
29932992

29942993
/* Frames */
29952994
BEGIN_FRAME = 'BeginFrame',

front_end/panels/timeline/CompatibilityTracksAppender.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ export function entryIsVisibleInTimeline(
7676
// events are hidden by default.
7777
const eventStyle = TimelineUtils.EntryStyles.getEventStyle(entry.name as Trace.Types.Events.Name);
7878
const eventIsTiming = Trace.Types.Events.isConsoleTime(entry) || Trace.Types.Events.isPerformanceMeasure(entry) ||
79-
Trace.Types.Events.isPerformanceMark(entry) || Trace.Types.Events.isConsoleTimeStamp(entry);
79+
Trace.Types.Events.isPerformanceMark(entry);
80+
8081
return (eventStyle && !eventStyle.hidden) || eventIsTiming;
8182
}
8283

front_end/panels/timeline/TimelineUIUtils.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ export class TimelineUIUtils {
661661
return title;
662662
}
663663
if (Trace.Types.Events.isConsoleTimeStamp(event) && event.args.data) {
664-
return i18nString(UIStrings.sS, {PH1: title, PH2: event.args.data.name ?? event.args.data.message});
664+
return i18nString(UIStrings.sS, {PH1: title, PH2: event.args.data.name});
665665
}
666666
if (Trace.Types.Events.isAnimation(event) && event.args.data.name) {
667667
return i18nString(UIStrings.sS, {PH1: title, PH2: event.args.data.name});
@@ -756,7 +756,7 @@ export class TimelineUIUtils {
756756
}
757757
break;
758758
}
759-
case Trace.Types.Events.Name.TIME_STAMP:
759+
case Trace.Types.Events.Name.CONSOLE_TIME_STAMP:
760760
detailsText = unsafeEventData['message'];
761761
break;
762762

@@ -2392,7 +2392,7 @@ export class TimelineUIUtils {
23922392
color = 'var(--sys-color-green)';
23932393
tall = true;
23942394
break;
2395-
case Trace.Types.Events.Name.TIME_STAMP:
2395+
case Trace.Types.Events.Name.CONSOLE_TIME_STAMP:
23962396
color = 'orange';
23972397
break;
23982398
}
@@ -2631,7 +2631,7 @@ export function timeStampForEventAdjustedForClosestNavigationIfPossible(
26312631
export function isMarkerEvent(parsedTrace: Trace.Handlers.Types.ParsedTrace, event: Trace.Types.Events.Event): boolean {
26322632
const {Name} = Trace.Types.Events;
26332633

2634-
if (event.name === Name.TIME_STAMP || event.name === Name.NAVIGATION_START) {
2634+
if (event.name === Name.CONSOLE_TIME_STAMP || event.name === Name.NAVIGATION_START) {
26352635
return true;
26362636
}
26372637

front_end/panels/timeline/TimingsTrackAppender.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class TimingsTrackAppender implements TrackAppender {
7777
m => !Trace.Handlers.ModelHandlers.ExtensionTraceData.extensionDataInPerformanceTiming(m));
7878
const timestampEvents = this.#parsedTrace.UserTimings.timestampEvents;
7979
const consoleTimings = this.#parsedTrace.UserTimings.consoleTimings;
80+
8081
if (extensionMarkersAreEmpty && performanceMarks.length === 0 && performanceMeasures.length === 0 &&
8182
timestampEvents.length === 0 && consoleTimings.length === 0) {
8283
return trackStartLevel;
@@ -243,7 +244,7 @@ export class TimingsTrackAppender implements TrackAppender {
243244
}
244245
}
245246
if (Trace.Types.Events.isConsoleTimeStamp(event)) {
246-
return `TimeStamp: ${event.args.data?.message ?? '(name unknown)'}`;
247+
return `TimeStamp: ${event.args.data?.name ?? '(name unknown)'}`;
247248
}
248249
if (Trace.Types.Events.isPerformanceMark(event)) {
249250
return `[mark]: ${event.name}`;
-1.1 KB
Binary file not shown.

0 commit comments

Comments
 (0)