Auto merge of #26113 - servo:layout-2020-rm-random-unsafe-removals-cause-i-am-bored, r=emilio

Drive-by refactoring of some unsafe code in stylo

Some stuff I found while thinking about other things between stylo and layout.

Third commit requires Gecko changes because I removed `From<u32>` for `FontLanguageOverride` and replaced it by the unsafe `FontLanguageOverride::from_u32` method.

components/style/driver.rs

@@ -165,10 +165,9 @@ pub fn traverse_dom<E, D>(
     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 {
+        if let Some(tls) = maybe_tls {
-            let slots = unsafe { tls.unsafe_get() };
+            for mut slot in tls.into_slots().into_vec() {
-            for slot in slots {
+                if let Some(cx) = slot.get_mut() {
-                if let Some(ref cx) = *slot.borrow() {
                     aggregate += cx.statistics.clone();
                 }
             }

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
+    /// Returns the slots, consuming the scope.
-    /// the caller knows that the pool does not have access to the TLS.
+    pub fn into_slots(self) -> Box<[RefCell<Option<T>>]> {
-    pub unsafe fn unsafe_get(&self) -> &[RefCell<Option<T>>] {
+        self.slots
-        &self.slots
     }
 }

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<VariationValue<Number>>;
 /// (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;
+        self.to_str(&mut [0; 4]).to_css(dest)
-
-        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<u32> for FontLanguageOverride {
-    fn from(bits: u32) -> FontLanguageOverride {
-        FontLanguageOverride(bits)
     }
 }
 

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 {
+                computed::FontLanguageOverride::from_str(lang)
-                    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))
             },
             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];
+        FontLanguageOverride::Override(computed.to_str(&mut [0; 4]).into())
-        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(),
-        )
     }
 }
 

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.
+        match (self.0 >> pos * PAINT_ORDER_SHIFT) & PAINT_ORDER_MASK {
-        unsafe { std::mem::transmute((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"),
+        }
     }
 }