From cb89835937550620aa9c0686d7463cd056099763 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 1/3] 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 4a17f387e910d16e0b416dc5f5ec2174a946994a 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 2/3] 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 5ff69bf258aeae5d1a99a6fceae1f6dee4282c4e 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 3/3] 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; };