Translate URL track-index state through profile sanitization#6000
Translate URL track-index state through profile sanitization#6000fatadel wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6000 +/- ##
==========================================
+ Coverage 83.77% 83.80% +0.02%
==========================================
Files 329 329
Lines 34423 34540 +117
Branches 9627 9649 +22
==========================================
+ Hits 28839 28946 +107
- Misses 5155 5165 +10
Partials 429 429 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Thanks for the PR, it looks like it's working mostly well! I found one more case where publishing makes some tracks reappear again. For example use this deploy preview:
Then click "re-upload", uncheck "Include hidden threads" and then upload it. Notice that firefox.com content process' network track appears again.
Note that I selected profiler.firefox.com origin in the top left corner in the tab selector. I think this is happening because the indexes for the tab selector are not translated (probably tabToThreadIndexesMap).
Previously, hidden non-thread tracks (screenshots, network, IPC) reappeared after publishing a sanitized profile when sanitization also removed a thread: the URL state's hidden-track sets and track-order were reset whenever threads were re-indexed, even though the non-thread tracks still existed in the sanitized profile. attemptToPublish now builds an old to new TrackIndex map by matching tracks on stable identity (pid, screenshot id, threadIndex, counterIndex, visual-progress singleton) and translates hiddenGlobalTracks, globalTrackOrder, hiddenLocalTracksByPid, and localTrackOrderByPid through it on SANITIZED_PROFILE_PUBLISHED. Marker tracks key on a string-table index that sanitization reshuffles, so they are not matched. Fixes firefox-devtools#2947
When a tab filter was active during publishing, a hidden non-thread track (e.g. a screenshot or a per-process network track) could reappear after sanitization. attemptToPublish was remapping the URL state's hidden-track set against a global track list filtered using the prePublishedState's tabToThreadIndexesMap; that map's Set<ThreadIndex> values referred to old thread indexes, so when sanitization shifted surviving threads down by removing an earlier thread, filterGlobalTracksByTab picked the wrong threads and the hidden track ended up at a stale TrackIndex. attemptToPublish now builds the map from the sanitized profile's threads and pages so its ThreadIndex values are valid in the new track-index space. The innerWindowID to tabID computation is extracted into a shared computeInnerWindowIDToTabMap helper that the getInnerWindowIDToTabMap selector also uses.
Thanks! Fixed now. |
2 participants
Previously, hidden non-thread tracks (screenshots, network, IPC) reappeared after publishing a sanitized profile when sanitization also removed a thread: the URL state's hidden-track sets and track-order were reset whenever threads were re-indexed, even though the non-thread tracks still existed in the sanitized profile. When a tab filter was active, the bug also affected tracks in surviving processes because the
tabToThreadIndexesMapused to filter the new global track list still referenced pre-sanitization thread indexes.attemptToPublishnow builds an old-to-newTrackIndexmap by matching tracks on stable identity (pid, screenshot id, threadIndex, counterIndex, visual-progress singleton) and translateshiddenGlobalTracks,globalTrackOrder,hiddenLocalTracksByPid, andlocalTrackOrderByPidthrough it onSANITIZED_PROFILE_PUBLISHED. ThetabToThreadIndexesMapis rebuilt from the sanitized profile's threads and pages so itsThreadIndexvalues are valid in the new track-index space; theinnerWindowID → tabIDcomputation is extracted into a sharedcomputeInnerWindowIDToTabMaphelper that the existinggetInnerWindowIDToTabMapselector also uses. Marker tracks key on a string-table index that sanitization reshuffles, so they are not matched.Fixes #2947.
The work is built upon the draft PR by @julienw.
STR are available in the original issues, here is the deploy preview profile.