Make it clear that the highlight will be unset when not specified.

mstange · mstange · commit 7b354c17caeb · 2026-01-19T15:41:37.000-05:00
diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts
@@ -1923,12 +1923,12 @@ export function updateBottomBoxContentsAndMaybeOpen(
   const {
     libIndex,
     sourceIndex,
-    scrollToLineNumber,
-    highlightLineNumber,
+    nativeSymbols,
     initialNativeSymbol,
+    scrollToLineNumber,
     scrollToInstructionAddress,
-    highlightInstructionAddress,
-    nativeSymbols,
+    highlightedLineNumber,
+    highlightedInstructionAddress,
   } = bottomBoxInfo;
 
   const haveSource = sourceIndex !== null;
@@ -1950,9 +1950,9 @@ export function updateBottomBoxContentsAndMaybeOpen(
     shouldOpenBottomBox,
     shouldOpenAssemblyView,
     scrollToLineNumber,
-    highlightLineNumber,
     scrollToInstructionAddress,
-    highlightInstructionAddress,
+    highlightedLineNumber,
+    highlightedInstructionAddress,
   };
 }
 
diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts
@@ -511,12 +511,14 @@ export function stateFromLocation(
     scrollGeneration: 0,
     libIndex: null,
     sourceIndex: null,
+    highlightedLine: null,
   };
   const assemblyView: AssemblyViewState = {
     isOpen: false,
     scrollGeneration: 0,
     nativeSymbols: [],
     currentNativeSymbol: null,
+    highlightedInstruction: null,
   };
   const isBottomBoxOpenPerPanel: any = {};
   tabSlugs.forEach((tabSlug) => (isBottomBoxOpenPerPanel[tabSlug] = false));
diff --git a/src/components/app/BottomBox.tsx b/src/components/app/BottomBox.tsx
@@ -53,14 +53,14 @@ type StateProps = {
   readonly sourceViewCode: SourceCodeStatus | void;
   readonly sourceViewScrollGeneration: number;
   readonly sourceViewScrollToLineNumber?: number;
-  readonly sourceViewHighlightedLine?: number;
+  readonly sourceViewHighlightedLine: number | null;
   readonly globalLineTimings: LineTimings;
   readonly assemblyViewIsOpen: boolean;
   readonly assemblyViewNativeSymbol: NativeSymbolInfo | null;
   readonly assemblyViewCode: AssemblyCodeStatus | void;
   readonly assemblyViewScrollGeneration: number;
   readonly assemblyViewScrollToInstructionAddress?: number;
-  readonly assemblyViewHighlightedInstruction?: number;
+  readonly assemblyViewHighlightedInstruction: number | null;
   readonly globalAddressTimings: AddressTimings;
 };
 
diff --git a/src/components/shared/AssemblyView.tsx b/src/components/shared/AssemblyView.tsx
@@ -46,7 +46,7 @@ type AssemblyViewProps = {
   readonly nativeSymbol: NativeSymbolInfo | null;
   readonly scrollGeneration: number;
   readonly scrollToInstructionAddress?: number;
-  readonly highlightedInstruction?: number;
+  readonly highlightedInstruction: number | null;
 };
 
 let editorModulePromise: Promise<any> | null = null;
@@ -119,7 +119,7 @@ export class AssemblyView extends React.PureComponent<AssemblyViewProps> {
       const editor = new AssemblyViewEditor(
         this._getAssemblyCodeOrFallback(),
         this.props.timings,
-        this.props.highlightedInstruction ?? null,
+        this.props.highlightedInstruction,
         domParent
       );
       this._editor = editor;
@@ -169,9 +169,7 @@ export class AssemblyView extends React.PureComponent<AssemblyViewProps> {
     if (
       this.props.highlightedInstruction !== prevProps.highlightedInstruction
     ) {
-      this._editor.setHighlightedInstruction(
-        this.props.highlightedInstruction ?? null
-      );
+      this._editor.setHighlightedInstruction(this.props.highlightedInstruction);
     }
   }
 }
diff --git a/src/components/shared/SourceView.tsx b/src/components/shared/SourceView.tsx
@@ -42,7 +42,7 @@ type SourceViewProps = {
   readonly filePath: string | null;
   readonly scrollGeneration: number;
   readonly scrollToLineNumber?: number;
-  readonly highlightedLine?: number;
+  readonly highlightedLine: number | null;
 };
 
 let editorModulePromise: Promise<any> | null = null;
@@ -109,7 +109,7 @@ export class SourceView extends React.PureComponent<SourceViewProps> {
         this._getSourceCodeOrFallback(),
         this.props.filePath,
         this.props.timings,
-        this.props.highlightedLine ?? null,
+        this.props.highlightedLine,
         domParent
       );
       this._editor = editor;
@@ -157,7 +157,7 @@ export class SourceView extends React.PureComponent<SourceViewProps> {
     }
 
     if (this.props.highlightedLine !== prevProps.highlightedLine) {
-      this._editor.setHighlightedLine(this.props.highlightedLine ?? null);
+      this._editor.setHighlightedLine(this.props.highlightedLine);
     }
   }
 }
diff --git a/src/profile-logic/bottom-box.ts b/src/profile-logic/bottom-box.ts
@@ -110,6 +110,8 @@ export function getBottomBoxInfoForCallNode(
     initialNativeSymbol,
     scrollToLineNumber: hottestLine,
     scrollToInstructionAddress: hottestInstructionAddress,
+    highlightedLineNumber: null,
+    highlightedInstructionAddress: null,
   };
 }
 
@@ -154,19 +156,21 @@ export function getBottomBoxInfoForStackFrame(
       : [];
 
   const instructionAddress =
-    nativeSymbol !== null ? frameTable.address[frameIndex] : undefined;
+    nativeSymbol !== null ? frameTable.address[frameIndex] : -1;
 
   // Extract line number from the frame
-  const lineNumber = frameTable.line[frameIndex] ?? undefined;
+  const lineNumber = frameTable.line[frameIndex];
 
   return {
     libIndex,
     sourceIndex,
     nativeSymbols: nativeSymbolInfos,
     initialNativeSymbol: 0,
-    scrollToLineNumber: lineNumber,
-    highlightLineNumber: lineNumber,
-    scrollToInstructionAddress: instructionAddress,
-    highlightInstructionAddress: instructionAddress,
+    scrollToLineNumber: lineNumber ?? undefined,
+    highlightedLineNumber: lineNumber,
+    scrollToInstructionAddress:
+      instructionAddress !== -1 ? instructionAddress : undefined,
+    highlightedInstructionAddress:
+      instructionAddress !== -1 ? instructionAddress : null,
   };
 }
diff --git a/src/reducers/url-state.ts b/src/reducers/url-state.ts
@@ -565,7 +565,12 @@ const profileName: Reducer<string | null> = (state = null, action) => {
 };
 
 const sourceView: Reducer<SourceViewState> = (
-  state = { scrollGeneration: 0, libIndex: null, sourceIndex: null },
+  state = {
+    scrollGeneration: 0,
+    libIndex: null,
+    sourceIndex: null,
+    highlightedLine: null,
+  },
   action
 ) => {
   switch (action.type) {
@@ -578,7 +583,7 @@ const sourceView: Reducer<SourceViewState> = (
         libIndex: action.libIndex,
         sourceIndex: action.sourceIndex,
         scrollToLineNumber: action.scrollToLineNumber,
-        highlightedLine: action.highlightLineNumber,
+        highlightedLine: action.highlightedLineNumber,
       };
     }
     default:
@@ -588,10 +593,11 @@ const sourceView: Reducer<SourceViewState> = (
 
 const assemblyView: Reducer<AssemblyViewState> = (
   state = {
+    isOpen: false,
     scrollGeneration: 0,
+    highlightedInstruction: null,
     nativeSymbols: [],
     currentNativeSymbol: null,
-    isOpen: false,
   },
   action
 ) => {
@@ -608,7 +614,7 @@ const assemblyView: Reducer<AssemblyViewState> = (
         currentNativeSymbol,
         isOpen: state.isOpen || shouldOpenAssemblyView,
         scrollToInstructionAddress: action.scrollToInstructionAddress,
-        highlightedInstruction: action.highlightInstructionAddress,
+        highlightedInstruction: action.highlightedInstructionAddress,
       };
     }
     case 'OPEN_ASSEMBLY_VIEW': {
diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts
@@ -78,9 +78,8 @@ export const getSourceViewScrollGeneration: Selector<number> = (state) =>
 export const getSourceViewScrollToLineNumber: Selector<number | undefined> = (
   state
 ) => getProfileSpecificState(state).sourceView.scrollToLineNumber;
-export const getSourceViewHighlightedLine: Selector<number | undefined> = (
-  state
-) => getProfileSpecificState(state).sourceView.highlightedLine;
+export const getSourceViewHighlightedLine: Selector<number | null> = (state) =>
+  getProfileSpecificState(state).sourceView.highlightedLine;
 export const getAssemblyViewIsOpen: Selector<boolean> = (state) =>
   getProfileSpecificState(state).assemblyView.isOpen;
 export const getAssemblyViewNativeSymbol: Selector<NativeSymbolInfo | null> = (
@@ -98,10 +97,9 @@ export const getAssemblyViewScrollToInstructionAddress: Selector<
   number | undefined
 > = (state) =>
   getProfileSpecificState(state).assemblyView.scrollToInstructionAddress;
-export const getAssemblyViewHighlightedInstruction: Selector<
-  number | undefined
-> = (state) =>
-  getProfileSpecificState(state).assemblyView.highlightedInstruction;
+export const getAssemblyViewHighlightedInstruction: Selector<number | null> = (
+  state
+) => getProfileSpecificState(state).assemblyView.highlightedInstruction;
 export const getShowJsTracerSummary: Selector<boolean> = (state) =>
   getProfileSpecificState(state).showJsTracerSummary;
 
diff --git a/src/test/store/bottom-box.test.ts b/src/test/store/bottom-box.test.ts
@@ -250,7 +250,7 @@ describe('bottom box', function () {
       62
     );
     expect(UrlStateSelectors.getSourceViewHighlightedLine(getState())).toBe(
-      undefined
+      null
     );
     expect(UrlStateSelectors.getAssemblyViewIsOpen(getState())).toBeFalse();
     expect(
@@ -295,7 +295,7 @@ describe('bottom box', function () {
     ).toEqual(new Map([[0x51, 1]]));
     expect(
       UrlStateSelectors.getAssemblyViewHighlightedInstruction(getState())
-    ).toBe(undefined);
+    ).toBe(null);
     expect(
       UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState())
     ).toBe(0x51);
@@ -339,7 +339,7 @@ describe('bottom box', function () {
     );
     expect(
       UrlStateSelectors.getAssemblyViewHighlightedInstruction(getState())
-    ).toBe(undefined);
+    ).toBe(null);
     expect(
       UrlStateSelectors.getAssemblyViewScrollToInstructionAddress(getState())
     ).toBeOneOf([0x40, 0x45]);
diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts
@@ -39,6 +39,7 @@ import type {
   State,
   ThreadIndex,
   IndexIntoSourceTable,
+  BottomBoxInfo,
 } from 'firefox-profiler/types';
 import getNiceProfile from './fixtures/profiles/call-nodes';
 import queryString from 'query-string';
@@ -1797,11 +1798,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi
     const sourceFile =
       'hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:5bb3e281dc9ec8a619c781d52882adb1cacf20bb';
     const sourceIndex = getSourceIndex(sourceFile);
-    const bottomBoxInfo = {
+    const bottomBoxInfo: BottomBoxInfo = {
       libIndex: 0,
       sourceIndex,
       nativeSymbols: [],
       initialNativeSymbol: null,
+      highlightedLineNumber: null,
+      highlightedInstructionAddress: null,
     };
     dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo));
     const newStore = _getStoreFromStateAfterUrlRoundtrip(getState());
@@ -1830,11 +1833,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi
     const sourceFile =
       'hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:5bb3e281dc9ec8a619c781d52882adb1cacf20bb';
     const sourceIndex = getSourceIndex(sourceFile);
-    const bottomBoxInfo = {
+    const bottomBoxInfo: BottomBoxInfo = {
       libIndex: 0,
       sourceIndex,
       nativeSymbols: [],
       initialNativeSymbol: null,
+      highlightedLineNumber: null,
+      highlightedInstructionAddress: null,
     };
     dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo));
     dispatch(closeBottomBox());
@@ -1859,11 +1864,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi
       functionSize: 14,
       functionSizeIsKnown: false,
     };
-    const bottomBoxInfo = {
+    const bottomBoxInfo: BottomBoxInfo = {
       libIndex: 0,
       sourceIndex: null,
       nativeSymbols: [nativeSymbolInfo],
       initialNativeSymbol: 0,
+      highlightedLineNumber: null,
+      highlightedInstructionAddress: null,
     };
     dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo));
     const newStore = _getStoreFromStateAfterUrlRoundtrip(getState());
@@ -1902,11 +1909,13 @@ describe('URL persistence of bottom box (source view and assembly view)', functi
       functionSize: 14,
       functionSizeIsKnown: false,
     };
-    const bottomBoxInfo = {
+    const bottomBoxInfo: BottomBoxInfo = {
       libIndex: 0,
       sourceIndex,
       nativeSymbols: [nativeSymbolInfo],
       initialNativeSymbol: 0,
+      highlightedLineNumber: null,
+      highlightedInstructionAddress: null,
     };
     dispatch(updateBottomBoxContentsAndMaybeOpen('calltree', bottomBoxInfo));
     const newStore = _getStoreFromStateAfterUrlRoundtrip(getState());
diff --git a/src/types/actions.ts b/src/types/actions.ts
@@ -344,9 +344,9 @@ type ProfileAction =
       readonly shouldOpenBottomBox: boolean;
       readonly shouldOpenAssemblyView: boolean;
       readonly scrollToLineNumber?: number;
-      readonly highlightLineNumber?: number;
       readonly scrollToInstructionAddress?: number;
-      readonly highlightInstructionAddress?: number;
+      readonly highlightedLineNumber: number | null;
+      readonly highlightedInstructionAddress: number | null;
     }
   | {
       readonly type: 'OPEN_ASSEMBLY_VIEW';
diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts
@@ -776,10 +776,13 @@ export type BottomBoxInfo = {
   sourceIndex: IndexIntoSourceTable | null;
   nativeSymbols: NativeSymbolInfo[];
   initialNativeSymbol: number | null; // index into `nativeSymbols`
+  // Optional line number + instruction address to scroll into view.
   scrollToLineNumber?: number;
-  highlightLineNumber?: number;
   scrollToInstructionAddress?: number;
-  highlightInstructionAddress?: number;
+  // Which lines / instructions to highlight (or none). This is used when clicking
+  // a stack frame in a marker stack.
+  highlightedLineNumber: number | null;
+  highlightedInstructionAddress: number | null;
 };
 
 /**
diff --git a/src/types/state.ts b/src/types/state.ts
@@ -241,6 +241,8 @@ export type ZippedProfilesState = {
 
 export type SourceViewState = {
   scrollGeneration: number;
+  // Optional line number to scroll to in the source view.
+  scrollToLineNumber?: number;
   // Non-null if this source file was opened for a function from native code.
   // In theory, multiple different libraries can have source files with the same
   // path but different content.
@@ -251,10 +253,8 @@ export type SourceViewState = {
   // Index into source table. Contains information (filename and uuid) about the
   // source. Null if a function without a file path was double clicked.
   sourceIndex: IndexIntoSourceTable | null;
-  // Optional line number to scroll to in the source view.
-  scrollToLineNumber?: number;
-  // Optional line number of the highlighted line in the source view.
-  highlightedLine?: number;
+  // The line number of the highlighted line in the source view.
+  highlightedLine: number | null;
 };
 
 export type AssemblyViewState = {
@@ -265,8 +265,8 @@ export type AssemblyViewState = {
   scrollGeneration: number;
   // Optional instruction address to scroll to in the assembly view.
   scrollToInstructionAddress?: number;
-  // Optional address of the highlighted instruction in the assembly view.
-  highlightedInstruction?: number;
+  // The address of the highlighted instruction in the assembly view.
+  highlightedInstruction: number | null;
   // The list of native symbols whose assembly code should be accessible in the
   // assembly view. Often empty or just one element, but sometimes this can have
   // multiple elements, for example the different JIT compilations of one JS function.
diff --git a/src/utils/codemirror-shared.ts b/src/utils/codemirror-shared.ts

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ type SourceViewProps = {`
`42`	`42`	`readonly filePath: string \| null;`
`43`	`43`	`readonly scrollGeneration: number;`
`44`	`44`	`readonly scrollToLineNumber?: number;`
`45`		`- readonly highlightedLine?: number;`
	`45`	`+ readonly highlightedLine: number \| null;`
`46`	`46`	`};`
`47`	`47`
`48`	`48`	`let editorModulePromise: Promise<any> \| null = null;`
`@@ -109,7 +109,7 @@ export class SourceView extends React.PureComponent<SourceViewProps> {`
`109`	`109`	`this._getSourceCodeOrFallback(),`
`110`	`110`	`this.props.filePath,`
`111`	`111`	`this.props.timings,`
`112`		`- this.props.highlightedLine ?? null,`
	`112`	`+ this.props.highlightedLine,`
`113`	`113`	`domParent`
`114`	`114`	`);`
`115`	`115`	`this._editor = editor;`
`@@ -157,7 +157,7 @@ export class SourceView extends React.PureComponent<SourceViewProps> {`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`if (this.props.highlightedLine !== prevProps.highlightedLine) {`
`160`		`- this._editor.setHighlightedLine(this.props.highlightedLine ?? null);`
	`160`	`+ this._editor.setHighlightedLine(this.props.highlightedLine);`
`161`	`161`	`}`
`162`	`162`	`}`
`163`	`163`	`}`