Skip to content

Commit 247df7e

Browse files
committed
Bug 1759891 - Ensure shared font keys have a unique namespace per WebRender instance. r=gw,gfx-reviewers
The blob font tables in Moz2DImageRenderer are currently shared across all WebRender instances. So if different blob fonts from different WebRender instances happen to share the same font keys, in particular the same namespace, the font keys can thus accidentally collide and blob fonts can end up getting shared across WebRender instances unintentionally. To guard against this, we need to ensure that SharedFontResources is provided with a unique namespace for the WebRender instance in which to allocate shared font keys so that collisions can no longer occur. Differential Revision: https://phabricator.services.mozilla.com/D141929 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/26768254de772eb7d2ff50bccafa80e9f4a69f63
1 parent f96ce85 commit 247df7e

File tree

4 files changed

+21
-17
lines changed

4 files changed

+21
-17
lines changed

webrender/src/glyph_rasterizer/mod.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ struct MappedFontKey {
549549
}
550550

551551
struct FontKeyMapLocked {
552+
namespace: IdNamespace,
552553
next_id: u32,
553554
template_map: FastHashMap<FontTemplate, Arc<MappedFontKey>>,
554555
key_map: FastHashMap<FontKey, Arc<MappedFontKey>>,
@@ -561,15 +562,15 @@ struct FontKeyMapLocked {
561562
/// final shared key. The shared key will stay alive so long as there are
562563
/// any strong references to the mapping entry. Care must be taken when
563564
/// clearing namespaces of shared keys as this may trigger shared font keys
564-
/// to expire which require individual processing.
565+
/// to expire which require individual processing. Shared font keys will be
566+
/// created within the provided unique namespace.
565567
#[derive(Clone)]
566568
pub struct FontKeyMap(Arc<RwLock<FontKeyMapLocked>>);
567569

568570
impl FontKeyMap {
569-
const SHARED_NAMESPACE: IdNamespace = IdNamespace(0);
570-
571-
pub fn new() -> Self {
571+
pub fn new(namespace: IdNamespace) -> Self {
572572
FontKeyMap(Arc::new(RwLock::new(FontKeyMapLocked {
573+
namespace,
573574
next_id: 1,
574575
template_map: FastHashMap::default(),
575576
key_map: FastHashMap::default(),
@@ -600,7 +601,7 @@ impl FontKeyMap {
600601
locked.key_map.insert(*font_key, mapped);
601602
return None;
602603
}
603-
let shared_key = FontKey::new(Self::SHARED_NAMESPACE, locked.next_id);
604+
let shared_key = FontKey::new(locked.namespace, locked.next_id);
604605
locked.next_id += 1;
605606
let mapped = Arc::new(MappedFontKey {
606607
font_key: shared_key,
@@ -717,6 +718,7 @@ impl FontTemplateMap {
717718
}
718719

719720
struct FontInstanceKeyMapLocked {
721+
namespace: IdNamespace,
720722
next_id: u32,
721723
instances: FastHashSet<Arc<BaseFontInstance>>,
722724
key_map: FastHashMap<FontInstanceKey, Weak<BaseFontInstance>>,
@@ -729,15 +731,15 @@ struct FontInstanceKeyMapLocked {
729731
/// key to assign to that instance. When the weak count of the mapping is zero,
730732
/// the entry is allowed to expire. Again, care must be taken when clearing
731733
/// a namespace within the key map as it may cause shared key expirations that
732-
/// require individual processing.
734+
/// require individual processing. Shared instance keys will be created within
735+
/// the provided unique namespace.
733736
#[derive(Clone)]
734737
pub struct FontInstanceKeyMap(Arc<RwLock<FontInstanceKeyMapLocked>>);
735738

736739
impl FontInstanceKeyMap {
737-
const SHARED_NAMESPACE: IdNamespace = IdNamespace(0);
738-
739-
pub fn new() -> Self {
740+
pub fn new(namespace: IdNamespace) -> Self {
740741
FontInstanceKeyMap(Arc::new(RwLock::new(FontInstanceKeyMapLocked {
742+
namespace,
741743
next_id: 1,
742744
instances: FastHashSet::default(),
743745
key_map: FastHashMap::default(),
@@ -769,7 +771,7 @@ impl FontInstanceKeyMap {
769771
return None;
770772
}
771773
let unmapped_key = instance.instance_key;
772-
instance.instance_key = FontInstanceKey::new(Self::SHARED_NAMESPACE, locked.next_id);
774+
instance.instance_key = FontInstanceKey::new(locked.namespace, locked.next_id);
773775
locked.next_id += 1;
774776
let shared_instance = Arc::new(instance);
775777
locked.instances.insert(shared_instance.clone());
@@ -911,12 +913,12 @@ pub struct SharedFontResources {
911913
}
912914

913915
impl SharedFontResources {
914-
pub fn new() -> Self {
916+
pub fn new(namespace: IdNamespace) -> Self {
915917
SharedFontResources {
916918
templates: FontTemplateMap::new(),
917919
instances: FontInstanceMap::new(),
918-
font_keys: FontKeyMap::new(),
919-
instance_keys: FontInstanceKeyMap::new(),
920+
font_keys: FontKeyMap::new(namespace),
921+
instance_keys: FontInstanceKeyMap::new(namespace),
920922
}
921923
}
922924
}

webrender/src/render_backend.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ impl RenderBackend {
733733
}
734734
}
735735

736-
fn next_namespace_id(&self) -> IdNamespace {
736+
pub fn next_namespace_id() -> IdNamespace {
737737
IdNamespace(NEXT_NAMESPACE_ID.fetch_add(1, Ordering::Relaxed) as u32)
738738
}
739739

@@ -908,7 +908,7 @@ impl RenderBackend {
908908
match msg {
909909
ApiMsg::CloneApi(sender) => {
910910
assert!(!self.namespace_alloc_by_client);
911-
sender.send(self.next_namespace_id()).unwrap();
911+
sender.send(Self::next_namespace_id()).unwrap();
912912
}
913913
ApiMsg::CloneApiByClient(namespace_id) => {
914914
assert!(self.namespace_alloc_by_client);

webrender/src/renderer/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,9 @@ impl Renderer {
12121212
let sampler = options.sampler;
12131213
let namespace_alloc_by_client = options.namespace_alloc_by_client;
12141214

1215-
let fonts = SharedFontResources::new();
1215+
// Ensure shared font keys exist within their own unique namespace so
1216+
// that they don't accidentally collide across Renderer instances.
1217+
let fonts = SharedFontResources::new(RenderBackend::next_namespace_id());
12161218

12171219
let blob_image_handler = options.blob_image_handler.take();
12181220
let scene_builder_hooks = options.scene_builder_hooks;

webrender/src/resource_cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ impl ResourceCache {
540540
let workers = Arc::new(ThreadPoolBuilder::new().build().unwrap());
541541
let glyph_rasterizer = GlyphRasterizer::new(workers, true).unwrap();
542542
let cached_glyphs = GlyphCache::new();
543-
let fonts = SharedFontResources::new();
543+
let fonts = SharedFontResources::new(IdNamespace(0));
544544
let picture_textures = PictureTextures::new(
545545
crate::picture::TILE_SIZE_DEFAULT,
546546
TextureFilter::Nearest,

0 commit comments

Comments
 (0)