From 9bcfa3e6f1a5dc25b1bf761c7cdf7bd2e74259a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 30 Mar 2022 13:52:24 +0200 Subject: [PATCH 1/3] Move the IPC tracks right below their threads --- src/profile-logic/tracks.js | 36 +++++++++++++++++++++---- src/test/components/MarkerTable.test.js | 16 +++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index c6d16011ef..9eaa1d2fe1 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -61,8 +61,12 @@ const LOCAL_TRACK_INDEX_ORDER = { const LOCAL_TRACK_DISPLAY_ORDER = { network: 0, memory: 1, - thread: 2, - ipc: 3, + // IPC tracks that belong to the global track will appear right after network + // and memory tracks. But we want to show the IPC tracks that belong to the + // local threads right after their track. This special handling happens inside + // the sort function. + ipc: 2, + thread: 3, 'event-delay': 4, 'process-cpu': 5, }; @@ -85,11 +89,33 @@ const GLOBAL_TRACK_DISPLAY_ORDER = { function _getDefaultLocalTrackOrder(tracks: LocalTrack[]) { const trackOrder = tracks.map((_, index) => index); // In place sort! - trackOrder.sort( - (a, b) => + trackOrder.sort((a, b) => { + if ( + tracks[a].type === 'thread' && + tracks[b].type === 'ipc' && + tracks[a].threadIndex === tracks[b].threadIndex + ) { + // If the IPC track belongs to that local thread, put the IPC tracks right + // after it. + return -1; + } + + if ( + tracks[a].type === 'ipc' && + tracks[b].type === 'thread' && + tracks[a].threadIndex === tracks[b].threadIndex + ) { + // If the IPC track belongs to that local thread, put the IPC tracks right + // after it. + return 1; + } + + return ( LOCAL_TRACK_DISPLAY_ORDER[tracks[a].type] - LOCAL_TRACK_DISPLAY_ORDER[tracks[b].type] - ); + ); + }); + return trackOrder; } diff --git a/src/test/components/MarkerTable.test.js b/src/test/components/MarkerTable.test.js index bbb86ca73f..58fc1952ba 100644 --- a/src/test/components/MarkerTable.test.js +++ b/src/test/components/MarkerTable.test.js @@ -295,10 +295,10 @@ describe('MarkerTable', function () { 'hide [thread GeckoMain process]', ' - show [ipc GeckoMain]', 'show [thread GeckoMain tab] SELECTED', - ' - show [thread DOM Worker]', - ' - show [thread Style]', ' - show [ipc GeckoMain] SELECTED', + ' - show [thread DOM Worker]', ' - show [ipc DOM Worker]', + ' - show [thread Style]', ' - show [ipc Style]', ]); @@ -313,10 +313,10 @@ describe('MarkerTable', function () { 'show [thread GeckoMain process] SELECTED', ' - show [ipc GeckoMain] SELECTED', 'show [thread GeckoMain tab]', - ' - show [thread DOM Worker]', - ' - show [thread Style]', ' - show [ipc GeckoMain]', + ' - show [thread DOM Worker]', ' - show [ipc DOM Worker]', + ' - show [thread Style]', ' - show [ipc Style]', ]); }); @@ -353,10 +353,10 @@ describe('MarkerTable', function () { 'show [thread GeckoMain process] SELECTED', ' - show [ipc GeckoMain] SELECTED', 'hide [thread GeckoMain tab]', - ' - hide [thread DOM Worker]', - ' - show [thread Style]', ' - show [ipc GeckoMain]', + ' - hide [thread DOM Worker]', ' - show [ipc DOM Worker]', + ' - show [thread Style]', ' - show [ipc Style]', ]); @@ -371,10 +371,10 @@ describe('MarkerTable', function () { 'show [thread GeckoMain process]', ' - show [ipc GeckoMain]', 'show [thread GeckoMain tab]', - ' - show [thread DOM Worker] SELECTED', - ' - show [thread Style]', ' - show [ipc GeckoMain]', + ' - show [thread DOM Worker] SELECTED', ' - show [ipc DOM Worker] SELECTED', + ' - show [thread Style]', ' - show [ipc Style]', ]); }); From c133938ce82097f88225cd7df6b4aed4274737a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 30 Mar 2022 14:02:22 +0200 Subject: [PATCH 2/3] Fix a typo --- src/test/store/tracks.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/store/tracks.test.js b/src/test/store/tracks.test.js index 59485208b0..5a8c387cde 100644 --- a/src/test/store/tracks.test.js +++ b/src/test/store/tracks.test.js @@ -351,7 +351,7 @@ describe('ordering and hiding', function () { ); }); - it('creates a separate user-facing ordering that is different from the internal sortiong', function () { + it('creates a separate user-facing ordering that is different from the internal sorting', function () { const { globalTracks, globalTrackOrder } = setup(); expect( globalTrackOrder.map((trackIndex) => globalTracks[trackIndex].type) @@ -623,7 +623,7 @@ describe('ordering and hiding', function () { ); }); - it('creates a separate user-facing ordering that is different from the internal sortiong', function () { + it('creates a separate user-facing ordering that is different from the internal sorting', function () { const { localTracks, localTrackOrder } = setup(); expect( localTrackOrder.map((trackIndex) => localTracks[trackIndex].type) From a32bb0e56e1a777e506646b7a1cf2ac48244843e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 30 Mar 2022 14:19:38 +0200 Subject: [PATCH 3/3] Add a test for the new IPC special case for track ordering --- src/test/store/tracks.test.js | 44 +++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/test/store/tracks.test.js b/src/test/store/tracks.test.js index 5a8c387cde..02094508c2 100644 --- a/src/test/store/tracks.test.js +++ b/src/test/store/tracks.test.js @@ -5,6 +5,7 @@ import { getScreenshotTrackProfile, getNetworkTrackProfile, + addIPCMarkerPairToThreads, } from '../fixtures/profiles/processed-profile'; import { getEmptyThread } from '../../profile-logic/data-structures'; import { storeWithProfile } from '../fixtures/stores'; @@ -630,6 +631,49 @@ describe('ordering and hiding', function () { ).toEqual(userFacingSortOrder); }); }); + + it('properly initializes the sorting with for the IPC tracks', function () { + // IPC tracks have a special case for the track ordering. They always + // appear after the thread tracks that they belong to. If they belong to + // the global track, then it should show up before the other local tracks. + // If it belongs to a local track, it should appear right after the local + // thread track. + const profile = getProfileWithNiceTracks(); + const { pid } = profile.threads[1]; + addIPCMarkerPairToThreads( + { + startTime: 1, + endTime: 10, + messageSeqno: 1, + }, + profile.threads[1], // tab process + profile.threads[2] // DOM Worker + ); + const { getState } = storeWithProfile(profile); + const localTracks = ProfileViewSelectors.getLocalTracks(getState(), pid); + const localTrackOrder = UrlStateSelectors.getLocalTrackOrder( + getState(), + pid + ); + + // Check that we properly put the two IPC tracks right after their threads. + // Since the first IPC track belongs to the global track, it should appear + // first, and the second IPC track should appear after the DOM Worker track. + expect( + localTrackOrder.map((trackIndex) => localTracks[trackIndex].type) + ).toEqual(['ipc', 'thread', 'ipc', 'thread']); + + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain process]', + 'show [thread GeckoMain tab] SELECTED', + // Belongs to the global tab track. + ' - show [ipc GeckoMain] SELECTED', + ' - show [thread DOM Worker]', + // Belongs to the DOM Worker local track. + ' - show [ipc DOM Worker]', + ' - show [thread Style]', + ]); + }); }); });