From a616dfc739831c49ac0c34e351fdf46354428b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunnlaugur=20=C3=9E=C3=B3r=20Briem?= Date: Thu, 4 Jan 2024 18:05:29 +0000 Subject: [PATCH 1/2] feat: support from-url profiles in compare Support profiles with dataSource `from-url` in the comparison view. This change is simply applying the suggestion in https://github.com/firefox-devtools/profiler/issues/3589#issuecomment-934257702 and removing the preceding check for dataSource being `public`, and running `yarn lint-fix`. It seems to work just fine for me when I run this project locally. I tested by pasting in two `https://profiler.firefox.com/from-url/https%253A%252F%252F` URLs that I had open in other browser tabs, and I get an apparently functioning comparison view. Closes #3589 --- src/actions/receive-profile.js | 54 +++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index 3ad846ad77..47695865c6 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -1531,33 +1531,39 @@ export function retrieveProfilesToCompare( }) ); - const hasSupportedDatasources = profileStates.every( - (state) => state.dataSource === 'public' - ); - if (!hasSupportedDatasources) { - throw new Error( - 'Only public uploaded profiles are supported by the comparison function.' - ); - } - // Then we retrieve the profiles from the online store, and unserialize // and process them if needed. - const promises = profileStates.map(async ({ hash }) => { - const profileUrl = getProfileUrlForHash(hash); - const response: ProfileOrZip = await _fetchProfile({ - url: profileUrl, - onTemporaryError: (e: TemporaryError) => { - dispatch(temporaryError(e)); - }, - }); - if (response.responseType !== 'PROFILE') { - throw new Error('Expected to receive a profile from _fetchProfile'); - } - const serializedProfile = response.profile; + const promises = profileStates.map( + async ({ dataSource, hash, profileUrl }) => { + switch (dataSource) { + case 'public': + // Use a URL from the public store. + profileUrl = getProfileUrlForHash(hash); + break; + case 'from-url': + // Use the profile URL in the decoded state, decoded from the input URL. + break; + default: + throw new Error( + 'Only public uploaded profiles are supported by the comparison function.' + ); + } + const response: ProfileOrZip = await _fetchProfile({ + url: profileUrl, + onTemporaryError: (e: TemporaryError) => { + dispatch(temporaryError(e)); + }, + }); + if (response.responseType !== 'PROFILE') { + throw new Error('Expected to receive a profile from _fetchProfile'); + } + const serializedProfile = response.profile; - const profile = unserializeProfileOfArbitraryFormat(serializedProfile); - return profile; - }); + const profile = + unserializeProfileOfArbitraryFormat(serializedProfile); + return profile; + } + ); // Once all profiles have been fetched and unserialized, we can start // pushing them to a brand new profile. This resulting profile will keep From 9c30ab7d5e13063c28997f4fb660bb1fe759c5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gunnlaugur=20=C3=9E=C3=B3r=20Briem?= Date: Mon, 8 Jan 2024 14:11:09 +0000 Subject: [PATCH 2/2] test: add case covering from-url URLs in compare --- src/test/store/receive-profile.test.js | 38 ++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/test/store/receive-profile.test.js b/src/test/store/receive-profile.test.js index df6b390eac..c5cc993d23 100644 --- a/src/test/store/receive-profile.test.js +++ b/src/test/store/receive-profile.test.js @@ -1853,6 +1853,44 @@ describe('actions/receive-profile', function () { expect(rootRange).toEqual({ start: 0, end: 9 }); }); + it('retrieves from-url profiles and puts them in the same view', async function () { + // /from-url/https%3A%2F%2Ffakeurl.com%2Ffakeprofile.json + const { profile1, profile2, resultProfile, globalTracks, rootRange } = + await setup(getSomeProfiles(), { + url1: 'https://fakeurl.com/from-url/https%3A%2F%2Ffakeurl.com%2Ffakeprofile1.json?thread=0', + url2: 'https://fakeurl.com/from-url/https%3A%2F%2Ffakeurl.com%2Ffakeprofile2.json?thread=0', + }); + + const expectedThreads = [ + { + ...profile1.threads[0], + pid: '0 from profile 1', + tid: '0 from profile 1', + isMainThread: true, + processName: 'Profile 1: Empty', + unregisterTime: getTimeRangeForThread(profile1.threads[0], 1).end, + }, + { + ...profile2.threads[0], + pid: '0 from profile 2', + tid: '0 from profile 2', + isMainThread: true, + processName: 'Profile 2: Empty', + unregisterTime: getTimeRangeForThread(profile2.threads[0], 1).end, + }, + // comparison thread + expect.objectContaining({ + processType: 'comparison', + pid: 'Diff between 1 and 2', + name: 'Diff between 1 and 2', + }), + ]; + + expect(resultProfile.threads).toEqual(expectedThreads); + expect(globalTracks).toHaveLength(3); // each thread + comparison track + expect(rootRange).toEqual({ start: 0, end: 9 }); + }); + it('expands the URL if needed', async function () { const { shortUrl1, shortUrl2, globalTracks, rootRange } = await setupWithShortUrl(getSomeProfiles(), {