Repository navigation
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6404 +/- ##
==========================================
+ Coverage 84.78% 85.04% +0.25%
==========================================
Files 358 361 +3
Lines 38946 39626 +680
Branches 11019 11221 +202
==========================================
+ Hits 33022 33701 +679
- Misses 5488 5489 +1
Partials 436 436 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7141c83 to
4c6f410
Compare
ca7aa16 to
18e02ab
Compare
| # $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. |
There was a problem hiding this comment.
"could not be downloaded`: is this correct, or should it be uploaded?
There was a problem hiding this comment.
It's correct, we first download some sources that are not available before uploading.
| # $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, which needs the browser that recorded the profile or a local symbol server. |
There was a problem hiding this comment.
| [one] { $unavailableCount } file can’t be downloaded from here, which needs the browser that recorded the profile or a local symbol server. | |
| [one] { $unavailableCount } file can’t be downloaded from here. This needs the browser that recorded the profile or a local symbol server. |
I feel like this sentence is missing something. What's "which"?
Also wondering about requires instead of needs (also in the other string).
There was a problem hiding this comment.
Updated it to use "requires" and reworded it to make it clearer.
18e02ab to
7d7e621
Compare
|
By the way, this PR is still a draft (as a deploy preview) and I intend to make some changes depending on the feedback that I get. |
7d7e621 to
ff91c6a
Compare
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.
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.
--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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
ff91c6a to
b554ab6
Compare
Main | Deploy preview
Fixes #4018.
Until now source code was always stripped from shared and downloaded profiles. This adds an "Include source code" checkbox to the publish panel, with a file tree to pick which files to include.
I think some of the patches can land before. I'll extract them and land them before this patch.
Example screenshot:
