Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/profile-logic/transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 37 additions & 0 deletions src/reducers/profile-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export const defaultThreadViewOptions: ThreadViewOptions = {
expandedInvertedCallNodePaths: new PathSet(),
selectedMarker: null,
selectedNetworkMarker: null,
lastSeenTransformCount: 0,
};

function _getThreadViewOptions(
Expand Down Expand Up @@ -378,11 +379,15 @@ const viewOptionsPerThread: Reducer<ThreadViewOptionsPerThreads> = (
threadViewOptions.expandedInvertedCallNodePaths
);

const lastSeenTransformCount =
threadViewOptions.lastSeenTransformCount + 1;

return _updateThreadViewOptions(state, threadsKey, {
selectedNonInvertedCallNodePath,
selectedInvertedCallNodePath,
expandedNonInvertedCallNodePaths,
expandedInvertedCallNodePaths,
lastSeenTransformCount,
});
}
case 'POP_TRANSFORMS_FROM_STACK': {
Expand All @@ -394,6 +399,38 @@ const viewOptionsPerThread: Reducer<ThreadViewOptionsPerThreads> = (
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': {
Expand Down
1 change: 1 addition & 0 deletions src/test/store/__snapshots__/profile-view.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4393,6 +4393,7 @@ Object {
],
},
},
"lastSeenTransformCount": 1,
"selectedInvertedCallNodePath": Array [],
"selectedMarker": 1,
"selectedNetworkMarker": null,
Expand Down
66 changes: 66 additions & 0 deletions src/test/store/transforms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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 () {
Expand Down
4 changes: 4 additions & 0 deletions src/types/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down