From 1efa24b4150744090307424d034a8235e4152ed1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 27 Nov 2025 01:58:21 +0100 Subject: [PATCH 1/2] Clear selected and expanded call node paths on browser back button if it removes transforms When using the browser back button after applying a focus-category transform, re-applying the transform would fail with the error: "We couldn't find a node with prefix -1 and func X, this shouldn't happen." The root cause was that expanded call node paths from the transformed tree remained in state when navigating back via the browser. These paths were invalid in the untransformed tree, and they were causing errors when the transform was re-applied. The fix adds a lastSeenTransformCount field to ThreadViewOptions to track the number of transforms applied to each thread. When UPDATE_URL_STATE fires via browser back button, we compare the new transform count with the old count. If we notice that a transform was removed, we reset all call node paths to ensure they're always valid for the current tree structure. This matches the existing behavior of POP_TRANSFORMS_FROM_STACK (removing the transform from the filter navigator bar), which already resets paths correctly. --- src/profile-logic/transforms.ts | 2 +- src/reducers/profile-view.ts | 37 +++++++++++++++++++ .../__snapshots__/profile-view.test.ts.snap | 1 + src/types/state.ts | 4 ++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/profile-logic/transforms.ts b/src/profile-logic/transforms.ts index 47f6001661..db277bf14e 100644 --- a/src/profile-logic/transforms.ts +++ b/src/profile-logic/transforms.ts @@ -600,7 +600,7 @@ function _dropFunctionInCallNodePath( return callNodePath.includes(funcIndex) ? [] : callNodePath; } -// removes all functions that are not in the category from the callNodePath +// Removes all functions that are not in the category from the callNodePath function _removeOtherCategoryFunctionsInNodePathWithFunction( category: IndexIntoCategoryList, callNodePath: CallNodePath, diff --git a/src/reducers/profile-view.ts b/src/reducers/profile-view.ts index 6039d1f810..26465510fd 100644 --- a/src/reducers/profile-view.ts +++ b/src/reducers/profile-view.ts @@ -139,6 +139,7 @@ export const defaultThreadViewOptions: ThreadViewOptions = { expandedInvertedCallNodePaths: new PathSet(), selectedMarker: null, selectedNetworkMarker: null, + lastSeenTransformCount: 0, }; function _getThreadViewOptions( @@ -378,11 +379,15 @@ const viewOptionsPerThread: Reducer = ( threadViewOptions.expandedInvertedCallNodePaths ); + const lastSeenTransformCount = + threadViewOptions.lastSeenTransformCount + 1; + return _updateThreadViewOptions(state, threadsKey, { selectedNonInvertedCallNodePath, selectedInvertedCallNodePath, expandedNonInvertedCallNodePaths, expandedInvertedCallNodePaths, + lastSeenTransformCount, }); } case 'POP_TRANSFORMS_FROM_STACK': { @@ -394,6 +399,38 @@ const viewOptionsPerThread: Reducer = ( selectedInvertedCallNodePath: [], expandedNonInvertedCallNodePaths: new PathSet(), expandedInvertedCallNodePaths: new PathSet(), + lastSeenTransformCount: 0, + }); + } + case 'UPDATE_URL_STATE': { + // When the URL state changes (e.g., via browser back button), check if the + // transform stack has been popped for each thread. If so, reset the stored paths + // because they may reference call nodes that only exist in a transformed tree. + // See: https://github.com/firefox-devtools/profiler/issues/5689 + if (!action.newUrlState) { + return state; + } + + const { transforms } = action.newUrlState.profileSpecific; + return objectMap(state, (viewOptions, threadsKey) => { + const transformStack = transforms[threadsKey] || []; + const newTransformCount = transformStack.length; + const oldTransformCount = viewOptions.lastSeenTransformCount; + + // If transform count changed, reset the paths. + if (newTransformCount < oldTransformCount) { + return { + ...viewOptions, + selectedNonInvertedCallNodePath: [], + selectedInvertedCallNodePath: [], + expandedNonInvertedCallNodePaths: new PathSet(), + expandedInvertedCallNodePaths: new PathSet(), + lastSeenTransformCount: newTransformCount, + }; + } + + // No change needed. + return viewOptions; }); } case 'CHANGE_IMPLEMENTATION_FILTER': { diff --git a/src/test/store/__snapshots__/profile-view.test.ts.snap b/src/test/store/__snapshots__/profile-view.test.ts.snap index a8c4c9ec9e..a0d8a23ea5 100644 --- a/src/test/store/__snapshots__/profile-view.test.ts.snap +++ b/src/test/store/__snapshots__/profile-view.test.ts.snap @@ -4393,6 +4393,7 @@ Object { ], }, }, + "lastSeenTransformCount": 1, "selectedInvertedCallNodePath": Array [], "selectedMarker": 1, "selectedNetworkMarker": null, diff --git a/src/types/state.ts b/src/types/state.ts index 54c83a32bd..aa6956090a 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -58,6 +58,10 @@ export type ThreadViewOptions = { 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 + // in the new transform state. + readonly lastSeenTransformCount: number; }; export type ThreadViewOptionsPerThreads = { From 27f4f533159a6bc68d6cd46e7a8a818aa487b15a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 27 Nov 2025 12:38:44 +0100 Subject: [PATCH 2/2] Add a test for the navigation back behavior when the transforms are removed --- src/test/store/transforms.test.ts | 66 +++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/test/store/transforms.test.ts b/src/test/store/transforms.test.ts index c646b311ec..8265804d96 100644 --- a/src/test/store/transforms.test.ts +++ b/src/test/store/transforms.test.ts @@ -25,6 +25,8 @@ import { changeSelectedCallNode, changeCallTreeSummaryStrategy, } from '../../actions/profile-view'; +import * as AppActions from '../../actions/app'; +import * as UrlStateSelectors from '../../selectors/url-state'; import { selectedThreadSelectors } from '../../selectors/per-thread'; describe('"focus-subtree" transform', function () { @@ -819,6 +821,70 @@ describe('"focus-category" transform', function () { expect(selectedCallNodePath).toEqual([A, A]); }); }); + + describe('browser back button behavior', function () { + // This test ensures that when using browser back button after applying + // a focus-category transform, re-applying the transform doesn't fail. + // The bug occurred because expanded paths from the transformed tree + // weren't being reset when the URL state changed via browser navigation. + const { threadIndex, categoryIndex, funcNamesDict, getState, dispatch } = + setup(` + A[cat:Other] A[cat:Other] + B[cat:Other] B[cat:Other] + C[cat:Graphics] C[cat:Graphics] + D[cat:Graphics] D[cat:Graphics] + E[cat:Graphics] F[cat:Graphics] + `); + + it('can re-apply transform after browser back without error', function () { + const { C, D, E } = funcNamesDict; + + // Apply focus-category transform. + dispatch( + addTransformToStack(threadIndex, { + type: 'focus-category', + category: categoryIndex, + }) + ); + + // Select a deep node in the transformed tree, which expands paths. + // In the transformed tree, C becomes a root since A and B are filtered out. + dispatch(changeSelectedCallNode(threadIndex, [C, D, E])); + + expect( + selectedThreadSelectors.getSelectedCallNodePath(getState()) + ).toEqual([C, D, E]); + + // Capture the current URL state with transforms. + const urlStateWithTransforms = UrlStateSelectors.getUrlState(getState()); + + // Simulate browser back button by creating a URL state without transforms. + const urlStateWithoutTransforms = { + ...urlStateWithTransforms, + profileSpecific: { + ...urlStateWithTransforms.profileSpecific, + transforms: {}, + }, + }; + + // Apply the URL state change (simulating browser back). + dispatch(AppActions.updateUrlState(urlStateWithoutTransforms)); + + // Re-apply the same transform. This should not throw an error. + expect(() => { + dispatch( + addTransformToStack(threadIndex, { + type: 'focus-category', + category: categoryIndex, + }) + ); + }).not.toThrow(); + + expect( + selectedThreadSelectors.getSelectedCallNodePath(getState()) + ).toEqual([]); + }); + }); }); describe('"collapse-resource" transform', function () {