Repository navigation
flamegraph: give the page a favicon - #203
Merged
Merged
Conversation
Every page perf-agent writes showed the browser's blank-document icon. A profiling session ends up with a dozen of these open at once -- on-CPU, off-CPU, GPU, fused, before and after -- and they were indistinguishable in the tab strip. The icon is inlined as a base64 data: URI, not linked. assets.go states the contract the whole package is built around: nothing may reference an external URL, because the deliverable is one file that renders from file:// with no server and no network. A <link href="brand/favicon.svg"> would be a broken icon everywhere but the directory it was written in. Base64 rather than percent-encoding because the markup contains #, < and " — the three characters that make a raw SVG data: URI fragile inside an HTML attribute. The artwork is the 16px mark, which is drawn for that size rather than scaled from the full one: the full mark loses its top rows below about 24px and reads as a smudge in a tab strip. TestRenderHTMLFetchesNothing forbade "<link " outright, so it failed on this change. The rule it exists to enforce is that the page fetches nothing, and a data: URI fetches nothing -- so it now parses every <link> and requires the href to be a data: URI. That is stricter than the old substring check, not looser: it also catches a <link> that the old list would have missed by spelling. Verified by mutation -- pointing the favicon at an https URL fails it with "a <link> must resolve to a data: URI".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every page perf-agent writes showed the browser's blank-document icon. A
profiling session ends up with a dozen of them open at once — on-CPU,
off-CPU, GPU, fused, before and after — and they were indistinguishable in
the tab strip.
Inlined, not linked
The icon is a base64
data:URI.assets.gostates the contract the wholepackage is built around: nothing may reference an external URL, because the
deliverable is one file that renders from
file://with no server and nonetwork. A
<link href="brand/favicon.svg">would be a broken iconeverywhere but the directory the page happened to be written in.
Base64 rather than percent-encoding because the markup contains
#,<and"— the three characters that make a raw SVGdata:URI fragile inside anHTML attribute.
The artwork is the 16px mark, drawn for that size rather than scaled from the
full one: the full mark loses its top rows below about 24px and reads as a
smudge in a tab strip.
The test it broke, and why it is now stricter
TestRenderHTMLFetchesNothingforbade the substring"<link "outright, sothis change failed it. The rule that test exists to enforce is that the page
fetches nothing — and a
data:URI fetches nothing.It now parses every
<link>in the output and requires itshrefto be adata:URI. That is stricter than the old substring check, not looser:the old one would have missed
<link\n rel=...or any other spelling, whilethe new one inspects every link the document actually contains.
Verified by mutation: pointing the favicon at an
https://URL fails thetest with
a <link> must resolve to a data: URI. Restoring it passes.Also confirmed on a real generated page (a 50k-sample PyTorch GPU profile):
the
rel="icon"link is present and the page still contains zerohttp(s)URLs of any kind.