diff --git a/src/profile-logic/active-tab.js b/src/profile-logic/active-tab.js index e4142acedb..f350437f8b 100644 --- a/src/profile-logic/active-tab.js +++ b/src/profile-logic/active-tab.js @@ -84,11 +84,17 @@ export function computeActiveTabTracks( mainTrackIndexes.push(threadIndex); } else { if (!isTabFilteredThreadEmpty(threadIndex, state)) { - resources.push({ - type: 'sub-frame', - threadIndex, - name: _getActiveTabResourceName(thread, innerWindowIDToPageMap), - }); + const resourceName = _getActiveTabResourceName( + thread, + innerWindowIDToPageMap + ); + if (resourceName !== null) { + resources.push({ + type: 'sub-frame', + threadIndex, + name: resourceName, + }); + } } } } else { @@ -96,11 +102,17 @@ export function computeActiveTabTracks( // track. Find out if that thread contains the active tab data, and add it // as a resource track if it does. if (!isTabFilteredThreadEmpty(threadIndex, state)) { - resources.push({ - type: 'thread', - threadIndex, - name: _getActiveTabResourceName(thread, innerWindowIDToPageMap), - }); + const resourceName = _getActiveTabResourceName( + thread, + innerWindowIDToPageMap + ); + if (resourceName !== null) { + resources.push({ + type: 'thread', + threadIndex, + name: resourceName, + }); + } } } @@ -189,11 +201,12 @@ function isTopmostThread( /** * Returns the name of active tab resource track. * It can be either a sub-frame or a regular thread. + * If it fails to find a name, returns null. */ function _getActiveTabResourceName( thread: Thread, innerWindowIDToPageMap: Map -): string { +): string | null { if (thread.isMainThread) { // This is a sub-frame. // Get the first innerWindowID inside the thread that's also present of innerWindowIDToPageMap. @@ -216,6 +229,16 @@ function _getActiveTabResourceName( // find them, so we can get the real resource name. const page = innerWindowIDToPageMap.get(data.innerWindowID); return ( + // During a page load, iframes usually start with an about:blank page + // and then they navigate to the url. While it's in about:blank, we + // might catch some markers. We would like to exclude them because + // `about:blank` name doesn't bring any value as a resource name and + // we would prefer to display the _real_ url instead which is + // captured after the about:blank. + // FIXME: I think we initially added `about:newtab` here to exclude + // the "new tab page -> website" navigation from showing the + // `about:newtab` all the time. But I don't think we have to worry + // about this for iframes. So maybe remove this? page && page.url !== 'about:blank' && page.url !== 'about:newtab' ); } @@ -225,9 +248,15 @@ function _getActiveTabResourceName( firstInnerWindowID = markerData.innerWindowID; } } + if (firstInnerWindowID === undefined || firstInnerWindowID === null) { - throw new Error('There must be an innerWindowID in the thread'); + // Since we excluded the about:blank and about:newtab pages, we might fail + // to find a valid innerWindowID. This might happen when an ad blocker + // blocks an iframe and therefore it fails to load. In that case, we + // should return null. + return null; } + const page = ensureExists(innerWindowIDToPageMap.get(firstInnerWindowID)); return page.url; } diff --git a/src/test/fixtures/profiles/tracks.js b/src/test/fixtures/profiles/tracks.js index 2829255f2e..c14ffee4ed 100644 --- a/src/test/fixtures/profiles/tracks.js +++ b/src/test/fixtures/profiles/tracks.js @@ -337,6 +337,7 @@ export function getStoreWithMemoryTrack(pid: Pid = '222') { */ export function getHumanReadableActiveTabTracks(state: State): string[] { const globalTracks = profileViewSelectors.getActiveTabGlobalTracks(state); + const resourceTracks = profileViewSelectors.getActiveTabResourceTracks(state); const selectedThreadIndexes = urlStateSelectors.getSelectedThreadIndexes(state); const text: string[] = []; @@ -369,6 +370,22 @@ export function getHumanReadableActiveTabTracks(state: State): string[] { } } + for (const resourceTrack of resourceTracks) { + switch (resourceTrack.type) { + case 'sub-frame': + text.push(` - iframe: ${resourceTrack.name}`); + break; + case 'thread': + text.push(` - ${resourceTrack.name}`); + break; + default: + throw assertExhaustiveCheck( + resourceTrack, + 'Unhandled ActiveTabResourceTrack.' + ); + } + } + return text; } diff --git a/src/test/store/active-tab.test.js b/src/test/store/active-tab.test.js index 8a5268f7b6..753956ca8f 100644 --- a/src/test/store/active-tab.test.js +++ b/src/test/store/active-tab.test.js @@ -26,12 +26,21 @@ import { getTimelineType, } from '../../selectors/url-state'; import { stateFromLocation } from '../../app-logic/url-handling'; +import { ensureExists } from '../../utils/flow'; import type { Profile } from 'firefox-profiler/types'; describe('ActiveTab', function () { - function setup(p = getProfileWithNiceTracks(), addInnerWindowID = true) { - const { profile, ...pageInfo } = addActiveTabInformationToProfile(p); + function setup( + p = getProfileWithNiceTracks(), + addInnerWindowID = true, + pageInfo = null + ) { + let profile = p; + if (!pageInfo) { + ({ profile, ...pageInfo } = addActiveTabInformationToProfile(profile)); + } + // Add the innerWindowIDs so we can compute the first thread as main track. if (addInnerWindowID) { profile.threads[0].frameTable.innerWindowID[0] = @@ -163,6 +172,62 @@ describe('ActiveTab', function () { ); const { getState } = setup(profile, false); + expect(getHumanReadableActiveTabTracks(getState())).toEqual([ + 'main track [tab] SELECTED', + ' - iframe: Page #2', + ]); + }); + + it('do not show the thread when it fails to find the iframe resource name', function () { + const { + profile: p, + funcNamesDictPerThread: [firstThread], + } = getProfileFromTextSamples( + // First thread is the main thread of the first tab (which is the + // active tab) + `A`, + // The second thread is an iframe of the first thread. + `B` + ); + + p.threads[0].name = 'GeckoMain'; + p.threads[0].isMainThread = true; + p.threads[1].name = 'GeckoMain'; + p.threads[1].isMainThread = true; + + const { profile, ...pageInfo } = addActiveTabInformationToProfile(p); + addInnerWindowIdToStacks( + profile.threads[0], + /* listOfOperations */ + [ + // first tab is the active tab. + { + innerWindowID: pageInfo.firstTabInnerWindowIDs[0], + callNodes: [[firstThread.A]], + }, + ] + ); + addMarkersToThreadWithCorrespondingSamples(profile.threads[1], [ + // All about:blank or about:newtab markers are ignored during the + // track name computation because they don't provide the correct innerWindowID. + // This thread SHOULD NOT be shown in the tracks. + [ + 'This marker will be filtered', + 1, + 2, + { + type: 'tracing', + category: 'Navigation', + innerWindowID: pageInfo.iframeInnerWindowIDsWithChild, + }, + ], + ]); + + // Lastly, we need to put the iframe innerWindowID url to about:blank to test this case. + ensureExists(profile.pages)[1].url = 'about:blank'; + + const { getState } = setup(profile, false, pageInfo); + expect(getHumanReadableActiveTabTracks(getState())).toEqual([ 'main track [tab] SELECTED', ]);