Skip to content

Keep the source code cache tied to the profile it was filled for - #6410

Merged
canova merged 3 commits into
firefox-devtools:mainfrom
canova:source-code-cache-per-profile
Nov 10, 2026
Merged

canova merged 3 commits into
firefox-devtools:mainfrom
canova:source-code-cache-per-profile

Conversation

@canova

@canova canova commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Main | Deploy preview

This is also extracted from #6404. It's independent of #6407 and can land in either order.

The source code cache is keyed by the source code index, but there were two things that broke it:

  1. Loading another profile kept the old cache entries around, pointing at unrelated files in the new profile.
  2. A fetch still running while another profile got loaded wrote its result into the new cache, under an index that might belong to a different file on the new profile.

To have one place for that check, I moved the source caching from SourceCodeFetcher to fetchSourceCode thunk action. The publish panel will also use it in #6404 to fetch the files picked for sharing.

@canova
canova requested review from fatadel and mstange October 21, 2026 15:31
@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.01887% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.97%. Comparing base (b3985cd) to head (68f63c0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/actions/code.ts 72.00% 7 Missing ⚠️
src/reducers/code.ts 91.30% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6410   +/-   ##
=======================================
  Coverage   84.96%   84.97%           
=======================================
  Files         359      359           
  Lines       39173    39170    -3     
  Branches    11106    11115    +9     
=======================================
+ Hits        33283    33284    +1     
+ Misses       5453     5449    -4     
  Partials      437      437           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/actions/code.ts Outdated
Comment on lines +95 to +97
if (isStale()) {
return result;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have expected the staleness checks to live in the reducer. We may still want to dispatch the actions in these cases in order to update any loading UI (hide throbbers), no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, initially I put it in the thunk to match how attemptToPublish handles uploadGeneration, but the reducer is better indeed. Moved it there. I had to change how we handle the generation as well.

@canova
canova force-pushed the source-code-cache-per-profile branch 2 times, most recently from 6b7a006 to 5ff69bf Compare October 22, 2026 14:53
@canova
canova requested a review from mstange October 22, 2026 14:54
@canova
canova force-pushed the source-code-cache-per-profile branch from 5ff69bf to df781d4 Compare November 10, 2026 19:58
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.
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.
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.
@canova
canova added this pull request to the merge queue Nov 10, 2026
Merged via the queue into firefox-devtools:main with commit 9b99b86 Nov 10, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants