Skip to content

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

Open
canova wants to merge 3 commits into
firefox-devtools:mainfrom
canova:source-code-cache-per-profile
Open

canova wants to merge 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.

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.
@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.79%. Comparing base (741f242) to head (5ff69bf).
⚠️ Report is 17 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.52%   84.79%   +0.27%     
==========================================
  Files         358      358              
  Lines       38837    38943     +106     
  Branches    11001    11023      +22     
==========================================
+ Hits        32828    33023     +195     
+ Misses       5574     5484      -90     
- Partials      435      436       +1     

☔ 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.

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 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
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