Skip to content

Commit c059a5a

Browse files
committed
Add priorityHeader field to network requests
2 parents af28bc2 + 27f4f53 commit c059a5a

File tree

7 files changed

+116
-1
lines changed

7 files changed

+116
-1
lines changed

src/components/tooltip/NetworkMarker.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,12 @@ export function getNetworkMarkerDetails(
308308
</TooltipDetail>
309309
);
310310

311+
details.push(
312+
<TooltipDetail label="PriorityHeader" key="Network-Priority-Header">
313+
{payload.priorityHeader}
314+
</TooltipDetail>
315+
);
316+
311317
if (mimeType) {
312318
details.push(
313319
<TooltipDetail label={mimeTypeLabel} key={'Network-' + mimeTypeLabel}>

src/profile-logic/transforms.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ function _dropFunctionInCallNodePath(
600600
return callNodePath.includes(funcIndex) ? [] : callNodePath;
601601
}
602602

603-
// removes all functions that are not in the category from the callNodePath
603+
// Removes all functions that are not in the category from the callNodePath
604604
function _removeOtherCategoryFunctionsInNodePathWithFunction(
605605
category: IndexIntoCategoryList,
606606
callNodePath: CallNodePath,

src/reducers/profile-view.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ export const defaultThreadViewOptions: ThreadViewOptions = {
139139
expandedInvertedCallNodePaths: new PathSet(),
140140
selectedMarker: null,
141141
selectedNetworkMarker: null,
142+
lastSeenTransformCount: 0,
142143
};
143144

144145
function _getThreadViewOptions(
@@ -378,11 +379,15 @@ const viewOptionsPerThread: Reducer<ThreadViewOptionsPerThreads> = (
378379
threadViewOptions.expandedInvertedCallNodePaths
379380
);
380381

382+
const lastSeenTransformCount =
383+
threadViewOptions.lastSeenTransformCount + 1;
384+
381385
return _updateThreadViewOptions(state, threadsKey, {
382386
selectedNonInvertedCallNodePath,
383387
selectedInvertedCallNodePath,
384388
expandedNonInvertedCallNodePaths,
385389
expandedInvertedCallNodePaths,
390+
lastSeenTransformCount,
386391
});
387392
}
388393
case 'POP_TRANSFORMS_FROM_STACK': {
@@ -394,6 +399,38 @@ const viewOptionsPerThread: Reducer<ThreadViewOptionsPerThreads> = (
394399
selectedInvertedCallNodePath: [],
395400
expandedNonInvertedCallNodePaths: new PathSet(),
396401
expandedInvertedCallNodePaths: new PathSet(),
402+
lastSeenTransformCount: 0,
403+
});
404+
}
405+
case 'UPDATE_URL_STATE': {
406+
// When the URL state changes (e.g., via browser back button), check if the
407+
// transform stack has been popped for each thread. If so, reset the stored paths
408+
// because they may reference call nodes that only exist in a transformed tree.
409+
// See: https://github.com/firefox-devtools/profiler/issues/5689
410+
if (!action.newUrlState) {
411+
return state;
412+
}
413+
414+
const { transforms } = action.newUrlState.profileSpecific;
415+
return objectMap(state, (viewOptions, threadsKey) => {
416+
const transformStack = transforms[threadsKey] || [];
417+
const newTransformCount = transformStack.length;
418+
const oldTransformCount = viewOptions.lastSeenTransformCount;
419+
420+
// If transform count changed, reset the paths.
421+
if (newTransformCount < oldTransformCount) {
422+
return {
423+
...viewOptions,
424+
selectedNonInvertedCallNodePath: [],
425+
selectedInvertedCallNodePath: [],
426+
expandedNonInvertedCallNodePaths: new PathSet(),
427+
expandedInvertedCallNodePaths: new PathSet(),
428+
lastSeenTransformCount: newTransformCount,
429+
};
430+
}
431+
432+
// No change needed.
433+
return viewOptions;
397434
});
398435
}
399436
case 'CHANGE_IMPLEMENTATION_FILTER': {

src/test/store/__snapshots__/profile-view.test.ts.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4393,6 +4393,7 @@ Object {
43934393
],
43944394
},
43954395
},
4396+
"lastSeenTransformCount": 1,
43964397
"selectedInvertedCallNodePath": Array [],
43974398
"selectedMarker": 1,
43984399
"selectedNetworkMarker": null,

src/test/store/transforms.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import {
2525
changeSelectedCallNode,
2626
changeCallTreeSummaryStrategy,
2727
} from '../../actions/profile-view';
28+
import * as AppActions from '../../actions/app';
29+
import * as UrlStateSelectors from '../../selectors/url-state';
2830
import { selectedThreadSelectors } from '../../selectors/per-thread';
2931

3032
describe('"focus-subtree" transform', function () {
@@ -819,6 +821,70 @@ describe('"focus-category" transform', function () {
819821
expect(selectedCallNodePath).toEqual([A, A]);
820822
});
821823
});
824+
825+
describe('browser back button behavior', function () {
826+
// This test ensures that when using browser back button after applying
827+
// a focus-category transform, re-applying the transform doesn't fail.
828+
// The bug occurred because expanded paths from the transformed tree
829+
// weren't being reset when the URL state changed via browser navigation.
830+
const { threadIndex, categoryIndex, funcNamesDict, getState, dispatch } =
831+
setup(`
832+
A[cat:Other] A[cat:Other]
833+
B[cat:Other] B[cat:Other]
834+
C[cat:Graphics] C[cat:Graphics]
835+
D[cat:Graphics] D[cat:Graphics]
836+
E[cat:Graphics] F[cat:Graphics]
837+
`);
838+
839+
it('can re-apply transform after browser back without error', function () {
840+
const { C, D, E } = funcNamesDict;
841+
842+
// Apply focus-category transform.
843+
dispatch(
844+
addTransformToStack(threadIndex, {
845+
type: 'focus-category',
846+
category: categoryIndex,
847+
})
848+
);
849+
850+
// Select a deep node in the transformed tree, which expands paths.
851+
// In the transformed tree, C becomes a root since A and B are filtered out.
852+
dispatch(changeSelectedCallNode(threadIndex, [C, D, E]));
853+
854+
expect(
855+
selectedThreadSelectors.getSelectedCallNodePath(getState())
856+
).toEqual([C, D, E]);
857+
858+
// Capture the current URL state with transforms.
859+
const urlStateWithTransforms = UrlStateSelectors.getUrlState(getState());
860+
861+
// Simulate browser back button by creating a URL state without transforms.
862+
const urlStateWithoutTransforms = {
863+
...urlStateWithTransforms,
864+
profileSpecific: {
865+
...urlStateWithTransforms.profileSpecific,
866+
transforms: {},
867+
},
868+
};
869+
870+
// Apply the URL state change (simulating browser back).
871+
dispatch(AppActions.updateUrlState(urlStateWithoutTransforms));
872+
873+
// Re-apply the same transform. This should not throw an error.
874+
expect(() => {
875+
dispatch(
876+
addTransformToStack(threadIndex, {
877+
type: 'focus-category',
878+
category: categoryIndex,
879+
})
880+
);
881+
}).not.toThrow();
882+
883+
expect(
884+
selectedThreadSelectors.getSelectedCallNodePath(getState())
885+
).toEqual([]);
886+
});
887+
});
822888
});
823889

824890
describe('"collapse-resource" transform', function () {

src/types/markers.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ export type NetworkPayload = {
500500
RedirectURI?: string;
501501
id: number;
502502
pri: number; // priority of the load; always included as it can change
503+
priorityHeader?: string; // priority header from HTTP request
503504
count?: number; // Total size of transfer, if any
504505
// See all possible values in tools/profiler/core/platform.cpp
505506
status: NetworkStatus;

src/types/state.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ export type ThreadViewOptions = {
5858
readonly expandedInvertedCallNodePaths: PathSet;
5959
readonly selectedMarker: MarkerIndex | null;
6060
readonly selectedNetworkMarker: MarkerIndex | null;
61+
// Track the number of transforms to detect when they change via browser
62+
// navigation. This helps us know when to reset paths that may be invalid
63+
// in the new transform state.
64+
readonly lastSeenTransformCount: number;
6165
};
6266

6367
export type ThreadViewOptionsPerThreads = {

0 commit comments

Comments
 (0)