From f755ee4a3a843e220622e01b1c5006362f58f78a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:38:03 +0200 Subject: [PATCH 01/15] Clear the source code cache when another profile is loaded The cache is keyed by source index, which only means something for the profile it was filled for. Loading another profile left the old entries around. Publishing a sanitized profile also ends with viewProfile, so the cache is now cleared there too. --- src/reducers/code.ts | 19 +++---------------- src/test/unit/source-code-cache.test.ts | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/reducers/code.ts b/src/reducers/code.ts index a6e2a7c928..e1c5e16c07 100644 --- a/src/reducers/code.ts +++ b/src/reducers/code.ts @@ -46,22 +46,9 @@ const sourceCodeCache: Reducer> = ( newState.set(sourceIndex, { type: 'ERROR', errors }); return newState; } - case 'SANITIZED_PROFILE_PUBLISHED': { - if (!action.translationMaps) { - return state; - } - const { oldSourceToNewSourcePlusOne } = action.translationMaps; - const newState = new Map(); - for (const [sourceIndex, status] of state) { - const newSourceIndexPlusOne = oldSourceToNewSourcePlusOne[sourceIndex]; - if (newSourceIndexPlusOne === 0) { - continue; - } - const newSourceIndex = newSourceIndexPlusOne - 1; - newState.set(newSourceIndex, status); - } - return newState; - } + case 'PROFILE_LOADED': + // The source indexes only mean something for the profile they came from. + return new Map(); default: return state; } diff --git a/src/test/unit/source-code-cache.test.ts b/src/test/unit/source-code-cache.test.ts index 26100e117f..6d539f6f48 100644 --- a/src/test/unit/source-code-cache.test.ts +++ b/src/test/unit/source-code-cache.test.ts @@ -12,6 +12,8 @@ import { finishLoadingSourceCode, failLoadingSourceCode, } from '../../actions/code'; +import { viewProfile } from '../../actions/receive-profile'; +import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; import type { IndexIntoSourceTable } from 'firefox-profiler/types'; @@ -130,6 +132,18 @@ describe('source code cache with IndexIntoSourceTable', function () { expect(result).toBeInstanceOf(Map); expect(result.size).toBe(0); }); + + it('forgets the cache when another profile is loaded', async function () { + const { getState, dispatch } = setupStoreWithSourceIndex(null); + dispatch(finishLoadingSourceCode(0, 'console.log("Source 0");')); + expect(getSourceCodeCache(getState()).size).toBe(1); + + // The indexes of the old cache would point at unrelated files in the new + // profile, so the whole cache has to go. + const { profile } = getProfileFromTextSamples('A[file:other.js]'); + await dispatch(viewProfile(profile)); + expect(getSourceCodeCache(getState()).size).toBe(0); + }); }); describe('state transitions', function () { From 8df717d3641b93c2877df007a25b0bc95612ee05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:39:01 +0200 Subject: [PATCH 02/15] Move source fetching into a fetchSourceCode thunk SourceCodeFetcher set up the communication delegate, called fetchSource and dispatched the outcome itself, which kept the logic out of reach of any other code that wants a file in the source code cache. Move it into a thunk. The component now only decides when to fetch. No behavior change. --- src/actions/code.ts | 59 +++++++++++++++ src/components/app/SourceCodeFetcher.tsx | 92 ++---------------------- 2 files changed, 65 insertions(+), 86 deletions(-) diff --git a/src/actions/code.ts b/src/actions/code.ts index aa235e2d1e..2a00b91538 100644 --- a/src/actions/code.ts +++ b/src/actions/code.ts @@ -1,8 +1,19 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import { getProfile } from 'firefox-profiler/selectors/profile'; +import { getBrowserConnection } from 'firefox-profiler/selectors/app'; +import { getSymbolServerUrl } from 'firefox-profiler/selectors/url-state'; +import { + fetchSource, + type FetchSourceResult, +} from 'firefox-profiler/utils/fetch-source'; +import { RegularExternalCommunicationDelegate } from 'firefox-profiler/utils/query-api'; +import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; +import { findAddressProofForFile } from 'firefox-profiler/profile-logic/profile-data'; import type { Action, + ThunkAction, SourceCodeLoadingError, ApiQueryError, DecodedInstruction, @@ -36,6 +47,54 @@ export function failLoadingSourceCode( return { type: 'SOURCE_CODE_LOADING_ERROR', sourceIndex, errors }; } +/** + * Fetch the contents of a source file into the source code cache, from the + * browser, a symbol server or the web, whichever can serve it. + * + * This always fetches, so callers need to skip sources that are already in the + * cache or embedded in the profile. Callers should keep one archiveCache across + * their fetches, so that each archive is only downloaded once. + */ +export function fetchSourceCode( + sourceIndex: IndexIntoSourceTable, + archiveCache: Map> +): ThunkAction> { + return async (dispatch, getState) => { + const state = getState(); + const { sources, stringArray } = getProfile(state).shared; + const delegate = new RegularExternalCommunicationDelegate( + getBrowserConnection(state), + { + onBeginUrlRequest: (url) => { + dispatch(beginLoadingSourceCodeFromUrl(sourceIndex, url)); + }, + onBeginBrowserConnectionQuery: () => { + dispatch(beginLoadingSourceCodeFromBrowserConnection(sourceIndex)); + }, + } + ); + const result = await fetchSource( + stringArray[sources.filename[sourceIndex]], + sources.id[sourceIndex], + getSymbolServerUrl(state), + findAddressProofForFile(getProfile(state), sourceIndex), + archiveCache, + delegate + ); + switch (result.type) { + case 'SUCCESS': + dispatch(finishLoadingSourceCode(sourceIndex, result.source)); + break; + case 'ERROR': + dispatch(failLoadingSourceCode(sourceIndex, result.errors)); + break; + default: + throw assertExhaustiveCheck(result); + } + return result; + }; +} + export function beginLoadingAssemblyCodeFromUrl( nativeSymbolKey: string, url: string diff --git a/src/components/app/SourceCodeFetcher.tsx b/src/components/app/SourceCodeFetcher.tsx index 26b8444fae..707c707c9c 100644 --- a/src/components/app/SourceCodeFetcher.tsx +++ b/src/components/app/SourceCodeFetcher.tsx @@ -5,49 +5,27 @@ import React from 'react'; import { - getProfileOrNull, getSourceViewCode, - getBrowserConnection, - getSymbolServerUrl, getSourceViewFile, getSourceViewSourceIndex, - getSourceViewSourceId, } from 'firefox-profiler/selectors'; -import { - beginLoadingSourceCodeFromUrl, - beginLoadingSourceCodeFromBrowserConnection, - finishLoadingSourceCode, - failLoadingSourceCode, -} from 'firefox-profiler/actions/code'; -import { fetchSource } from 'firefox-profiler/utils/fetch-source'; -import { RegularExternalCommunicationDelegate } from 'firefox-profiler/utils/query-api'; -import { findAddressProofForFile } from 'firefox-profiler/profile-logic/profile-data'; -import type { BrowserConnection } from 'firefox-profiler/app-logic/browser-connection'; -import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; +import { fetchSourceCode } from 'firefox-profiler/actions/code'; import explicitConnect from 'firefox-profiler/utils/connect'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; import type { SourceCodeStatus, - Profile, IndexIntoSourceTable, } from 'firefox-profiler/types'; type StateProps = { readonly sourceViewFile: string | null; readonly sourceViewSourceIndex: IndexIntoSourceTable | null; - readonly sourceViewSourceId: string | null; readonly sourceViewCode: SourceCodeStatus | void; - readonly symbolServerUrl: string; - readonly profile: Profile | null; - readonly browserConnection: BrowserConnection | null; }; type DispatchProps = { - readonly beginLoadingSourceCodeFromUrl: typeof beginLoadingSourceCodeFromUrl; - readonly beginLoadingSourceCodeFromBrowserConnection: typeof beginLoadingSourceCodeFromBrowserConnection; - readonly finishLoadingSourceCode: typeof finishLoadingSourceCode; - readonly failLoadingSourceCode: typeof failLoadingSourceCode; + readonly fetchSourceCode: typeof fetchSourceCode; }; type Props = ConnectedProps<{}, StateProps, DispatchProps>; @@ -63,68 +41,19 @@ class SourceCodeFetcherImpl extends React.PureComponent { this._triggerSourceLoadingIfNeeded(); } - async _triggerSourceLoadingIfNeeded() { + _triggerSourceLoadingIfNeeded() { const { sourceViewSourceIndex, sourceViewCode, sourceViewFile, - sourceViewSourceId, - beginLoadingSourceCodeFromUrl, - beginLoadingSourceCodeFromBrowserConnection, - finishLoadingSourceCode, - failLoadingSourceCode, - symbolServerUrl, - profile, - browserConnection, + fetchSourceCode, } = this.props; if (sourceViewSourceIndex === null || sourceViewCode || !sourceViewFile) { return; } - if (!profile) { - return; - } - - const addressProof = findAddressProofForFile( - profile, - sourceViewSourceIndex - ); - - const delegate = new RegularExternalCommunicationDelegate( - browserConnection, - { - onBeginUrlRequest: (url: string) => { - beginLoadingSourceCodeFromUrl(sourceViewSourceIndex, url); - }, - onBeginBrowserConnectionQuery: () => { - beginLoadingSourceCodeFromBrowserConnection(sourceViewSourceIndex); - }, - } - ); - - const fetchSourceResult = await fetchSource( - sourceViewFile, - sourceViewSourceId, - symbolServerUrl, - addressProof, - this._archiveCache, - delegate - ); - - switch (fetchSourceResult.type) { - case 'SUCCESS': - finishLoadingSourceCode( - sourceViewSourceIndex, - fetchSourceResult.source - ); - break; - case 'ERROR': - failLoadingSourceCode(sourceViewSourceIndex, fetchSourceResult.errors); - break; - default: - throw assertExhaustiveCheck(fetchSourceResult); - } + fetchSourceCode(sourceViewSourceIndex, this._archiveCache); } override render() { @@ -137,18 +66,9 @@ export const SourceCodeFetcher = explicitConnect<{}, StateProps, DispatchProps>( mapStateToProps: (state) => ({ sourceViewSourceIndex: getSourceViewSourceIndex(state), sourceViewFile: getSourceViewFile(state), - sourceViewSourceId: getSourceViewSourceId(state), sourceViewCode: getSourceViewCode(state), - symbolServerUrl: getSymbolServerUrl(state), - profile: getProfileOrNull(state), - browserConnection: getBrowserConnection(state), }), - mapDispatchToProps: { - beginLoadingSourceCodeFromUrl, - beginLoadingSourceCodeFromBrowserConnection, - finishLoadingSourceCode, - failLoadingSourceCode, - }, + mapDispatchToProps: { fetchSourceCode }, component: SourceCodeFetcherImpl, } ); From df781d46fc238cdfa34e768d0197c34920623e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 21 Oct 2026 16:50:41 +0200 Subject: [PATCH 03/15] Ignore source fetches that outlive the profile they started for A fetch that was still running when another profile got loaded wrote its result into the new source code cache, under an index that may now belong to a different file. Publishing a sanitized profile while the source view is loading is one way to get there, switching files in a zip another. Give the source code cache a generation, bumped on PROFILE_LOADED. The loading actions carry the generation from when their fetch started, and the reducer drops the ones from an older generation. That includes the LOADING states fetchSource reports after awaiting, for example when the symbol server fails and it moves on to the web. Such a LOADING entry would never be finished, and the source view doesn't fetch files that already have an entry. --- src/actions/code.ts | 48 ++++-- src/reducers/code.ts | 65 ++++---- src/selectors/code.tsx | 5 +- src/test/unit/source-code-cache.test.ts | 195 ++++++++++++++++++++---- src/types/actions.ts | 6 +- src/types/state.ts | 9 +- 6 files changed, 252 insertions(+), 76 deletions(-) diff --git a/src/actions/code.ts b/src/actions/code.ts index 2a00b91538..c55cead2a2 100644 --- a/src/actions/code.ts +++ b/src/actions/code.ts @@ -4,6 +4,7 @@ import { getProfile } from 'firefox-profiler/selectors/profile'; import { getBrowserConnection } from 'firefox-profiler/selectors/app'; import { getSymbolServerUrl } from 'firefox-profiler/selectors/url-state'; +import { getSourceCodeCacheGeneration } from 'firefox-profiler/selectors/code'; import { fetchSource, type FetchSourceResult, @@ -20,31 +21,47 @@ import type { IndexIntoSourceTable, } from 'firefox-profiler/types'; +// The `generation` argument of these is the source code cache generation from +// when the fetch started, see SourceCodeCacheState. + export function beginLoadingSourceCodeFromUrl( sourceIndex: IndexIntoSourceTable, - url: string + url: string, + generation: number ): Action { - return { type: 'SOURCE_CODE_LOADING_BEGIN_URL', sourceIndex, url }; + return { + type: 'SOURCE_CODE_LOADING_BEGIN_URL', + sourceIndex, + url, + generation, + }; } export function beginLoadingSourceCodeFromBrowserConnection( - sourceIndex: IndexIntoSourceTable + sourceIndex: IndexIntoSourceTable, + generation: number ): Action { - return { type: 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION', sourceIndex }; + return { + type: 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION', + sourceIndex, + generation, + }; } export function finishLoadingSourceCode( sourceIndex: IndexIntoSourceTable, - code: string + code: string, + generation: number ): Action { - return { type: 'SOURCE_CODE_LOADING_SUCCESS', sourceIndex, code }; + return { type: 'SOURCE_CODE_LOADING_SUCCESS', sourceIndex, code, generation }; } export function failLoadingSourceCode( sourceIndex: IndexIntoSourceTable, - errors: SourceCodeLoadingError[] + errors: SourceCodeLoadingError[], + generation: number ): Action { - return { type: 'SOURCE_CODE_LOADING_ERROR', sourceIndex, errors }; + return { type: 'SOURCE_CODE_LOADING_ERROR', sourceIndex, errors, generation }; } /** @@ -62,14 +79,19 @@ export function fetchSourceCode( return async (dispatch, getState) => { const state = getState(); const { sources, stringArray } = getProfile(state).shared; + // Read once here, so that the reducer can drop everything this fetch + // dispatches after another profile got loaded. + const generation = getSourceCodeCacheGeneration(state); const delegate = new RegularExternalCommunicationDelegate( getBrowserConnection(state), { onBeginUrlRequest: (url) => { - dispatch(beginLoadingSourceCodeFromUrl(sourceIndex, url)); + dispatch(beginLoadingSourceCodeFromUrl(sourceIndex, url, generation)); }, onBeginBrowserConnectionQuery: () => { - dispatch(beginLoadingSourceCodeFromBrowserConnection(sourceIndex)); + dispatch( + beginLoadingSourceCodeFromBrowserConnection(sourceIndex, generation) + ); }, } ); @@ -83,10 +105,12 @@ export function fetchSourceCode( ); switch (result.type) { case 'SUCCESS': - dispatch(finishLoadingSourceCode(sourceIndex, result.source)); + dispatch( + finishLoadingSourceCode(sourceIndex, result.source, generation) + ); break; case 'ERROR': - dispatch(failLoadingSourceCode(sourceIndex, result.errors)); + dispatch(failLoadingSourceCode(sourceIndex, result.errors, generation)); break; default: throw assertExhaustiveCheck(result); diff --git a/src/reducers/code.ts b/src/reducers/code.ts index e1c5e16c07..6c916f7ff9 100644 --- a/src/reducers/code.ts +++ b/src/reducers/code.ts @@ -5,55 +5,56 @@ import type { Reducer, SourceCodeStatus, + SourceCodeCacheState, + SourcesAction, AssemblyCodeStatus, CodeState, - IndexIntoSourceTable, } from 'firefox-profiler/types'; import { combineReducers } from 'redux'; +import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; -const sourceCodeCache: Reducer> = ( - state = new Map(), +const sourceCodeCache: Reducer = ( + state = { generation: 0, statuses: new Map() }, action ) => { switch (action.type) { - case 'SOURCE_CODE_LOADING_BEGIN_URL': { - const { sourceIndex, url } = action; - const newState = new Map(state); - newState.set(sourceIndex, { - type: 'LOADING', - source: { type: 'URL', url }, - }); - return newState; - } - case 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION': { - const { sourceIndex } = action; - const newState = new Map(state); - newState.set(sourceIndex, { - type: 'LOADING', - source: { type: 'BROWSER_CONNECTION' }, - }); - return newState; - } - case 'SOURCE_CODE_LOADING_SUCCESS': { - const { sourceIndex, code } = action; - const newState = new Map(state); - newState.set(sourceIndex, { type: 'AVAILABLE', code }); - return newState; - } + case 'SOURCE_CODE_LOADING_BEGIN_URL': + case 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION': + case 'SOURCE_CODE_LOADING_SUCCESS': case 'SOURCE_CODE_LOADING_ERROR': { - const { sourceIndex, errors } = action; - const newState = new Map(state); - newState.set(sourceIndex, { type: 'ERROR', errors }); - return newState; + if (action.generation !== state.generation) { + // The fetch started for another profile, so its sourceIndex may point + // at a different file now. This includes the LOADING states, which + // nothing would ever finish. + return state; + } + const statuses = new Map(state.statuses); + statuses.set(action.sourceIndex, _getSourceCodeStatus(action)); + return { ...state, statuses }; } case 'PROFILE_LOADED': // The source indexes only mean something for the profile they came from. - return new Map(); + return { generation: state.generation + 1, statuses: new Map() }; default: return state; } }; +function _getSourceCodeStatus(action: SourcesAction): SourceCodeStatus { + switch (action.type) { + case 'SOURCE_CODE_LOADING_BEGIN_URL': + return { type: 'LOADING', source: { type: 'URL', url: action.url } }; + case 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION': + return { type: 'LOADING', source: { type: 'BROWSER_CONNECTION' } }; + case 'SOURCE_CODE_LOADING_SUCCESS': + return { type: 'AVAILABLE', code: action.code }; + case 'SOURCE_CODE_LOADING_ERROR': + return { type: 'ERROR', errors: action.errors }; + default: + throw assertExhaustiveCheck(action); + } +} + const assemblyCodeCache: Reducer> = ( state = new Map(), action diff --git a/src/selectors/code.tsx b/src/selectors/code.tsx index 0356388d7d..0a26c4c1c0 100644 --- a/src/selectors/code.tsx +++ b/src/selectors/code.tsx @@ -17,7 +17,10 @@ import { getProfileOrNull } from './profile'; export const getSourceCodeCache: Selector< Map -> = (state) => state.code.sourceCodeCache; +> = (state) => state.code.sourceCodeCache.statuses; + +export const getSourceCodeCacheGeneration: Selector = (state) => + state.code.sourceCodeCache.generation; export const getSourceViewCode: Selector = createSelector( diff --git a/src/test/unit/source-code-cache.test.ts b/src/test/unit/source-code-cache.test.ts index 6d539f6f48..8da8972eb0 100644 --- a/src/test/unit/source-code-cache.test.ts +++ b/src/test/unit/source-code-cache.test.ts @@ -2,8 +2,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { getSourceViewCode, getSourceCodeCache } from '../../selectors/code'; -import { storeWithProfile } from '../fixtures/stores'; +import { + getSourceViewCode, + getSourceCodeCache, + getSourceCodeCacheGeneration, +} from '../../selectors/code'; +import { blankStore, storeWithProfile } from '../fixtures/stores'; import { updateUrlState } from '../../actions/app'; import { stateFromLocation } from '../../app-logic/url-handling'; import { @@ -11,9 +15,11 @@ import { beginLoadingSourceCodeFromBrowserConnection, finishLoadingSourceCode, failLoadingSourceCode, + fetchSourceCode, } from '../../actions/code'; import { viewProfile } from '../../actions/receive-profile'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; +import { waitUntilState } from '../fixtures/utils'; import type { IndexIntoSourceTable } from 'firefox-profiler/types'; @@ -28,7 +34,10 @@ describe('source code cache with IndexIntoSourceTable', function () { }); store.dispatch(updateUrlState(urlState)); } - return store; + return { + ...store, + generation: getSourceCodeCacheGeneration(store.getState()), + }; } it('returns undefined when no source code is cached', function () { @@ -44,17 +53,19 @@ describe('source code cache with IndexIntoSourceTable', function () { }); it('returns undefined when sourceIndex is not in cache', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(999); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(999); // Add some other source to cache but not index 999 - dispatch(finishLoadingSourceCode(0, 'some code')); + dispatch(finishLoadingSourceCode(0, 'some code', generation)); const sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toBeUndefined(); }); it('retrieves cached source code for AVAILABLE status', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(0); - dispatch(finishLoadingSourceCode(0, 'console.log("Source 0");')); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(0); + dispatch( + finishLoadingSourceCode(0, 'console.log("Source 0");', generation) + ); const sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toEqual({ @@ -64,9 +75,13 @@ describe('source code cache with IndexIntoSourceTable', function () { }); it('retrieves cached source code for LOADING status with URL', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(2); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(2); dispatch( - beginLoadingSourceCodeFromUrl(2, 'https://example.com/source2.js') + beginLoadingSourceCodeFromUrl( + 2, + 'https://example.com/source2.js', + generation + ) ); const sourceViewCode = getSourceViewCode(getState()); @@ -77,15 +92,19 @@ describe('source code cache with IndexIntoSourceTable', function () { }); it('retrieves cached source code for ERROR status', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(3); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(3); dispatch( - failLoadingSourceCode(3, [ - { - type: 'NETWORK_ERROR', - url: 'https://example.com/source3.js', - networkErrorMessage: 'Failed to fetch', - }, - ]) + failLoadingSourceCode( + 3, + [ + { + type: 'NETWORK_ERROR', + url: 'https://example.com/source3.js', + networkErrorMessage: 'Failed to fetch', + }, + ], + generation + ) ); const sourceViewCode = getSourceViewCode(getState()); @@ -102,8 +121,8 @@ describe('source code cache with IndexIntoSourceTable', function () { }); it('retrieves cached source code for LOADING status with BROWSER_CONNECTION', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(4); - dispatch(beginLoadingSourceCodeFromBrowserConnection(4)); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(4); + dispatch(beginLoadingSourceCodeFromBrowserConnection(4, generation)); const sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toEqual({ @@ -114,8 +133,11 @@ describe('source code cache with IndexIntoSourceTable', function () { describe('getSourceCodeCache selector', function () { it('returns the cache Map with cached entries', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(null); - dispatch(finishLoadingSourceCode(0, 'console.log("Source 0");')); + const { getState, dispatch, generation } = + setupStoreWithSourceIndex(null); + dispatch( + finishLoadingSourceCode(0, 'console.log("Source 0");', generation) + ); const result = getSourceCodeCache(getState()); expect(result).toBeInstanceOf(Map); @@ -134,8 +156,11 @@ describe('source code cache with IndexIntoSourceTable', function () { }); it('forgets the cache when another profile is loaded', async function () { - const { getState, dispatch } = setupStoreWithSourceIndex(null); - dispatch(finishLoadingSourceCode(0, 'console.log("Source 0");')); + const { getState, dispatch, generation } = + setupStoreWithSourceIndex(null); + dispatch( + finishLoadingSourceCode(0, 'console.log("Source 0");', generation) + ); expect(getSourceCodeCache(getState()).size).toBe(1); // The indexes of the old cache would point at unrelated files in the new @@ -144,17 +169,44 @@ describe('source code cache with IndexIntoSourceTable', function () { await dispatch(viewProfile(profile)); expect(getSourceCodeCache(getState()).size).toBe(0); }); + + it('ignores loading actions from before another profile was loaded', async function () { + const { getState, dispatch, generation } = + setupStoreWithSourceIndex(null); + const { profile } = getProfileFromTextSamples('A[file:other.js]'); + await dispatch(viewProfile(profile)); + + dispatch( + beginLoadingSourceCodeFromUrl( + 0, + 'https://example.com/test.js', + generation + ) + ); + dispatch(beginLoadingSourceCodeFromBrowserConnection(1, generation)); + dispatch(finishLoadingSourceCode(2, 'stale code', generation)); + dispatch( + failLoadingSourceCode(3, [{ type: 'NO_KNOWN_CORS_URL' }], generation) + ); + expect(getSourceCodeCache(getState()).size).toBe(0); + }); }); describe('state transitions', function () { it('can transition from LOADING to AVAILABLE', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(0); - dispatch(beginLoadingSourceCodeFromUrl(0, 'https://example.com/test.js')); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(0); + dispatch( + beginLoadingSourceCodeFromUrl( + 0, + 'https://example.com/test.js', + generation + ) + ); let sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode?.type).toBe('LOADING'); - dispatch(finishLoadingSourceCode(0, 'const result = 42;')); + dispatch(finishLoadingSourceCode(0, 'const result = 42;', generation)); sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toEqual({ @@ -164,13 +216,21 @@ describe('source code cache with IndexIntoSourceTable', function () { }); it('can transition from LOADING to ERROR', function () { - const { getState, dispatch } = setupStoreWithSourceIndex(1); - dispatch(beginLoadingSourceCodeFromUrl(1, 'https://example.com/test.js')); + const { getState, dispatch, generation } = setupStoreWithSourceIndex(1); + dispatch( + beginLoadingSourceCodeFromUrl( + 1, + 'https://example.com/test.js', + generation + ) + ); let sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode?.type).toBe('LOADING'); - dispatch(failLoadingSourceCode(1, [{ type: 'NO_KNOWN_CORS_URL' }])); + dispatch( + failLoadingSourceCode(1, [{ type: 'NO_KNOWN_CORS_URL' }], generation) + ); sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toEqual({ @@ -179,4 +239,81 @@ describe('source code cache with IndexIntoSourceTable', function () { }); }); }); + + describe('fetchSourceCode', function () { + const FILE = + 'git:github.com/rust-lang/rust:library/core/src/intrinsics.rs:acbe4443cc4c9695c0b74a7b64b60333c990a400'; + const FILE_URL = + 'https://raw.githubusercontent.com/rust-lang/rust/acbe4443cc4c9695c0b74a7b64b60333c990a400/library/core/src/intrinsics.rs'; + + function deferredResponse() { + let resolve: (response: number | string) => void = () => {}; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; + } + + it('ignores a fetch that finishes after another profile is loaded', async function () { + const response = deferredResponse(); + window.fetchMock.get(FILE_URL, () => response.promise); + const { profile } = getProfileFromTextSamples(`A[file:${FILE}]`); + const { getState, dispatch } = storeWithProfile(profile); + + const fetchPromise = dispatch(fetchSourceCode(0, new Map())); + expect(getSourceCodeCache(getState()).get(0)?.type).toBe('LOADING'); + + const { profile: otherProfile } = + getProfileFromTextSamples('B[file:other.js]'); + await dispatch(viewProfile(otherProfile)); + response.resolve('contents of intrinsics.rs'); + await fetchPromise; + + // Index 0 is other.js now, which must not get the contents of + // intrinsics.rs. + expect(getSourceCodeCache(getState()).size).toBe(0); + }); + + it('leaves no LOADING entry behind when a stale fetch tries another server', async function () { + const symbolServerResponse = deferredResponse(); + window.fetchMock + .post( + 'http://127.0.0.1:8000/source/v1', + () => symbolServerResponse.promise + ) + .get(FILE_URL, 'contents of intrinsics.rs'); + const { profile } = getProfileFromTextSamples( + `A[lib:libA.so][address:30][file:${FILE}]` + ); + const store = blankStore(); + const { getState, dispatch } = store; + dispatch( + updateUrlState( + stateFromLocation({ + pathname: '/from-browser', + search: '?symbolServer=http://127.0.0.1:8000', + hash: '', + }) + ) + ); + await dispatch(viewProfile(profile)); + + const fetchPromise = dispatch(fetchSourceCode(0, new Map())); + await waitUntilState(store, (state) => getSourceCodeCache(state).has(0)); + expect(getSourceCodeCache(getState()).get(0)).toEqual({ + type: 'LOADING', + source: { type: 'URL', url: 'http://127.0.0.1:8000/source/v1' }, + }); + + const { profile: otherProfile } = + getProfileFromTextSamples('B[file:other.js]'); + await dispatch(viewProfile(otherProfile)); + // The symbol server fails, so the fetch moves on to the web. + symbolServerResponse.resolve(500); + await fetchPromise; + + expect(window.fetchMock.callHistory.called(FILE_URL)).toBe(true); + expect(getSourceCodeCache(getState()).size).toBe(0); + }); + }); }); diff --git a/src/types/actions.ts b/src/types/actions.ts index 01d57310b3..ee727babf7 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -719,25 +719,29 @@ type CurrentProfileUploadedInformationAction = { readonly uploadedProfileInformation: UploadedProfileInformation | null; }; -type SourcesAction = +export type SourcesAction = | { readonly type: 'SOURCE_CODE_LOADING_BEGIN_URL'; readonly sourceIndex: IndexIntoSourceTable; readonly url: string; + readonly generation: number; } | { readonly type: 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION'; readonly sourceIndex: IndexIntoSourceTable; + readonly generation: number; } | { readonly type: 'SOURCE_CODE_LOADING_SUCCESS'; readonly sourceIndex: IndexIntoSourceTable; readonly code: string; + readonly generation: number; } | { readonly type: 'SOURCE_CODE_LOADING_ERROR'; readonly sourceIndex: IndexIntoSourceTable; readonly errors: SourceCodeLoadingError[]; + readonly generation: number; }; // nativeSymbolKey == `${lib.debugName}/${lib.breakpadID}/${nativeSymbolInfo.address.toString(16)}` diff --git a/src/types/state.ts b/src/types/state.ts index 5583e72181..c716e6995f 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -414,8 +414,15 @@ export type PseudoStrategy = null | 'bidi' | 'accented'; */ export type IconsWithClassNames = Map; +export type SourceCodeCacheState = { + // Bumped on PROFILE_LOADED. Loading actions carry the value from when their + // fetch started, and the ones from an older generation are dropped. + readonly generation: number; + readonly statuses: Map; +}; + export type CodeState = { - readonly sourceCodeCache: Map; + readonly sourceCodeCache: SourceCodeCacheState; readonly assemblyCodeCache: Map; }; From 7489a8a13fd9cfd2711cd977194db8cefb0f92bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:39:48 +0200 Subject: [PATCH 04/15] Build the per-sharing-mode publish selectors with a shared helper getRemoveProfileInformation and getSanitizedProfile each spelled out a maker, a record with one selector per mode, and a wrapper picking from it. Fold that into _perSharingMode, so that more per-mode selectors don't have to repeat it. No behavior change. --- src/selectors/publish.ts | 58 ++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/src/selectors/publish.ts b/src/selectors/publish.ts index fe3d1aa103..617090e57d 100644 --- a/src/selectors/publish.ts +++ b/src/selectors/publish.ts @@ -45,6 +45,20 @@ import { getThreadSelectors } from './per-thread'; export const getPublishState: Selector = (state) => state.publish; +/** + * Build one memoized selector per sharing mode, so that the download and upload + * panels don't evict each other's results. + */ +function _perSharingMode( + makeSelector: (mode: SharingMode) => Selector +): DangerousSelectorWithArguments { + const selectors: Record> = { + download: makeSelector('download'), + upload: makeSelector('upload'), + }; + return (state, mode) => selectors[mode](state); +} + export const getCheckedSharingOptions: DangerousSelectorWithArguments< CheckedSharingOptions, SharingMode @@ -73,7 +87,10 @@ export const getFilenameString: Selector = createSelector( } ); -const _makeGetRemoveProfileInformation = (mode: SharingMode) => +export const getRemoveProfileInformation: DangerousSelectorWithArguments< + RemoveProfileInformation | null, + SharingMode +> = _perSharingMode((mode) => createSelector( (state: State) => getCheckedSharingOptions(state, mode), getProfile, @@ -190,22 +207,8 @@ const _makeGetRemoveProfileInformation = (mode: SharingMode) => !checkedSharingOptions.includeArgumentValues, }; } - ); - -// One memoized selector per mode, so switching panels doesn't recompute the -// expensive sanitization. -const _removeProfileInformationSelectors: Record< - SharingMode, - Selector -> = { - download: _makeGetRemoveProfileInformation('download'), - upload: _makeGetRemoveProfileInformation('upload'), -}; - -export const getRemoveProfileInformation: DangerousSelectorWithArguments< - RemoveProfileInformation | null, - SharingMode -> = (state, mode) => _removeProfileInformationSelectors[mode](state); + ) +); /** * The derived markers are needed for profile sanitization, but they are also @@ -252,7 +255,10 @@ function getTracedValuesBuffersForAllThreads( * UrlState needs to be updated, with things like mapping thread indexes, * or providing a new committed range. */ -const _makeGetSanitizedProfile = (mode: SharingMode) => +export const getSanitizedProfile: DangerousSelectorWithArguments< + SanitizeProfileResult, + SharingMode +> = _perSharingMode((mode) => createSelector( getProfile, getDerivedMarkerInfoForAllThreads, @@ -260,20 +266,8 @@ const _makeGetSanitizedProfile = (mode: SharingMode) => (state: State) => getRemoveProfileInformation(state, mode), getMarkerSchemaByName, sanitizePII - ); - -const _sanitizedProfileSelectors: Record< - SharingMode, - Selector -> = { - download: _makeGetSanitizedProfile('download'), - upload: _makeGetSanitizedProfile('upload'), -}; - -export const getSanitizedProfile: DangerousSelectorWithArguments< - SanitizeProfileResult, - SharingMode -> = (state, mode) => _sanitizedProfileSelectors[mode](state); + ) +); export const getSanitizedProfileEncodingState: Selector< SanitizedProfileEncodingState From b8006105f31576990488024c6e368efdfe054b57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:40:10 +0200 Subject: [PATCH 05/15] Give arrow panels that set --width an exact width --width was used as a min-width, so a long unbreakable string inside the panel could stretch it, even past the edge of the window. Use it as the width. Panels without --width still shrink to fit their content. The publish panel is the only one that sets it. Also cap every arrow panel to the window, whether it sets --width or not. --- src/components/shared/ButtonWithPanel/ArrowPanel.css | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/shared/ButtonWithPanel/ArrowPanel.css b/src/components/shared/ButtonWithPanel/ArrowPanel.css index c8688b1fc3..c002dbf334 100644 --- a/src/components/shared/ButtonWithPanel/ArrowPanel.css +++ b/src/components/shared/ButtonWithPanel/ArrowPanel.css @@ -21,7 +21,14 @@ position: absolute; top: var(--internal-offset-from-top); right: calc(var(--internal-offset-from-right) * -1); - min-width: var(--internal-width); + + /* Panels that don't set --width shrink to fit their content. The ones that do + * get that exact width, so that a long unbreakable string inside can't stretch + * the panel. */ + width: var(--internal-width); + + /* No arrow panel grows wider than the window. */ + max-width: calc(100vw - 40px); border: 0.5px solid var(--internal-border-color); border-radius: 5px; background: var(--internal-background-color); From 6429b83cc68b4f8f6725e6307f9e77b656e5e2fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:40:36 +0200 Subject: [PATCH 06/15] Style the indeterminate state of photon checkboxes Photon checkboxes draw themselves with appearance: none, so one set to indeterminate through the DOM looked unchecked. Draw it like a checked one, with a dash instead of the check mark, in both the default and micro sizes. --- res/css/photon/checkbox.css | 22 ++++++++++++++++++++++ res/img/svg/checkbox-indeterminate-13.svg | 3 +++ res/img/svg/checkbox-indeterminate-16.svg | 3 +++ 3 files changed, 28 insertions(+) create mode 100644 res/img/svg/checkbox-indeterminate-13.svg create mode 100644 res/img/svg/checkbox-indeterminate-16.svg diff --git a/res/css/photon/checkbox.css b/res/css/photon/checkbox.css index b0bb6a385b..71f803e414 100644 --- a/res/css/photon/checkbox.css +++ b/res/css/photon/checkbox.css @@ -39,6 +39,12 @@ background-color: var(--clickable-checked-background-color); } +/* Only the DOM `indeterminate` property sets this, there is no HTML attribute. */ +.photon-checkbox:indeterminate:not([disabled]) { + border: none; + background-color: var(--clickable-checked-background-color); +} + .photon-checkbox:hover:active:not([disabled]) { background-color: var(--clickable-active-background-color); } @@ -47,14 +53,30 @@ background-image: url(../../img/svg/checkbox-check-16.svg); } +.photon-checkbox-default:indeterminate:not([disabled]) { + background-image: url(../../img/svg/checkbox-indeterminate-16.svg); +} + .photon-checkbox-micro:checked:not([disabled]) { background-image: url(../../img/svg/checkbox-check-13.svg); } +.photon-checkbox-micro:indeterminate:not([disabled]) { + background-image: url(../../img/svg/checkbox-indeterminate-13.svg); +} + .photon-checkbox:checked:hover:not([disabled]) { background-color: var(--clickable-checked-hover-background-color); } +.photon-checkbox:indeterminate:hover:not([disabled]) { + background-color: var(--clickable-checked-hover-background-color); +} + .photon-checkbox:checked:hover:active:not([disabled]) { background-color: var(--clickable-checked-hover-background-color); } + +.photon-checkbox:indeterminate:hover:active:not([disabled]) { + background-color: var(--clickable-checked-hover-background-color); +} diff --git a/res/img/svg/checkbox-indeterminate-13.svg b/res/img/svg/checkbox-indeterminate-13.svg new file mode 100644 index 0000000000..5208d996c3 --- /dev/null +++ b/res/img/svg/checkbox-indeterminate-13.svg @@ -0,0 +1,3 @@ + + + diff --git a/res/img/svg/checkbox-indeterminate-16.svg b/res/img/svg/checkbox-indeterminate-16.svg new file mode 100644 index 0000000000..b360c76abf --- /dev/null +++ b/res/img/svg/checkbox-indeterminate-16.svg @@ -0,0 +1,3 @@ + + + From b2d6aa127be6f48473976efc52074d7c3ea29f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:42:19 +0200 Subject: [PATCH 07/15] Add getOriginalSourceForFrame Pull the frame and func source-map lookup of getOriginalPositionForFrame into a helper, and add getOriginalSourceForFrame on top of it, for callers that only need the source and run this for every stack, where allocating a position object each time adds up. The source code sharing in the publish panel will be the first one. --- src/profile-logic/profile-data.ts | 85 +++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 21 deletions(-) diff --git a/src/profile-logic/profile-data.ts b/src/profile-logic/profile-data.ts index 01ebf66d3c..49c9c5a92a 100644 --- a/src/profile-logic/profile-data.ts +++ b/src/profile-logic/profile-data.ts @@ -76,6 +76,7 @@ import type { IndexIntoStackTable, IndexIntoResourceTable, IndexIntoNativeSymbolTable, + IndexIntoSourceLocationTable, CallNodeTableBitSet, ThreadIndex, Category, @@ -3645,6 +3646,32 @@ function _shouldShowBothOriginAndFileName( return true; } +/** + * The source-map entry used by tiers 1 and 2 of `getOriginalPositionForFrame`, + * or null if it falls back to the compiled position. + */ +function _getOriginalLocationForFrame( + frameIndex: IndexIntoFrameTable | null, + funcIndex: IndexIntoFuncTable, + frameTable: FrameTable | RawFrameTable, + funcTable: FuncTable | RawFuncTable, + sourceLocationTable: SourceLocationTable | null +): IndexIntoSourceLocationTable | null { + if (sourceLocationTable === null) { + return null; + } + if ( + frameIndex !== null && + (frameTable.flags[frameIndex] & FrameFlag.HasOriginalLocation) !== 0 + ) { + return frameTable.originalLocation[frameIndex]; + } + if ((funcTable.flags[funcIndex] & FuncFlag.HasOriginalLocation) !== 0) { + return funcTable.originalLocation[funcIndex]; + } + return null; +} + /** * Resolves the original (source-mapped) position for a frame's execution point, * with a 3-tier fallback that always produces a self-consistent @@ -3671,32 +3698,22 @@ export function getOriginalPositionForFrame( line: number | null; column: number | null; } { - if ( - sourceLocationTable !== null && - frameIndex !== null && - (frameTable.flags[frameIndex] & FrameFlag.HasOriginalLocation) !== 0 - ) { - const frameOriginalLocationIdx = frameTable.originalLocation[frameIndex]; + const originalLocation = _getOriginalLocationForFrame( + frameIndex, + funcIndex, + frameTable, + funcTable, + sourceLocationTable + ); + if (sourceLocationTable !== null && originalLocation !== null) { return { - source: sourceLocationTable.source[frameOriginalLocationIdx], - line: sourceLocationTable.line[frameOriginalLocationIdx], - column: sourceLocationTable.column[frameOriginalLocationIdx], + source: sourceLocationTable.source[originalLocation], + line: sourceLocationTable.line[originalLocation], + column: sourceLocationTable.column[originalLocation], }; } const funcFlags = funcTable.flags[funcIndex]; - if ( - sourceLocationTable !== null && - (funcFlags & FuncFlag.HasOriginalLocation) !== 0 - ) { - const funcOriginalLocationIdx = funcTable.originalLocation[funcIndex]; - return { - source: sourceLocationTable.source[funcOriginalLocationIdx], - line: sourceLocationTable.line[funcOriginalLocationIdx], - column: sourceLocationTable.column[funcOriginalLocationIdx], - }; - } - const funcSource = (funcFlags & FuncFlag.HasSource) !== 0 ? funcTable.source[funcIndex] : null; const funcLine = @@ -3730,6 +3747,32 @@ export function getOriginalPositionForFrame( }; } +/** + * The source that `getOriginalPositionForFrame` resolves to, without allocating + * the position object, for callers that run this for every stack. + */ +export function getOriginalSourceForFrame( + frameIndex: IndexIntoFrameTable | null, + funcIndex: IndexIntoFuncTable, + frameTable: FrameTable | RawFrameTable, + funcTable: FuncTable | RawFuncTable, + sourceLocationTable: SourceLocationTable | null +): IndexIntoSourceTable | null { + const originalLocation = _getOriginalLocationForFrame( + frameIndex, + funcIndex, + frameTable, + funcTable, + sourceLocationTable + ); + if (sourceLocationTable !== null && originalLocation !== null) { + return sourceLocationTable.source[originalLocation]; + } + return (funcTable.flags[funcIndex] & FuncFlag.HasSource) !== 0 + ? funcTable.source[funcIndex] + : null; +} + /** * This function returns the source origin for a function. This can be: * - a filename (javascript or object file or c++ source file) From 0fdc108dc8e488d4b91ea739212ff6139c15ffaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:43:04 +0200 Subject: [PATCH 08/15] Record where fetched source code came from fetchSource tries the browser connection and local symbol servers before the web. Contents that come from the former were read from the user's disk, so they may not exist anywhere else, even when the path has a public download URL. Tag each fetch as LOCAL or WEB and keep that in the source code cache, and tag contents embedded in the profile as PROFILE. The source code sharing in the publish panel needs this to tell which files anybody could download again. --- src/actions/code.ts | 17 ++++++++-- src/reducers/code.ts | 2 +- src/selectors/code.tsx | 2 +- .../store/source-map-symbolication.test.ts | 1 + src/test/unit/fetch-source.test.ts | 9 ++++++ src/test/unit/source-code-cache.test.ts | 32 +++++++++++++++---- src/types/actions.ts | 2 ++ src/types/state.ts | 10 +++++- src/utils/fetch-source.ts | 9 ++++-- 9 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/actions/code.ts b/src/actions/code.ts index c55cead2a2..bebfea19d0 100644 --- a/src/actions/code.ts +++ b/src/actions/code.ts @@ -19,6 +19,7 @@ import type { ApiQueryError, DecodedInstruction, IndexIntoSourceTable, + FetchedSourceCodeOrigin, } from 'firefox-profiler/types'; // The `generation` argument of these is the source code cache generation from @@ -51,9 +52,16 @@ export function beginLoadingSourceCodeFromBrowserConnection( export function finishLoadingSourceCode( sourceIndex: IndexIntoSourceTable, code: string, + origin: FetchedSourceCodeOrigin, generation: number ): Action { - return { type: 'SOURCE_CODE_LOADING_SUCCESS', sourceIndex, code, generation }; + return { + type: 'SOURCE_CODE_LOADING_SUCCESS', + sourceIndex, + code, + origin, + generation, + }; } export function failLoadingSourceCode( @@ -106,7 +114,12 @@ export function fetchSourceCode( switch (result.type) { case 'SUCCESS': dispatch( - finishLoadingSourceCode(sourceIndex, result.source, generation) + finishLoadingSourceCode( + sourceIndex, + result.source, + result.origin, + generation + ) ); break; case 'ERROR': diff --git a/src/reducers/code.ts b/src/reducers/code.ts index 6c916f7ff9..cb0815fd24 100644 --- a/src/reducers/code.ts +++ b/src/reducers/code.ts @@ -47,7 +47,7 @@ function _getSourceCodeStatus(action: SourcesAction): SourceCodeStatus { case 'SOURCE_CODE_LOADING_BEGIN_BROWSER_CONNECTION': return { type: 'LOADING', source: { type: 'BROWSER_CONNECTION' } }; case 'SOURCE_CODE_LOADING_SUCCESS': - return { type: 'AVAILABLE', code: action.code }; + return { type: 'AVAILABLE', code: action.code, origin: action.origin }; case 'SOURCE_CODE_LOADING_ERROR': return { type: 'ERROR', errors: action.errors }; default: diff --git a/src/selectors/code.tsx b/src/selectors/code.tsx index 0a26c4c1c0..25911db659 100644 --- a/src/selectors/code.tsx +++ b/src/selectors/code.tsx @@ -35,7 +35,7 @@ export const getSourceViewCode: Selector = // so the source view works offline and when sharing profiles. const inlineCode = profile?.shared.sources.content[sourceIndex]; if (inlineCode !== null && inlineCode !== undefined) { - return { type: 'AVAILABLE', code: inlineCode }; + return { type: 'AVAILABLE', code: inlineCode, origin: 'PROFILE' }; } return sourceCodeCache.get(sourceIndex); } diff --git a/src/test/store/source-map-symbolication.test.ts b/src/test/store/source-map-symbolication.test.ts index 545f15afc3..666c5223dd 100644 --- a/src/test/store/source-map-symbolication.test.ts +++ b/src/test/store/source-map-symbolication.test.ts @@ -471,6 +471,7 @@ describe('receive-profile -> JS source map symbolication', function () { expect(getSourceViewCode(getState())).toEqual({ type: 'AVAILABLE', code: ORIGINAL_SOURCE, + origin: 'PROFILE', }); }); }); diff --git a/src/test/unit/fetch-source.test.ts b/src/test/unit/fetch-source.test.ts index 027e8db6ee..3ca3ce4b65 100644 --- a/src/test/unit/fetch-source.test.ts +++ b/src/test/unit/fetch-source.test.ts @@ -38,6 +38,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'WEB', source: 'Fake response from https://hg.mozilla.org/mozilla-central/raw-file/997f00815e6bc28806b75448c8829f0259d2cb28/widget/cocoa/nsAppShell.mm', }); @@ -100,6 +101,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'WEB', source: 'Fake addr2line-0.17.0 src/lib.rs contents', }); expect(fetchUrlResponse).toHaveBeenCalledTimes(1); @@ -124,6 +126,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'WEB', source: 'Fake addr2line-0.17.0 src/function.rs contents', }); // No additional call should have been made. @@ -237,6 +240,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'LOCAL', source: 'Fake source from browser symbolication API, for request JSON {"debugName":"FAKE_DEBUGNAME","debugId":"FAKE_DEBUGID","moduleOffset":"0x1234","file":"/Users/mstange/code/mozilla/gfx/wr/webrender/src/renderer/mod.rs"}', }); @@ -294,6 +298,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'LOCAL', source: 'Fake source from local server symbolication API, with post data {"debugName":"FAKE_DEBUGNAME","debugId":"FAKE_DEBUGID","moduleOffset":"0x1234","file":"/Users/mstange/code/mozilla/gfx/wr/webrender/src/renderer/mod.rs"}', }); @@ -351,6 +356,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'LOCAL', source: 'Fake source from local server symbolication API, with post data {"debugName":"FAKE_DEBUGNAME","debugId":"FAKE_DEBUGID","moduleOffset":"0x1234","file":"git:github.com/rust-lang/rust:library/core/src/intrinsics.rs:acbe4443cc4c9695c0b74a7b64b60333c990a400"}', }); @@ -406,6 +412,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'WEB', source: 'Fake response from https://raw.githubusercontent.com/rust-lang/rust/acbe4443cc4c9695c0b74a7b64b60333c990a400/library/core/src/intrinsics.rs', }); @@ -624,6 +631,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'LOCAL', source: `console.log("Hello from browser with sourceUuid ${TEST_SOURCE_UUID}");`, }); }); @@ -704,6 +712,7 @@ describe('fetchSource', function () { ) ).toEqual({ type: 'SUCCESS', + origin: 'WEB', source: 'Fallback response from https://hg.mozilla.org/mozilla-central/raw-file/997f00815e6bc28806b75448c8829f0259d2cb28/widget/cocoa/nsAppShell.mm', }); diff --git a/src/test/unit/source-code-cache.test.ts b/src/test/unit/source-code-cache.test.ts index 8da8972eb0..aa1ddb8be3 100644 --- a/src/test/unit/source-code-cache.test.ts +++ b/src/test/unit/source-code-cache.test.ts @@ -55,7 +55,7 @@ describe('source code cache with IndexIntoSourceTable', function () { it('returns undefined when sourceIndex is not in cache', function () { const { getState, dispatch, generation } = setupStoreWithSourceIndex(999); // Add some other source to cache but not index 999 - dispatch(finishLoadingSourceCode(0, 'some code', generation)); + dispatch(finishLoadingSourceCode(0, 'some code', 'LOCAL', generation)); const sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toBeUndefined(); @@ -64,13 +64,19 @@ describe('source code cache with IndexIntoSourceTable', function () { it('retrieves cached source code for AVAILABLE status', function () { const { getState, dispatch, generation } = setupStoreWithSourceIndex(0); dispatch( - finishLoadingSourceCode(0, 'console.log("Source 0");', generation) + finishLoadingSourceCode( + 0, + 'console.log("Source 0");', + 'LOCAL', + generation + ) ); const sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toEqual({ type: 'AVAILABLE', code: 'console.log("Source 0");', + origin: 'LOCAL', }); }); @@ -136,7 +142,12 @@ describe('source code cache with IndexIntoSourceTable', function () { const { getState, dispatch, generation } = setupStoreWithSourceIndex(null); dispatch( - finishLoadingSourceCode(0, 'console.log("Source 0");', generation) + finishLoadingSourceCode( + 0, + 'console.log("Source 0");', + 'LOCAL', + generation + ) ); const result = getSourceCodeCache(getState()); @@ -144,6 +155,7 @@ describe('source code cache with IndexIntoSourceTable', function () { expect(result.get(0)).toEqual({ type: 'AVAILABLE', code: 'console.log("Source 0");', + origin: 'LOCAL', }); }); @@ -159,7 +171,12 @@ describe('source code cache with IndexIntoSourceTable', function () { const { getState, dispatch, generation } = setupStoreWithSourceIndex(null); dispatch( - finishLoadingSourceCode(0, 'console.log("Source 0");', generation) + finishLoadingSourceCode( + 0, + 'console.log("Source 0");', + 'LOCAL', + generation + ) ); expect(getSourceCodeCache(getState()).size).toBe(1); @@ -184,7 +201,7 @@ describe('source code cache with IndexIntoSourceTable', function () { ) ); dispatch(beginLoadingSourceCodeFromBrowserConnection(1, generation)); - dispatch(finishLoadingSourceCode(2, 'stale code', generation)); + dispatch(finishLoadingSourceCode(2, 'stale code', 'LOCAL', generation)); dispatch( failLoadingSourceCode(3, [{ type: 'NO_KNOWN_CORS_URL' }], generation) ); @@ -206,12 +223,15 @@ describe('source code cache with IndexIntoSourceTable', function () { let sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode?.type).toBe('LOADING'); - dispatch(finishLoadingSourceCode(0, 'const result = 42;', generation)); + dispatch( + finishLoadingSourceCode(0, 'const result = 42;', 'LOCAL', generation) + ); sourceViewCode = getSourceViewCode(getState()); expect(sourceViewCode).toEqual({ type: 'AVAILABLE', code: 'const result = 42;', + origin: 'LOCAL', }); }); diff --git a/src/types/actions.ts b/src/types/actions.ts index ee727babf7..7e47951285 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -46,6 +46,7 @@ import type { TableViewOptions, DecodedInstruction, ProfileEncodingResult, + FetchedSourceCodeOrigin, } from './state'; import type { CssPixels, StartEndRange, Milliseconds } from './units'; import type { BrowserConnectionStatus } from '../app-logic/browser-connection'; @@ -735,6 +736,7 @@ export type SourcesAction = readonly type: 'SOURCE_CODE_LOADING_SUCCESS'; readonly sourceIndex: IndexIntoSourceTable; readonly code: string; + readonly origin: FetchedSourceCodeOrigin; readonly generation: number; } | { diff --git a/src/types/state.ts b/src/types/state.ts index c716e6995f..0ad1335876 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -290,10 +290,18 @@ export type DecodedInstruction = { decodedString: string; }; +// Where the contents of a source file were fetched from. LOCAL covers the +// browser connection and local symbol servers, which read the user's disk, so +// the file may not exist anywhere else even if its path looks like a public URL. +export type FetchedSourceCodeOrigin = 'LOCAL' | 'WEB'; + +// FetchedSourceCodeOrigin, plus PROFILE for contents embedded in the profile. +export type SourceCodeOrigin = FetchedSourceCodeOrigin | 'PROFILE'; + export type SourceCodeStatus = | { type: 'LOADING'; source: CodeLoadingSource } | { type: 'ERROR'; errors: SourceCodeLoadingError[] } - | { type: 'AVAILABLE'; code: string }; + | { type: 'AVAILABLE'; code: string; origin: SourceCodeOrigin }; export type AssemblyCodeStatus = | { type: 'LOADING'; source: CodeLoadingSource } diff --git a/src/utils/fetch-source.ts b/src/utils/fetch-source.ts index 4ad15da0db..ea2c453ab7 100644 --- a/src/utils/fetch-source.ts +++ b/src/utils/fetch-source.ts @@ -15,10 +15,11 @@ import type { ExternalCommunicationDelegate } from './query-api'; import type { SourceCodeLoadingError, AddressProof, + FetchedSourceCodeOrigin, } from 'firefox-profiler/types'; export type FetchSourceResult = - | { type: 'SUCCESS'; source: string } + | { type: 'SUCCESS'; source: string; origin: FetchedSourceCodeOrigin } | { type: 'ERROR'; errors: SourceCodeLoadingError[] }; /** @@ -78,6 +79,7 @@ export async function fetchSource( return { type: 'SUCCESS', source: queryResult.convertedResponse, + origin: 'LOCAL', }; case 'ERROR': { errors.push(...queryResult.errors); @@ -97,6 +99,7 @@ export async function fetchSource( return { type: 'SUCCESS', source: response, + origin: 'LOCAL', }; } } catch (e) { @@ -120,7 +123,7 @@ export async function fetchSource( try { const response = await delegate.fetchUrlResponse(url); const source = await response.text(); - return { type: 'SUCCESS', source }; + return { type: 'SUCCESS', source, origin: 'WEB' }; } catch (e) { errors.push({ type: 'NETWORK_ERROR', @@ -169,7 +172,7 @@ export async function fetchSource( const entry = stream.next(); if (entry.name === pathInArchive && entry.buffer !== null) { const source = textDecoder.decode(entry.buffer); - return { type: 'SUCCESS', source }; + return { type: 'SUCCESS', source, origin: 'WEB' }; } } errors.push({ From 81bbc25d84efbdc7e05beab7ee943e5cbc4b9a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:48:37 +0200 Subject: [PATCH 09/15] Add the computations behind sharing source code from the publish panel The publish panel is going to offer the source files of a profile for sharing. This adds the profile logic for it, and the next commit hooks it up: - Which files are on a sampled stack, ordered by the time spent in them, callers included and recursion counted once. - Which of those to offer: files anybody can download again are left out, unless their contents came from the user's own machine. The ones we have the contents of come first, and only those can be picked. - The files a set of threads runs code from, for scoping to the selected threads. - A tree of the files for the panel, with chains of only children collapsed into one row. - A copy of the profile that carries the contents of the picked files and nothing else. --- src/profile-logic/source-sharing.ts | 495 +++++++++++++++++++++++++++ src/test/unit/source-sharing.test.ts | 493 ++++++++++++++++++++++++++ src/types/profile-derived.ts | 92 +++++ src/types/state.ts | 3 + 4 files changed, 1083 insertions(+) create mode 100644 src/profile-logic/source-sharing.ts create mode 100644 src/test/unit/source-sharing.test.ts diff --git a/src/profile-logic/source-sharing.ts b/src/profile-logic/source-sharing.ts new file mode 100644 index 0000000000..33bb3ecc3f --- /dev/null +++ b/src/profile-logic/source-sharing.ts @@ -0,0 +1,495 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { + getDownloadRecipeForSourceFile, + parseFileNameFromSymbolication, +} from 'firefox-profiler/utils/special-paths'; +import { SetCollectionBuilder } from 'firefox-profiler/utils/set-collection'; +import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; +import { getOriginalSourceForFrame } from 'firefox-profiler/profile-logic/profile-data'; + +import type { + IndexIntoSourceTable, + Profile, + SourceCodeAvailability, + SourceCodeContents, + SourceCodeSharingCandidate, + SourceCodeSharingFile, + SourceCodeSharingStats, + SourceCodeSharingSummary, + SourceCodeStatus, + SourceFileTreeNode, + ThreadIndex, +} from 'firefox-profiler/types'; + +// Files that anybody can download don't need to be embedded, the profiler +// fetches them again when the shared profile is opened. +function _isPubliclyDownloadable(filename: string): boolean { + const recipe = getDownloadRecipeForSourceFile( + parseFileNameFromSymbolication(filename) + ); + return recipe.type !== 'NO_KNOWN_CORS_URL'; +} + +/** + * The source contents we have on hand, from the profile or fetched during this + * session, by source index. The profile's own contents win over the cache. + */ +export function computeSourceContentsForSharing( + profile: Profile, + sourceCodeCache: Map +): Map { + const contents = new Map(); + const { sources } = profile.shared; + + for (let sourceIndex = 0; sourceIndex < sources.length; sourceIndex++) { + const code = sources.content[sourceIndex]; + if (code !== null) { + contents.set(sourceIndex, { code, origin: 'PROFILE' }); + } + } + + for (const [sourceIndex, status] of sourceCodeCache) { + if (status.type !== 'AVAILABLE' || contents.has(sourceIndex)) { + continue; + } + if (sourceIndex < 0 || sourceIndex >= sources.length) { + continue; + } + contents.set(sourceIndex, { code: status.code, origin: status.origin }); + } + + return contents; +} + +function _splitPath(path: string): string[] { + return path.split('/').filter((s) => s !== ''); +} + +// URLs keep "://" in one segment, so that the "//" doesn't turn +// into empty rows. +const URL_ORIGIN_REGEX = /^([a-z][a-z0-9+.-]*):\/\/([^/]*)(\/.*)?$/i; + +// The tree segments for a file. +function _pathSegmentsForFilename(path: string): string[] { + const originMatch = URL_ORIGIN_REGEX.exec(path); + if (originMatch !== null) { + const [, scheme, host, rest] = originMatch; + return [`${scheme}://${host}`, ..._splitPath(rest ?? '')]; + } + + const segments = path.split(/[/\\]/).filter((s) => s !== ''); + return segments.length > 0 ? segments : [path]; +} + +/** + * The source of each stack's own frame, or -1 when it has none. Source-mapped + * frames resolve to their original file. + */ +export function computeStackToSource(profile: Profile): Int32Array { + const { stackTable, frameTable, funcTable, sourceLocationTable } = + profile.shared; + const stackToSource = new Int32Array(stackTable.length); + + for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { + const frameIndex = stackTable.frame[stackIndex]; + const source = getOriginalSourceForFrame( + frameIndex, + frameTable.func[frameIndex], + frameTable, + funcTable, + sourceLocationTable + ); + stackToSource[stackIndex] = source ?? -1; + } + + return stackToSource; +} + +/** + * The pickable source files anywhere on the given threads' stacks, not only in + * the leaf frames. + */ +export function computeSourcesForThreads( + profile: Profile, + threadIndexes: ReadonlySet, + stackToSource: Int32Array, + pickableSources: ReadonlySet +): Set { + const sources = new Set(); + if (threadIndexes.size === 0 || pickableSources.size === 0) { + return sources; + } + + const { stackTable } = profile.shared; + // Mark the sampled stacks, then propagate the marks to their prefixes. + const isOnAStack = new Uint8Array(stackTable.length); + for (const threadIndex of threadIndexes) { + const thread = profile.threads[threadIndex]; + if (thread === undefined) { + // A thread index left over from a profile that has since been replaced. + continue; + } + const { samples } = thread; + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + const stackIndex = samples.stack[sampleIndex]; + if (stackIndex !== null) { + isOnAStack[stackIndex] = 1; + } + } + } + + // A stack's prefix always has a lower index than the stack itself, so one + // reverse pass reaches every ancestor. + for (let stackIndex = stackTable.length - 1; stackIndex >= 0; stackIndex--) { + if (isOnAStack[stackIndex] === 0) { + continue; + } + const prefixOffset = stackTable.prefixOffset[stackIndex]; + if (prefixOffset !== 0) { + isOnAStack[stackIndex - prefixOffset] = 1; + } + const source = stackToSource[stackIndex]; + if (source !== -1 && pickableSources.has(source)) { + sources.add(source); + } + } + + return sources; +} + +/** + * The total (not self) sample weight of each source across all threads, + * counting a source once per stack even if it appears several times. Only + * meaningful relative to each other. Sources which are never on a sampled + * stack are absent. + */ +function _computeSourceTotalWeights( + profile: Profile, + stackToSource: Int32Array +): Map { + const { stackTable } = profile.shared; + + // For each stack, the set of sources on its path to the root, as an index + // into a trie of sets. -1 means there are none. + const builder = new SetCollectionBuilder(); + const stackToSourceSet = new Int32Array(stackTable.length); + + for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) { + const prefixOffset = stackTable.prefixOffset[stackIndex]; + const prefixSet = + prefixOffset !== 0 ? stackToSourceSet[stackIndex - prefixOffset] : -1; + + const source = stackToSource[stackIndex]; + if (source === -1) { + stackToSourceSet[stackIndex] = prefixSet; + continue; + } + stackToSourceSet[stackIndex] = builder.extend( + prefixSet !== -1 ? prefixSet : null, + source + ); + } + + const setTable = builder.finish(); + + const weightPerSet = new Float64Array(setTable.length); + for (const { samples } of profile.threads) { + const { stack, weight } = samples; + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + const stackIndex = stack[sampleIndex]; + if (stackIndex === null) { + continue; + } + const setIndex = stackToSourceSet[stackIndex]; + if (setIndex === -1) { + continue; + } + // Weights are negative in comparison profiles. + weightPerSet[setIndex] += + weight !== null ? Math.abs(weight[sampleIndex]) : 1; + } + } + + // Push each set's weight up to its parent, so that every entry ends up with + // the weight of all the sets containing its value. Parents have lower + // indexes, so one reverse pass is enough. + for (let i = setTable.length - 1; i >= 0; i--) { + const parent = setTable.parent[i]; + if (parent !== null) { + weightPerSet[parent] += weightPerSet[i]; + } + } + + const totals = new Map(); + for (let i = 0; i < setTable.length; i++) { + if (weightPerSet[i] === 0) { + continue; + } + const source = setTable.value[i]; + totals.set(source, (totals.get(source) ?? 0) + weightPerSet[i]); + } + + return totals; +} + +/** + * Every source file that is on a sampled stack, ordered by the time spent in + * it, most first. This depends on the profile only, so it's computed once. + */ +export function computeSourceCodeSharingCandidates( + profile: Profile, + stackToSource: Int32Array +): SourceCodeSharingCandidate[] { + const { sources, stringArray } = profile.shared; + const totalWeights = _computeSourceTotalWeights(profile, stackToSource); + + const candidates = Array.from(totalWeights.keys(), (sourceIndex) => { + const filename = stringArray[sources.filename[sourceIndex]]; + const path = parseFileNameFromSymbolication(filename).path; + return { + sourceIndex, + path, + pathSegments: _pathSegmentsForFilename(path), + isPubliclyDownloadable: _isPubliclyDownloadable(filename), + }; + }); + + candidates.sort((a, b) => { + const weightDifference = + (totalWeights.get(b.sourceIndex) ?? 0) - + (totalWeights.get(a.sourceIndex) ?? 0); + if (weightDifference !== 0) { + return weightDifference; + } + return a.path.localeCompare(b.path); + }); + + return candidates; +} + +/** + * The files worth offering in the publish panel: downloaded ones first, then + * the rest, each in candidate order. Files that anybody can download are left + * out, unless their contents came from the user's own machine, in which case + * the public URL may well not work. + */ +export function computeSourceCodeSharingFiles( + candidates: SourceCodeSharingCandidate[], + contents: Map +): SourceCodeSharingFile[] { + const byAvailability: Record< + SourceCodeAvailability, + SourceCodeSharingFile[] + > = { DOWNLOADED: [], UNAVAILABLE: [] }; + for (const candidate of candidates) { + const { sourceIndex } = candidate; + const fileContents = contents.get(sourceIndex); + if (candidate.isPubliclyDownloadable && fileContents?.origin !== 'LOCAL') { + continue; + } + const availability: SourceCodeAvailability = + fileContents !== undefined ? 'DOWNLOADED' : 'UNAVAILABLE'; + byAvailability[availability].push({ + sourceIndex, + path: candidate.path, + pathSegments: candidate.pathSegments, + availability, + byteCount: fileContents !== undefined ? fileContents.code.length : null, + }); + } + return [...byAvailability.DOWNLOADED, ...byAvailability.UNAVAILABLE]; +} + +export function computeSourceCodeSharingStats( + candidates: SourceCodeSharingCandidate[], + files: SourceCodeSharingFile[] +): SourceCodeSharingStats { + let downloadedFileCount = 0; + let downloadedByteCount = 0; + let unavailableFileCount = 0; + for (const file of files) { + switch (file.availability) { + case 'DOWNLOADED': + downloadedFileCount++; + downloadedByteCount += file.byteCount ?? 0; + break; + case 'UNAVAILABLE': + unavailableFileCount++; + break; + default: + throw assertExhaustiveCheck(file.availability); + } + } + return { + fileCount: files.length, + downloadedFileCount, + downloadedByteCount, + unavailableFileCount, + publiclyDownloadableFileCount: candidates.length - files.length, + }; +} + +/** + * The files the user can pick, which are the downloaded ones. + */ +export function computePickableSources( + files: SourceCodeSharingFile[] +): Set { + const pickable = new Set(); + for (const file of files) { + if (file.availability === 'DOWNLOADED') { + pickable.add(file.sourceIndex); + } + } + return pickable; +} + +// A directory or file while the tree is still being built. +type PartialTreeNode = { + segment: string; + children: Map; + // Gecko keys sources by a hash of path and contents, so one path can stand + // for several sources, like a script that changed between page loads. + files: SourceCodeSharingFile[]; +}; + +function _newPartialTreeNode(segment: string): PartialTreeNode { + return { segment, children: new Map(), files: [] }; +} + +/** + * Turn the flat file list into a tree. Children keep the order of `files`, so + * the directories holding downloaded files come first, heaviest first. + */ +export function buildSourceFileTree( + files: SourceCodeSharingFile[] +): SourceFileTreeNode[] { + const root = _newPartialTreeNode(''); + for (const file of files) { + let node = root; + for (const segment of file.pathSegments) { + let child = node.children.get(segment); + if (child === undefined) { + child = _newPartialTreeNode(segment); + node.children.set(segment, child); + } + node = child; + } + node.files.push(file); + } + + return Array.from(root.children.values(), (child) => + _finishTreeNode(child, '') + ); +} + +function _finishTreeNode( + startNode: PartialTreeNode, + parentPath: string +): SourceFileTreeNode { + // Collapse chains of only children into a single row. + const segments = [startNode.segment]; + let node = startNode; + while (node.files.length === 0 && node.children.size === 1) { + node = node.children.values().next().value as PartialTreeNode; + segments.push(node.segment); + } + + const label = segments.join('/'); + const treePath = parentPath === '' ? label : `${parentPath}/${label}`; + // File rows show the file's real path, which the tree path can differ from, + // for example by dropping the leading "/". + const path = node.files.length > 0 ? node.files[0].path : treePath; + const children = Array.from(node.children.values(), (child) => + _finishTreeNode(child, treePath) + ); + + const sourceIndexes = []; + let byteCount = 0; + let downloadedFileCount = 0; + for (const file of node.files) { + switch (file.availability) { + case 'DOWNLOADED': + sourceIndexes.push(file.sourceIndex); + byteCount += file.byteCount ?? 0; + downloadedFileCount++; + break; + case 'UNAVAILABLE': + break; + default: + throw assertExhaustiveCheck(file.availability); + } + } + for (const child of children) { + sourceIndexes.push(...child.sourceIndexes); + byteCount += child.byteCount; + downloadedFileCount += child.downloadedFileCount; + } + + return { + key: treePath, + label, + path, + availability: node.files.length > 0 ? node.files[0].availability : null, + sourceIndexes, + byteCount, + downloadedFileCount, + children, + }; +} + +/** + * Return a copy of the profile carrying the contents of the selected files and + * nothing else, including dropping unselected contents that were already + * embedded. Returns the profile itself if nothing changes. + */ +export function setSharedSourceCode( + profile: Profile, + contents: Map, + selectedSourceIndexes: ReadonlySet +): Profile { + const { sources } = profile.shared; + const content = sources.content.slice(); + let changed = false; + + for (let sourceIndex = 0; sourceIndex < sources.length; sourceIndex++) { + const newContent = selectedSourceIndexes.has(sourceIndex) + ? (contents.get(sourceIndex)?.code ?? null) + : null; + if (content[sourceIndex] !== newContent) { + content[sourceIndex] = newContent; + changed = true; + } + } + + if (!changed) { + return profile; + } + + return { + ...profile, + shared: { + ...profile.shared, + sources: { ...sources, content }, + }, + }; +} + +export function computeSourceCodeSharingSummary( + contents: Map, + selectedSourceIndexes: ReadonlySet +): SourceCodeSharingSummary { + let fileCount = 0; + let byteCount = 0; + for (const sourceIndex of selectedSourceIndexes) { + const fileContents = contents.get(sourceIndex); + if (fileContents !== undefined) { + fileCount++; + byteCount += fileContents.code.length; + } + } + return { fileCount, byteCount }; +} diff --git a/src/test/unit/source-sharing.test.ts b/src/test/unit/source-sharing.test.ts new file mode 100644 index 0000000000..d276a855bd --- /dev/null +++ b/src/test/unit/source-sharing.test.ts @@ -0,0 +1,493 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { + buildSourceFileTree, + computePickableSources, + computeSourceCodeSharingCandidates, + computeSourceCodeSharingFiles, + computeSourceCodeSharingStats, + computeSourceContentsForSharing, + computeSourcesForThreads, + computeStackToSource, +} from '../../profile-logic/source-sharing'; +import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; +import { ensureExists } from '../../utils/types'; + +import type { + IndexIntoSourceTable, + SourceCodeAvailability, + SourceCodeSharingFile, + SourceCodeStatus, + FetchedSourceCodeOrigin, + SourceFileTreeNode, +} from 'firefox-profiler/types'; + +// A path in the form the symbolication API returns for official builds. Anybody +// who opens the shared profile can download this file from hg.mozilla.org. +const PUBLIC_FILE = + 'hg:hg.mozilla.org/mozilla-central:widget/cocoa/nsAppShell.mm:997f00815e6bc28806b75448c8829f0259d2cb28'; +// A path on the machine that recorded the profile. +const LOCAL_FILE = '/home/user/moz/mozilla-central/dom/base/nsGlobalWindow.cpp'; + +function setup() { + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples( + ` + A[file:${LOCAL_FILE}] + B[file:${PUBLIC_FILE}] + ` + ); + const { funcTable } = profile.shared; + const [{ A, B }] = funcNamesDictPerThread; + + const sourceIndexForFunc = (funcIndex: number): IndexIntoSourceTable => + ensureExists( + funcTable.source[funcIndex], + 'Expected the func to have a source' + ); + + return { + profile, + candidates: computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ), + localSourceIndex: sourceIndexForFunc(A), + publicSourceIndex: sourceIndexForFunc(B), + }; +} + +function filePaths(candidates: Array<{ path: string }>): string[] { + return candidates.map((candidate) => candidate.path); +} + +function availableCode( + entries: Array<[IndexIntoSourceTable, string]>, + origin: FetchedSourceCodeOrigin = 'LOCAL' +): Map { + return new Map( + entries.map(([sourceIndex, code]) => [ + sourceIndex, + { type: 'AVAILABLE', code, origin }, + ]) + ); +} + +describe('computeSourceContentsForSharing', function () { + it('finds the contents already embedded in the profile', function () { + const { profile, localSourceIndex } = setup(); + profile.shared.sources.content[localSourceIndex] = 'int main() {}'; + + expect(computeSourceContentsForSharing(profile, new Map())).toEqual( + new Map([ + [localSourceIndex, { code: 'int main() {}', origin: 'PROFILE' }], + ]) + ); + }); + + it('finds the source code fetched during this session', function () { + const { profile, localSourceIndex } = setup(); + const cache = availableCode([[localSourceIndex, 'int main() {}']]); + + expect(computeSourceContentsForSharing(profile, cache)).toEqual( + new Map([[localSourceIndex, { code: 'int main() {}', origin: 'LOCAL' }]]) + ); + }); + + it('ignores sources which are still loading or failed to load', function () { + const { profile, localSourceIndex } = setup(); + const cache: Map = new Map([ + [ + localSourceIndex, + { type: 'LOADING', source: { type: 'URL', url: 'u' } }, + ], + ]); + + expect(computeSourceContentsForSharing(profile, cache)).toEqual(new Map()); + }); + + it('prefers the contents in the profile over the cached ones', function () { + const { profile, localSourceIndex } = setup(); + profile.shared.sources.content[localSourceIndex] = 'from the source map'; + const cache = availableCode([[localSourceIndex, 'from the local disk']]); + + expect(computeSourceContentsForSharing(profile, cache)).toEqual( + new Map([ + [localSourceIndex, { code: 'from the source map', origin: 'PROFILE' }], + ]) + ); + }); +}); + +describe('computeSourcesForThreads', function () { + function setupTwoThreads() { + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples( + ` + A[file:/src/a.cpp] + B[file:/src/b.cpp] + `, + `C[file:/src/c.cpp]` + ); + const { funcTable } = profile.shared; + const sourceOf = (funcIndex: number) => + ensureExists(funcTable.source[funcIndex]); + const [{ A, B }, { C }] = funcNamesDictPerThread; + const contents = new Map([ + [sourceOf(A), 'a'], + [sourceOf(B), 'b'], + [sourceOf(C), 'c'], + ]); + return { + profile, + contents, + stackToSource: computeStackToSource(profile), + sourceA: sourceOf(A), + sourceB: sourceOf(B), + sourceC: sourceOf(C), + }; + } + + it('finds the files a thread runs code from, callers included', function () { + const { profile, contents, stackToSource, sourceA, sourceB } = + setupTwoThreads(); + + // Thread 0's only sample has B as its leaf and A as its caller, so both + // files count even though only B has self time. + expect( + computeSourcesForThreads( + profile, + new Set([0]), + stackToSource, + new Set(contents.keys()) + ) + ).toEqual(new Set([sourceA, sourceB])); + }); + + it('keeps the threads apart', function () { + const { profile, contents, stackToSource, sourceC } = setupTwoThreads(); + + expect( + computeSourcesForThreads( + profile, + new Set([1]), + stackToSource, + new Set(contents.keys()) + ) + ).toEqual(new Set([sourceC])); + }); + + it('leaves out files whose contents we cannot share', function () { + const { profile, stackToSource, sourceB } = setupTwoThreads(); + + expect( + computeSourcesForThreads( + profile, + new Set([0]), + stackToSource, + new Set([sourceB]) + ) + ).toEqual(new Set([sourceB])); + }); +}); + +describe('computeSourceCodeSharingCandidates', function () { + it('lists every sampled file, and tells which ones anybody can download', function () { + const { candidates, localSourceIndex, publicSourceIndex } = setup(); + + expect(candidates).toEqual([ + { + sourceIndex: localSourceIndex, + path: LOCAL_FILE, + pathSegments: [ + 'home', + 'user', + 'moz', + 'mozilla-central', + 'dom', + 'base', + 'nsGlobalWindow.cpp', + ], + isPubliclyDownloadable: false, + }, + { + sourceIndex: publicSourceIndex, + path: 'widget/cocoa/nsAppShell.mm', + pathSegments: ['widget', 'cocoa', 'nsAppShell.mm'], + isPubliclyDownloadable: true, + }, + ]); + }); + + it('leaves out files which are never on a sampled stack', function () { + const { profile } = setup(); + // A source that symbolication knows about but no sampled frame points at. + const { sources, stringArray } = profile.shared; + sources.id.push(null); + sources.filename.push(stringArray.length); + stringArray.push('/home/user/moz/unsampled.cpp'); + sources.startLine.push(1); + sources.startColumn.push(1); + sources.sourceMapURL.push(null); + sources.content.push('int unused() {}'); + sources.length++; + + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + expect(filePaths(candidates)).not.toContain('/home/user/moz/unsampled.cpp'); + }); + + it('orders the files by the time spent in them, most first', function () { + // C is on the stack for 3 samples, A for 2, and B for 1. + const { profile } = getProfileFromTextSamples(` + C[file:/src/c.cpp] C[file:/src/c.cpp] C[file:/src/c.cpp] + A[file:/src/a.cpp] A[file:/src/a.cpp] B[file:/src/b.cpp] + `); + + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + expect(filePaths(candidates)).toEqual([ + '/src/c.cpp', + '/src/a.cpp', + '/src/b.cpp', + ]); + }); + + it('counts recursion through the same file only once', function () { + // b.cpp is on the stack for 2 samples. a.cpp is on it for 1, twice. If the + // recursion were counted twice, a.cpp would tie with b.cpp and win the tie + // on its path. + const { profile } = getProfileFromTextSamples(` + B[file:/src/b.cpp] B[file:/src/b.cpp] A[file:/src/a.cpp] + A2[file:/src/a.cpp] + `); + + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + expect(filePaths(candidates)).toEqual(['/src/b.cpp', '/src/a.cpp']); + }); + + it('splits URLs so that the "//" does not become an empty segment', function () { + const { profile } = getProfileFromTextSamples( + `Ajs[file:https://example.com/static/app.js]` + ); + + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + expect(candidates[0].pathSegments).toEqual([ + 'https://example.com', + 'static', + 'app.js', + ]); + }); +}); + +describe('computeSourceCodeSharingFiles', function () { + it('tells the files we have from the ones we do not', function () { + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples(` + A[file:/src/a.cpp] B[file:/src/b.cpp] + `); + const [{ B }] = funcNamesDictPerThread; + const sourceB = ensureExists(profile.shared.funcTable.source[B]); + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + + const files = computeSourceCodeSharingFiles( + candidates, + computeSourceContentsForSharing(profile, availableCode([[sourceB, 'b']])) + ); + expect( + files.map((file) => [file.path, file.availability, file.byteCount]) + ).toEqual([ + ['/src/b.cpp', 'DOWNLOADED', 1], + ['/src/a.cpp', 'UNAVAILABLE', null], + ]); + expect(computePickableSources(files)).toEqual(new Set([sourceB])); + expect(computeSourceCodeSharingStats(candidates, files)).toEqual({ + fileCount: 2, + downloadedFileCount: 1, + downloadedByteCount: 1, + unavailableFileCount: 1, + publiclyDownloadableFileCount: 0, + }); + }); + + it('lists files we have not downloaded, without a size', function () { + const { candidates, localSourceIndex } = setup(); + + expect(computeSourceCodeSharingFiles(candidates, new Map())).toEqual([ + { + sourceIndex: localSourceIndex, + path: LOCAL_FILE, + pathSegments: candidates[0].pathSegments, + availability: 'UNAVAILABLE', + byteCount: null, + }, + ]); + }); + + it('puts the files we have before the ones we do not, whatever their weight', function () { + // B is on the stack for 2 samples and A for 1, but only A was downloaded. + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples(` + B[file:/src/b.cpp] B[file:/src/b.cpp] A[file:/src/a.cpp] + `); + const [{ A }] = funcNamesDictPerThread; + const sourceA = ensureExists(profile.shared.funcTable.source[A]); + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + const contents = computeSourceContentsForSharing( + profile, + availableCode([[sourceA, 'a']]) + ); + + expect(filePaths(candidates)).toEqual(['/src/b.cpp', '/src/a.cpp']); + expect( + filePaths(computeSourceCodeSharingFiles(candidates, contents)) + ).toEqual(['/src/a.cpp', '/src/b.cpp']); + }); + + it('sizes the files we have', function () { + const { profile, candidates, localSourceIndex } = setup(); + const contents = computeSourceContentsForSharing( + profile, + availableCode([[localSourceIndex, 'int main() {}']]) + ); + + const files = computeSourceCodeSharingFiles(candidates, contents); + expect(files[0].byteCount).toBe('int main() {}'.length); + }); + + it('leaves out files which anybody can download, even when we have them', function () { + const { profile, candidates, publicSourceIndex } = setup(); + profile.shared.sources.content[publicSourceIndex] = 'from a source map'; + const contents = computeSourceContentsForSharing( + profile, + availableCode([[publicSourceIndex, 'nsAppShell contents']], 'WEB') + ); + + expect( + filePaths(computeSourceCodeSharingFiles(candidates, contents)) + ).toEqual([LOCAL_FILE]); + }); + + it('offers a public-looking file when its contents came from the local machine', function () { + // The path has a download recipe, but the fetch was answered by the + // browser connection, so the repo may well be private. + const { profile, candidates, publicSourceIndex } = setup(); + const contents = computeSourceContentsForSharing( + profile, + availableCode([[publicSourceIndex, 'nsAppShell contents']]) + ); + + // The downloaded file comes first, however the candidates are ordered. + const files = computeSourceCodeSharingFiles(candidates, contents); + expect(filePaths(files)).toEqual([ + 'widget/cocoa/nsAppShell.mm', + LOCAL_FILE, + ]); + }); +}); + +describe('buildSourceFileTree', function () { + function makeFiles( + paths: string[], + availabilityByPath: Record = {} + ): SourceCodeSharingFile[] { + return paths.map((path, index) => { + const availability = availabilityByPath[path] ?? 'DOWNLOADED'; + return { + sourceIndex: index, + path, + pathSegments: path.split('/').filter((s) => s !== ''), + availability, + byteCount: availability === 'DOWNLOADED' ? 1 : null, + }; + }); + } + + // Render the tree as indented lines, so the expectations read like the UI. + function formatTree(nodes: SourceFileTreeNode[], depth = 0): string[] { + const lines: string[] = []; + for (const node of nodes) { + lines.push(`${' '.repeat(depth)}${node.label}`); + lines.push(...formatTree(node.children, depth + 1)); + } + return lines; + } + + it('gives a directory with several files a row of its own', function () { + const tree = buildSourceFileTree( + makeFiles(['/src/a.cpp', '/src/b.cpp', '/src/sub/c.cpp']) + ); + expect(formatTree(tree)).toEqual([ + 'src', + ' a.cpp', + ' b.cpp', + ' sub/c.cpp', + ]); + }); + + it('keeps the file order, so the heaviest files come first', function () { + const tree = buildSourceFileTree( + makeFiles(['/src/z/late.cpp', '/src/a/early.cpp']) + ); + expect(formatTree(tree)).toEqual(['src', ' z/late.cpp', ' a/early.cpp']); + }); + + it('handles a file whose path is a prefix of another', function () { + const tree = buildSourceFileTree( + makeFiles(['/src/thing', '/src/thing/nested.cpp']) + ); + // "thing" is both a file and a directory here, so it can't be collapsed + // into its child. + expect(formatTree(tree)).toEqual(['src/thing', ' nested.cpp']); + }); + + it('does not let files we have no contents for be picked', function () { + const tree = buildSourceFileTree( + makeFiles(['/src/a.cpp', '/src/b.cpp'], { '/src/b.cpp': 'UNAVAILABLE' }) + ); + const [src] = tree; + expect(src).toMatchObject({ + sourceIndexes: [0], + byteCount: 1, + downloadedFileCount: 1, + availability: null, + }); + expect(src.children[1]).toMatchObject({ + label: 'b.cpp', + availability: 'UNAVAILABLE', + sourceIndexes: [], + }); + }); + + it('shows the real path on file rows and the tree path on directories', function () { + const tree = buildSourceFileTree(makeFiles(['/src/a.cpp', '/src/b.cpp'])); + expect(tree[0]).toMatchObject({ key: 'src', path: 'src' }); + expect(tree[0].children[0]).toMatchObject({ + key: 'src/a.cpp', + path: '/src/a.cpp', + }); + }); + + it('gives sources with the same path a single row', function () { + // The same script loaded twice with different contents. + const tree = buildSourceFileTree(makeFiles(['/src/app.js', '/src/app.js'])); + expect(formatTree(tree)).toEqual(['src/app.js']); + expect(tree[0].availability).toBe('DOWNLOADED'); + expect(tree[0].sourceIndexes).toEqual([0, 1]); + expect(tree[0].byteCount).toBe(2); + }); +}); diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index 10fbae4239..8e2a3edadd 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -761,6 +761,98 @@ export type Track = GlobalTrack | LocalTrack; // - for local tracks, this is the index in the local tracks array for a specific pid. export type TrackIndex = number; +/** + * A source file that is on a sampled stack, as far as the profile alone can + * tell. Which candidates the publish panel offers also depends on the + * contents fetched during the session. + */ +export type SourceCodeSharingCandidate = { + sourceIndex: IndexIntoSourceTable; + // The full file path, with any symbolication decorations stripped. + path: string; + // `path`, split into the segments shown in the file tree. + pathSegments: string[]; + // Whether the path has a download recipe, so that anybody who opens the + // shared profile can fetch the file themselves. + isPubliclyDownloadable: boolean; +}; + +/** + * Whether the contents of a source file are in hand. + * - DOWNLOADED: we have the contents. + * - UNAVAILABLE: we don't, so the file can't be picked. + */ +export type SourceCodeAvailability = 'DOWNLOADED' | 'UNAVAILABLE'; + +/** + * One source file offered in the publish panel. + */ +export type SourceCodeSharingFile = { + sourceIndex: IndexIntoSourceTable; + path: string; + pathSegments: string[]; + availability: SourceCodeAvailability; + // The size of the file contents, or null when we don't have them. This counts + // UTF-16 code units rather than encoded bytes, which is close enough for + // source code. + byteCount: number | null; +}; + +/** + * A row in the file tree shown under the "Include source code" checkbox + * in the publish panel. + * + * A row can stand for a single file, for a chain of directories, or for a chain + * of directories ending in a file: any node which is an only child is collapsed + * into its parent's row, so that deep paths don't produce a row per component. + */ +export type SourceFileTreeNode = { + // Identifies this row across renders. This is the full path, which is unique + // within the tree. + key: string; + // What the row displays: the segments this row covers, joined with "/". + label: string; + // For a file row, the file's path. For a directory row, the path from the + // root of the tree to it. + path: string; + // For a file row, the availability of the file. null for rows that only + // stand for directories. + availability: SourceCodeAvailability | null; + // Every source index at or below this row that can be picked, so downloaded, + // in tree order. A file row can hold several when the same path was loaded + // with different contents. + sourceIndexes: IndexIntoSourceTable[]; + // The size of the contents we have for the files at or below this row. + byteCount: number; + downloadedFileCount: number; + children: SourceFileTreeNode[]; +}; + +/** + * Counts over the source files listed in the publish panel, which the text + * around the "Include source code" checkbox is made of. + */ +export type SourceCodeSharingStats = { + // The files listed in the panel. + fileCount: number; + // The listed files whose contents we have, and their total size. + downloadedFileCount: number; + downloadedByteCount: number; + // The listed files whose contents we don't have. + unavailableFileCount: number; + // The sampled files left out of the panel because anybody can download them. + publiclyDownloadableFileCount: number; +}; + +/** + * How many files, and how many bytes of source code, the publish panel's + * current selection would add to the shared profile. + */ +export type SourceCodeSharingSummary = { + fileCount: number; + byteCount: number; +}; + /** * Type that holds the values of personally identifiable information that user * wants to remove. diff --git a/src/types/state.ts b/src/types/state.ts index 0ad1335876..c852092afe 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -298,6 +298,9 @@ export type FetchedSourceCodeOrigin = 'LOCAL' | 'WEB'; // FetchedSourceCodeOrigin, plus PROFILE for contents embedded in the profile. export type SourceCodeOrigin = FetchedSourceCodeOrigin | 'PROFILE'; +// The contents of a source file that we have on hand, and where they came from. +export type SourceCodeContents = { code: string; origin: SourceCodeOrigin }; + export type SourceCodeStatus = | { type: 'LOADING'; source: CodeLoadingSource } | { type: 'ERROR'; errors: SourceCodeLoadingError[] } From b3b3f9f4bc998e5bc29390cbfd1797407f524fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:51:58 +0200 Subject: [PATCH 10/15] Add an option to share source code when publishing Until now the sanitizer always dropped the contents of profile.shared.sources when sharing or downloading a profile. Add an "Include source code" checkbox to the publish panel, with a file tree to pick which files to include, and presets for the current file, the selected threads and all available files. The tree lists every file on a sampled stack that can't be downloaded again by whoever opens the shared profile, sorted by the time spent in it. Files whose contents we don't have show as unavailable and can't be picked, and the option stays disabled until at least one file is downloaded. While it's off, the checkbox says how much source code is on offer. A status line under the tree says how much the picked files add, with an info icon about the files that are left out. Turning the option on picks the narrowest preset, and it starts out on when the source view shows a pickable file. The picked files and preset are kept per panel, and forgotten when another profile is loaded. The source contents are picked in a selector before sanitizePII, so that including them doesn't turn an unsanitized upload into a sanitized one and reset the URL state. The publish panel is a bit wider, so that the tree can show longer paths. --- locales/en-US/app.ftl | 78 +++ src/actions/publish.ts | 37 ++ src/components/app/MenuButtons/Publish.css | 7 +- src/components/app/MenuButtons/Publish.tsx | 8 + .../app/MenuButtons/PublishSourceCode.tsx | 327 ++++++++++++ .../app/MenuButtons/SourceFileTree.css | 162 ++++++ .../app/MenuButtons/SourceFileTree.tsx | 229 ++++++++ src/profile-logic/sanitize.ts | 19 +- src/reducers/publish.ts | 23 + src/selectors/publish.ts | 259 ++++++++- src/test/components/MenuButtons.test.tsx | 498 ++++++++++++++++++ src/test/store/publish.test.ts | 285 +++++++++- src/test/unit/sanitize.test.ts | 23 +- src/types/actions.ts | 7 + src/types/profile-derived.ts | 16 + src/types/state.ts | 13 + 16 files changed, 1964 insertions(+), 27 deletions(-) create mode 100644 src/components/app/MenuButtons/PublishSourceCode.tsx create mode 100644 src/components/app/MenuButtons/SourceFileTree.css create mode 100644 src/components/app/MenuButtons/SourceFileTree.tsx diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 5a1811bd72..381f04f7c1 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -761,6 +761,84 @@ MenuButtons--publish--renderCheckbox-label-private-browsing-warning-image = MenuButtons--publish--renderCheckbox-label-argument-values = Include JavaScript execution tracing function argument values MenuButtons--publish--renderCheckbox-label-argument-values-warning-image = .title = This profile contains function argument values recorded from the page, which may include personal data +MenuButtons--publish--renderCheckbox-label-source-code = Include source code +# This is shown next to the "Include source code" checkbox while it is +# unchecked, so that the amount of source code on offer is visible without +# having to turn the option on. +# Variables: +# $fileCount (Number) - Number of source files that could be shared +# $size (String) - Pretty-printed total size of those files, e.g. "12 kB" +MenuButtons--publish--source-code-available = + { $fileCount -> + [one] ({ $fileCount } file available, { $size }) + *[other] ({ $fileCount } files available, { $size }) + } +# Same place as the previous string, when some of the files haven't been +# downloaded yet. Only downloaded files can be included. +# Variables: +# $fileCount (Number) - Number of source files whose contents we have +# $totalFileCount (Number) - Number of source files that could be shared +# $size (String) - Pretty-printed total size of the files we have, e.g. "12 kB" +MenuButtons--publish--source-code-partially-downloaded = + { $totalFileCount -> + [one] ({ $fileCount } of { $totalFileCount } file downloaded, { $size }) + *[other] ({ $fileCount } of { $totalFileCount } files downloaded, { $size }) + } +# Same place as the previous string, when none of the files have been downloaded +# yet. The checkbox is disabled in that case. +# Variables: +# $totalFileCount (Number) - Number of source files that could be shared +MenuButtons--publish--source-code-none-downloaded = + { $totalFileCount -> + [one] ({ $totalFileCount } file, none downloaded yet) + *[other] ({ $totalFileCount } files, none downloaded yet) + } +# Shown in the size column of the file tree for files that can't be downloaded +# from here. +MenuButtons--publish--source-code-unavailable = Unavailable + .title = Downloading this file requires the browser that recorded the profile or a local symbol server. +# This labels the row of ready-made file selections shown above the file tree of +# the "Include source code" option. +MenuButtons--publish--source-code-preset-legend = Include: +# Selects only the source file that the source view is currently showing. +MenuButtons--publish--source-code-preset-current-file = Current file +# Selects the source files that the currently selected threads run code from. +MenuButtons--publish--source-code-preset-selected-threads = Selected threads +# Selects every source file that was fetched or came with the profile, and that +# can be shared. +MenuButtons--publish--source-code-preset-all-available-files = All available files +# Not something the user can pick. This is shown as the selected option once the +# user has edited the file tree so that it no longer matches any of the others. +MenuButtons--publish--source-code-preset-custom = Custom +# The status line underneath the file tree of the "Include source code" +# option: how much source code the picked files add to the shared profile. +# Variables: +# $fileCount (Number) - Number of picked source files whose contents we have +# $size (String) - Pretty-printed total size of those contents, e.g. "12 kB" +MenuButtons--publish--source-code-status = + { $fileCount -> + [one] { $fileCount } file, { $size } uncompressed. + *[other] { $fileCount } files, { $size } uncompressed. + } +# A sentence in the tooltip of an info icon at the end of the status line, when +# some of the listed files can't be picked because they can't be downloaded +# from here. It can be followed by the next string, on a line of its own. +# Variables: +# $unavailableCount (Number) - Number of listed files that can't be downloaded from here +MenuButtons--publish--source-code-left-out-unavailable = + { $unavailableCount -> + [one] { $unavailableCount } file can’t be downloaded from here. Downloading it requires the browser that recorded the profile or a local symbol server. + *[other] { $unavailableCount } files can’t be downloaded from here. Downloading them requires the browser that recorded the profile or a local symbol server. + } +# A sentence in the same tooltip as the previous string, when some sampled files +# aren't listed because anybody can download them. +# Variables: +# $publicCount (Number) - Number of files left out because anybody can download them +MenuButtons--publish--source-code-left-out-public = + { $publicCount -> + [one] { $publicCount } file is available on the web and isn’t included. + *[other] { $publicCount } files are available on the web and aren’t included. + } MenuButtons--publish--share-performance-profile = Share Performance Profile MenuButtons--publish--reshare-performance-profile = Re-share Performance Profile MenuButtons--publish--save-performance-profile = Save Performance Profile diff --git a/src/actions/publish.ts b/src/actions/publish.ts index f4b3869bd2..040650bf51 100644 --- a/src/actions/publish.ts +++ b/src/actions/publish.ts @@ -12,6 +12,7 @@ import { getRemoveProfileInformation, getPrePublishedState, getSanitizedProfileEncodingState, + getSourceFilesForOfferedPresets, } from 'firefox-profiler/selectors/publish'; import { getDataSource, @@ -60,6 +61,8 @@ import type { ProfileIndexTranslationMaps, ProfileEncodingResult, SharingMode, + IndexIntoSourceTable, + OfferedSourceSharingPreset, } from 'firefox-profiler/types'; import { compress } from 'firefox-profiler/utils/gz'; import { serializeProfileToJsonString } from 'firefox-profiler/profile-logic/process-profile'; @@ -77,6 +80,25 @@ export function updateSharingOption( }; } +/** + * Replace the set of source files whose contents will be part of the shared + * profile. The publish panel's file tree passes the full new set, rather than a + * single file, because checking a directory row toggles all the files below it. + * `preset` is the preset the user picked to get this selection, if any. + */ +export function updateSourceFilesToShare( + mode: SharingMode, + sourceIndexes: ReadonlySet, + preset: OfferedSourceSharingPreset | null = null +): Action { + return { + type: 'UPDATE_SOURCE_FILES_TO_SHARE', + mode, + sourceIndexes, + preset, + }; +} + export function sanitizedProfileEncodingStarted( sanitizedProfile: Profile, encodingPromise: Promise @@ -347,6 +369,21 @@ export function encodeSanitizedProfile( }; } +/** + * Turn the "Include source code" option on, picking the narrowest preset. + */ +export function turnOnSourceCodeSharing(mode: SharingMode): ThunkAction { + return (dispatch, getState) => { + const [firstPreset] = getSourceFilesForOfferedPresets(getState()); + if (firstPreset === undefined) { + return; + } + const [preset, files] = firstPreset; + dispatch(updateSourceFilesToShare(mode, files, preset)); + dispatch(encodeSanitizedProfile(mode)); + }; +} + /** * This function starts the profile sharing process. * diff --git a/src/components/app/MenuButtons/Publish.css b/src/components/app/MenuButtons/Publish.css index 283f1f053a..930834f1a1 100644 --- a/src/components/app/MenuButtons/Publish.css +++ b/src/components/app/MenuButtons/Publish.css @@ -49,7 +49,8 @@ } .publishPanelPanel { - --width: 510px; + /* Wide enough that the source file tree can show a full-ish path on one row. */ + --width: 560px; --internal-info-icon: url(../../../../res/img/svg/info.svg); --internal-button-disabled-hover-background-color: var(--grey-90-a10); --internal-upload-icon: url(../../../../res/img/svg/upload.svg); @@ -57,6 +58,8 @@ --internal-upload-percentage-foreground-color: var(--blue-60); --internal-upload-bar-background-color: var(--grey-40); --internal-warning-indicator-icon: url(../../../../res/img/svg/warning.svg); + --internal-dimmed-foreground-color: var(--grey-50); + --internal-source-file-tree-border-color: var(--grey-30); } :root.dark-mode { @@ -80,6 +83,8 @@ --internal-upload-percentage-foreground-color: var(--blue-60); --internal-upload-bar-background-color: var(--grey-40); --internal-warning-indicator-icon: url(../../../../res/img/svg/warning-light.svg); + --internal-dimmed-foreground-color: var(--grey-40); + --internal-source-file-tree-border-color: var(--grey-60); } } diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 5d4002f00f..97d7099f62 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -27,7 +27,9 @@ import { getUploadError, getShouldSanitizeByDefault, getSanitizedProfileEncodingState, + getHasSourceCodeToShare, } from 'firefox-profiler/selectors/publish'; +import { PublishSourceCodeSection } from './PublishSourceCode'; import { BlobUrlLink } from 'firefox-profiler/components/shared/BlobUrlLink'; import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; import prettyBytes from 'firefox-profiler/utils/pretty-bytes'; @@ -60,6 +62,7 @@ type StateProps = { readonly shouldShowPreferenceOption: boolean; readonly profileContainsPrivateBrowsingInformation: boolean; readonly profileHasJSTracingArgumentValues: boolean; + readonly hasSourceCodeToShare: boolean; readonly checkedSharingOptions: CheckedSharingOptions; readonly sanitizedProfileEncodingState: SanitizedProfileEncodingState; readonly downloadFileName: string; @@ -131,6 +134,7 @@ class PublishPanelImpl extends React.PureComponent { shouldShowPreferenceOption, profileContainsPrivateBrowsingInformation, profileHasJSTracingArgumentValues, + hasSourceCodeToShare, sanitizedProfileEncodingState, downloadFileName, shouldSanitizeByDefault, @@ -252,6 +256,9 @@ class PublishPanelImpl extends React.PureComponent { /> ) : null} + {hasSourceCodeToShare ? ( + + ) : null} {sanitizedProfileEncodingState.phase === 'ERROR' ? (
@@ -420,6 +427,7 @@ export const PublishPanel = explicitConnect< profileContainsPrivateBrowsingInformation: getContainsPrivateBrowsingInformation(state), profileHasJSTracingArgumentValues: getHasJSTracingArgumentValues(state), + hasSourceCodeToShare: getHasSourceCodeToShare(state), checkedSharingOptions: getCheckedSharingOptions(state, ownProps.mode), downloadFileName: getFilenameString(state), sanitizedProfileEncodingState: getSanitizedProfileEncodingState(state), diff --git a/src/components/app/MenuButtons/PublishSourceCode.tsx b/src/components/app/MenuButtons/PublishSourceCode.tsx new file mode 100644 index 0000000000..8f34a5f7ab --- /dev/null +++ b/src/components/app/MenuButtons/PublishSourceCode.tsx @@ -0,0 +1,327 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import * as React from 'react'; +import { Localized, useLocalization } from '@fluent/react'; + +import { + updateSourceFilesToShare, + turnOnSourceCodeSharing, + encodeSanitizedProfile, +} from 'firefox-profiler/actions/publish'; +import { + getUploadPhase, + getActiveSourceSharingPreset, + getSourceCodeSharingStats, + getSourceCodeSharingSummary, + getSourceFileTree, + getSourceFilesForOfferedPresets, + getSourceFilesToShare, +} from 'firefox-profiler/selectors/publish'; +import { SourceFileTree } from './SourceFileTree'; +import prettyBytes from 'firefox-profiler/utils/pretty-bytes'; +import explicitConnect, { + type ConnectedProps, +} from 'firefox-profiler/utils/connect'; + +import type { + IndexIntoSourceTable, + SharingMode, + SourceCodeSharingStats, + SourceCodeSharingSummary, + SourceFileTreeNode, + OfferedSourceSharingPreset, + SourceSharingPreset, + UploadPhase, +} from 'firefox-profiler/types'; + +const PRESET_L10N_IDS: Record = { + 'current-file': 'MenuButtons--publish--source-code-preset-current-file', + 'selected-threads': + 'MenuButtons--publish--source-code-preset-selected-threads', + 'all-files': 'MenuButtons--publish--source-code-preset-all-available-files', + custom: 'MenuButtons--publish--source-code-preset-custom', +}; + +type SourceCodeLeftOutInfoProps = { + readonly unavailableCount: number; + readonly publicCount: number; +}; + +/** + * An info icon whose tooltip has a sentence for the listed files that can't be + * downloaded from here, and one for the files left out because anybody can + * download them, each only when there are any. The sentences are separate + * messages so that each one carries a single plural. + */ +function SourceCodeLeftOutInfo({ + unavailableCount, + publicCount, +}: SourceCodeLeftOutInfoProps) { + const { l10n } = useLocalization(); + const sentences = []; + if (unavailableCount > 0) { + sentences.push( + l10n.getString('MenuButtons--publish--source-code-left-out-unavailable', { + unavailableCount, + }) + ); + } + if (publicCount > 0) { + sentences.push( + l10n.getString('MenuButtons--publish--source-code-left-out-public', { + publicCount, + }) + ); + } + if (sentences.length === 0) { + return null; + } + return ( + + ); +} + +type OwnProps = { + readonly mode: SharingMode; +}; + +type StateProps = { + readonly uploadPhase: UploadPhase; + readonly sourceCodeSharingStats: SourceCodeSharingStats; + readonly sourceFileTree: SourceFileTreeNode[]; + readonly sourceFilesToShare: ReadonlySet; + readonly sourceFilesForPresets: Map< + OfferedSourceSharingPreset, + ReadonlySet + >; + readonly activeSourceSharingPreset: SourceSharingPreset; + readonly sourceCodeSharingSummary: SourceCodeSharingSummary; +}; + +type DispatchProps = { + readonly updateSourceFilesToShare: typeof updateSourceFilesToShare; + readonly turnOnSourceCodeSharing: typeof turnOnSourceCodeSharing; + readonly encodeSanitizedProfile: typeof encodeSanitizedProfile; +}; + +type Props = ConnectedProps; + +/** + * The "Include source code" checkbox of the publish panel, with the presets, + * file tree and status line that come with it once it's on. + */ +class PublishSourceCodeSectionImpl extends React.PureComponent { + _isUploading(): boolean { + const { uploadPhase } = this.props; + return uploadPhase === 'uploading' || uploadPhase === 'compressing'; + } + + _onToggleSourceFiles = ( + sourceIndexes: IndexIntoSourceTable[], + selected: boolean + ) => { + const { mode, sourceFilesToShare, updateSourceFilesToShare } = this.props; + const newSelection = new Set(sourceFilesToShare); + for (const sourceIndex of sourceIndexes) { + if (selected) { + newSelection.add(sourceIndex); + } else { + newSelection.delete(sourceIndex); + } + } + updateSourceFilesToShare(mode, newSelection); + this.props.encodeSanitizedProfile(mode); + }; + + // Turning sharing on picks the narrowest preset rather than every file, so + // that opting in doesn't share more than the user looked at. + _onToggleSourceCode = () => { + const { mode, sourceFilesToShare, updateSourceFilesToShare } = this.props; + if (sourceFilesToShare.size === 0) { + this.props.turnOnSourceCodeSharing(mode); + return; + } + updateSourceFilesToShare(mode, new Set()); + this.props.encodeSanitizedProfile(mode); + }; + + _onPresetChange = (e: React.ChangeEvent) => { + const { mode, sourceFilesForPresets, updateSourceFilesToShare } = + this.props; + // The 'custom' radio is disabled, so only offered presets get here. + const preset = e.target.value as OfferedSourceSharingPreset; + const sourceIndexes = sourceFilesForPresets.get(preset); + if (sourceIndexes === undefined) { + return; + } + updateSourceFilesToShare(mode, sourceIndexes, preset); + this.props.encodeSanitizedProfile(mode); + }; + + // One line under the tree: what will be included, and an info icon for the + // files that are left out. + _renderStatusRow() { + const { sourceCodeSharingSummary, sourceCodeSharingStats } = this.props; + const { unavailableFileCount, publiclyDownloadableFileCount } = + sourceCodeSharingStats; + + return ( +
+ + + + + + +
+ ); + } + + _renderPresets() { + const { mode, activeSourceSharingPreset, sourceFilesForPresets } = + this.props; + if (sourceFilesForPresets.size < 2) { + return null; + } + + const presets: SourceSharingPreset[] = [...sourceFilesForPresets.keys()]; + if (activeSourceSharingPreset === 'custom') { + presets.push('custom'); + } + + return ( +
+ + + Include: + + + {presets.map((preset) => ( + + ))} +
+ ); + } + + // Shown next to the checkbox while it's off, so that the amount of source + // code on offer is visible without turning the option on. + _renderAvailability() { + const { fileCount, downloadedFileCount, downloadedByteCount } = + this.props.sourceCodeSharingStats; + const missingFileCount = fileCount - downloadedFileCount; + const size = prettyBytes(downloadedByteCount); + + let l10nId = 'MenuButtons--publish--source-code-available'; + if (downloadedFileCount === 0) { + l10nId = 'MenuButtons--publish--source-code-none-downloaded'; + } else if (missingFileCount > 0) { + l10nId = 'MenuButtons--publish--source-code-partially-downloaded'; + } + + return ( + + + + ); + } + + override render() { + const { sourceFileTree, sourceFilesToShare, sourceCodeSharingStats } = + this.props; + const isSharingSourceCode = sourceFilesToShare.size > 0; + + return ( + <> + + {isSharingSourceCode ? ( + <> + {this._renderPresets()} +
+ +
+ {this._renderStatusRow()} + + ) : null} + + ); + } +} + +export const PublishSourceCodeSection = explicitConnect< + OwnProps, + StateProps, + DispatchProps +>({ + mapStateToProps: (state, { mode }) => ({ + uploadPhase: getUploadPhase(state), + sourceCodeSharingStats: getSourceCodeSharingStats(state), + sourceFileTree: getSourceFileTree(state), + sourceFilesToShare: getSourceFilesToShare(state, mode), + sourceFilesForPresets: getSourceFilesForOfferedPresets(state), + activeSourceSharingPreset: getActiveSourceSharingPreset(state, mode), + sourceCodeSharingSummary: getSourceCodeSharingSummary(state, mode), + }), + mapDispatchToProps: { + updateSourceFilesToShare, + turnOnSourceCodeSharing, + encodeSanitizedProfile, + }, + component: PublishSourceCodeSectionImpl, +}); diff --git a/src/components/app/MenuButtons/SourceFileTree.css b/src/components/app/MenuButtons/SourceFileTree.css new file mode 100644 index 0000000000..18047660d2 --- /dev/null +++ b/src/components/app/MenuButtons/SourceFileTree.css @@ -0,0 +1,162 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +.sourceFileTree { + padding: 0; + margin: 0; + list-style: none; +} + +.sourceFileTree .sourceFileTree { + padding-left: 12px; +} + +.sourceFileTreeItem { + padding: 0; + margin: 0; +} + +.sourceFileTreeRow { + display: flex; + min-height: 20px; + align-items: center; +} + +.sourceFileTreeTwisty, +.sourceFileTreeTwistyPlaceholder { + width: 12px; + height: 12px; + flex-shrink: 0; + padding: 0; + border: none; + background: none; +} + +.sourceFileTreeTwisty::before { + display: block; + width: 0; + height: 0; + border-top: 4px solid transparent; + border-bottom: 4px solid transparent; + border-left: 6px solid currentcolor; + margin: 2px 0 0 3px; + content: ''; + + /* Opt-out of forced colors so the transparent borders aren't turned into opaque ones */ + forced-color-adjust: none; +} + +.sourceFileTreeTwistyExpanded::before { + border-top: 6px solid currentcolor; + border-right: 4px solid transparent; + border-bottom: none; + border-left: 4px solid transparent; + margin: 3px 0 0 2px; +} + +.sourceFileTreeRow:hover { + background-color: var(--clickable-ghost-hover-background-color); +} + +/* The label spans the whole row, so that clicking anywhere on it toggles the + * checkbox. */ +.sourceFileTreeLabel { + display: flex; + overflow: hidden; + min-width: 0; + flex: 1; + align-items: center; + align-self: stretch; + gap: 4px; +} + +/* Smaller than photon's own sizes so the rows stay compact. The default + * variant's icons are centered, so they scale down cleanly. */ +.sourceFileTreeLabel > .photon-checkbox { + width: 12px; + height: 12px; + flex-shrink: 0; + margin: 0 2px; + background-size: 100%; +} + +.sourceFileTreeName { + overflow: hidden; + color: var(--internal-dimmed-foreground-color); + text-overflow: ellipsis; + white-space: nowrap; +} + +.sourceFileTreeSize { + flex-shrink: 0; + padding: 0 6px 0 12px; + margin-left: auto; + color: var(--internal-dimmed-foreground-color); + font-variant-numeric: tabular-nums; +} + +/* Directory-only rows stay dimmed. */ +.sourceFileTreeNameFile { + color: inherit; +} + +.publishPanelSourceFileTreeContainer { + overflow: auto; + max-height: 180px; + border: 1px solid var(--internal-source-file-tree-border-color); + border-radius: 2px; + margin: 4px 0 0; +} + +.publishPanelSourceCodeSummary { + margin: 4px 0 0; + color: var(--internal-dimmed-foreground-color); +} + +.publishPanelSourceCodeStatus { + display: flex; + align-items: center; + margin: 4px 0 0; + gap: 8px; +} + +.publishPanelSourceCodeStatus > .publishPanelSourceCodeSummary { + margin: 0; +} + +.publishPanelSourceCodeInfoIcon { + width: 14px; + height: 14px; + flex-shrink: 0; + background: var(--internal-info-icon) center no-repeat; + background-size: 14px 14px; +} + +.publishPanelSourceCodeAvailable { + margin-left: 5px; + color: var(--internal-dimmed-foreground-color); +} + +.publishPanelSourceCodePresets { + display: flex; + flex-wrap: wrap; + align-items: center; + padding: 0; + border: none; + margin: 4px 0 0; + gap: 2px 10px; +} + +/* A floated legend isn't rendered on a line of its own above the fieldset, so + * it sits in the row with the presets and still names the group. */ +.publishPanelSourceCodePresetsLegend { + padding: 0; + color: var(--internal-dimmed-foreground-color); + float: left; +} + +.publishPanelSourceCodePresetLabel { + display: flex; + align-items: center; +} diff --git a/src/components/app/MenuButtons/SourceFileTree.tsx b/src/components/app/MenuButtons/SourceFileTree.tsx new file mode 100644 index 0000000000..ae2ce8d82b --- /dev/null +++ b/src/components/app/MenuButtons/SourceFileTree.tsx @@ -0,0 +1,229 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import * as React from 'react'; +import classNames from 'classnames'; + +import { Localized } from '@fluent/react'; + +import prettyBytes from 'firefox-profiler/utils/pretty-bytes'; + +import type { + IndexIntoSourceTable, + SourceFileTreeNode, +} from 'firefox-profiler/types'; + +import './SourceFileTree.css'; + +type CheckState = 'checked' | 'unchecked' | 'mixed'; + +function getCheckState( + sourceIndexes: IndexIntoSourceTable[], + selectedSourceIndexes: ReadonlySet +): CheckState { + let selectedCount = 0; + for (const sourceIndex of sourceIndexes) { + if (selectedSourceIndexes.has(sourceIndex)) { + selectedCount++; + } + } + if (selectedCount === 0) { + return 'unchecked'; + } + return selectedCount === sourceIndexes.length ? 'checked' : 'mixed'; +} + +type TriStateCheckboxProps = { + readonly checkState: CheckState; + readonly disabled?: boolean; + readonly onChange: () => void; + // Overrides the accessible name from the surrounding label, which can be + // ambiguous, like a file name that appears in several directories. + readonly ariaLabel?: string; +}; + +/** + * A checkbox with a "mixed" state. The DOM only exposes it through the + * `indeterminate` property, so it's set from a ref. + */ +class TriStateCheckbox extends React.PureComponent { + _input: HTMLInputElement | null = null; + + _takeInputRef = (input: HTMLInputElement | null) => { + this._input = input; + this._updateIndeterminate(); + }; + + _updateIndeterminate() { + if (this._input !== null) { + this._input.indeterminate = this.props.checkState === 'mixed'; + } + } + + override componentDidUpdate() { + this._updateIndeterminate(); + } + + _onChange = () => { + this.props.onChange(); + }; + + override render() { + const { checkState, disabled, ariaLabel } = this.props; + return ( + + ); + } +} + +// The size column: the size of what's downloaded, or why there is none. +function _renderSize(node: SourceFileTreeNode) { + if (node.downloadedFileCount > 0) { + return prettyBytes(node.byteCount); + } + return ( + + + Unavailable + + + ); +} + +type SourceFileTreeProps = { + readonly nodes: SourceFileTreeNode[]; + readonly selectedSourceIndexes: ReadonlySet; + readonly disabled: boolean; + // Called with the clicked row's source indexes, and whether they should all + // become selected or deselected. + readonly onToggle: ( + sourceIndexes: IndexIntoSourceTable[], + selected: boolean + ) => void; +}; + +/** + * A plain nested list rather than the shared TreeView: the rows carry + * checkboxes, there are no columns, and the list is short enough not to need + * virtualization. + */ +export class SourceFileTree extends React.PureComponent { + override render() { + const { nodes, selectedSourceIndexes, disabled, onToggle } = this.props; + return ( +
    + {nodes.map((node) => ( + + ))} +
+ ); + } +} + +type SourceFileTreeRowProps = { + readonly node: SourceFileTreeNode; + readonly selectedSourceIndexes: ReadonlySet; + readonly disabled: boolean; + readonly onToggle: ( + sourceIndexes: IndexIntoSourceTable[], + selected: boolean + ) => void; +}; + +class SourceFileTreeRow extends React.PureComponent< + SourceFileTreeRowProps, + { isExpanded: boolean } +> { + // Directories with nothing downloaded below them start folded, so that the + // pickable files are in view. + override state = { isExpanded: this.props.node.sourceIndexes.length > 0 }; + + _onToggleExpansion = () => { + this.setState(({ isExpanded }) => ({ isExpanded: !isExpanded })); + }; + + _onToggleCheckbox = () => { + const { node, selectedSourceIndexes, onToggle } = this.props; + const checkState = getCheckState(node.sourceIndexes, selectedSourceIndexes); + // A partially selected row becomes fully selected, matching how the + // checkbox looks after the click. + onToggle(node.sourceIndexes, checkState !== 'checked'); + }; + + override render() { + const { node, selectedSourceIndexes, disabled, onToggle } = this.props; + const { isExpanded } = this.state; + const hasChildren = node.children.length > 0; + const checkState = getCheckState(node.sourceIndexes, selectedSourceIndexes); + // Nothing below this row is downloaded or fetchable, so nothing to pick. + const isPickable = node.sourceIndexes.length > 0; + + return ( +
  • +
    + {hasChildren ? ( +
    + {hasChildren && isExpanded ? ( +
      + {node.children.map((child) => ( + + ))} +
    + ) : null} +
    • + ); + } +} diff --git a/src/profile-logic/sanitize.ts b/src/profile-logic/sanitize.ts index 97a143f927..004d2ca1dc 100644 --- a/src/profile-logic/sanitize.ts +++ b/src/profile-logic/sanitize.ts @@ -63,23 +63,10 @@ export function sanitizePII( maybePIIToBeRemoved: RemoveProfileInformation | null, markerSchemaByName: MarkerSchemaByName ): SanitizeProfileResult { - // FIXME: Currently we always sanitize the source contents while publishing. - // But we should have a sharing option in the publish panel to be able to - // include sources. This needs to happen before the early return below, so - // that the source contents are removed even when no other PII removal is - // requested. - const unconditionallySanitizedShared = { - ...profile.shared, - sources: { - ...profile.shared.sources, - content: Array(profile.shared.sources.content.length).fill(null), - }, - }; - if (maybePIIToBeRemoved === null) { - // Nothing else is sanitized, but the source contents are still removed. + // The user opted into sharing everything, so there's nothing to do. return { - profile: { ...profile, shared: unconditionallySanitizedShared }, + profile, isSanitized: false, translationMaps: null, committedRanges: null, @@ -137,7 +124,7 @@ export function sanitizePII( } const stringTable = StringTable.withBackingArray(stringArray); const newShared = { - ...unconditionallySanitizedShared, + ...profile.shared, stringArray, }; diff --git a/src/reducers/publish.ts b/src/reducers/publish.ts index 896d1e4cc8..0bd17127a8 100644 --- a/src/reducers/publish.ts +++ b/src/reducers/publish.ts @@ -14,6 +14,7 @@ import type { Reducer, State, SanitizedProfileEncodingState, + SourceFileSelection, } from 'firefox-profiler/types'; function _getSanitizingSharingOptions(): CheckedSharingOptions { @@ -80,6 +81,27 @@ const checkedSharingOptions: Reducer< } }; +// `null` means untouched, in which case `getSourceFilesToShare` falls back to +// the default selection. +const sourceFileSelection: Reducer< + Record +> = (state = { download: null, upload: null }, action) => { + switch (action.type) { + case 'PROFILE_LOADED': + return { download: null, upload: null }; + case 'UPDATE_SOURCE_FILES_TO_SHARE': + return { + ...state, + [action.mode]: { + sourceIndexes: action.sourceIndexes, + preset: action.preset, + }, + }; + default: + return state; + } +}; + // This is a diagram explaining the ordering of actions for uploading // // UPLOAD_COMPRESSION_STARTED @@ -266,6 +288,7 @@ const sanitizedProfileEncodingState: Reducer = ( const publishReducer: Reducer = combineReducers({ checkedSharingOptions, + sourceFileSelection, sanitizedProfileEncodingState, upload, isHidingStaleProfile, diff --git a/src/selectors/publish.ts b/src/selectors/publish.ts index 617090e57d..46154a7077 100644 --- a/src/selectors/publish.ts +++ b/src/selectors/publish.ts @@ -22,9 +22,28 @@ import { getShouldSanitizeByDefault as getShouldSanitizeByDefaultImpl, type SanitizeProfileResult, } from '../profile-logic/sanitize'; +import { + buildSourceFileTree, + computePickableSources, + computeSourceCodeSharingCandidates, + computeSourceCodeSharingFiles, + computeSourceCodeSharingStats, + computeSourceCodeSharingSummary, + computeSourceContentsForSharing, + computeSourcesForThreads, + computeStackToSource, + setSharedSourceCode, +} from '../profile-logic/source-sharing'; +import { getSourceCodeCache } from './code'; import { ensureExists } from '../utils/types'; import { formatNumber } from '../utils/format-numbers'; -import { getHiddenGlobalTracks, getHiddenLocalTracksByPid } from './url-state'; +import { + getHiddenGlobalTracks, + getHiddenLocalTracksByPid, + getIsBottomBoxOpen, + getSelectedThreadIndexesOrNull, + getSourceViewSourceIndex, +} from './url-state'; import type { PublishState, @@ -40,6 +59,16 @@ import type { ThreadIndex, CounterIndex, SanitizedProfileEncodingState, + IndexIntoSourceTable, + SourceCodeContents, + SourceCodeSharingCandidate, + SourceCodeSharingFile, + SourceCodeSharingStats, + SourceCodeSharingSummary, + SourceFileTreeNode, + OfferedSourceSharingPreset, + SourceSharingPreset, + Profile, } from 'firefox-profiler/types'; import { getThreadSelectors } from './per-thread'; @@ -87,6 +116,217 @@ export const getFilenameString: Selector = createSelector( } ); +const getSourceContentsForSharing: Selector< + Map +> = createSelector( + getProfile, + getSourceCodeCache, + computeSourceContentsForSharing +); + +// Shared by the file list and the thread scoping, which both need it. +const getStackToSource: Selector = createSelector( + getProfile, + computeStackToSource +); + +const getSourceCodeSharingCandidates: Selector = + createSelector( + getProfile, + getStackToSource, + computeSourceCodeSharingCandidates + ); + +export const getSourceCodeSharingFiles: Selector = + createSelector( + getSourceCodeSharingCandidates, + getSourceContentsForSharing, + computeSourceCodeSharingFiles + ); + +// The files the user can pick: downloaded, or fetchable when sharing. +const getPickableSources: Selector> = + createSelector(getSourceCodeSharingFiles, computePickableSources); + +export const getSourceCodeSharingStats: Selector = + createSelector( + getSourceCodeSharingCandidates, + getSourceCodeSharingFiles, + computeSourceCodeSharingStats + ); + +export const getHasSourceCodeToShare: Selector = createSelector( + getSourceCodeSharingFiles, + (files) => files.length > 0 +); + +export const getSourceFilesForSelectedThreads: Selector< + ReadonlySet +> = createSelector( + getProfile, + // A navigation can replace the UrlState and drop the thread selection. + getSelectedThreadIndexesOrNull, + getStackToSource, + getPickableSources, + (profile, threadIndexes, stackToSource, pickableSources) => + threadIndexes === null + ? new Set() + : computeSourcesForThreads( + profile, + threadIndexes, + stackToSource, + pickableSources + ) +); + +export const getSourceFileTree: Selector = createSelector( + getSourceCodeSharingFiles, + buildSourceFileTree +); + +// The "Current file" preset: the file in the source view, if it's pickable. +export const getSourceFilesForCurrentFile: Selector< + ReadonlySet +> = createSelector( + getPickableSources, + getSourceViewSourceIndex, + (pickableSources, sourceViewSourceIndex) => + sourceViewSourceIndex !== null && pickableSources.has(sourceViewSourceIndex) + ? new Set([sourceViewSourceIndex]) + : new Set() +); + +// The selection the panel starts from. The source view keeps its file after +// the bottom box is closed, so only start from it while it's still visible. +const _getDefaultSourceFilesToShare: Selector< + ReadonlySet +> = createSelector( + getIsBottomBoxOpen, + getSourceFilesForCurrentFile, + (isBottomBoxOpen, currentFileSelection) => + isBottomBoxOpen ? currentFileSelection : new Set() +); + +export const getSourceFilesToShare: DangerousSelectorWithArguments< + ReadonlySet, + SharingMode +> = _perSharingMode((mode) => + createSelector( + (state: State) => getPublishState(state).sourceFileSelection[mode], + getPickableSources, + _getDefaultSourceFilesToShare, + (storedSelection, pickableSources, defaultSelection) => { + if (storedSelection === null) { + return defaultSelection; + } + const selection = new Set(); + for (const sourceIndex of storedSelection.sourceIndexes) { + if (pickableSources.has(sourceIndex)) { + selection.add(sourceIndex); + } + } + return selection; + } + ) +); + +function _areSetsEqual(a: ReadonlySet, b: ReadonlySet): boolean { + if (a.size !== b.size) { + return false; + } + for (const value of a) { + if (!b.has(value)) { + return false; + } + } + return true; +} + +// The files behind every preset, whether offered or not. +const _getSourceFilesForPresets: Selector< + Record> +> = createSelector( + getSourceFilesForCurrentFile, + getSourceFilesForSelectedThreads, + getPickableSources, + (currentFile, selectedThreads, allFiles) => ({ + 'current-file': currentFile, + 'selected-threads': selectedThreads, + 'all-files': allFiles, + }) +); + +/** + * The ready-made selections that are worth offering, and their files, + * narrowest first. A preset is left out when it would select nothing. + * "Selected threads" is also left out when it selects every file. + */ +export const getSourceFilesForOfferedPresets: Selector< + Map> +> = createSelector(_getSourceFilesForPresets, (filesForPreset) => { + const currentFile = filesForPreset['current-file']; + const selectedThreads = filesForPreset['selected-threads']; + const allFiles = filesForPreset['all-files']; + const offered = new Map< + OfferedSourceSharingPreset, + ReadonlySet + >(); + if (currentFile.size > 0) { + offered.set('current-file', currentFile); + } + if (selectedThreads.size > 0 && !_areSetsEqual(selectedThreads, allFiles)) { + offered.set('selected-threads', selectedThreads); + } + if (allFiles.size > 0) { + offered.set('all-files', allFiles); + } + return offered; +}); + +// The offered preset that the selection matches, or 'custom'. +export const getActiveSourceSharingPreset: DangerousSelectorWithArguments< + SourceSharingPreset, + SharingMode +> = _perSharingMode((mode) => + createSelector( + (state: State) => getSourceFilesToShare(state, mode), + (state: State) => + getPublishState(state).sourceFileSelection[mode]?.preset ?? null, + getSourceFilesForOfferedPresets, + (selection, pickedPreset, filesForPresets): SourceSharingPreset => { + // Several presets can select the same files, so the one the user picked + // goes first. Otherwise the narrowest match wins, so that a one-file + // profile reports "Current file" rather than "All available files". + if (pickedPreset !== null) { + const pickedFiles = filesForPresets.get(pickedPreset); + if ( + pickedFiles !== undefined && + _areSetsEqual(selection, pickedFiles) + ) { + return pickedPreset; + } + } + for (const [preset, files] of filesForPresets) { + if (_areSetsEqual(selection, files)) { + return preset; + } + } + return 'custom'; + } + ) +); + +export const getSourceCodeSharingSummary: DangerousSelectorWithArguments< + SourceCodeSharingSummary, + SharingMode +> = _perSharingMode((mode) => + createSelector( + getSourceContentsForSharing, + (state: State) => getSourceFilesToShare(state, mode), + computeSourceCodeSharingSummary + ) +); + export const getRemoveProfileInformation: DangerousSelectorWithArguments< RemoveProfileInformation | null, SharingMode @@ -250,6 +490,21 @@ function getTracedValuesBuffersForAllThreads( return _tracedValuesBuffers; } +/** + * The profile with only the picked source contents in it. This is kept out of + * `sanitizePII` because it doesn't need reindexing, so it shouldn't turn an + * unsanitized upload into a sanitized one. + */ +const _getProfileToShare: DangerousSelectorWithArguments = + _perSharingMode((mode) => + createSelector( + getProfile, + getSourceContentsForSharing, + (state: State) => getSourceFilesToShare(state, mode), + setSharedSourceCode + ) + ); + /** * Run the profile sanitization step, and also get information about how any * UrlState needs to be updated, with things like mapping thread indexes, @@ -260,7 +515,7 @@ export const getSanitizedProfile: DangerousSelectorWithArguments< SharingMode > = _perSharingMode((mode) => createSelector( - getProfile, + (state: State) => _getProfileToShare(state, mode), getDerivedMarkerInfoForAllThreads, getTracedValuesBuffersForAllThreads, (state: State) => getRemoveProfileInformation(state, mode), diff --git a/src/test/components/MenuButtons.test.tsx b/src/test/components/MenuButtons.test.tsx index 6f9dda87c8..744523db5c 100644 --- a/src/test/components/MenuButtons.test.tsx +++ b/src/test/components/MenuButtons.test.tsx @@ -24,6 +24,10 @@ import { retrieveUploadedProfileInformationFromDb, } from 'firefox-profiler/app-logic/uploaded-profiles-db'; import { updateUrlState } from 'firefox-profiler/actions/app'; +import { + changeSelectedThreads, + updateBottomBoxContentsAndMaybeOpen, +} from 'firefox-profiler/actions/profile-view'; import { loadProfile } from 'firefox-profiler/actions/receive-profile'; import { getHash, getDataSource } from 'firefox-profiler/selectors/url-state'; @@ -36,6 +40,8 @@ import { markTabIdsAsPrivateBrowsing, } from '../fixtures/profiles/processed-profile'; import { createGeckoProfile } from '../fixtures/profiles/gecko-profile'; +import { finishLoadingSourceCode } from '../../actions/code'; +import { getSourceCodeCacheGeneration } from 'firefox-profiler/selectors/code'; import { fireFullClick } from '../fixtures/utils'; import { storeWithProfile, blankStore } from '../fixtures/stores'; @@ -106,6 +112,50 @@ describe('app/MenuButtons', function () { return { profile }; } + // Paths on the machine that recorded the profile: there's no way to download + // these files after the profile has been shared, so they're worth embedding. + const LOCAL_SOURCE_FILE = '/home/user/moz/dom/base/nsDocument.cpp'; + const OTHER_SOURCE_FILE = '/home/user/moz/gfx/nsFoo.cpp'; + + function createSourceCodeProfile(updateChannel = 'release') { + const { profile } = getProfileFromTextSamples(` + A[file:${LOCAL_SOURCE_FILE}] A[file:${LOCAL_SOURCE_FILE}] + B[file:${OTHER_SOURCE_FILE}] + `); + profile.meta.updateChannel = updateChannel; + profile.shared.sources.content[0] = 'void nsDocument::Foo() {}'; + profile.shared.sources.content[1] = 'void nsFoo::Bar() {}'; + return { profile }; + } + + // Same files, but only the first one has been downloaded. Two more files in + // another directory take most of the samples and aren't downloaded either. + function createPartlyDownloadedSourceCodeProfile() { + const { profile } = getProfileFromTextSamples(` + A[file:${LOCAL_SOURCE_FILE}] B[file:${OTHER_SOURCE_FILE}] C[file:/opt/lib/c.cpp] D[file:/opt/lib/d.cpp] C[file:/opt/lib/c.cpp] + `); + profile.meta.updateChannel = 'release'; + profile.shared.sources.content[0] = 'void nsDocument::Foo() {}'; + return { profile }; + } + + // Two threads, where the second one only runs code from the second file, so + // that scoping the shared files to the selected threads is a real narrowing + // and the publish panel offers that preset. + function createTwoThreadSourceCodeProfile() { + const { profile } = getProfileFromTextSamples( + ` + A[file:${LOCAL_SOURCE_FILE}] + B[file:${OTHER_SOURCE_FILE}] + `, + `B[file:${OTHER_SOURCE_FILE}]` + ); + profile.meta.updateChannel = 'release'; + profile.shared.sources.content[0] = 'void nsDocument::Foo() {}'; + profile.shared.sources.content[1] = 'void nsFoo::Bar() {}'; + return { profile }; + } + // Gecko emits the `argumentValues` column even when the JS execution tracer // wasn't running, so most profiles carry an all-null column and no buffer. function createEmptyArgumentValuesProfile(updateChannel = 'release') { @@ -146,6 +196,9 @@ describe('app/MenuButtons', function () { }) ) ); + // Replacing the UrlState above drops the thread selection, which the real + // app always has once a profile is being viewed. + store.dispatch(changeSelectedThreads(new Set([0]))); const renderResult = render( @@ -257,6 +310,75 @@ describe('app/MenuButtons', function () { screen.queryByRole('checkbox', { name: /Include JavaScript execution tracing function argument values/, }); + const querySourceCodeCheckbox = () => + screen.queryByRole('checkbox', { + name: /Include source code/, + }); + const getSourceTreeCheckbox = (path: string) => + screen.getByRole('checkbox', { name: path }); + const getSourceFileTreeLabels = () => + Array.from( + document.querySelectorAll('.sourceFileTreeName'), + (element) => element.textContent + ); + const getSourceCodeSummary = () => + ensureExists( + document.querySelector('.publishPanelSourceCodeSummary') + ).textContent?.replace(/[\u2068\u2069]/g, ''); + const getSourceCodeSummaryLines = () => + Array.from( + document.querySelectorAll('.publishPanelSourceCodeSummary'), + (element) => element.textContent?.replace(/[\u2068\u2069]/g, '') + ); + const getSourceCodeLeftOutInfo = () => + document + .querySelector('.publishPanelSourceCodeInfoIcon') + ?.getAttribute('title') + ?.replace(/[\u2068\u2069]/g, '') + .trim(); + const querySourcePresets = () => + document.querySelector('.publishPanelSourceCodePresets'); + const getSourcePresetLabels = () => + Array.from( + document.querySelectorAll('.publishPanelSourceCodePresetLabel'), + (label) => label.textContent + ); + const getCheckedSourcePreset = () => { + const checked = document.querySelector( + '.publishPanelSourceCodePresets input:checked' + ); + return checked === null + ? null + : ensureExists(checked.closest('label')).textContent; + }; + const getSourcePresetRadio = (label: string) => + screen.getByRole('radio', { name: label }); + const getSourceFileSizes = () => + Array.from( + document.querySelectorAll('.sourceFileTreeSize'), + (element) => element.textContent + ); + const enableSourceCode = () => { + fireFullClick(ensureExists(querySourceCodeCheckbox())); + }; + const openSourceView = (sourceIndex: number) => { + act(() => { + setupResult.dispatch( + updateBottomBoxContentsAndMaybeOpen('calltree', { + sourceIndex, + nativeSymbols: [], + initialNativeSymbol: null, + highlightedLineNumber: null, + highlightedInstructionAddress: null, + }) + ); + }); + }; + const selectThread = (threadIndex: number) => { + act(() => { + setupResult.dispatch(changeSelectedThreads(new Set([threadIndex]))); + }); + }; const getPanel = () => screen.getByTestId('PublishPanel-container'); // The Share button opens the panel with the Upload CTA and the submit form. const openPublishPanel = async () => { @@ -281,6 +403,20 @@ describe('app/MenuButtons', function () { getPrivateBrowsingCheckbox, getRemoveOtherTabsCheckbox, queryArgumentValuesCheckbox, + querySourceCodeCheckbox, + getSourceTreeCheckbox, + getSourceFileTreeLabels, + getSourceCodeSummary, + getSourceCodeSummaryLines, + getSourceCodeLeftOutInfo, + querySourcePresets, + getSourcePresetLabels, + getCheckedSourcePreset, + getSourcePresetRadio, + getSourceFileSizes, + enableSourceCode, + selectThread, + openSourceView, openPublishPanel, openDownloadPanel, resolveUpload, @@ -474,6 +610,368 @@ describe('app/MenuButtons', function () { expect(queryArgumentValuesCheckbox()).not.toBeInTheDocument(); }); + it('keeps the file picker out of the way until the option is on', async () => { + const { profile } = createSourceCodeProfile(); + const { + querySourceCodeCheckbox, + getSourceFileTreeLabels, + querySourcePresets, + openPublishPanel, + } = setupForPublish(profile); + await openPublishPanel(); + + const sourceCodeCheckbox = querySourceCodeCheckbox(); + expect(sourceCodeCheckbox).toBeInTheDocument(); + // Nothing is open in the source view, so the option starts unchecked. + expect(sourceCodeCheckbox).not.toBeChecked(); + + // There is nothing to configure yet, so no tree and no presets. The row + // still says how much source code is on offer. + expect(getSourceFileTreeLabels()).toEqual([]); + expect(querySourcePresets()).not.toBeInTheDocument(); + expect( + screen.getByText('(2 files available, 45 B)', { + // Fluent wraps variables in the isolation characters \u2068 and \u2069. + normalizer: (text) => text.replace(/[\u2068\u2069]/g, '').trim(), + }) + ).toBeInTheDocument(); + }); + + it('shows the file tree once the option is on', async () => { + const { profile } = createSourceCodeProfile(); + const { + getSourceFileTreeLabels, + getSourceCodeSummary, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + + // The two files branch off at moz/, and the single-child chains above and + // below that are collapsed into one row each. + expect(getSourceFileTreeLabels()).toEqual([ + 'home/user/moz', + 'dom/base/nsDocument.cpp', + 'gfx/nsFoo.cpp', + ]); + // Only one thread and no source view, so the narrowest thing on offer is + // every file. + expect(getSourceCodeSummary()).toBe('2 files, 45 B uncompressed.'); + }); + + it('turns the option back off without remembering the picked files', async () => { + const { profile } = createSourceCodeProfile(); + const { + querySourceCodeCheckbox, + getSourceFileTreeLabels, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + await openPublishPanel(); + + enableSourceCode(); + expect(querySourceCodeCheckbox()).toBeChecked(); + + enableSourceCode(); + expect(querySourceCodeCheckbox()).not.toBeChecked(); + expect(getSourceFileTreeLabels()).toEqual([]); + }); + + it('does not show the source code checkbox when there is no source code', async () => { + const { profile } = createSimpleProfile(); + const { querySourceCodeCheckbox, openPublishPanel } = + setupForPublish(profile); + await openPublishPanel(); + expect(querySourceCodeCheckbox()).not.toBeInTheDocument(); + }); + + it('does not check the source code checkbox in nightly either', async () => { + // Nightly starts from the mostly-non-sanitizing defaults, so this guards + // against the source code being shared without the user asking for it. + const { profile } = createSourceCodeProfile('nightly'); + const { querySourceCodeCheckbox, openPublishPanel } = + setupForPublish(profile); + await openPublishPanel(); + expect(querySourceCodeCheckbox()).not.toBeChecked(); + }); + + it('lets the user pick single files or whole directories', async () => { + const { profile } = createSourceCodeProfile(); + const { + getSourceTreeCheckbox, + getSourceCodeSummary, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + + fireFullClick(getSourceTreeCheckbox(LOCAL_SOURCE_FILE)); + expect(getSourceCodeSummary()).toBe('1 file, 20 B uncompressed.'); + // One of the directory's two files is picked, so its row is in between. + const directoryCheckbox = getSourceTreeCheckbox( + 'home/user/moz' + ) as HTMLInputElement; + expect(directoryCheckbox).not.toBeChecked(); + expect(directoryCheckbox.indeterminate).toBe(true); + + // Clicking the half-picked directory row takes everything below it. + fireFullClick(directoryCheckbox); + expect(getSourceCodeSummary()).toBe('2 files, 45 B uncompressed.'); + expect(getSourceTreeCheckbox(LOCAL_SOURCE_FILE)).toBeChecked(); + }); + + it('lists files that have not been downloaded, without letting the user pick them', async () => { + const { profile } = createPartlyDownloadedSourceCodeProfile(); + const { + dispatch, + getState, + getSourceTreeCheckbox, + getSourceFileTreeLabels, + getSourceFileSizes, + getSourceCodeSummaryLines, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + await openPublishPanel(); + expect( + screen.getByText('(1 of 4 files downloaded, 25 B)', { + normalizer: (text) => text.replace(/[\u2068\u2069]/g, '').trim(), + }) + ).toBeInTheDocument(); + enableSourceCode(); + + // The directory with the downloaded file comes first and is unfolded, + // even though opt/lib has more samples. opt/lib has nothing to pick, so + // it stays folded and its files aren't rendered. + expect(getSourceFileTreeLabels()).toEqual([ + 'home/user/moz', + 'dom/base/nsDocument.cpp', + 'gfx/nsFoo.cpp', + 'opt/lib', + ]); + // Nothing is downloaded for these, so there's nothing to pick. + const missingCheckbox = getSourceTreeCheckbox(OTHER_SOURCE_FILE); + expect(missingCheckbox).toBeDisabled(); + expect(getSourceTreeCheckbox('opt/lib')).toBeDisabled(); + expect(getSourceFileSizes()).toEqual([ + '25 B', + '25 B', + 'Unavailable', + 'Unavailable', + ]); + expect(getSourceCodeSummaryLines()).toEqual([ + '1 file, 25 B uncompressed.', + ]); + + // Opening the file in the source view downloads it. + act(() => { + dispatch( + finishLoadingSourceCode( + 1, + 'void nsFoo::Bar() {}', + 'LOCAL', + getSourceCodeCacheGeneration(getState()) + ) + ); + }); + expect(missingCheckbox).toBeEnabled(); + expect(getSourceFileSizes()).toEqual([ + '45 B', + '25 B', + '20 B', + 'Unavailable', + ]); + expect(getSourceCodeSummaryLines()).toEqual([ + '1 file, 25 B uncompressed.', + ]); + }); + + it('disables the option until at least one file has been downloaded', async () => { + const { profile } = createSourceCodeProfile(); + profile.shared.sources.content[0] = null; + profile.shared.sources.content[1] = null; + const { querySourceCodeCheckbox, openPublishPanel } = + setupForPublish(profile); + await openPublishPanel(); + + expect(querySourceCodeCheckbox()).toBeDisabled(); + expect( + screen.getByText('(2 files, none downloaded yet)', { + normalizer: (text) => text.replace(/[\u2068\u2069]/g, '').trim(), + }) + ).toBeInTheDocument(); + }); + + it('shows the size of every row in the file tree', async () => { + const { profile } = createSourceCodeProfile(); + const { getSourceFileSizes, enableSourceCode, openPublishPanel } = + setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + + // The directory row adds up its two files, 25 B and 20 B. + expect(getSourceFileSizes()).toEqual(['45 B', '25 B', '20 B']); + }); + + it('does not offer presets when there is only one thing to pick', async () => { + // One thread, source view closed: "current file" picks nothing and + // "selected threads" picks the same files as "all files", so the only + // option left is "all files" and the row would be pointless. + const { profile } = createSourceCodeProfile(); + const { querySourcePresets, enableSourceCode, openPublishPanel } = + setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + expect(querySourcePresets()).not.toBeInTheDocument(); + }); + + it('offers a preset for the selected threads when it narrows things down', async () => { + const { profile } = createTwoThreadSourceCodeProfile(); + const { + getSourcePresetLabels, + getCheckedSourcePreset, + getSourceCodeSummary, + selectThread, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + // Thread 1 only runs code from one of the two files, so scoping to it is + // a real narrowing. Thread 0 runs code from both, which would make the + // preset the same as "All available files". + selectThread(1); + await openPublishPanel(); + enableSourceCode(); + + expect(getSourcePresetLabels()).toEqual([ + 'Selected threads', + 'All available files', + ]); + // Turning the option on starts from the narrowest thing on offer, which + // here is the selected thread's one file rather than both files. + expect(getCheckedSourcePreset()).toBe('Selected threads'); + expect(getSourceCodeSummary()).toBe('1 file, 20 B uncompressed.'); + }); + + it('starts from the file the source view is showing when there is one', async () => { + const { profile } = createTwoThreadSourceCodeProfile(); + const { + openSourceView, + querySourceCodeCheckbox, + getSourcePresetLabels, + getCheckedSourcePreset, + getSourceCodeSummary, + selectThread, + openPublishPanel, + } = setupForPublish(profile); + selectThread(1); + openSourceView(0); + await openPublishPanel(); + + // Having a file open in the source view is enough to start out sharing + // it, so the option is already on. + expect(querySourceCodeCheckbox()).toBeChecked(); + expect(getSourcePresetLabels()).toEqual([ + 'Current file', + 'Selected threads', + 'All available files', + ]); + // The current file wins over the selected thread's files, which is the + // narrower of the two. + expect(getCheckedSourcePreset()).toBe('Current file'); + expect(getSourceCodeSummary()).toBe('1 file, 25 B uncompressed.'); + }); + + it('keeps the picked preset checked when another one selects the same files', async () => { + const { profile } = createTwoThreadSourceCodeProfile(); + const { + openSourceView, + getSourcePresetRadio, + getCheckedSourcePreset, + getSourceTreeCheckbox, + selectThread, + openPublishPanel, + } = setupForPublish(profile); + // Thread 1 only runs code from the second file, which is also the one + // open in the source view. + selectThread(1); + openSourceView(1); + await openPublishPanel(); + expect(getCheckedSourcePreset()).toBe('Current file'); + + fireFullClick(getSourcePresetRadio('Selected threads')); + expect(getCheckedSourcePreset()).toBe('Selected threads'); + + // Editing the tree forgets the picked preset, even when it ends up with + // the same files. + const otherFileCheckbox = getSourceTreeCheckbox(LOCAL_SOURCE_FILE); + fireFullClick(otherFileCheckbox); + fireFullClick(otherFileCheckbox); + expect(getCheckedSourcePreset()).toBe('Current file'); + }); + + it('falls back to Custom once the tree no longer matches a preset', async () => { + const { profile } = createTwoThreadSourceCodeProfile(); + const { + getSourcePresetRadio, + getCheckedSourcePreset, + getSourceTreeCheckbox, + selectThread, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + selectThread(1); + await openPublishPanel(); + enableSourceCode(); + + fireFullClick(getSourcePresetRadio('All available files')); + expect(getCheckedSourcePreset()).toBe('All available files'); + + // Dropping thread 1's file leaves a selection that no preset stands for. + // Dropping the other one would land back on "Selected threads". + fireFullClick(getSourceTreeCheckbox(OTHER_SOURCE_FILE)); + expect(getCheckedSourcePreset()).toBe('Custom'); + + // Picking a preset again gets back out of the custom state. + fireFullClick(getSourcePresetRadio('All available files')); + expect(getCheckedSourcePreset()).toBe('All available files'); + }); + + it('says how many files do not need sharing because they are public', async () => { + const PUBLIC_FILE = + 'hg:hg.mozilla.org/mozilla-central:widget/cocoa/nsAppShell.mm:997f00815e6bc28806b75448c8829f0259d2cb28'; + const { profile } = getProfileFromTextSamples(` + A[file:${LOCAL_SOURCE_FILE}] + B[file:${PUBLIC_FILE}] + `); + profile.meta.updateChannel = 'release'; + profile.shared.sources.content[0] = 'void nsDocument::Foo() {}'; + profile.shared.sources.content[1] = 'nsAppShell contents'; + + const { + getSourceFileTreeLabels, + getSourceCodeSummaryLines, + getSourceCodeLeftOutInfo, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + + // The public file isn't offered, and the panel says so rather than + // leaving its absence unexplained. + expect(getSourceFileTreeLabels()).toEqual([ + 'home/user/moz/dom/base/nsDocument.cpp', + ]); + expect(getSourceCodeSummaryLines()).toEqual([ + '1 file, 25 B uncompressed.', + ]); + expect(getSourceCodeLeftOutInfo()).toBe( + '1 file is available on the web and isn’t included.' + ); + }); + it('can publish and revert', async () => { const { openPublishPanel, getPanelForm, resolveUpload, getState } = setupForPublish(); diff --git a/src/test/store/publish.test.ts b/src/test/store/publish.test.ts index 8d9423af1a..17524880e3 100644 --- a/src/test/store/publish.test.ts +++ b/src/test/store/publish.test.ts @@ -7,9 +7,11 @@ import { encodeSanitizedProfile, resetUploadState, updateSharingOption, + updateSourceFilesToShare, revertToPrePublishedState, } from '../../actions/publish'; import { changeSelectedTab } from '../../actions/app'; +import { finishLoadingSourceCode } from '../../actions/code'; import { viewProfileFromPathInZipFile, returnToZipFileList, @@ -18,6 +20,12 @@ import { getAbortFunction, getCheckedSharingOptions, getRemoveProfileInformation, + getSanitizedProfile, + getSourceFilesToShare, + getSourceCodeSharingFiles, + getHasSourceCodeToShare, + getSourceCodeSharingStats, + getSourceFilesForOfferedPresets, getUploadPhase, getUploadError, getUploadProgress, @@ -34,11 +42,18 @@ import { getHash, getTransformStack, getHiddenGlobalTracks, + getSourceViewSourceIndex, } from '../../selectors/url-state'; +import { + getSourceCodeCache, + getSourceCodeCacheGeneration, + getSourceViewCode, +} from '../../selectors/code'; import { getHasPreferenceMarkers, getHasJSTracingArgumentValues, getGlobalTracks, + getProfile, } from '../../selectors/profile'; import { urlFromState } from '../../app-logic/url-handling'; import { getHasZipFile } from '../../selectors/zipped-profiles'; @@ -61,6 +76,8 @@ import { addTransformToStack, hideGlobalTrack, commitRange, + updateBottomBoxContentsAndMaybeOpen, + closeBottomBox, } from '../../actions/profile-view'; import { changeTabFilter } from '../../actions/receive-profile'; @@ -69,7 +86,16 @@ import { listAllUploadedProfileInformationFromDb, } from 'firefox-profiler/app-logic/uploaded-profiles-db'; -import type { Store, UploadPhase } from 'firefox-profiler/types'; +import type { + IndexIntoSourceTable, + Profile, + State, + Store, + UploadPhase, +} from 'firefox-profiler/types'; + +import { decompress } from 'firefox-profiler/utils/gz'; +import { unserializeProfileOfArbitraryFormat } from 'firefox-profiler/profile-logic/process-profile'; import { autoMockIndexedDB } from 'firefox-profiler/test/fixtures/mocks/indexeddb'; autoMockIndexedDB(); @@ -81,6 +107,20 @@ const { UploadAbortedError } = jest.requireActual( '../../profile-logic/profile-store' ); +function offeredPresets(state: State) { + return [...getSourceFilesForOfferedPresets(state).keys()]; +} + +function openSourceView(sourceIndex: IndexIntoSourceTable) { + return updateBottomBoxContentsAndMaybeOpen('calltree', { + sourceIndex, + nativeSymbols: [], + initialNativeSymbol: null, + highlightedLineNumber: null, + highlightedInstructionAddress: null, + }); +} + describe('getCheckedSharingOptions', function () { describe('default filtering by channel', function () { const isFiltering = { @@ -309,6 +349,100 @@ describe('getRemoveProfileInformation', function () { expect(getRemoveProfileInformation(getState(), 'upload')).toEqual(null); }); }); + + describe('source code', function () { + // A path on the machine that recorded the profile: nobody who opens the + // shared profile can download this file, so it's worth embedding. + const LOCAL_FILE = '/home/user/moz/mozilla-central/dom/base/nsDocument.cpp'; + const SOURCE = 'void nsDocument::Foo() {}'; + + function getStoreWithEmbeddedSource() { + const { profile } = getProfileFromTextSamples(`A[file:${LOCAL_FILE}]`); + profile.shared.sources.content[0] = SOURCE; + return storeWithProfile(profile); + } + + it('should select the file the source view is showing', function () { + const { getState, dispatch } = getStoreWithEmbeddedSource(); + // Simulate opening this file in the source view. + dispatch(openSourceView(0)); + + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set([0])); + expect( + getSanitizedProfile(getState(), 'upload').profile.shared.sources.content + ).toEqual([SOURCE]); + }); + + it('should select nothing once the source view is closed again', function () { + const { getState, dispatch } = getStoreWithEmbeddedSource(); + dispatch(openSourceView(0)); + dispatch(closeBottomBox()); + + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set()); + expect( + getSanitizedProfile(getState(), 'upload').profile.shared.sources.content + ).toEqual([null]); + }); + + it('should list a sampled file before its contents are downloaded', function () { + const { profile } = getProfileFromTextSamples(`A[file:${LOCAL_FILE}]`); + const { getState, dispatch } = storeWithProfile(profile); + + expect(getHasSourceCodeToShare(getState())).toBe(true); + expect(getSourceCodeSharingFiles(getState())).toMatchObject([ + { path: LOCAL_FILE, byteCount: null }, + ]); + expect(getSourceCodeSharingStats(getState())).toMatchObject({ + fileCount: 1, + downloadedFileCount: 0, + }); + // There's nothing to pick yet, so nothing to offer either. + expect(offeredPresets(getState())).toEqual([]); + // Picking the file anyway changes nothing, since we have no contents. + dispatch(updateSourceFilesToShare('upload', new Set([0]))); + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set()); + expect( + getSanitizedProfile(getState(), 'upload').profile.shared.sources.content + ).toEqual([null]); + + dispatch( + finishLoadingSourceCode( + 0, + SOURCE, + 'LOCAL', + getSourceCodeCacheGeneration(getState()) + ) + ); + expect(getSourceCodeSharingFiles(getState())).toMatchObject([ + { path: LOCAL_FILE, byteCount: SOURCE.length }, + ]); + expect(getSourceCodeSharingStats(getState())).toMatchObject({ + fileCount: 1, + downloadedFileCount: 1, + downloadedByteCount: SOURCE.length, + }); + expect(offeredPresets(getState())).toEqual(['all-files']); + // The earlier pick now has contents to go with it. + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set([0])); + }); + + it('should keep the picked files apart for downloading and uploading', function () { + const { getState, dispatch } = getStoreWithEmbeddedSource(); + dispatch(updateSourceFilesToShare('download', new Set([0]))); + + expect(getSourceFilesToShare(getState(), 'download')).toEqual( + new Set([0]) + ); + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set()); + expect( + getSanitizedProfile(getState(), 'download').profile.shared.sources + .content + ).toEqual([SOURCE]); + expect( + getSanitizedProfile(getState(), 'upload').profile.shared.sources.content + ).toEqual([null]); + }); + }); }); describe('attemptToPublish', function () { @@ -319,6 +453,7 @@ describe('attemptToPublish', function () { function setupFakeUpload() { let updateUploadProgress: ((progress: number) => void) | undefined; + let uploadedData: unknown; // Create a promise with the resolve function outside of it. // const { promise, resolve: resolveUpload, reject: rejectUpload } = Promise.withResolvers(); @@ -346,7 +481,8 @@ describe('attemptToPublish', function () { rejectUpload(new UploadAbortedError()); abortFunction(); }, - startUpload: (_data, callback) => { + startUpload: (data, callback) => { + uploadedData = data; updateUploadProgress = callback; return promise; }, @@ -360,7 +496,28 @@ describe('attemptToPublish', function () { ); } + // Inflate and parse the bytes that were handed to the upload, so that tests + // can assert on what actually left the browser. + async function getUploadedProfile(): Promise { + const blob = ensureExists( + uploadedData, + 'Expected the upload to have been started.' + ) as Blob; + // jsdom's Blob has no arrayBuffer(), so go through a FileReader. + const buffer = await new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => resolve(reader.result as ArrayBuffer); + reader.onerror = () => reject(reader.error); + reader.readAsArrayBuffer(blob); + }); + const json = new TextDecoder().decode( + await decompress(new Uint8Array(buffer)) + ); + return unserializeProfileOfArbitraryFormat(json); + } + return { + getUploadedProfile, // @ts-expect-error - TS2454: Variable 'resolveUpload' is used before being assigned. // This is incorrect; new Promise runs its callback synchronously so these variables // are already assigned. Anyway, once we can use Promise.withResolvers, that'll be @@ -429,6 +586,130 @@ describe('attemptToPublish', function () { }); }); + describe('sharing source code', function () { + // A path on the machine that recorded the profile, so there's no way to + // download this file after the profile has been shared. + const LOCAL_FILE = '/home/user/moz/mozilla-central/dom/base/nsDocument.cpp'; + const SOURCE = 'void nsDocument::Foo() {}'; + + function setupWithSourceCode() { + const { profile } = getProfileFromTextSamples(`A[file:${LOCAL_FILE}]`); + const store = storeWithProfile(profile); + // Pretend the user opened this file in the source view. + store.dispatch( + finishLoadingSourceCode( + 0, + SOURCE, + 'LOCAL', + getSourceCodeCacheGeneration(store.getState()) + ) + ); + return setupFakeUploadsWithStore(store); + } + + async function getSourceContentsOfUploadedProfile( + setupResult: ReturnType + ) { + const { + dispatch, + resolveUpload, + assertUploadSuccess, + getUploadedProfile, + } = setupResult; + const publishAttempt = dispatch(attemptToPublish()); + resolveUpload(JWT_TOKEN); + await assertUploadSuccess(publishAttempt); + + const uploadedProfile = await getUploadedProfile(); + const { sources, stringArray } = uploadedProfile.shared; + return new Map( + Array.from(sources.content.entries()).map(([sourceIndex, content]) => [ + stringArray[sources.filename[sourceIndex]], + content, + ]) + ); + } + + it('uploads the source code when the user opted in', async function () { + const setupResult = setupWithSourceCode(); + setupResult.dispatch(updateSourceFilesToShare('upload', new Set([0]))); + + expect(await getSourceContentsOfUploadedProfile(setupResult)).toEqual( + new Map([[LOCAL_FILE, SOURCE]]) + ); + }); + + it('does not upload the source code by default', async function () { + const setupResult = setupWithSourceCode(); + + expect(await getSourceContentsOfUploadedProfile(setupResult)).toEqual( + new Map([[LOCAL_FILE, null]]) + ); + }); + + // Two main threads with a file each, the first one hidden. Sanitizing drops + // the hidden thread along with its source file, so LOCAL_FILE moves from + // index 1 to index 0. + function setupWithHiddenThread() { + const OTHER_FILE = '/home/user/moz/mozilla-central/dom/base/nsNode.cpp'; + const { profile } = getProfileFromTextSamples( + `A[file:${OTHER_FILE}]`, + `B[file:${LOCAL_FILE}]` + ); + profile.meta.updateChannel = 'release'; + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[1].name = 'GeckoMain'; + profile.threads[1].isMainThread = true; + profile.threads[1].pid = '1'; + const store = storeWithProfile(profile); + const generation = getSourceCodeCacheGeneration(store.getState()); + store.dispatch( + finishLoadingSourceCode(0, 'void nsNode::Foo() {}', 'LOCAL', generation) + ); + store.dispatch(finishLoadingSourceCode(1, SOURCE, 'LOCAL', generation)); + store.dispatch(hideGlobalTrack(0)); + return setupFakeUploadsWithStore(store); + } + + it('forgets the picked files once the sources are reindexed', async function () { + const { dispatch, getState, resolveUpload, assertUploadSuccess } = + setupWithHiddenThread(); + + dispatch(updateSourceFilesToShare('upload', new Set([1]))); + const publishAttempt = dispatch(attemptToPublish()); + resolveUpload(JWT_TOKEN); + await assertUploadSuccess(publishAttempt); + + const { sources, stringArray } = getProfile(getState()).shared; + expect(sources.length).toBe(1); + expect(stringArray[sources.filename[0]]).toBe(LOCAL_FILE); + // The old index 1 would now be out of bounds, or point at another file. + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set()); + }); + + it('keeps showing a shared file in the source view after a sanitized publish', async function () { + const { dispatch, getState, resolveUpload, assertUploadSuccess } = + setupWithHiddenThread(); + + dispatch(openSourceView(1)); + dispatch(updateSourceFilesToShare('upload', new Set([1]))); + const publishAttempt = dispatch(attemptToPublish()); + resolveUpload(JWT_TOKEN); + await assertUploadSuccess(publishAttempt); + + // The fetched contents were indexed for the old profile, so they are + // gone, and the source view reads the file from the profile instead. + expect(getSourceCodeCache(getState()).size).toBe(0); + expect(getSourceViewSourceIndex(getState())).toBe(0); + expect(getSourceViewCode(getState())).toEqual({ + type: 'AVAILABLE', + code: SOURCE, + origin: 'PROFILE', + }); + }); + }); + it('works when the server returns a bare hash instead of a JWT token', async function () { const { dispatch, getState, resolveUpload, assertUploadSuccess } = setup(); expect(getUploadPhase(getState())).toEqual('local'); diff --git a/src/test/unit/sanitize.test.ts b/src/test/unit/sanitize.test.ts index 5a00a8aa3e..d962bfe080 100644 --- a/src/test/unit/sanitize.test.ts +++ b/src/test/unit/sanitize.test.ts @@ -1751,15 +1751,29 @@ describe('sanitizePII', function () { ).toEqual([true, false]); }); - it('always removes source contents even when no PII removal is requested', function () { + // Whether the source contents are shared is decided before sanitization, by + // the selectors in src/selectors/publish.ts, so sanitization itself has to + // leave whatever it is handed alone. + it('keeps the source contents it is handed', function () { const { profile } = getProfileFromTextSamples(`A[file:file1.js]`); // There should be only one source. expect(profile.shared.sources.content.length).toEqual(1); // Pretend the source content have been loaded. profile.shared.sources.content[0] = 'source code'; + // Ask for some unrelated sanitization, so that the whole sanitization path + // runs, including the compaction step. + const { sanitizedProfile } = setup({ shouldRemoveUrls: true }, profile); + + expect(sanitizedProfile.shared.sources.content).toEqual(['source code']); + }); + + it('keeps the source contents when no PII removal is requested', function () { + const { profile } = getProfileFromTextSamples(`A[file:file1.js]`); + profile.shared.sources.content[0] = 'source code'; + // A null `RemoveProfileInformation` means the user kept all sharing options - // checked, so no other sanitization happens. + // checked, so nothing is sanitized at all. const { profile: sanitizedProfile } = sanitizePII( profile, [], @@ -1768,9 +1782,6 @@ describe('sanitizePII', function () { {} ); - expect(sanitizedProfile.shared.sources.content.length).toEqual( - profile.shared.sources.content.length - ); - expect(sanitizedProfile.shared.sources.content[0]).toBe(null); + expect(sanitizedProfile.shared.sources.content).toEqual(['source code']); }); }); diff --git a/src/types/actions.ts b/src/types/actions.ts index 7e47951285..4933e7187a 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -28,6 +28,7 @@ import type { ThreadsKey, NativeSymbolInfo, ProfileIndexTranslationMaps, + OfferedSourceSharingPreset, } from './profile-derived'; import type { FuncToFuncsMap } from '../profile-logic/symbolication'; import type { TemporaryError } from '../utils/errors'; @@ -655,6 +656,12 @@ type PublishAction = readonly slug: keyof CheckedSharingOptions; readonly value: boolean; } + | { + readonly type: 'UPDATE_SOURCE_FILES_TO_SHARE'; + readonly mode: SharingMode; + readonly sourceIndexes: ReadonlySet; + readonly preset: OfferedSourceSharingPreset | null; + } | { readonly type: 'SANITIZED_PROFILE_ENCODING_STARTED'; readonly sanitizedProfile: Profile; diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index 8e2a3edadd..b81d9c834b 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -853,6 +853,22 @@ export type SourceCodeSharingSummary = { byteCount: number; }; +/** + * One of the ready-made file selections offered in the publish panel, above the + * file tree. + */ +export type OfferedSourceSharingPreset = + | 'current-file' + | 'selected-threads' + | 'all-files'; + +/** + * The preset the publish panel shows as checked. `custom` isn't offered, it's + * what the panel falls back to once the tree no longer matches any of the + * offered presets. + */ +export type SourceSharingPreset = OfferedSourceSharingPreset | 'custom'; + /** * Type that holds the values of personally identifiable information that user * wants to remove. diff --git a/src/types/state.ts b/src/types/state.ts index c852092afe..c037ba2e72 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -33,6 +33,7 @@ import type { MarkerIndex, ThreadsKey, NativeSymbolInfo, + OfferedSourceSharingPreset, } from './profile-derived'; import type { Attempt } from '../utils/errors'; import type { TransformStacksPerThread } from './transforms'; @@ -239,6 +240,10 @@ export type SanitizedProfileEncodingState = export type PublishState = { readonly checkedSharingOptions: Record; + // The source files the user picked in the publish panel, per sharing mode. + // `null` means the user hasn't touched the selection yet, in which case the + // default selection applies. + readonly sourceFileSelection: Record; readonly sanitizedProfileEncodingState: SanitizedProfileEncodingState; readonly upload: UploadState; readonly isHidingStaleProfile: boolean; @@ -246,6 +251,14 @@ export type PublishState = { readonly prePublishedState: State | null; }; +export type SourceFileSelection = { + readonly sourceIndexes: ReadonlySet; + // The preset the user picked to get this selection, if any. Several presets + // can select the same files, so the files alone can't tell which one to show + // as checked. + readonly preset: OfferedSourceSharingPreset | null; +}; + export type ZippedProfilesState = { zipFile: ZipFileState; error: Error | null; From 22aa1c65291292b10f8cf5dd41ce52f1d6f6f5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:52:53 +0200 Subject: [PATCH 11/15] Show a warning about distribution rights in the share panel The panel didn't mention distribution rights anywhere, and the file in the source view is included by default. When the option is on in the share panel, show a warning bar under the checkbox, where it's read before picking files. The save panel doesn't get it, since saving to disk isn't distribution. --- locales/en-US/app.ftl | 3 +++ .../app/MenuButtons/PublishSourceCode.tsx | 12 ++++++++++- .../app/MenuButtons/SourceFileTree.css | 14 +++++++++++++ src/test/components/MenuButtons.test.tsx | 20 +++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 381f04f7c1..f96c09c197 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -810,6 +810,9 @@ MenuButtons--publish--source-code-preset-all-available-files = All available fil # Not something the user can pick. This is shown as the selected option once the # user has edited the file tree so that it no longer matches any of the others. MenuButtons--publish--source-code-preset-custom = Custom +# Shown under the "Include source code" checkbox in the upload panel, next to a +# warning icon, since the uploaded profile is public. +MenuButtons--publish--source-code-rights-notice = These files will be public. Only include code you have the right to share. # The status line underneath the file tree of the "Include source code" # option: how much source code the picked files add to the shared profile. # Variables: diff --git a/src/components/app/MenuButtons/PublishSourceCode.tsx b/src/components/app/MenuButtons/PublishSourceCode.tsx index 8f34a5f7ab..cd501480ca 100644 --- a/src/components/app/MenuButtons/PublishSourceCode.tsx +++ b/src/components/app/MenuButtons/PublishSourceCode.tsx @@ -262,7 +262,7 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { } override render() { - const { sourceFileTree, sourceFilesToShare, sourceCodeSharingStats } = + const { mode, sourceFileTree, sourceFilesToShare, sourceCodeSharingStats } = this.props; const isSharingSourceCode = sourceFilesToShare.size > 0; @@ -287,6 +287,16 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { {isSharingSourceCode ? ( <> + {mode === 'upload' ? ( +
    + +
    + These files will be public. Only include code you have the + right to share. +
    +     +
    + ) : null} {this._renderPresets()}
    .photon-message-bar-inner-text { + padding: 0; +} + .publishPanelSourceCodeAvailable { margin-left: 5px; color: var(--internal-dimmed-foreground-color); diff --git a/src/test/components/MenuButtons.test.tsx b/src/test/components/MenuButtons.test.tsx index 744523db5c..e91ce7d2d2 100644 --- a/src/test/components/MenuButtons.test.tsx +++ b/src/test/components/MenuButtons.test.tsx @@ -804,6 +804,26 @@ describe('app/MenuButtons', function () { ).toBeInTheDocument(); }); + it('warns about the distribution rights when uploading', async () => { + const { profile } = createSourceCodeProfile(); + const { enableSourceCode, openPublishPanel } = setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + expect( + screen.getByText(/Only include code you have the right to share/) + ).toBeInTheDocument(); + }); + + it('does not bring up the distribution rights when saving to disk', async () => { + const { profile } = createSourceCodeProfile(); + const { enableSourceCode, openDownloadPanel } = setupForPublish(profile); + await openDownloadPanel(); + enableSourceCode(); + expect( + screen.queryByText(/Only include code you have the right to share/) + ).not.toBeInTheDocument(); + }); + it('shows the size of every row in the file tree', async () => { const { profile } = createSourceCodeProfile(); const { getSourceFileSizes, enableSourceCode, openPublishPanel } = From 8d0833e46067d55c60879f1a10546bc122364ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:53:33 +0200 Subject: [PATCH 12/15] Group the browser's own files under one root in the source file tree chrome://, resource:// and moz-src:// files, and the jar: URLs into omni.ja that built-in add-ons load from, each got a root of their own in the publish panel's file tree. Put them under a single root named after the product, with the install path dropped from the omni.ja ones. --- src/profile-logic/source-sharing.ts | 24 ++++++++++++++++---- src/test/unit/source-sharing.test.ts | 33 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/profile-logic/source-sharing.ts b/src/profile-logic/source-sharing.ts index 33bb3ecc3f..663c4512ea 100644 --- a/src/profile-logic/source-sharing.ts +++ b/src/profile-logic/source-sharing.ts @@ -64,6 +64,10 @@ export function computeSourceContentsForSharing( return contents; } +// The URL schemes for files that ship inside the browser. They get a common +// root in the tree, named after the product, rather than one root per scheme. +const INTERNAL_URL_SCHEMES = new Set(['chrome', 'resource', 'moz-src']); + function _splitPath(path: string): string[] { return path.split('/').filter((s) => s !== ''); } @@ -73,10 +77,21 @@ function _splitPath(path: string): string[] { const URL_ORIGIN_REGEX = /^([a-z][a-z0-9+.-]*):\/\/([^/]*)(\/.*)?$/i; // The tree segments for a file. -function _pathSegmentsForFilename(path: string): string[] { +function _pathSegmentsForFilename(path: string, productName: string): string[] { + // Files inside the browser's omni.ja archive, which is the packaged form of + // the chrome:// and resource:// files. The install path in front of it is + // noise. + const omniJarMatch = /^jar:.*\bomni\.ja!(\/.*)$/i.exec(path); + if (omniJarMatch !== null) { + return [productName, 'omni.ja', ..._splitPath(omniJarMatch[1])]; + } + const originMatch = URL_ORIGIN_REGEX.exec(path); if (originMatch !== null) { const [, scheme, host, rest] = originMatch; + if (INTERNAL_URL_SCHEMES.has(scheme.toLowerCase())) { + return [productName, `${scheme}://${host}`, ..._splitPath(rest ?? '')]; + } return [`${scheme}://${host}`, ..._splitPath(rest ?? '')]; } @@ -244,6 +259,7 @@ export function computeSourceCodeSharingCandidates( stackToSource: Int32Array ): SourceCodeSharingCandidate[] { const { sources, stringArray } = profile.shared; + const productName = profile.meta.product || 'Firefox'; const totalWeights = _computeSourceTotalWeights(profile, stackToSource); const candidates = Array.from(totalWeights.keys(), (sourceIndex) => { @@ -252,7 +268,7 @@ export function computeSourceCodeSharingCandidates( return { sourceIndex, path, - pathSegments: _pathSegmentsForFilename(path), + pathSegments: _pathSegmentsForFilename(path, productName), isPubliclyDownloadable: _isPubliclyDownloadable(filename), }; }); @@ -400,8 +416,8 @@ function _finishTreeNode( const label = segments.join('/'); const treePath = parentPath === '' ? label : `${parentPath}/${label}`; - // File rows show the file's real path, which the tree path can differ from, - // for example by dropping the leading "/". + // File rows show the file's real path, which the tree path can differ from: + // it drops the leading "/" and adds the group root for internal URLs. const path = node.files.length > 0 ? node.files[0].path : treePath; const children = Array.from(node.children.values(), (child) => _finishTreeNode(child, treePath) diff --git a/src/test/unit/source-sharing.test.ts b/src/test/unit/source-sharing.test.ts index d276a855bd..4f6e693aa6 100644 --- a/src/test/unit/source-sharing.test.ts +++ b/src/test/unit/source-sharing.test.ts @@ -273,6 +273,39 @@ describe('computeSourceCodeSharingCandidates', function () { expect(filePaths(candidates)).toEqual(['/src/b.cpp', '/src/a.cpp']); }); + it('groups the files that ship inside the browser under one root', function () { + const { profile } = getProfileFromTextSamples(` + A[file:chrome://browser/content/browser.js] + B[file:resource://gre/modules/AppConstants.sys.mjs] + C[file:moz-src:///toolkit/components/foo.mjs] + D[file:https://example.com/app.js] + E[file:jar:file:///Applications/Firefox%20Nightly.app/Contents/Resources/omni.ja!/chrome/toolkit/content/global/bar.js] + F[file:C:\\Users\\me\\win.cpp] + `); + + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + // All of these have the same weight, so they are ordered by path. + expect(candidates.map((c) => c.pathSegments)).toEqual([ + ['C:', 'Users', 'me', 'win.cpp'], + ['Firefox', 'chrome://browser', 'content', 'browser.js'], + ['https://example.com', 'app.js'], + [ + 'Firefox', + 'omni.ja', + 'chrome', + 'toolkit', + 'content', + 'global', + 'bar.js', + ], + ['Firefox', 'moz-src://', 'toolkit', 'components', 'foo.mjs'], + ['Firefox', 'resource://gre', 'modules', 'AppConstants.sys.mjs'], + ]); + }); + it('splits URLs so that the "//" does not become an empty segment', function () { const { profile } = getProfileFromTextSamples( `Ajs[file:https://example.com/static/app.js]` From 4734092c404630b4fea22c5cc0069eb14efdece1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:54:14 +0200 Subject: [PATCH 13/15] Group source-mapped files under the site that served their bundle Source maps name their files relative to the bundle, with schemes like webpack://, or as paths on the build machine. Nothing in those paths says which site the code ran on, so a tree of them started with ".." or "webpack://" at the root. Find the generated bundle for each original file through the func and frame tables' original locations, and put the file under the bundle's origin. The path itself is kept as the source map wrote it. Original files that are web URLs on their own are left where they are. --- src/profile-logic/source-sharing.ts | 97 ++++++++++++++++++++++++++-- src/test/unit/source-sharing.test.ts | 62 ++++++++++++++++++ 2 files changed, 153 insertions(+), 6 deletions(-) diff --git a/src/profile-logic/source-sharing.ts b/src/profile-logic/source-sharing.ts index 663c4512ea..736aa11562 100644 --- a/src/profile-logic/source-sharing.ts +++ b/src/profile-logic/source-sharing.ts @@ -10,7 +10,10 @@ import { SetCollectionBuilder } from 'firefox-profiler/utils/set-collection'; import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; import { getOriginalSourceForFrame } from 'firefox-profiler/profile-logic/profile-data'; +import { FrameFlag, FuncFlag } from 'firefox-profiler/types'; import type { + IndexIntoFuncTable, + IndexIntoSourceLocationTable, IndexIntoSourceTable, Profile, SourceCodeAvailability, @@ -76,8 +79,15 @@ function _splitPath(path: string): string[] { // into empty rows. const URL_ORIGIN_REGEX = /^([a-z][a-z0-9+.-]*):\/\/([^/]*)(\/.*)?$/i; -// The tree segments for a file. -function _pathSegmentsForFilename(path: string, productName: string): string[] { +/** + * The tree segments for a file. `bundleUrl` is set for files that a source + * map produced, and is the URL of the generated file they were mapped from. + */ +function _pathSegmentsForFilename( + path: string, + productName: string, + bundleUrl: string | null +): string[] { // Files inside the browser's omni.ja archive, which is the packaged form of // the chrome:// and resource:// files. The install path in front of it is // noise. @@ -86,17 +96,84 @@ function _pathSegmentsForFilename(path: string, productName: string): string[] { return [productName, 'omni.ja', ..._splitPath(omniJarMatch[1])]; } + let segments; + let isWebUrl = false; const originMatch = URL_ORIGIN_REGEX.exec(path); if (originMatch !== null) { const [, scheme, host, rest] = originMatch; if (INTERNAL_URL_SCHEMES.has(scheme.toLowerCase())) { return [productName, `${scheme}://${host}`, ..._splitPath(rest ?? '')]; } - return [`${scheme}://${host}`, ..._splitPath(rest ?? '')]; + segments = [`${scheme}://${host}`, ..._splitPath(rest ?? '')]; + isWebUrl = /^https?$/i.test(scheme) && host !== ''; + } else { + segments = path.split(/[/\\]/).filter((s) => s !== ''); + if (segments.length === 0) { + segments = [path]; + } } - const segments = path.split(/[/\\]/).filter((s) => s !== ''); - return segments.length > 0 ? segments : [path]; + // Source maps name their files in all sorts of ways: relative to the + // bundle, with a webpack:// scheme, or as paths on the build machine. None + // of those say which site the code ran on, so they go under the site that + // served the bundle, as they are. + const bundleOrigin = _siteOrigin(bundleUrl); + return bundleOrigin !== null && !isWebUrl + ? [bundleOrigin, ...segments] + : segments; +} + +// "://" for URLs with a host, which is what tells sites apart. +function _siteOrigin(url: string | null): string | null { + if (url === null) { + return null; + } + const match = URL_ORIGIN_REGEX.exec(url); + return match !== null && match[2] !== '' ? `${match[1]}://${match[2]}` : null; +} + +/** + * For each file that a source map produced, the generated file it was mapped + * from. Original files are only reachable through funcs and frames, so this + * is a pass over both tables. + */ +function _computeOriginalToGeneratedSource( + profile: Profile +): Map { + const { funcTable, frameTable, sourceLocationTable } = profile.shared; + const originalToGenerated = new Map< + IndexIntoSourceTable, + IndexIntoSourceTable + >(); + if (sourceLocationTable === null) { + return originalToGenerated; + } + const record = ( + locationIndex: IndexIntoSourceLocationTable, + funcIndex: IndexIntoFuncTable + ) => { + if ((funcTable.flags[funcIndex] & FuncFlag.HasSource) === 0) { + return; + } + const original = sourceLocationTable.source[locationIndex]; + if (!originalToGenerated.has(original)) { + originalToGenerated.set(original, funcTable.source[funcIndex]); + } + }; + for (let funcIndex = 0; funcIndex < funcTable.length; funcIndex++) { + if ((funcTable.flags[funcIndex] & FuncFlag.HasOriginalLocation) !== 0) { + record(funcTable.originalLocation[funcIndex], funcIndex); + } + } + for (let frameIndex = 0; frameIndex < frameTable.length; frameIndex++) { + if ((frameTable.flags[frameIndex] & FrameFlag.HasOriginalLocation) !== 0) { + record( + frameTable.originalLocation[frameIndex], + frameTable.func[frameIndex] + ); + } + } + return originalToGenerated; } /** @@ -261,14 +338,22 @@ export function computeSourceCodeSharingCandidates( const { sources, stringArray } = profile.shared; const productName = profile.meta.product || 'Firefox'; const totalWeights = _computeSourceTotalWeights(profile, stackToSource); + const originalToGenerated = _computeOriginalToGeneratedSource(profile); const candidates = Array.from(totalWeights.keys(), (sourceIndex) => { const filename = stringArray[sources.filename[sourceIndex]]; const path = parseFileNameFromSymbolication(filename).path; + const generated = originalToGenerated.get(sourceIndex); + const bundleUrl = + generated !== undefined + ? parseFileNameFromSymbolication( + stringArray[sources.filename[generated]] + ).path + : null; return { sourceIndex, path, - pathSegments: _pathSegmentsForFilename(path, productName), + pathSegments: _pathSegmentsForFilename(path, productName, bundleUrl), isPubliclyDownloadable: _isPubliclyDownloadable(filename), }; }); diff --git a/src/test/unit/source-sharing.test.ts b/src/test/unit/source-sharing.test.ts index 4f6e693aa6..3df2141755 100644 --- a/src/test/unit/source-sharing.test.ts +++ b/src/test/unit/source-sharing.test.ts @@ -14,6 +14,8 @@ import { } from '../../profile-logic/source-sharing'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; import { ensureExists } from '../../utils/types'; +import { getEmptySourceLocationTable } from '../../profile-logic/data-structures'; +import { FuncFlag } from 'firefox-profiler/types'; import type { IndexIntoSourceTable, @@ -306,6 +308,66 @@ describe('computeSourceCodeSharingCandidates', function () { ]); }); + it('puts source-mapped files under the site that served their bundle', function () { + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples(` + A[file:https://example.com/assets/app.js] + B[file:https://example.com/assets/app.js] + C[file:https://example.com/assets/app.js] + D[file:https://example.com/assets/app.js] + `); + const [{ A, B, C, D }] = funcNamesDictPerThread; + const { funcTable, sources, stringArray } = profile.shared; + + // Pretend a source map resolved these funcs to the shapes source maps + // commonly produce. The paths are kept as they are, under the site. + const addOriginal = (funcIndex: number, filename: string) => { + const sourceIndex = sources.length++; + sources.id.push(null); + sources.filename.push(stringArray.push(filename) - 1); + sources.startLine.push(1); + sources.startColumn.push(1); + sources.sourceMapURL.push(null); + sources.content.push(null); + const table = ensureExists(profile.shared.sourceLocationTable); + table.source.push(sourceIndex); + table.line.push(1); + table.column.push(1); + funcTable.flags[funcIndex] |= FuncFlag.HasOriginalLocation; + funcTable.originalLocation[funcIndex] = table.length++; + }; + profile.shared.sourceLocationTable = getEmptySourceLocationTable(); + addOriginal(A, '../node_modules/react-dom/cjs/react-dom.production.js'); + addOriginal(B, 'webpack:///./src/components/App.tsx'); + addOriginal(C, 'webpack://my-app/./src/index.js'); + // A file that is a web URL on its own already says where it came from. + addOriginal(D, 'https://cdn.example.org/lib.js'); + + const candidates = computeSourceCodeSharingCandidates( + profile, + computeStackToSource(profile) + ); + expect(candidates.map((c) => c.pathSegments)).toEqual([ + [ + 'https://example.com', + '..', + 'node_modules', + 'react-dom', + 'cjs', + 'react-dom.production.js', + ], + ['https://cdn.example.org', 'lib.js'], + [ + 'https://example.com', + 'webpack://', + '.', + 'src', + 'components', + 'App.tsx', + ], + ['https://example.com', 'webpack://my-app', '.', 'src', 'index.js'], + ]); + }); + it('splits URLs so that the "//" does not become an empty segment', function () { const { profile } = getProfileFromTextSamples( `Ajs[file:https://example.com/static/app.js]` From d43c2662e94c644c6d233c9675a955251eff0256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:55:32 +0200 Subject: [PATCH 14/15] Fetch the picked source files when sharing Files in the publish panel's tree could only be included once their contents were in hand. Now a file is pickable when the browser connection or a local symbol server can serve it: native files through the symbolication API, given a frame address that symbolicated to them, and JS files from the browser by their source id. Only files nothing can serve show as unavailable, and files whose fetch failed say so. Picked files that aren't downloaded yet are fetched when the upload starts, with progress in the upload panel and the usual cancel. A "Download now" button under the tree does the same on request, which is also the way to get them into a saved profile. Fetches run six at a time and land in the source code cache, so the source view benefits too. If a fetch fails, the upload stops with a message, and the file is left out of the next attempt. The address proofs for all sources are computed in one pass over the frame table and memoized, and the source view uses them too. --- locales/en-US/app.ftl | 31 ++++ src/actions/code.ts | 8 +- src/actions/publish.ts | 95 ++++++++++- src/components/app/MenuButtons/Publish.tsx | 33 +++- .../app/MenuButtons/PublishSourceCode.tsx | 110 ++++++++++--- .../app/MenuButtons/SourceFileTree.css | 5 + .../app/MenuButtons/SourceFileTree.tsx | 14 ++ src/profile-logic/profile-data.ts | 22 +++ src/profile-logic/source-sharing.ts | 79 +++++++-- src/reducers/publish.ts | 31 ++++ src/selectors/profile.ts | 9 ++ src/selectors/publish.ts | 60 +++++++ src/test/components/MenuButtons.test.tsx | 105 +++++++++++- src/test/store/publish.test.ts | 152 +++++++++++++++++- src/test/unit/profile-data.test.ts | 21 +++ src/test/unit/source-sharing.test.ts | 114 +++++++++++-- src/types/actions.ts | 7 + src/types/profile-derived.ts | 24 ++- src/types/state.ts | 13 ++ src/utils/errors.ts | 16 ++ 20 files changed, 887 insertions(+), 62 deletions(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index f96c09c197..551ce822bc 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -793,6 +793,12 @@ MenuButtons--publish--source-code-none-downloaded = [one] ({ $totalFileCount } file, none downloaded yet) *[other] ({ $totalFileCount } files, none downloaded yet) } +# Shown in the size column of the file tree for files whose contents we don't +# have yet. They are downloaded when sharing, if picked. +MenuButtons--publish--source-code-not-downloaded = Not downloaded +# Shown in the size column of the file tree for files that we tried to download +# during this session and couldn't. +MenuButtons--publish--source-code-download-failed = Download failed # Shown in the size column of the file tree for files that can't be downloaded # from here. MenuButtons--publish--source-code-unavailable = Unavailable @@ -823,6 +829,31 @@ MenuButtons--publish--source-code-status = [one] { $fileCount } file, { $size } uncompressed. *[other] { $fileCount } files, { $size } uncompressed. } +# Shown on a line of its own under the previous string when some of the picked +# files still have to be downloaded. A "Download now" button comes after it. +# Variables: +# $fileCount (Number) - Number of picked source files whose contents we don't have +MenuButtons--publish--source-code-status-to-download = + { $fileCount -> + [one] { $fileCount } file will be downloaded when sharing. + *[other] { $fileCount } files will be downloaded when sharing. + } +MenuButtons--publish--source-code-download-now = Download now +# Replaces the status line while the download is underway. +# Variables: +# $completedCount (Number) - Number of source files downloaded so far +# $totalCount (Number) - Number of source files being downloaded +MenuButtons--publish--source-code-downloading = Downloading source files… ({ $completedCount } of { $totalCount }) +MenuButtons--publish--source-code-cancel-download = Cancel +# Shown under the upload error message when the upload stopped because some of +# the picked source files couldn't be downloaded. +# Variables: +# $failedCount (Number) - Number of picked source files that couldn't be downloaded +MenuButtons--publish--source-code-fetch-failed = + { $failedCount -> + [one] { $failedCount } of the picked source files could not be downloaded. It has been left out, so uploading again will share the rest. + *[other] { $failedCount } of the picked source files could not be downloaded. They have been left out, so uploading again will share the rest. + } # A sentence in the tooltip of an info icon at the end of the status line, when # some of the listed files can't be picked because they can't be downloaded # from here. It can be followed by the next string, on a line of its own. diff --git a/src/actions/code.ts b/src/actions/code.ts index bebfea19d0..8369bf0590 100644 --- a/src/actions/code.ts +++ b/src/actions/code.ts @@ -1,7 +1,10 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { getProfile } from 'firefox-profiler/selectors/profile'; +import { + getProfile, + getSourceAddressProofs, +} from 'firefox-profiler/selectors/profile'; import { getBrowserConnection } from 'firefox-profiler/selectors/app'; import { getSymbolServerUrl } from 'firefox-profiler/selectors/url-state'; import { getSourceCodeCacheGeneration } from 'firefox-profiler/selectors/code'; @@ -11,7 +14,6 @@ import { } from 'firefox-profiler/utils/fetch-source'; import { RegularExternalCommunicationDelegate } from 'firefox-profiler/utils/query-api'; import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; -import { findAddressProofForFile } from 'firefox-profiler/profile-logic/profile-data'; import type { Action, ThunkAction, @@ -107,7 +109,7 @@ export function fetchSourceCode( stringArray[sources.filename[sourceIndex]], sources.id[sourceIndex], getSymbolServerUrl(state), - findAddressProofForFile(getProfile(state), sourceIndex), + getSourceAddressProofs(state).get(sourceIndex) ?? null, archiveCache, delegate ); diff --git a/src/actions/publish.ts b/src/actions/publish.ts index 040650bf51..a6701f6073 100644 --- a/src/actions/publish.ts +++ b/src/actions/publish.ts @@ -12,8 +12,11 @@ import { getRemoveProfileInformation, getPrePublishedState, getSanitizedProfileEncodingState, + getSourceFilesToFetch, + getSourceCodeFetchState, getSourceFilesForOfferedPresets, } from 'firefox-profiler/selectors/publish'; +import { fetchSourceCode } from './code'; import { getDataSource, getProfileNameForStorage, @@ -33,6 +36,7 @@ import { } from 'firefox-profiler/selectors/profile'; import { viewProfile } from './receive-profile'; import { ensureExists } from 'firefox-profiler/utils/types'; +import { SourceCodeFetchError } from 'firefox-profiler/utils/errors'; 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'; @@ -384,6 +388,81 @@ export function turnOnSourceCodeSharing(mode: SharingMode): ThunkAction { }; } +export function cancelSourceCodeFetch(): Action { + return { type: 'SOURCE_CODE_FETCH_CANCELLED' }; +} + +type SourceCodeFetchResult = { + failedCount: number; + wasCancelled: boolean; +}; + +// How many source files to fetch at once. +const SOURCE_CODE_FETCH_CONCURRENCY = 6; + +/** + * Fetch the picked source files whose contents we don't have yet into the + * source code cache, so that the shared profile can carry them. This runs + * before an upload, and on request from the publish panel. + */ +export function fetchSourceFilesToShare( + mode: SharingMode +): ThunkAction> { + return async (dispatch, getState) => { + const state = getState(); + const sourceIndexes = Array.from(getSourceFilesToFetch(state, mode)); + if (sourceIndexes.length === 0) { + return { failedCount: 0, wasCancelled: false }; + } + + const { generation } = getSourceCodeFetchState(state); + const isCancelled = () => + getSourceCodeFetchState(getState()).generation !== generation; + const archiveCache = new Map>(); + + dispatch({ + type: 'SOURCE_CODE_FETCH_STARTED', + totalCount: sourceIndexes.length, + }); + + let completedCount = 0; + let failedCount = 0; + let nextIndex = 0; + + const fetchOne = async (sourceIndex: IndexIntoSourceTable) => { + // The outcome goes into the cache even after a cancellation, so that + // the source view doesn't show the file as loading forever. + const result = await dispatch(fetchSourceCode(sourceIndex, archiveCache)); + if (result.type === 'ERROR') { + failedCount++; + } + if (!isCancelled()) { + completedCount++; + dispatch({ type: 'SOURCE_CODE_FETCH_PROGRESS', completedCount }); + } + }; + + const worker = async () => { + while (nextIndex < sourceIndexes.length && !isCancelled()) { + await fetchOne(sourceIndexes[nextIndex++]); + } + }; + const workerCount = Math.min( + SOURCE_CODE_FETCH_CONCURRENCY, + sourceIndexes.length + ); + await Promise.all(Array.from({ length: workerCount }, worker)); + + if (isCancelled()) { + return { failedCount, wasCancelled: true }; + } + dispatch({ type: 'SOURCE_CODE_FETCH_FINISHED' }); + // The profile to share has changed, so encode it again. + dispatch(encodeSanitizedProfile(mode)); + return { failedCount, wasCancelled: false }; + }; +} + /** * This function starts the profile sharing process. * @@ -423,14 +502,24 @@ export function attemptToPublish(): ThunkAction> { // Later on the promise from `startUpload` will get rejected too, and we // handle this in the `catch` block. dispatch({ type: 'UPLOAD_ABORTED' }); + dispatch(cancelSourceCodeFetch()); abortUpload(); }; dispatch(uploadCompressionStarted(abortfunction)); - const sanitizedInformation = getSanitizedProfile( - prePublishedState, - 'upload' + // Picked source files that haven't been downloaded yet go first, since + // the profile to share depends on them. + const sourceFetchResult = await dispatch( + fetchSourceFilesToShare('upload') ); + if (uploadGeneration !== getUploadGeneration(getState())) { + return false; + } + if (sourceFetchResult.failedCount > 0) { + throw new SourceCodeFetchError(sourceFetchResult.failedCount); + } + + const sanitizedInformation = getSanitizedProfile(getState(), 'upload'); const profileEncoding = dispatch(encodeSanitizedProfile('upload')); const encodingResult = await profileEncoding.encodingPromise; diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 97d7099f62..ecb00a52c1 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -28,10 +28,12 @@ import { getShouldSanitizeByDefault, getSanitizedProfileEncodingState, getHasSourceCodeToShare, + getSourceCodeFetchState, } from 'firefox-profiler/selectors/publish'; import { PublishSourceCodeSection } from './PublishSourceCode'; import { BlobUrlLink } from 'firefox-profiler/components/shared/BlobUrlLink'; import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; +import { SourceCodeFetchError } from 'firefox-profiler/utils/errors'; import prettyBytes from 'firefox-profiler/utils/pretty-bytes'; import explicitConnect, { @@ -42,6 +44,7 @@ import type { Profile, CheckedSharingOptions, SharingMode, + SourceCodeFetchState, StartEndRange, UploadPhase, SanitizedProfileEncodingState, @@ -63,6 +66,7 @@ type StateProps = { readonly profileContainsPrivateBrowsingInformation: boolean; readonly profileHasJSTracingArgumentValues: boolean; readonly hasSourceCodeToShare: boolean; + readonly sourceCodeFetchState: SourceCodeFetchState; readonly checkedSharingOptions: CheckedSharingOptions; readonly sanitizedProfileEncodingState: SanitizedProfileEncodingState; readonly downloadFileName: string; @@ -311,6 +315,7 @@ class PublishPanelImpl extends React.PureComponent { abortFunction, downloadFileName, sanitizedProfileEncodingState, + sourceCodeFetchState, } = this.props; return ( @@ -325,7 +330,19 @@ class PublishPanelImpl extends React.PureComponent {
    - {uploadProgressString} + {sourceCodeFetchState.isFetching ? ( + + + + ) : ( + uploadProgressString + )}
    { } else if (typeof error === 'string') { message = error; } + const errorDetails = + error instanceof SourceCodeFetchError ? ( + + {message} + + ) : ( + message + ); return (
    {
    -
    {message}
    +
    {errorDetails}
    ); } @@ -428,6 +456,7 @@ export const PublishPanel = explicitConnect< getContainsPrivateBrowsingInformation(state), profileHasJSTracingArgumentValues: getHasJSTracingArgumentValues(state), hasSourceCodeToShare: getHasSourceCodeToShare(state), + sourceCodeFetchState: getSourceCodeFetchState(state), checkedSharingOptions: getCheckedSharingOptions(state, ownProps.mode), downloadFileName: getFilenameString(state), sanitizedProfileEncodingState: getSanitizedProfileEncodingState(state), diff --git a/src/components/app/MenuButtons/PublishSourceCode.tsx b/src/components/app/MenuButtons/PublishSourceCode.tsx index cd501480ca..02240bd181 100644 --- a/src/components/app/MenuButtons/PublishSourceCode.tsx +++ b/src/components/app/MenuButtons/PublishSourceCode.tsx @@ -7,6 +7,8 @@ import { Localized, useLocalization } from '@fluent/react'; import { updateSourceFilesToShare, + fetchSourceFilesToShare, + cancelSourceCodeFetch, turnOnSourceCodeSharing, encodeSanitizedProfile, } from 'firefox-profiler/actions/publish'; @@ -14,6 +16,8 @@ import { getUploadPhase, getActiveSourceSharingPreset, getSourceCodeSharingStats, + getSourceFilesToFetch, + getSourceCodeFetchState, getSourceCodeSharingSummary, getSourceFileTree, getSourceFilesForOfferedPresets, @@ -28,6 +32,7 @@ import explicitConnect, { import type { IndexIntoSourceTable, SharingMode, + SourceCodeFetchState, SourceCodeSharingStats, SourceCodeSharingSummary, SourceFileTreeNode, @@ -93,6 +98,8 @@ type OwnProps = { type StateProps = { readonly uploadPhase: UploadPhase; readonly sourceCodeSharingStats: SourceCodeSharingStats; + readonly sourceFilesToFetch: ReadonlySet; + readonly sourceCodeFetchState: SourceCodeFetchState; readonly sourceFileTree: SourceFileTreeNode[]; readonly sourceFilesToShare: ReadonlySet; readonly sourceFilesForPresets: Map< @@ -105,6 +112,8 @@ type StateProps = { type DispatchProps = { readonly updateSourceFilesToShare: typeof updateSourceFilesToShare; + readonly fetchSourceFilesToShare: typeof fetchSourceFilesToShare; + readonly cancelSourceCodeFetch: typeof cancelSourceCodeFetch; readonly turnOnSourceCodeSharing: typeof turnOnSourceCodeSharing; readonly encodeSanitizedProfile: typeof encodeSanitizedProfile; }; @@ -150,6 +159,14 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { this.props.encodeSanitizedProfile(mode); }; + _onFetchSourceFiles = () => { + this.props.fetchSourceFilesToShare(this.props.mode); + }; + + _onCancelSourceFetch = () => { + this.props.cancelSourceCodeFetch(); + }; + _onPresetChange = (e: React.ChangeEvent) => { const { mode, sourceFilesForPresets, updateSourceFilesToShare } = this.props; @@ -163,31 +180,84 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { this.props.encodeSanitizedProfile(mode); }; - // One line under the tree: what will be included, and an info icon for the - // files that are left out. + // Under the tree: what will be included, with an info icon for the files that + // are left out, then what still has to be downloaded, with a button to do it + // now. While a download runs, its progress and a cancel instead. _renderStatusRow() { - const { sourceCodeSharingSummary, sourceCodeSharingStats } = this.props; + const { + sourceFilesToFetch, + sourceCodeFetchState, + sourceCodeSharingSummary, + sourceCodeSharingStats, + } = this.props; const { unavailableFileCount, publiclyDownloadableFileCount } = sourceCodeSharingStats; - return ( -
    - + if (sourceCodeFetchState.isFetching) { + return ( +
    - + - - -
    + +
    + ); + } + + return ( + <> +
    + + + + + + +
    + {sourceFilesToFetch.size > 0 ? ( +
    + + + + +
    + ) : null} + ); } @@ -274,10 +344,10 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { className="photon-checkbox photon-checkbox-default" onChange={this._onToggleSourceCode} checked={isSharingSourceCode} - // Nothing can be shared when no file is downloaded. + // Nothing can be shared when no file is downloaded or fetchable. disabled={ this._isUploading() || - sourceCodeSharingStats.downloadedFileCount === 0 + sourceCodeSharingStats.pickableFileCount === 0 } /> @@ -322,6 +392,8 @@ export const PublishSourceCodeSection = explicitConnect< mapStateToProps: (state, { mode }) => ({ uploadPhase: getUploadPhase(state), sourceCodeSharingStats: getSourceCodeSharingStats(state), + sourceFilesToFetch: getSourceFilesToFetch(state, mode), + sourceCodeFetchState: getSourceCodeFetchState(state), sourceFileTree: getSourceFileTree(state), sourceFilesToShare: getSourceFilesToShare(state, mode), sourceFilesForPresets: getSourceFilesForOfferedPresets(state), @@ -330,6 +402,8 @@ export const PublishSourceCodeSection = explicitConnect< }), mapDispatchToProps: { updateSourceFilesToShare, + fetchSourceFilesToShare, + cancelSourceCodeFetch, turnOnSourceCodeSharing, encodeSanitizedProfile, }, diff --git a/src/components/app/MenuButtons/SourceFileTree.css b/src/components/app/MenuButtons/SourceFileTree.css index cd4a8c5bdb..4cf00397d6 100644 --- a/src/components/app/MenuButtons/SourceFileTree.css +++ b/src/components/app/MenuButtons/SourceFileTree.css @@ -125,6 +125,11 @@ margin: 0; } +.publishPanelSourceCodeStatus > .photon-button { + flex-shrink: 0; + white-space: nowrap; +} + .publishPanelSourceCodeInfoIcon { width: 14px; height: 14px; diff --git a/src/components/app/MenuButtons/SourceFileTree.tsx b/src/components/app/MenuButtons/SourceFileTree.tsx index ae2ce8d82b..1dcdc5ce3c 100644 --- a/src/components/app/MenuButtons/SourceFileTree.tsx +++ b/src/components/app/MenuButtons/SourceFileTree.tsx @@ -90,6 +90,20 @@ function _renderSize(node: SourceFileTreeNode) { if (node.downloadedFileCount > 0) { return prettyBytes(node.byteCount); } + if (node.fetchableFileCount > 0) { + return ( + + Not downloaded + + ); + } + if (node.availability === 'FAILED') { + return ( + + Download failed + + ); + } return ( { + const { frameTable, funcTable } = profile.shared; + const proofs = new Map(); + for (let frameIndex = 0; frameIndex < frameTable.length; frameIndex++) { + const sourceIndex = _getAddressProvenSource( + frameTable, + funcTable, + frameIndex + ); + if (sourceIndex !== null && !proofs.has(sourceIndex)) { + proofs.set(sourceIndex, _getAddressProofForFrame(profile, frameIndex)); + } + } + return proofs; +} + /** * Calculate a lower bound for the function size, in bytes, of a native symbol. * This is used when the symbol server does not return a size for a function. diff --git a/src/profile-logic/source-sharing.ts b/src/profile-logic/source-sharing.ts index 736aa11562..9dd2f4665e 100644 --- a/src/profile-logic/source-sharing.ts +++ b/src/profile-logic/source-sharing.ts @@ -12,6 +12,7 @@ import { getOriginalSourceForFrame } from 'firefox-profiler/profile-logic/profil import { FrameFlag, FuncFlag } from 'firefox-profiler/types'; import type { + AddressProof, IndexIntoFuncTable, IndexIntoSourceLocationTable, IndexIntoSourceTable, @@ -252,6 +253,32 @@ export function computeSourcesForThreads( return sources; } +/** + * The sources whose contents could be fetched from here. Native files go + * through the symbolication API, which needs an address proof and either the + * browser connection or a local symbol server. JS files come straight from + * the browser, by their source id. + */ +export function computeFetchableSources( + profile: Profile, + addressProofs: Map, + hasBrowserConnection: boolean, + hasLocalSymbolServer: boolean +): Set { + const fetchable = new Set(); + const { sources } = profile.shared; + const canUseSourceApi = hasBrowserConnection || hasLocalSymbolServer; + for (let sourceIndex = 0; sourceIndex < sources.length; sourceIndex++) { + if ( + (canUseSourceApi && addressProofs.has(sourceIndex)) || + (hasBrowserConnection && sources.id[sourceIndex] !== null) + ) { + fetchable.add(sourceIndex); + } + } + return fetchable; +} + /** * The total (not self) sample weight of each source across all threads, * counting a source once per stack even if it appears several times. Only @@ -373,26 +400,34 @@ export function computeSourceCodeSharingCandidates( /** * The files worth offering in the publish panel: downloaded ones first, then - * the rest, each in candidate order. Files that anybody can download are left - * out, unless their contents came from the user's own machine, in which case - * the public URL may well not work. + * the ones that could be fetched, then the rest, each in candidate order. + * Files that anybody can download are left out, unless their contents came + * from the user's own machine, in which case the public URL may well not work. */ export function computeSourceCodeSharingFiles( candidates: SourceCodeSharingCandidate[], - contents: Map + contents: Map, + fetchableSources: ReadonlySet, + failedSources: ReadonlySet ): SourceCodeSharingFile[] { const byAvailability: Record< SourceCodeAvailability, SourceCodeSharingFile[] - > = { DOWNLOADED: [], UNAVAILABLE: [] }; + > = { DOWNLOADED: [], FETCHABLE: [], FAILED: [], UNAVAILABLE: [] }; for (const candidate of candidates) { const { sourceIndex } = candidate; const fileContents = contents.get(sourceIndex); if (candidate.isPubliclyDownloadable && fileContents?.origin !== 'LOCAL') { continue; } - const availability: SourceCodeAvailability = - fileContents !== undefined ? 'DOWNLOADED' : 'UNAVAILABLE'; + let availability: SourceCodeAvailability = 'UNAVAILABLE'; + if (fileContents !== undefined) { + availability = 'DOWNLOADED'; + } else if (failedSources.has(sourceIndex)) { + availability = 'FAILED'; + } else if (fetchableSources.has(sourceIndex)) { + availability = 'FETCHABLE'; + } byAvailability[availability].push({ sourceIndex, path: candidate.path, @@ -401,7 +436,12 @@ export function computeSourceCodeSharingFiles( byteCount: fileContents !== undefined ? fileContents.code.length : null, }); } - return [...byAvailability.DOWNLOADED, ...byAvailability.UNAVAILABLE]; + return [ + ...byAvailability.DOWNLOADED, + ...byAvailability.FETCHABLE, + ...byAvailability.FAILED, + ...byAvailability.UNAVAILABLE, + ]; } export function computeSourceCodeSharingStats( @@ -410,6 +450,7 @@ export function computeSourceCodeSharingStats( ): SourceCodeSharingStats { let downloadedFileCount = 0; let downloadedByteCount = 0; + let fetchableFileCount = 0; let unavailableFileCount = 0; for (const file of files) { switch (file.availability) { @@ -417,9 +458,14 @@ export function computeSourceCodeSharingStats( downloadedFileCount++; downloadedByteCount += file.byteCount ?? 0; break; + case 'FETCHABLE': + fetchableFileCount++; + break; case 'UNAVAILABLE': unavailableFileCount++; break; + case 'FAILED': + break; default: throw assertExhaustiveCheck(file.availability); } @@ -428,20 +474,25 @@ export function computeSourceCodeSharingStats( fileCount: files.length, downloadedFileCount, downloadedByteCount, + pickableFileCount: downloadedFileCount + fetchableFileCount, unavailableFileCount, publiclyDownloadableFileCount: candidates.length - files.length, }; } /** - * The files the user can pick, which are the downloaded ones. + * The files the user can pick: the downloaded ones and the ones that will be + * fetched when sharing. */ export function computePickableSources( files: SourceCodeSharingFile[] ): Set { const pickable = new Set(); for (const file of files) { - if (file.availability === 'DOWNLOADED') { + if ( + file.availability === 'DOWNLOADED' || + file.availability === 'FETCHABLE' + ) { pickable.add(file.sourceIndex); } } @@ -511,6 +562,7 @@ function _finishTreeNode( const sourceIndexes = []; let byteCount = 0; let downloadedFileCount = 0; + let fetchableFileCount = 0; for (const file of node.files) { switch (file.availability) { case 'DOWNLOADED': @@ -518,6 +570,11 @@ function _finishTreeNode( byteCount += file.byteCount ?? 0; downloadedFileCount++; break; + case 'FETCHABLE': + sourceIndexes.push(file.sourceIndex); + fetchableFileCount++; + break; + case 'FAILED': case 'UNAVAILABLE': break; default: @@ -528,6 +585,7 @@ function _finishTreeNode( sourceIndexes.push(...child.sourceIndexes); byteCount += child.byteCount; downloadedFileCount += child.downloadedFileCount; + fetchableFileCount += child.fetchableFileCount; } return { @@ -538,6 +596,7 @@ function _finishTreeNode( sourceIndexes, byteCount, downloadedFileCount, + fetchableFileCount, children, }; } diff --git a/src/reducers/publish.ts b/src/reducers/publish.ts index 0bd17127a8..cce58ca4f8 100644 --- a/src/reducers/publish.ts +++ b/src/reducers/publish.ts @@ -14,6 +14,7 @@ import type { Reducer, State, SanitizedProfileEncodingState, + SourceCodeFetchState, SourceFileSelection, } from 'firefox-profiler/types'; @@ -286,10 +287,40 @@ const sanitizedProfileEncodingState: Reducer = ( } }; +const sourceCodeFetch: Reducer = ( + state = { + isFetching: false, + totalCount: 0, + completedCount: 0, + generation: 0, + }, + action +) => { + switch (action.type) { + case 'SOURCE_CODE_FETCH_STARTED': + return { + ...state, + isFetching: true, + totalCount: action.totalCount, + completedCount: 0, + }; + case 'SOURCE_CODE_FETCH_PROGRESS': + return { ...state, completedCount: action.completedCount }; + case 'SOURCE_CODE_FETCH_FINISHED': + return { ...state, isFetching: false }; + case 'SOURCE_CODE_FETCH_CANCELLED': + case 'PROFILE_LOADED': + return { ...state, isFetching: false, generation: state.generation + 1 }; + default: + return state; + } +}; + const publishReducer: Reducer = combineReducers({ checkedSharingOptions, sourceFileSelection, sanitizedProfileEncodingState, + sourceCodeFetch, upload, isHidingStaleProfile, hasSanitizedProfile, diff --git a/src/selectors/profile.ts b/src/selectors/profile.ts index c2a1b891eb..fd9668ded8 100644 --- a/src/selectors/profile.ts +++ b/src/selectors/profile.ts @@ -25,6 +25,7 @@ import { computeNativeSymbolTableFromRawNativeSymbolTable, reserveFunctionsForCollapsedResources, computeSamplesTableFromRawSamplesTable, + computeSourceAddressProofs, } from '../profile-logic/profile-data'; import type { IPCMarkerCorrelations } from '../profile-logic/marker-data'; import { @@ -41,6 +42,8 @@ import type { EligibleSource } from '../profile-logic/source-maps/matching'; import type { TabSlug } from '../app-logic/tabs-handling'; import type { + AddressProof, + IndexIntoSourceTable, Profile, RawProfileSharedData, StackTable, @@ -1158,6 +1161,12 @@ export const getSourceViewFile: Selector = createSelector( } ); +// A frame address per source, which the symbolication API needs to serve a +// native file. +export const getSourceAddressProofs: Selector< + Map +> = createSelector(getProfile, computeSourceAddressProofs); + export const getSourceViewSourceId: Selector = createSelector( getSourceTable, UrlState.getSourceViewSourceIndex, diff --git a/src/selectors/publish.ts b/src/selectors/publish.ts index 46154a7077..2a1bd63d17 100644 --- a/src/selectors/publish.ts +++ b/src/selectors/publish.ts @@ -16,6 +16,7 @@ import { getHasJSTracingArgumentValues, getThreads, getMarkerSchemaByName, + getSourceAddressProofs, } from './profile'; import { sanitizePII, @@ -24,6 +25,7 @@ import { } from '../profile-logic/sanitize'; import { buildSourceFileTree, + computeFetchableSources, computePickableSources, computeSourceCodeSharingCandidates, computeSourceCodeSharingFiles, @@ -35,6 +37,8 @@ import { setSharedSourceCode, } from '../profile-logic/source-sharing'; import { getSourceCodeCache } from './code'; +import { getBrowserConnection } from './app'; +import { isLocalURL } from '../utils/url'; import { ensureExists } from '../utils/types'; import { formatNumber } from '../utils/format-numbers'; import { @@ -43,6 +47,7 @@ import { getIsBottomBoxOpen, getSelectedThreadIndexesOrNull, getSourceViewSourceIndex, + getSymbolServerUrl, } from './url-state'; import type { @@ -61,6 +66,7 @@ import type { SanitizedProfileEncodingState, IndexIntoSourceTable, SourceCodeContents, + SourceCodeFetchState, SourceCodeSharingCandidate, SourceCodeSharingFile, SourceCodeSharingStats, @@ -137,10 +143,39 @@ const getSourceCodeSharingCandidates: Selector = computeSourceCodeSharingCandidates ); +// The symbolication API is only asked for sources when it runs locally, since +// the public one doesn't have them. +const getHasLocalSymbolServer: Selector = createSelector( + getSymbolServerUrl, + isLocalURL +); + +const getFetchableSources: Selector> = + createSelector( + getProfile, + getSourceAddressProofs, + (state: State) => getBrowserConnection(state) !== null, + getHasLocalSymbolServer, + computeFetchableSources + ); + +const getFailedSources: Selector> = + createSelector(getSourceCodeCache, (cache) => { + const failed = new Set(); + for (const [sourceIndex, status] of cache) { + if (status.type === 'ERROR') { + failed.add(sourceIndex); + } + } + return failed; + }); + export const getSourceCodeSharingFiles: Selector = createSelector( getSourceCodeSharingCandidates, getSourceContentsForSharing, + getFetchableSources, + getFailedSources, computeSourceCodeSharingFiles ); @@ -327,6 +362,31 @@ export const getSourceCodeSharingSummary: DangerousSelectorWithArguments< ) ); +// The picked files whose contents we don't have yet. They get fetched when +// sharing, or when the user asks for it. +export const getSourceFilesToFetch: DangerousSelectorWithArguments< + ReadonlySet, + SharingMode +> = _perSharingMode((mode) => + createSelector( + (state: State) => getSourceFilesToShare(state, mode), + getSourceContentsForSharing, + (selection, contents) => { + const toFetch = new Set(); + for (const sourceIndex of selection) { + if (!contents.has(sourceIndex)) { + toFetch.add(sourceIndex); + } + } + return toFetch; + } + ) +); + +export const getSourceCodeFetchState: Selector = ( + state +) => getPublishState(state).sourceCodeFetch; + export const getRemoveProfileInformation: DangerousSelectorWithArguments< RemoveProfileInformation | null, SharingMode diff --git a/src/test/components/MenuButtons.test.tsx b/src/test/components/MenuButtons.test.tsx index e91ce7d2d2..1443121d53 100644 --- a/src/test/components/MenuButtons.test.tsx +++ b/src/test/components/MenuButtons.test.tsx @@ -23,7 +23,11 @@ import { persistUploadedProfileInformationToDb, retrieveUploadedProfileInformationFromDb, } from 'firefox-profiler/app-logic/uploaded-profiles-db'; -import { updateUrlState } from 'firefox-profiler/actions/app'; +import { + updateUrlState, + updateBrowserConnectionStatus, +} from 'firefox-profiler/actions/app'; +import type { BrowserConnection } from 'firefox-profiler/app-logic/browser-connection'; import { changeSelectedThreads, updateBottomBoxContentsAndMaybeOpen, @@ -751,7 +755,7 @@ describe('app/MenuButtons', function () { 'gfx/nsFoo.cpp', 'opt/lib', ]); - // Nothing is downloaded for these, so there's nothing to pick. + // Without a browser connection nothing can fetch these files. const missingCheckbox = getSourceTreeCheckbox(OTHER_SOURCE_FILE); expect(missingCheckbox).toBeDisabled(); expect(getSourceTreeCheckbox('opt/lib')).toBeDisabled(); @@ -788,6 +792,103 @@ describe('app/MenuButtons', function () { ]); }); + it('downloads picked files from the browser on request', async () => { + const { profile } = createPartlyDownloadedSourceCodeProfile(); + // The browser knows the second file as a JS source. + profile.shared.sources.id[1] = 'ff6d24c3-b8f5-45cd-a7d3-b643b3292e41'; + const getJSSource = jest.fn(async () => 'void nsFoo::Bar() {}'); + const browserConnection = { + getJSSource, + querySymbolicationApi: async () => { + throw new Error('No source for this address'); + }, + } as unknown as BrowserConnection; + const { + dispatch, + getSourceTreeCheckbox, + getSourceFileSizes, + getSourceCodeSummaryLines, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + act(() => { + dispatch( + updateBrowserConnectionStatus({ + status: 'ESTABLISHED', + browserConnection, + }) + ); + }); + await openPublishPanel(); + enableSourceCode(); + + // Only the file the browser can serve is pickable. + const fetchableCheckbox = getSourceTreeCheckbox(OTHER_SOURCE_FILE); + expect(fetchableCheckbox).toBeEnabled(); + expect(getSourceTreeCheckbox('opt/lib')).toBeDisabled(); + expect(getSourceFileSizes()).toEqual([ + '25 B', + '25 B', + 'Not downloaded', + 'Unavailable', + ]); + + // Turning the option on picked every pickable file, this one included. + expect(fetchableCheckbox).toBeChecked(); + expect(getSourceCodeSummaryLines()).toEqual([ + '1 file, 25 B uncompressed.', + '1 file will be downloaded when sharing.', + ]); + + fireFullClick(screen.getByText('Download now')); + await screen.findByText('20 B'); + expect(getJSSource).toHaveBeenCalledWith( + 'ff6d24c3-b8f5-45cd-a7d3-b643b3292e41' + ); + expect(getSourceFileSizes()).toEqual([ + '45 B', + '25 B', + '20 B', + 'Unavailable', + ]); + expect(getSourceCodeSummaryLines()).toEqual([ + '2 files, 45 B uncompressed.', + ]); + }); + + it('says why the upload stopped when a picked file cannot be downloaded', async () => { + const { profile } = createPartlyDownloadedSourceCodeProfile(); + profile.shared.sources.id[1] = 'ff6d24c3-b8f5-45cd-a7d3-b643b3292e41'; + const browserConnection = { + getJSSource: async () => { + throw new Error('No such source'); + }, + querySymbolicationApi: async () => { + throw new Error('No source for this address'); + }, + } as unknown as BrowserConnection; + const { dispatch, getPanelForm, enableSourceCode, openPublishPanel } = + setupForPublish(profile); + act(() => { + dispatch( + updateBrowserConnectionStatus({ + status: 'ESTABLISHED', + browserConnection, + }) + ); + }); + await openPublishPanel(); + enableSourceCode(); + fireEvent.submit(getPanelForm()); + + fireFullClick(await screen.findByText('Error uploading')); + expect( + await screen.findByText( + /of the picked source files could not be downloaded. It has been left out/ + ) + ).toBeInTheDocument(); + }); + it('disables the option until at least one file has been downloaded', async () => { const { profile } = createSourceCodeProfile(); profile.shared.sources.content[0] = null; diff --git a/src/test/store/publish.test.ts b/src/test/store/publish.test.ts index 17524880e3..5d7516708c 100644 --- a/src/test/store/publish.test.ts +++ b/src/test/store/publish.test.ts @@ -8,9 +8,15 @@ import { resetUploadState, updateSharingOption, updateSourceFilesToShare, + fetchSourceFilesToShare, + cancelSourceCodeFetch, revertToPrePublishedState, } from '../../actions/publish'; -import { changeSelectedTab } from '../../actions/app'; +import { + changeSelectedTab, + updateBrowserConnectionStatus, +} from '../../actions/app'; +import type { BrowserConnection } from 'firefox-profiler/app-logic/browser-connection'; import { finishLoadingSourceCode } from '../../actions/code'; import { viewProfileFromPathInZipFile, @@ -25,6 +31,7 @@ import { getSourceCodeSharingFiles, getHasSourceCodeToShare, getSourceCodeSharingStats, + getSourceCodeFetchState, getSourceFilesForOfferedPresets, getUploadPhase, getUploadError, @@ -95,6 +102,7 @@ import type { } from 'firefox-profiler/types'; import { decompress } from 'firefox-profiler/utils/gz'; +import { SourceCodeFetchError } from 'firefox-profiler/utils/errors'; import { unserializeProfileOfArbitraryFormat } from 'firefox-profiler/profile-logic/process-profile'; import { autoMockIndexedDB } from 'firefox-profiler/test/fixtures/mocks/indexeddb'; @@ -672,6 +680,148 @@ describe('attemptToPublish', function () { return setupFakeUploadsWithStore(store); } + describe('fetching picked files', function () { + const NATIVE_SOURCE = 'void nsDocument::Native() {}'; + + // A fake browser connection whose symbolication API serves one file. + function makeBrowserConnection( + serve: (requestJson: string) => Promise + ) { + const querySymbolicationApi = jest.fn( + async (path: string, requestJson: string) => { + if (path !== '/source/v1') { + throw new Error(`Unexpected API path ${path}`); + } + const source = await serve(requestJson); + return JSON.stringify({ + symbolsLastModified: null, + sourceLastModified: null, + file: JSON.parse(requestJson).file, + source, + }); + } + ); + return { + querySymbolicationApi, + browserConnection: { + querySymbolicationApi, + getJSSource: async () => { + throw new Error('Not a JS source'); + }, + } as unknown as BrowserConnection, + }; + } + + function setupWithBrowserConnection( + serve: (requestJson: string) => Promise = async () => + NATIVE_SOURCE + ) { + // A native frame with an address, which is what the symbolication + // API needs to find the file. + const { profile } = getProfileFromTextSamples( + `A[file:${LOCAL_FILE}][lib:xul][address:1234]` + ); + const store = storeWithProfile(profile); + const { querySymbolicationApi, browserConnection } = + makeBrowserConnection(serve); + store.dispatch( + updateBrowserConnectionStatus({ + status: 'ESTABLISHED', + browserConnection, + }) + ); + return { ...setupFakeUploadsWithStore(store), querySymbolicationApi }; + } + + it('offers a native file for fetching once the browser connection is there', function () { + const { getState } = setupWithBrowserConnection(); + expect(getSourceCodeSharingFiles(getState())).toMatchObject([ + { path: LOCAL_FILE, availability: 'FETCHABLE', byteCount: null }, + ]); + expect(offeredPresets(getState())).toEqual(['all-files']); + }); + + it('downloads the picked files on request', async function () { + const { dispatch, getState, querySymbolicationApi } = + setupWithBrowserConnection(); + dispatch(updateSourceFilesToShare('download', new Set([0]))); + + const result = await dispatch(fetchSourceFilesToShare('download')); + expect(result).toEqual({ failedCount: 0, wasCancelled: false }); + expect(querySymbolicationApi).toHaveBeenCalledTimes(1); + expect(JSON.parse(querySymbolicationApi.mock.calls[0][1])).toEqual({ + debugName: 'xul', + debugId: expect.any(String), + moduleOffset: '0x1234', + file: LOCAL_FILE, + }); + expect(getSourceCodeSharingFiles(getState())).toMatchObject([ + { availability: 'DOWNLOADED', byteCount: NATIVE_SOURCE.length }, + ]); + expect(getSourceCodeFetchState(getState())).toMatchObject({ + isFetching: false, + totalCount: 1, + completedCount: 1, + }); + expect( + getSanitizedProfile(getState(), 'download').profile.shared.sources + .content + ).toEqual([NATIVE_SOURCE]); + }); + + it('downloads the picked files before uploading', async function () { + const setupResult = setupWithBrowserConnection(); + setupResult.dispatch(updateSourceFilesToShare('upload', new Set([0]))); + + expect(await getSourceContentsOfUploadedProfile(setupResult)).toEqual( + new Map([[LOCAL_FILE, NATIVE_SOURCE]]) + ); + }); + + it('stops the upload when a picked file cannot be downloaded', async function () { + const { dispatch, getState } = setupWithBrowserConnection(async () => { + throw new Error('No such file'); + }); + dispatch(updateSourceFilesToShare('upload', new Set([0]))); + + await dispatch(attemptToPublish()); + expect(getUploadPhase(getState())).toBe('error'); + const error = getUploadError(getState()); + expect(error).toBeInstanceOf(SourceCodeFetchError); + expect(error).toHaveProperty('failedCount', 1); + // The file is marked as failed and drops out of the selection, so + // that the next attempt goes through. + expect(getSourceCodeSharingFiles(getState())).toMatchObject([ + { availability: 'FAILED' }, + ]); + expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set()); + }); + + it('can be cancelled', async function () { + let resolveFetch: (source: string) => void = () => {}; + const { dispatch, getState } = setupWithBrowserConnection( + () => new Promise((resolve) => (resolveFetch = resolve)) + ); + dispatch(updateSourceFilesToShare('download', new Set([0]))); + + const fetchPromise = dispatch(fetchSourceFilesToShare('download')); + expect(getSourceCodeFetchState(getState()).isFetching).toBe(true); + dispatch(cancelSourceCodeFetch()); + expect(getSourceCodeFetchState(getState()).isFetching).toBe(false); + + // The late result still lands in the cache, so the file isn't stuck + // as loading, but the fetch reports the cancellation. + resolveFetch(NATIVE_SOURCE); + expect(await fetchPromise).toEqual({ + failedCount: 0, + wasCancelled: true, + }); + expect(getSourceCodeSharingFiles(getState())).toMatchObject([ + { availability: 'DOWNLOADED' }, + ]); + }); + }); + it('forgets the picked files once the sources are reindexed', async function () { const { dispatch, getState, resolveUpload, assertUploadSuccess } = setupWithHiddenThread(); diff --git a/src/test/unit/profile-data.test.ts b/src/test/unit/profile-data.test.ts index d8256f3af6..35d46e6116 100644 --- a/src/test/unit/profile-data.test.ts +++ b/src/test/unit/profile-data.test.ts @@ -22,6 +22,7 @@ import { SampleRelations, extractProfileFilterPageData, findAddressProofForFile, + computeSourceAddressProofs, calculateFunctionSizeLowerBound, computeFrameTableFromRawFrameTable, computeNativeSymbolTableFromRawNativeSymbolTable, @@ -1567,6 +1568,26 @@ describe('findAddressProofForFile', function () { }); }); +describe('computeSourceAddressProofs', function () { + it('finds a frame address for each native file', function () { + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples(` + A[file:/src/a.cpp][lib:xul][address:1234] + B[file:/src/b.cpp] + `); + const [{ A, B }] = funcNamesDictPerThread; + const { funcTable } = profile.shared; + + const proofs = computeSourceAddressProofs(profile); + expect(proofs.get(ensureExists(funcTable.source[A]))).toEqual({ + debugName: 'xul', + breakpadId: expect.any(String), + address: 0x1234, + }); + // B has no address, so nothing can vouch for its file. + expect(proofs.has(ensureExists(funcTable.source[B]))).toBe(false); + }); +}); + describe('calculateFunctionSizeLowerBound', function () { it('calculates the correct minimum size', function () { const { profile, nativeSymbolsDictPerThread } = getProfileFromTextSamples(` diff --git a/src/test/unit/source-sharing.test.ts b/src/test/unit/source-sharing.test.ts index 3df2141755..353401d0ca 100644 --- a/src/test/unit/source-sharing.test.ts +++ b/src/test/unit/source-sharing.test.ts @@ -4,6 +4,7 @@ import { buildSourceFileTree, + computeFetchableSources, computePickableSources, computeSourceCodeSharingCandidates, computeSourceCodeSharingFiles, @@ -12,6 +13,7 @@ import { computeSourcesForThreads, computeStackToSource, } from '../../profile-logic/source-sharing'; +import { computeSourceAddressProofs } from '../../profile-logic/profile-data'; import { getProfileFromTextSamples } from '../fixtures/profiles/processed-profile'; import { ensureExists } from '../../utils/types'; import { getEmptySourceLocationTable } from '../../profile-logic/data-structures'; @@ -385,13 +387,62 @@ describe('computeSourceCodeSharingCandidates', function () { }); }); +describe('computeFetchableSources', function () { + function setupNativeAndJs() { + const { profile, funcNamesDictPerThread } = getProfileFromTextSamples(` + A[file:/src/a.cpp][lib:xul][address:1234] + B[file:https://example.com/app.js] + C[file:/src/c.cpp] + `); + const [{ A, B, C }] = funcNamesDictPerThread; + const { funcTable, sources } = profile.shared; + const native = ensureExists(funcTable.source[A]); + const js = ensureExists(funcTable.source[B]); + const unprovable = ensureExists(funcTable.source[C]); + // The browser knows this JS source by its id. + sources.id[js] = 'ff6d24c3-b8f5-45cd-a7d3-b643b3292e41'; + return { + profile, + proofs: computeSourceAddressProofs(profile), + native, + js, + unprovable, + }; + } + + it('can fetch native files through the API and JS files from the browser', function () { + const { profile, proofs, native, js } = setupNativeAndJs(); + expect(computeFetchableSources(profile, proofs, true, false)).toEqual( + new Set([native, js]) + ); + }); + + it('only gets native files from a local symbol server', function () { + const { profile, proofs, native } = setupNativeAndJs(); + expect(computeFetchableSources(profile, proofs, false, true)).toEqual( + new Set([native]) + ); + }); + + it('can fetch nothing without a browser connection or a local symbol server', function () { + const { profile, proofs } = setupNativeAndJs(); + expect(computeFetchableSources(profile, proofs, false, false)).toEqual( + new Set() + ); + }); +}); + describe('computeSourceCodeSharingFiles', function () { - it('tells the files we have from the ones we do not', function () { + const NONE: ReadonlySet = new Set(); + + it('tells fetchable, failed and unavailable files apart, in that order', function () { const { profile, funcNamesDictPerThread } = getProfileFromTextSamples(` - A[file:/src/a.cpp] B[file:/src/b.cpp] + A[file:/src/a.cpp] B[file:/src/b.cpp] C[file:/src/c.cpp] D[file:/src/d.cpp] `); - const [{ B }] = funcNamesDictPerThread; - const sourceB = ensureExists(profile.shared.funcTable.source[B]); + const [{ B, C, D }] = funcNamesDictPerThread; + const { funcTable } = profile.shared; + const sourceOf = (funcIndex: number) => + ensureExists(funcTable.source[funcIndex]); const candidates = computeSourceCodeSharingCandidates( profile, computeStackToSource(profile) @@ -399,19 +450,29 @@ describe('computeSourceCodeSharingFiles', function () { const files = computeSourceCodeSharingFiles( candidates, - computeSourceContentsForSharing(profile, availableCode([[sourceB, 'b']])) + computeSourceContentsForSharing( + profile, + availableCode([[sourceOf(D), 'd']]) + ), + new Set([sourceOf(B)]), + new Set([sourceOf(C)]) ); expect( files.map((file) => [file.path, file.availability, file.byteCount]) ).toEqual([ - ['/src/b.cpp', 'DOWNLOADED', 1], + ['/src/d.cpp', 'DOWNLOADED', 1], + ['/src/b.cpp', 'FETCHABLE', null], + ['/src/c.cpp', 'FAILED', null], ['/src/a.cpp', 'UNAVAILABLE', null], ]); - expect(computePickableSources(files)).toEqual(new Set([sourceB])); + expect(computePickableSources(files)).toEqual( + new Set([sourceOf(D), sourceOf(B)]) + ); expect(computeSourceCodeSharingStats(candidates, files)).toEqual({ - fileCount: 2, + fileCount: 4, downloadedFileCount: 1, downloadedByteCount: 1, + pickableFileCount: 2, unavailableFileCount: 1, publiclyDownloadableFileCount: 0, }); @@ -420,7 +481,9 @@ describe('computeSourceCodeSharingFiles', function () { it('lists files we have not downloaded, without a size', function () { const { candidates, localSourceIndex } = setup(); - expect(computeSourceCodeSharingFiles(candidates, new Map())).toEqual([ + expect( + computeSourceCodeSharingFiles(candidates, new Map(), NONE, NONE) + ).toEqual([ { sourceIndex: localSourceIndex, path: LOCAL_FILE, @@ -449,7 +512,7 @@ describe('computeSourceCodeSharingFiles', function () { expect(filePaths(candidates)).toEqual(['/src/b.cpp', '/src/a.cpp']); expect( - filePaths(computeSourceCodeSharingFiles(candidates, contents)) + filePaths(computeSourceCodeSharingFiles(candidates, contents, NONE, NONE)) ).toEqual(['/src/a.cpp', '/src/b.cpp']); }); @@ -460,7 +523,12 @@ describe('computeSourceCodeSharingFiles', function () { availableCode([[localSourceIndex, 'int main() {}']]) ); - const files = computeSourceCodeSharingFiles(candidates, contents); + const files = computeSourceCodeSharingFiles( + candidates, + contents, + NONE, + NONE + ); expect(files[0].byteCount).toBe('int main() {}'.length); }); @@ -473,7 +541,7 @@ describe('computeSourceCodeSharingFiles', function () { ); expect( - filePaths(computeSourceCodeSharingFiles(candidates, contents)) + filePaths(computeSourceCodeSharingFiles(candidates, contents, NONE, NONE)) ).toEqual([LOCAL_FILE]); }); @@ -487,7 +555,12 @@ describe('computeSourceCodeSharingFiles', function () { ); // The downloaded file comes first, however the candidates are ordered. - const files = computeSourceCodeSharingFiles(candidates, contents); + const files = computeSourceCodeSharingFiles( + candidates, + contents, + NONE, + NONE + ); expect(filePaths(files)).toEqual([ 'widget/cocoa/nsAppShell.mm', LOCAL_FILE, @@ -550,19 +623,28 @@ describe('buildSourceFileTree', function () { expect(formatTree(tree)).toEqual(['src/thing', ' nested.cpp']); }); - it('does not let files we have no contents for be picked', function () { + it('lets fetchable files be picked, but not unavailable ones', function () { const tree = buildSourceFileTree( - makeFiles(['/src/a.cpp', '/src/b.cpp'], { '/src/b.cpp': 'UNAVAILABLE' }) + makeFiles(['/src/a.cpp', '/src/b.cpp', '/src/c.cpp'], { + '/src/b.cpp': 'FETCHABLE', + '/src/c.cpp': 'UNAVAILABLE', + }) ); const [src] = tree; expect(src).toMatchObject({ - sourceIndexes: [0], + sourceIndexes: [0, 1], byteCount: 1, downloadedFileCount: 1, + fetchableFileCount: 1, availability: null, }); expect(src.children[1]).toMatchObject({ label: 'b.cpp', + availability: 'FETCHABLE', + sourceIndexes: [1], + }); + expect(src.children[2]).toMatchObject({ + label: 'c.cpp', availability: 'UNAVAILABLE', sourceIndexes: [], }); diff --git a/src/types/actions.ts b/src/types/actions.ts index 4933e7187a..31d9ed09f9 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -662,6 +662,13 @@ type PublishAction = readonly sourceIndexes: ReadonlySet; readonly preset: OfferedSourceSharingPreset | null; } + | { readonly type: 'SOURCE_CODE_FETCH_STARTED'; readonly totalCount: number } + | { + readonly type: 'SOURCE_CODE_FETCH_PROGRESS'; + readonly completedCount: number; + } + | { readonly type: 'SOURCE_CODE_FETCH_FINISHED' } + | { readonly type: 'SOURCE_CODE_FETCH_CANCELLED' } | { readonly type: 'SANITIZED_PROFILE_ENCODING_STARTED'; readonly sanitizedProfile: Profile; diff --git a/src/types/profile-derived.ts b/src/types/profile-derived.ts index b81d9c834b..b10cbbf013 100644 --- a/src/types/profile-derived.ts +++ b/src/types/profile-derived.ts @@ -778,11 +778,18 @@ export type SourceCodeSharingCandidate = { }; /** - * Whether the contents of a source file are in hand. + * Whether the contents of a source file are in hand, or could be. * - DOWNLOADED: we have the contents. - * - UNAVAILABLE: we don't, so the file can't be picked. + * - FETCHABLE: we don't, but the browser connection or a local symbol server + * can get them, so the file can be picked and is fetched when sharing. + * - FAILED: a fetch was tried during this session and failed. + * - UNAVAILABLE: nothing we can reach has this file. */ -export type SourceCodeAvailability = 'DOWNLOADED' | 'UNAVAILABLE'; +export type SourceCodeAvailability = + | 'DOWNLOADED' + | 'FETCHABLE' + | 'FAILED' + | 'UNAVAILABLE'; /** * One source file offered in the publish panel. @@ -818,13 +825,14 @@ export type SourceFileTreeNode = { // For a file row, the availability of the file. null for rows that only // stand for directories. availability: SourceCodeAvailability | null; - // Every source index at or below this row that can be picked, so downloaded, - // in tree order. A file row can hold several when the same path was loaded - // with different contents. + // Every source index at or below this row that can be picked, so downloaded + // or fetchable, in tree order. A file row can hold several when the same + // path was loaded with different contents. sourceIndexes: IndexIntoSourceTable[]; // The size of the contents we have for the files at or below this row. byteCount: number; downloadedFileCount: number; + fetchableFileCount: number; children: SourceFileTreeNode[]; }; @@ -838,7 +846,9 @@ export type SourceCodeSharingStats = { // The listed files whose contents we have, and their total size. downloadedFileCount: number; downloadedByteCount: number; - // The listed files whose contents we don't have. + // The listed files that can be picked, so downloaded or fetchable. + pickableFileCount: number; + // The listed files that nothing reachable from here can serve. unavailableFileCount: number; // The sampled files left out of the panel because anybody can download them. publiclyDownloadableFileCount: number; diff --git a/src/types/state.ts b/src/types/state.ts index c037ba2e72..5d08173db3 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -245,6 +245,7 @@ export type PublishState = { // default selection applies. readonly sourceFileSelection: Record; readonly sanitizedProfileEncodingState: SanitizedProfileEncodingState; + readonly sourceCodeFetch: SourceCodeFetchState; readonly upload: UploadState; readonly isHidingStaleProfile: boolean; readonly hasSanitizedProfile: boolean; @@ -259,6 +260,18 @@ export type SourceFileSelection = { readonly preset: OfferedSourceSharingPreset | null; }; +/** + * The progress of fetching the source files picked in the publish panel, which + * happens when sharing or on request. + */ +export type SourceCodeFetchState = { + readonly isFetching: boolean; + readonly totalCount: number; + readonly completedCount: number; + // Bumped when a fetch is cancelled, so that the in-flight one can tell. + readonly generation: number; +}; + export type ZippedProfilesState = { zipFile: ZipFileState; error: Error | null; diff --git a/src/utils/errors.ts b/src/utils/errors.ts index b4a4741092..dbea85902b 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -18,3 +18,19 @@ export class TemporaryError extends Error { this.attempt = attempt; } } + +/** + * Stops an upload when some of the source files picked for sharing could not + * be downloaded. + */ +export class SourceCodeFetchError extends Error { + failedCount: number; + + constructor(failedCount: number) { + super(`${failedCount} of the picked source files could not be downloaded.`); + // Workaround for a babel issue when extending Errors + (this as any).__proto__ = SourceCodeFetchError.prototype; + this.name = 'SourceCodeFetchError'; + this.failedCount = failedCount; + } +} From b554ab6a9ee4de86d19aed5f5f24617e4e524c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 20 Oct 2026 13:56:31 +0200 Subject: [PATCH 15/15] Let the presets stick to files that are already downloaded Now that picked files can be fetched when sharing, "Selected threads" and "All available files" can set off a lot of downloads from one click. Add an "Only already downloaded files" checkbox under the presets, per panel, on by default. With it on, the presets only pick files whose contents are in hand, and toggling it re-applies the active preset. Fetchable files can still be picked by hand in the tree. The checkbox only shows up while some pickable file would have to be fetched. Turning the option on when nothing is downloaded yet would pick nothing, so in that case it falls back to the full presets and unchecks the box. --- locales/en-US/app.ftl | 4 + src/actions/publish.ts | 45 ++++++- .../app/MenuButtons/PublishSourceCode.tsx | 96 +++++++++----- .../app/MenuButtons/SourceFileTree.css | 6 + src/reducers/publish.ts | 17 +++ src/selectors/publish.ts | 119 +++++++++++++----- src/test/components/MenuButtons.test.tsx | 75 ++++++++++- src/test/store/publish.test.ts | 71 ++++++++++- src/types/actions.ts | 5 + src/types/state.ts | 3 + 10 files changed, 371 insertions(+), 70 deletions(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 551ce822bc..52a1e88f63 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -813,6 +813,10 @@ MenuButtons--publish--source-code-preset-selected-threads = Selected threads # Selects every source file that was fetched or came with the profile, and that # can be shared. MenuButtons--publish--source-code-preset-all-available-files = All available files +# A checkbox next to the presets. When checked, "Selected threads" and "All +# available files" only pick files whose contents are already downloaded, so +# that sharing doesn't download anything more. +MenuButtons--publish--source-code-downloaded-only = Only already downloaded files # Not something the user can pick. This is shown as the selected option once the # user has edited the file tree so that it no longer matches any of the others. MenuButtons--publish--source-code-preset-custom = Custom diff --git a/src/actions/publish.ts b/src/actions/publish.ts index a6701f6073..b04158d14b 100644 --- a/src/actions/publish.ts +++ b/src/actions/publish.ts @@ -14,7 +14,9 @@ import { getSanitizedProfileEncodingState, getSourceFilesToFetch, getSourceCodeFetchState, + getActiveSourceSharingPreset, getSourceFilesForOfferedPresets, + getSourceSharingDownloadedOnly, } from 'firefox-profiler/selectors/publish'; import { fetchSourceCode } from './code'; import { @@ -374,11 +376,26 @@ export function encodeSanitizedProfile( } /** - * Turn the "Include source code" option on, picking the narrowest preset. + * Turn the "Include source code" option on, picking the narrowest preset. When + * nothing is downloaded yet, the downloaded-only presets are all empty, so fall + * back to the full ones and show that by turning the downloaded-only option + * off. */ export function turnOnSourceCodeSharing(mode: SharingMode): ThunkAction { return (dispatch, getState) => { - const [firstPreset] = getSourceFilesForOfferedPresets(getState()); + let presets = getSourceFilesForOfferedPresets(getState(), mode); + if ( + presets.size === 0 && + getSourceSharingDownloadedOnly(getState(), mode) + ) { + dispatch({ + type: 'UPDATE_SOURCE_SHARING_DOWNLOADED_ONLY', + mode, + value: false, + }); + presets = getSourceFilesForOfferedPresets(getState(), mode); + } + const [firstPreset] = presets; if (firstPreset === undefined) { return; } @@ -388,6 +405,30 @@ export function turnOnSourceCodeSharing(mode: SharingMode): ThunkAction { }; } +/** + * Turn "only files that are already downloaded" on or off for the presets. + * When a preset is active, the selection follows it to its new meaning. + */ +export function setSourceSharingDownloadedOnly( + mode: SharingMode, + value: boolean +): ThunkAction { + return (dispatch, getState) => { + const activePreset = getActiveSourceSharingPreset(getState(), mode); + dispatch({ type: 'UPDATE_SOURCE_SHARING_DOWNLOADED_ONLY', mode, value }); + if (activePreset === 'custom') { + return; + } + const files = getSourceFilesForOfferedPresets(getState(), mode).get( + activePreset + ); + if (files !== undefined) { + dispatch(updateSourceFilesToShare(mode, files, activePreset)); + dispatch(encodeSanitizedProfile(mode)); + } + }; +} + export function cancelSourceCodeFetch(): Action { return { type: 'SOURCE_CODE_FETCH_CANCELLED' }; } diff --git a/src/components/app/MenuButtons/PublishSourceCode.tsx b/src/components/app/MenuButtons/PublishSourceCode.tsx index 02240bd181..c4aa664811 100644 --- a/src/components/app/MenuButtons/PublishSourceCode.tsx +++ b/src/components/app/MenuButtons/PublishSourceCode.tsx @@ -9,6 +9,7 @@ import { updateSourceFilesToShare, fetchSourceFilesToShare, cancelSourceCodeFetch, + setSourceSharingDownloadedOnly, turnOnSourceCodeSharing, encodeSanitizedProfile, } from 'firefox-profiler/actions/publish'; @@ -18,6 +19,7 @@ import { getSourceCodeSharingStats, getSourceFilesToFetch, getSourceCodeFetchState, + getSourceSharingDownloadedOnly, getSourceCodeSharingSummary, getSourceFileTree, getSourceFilesForOfferedPresets, @@ -100,6 +102,7 @@ type StateProps = { readonly sourceCodeSharingStats: SourceCodeSharingStats; readonly sourceFilesToFetch: ReadonlySet; readonly sourceCodeFetchState: SourceCodeFetchState; + readonly sourceSharingDownloadedOnly: boolean; readonly sourceFileTree: SourceFileTreeNode[]; readonly sourceFilesToShare: ReadonlySet; readonly sourceFilesForPresets: Map< @@ -114,6 +117,7 @@ type DispatchProps = { readonly updateSourceFilesToShare: typeof updateSourceFilesToShare; readonly fetchSourceFilesToShare: typeof fetchSourceFilesToShare; readonly cancelSourceCodeFetch: typeof cancelSourceCodeFetch; + readonly setSourceSharingDownloadedOnly: typeof setSourceSharingDownloadedOnly; readonly turnOnSourceCodeSharing: typeof turnOnSourceCodeSharing; readonly encodeSanitizedProfile: typeof encodeSanitizedProfile; }; @@ -159,6 +163,13 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { this.props.encodeSanitizedProfile(mode); }; + _onToggleDownloadedOnly = (e: React.ChangeEvent) => { + this.props.setSourceSharingDownloadedOnly( + this.props.mode, + e.target.checked + ); + }; + _onFetchSourceFiles = () => { this.props.fetchSourceFilesToShare(this.props.mode); }; @@ -262,11 +273,20 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { } _renderPresets() { - const { mode, activeSourceSharingPreset, sourceFilesForPresets } = - this.props; - if (sourceFilesForPresets.size < 2) { - return null; - } + const { + mode, + activeSourceSharingPreset, + sourceFilesForPresets, + sourceCodeSharingStats, + sourceSharingDownloadedOnly, + } = this.props; + + const showPresets = sourceFilesForPresets.size >= 2; + // The downloaded-only option only matters while some pickable file would + // have to be fetched. + const showDownloadedOnly = + sourceCodeSharingStats.pickableFileCount > + sourceCodeSharingStats.downloadedFileCount; const presets: SourceSharingPreset[] = [...sourceFilesForPresets.keys()]; if (activeSourceSharingPreset === 'custom') { @@ -274,31 +294,49 @@ class PublishSourceCodeSectionImpl extends React.PureComponent { } return ( -
    - - - Include: - - - {presets.map((preset) => ( -
    + ) : null} + ); } @@ -394,9 +432,10 @@ export const PublishSourceCodeSection = explicitConnect< sourceCodeSharingStats: getSourceCodeSharingStats(state), sourceFilesToFetch: getSourceFilesToFetch(state, mode), sourceCodeFetchState: getSourceCodeFetchState(state), + sourceSharingDownloadedOnly: getSourceSharingDownloadedOnly(state, mode), sourceFileTree: getSourceFileTree(state), sourceFilesToShare: getSourceFilesToShare(state, mode), - sourceFilesForPresets: getSourceFilesForOfferedPresets(state), + sourceFilesForPresets: getSourceFilesForOfferedPresets(state, mode), activeSourceSharingPreset: getActiveSourceSharingPreset(state, mode), sourceCodeSharingSummary: getSourceCodeSharingSummary(state, mode), }), @@ -404,6 +443,7 @@ export const PublishSourceCodeSection = explicitConnect< updateSourceFilesToShare, fetchSourceFilesToShare, cancelSourceCodeFetch, + setSourceSharingDownloadedOnly, turnOnSourceCodeSharing, encodeSanitizedProfile, }, diff --git a/src/components/app/MenuButtons/SourceFileTree.css b/src/components/app/MenuButtons/SourceFileTree.css index 4cf00397d6..d5c5bcc435 100644 --- a/src/components/app/MenuButtons/SourceFileTree.css +++ b/src/components/app/MenuButtons/SourceFileTree.css @@ -175,6 +175,12 @@ float: left; } +.publishPanelSourceCodeDownloadedOnly { + display: flex; + align-items: center; + margin: 2px 0 0; +} + .publishPanelSourceCodePresetLabel { display: flex; align-items: center; diff --git a/src/reducers/publish.ts b/src/reducers/publish.ts index cce58ca4f8..3545683aeb 100644 --- a/src/reducers/publish.ts +++ b/src/reducers/publish.ts @@ -103,6 +103,22 @@ const sourceFileSelection: Reducer< } }; +// On by default, so that picking a preset doesn't download anything unless +// the user asks for it. +const sourceSharingDownloadedOnly: Reducer> = ( + state = { download: true, upload: true }, + action +) => { + switch (action.type) { + case 'PROFILE_LOADED': + return { download: true, upload: true }; + case 'UPDATE_SOURCE_SHARING_DOWNLOADED_ONLY': + return { ...state, [action.mode]: action.value }; + default: + return state; + } +}; + // This is a diagram explaining the ordering of actions for uploading // // UPLOAD_COMPRESSION_STARTED @@ -319,6 +335,7 @@ const sourceCodeFetch: Reducer = ( const publishReducer: Reducer = combineReducers({ checkedSharingOptions, sourceFileSelection, + sourceSharingDownloadedOnly, sanitizedProfileEncodingState, sourceCodeFetch, upload, diff --git a/src/selectors/publish.ts b/src/selectors/publish.ts index 2a1bd63d17..3cfaa38d3d 100644 --- a/src/selectors/publish.ts +++ b/src/selectors/publish.ts @@ -277,18 +277,60 @@ function _areSetsEqual(a: ReadonlySet, b: ReadonlySet): boolean { return true; } -// The files behind every preset, whether offered or not. -const _getSourceFilesForPresets: Selector< - Record> -> = createSelector( - getSourceFilesForCurrentFile, - getSourceFilesForSelectedThreads, - getPickableSources, - (currentFile, selectedThreads, allFiles) => ({ - 'current-file': currentFile, - 'selected-threads': selectedThreads, - 'all-files': allFiles, - }) +export const getSourceSharingDownloadedOnly: DangerousSelectorWithArguments< + boolean, + SharingMode +> = (state, mode) => getPublishState(state).sourceSharingDownloadedOnly[mode]; + +function _intersect( + a: ReadonlySet, + b: { has(value: T): boolean } +): Set { + const result = new Set(); + for (const value of a) { + if (b.has(value)) { + result.add(value); + } + } + return result; +} + +// The files behind every preset, whether offered or not. With "downloaded +// only" on, each is narrowed to the files whose contents we already have. +const _getSourceFilesForPresets: DangerousSelectorWithArguments< + Record>, + SharingMode +> = _perSharingMode((mode) => + createSelector( + getSourceFilesForCurrentFile, + getSourceFilesForSelectedThreads, + getPickableSources, + getSourceContentsForSharing, + (state: State) => getSourceSharingDownloadedOnly(state, mode), + ( + currentFile, + selectedThreads, + allFiles, + contents, + downloadedOnly + ): Record< + OfferedSourceSharingPreset, + ReadonlySet + > => { + if (!downloadedOnly) { + return { + 'current-file': currentFile, + 'selected-threads': selectedThreads, + 'all-files': allFiles, + }; + } + return { + 'current-file': _intersect(currentFile, contents), + 'selected-threads': _intersect(selectedThreads, contents), + 'all-files': _intersect(allFiles, contents), + }; + } + ) ); /** @@ -296,27 +338,36 @@ const _getSourceFilesForPresets: Selector< * narrowest first. A preset is left out when it would select nothing. * "Selected threads" is also left out when it selects every file. */ -export const getSourceFilesForOfferedPresets: Selector< - Map> -> = createSelector(_getSourceFilesForPresets, (filesForPreset) => { - const currentFile = filesForPreset['current-file']; - const selectedThreads = filesForPreset['selected-threads']; - const allFiles = filesForPreset['all-files']; - const offered = new Map< - OfferedSourceSharingPreset, - ReadonlySet - >(); - if (currentFile.size > 0) { - offered.set('current-file', currentFile); - } - if (selectedThreads.size > 0 && !_areSetsEqual(selectedThreads, allFiles)) { - offered.set('selected-threads', selectedThreads); - } - if (allFiles.size > 0) { - offered.set('all-files', allFiles); - } - return offered; -}); +export const getSourceFilesForOfferedPresets: DangerousSelectorWithArguments< + Map>, + SharingMode +> = _perSharingMode((mode) => + createSelector( + (state: State) => _getSourceFilesForPresets(state, mode), + (filesForPreset) => { + const currentFile = filesForPreset['current-file']; + const selectedThreads = filesForPreset['selected-threads']; + const allFiles = filesForPreset['all-files']; + const offered = new Map< + OfferedSourceSharingPreset, + ReadonlySet + >(); + if (currentFile.size > 0) { + offered.set('current-file', currentFile); + } + if ( + selectedThreads.size > 0 && + !_areSetsEqual(selectedThreads, allFiles) + ) { + offered.set('selected-threads', selectedThreads); + } + if (allFiles.size > 0) { + offered.set('all-files', allFiles); + } + return offered; + } + ) +); // The offered preset that the selection matches, or 'custom'. export const getActiveSourceSharingPreset: DangerousSelectorWithArguments< @@ -327,7 +378,7 @@ export const getActiveSourceSharingPreset: DangerousSelectorWithArguments< (state: State) => getSourceFilesToShare(state, mode), (state: State) => getPublishState(state).sourceFileSelection[mode]?.preset ?? null, - getSourceFilesForOfferedPresets, + (state: State) => getSourceFilesForOfferedPresets(state, mode), (selection, pickedPreset, filesForPresets): SourceSharingPreset => { // Several presets can select the same files, so the one the user picked // goes first. Otherwise the narrowest match wins, so that a one-file diff --git a/src/test/components/MenuButtons.test.tsx b/src/test/components/MenuButtons.test.tsx index 1443121d53..1b8817e050 100644 --- a/src/test/components/MenuButtons.test.tsx +++ b/src/test/components/MenuButtons.test.tsx @@ -833,8 +833,10 @@ describe('app/MenuButtons', function () { 'Unavailable', ]); - // Turning the option on picked every pickable file, this one included. - expect(fetchableCheckbox).toBeChecked(); + // The presets stick to downloaded files by default, so this one has to + // be picked by hand. + expect(fetchableCheckbox).not.toBeChecked(); + fireFullClick(fetchableCheckbox); expect(getSourceCodeSummaryLines()).toEqual([ '1 file, 25 B uncompressed.', '1 file will be downloaded when sharing.', @@ -867,8 +869,13 @@ describe('app/MenuButtons', function () { throw new Error('No source for this address'); }, } as unknown as BrowserConnection; - const { dispatch, getPanelForm, enableSourceCode, openPublishPanel } = - setupForPublish(profile); + const { + dispatch, + getPanelForm, + getSourceTreeCheckbox, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); act(() => { dispatch( updateBrowserConnectionStatus({ @@ -879,6 +886,7 @@ describe('app/MenuButtons', function () { }); await openPublishPanel(); enableSourceCode(); + fireFullClick(getSourceTreeCheckbox(OTHER_SOURCE_FILE)); fireEvent.submit(getPanelForm()); fireFullClick(await screen.findByText('Error uploading')); @@ -905,6 +913,65 @@ describe('app/MenuButtons', function () { ).toBeInTheDocument(); }); + it('can keep the presets to the files that are already downloaded', async () => { + const { profile } = createPartlyDownloadedSourceCodeProfile(); + profile.shared.sources.id[1] = 'ff6d24c3-b8f5-45cd-a7d3-b643b3292e41'; + const browserConnection = { + getJSSource: async () => 'void nsFoo::Bar() {}', + querySymbolicationApi: async () => { + throw new Error('No source for this address'); + }, + } as unknown as BrowserConnection; + const { + dispatch, + getSourceTreeCheckbox, + getSourceCodeSummaryLines, + enableSourceCode, + openPublishPanel, + } = setupForPublish(profile); + act(() => { + dispatch( + updateBrowserConnectionStatus({ + status: 'ESTABLISHED', + browserConnection, + }) + ); + }); + await openPublishPanel(); + enableSourceCode(); + + // On by default: "All available files" picked only the downloaded one. + const downloadedOnly = screen.getByRole('checkbox', { + name: 'Only already downloaded files', + }); + expect(downloadedOnly).toBeChecked(); + expect(getSourceTreeCheckbox(LOCAL_SOURCE_FILE)).toBeChecked(); + expect(getSourceTreeCheckbox(OTHER_SOURCE_FILE)).not.toBeChecked(); + expect(getSourceCodeSummaryLines()).toEqual([ + '1 file, 25 B uncompressed.', + ]); + + // Turning it off widens the active preset to the fetchable file. + fireFullClick(downloadedOnly); + expect(getSourceTreeCheckbox(OTHER_SOURCE_FILE)).toBeChecked(); + expect(getSourceCodeSummaryLines()).toEqual([ + '1 file, 25 B uncompressed.', + '1 file will be downloaded when sharing.', + ]); + }); + + it('does not offer the downloaded-only option when every file is downloaded', async () => { + const { profile } = createSourceCodeProfile(); + const { enableSourceCode, openPublishPanel } = setupForPublish(profile); + await openPublishPanel(); + enableSourceCode(); + expect( + screen.queryByRole('checkbox', { + name: 'Only already downloaded files', + }) + ).not.toBeInTheDocument(); + }); + it('warns about the distribution rights when uploading', async () => { const { profile } = createSourceCodeProfile(); const { enableSourceCode, openPublishPanel } = setupForPublish(profile); diff --git a/src/test/store/publish.test.ts b/src/test/store/publish.test.ts index 5d7516708c..3e9c068218 100644 --- a/src/test/store/publish.test.ts +++ b/src/test/store/publish.test.ts @@ -10,6 +10,8 @@ import { updateSourceFilesToShare, fetchSourceFilesToShare, cancelSourceCodeFetch, + setSourceSharingDownloadedOnly, + turnOnSourceCodeSharing, revertToPrePublishedState, } from '../../actions/publish'; import { @@ -32,6 +34,9 @@ import { getHasSourceCodeToShare, getSourceCodeSharingStats, getSourceCodeFetchState, + getSourceFilesToFetch, + getActiveSourceSharingPreset, + getSourceSharingDownloadedOnly, getSourceFilesForOfferedPresets, getUploadPhase, getUploadError, @@ -116,7 +121,7 @@ const { UploadAbortedError } = jest.requireActual( ); function offeredPresets(state: State) { - return [...getSourceFilesForOfferedPresets(state).keys()]; + return [...getSourceFilesForOfferedPresets(state, 'upload').keys()]; } function openSourceView(sourceIndex: IndexIntoSourceTable) { @@ -734,10 +739,13 @@ describe('attemptToPublish', function () { } it('offers a native file for fetching once the browser connection is there', function () { - const { getState } = setupWithBrowserConnection(); + const { getState, dispatch } = setupWithBrowserConnection(); expect(getSourceCodeSharingFiles(getState())).toMatchObject([ { path: LOCAL_FILE, availability: 'FETCHABLE', byteCount: null }, ]); + // The presets only reach it once they may pick files to be fetched. + expect(offeredPresets(getState())).toEqual([]); + dispatch(setSourceSharingDownloadedOnly('upload', false)); expect(offeredPresets(getState())).toEqual(['all-files']); }); @@ -797,6 +805,65 @@ describe('attemptToPublish', function () { expect(getSourceFilesToShare(getState(), 'upload')).toEqual(new Set()); }); + it('can keep the presets to the files that are already downloaded', function () { + const OTHER_FILE = '/home/user/moz/mozilla-central/dom/base/nsNode.cpp'; + const { profile } = getProfileFromTextSamples( + `A[file:${LOCAL_FILE}][lib:xul][address:1234] B[file:${OTHER_FILE}]` + ); + // B is downloaded, A would have to be fetched. + profile.shared.sources.content[1] = 'void nsNode::Foo() {}'; + const store = storeWithProfile(profile); + store.dispatch( + updateBrowserConnectionStatus({ + status: 'ESTABLISHED', + browserConnection: makeBrowserConnection(async () => NATIVE_SOURCE) + .browserConnection, + }) + ); + const { dispatch, getState } = store; + + dispatch(setSourceSharingDownloadedOnly('upload', false)); + dispatch( + updateSourceFilesToShare('upload', new Set([0, 1]), 'all-files') + ); + expect(getSourceFilesToFetch(getState(), 'upload')).toEqual( + new Set([0]) + ); + + // The active preset follows the option to its new meaning. + dispatch(setSourceSharingDownloadedOnly('upload', true)); + expect(getSourceFilesToShare(getState(), 'upload')).toEqual( + new Set([1]) + ); + expect(getSourceFilesToFetch(getState(), 'upload')).toEqual(new Set()); + expect(getActiveSourceSharingPreset(getState(), 'upload')).toBe( + 'all-files' + ); + + dispatch(setSourceSharingDownloadedOnly('upload', false)); + expect(getSourceFilesToShare(getState(), 'upload')).toEqual( + new Set([0, 1]) + ); + }); + + it('falls back to fetchable files when turning the option on with nothing downloaded', function () { + const { dispatch, getState } = setupWithBrowserConnection(); + expect(getSourceSharingDownloadedOnly(getState(), 'upload')).toBe(true); + // The downloaded-only presets are all empty here. + expect(offeredPresets(getState())).toEqual([]); + + dispatch(turnOnSourceCodeSharing('upload')); + expect(getSourceSharingDownloadedOnly(getState(), 'upload')).toBe( + false + ); + expect(getSourceFilesToShare(getState(), 'upload')).toEqual( + new Set([0]) + ); + expect(getActiveSourceSharingPreset(getState(), 'upload')).toBe( + 'all-files' + ); + }); + it('can be cancelled', async function () { let resolveFetch: (source: string) => void = () => {}; const { dispatch, getState } = setupWithBrowserConnection( diff --git a/src/types/actions.ts b/src/types/actions.ts index 31d9ed09f9..0e37ce2082 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -662,6 +662,11 @@ type PublishAction = readonly sourceIndexes: ReadonlySet; readonly preset: OfferedSourceSharingPreset | null; } + | { + readonly type: 'UPDATE_SOURCE_SHARING_DOWNLOADED_ONLY'; + readonly mode: SharingMode; + readonly value: boolean; + } | { readonly type: 'SOURCE_CODE_FETCH_STARTED'; readonly totalCount: number } | { readonly type: 'SOURCE_CODE_FETCH_PROGRESS'; diff --git a/src/types/state.ts b/src/types/state.ts index 5d08173db3..f648fe0944 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -244,6 +244,9 @@ export type PublishState = { // `null` means the user hasn't touched the selection yet, in which case the // default selection applies. readonly sourceFileSelection: Record; + // Whether the presets should only pick files whose contents we already + // have, per sharing mode, rather than also the ones that would be fetched. + readonly sourceSharingDownloadedOnly: Record; readonly sanitizedProfileEncodingState: SanitizedProfileEncodingState; readonly sourceCodeFetch: SourceCodeFetchState; readonly upload: UploadState;