-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fonts: Remove web fonts when their stylesheet is removed #32346
fonts: Remove web fonts when their stylesheet is removed #32346
Conversation
🔨 Triggering try run (#9190661994) for Linux WPT |
c1d7fff
to
b6958b0
Compare
🔨 Triggering try run (#9190999686) for Linux WPT |
Test results for linux-wpt-layout-2013 from try job (#9190661994): Flaky unexpected result (13)
Stable unexpected results that are known to be intermittent (12)
|
Test results for linux-wpt-layout-2020 from try job (#9190661994): Flaky unexpected result (20)
Stable unexpected results that are known to be intermittent (10)
|
✨ Try run (#9190661994) succeeded. |
Test results for linux-wpt-layout-2013 from try job (#9190999686): Flaky unexpected result (13)
Stable unexpected results that are known to be intermittent (13)
|
Test results for linux-wpt-layout-2020 from try job (#9190999686): Flaky unexpected result (16)
Stable unexpected results that are known to be intermittent (15)
|
✨ Try run (#9190999686) succeeded. |
b6958b0
to
278b758
Compare
I have added a test for this change. |
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#46437) with upstreamable changes. |
components/gfx/font_store.rs
Outdated
|
||
let family_name = state.css_font_face_descriptors.family_name.clone(); | ||
self.families | ||
.entry(family_name.clone()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.entry(family_name.clone()) | |
.entry(family_name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this as well as a couple other places where we didn't need to pass as a reference.
components/gfx/font_store.rs
Outdated
@@ -127,4 +201,14 @@ impl FontTemplates { | |||
self.templates | |||
.push(Arc::new(AtomicRefCell::new(new_template))); | |||
} | |||
|
|||
pub(crate) fn remove_add_templates_for_stylesheet( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub(crate) fn remove_add_templates_for_stylesheet( | |
pub(crate) fn remove_templates_for_stylesheet( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅 Fixed this one.
278b758
to
520c0b0
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46437). |
This is the first part of ensuring that unused fonts do not leak. This change makes it so that when a stylesheet is removed, the corresponding web fonts are removed from the `FontContext`. Note: WebRender assets are still leaked, which was the situation before for all fonts. A followup change will fix this issue. Fixes servo#15139. Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Mukilan Thiyagarajan <[email protected]>
520c0b0
to
2448611
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46437). |
This is the first part of ensuring that unused fonts do not leak. This
change makes it so that when a stylesheet is removed, the corresponding
web fonts are removed from the
FontContext
.Note: WebRender assets are still leaked, which was the situation before
for all fonts. A followup change will fix this issue.
Fixes #15139.
Signed-off-by: Martin Robinson [email protected]
Co-authored-by: Mukilan Thiyagarajan [email protected]
./mach build -d
does not report any errors./mach test-tidy
does not report any errors