From 5d6c5132c130ac1250a17048249815ef1f5939ac Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 4 Apr 2020 20:29:25 +0200 Subject: [PATCH 1/3] Don't use transmute to create PaintOrder values I checked that rustc optimises the code just as well as with the transmute. https://rust.godbolt.org/z/w6UJN4 --- components/style/values/specified/svg.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/style/values/specified/svg.rs b/components/style/values/specified/svg.rs index 6ae1eac2d4a..e603ddfda07 100644 --- a/components/style/values/specified/svg.rs +++ b/components/style/values/specified/svg.rs @@ -146,8 +146,13 @@ impl SVGPaintOrder { /// Get variant of `paint-order` pub fn order_at(&self, pos: u8) -> PaintOrder { - // Safe because PaintOrder covers all possible patterns. - unsafe { std::mem::transmute((self.0 >> pos * PAINT_ORDER_SHIFT) & PAINT_ORDER_MASK) } + match (self.0 >> pos * PAINT_ORDER_SHIFT) & PAINT_ORDER_MASK { + 0 => PaintOrder::Normal, + 1 => PaintOrder::Fill, + 2 => PaintOrder::Stroke, + 3 => PaintOrder::Markers, + _ => unreachable!("this cannot happen"), + } } } From e7cb736796e43bd564fd3994da8915f7d06f3768 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 4 Apr 2020 22:26:58 +0200 Subject: [PATCH 2/3] Replace ScopedTLS::unsafe_get by ScopedTLS::into_slots We only ever look at the slots after we are done with the thread pool, so we don't need to expose any unsafety to inspect the slots. --- components/style/driver.rs | 7 +++---- components/style/scoped_tls.rs | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/components/style/driver.rs b/components/style/driver.rs index 88ec933874e..e8dfe7cf3db 100644 --- a/components/style/driver.rs +++ b/components/style/driver.rs @@ -165,10 +165,9 @@ pub fn traverse_dom( if dump_stats || report_stats { let mut aggregate = mem::replace(&mut context.thread_local.statistics, Default::default()); let parallel = maybe_tls.is_some(); - if let Some(ref mut tls) = maybe_tls { - let slots = unsafe { tls.unsafe_get() }; - for slot in slots { - if let Some(ref cx) = *slot.borrow() { + if let Some(tls) = maybe_tls { + for mut slot in tls.into_slots().into_vec() { + if let Some(cx) = slot.get_mut() { aggregate += cx.statistics.clone(); } } diff --git a/components/style/scoped_tls.rs b/components/style/scoped_tls.rs index e1e5209a058..7491456669d 100644 --- a/components/style/scoped_tls.rs +++ b/components/style/scoped_tls.rs @@ -71,9 +71,8 @@ impl<'scope, T: Send> ScopedTLS<'scope, T> { RefMut::map(opt, |x| x.as_mut().unwrap()) } - /// Unsafe access to the slots. This can be used to access the TLS when - /// the caller knows that the pool does not have access to the TLS. - pub unsafe fn unsafe_get(&self) -> &[RefCell>] { - &self.slots + /// Returns the slots, consuming the scope. + pub fn into_slots(self) -> Box<[RefCell>]> { + self.slots } } From 94ee8b34ca9ed4b0c9beeac66a5a10cde2db0421 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 4 Apr 2020 21:22:42 +0200 Subject: [PATCH 3/3] Rearrange FontLanguageOverride Creating one from a u32 should be unsafe because we rely on the fact that the value is a valid &str. --- components/style/values/computed/font.rs | 65 +++++++++++++++-------- components/style/values/specified/font.rs | 28 ++-------- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 658d3412475..6867f582d7d 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -21,7 +21,6 @@ use crate::values::specified::font::{ use crate::values::specified::length::{FontBaseSize, NoCalcLength}; use crate::values::CSSFloat; use crate::Atom; -use byteorder::{BigEndian, ByteOrder}; use cssparser::{serialize_identifier, CssStringWriter, Parser}; #[cfg(feature = "gecko")] use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; @@ -676,7 +675,7 @@ pub type FontVariationSettings = FontSettings>; /// (see http://www.microsoft.com/typography/otspec/languagetags.htm). #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToResolvedValue)] #[repr(C)] -pub struct FontLanguageOverride(pub u32); +pub struct FontLanguageOverride(u32); impl FontLanguageOverride { #[inline] @@ -684,6 +683,46 @@ impl FontLanguageOverride { pub fn zero() -> FontLanguageOverride { FontLanguageOverride(0) } + + /// Returns this value as a `&str`, backed by `storage`. + #[inline] + pub fn to_str(self, storage: &mut [u8; 4]) -> &str { + if self.0 == 0 { + return "normal"; + } + *storage = u32::to_be_bytes(self.0); + // Safe because we ensure it's ASCII during computing + let slice = if cfg!(debug_assertions) { + std::str::from_utf8(&storage[..]).unwrap() + } else { + unsafe { std::str::from_utf8_unchecked(&storage[..]) } + }; + slice.trim_end() + } + + /// Parses a str, return `Self::zero()` if the input isn't a valid OpenType + /// "language system" tag. + #[inline] + pub fn from_str(lang: &str) -> Self { + if lang.is_empty() || lang.len() > 4 { + return Self::zero(); + } + let mut bytes = [b' '; 4]; + for (byte, lang_byte) in bytes.iter_mut().zip(lang.as_bytes()) { + if !lang_byte.is_ascii() { + return Self::zero(); + } + *byte = *lang_byte; + } + Self(u32::from_be_bytes(bytes)) + } + + /// Unsafe because `Self::to_str` requires the value to represent a UTF-8 + /// string. + #[inline] + pub unsafe fn from_u32(value: u32) -> Self { + Self(value) + } } impl ToCss for FontLanguageOverride { @@ -691,27 +730,7 @@ impl ToCss for FontLanguageOverride { where W: fmt::Write, { - use std::str; - - if self.0 == 0 { - return dest.write_str("normal"); - } - let mut buf = [0; 4]; - BigEndian::write_u32(&mut buf, self.0); - // Safe because we ensure it's ASCII during computing - let slice = if cfg!(debug_assertions) { - str::from_utf8(&buf).unwrap() - } else { - unsafe { str::from_utf8_unchecked(&buf) } - }; - slice.trim_end().to_css(dest) - } -} - -#[cfg(feature = "gecko")] -impl From for FontLanguageOverride { - fn from(bits: u32) -> FontLanguageOverride { - FontLanguageOverride(bits) + self.to_str(&mut [0; 4]).to_css(dest) } } diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index f7823e5f9ff..94c28d068f9 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -20,7 +20,6 @@ use crate::values::specified::{AllowQuirks, Angle, Integer, LengthPercentage}; use crate::values::specified::{NoCalcLength, NonNegativeNumber, Number, Percentage}; use crate::values::CustomIdent; use crate::Atom; -use byteorder::{BigEndian, ByteOrder}; use cssparser::{Parser, Token}; #[cfg(feature = "gecko")] use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; @@ -2057,19 +2056,9 @@ impl FontLanguageOverride { #[inline] pub fn compute_non_system(&self) -> computed::FontLanguageOverride { match *self { - FontLanguageOverride::Normal => computed::FontLanguageOverride(0), + FontLanguageOverride::Normal => computed::FontLanguageOverride::zero(), FontLanguageOverride::Override(ref lang) => { - if lang.is_empty() || lang.len() > 4 { - return computed::FontLanguageOverride(0); - } - let mut bytes = [b' '; 4]; - for (byte, lang_byte) in bytes.iter_mut().zip(lang.as_bytes()) { - if !lang_byte.is_ascii() { - return computed::FontLanguageOverride(0); - } - *byte = *lang_byte; - } - computed::FontLanguageOverride(BigEndian::read_u32(&bytes)) + computed::FontLanguageOverride::from_str(lang) }, FontLanguageOverride::System(..) => unreachable!(), } @@ -2090,19 +2079,10 @@ impl ToComputedValue for FontLanguageOverride { } #[inline] fn from_computed_value(computed: &computed::FontLanguageOverride) -> Self { - if computed.0 == 0 { + if *computed == computed::FontLanguageOverride::zero() { return FontLanguageOverride::Normal; } - let mut buf = [0; 4]; - BigEndian::write_u32(&mut buf, computed.0); - FontLanguageOverride::Override( - if cfg!(debug_assertions) { - String::from_utf8(buf.to_vec()).unwrap() - } else { - unsafe { String::from_utf8_unchecked(buf.to_vec()) } - } - .into_boxed_str(), - ) + FontLanguageOverride::Override(computed.to_str(&mut [0; 4]).into()) } }