diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 3c427a9aa0..f27c1e595e 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -40,6 +40,8 @@ import type { NativeSymbolInfo, Transform, IndexIntoFrameTable, + MarkerIndex, + SelectedMarkersPerThread, } from 'firefox-profiler/types'; import { decodeUintArrayFromUrlComponent, @@ -49,7 +51,7 @@ import { import { tabSlugs } from '../app-logic/tabs-handling'; import { StringTable } from 'firefox-profiler/utils/string-table'; -export const CURRENT_URL_VERSION = 13; +export const CURRENT_URL_VERSION = 14; /** * This static piece of state might look like an anti-pattern, but it's a relatively @@ -184,6 +186,7 @@ type CallTreeQuery = BaseQuery & { type MarkersQuery = BaseQuery & { markerSearch: string; // "DOMEvent" + marker?: MarkerIndex; // Selected marker index for the current thread, e.g. 42 }; type NetworkQuery = BaseQuery & { @@ -218,6 +221,7 @@ type Query = BaseQuery & { // Markers specific markerSearch?: string; + marker?: MarkerIndex; // Network specific networkSearch?: string; @@ -367,6 +371,11 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { query = baseQuery as MarkersQueryShape; query.markerSearch = urlState.profileSpecific.markersSearchString || undefined; + query.marker = + selectedThreadsKey !== null && + urlState.profileSpecific.selectedMarkers[selectedThreadsKey] !== null + ? urlState.profileSpecific.selectedMarkers[selectedThreadsKey] + : undefined; break; case 'network-chart': query = baseQuery as NetworkQueryShape; @@ -499,6 +508,19 @@ export function stateFromLocation( transforms[selectedThreadsKey] = parseTransforms(query.transforms); } + // Parse the selected marker for the current thread + const selectedMarkers: SelectedMarkersPerThread = {}; + if ( + selectedThreadsKey !== null && + query.marker !== undefined && + query.marker !== null + ) { + const markerIndex = Number(query.marker); + if (!isNaN(markerIndex)) { + selectedMarkers[selectedThreadsKey] = markerIndex; + } + } + // tabID is used for the tab selector that we have in our full view. let tabID = null; if (query.tabID && Number.isInteger(Number(query.tabID))) { @@ -587,6 +609,7 @@ export function stateFromLocation( legacyHiddenThreads: query.hiddenThreads ? query.hiddenThreads.split('-').map((index) => Number(index)) : null, + selectedMarkers, }, }; } @@ -1196,6 +1219,10 @@ const _upgraders: { [13]: (_) => { // just added the focus-self transform }, + [14]: (_) => { + // Added marker parameter for persisting highlighted markers in URLs. + // This is backward compatible as the marker parameter is optional. + }, }; for (let destVersion = 1; destVersion <= CURRENT_URL_VERSION; destVersion++) { diff --git a/src/components/marker-chart/Canvas.tsx b/src/components/marker-chart/Canvas.tsx index 3f49c0d3e6..946002404e 100644 --- a/src/components/marker-chart/Canvas.tsx +++ b/src/components/marker-chart/Canvas.tsx @@ -147,6 +147,42 @@ function getDefaultMarkerColors(isHighlighted: boolean) { class MarkerChartCanvasImpl extends React.PureComponent { _textMeasurement: TextMeasurement | null = null; + override componentDidUpdate(prevProps: Props) { + // When the viewport finishes sizing itself, or when the selected marker changes, + // scroll to bring the selected marker into view (e.g. on initial load from URL). + const viewportDidMount = + !prevProps.viewport.isSizeSet && this.props.viewport.isSizeSet; + const selectedMarkerChanged = + this.props.selectedMarkerIndex !== prevProps.selectedMarkerIndex; + + if (viewportDidMount || selectedMarkerChanged) { + this._scrollSelectionIntoView(); + } + } + + _scrollSelectionIntoView = () => { + const { selectedMarkerIndex, markerTimingAndBuckets, rowHeight, viewport } = + this.props; + + if (selectedMarkerIndex === null) { + return; + } + + const markerIndexToTimingRow = this._getMarkerIndexToTimingRow( + markerTimingAndBuckets + ); + const rowIndex = markerIndexToTimingRow[selectedMarkerIndex]; + const y: CssPixels = rowIndex * rowHeight; + const { viewportTop, viewportBottom } = viewport; + + if (y < viewportTop || y + rowHeight > viewportBottom) { + // Scroll the marker to the vertical center of the viewport. + const viewportHeight = viewportBottom - viewportTop; + const targetViewportTop = y + rowHeight / 2 - viewportHeight / 2; + viewport.moveViewport(0, viewportTop - targetViewportTop); + } + }; + /** * Get the fill, stroke, and text colors for a marker based on its schema and data. * If the marker schema has a colorField, use that field's value. @@ -957,8 +993,92 @@ class MarkerChartCanvasImpl extends React.PureComponent { ); }; + /** + * Calculate the canvas position for a marker's tooltip based on the marker's index. + * The method is used when the selected marker comes from the URL and does NOT + * necessarily correspond to what's being hovered at the moment. + * + * @param markerIndex - The marker's index in the original thread's marker table + * @returns Canvas-relative coordinates {offsetX, offsetY} or null if marker not visible + */ + getTooltipPosition = ( + markerIndex: MarkerIndex + ): { offsetX: CssPixels; offsetY: CssPixels } | null => { + const { + rangeStart, + rangeEnd, + markerTimingAndBuckets, + rowHeight, + marginLeft, + marginRight, + viewport: { containerWidth, viewportLeft, viewportRight, viewportTop }, + } = this.props; + + // Step 1: Find which row this marker is displayed in + const markerIndexToTimingRow = this._getMarkerIndexToTimingRow( + markerTimingAndBuckets + ); + const rowIndex = markerIndexToTimingRow[markerIndex]; + + // Step 2: Get the timing data for all markers in this row + const markerTiming = markerTimingAndBuckets[rowIndex]; + if (!markerTiming || typeof markerTiming === 'string') { + // Row is empty or is a bucket label (string), not actual marker data + return null; + } + + // Step 3: Find the position of our specific marker within this row's data + let markerTimingIndex = -1; + for (let i = 0; i < markerTiming.length; i++) { + if (markerTiming.index[i] === markerIndex) { + markerTimingIndex = i; + break; + } + } + + if (markerTimingIndex === -1) { + // Marker not found in this row's data (shouldn't happen, but handle gracefully) + return null; + } + + // Step 4: Calculate horizontal (X) position + const startTimestamp = markerTiming.start[markerTimingIndex]; + const endTimestamp = markerTiming.end[markerTimingIndex]; + + // Convert absolute timestamps to relative positions (0.0 to 1.0 of the full range) + const markerContainerWidth = containerWidth - marginLeft - marginRight; + const rangeLength: Milliseconds = rangeEnd - rangeStart; + const viewportLength: UnitIntervalOfProfileRange = + viewportRight - viewportLeft; + const startTime: UnitIntervalOfProfileRange = + (startTimestamp - rangeStart) / rangeLength; + const endTime: UnitIntervalOfProfileRange = + (endTimestamp - rangeStart) / rangeLength; + + // Calculate pixel position: map the time range to the visible viewport + const x: CssPixels = + ((startTime - viewportLeft) * markerContainerWidth) / viewportLength + + marginLeft; + const w: CssPixels = + ((endTime - startTime) * markerContainerWidth) / viewportLength; + + // For instant markers (start === end), use the center point + // For interval markers, use a point 1/3 into the marker (or 30px, whichever is smaller) + const isInstantMarker = startTimestamp === endTimestamp; + const offsetX = isInstantMarker ? x : x + Math.min(w / 3, 30); + + // Step 5: Calculate vertical (Y) position + // Place the tooltip at the top of the marker's row, with a small offset + const offsetY: CssPixels = rowIndex * rowHeight - viewportTop + 5; + + // Return canvas-relative coordinates (should be converted to page coordinates by caller) + return { offsetX, offsetY }; + }; + override render() { - const { containerWidth, containerHeight, isDragging } = this.props.viewport; + const { containerWidth, containerHeight, isDragging, viewportTop } = + this.props.viewport; + const { selectedMarkerIndex } = this.props; return ( { onMouseMove={this.onMouseMove} onMouseLeave={this.onMouseLeave} stickyTooltips={true} + selectedItem={selectedMarkerIndex} + getTooltipPosition={this.getTooltipPosition} + viewportTop={viewportTop} /> ); } diff --git a/src/components/shared/chart/Canvas.tsx b/src/components/shared/chart/Canvas.tsx index c181d59158..dcdd6f395f 100644 --- a/src/components/shared/chart/Canvas.tsx +++ b/src/components/shared/chart/Canvas.tsx @@ -33,6 +33,14 @@ type Props = { readonly onMouseLeave?: (e: { nativeEvent: MouseEvent }) => unknown; // Defaults to false. Set to true if the chart should persist the tooltips on click. readonly stickyTooltips?: boolean; + // Selected item coming from URL or other non-click mechanism. + readonly selectedItem?: Item | null; + // Method to compute tooltip position for the selected item above. + readonly getTooltipPosition?: ( + item: Item + ) => { offsetX: CssPixels; offsetY: CssPixels } | null; + // Current vertical scroll offset of the viewport. + readonly viewportTop?: CssPixels; }; // The naming of the X and Y coordinates here correspond to the ones @@ -359,6 +367,54 @@ export class ChartCanvas extends React.Component< this._canvas = canvas; }; + _syncSelectedItemFromProp = (selectedItem: Item | null) => { + if (selectedItem === null) { + this.setState({ selectedItem: null }); + return; + } + + if (!this.props.getTooltipPosition || !this._canvas) { + return; + } + + const tooltipPosition = this.props.getTooltipPosition(selectedItem); + if (!tooltipPosition) { + // Can't get position, but still set the selectedItem + this.setState({ selectedItem }); + return; + } + + const { offsetX, offsetY } = tooltipPosition; + + // If the item is outside the visible canvas area, skip setting the tooltip + // position for now. The viewport will scroll to bring it into view, and the + // viewportTop change will trigger a re-sync with the correct position. + if (offsetY < 0 || offsetY > this.props.containerHeight) { + return; + } + + const canvasRect = this._canvas.getBoundingClientRect(); + const pageX = canvasRect.left + window.scrollX + offsetX; + const pageY = canvasRect.top + window.scrollY + offsetY; + this.setState({ + selectedItem, + pageX, + pageY, + }); + }; + + override componentDidMount() { + // Initialize selectedItem from props on mount if provided and the canvas + // already has dimensions. If containerWidth is still 0, the viewport hasn't + // been laid out yet - componentDidUpdate will pick it up once it becomes non-zero. + if ( + this.props.selectedItem !== undefined && + this.props.containerWidth !== 0 + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + } + override UNSAFE_componentWillReceiveProps() { // It is possible that the data backing the chart has been // changed, for instance after symbolication. Clear the @@ -377,6 +433,37 @@ export class ChartCanvas extends React.Component< override componentDidUpdate(prevProps: Props, prevState: State) { if (prevProps !== this.props) { + // Sync selectedItem state with selectedItem prop if it changed + // and the state doesn't already have this item selected. + if ( + this.props.selectedItem !== undefined && + this.props.selectedItem !== prevProps.selectedItem && + !hoveredItemsAreEqual(this.state.selectedItem, this.props.selectedItem) + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + + // The canvas just received its dimensions for the first time (containerWidth + // went from 0 to non-zero). If a selectedItem was provided but couldn't be + // synced in componentDidMount, do it now. + if ( + prevProps.containerWidth === 0 && + this.props.containerWidth !== 0 && + this.props.selectedItem !== undefined + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + + // The viewport scrolled (e.g. to bring the selected item into view on + // load). Re-sync the tooltip position so it stays on the selected item. + if ( + this.props.viewportTop !== undefined && + this.props.viewportTop !== prevProps.viewportTop && + this.props.selectedItem !== undefined + ) { + this._syncSelectedItemFromProp(this.props.selectedItem); + } + if ( this.state.selectedItem !== null && prevState.selectedItem === this.state.selectedItem diff --git a/src/reducers/profile-view.ts b/src/reducers/profile-view.ts index 26465510fd..1a641310e6 100644 --- a/src/reducers/profile-view.ts +++ b/src/reducers/profile-view.ts @@ -137,7 +137,6 @@ export const defaultThreadViewOptions: ThreadViewOptions = { selectedInvertedCallNodePath: [], expandedNonInvertedCallNodePaths: new PathSet(), expandedInvertedCallNodePaths: new PathSet(), - selectedMarker: null, selectedNetworkMarker: null, lastSeenTransformCount: 0, }; @@ -328,10 +327,6 @@ const viewOptionsPerThread: Reducer = ( : { expandedNonInvertedCallNodePaths: expandedCallNodePaths } ); } - case 'CHANGE_SELECTED_MARKER': { - const { threadsKey, selectedMarker } = action; - return _updateThreadViewOptions(state, threadsKey, { selectedMarker }); - } case 'CHANGE_SELECTED_NETWORK_MARKER': { const { threadsKey, selectedNetworkMarker } = action; return _updateThreadViewOptions(state, threadsKey, { diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts index 8903ea34b2..3e3f5b6379 100644 --- a/src/reducers/url-state.ts +++ b/src/reducers/url-state.ts @@ -23,6 +23,7 @@ import type { AssemblyViewState, IsOpenPerPanelState, TabID, + SelectedMarkersPerThread, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; @@ -677,6 +678,24 @@ const symbolServerUrl: Reducer = (state = null) => { return state; }; +const selectedMarkers: Reducer = ( + state = {}, + action +): SelectedMarkersPerThread => { + switch (action.type) { + case 'CHANGE_SELECTED_MARKER': { + const { threadsKey, selectedMarker } = action; + // Store the selected marker for this specific threadsKey + return { + ...state, + [threadsKey]: selectedMarker, + }; + } + default: + return state; + } +}; + /** * These values are specific to an individual profile. */ @@ -703,6 +722,7 @@ const profileSpecific = combineReducers({ localTrackOrderChangedPids, showJsTracerSummary, tabFilter, + selectedMarkers, // The timeline tracks used to be hidden and sorted by thread indexes, rather than // track indexes. The only way to migrate this information to tracks-based data is to // first retrieve the profile, so they can't be upgraded by the normal url upgrading diff --git a/src/selectors/per-thread/markers.ts b/src/selectors/per-thread/markers.ts index 333427b3e7..542eb71afd 100644 --- a/src/selectors/per-thread/markers.ts +++ b/src/selectors/per-thread/markers.ts @@ -552,7 +552,7 @@ export function getMarkerSelectorsPerThread( * This returns the marker index for the currently selected marker. */ const getSelectedMarkerIndex: Selector = (state) => - threadSelectors.getViewOptions(state).selectedMarker; + UrlState.getSelectedMarker(state, threadsKey); /** * From the previous value, this returns the full marker object for the diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index 3045882da5..c3ff9e27e6 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -31,6 +31,7 @@ import type { NativeSymbolInfo, TabID, IndexIntoSourceTable, + MarkerIndex, } from 'firefox-profiler/types'; import type { TabSlug } from '../app-logic/tabs-handling'; @@ -236,6 +237,12 @@ export const getTransformStack: DangerousSelectorWithArguments< ); }; +export const getSelectedMarker: DangerousSelectorWithArguments< + MarkerIndex | null, + ThreadsKey +> = (state, threadsKey) => + getProfileSpecificState(state).selectedMarkers[threadsKey] ?? null; + export const getIsBottomBoxOpen: Selector = (state) => { const tab = getSelectedTab(state); return getProfileSpecificState(state).isBottomBoxOpenPerPanel[tab]; diff --git a/src/test/store/__snapshots__/profile-view.test.ts.snap b/src/test/store/__snapshots__/profile-view.test.ts.snap index 9a12a5e6a9..c8e4181a52 100644 --- a/src/test/store/__snapshots__/profile-view.test.ts.snap +++ b/src/test/store/__snapshots__/profile-view.test.ts.snap @@ -4428,7 +4428,6 @@ Object { }, "lastSeenTransformCount": 1, "selectedInvertedCallNodePath": Array [], - "selectedMarker": 1, "selectedNetworkMarker": null, "selectedNonInvertedCallNodePath": Array [ 0, diff --git a/src/test/store/profile-view.test.ts b/src/test/store/profile-view.test.ts index 46d1fee453..46c5a627b0 100644 --- a/src/test/store/profile-view.test.ts +++ b/src/test/store/profile-view.test.ts @@ -1046,11 +1046,11 @@ describe('actions/ProfileView', function () { const { dispatch, getState } = storeWithProfile(profile); expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker + selectedThreadSelectors.getSelectedMarkerIndex(getState()) ).toEqual(null); dispatch(ProfileView.changeSelectedMarker(0, 0)); expect( - selectedThreadSelectors.getViewOptions(getState()).selectedMarker + selectedThreadSelectors.getSelectedMarkerIndex(getState()) ).toEqual(0); }); }); diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index 665069f8f1..7b03d6a78e 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -20,6 +20,7 @@ import { closeBottomBox, changeShowUserTimings, changeStackChartSameWidths, + changeSelectedMarker, } from '../actions/profile-view'; import { changeSelectedTab, changeProfilesToCompare } from '../actions/app'; import { @@ -47,7 +48,10 @@ import { getHumanReadableTracks, getProfileWithNiceTracks, } from './fixtures/profiles/tracks'; -import { getProfileFromTextSamples } from './fixtures/profiles/processed-profile'; +import { + getProfileFromTextSamples, + getProfileWithMarkers, +} from './fixtures/profiles/processed-profile'; import { selectedThreadSelectors } from '../selectors/per-thread'; import { encodeUintArrayForUrlComponent, @@ -1987,3 +1991,315 @@ describe('tab selector', function () { expect(urlStateSelectors.hasTabFilter(getState())).toEqual(true); }); }); + +describe('selectedMarker', function () { + function setup(search = '', pathname?: string) { + const profile = getProfileWithMarkers([ + ['Marker A', 0, null], + ['Marker B', 1, null], + ['Marker C', 2, null], + ]); + const settings: StoreUrlSettings = { search }; + if (pathname) { + settings.pathname = pathname; + } + return _getStoreWithURL(settings, profile); + } + + it('can serialize a selected marker to the URL on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('can unserialize a selected marker from the URL on marker-chart tab', () => { + const markerIndex = 2; + const profile = getProfileWithMarkers([ + ['Marker A', 0, null], + ['Marker B', 1, null], + ['Marker C', 2, null], + ]); + const { getState } = _getStoreWithURL( + { + pathname: + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/', + search: `?thread=0&marker=${markerIndex}`, + }, + profile + ); + + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + markerIndex + ); + }); + + it('handles marker index 0 correctly on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 0; + const threadsKey = getThreadsKey(new Set([0])); + + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + expect(queryString).not.toContain('marker=undefined'); + }); + + it('does not include marker in URL when switching to a thread without a selected marker on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey0 = getThreadsKey(new Set([0])); + + // Select a marker on thread 0 + dispatch(changeSelectedMarker(threadsKey0, markerIndex)); + expect(getQueryStringFromState(getState())).toContain( + `marker=${markerIndex}` + ); + + // Switch to thread 1 (no marker selected) + dispatch(changeSelectedThreads(new Set([1]))); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('preserves selected marker in internal state when switching back to the thread on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey0 = getThreadsKey(new Set([0])); + + // Select a marker on thread 0 + dispatch(changeSelectedMarker(threadsKey0, markerIndex)); + + // Switch to thread 1 + dispatch(changeSelectedThreads(new Set([1]))); + + // Marker should not be in URL for thread 1 + expect(getQueryStringFromState(getState())).not.toContain('marker='); + + // Switch back to thread 0 + dispatch(changeSelectedThreads(new Set([0]))); + + // The marker should reappear in the URL + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('handles null marker index correctly on marker-chart tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const threadsKey = getThreadsKey(new Set([0])); + + // Select a marker first + dispatch(changeSelectedMarker(threadsKey, 1)); + expect(getQueryStringFromState(getState())).toContain('marker=1'); + + // Deselect the marker + dispatch(changeSelectedMarker(threadsKey, null)); + + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('survives a URL round-trip on marker-chart tab', () => { + const profile = getProfileWithMarkers([ + ['Marker A', 0, null], + ['Marker B', 1, null], + ['Marker C', 2, null], + ]); + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 2; + const threadsKey = getThreadsKey(new Set([0])); + + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Get the URL + const urlState = urlStateSelectors.getUrlState(getState()); + const url = urlFromState(urlState); + + // URL should contain the marker + expect(url).toContain(`marker=${markerIndex}`); + + // Load a fresh store from the URL and verify via selector + const { getState: getState2 } = _getStoreWithURL( + { + pathname: + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/', + search: new URL(url, 'https://profiler.firefox.com').search, + }, + profile + ); + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState2())).toEqual( + markerIndex + ); + }); + + it('does NOT include marker in URL on calltree tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/calltree/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Force tab to calltree since viewProfile auto-switches to marker-chart for profiles without samples + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'calltree' }); + + // Select a marker on calltree tab + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Marker should NOT be in URL for calltree tab + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('does NOT include marker in URL on flame-graph tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/flame-graph/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Force tab to flame-graph since viewProfile auto-switches to marker-chart for profiles without samples + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'flame-graph' }); + + // Select a marker on flame-graph tab + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Marker should NOT be in URL for flame-graph tab + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('includes marker in URL on marker-table tab', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-table/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Select a marker on marker-table tab + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + + // Marker SHOULD be in URL for marker-table tab + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('removes marker from URL when switching from marker-chart to calltree', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Select a marker on marker-chart + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + expect(getQueryStringFromState(getState())).toContain( + `marker=${markerIndex}` + ); + + // Switch to calltree tab + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'calltree' }); + + // Marker should be removed from URL + const queryString = getQueryStringFromState(getState()); + expect(queryString).not.toContain('marker='); + }); + + it('adds marker to URL when switching from calltree to marker-chart', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/calltree/' + ); + const markerIndex = 1; + const threadsKey = getThreadsKey(new Set([0])); + + // Force tab to calltree since viewProfile auto-switches to marker-chart for profiles without samples + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'calltree' }); + + // Select a marker on calltree (won't be in URL) + dispatch(changeSelectedMarker(threadsKey, markerIndex)); + expect(getQueryStringFromState(getState())).not.toContain('marker='); + + // Switch to marker-chart tab + dispatch({ type: 'CHANGE_SELECTED_TAB', selectedTab: 'marker-chart' }); + + // Marker should now appear in URL + const queryString = getQueryStringFromState(getState()); + expect(queryString).toContain(`marker=${markerIndex}`); + }); + + it('syncs selected marker when URL state changes (browser back/forward)', () => { + const { dispatch, getState } = setup( + '?thread=0', + '/public/1ecd7a421948995171a4bb483b7bcc8e1868cc57/marker-chart/' + ); + const threadsKey = getThreadsKey(new Set([0])); + + // Select marker 1 + dispatch(changeSelectedMarker(threadsKey, 1)); + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + 1 + ); + + // Simulate browser navigation by dispatching UPDATE_URL_STATE + // with a different selected marker (as if we navigated back) + const currentUrlState = urlStateSelectors.getUrlState(getState()); + const newUrlState = { + ...currentUrlState, + profileSpecific: { + ...currentUrlState.profileSpecific, + selectedMarkers: { + [threadsKey]: 2, // Different marker + }, + }, + }; + dispatch({ type: 'UPDATE_URL_STATE', newUrlState }); + + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + 2 + ); + + // Now simulate navigating to a state with no selected marker + const urlStateWithNoMarker = { + ...currentUrlState, + profileSpecific: { + ...currentUrlState.profileSpecific, + selectedMarkers: { + [threadsKey]: null, + }, + }, + }; + dispatch({ type: 'UPDATE_URL_STATE', newUrlState: urlStateWithNoMarker }); + + expect(selectedThreadSelectors.getSelectedMarkerIndex(getState())).toEqual( + null + ); + }); +}); diff --git a/src/types/state.ts b/src/types/state.ts index 792a1ded08..6e18bb2630 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -56,7 +56,6 @@ export type ThreadViewOptions = { readonly selectedInvertedCallNodePath: CallNodePath; readonly expandedNonInvertedCallNodePaths: PathSet; readonly expandedInvertedCallNodePaths: PathSet; - readonly selectedMarker: MarkerIndex | null; readonly selectedNetworkMarker: MarkerIndex | null; // Track the number of transforms to detect when they change via browser // navigation. This helps us know when to reset paths that may be invalid @@ -84,6 +83,10 @@ export type MarkerReference = { readonly markerIndex: MarkerIndex; }; +export type SelectedMarkersPerThread = { + [key: ThreadsKey]: MarkerIndex | null; +}; + /** * Profile view state */ @@ -373,6 +376,7 @@ export type ProfileSpecificUrlState = { tabFilter: TabID | null; legacyThreadOrder: ThreadIndex[] | null; legacyHiddenThreads: ThreadIndex[] | null; + selectedMarkers: SelectedMarkersPerThread; }; export type UrlState = {