From 30cb8407e51476c18477f778ba125e432edc201d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= Date: Tue, 12 Sep 2023 16:13:08 +0200 Subject: [PATCH] Avoid showing the calltree panel in profiles without samples. --- src/actions/receive-profile.js | 29 ++++++++++++++++++- src/components/timeline/TrackThread.js | 5 +++- src/reducers/url-state.js | 1 + .../components/ThreadActivityGraph.test.js | 12 ++++++++ src/test/store/profile-view.test.js | 6 ++-- src/types/actions.js | 1 + 6 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index 214c38a453..0e4c2d0687 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -38,12 +38,14 @@ import { getActiveTabID, getMarkerSchemaByName, } from 'firefox-profiler/selectors'; +import { getSelectedTab } from 'firefox-profiler/selectors/url-state'; import { withHistoryReplaceStateAsync, withHistoryReplaceStateSync, stateFromLocation, ensureIsValidDataSource, } from 'firefox-profiler/app-logic/url-handling'; +import { tabsShowingSampleData } from 'firefox-profiler/app-logic/tabs-handling'; import { initializeLocalTrackOrderByPid, computeLocalTracksByPid, @@ -59,7 +61,10 @@ import { computeActiveTabTracks } from 'firefox-profiler/profile-logic/active-ta import { setDataSource } from './profile-view'; import { fatalError } from './errors'; import { GOOGLE_STORAGE_BUCKET } from 'firefox-profiler/app-logic/constants'; -import { determineTimelineType } from 'firefox-profiler/profile-logic/profile-data'; +import { + determineTimelineType, + hasUsefulSamples, +} from 'firefox-profiler/profile-logic/profile-data'; import type { RequestedLib, @@ -345,10 +350,32 @@ export function finalizeFullProfileView( timelineType = determineTimelineType(profile); } + // If the currently selected tab is only visible when the selected track + // has samples, verify that the selected track has samples, and if not + // select the marker chart. + let selectedTab = getSelectedTab(getState()); + if (tabsShowingSampleData.includes(selectedTab)) { + let hasSamples = false; + for (const threadIndex of selectedThreadIndexes) { + const thread = profile.threads[threadIndex]; + const { samples, jsAllocations, nativeAllocations } = thread; + hasSamples = [samples, jsAllocations, nativeAllocations].some((table) => + hasUsefulSamples(table, thread) + ); + if (hasSamples) { + break; + } + } + if (!hasSamples) { + selectedTab = 'marker-chart'; + } + } + withHistoryReplaceStateSync(() => { dispatch({ type: 'VIEW_FULL_PROFILE', selectedThreadIndexes, + selectedTab, globalTracks, globalTrackOrder, localTracksByPid, diff --git a/src/components/timeline/TrackThread.js b/src/components/timeline/TrackThread.js index f9c82fb4dc..2f0e4dfd09 100644 --- a/src/components/timeline/TrackThread.js +++ b/src/components/timeline/TrackThread.js @@ -99,6 +99,7 @@ type StateProps = {| +isExperimentalCPUGraphsEnabled: boolean, +maxThreadCPUDeltaPerMs: number, +implementationFilter: ImplementationFilter, + +callTreeVisible: boolean, |}; type DispatchProps = {| @@ -137,6 +138,7 @@ class TimelineTrackThreadImpl extends PureComponent { focusCallTree, invertCallstack, selectedThreadIndexes, + callTreeVisible, } = this.props; // Sample clicking only works for one thread. See issue #2709 @@ -149,7 +151,7 @@ class TimelineTrackThreadImpl extends PureComponent { selectLeafCallNode(threadsKey, sampleIndex); } - if (sampleIndex !== null) { + if (sampleIndex !== null && callTreeVisible) { // If the user clicked outside of the activity graph (sampleIndex === null), // then we don't need to focus the call tree. This action also selects // the call tree panel, which we don't want either in this case. @@ -396,6 +398,7 @@ export const TimelineTrackThread = explicitConnect< isExperimentalCPUGraphsEnabled: getIsExperimentalCPUGraphsEnabled(state), maxThreadCPUDeltaPerMs: getMaxThreadCPUDeltaPerMs(state), implementationFilter: getImplementationFilter(state), + callTreeVisible: selectors.getUsefulTabs(state).includes('calltree'), }; }, mapDispatchToProps: { diff --git a/src/reducers/url-state.js b/src/reducers/url-state.js index e6b18f44f2..d08d0b906e 100644 --- a/src/reducers/url-state.js +++ b/src/reducers/url-state.js @@ -95,6 +95,7 @@ const selectedTab: Reducer = (state = 'calltree', action) => { switch (action.type) { case 'CHANGE_SELECTED_TAB': case 'SELECT_TRACK': + case 'VIEW_FULL_PROFILE': return action.selectedTab; case 'FOCUS_CALL_TREE': return 'calltree'; diff --git a/src/test/components/ThreadActivityGraph.test.js b/src/test/components/ThreadActivityGraph.test.js index 86656bbe77..c9b4683de3 100644 --- a/src/test/components/ThreadActivityGraph.test.js +++ b/src/test/components/ThreadActivityGraph.test.js @@ -266,6 +266,7 @@ describe('ThreadActivityGraph', function () { it('when clicking a stack, this selects the call tree panel', function () { const { dispatch, getState, clickActivityGraph } = setup(); + expect(getSelectedTab(getState())).toBe('calltree'); dispatch(changeSelectedTab('marker-chart')); // The full call node at this sample is: @@ -278,6 +279,7 @@ describe('ThreadActivityGraph', function () { it(`when clicking outside of the graph, this doesn't select the call tree panel`, function () { const { dispatch, getState, clickActivityGraph } = setup(); + expect(getSelectedTab(getState())).toBe('calltree'); dispatch(changeSelectedTab('marker-chart')); // There's no sample at this location. @@ -286,6 +288,16 @@ describe('ThreadActivityGraph', function () { expect(getLastVisibleThreadTabSlug(getState())).toBe('marker-chart'); }); + it("when clicking a sample in a track with only '(root)' samples, this doesn't select the hidden call tree panel", function () { + const { profile } = getProfileFromTextSamples('(root)'); + const { getState, clickActivityGraph } = setup(profile); + + expect(getSelectedTab(getState())).toBe('marker-chart'); + + clickActivityGraph(1, 0.2); + expect(getSelectedTab(getState())).toBe('marker-chart'); + }); + it('will redraw even when there are no samples in range', function () { const { dispatch } = setup(); flushDrawLog(); diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index 69affaaeb1..f4a5ada9d8 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -416,14 +416,16 @@ describe('actions/ProfileView', function () { pid: '0', }; - it('starts out with the thread track and call tree selected', function () { + it('starts out with the thread track and marker chart selected', function () { const profile = getNetworkTrackProfile(); const { getState } = storeWithProfile(profile); expect(UrlStateSelectors.getSelectedThreadIndexes(getState())).toEqual( new Set([0]) ); + // The profile contains only markers, so the default tab is the + // marker-chart rather than the calltree. expect(UrlStateSelectors.getSelectedTab(getState())).toEqual( - 'calltree' + 'marker-chart' ); }); diff --git a/src/types/actions.js b/src/types/actions.js index c3a881b96a..c066c51376 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -407,6 +407,7 @@ type ReceiveProfileAction = +hiddenLocalTracksByPid: Map>, +localTrackOrderByPid: Map, +timelineType: TimelineType | null, + +selectedTab: TabSlug, |} | {| +type: 'VIEW_ORIGINS_PROFILE',