diff --git a/src/actions/publish.js b/src/actions/publish.js index f7bb1f020c..4e2756965f 100644 --- a/src/actions/publish.js +++ b/src/actions/publish.js @@ -18,26 +18,43 @@ import { getDataSource, getProfileNameForStorage, getUrlPredictor, + getHiddenGlobalTracks, + getHiddenLocalTracksByPid, + getGlobalTrackOrder, + getLocalTrackOrderByPid, + getTimelineTrackOrganization, } from 'firefox-profiler/selectors/url-state'; import { getProfile, getZeroAt, getCommittedRange, getProfileFilterPageData, + getGlobalTracks, + getLocalTracksByPid, } from 'firefox-profiler/selectors/profile'; -import { viewProfile } from './receive-profile'; import { ensureExists } from 'firefox-profiler/utils/flow'; import { extractProfileTokenFromJwt } from 'firefox-profiler/utils/jwt'; -import { withHistoryReplaceStateSync } from 'firefox-profiler/app-logic/url-handling'; import { persistUploadedProfileInformationToDb } from 'firefox-profiler/app-logic/uploaded-profiles-db'; +import { + computeGlobalTracks, + computeLocalTracksByPid, + computeOldTrackIndexToNewTrackIndexMap, + computeHiddenTracksAfterSanitization, + computeTrackOrderAfterSanitization, +} from 'firefox-profiler/profile-logic/tracks'; import type { Action, ThunkAction, CheckedSharingOptions, + GlobalTrack, + LocalTrack, + Pid, + Profile, StartEndRange, ThreadIndex, State, + TrackIndex, } from 'firefox-profiler/types'; export function toggleCheckedSharingOptions( @@ -92,7 +109,7 @@ export function uploadFailed(error: mixed): Action { async function persistJustUploadedProfileInformationToDb( profileToken: string, jwtToken: string | null, - sanitizedInformation, + finalPublishAction: Action, prepublishedState: State ): Promise { if (process.env.NODE_ENV === 'test' && !window.indexedDB) { @@ -119,34 +136,11 @@ async function persistJustUploadedProfileInformationToDb( // happens before the sanitization really happens in the main state, so we // need to simulate it. const urlPredictor = getUrlPredictor(prepublishedState); - let predictedUrl; - - const removeProfileInformation = - getRemoveProfileInformation(prepublishedState); - if (removeProfileInformation) { - // In case you wonder, committedRanges is either an empty array (if the - // range was sanitized) or `null` (otherwise). - const { committedRanges, oldThreadIndexToNew } = sanitizedInformation; - - // Predicts the URL we'll have after local sanitization. - predictedUrl = urlPredictor( - profileSanitized( - profileToken, - committedRanges, - oldThreadIndexToNew, - profileName, - null /* prepublished State */ - ) - ); - } else { - // Predicts the URL we'll have after the process is finished. - predictedUrl = urlPredictor( - profilePublished(profileToken, profileName, null /* prepublished State */) - ); - } - + const predictedUrl = urlPredictor(finalPublishAction); const profileMeta = getProfile(prepublishedState).meta; const profileFilterPageData = getProfileFilterPageData(prepublishedState); + const removeProfileInformation = + getRemoveProfileInformation(prepublishedState); try { await persistUploadedProfileInformationToDb({ @@ -267,6 +261,126 @@ export function attemptToPublish(): ThunkAction> { }); const hash = extractProfileTokenFromJwt(hashOrToken); + const newGlobalTracks = computeGlobalTracks(sanitizedInformation.profile); + const newLocalTracksByPid = computeLocalTracksByPid( + sanitizedInformation.profile + ); + + // These old states could still be valid, or we may need to recompute them + // if we remove some tracks. + let hiddenGlobalTracks = getHiddenGlobalTracks(prePublishedState); + let hiddenLocalTracksByPid = getHiddenLocalTracksByPid(prePublishedState); + let globalTrackOrder = getGlobalTrackOrder(prePublishedState); + let localTrackOrderByPid = getLocalTrackOrderByPid(prePublishedState); + + const timelineTrackOrganization = + getTimelineTrackOrganization(prePublishedState); + if ( + timelineTrackOrganization.type === 'full' && + sanitizedInformation.oldThreadIndexToNew + ) { + // Some threads were removed, this means we need to compute the new hidden + // tracks and track order information. + // oldThreadIndexToNew contains the old thread indexes, but here we're + // talking about track indexes and doesn't contain only threads but also + // other types of tracks. Therefore we need to find and match the tracks + // that aren't threads. + const oldGlobalTracks = getGlobalTracks(prePublishedState); + const oldLocalTracksByPid = getLocalTracksByPid(prePublishedState); + + const globalOldTrackIndexToNewTrackIndex = + computeOldTrackIndexToNewTrackIndexMap({ + oldTracks: oldGlobalTracks, + newTracks: newGlobalTracks, + }); + + hiddenGlobalTracks = computeHiddenTracksAfterSanitization({ + oldHiddenTracks: hiddenGlobalTracks, + oldTrackIndexToNewTrackIndex: globalOldTrackIndexToNewTrackIndex, + }); + globalTrackOrder = computeTrackOrderAfterSanitization({ + oldTrackOrder: globalTrackOrder, + oldTrackIndexToNewTrackIndex: globalOldTrackIndexToNewTrackIndex, + }); + + const newHiddenLocalTracksByPid = new Map(); + const newLocalTrackOrderByPid = new Map(); + + for (const [pid, newLocalTracks] of newLocalTracksByPid.entries()) { + const oldHiddenLocalTracks = hiddenLocalTracksByPid.get(pid); + const oldLocalTrackOrder = localTrackOrderByPid.get(pid); + if ( + oldHiddenLocalTracks === undefined && + oldLocalTrackOrder === undefined + ) { + // Nothing to do for this pid, let's move to the next. + continue; + } + + // Compute the map for this PID. + const oldTrackIndexToNewTrackIndex = + computeOldTrackIndexToNewTrackIndexMap({ + oldTracks: oldLocalTracksByPid.get(pid) ?? [], + newTracks: newLocalTracks, + }); + + // Handle the hidden local tracks for this PID + if (oldHiddenLocalTracks !== undefined) { + const newHiddenLocalTracks = computeHiddenTracksAfterSanitization({ + oldHiddenTracks: oldHiddenLocalTracks, + oldTrackIndexToNewTrackIndex, + }); + newHiddenLocalTracksByPid.set(pid, newHiddenLocalTracks); + } + + // Handle the local track order for this PID + if (oldLocalTrackOrder !== undefined) { + const newLocalTrackOrder = computeTrackOrderAfterSanitization({ + oldTrackOrder: oldLocalTrackOrder, + oldTrackIndexToNewTrackIndex, + }); + + newLocalTrackOrderByPid.set(pid, newLocalTrackOrder); + } + } + hiddenLocalTracksByPid = newHiddenLocalTracksByPid; + localTrackOrderByPid = newLocalTrackOrderByPid; + } + + let finalPublishAction; + if (sanitizedInformation.isSanitized) { + // In case you wonder, committedRanges is either an empty array (if the + // range was sanitized) or `null` (otherwise). + const { committedRanges, oldThreadIndexToNew, profile } = + sanitizedInformation; + + finalPublishAction = profileSanitized( + profile, + hash, + committedRanges, + oldThreadIndexToNew, + profileName, + prePublishedState, + newGlobalTracks, + newLocalTracksByPid, + hiddenGlobalTracks, + globalTrackOrder, + hiddenLocalTracksByPid, + localTrackOrderByPid + ); + } else { + const dataSource = getDataSource(prePublishedState); + const isUnpublished = + dataSource === 'unpublished' || dataSource === 'from-browser'; + finalPublishAction = profilePublished( + hash, + profileName, + // Only include the pre-published state if we want to be able to revert + // the profile. If we are viewing from-browser, then it's only a single + // profile. + isUnpublished ? null : prePublishedState + ); + } // Because we want to store the published profile even when the upload // generation changed, we store the data here, before the state is fully @@ -275,7 +389,7 @@ export function attemptToPublish(): ThunkAction> { await persistJustUploadedProfileInformationToDb( hash, hashOrToken === hash ? null : hashOrToken, - sanitizedInformation, + finalPublishAction, prePublishedState ); @@ -286,63 +400,13 @@ export function attemptToPublish(): ThunkAction> { return false; } - const removeProfileInformation = - getRemoveProfileInformation(prePublishedState); - if (removeProfileInformation) { - const { committedRanges, oldThreadIndexToNew, profile } = - sanitizedInformation; + if (sanitizedInformation.isSanitized) { // Hide the old UI gracefully. await dispatch(hideStaleProfile()); - - // Update the UrlState so that we are sanitized. - dispatch( - profileSanitized( - hash, - committedRanges, - oldThreadIndexToNew, - profileName, - prePublishedState - ) - ); - - // At this moment, we don't have the profile data in state anymore, - // because the action profileSanitized will reset all the state of - // profile-view, including the profile data. In the future we may want - // to fix this (for example move the profile data in another reducer, or - // keep the profile state when resetting the state). - // This still works because it also sets the "phase" state to - // "TRANSITIONING_FROM_STALE_PROFILE" that avoids rendering anything - // (see AppViewRouter). - // viewProfile below needs to synchronously dispatch the new profile - // again so that the user doesn't see a glitch. This is still most - // probably a performance problem because all components are unmounted - // and mounted again. - - // Swap out the URL state, since the view profile calculates all of the default - // settings. If we don't do this then we can go back in history to where we - // are trying to view a profile without valid view settings. - withHistoryReplaceStateSync(() => { - // Multiple dispatches are usually to be avoided, but viewProfile requires - // the next UrlState in place. It could be rewritten to have a UrlState passed - // in as a paremeter, but that doesn't seem worth it at the time of this writing. - dispatch(viewProfile(profile)); - }); - } else { - const dataSource = getDataSource(prePublishedState); - const isUnpublished = - dataSource === 'unpublished' || dataSource === 'from-browser'; - dispatch( - profilePublished( - hash, - profileName, - // Only include the pre-published state if we want to be able to revert - // the profile. If we are viewing from-browser, then it's only a single - // profile. - isUnpublished ? null : prePublishedState - ) - ); } + dispatch(finalPublishAction); + sendAnalytics({ hitType: 'event', eventCategory: 'profile upload', @@ -382,19 +446,33 @@ export function resetUploadState(): Action { * indexes or information that has been sanitized away. */ export function profileSanitized( + profile: Profile, hash: string, committedRanges: StartEndRange[] | null, oldThreadIndexToNew: Map | null, profileName: string, - prePublishedState: State | null + prePublishedState: State | null, + globalTracks: GlobalTrack[], + localTracksByPid: Map, + hiddenGlobalTracks: Set, + globalTrackOrder: TrackIndex[], + hiddenLocalTracksByPid: Map>, + localTrackOrderByPid: Map ): Action { return { type: 'SANITIZED_PROFILE_PUBLISHED', + profile, hash, committedRanges, oldThreadIndexToNew, profileName, prePublishedState, + globalTracks, + localTracksByPid, + hiddenGlobalTracks, + globalTrackOrder, + hiddenLocalTracksByPid, + localTrackOrderByPid, }; } diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 38d3b0d4f7..18788f6fc4 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -11,6 +11,7 @@ import type { Pid, GlobalTrack, LocalTrack, + Track, TrackIndex, Counter, Tid, @@ -713,6 +714,154 @@ export function getVisibleThreads( return visibleThreads; } +/** + * This computes a map to translate old track indexes to the new track indexes + * we have after sanitization. Then it allows quick computation for hidden + * tracks and track orders. + * This function is used for both global and local track lists. + */ +export function computeOldTrackIndexToNewTrackIndexMap({ + oldTracks, + newTracks, +}: {| + +oldTracks: $ReadOnlyArray, + +newTracks: $ReadOnlyArray, +|}): Map { + const trackToNewTrackIndex = new Map(); + for ( + let newTrackIndex = 0; + newTrackIndex < newTracks.length; + newTrackIndex++ + ) { + const track = newTracks[newTrackIndex]; + let newTrackIndexMapForType = trackToNewTrackIndex.get(track.type); + if (newTrackIndexMapForType === undefined) { + newTrackIndexMapForType = new Map(); + trackToNewTrackIndex.set(track.type, newTrackIndexMapForType); + } + switch (track.type) { + /* Global tracks */ + case 'process': + newTrackIndexMapForType.set(track.pid, newTrackIndex); + break; + case 'screenshots': + newTrackIndexMapForType.set(track.id, newTrackIndex); + break; + case 'visual-progress': + case 'perceptual-visual-progress': + case 'contentful-visual-progress': + newTrackIndexMapForType.set(null, newTrackIndex); + break; + + /* Local tracks */ + case 'thread': + case 'network': + case 'ipc': + case 'event-delay': + newTrackIndexMapForType.set(track.threadIndex, newTrackIndex); + break; + case 'memory': + case 'process-cpu': + newTrackIndexMapForType.set(track.counterIndex, newTrackIndex); + break; + + default: + throw new Error(`Unknown track type ${track.type}`); + } + } + + const oldTrackIndexToNewTrackIndex = new Map(); + for ( + let oldTrackIndex = 0; + oldTrackIndex < oldTracks.length; + oldTrackIndex++ + ) { + const track = oldTracks[oldTrackIndex]; + const newTrackIndexMapForType = + trackToNewTrackIndex.get(track.type) ?? new Map(); + let newTrackIndex; + switch (track.type) { + /* Global tracks */ + case 'process': + newTrackIndex = newTrackIndexMapForType.get(track.pid); + break; + case 'screenshots': + newTrackIndex = newTrackIndexMapForType.get(track.id); + break; + case 'visual-progress': + case 'perceptual-visual-progress': + case 'contentful-visual-progress': + newTrackIndex = newTrackIndexMapForType.get(null); + break; + + /* Local tracks */ + case 'thread': + case 'network': + case 'ipc': + case 'event-delay': + newTrackIndex = newTrackIndexMapForType.get(track.threadIndex); + break; + case 'memory': + case 'process-cpu': + newTrackIndex = newTrackIndexMapForType.get(track.counterIndex); + break; + default: + throw new Error(`Unknown track type ${track.type}`); + } + + if (newTrackIndex !== undefined) { + oldTrackIndexToNewTrackIndex.set(oldTrackIndex, newTrackIndex); + } + } + + return oldTrackIndexToNewTrackIndex; +} + +/** + * Compute the new state for hidden tracks after some of them have been + * removed from the profile. + * This is used for both global and local tracks. + */ +export function computeHiddenTracksAfterSanitization({ + oldHiddenTracks, + oldTrackIndexToNewTrackIndex, +}: {| + +oldHiddenTracks: Set, + +oldTrackIndexToNewTrackIndex: Map, +|}): Set { + const newHiddenTracks = new Set(); + for (const oldHiddenTrackIndex of oldHiddenTracks) { + const newHiddenTrackIndex = + oldTrackIndexToNewTrackIndex.get(oldHiddenTrackIndex); + if (newHiddenTrackIndex !== undefined) { + newHiddenTracks.add(newHiddenTrackIndex); + } + } + + return newHiddenTracks; +} + +/** + * Compute the new state for track orders after sanitization. + * This is used for both global and local track lists. + */ +export function computeTrackOrderAfterSanitization({ + oldTrackOrder, + oldTrackIndexToNewTrackIndex, +}: {| + +oldTrackOrder: TrackIndex[], + +oldTrackIndexToNewTrackIndex: Map, +|}): TrackIndex[] { + const newTrackOrder = []; + for (const oldTrackIndex of oldTrackOrder) { + const newTrackIndex = oldTrackIndexToNewTrackIndex.get(oldTrackIndex); + if (newTrackIndex !== undefined) { + newTrackOrder.push(newTrackIndex); + } + } + return newTrackOrder; +} + export function getGlobalTrackName( globalTrack: GlobalTrack, threads: Thread[] diff --git a/src/reducers/app.js b/src/reducers/app.js index 852b124e00..dcedf60bca 100644 --- a/src/reducers/app.js +++ b/src/reducers/app.js @@ -41,9 +41,6 @@ const view: Reducer = ( return { phase: 'INITIALIZING' }; case 'ROUTE_NOT_FOUND': return { phase: 'ROUTE_NOT_FOUND' }; - case 'REVERT_TO_PRE_PUBLISHED_STATE': - case 'SANITIZED_PROFILE_PUBLISHED': - return { phase: 'TRANSITIONING_FROM_STALE_PROFILE' }; case 'PROFILE_LOADED': return { phase: 'PROFILE_LOADED' }; case 'DATA_RELOAD': diff --git a/src/reducers/profile-view.js b/src/reducers/profile-view.js index f97f7359cb..45374c2975 100644 --- a/src/reducers/profile-view.js +++ b/src/reducers/profile-view.js @@ -39,6 +39,7 @@ import { objectMap } from '../utils/flow'; const profile: Reducer = (state = null, action) => { switch (action.type) { + case 'SANITIZED_PROFILE_PUBLISHED': case 'PROFILE_LOADED': return action.profile; case 'BULK_SYMBOLICATION': { @@ -81,6 +82,7 @@ const profile: Reducer = (state = null, action) => { const globalTracks: Reducer = (state = [], action) => { switch (action.type) { case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': return action.globalTracks; default: return state; @@ -97,6 +99,7 @@ const localTracksByPid: Reducer> = ( ) => { switch (action.type) { case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': case 'ENABLE_EVENT_DELAY_TRACKS': case 'ENABLE_EXPERIMENTAL_PROCESS_CPU_TRACKS': return action.localTracksByPid; @@ -637,8 +640,6 @@ const wrapReducerInResetter = ( ): Reducer => { return (state, action) => { switch (action.type) { - case 'SANITIZED_PROFILE_PUBLISHED': - case 'REVERT_TO_PRE_PUBLISHED_STATE': case 'RETURN_TO_ZIP_FILE_LIST': // Provide a mechanism to wipe the state clean when changing out profiles. // All of the profile view information is invalidated. diff --git a/src/reducers/publish.js b/src/reducers/publish.js index 6c7f30fea5..368cc9190f 100644 --- a/src/reducers/publish.js +++ b/src/reducers/publish.js @@ -175,8 +175,6 @@ const prePublishedState: Reducer = (state = null, action) => { case 'SANITIZED_PROFILE_PUBLISHED': case 'PROFILE_PUBLISHED': return action.prePublishedState; - case 'REVERT_TO_PRE_PUBLISHED_STATE': - return null; default: return state; } @@ -194,6 +192,7 @@ const isHidingStaleProfile: Reducer = (state = false, action) => { case 'HIDE_STALE_PROFILE': return true; case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': case 'VIEW_ORIGINS_PROFILE': case 'VIEW_ACTIVE_TAB_PROFILE': return false; @@ -210,6 +209,9 @@ const hasSanitizedProfile: Reducer = (state = false, action) => { switch (action.type) { case 'HIDE_STALE_PROFILE': return true; + case 'SANITIZED_PROFILE_PUBLISHED': + case 'REVERT_TO_PRE_PUBLISHED_STATE': + return false; default: return state; } diff --git a/src/reducers/url-state.js b/src/reducers/url-state.js index 2c5de977a2..f3815483a6 100644 --- a/src/reducers/url-state.js +++ b/src/reducers/url-state.js @@ -321,12 +321,9 @@ const showJsTracerSummary: Reducer = (state = false, action) => { const globalTrackOrder: Reducer = (state = [], action) => { switch (action.type) { case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': case 'CHANGE_GLOBAL_TRACK_ORDER': return action.globalTrackOrder; - case 'SANITIZED_PROFILE_PUBLISHED': - // If some threads were removed, do not even attempt to figure this out. It's - // complicated, and not many people use this feature. - return action.oldThreadIndexToNew ? [] : state; default: return state; } @@ -338,6 +335,7 @@ const hiddenGlobalTracks: Reducer> = ( ) => { switch (action.type) { case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': case 'ISOLATE_LOCAL_TRACK': case 'ISOLATE_PROCESS': case 'ISOLATE_PROCESS_MAIN_THREAD': @@ -364,10 +362,6 @@ const hiddenGlobalTracks: Reducer> = ( hiddenGlobalTracks.delete(action.trackIndex); return hiddenGlobalTracks; } - case 'SANITIZED_PROFILE_PUBLISHED': - // If any threads were removed, this was because they were hidden. - // Reset this state. - return action.oldThreadIndexToNew ? new Set() : state; default: return state; } @@ -379,6 +373,7 @@ const hiddenLocalTracksByPid: Reducer>> = ( ) => { switch (action.type) { case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': return action.hiddenLocalTracksByPid; case 'HIDE_LOCAL_TRACK': { const hiddenLocalTracksByPid = new Map(state); @@ -425,9 +420,6 @@ const hiddenLocalTracksByPid: Reducer>> = ( hiddenLocalTracksByPid.set(action.pid, action.hiddenLocalTracks); return hiddenLocalTracksByPid; } - case 'SANITIZED_PROFILE_PUBLISHED': - // If any threads were removed then this information is no longer valid. - return action.oldThreadIndexToNew ? new Map() : state; default: return state; } @@ -439,6 +431,7 @@ const localTrackOrderByPid: Reducer> = ( ) => { switch (action.type) { case 'VIEW_FULL_PROFILE': + case 'SANITIZED_PROFILE_PUBLISHED': case 'ENABLE_EVENT_DELAY_TRACKS': case 'ENABLE_EXPERIMENTAL_PROCESS_CPU_TRACKS': return action.localTrackOrderByPid; @@ -447,10 +440,6 @@ const localTrackOrderByPid: Reducer> = ( localTrackOrderByPid.set(action.pid, action.localTrackOrder); return localTrackOrderByPid; } - case 'SANITIZED_PROFILE_PUBLISHED': - // If any threads were removed then remove this information. It's complicated - // to compute, and not many people use it. - return action.oldThreadIndexToNew ? new Map() : state; default: return state; } diff --git a/src/types/actions.js b/src/types/actions.js index 2b345a7aa7..230691ae6d 100644 --- a/src/types/actions.js +++ b/src/types/actions.js @@ -463,7 +463,14 @@ type UrlStateAction = +committedRanges: StartEndRange[] | null, +oldThreadIndexToNew: Map | null, +profileName: string, + +profile: Profile, +prePublishedState: State | null, + +globalTracks: GlobalTrack[], + +localTracksByPid: Map, + +hiddenGlobalTracks: Set, + +globalTrackOrder: TrackIndex[], + +hiddenLocalTracksByPid: Map>, + +localTrackOrderByPid: Map, |} | {| +type: 'SET_DATA_SOURCE', diff --git a/src/types/state.js b/src/types/state.js index 6ad52ed30f..d43cc097ad 100644 --- a/src/types/state.js +++ b/src/types/state.js @@ -115,7 +115,6 @@ export type ProfileViewState = { export type AppViewState = | {| +phase: 'ROUTE_NOT_FOUND' |} - | {| +phase: 'TRANSITIONING_FROM_STALE_PROFILE' |} | {| +phase: 'PROFILE_LOADED' |} | {| +phase: 'DATA_LOADED' |} | {| +phase: 'DATA_RELOAD' |}