From 659cf723badf9ecf5d9913ee828deaad440e25b0 Mon Sep 17 00:00:00 2001 From: Steve Golton Date: Sat, 28 Sep 2024 19:23:29 +0100 Subject: [PATCH 1/2] ui: Add selection resolver API If we are going to make the selection objects 'fat' i.e. contain various pieces of information related to the entity that has been selected, it's important to reduce the number of places where this selection object is directly set, to reduce duplication and coupling. This CL introduces a new selection resolver system where plugins can register their ability to resolve selection objects on behalf of of other parts of the UI. This is currently limited to SQL table names and ids like so: ``` registerResolver('slice', (id) => {...selection object....}); // ~~~ snip ~~~ resolveSelection('slice', 123); ``` This could be made more generic way in the future, possibly using URIs, but for now, sql table and ID are the only way folks look up selections on other plugins, so this will do for now. Change-Id: I297247629a7a504206b28ccb0a8ecdc72c7cff0c --- ui/src/common/state_serialization.ts | 10 +- ui/src/controller/trace_controller.ts | 20 +-- ui/src/core/selection_manager.ts | 115 +++++++++++------- .../critical_user_interaction_track.ts | 2 +- .../scroll_jank_cause_link_utils.ts | 2 +- .../chrome_scroll_jank/scroll_jank_slice.ts | 2 +- .../counter/trace_processor_counter_track.ts | 2 +- .../cpu_profile/cpu_profile_track.ts | 2 +- .../cpu_slices/cpu_slice_track.ts | 2 +- ui/src/core_plugins/cpu_slices/index.ts | 31 ++++- .../cpu_slices/sched_details_tab.ts | 10 +- .../heap_profile/heap_profile_track.ts | 2 +- .../perf_samples_profile_track.ts | 4 +- ui/src/core_plugins/thread_slice/index.ts | 43 ++++++- ui/src/core_plugins/thread_state/index.ts | 36 +++++- .../thread_state/thread_state_track.ts | 2 +- ui/src/core_plugins/track_utils/index.ts | 2 +- ui/src/frontend/flow_events_panel.ts | 14 +-- ui/src/frontend/keyboard_event_handler.ts | 14 +-- ui/src/frontend/named_slice_track.ts | 2 +- ui/src/frontend/notes_panel.ts | 4 +- ui/src/frontend/thread_slice_track.ts | 2 +- .../tracks/custom_sql_table_slice_track.ts | 2 +- ui/src/frontend/ui_main.ts | 2 +- ui/src/frontend/viewer_page.ts | 4 +- ui/src/frontend/widgets/sched.ts | 15 +-- ui/src/frontend/widgets/slice.ts | 16 +-- ui/src/frontend/widgets/thread_state.ts | 9 +- .../dev.perfetto.AndroidCujs/trackUtils.ts | 25 +--- .../handlers/pinCujScoped.ts | 2 +- ui/src/public/lib/query_table/query_table.ts | 14 +-- ui/src/public/selection.ts | 73 ++++++++++- 32 files changed, 306 insertions(+), 179 deletions(-) diff --git a/ui/src/common/state_serialization.ts b/ui/src/common/state_serialization.ts index 3f8442f8da..5b3527109c 100644 --- a/ui/src/common/state_serialization.ts +++ b/ui/src/common/state_serialization.ts @@ -229,19 +229,19 @@ export function deserializeAppStatePhase2(appState: SerializedAppState): void { const selMgr = globals.selectionManager; switch (sel.kind) { case 'TRACK_EVENT': - selMgr.setEvent(sel.trackKey, parseInt(sel.eventId)); + selMgr.selectTrackEvent(sel.trackKey, parseInt(sel.eventId)); break; case 'LEGACY_SCHED_SLICE': - selMgr.setLegacy({kind: 'SCHED_SLICE', id: sel.id}); + selMgr.selectSqlEvent('sched_slice', sel.id); break; case 'LEGACY_SLICE': - selMgr.setLegacy({kind: 'SLICE', id: sel.id}); + selMgr.selectSqlEvent('slice', sel.id); break; case 'LEGACY_THREAD_STATE': - selMgr.setLegacy({kind: 'THREAD_STATE', id: sel.id}); + selMgr.selectSqlEvent('thread_slice', sel.id); break; case 'LEGACY_HEAP_PROFILE': - selMgr.setLegacy({ + selMgr.selectLegacy({ kind: 'HEAP_PROFILE', id: sel.id, upid: sel.upid, diff --git a/ui/src/controller/trace_controller.ts b/ui/src/controller/trace_controller.ts index a0b793cce8..749e3f40ad 100644 --- a/ui/src/controller/trace_controller.ts +++ b/ui/src/controller/trace_controller.ts @@ -500,7 +500,7 @@ export class TraceController extends Controller { const upid = row.upid; const leftTs = traceTime.start; const rightTs = traceTime.end; - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'PERF_SAMPLES', id: 0, upid, @@ -531,7 +531,7 @@ export class TraceController extends Controller { const row = profile.firstRow({ts: LONG, type: STR, upid: NUM}); const ts = Time.fromRaw(row.ts); const upid = row.upid; - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'HEAP_PROFILE', id: 0, upid, @@ -581,18 +581,10 @@ export class TraceController extends Controller { if (track === undefined) { return; } - globals.selectionManager.setLegacy( - { - kind: 'SLICE', - id: row.id, - trackUri: track.uri, - table: 'slice', - }, - { - pendingScrollId: row.id, - switchToCurrentSelectionTab: false, - }, - ); + globals.selectionManager.selectSqlEvent('slice', row.id, { + pendingScrollId: row.id, + switchToCurrentSelectionTab: false, + }); } } diff --git a/ui/src/core/selection_manager.ts b/ui/src/core/selection_manager.ts index e826a7629a..6b0651ebf5 100644 --- a/ui/src/core/selection_manager.ts +++ b/ui/src/core/selection_manager.ts @@ -20,6 +20,7 @@ import { SelectionOpts, SelectionManager, AreaSelectionAggregator, + SqlSelectionResolver, } from '../public/selection'; import {duration, Time, time, TimeSpan} from '../base/time'; import { @@ -57,6 +58,7 @@ export class SelectionManagerImpl implements SelectionManager { // Incremented every time _selection changes. private _selectionGeneration = 0; private _limiter = new AsyncLimiter(); + private readonly selectionResolvers = new Array(); constructor( engine: Engine, @@ -79,29 +81,38 @@ export class SelectionManagerImpl implements SelectionManager { this.setSelection({kind: 'empty'}); } - setEvent(trackUri: string, eventId: number) { - this.setSelection({ - kind: 'single', - trackUri, - eventId, - }); + selectTrackEvent(trackUri: string, eventId: number, opts?: SelectionOpts) { + this.setSelection( + { + kind: 'single', + trackUri, + eventId, + }, + opts, + ); } - setNote(args: {id: string}) { - this.setSelection({ - kind: 'note', - id: args.id, - }); + selectNote(args: {id: string}, opts?: SelectionOpts) { + this.setSelection( + { + kind: 'note', + id: args.id, + }, + opts, + ); } - setArea(args: Area): void { + selectArea(args: Area, opts?: SelectionOpts): void { const {start, end} = args; assertTrue(start <= end); - this.setSelection({ - kind: 'area', - tracks: [], - ...args, - }); + this.setSelection( + { + kind: 'area', + tracks: [], + ...args, + }, + opts, + ); } toggleTrackAreaSelection(trackUri: string) { @@ -150,7 +161,7 @@ export class SelectionManagerImpl implements SelectionManager { // There is no matching addLegacy as we did not support multi-single // selection with the legacy selection system. - setLegacy(legacySelection: LegacySelection, opts?: SelectionOpts): void { + selectLegacy(legacySelection: LegacySelection, opts?: SelectionOpts): void { this.setSelection( { kind: 'legacy', @@ -160,7 +171,7 @@ export class SelectionManagerImpl implements SelectionManager { ); } - setGenericSlice(args: { + selectGenericSlice(args: { id: number; sqlTableName: string; start: time; @@ -204,6 +215,35 @@ export class SelectionManagerImpl implements SelectionManager { return this._selectedDetails; } + registerSqlSelectionResolver(resolver: SqlSelectionResolver): void { + this.selectionResolvers.push(resolver); + } + + async resolveSqlEvent( + sqlTableName: string, + id: number, + ): Promise { + const matchingResolvers = this.selectionResolvers.filter( + (r) => r.sqlTableName === sqlTableName, + ); + + for (const resolver of matchingResolvers) { + const result = await resolver.callback(id, sqlTableName); + if (result) { + // If we have multiple resolvers for the same table, just return the first one. + return result; + } + } + + return undefined; + } + + selectSqlEvent(sqlTableName: string, id: number, opts?: SelectionOpts): void { + this.resolveSqlEvent(sqlTableName, id).then((selection) => { + selection && this.setSelection(selection, opts); + }); + } + private setSelection(selection: Selection, opts?: SelectionOpts) { if (selection.kind === 'area') { // In the case of area selection, the caller provides a list of trackUris. @@ -278,21 +318,14 @@ export class SelectionManagerImpl implements SelectionManager { }); break; case 'cpu': - this.setLegacy( - { - kind: 'SCHED_SLICE', - id: eventId, - trackUri, - }, - { - clearSearch: false, - pendingScrollId: eventId, - switchToCurrentSelectionTab: true, - }, - ); + this.selectSqlEvent('sched_slice', eventId, { + clearSearch: false, + pendingScrollId: eventId, + switchToCurrentSelectionTab: true, + }); break; case 'log': - this.setLegacy( + this.selectLegacy( { kind: 'LOG', id: eventId, @@ -307,19 +340,11 @@ export class SelectionManagerImpl implements SelectionManager { case 'slice': // Search results only include slices from the slice table for now. // When we include annotations we need to pass the correct table. - this.setLegacy( - { - kind: 'SLICE', - id: eventId, - trackUri, - table: 'slice', - }, - { - clearSearch: false, - pendingScrollId: eventId, - switchToCurrentSelectionTab: true, - }, - ); + this.selectSqlEvent('slice', eventId, { + clearSearch: false, + pendingScrollId: eventId, + switchToCurrentSelectionTab: true, + }); break; default: assertUnreachable(source); diff --git a/ui/src/core_plugins/chrome_critical_user_interactions/critical_user_interaction_track.ts b/ui/src/core_plugins/chrome_critical_user_interactions/critical_user_interaction_track.ts index 1760b22f7b..3f5888a7ec 100644 --- a/ui/src/core_plugins/chrome_critical_user_interactions/critical_user_interaction_track.ts +++ b/ui/src/core_plugins/chrome_critical_user_interactions/critical_user_interaction_track.ts @@ -131,7 +131,7 @@ export class CriticalUserInteractionTrack extends CustomSqlTableSliceTrack { onSliceClick(args: OnSliceClickArgs) { const detailsPanelConfig = this.getDetailsPanel(args); - this.trace.selection.setGenericSlice({ + this.trace.selection.selectGenericSlice({ id: args.slice.scopedId, sqlTableName: this.tableName, start: args.slice.ts, diff --git a/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_cause_link_utils.ts b/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_cause_link_utils.ts index bbd4fa1429..a804ca9808 100644 --- a/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_cause_link_utils.ts +++ b/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_cause_link_utils.ts @@ -211,7 +211,7 @@ export function getCauseLink( viewPercentage: 0.3, }, }); - trace.selection.setArea({ + trace.selection.selectArea({ start: ts, end: Time.fromRaw(ts + dur), trackUris, diff --git a/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_slice.ts b/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_slice.ts index 8d231a7954..2b3e33c36a 100644 --- a/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_slice.ts +++ b/ui/src/core_plugins/chrome_scroll_jank/scroll_jank_slice.ts @@ -179,7 +179,7 @@ export class ScrollJankSliceRef } const trackUri = track.key; - globals.selectionManager.setGenericSlice({ + globals.selectionManager.selectGenericSlice({ id: vnode.attrs.id, sqlTableName: track.sqlTableName, start: vnode.attrs.ts, diff --git a/ui/src/core_plugins/counter/trace_processor_counter_track.ts b/ui/src/core_plugins/counter/trace_processor_counter_track.ts index 08b682defa..2566470565 100644 --- a/ui/src/core_plugins/counter/trace_processor_counter_track.ts +++ b/ui/src/core_plugins/counter/trace_processor_counter_track.ts @@ -78,7 +78,7 @@ export class TraceProcessorCounterTrack extends BaseCounterTrack { return; } const id = it.id; - this.trace.selection.setEvent(this.uri, id); + this.trace.selection.selectTrackEvent(this.uri, id); }); return true; diff --git a/ui/src/core_plugins/cpu_profile/cpu_profile_track.ts b/ui/src/core_plugins/cpu_profile/cpu_profile_track.ts index ec30c8ba79..5f23f7a7ee 100644 --- a/ui/src/core_plugins/cpu_profile/cpu_profile_track.ts +++ b/ui/src/core_plugins/cpu_profile/cpu_profile_track.ts @@ -75,7 +75,7 @@ export class CpuProfileTrack extends BaseSliceTrack { } onSliceClick({slice}: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'CPU_PROFILE_SAMPLE', id: slice.id, utid: this.utid, diff --git a/ui/src/core_plugins/cpu_slices/cpu_slice_track.ts b/ui/src/core_plugins/cpu_slices/cpu_slice_track.ts index 99c98f0f85..d6d30e3762 100644 --- a/ui/src/core_plugins/cpu_slices/cpu_slice_track.ts +++ b/ui/src/core_plugins/cpu_slices/cpu_slice_track.ts @@ -434,7 +434,7 @@ export class CpuSliceTrack implements Track { // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions if (!id || this.utidHoveredInThisTrack === -1) return false; - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'SCHED_SLICE', id, trackUri: this.uri, diff --git a/ui/src/core_plugins/cpu_slices/index.ts b/ui/src/core_plugins/cpu_slices/index.ts index d16ae2f7be..278a29e8fb 100644 --- a/ui/src/core_plugins/cpu_slices/index.ts +++ b/ui/src/core_plugins/cpu_slices/index.ts @@ -26,6 +26,10 @@ import {asSchedSqlId} from '../../trace_processor/sql_utils/core_types'; import {CpuSliceSelectionAggregator} from './cpu_slice_selection_aggregator'; import {CpuSliceByProcessSelectionAggregator} from './cpu_slice_by_process_selection_aggregator'; +function uriForSchedTrack(cpu: number): string { + return `/sched_cpu${cpu}`; +} + class CpuSlices implements PerfettoPlugin { async onTraceLoad(ctx: Trace): Promise { ctx.selection.registerAreaSelectionAggreagtor( @@ -40,7 +44,7 @@ class CpuSlices implements PerfettoPlugin { for (const cpu of cpus) { const size = cpuToClusterType.get(cpu); - const uri = `/sched_cpu${cpu}`; + const uri = uriForSchedTrack(cpu); const name = size === undefined ? `Cpu ${cpu}` : `Cpu ${cpu} (${size})`; ctx.tracks.registerTrack({ @@ -72,6 +76,31 @@ class CpuSlices implements PerfettoPlugin { }, }), ); + + ctx.selection.registerSqlSelectionResolver({ + sqlTableName: 'sched_slice', + callback: async (id: number) => { + const result = await ctx.engine.query(` + select + cpu + from sched_slice + where id = ${id} + `); + + const cpu = result.firstRow({ + cpu: NUM, + }).cpu; + + return { + kind: 'legacy', + legacySelection: { + kind: 'SCHED_SLICE', + id, + trackUri: uriForSchedTrack(cpu), + }, + }; + }, + }); } async getAndroidCpuClusterTypes( diff --git a/ui/src/core_plugins/cpu_slices/sched_details_tab.ts b/ui/src/core_plugins/cpu_slices/sched_details_tab.ts index a734f349d1..90704512e9 100644 --- a/ui/src/core_plugins/cpu_slices/sched_details_tab.ts +++ b/ui/src/core_plugins/cpu_slices/sched_details_tab.ts @@ -282,12 +282,10 @@ export class SchedDetailsTab extends BottomTab { ); if (trackDescriptor && data.sched.threadStateId) { - globals.selectionManager.setLegacy({ - kind: 'THREAD_STATE', - id: data.sched.threadStateId, - trackUri: trackDescriptor.uri, - }); - + globals.selectionManager.selectSqlEvent( + 'thread_state', + data.sched.threadStateId, + ); scrollTo({ track: {uri: trackDescriptor.uri, expandGroup: true}, time: {start: data.sched.ts}, diff --git a/ui/src/core_plugins/heap_profile/heap_profile_track.ts b/ui/src/core_plugins/heap_profile/heap_profile_track.ts index a2dbe3dd40..c2d83a3b5d 100644 --- a/ui/src/core_plugins/heap_profile/heap_profile_track.ts +++ b/ui/src/core_plugins/heap_profile/heap_profile_track.ts @@ -112,7 +112,7 @@ export class HeapProfileTrack extends BaseSliceTrack< } onSliceClick(args: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'HEAP_PROFILE', id: args.slice.id, upid: this.upid, diff --git a/ui/src/core_plugins/perf_samples_profile/perf_samples_profile_track.ts b/ui/src/core_plugins/perf_samples_profile/perf_samples_profile_track.ts index 9afd2747de..d06e77607f 100644 --- a/ui/src/core_plugins/perf_samples_profile/perf_samples_profile_track.ts +++ b/ui/src/core_plugins/perf_samples_profile/perf_samples_profile_track.ts @@ -86,7 +86,7 @@ export class ProcessPerfSamplesProfileTrack extends BasePerfSamplesProfileTrack } onSliceClick({slice}: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'PERF_SAMPLES', id: slice.id, upid: this.upid, @@ -121,7 +121,7 @@ export class ThreadPerfSamplesProfileTrack extends BasePerfSamplesProfileTrack { } onSliceClick({slice}: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'PERF_SAMPLES', id: slice.id, utid: this.utid, diff --git a/ui/src/core_plugins/thread_slice/index.ts b/ui/src/core_plugins/thread_slice/index.ts index c731a8c80b..e624f2a208 100644 --- a/ui/src/core_plugins/thread_slice/index.ts +++ b/ui/src/core_plugins/thread_slice/index.ts @@ -25,6 +25,14 @@ import {removeFalsyValues} from '../../base/array_utils'; import {getOrCreateGroupForThread} from '../../public/standard_groups'; import {TrackNode} from '../../public/workspace'; +function uriForSliceTrack( + upid: number | null, + utid: number, + trackId: number, +): string { + return `${getThreadUriPrefix(upid, utid)}_slice_${trackId}`; +} + class ThreadSlicesPlugin implements PerfettoPlugin { async onTraceLoad(ctx: Trace): Promise { const {engine} = ctx; @@ -86,7 +94,7 @@ class ThreadSlicesPlugin implements PerfettoPlugin { kind: 'Slices', }); - const uri = `${getThreadUriPrefix(upid, utid)}_slice_${trackId}`; + const uri = uriForSliceTrack(upid, utid, trackId); ctx.tracks.registerTrack({ uri, title, @@ -132,6 +140,39 @@ class ThreadSlicesPlugin implements PerfettoPlugin { }, }), ); + + ctx.selection.registerSqlSelectionResolver({ + sqlTableName: 'slice', + callback: async (id: number) => { + const result = await ctx.engine.query(` + select + tt.utid as utid, + t.upid as upid, + track_id as trackId + from + slice + join thread_track tt on slice.track_id = tt.id + join _threads_with_kernel_flag t using(utid) + where slice.id = ${id} + `); + + const {upid, utid, trackId} = result.firstRow({ + upid: NUM_NULL, + utid: NUM, + trackId: NUM, + }); + + return { + kind: 'legacy', + legacySelection: { + kind: 'SLICE', + id, + trackUri: uriForSliceTrack(upid, utid, trackId), + table: 'slice', + }, + }; + }, + }); } } diff --git a/ui/src/core_plugins/thread_state/index.ts b/ui/src/core_plugins/thread_state/index.ts index 49b6622ecc..1e67975c42 100644 --- a/ui/src/core_plugins/thread_state/index.ts +++ b/ui/src/core_plugins/thread_state/index.ts @@ -30,6 +30,10 @@ import {TrackNode} from '../../public/workspace'; import {getOrCreateGroupForThread} from '../../public/standard_groups'; import {ThreadStateSelectionAggregator} from './thread_state_selection_aggregator'; +function uriForThreadStateTrack(upid: number | null, utid: number): string { + return `${getThreadUriPrefix(upid, utid)}_state`; +} + class ThreadState implements PerfettoPlugin { async onTraceLoad(ctx: Trace): Promise { const {engine} = ctx; @@ -70,7 +74,7 @@ class ThreadState implements PerfettoPlugin { kind: THREAD_STATE_TRACK_KIND, }); - const uri = `${getThreadUriPrefix(upid, utid)}_state`; + const uri = uriForThreadStateTrack(upid, utid); ctx.tracks.registerTrack({ uri, title, @@ -124,6 +128,36 @@ class ThreadState implements PerfettoPlugin { }); }, }); + + ctx.selection.registerSqlSelectionResolver({ + sqlTableName: 'thread_state', + callback: async (id: number) => { + const result = await ctx.engine.query(` + select + thread_state.utid, + t.upid + from + thread_state + join _threads_with_kernel_flag t + where thread_state.id = ${id} + `); + + const {upid, utid} = result.firstRow({ + upid: NUM_NULL, + utid: NUM, + }); + + return { + kind: 'legacy', + legacySelection: { + kind: 'THREAD_STATE', + id, + trackUri: uriForThreadStateTrack(upid, utid), + table: 'slice', + }, + }; + }, + }); } } diff --git a/ui/src/core_plugins/thread_state/thread_state_track.ts b/ui/src/core_plugins/thread_state/thread_state_track.ts index 9184af5c06..0a61919257 100644 --- a/ui/src/core_plugins/thread_state/thread_state_track.ts +++ b/ui/src/core_plugins/thread_state/thread_state_track.ts @@ -85,7 +85,7 @@ export class ThreadStateTrack extends BaseSliceTrack { } onSliceClick(args: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'THREAD_STATE', id: args.slice.id, trackUri: this.uri, diff --git a/ui/src/core_plugins/track_utils/index.ts b/ui/src/core_plugins/track_utils/index.ts index a8463bb13d..50d97a5aec 100644 --- a/ui/src/core_plugins/track_utils/index.ts +++ b/ui/src/core_plugins/track_utils/index.ts @@ -60,7 +60,7 @@ class TrackUtilsPlugin implements PerfettoPlugin { ); if (selectedUri === undefined) return; // Prompt cancelled. ctx.scrollTo({track: {uri: selectedUri, expandGroup: true}}); - ctx.selection.setArea({ + ctx.selection.selectArea({ start: ctx.traceInfo.start, end: ctx.traceInfo.end, trackUris: [selectedUri], diff --git a/ui/src/frontend/flow_events_panel.ts b/ui/src/frontend/flow_events_panel.ts index 3fca7b142f..7c86c25f63 100644 --- a/ui/src/frontend/flow_events_panel.ts +++ b/ui/src/frontend/flow_events_panel.ts @@ -66,17 +66,9 @@ export class FlowEventsPanel implements m.ClassComponent { td.tags?.trackIds?.includes(trackId), ); if (track) { - globals.selectionManager.setLegacy( - { - kind: 'SLICE', - id: sliceId, - trackUri: track.uri, - table: 'slice', - }, - { - switchToCurrentSelectionTab: false, - }, - ); + globals.selectionManager.selectSqlEvent('slice', sliceId, { + switchToCurrentSelectionTab: false, + }); } }; diff --git a/ui/src/frontend/keyboard_event_handler.ts b/ui/src/frontend/keyboard_event_handler.ts index 7bbad4c88f..72184a5555 100644 --- a/ui/src/frontend/keyboard_event_handler.ts +++ b/ui/src/frontend/keyboard_event_handler.ts @@ -99,17 +99,9 @@ export function moveByFocusedFlow(direction: Direction): void { ?.tags?.trackIds?.includes(flowPoint.trackId); }); if (track) { - globals.selectionManager.setLegacy( - { - kind: 'SLICE', - id: flowPoint.sliceId, - trackUri: track.uri, - table: 'slice', - }, - { - pendingScrollId: flowPoint.sliceId, - }, - ); + globals.selectionManager.selectSqlEvent('slice', flowPoint.sliceId, { + pendingScrollId: flowPoint.sliceId, + }); } } } diff --git a/ui/src/frontend/named_slice_track.ts b/ui/src/frontend/named_slice_track.ts index a21d49def7..bee03e621c 100644 --- a/ui/src/frontend/named_slice_track.ts +++ b/ui/src/frontend/named_slice_track.ts @@ -67,7 +67,7 @@ export abstract class NamedSliceTrack< } onSliceClick(args: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'SLICE', id: args.slice.id, trackUri: this.uri, diff --git a/ui/src/frontend/notes_panel.ts b/ui/src/frontend/notes_panel.ts index ef4e4db389..1752baea7f 100644 --- a/ui/src/frontend/notes_panel.ts +++ b/ui/src/frontend/notes_panel.ts @@ -338,14 +338,14 @@ export class NotesPanel implements Panel { if (x < 0) return; for (const note of globals.noteManager.notes.values()) { if (this.hoveredX !== null && this.hitTestNote(this.hoveredX, note)) { - globals.selectionManager.setNote({id: note.id}); + globals.selectionManager.selectNote({id: note.id}); return; } } const timestamp = this.timescale.pxToHpTime(x).toTime(); const color = randomColor(); const noteId = globals.noteManager.addNote({timestamp, color}); - globals.selectionManager.setNote({id: noteId}); + globals.selectionManager.selectNote({id: noteId}); } private hitTestNote(x: number, note: SpanNote | Note): boolean { diff --git a/ui/src/frontend/thread_slice_track.ts b/ui/src/frontend/thread_slice_track.ts index 8f7a2586e8..b0fa1b7750 100644 --- a/ui/src/frontend/thread_slice_track.ts +++ b/ui/src/frontend/thread_slice_track.ts @@ -83,7 +83,7 @@ export class ThreadSliceTrack extends NamedSliceTrack { } onSliceClick(args: OnSliceClickArgs) { - globals.selectionManager.setLegacy({ + globals.selectionManager.selectLegacy({ kind: 'SLICE', id: args.slice.id, trackUri: this.uri, diff --git a/ui/src/frontend/tracks/custom_sql_table_slice_track.ts b/ui/src/frontend/tracks/custom_sql_table_slice_track.ts index 757cb5a667..f97142b96f 100644 --- a/ui/src/frontend/tracks/custom_sql_table_slice_track.ts +++ b/ui/src/frontend/tracks/custom_sql_table_slice_track.ts @@ -122,7 +122,7 @@ export abstract class CustomSqlTableSliceTrack extends NamedSliceTrack< } const detailsPanelConfig = this.getDetailsPanel(args); - globals.selectionManager.setGenericSlice({ + globals.selectionManager.selectGenericSlice({ id: args.slice.id, sqlTableName: this.tableName, start: args.slice.ts, diff --git a/ui/src/frontend/ui_main.ts b/ui/src/frontend/ui_main.ts index 263c64d6d2..ece7c236fd 100644 --- a/ui/src/frontend/ui_main.ts +++ b/ui/src/frontend/ui_main.ts @@ -366,7 +366,7 @@ export class UiMainPerTrace implements m.ClassComponent { .filter((uri) => uri !== undefined); } const {start, end} = trace.traceInfo; - trace.selection.setArea({ + trace.selection.selectArea({ start, end, trackUris: tracksToSelect, diff --git a/ui/src/frontend/viewer_page.ts b/ui/src/frontend/viewer_page.ts index aa030cf984..f7e273f8e0 100644 --- a/ui/src/frontend/viewer_page.ts +++ b/ui/src/frontend/viewer_page.ts @@ -222,10 +222,10 @@ export class ViewerPage implements m.ClassComponent { if (edit) { const selection = globals.selectionManager.selection; if (selection.kind === 'area' && area) { - globals.selectionManager.setArea({...area}); + globals.selectionManager.selectArea({...area}); } } else if (area) { - globals.selectionManager.setArea({...area}); + globals.selectionManager.selectArea({...area}); } // Now the selection has ended we stored the final selected area in the // global state and can remove the in progress selection from the diff --git a/ui/src/frontend/widgets/sched.ts b/ui/src/frontend/widgets/sched.ts index 8eafe4f773..91ce75d4c6 100644 --- a/ui/src/frontend/widgets/sched.ts +++ b/ui/src/frontend/widgets/sched.ts @@ -46,11 +46,7 @@ export function goToSchedSlice(cpu: number, id: SchedSqlId, ts: time) { if (trackUri === undefined) { return; } - globals.selectionManager.setLegacy({ - kind: 'SCHED_SLICE', - id, - trackUri, - }); + globals.selectionManager.selectSqlEvent('sched_slice', id); scrollTo({ track: {uri: trackUri, expandGroup: true}, time: {start: ts}, @@ -67,12 +63,9 @@ export class SchedRef implements m.ClassComponent { const trackUri = findSchedTrack(vnode.attrs.cpu); if (trackUri === undefined) return; - globals.selectionManager.setLegacy( - { - kind: 'SCHED_SLICE', - id: vnode.attrs.id, - trackUri, - }, + globals.selectionManager.selectSqlEvent( + 'sched_slice', + vnode.attrs.id, { switchToCurrentSelectionTab: vnode.attrs.switchToCurrentSelectionTab ?? true, diff --git a/ui/src/frontend/widgets/slice.ts b/ui/src/frontend/widgets/slice.ts index 4aa44fede6..3dc98e3ad0 100644 --- a/ui/src/frontend/widgets/slice.ts +++ b/ui/src/frontend/widgets/slice.ts @@ -67,18 +67,10 @@ export class SliceRef implements m.ClassComponent { end: Time.fromRaw(vnode.attrs.ts + dur), }, }); - globals.selectionManager.setLegacy( - { - kind: 'SLICE', - id: vnode.attrs.id, - trackUri: track.uri, - table: 'slice', - }, - { - switchToCurrentSelectionTab: - vnode.attrs.switchToCurrentSelectionTab, - }, - ); + globals.selectionManager.selectSqlEvent('slice', vnode.attrs.id, { + switchToCurrentSelectionTab: + vnode.attrs.switchToCurrentSelectionTab, + }); }, }, vnode.attrs.name, diff --git a/ui/src/frontend/widgets/thread_state.ts b/ui/src/frontend/widgets/thread_state.ts index 04b94636b4..284369c824 100644 --- a/ui/src/frontend/widgets/thread_state.ts +++ b/ui/src/frontend/widgets/thread_state.ts @@ -56,12 +56,9 @@ export class ThreadStateRef implements m.ClassComponent { if (trackDescriptor === undefined) return; - globals.selectionManager.setLegacy( - { - kind: 'THREAD_STATE', - id: vnode.attrs.id, - trackUri: trackDescriptor.uri, - }, + globals.selectionManager.selectSqlEvent( + 'thread_state', + vnode.attrs.id, { switchToCurrentSelectionTab: vnode.attrs.switchToCurrentSelectionTab, diff --git a/ui/src/plugins/dev.perfetto.AndroidCujs/trackUtils.ts b/ui/src/plugins/dev.perfetto.AndroidCujs/trackUtils.ts index f3dde8a11a..28f9d7a775 100644 --- a/ui/src/plugins/dev.perfetto.AndroidCujs/trackUtils.ts +++ b/ui/src/plugins/dev.perfetto.AndroidCujs/trackUtils.ts @@ -61,25 +61,8 @@ export function addAndPinSliceTrack( * Takes and adds desired slice to current selection * Retrieves the track key and scrolls to the desired slice */ -export function focusOnSlice( - ctx: Trace, - sqlSliceId: number, - sqlTrackId: number, -) { - // TODO(primiano): there should be probably a public API to select slices - // given their SQL details. We need to rationalize selection first. - const trackUri = ctx.tracks.findTrack((trackDescriptor) => { - return trackDescriptor?.tags?.trackIds?.includes(sqlTrackId); - })?.uri; - ctx.selection.setLegacy( - { - kind: 'SLICE', - id: sqlSliceId, - trackUri, - table: 'slice', - }, - { - pendingScrollId: sqlSliceId, - }, - ); +export function focusOnSlice(ctx: Trace, sqlSliceId: number) { + ctx.selection.selectSqlEvent('slice', sqlSliceId, { + pendingScrollId: sqlSliceId, + }); } diff --git a/ui/src/plugins/dev.perfetto.PinAndroidPerfMetrics/handlers/pinCujScoped.ts b/ui/src/plugins/dev.perfetto.PinAndroidPerfMetrics/handlers/pinCujScoped.ts index 623b255ab3..4ae752aed4 100644 --- a/ui/src/plugins/dev.perfetto.PinAndroidPerfMetrics/handlers/pinCujScoped.ts +++ b/ui/src/plugins/dev.perfetto.PinAndroidPerfMetrics/handlers/pinCujScoped.ts @@ -150,7 +150,7 @@ class PinCujScopedJank implements MetricHandler { slice_id: NUM, track_id: NUM, }); - focusOnSlice(ctx, row.slice_id, row.track_id); + focusOnSlice(ctx, row.slice_id); } } diff --git a/ui/src/public/lib/query_table/query_table.ts b/ui/src/public/lib/query_table/query_table.ts index e4e7092d20..d7c7a63acf 100644 --- a/ui/src/public/lib/query_table/query_table.ts +++ b/ui/src/public/lib/query_table/query_table.ts @@ -162,18 +162,12 @@ class QueryTableRow implements m.ClassComponent { private selectSlice( sliceId: number, - trackUuid: string, + _trackUuid: string, switchToCurrentSelectionTab: boolean, ) { - globals.selectionManager.setLegacy( - { - kind: 'SLICE', - id: sliceId, - trackUri: trackUuid, - table: 'slice', - }, - {switchToCurrentSelectionTab}, - ); + globals.selectionManager.selectSqlEvent('slice', sliceId, { + switchToCurrentSelectionTab, + }); } } diff --git a/ui/src/public/selection.ts b/ui/src/public/selection.ts index 620e04b5da..3bacde0b56 100644 --- a/ui/src/public/selection.ts +++ b/ui/src/public/selection.ts @@ -23,16 +23,53 @@ export interface SelectionManager { readonly selection: Selection; readonly legacySelection: LegacySelection | null; findTimeRangeOfSelection(): Promise>; + clear(): void; - setEvent(trackUri: string, eventId: number): void; - setLegacy(args: LegacySelection, opts?: SelectionOpts): void; - setArea(args: Area): void; + + /** + * Select a track event. + * + * @param trackUri - The URI of the track to select. + * @param eventId - The value of the events ID column. + * @param opts - Additional options. + */ + selectTrackEvent( + trackUri: string, + eventId: number, + opts?: SelectionOpts, + ): void; + + /** + * Select a track event via a sql table name + id. + * + * @param sqlTableName - The name of the SQL table to resolve. + * @param id - The ID of the event in that table. + * @param opts - Additional options. + */ + selectSqlEvent(sqlTableName: string, id: number, opts?: SelectionOpts): void; + + /** + * Select a legacy selection. + * + * @param selection - The legacy selection to select. + * @param opts - Additional options. + */ + selectLegacy(selection: LegacySelection, opts?: SelectionOpts): void; + + /** + * Create an area selection for the purposes of aggregation. + * + * @param args - The area to select. + * @param opts - Additional options. + */ + selectArea(args: Area, opts?: SelectionOpts): void; + scrollToCurrentSelection(): void; registerAreaSelectionAggreagtor(aggr: AreaSelectionAggregator): void; // TODO(primiano): I don't undertsand what this generic slice is, but now // is exposed to plugins. For now i'm just carrying it forward. - setGenericSlice(args: { + selectGenericSlice(args: { id: number; sqlTableName: string; start: time; @@ -43,6 +80,26 @@ export interface SelectionManager { config: GenericSliceDetailsTabConfigBase; }; }): void; + + /** + * Register a new SQL selection resolver. + * + * A resolver consists of a SQL table name and a callback. When someone + * expresses an interest in selecting a slice on a matching table, the + * callback is called which can return a selection object or undefined. + */ + registerSqlSelectionResolver(resolver: SqlSelectionResolver): void; + + /** + * Resolve a section object for a given table and ID. + * + * @param sqlTableName - The name of the SQL table to resolve. + * @param id - The ID of the event in that table. + */ + resolveSqlEvent( + sqlTableName: string, + id: number, + ): Promise; } export interface AreaSelectionAggregator { @@ -212,3 +269,11 @@ export function profileType(s: string): ProfileType { } throw new Error('Unknown type ${s}'); } + +export interface SqlSelectionResolver { + readonly sqlTableName: string; + readonly callback: ( + id: number, + sqlTable: string, + ) => Promise; +} From b9ff56b011af6f232e4303fdd505ff6a7fa0a739 Mon Sep 17 00:00:00 2001 From: Steve Golton Date: Sat, 28 Sep 2024 19:25:08 +0100 Subject: [PATCH 2/2] ui: Remove LegacyDetailsPanel Change-Id: I353ea405be8da9909e3372ea70386dbb389eb512 --- ui/src/core/app_trace_impl.ts | 11 ------- ui/src/core/tab_manager.ts | 14 +-------- .../index.ts | 6 ++-- .../core_plugins/chrome_scroll_jank/index.ts | 6 ++-- ui/src/core_plugins/chrome_tasks/index.ts | 2 +- ui/src/core_plugins/cpu_profile/index.ts | 26 +++++++++------- ui/src/core_plugins/cpu_slices/index.ts | 2 +- ui/src/core_plugins/debug/index.ts | 2 +- ui/src/core_plugins/heap_profile/index.ts | 31 +++++++++++-------- .../perf_samples_profile/index.ts | 25 +++++++++------ ui/src/core_plugins/screenshots/index.ts | 2 +- ui/src/core_plugins/thread_slice/index.ts | 2 +- ui/src/core_plugins/thread_state/index.ts | 2 +- ui/src/frontend/aggregation_tab.ts | 1 - ui/src/frontend/notes_panel.ts | 2 -- ui/src/frontend/tab_panel.ts | 16 +--------- ui/src/frontend/ui_main.ts | 2 +- ui/src/public/details_panel.ts | 9 +----- ui/src/public/tab.ts | 2 ++ ui/src/public/trace.ts | 7 ----- ui/src/public/utils.ts | 17 +++++----- 21 files changed, 76 insertions(+), 111 deletions(-) diff --git a/ui/src/core/app_trace_impl.ts b/ui/src/core/app_trace_impl.ts index a4048517fe..2e26407b67 100644 --- a/ui/src/core/app_trace_impl.ts +++ b/ui/src/core/app_trace_impl.ts @@ -19,7 +19,6 @@ import {TimeSpan} from '../base/time'; import {TimelineImpl} from '../core/timeline'; import {App} from '../public/app'; import {Command} from '../public/command'; -import {DetailsPanel, LegacyDetailsPanel} from '../public/details_panel'; import {Trace} from '../public/trace'; import {setScrollToFunction} from '../public/scroll_helper'; import {ScrollToArgs} from '../public/scroll_helper'; @@ -328,16 +327,6 @@ export class TraceImpl implements Trace { return new TraceImpl(this.appImpl.forkForPlugin(pluginId), this.traceCtx); } - registerDetailsPanel( - detailsPanel: DetailsPanel | LegacyDetailsPanel, - ): Disposable { - if (detailsPanel.panelType === 'LegacyDetailsPanel') { - return this.traceCtx.tabMgr.registerLegacyDetailsPanel(detailsPanel); - } else { - return this.traceCtx.tabMgr.registerDetailsPanel(detailsPanel); - } - } - mountStore(migrate: Migrate): Store { return this.traceCtx.pluginSerializableState.createSubStore( [this.pluginId], diff --git a/ui/src/core/tab_manager.ts b/ui/src/core/tab_manager.ts index 759f47107d..8911f195a4 100644 --- a/ui/src/core/tab_manager.ts +++ b/ui/src/core/tab_manager.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {DetailsPanel, LegacyDetailsPanel} from '../public/details_panel'; +import {DetailsPanel} from '../public/details_panel'; import {TabDescriptor, TabManager} from '../public/tab'; import {raf} from './raf_scheduler'; @@ -28,7 +28,6 @@ export interface ResolvedTab { export class TabManagerImpl implements TabManager, Disposable { private _registry = new Map(); private _defaultTabs = new Set(); - private _legacyDetailsPanelRegistry = new Set(); private _detailsPanelRegistry = new Set(); private _instantiatedTabs = new Map(); private _openTabs: string[] = []; // URIs of the tabs open. @@ -56,13 +55,6 @@ export class TabManagerImpl implements TabManager, Disposable { }; } - registerLegacyDetailsPanel(section: LegacyDetailsPanel): Disposable { - this._legacyDetailsPanelRegistry.add(section); - return { - [Symbol.dispose]: () => this._legacyDetailsPanelRegistry.delete(section), - }; - } - registerDetailsPanel(section: DetailsPanel): Disposable { this._detailsPanelRegistry.add(section); return { @@ -146,10 +138,6 @@ export class TabManagerImpl implements TabManager, Disposable { return Array.from(this._defaultTabs); } - get legacyDetailsPanels(): LegacyDetailsPanel[] { - return Array.from(this._legacyDetailsPanelRegistry); - } - get detailsPanels(): DetailsPanel[] { return Array.from(this._detailsPanelRegistry); } diff --git a/ui/src/core_plugins/chrome_critical_user_interactions/index.ts b/ui/src/core_plugins/chrome_critical_user_interactions/index.ts index 8d75271682..23560c956a 100644 --- a/ui/src/core_plugins/chrome_critical_user_interactions/index.ts +++ b/ui/src/core_plugins/chrome_critical_user_interactions/index.ts @@ -50,7 +50,7 @@ class CriticalUserInteractionPlugin implements PerfettoPlugin { }), }); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( @@ -69,7 +69,7 @@ class CriticalUserInteractionPlugin implements PerfettoPlugin { }), ); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( @@ -88,7 +88,7 @@ class CriticalUserInteractionPlugin implements PerfettoPlugin { }), ); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( diff --git a/ui/src/core_plugins/chrome_scroll_jank/index.ts b/ui/src/core_plugins/chrome_scroll_jank/index.ts index 3a573b4542..a585f6b392 100644 --- a/ui/src/core_plugins/chrome_scroll_jank/index.ts +++ b/ui/src/core_plugins/chrome_scroll_jank/index.ts @@ -163,7 +163,7 @@ class ChromeScrollJankPlugin implements PerfettoPlugin { const track = new TrackNode({uri, title}); group.addChildInOrder(track); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( @@ -298,7 +298,7 @@ class ChromeScrollJankPlugin implements PerfettoPlugin { const track = new TrackNode({uri, title}); group.addChildInOrder(track); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( @@ -345,7 +345,7 @@ class ChromeScrollJankPlugin implements PerfettoPlugin { const track = new TrackNode({uri, title}); group.addChildInOrder(track); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( diff --git a/ui/src/core_plugins/chrome_tasks/index.ts b/ui/src/core_plugins/chrome_tasks/index.ts index 51f3fd9b1f..513da9f355 100644 --- a/ui/src/core_plugins/chrome_tasks/index.ts +++ b/ui/src/core_plugins/chrome_tasks/index.ts @@ -109,7 +109,7 @@ class ChromeTasksPlugin implements PerfettoPlugin { ctx.workspace.addChildInOrder(group); } - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( diff --git a/ui/src/core_plugins/cpu_profile/index.ts b/ui/src/core_plugins/cpu_profile/index.ts index 5b59e53dd7..c43150f49d 100644 --- a/ui/src/core_plugins/cpu_profile/index.ts +++ b/ui/src/core_plugins/cpu_profile/index.ts @@ -15,7 +15,7 @@ import m from 'mithril'; import {CPU_PROFILE_TRACK_KIND} from '../../public/track_kinds'; import {Engine} from '../../trace_processor/engine'; -import {LegacyDetailsPanel} from '../../public/details_panel'; +import {DetailsPanel} from '../../public/details_panel'; import {Trace} from '../../public/trace'; import {PerfettoPlugin, PluginDescriptor} from '../../public/plugin'; import {NUM, NUM_NULL, STR_NULL} from '../../trace_processor/query_result'; @@ -31,10 +31,7 @@ import { import {Timestamp} from '../../frontend/widgets/timestamp'; import {assertExists} from '../../base/logging'; import {DetailsShell} from '../../widgets/details_shell'; -import { - CpuProfileSampleSelection, - LegacySelection, -} from '../../public/selection'; +import {CpuProfileSampleSelection, Selection} from '../../public/selection'; import {getOrCreateGroupForThread} from '../../public/standard_groups'; import {TrackNode} from '../../public/workspace'; @@ -87,27 +84,32 @@ class CpuProfile implements PerfettoPlugin { const track = new TrackNode({uri, title, sortOrder: -40}); group.addChildInOrder(track); } - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new CpuProfileSampleFlamegraphDetailsPanel(ctx.engine), ); } } -class CpuProfileSampleFlamegraphDetailsPanel implements LegacyDetailsPanel { - readonly panelType = 'LegacyDetailsPanel'; +class CpuProfileSampleFlamegraphDetailsPanel implements DetailsPanel { private sel?: CpuProfileSampleSelection; private selMonitor = new Monitor([() => this.sel?.ts, () => this.sel?.utid]); private flamegraphAttrs?: QueryFlamegraphAttrs; constructor(private engine: Engine) {} - render(sel: LegacySelection) { - if (sel.kind !== 'CPU_PROFILE_SAMPLE') { + render(sel: Selection) { + if (sel.kind !== 'legacy') { + this.sel = undefined; + return; + } + + const legacySel = sel.legacySelection; + if (legacySel.kind !== 'CPU_PROFILE_SAMPLE') { this.sel = undefined; return undefined; } - const {ts, utid} = sel; - this.sel = sel; + const {ts, utid} = legacySel; + this.sel = legacySel; if (this.selMonitor.ifStateChanged()) { this.flamegraphAttrs = { engine: this.engine, diff --git a/ui/src/core_plugins/cpu_slices/index.ts b/ui/src/core_plugins/cpu_slices/index.ts index 278a29e8fb..655bf17371 100644 --- a/ui/src/core_plugins/cpu_slices/index.ts +++ b/ui/src/core_plugins/cpu_slices/index.ts @@ -60,7 +60,7 @@ class CpuSlices implements PerfettoPlugin { ctx.workspace.addChildInOrder(trackNode); } - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (sel) => { if (sel.kind !== 'SCHED_SLICE') { diff --git a/ui/src/core_plugins/debug/index.ts b/ui/src/core_plugins/debug/index.ts index eac89798f0..3a439770f6 100644 --- a/ui/src/core_plugins/debug/index.ts +++ b/ui/src/core_plugins/debug/index.ts @@ -69,7 +69,7 @@ class DebugTracksPlugin implements PerfettoPlugin { // TODO(stevegolton): While debug tracks are in their current state, we rely // on this plugin to provide the details panel for them. In the future, this // details panel will become part of the debug track's definition. - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( diff --git a/ui/src/core_plugins/heap_profile/index.ts b/ui/src/core_plugins/heap_profile/index.ts index f34cc1c2be..37c3b92f04 100644 --- a/ui/src/core_plugins/heap_profile/index.ts +++ b/ui/src/core_plugins/heap_profile/index.ts @@ -15,12 +15,12 @@ import m from 'mithril'; import {assertExists, assertFalse} from '../../base/logging'; import {Monitor} from '../../base/monitor'; -import {ProfileType} from '../../public/selection'; -import {HeapProfileSelection, LegacySelection} from '../../public/selection'; +import {ProfileType, Selection} from '../../public/selection'; +import {HeapProfileSelection} from '../../public/selection'; import {Timestamp} from '../../frontend/widgets/timestamp'; import {Engine} from '../../trace_processor/engine'; import {HEAP_PROFILE_TRACK_KIND} from '../../public/track_kinds'; -import {LegacyDetailsPanel} from '../../public/details_panel'; +import {DetailsPanel} from '../../public/details_panel'; import {Trace} from '../../public/trace'; import {PerfettoPlugin, PluginDescriptor} from '../../public/plugin'; import {NUM} from '../../trace_processor/query_result'; @@ -81,14 +81,13 @@ class HeapProfilePlugin implements PerfettoPlugin { where name = 'heap_graph_non_finalized_graph' `); const incomplete = it.firstRow({value: NUM}).value > 0; - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new HeapProfileFlamegraphDetailsPanel(ctx.engine, incomplete), ); } } -class HeapProfileFlamegraphDetailsPanel implements LegacyDetailsPanel { - readonly panelType = 'LegacyDetailsPanel'; +class HeapProfileFlamegraphDetailsPanel implements DetailsPanel { private sel?: HeapProfileSelection; private selMonitor = new Monitor([ () => this.sel?.ts, @@ -102,14 +101,20 @@ class HeapProfileFlamegraphDetailsPanel implements LegacyDetailsPanel { private heapGraphIncomplete: boolean, ) {} - render(sel: LegacySelection) { - if (sel.kind !== 'HEAP_PROFILE') { + render(sel: Selection) { + if (sel.kind !== 'legacy') { + this.sel = undefined; + return; + } + + const legacySel = sel.legacySelection; + if (legacySel.kind !== 'HEAP_PROFILE') { this.sel = undefined; return undefined; } - const {ts, upid, type} = sel; - this.sel = sel; + const {ts, upid, type} = legacySel; + this.sel = legacySel; if (this.selMonitor.ifStateChanged()) { this.flamegraphAttrs = flamegraphAttrs(this.engine, ts, upid, type); } @@ -123,7 +128,7 @@ class HeapProfileFlamegraphDetailsPanel implements LegacyDetailsPanel { title: m( '.title', getFlamegraphTitle(type), - sel.type === ProfileType.MIXED_HEAP_PROFILE && + legacySel.type === ProfileType.MIXED_HEAP_PROFILE && m( Popup, { @@ -139,8 +144,8 @@ class HeapProfileFlamegraphDetailsPanel implements LegacyDetailsPanel { description: [], buttons: [ m('.time', `Snapshot time: `, m(Timestamp, {ts})), - (sel.type === ProfileType.NATIVE_HEAP_PROFILE || - sel.type === ProfileType.JAVA_HEAP_SAMPLES) && + (legacySel.type === ProfileType.NATIVE_HEAP_PROFILE || + legacySel.type === ProfileType.JAVA_HEAP_SAMPLES) && m(Button, { icon: 'file_download', intent: Intent.Primary, diff --git a/ui/src/core_plugins/perf_samples_profile/index.ts b/ui/src/core_plugins/perf_samples_profile/index.ts index 53dffdc4b9..3a94baad36 100644 --- a/ui/src/core_plugins/perf_samples_profile/index.ts +++ b/ui/src/core_plugins/perf_samples_profile/index.ts @@ -15,12 +15,12 @@ import m from 'mithril'; import {TrackData} from '../../common/track_data'; import {Engine} from '../../trace_processor/engine'; -import {LegacyDetailsPanel} from '../../public/details_panel'; +import {DetailsPanel} from '../../public/details_panel'; import {PERF_SAMPLES_PROFILE_TRACK_KIND} from '../../public/track_kinds'; import {Trace} from '../../public/trace'; import {PerfettoPlugin, PluginDescriptor} from '../../public/plugin'; import {NUM, NUM_NULL, STR_NULL} from '../../trace_processor/query_result'; -import {LegacySelection, PerfSamplesSelection} from '../../public/selection'; +import {PerfSamplesSelection, Selection} from '../../public/selection'; import { QueryFlamegraph, QueryFlamegraphAttrs, @@ -122,12 +122,13 @@ class PerfSamplesProfilePlugin implements PerfettoPlugin { const track = new TrackNode({uri, title, sortOrder: -50}); group.addChildInOrder(track); } - ctx.registerDetailsPanel(new PerfSamplesFlamegraphDetailsPanel(ctx.engine)); + ctx.tabs.registerDetailsPanel( + new PerfSamplesFlamegraphDetailsPanel(ctx.engine), + ); } } -class PerfSamplesFlamegraphDetailsPanel implements LegacyDetailsPanel { - readonly panelType = 'LegacyDetailsPanel'; +class PerfSamplesFlamegraphDetailsPanel implements DetailsPanel { private sel?: PerfSamplesSelection; private selMonitor = new Monitor([ () => this.sel?.leftTs, @@ -140,14 +141,20 @@ class PerfSamplesFlamegraphDetailsPanel implements LegacyDetailsPanel { constructor(private engine: Engine) {} - render(sel: LegacySelection) { - if (sel.kind !== 'PERF_SAMPLES') { + render(sel: Selection) { + if (sel.kind !== 'legacy') { + this.sel = undefined; + return; + } + + const legacySel = sel.legacySelection; + if (legacySel.kind !== 'PERF_SAMPLES') { this.sel = undefined; return undefined; } - const {leftTs, rightTs, upid, utid} = sel; - this.sel = sel; + const {leftTs, rightTs, upid, utid} = legacySel; + this.sel = legacySel; if (this.selMonitor.ifStateChanged()) { this.flamegraphAttrs = { engine: this.engine, diff --git a/ui/src/core_plugins/screenshots/index.ts b/ui/src/core_plugins/screenshots/index.ts index cca7c4ef7b..b7e41e2281 100644 --- a/ui/src/core_plugins/screenshots/index.ts +++ b/ui/src/core_plugins/screenshots/index.ts @@ -49,7 +49,7 @@ class ScreenshotsPlugin implements PerfettoPlugin { const trackNode = new TrackNode({uri, title, sortOrder: -60}); ctx.workspace.addChildInOrder(trackNode); - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (selection) => { if ( diff --git a/ui/src/core_plugins/thread_slice/index.ts b/ui/src/core_plugins/thread_slice/index.ts index e624f2a208..d57d41de23 100644 --- a/ui/src/core_plugins/thread_slice/index.ts +++ b/ui/src/core_plugins/thread_slice/index.ts @@ -123,7 +123,7 @@ class ThreadSlicesPlugin implements PerfettoPlugin { group.addChildInOrder(track); } - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (sel) => { if (sel.kind !== 'SLICE') { diff --git a/ui/src/core_plugins/thread_state/index.ts b/ui/src/core_plugins/thread_state/index.ts index 1e67975c42..f650ad8d94 100644 --- a/ui/src/core_plugins/thread_state/index.ts +++ b/ui/src/core_plugins/thread_state/index.ts @@ -101,7 +101,7 @@ class ThreadState implements PerfettoPlugin { group.addChildInOrder(track); } - ctx.registerDetailsPanel( + ctx.tabs.registerDetailsPanel( new BottomTabToSCSAdapter({ tabFactory: (sel) => { if (sel.kind !== 'THREAD_STATE') { diff --git a/ui/src/frontend/aggregation_tab.ts b/ui/src/frontend/aggregation_tab.ts index 9ee281c2f1..2eedad2952 100644 --- a/ui/src/frontend/aggregation_tab.ts +++ b/ui/src/frontend/aggregation_tab.ts @@ -372,7 +372,6 @@ export class AggregationsTabs implements Disposable { constructor(trace: TraceImpl) { const unregister = globals.tabManager.registerDetailsPanel({ - panelType: 'DetailsPanel', render(selection) { if (selection.kind === 'area') { return m(AreaDetailsPanel, {trace}); diff --git a/ui/src/frontend/notes_panel.ts b/ui/src/frontend/notes_panel.ts index 1752baea7f..ad90d8803a 100644 --- a/ui/src/frontend/notes_panel.ts +++ b/ui/src/frontend/notes_panel.ts @@ -369,8 +369,6 @@ export class NotesPanel implements Panel { } export class NotesEditorTab implements DetailsPanel { - readonly panelType = 'DetailsPanel'; - constructor(private trace: Trace) {} render(selection: Selection) { diff --git a/ui/src/frontend/tab_panel.ts b/ui/src/frontend/tab_panel.ts index 36d68d8dd8..735377153e 100644 --- a/ui/src/frontend/tab_panel.ts +++ b/ui/src/frontend/tab_panel.ts @@ -122,7 +122,6 @@ export class TabPanel implements m.ClassComponent { private renderCSTabContent(): {isLoading: boolean; content: m.Children} { const currentSelection = globals.selectionManager.selection; - const legacySelection = globals.selectionManager.legacySelection; if (currentSelection.kind === 'empty') { return { isLoading: false, @@ -154,26 +153,13 @@ export class TabPanel implements m.ClassComponent { } // Get the first "truthy" details panel - let detailsPanels = globals.tabManager.detailsPanels.map((dp) => { + const detailsPanels = globals.tabManager.detailsPanels.map((dp) => { return { content: dp.render(currentSelection), isLoading: dp.isLoading?.() ?? false, }; }); - if (legacySelection !== null) { - const legacyDetailsPanels = globals.tabManager.legacyDetailsPanels.map( - (dp) => { - return { - content: dp.render(legacySelection), - isLoading: dp.isLoading?.() ?? false, - }; - }, - ); - - detailsPanels = detailsPanels.concat(legacyDetailsPanels); - } - const panel = detailsPanels.find(({content}) => content); if (panel) { diff --git a/ui/src/frontend/ui_main.ts b/ui/src/frontend/ui_main.ts index ece7c236fd..06f1651756 100644 --- a/ui/src/frontend/ui_main.ts +++ b/ui/src/frontend/ui_main.ts @@ -134,7 +134,7 @@ export class UiMainPerTrace implements m.ClassComponent { this.trash.use(new AggregationsTabs(trace)); // Register the notes manager+editor. - this.trash.use(trace.registerDetailsPanel(new NotesEditorTab(trace))); + this.trash.use(trace.tabs.registerDetailsPanel(new NotesEditorTab(trace))); this.trash.use( trace.tabs.registerTab({ diff --git a/ui/src/public/details_panel.ts b/ui/src/public/details_panel.ts index 3cd8d827f3..160c99dab2 100644 --- a/ui/src/public/details_panel.ts +++ b/ui/src/public/details_panel.ts @@ -13,16 +13,9 @@ // limitations under the License. import m from 'mithril'; -import {LegacySelection, Selection} from './selection'; - -export interface LegacyDetailsPanel { - readonly panelType: 'LegacyDetailsPanel'; - render(selection: LegacySelection): m.Children; - isLoading?(): boolean; -} +import {Selection} from './selection'; export interface DetailsPanel { - readonly panelType: 'DetailsPanel'; render(selection: Selection): m.Children; isLoading?(): boolean; } diff --git a/ui/src/public/tab.ts b/ui/src/public/tab.ts index 97d706510c..12258c228d 100644 --- a/ui/src/public/tab.ts +++ b/ui/src/public/tab.ts @@ -13,9 +13,11 @@ // limitations under the License. import m from 'mithril'; +import {DetailsPanel} from './details_panel'; export interface TabManager { registerTab(tab: TabDescriptor): void; + registerDetailsPanel(detailsPanel: DetailsPanel): Disposable; showTab(uri: string): void; hideTab(uri: string): void; addDefaultTab(uri: string): void; diff --git a/ui/src/public/trace.ts b/ui/src/public/trace.ts index 2016dc85f8..e7e1602735 100644 --- a/ui/src/public/trace.ts +++ b/ui/src/public/trace.ts @@ -20,7 +20,6 @@ import {TabManager} from './tab'; import {TrackManager} from './track'; import {Timeline} from './timeline'; import {Workspace, WorkspaceManager} from './workspace'; -import {DetailsPanel, LegacyDetailsPanel} from './details_panel'; import {SelectionManager} from './selection'; import {ScrollToArgs} from './scroll_helper'; import {NoteManager} from './note'; @@ -48,12 +47,6 @@ export interface Trace extends App { // selection. scrollTo(args: ScrollToArgs): void; - // TODO(primiano): remove this once the Legacy vs non-Legacy details panel is - // gone. This method is particularly problematic because the method called - // registerDetailsPanel in TabManagerImpl takes a non-Legacy DetailsPanel, but - // all plugins use a Legacy one. Keeping this as a bridge for now. - registerDetailsPanel(detailsPanel: DetailsPanel | LegacyDetailsPanel): void; - // Create a store mounted over the top of this plugin's persistent state. mountStore(migrate: Migrate): Store; diff --git a/ui/src/public/utils.ts b/ui/src/public/utils.ts index 10ea3e815c..727ae8e5b6 100644 --- a/ui/src/public/utils.ts +++ b/ui/src/public/utils.ts @@ -13,11 +13,11 @@ // limitations under the License. import m from 'mithril'; -import {LegacySelection} from '../public/selection'; +import {LegacySelection, Selection} from '../public/selection'; import {BottomTab} from './lib/bottom_tab'; import {Tab} from './tab'; import {exists} from '../base/utils'; -import {LegacyDetailsPanel} from './details_panel'; +import {DetailsPanel} from './details_panel'; import {Trace} from './trace'; import {TimeSpan} from '../base/time'; @@ -130,9 +130,8 @@ export interface BottomTabAdapterAttrs { }, }) */ -export class BottomTabToSCSAdapter implements LegacyDetailsPanel { - readonly panelType = 'LegacyDetailsPanel'; - private oldSelection?: LegacySelection; +export class BottomTabToSCSAdapter implements DetailsPanel { + private oldSelection?: Selection; private bottomTab?: BottomTab; private attrs: BottomTabAdapterAttrs; @@ -140,11 +139,15 @@ export class BottomTabToSCSAdapter implements LegacyDetailsPanel { this.attrs = attrs; } - render(selection: LegacySelection): m.Children { + render(selection: Selection): m.Children { // Detect selection changes, assuming selection is immutable if (selection !== this.oldSelection) { this.oldSelection = selection; - this.bottomTab = this.attrs.tabFactory(selection); + if (selection.kind === 'legacy') { + this.bottomTab = this.attrs.tabFactory(selection.legacySelection); + } else { + this.bottomTab = undefined; + } } return this.bottomTab?.renderPanel();