Skip to content

Commit b88894b

Browse files
jackfranklinDevtools-frontend LUCI CQ
jackfranklin
authored and
Devtools-frontend LUCI CQ
committed
RPP: Reveal detail attached to extension events and hyperlink URLs
UI: - If any custom data is attached to a measure/mark/timestamp, we'll now render it. - non-string data is now allowed, we'll reuse `renderObjectJson` for rich object rendering - Same for detail.devtools.properties[], little DX upgrade. - Linkify any URLs found within attached data. - If we find an object with keys `url` and `description`, they get their own exclusive row in the summary UI - Bugfix: extension perf.mark was being rendered with tooltip as its name, rather than name. Refactor/naming: - The extension data (track, trackGroup, color) is kept together in a devtoolsObj, renamed to distinguish from a generic `args` - Any user-supplied data (via measure/mark `details` object, or 7th arg of console.timeStamp()) is extracted into a userDetail object. Bug: 436356278 Change-Id: Id75b92ac1b8c7801b08e2d154b18b40d723d0218 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6819859 Reviewed-by: Nikolay Vitkov <[email protected]> Commit-Queue: Nikolay Vitkov <[email protected]> Auto-Submit: Paul Irish <[email protected]> Reviewed-by: Finnur Thorarinsson <[email protected]> Reviewed-by: Mathias Bynens <[email protected]>
1 parent 9bd439c commit b88894b

File tree

15 files changed

+240
-132
lines changed

15 files changed

+240
-132
lines changed

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ describe('ExtensionTraceDataHandler', function() {
106106
{
107107
detail: {
108108
devtools: {
109-
dataType: 'invalid-type' as Trace.Types.Extensions.ExtensionDataPayload['dataType'],
109+
dataType: 'invalid-type' as Trace.Types.Extensions.DevToolsObj['dataType'],
110110
track: 'Another Extension Track',
111111
},
112112
},
@@ -141,15 +141,15 @@ describe('ExtensionTraceDataHandler', function() {
141141
// The first track is the one labeled 'An Extension Track'.
142142
{
143143
const {track, properties} =
144-
extensionHandlerOutput.extensionTrackData[0].entriesByTrack['An Extension Track'][0].args;
144+
extensionHandlerOutput.extensionTrackData[0].entriesByTrack['An Extension Track'][0].devtoolsObj;
145145
assert.strictEqual(JSON.stringify(properties), '[["Description","Something"]]');
146146
assert.strictEqual(track, 'An Extension Track');
147147
}
148148

149149
// Now look for 'Another Extension Track'.
150150
{
151151
const {tooltipText, track, properties} =
152-
extensionHandlerOutput.extensionTrackData[1].entriesByTrack['Another Extension Track'][0].args;
152+
extensionHandlerOutput.extensionTrackData[1].entriesByTrack['Another Extension Track'][0].devtoolsObj;
153153
assert.strictEqual(tooltipText, 'A hint if needed');
154154
assert.strictEqual(track, 'Another Extension Track');
155155
assert.strictEqual(
@@ -162,7 +162,7 @@ describe('ExtensionTraceDataHandler', function() {
162162
// Ensure it is discarded.
163163
const allTrackEntries =
164164
extensionHandlerOutput.extensionTrackData.map(track => Object.values(track.entriesByTrack)).flat(2);
165-
const validTrackEntries = allTrackEntries.filter(entry => entry.args.track);
165+
const validTrackEntries = allTrackEntries.filter(entry => entry.devtoolsObj.track);
166166
assert.lengthOf(validTrackEntries, allTrackEntries.length);
167167
});
168168

@@ -172,8 +172,8 @@ describe('ExtensionTraceDataHandler', function() {
172172
// Ensure it is discarded.
173173
const allTrackEntries =
174174
extensionHandlerOutput.extensionTrackData.map(track => Object.values(track.entriesByTrack)).flat(2);
175-
const validTrackEntries =
176-
allTrackEntries.filter(entry => entry.args.dataType === 'track-entry' || entry.args.dataType === undefined);
175+
const validTrackEntries = allTrackEntries.filter(
176+
entry => entry.devtoolsObj.dataType === 'track-entry' || entry.devtoolsObj.dataType === undefined);
177177
assert.lengthOf(validTrackEntries, allTrackEntries.length);
178178
});
179179
});
@@ -185,7 +185,7 @@ describe('ExtensionTraceDataHandler', function() {
185185
it('parses marker data correctly', async () => {
186186
assert.lengthOf(extensionHandlerOutput.extensionMarkers, 1);
187187
assert.strictEqual(extensionHandlerOutput.extensionMarkers[0].name, 'A custom mark');
188-
const {tooltipText, properties} = extensionHandlerOutput.extensionMarkers[0].args;
188+
const {tooltipText, properties} = extensionHandlerOutput.extensionMarkers[0].devtoolsObj;
189189
assert.strictEqual(tooltipText, 'A mark');
190190
assert.strictEqual(JSON.stringify(properties), '[["Description","This marks the start of a task"]]');
191191
});
@@ -206,7 +206,8 @@ describe('ExtensionTraceDataHandler', function() {
206206
};
207207

208208
assert.isNull(
209-
Trace.Handlers.ModelHandlers.ExtensionTraceData.extensionDataInPerformanceTiming(performanceMarkEvent),
209+
Trace.Handlers.ModelHandlers.ExtensionTraceData.extensionDataInPerformanceTiming(performanceMarkEvent)
210+
.devtoolsObj,
210211
);
211212
});
212213

@@ -215,7 +216,7 @@ describe('ExtensionTraceDataHandler', function() {
215216
// value.
216217
// Ensure it is discarded.
217218
const allMarkers = extensionHandlerOutput.extensionMarkers;
218-
const validTrackEntries = allMarkers.filter(entry => entry.args.dataType === 'marker');
219+
const validTrackEntries = allMarkers.filter(entry => entry.devtoolsObj.dataType === 'marker');
219220
assert.lengthOf(validTrackEntries, allMarkers.length);
220221
});
221222
});
@@ -257,7 +258,7 @@ describe('ExtensionTraceDataHandler', function() {
257258
detail: {
258259
devtools: {
259260
color: 'error',
260-
dataType: 'invalid' as Trace.Types.Extensions.ExtensionDataPayload['dataType'],
261+
dataType: 'invalid' as Trace.Types.Extensions.DevToolsObj['dataType'],
261262
},
262263
},
263264
name: 'A custom mark',
@@ -696,7 +697,7 @@ describe('ExtensionTraceDataHandler', function() {
696697
// Ensure it is discarded.
697698
const allTrackEntries =
698699
extensionHandlerOutput.extensionTrackData.map(track => Object.values(track.entriesByTrack)).flat(2);
699-
const validTrackEntries = allTrackEntries.filter(entry => entry.args.track);
700+
const validTrackEntries = allTrackEntries.filter(entry => entry.devtoolsObj.track);
700701
assert.lengthOf(validTrackEntries, allTrackEntries.length);
701702
});
702703
});

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

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export function extractConsoleAPIExtensionEntries(): void {
8787
}
8888
const timeStampName = String(currentTimeStamp.args.data.name ?? currentTimeStamp.args.data.message);
8989
timeStampByName.set(timeStampName, currentTimeStamp);
90-
const extensionData = extensionDataInConsoleTimeStamp(currentTimeStamp);
90+
const {devtoolsObj: extensionData, userDetail} = extensionDataInConsoleTimeStamp(currentTimeStamp);
9191
const start = currentTimeStamp.args.data.start;
9292
const end = currentTimeStamp.args.data.end;
9393
if (!extensionData && !start && !end) {
@@ -111,7 +111,8 @@ export function extractConsoleAPIExtensionEntries(): void {
111111
...currentTimeStamp,
112112
name: timeStampName,
113113
cat: 'devtools.extension',
114-
args: extensionData,
114+
devtoolsObj: extensionData,
115+
userDetail,
115116
rawSourceEvent: currentTimeStamp,
116117
dur: Types.Timing.Micro(entryEndTime - entryStartTime),
117118
ts: entryStartTime,
@@ -175,26 +176,27 @@ export function extractConsoleAPIExtensionEntries(): void {
175176
export function extractPerformanceAPIExtensionEntries(
176177
timings: Array<Types.Events.SyntheticUserTimingPair|Types.Events.PerformanceMark>): void {
177178
for (const timing of timings) {
178-
const extensionPayload = extensionDataInPerformanceTiming(timing);
179-
if (!extensionPayload) {
179+
const {devtoolsObj, userDetail} = extensionDataInPerformanceTiming(timing);
180+
if (!devtoolsObj) {
180181
// Not an extension user timing.
181182
continue;
182183
}
183184

184185
const extensionSyntheticEntry = {
185186
name: timing.name,
186-
ph: Types.Extensions.isExtensionPayloadMarker(extensionPayload) ? Types.Events.Phase.INSTANT :
187-
Types.Events.Phase.COMPLETE,
187+
ph: Types.Extensions.isExtensionPayloadMarker(devtoolsObj) ? Types.Events.Phase.INSTANT :
188+
Types.Events.Phase.COMPLETE,
188189
pid: timing.pid,
189190
tid: timing.tid,
190191
ts: timing.ts,
191192
dur: timing.dur as Types.Timing.Micro,
192193
cat: 'devtools.extension',
193-
args: extensionPayload,
194+
devtoolsObj,
195+
userDetail,
194196
rawSourceEvent: Types.Events.isSyntheticUserTiming(timing) ? timing.rawSourceEvent : timing,
195197
};
196198

197-
if (Types.Extensions.isExtensionPayloadMarker(extensionPayload)) {
199+
if (Types.Extensions.isExtensionPayloadMarker(devtoolsObj)) {
198200
const extensionMarker =
199201
Helpers.SyntheticEvents.SyntheticEventsManager
200202
.registerSyntheticEvent<Types.Extensions.SyntheticExtensionMarker>(
@@ -203,7 +205,7 @@ export function extractPerformanceAPIExtensionEntries(
203205
continue;
204206
}
205207

206-
if (Types.Extensions.isExtensionPayloadTrackEntry(extensionSyntheticEntry.args)) {
208+
if (Types.Extensions.isExtensionEntryObj(extensionSyntheticEntry.devtoolsObj)) {
207209
const extensionTrackEntry =
208210
Helpers.SyntheticEvents.SyntheticEventsManager
209211
.registerSyntheticEvent<Types.Extensions.SyntheticExtensionTrackEntry>(
@@ -214,26 +216,33 @@ export function extractPerformanceAPIExtensionEntries(
214216
}
215217
}
216218

217-
function extensionPayloadForConsoleApi(timing: Types.Events.ConsoleTimeStamp):
218-
Types.Extensions.ExtensionTrackEntryPayloadDeeplink|null {
219-
if (!timing.args.data || !('devtools' in timing.args.data)) {
220-
return null;
221-
}
222-
223-
return Helpers.Trace.parseDevtoolsDetails(
224-
`{"additionalContext": ${timing.args.data.devtools} }`, 'additionalContext') as
225-
Types.Extensions.ExtensionTrackEntryPayloadDeeplink;
226-
}
227-
219+
/**
220+
* Parses out the data in a performance.measure / mark call into two parts:
221+
* 1. devtoolsObj: this is the data required to be passed by the user for the
222+
* event to be used to create a custom track in the performance panel.
223+
* 2. userDetail: this is arbitrary data the user has attached to the event
224+
* that we show in the summary drawer.
225+
*/
228226
export function extensionDataInPerformanceTiming(
229-
timing: Types.Events.SyntheticUserTimingPair|Types.Events.PerformanceMark): Types.Extensions.ExtensionDataPayload|
230-
null {
227+
timing: Types.Events.SyntheticUserTimingPair|Types.Events.PerformanceMark):
228+
{devtoolsObj: Types.Extensions.DevToolsObj|null, userDetail: Types.Extensions.JsonValue|null} {
231229
const timingDetail =
232230
Types.Events.isPerformanceMark(timing) ? timing.args.data?.detail : timing.args.data.beginEvent.args.detail;
233231
if (!timingDetail) {
234-
return null;
232+
return {devtoolsObj: null, userDetail: null};
235233
}
236-
return Helpers.Trace.parseDevtoolsDetails(timingDetail, 'devtools') as Types.Extensions.ExtensionDataPayload;
234+
const devtoolsObj = Helpers.Trace.parseDevtoolsDetails(timingDetail, 'devtools') as Types.Extensions.DevToolsObj;
235+
236+
let userDetail = null;
237+
try {
238+
userDetail = JSON.parse(timingDetail);
239+
delete userDetail.devtools;
240+
} catch {
241+
// Nothing to do here, we still want to return the `devtools` part to make
242+
// this a custom event, even if the user detail failed to parse.
243+
}
244+
245+
return {devtoolsObj, userDetail};
237246
}
238247

239248
/**
@@ -257,31 +266,30 @@ export function extensionDataInPerformanceTiming(
257266
* valid extension data for a track entry, or `null` otherwise.
258267
*/
259268
export function extensionDataInConsoleTimeStamp(timeStamp: Types.Events.ConsoleTimeStamp):
260-
Types.Extensions.ExtensionTrackEntryPayload|null {
261-
if (!timeStamp.args.data) {
262-
return null;
263-
}
264-
const trackName = timeStamp.args.data.track;
265-
if (trackName === '' || trackName === undefined) {
266-
return null;
269+
{devtoolsObj: Types.Extensions.DevToolsObjEntry|null, userDetail: Types.Extensions.JsonValue|null} {
270+
if (!timeStamp.args.data || !timeStamp.args.data.track) {
271+
return {devtoolsObj: null, userDetail: null};
267272
}
268273

269-
let additionalContext: Types.Extensions.ExtensionTrackEntryPayloadDeeplink|undefined;
270-
const payload = extensionPayloadForConsoleApi(timeStamp);
271-
if (payload) {
272-
additionalContext = payload;
274+
let userDetail = null;
275+
try {
276+
// While it's in the trace as 'devtools', it's just the 7th argument to console.timeStamp(), stringified.
277+
// If no data, fall back to falsy empty string.
278+
userDetail = JSON.parse(timeStamp.args.data?.devtools || '""') as Types.Extensions.JsonValue;
279+
} catch {
273280
}
274281

275-
return {
282+
const devtoolsObj: Types.Extensions.DevToolsObjEntry = {
276283
// the color is defaulted to primary if it's value isn't one from
277284
// the defined palette (see ExtensionUI::extensionEntryColor) so
278285
// we don't need to check the value is valid here.
279-
color: String(timeStamp.args.data.color) as Types.Extensions.ExtensionTrackEntryPayload['color'],
280-
track: String(trackName),
286+
color: String(timeStamp.args.data.color) as Types.Extensions.DevToolsObjEntry['color'],
287+
track: String(timeStamp.args.data.track),
281288
dataType: 'track-entry',
282289
trackGroup: timeStamp.args.data.trackGroup !== undefined ? String(timeStamp.args.data.trackGroup) : undefined,
283-
additionalContext
284290
};
291+
292+
return {devtoolsObj, userDetail};
285293
}
286294

287295
export function data(): ExtensionTraceData {

‎front_end/models/trace/helpers/Extensions.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ export function buildTrackDataFromExtensionEntries(
2323
// we use the standalone track data, but use a fixed prefix in the
2424
// batch key to prevent collisions where a track group has the
2525
// same name as a standalone track.
26-
const key = entry.args.trackGroup || `track-name-${entry.args.track}`;
27-
const batchedData =
28-
Platform.MapUtilities.getWithDefault(dataByTrack, key, () => ({
29-
name: entry.args.trackGroup || entry.args.track,
30-
isTrackGroup: Boolean(entry.args.trackGroup),
31-
entriesByTrack: {[entry.args.track]: []},
32-
}));
26+
const key = entry.devtoolsObj.trackGroup || `track-name-${entry.devtoolsObj.track}`;
27+
const batchedData = Platform.MapUtilities.getWithDefault(
28+
dataByTrack, key, () => ({
29+
name: entry.devtoolsObj.trackGroup || entry.devtoolsObj.track,
30+
isTrackGroup: Boolean(entry.devtoolsObj.trackGroup),
31+
entriesByTrack: {[entry.devtoolsObj.track]: []},
32+
}));
3333

34-
if (!batchedData.entriesByTrack[entry.args.track]) {
35-
batchedData.entriesByTrack[entry.args.track] = [];
34+
if (!batchedData.entriesByTrack[entry.devtoolsObj.track]) {
35+
batchedData.entriesByTrack[entry.devtoolsObj.track] = [];
3636
}
37-
const entriesInTrack = batchedData.entriesByTrack[entry.args.track];
37+
const entriesInTrack = batchedData.entriesByTrack[entry.devtoolsObj.track];
3838
entriesInTrack.push(entry);
3939
}
4040
// Calculate self time if possible for track entries, on a track

‎front_end/models/trace/helpers/Trace.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ export function mergeEventsInOrder<T1 extends Types.Events.Event, T2 extends Typ
187187
return result;
188188
}
189189

190-
export function parseDevtoolsDetails(timingDetail: string, key: string): Types.Extensions.ExtensionDataPayload|
190+
export function parseDevtoolsDetails(timingDetail: string, key: string): Types.Extensions.DevToolsObj|
191191
Types.Extensions.ExtensionTrackEntryPayloadDeeplink|null {
192192
try {
193193
// Attempt to parse the detail as an object that might be coming from a
@@ -233,8 +233,8 @@ export function getNavigationForTraceEvent(
233233
return navigations[eventNavigationIndex];
234234
}
235235

236-
export function extractId(event: Types.Events.PairableAsync|
237-
Types.Events.SyntheticEventPair<Types.Events.PairableAsync>): string|undefined {
236+
export function extractId(
237+
event: Types.Events.PairableAsync|Types.Events.SyntheticEventPair<Types.Events.PairableAsync>): string|undefined {
238238
return event.id ?? event.id2?.global ?? event.id2?.local;
239239
}
240240

0 commit comments

Comments
 (0)