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/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 () { 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 = {