fonts: Remove web fonts when their stylsheet is removed (#32346)

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 <mrobinson@igalia.com>
Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
This commit is contained in:
Martin Robinson 2024-05-23 08:49:31 +02:00 committed by GitHub
parent a772ecf786
commit 14286d913d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 258 additions and 57 deletions

View file

@ -484,6 +484,7 @@ impl FontSource for FontCacheThread {
identifier: serialized_font_template.identifier, identifier: serialized_font_template.identifier,
descriptor: serialized_font_template.descriptor.clone(), descriptor: serialized_font_template.descriptor.clone(),
data: font_data.map(Arc::new), data: font_data.map(Arc::new),
stylesheet: None,
})) }))
}) })
.collect() .collect()

View file

@ -5,7 +5,6 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::default::Default; use std::default::Default;
use std::hash::{BuildHasherDefault, Hash, Hasher}; use std::hash::{BuildHasherDefault, Hash, Hasher};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc; use std::sync::Arc;
use app_units::Au; use app_units::Au;
@ -24,7 +23,7 @@ use style::font_face::{FontFaceSourceFormat, FontFaceSourceFormatKeyword, Source
use style::media_queries::Device; use style::media_queries::Device;
use style::properties::style_structs::Font as FontStyleStruct; use style::properties::style_structs::Font as FontStyleStruct;
use style::shared_lock::SharedRwLockReadGuard; use style::shared_lock::SharedRwLockReadGuard;
use style::stylesheets::{Stylesheet, StylesheetInDocument}; use style::stylesheets::{DocumentStyleSheet, StylesheetInDocument};
use style::Atom; use style::Atom;
use url::Url; use url::Url;
@ -51,7 +50,6 @@ pub struct FontContext<S: FontSource> {
cache: CachingFontSource<S>, cache: CachingFontSource<S>,
web_fonts: CrossThreadFontStore, web_fonts: CrossThreadFontStore,
webrender_font_store: CrossThreadWebRenderFontStore, webrender_font_store: CrossThreadWebRenderFontStore,
web_fonts_still_loading: AtomicUsize,
} }
impl<S: FontSource> MallocSizeOf for FontContext<S> { impl<S: FontSource> MallocSizeOf for FontContext<S> {
@ -69,12 +67,11 @@ impl<S: FontSource> FontContext<S> {
cache: CachingFontSource::new(font_source), cache: CachingFontSource::new(font_source),
web_fonts: Arc::new(RwLock::default()), web_fonts: Arc::new(RwLock::default()),
webrender_font_store: Arc::new(RwLock::default()), webrender_font_store: Arc::new(RwLock::default()),
web_fonts_still_loading: Default::default(),
} }
} }
pub fn web_fonts_still_loading(&self) -> usize { pub fn web_fonts_still_loading(&self) -> usize {
self.web_fonts_still_loading.load(Ordering::SeqCst) self.web_fonts.read().number_of_fonts_still_loading()
} }
/// Handle the situation where a web font finishes loading, specifying if the load suceeded or failed. /// Handle the situation where a web font finishes loading, specifying if the load suceeded or failed.
@ -83,7 +80,6 @@ impl<S: FontSource> FontContext<S> {
finished_callback: &WebFontLoadFinishedCallback, finished_callback: &WebFontLoadFinishedCallback,
succeeded: bool, succeeded: bool,
) { ) {
self.web_fonts_still_loading.fetch_sub(1, Ordering::SeqCst);
if succeeded { if succeeded {
self.cache.invalidate_after_web_font_load(); self.cache.invalidate_after_web_font_load();
} }
@ -159,6 +155,8 @@ impl<S: FontSource> FontContext<S> {
font_template, font_descriptor font_template, font_descriptor
); );
// TODO: Inserting `None` into the cache here is a bit bogus. Instead we should somehow
// mark this template as invalid so it isn't tried again.
let font = self let font = self
.create_font( .create_font(
font_template, font_template,
@ -229,29 +227,31 @@ impl<S: FontSource> FontContext<S> {
#[derive(Clone)] #[derive(Clone)]
pub struct WebFontDownloadState { pub struct WebFontDownloadState {
css_font_face_descriptors: Arc<CSSFontFaceDescriptors>, pub css_font_face_descriptors: Arc<CSSFontFaceDescriptors>,
remaining_sources: Vec<Source>, remaining_sources: Vec<Source>,
finished_callback: WebFontLoadFinishedCallback, finished_callback: WebFontLoadFinishedCallback,
core_resource_thread: CoreResourceThread, core_resource_thread: CoreResourceThread,
local_fonts: Arc<HashMap<Atom, Option<FontTemplateRef>>>, local_fonts: Arc<HashMap<Atom, Option<FontTemplateRef>>>,
pub stylesheet: DocumentStyleSheet,
} }
pub trait FontContextWebFontMethods { pub trait FontContextWebFontMethods {
fn add_all_web_fonts_from_stylesheet( fn add_all_web_fonts_from_stylesheet(
&self, &self,
stylesheet: &Stylesheet, stylesheet: &DocumentStyleSheet,
guard: &SharedRwLockReadGuard, guard: &SharedRwLockReadGuard,
device: &Device, device: &Device,
finished_callback: WebFontLoadFinishedCallback, finished_callback: WebFontLoadFinishedCallback,
synchronous: bool, synchronous: bool,
) -> usize; ) -> usize;
fn process_next_web_font_source(&self, web_font_download_state: WebFontDownloadState); fn process_next_web_font_source(&self, web_font_download_state: WebFontDownloadState);
fn remove_all_web_fonts_from_stylesheet(&self, stylesheet: &DocumentStyleSheet);
} }
impl<S: FontSource + Send + 'static> FontContextWebFontMethods for Arc<FontContext<S>> { impl<S: FontSource + Send + 'static> FontContextWebFontMethods for Arc<FontContext<S>> {
fn add_all_web_fonts_from_stylesheet( fn add_all_web_fonts_from_stylesheet(
&self, &self,
stylesheet: &Stylesheet, stylesheet: &DocumentStyleSheet,
guard: &SharedRwLockReadGuard, guard: &SharedRwLockReadGuard,
device: &Device, device: &Device,
finished_callback: WebFontLoadFinishedCallback, finished_callback: WebFontLoadFinishedCallback,
@ -312,17 +312,20 @@ impl<S: FontSource + Send + 'static> FontContextWebFontMethods for Arc<FontConte
} }
} }
number_loading += 1;
self.web_fonts
.write()
.handle_web_font_load_started_for_stylesheet(stylesheet);
self.process_next_web_font_source(WebFontDownloadState { self.process_next_web_font_source(WebFontDownloadState {
css_font_face_descriptors: Arc::new(rule.into()), css_font_face_descriptors: Arc::new(rule.into()),
remaining_sources: sources, remaining_sources: sources,
finished_callback: finished_callback.clone(), finished_callback: finished_callback.clone(),
core_resource_thread: self.resource_threads.lock().clone(), core_resource_thread: self.resource_threads.lock().clone(),
local_fonts: Arc::new(local_fonts), local_fonts: Arc::new(local_fonts),
stylesheet: stylesheet.clone(),
}); });
number_loading += 1;
self.web_fonts_still_loading.fetch_add(1, Ordering::SeqCst);
// If the load is synchronous wait for it to be signalled. // If the load is synchronous wait for it to be signalled.
if let Some(ref synchronous_receiver) = synchronous_receiver { if let Some(ref synchronous_receiver) = synchronous_receiver {
synchronous_receiver.recv().unwrap(); synchronous_receiver.recv().unwrap();
@ -334,6 +337,9 @@ impl<S: FontSource + Send + 'static> FontContextWebFontMethods for Arc<FontConte
fn process_next_web_font_source(&self, mut state: WebFontDownloadState) { fn process_next_web_font_source(&self, mut state: WebFontDownloadState) {
let Some(source) = state.remaining_sources.pop() else { let Some(source) = state.remaining_sources.pop() else {
self.web_fonts
.write()
.handle_web_font_failed_to_load(&state);
self.handle_web_font_load_finished(&state.finished_callback, false); self.handle_web_font_load_finished(&state.finished_callback, false);
return; return;
}; };
@ -354,23 +360,48 @@ impl<S: FontSource + Send + 'static> FontContextWebFontMethods for Arc<FontConte
FontTemplate::new_for_local_web_font( FontTemplate::new_for_local_web_font(
local_template, local_template,
&state.css_font_face_descriptors, &state.css_font_face_descriptors,
state.stylesheet.clone(),
) )
.ok() .ok()
}) })
{ {
this.web_fonts let not_cancelled = self
.web_fonts
.write() .write()
.families .handle_web_font_loaded(&state, new_template);
.entry(web_font_family_name.clone()) self.handle_web_font_load_finished(&state.finished_callback, not_cancelled);
.or_default()
.add_template(new_template);
self.handle_web_font_load_finished(&state.finished_callback, true);
} else { } else {
this.process_next_web_font_source(state); this.process_next_web_font_source(state);
} }
}, },
} }
} }
fn remove_all_web_fonts_from_stylesheet(&self, stylesheet: &DocumentStyleSheet) {
let mut web_fonts = self.web_fonts.write();
let mut fonts = self.cache.fonts.write();
let mut font_groups = self.cache.resolved_font_groups.write();
// Cancel any currently in-progress web font loads.
web_fonts.handle_stylesheet_removed(stylesheet);
let mut removed_any = false;
for family in web_fonts.families.values_mut() {
removed_any |= family.remove_templates_for_stylesheet(stylesheet);
}
if !removed_any {
return;
};
fonts.retain(|_, font| match font {
Some(font) => font.template.borrow().stylesheet.as_ref() != Some(stylesheet),
_ => true,
});
// Removing this stylesheet modified the available fonts, so invalidate the cache
// of resolved font groups.
font_groups.clear();
}
} }
struct RemoteWebFontDownloader<FCT: FontSource> { struct RemoteWebFontDownloader<FCT: FontSource> {
@ -437,6 +468,18 @@ impl<FCT: FontSource + Send + 'static> RemoteWebFontDownloader<FCT> {
/// After a download finishes, try to process the downloaded data, returning true if /// After a download finishes, try to process the downloaded data, returning true if
/// the font is added successfully to the [`FontContext`] or false if it isn't. /// the font is added successfully to the [`FontContext`] or false if it isn't.
fn process_downloaded_font_and_signal_completion(&self, state: &WebFontDownloadState) -> bool { fn process_downloaded_font_and_signal_completion(&self, state: &WebFontDownloadState) -> bool {
if self
.font_context
.web_fonts
.read()
.font_load_cancelled_for_stylesheet(&state.stylesheet)
{
self.font_context
.handle_web_font_load_finished(&state.finished_callback, false);
// Returning true here prevents trying to load the next font on the source list.
return true;
}
let font_data = std::mem::take(&mut *self.response_data.lock()); let font_data = std::mem::take(&mut *self.response_data.lock());
trace!( trace!(
"@font-face {} data={:?}", "@font-face {} data={:?}",
@ -459,21 +502,21 @@ impl<FCT: FontSource + Send + 'static> RemoteWebFontDownloader<FCT> {
self.url.clone().into(), self.url.clone().into(),
Arc::new(font_data), Arc::new(font_data),
&state.css_font_face_descriptors, &state.css_font_face_descriptors,
Some(state.stylesheet.clone()),
) else { ) else {
return false; return false;
}; };
let family_name = state.css_font_face_descriptors.family_name.clone(); let not_cancelled = self
self.font_context .font_context
.web_fonts .web_fonts
.write() .write()
.families .handle_web_font_loaded(state, new_template);
.entry(family_name.clone())
.or_default()
.add_template(new_template);
self.font_context self.font_context
.handle_web_font_load_finished(&state.finished_callback, true); .handle_web_font_load_finished(&state.finished_callback, not_cancelled);
// If the load was canceled above, then we still want to return true from this function in
// order to halt any attempt to load sources that come later on the source list.
true true
} }

View file

@ -8,15 +8,18 @@ use std::sync::Arc;
use app_units::Au; use app_units::Au;
use atomic_refcell::AtomicRefCell; use atomic_refcell::AtomicRefCell;
use parking_lot::RwLock; use parking_lot::RwLock;
use style::stylesheets::DocumentStyleSheet;
use webrender_api::{FontInstanceFlags, FontInstanceKey, FontKey}; use webrender_api::{FontInstanceFlags, FontInstanceKey, FontKey};
use crate::font::FontDescriptor; use crate::font::FontDescriptor;
use crate::font_cache_thread::{FontIdentifier, FontSource, LowercaseString}; use crate::font_cache_thread::{FontIdentifier, FontSource, LowercaseString};
use crate::font_context::WebFontDownloadState;
use crate::font_template::{FontTemplate, FontTemplateRef, FontTemplateRefMethods}; use crate::font_template::{FontTemplate, FontTemplateRef, FontTemplateRefMethods};
#[derive(Default)] #[derive(Default)]
pub struct FontStore { pub struct FontStore {
pub(crate) families: HashMap<LowercaseString, FontTemplates>, pub(crate) families: HashMap<LowercaseString, FontTemplates>,
web_fonts_loading: Vec<(DocumentStyleSheet, usize)>,
} }
pub(crate) type CrossThreadFontStore = Arc<RwLock<FontStore>>; pub(crate) type CrossThreadFontStore = Arc<RwLock<FontStore>>;
@ -24,6 +27,77 @@ impl FontStore {
pub(crate) fn clear(&mut self) { pub(crate) fn clear(&mut self) {
self.families.clear(); self.families.clear();
} }
pub(crate) fn font_load_cancelled_for_stylesheet(
&self,
stylesheet: &DocumentStyleSheet,
) -> bool {
!self
.web_fonts_loading
.iter()
.any(|(loading_stylesheet, _)| loading_stylesheet == stylesheet)
}
pub(crate) fn handle_stylesheet_removed(&mut self, stylesheet: &DocumentStyleSheet) {
self.web_fonts_loading
.retain(|(loading_stylesheet, _)| loading_stylesheet != stylesheet);
}
pub(crate) fn handle_web_font_load_started_for_stylesheet(
&mut self,
stylesheet: &DocumentStyleSheet,
) {
if let Some((_, count)) = self
.web_fonts_loading
.iter_mut()
.find(|(loading_stylesheet, _)| loading_stylesheet == stylesheet)
{
*count += 1;
} else {
self.web_fonts_loading.push((stylesheet.clone(), 1))
}
}
fn remove_one_web_font_loading_for_stylesheet(&mut self, stylesheet: &DocumentStyleSheet) {
if let Some((_, count)) = self
.web_fonts_loading
.iter_mut()
.find(|(loading_stylesheet, _)| loading_stylesheet == stylesheet)
{
*count -= 1;
}
self.web_fonts_loading.retain(|(_, count)| *count != 0);
}
pub(crate) fn handle_web_font_failed_to_load(&mut self, state: &WebFontDownloadState) {
self.remove_one_web_font_loading_for_stylesheet(&state.stylesheet);
}
/// Handle a web font load finishing, adding the new font to the [`FontStore`]. If the web font
/// load was canceled (for instance, if the stylesheet was removed), then do nothing and return
/// false.
pub(crate) fn handle_web_font_loaded(
&mut self,
state: &WebFontDownloadState,
new_template: FontTemplate,
) -> bool {
// Abort processing this web font if the originating stylesheet was removed.
if self.font_load_cancelled_for_stylesheet(&state.stylesheet) {
return false;
}
let family_name = state.css_font_face_descriptors.family_name.clone();
self.families
.entry(family_name)
.or_default()
.add_template(new_template);
self.remove_one_web_font_loading_for_stylesheet(&state.stylesheet);
true
}
pub(crate) fn number_of_fonts_still_loading(&self) -> usize {
self.web_fonts_loading.iter().map(|(_, count)| count).sum()
}
} }
#[derive(Default)] #[derive(Default)]
@ -127,4 +201,14 @@ impl FontTemplates {
self.templates self.templates
.push(Arc::new(AtomicRefCell::new(new_template))); .push(Arc::new(AtomicRefCell::new(new_template)));
} }
pub(crate) fn remove_templates_for_stylesheet(
&mut self,
stylesheet: &DocumentStyleSheet,
) -> bool {
let length_before = self.templates.len();
self.templates
.retain(|template| template.borrow().stylesheet.as_ref() != Some(stylesheet));
length_before != self.templates.len()
}
} }

View file

@ -12,6 +12,7 @@ use serde::{Deserialize, Serialize};
use servo_url::ServoUrl; use servo_url::ServoUrl;
use style::computed_values::font_stretch::T as FontStretch; use style::computed_values::font_stretch::T as FontStretch;
use style::computed_values::font_style::T as FontStyle; use style::computed_values::font_style::T as FontStyle;
use style::stylesheets::DocumentStyleSheet;
use style::values::computed::font::FontWeight; use style::values::computed::font::FontWeight;
use crate::font::{FontDescriptor, PlatformFontMethods}; use crate::font::{FontDescriptor, PlatformFontMethods};
@ -132,9 +133,11 @@ pub struct FontTemplate {
pub descriptor: FontTemplateDescriptor, pub descriptor: FontTemplateDescriptor,
/// The data to use for this [`FontTemplate`]. For web fonts, this is always filled, but /// The data to use for this [`FontTemplate`]. For web fonts, this is always filled, but
/// for local fonts, this is loaded only lazily in layout. /// for local fonts, this is loaded only lazily in layout.
///
/// TODO: There is no mechanism for web fonts to unset their data!
pub data: Option<Arc<Vec<u8>>>, pub data: Option<Arc<Vec<u8>>>,
/// If this font is a web font, this is a reference to the stylesheet that
/// created it. This will be used to remove this font from caches, when the
/// stylesheet is removed.
pub stylesheet: Option<DocumentStyleSheet>,
} }
impl malloc_size_of::MallocSizeOf for FontTemplate { impl malloc_size_of::MallocSizeOf for FontTemplate {
@ -164,6 +167,7 @@ impl FontTemplate {
identifier: FontIdentifier::Local(identifier), identifier: FontIdentifier::Local(identifier),
descriptor, descriptor,
data: None, data: None,
stylesheet: None,
} }
} }
@ -172,6 +176,7 @@ impl FontTemplate {
url: ServoUrl, url: ServoUrl,
data: Arc<Vec<u8>>, data: Arc<Vec<u8>>,
css_font_template_descriptors: &CSSFontFaceDescriptors, css_font_template_descriptors: &CSSFontFaceDescriptors,
stylesheet: Option<DocumentStyleSheet>,
) -> Result<FontTemplate, &'static str> { ) -> Result<FontTemplate, &'static str> {
let identifier = FontIdentifier::Web(url.clone()); let identifier = FontIdentifier::Web(url.clone());
let Ok(handle) = PlatformFont::new_from_data(identifier, data.clone(), 0, None) else { let Ok(handle) = PlatformFont::new_from_data(identifier, data.clone(), 0, None) else {
@ -185,6 +190,7 @@ impl FontTemplate {
identifier: FontIdentifier::Web(url), identifier: FontIdentifier::Web(url),
descriptor, descriptor,
data: Some(data), data: Some(data),
stylesheet,
}) })
} }
@ -194,11 +200,13 @@ impl FontTemplate {
pub fn new_for_local_web_font( pub fn new_for_local_web_font(
local_template: FontTemplateRef, local_template: FontTemplateRef,
css_font_template_descriptors: &CSSFontFaceDescriptors, css_font_template_descriptors: &CSSFontFaceDescriptors,
stylesheet: DocumentStyleSheet,
) -> Result<FontTemplate, &'static str> { ) -> Result<FontTemplate, &'static str> {
let mut alias_template = local_template.borrow().clone(); let mut alias_template = local_template.borrow().clone();
alias_template alias_template
.descriptor .descriptor
.override_values_with_css_font_template_descriptors(css_font_template_descriptors); .override_values_with_css_font_template_descriptors(css_font_template_descriptors);
alias_template.stylesheet = Some(stylesheet);
Ok(alias_template) Ok(alias_template)
} }

View file

@ -24,6 +24,7 @@ use servo_atoms::Atom;
use servo_url::ServoUrl; use servo_url::ServoUrl;
use style::properties::longhands::font_variant_caps::computed_value::T as FontVariantCaps; use style::properties::longhands::font_variant_caps::computed_value::T as FontVariantCaps;
use style::properties::style_structs::Font as FontStyleStruct; use style::properties::style_structs::Font as FontStyleStruct;
use style::stylesheets::Stylesheet;
use style::values::computed::font::{ use style::values::computed::font::{
FamilyName, FontFamily, FontFamilyList, FontFamilyNameSyntax, FontSize, FontStretch, FontStyle, FamilyName, FontFamily, FontFamilyList, FontFamilyNameSyntax, FontSize, FontStretch, FontStyle,
FontWeight, SingleFontFamily, FontWeight, SingleFontFamily,
@ -85,6 +86,7 @@ impl MockFontCacheThread {
Self::url_for_font_name(name), Self::url_for_font_name(name),
std::sync::Arc::new(data), std::sync::Arc::new(data),
&CSSFontFaceDescriptors::new(name), &CSSFontFaceDescriptors::new(name),
None,
) )
.unwrap(), .unwrap(),
); );

View file

@ -273,7 +273,10 @@ impl Layout for LayoutThread {
fn load_web_fonts_from_stylesheet(&self, stylesheet: ServoArc<Stylesheet>) { fn load_web_fonts_from_stylesheet(&self, stylesheet: ServoArc<Stylesheet>) {
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &guard); self.load_all_web_fonts_from_stylesheet_with_guard(
&DocumentStyleSheet(stylesheet.clone()),
&guard,
);
} }
fn add_stylesheet( fn add_stylesheet(
@ -282,24 +285,25 @@ impl Layout for LayoutThread {
before_stylesheet: Option<ServoArc<Stylesheet>>, before_stylesheet: Option<ServoArc<Stylesheet>>,
) { ) {
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
let stylesheet = DocumentStyleSheet(stylesheet.clone());
self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &guard); self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &guard);
match before_stylesheet { match before_stylesheet {
Some(insertion_point) => self.stylist.insert_stylesheet_before( Some(insertion_point) => self.stylist.insert_stylesheet_before(
DocumentStyleSheet(stylesheet.clone()), stylesheet,
DocumentStyleSheet(insertion_point), DocumentStyleSheet(insertion_point),
&guard, &guard,
), ),
None => self None => self.stylist.append_stylesheet(stylesheet, &guard),
.stylist
.append_stylesheet(DocumentStyleSheet(stylesheet.clone()), &guard),
} }
} }
fn remove_stylesheet(&mut self, stylesheet: ServoArc<Stylesheet>) { fn remove_stylesheet(&mut self, stylesheet: ServoArc<Stylesheet>) {
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
self.stylist let stylesheet = DocumentStyleSheet(stylesheet.clone());
.remove_stylesheet(DocumentStyleSheet(stylesheet.clone()), &guard); self.stylist.remove_stylesheet(stylesheet.clone(), &guard);
self.font_context
.remove_all_web_fonts_from_stylesheet(&stylesheet);
} }
fn query_content_box(&self, node: OpaqueNode) -> Option<UntypedRect<Au>> { fn query_content_box(&self, node: OpaqueNode) -> Option<UntypedRect<Au>> {
@ -668,7 +672,7 @@ impl LayoutThread {
fn load_all_web_fonts_from_stylesheet_with_guard( fn load_all_web_fonts_from_stylesheet_with_guard(
&self, &self,
stylesheet: &Stylesheet, stylesheet: &DocumentStyleSheet,
guard: &SharedRwLockReadGuard, guard: &SharedRwLockReadGuard,
) { ) {
if !stylesheet.is_effective_for_device(self.stylist.device(), guard) { if !stylesheet.is_effective_for_device(self.stylist.device(), guard) {
@ -982,10 +986,7 @@ impl LayoutThread {
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets { for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
self.stylist self.stylist
.append_stylesheet(stylesheet.clone(), &ua_or_user_guard); .append_stylesheet(stylesheet.clone(), &ua_or_user_guard);
self.load_all_web_fonts_from_stylesheet_with_guard( self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &ua_or_user_guard);
&stylesheet.0,
&ua_or_user_guard,
);
} }
if self.stylist.quirks_mode() != QuirksMode::NoQuirks { if self.stylist.quirks_mode() != QuirksMode::NoQuirks {
@ -994,7 +995,7 @@ impl LayoutThread {
&ua_or_user_guard, &ua_or_user_guard,
); );
self.load_all_web_fonts_from_stylesheet_with_guard( self.load_all_web_fonts_from_stylesheet_with_guard(
&ua_stylesheets.quirks_mode_stylesheet.0, &ua_stylesheets.quirks_mode_stylesheet,
&ua_or_user_guard, &ua_or_user_guard,
); );
} }

View file

@ -249,7 +249,10 @@ impl Layout for LayoutThread {
fn load_web_fonts_from_stylesheet(&self, stylesheet: ServoArc<Stylesheet>) { fn load_web_fonts_from_stylesheet(&self, stylesheet: ServoArc<Stylesheet>) {
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &guard); self.load_all_web_fonts_from_stylesheet_with_guard(
&DocumentStyleSheet(stylesheet.clone()),
&guard,
);
} }
fn add_stylesheet( fn add_stylesheet(
@ -258,25 +261,25 @@ impl Layout for LayoutThread {
before_stylesheet: Option<ServoArc<Stylesheet>>, before_stylesheet: Option<ServoArc<Stylesheet>>,
) { ) {
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
let stylesheet = DocumentStyleSheet(stylesheet.clone());
self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &guard); self.load_all_web_fonts_from_stylesheet_with_guard(&stylesheet, &guard);
match before_stylesheet { match before_stylesheet {
Some(insertion_point) => self.stylist.insert_stylesheet_before( Some(insertion_point) => self.stylist.insert_stylesheet_before(
DocumentStyleSheet(stylesheet.clone()), stylesheet,
DocumentStyleSheet(insertion_point), DocumentStyleSheet(insertion_point),
&guard, &guard,
), ),
None => self None => self.stylist.append_stylesheet(stylesheet, &guard),
.stylist
.append_stylesheet(DocumentStyleSheet(stylesheet.clone()), &guard),
} }
} }
fn remove_stylesheet(&mut self, stylesheet: ServoArc<Stylesheet>) { fn remove_stylesheet(&mut self, stylesheet: ServoArc<Stylesheet>) {
// TODO(mrobinson): This should also unload web fonts from the FontCacheThread.
let guard = stylesheet.shared_lock.read(); let guard = stylesheet.shared_lock.read();
self.stylist let stylesheet = DocumentStyleSheet(stylesheet.clone());
.remove_stylesheet(DocumentStyleSheet(stylesheet.clone()), &guard); self.stylist.remove_stylesheet(stylesheet.clone(), &guard);
self.font_context
.remove_all_web_fonts_from_stylesheet(&stylesheet);
} }
fn query_content_box(&self, node: OpaqueNode) -> Option<UntypedRect<Au>> { fn query_content_box(&self, node: OpaqueNode) -> Option<UntypedRect<Au>> {
@ -584,7 +587,7 @@ impl LayoutThread {
fn load_all_web_fonts_from_stylesheet_with_guard( fn load_all_web_fonts_from_stylesheet_with_guard(
&self, &self,
stylesheet: &Stylesheet, stylesheet: &DocumentStyleSheet,
guard: &SharedRwLockReadGuard, guard: &SharedRwLockReadGuard,
) { ) {
if !stylesheet.is_effective_for_device(self.stylist.device(), guard) { if !stylesheet.is_effective_for_device(self.stylist.device(), guard) {
@ -651,10 +654,7 @@ impl LayoutThread {
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets { for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
self.stylist self.stylist
.append_stylesheet(stylesheet.clone(), &ua_or_user_guard); .append_stylesheet(stylesheet.clone(), &ua_or_user_guard);
self.load_all_web_fonts_from_stylesheet_with_guard( self.load_all_web_fonts_from_stylesheet_with_guard(stylesheet, &ua_or_user_guard);
&stylesheet.0,
&ua_or_user_guard,
);
} }
if self.stylist.quirks_mode() != QuirksMode::NoQuirks { if self.stylist.quirks_mode() != QuirksMode::NoQuirks {
@ -663,7 +663,7 @@ impl LayoutThread {
&ua_or_user_guard, &ua_or_user_guard,
); );
self.load_all_web_fonts_from_stylesheet_with_guard( self.load_all_web_fonts_from_stylesheet_with_guard(
&ua_stylesheets.quirks_mode_stylesheet.0, &ua_stylesheets.quirks_mode_stylesheet,
&ua_or_user_guard, &ua_or_user_guard,
); );
} }

View file

@ -179113,7 +179113,20 @@
{} {}
] ]
] ]
} },
"web-font-no-longer-accessible-when-stylesheet-removed.html": [
"aac3e77a819019d349123fac2265531f3746f826",
[
null,
[
[
"/css/css-fonts/web-font-no-longer-accessible-when-stylesheet-removed-ref.html",
"=="
]
],
{}
]
]
}, },
"css-grid": { "css-grid": {
"abspos": { "abspos": {
@ -403830,7 +403843,11 @@
"4e88cace350e32711fc06220c2d14dcbd3ab3339", "4e88cace350e32711fc06220c2d14dcbd3ab3339",
[] []
] ]
} },
"web-font-no-longer-accessible-when-stylesheet-removed-ref.html": [
"363b88d7180a9471c647cd69b9c69678e2f88da6",
[]
]
}, },
"css-gcpm": { "css-gcpm": {
"META.yml": [ "META.yml": [

View file

@ -0,0 +1,16 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS fonts: Web fonts from removed stylsheets should not be accessible</title>
<link rel="author" title="Martin Robinson" href="mrobinson@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-fonts/#font-face-rule">
</head>
<body>
<p>Test passes if the text below is not rendered with Ahem.</p>
<p>TEXT</p>
</body>
</html>

View file

@ -0,0 +1,29 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS fonts: Web fonts from removed stylsheets should not be accessible</title>
<link rel="author" title="Martin Robinson" href="mrobinson@igalia.com">
<link rel="match" href="web-font-no-longer-accessible-when-stylesheet-removed-ref.html">
<link rel="help" href="https://drafts.csswg.org/css-fonts/#font-face-rule">
</head>
<body>
<style id="web-font-stylesheet">
@font-face {
font-family: CustomFontFamily;
src: url(/fonts/Ahem.ttf);
}
</style>
<p>Test passes if the text below is not rendered with Ahem.</p>
<p style="font-family: CustomFontFamily;">TEXT</p>
<script>
let styleTag = document.getElementById("web-font-stylesheet");
styleTag.parentNode.removeChild(styleTag);
</script>
</body>
</html>